Merge branch '481955-fix-duo-cli-generate-commit-messages-policies' into 'master'

Fix Code Review AI features policies to check duo features enabled toggle

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165021



Merged-by: Jarka Košanová <jarka@gitlab.com>
Approved-by: Jan Provaznik <jprovaznik@gitlab.com>
Approved-by: Jarka Košanová <jarka@gitlab.com>
Reviewed-by: Jarka Košanová <jarka@gitlab.com>
Reviewed-by: Patrick Bajao <ebajao@gitlab.com>
Reviewed-by: Gosia Ksionek <mksionek@gitlab.com>
Co-authored-by: Patrick Bajao <ebajao@gitlab.com>

(cherry picked from commit 2a2568ed)

3ed56e62 Fix Duo for CLI policy to check instance setting for SM
a9d74e74 Fix Generate Commit Message policy to check project settings
fce8d092 Remove unneeded use of safe navigation operator

Co-authored-by: Jarka Košanová <jarka@gitlab.com>

ee/app/policies/ee/global_policy.rb

@@ -69,19 +69,23 @@ module GlobalPolicy
         self_hosted_models.free_access? || self_hosted_models.allowed_for?(@user)
       end
-      condition(:user_allowed_to_use_glab_ask_git_command) do
-        next true if glab_ask_git_command_data.allowed_for?(@user)
-
-        next false unless glab_ask_git_command_data.free_access?
-
+      condition(:glab_ask_git_command_licensed) do
         if ::Gitlab::Saas.feature_available?(:duo_chat_on_saas) # check if we are on SaaS
-          @user.any_group_with_ga_ai_available?(:glab_ask_git_command)
-        else
-          ::License.feature_available?(:glab_ask_git_command)
+          next @user.any_group_with_ga_ai_available?(:glab_ask_git_command)
         end
+
+        next false unless ::Gitlab::CurrentSettings.duo_features_enabled?
+
+        ::License.feature_available?(:glab_ask_git_command)
       end
-      rule { user_allowed_to_use_glab_ask_git_command }.policy do
+      condition(:user_allowed_to_use_glab_ask_git_command) do
+        next true if glab_ask_git_command_data.free_access?
+
+        glab_ask_git_command_data.allowed_for?(@user)
+      end
+
+      rule { glab_ask_git_command_licensed & user_allowed_to_use_glab_ask_git_command }.policy do
         enable :access_glab_ask_git_command
       end
@@ -213,30 +217,6 @@ module GlobalPolicy
       rule { security_policy_bot }.policy do
         enable :access_git
       end
-
-      condition(:generate_commit_message_enabled) do
-        ::Feature.enabled?(:generate_commit_message_flag, @user)
-      end
-
-      condition(:user_allowed_to_use_generate_commit_message) do
-        next true if generate_commit_message_data.allowed_for?(@user)
-
-        next false unless generate_commit_message_data.free_access?
-
-        if ::Gitlab::Saas.feature_available?(:duo_chat_on_saas) # check if we are on SaaS
-          @user.any_group_with_ga_ai_available?(:generate_commit_message)
-        else
-          ::License.feature_available?(:generate_commit_message)
-        end
-      end
-
-      rule { generate_commit_message_enabled & user_allowed_to_use_generate_commit_message }.policy do
-        enable :access_generate_commit_message
-      end
-    end
-
-    def generate_commit_message_data
-      CloudConnector::AvailableServices.find_by_name(:generate_commit_message)
     end
     def glab_ask_git_command_data

ee/app/policies/ee/merge_request_policy.rb

@@ -51,6 +51,25 @@ module MergeRequestPolicy
         subject&.project&.custom_roles_enabled?
       end
+      condition(:generate_commit_message_enabled) do
+        ::Feature.enabled?(:generate_commit_message_flag, @user) &&
+          subject.project.project_setting.duo_features_enabled?
+      end
+
+      condition(:generate_commit_message_licensed) do
+        if ::Gitlab::Saas.feature_available?(:duo_chat_on_saas) # check if we are on SaaS
+          next @user.any_group_with_ga_ai_available?(:generate_commit_message)
+        end
+
+        ::License.feature_available?(:generate_commit_message)
+      end
+
+      condition(:user_allowed_to_use_generate_commit_message) do
+        next true if generate_commit_message_data.free_access?
+
+        generate_commit_message_data.allowed_for?(@user)
+      end
+
       def read_only?
         @subject.target_project&.namespace&.read_only?
       end
@@ -87,6 +106,12 @@ def group_access?(protected_branch)
       rule do
         summarize_draft_code_review_enabled & can?(:read_merge_request)
       end.enable :summarize_draft_code_review
+
+      rule do
+        generate_commit_message_enabled &
+          generate_commit_message_licensed &
+          user_allowed_to_use_generate_commit_message
+      end.enable :access_generate_commit_message
     end
     private
@@ -97,5 +122,9 @@ def can_approve?
       super
     end
+
+    def generate_commit_message_data
+      CloudConnector::AvailableServices.find_by_name(:generate_commit_message)
+    end
   end
 end

ee/app/services/llm/generate_commit_message_service.rb

@@ -5,7 +5,7 @@ class GenerateCommitMessageService < BaseService
     def valid?
       super &&
         Gitlab::Llm::StageCheck.available?(resource.resource_parent, :generate_commit_message) &&
-        user.can?(:access_generate_commit_message)
+        user.can?(:access_generate_commit_message, resource)
     end
     private

ee/spec/policies/global_policy_spec.rb

@@ -793,17 +793,19 @@
     let(:policy) { :access_glab_ask_git_command }
     context 'for self-managed' do
-      where(:licensed, :free_access, :allowed_for, :enabled_for_user) do
-        false | false | false | be_disallowed(:access_glab_ask_git_command)
-        true  | false | false | be_disallowed(:access_glab_ask_git_command)
-        true  | false | true  | be_allowed(:access_glab_ask_git_command)
-        true  | true  | false | be_allowed(:access_glab_ask_git_command)
-        true  | true  | true  | be_allowed(:access_glab_ask_git_command)
+      where(:duo_features_enabled, :licensed, :free_access, :allowed_for, :enabled_for_user) do
+        true  | false | false | false | be_disallowed(:access_glab_ask_git_command)
+        true  | true  | false | false | be_disallowed(:access_glab_ask_git_command)
+        false | true  | true  | true  | be_disallowed(:access_glab_ask_git_command)
+        true  | true  | false | true  | be_allowed(:access_glab_ask_git_command)
+        true  | true  | true  | false | be_allowed(:access_glab_ask_git_command)
+        true  | true  | true  | true  | be_allowed(:access_glab_ask_git_command)
       end
       with_them do
         before do
           stub_licensed_features(glab_ask_git_command: licensed)
+          stub_application_setting(duo_features_enabled: duo_features_enabled)
           service_data = CloudConnector::SelfManaged::AvailableServiceData.new(:glab_ask_git_command, nil, nil)
           allow(CloudConnector::AvailableServices).to receive(:find_by_name)
@@ -820,7 +822,7 @@
         where(:free_access, :any_group_with_ga_ai_available, :allowed_for, :enabled_for_user) do
           false | false | false | be_disallowed(:access_glab_ask_git_command)
           true  | false | false | be_disallowed(:access_glab_ask_git_command)
-          false | false | true  | be_allowed(:access_glab_ask_git_command)
+          false | false | true  | be_disallowed(:access_glab_ask_git_command)
           true  | true  | false | be_allowed(:access_glab_ask_git_command)
           true  | true  | true  | be_allowed(:access_glab_ask_git_command)
         end
@@ -879,62 +881,4 @@
       it { is_expected.to be_disallowed(:manage_ai_settings) }
     end
   end
-
-  describe 'access_generate_commit_message' do
-    let(:policy) { :access_generate_commit_message }
-
-    context 'for self-managed' do
-      where(:flag_enabled, :licensed, :free_access, :allowed_for, :enabled_for_user) do
-        false | false | false | false | be_disallowed(:access_generate_commit_message)
-        true  | false | false | false | be_disallowed(:access_generate_commit_message)
-        true  | true  | false | false | be_disallowed(:access_generate_commit_message)
-        true  | true  | false | true  | be_allowed(:access_generate_commit_message)
-        true  | true  | true  | false | be_allowed(:access_generate_commit_message)
-      end
-
-      with_them do
-        before do
-          stub_licensed_features(generate_commit_message: licensed)
-          stub_feature_flags(generate_commit_message_flag: flag_enabled)
-
-          service_data = CloudConnector::SelfManaged::AvailableServiceData.new(:generate_commit_message, nil, nil)
-          allow(CloudConnector::AvailableServices).to receive(:find_by_name)
-                                                        .with(:generate_commit_message)
-                                                        .and_return(service_data)
-          allow(service_data).to receive(:allowed_for?).with(current_user).and_return(allowed_for)
-          allow(service_data).to receive(:free_access?).and_return(free_access)
-        end
-
-        it { is_expected.to enabled_for_user }
-      end
-
-      context 'for SaaS', :saas do
-        where(:flag_enabled, :free_access, :any_group_with_ga_ai_available, :allowed_for, :enabled_for_user) do
-          false | false | false | false | be_disallowed(:access_generate_commit_message)
-          true  | false | false | false | be_disallowed(:access_generate_commit_message)
-          true  | true  | false | false | be_disallowed(:access_generate_commit_message)
-          true  | false | false | false | be_disallowed(:access_generate_commit_message)
-          true  | false | false | true  | be_allowed(:access_generate_commit_message)
-          true  | true  | true  | false | be_allowed(:access_generate_commit_message)
-        end
-
-        with_them do
-          before do
-            stub_feature_flags(generate_commit_message_flag: flag_enabled)
-
-            service_data = CloudConnector::SelfManaged::AvailableServiceData.new(:generate_commit_message, nil, nil)
-            allow(CloudConnector::AvailableServices).to receive(:find_by_name)
-                                                          .with(:generate_commit_message)
-                                                          .and_return(service_data)
-            allow(service_data).to receive(:allowed_for?).with(current_user).and_return(allowed_for)
-            allow(service_data).to receive(:free_access?).and_return(free_access)
-            allow(current_user).to receive(:any_group_with_ga_ai_available?)
-                                     .and_return(any_group_with_ga_ai_available)
-          end
-
-          it { is_expected.to enabled_for_user }
-        end
-      end
-    end
-  end
 end

ee/spec/policies/merge_request_policy_spec.rb

@@ -5,6 +5,7 @@
 RSpec.describe MergeRequestPolicy, :aggregate_failures, feature_category: :code_review_workflow do
   include ProjectForksHelper
   include AdminModeHelper
+  using RSpec::Parameterized::TableSyntax
   let_it_be(:guest) { create(:user) }
   let_it_be(:developer) { create(:user) }
@@ -401,4 +402,76 @@ def policy_for(user)
       end
     end
   end
+
+  describe 'access_generate_commit_message' do
+    let(:user) { owner }
+
+    subject(:policy) { policy_for(user) }
+
+    context 'for self-managed' do
+      where(:flag_enabled, :duo_features_enabled, :licensed, :free_access, :allowed_for, :enabled_for_user) do
+        false | true  | false | false | false | be_disallowed(:access_generate_commit_message)
+        true  | true  | false | false | false | be_disallowed(:access_generate_commit_message)
+        true  | true  | true  | false | false | be_disallowed(:access_generate_commit_message)
+        true  | false | true  | true  | true  | be_disallowed(:access_generate_commit_message)
+        true  | true  | true  | false | true  | be_allowed(:access_generate_commit_message)
+        true  | true  | true  | true  | false | be_allowed(:access_generate_commit_message)
+        true  | true  | true  | true  | true  | be_allowed(:access_generate_commit_message)
+      end
+
+      with_them do
+        before do
+          stub_licensed_features(generate_commit_message: licensed)
+          stub_feature_flags(generate_commit_message_flag: flag_enabled)
+
+          allow(project)
+            .to receive_message_chain(:project_setting, :duo_features_enabled?)
+            .and_return(duo_features_enabled)
+
+          service_data = CloudConnector::SelfManaged::AvailableServiceData.new(:generate_commit_message, nil, nil)
+          allow(CloudConnector::AvailableServices).to receive(:find_by_name)
+                                                        .with(:generate_commit_message)
+                                                        .and_return(service_data)
+          allow(service_data).to receive(:allowed_for?).with(user).and_return(allowed_for)
+          allow(service_data).to receive(:free_access?).and_return(free_access)
+        end
+
+        it { is_expected.to enabled_for_user }
+      end
+
+      context 'for SaaS', :saas do
+        where(:flag_enabled, :duo_features_enabled, :free_access, :any_group_with_ga_ai_available, :allowed_for, :enabled_for_user) do
+          false | true  | false | false | false | be_disallowed(:access_generate_commit_message)
+          true  | true  | false | false | false | be_disallowed(:access_generate_commit_message)
+          true  | true  | true  | false | false | be_disallowed(:access_generate_commit_message)
+          true  | true  | false | false | false | be_disallowed(:access_generate_commit_message)
+          true  | true  | false | false | true  | be_disallowed(:access_generate_commit_message)
+          true  | false | true  | true  | true  | be_disallowed(:access_generate_commit_message)
+          true  | true  | true  | true  | false | be_allowed(:access_generate_commit_message)
+          true  | true  | true  | true  | true  | be_allowed(:access_generate_commit_message)
+        end
+
+        with_them do
+          before do
+            stub_feature_flags(generate_commit_message_flag: flag_enabled)
+
+            allow(project)
+              .to receive_message_chain(:project_setting, :duo_features_enabled?)
+              .and_return(duo_features_enabled)
+
+            service_data = CloudConnector::SelfManaged::AvailableServiceData.new(:generate_commit_message, nil, nil)
+            allow(CloudConnector::AvailableServices).to receive(:find_by_name)
+                                                          .with(:generate_commit_message)
+                                                          .and_return(service_data)
+            allow(service_data).to receive(:allowed_for?).with(user).and_return(allowed_for)
+            allow(service_data).to receive(:free_access?).and_return(free_access)
+            allow(user).to receive(:any_group_with_ga_ai_available?)
+                                     .and_return(any_group_with_ga_ai_available)
+          end
+
+          it { is_expected.to enabled_for_user }
+        end
+      end
+    end
+  end
 end

ee/spec/services/llm/generate_commit_message_service_spec.rb

@@ -31,7 +31,10 @@
       before do
         group.add_developer(user)
-        allow(user).to receive(:can?).with(:access_generate_commit_message).and_return(true)
+        allow(user)
+          .to receive(:can?)
+          .with(:access_generate_commit_message, resource)
+          .and_return(true)
       end
       it_behaves_like 'schedules completion worker' do
@@ -76,7 +79,10 @@
       before do
         group.add_maintainer(user)
-        allow(user).to receive(:can?).with(:access_generate_commit_message).and_return(access_generate_commit_message)
+        allow(user)
+          .to receive(:can?)
+          .with(:access_generate_commit_message, resource)
+          .and_return(access_generate_commit_message)
       end
       subject { described_class.new(user, resource, options) }