Merge branch '2445-commit-author-restriction' into 'master'

Add ability to enforce that only the author of a commit can push those changes back to the repository

Closes #2445

See merge request gitlab-org/gitlab-ee!3086

app/controllers/admin/push_rules_controller.rb

@@ -24,9 +24,19 @@ class Admin::PushRulesController < Admin::ApplicationController
   end
 
   def push_rule_params
-    params.require(:push_rule).permit(:deny_delete_tag, :delete_branch_regex,
-      :commit_message_regex, :branch_name_regex, :force_push_regex, :author_email_regex, :member_check,
-      :file_name_regex, :max_file_size, :prevent_secrets, :reject_unsigned_commits)
+    allowed_fields = %i[deny_delete_tag delete_branch_regex commit_message_regex
+                        branch_name_regex force_push_regex author_email_regex
+                        member_check file_name_regex max_file_size prevent_secrets]
+
+    if @push_rule.available?(:reject_unsigned_commits)
+      allowed_fields << :reject_unsigned_commits
+    end
+
+    if @push_rule.available?(:commit_committer_check)
+      allowed_fields << :commit_committer_check
+    end
+
+    params.require(:push_rule).permit(allowed_fields)
   end
 
   def push_rule

app/controllers/projects/push_rules_controller.rb

@@ -33,6 +33,10 @@ class Projects::PushRulesController < Projects::ApplicationController
       allowed_fields << :reject_unsigned_commits
     end
 
+    if can?(current_user, :change_commit_committer_check, project)
+      allowed_fields << :commit_committer_check
+    end
+
     params.require(:push_rule).permit(allowed_fields)
   end
 end

app/helpers/push_rules_helper.rb

 module PushRulesHelper
   def reject_unsigned_commits_description(push_rule)
-    message = [s_("ProjectSettings|Only signed commits can be pushed to this repository.")]
+    message = s_("ProjectSettings|Only signed commits can be pushed to this repository.")
 
+    push_rule_update_description(message, push_rule, :reject_unsigned_commits)
+  end
+
+  def commit_committer_check_description(push_rule)
+    message = s_("ProjectSettings|Users can only push commits to this repository "\
+                 "that were committed with one of their own verified emails.")
+
+    push_rule_update_description(message, push_rule, :commit_committer_check)
+  end
+
+  private
+
+  def push_rule_update_description(message, push_rule, rule)
+    messages = [message]
     if push_rule.global?
-      message << s_("ProjectSettings|This setting will be applied to all projects unless overridden by an admin.")
+      messages << s_("ProjectSettings|This setting will be applied to all projects unless overridden by an admin.")
     else
-      if PushRule.global&.reject_unsigned_commits
-        message << if push_rule.reject_unsigned_commits
-                     s_("ProjectSettings|This setting is applied on the server level and can be overridden by an admin.")
-                   else
-                     s_("ProjectSettings|This setting is applied on the server level but has been overridden for this project.")
-                   end
-
-        message << s_("ProjectSettings|Contact an admin to change this setting.") unless current_user.admin?
+      enabled_globally = PushRule.global&.public_send(rule) # rubocop:disable GitlabSecurity/PublicSend
+      enabled_in_project = push_rule.public_send(rule) # rubocop:disable GitlabSecurity/PublicSend
+
+      if enabled_globally
+        messages << if enabled_in_project
+                      s_("ProjectSettings|This setting is applied on the server level and can be overridden by an admin.")
+                    else
+                      s_("ProjectSettings|This setting is applied on the server level but has been overridden for this project.")
+                    end
+
+        messages << s_("ProjectSettings|Contact an admin to change this setting.") unless current_user.admin?
       end
     end
 
-    message.join(' ')
+    messages.join(' ')
   end
 end

app/models/license.rb

@@ -48,6 +48,7 @@ class License < ActiveRecord::Base
     service_desk
     variable_environment_scope
     reject_unsigned_commits
+    commit_committer_check
   ].freeze
 
   EEU_FEATURES = EEP_FEATURES

app/models/push_rule.rb

@@ -5,6 +5,10 @@ class PushRule < ActiveRecord::Base
   validates :max_file_size, numericality: { greater_than_or_equal_to: 0, only_integer: true }
 
   FILES_BLACKLIST = YAML.load_file(Rails.root.join('lib/gitlab/checks/files_blacklist.yml'))
+  SETTINGS_WITH_GLOBAL_DEFAULT = %i[
+    reject_unsigned_commits
+    commit_committer_check
+  ].freeze
 
   def self.global
     find_by(is_sample: true)
@@ -15,6 +19,7 @@ class PushRule < ActiveRecord::Base
       branch_name_regex.present? ||
       author_email_regex.present? ||
       reject_unsigned_commits ||
+      commit_committer_check ||
       member_check ||
       file_name_regex.present? ||
       max_file_size > 0 ||
@@ -28,6 +33,13 @@ class PushRule < ActiveRecord::Base
     commit.has_signature?
   end
 
+  def committer_allowed?(committer_email, current_user)
+    return true unless available?(:commit_committer_check)
+    return true unless commit_committer_check
+
+    current_user.verified_email?(committer_email)
+  end
+
   def commit_message_allowed?(message)
     data_match?(message, commit_message_regex)
   end
@@ -48,39 +60,34 @@ class PushRule < ActiveRecord::Base
     regex_list.find { |regex| data_match?(file_path, regex) }
   end
 
-  def reject_unsigned_commits=(value)
-    enabled_globally = PushRule.global&.reject_unsigned_commits
-    is_disabled = !Gitlab::Utils.to_boolean(value)
+  def global?
+    is_sample?
+  end
 
-    # If setting is globally disabled and user disable it at project level,
-    # reset the attr so we can use the default global if required later.
-    if !enabled_globally && is_disabled
-      super(nil)
+  def available?(feature_sym)
+    if global?
+      License.feature_available?(feature_sym)
     else
-      super(value)
+      project&.feature_available?(feature_sym)
     end
   end
 
   def reject_unsigned_commits
-    value = super
-
-    # return if value is true/false or if current object is the global setting
-    return value if global? || !value.nil?
-
-    PushRule.global&.reject_unsigned_commits
+    read_setting_with_global_default(:reject_unsigned_commits)
   end
   alias_method :reject_unsigned_commits?, :reject_unsigned_commits
 
-  def global?
-    is_sample?
+  def reject_unsigned_commits=(value)
+    write_setting_with_global_default(:reject_unsigned_commits, value)
   end
 
-  def available?(feature_sym)
-    if global?
-      License.feature_available?(feature_sym)
-    else
-      project&.feature_available?(feature_sym)
-    end
+  def commit_committer_check
+    read_setting_with_global_default(:commit_committer_check)
+  end
+  alias_method :commit_committer_check?, :commit_committer_check
+
+  def commit_committer_check=(value)
+    write_setting_with_global_default(:commit_committer_check, value)
   end
 
   private
@@ -92,4 +99,26 @@ class PushRule < ActiveRecord::Base
       true
     end
   end
+
+  def read_setting_with_global_default(setting)
+    value = read_attribute(setting)
+
+    # return if value is true/false or if current object is the global setting
+    return value if global? || !value.nil?
+
+    PushRule.global&.public_send(setting)
+  end
+
+  def write_setting_with_global_default(setting, value)
+    enabled_globally = PushRule.global&.public_send(setting)
+    is_disabled = !Gitlab::Utils.to_boolean(value)
+
+    # If setting is globally disabled and user disable it at project level,
+    # reset the attr so we can use the default global if required later.
+    if !enabled_globally && is_disabled
+      write_attribute(setting, nil)
+    else
+      write_attribute(setting, value)
+    end
+  end
 end

app/views/shared/push_rules/_commit_committer_check_setting.html.haml

+- return unless push_rule.available?(:commit_committer_check)
+
+- form = local_assigns.fetch(:form)
+- push_rule = local_assigns.fetch(:push_rule)
+
+.form-group
+  = form.check_box :commit_committer_check, class: "pull-left", disabled: !can_change_push_rule?(form.object, :commit_committer_check)
+  .prepend-left-20
+    = form.label :commit_committer_check, class: "label-light append-bottom-0" do
+      = s_("PushRule|Committer restriction")
+    %p.light.append-bottom-0
+      = commit_committer_check_description(push_rule)

app/views/shared/push_rules/_form.html.haml

+= render 'shared/push_rules/commit_committer_check_setting', form: f, push_rule: f.object
+
 = render 'shared/push_rules/reject_unsigned_commits_setting', form: f, push_rule: f.object
 
 .form-group

app/views/shared/push_rules/_reject_unsigned_commits_setting.html.haml

@@ -4,7 +4,7 @@
 - push_rule = local_assigns.fetch(:push_rule)
 
 .form-group
-  = form.check_box :reject_unsigned_commits, class: "pull-left", disabled: !can_change_reject_unsigned_commits?(push_rule)
+  = form.check_box :reject_unsigned_commits, class: "pull-left", disabled: !can_change_push_rule?(form.object, :reject_unsigned_commits)
   .prepend-left-20
     = form.label :reject_unsigned_commits, class: "label-light append-bottom-0" do
       Reject unsigned commits

changelogs/unreleased/add_commit_author_check_feature.yml

+---
+title: Add new push rule to enforce that only the author of a commit can push to the repository
+merge_request: 3086
+author:
+type: added

db/migrate/20171006201108_add_commit_committer_check_to_push_rules.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddCommitCommitterCheckToPushRules < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def change
+    add_column :push_rules, :commit_committer_check, :boolean
+  end
+end

db/schema.rb

@@ -1750,6 +1750,7 @@ ActiveRecord::Schema.define(version: 20171017145932) do
     t.boolean "prevent_secrets", default: false, null: false
     t.string "branch_name_regex"
     t.boolean "reject_unsigned_commits"
+    t.boolean "commit_committer_check"
   end
 
   add_index "push_rules", ["is_sample"], name: "index_push_rules_on_is_sample", where: "is_sample", using: :btree

doc/push_rules/push_rules.md

@@ -63,6 +63,7 @@ The following options are available.
 | --------- | :------------: | ----------- |
 | Removal of tags with `git push` | 7.10 | Forbid users to remove git tags with `git push`. Tags will still be able to be deleted through the web UI. |
 | Check whether author is a GitLab user | 7.10 | Restrict commits by author (email) to existing GitLab users. |
+| Check whether committer is the current authenticated user | 10.2 | GitLab will reject any commit that was not committed by the current authenticated user |
 | Check whether commit is signed through GPG | 10.1 | Reject commit when it is not signed through GPG. Read [signing commits with GPG][signing-commits]. |
 | Prevent committing secrets to Git | 8.12 | GitLab will reject any files that are likely to contain secrets. Read [what files are forbidden](#prevent-pushing-secrets-to-the-repository). |
 | Restrict by commit message | 7.10 | Only commit messages that match this Ruby regular expression are allowed to be pushed. Leave empty to allow any commit message. |

ee/app/helpers/ee/projects_helper.rb

 module EE
   module ProjectsHelper
-    def can_change_reject_unsigned_commits?(push_rule)
+    def can_change_push_rule?(push_rule, rule)
       return true if push_rule.global?
 
-      can?(current_user, :change_reject_unsigned_commits, @project)
+      can?(current_user, :"change_#{rule}", @project)
     end
   end
 end

ee/app/policies/ee/project_policy.rb

@@ -20,6 +20,11 @@ module EE
         !PushRule.global&.reject_unsigned_commits
       end
 
+      with_scope :global
+      condition(:commit_committer_check_disabled_globally) do
+        !PushRule.global&.commit_committer_check
+      end
+
       with_scope :global
       condition(:remote_mirror_available) do
         ::Gitlab::CurrentSettings.current_application_settings.remote_mirror_available
@@ -84,6 +89,8 @@ module EE
       rule { ~can?(:push_code) }.prevent :push_code_to_protected_branches
 
       rule { admin | (reject_unsigned_commits_disabled_globally & can?(:master_access)) }.enable :change_reject_unsigned_commits
+
+      rule { admin | (commit_committer_check_disabled_globally & can?(:master_access)) }.enable :change_commit_committer_check
     end
   end
 end

lib/gitlab/checks/change_access.rb

@@ -15,7 +15,9 @@ module Gitlab
         update_protected_tag: 'Protected tags cannot be updated.',
         delete_protected_tag: 'Protected tags cannot be deleted.',
         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_not_verified: "Comitter email '%{commiter_email}' is not verified.",
+        push_rule_committer_not_allowed: "You cannot push commits for '%{committer_email}'. You can only push commits that were committed with one of your own verified emails."
       }.freeze
 
       # protocol is currently used only in EE
@@ -213,6 +215,9 @@ module Gitlab
           return "Author's email '#{commit.author_email}' does not follow the pattern '#{push_rule.author_email_regex}'"
         end
 
+        committer_error_message = committer_check(commit, push_rule)
+        return committer_error_message if committer_error_message
+
         if !updated_from_web? && !push_rule.commit_signature_allowed?(commit)
           return "Commit must be signed with a GPG key"
         end
@@ -233,6 +238,18 @@ module Gitlab
         nil
       end
 
+      def committer_check(commit, push_rule)
+        unless push_rule.committer_allowed?(commit.committer_email, user_access.user)
+          committer_is_current_user = commit.committer == user_access.user
+
+          if committer_is_current_user && !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)
         validations = validations_for_commit(commit, push_rule)
 

locale/gitlab.pot

@@ -8,8 +8,8 @@ msgid ""
 msgstr ""
 "Project-Id-Version: gitlab 1.0.0\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2017-10-22 16:40+0300\n"
-"PO-Revision-Date: 2017-10-22 16:40+0300\n"
+"POT-Creation-Date: 2017-11-02 14:42+0100\n"
+"PO-Revision-Date: 2017-11-02 14:42+0100\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
 "Language: \n"
@@ -36,6 +36,11 @@ msgstr[1] ""
 msgid "%{commit_author_link} committed %{commit_timeago}"
 msgstr ""
 
+msgid "%{count} participant"
+msgid_plural "%{count} participants"
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "%{number_commits_behind} commits behind %{default_branch}, %{number_commits_ahead} commits ahead"
 msgstr ""
 
@@ -56,6 +61,12 @@ msgstr[1] ""
 msgid "(checkout the %{link} for information on how to install it)."
 msgstr ""
 
+msgid "+ %{moreCount} more"
+msgstr ""
+
+msgid "- show less"
+msgstr ""
+
 msgid "1 pipeline"
 msgid_plural "%d pipelines"
 msgstr[0] ""
@@ -480,9 +491,6 @@ msgstr ""
 msgid "ClusterIntegration|A %{link_to_container_project} must have been created under this account"
 msgstr ""
 
-msgid "ClusterIntegration|Advanced settings"
-msgstr ""
-
 msgid "ClusterIntegration|Cluster details"
 msgstr ""
 
@@ -567,9 +575,6 @@ msgstr ""
 msgid "ClusterIntegration|See and edit the details for your cluster"
 msgstr ""
 
-msgid "ClusterIntegration|See and edit the details for your cluster"
-msgstr ""
-
 msgid "ClusterIntegration|See machine types"
 msgstr ""
 
@@ -620,6 +625,11 @@ msgid_plural "Commits"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "Commit %d file"
+msgid_plural "Commit %d files"
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "Commit Message"
 msgstr ""
 
@@ -728,12 +738,21 @@ msgstr ""
 msgid "Create empty bare repository"
 msgstr ""
 
+msgid "Create file"
+msgstr ""
+
 msgid "Create merge request"
 msgstr ""
 
 msgid "Create new branch"
 msgstr ""
 
+msgid "Create new directory"
+msgstr ""
+
+msgid "Create new file"
+msgstr ""
+
 msgid "Create new..."
 msgstr ""
 
@@ -904,6 +923,9 @@ msgstr ""
 msgid "Failed to remove the pipeline schedule"
 msgstr ""
 
+msgid "File name"
+msgstr ""
+
 msgid "Files"
 msgstr ""
 
@@ -1343,6 +1365,12 @@ msgstr ""
 msgid "Notifications"
 msgstr ""
 
+msgid "Number of access attempts"
+msgstr ""
+
+msgid "Number of failures before backing off"
+msgstr ""
+
 msgid "OfSearchInADropdown|Filter"
 msgstr ""
 
@@ -1598,6 +1626,9 @@ msgstr ""
 msgid "ProjectSettings|This setting will be applied to all projects unless overridden by an admin."
 msgstr ""
 
+msgid "ProjectSettings|Users can only push commits to this repository that were committed with one of their own verified emails."
+msgstr ""
+
 msgid "Projects"
 msgstr ""
 
@@ -1634,6 +1665,9 @@ msgstr ""
 msgid "Push events"
 msgstr ""
 
+msgid "PushRule|Committer restriction"
+msgstr ""
+
 msgid "Read more"
 msgstr ""
 
@@ -1906,6 +1940,9 @@ msgstr ""
 msgid "Subgroups"
 msgstr ""
 
+msgid "Subscribe"
+msgstr ""
+
 msgid "Switch branch/tag"
 msgstr ""
 
@@ -1932,6 +1969,9 @@ msgstr ""
 msgid "The Advanced Global Search in GitLab is a powerful search service that saves you time. Instead of creating duplicate code and wasting time, you can now search for code within other teams that can help your own project."
 msgstr ""
 
+msgid "The circuitbreaker backoff threshold should be lower than the failure count threshold"
+msgstr ""
+
 msgid "The coding stage shows the time from the first commit to creating the merge request. The data will automatically be added here once you create your first merge request."
 msgstr ""
 
@@ -1944,6 +1984,12 @@ msgstr ""
 msgid "The issue stage shows the time it takes from creating an issue to assigning the issue to a milestone, or add the issue to a list on your Issue Board. Begin creating issues to see data for this stage."
 msgstr ""
 
+msgid "The number of attempts GitLab will make to access a storage."
+msgstr ""
+
+msgid "The number of failures after which GitLab will start temporarily disabling access to a storage shard on a host"
+msgstr ""
+
 msgid "The number of failures of after which GitLab will completely prevent access to the storage. The number of failures can be reset in the admin interface: %{link_to_health_page} or using the %{api_documentation_link}."
 msgstr ""
 
@@ -2179,6 +2225,9 @@ msgstr ""
 msgid "Unstar"
 msgstr ""
 
+msgid "Unsubscribe"
+msgstr ""
+
 msgid "Upgrade your plan to activate Advanced Global Search."
 msgstr ""
 

spec/controllers/projects/push_rules_controller_spec.rb

@@ -34,52 +34,54 @@ describe Projects::PushRulesController do
       end
     end
 
-    context 'Updating reject unsigned commit rule' do
-      context 'as an admin' do
-        let(:user) { create(:admin) }
+    PushRule::SETTINGS_WITH_GLOBAL_DEFAULT.each do |rule_attr|
+      context "Updating #{rule_attr} rule" do
+        context 'as an admin' do
+          let(:user) { create(:admin) }
 
-        it 'updates the setting' do
-          patch :update, namespace_id: project.namespace, project_id: project, id: 1, push_rule: { reject_unsigned_commits: true }
-
-          expect(project.push_rule(true).reject_unsigned_commits).to be_truthy
-        end
-      end
-
-      context 'as a master user' do
-        before do
-          project.add_master(user)
-        end
-
-        context 'when global setting is disabled' do
           it 'updates the setting' do
-            patch :update, namespace_id: project.namespace, project_id: project, id: 1, push_rule: { reject_unsigned_commits: true }
+            patch :update, namespace_id: project.namespace, project_id: project, id: 1, push_rule: { rule_attr => true }
 
-            expect(project.push_rule(true).reject_unsigned_commits).to be_truthy
+            expect(project.push_rule(true).public_send(rule_attr)).to be_truthy
           end
         end
 
-        context 'when global setting is enabled' do
+        context 'as a master user' do
           before do
-            create(:push_rule_sample, reject_unsigned_commits: true)
+            project.add_master(user)
           end
 
-          it 'does not update the setting' do
-            patch :update, namespace_id: project.namespace, project_id: project, id: 1, push_rule: { reject_unsigned_commits: false }
+          context 'when global setting is disabled' do
+            it 'updates the setting' do
+              patch :update, namespace_id: project.namespace, project_id: project, id: 1, push_rule: { rule_attr => true }
 
-            expect(project.push_rule(true).reject_unsigned_commits).to be_truthy
+              expect(project.push_rule(true).public_send(rule_attr)).to be_truthy
+            end
           end
-        end
-      end
 
-      context 'as a developer user' do
-        before do
-          project.add_developer(user)
+          context 'when global setting is enabled' do
+            before do
+              create(:push_rule_sample, rule_attr => true)
+            end
+
+            it 'does not update the setting' do
+              patch :update, namespace_id: project.namespace, project_id: project, id: 1, push_rule: { rule_attr => false }
+
+              expect(project.push_rule(true).public_send(rule_attr)).to be_truthy
+            end
+          end
         end
 
-        it 'does not update the setting' do
-          patch :update, namespace_id: project.namespace, project_id: project, id: 1, push_rule: { reject_unsigned_commits: true }
+        context 'as a developer user' do
+          before do
+            project.add_developer(user)
+          end
+
+          it 'does not update the setting' do
+            patch :update, namespace_id: project.namespace, project_id: project, id: 1, push_rule: { rule_attr => true }
 
-          expect(project.push_rule(true).reject_unsigned_commits).to be_falsy
+            expect(project.push_rule(true).public_send(rule_attr)).to be_falsy
+          end
         end
       end
     end

spec/features/admin/admin_push_rules_spec.rb

@@ -7,27 +7,34 @@ describe "Admin::PushRules"  do
     sign_in(current_user)
   end
 
-  context 'when reject_unsigned_commits is unlicensed' do
-    before do
-      stub_licensed_features(reject_unsigned_commits: false)
+  push_rules_with_titles = {
+    reject_unsigned_commits: 'Reject unsigned commits',
+    commit_committer_check: 'Committer restriction'
+  }
+
+  push_rules_with_titles.each do |rule_attr, title|
+    context "when #{rule_attr} is unlicensed" do
+      before do
+        stub_licensed_features(rule_attr => false)
+      end
+
+      it 'does not render the setting checkbox' do
+        visit admin_push_rule_path
+
+        expect(page).not_to have_content(title)
+      end
     end
 
-    it 'does not render the setting checkbox' do
-      visit admin_push_rule_path
+    context "when #{rule_attr} is licensed" do
+      before do
+        stub_licensed_features(rule_attr => true)
+      end
 
-      expect(page).not_to have_content('Reject unsigned commits')
-    end
-  end
-
-  context 'when reject_unsigned_commits is licensed' do
-    before do
-      stub_licensed_features(reject_unsigned_commits: true)
-    end
-
-    it 'renders the setting checkbox' do
-      visit admin_push_rule_path
+      it 'renders the setting checkbox' do
+        visit admin_push_rule_path
 
-      expect(page).to have_content('Reject unsigned commits')
+        expect(page).to have_content(title)
+      end
     end
   end
 end

spec/features/projects/push_rules_spec.rb

@@ -3,61 +3,69 @@ require 'spec_helper'
 feature 'Projects > Push Rules', :js do
   let(:user) { create(:user) }
   let(:project) { create(:project, :repository, namespace: user.namespace) }
+  let(:foo) {{ reject_unsigned_commits: 'Reject unsigned commits' }}
 
   before do
     project.team << [user, :master]
     sign_in(user)
   end
 
-  describe 'Reject unsigned commits rule' do
-    context 'unlicensed' do
-      before do
-        stub_licensed_features(reject_unsigned_commits: false)
-      end
-
-      it 'does not render the setting checkbox' do
-        visit project_settings_repository_path(project)
+  push_rules_with_titles = {
+    reject_unsigned_commits: 'Reject unsigned commits',
+    commit_committer_check: 'Committer restriction'
+  }
 
-        expect(page).not_to have_content('Reject unsigned commits')
-      end
-    end
+  push_rules_with_titles.each do |rule_attr, title|
+    describe "#{rule_attr} rule" do
+      context 'unlicensed' do
+        before do
+          stub_licensed_features(rule_attr => false)
+        end
 
-    context 'licensed' do
-      let(:bronze_plan) { Plan.find_by!(name: 'bronze') }
-      let(:gold_plan) { Plan.find_by!(name: 'gold') }
+        it 'does not render the setting checkbox' do
+          visit project_settings_repository_path(project)
 
-      before do
-        stub_licensed_features(reject_unsigned_commits: true)
+          expect(page).not_to have_content(title)
+        end
       end
 
-      it 'renders the setting checkbox' do
-        visit project_settings_repository_path(project)
+      context 'licensed' do
+        let(:bronze_plan) { Plan.find_by!(name: 'bronze') }
+        let(:gold_plan) { Plan.find_by!(name: 'gold') }
 
-        expect(page).to have_content('Reject unsigned commits')
-      end
-
-      describe 'with GL.com plans' do
         before do
-          stub_application_setting(check_namespace_plan: true)
+          stub_licensed_features(rule_attr => true)
         end
 
-        context 'when disabled' do
-          it 'does not render the setting checkbox' do
-            project.namespace.update!(plan_id: bronze_plan.id)
+        it 'renders the setting checkbox' do
+          visit project_settings_repository_path(project)
 
-            visit project_settings_repository_path(project)
+          expect(page).to have_content(title)
+        end
 
-            expect(page).not_to have_content('Reject unsigned commits')
+        describe 'with GL.com plans' do
+          before do
+            stub_application_setting(check_namespace_plan: true)
+          end
+
+          context 'when disabled' do
+            it 'does not render the setting checkbox' do
+              project.namespace.update!(plan_id: bronze_plan.id)
+
+              visit project_settings_repository_path(project)
+
+              expect(page).not_to have_content(title)
+            end
           end
-        end
 
-        context 'when enabled' do
-          it 'renders the setting checkbox' do
-            project.namespace.update!(plan_id: gold_plan.id)
+          context 'when enabled' do
+            it 'renders the setting checkbox' do
+              project.namespace.update!(plan_id: gold_plan.id)
 
-            visit project_settings_repository_path(project)
+              visit project_settings_repository_path(project)
 
-            expect(page).to have_content('Reject unsigned commits')
+              expect(page).to have_content(title)
+            end
           end
         end
       end

spec/helpers/push_rules_helper_spec.rb

@@ -7,7 +7,8 @@ describe PushRulesHelper do
   let(:project_owner) { push_rule.project.owner }
   let(:possible_help_texts) do
     {
-      base_help: /Only signed commits can be pushed to this repository/,
+      commit_committer_check_base_help: /Users can only push commits to this repository that were committed with one of their own verified emails/,
+      reject_unsigned_commits_base_help: /Only signed commits can be pushed to this repository/,
       default_admin_help: /This setting will be applied to all projects unless overridden by an admin/,
       setting_can_be_overridden: /This setting is applied on the server level and can be overridden by an admin/,
       setting_has_been_overridden: /This setting is applied on the server level but has been overridden for this project/,
@@ -43,20 +44,23 @@ describe PushRulesHelper do
   end
 
   with_them do
-    before do
-      global_push_rule.update_column(:reject_unsigned_commits, enabled_globally)
-      push_rule.update_column(:reject_unsigned_commits, enabled_in_project)
+    PushRule::SETTINGS_WITH_GLOBAL_DEFAULT.each do |rule_attr|
+      before do
+        global_push_rule.update_column(rule_attr, enabled_globally)
+        push_rule.update_column(rule_attr, enabled_in_project)
 
-      allow(helper).to receive(:current_user).and_return(users[current_user])
-    end
+        allow(helper).to receive(:current_user).and_return(users[current_user])
+      end
 
-    it 'has the correct help text' do
-      rule = global_setting ? global_push_rule : push_rule
+      it 'has the correct help text' do
+        rule = global_setting ? global_push_rule : push_rule
+        message = possible_help_texts["#{rule_attr}_#{help_text}".to_sym].presence || possible_help_texts[help_text]
 
-      expect(helper.reject_unsigned_commits_description(rule)).to match(possible_help_texts[help_text])
+        expect(helper.public_send("#{rule_attr}_description", rule)).to match(message)
 
-      if invalid_text
-        expect(helper.reject_unsigned_commits_description(rule)).not_to match(possible_help_texts[invalid_text])
+        if invalid_text
+          expect(helper.public_send("#{rule_attr}_description", rule)).not_to match(possible_help_texts[invalid_text])
+        end
       end
     end
   end

spec/lib/gitlab/checks/change_access_spec.rb

@@ -10,16 +10,17 @@ describe Gitlab::Checks::ChangeAccess do
     let(:ref) { 'refs/heads/master' }
     let(:changes) { { oldrev: oldrev, newrev: newrev, ref: ref } }
     let(:protocol) { 'ssh' }
-
-    subject do
+    let(:change_access) do
       described_class.new(
         changes,
         project: project,
         user_access: user_access,
         protocol: protocol
-      ).exec
+      )
     end
 
+    subject { change_access.exec }
+
     before do
       project.add_developer(user)
     end
@@ -455,5 +456,89 @@ describe Gitlab::Checks::ChangeAccess do
         expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "The path 'README' is locked by #{path_lock.user.name}")
       end
     end
+
+    context 'Check commit author rules' do
+      before do
+        stub_licensed_features(commit_committer_check: true)
+      end
+
+      let(:push_rule) { create(:push_rule, commit_committer_check: true) }
+      let(:project) { create(:project, :public, :repository, push_rule: push_rule) }
+
+      context 'with a commit from the authenticated user' do
+        before do
+          allow(project.repository).to receive(:new_commits).and_return(
+            project.repository.commits_between('be93687618e4b132087f430a4d8fc3a609c9b77c', '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51')
+          )
+          allow_any_instance_of(Commit).to receive(:committer_email).and_return(user.email)
+        end
+
+        it 'does not return an error' do
+          expect { subject }.not_to raise_error
+        end
+
+        it 'allows the commit when they were done with another email that belongs to the current user' do
+          user.emails.create(email: 'secondary_email@user.com', confirmed_at: Time.now)
+          allow_any_instance_of(Commit).to receive(:committer_email).and_return('secondary_email@user.com')
+
+          expect { subject }.not_to raise_error
+        end
+
+        it 'raises an error when the commit was done with an unverified email' do
+          user.emails.create(email: 'secondary_email@user.com')
+          allow_any_instance_of(Commit).to receive(:committer_email).and_return('secondary_email@user.com')
+
+          expect { subject }
+            .to raise_error(Gitlab::GitAccess::UnauthorizedError,
+                            "Comitter email '%{commiter_email}' is not verified.")
+        end
+
+        it 'raises an error when using an unknown email' do
+          allow_any_instance_of(Commit).to receive(:committer_email).and_return('some@mail.com')
+          expect { subject }
+            .to raise_error(Gitlab::GitAccess::UnauthorizedError,
+                            "You cannot push commits for 'some@mail.com'. You can only push commits that were committed with one of your own verified emails.")
+        end
+      end
+
+      context 'for an ff merge request' do
+        # the signed-commits branch fast-forwards onto master
+        let(:newrev) { '2d1096e3' }
+
+        it 'does not raise errors for a fast forward' do
+          expect(change_access).not_to receive(:committer_check)
+          expect { subject }.not_to raise_error
+        end
+      end
+
+      context 'for a normal merge' do
+        # This creates a merge commit without adding it to a target branch
+        # that is what the repository would look like during the `pre-receive` hook.
+        #
+        # That means only the merge commit should be validated.
+        let(:newrev) do
+          rugged = project.repository.raw_repository.rugged
+          base = oldrev
+          to_merge = '2d1096e3a0ecf1d2baf6dee036cc80775d4940ba'
+
+          merge_index = rugged.merge_commits(base, to_merge)
+          options = {
+            parents: [base, to_merge],
+            tree: merge_index.write_tree(rugged),
+            message: 'The merge commit',
+            author: { name: user.name, email: user.email, time: Time.now },
+            committer: { name: user.name, email: user.email, time: Time.now }
+          }
+
+          Rugged::Commit.create(rugged, options)
+        end
+
+        it 'does not raise errors for a merge commit' do
+          expect(change_access).to receive(:committer_check).once
+                                     .and_call_original
+          expect { subject }.not_to raise_error
+        end
+      end
+    end
   end
 end

spec/models/push_rule_spec.rb

@@ -24,6 +24,7 @@ describe PushRule do
       author_email_regex: 'regex',
       file_name_regex: 'regex',
       reject_unsigned_commits: true,
+      commit_committer_check: true,
       member_check: true,
       prevent_secrets: true,
       max_file_size: 1