Add tests to the API call

This commits tests API calls and policies related to
reject_unsigned_commit property of the push_rules API endpoint.

8 files changed, 162 insertions, 15 deletions

diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb
index d7873d4a415..bdc1c0b939b 100644
--- a/ee/app/policies/ee/project_policy.rb
+++ b/ee/app/policies/ee/project_policy.rb
@@ -200,6 +200,8 @@ module EE
 
       rule { admin | (reject_unsigned_commits_disabled_globally & can?(:maintainer_access)) }.enable :change_reject_unsigned_commits
 
+      rule { reject_unsigned_commits_available }.enable :read_reject_unsigned_commits
+
       rule { ~reject_unsigned_commits_available }.prevent :change_reject_unsigned_commits
 
       rule { admin | (commit_committer_check_disabled_globally & can?(:maintainer_access)) }.policy do
diff --git a/ee/changelogs/unreleased/tpresa-expose-reject-unsigned-commits.yml b/ee/changelogs/unreleased/tpresa-expose-reject-unsigned-commits.yml
new file mode 100644
index 00000000000..023e258b9cd
--- /dev/null
+++ b/ee/changelogs/unreleased/tpresa-expose-reject-unsigned-commits.yml
@@ -0,0 +1,5 @@
+---
+title: Expose reject_unsigned_commits option via the API
+merge_request: 14165
+author:
+type: added
diff --git a/ee/lib/api/project_push_rule.rb b/ee/lib/api/project_push_rule.rb
index 167d1a2f21d..b02e96e5314 100644
--- a/ee/lib/api/project_push_rule.rb
+++ b/ee/lib/api/project_push_rule.rb
@@ -5,11 +5,7 @@ module API
     before { authenticate! }
     before { authorize_admin_project }
     before { check_project_feature_available!(:push_rules) }
-    before do
-      if params.has_key?(:commit_committer_check)
-        authorize! :change_commit_committer_check, user_project
-      end
-    end
+    before { authorize_change_param(user_project, :commit_committer_check, :reject_unsigned_commits) }
 
     params do
       requires :id, type: String, desc: 'The ID of a project'
@@ -27,10 +23,12 @@ module API
           optional :file_name_regex, type: String, desc: 'All commited filenames must not match this'
           optional :max_file_size, type: Integer, desc: 'Maximum file size (MB)'
           optional :commit_committer_check, type: Boolean, desc: 'Users may only push their own commits'
+          optional :reject_unsigned_commits, type: Boolean, desc: 'Only GPG signed commits can be pushed to this project'
           at_least_one_of :deny_delete_tag, :member_check, :prevent_secrets,
                           :commit_message_regex, :commit_message_negative_regex, :branch_name_regex, :author_email_regex,
                           :file_name_regex, :max_file_size,
-                          :commit_committer_check
+                          :commit_committer_check,
+                          :reject_unsigned_commits
         end
       end
 
diff --git a/ee/lib/ee/api/entities.rb b/ee/lib/ee/api/entities.rb
index ddb9c559861..d92ddf52a59 100644
--- a/ee/lib/ee/api/entities.rb
+++ b/ee/lib/ee/api/entities.rb
@@ -21,6 +21,16 @@ module EE
         end
       end
 
+      module EntityHelpers
+        def can_read(attr, &block)
+          ->(obj, opts) { Ability.allowed?(opts[:user], "read_#{attr}".to_sym, yield(obj)) }
+        end
+
+        def expose_restricted(attr, &block)
+          expose attr, if: can_read(attr, &block)
+        end
+      end
+
       module UserPublic
         extend ActiveSupport::Concern
 
@@ -206,13 +216,13 @@ module EE
       # EE-specific entities #
       ########################
       class ProjectPushRule < Grape::Entity
+        extend EntityHelpers
         expose :id, :project_id, :created_at
         expose :commit_message_regex, :commit_message_negative_regex, :branch_name_regex, :deny_delete_tag
         expose :member_check, :prevent_secrets, :author_email_regex
         expose :file_name_regex, :max_file_size
-        expose :commit_committer_check, if: ->(rule, opts) do
-          Ability.allowed?(opts[:user], :read_commit_committer_check, rule.project)
-        end
+        expose_restricted :commit_committer_check, &:project
+        expose_restricted :reject_unsigned_commits, &:project
       end
 
       class LdapGroupLink < Grape::Entity
diff --git a/ee/lib/ee/api/helpers.rb b/ee/lib/ee/api/helpers.rb
index 42346cc96ea..206b44cace2 100644
--- a/ee/lib/ee/api/helpers.rb
+++ b/ee/lib/ee/api/helpers.rb
@@ -50,6 +50,12 @@ module EE
         not_found! unless user_project.feature_available?(feature)
       end
 
+      def authorize_change_param(subject, *keys)
+        keys.each do |key|
+          authorize!("change_#{key}".to_sym, subject) if params.has_key?(key)
+        end
+      end
+
       def check_sha_param!(params, merge_request)
         if params[:sha] && merge_request.diff_head_sha != params[:sha]
           render_api_error!("SHA does not match HEAD of source branch: #{merge_request.diff_head_sha}", 409)
diff --git a/ee/spec/lib/ee/api/helpers_spec.rb b/ee/spec/lib/ee/api/helpers_spec.rb
index a74dd8d162e..8dbc2b1d7c0 100644
--- a/ee/spec/lib/ee/api/helpers_spec.rb
+++ b/ee/spec/lib/ee/api/helpers_spec.rb
@@ -90,4 +90,32 @@ describe EE::API::Helpers do
       expect(JSON.parse(last_response.body)).to eq({ 'message' => '401 Unauthorized' })
     end
   end
+
+  describe '#authorize_change_param' do
+    subject { Class.new.include(described_class).new }
+    let(:project) { create(:project) }
+
+    before do
+      allow(subject).to receive(:params).and_return({ change_commit_committer_check: true })
+    end
+
+    it 'does not throw exception if param is authorized' do
+      allow(subject).to receive(:authorize!).and_return(nil)
+
+      expect { subject.authorize_change_param(project, :change_commit_committer_check) }.not_to raise_error
+    end
+
+    context 'unauthorized param' do
+      before do
+        allow(subject).to receive(:authorize!).and_raise(Exception.new("Forbidden"))
+      end
+      it 'throws exception if unauthorized param is present' do
+        expect { subject.authorize_change_param(project, :change_commit_committer_check) }.to raise_error
+      end
+
+      it 'does not throw exception is unauthorized param is not present' do
+        expect { subject.authorize_change_param(project, :reject_unsigned_commit) }.not_to raise_error
+      end
+    end
+  end
 end
diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb
index a689cf3e7c7..dbfa12f7b76 100644
--- a/ee/spec/policies/project_policy_spec.rb
+++ b/ee/spec/policies/project_policy_spec.rb
@@ -896,4 +896,35 @@ describe ProjectPolicy do
       it { is_expected.to be_allowed(:read_commit_committer_check) }
     end
   end
+
+  context 'reject_unsigned_commits is not enabled by the current license' do
+    before do
+      stub_licensed_features(reject_unsigned_commits: false)
+    end
+
+    let(:current_user) { maintainer }
+
+    it { is_expected.not_to be_allowed(:change_reject_unsigned_commits) }
+    it { is_expected.not_to be_allowed(:read_reject_unsigned_commits) }
+  end
+
+  context 'reject_unsigned_commits is enabled by the current license' do
+    before do
+      stub_licensed_features(reject_unsigned_commits: true)
+    end
+
+    context 'the user is a maintainer' do
+      let(:current_user) { maintainer }
+
+      it { is_expected.to be_allowed(:change_reject_unsigned_commits) }
+      it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
+    end
+
+    context 'the user is a developer' do
+      let(:current_user) { developer }
+
+      it { is_expected.not_to be_allowed(:change_reject_unsigned_commits) }
+      it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
+    end
+  end
 end
diff --git a/ee/spec/requests/api/project_push_rule_spec.rb b/ee/spec/requests/api/project_push_rule_spec.rb
index 92e62e413e9..e48e0fb6e92 100644
--- a/ee/spec/requests/api/project_push_rule_spec.rb
+++ b/ee/spec/requests/api/project_push_rule_spec.rb
@@ -8,13 +8,15 @@ describe API::ProjectPushRule, 'ProjectPushRule', api: true do
 
   before do
     stub_licensed_features(push_rules: push_rules_enabled,
-                           commit_committer_check: ccc_enabled)
+                           commit_committer_check: ccc_enabled,
+                           reject_unsigned_commits: ruc_enabled)
     project.add_maintainer(user)
     project.add_developer(user3)
   end
 
   let(:push_rules_enabled) { true }
   let(:ccc_enabled) { true }
+  let(:ruc_enabled) { true }
 
   describe "GET /projects/:id/push_rule" do
     before do
@@ -47,15 +49,26 @@ describe API::ProjectPushRule, 'ProjectPushRule', api: true do
         end
       end
 
-      context 'the commit_committer_check feature is not enabled' do
-        let(:ccc_enabled) { false }
+      context 'the reject_unsigned_commits feature is enabled' do
+        let(:ruc_enabled) { true }
+
+        it 'returns the reject_unsigned_commits information' do
+          subset = attributes
+            .slice(:reject_unsigned_commits)
+            .transform_keys(&:to_s)
+          expect(json_response).to include(subset)
+        end
+      end
+
+      context 'the reject_unsigned_commits feature is not enabled' do
+        let(:ruc_enabled) { false }
 
         it 'succeeds' do
           expect(response).to have_gitlab_http_status(200)
         end
 
-        it 'does not return the commit_committer_check information' do
-          expect(json_response).not_to have_key('commit_committer_check')
+        it 'does not return the reject_unsigned_commits information' do
+          expect(json_response).not_to have_key('reject_unsigned_commits')
         end
       end
 
@@ -87,7 +100,8 @@ describe API::ProjectPushRule, 'ProjectPushRule', api: true do
         author_email_regex: '[a-zA-Z0-9]+@gitlab.com',
         file_name_regex: '[a-zA-Z0-9]+.key',
         max_file_size: 5,
-        commit_committer_check: true }
+        commit_committer_check: true,
+        reject_unsigned_commits: true }
     end
 
     let(:expected_response) do
@@ -107,6 +121,14 @@ describe API::ProjectPushRule, 'ProjectPushRule', api: true do
         end
       end
 
+      context 'reject_unsigned_commits not allowed by License' do
+        let(:ruc_enabled) { false }
+
+        it "is forbidden to use this service" do
+          expect(response).to have_gitlab_http_status(403)
+        end
+      end
+
       it "is accepted" do
         expect(response).to have_gitlab_http_status(201)
       end
@@ -143,6 +165,31 @@ describe API::ProjectPushRule, 'ProjectPushRule', api: true do
           end
         end
       end
+
+      context 'reject_unsigned_commits is not enabled' do
+        let(:ruc_enabled) { false }
+
+        it "is forbidden to send the the :reject_unsigned_commits parameter" do
+          expect(response).to have_gitlab_http_status(403)
+        end
+
+        context "without the :reject_unsigned_commits parameter" do
+          let(:rules_params) do
+            { deny_delete_tag: true,
+              member_check: true,
+              prevent_secrets: true,
+              commit_message_regex: 'JIRA\-\d+',
+              branch_name_regex: '(feature|hotfix)\/*',
+              author_email_regex: '[a-zA-Z0-9]+@gitlab.com',
+              file_name_regex: '[a-zA-Z0-9]+.key',
+              max_file_size: 5 }
+          end
+
+          it "sets all given parameters" do
+            expect(json_response).to include(expected_response)
+          end
+        end
+      end
     end
 
     it 'adds push rule to project with no file size' do
@@ -220,6 +267,26 @@ describe API::ProjectPushRule, 'ProjectPushRule', api: true do
       context 'the commit_committer_check feature is not enabled' do
         let(:ccc_enabled) { false }
 
+        it "is an error to provide this parameter" do
+          expect(response).to have_gitlab_http_status(403)
+        end
+      end
+    end
+
+    context "setting reject_unsigned_commits" do
+      let(:new_settings) { { reject_unsigned_commits: true } }
+
+      it "is successful" do
+        expect(response).to have_gitlab_http_status(200)
+      end
+
+      it "sets the reject_unsigned_commits" do
+        expect(json_response).to include('reject_unsigned_commits' => true)
+      end
+
+      context 'the reject_unsigned_commits feature is not enabled' do
+        let(:ruc_enabled) { false }
+
         it "is an error to provide the this parameter" do
           expect(response).to have_gitlab_http_status(403)
         end