From 57bd16428b352c1038d91df0ba45ebe27f3bb6f4 Mon Sep 17 00:00:00 2001 From: Jessie Young Date: Fri, 11 Jul 2025 14:25:43 -0700 Subject: [PATCH] Fix workflow authorizaton checkpointing * Right now, if a user has `agentic_duo_chat` feature flag enabled but not `duo_workflow`, the UX of agentic chat is problematic because the checkpointing requests fail. * With this fix, we are creating specific checkpointing authorization for chat workflow types. Changelog: fixed EE: true --- .../ai/duo_workflows/checkpoint_policy.rb | 18 ++++- .../duo_workflows/checkpoint_policy_spec.rb | 77 +++++++++++-------- 2 files changed, 61 insertions(+), 34 deletions(-) diff --git a/ee/app/policies/ai/duo_workflows/checkpoint_policy.rb b/ee/app/policies/ai/duo_workflows/checkpoint_policy.rb index ae6b4830d1d8c1..5a871187a1dd13 100644 --- a/ee/app/policies/ai/duo_workflows/checkpoint_policy.rb +++ b/ee/app/policies/ai/duo_workflows/checkpoint_policy.rb @@ -3,6 +3,10 @@ module Ai module DuoWorkflows class CheckpointPolicy < BasePolicy + condition(:can_use_agentic_chat_in_project) do + can?(:access_duo_agentic_chat, @subject.project) + end + condition(:can_use_duo_workflows_in_project) do can?(:duo_workflow, @subject.project) end @@ -11,7 +15,19 @@ class CheckpointPolicy < BasePolicy @subject.workflow.user == @user end - rule { can_use_duo_workflows_in_project & is_workflow_owner }.policy do + condition(:agentic_chat_workflow) do + @subject.workflow.chat? + end + + condition(:other_workflow_type) do + !@subject.workflow.chat? + end + + rule { other_workflow_type & can_use_duo_workflows_in_project & is_workflow_owner }.policy do + enable :read_duo_workflow_event + end + + rule { agentic_chat_workflow & can_use_agentic_chat_in_project & is_workflow_owner }.policy do enable :read_duo_workflow_event end end diff --git a/ee/spec/policies/ai/duo_workflows/checkpoint_policy_spec.rb b/ee/spec/policies/ai/duo_workflows/checkpoint_policy_spec.rb index 37b9f238d27494..e41cee01da78ca 100644 --- a/ee/spec/policies/ai/duo_workflows/checkpoint_policy_spec.rb +++ b/ee/spec/policies/ai/duo_workflows/checkpoint_policy_spec.rb @@ -7,58 +7,69 @@ let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } - let_it_be(:checkpoint) { create(:duo_workflows_checkpoint, project: project) } - let_it_be(:guest) { create(:user, guest_of: checkpoint.project) } - let_it_be(:developer) { create(:user, developer_of: checkpoint.project) } + let_it_be(:workflow) { create(:duo_workflows_workflow, project: project) } + let_it_be(:checkpoint) { create(:duo_workflows_checkpoint, project: project, workflow: workflow) } + let_it_be(:guest) { create(:user, guest_of: project) } + let_it_be(:developer) { create(:user, developer_of: project) } let(:current_user) { guest } describe "read_duo_workflow_event" do - context "when duo_workflow FF is disabled" do - before do - stub_feature_flags(duo_workflow: false) - end - + context "when user is guest" do it { is_expected.to be_disallowed(:read_duo_workflow_event) } end - context "when duo workflow is not available" do - before do - allow(::Gitlab::Llm::StageCheck).to receive(:available?).with(project, :duo_workflow).and_return(false) + context "when user is a developer" do + let(:current_user) { developer } + + context "when user is not workflow owner" do + it { is_expected.to be_disallowed(:read_duo_workflow_event) } end - it { is_expected.to be_disallowed(:read_duo_workflow_event) } - end + context "when user is workflow owner" do + before do + workflow.update!(user: current_user) + end - context "when duo workflow is available" do - before do - allow(::Gitlab::Llm::StageCheck).to receive(:available?).with(project, :duo_workflow).and_return(true) - end + context "when workflow is a chat workflow" do + let_it_be(:chat_workflow) { create(:duo_workflows_workflow, :agentic_chat, project: project) } + let_it_be(:chat_checkpoint) { create(:duo_workflows_checkpoint, project: project, workflow: chat_workflow) } + let(:policy) { described_class.new(current_user, chat_checkpoint) } - context "when user is guest" do - it { is_expected.to be_disallowed(:read_duo_workflow_event) } - end + before do + chat_workflow.update!(user: current_user) + allow(policy).to receive(:can?).with(:access_duo_agentic_chat, project) + .and_return(can_use_agentic_chat) + end - context "when user is a developer" do - let(:current_user) { developer } + context "when agentic chat is allowed" do + let(:can_use_agentic_chat) { true } + + it { is_expected.to be_allowed(:read_duo_workflow_event) } + end - context "when user is not workflow owner" do - it { is_expected.to be_disallowed(:read_duo_workflow_event) } + context "when agentic chat is disallowed" do + let(:can_use_agentic_chat) { false } + + it { is_expected.to be_disallowed(:read_duo_workflow_event) } + end end - context "when user is workflow owner" do + context "when workflow is not a chat workflow" do before do - checkpoint.workflow.update!(user: current_user) + allow(policy).to receive(:can?).with(:duo_workflow, project) + .and_return(can_use_duo_workflow) end - it { is_expected.to be_allowed(:read_duo_workflow_event) } + context "when duo workflow is allowed" do + let(:can_use_duo_workflow) { true } + + it { is_expected.to be_allowed(:read_duo_workflow_event) } + end - context "when duo_features_enabled settings is turned off" do - before do - project.project_setting.update!(duo_features_enabled: false) - end + context "when duo workflow is disallowed" do + let(:can_use_duo_workflow) { false } - it { is_expected.to be_disallowed(:read_duo_workflow) } - it { is_expected.to be_disallowed(:update_duo_workflow) } + it { is_expected.to be_disallowed(:read_duo_workflow_event) } end end end -- GitLab