diff --git a/app/assets/javascripts/access_tokens/components/access_token_table_app.vue b/app/assets/javascripts/access_tokens/components/access_token_table_app.vue index 52ac3f2a5ffd3b71fb656a85b514d9e9dffbf862..ff421eb3ec33a8fee2b392c8b05a3216df00f3be 100644 --- a/app/assets/javascripts/access_tokens/components/access_token_table_app.vue +++ b/app/assets/javascripts/access_tokens/components/access_token_table_app.vue @@ -12,6 +12,7 @@ import DomElementListener from '~/vue_shared/components/dom_element_listener.vue import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import HelpIcon from '~/vue_shared/components/help_icon/help_icon.vue'; import UserDate from '~/vue_shared/components/user_date.vue'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { EVENT_SUCCESS, FIELDS, INITIAL_PAGE, PAGE_SIZE } from './constants'; /** @@ -37,11 +38,12 @@ export default { directives: { GlTooltip: GlTooltipDirective, }, + mixins: [glFeatureFlagsMixin()], lastUsedHelpLink: helpPagePath('/user/profile/personal_access_tokens.md', { - anchor: 'view-the-last-time-a-token-was-used', + anchor: 'view-the-time-at-and-ips-where-a-token-was-last-used', }), i18n: { - emptyField: __('Never'), + emptyDateField: __('Never'), expired: __('Expired'), modalMessage: { revoke: s__( @@ -93,6 +95,10 @@ export default { ignoredFields.push('role'); } + if (!this.glFeatures.patIp) { + ignoredFields.push('lastUsedIps'); + } + const fields = FIELDS.filter(({ key }) => !ignoredFields.includes(key)); // Remove the sortability of the columns if backend pagination is on. @@ -218,7 +224,18 @@ export default { + + {{ - $options.i18n.emptyField + $options.i18n.emptyDateField }} diff --git a/app/assets/javascripts/access_tokens/components/constants.js b/app/assets/javascripts/access_tokens/components/constants.js index f131b3d6dabb61ff30aa378c55ab9b150581dcc5..fd4ae7a91afb9fc4d5d95a9b5770994737f74d6b 100644 --- a/app/assets/javascripts/access_tokens/components/constants.js +++ b/app/assets/javascripts/access_tokens/components/constants.js @@ -49,6 +49,14 @@ const ROLE_FIELD = { export const FIELDS = [ ...BASE_FIELDS, + { + formatter(ips) { + return ips?.length ? ips?.join(', ') : '-'; + }, + key: 'lastUsedIps', + label: __('Last Used IPs'), + sortable: false, + }, { key: 'expiresAt', label: __('Expires'), diff --git a/app/assets/javascripts/access_tokens/components/inactive_access_token_table_app.vue b/app/assets/javascripts/access_tokens/components/inactive_access_token_table_app.vue index 6a4d584f1f6c1c63ce00bb09d8078ca0a7f87857..b7b7ea2b3fb800aec327e5513b2e1f0c4169a7d3 100644 --- a/app/assets/javascripts/access_tokens/components/inactive_access_token_table_app.vue +++ b/app/assets/javascripts/access_tokens/components/inactive_access_token_table_app.vue @@ -23,7 +23,7 @@ export default { GlTooltip: GlTooltipDirective, }, lastUsedHelpLink: helpPagePath('/user/profile/personal_access_tokens.md', { - anchor: 'view-the-last-time-a-token-was-used', + anchor: 'view-the-time-at-and-ips-where-a-token-was-last-used', }), i18n: { emptyField: __('Never'), diff --git a/app/controllers/user_settings/personal_access_tokens_controller.rb b/app/controllers/user_settings/personal_access_tokens_controller.rb index d764e0ac7410b647672287b617d7ac8dce1e5da0..55579bc2d8c0c7c0a9720faecc4028336e945f01 100644 --- a/app/controllers/user_settings/personal_access_tokens_controller.rb +++ b/app/controllers/user_settings/personal_access_tokens_controller.rb @@ -8,6 +8,9 @@ class PersonalAccessTokensController < ApplicationController feature_category :system_access before_action :check_personal_access_tokens_enabled + before_action do + push_frontend_feature_flag(:pat_ip, current_user) + end prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:ics) } def index diff --git a/app/models/authn/personal_access_token_last_used_ip.rb b/app/models/authn/personal_access_token_last_used_ip.rb new file mode 100644 index 0000000000000000000000000000000000000000..d0324844fd18393ff9e180e5cfb33f7f134415e0 --- /dev/null +++ b/app/models/authn/personal_access_token_last_used_ip.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Authn + class PersonalAccessTokenLastUsedIp < ApplicationRecord + self.table_name = 'personal_access_token_last_used_ips' + belongs_to :personal_access_token + belongs_to :organization, class_name: 'Organizations::Organization' + end +end diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb index 16e1dbd9b305811fb8f51e0a278e5b695fad738f..7c0a9cf7052378b07fb3e0aa5386339f4f3da844 100644 --- a/app/models/personal_access_token.rb +++ b/app/models/personal_access_token.rb @@ -43,6 +43,8 @@ class PersonalAccessToken < ApplicationRecord belongs_to :organization, class_name: 'Organizations::Organization' belongs_to :previous_personal_access_token, class_name: 'PersonalAccessToken' + has_many :last_used_ips, class_name: 'Authn::PersonalAccessTokenLastUsedIp' + after_initialize :set_default_scopes, if: :persisted? before_save :ensure_token diff --git a/app/services/personal_access_tokens/last_used_service.rb b/app/services/personal_access_tokens/last_used_service.rb index 536d7fd90c1a4a724969c65611508602beae61cf..1c479b1db1f3ea08bd12a7aa8496e921367f9f41 100644 --- a/app/services/personal_access_tokens/last_used_service.rb +++ b/app/services/personal_access_tokens/last_used_service.rb @@ -5,6 +5,9 @@ class LastUsedService include ExclusiveLeaseGuard LEASE_TIMEOUT = 60.seconds.to_i + LAST_USED_IP_TIMEOUT = 1.minute + LAST_USED_AT_TIMEOUT = 10.minutes + NUM_IPS_TO_STORE = 5 def initialize(personal_access_token) @personal_access_token = personal_access_token @@ -16,12 +19,13 @@ def execute # We _only_ want to update last_used_at and not also updated_at (which # would be updated when using #touch). - return unless update? + return if ::Gitlab::Database.read_only? lb = @personal_access_token.load_balancer try_obtain_lease do ::Gitlab::Database::LoadBalancing::SessionMap.current(lb).without_sticky_writes do - @personal_access_token.update_column(:last_used_at, Time.zone.now) + update_pat_ip if last_used_ip_needs_update? + update_timestamp if last_used_at_needs_update? end end end @@ -36,14 +40,48 @@ def lease_key @lease_key ||= "pat:last_used_update_lock:#{@personal_access_token.id}" end - def update? - return false if ::Gitlab::Database.read_only? + def update_timestamp + @personal_access_token.update_columns(last_used_at: Time.zone.now) + end + + # rubocop:disable CodeReuse/ActiveRecord -- this is specific to this service + def update_pat_ip + @personal_access_token.last_used_ips << Authn::PersonalAccessTokenLastUsedIp.new( + organization: @personal_access_token.organization, + ip_address: Gitlab::IpAddressState.current) + + ip_count = @personal_access_token.last_used_ips.where( + personal_access_token_id: @personal_access_token.id).count + + return unless ip_count > NUM_IPS_TO_STORE + + @personal_access_token + .last_used_ips + .order(created_at: :asc) + .limit(ip_count - NUM_IPS_TO_STORE) + .delete_all + end + + def last_used_ip_needs_update? + return false unless Feature.enabled?(:pat_ip, @personal_access_token.user) + return false unless Gitlab::IpAddressState.current + return true if @personal_access_token.last_used_at.nil? + + return false if + Authn::PersonalAccessTokenLastUsedIp + .where(personal_access_token_id: @personal_access_token.id, ip_address: Gitlab::IpAddressState.current) + .exists? + + @personal_access_token.last_used_at <= LAST_USED_IP_TIMEOUT.ago + end + # rubocop:enable CodeReuse/ActiveRecord + def last_used_at_needs_update? last_used = @personal_access_token.last_used_at return true if last_used.nil? - last_used <= 10.minutes.ago + last_used <= LAST_USED_AT_TIMEOUT.ago end end end diff --git a/config/feature_flags/gitlab_com_derisk/pat_ip.yml b/config/feature_flags/gitlab_com_derisk/pat_ip.yml new file mode 100644 index 0000000000000000000000000000000000000000..bc1d46553818bf6b277c46fc5f958531b3f640e3 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/pat_ip.yml @@ -0,0 +1,9 @@ +--- +name: pat_ip +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/428577 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/161076 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/428577 +milestone: '17.8' +group: group::authentication +type: gitlab_com_derisk +default_enabled: false diff --git a/db/docs/personal_access_token_last_used_ips.yml b/db/docs/personal_access_token_last_used_ips.yml new file mode 100644 index 0000000000000000000000000000000000000000..6471bbc11e9ea2310f17a46e10af73cfbd939eec --- /dev/null +++ b/db/docs/personal_access_token_last_used_ips.yml @@ -0,0 +1,10 @@ +--- +table_name: personal_access_token_last_used_ips +feature_categories: + - system_access +description: Keeps the data for last used IP addresses for personal access tokens +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/161076 +milestone: '17.8' +gitlab_schema: gitlab_main_cell +sharding_key: + organization_id: organizations \ No newline at end of file diff --git a/db/migrate/20241112103518_add_personal_access_token_last_used_ips_table.rb b/db/migrate/20241112103518_add_personal_access_token_last_used_ips_table.rb new file mode 100644 index 0000000000000000000000000000000000000000..e9f59d5550681b83283c9d2245eefbfaebe2c4eb --- /dev/null +++ b/db/migrate/20241112103518_add_personal_access_token_last_used_ips_table.rb @@ -0,0 +1,23 @@ +# frozen_string_literal:true + +class AddPersonalAccessTokenLastUsedIpsTable < Gitlab::Database::Migration[2.2] + INDEX_NAME = 'idx_pat_last_used_ips_on_pat_id' + + milestone '17.8' + + def up + create_table :personal_access_token_last_used_ips do |t| + t.references :personal_access_token, + foreign_key: { on_delete: :cascade }, + index: { name: INDEX_NAME }, + null: false + t.references :organization, foreign_key: { on_delete: :cascade }, null: false + t.timestamps_with_timezone + t.inet :ip_address + end + end + + def down + drop_table :personal_access_token_last_used_ips + end +end diff --git a/db/schema_migrations/20241112103518 b/db/schema_migrations/20241112103518 new file mode 100644 index 0000000000000000000000000000000000000000..9ceb4a84ce304e5e6f77118d052b414e3b442723 --- /dev/null +++ b/db/schema_migrations/20241112103518 @@ -0,0 +1 @@ +144e3d4c6d76ac2c138ab22eb6b83cf959e06ce0c1a65b85dc0420a7a34a20ee \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c88d4cf98eb080a79e0064f32a1235e30e11b227..cd2c3250ccdeba6a154c7f12bc7118eb58ab009b 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17331,6 +17331,24 @@ CREATE SEQUENCE path_locks_id_seq ALTER SEQUENCE path_locks_id_seq OWNED BY path_locks.id; +CREATE TABLE personal_access_token_last_used_ips ( + id bigint NOT NULL, + personal_access_token_id bigint NOT NULL, + organization_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + ip_address inet +); + +CREATE SEQUENCE personal_access_token_last_used_ips_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE personal_access_token_last_used_ips_id_seq OWNED BY personal_access_token_last_used_ips.id; + CREATE TABLE personal_access_tokens ( id bigint NOT NULL, user_id bigint NOT NULL, @@ -24231,6 +24249,8 @@ ALTER TABLE ONLY pages_domains ALTER COLUMN id SET DEFAULT nextval('pages_domain ALTER TABLE ONLY path_locks ALTER COLUMN id SET DEFAULT nextval('path_locks_id_seq'::regclass); +ALTER TABLE ONLY personal_access_token_last_used_ips ALTER COLUMN id SET DEFAULT nextval('personal_access_token_last_used_ips_id_seq'::regclass); + ALTER TABLE ONLY personal_access_tokens ALTER COLUMN id SET DEFAULT nextval('personal_access_tokens_id_seq'::regclass); ALTER TABLE ONLY plan_limits ALTER COLUMN id SET DEFAULT nextval('plan_limits_id_seq'::regclass); @@ -26823,6 +26843,9 @@ ALTER TABLE ONLY pages_domains ALTER TABLE ONLY path_locks ADD CONSTRAINT path_locks_pkey PRIMARY KEY (id); +ALTER TABLE ONLY personal_access_token_last_used_ips + ADD CONSTRAINT personal_access_token_last_used_ips_pkey PRIMARY KEY (id); + ALTER TABLE ONLY personal_access_tokens ADD CONSTRAINT personal_access_tokens_pkey PRIMARY KEY (id); @@ -29189,6 +29212,8 @@ CREATE INDEX idx_packages_packages_on_npm_scope_and_project_id ON packages_packa CREATE INDEX idx_packages_packages_on_project_id_name_version_package_type ON packages_packages USING btree (project_id, name, version, package_type); +CREATE INDEX idx_pat_last_used_ips_on_pat_id ON personal_access_token_last_used_ips USING btree (personal_access_token_id); + CREATE INDEX idx_personal_access_tokens_on_previous_personal_access_token_id ON personal_access_tokens USING btree (previous_personal_access_token_id); CREATE INDEX idx_pkgs_conan_file_metadata_on_pkg_file_id_when_recipe_file ON packages_conan_file_metadata USING btree (package_file_id) WHERE (conan_file_type = 1); @@ -32237,6 +32262,8 @@ CREATE INDEX index_pats_on_expiring_at_thirty_days_notification_sent_at ON perso CREATE INDEX index_pe_approval_rules_on_required_approvals_and_created_at ON protected_environment_approval_rules USING btree (required_approvals, created_at); +CREATE INDEX index_personal_access_token_last_used_ips_on_organization_id ON personal_access_token_last_used_ips USING btree (organization_id); + CREATE INDEX index_personal_access_tokens_on_id_and_created_at ON personal_access_tokens USING btree (id, created_at); CREATE INDEX index_personal_access_tokens_on_organization_id ON personal_access_tokens USING btree (organization_id); @@ -38916,6 +38943,9 @@ ALTER TABLE ONLY resource_state_events ALTER TABLE ONLY audit_events_group_external_streaming_destinations ADD CONSTRAINT fk_rails_7dffb88f29 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; +ALTER TABLE ONLY personal_access_token_last_used_ips + ADD CONSTRAINT fk_rails_7e650a7967 FOREIGN KEY (personal_access_token_id) REFERENCES personal_access_tokens(id) ON DELETE CASCADE; + ALTER TABLE ONLY clusters_kubernetes_namespaces ADD CONSTRAINT fk_rails_7e7688ecaf FOREIGN KEY (cluster_id) REFERENCES clusters(id) ON DELETE CASCADE; @@ -39114,6 +39144,9 @@ ALTER TABLE p_catalog_resource_component_usages ALTER TABLE ONLY packages_debian_project_distributions ADD CONSTRAINT fk_rails_94b95e1f84 FOREIGN KEY (creator_id) REFERENCES users(id) ON DELETE SET NULL; +ALTER TABLE ONLY personal_access_token_last_used_ips + ADD CONSTRAINT fk_rails_95388fbaeb FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE; + ALTER TABLE ONLY packages_rubygems_metadata ADD CONSTRAINT fk_rails_95a3f5ce78 FOREIGN KEY (package_id) REFERENCES packages_packages(id) ON DELETE CASCADE; diff --git a/doc/user/profile/personal_access_tokens.md b/doc/user/profile/personal_access_tokens.md index cfdf8343551b9d4c9ba9744b948fd44c44c43d27..6a9dcb7207e1bade8a0c23a48103b7765e06230b 100644 --- a/doc/user/profile/personal_access_tokens.md +++ b/doc/user/profile/personal_access_tokens.md @@ -160,23 +160,24 @@ To disable the enterprise users' personal access tokens: 1. Under **Personal access tokens**, select **Disable personal access tokens**. 1. Select **Save changes**. -## View the last time a token was used +## View the time at and IPs where a token was last used > - In GitLab 16.0 and earlier, token usage information is updated every 24 hours. > - The frequency of token usage information updates [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/410168) in GitLab 16.1 from 24 hours to 10 minutes. +> - Ability to view IP addresses [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/428577) in GitLab 17.8 [with a flag](../../administration/feature_flags.md) named `pat_ip`. Disabled by default. Token usage information is updated every 10 minutes. GitLab considers a token used when the token is used to: - Authenticate with the [REST](../../api/rest/index.md) or [GraphQL](../../api/graphql/index.md) APIs. - Perform a Git operation. -To view the last time a token was used: +To view the last time a token was used, and the IP addresses from where the token was used: 1. On the left sidebar, select your avatar. 1. Select **Edit profile**. 1. On the left sidebar, select **Access tokens**. -1. In the **Active personal access tokens** area, view the **Last Used** date for - the relevant token. +1. In the **Active personal access tokens** area, view the **Last Used** date and **Last Used IPs** for + the relevant token. **Last Used IPs** shows the last five distinct IP addresses. ## Personal access token scopes diff --git a/ee/spec/requests/api/epic_links_spec.rb b/ee/spec/requests/api/epic_links_spec.rb index 3c385903bf77d2ec047d58f6d731cf234e1500ac..6e1fddf7e4d3612ab74c0b0977ef2c2f6b5b82e7 100644 --- a/ee/spec/requests/api/epic_links_spec.rb +++ b/ee/spec/requests/api/epic_links_spec.rb @@ -214,7 +214,7 @@ def get_epics group.add_reporter(user) # TODO: remove threshold after epic-work item sync # issue: https://gitlab.com/gitlab-org/gitlab/-/issues/438295 - allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(130) + allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(132) end it 'returns 201 status' do diff --git a/ee/spec/requests/api/graphql/analytics/contribution_analytics/contributions_spec.rb b/ee/spec/requests/api/graphql/analytics/contribution_analytics/contributions_spec.rb index de06a7d406f5726e921b16bed9ff2e5c480622bd..316ddda9dd39d2424e41a7f7ab1e79b0ac1e6a63 100644 --- a/ee/spec/requests/api/graphql/analytics/contribution_analytics/contributions_spec.rb +++ b/ee/spec/requests/api/graphql/analytics/contribution_analytics/contributions_spec.rb @@ -103,7 +103,7 @@ def run_query # warm the query to avoid flakiness run_query - control = ActiveRecord::QueryRecorder.new { run_query } + control = ActiveRecord::QueryRecorder.new(skip_cached: false) { run_query } create(:event, :pushed, project: project, author: create(:user), created_at: Date.parse('2022-01-05')) expect { run_query }.not_to exceed_all_query_limit(control) diff --git a/ee/spec/requests/api/graphql/vulnerabilities/issue_links_spec.rb b/ee/spec/requests/api/graphql/vulnerabilities/issue_links_spec.rb index 9d87f8c16cf8cabce8914b3c7127785520a8392b..05e16fd55a4ea60489ad3481a1c0899d9cd51038 100644 --- a/ee/spec/requests/api/graphql/vulnerabilities/issue_links_spec.rb +++ b/ee/spec/requests/api/graphql/vulnerabilities/issue_links_spec.rb @@ -109,8 +109,10 @@ # 31) Loading the organization for access token (only inside specs) # 32) Loading the organization_details for avatar_url # 33/34) Likely transitional in nature during decomposition. Investigate when all tables are transitioned + # 35) Load last used IPs of personal access tokens + # 36) Saver current IP of the request in personal access token last used IPs # https://gitlab.com/gitlab-org/gitlab/-/issues/480882 - expect { query_issue_links }.not_to exceed_query_limit(34) + expect { query_issue_links }.not_to exceed_query_limit(36) end end diff --git a/ee/spec/requests/api/graphql/vulnerabilities/representation_information_spec.rb b/ee/spec/requests/api/graphql/vulnerabilities/representation_information_spec.rb index 238fbaf9ee727434f5d8df981b910d88d9109c5e..4e43b866c271e276b20c2708d54718b32ae74cda 100644 --- a/ee/spec/requests/api/graphql/vulnerabilities/representation_information_spec.rb +++ b/ee/spec/requests/api/graphql/vulnerabilities/representation_information_spec.rb @@ -60,7 +60,7 @@ it 'avoids N+1 queries' do post_graphql(vulnerabilities_query, current_user: user) - control = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do post_graphql(vulnerabilities_query, current_user: user) end diff --git a/ee/spec/requests/api/members_spec.rb b/ee/spec/requests/api/members_spec.rb index 2c44ee06e55afb223203666f9c452cd404724f0b..ff27bd41547b24aeb45c3e0db1431b0006d24896 100644 --- a/ee/spec/requests/api/members_spec.rb +++ b/ee/spec/requests/api/members_spec.rb @@ -2352,6 +2352,10 @@ end RSpec.shared_examples 'creates multiple memberships' do + before do + allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(104) + end + it do expect { post_request }.to change { ::Member.count }.by(2) diff --git a/lib/api/entities/personal_access_token_with_last_used_ips.rb b/lib/api/entities/personal_access_token_with_last_used_ips.rb new file mode 100644 index 0000000000000000000000000000000000000000..6d84ad57ca142db7441b68c6f96feed8d7a65622 --- /dev/null +++ b/lib/api/entities/personal_access_token_with_last_used_ips.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module API + module Entities + class PersonalAccessTokenWithLastUsedIps < Entities::PersonalAccessToken + expose :last_used_ips, documentation: { type: 'array', + example: ['127.0.0.1', + '127.0.0.2', + '127.0.0.3'] } do |personal_access_token| + personal_access_token.last_used_ips.map(&:ip_address) + end + end + end +end diff --git a/lib/api/personal_access_tokens.rb b/lib/api/personal_access_tokens.rb index 32f7726ca94fca6b6e5b7d6b0da35be37b702f07..9145c492dd8c323a166e9a1d1f47cb108052040f 100644 --- a/lib/api/personal_access_tokens.rb +++ b/lib/api/personal_access_tokens.rb @@ -49,7 +49,7 @@ class PersonalAccessTokens < ::API::Base desc 'Get single personal access token' do detail 'Get a personal access token by using the ID of the personal access token.' - success Entities::PersonalAccessToken + success Entities::PersonalAccessTokenWithLastUsedIps failure [ { code: 401, message: 'Unauthorized' }, { code: 404, message: 'Not found' } @@ -61,7 +61,7 @@ class PersonalAccessTokens < ::API::Base allowed = Ability.allowed?(current_user, :read_user_personal_access_tokens, token&.user) if allowed - present token, with: Entities::PersonalAccessToken + present token, with: Entities::PersonalAccessTokenWithLastUsedIps else # Only admins should be informed if the token doesn't exist current_user.can_admin_all_resources? ? not_found! : unauthorized! diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 59cbb12b855c13e3e0a3cdda8b3000fed42f9622..af701ac99546d9b1bc8a82996dc7debe8b9b8bba 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2939,6 +2939,9 @@ msgstr "" msgid "AccessTokens|Static object token" msgstr "" +msgid "AccessTokens|The last five distinct IP addresses from where the token was used" +msgstr "" + msgid "AccessTokens|The last time a token was used" msgstr "" @@ -32303,6 +32306,9 @@ msgstr "" msgid "Last Used" msgstr "" +msgid "Last Used IPs" +msgstr "" + msgid "Last accessed on" msgstr "" diff --git a/spec/factories/authn/personal_access_token_last_used_ips.rb b/spec/factories/authn/personal_access_token_last_used_ips.rb new file mode 100644 index 0000000000000000000000000000000000000000..9361585d32e390a65b5f32964ef50840b3a56ecc --- /dev/null +++ b/spec/factories/authn/personal_access_token_last_used_ips.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :personal_access_token_last_used_ip, class: 'Authn::PersonalAccessTokenLastUsedIp' do + personal_access_token + ip_address { IPAddr.new('42.43.44.45') } + end +end diff --git a/spec/frontend/access_tokens/components/access_token_table_app_spec.js b/spec/frontend/access_tokens/components/access_token_table_app_spec.js index ca0b75856e5659b57780f212c5c0949923c5a31d..570a83d4028ef3edb1882cdbab439bfe158c9474 100644 --- a/spec/frontend/access_tokens/components/access_token_table_app_spec.js +++ b/spec/frontend/access_tokens/components/access_token_table_app_spec.js @@ -25,6 +25,7 @@ describe('~/access_tokens/components/access_token_table_app', () => { scopes: ['api'], created_at: '2021-05-01T00:00:00.000Z', last_used_at: null, + last_used_ips: null, expired: false, expires_soon: true, expires_at: null, @@ -39,6 +40,7 @@ describe('~/access_tokens/components/access_token_table_app', () => { scopes: ['api', 'sudo'], created_at: '2022-04-21T00:00:00.000Z', last_used_at: '2022-04-21T00:00:00.000Z', + last_used_ips: ['192.168.0.1', '192.168.0.2'], expired: true, expires_soon: false, expires_at: new Date().toISOString(), @@ -57,6 +59,9 @@ describe('~/access_tokens/components/access_token_table_app', () => { initialActiveAccessTokens: defaultActiveAccessTokens, noActiveTokensMessage, showRole, + glFeatures: { + patIp: true, + }, ...props, }, }); @@ -137,6 +142,7 @@ describe('~/access_tokens/components/access_token_table_app', () => { 'Scopes', 'Created', 'Last Used', + 'Last Used IPs', 'Expires', 'Action', ]); @@ -152,6 +158,7 @@ describe('~/access_tokens/components/access_token_table_app', () => { 'Scopes', 'Created', 'Last Used', + 'Last Used IPs', 'Expires', 'Role', 'Action', @@ -168,17 +175,33 @@ describe('~/access_tokens/components/access_token_table_app', () => { const assistiveElement = lastUsed.find('.gl-sr-only'); expect(anchor.exists()).toBe(true); expect(anchor.attributes('href')).toBe( - '/help/user/profile/personal_access_tokens.md#view-the-last-time-a-token-was-used', + '/help/user/profile/personal_access_tokens.md#view-the-time-at-and-ips-where-a-token-was-last-used', ); expect(assistiveElement.text()).toBe('The last time a token was used'); }); + it('`Last Used IPs` header should contain a link and an assistive message', () => { + createComponent({ backendPagination }); + + const headers = wrapper.findAll('th'); + const lastUsedIPs = headers.at(5); + const anchor = lastUsedIPs.find('a'); + const assistiveElement = lastUsedIPs.find('.gl-sr-only'); + expect(anchor.exists()).toBe(true); + expect(anchor.attributes('href')).toBe( + '/help/user/profile/personal_access_tokens.md#view-the-time-at-and-ips-where-a-token-was-last-used', + ); + expect(assistiveElement.text()).toBe( + 'The last five distinct IP addresses from where the token was used', + ); + }); + it('updates the table after new tokens are created', async () => { createComponent({ initialActiveAccessTokens: [], showRole: true, backendPagination }); await triggerSuccess(); const cells = findCells(); - expect(cells).toHaveLength(16); + expect(cells).toHaveLength(18); // First row expect(cells.at(0).text()).toBe('a'); @@ -186,9 +209,10 @@ describe('~/access_tokens/components/access_token_table_app', () => { expect(cells.at(2).text()).toBe('api'); expect(cells.at(3).text()).not.toBe('Never'); expect(cells.at(4).text()).toBe('Never'); - expect(cells.at(5).text()).toBe('Never'); - expect(cells.at(6).text()).toBe('Maintainer'); - let buttons = cells.at(7).findAllComponents(GlButton); + expect(cells.at(5).text()).toBe('-'); + expect(cells.at(6).text()).toBe('Never'); + expect(cells.at(7).text()).toBe('Maintainer'); + let buttons = cells.at(8).findAllComponents(GlButton); expect(buttons).toHaveLength(2); expect(buttons.at(0).attributes()).toMatchObject({ 'aria-label': 'Revoke', @@ -213,14 +237,15 @@ describe('~/access_tokens/components/access_token_table_app', () => { expect(buttons.at(1).props('category')).toBe('tertiary'); // Second row - expect(cells.at(8).text()).toBe('b'); - expect(cells.at(9).text()).toBe('Test description'); - expect(cells.at(10).text()).toBe('api, sudo'); - expect(cells.at(11).text()).not.toBe('Never'); + expect(cells.at(9).text()).toBe('b'); + expect(cells.at(10).text()).toBe('Test description'); + expect(cells.at(11).text()).toBe('api, sudo'); expect(cells.at(12).text()).not.toBe('Never'); - expect(cells.at(13).text()).toBe('Expired'); - expect(cells.at(14).text()).toBe('Maintainer'); - buttons = cells.at(15).findAllComponents(GlButton); + expect(cells.at(13).text()).not.toBe('Never'); + expect(cells.at(14).text()).toBe('192.168.0.1, 192.168.0.2'); + expect(cells.at(15).text()).toBe('Expired'); + expect(cells.at(16).text()).toBe('Maintainer'); + buttons = cells.at(17).findAllComponents(GlButton); expect(buttons.at(0).attributes('href')).toBe( '/-/user_settings/personal_access_tokens/2/revoke', ); @@ -243,13 +268,14 @@ describe('~/access_tokens/components/access_token_table_app', () => { }); const headers = findHeaders(); - expect(headers).toHaveLength(7); + expect(headers).toHaveLength(8); [ 'Token name', 'Description', 'Scopes', 'Created', 'Last Used', + 'Last Used IPs', 'Expires', 'Role', ].forEach((text, index) => { @@ -271,14 +297,14 @@ describe('~/access_tokens/components/access_token_table_app', () => { backendPagination, }); - expect(findHeaders().at(7).text()).toBe('Action'); - expect(findCells().at(7).findComponent(GlButton).exists()).toBe(false); + expect(findHeaders().at(8).text()).toBe('Action'); + expect(findCells().at(8).findComponent(GlButton).exists()).toBe(false); }); it.each([ { revoke_path: '/-/user_settings/personal_access_tokens/1/revoke', rotate_path: null }, { revoke_path: null, rotate_path: '/-/user_settings/personal_access_tokens/1/rotate' }, - ])(`%p in some tokens, shows revoke or rotate button`, (input) => { + ])(`% in some tokens, shows revoke or rotate button`, (input) => { createComponent({ initialActiveAccessTokens: [ defaultActiveAccessTokens.map((data) => ({ ...data, ...input }))[0], @@ -288,8 +314,8 @@ describe('~/access_tokens/components/access_token_table_app', () => { backendPagination, }); - expect(findHeaders().at(7).text()).toBe('Action'); - expect(findCells().at(7).findComponent(GlButton).exists()).toBe(true); + expect(findHeaders().at(8).text()).toBe('Action'); + expect(findCells().at(8).findComponent(GlButton).exists()).toBe(true); }); }); }); @@ -302,7 +328,7 @@ describe('~/access_tokens/components/access_token_table_app', () => { // First and second rows expect(cells.at(0).text()).toBe('a'); - expect(cells.at(8).text()).toBe('b'); + expect(cells.at(9).text()).toBe('b'); const headers = findHeaders(); await headers.at(0).trigger('click'); @@ -310,7 +336,7 @@ describe('~/access_tokens/components/access_token_table_app', () => { // First and second rows have swapped expect(cells.at(0).text()).toBe('b'); - expect(cells.at(8).text()).toBe('a'); + expect(cells.at(9).text()).toBe('a'); }); it('sorts rows by date', async () => { @@ -320,14 +346,14 @@ describe('~/access_tokens/components/access_token_table_app', () => { // First and second rows expect(cells.at(4).text()).toBe('Never'); - expect(cells.at(12).text()).not.toBe('Never'); + expect(cells.at(13).text()).not.toBe('Never'); const headers = findHeaders(); await headers.at(4).trigger('click'); // First and second rows have swapped expect(cells.at(4).text()).not.toBe('Never'); - expect(cells.at(12).text()).toBe('Never'); + expect(cells.at(13).text()).toBe('Never'); }); describe('pagination', () => { @@ -354,11 +380,11 @@ describe('~/access_tokens/components/access_token_table_app', () => { describe('when clicked on the second page', () => { it('shows only one token in the table', async () => { - expect(findCells()).toHaveLength(PAGE_SIZE * 7); + expect(findCells()).toHaveLength(PAGE_SIZE * 8); await findPagination().vm.$emit('input', 2); await nextTick(); - expect(findCells()).toHaveLength(7); + expect(findCells()).toHaveLength(8); }); it('scrolls to the top', async () => { @@ -379,12 +405,11 @@ describe('~/access_tokens/components/access_token_table_app', () => { }); it('does not sort rows alphabetically', async () => { - // await axios.waitForAll(); const cells = findCells(); // First and second rows expect(cells.at(0).text()).toBe('a'); - expect(cells.at(8).text()).toBe('b'); + expect(cells.at(9).text()).toBe('b'); const headers = findHeaders(); await headers.at(0).trigger('click'); @@ -392,7 +417,7 @@ describe('~/access_tokens/components/access_token_table_app', () => { // First and second rows are not swapped expect(cells.at(0).text()).toBe('a'); - expect(cells.at(8).text()).toBe('b'); + expect(cells.at(9).text()).toBe('b'); }); it('change the busy state in the table', async () => { diff --git a/spec/frontend/access_tokens/components/inactive_access_token_table_app_spec.js b/spec/frontend/access_tokens/components/inactive_access_token_table_app_spec.js index a79115434776a7124f3ee6e65867c460832f390f..4573b0dc693429d37820eb168cc8a8346e52d37b 100644 --- a/spec/frontend/access_tokens/components/inactive_access_token_table_app_spec.js +++ b/spec/frontend/access_tokens/components/inactive_access_token_table_app_spec.js @@ -102,7 +102,7 @@ describe('~/access_tokens/components/inactive_access_token_table_app', () => { const assistiveElement = lastUsed.find('.gl-sr-only'); expect(anchor.exists()).toBe(true); expect(anchor.attributes('href')).toBe( - '/help/user/profile/personal_access_tokens.md#view-the-last-time-a-token-was-used', + '/help/user/profile/personal_access_tokens.md#view-the-time-at-and-ips-where-a-token-was-last-used', ); expect(assistiveElement.text()).toBe('The last time a token was used'); }); diff --git a/spec/models/authn/personal_access_token_last_used_ip_spec.rb b/spec/models/authn/personal_access_token_last_used_ip_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..97c7f2105c15751f706b6e52b64d68f95754cfe8 --- /dev/null +++ b/spec/models/authn/personal_access_token_last_used_ip_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Authn::PersonalAccessTokenLastUsedIp, feature_category: :system_access do + describe 'associations' do + subject { build(:personal_access_token_last_used_ip) } + + it { is_expected.to belong_to(:personal_access_token) } + end +end diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index fb1ece4e54cf4cbb8fe3f4addcc51ceb6b6fa719..8fd86afd885a5205e06e1490dd70c835c3fc41f5 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -34,6 +34,7 @@ it { is_expected.to belong_to(:previous_personal_access_token).class_name('PersonalAccessToken') } it { is_expected.to belong_to(:organization).class_name('Organizations::Organization') } + it { is_expected.to have_many(:last_used_ips) } end describe 'scopes' do diff --git a/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb index 8185ed0d0e7f425fbfc6dae33bfefff6fca470e4..3dc7d5f800bc1a0c4868925fb6d3cd0b0e9474e9 100644 --- a/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb +++ b/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb @@ -70,7 +70,7 @@ def run_mutation! context 'when the current user does not have permission to add assignees' do let(:current_user) { create(:user) } - let(:db_query_limit) { 27 } + let(:db_query_limit) { 29 } it 'does not change the assignees' do project.add_guest(current_user) diff --git a/spec/requests/api/graphql/projects/projects_spec.rb b/spec/requests/api/graphql/projects/projects_spec.rb index e47f7b23b0aa99188f20be947fa6397c5769cb03..90beddc4088d13202090094c738112c408764871 100644 --- a/spec/requests/api/graphql/projects/projects_spec.rb +++ b/spec/requests/api/graphql/projects/projects_spec.rb @@ -116,7 +116,7 @@ it 'avoids N+1 queries', :use_sql_query_cache, :clean_gitlab_redis_cache do post_graphql(single_project_query, current_user: current_user) - control = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do post_graphql(single_project_query, current_user: current_user) end @@ -125,7 +125,7 @@ # https://gitlab.com/gitlab-org/gitlab/-/issues/442164 expect do post_graphql(query, current_user: current_user) - end.not_to exceed_all_query_limit(control).with_threshold(17) + end.not_to exceed_all_query_limit(control).with_threshold(16) end it 'returns the expected projects' do diff --git a/spec/requests/api/personal_access_tokens_spec.rb b/spec/requests/api/personal_access_tokens_spec.rb index b75a5c974ec33d5ac45b417b8261f20eefcb5a9a..c5b2bb1f3c622d5cb04f54733f508f86e6787f45 100644 --- a/spec/requests/api/personal_access_tokens_spec.rb +++ b/spec/requests/api/personal_access_tokens_spec.rb @@ -426,6 +426,26 @@ def map_id(json_resonse) expect(json_response['id']).to eq(user_token.id) end + context 'when an ip is recently used' do + let(:current_ip_address) { '127.0.0.1' } + + it 'returns ips used' do + get api(user_token_path, current_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['last_used_ips']).to match_array(user_token.last_used_ips) + end + end + + context 'when there is not an ip recently used' do + it 'does not return an ip' do + get api(user_token_path, current_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['last_used_ip']).to be_nil + end + end + it 'fails to return other users PAT by id' do get api(other_users_path, current_user) diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index 799f65d5ddfa451ea83c4f3784876f410f109f43..202ca82bb4ba1cecbffbb62324e580743e9891b9 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -64,7 +64,7 @@ it 'executes a limited number of queries', :use_clean_rails_redis_caching do control = ActiveRecord::QueryRecorder.new { perform_archive_upload } - expect(control.count).to be <= 119 + expect(control.count).to be <= 123 end it 'schedules an import using a namespace' do diff --git a/spec/services/personal_access_tokens/last_used_service_spec.rb b/spec/services/personal_access_tokens/last_used_service_spec.rb index 28fe8ecf98edbc7003132ceb768b984ec8dbb1c9..e17cbc581b7765281007fc63f8bc81e90d3271b0 100644 --- a/spec/services/personal_access_tokens/last_used_service_spec.rb +++ b/spec/services/personal_access_tokens/last_used_service_spec.rb @@ -6,13 +6,158 @@ include ExclusiveLeaseHelpers describe '#execute' do - subject { described_class.new(personal_access_token).execute } + subject(:service_execution) { described_class.new(personal_access_token).execute } context 'when the personal access token was used 10 minutes ago', :freeze_time do let(:personal_access_token) { create(:personal_access_token, last_used_at: 10.minutes.ago) } it 'updates the last_used_at timestamp' do - expect { subject }.to change { personal_access_token.last_used_at } + expect { service_execution }.to change { personal_access_token.last_used_at } + end + + context 'when client is using ipv4' do + let(:current_ip_address) { '127.0.0.1' } + + it "does update the personal access token's last used ips" do + allow(Gitlab::IpAddressState).to receive(:current).and_return(current_ip_address) + + expect { service_execution }.to change { personal_access_token.last_used_ips.count } + expect( + Authn::PersonalAccessTokenLastUsedIp + .where(personal_access_token_id: personal_access_token.id, ip_address: Gitlab::IpAddressState.current) + .exists? + ).to be_truthy + end + end + + context 'when client is using ipv6' do + let(:current_ip_address) { '::1' } + + it "does update the personal access token's last used ips" do + allow(Gitlab::IpAddressState).to receive(:current).and_return(current_ip_address) + + expect { service_execution }.to change { personal_access_token.last_used_ips.count } + expect( + Authn::PersonalAccessTokenLastUsedIp + .where(personal_access_token_id: personal_access_token.id, ip_address: Gitlab::IpAddressState.current) + .exists? + ).to be_truthy + end + end + + context 'when PAT IP feature flag is disabled' do + let(:current_ip_address) { '127.0.0.1' } + + before do + stub_feature_flags(pat_ip: false) + end + + it "does not update the personal access token's last used ips" do + allow(Gitlab::IpAddressState).to receive(:current).and_return(current_ip_address) + + expect { service_execution }.not_to change { personal_access_token.last_used_ips.count } + expect( + Authn::PersonalAccessTokenLastUsedIp + .where(personal_access_token_id: personal_access_token.id, ip_address: Gitlab::IpAddressState.current) + .exists? + ).to be_falsy + end + end + + context 'when the personal access token was used more than 1 minute ago', :freeze_time do + let(:current_ip_address) { '::1' } + let(:personal_access_token) { create(:personal_access_token, last_used_at: 2.minutes.ago) } + + it "updates the personal access token's last used ips" do + allow(Gitlab::IpAddressState).to receive(:current).and_return(current_ip_address) + + expect { service_execution }.to change { personal_access_token.last_used_ips.count } + expect( + Authn::PersonalAccessTokenLastUsedIp + .where(personal_access_token_id: personal_access_token.id, ip_address: Gitlab::IpAddressState.current) + .exists? + ).to be_truthy + end + end + + context 'when the personal access token was used less than 1 minute ago', :freeze_time do + let(:current_ip_address) { '::1' } + let(:personal_access_token) { create(:personal_access_token, last_used_at: 30.seconds.ago) } + + it "does not update the personal access token's last used ips" do + allow(Gitlab::IpAddressState).to receive(:current).and_return(current_ip_address) + + expect { service_execution }.not_to change { personal_access_token.last_used_ips.count } + expect( + Authn::PersonalAccessTokenLastUsedIp + .where(personal_access_token_id: personal_access_token.id, ip_address: Gitlab::IpAddressState.current) + .exists? + ).to be_falsy + end + end + + context "when the current ip address is already saved" do + let(:current_ip_address) { '::1' } + + before do + personal_access_token.last_used_ips << Authn::PersonalAccessTokenLastUsedIp.new( + organization: personal_access_token.organization, + ip_address: current_ip_address) + end + + context "when the timestamp does not need an update" do + it "does not update the database" do + expect(Authn::PersonalAccessTokenLastUsedIp).not_to receive(:new) + + service_execution + end + end + + context "when timestamp needs an update", :freeze_time do + let(:personal_access_token) { create(:personal_access_token, last_used_at: 11.minutes.ago) } + + it "does update the timestamp, but does not update the ip" do + allow(Gitlab::IpAddressState).to receive(:current).and_return(current_ip_address) + + expect(personal_access_token.last_used_ips.count).to eq(1) + expect { service_execution }.to change { personal_access_token.last_used_at } + expect(personal_access_token.last_used_ips.count).to eq(1) + end + end + end + + context "when the count of personal access token's last used ips are above the limit" do + let(:current_ip_address) { '123.12.123.1' } + + before do + 1.upto(5) do |i| + personal_access_token.last_used_ips << Authn::PersonalAccessTokenLastUsedIp.new( + organization: personal_access_token.organization, + ip_address: "127.0.0.#{i}", created_at: i.days.ago) + end + end + + it "keeps no. of ips at 5" do + allow(Gitlab::IpAddressState).to receive(:current).and_return(current_ip_address) + + expect( + Authn::PersonalAccessTokenLastUsedIp + .where(personal_access_token_id: personal_access_token.id, ip_address: "127.0.0.5") + .exists? + ).to be_truthy + expect { service_execution }.not_to change { personal_access_token.last_used_ips.count } + end + + it "removes the oldest PAT ip" do + allow(Gitlab::IpAddressState).to receive(:current).and_return(current_ip_address) + + expect { service_execution }.to( + change do + Authn::PersonalAccessTokenLastUsedIp + .where(personal_access_token_id: personal_access_token.id, ip_address: "127.0.0.5") + .exists? + end.from(true).to(false)) + end end it 'obtains an exclusive lease before updating' do @@ -25,13 +170,13 @@ ).and_call_original end - expect { subject }.to change { personal_access_token.last_used_at } + expect { service_execution }.to change { personal_access_token.last_used_at } end it 'does not run on read-only GitLab instances' do allow(::Gitlab::Database).to receive(:read_only?).and_return(true) - expect { subject }.not_to change { personal_access_token.last_used_at } + expect { service_execution }.not_to change { personal_access_token.last_used_at } end context 'when lease is already acquired by another process' do @@ -42,7 +187,7 @@ end it 'does not update last_used_at' do - expect { subject }.not_to change { personal_access_token.last_used_at } + expect { service_execution }.not_to change { personal_access_token.last_used_at } end end @@ -65,7 +210,7 @@ let(:personal_access_token) { create(:personal_access_token, last_used_at: (10.minutes - 1.second).ago) } it 'does not update the last_used_at timestamp' do - expect { subject }.not_to change { personal_access_token.last_used_at } + expect { service_execution }.not_to change { personal_access_token.last_used_at } end end @@ -73,7 +218,7 @@ let_it_be(:personal_access_token) { create(:personal_access_token, last_used_at: nil) } it 'updates the last_used_at timestamp' do - expect { subject }.to change { personal_access_token.last_used_at } + expect { service_execution }.to change { personal_access_token.last_used_at } end end @@ -81,7 +226,7 @@ let_it_be(:personal_access_token) { create(:oauth_access_token) } it 'does not execute' do - expect(subject).to be_nil + expect(service_execution).to be_nil end end end diff --git a/spec/services/personal_access_tokens/rotate_service_spec.rb b/spec/services/personal_access_tokens/rotate_service_spec.rb index 5ae9bd31584394cf928e7ebf493cc723036d964d..923eab9a6dd3d29b0e6ff81f324d9ec2a4bedb85 100644 --- a/spec/services/personal_access_tokens/rotate_service_spec.rb +++ b/spec/services/personal_access_tokens/rotate_service_spec.rb @@ -97,19 +97,14 @@ end context 'when creating the new token fails' do - let(:error_message) { 'boom!' } - before do - allow_next_instance_of(PersonalAccessToken) do |token| - allow(token).to receive_message_chain(:errors, :full_messages, :to_sentence).and_return(error_message) - allow(token).to receive_message_chain(:errors, :clear) - allow(token).to receive_message_chain(:errors, :empty?).and_return(false) - end + # change the default expiration for rotation to create an invalid token + stub_const('::PersonalAccessTokens::RotateService::EXPIRATION_PERIOD', 10.years) end it 'returns an error' do expect(response).to be_error - expect(response.message).to eq(error_message) + expect(response.message).to include('Expiration date must be before') end it 'reverts the changes' do diff --git a/spec/services/project_access_tokens/rotate_service_spec.rb b/spec/services/project_access_tokens/rotate_service_spec.rb index 8e16d6058981e9cacd9cd942c3f5210f8bcadf07..90ab1cf857c910f68e057ccb8fe44e90837c69d7 100644 --- a/spec/services/project_access_tokens/rotate_service_spec.rb +++ b/spec/services/project_access_tokens/rotate_service_spec.rb @@ -35,19 +35,14 @@ it_behaves_like "rotates token successfully" context 'when creating the new token fails' do - let(:error_message) { 'boom!' } - before do - allow_next_instance_of(PersonalAccessToken) do |token| - allow(token).to receive_message_chain(:errors, :full_messages, :to_sentence).and_return(error_message) - allow(token).to receive_message_chain(:errors, :clear) - allow(token).to receive_message_chain(:errors, :empty?).and_return(false) - end + # change the default expiration for rotation to create an invalid token + stub_const('::PersonalAccessTokens::RotateService::EXPIRATION_PERIOD', 10.years) end it 'returns an error' do expect(response).to be_error - expect(response.message).to eq(error_message) + expect(response.message).to include('Expiration date must be before') end it 'reverts the changes' do diff --git a/spec/support/matchers/exceed_query_limit.rb b/spec/support/matchers/exceed_query_limit.rb index 6f2c474ad55393894a8386b7a50dd9d9c5f926aa..90fb338b86878befbb5117b3c87bc8b197c1c688 100644 --- a/spec/support/matchers/exceed_query_limit.rb +++ b/spec/support/matchers/exceed_query_limit.rb @@ -291,7 +291,7 @@ def expected_count @expected_count ||= if expected.is_a?(ActiveRecord::QueryRecorder) query_recorder_count(expected) else - ActiveRecord::QueryRecorder.new(&block_arg).count + ActiveRecord::QueryRecorder.new(skip_cached: skip_cached, &block_arg).count end end