diff --git a/.gitleaksignore b/.gitleaksignore index 8d44c5eccc1d1be0353568efc2cf9a15fe6d1905..306bed4adbdf59d13d6e2a55c0bfa76daea8300f 100644 --- a/.gitleaksignore +++ b/.gitleaksignore @@ -10,4 +10,5 @@ spec/frontend/lib/utils/secret_detection_spec.js:generic-api-key:40 spec/frontend/lib/utils/secret_detection_spec.js:generic-api-key:41 spec/frontend/lib/utils/secret_detection_spec.js:generic-api-key:55 spec/frontend/lib/utils/secret_detection_spec.js:generic-api-key:57 -spec/frontend/lib/utils/secret_detection_spec.js:gitlab-pat:28 \ No newline at end of file +spec/frontend/lib/utils/secret_detection_spec.js:gitlab-pat:28 +d39da82ae304b8813a8fb2c79c3d7cd6f173590e:doc/api/groups.md:gitlab-pat:2342 \ No newline at end of file diff --git a/app/services/groups/agnostic_token_revocation_service.rb b/app/services/groups/agnostic_token_revocation_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..e0992452a113c8dbc6014c625e503b7ac8ead763 --- /dev/null +++ b/app/services/groups/agnostic_token_revocation_service.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +# This Service takes an authentication token of multiple types, and will +# call a RevokeService for it if the token has access to the group or +# any of the group's descendants. +# +# If the token provided has access to the group and is revoked, it will +# be returned by the service with a :success status. +# If the token type is not supported, if the token doesn't have access +# to the group, or if any error occurs, a generic :failure status is +# returned. +# +# This Service does not create logs or Audit events. Those can be found +# at the API layer or in specific RevokeServices. +# +# This Service returns a ServiceResponse and will: +# - include the token object at payload[:token] +# - the token's class at payload[:type] +module Groups # rubocop:disable Gitlab/BoundedContexts -- This service is strictly related to groups + class AgnosticTokenRevocationService < Groups::BaseService + AUDIT_SOURCE = :group_token_revocation_service + + attr_reader :token + + def initialize(group, current_user, plaintext) + @group = group + @current_user = current_user + @plaintext = plaintext.to_s + end + + def execute + return error("Feature not enabled") unless Feature.enabled?(:group_agnostic_token_revocation, group) + return error("Group cannot be a subgroup") if group.subgroup? + return error("Unauthorized") unless can?(current_user, :admin_group, group) + + # Determine the type of token + if plaintext.start_with?(Gitlab::CurrentSettings.current_application_settings.personal_access_token_prefix, + 'glpat-') + @token = PersonalAccessToken.find_by_token(plaintext) + return error('PAT not found') unless token + + handle_personal_access_token + elsif plaintext.start_with?(DeployToken::DEPLOY_TOKEN_PREFIX) + @token = DeployToken.find_by_token(plaintext) + return error('DeployToken not found') unless token && token.group_type? + + handle_deploy_token + else + error('Unsupported token type') + end + end + + private + + attr_reader :plaintext, :group, :current_user + + def success(token, type) + ServiceResponse.success( + message: "#{type} is revoked", + payload: { + token: token, + type: type + } + ) + end + + def error(message) + ServiceResponse.error(message: message) + end + + def handle_personal_access_token + if user_has_group_membership? + # Only revoke active tokens. (Ignore expired tokens) + if token.active? + ::PersonalAccessTokens::RevokeService.new( + current_user, + token: token, + source: AUDIT_SOURCE + ).execute + end + + # Always validate that, if we're returning token info, it + # has been successfully revoked + return success(token, 'PersonalAccessToken') if token.reset.revoked? + end + + # If we get here the token exists but either: + # - didn't belong to the group or descendants + # - did, but was already expired + # - does and is active, but revocation failed for some reason + error('PAT revocation failed') + end + + # Validate whether the user has access to a group or any of its + # descendants. Includes membership that might not be active, but + # could be later, e.g. bans. Includes membership of non-human + # users. + def user_has_group_membership? + ::GroupMember + .with_user(token.user) + .with_source_id(group.self_and_descendants) + .any? || + ::ProjectMember + .with_user(token.user) + .in_namespaces(group.self_and_descendants) + .any? + end + + def handle_deploy_token + if group.self_and_descendants.include?(token.group) + if token.active? + service = ::Groups::DeployTokens::RevokeService.new( + token.group, + current_user, + { id: token.id } + ) + + service.source = AUDIT_SOURCE + service.execute + end + + return success(token, 'DeployToken') if token.reset.revoked? + end + + error('DeployToken revocation failed') + end + end +end diff --git a/app/services/groups/deploy_tokens/revoke_service.rb b/app/services/groups/deploy_tokens/revoke_service.rb index cf91d3b27fad6f6299f39fa5c6934019b98e28a0..0aa88f6190d3ff96b0c67df1c2c823f73a0b164b 100644 --- a/app/services/groups/deploy_tokens/revoke_service.rb +++ b/app/services/groups/deploy_tokens/revoke_service.rb @@ -3,7 +3,7 @@ module Groups module DeployTokens class RevokeService < BaseService - attr_accessor :token + attr_accessor :token, :source def execute @token = group.deploy_tokens.find(params[:id]) diff --git a/app/services/personal_access_tokens/revoke_service.rb b/app/services/personal_access_tokens/revoke_service.rb index 5df0f5325b0c0355e269aee2be83c70dfe8447a3..897a2b12caa7f41961459016f82cf099e2d057fc 100644 --- a/app/services/personal_access_tokens/revoke_service.rb +++ b/app/services/personal_access_tokens/revoke_service.rb @@ -4,7 +4,7 @@ module PersonalAccessTokens class RevokeService < BaseService attr_reader :token, :current_user, :group, :source - VALID_SOURCES = %i[self secret_detection].freeze + VALID_SOURCES = %i[self secret_detection group_token_revocation_service].freeze def initialize(current_user = nil, token: nil, group: nil, source: nil) @current_user = current_user @@ -43,7 +43,7 @@ def revocation_permitted? case source when :self Ability.allowed?(current_user, :revoke_token, token) - when :secret_detection + when :secret_detection, :group_token_revocation_service true else false @@ -54,10 +54,16 @@ def log_event Gitlab::AppLogger.info( class: self.class.name, message: "PAT Revoked", - revoked_by: current_user&.username || source, + revoked_by: revoked_by, revoked_for: token.user.username, token_id: token.id) end + + def revoked_by + return current_user&.username if source == :self + + source + end end end diff --git a/config/feature_flags/beta/group_agnostic_token_revocation.yml b/config/feature_flags/beta/group_agnostic_token_revocation.yml new file mode 100644 index 0000000000000000000000000000000000000000..4ec5c40785f5599b72e3698b345cba9872ecfde7 --- /dev/null +++ b/config/feature_flags/beta/group_agnostic_token_revocation.yml @@ -0,0 +1,9 @@ +--- +name: group_agnostic_token_revocation +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/460777 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/153376 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/463157 +milestone: '17.2' +group: group::authentication +type: beta +default_enabled: false diff --git a/doc/api/groups.md b/doc/api/groups.md index 75c62b783e7d30097c4a402ae49887ec6d47e2d8..170e2f41d91f806a83e003c4b8b14e54623e39e3 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -2427,3 +2427,69 @@ DELETE /groups/:id/push_rule | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | | `id` | integer/string | yes | The ID or [URL-encoded path of the group](rest/index.md#namespaced-path-encoding) | + +## Revoke Token + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/371117) in GitLab 17.2 [with a flag](../administration/feature_flags.md) named `group_agnostic_token_revocation`. Disabled by default. + +FLAG: +The availability of this feature is controlled by a feature flag. +For more information, see the history. + +Revoke a token, if it has access to the group or any of its subgroups +and projects. If the token is revoked, or was already revoked, its +details are returned in the response. + +The following criteria must be met: + +- The group must be a top-level group. +- You must have the Owner role in the group. +- The token type is one of: + - Personal Access Token + - Group Access Token + - Project Access Token + - Group Deploy Token + +Additional token types may be supported at a later date. + +```plaintext +POST /groups/:id/tokens/revoke +``` + +| Attribute | Type | Required | Description | +|-------------|----------------|------------------------|-------------| +| `id` | integer or string | Yes | The ID or [URL-encoded path of the group](rest/index.md#namespaced-path-encoding). | +| `token` | string | Yes | The plaintext token. | + +If successful, returns [`200 OK`](rest/index.md#status-codes) and +a JSON representation of the token. The attributes returned will vary by +token type. + +Example request: + +```shell +curl --request POST \ + --header "PRIVATE-TOKEN: <your_access_token>" \ + --header "Content-Type: application/json" \ + --data '{"token":"glpat-1234567890abcdefghij"}' \ + --url "https://gitlab.example.com/api/v4/groups/63/tokens/revoke" +``` + +Example response: + +```json +{ + "id": 9, + "name": "my-subgroup-deploytoken", + "username": "gitlab+deploy-token-9", + "expires_at": null, + "scopes": + [ + "read_repository", + "read_package_registry", + "write_package_registry" + ], + "revoked": true, + "expired": false +} +``` diff --git a/ee/app/services/ee/groups/deploy_tokens/revoke_service.rb b/ee/app/services/ee/groups/deploy_tokens/revoke_service.rb index b3cf6ca0745fa75f55cda0dfe3bc4d5eb6f83e64..e9a494b5c582002f2c10f3a111306e00001edbb3 100644 --- a/ee/app/services/ee/groups/deploy_tokens/revoke_service.rb +++ b/ee/app/services/ee/groups/deploy_tokens/revoke_service.rb @@ -23,7 +23,8 @@ def log_audit_event target: token, message: message, additional_details: { - action: :custom + action: :custom, + revocation_source: source } } ::Gitlab::Audit::Auditor.audit(audit_context) diff --git a/ee/spec/services/ee/groups/deploy_tokens/revoke_service_spec.rb b/ee/spec/services/ee/groups/deploy_tokens/revoke_service_spec.rb index 17fb18eae6bfbbb05581a479a36bd4addf6314c1..0925f97ce8bd3f02bda2e30852d44262c0d5f091 100644 --- a/ee/spec/services/ee/groups/deploy_tokens/revoke_service_spec.rb +++ b/ee/spec/services/ee/groups/deploy_tokens/revoke_service_spec.rb @@ -10,21 +10,27 @@ let_it_be(:deploy_token_params) { { id: deploy_token.id } } describe '#execute' do - subject { described_class.new(entity, user, deploy_token_params).execute } + let(:revoke_service) { described_class.new(entity, user, deploy_token_params) } + + subject(:revoke) { revoke_service.execute } before do stub_licensed_features(external_audit_events: true) end it "creates an audit event" do - expect { subject }.to change { AuditEvent.count }.by(1) + expect { revoke }.to change { AuditEvent.count }.by(1) expected_message = <<~MESSAGE.squish Revoked group deploy token with name: #{deploy_token.name} with token_id: #{deploy_token.id} with scopes: #{deploy_token.scopes}. MESSAGE - expect(AuditEvent.last.details[:custom_message]).to eq(expected_message) + details = AuditEvent.last.details + + expect(details[:custom_message]).to eq(expected_message) + expect(details[:action]).to eq(:custom) + expect(details[:revocation_source]).to be_nil end it_behaves_like 'sends correct event type in audit event stream' do @@ -37,7 +43,7 @@ let_it_be(:deploy_token) { create(:deploy_token, :group, groups: [group]) } let_it_be(:deploy_token_params) { { id: deploy_token.id } } - subject { described_class.new(group, user, deploy_token_params).execute } + let(:revoke_service) { described_class.new(group, user, deploy_token_params) } before do group.add_owner(user) @@ -45,5 +51,15 @@ include_examples 'sends streaming audit event' end + + context 'when source is set' do + it 'includes source in audit event' do + revoke_service.source = :group_token_revocation_service + + revoke + + expect(AuditEvent.last.details[:revocation_source]).to eq(:group_token_revocation_service) + end + end end end diff --git a/lib/api/groups.rb b/lib/api/groups.rb index f5c54d36d189cb17a2e6736bffc5ebae90d2e76f..88c55a0d5ab19fd0ad2245efa42369217422a949 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -580,6 +580,48 @@ def check_subscription!(group) no_content! end # rubocop: enable CodeReuse/ActiveRecord + + desc 'Revoke a single token' do + detail <<-DETAIL +Revoke a token, if it has access to the group or any of its subgroups +and projects. If the token is revoked, or was already revoked, its +details are returned in the response. + +The following criteria must be met: + +- The group must be a top-level group. +- You must have Owner permission in the group. +- The token type is one of: + - Personal Access Token + - Group Access Token + - Project Access Token + - Group Deploy Token + +This feature is gated by the :group_agnostic_token_revocation feature flag. + DETAIL + end + params do + requires :id, type: String, desc: 'The ID of a top-level group' + requires :token, type: String, desc: 'The token to revoke' + end + post ":id/tokens/revoke", urgency: :low, feature_category: :groups_and_projects do + group = find_group!(params[:id]) + not_found! unless Feature.enabled?(:group_agnostic_token_revocation, group) + bad_request!('Must be a top-level group') if group.subgroup? + authorize! :admin_group, group + + result = ::Groups::AgnosticTokenRevocationService.new(group, current_user, params[:token]).execute + + if result.success? + status :ok + present result.payload[:token], with: "API::Entities::#{result.payload[:type]}".constantize + else + # No matter the error, we always return a 422. + # This prevents disclosing cases like: token is invalid, + # or token is valid but in a different group. + unprocessable_entity! + end + end end end end diff --git a/spec/factories/deploy_tokens.rb b/spec/factories/deploy_tokens.rb index 379178f1a41b234f98839ff5c9ed24bdd08fa9e4..f8366d43b05de700dfe8e4e0b157a1fba2ef6277 100644 --- a/spec/factories/deploy_tokens.rb +++ b/spec/factories/deploy_tokens.rb @@ -2,7 +2,6 @@ FactoryBot.define do factory :deploy_token do - token_encrypted { Gitlab::CryptoHelper.aes256_gcm_encrypt(SecureRandom.hex(50)) } sequence(:name) { |n| "PDT #{n}" } read_repository { true } read_registry { true } diff --git a/spec/factories/group_deploy_tokens.rb b/spec/factories/group_deploy_tokens.rb index 9ec7d0701be3a9daeaadfa4772b9b830eff6eef0..fb070bf6267d63449b148a8325b85999f367b6e0 100644 --- a/spec/factories/group_deploy_tokens.rb +++ b/spec/factories/group_deploy_tokens.rb @@ -3,6 +3,6 @@ FactoryBot.define do factory :group_deploy_token do group - deploy_token + association :deploy_token, factory: [:deploy_token, :group] end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index f957ca0a6bfe28bf77d6246881c889647c5205ce..86dbd91eac0bbad6f5453f15286ca7712aad77af 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -3150,4 +3150,117 @@ def make_request(user) end end end + + describe 'POST groups/:id/tokens/revoke' do + let(:token) { 'glprefix-AABBCCDDEE1122334455' } + let(:service_response) { ServiceResponse.error(message: '') } + let(:service) { instance_double(service_class, execute: service_response) } + let(:service_class) { Groups::AgnosticTokenRevocationService } + let_it_be(:group) { create(:group, :with_hierarchy, children: 1) } + + let(:path) { "/groups/#{group.id}/tokens/revoke" } + + before do + allow(service_class).to receive(:new).and_return(service) + end + + shared_examples 'revoking token fails' do |status, message| + it 'cannot revoke token' do + revoke_token + + expect(response).to have_gitlab_http_status(status) + expect(json_response['message'] || json_response['error']).to include(message) + end + end + + context 'when not a group owner' do + subject(:revoke_token) { post api(path, user1), params: { token: token } } + + before do + group.add_maintainer(user1) + end + + it_behaves_like 'revoking token fails', :forbidden, 'Forbidden' + end + + context 'when authenticated as a group owner' do + subject(:revoke_token) { post api(path, user1), params: { token: token } } + + before do + group.add_owner(user1) + end + + context 'when group is a top level group' do + it 'calls revocation service' do + revoke_token + expect(service_class).to have_received(:new).with(group, user1, token) + end + + context 'when the service returns successfully' do + let(:token) { create(:personal_access_token, :revoked) } + let(:service_response) do + ServiceResponse.success( + message: 'PersonalAccessToken is revoked', + payload: { + token: token, + type: 'PersonalAccessToken' + } + ) + end + + it 'renders the token with a presenter', :aggregate_failures do + revoke_token + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.with_indifferent_access).to include(revoked: true, id: token.id) + expect(json_response.keys).not_to include(%w[token token_digest]) + end + end + + context 'when the service returns unsuccessfully' do + let(:service_response) do + ServiceResponse.error( + message: 'Some error' + ) + end + + it_behaves_like 'revoking token fails', :unprocessable_entity, 'Unprocessable Entity' + end + + context 'when ff disabled' do + before do + Feature.disable(:group_agnostic_token_revocation, group) + end + + it_behaves_like 'revoking token fails', :not_found, 'Not Found' + + it 'does not call revocation service' do + revoke_token + expect(service_class).not_to have_received(:new) + end + end + end + + context 'when group does not exist' do + let(:path) { "/groups/0/tokens/revoke" } + + it_behaves_like 'revoking token fails', :not_found, 'Group Not Found' + + it 'does not call revocation service' do + revoke_token + expect(service_class).not_to have_received(:new) + end + end + + context 'when group is a subgroup' do + let(:path) { "/groups/#{group.children.first.id}/tokens/revoke" } + + it_behaves_like 'revoking token fails', :bad_request, 'Must be a top-level' + + it 'does not call revocation service' do + revoke_token + expect(service_class).not_to have_received(:new) + end + end + end + end end diff --git a/spec/services/groups/agnostic_token_revocation_service_spec.rb b/spec/services/groups/agnostic_token_revocation_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..7dcbf747f01815085088ca7bdcd02dfe0605049f --- /dev/null +++ b/spec/services/groups/agnostic_token_revocation_service_spec.rb @@ -0,0 +1,202 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::AgnosticTokenRevocationService, feature_category: :system_access do + let_it_be(:group) { create(:group) } + let_it_be(:owner) { create(:user) } + let_it_be(:maintainer) { create(:user) } + let_it_be(:user) { create(:user) } + + before_all do + group.add_owner(owner) + group.add_maintainer(maintainer) + end + + describe '#initialize' do + subject(:result) { described_class.new(group, owner, 'plaintext') } + + it 'accepts a group, user, and plaintext' do + expect(result).to be_a(described_class) + end + end + + shared_examples_for 'a successfully revoked token' do + it { expect(result.success?).to be(true), result.message } + + it 'revokes the token' do + result + expect(token.reload).to be_revoked + end + + it 'returns the token in the payload' do + result + expect(result.payload[:token]).to eq(token) + end + + it 'returns the token class in the payload' do + result + expect(result.payload[:type]).to be(type) + end + end + + shared_examples_for 'an unsuccessfully revoked token' do + it { expect(result.success?).to be(false) } + + it 'does not revoke the token' do + result + expect(token.reload.revoked?).to be(false) + end + end + + describe '#execute' do + subject(:result) { service.execute } + + let_it_be(:member) { create(:user, guest_of: group) } + + let(:service) { described_class.new(group, owner, token.token) } + + context 'with a personal access token' do + let(:type) { 'PersonalAccessToken' } + + context 'when it can access the group' do + let_it_be(:token) { create(:personal_access_token, user: member) } + + it_behaves_like 'a successfully revoked token' + end + + context 'when it can access a sub group' do + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:member) { create(:user, guest_of: group) } + let_it_be(:token) { create(:personal_access_token, user: member) } + + it_behaves_like 'a successfully revoked token' + end + + context 'when it can access a group\'s project' do + let_it_be(:project) { create(:project, group: group) } + let_it_be(:member) { create(:user, :project_bot, guest_of: project) } + let_it_be(:token) { create(:personal_access_token, user: member) } + + it_behaves_like 'a successfully revoked token' + end + + context 'when it belongs to a member with no relation to the group' do + let_it_be(:token) { create(:personal_access_token, user: user) } + + it_behaves_like 'an unsuccessfully revoked token' + end + + context 'when it belongs to a member of multiple groups' do + let_it_be(:group_b) { create(:group) } + let_it_be(:member) { create(:user, guest_of: [group, group_b]) } + let_it_be(:token) { create(:personal_access_token, user: member) } + + it_behaves_like 'a successfully revoked token' + end + + context 'with an already revoked personal access token that can access the group' do + let(:token) { create(:personal_access_token, user: member, revoked: true) } + + it_behaves_like 'a successfully revoked token' + end + + context 'with an already expired token' do + let(:token) { create(:personal_access_token, :expired, user: member) } + + it_behaves_like 'an unsuccessfully revoked token' + end + + context 'with an already expired and revoked token' do + let(:token) { create(:personal_access_token, :expired, revoked: true, user: member) } + + it_behaves_like 'a successfully revoked token' + end + end + + context 'with a group deploy token' do + let(:type) { 'DeployToken' } + + let_it_be(:subgroup) { create(:group, parent: group) } + + context 'when it can access the group' do + let_it_be(:token) { create(:group_deploy_token, group: group).deploy_token } + + it_behaves_like 'a successfully revoked token' + end + + context 'when it can access a subgroup' do + let_it_be(:token) { create(:group_deploy_token, group: subgroup).deploy_token } + + it_behaves_like 'a successfully revoked token' + end + + context 'when it belongs to another group' do + let_it_be(:token) { create(:group_deploy_token).deploy_token } + + it_behaves_like 'an unsuccessfully revoked token' + end + + context 'when it belongs to a project' do + let_it_be(:token) { create(:project_deploy_token).deploy_token } + + it_behaves_like 'an unsuccessfully revoked token' + end + end + + context 'with a token that would otherwise be revoked' do + let_it_be(:token) { create(:personal_access_token, user: member) } + + context 'when ff disabled for group' do + before do + Feature.disable(:group_agnostic_token_revocation, group) + end + + it_behaves_like 'an unsuccessfully revoked token' + end + + context 'when group is a subgroup' do + let_it_be(:group) { create(:group, :nested) } + let_it_be(:member) { create(:user, guest_of: group) } + + before_all do + group.add_owner(owner) + end + + it_behaves_like 'an unsuccessfully revoked token' + end + + context 'when current_user is a maintainer' do + let(:service) { described_class.new(group, maintainer, token.token) } + + it_behaves_like 'an unsuccessfully revoked token' + end + + context 'when current_user is not a member' do + let(:service) { described_class.new(group, user, token.token) } + + it_behaves_like 'an unsuccessfully revoked token' + end + end + + context 'with an unsupported token type' do + let(:token) { create(:oauth_access_token) } + + it_behaves_like 'an unsuccessfully revoked token' + end + + context 'with a plaintext that does not exist' do + let(:plaintext) { 'glpat-abc123' } + let(:service) { described_class.new(group, owner, plaintext) } + + it { expect(result.success?).to be(false) } + end + + context 'with a nil plaintext' do + let(:plaintext) { nil } + let(:service) { described_class.new(group, owner, plaintext) } + + it { expect(result.success?).to be(false) } + end + end +end