From 8f61cf55d3bff70ab206d33e7f40df0d5acc8cf6 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Wed, 13 Nov 2024 15:49:37 +0100 Subject: [PATCH 01/10] Add audit logs for ai_workflows actions Adds audit logs for any API requests which were authenticated using a token with ai_workflows scope. --- app/models/current.rb | 1 + .../types/api_request_access_with_scope.yml | 10 ++++++++++ lib/api/api_guard.rb | 13 +++++++++++++ lib/api/helpers.rb | 16 ++++++++++++++++ lib/api/issues.rb | 2 +- 5 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 config/audit_events/types/api_request_access_with_scope.yml diff --git a/app/models/current.rb b/app/models/current.rb index 9dff4318c95f9f..011dd35940cb41 100644 --- a/app/models/current.rb +++ b/app/models/current.rb @@ -18,6 +18,7 @@ def message # watch background jobs need to reset on each job if using attribute :organization, :organization_assigned + attribute :audit_access_scopes def organization=(value) # We want to explicitly allow only one organization assignment per thread diff --git a/config/audit_events/types/api_request_access_with_scope.yml b/config/audit_events/types/api_request_access_with_scope.yml new file mode 100644 index 00000000000000..d92f48964b4450 --- /dev/null +++ b/config/audit_events/types/api_request_access_with_scope.yml @@ -0,0 +1,10 @@ +--- +name: api_request_access_with_scope +description: A susbset of API requests authenticated by token with ai_workflows scope +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/499461 +introduced_by_mr: https://to-be-done/ +feature_category: duo_workflow +milestone: '17.6' +saved_to_database: true +scope: [User] +streamed: true diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 279d653efb9ba6..0949839ddead66 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -24,6 +24,10 @@ module APIGuard helpers HelperMethods install_error_responders(base) + + finally do + audit_request_with_access_scopes(Current.audit_access_scopes) if Current.audit_access_scopes + end end class_methods do @@ -38,6 +42,15 @@ def allow_access_with_scope(scopes, options = {}) end end + def allow_audited_access_with_scope(scopes, options = {}) + original_if = options.delete(:if) + options[:if] = ->(request) do + Current.audit_access_scopes = scopes + original_if.call(request) + end + allow_access_with_scope(scopes, options) + end + def allowed_scopes @scopes ||= [] end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index cb8face9911e7e..84d40c7ae11f12 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -816,6 +816,22 @@ def validate_search_rate_limit! end end + def audit_request_with_access_scopes(scopes) + context = { + name: 'api_request_access_with_scope', + author: current_user || ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'), + scope: current_user, + target: ::Gitlab::Audit::NullTarget.new, + message: "API request with scope #{scopes} - #{request.request_method} #{request.path}", + additional_details: { + request: request.path, + method: request.request_method + } + } + + ::Gitlab::Audit::Auditor.audit(context) + end + private # rubocop:disable Gitlab/ModuleWithInstanceVariables diff --git a/lib/api/issues.rb b/lib/api/issues.rb index ff37230cf9c5a0..7f67049c899fda 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -9,7 +9,7 @@ class Issues < ::API::Base before { authenticate_non_get! } - allow_access_with_scope :ai_workflows, if: ->(request) do + allow_audited_access_with_scope :ai_workflows, if: ->(request) do request.get? || request.head? || request.post? || request.put? end -- GitLab From 394a1b38ae8245d212697373eff9fe8378f4171e Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Fri, 15 Nov 2024 08:34:46 +0100 Subject: [PATCH 02/10] Make audit scopes more generic --- app/models/current.rb | 2 +- .../types/api_request_access_with_scope.yml | 2 +- lib/api/api_guard.rb | 14 ++++---------- lib/api/helpers.rb | 7 ++++--- lib/api/issues.rb | 2 +- lib/api/scope.rb | 7 ++++++- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/app/models/current.rb b/app/models/current.rb index 011dd35940cb41..9bf40b5d3c4f22 100644 --- a/app/models/current.rb +++ b/app/models/current.rb @@ -18,7 +18,7 @@ def message # watch background jobs need to reset on each job if using attribute :organization, :organization_assigned - attribute :audit_access_scopes + attribute :audit_access_scope def organization=(value) # We want to explicitly allow only one organization assignment per thread diff --git a/config/audit_events/types/api_request_access_with_scope.yml b/config/audit_events/types/api_request_access_with_scope.yml index d92f48964b4450..62fc2eb4110928 100644 --- a/config/audit_events/types/api_request_access_with_scope.yml +++ b/config/audit_events/types/api_request_access_with_scope.yml @@ -2,7 +2,7 @@ name: api_request_access_with_scope description: A susbset of API requests authenticated by token with ai_workflows scope introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/499461 -introduced_by_mr: https://to-be-done/ +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/172548 feature_category: duo_workflow milestone: '17.6' saved_to_database: true diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 0949839ddead66..015243c497631e 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -26,7 +26,10 @@ module APIGuard install_error_responders(base) finally do - audit_request_with_access_scopes(Current.audit_access_scopes) if Current.audit_access_scopes + if Current.audit_access_scope + audit_request_with_access_scope(Current.audit_access_scope) + Current.audit_access_scope = nil + end end end @@ -42,15 +45,6 @@ def allow_access_with_scope(scopes, options = {}) end end - def allow_audited_access_with_scope(scopes, options = {}) - original_if = options.delete(:if) - options[:if] = ->(request) do - Current.audit_access_scopes = scopes - original_if.call(request) - end - allow_access_with_scope(scopes, options) - end - def allowed_scopes @scopes ||= [] end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 84d40c7ae11f12..c5b5c56a9c11a6 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -816,16 +816,17 @@ def validate_search_rate_limit! end end - def audit_request_with_access_scopes(scopes) + def audit_request_with_access_scope(scope) context = { name: 'api_request_access_with_scope', author: current_user || ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'), scope: current_user, target: ::Gitlab::Audit::NullTarget.new, - message: "API request with scope #{scopes} - #{request.request_method} #{request.path}", + message: "API request with token scope #{scope} - #{request.request_method} #{request.path}", additional_details: { request: request.path, - method: request.request_method + method: request.request_method, + token_scope: scope } } diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 7f67049c899fda..ff37230cf9c5a0 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -9,7 +9,7 @@ class Issues < ::API::Base before { authenticate_non_get! } - allow_audited_access_with_scope :ai_workflows, if: ->(request) do + allow_access_with_scope :ai_workflows, if: ->(request) do request.get? || request.head? || request.post? || request.put? end diff --git a/lib/api/scope.rb b/lib/api/scope.rb index 62aefcceb4bae9..a3ff4833140d8f 100644 --- a/lib/api/scope.rb +++ b/lib/api/scope.rb @@ -6,6 +6,8 @@ module API class Scope attr_reader :name, :if + AUDIT_SCOPES = [:ai_workflows].freeze + def initialize(name, options = {}) @name = name.to_sym @if = options[:if] @@ -14,7 +16,10 @@ def initialize(name, options = {}) # Are the `scopes` passed in sufficient to adequately authorize the passed # request for the scope represented by the current instance of this class? def sufficient?(scopes, request) - scopes.include?(self.name) && verify_if_condition(request) + sufficient = scopes.include?(self.name) && verify_if_condition(request) + Current.audit_access_scope = name if sufficient && AUDIT_SCOPES.include?(name) + + sufficient end private -- GitLab From dbb1db21523f395a251b0b0c2bc3db9c0a988a34 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Fri, 15 Nov 2024 13:18:02 +0100 Subject: [PATCH 03/10] Move info about audited scopes to API --- app/models/current.rb | 2 +- doc/user/compliance/audit_event_types.md | 6 ++ lib/api/api_guard.rb | 8 ++- lib/api/helpers.rb | 2 +- lib/api/scope.rb | 4 +- spec/requests/api/api_guard_spec.rb | 70 ++++++++++++++++++++++++ 6 files changed, 84 insertions(+), 8 deletions(-) create mode 100644 spec/requests/api/api_guard_spec.rb diff --git a/app/models/current.rb b/app/models/current.rb index 9bf40b5d3c4f22..0fb8a5ac8c88fc 100644 --- a/app/models/current.rb +++ b/app/models/current.rb @@ -18,7 +18,7 @@ def message # watch background jobs need to reset on each job if using attribute :organization, :organization_assigned - attribute :audit_access_scope + attribute :token_scope def organization=(value) # We want to explicitly allow only one organization assignment per thread diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 41be7a31163360..6a4e36a266e961 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -257,6 +257,12 @@ Audit event types belong to the following product categories. | [`cluster_agent_token_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036) | Triggered when a user creates a cluster agent token | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.10](https://gitlab.com/gitlab-org/gitlab/-/issues/382133) | Project | | [`cluster_agent_token_revoked`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036) | Triggered when a user revokes a cluster agent token | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.10](https://gitlab.com/gitlab-org/gitlab/-/issues/382133) | Project | +### Duo workflow + +| Name | Description | Saved to database | Streamed | Introduced in | Scope | +|:------------|:------------|:------------------|:---------|:--------------|:--------------| +| [`api_request_access_with_scope`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/172548) | A susbset of API requests authenticated by token with ai_workflows scope | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.6](https://gitlab.com/gitlab-org/gitlab/-/issues/499461) | User | + ### Dynamic application security testing | Name | Description | Saved to database | Streamed | Introduced in | Scope | diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 015243c497631e..591774caad9f9f 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -9,6 +9,8 @@ module APIGuard extend ActiveSupport::Concern include Gitlab::Utils::StrongMemoize + TOKEN_SCOPES_TO_AUDIT = [:ai_workflows].freeze + included do |base| # OAuth2 Resource Server Authentication use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request| @@ -26,9 +28,9 @@ module APIGuard install_error_responders(base) finally do - if Current.audit_access_scope - audit_request_with_access_scope(Current.audit_access_scope) - Current.audit_access_scope = nil + if Current.token_scope && ::API::APIGuard::TOKEN_SCOPES_TO_AUDIT.include?(Current.token_scope) + audit_request_with_token_scope(Current.token_scope) + Current.token_scope = nil end end end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index c5b5c56a9c11a6..2672c9047cfc84 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -816,7 +816,7 @@ def validate_search_rate_limit! end end - def audit_request_with_access_scope(scope) + def audit_request_with_token_scope(scope) context = { name: 'api_request_access_with_scope', author: current_user || ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'), diff --git a/lib/api/scope.rb b/lib/api/scope.rb index a3ff4833140d8f..13b0ef8e89b356 100644 --- a/lib/api/scope.rb +++ b/lib/api/scope.rb @@ -6,8 +6,6 @@ module API class Scope attr_reader :name, :if - AUDIT_SCOPES = [:ai_workflows].freeze - def initialize(name, options = {}) @name = name.to_sym @if = options[:if] @@ -17,7 +15,7 @@ def initialize(name, options = {}) # request for the scope represented by the current instance of this class? def sufficient?(scopes, request) sufficient = scopes.include?(self.name) && verify_if_condition(request) - Current.audit_access_scope = name if sufficient && AUDIT_SCOPES.include?(name) + Current.token_scope = name if sufficient sufficient end diff --git a/spec/requests/api/api_guard_spec.rb b/spec/requests/api/api_guard_spec.rb new file mode 100644 index 00000000000000..e628afdca2eefb --- /dev/null +++ b/spec/requests/api/api_guard_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::APIGuard, feature_category: :shared do + let_it_be(:user) { create(:user) } + let_it_be(:token) { create(:oauth_access_token, user: user, scopes: [:ai_workflows]) } + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:path) { "/projects/#{issue.project.id}/issues/#{issue.iid}" } + + before_all do + project.add_developer(user) + end + + shared_examples 'audited request' do + it 'adds audit log' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including({ + name: 'api_request_access_with_scope', + message: "API request with token scope ai_workflows - GET /api/v4#{path}" + })).and_call_original + + subject + + expect(response).to have_gitlab_http_status(status) + end + end + + shared_examples 'not audited request' do + it "doesn't adds audit log" do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + subject + + expect(response).to have_gitlab_http_status(status) + end + end + + context 'when endpoint allows token with ai_workflow scope' do + subject { get api(path, oauth_access_token: token) } + + context 'when token with ai_workflows scope is used' do + let(:status) { :ok } + + it_behaves_like 'audited request' + + context 'when request fails' do + let_it_be(:path) { "/projects/#{issue.project.id}/issues/#{non_existing_record_id}" } + let(:status) { :not_found } + + it_behaves_like 'audited request' + end + end + + context 'when token with ai_workflows scope is not used' do + let_it_be(:token) { create(:oauth_access_token, user: user, scopes: [:api]) } + let(:status) { :ok } + + it_behaves_like 'not audited request' + end + end + + context "when endpoint doesn't allow token with ai_workflow scope" do + subject { delete api(path, oauth_access_token: token) } + + let(:status) { :forbidden } + + it_behaves_like 'not audited request' + end +end -- GitLab From 2ce8e584efce046917e3c061ba37729bcb3bf0bf Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Tue, 19 Nov 2024 14:32:13 +0100 Subject: [PATCH 04/10] Move audit logging to current_user method --- app/models/current.rb | 1 - lib/api/api_guard.rb | 9 ---- lib/api/helpers.rb | 16 ++++--- lib/api/scope.rb | 5 +-- lib/gitlab/auth/auth_finders.rb | 6 ++- spec/requests/api/api_guard_spec.rb | 70 ----------------------------- spec/requests/api/api_spec.rb | 67 +++++++++++++++++++++++++++ 7 files changed, 84 insertions(+), 90 deletions(-) delete mode 100644 spec/requests/api/api_guard_spec.rb diff --git a/app/models/current.rb b/app/models/current.rb index 0fb8a5ac8c88fc..9dff4318c95f9f 100644 --- a/app/models/current.rb +++ b/app/models/current.rb @@ -18,7 +18,6 @@ def message # watch background jobs need to reset on each job if using attribute :organization, :organization_assigned - attribute :token_scope def organization=(value) # We want to explicitly allow only one organization assignment per thread diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 591774caad9f9f..279d653efb9ba6 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -9,8 +9,6 @@ module APIGuard extend ActiveSupport::Concern include Gitlab::Utils::StrongMemoize - TOKEN_SCOPES_TO_AUDIT = [:ai_workflows].freeze - included do |base| # OAuth2 Resource Server Authentication use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request| @@ -26,13 +24,6 @@ module APIGuard helpers HelperMethods install_error_responders(base) - - finally do - if Current.token_scope && ::API::APIGuard::TOKEN_SCOPES_TO_AUDIT.include?(Current.token_scope) - audit_request_with_token_scope(Current.token_scope) - Current.token_scope = nil - end - end end class_methods do diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 2672c9047cfc84..306aa092cb1cf5 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -18,6 +18,7 @@ module Helpers API_EXCEPTION_ENV = 'gitlab.api.exception' API_RESPONSE_STATUS_CODE = 'gitlab.api.response_status_code' INTEGER_ID_REGEX = /^-?\d+$/ + TOKEN_SCOPES_TO_AUDIT = [:ai_workflows].freeze def logger API.logger @@ -89,6 +90,7 @@ def current_user if @current_user load_balancer_stick_request(::ApplicationRecord, :user, @current_user.id) + audit_request_with_token_scope(@current_user) end @current_user @@ -816,17 +818,21 @@ def validate_search_rate_limit! end end - def audit_request_with_token_scope(scope) + def audit_request_with_token_scope(user) + token_info = request.env[::Gitlab::Auth::AuthFinders::API_TOKEN_ENV] + return unless token_info + return unless TOKEN_SCOPES_TO_AUDIT.intersect?(token_info[:token_scopes]) + context = { name: 'api_request_access_with_scope', - author: current_user || ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'), - scope: current_user, + author: user, + scope: user, target: ::Gitlab::Audit::NullTarget.new, - message: "API request with token scope #{scope} - #{request.request_method} #{request.path}", + message: "API request with token scopes #{token_info[:token_scopes]} - #{request.request_method} #{request.path}", additional_details: { request: request.path, method: request.request_method, - token_scope: scope + token_scopes: token_info[:token_scopes] } } diff --git a/lib/api/scope.rb b/lib/api/scope.rb index 13b0ef8e89b356..62aefcceb4bae9 100644 --- a/lib/api/scope.rb +++ b/lib/api/scope.rb @@ -14,10 +14,7 @@ def initialize(name, options = {}) # Are the `scopes` passed in sufficient to adequately authorize the passed # request for the scope represented by the current instance of this class? def sufficient?(scopes, request) - sufficient = scopes.include?(self.name) && verify_if_condition(request) - Current.token_scope = name if sufficient - - sufficient + scopes.include?(self.name) && verify_if_condition(request) end private diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb index 7288245fc0a992..0ad4845a768c7c 100644 --- a/lib/gitlab/auth/auth_finders.rb +++ b/lib/gitlab/auth/auth_finders.rb @@ -221,7 +221,11 @@ def authentication_token_present? private def save_current_token_in_env - request.env[API_TOKEN_ENV] = { token_id: access_token.id, token_type: access_token.class.to_s } + request.env[API_TOKEN_ENV] = { + token_id: access_token.id, + token_type: access_token.class.to_s, + token_scopes: access_token.scopes.map(&:to_sym) + } end def save_auth_failure_in_application_context(access_token, cause) diff --git a/spec/requests/api/api_guard_spec.rb b/spec/requests/api/api_guard_spec.rb deleted file mode 100644 index e628afdca2eefb..00000000000000 --- a/spec/requests/api/api_guard_spec.rb +++ /dev/null @@ -1,70 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe API::APIGuard, feature_category: :shared do - let_it_be(:user) { create(:user) } - let_it_be(:token) { create(:oauth_access_token, user: user, scopes: [:ai_workflows]) } - let_it_be(:project) { create(:project) } - let_it_be(:issue) { create(:issue, project: project) } - let_it_be(:path) { "/projects/#{issue.project.id}/issues/#{issue.iid}" } - - before_all do - project.add_developer(user) - end - - shared_examples 'audited request' do - it 'adds audit log' do - expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including({ - name: 'api_request_access_with_scope', - message: "API request with token scope ai_workflows - GET /api/v4#{path}" - })).and_call_original - - subject - - expect(response).to have_gitlab_http_status(status) - end - end - - shared_examples 'not audited request' do - it "doesn't adds audit log" do - expect(::Gitlab::Audit::Auditor).not_to receive(:audit) - - subject - - expect(response).to have_gitlab_http_status(status) - end - end - - context 'when endpoint allows token with ai_workflow scope' do - subject { get api(path, oauth_access_token: token) } - - context 'when token with ai_workflows scope is used' do - let(:status) { :ok } - - it_behaves_like 'audited request' - - context 'when request fails' do - let_it_be(:path) { "/projects/#{issue.project.id}/issues/#{non_existing_record_id}" } - let(:status) { :not_found } - - it_behaves_like 'audited request' - end - end - - context 'when token with ai_workflows scope is not used' do - let_it_be(:token) { create(:oauth_access_token, user: user, scopes: [:api]) } - let(:status) { :ok } - - it_behaves_like 'not audited request' - end - end - - context "when endpoint doesn't allow token with ai_workflow scope" do - subject { delete api(path, oauth_access_token: token) } - - let(:status) { :forbidden } - - it_behaves_like 'not audited request' - end -end diff --git a/spec/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb index f33c2e93bd3ce7..c9ca80e9cb548e 100644 --- a/spec/requests/api/api_spec.rb +++ b/spec/requests/api/api_spec.rb @@ -500,4 +500,71 @@ expect(response).to have_gitlab_http_status(:bad_request) end end + + describe 'audit logging of requests with a specific token scope' do + let_it_be(:user) { create(:user) } + let_it_be(:token) { create(:oauth_access_token, user: user, scopes: [:ai_workflows]) } + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:path) { "/projects/#{issue.project.id}/issues/#{issue.iid}" } + + before_all do + project.add_developer(user) + end + + shared_examples 'audited request' do + it 'adds audit log' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including({ + name: 'api_request_access_with_scope', + message: "API request with token scopes [:ai_workflows] - GET /api/v4#{path}" + })).and_call_original + + subject + + expect(response).to have_gitlab_http_status(status) + end + end + + shared_examples 'not audited request' do + it "doesn't add audit log" do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + subject + + expect(response).to have_gitlab_http_status(status) + end + end + + context 'when endpoint allows token with ai_workflow scope' do + subject { get api(path, oauth_access_token: token) } + + context 'when token with ai_workflows scope is used' do + let(:status) { :ok } + + it_behaves_like 'audited request' + + context 'when request fails' do + let_it_be(:path) { "/projects/#{issue.project.id}/issues/#{non_existing_record_id}" } + let(:status) { :not_found } + + it_behaves_like 'audited request' + end + end + + context 'when token with ai_workflows scope is not used' do + let_it_be(:token) { create(:oauth_access_token, user: user, scopes: [:api]) } + let(:status) { :ok } + + it_behaves_like 'not audited request' + end + end + + context "when endpoint doesn't allow token with ai_workflow scope" do + subject { delete api(path, oauth_access_token: token) } + + let(:status) { :forbidden } + + it_behaves_like 'not audited request' + end + end end -- GitLab From 5edb60dd1dd73d45d00d62cbdf38a0cc107b15b2 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Wed, 20 Nov 2024 10:04:43 +0100 Subject: [PATCH 05/10] Address review feedback --- config/audit_events/types/api_request_access_with_scope.yml | 4 ++-- doc/user/compliance/audit_event_types.md | 2 +- doc/user/duo_workflow/index.md | 6 +++++- lib/api/helpers.rb | 5 ++++- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/config/audit_events/types/api_request_access_with_scope.yml b/config/audit_events/types/api_request_access_with_scope.yml index 62fc2eb4110928..015a60fd8987e5 100644 --- a/config/audit_events/types/api_request_access_with_scope.yml +++ b/config/audit_events/types/api_request_access_with_scope.yml @@ -1,10 +1,10 @@ --- name: api_request_access_with_scope -description: A susbset of API requests authenticated by token with ai_workflows scope +description: A susbset of API requests authenticated by a token with an audited scope introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/499461 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/172548 feature_category: duo_workflow -milestone: '17.6' +milestone: '17.7' saved_to_database: true scope: [User] streamed: true diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 6a4e36a266e961..c52c54150f272f 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -261,7 +261,7 @@ Audit event types belong to the following product categories. | Name | Description | Saved to database | Streamed | Introduced in | Scope | |:------------|:------------|:------------------|:---------|:--------------|:--------------| -| [`api_request_access_with_scope`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/172548) | A susbset of API requests authenticated by token with ai_workflows scope | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.6](https://gitlab.com/gitlab-org/gitlab/-/issues/499461) | User | +| [`api_request_access_with_scope`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/172548) | A susbset of API requests authenticated by a token with an audited scope | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.7](https://gitlab.com/gitlab-org/gitlab/-/issues/499461) | User | ### Dynamic application security testing diff --git a/doc/user/duo_workflow/index.md b/doc/user/duo_workflow/index.md index 74cc48764154ff..c997778703f5b6 100644 --- a/doc/user/duo_workflow/index.md +++ b/doc/user/duo_workflow/index.md @@ -77,7 +77,7 @@ If you have [Docker Desktop](https://handbook.gitlab.com/handbook/tools-and-tips or a container manager other than Colima installed already: 1. Pull the base Docker image: - + ```shell docker pull registry.gitlab.com/gitlab-org/duo-workflow/default-docker-image/workflow-generic-image:v0.0.4 ``` @@ -160,6 +160,10 @@ If you encounter issues: 1. Search for the setting **GitLab: Debug** and enable it. 1. Examine the [Duo Workflow Service production LangSmith trace](https://smith.langchain.com/o/477de7ad-583e-47b6-a1c4-c4a0300e7aca/projects/p/5409132b-2cf3-4df8-9f14-70204f90ed9b?timeModel=%7B%22duration%22%3A%227d%22%7D&tab=0). +## Audit log + +Audit event is created for each API request done by Duo Workflow. You can view these events on [instance audit events](../../administration/audit_event_reports.md#instance-audit-events) page. + ## Give feedback Duo Workflow is an experiment and your feedback is crucial. To report issues or suggest improvements, diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 306aa092cb1cf5..22e1683530467f 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -18,6 +18,9 @@ module Helpers API_EXCEPTION_ENV = 'gitlab.api.exception' API_RESPONSE_STATUS_CODE = 'gitlab.api.response_status_code' INTEGER_ID_REGEX = /^-?\d+$/ + + # ai_workflows scope is used by Duo Workflow which is an AI automation tool, requests authenticated by token with + # this scope are audited to keep track of all actions done by Duo Workflow. TOKEN_SCOPES_TO_AUDIT = [:ai_workflows].freeze def logger @@ -827,7 +830,7 @@ def audit_request_with_token_scope(user) name: 'api_request_access_with_scope', author: user, scope: user, - target: ::Gitlab::Audit::NullTarget.new, + target: access_token, message: "API request with token scopes #{token_info[:token_scopes]} - #{request.request_method} #{request.path}", additional_details: { request: request.path, -- GitLab From 28809468560fc4ca7fd56b8606366f02f9b7da54 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Fri, 22 Nov 2024 08:09:37 +0100 Subject: [PATCH 06/10] Change target back to null --- lib/api/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 22e1683530467f..b51a3095a9e76c 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -830,7 +830,7 @@ def audit_request_with_token_scope(user) name: 'api_request_access_with_scope', author: user, scope: user, - target: access_token, + target: ::Gitlab::Audit::NullTarget.new, message: "API request with token scopes #{token_info[:token_scopes]} - #{request.request_method} #{request.path}", additional_details: { request: request.path, -- GitLab From 3b6f51a46ca96700d69ed5d132c9eada677e919a Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Fri, 22 Nov 2024 12:22:36 +0000 Subject: [PATCH 07/10] Apply 2 suggestion(s) to 2 file(s) Co-authored-by: Fiona Neill --- doc/user/compliance/audit_event_types.md | 2 +- doc/user/duo_workflow/index.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index c52c54150f272f..e74cb4bd7aca4a 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -257,7 +257,7 @@ Audit event types belong to the following product categories. | [`cluster_agent_token_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036) | Triggered when a user creates a cluster agent token | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.10](https://gitlab.com/gitlab-org/gitlab/-/issues/382133) | Project | | [`cluster_agent_token_revoked`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036) | Triggered when a user revokes a cluster agent token | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.10](https://gitlab.com/gitlab-org/gitlab/-/issues/382133) | Project | -### Duo workflow +### GitLab Duo Workflow | Name | Description | Saved to database | Streamed | Introduced in | Scope | |:------------|:------------|:------------------|:---------|:--------------|:--------------| diff --git a/doc/user/duo_workflow/index.md b/doc/user/duo_workflow/index.md index c997778703f5b6..7d46e02a6cea9f 100644 --- a/doc/user/duo_workflow/index.md +++ b/doc/user/duo_workflow/index.md @@ -162,7 +162,7 @@ If you encounter issues: ## Audit log -Audit event is created for each API request done by Duo Workflow. You can view these events on [instance audit events](../../administration/audit_event_reports.md#instance-audit-events) page. +Audit event is created for each API request done by Duo Workflow. View these events on the [instance audit events](../../administration/audit_event_reports.md#instance-audit-events) page. ## Give feedback -- GitLab From b447fcf3a545ec03d1a4ba7b277697eaeac97f92 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Fri, 22 Nov 2024 14:36:05 +0100 Subject: [PATCH 08/10] Updated graphql doc --- doc/user/compliance/audit_event_types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index e74cb4bd7aca4a..c52c54150f272f 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -257,7 +257,7 @@ Audit event types belong to the following product categories. | [`cluster_agent_token_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036) | Triggered when a user creates a cluster agent token | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.10](https://gitlab.com/gitlab-org/gitlab/-/issues/382133) | Project | | [`cluster_agent_token_revoked`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112036) | Triggered when a user revokes a cluster agent token | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.10](https://gitlab.com/gitlab-org/gitlab/-/issues/382133) | Project | -### GitLab Duo Workflow +### Duo workflow | Name | Description | Saved to database | Streamed | Introduced in | Scope | |:------------|:------------|:------------------|:---------|:--------------|:--------------| -- GitLab From 5697a3d311a15e2a7c2ca3f639d507ae07982dd5 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Mon, 25 Nov 2024 08:50:36 +0100 Subject: [PATCH 09/10] Add a feature flag --- .../gitlab_com_derisk/api_audit_requests_with_scope.yml | 9 +++++++++ lib/api/helpers.rb | 4 +++- spec/requests/api/api_spec.rb | 8 ++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 config/feature_flags/gitlab_com_derisk/api_audit_requests_with_scope.yml diff --git a/config/feature_flags/gitlab_com_derisk/api_audit_requests_with_scope.yml b/config/feature_flags/gitlab_com_derisk/api_audit_requests_with_scope.yml new file mode 100644 index 00000000000000..20f059870cbc03 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/api_audit_requests_with_scope.yml @@ -0,0 +1,9 @@ +--- +name: api_audit_requests_with_scope +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/499461 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/172548 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/505974 +milestone: '17.6' +group: group::duo workflow +type: gitlab_com_derisk +default_enabled: false diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index b51a3095a9e76c..72c9e3f77cf71b 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -822,9 +822,11 @@ def validate_search_rate_limit! end def audit_request_with_token_scope(user) + return unless Feature.enabled?(:api_audit_requests_with_scope, user) + token_info = request.env[::Gitlab::Auth::AuthFinders::API_TOKEN_ENV] return unless token_info - return unless TOKEN_SCOPES_TO_AUDIT.intersect?(token_info[:token_scopes]) + return unless TOKEN_SCOPES_TO_AUDIT.intersect?(Array.wrap(token_info[:token_scopes])) context = { name: 'api_request_access_with_scope', diff --git a/spec/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb index c9ca80e9cb548e..757cbf88989cee 100644 --- a/spec/requests/api/api_spec.rb +++ b/spec/requests/api/api_spec.rb @@ -549,6 +549,14 @@ it_behaves_like 'audited request' end + + context 'when api_audit_requests_with_scope flag is disabled' do + before do + stub_feature_flags(api_audit_requests_with_scope: false) + end + + it_behaves_like 'not audited request' + end end context 'when token with ai_workflows scope is not used' do -- GitLab From 0e9b48f97070a587b27589907f9adff0b04c0543 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Tue, 26 Nov 2024 09:42:19 +0100 Subject: [PATCH 10/10] Fixed flag mielstone --- .../gitlab_com_derisk/api_audit_requests_with_scope.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/gitlab_com_derisk/api_audit_requests_with_scope.yml b/config/feature_flags/gitlab_com_derisk/api_audit_requests_with_scope.yml index 20f059870cbc03..fd822e78f8f9d9 100644 --- a/config/feature_flags/gitlab_com_derisk/api_audit_requests_with_scope.yml +++ b/config/feature_flags/gitlab_com_derisk/api_audit_requests_with_scope.yml @@ -3,7 +3,7 @@ name: api_audit_requests_with_scope feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/499461 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/172548 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/505974 -milestone: '17.6' +milestone: '17.7' group: group::duo workflow type: gitlab_com_derisk default_enabled: false -- GitLab