Merge branch 'cherry-pick-2a2568ed' into '17-3-stable-ee'

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

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



Merged-by: Gary Holtz <gholtz@gitlab.com>
Approved-by: Gary Holtz <gholtz@gitlab.com>
Approved-by: mo khan <mo@mokhan.ca>
Co-authored-by: Patrick Bajao <ebajao@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) }