Commit c5159a49 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Validate that the committer email is verified on the user.

parent 2df57e02
...@@ -33,11 +33,11 @@ class PushRule < ActiveRecord::Base ...@@ -33,11 +33,11 @@ class PushRule < ActiveRecord::Base
commit.has_signature? commit.has_signature?
end end
def committer_allowed?(committer, current_user) def committer_allowed?(committer_email, current_user)
return true unless available?(:commit_committer_check) return true unless available?(:commit_committer_check)
return true unless commit_committer_check return true unless commit_committer_check
current_user == committer current_user.verified_email?(committer_email)
end end
def commit_message_allowed?(message) def commit_message_allowed?(message)
......
...@@ -15,7 +15,10 @@ module Gitlab ...@@ -15,7 +15,10 @@ module Gitlab
update_protected_tag: 'Protected tags cannot be updated.', update_protected_tag: 'Protected tags cannot be updated.',
delete_protected_tag: 'Protected tags cannot be deleted.', delete_protected_tag: 'Protected tags cannot be deleted.',
create_protected_tag: 'You are not allowed to create this tag as it is protected.', create_protected_tag: 'You are not allowed to create this tag as it is protected.',
push_rule_branch_name: "Branch name does not follow the pattern '%{branch_name_regex}'" push_rule_branch_name: "Branch name does not follow the pattern '%{branch_name_regex}'",
push_rule_committer_unknown: "Committer '%{committer_email}' unknown, do you need to add that email to your profile?",
push_rule_committer_not_verified: "Committer email '%{committer_email}' not verified. Verify the email on your profile page.",
push_rule_committer_not_allowed: "You cannot push commits for '%{committer_email}', you can only push your own commits."
}.freeze }.freeze
# protocol is currently used only in EE # protocol is currently used only in EE
...@@ -213,9 +216,8 @@ module Gitlab ...@@ -213,9 +216,8 @@ module Gitlab
return "Author's email '#{commit.author_email}' does not follow the pattern '#{push_rule.author_email_regex}'" return "Author's email '#{commit.author_email}' does not follow the pattern '#{push_rule.author_email_regex}'"
end end
unless push_rule.committer_allowed?(commit.committer, user_access.user) committer_error_message = committer_check(commit, push_rule)
return "You can only push your own commits to this repository" return committer_error_message if committer_error_message
end
if !updated_from_web? && !push_rule.commit_signature_allowed?(commit) if !updated_from_web? && !push_rule.commit_signature_allowed?(commit)
return "Commit must be signed with a GPG key" return "Commit must be signed with a GPG key"
...@@ -237,6 +239,18 @@ module Gitlab ...@@ -237,6 +239,18 @@ module Gitlab
nil nil
end end
def committer_check(commit, push_rule)
unless push_rule.committer_allowed?(commit.committer_email, user_access.user)
if commit.committer.nil?
ERROR_MESSAGES[:push_rule_committer_unknown] % { committer_email: commit.committer_email }
elsif !commit.committer.verified_email?(commit.committer_email)
ERROR_MESSAGES[:push_rule_committer_not_verified] % { committer_email: commit.committer_email }
else
ERROR_MESSAGES[:push_rule_committer_not_allowed] % { committer_email: commit.committer_email }
end
end
end
def check_commit_diff(commit, push_rule) def check_commit_diff(commit, push_rule)
validations = validations_for_commit(commit, push_rule) validations = validations_for_commit(commit, push_rule)
......
...@@ -459,19 +459,30 @@ describe Gitlab::Checks::ChangeAccess do ...@@ -459,19 +459,30 @@ describe Gitlab::Checks::ChangeAccess do
it 'allows the commit when they were done with another email that belongs to the current user' do it 'allows the commit when they were done with another email that belongs to the current user' do
allow_any_instance_of(Commit).to receive(:committer_email).and_return('secondary_email@user.com') allow_any_instance_of(Commit).to receive(:committer_email).and_return('secondary_email@user.com')
user.emails.create(email: 'secondary_email@user.com') user.emails.create(email: 'secondary_email@user.com', confirmed_at: Time.now)
expect { subject }.not_to raise_error expect { subject }.not_to raise_error
end end
it 'raises an error when the commit was done with an unverified email' do
allow_any_instance_of(Commit).to receive(:committer_email).and_return('secondary_email@user.com')
user.emails.create(email: 'secondary_email@user.com')
expect { subject }
.to raise_error(Gitlab::GitAccess::UnauthorizedError,
"Committer email 'secondary_email@user.com' not verified. Verify the email on your profile page.")
end
end end
context 'with a commit from a different user' do context 'with a commit using an unknown e-mail' do
before do before do
allow_any_instance_of(Commit).to receive(:committer_email).and_return('some@mail.com') allow_any_instance_of(Commit).to receive(:committer_email).and_return('some@mail.com')
end end
it 'returns an error' do it 'returns an error' do
expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "You can only push your own commits to this repository") expect { subject }
.to raise_error(Gitlab::GitAccess::UnauthorizedError,
"Committer 'some@mail.com' unknown, do you need to add that email to your profile?")
end end
end end
end end
......
...@@ -352,6 +352,4 @@ describe MergeRequests::MergeService do ...@@ -352,6 +352,4 @@ describe MergeRequests::MergeService do
end end
end end
end end
context ''
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment