diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb index 3ebb2126f4dfbce3a031ef4d3e367471fbd2fdd3..75afff6a2fafcf5074ac4c9d2c6c291a9e5921b7 100644 --- a/app/models/personal_access_token.rb +++ b/app/models/personal_access_token.rb @@ -15,6 +15,7 @@ class PersonalAccessToken < ApplicationRecord # PATs are 20 characters + optional configurable settings prefix (0..20) TOKEN_LENGTH_RANGE = (20..40).freeze + MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS = 365 serialize :scopes, Array # rubocop:disable Cop/ActiveRecordSerialize @@ -48,6 +49,7 @@ class PersonalAccessToken < ApplicationRecord validates :scopes, presence: true validate :validate_scopes + validate :expires_at_before_instance_max_expiry_date, on: :create def revoke! update!(revoked: true) @@ -57,6 +59,19 @@ def active? !revoked? && !expired? end + # fall back to default value until background migration has updated all + # existing PATs and we can add a validation + # https://gitlab.com/gitlab-org/gitlab/-/issues/369123 + def expires_at=(value) + datetime = if Feature.enabled?(:default_pat_expiration) + value.presence || MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now + else + value + end + + super(datetime) + end + override :simple_sorts def self.simple_sorts super.merge( @@ -108,6 +123,15 @@ def add_admin_mode_scope def prefix_from_application_current_settings self.class.token_prefix end + + def expires_at_before_instance_max_expiry_date + return unless Feature.enabled?(:default_pat_expiration) + return unless expires_at + + if expires_at > MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now + errors.add(:expires_at, _('must expire in 365 days')) + end + end end PersonalAccessToken.prepend_mod_with('PersonalAccessToken') diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index cfa43f5d9c823ec0ffe38a427564306ffef44420..553315f08f9652196527f50731840a4152b5426a 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -100,7 +100,15 @@ def default_scopes end def create_membership(resource, user, access_level) - resource.add_member(user, access_level, expires_at: params[:expires_at]) + resource.add_member(user, access_level, expires_at: default_pat_expiration) + end + + def default_pat_expiration + if Feature.enabled?(:default_pat_expiration) + params[:expires_at].presence || PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now + else + params[:expires_at] + end end def log_event(token) diff --git a/config/feature_flags/development/default_pat_expiration.yml b/config/feature_flags/development/default_pat_expiration.yml new file mode 100644 index 0000000000000000000000000000000000000000..b48d6a027235af6559030178e51a8660d1aa8b8a --- /dev/null +++ b/config/feature_flags/development/default_pat_expiration.yml @@ -0,0 +1,7 @@ +name: default_pat_expiration +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120213 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/410440 +milestone: '16.0' +type: development +group: group::authentication and authorization +default_enabled: true diff --git a/data/removals/16_0/16-0-non-expiring-access-tokens.yml b/data/removals/16_0/16-0-non-expiring-access-tokens.yml new file mode 100644 index 0000000000000000000000000000000000000000..5ef7df3a13172a20e0e957ca27fcbb99c13966e2 --- /dev/null +++ b/data/removals/16_0/16-0-non-expiring-access-tokens.yml @@ -0,0 +1,19 @@ +- title: "Non-expiring access tokens no longer supported" + announcement_milestone: "15.4" # (required) The milestone when this feature was deprecated. + removal_milestone: "16.0" # (required) The milestone when this feature is being removed. + breaking_change: true # (required) Change to false if this is not a breaking change. + reporter: jessieay # (required) GitLab username of the person reporting the removal + stage: Manage # (required) String value of the stage that the feature was created in. e.g., Growth + issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/369123 + body: | # (required) Do not modify this line, instead modify the lines below. + Currently, you can create access tokens that have no expiration date. These access tokens are valid indefinitely, which presents a security risk if the access token is + divulged. Because expiring access tokens are better, from GitLab 15.4 we [populate a default expiration date](https://gitlab.com/gitlab-org/gitlab/-/issues/348660). + + In GitLab 16.0, any personal, project, or group access token that does not have an expiration date will automatically have an expiration date set at 365 days later than the current date. +# +# OPTIONAL FIELDS +# + tiers: # (optional - may be required in the future) An array of tiers that the feature is available in currently. e.g., [Free, Silver, Gold, Core, Premium, Ultimate] + documentation_url: # (optional) This is a link to the current documentation page + image_url: # (optional) This is a link to a thumbnail image depicting the feature + video_url: # (optional) Use the youtube thumbnail URL with the structure of https://img.youtube.com/vi/UNIQUEID/hqdefault.jpg diff --git a/doc/update/removals.md b/doc/update/removals.md index d0a6952745a8c24cbcf2cd0958d98d6cd1c7a6ef..0b6e7ccdadf6cb2100df20894e12d5e7d61cb77f 100644 --- a/doc/update/removals.md +++ b/doc/update/removals.md @@ -190,6 +190,17 @@ The [**Maximum number of active pipelines per project** limit](https://docs.gitl - [**Pipelines rate limits**](https://docs.gitlab.com/ee/user/admin_area/settings/rate_limit_on_pipelines_creation.html). - [**Total number of jobs in currently active pipelines**](https://docs.gitlab.com/ee/user/admin_area/settings/continuous_integration.html#set-cicd-limits). +### Non-expiring access tokens no longer supported + +WARNING: +This is a [breaking change](https://docs.gitlab.com/ee/development/deprecation_guidelines/). +Review the details carefully before upgrading. + +Currently, you can create access tokens that have no expiration date. These access tokens are valid indefinitely, which presents a security risk if the access token is +divulged. Because expiring access tokens are better, from GitLab 15.4 we [populate a default expiration date](https://gitlab.com/gitlab-org/gitlab/-/issues/348660). + +In GitLab 16.0, any personal, project, or group access token that does not have an expiration date will automatically have an expiration date set at 365 days later than the current date. + ### Non-standard default Redis ports are no longer supported WARNING: diff --git a/ee/spec/models/ee/personal_access_token_spec.rb b/ee/spec/models/ee/personal_access_token_spec.rb index eed2e2e73bdd9c861307e2e0bd178f2632af91b5..49dd9eaf44fbd47554c3b4e9726c47b88bdf4d93 100644 --- a/ee/spec/models/ee/personal_access_token_spec.rb +++ b/ee/spec/models/ee/personal_access_token_spec.rb @@ -2,11 +2,16 @@ require 'spec_helper' -RSpec.describe PersonalAccessToken do +RSpec.describe PersonalAccessToken, feature_category: :system_access do describe 'scopes' do let_it_be(:expired_token) { create(:personal_access_token, expires_at: 1.day.ago) } let_it_be(:valid_token) { create(:personal_access_token, expires_at: 1.day.from_now) } - let_it_be(:long_expiry_token) { create(:personal_access_token, expires_at: '999999-12-31'.to_date) } + let_it_be(:long_expiry_token) do + create( + :personal_access_token, + expires_at: described_class::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now + ) + end let!(:pat) { create(:personal_access_token, expires_at: expiration_date) } @@ -31,7 +36,12 @@ describe 'with_no_expires_at' do subject { described_class.with_expires_at_after(2.days.from_now) } - let(:expiration_date) { nil } + let!(:pat) { create(:personal_access_token) } + + before do + stub_feature_flags(default_pat_expiration: false) + pat.update_attribute(:expires_at, nil) + end it 'includes the tokens with nil expires_at' do expect(described_class.with_no_expires_at).to contain_exactly(pat) @@ -103,11 +113,26 @@ expect(personal_access_token.errors[:expires_at].first).to eq('is invalid') end - it "can't be blank" do - personal_access_token.expires_at = nil + context 'when default_pat_expiration feature flag is true' do + it "is invalid" do + personal_access_token.expires_at = nil - expect(personal_access_token).not_to be_valid - expect(personal_access_token.errors[:expires_at].first).to eq("can't be blank") + expect(personal_access_token).not_to be_valid + expect(personal_access_token.errors[:expires_at].first).to eq("is invalid") + end + end + + context 'when default_pat_expiration feature flag is false' do + before do + stub_feature_flags(default_pat_expiration: false) + end + + it "can't be blank" do + personal_access_token.expires_at = nil + + expect(personal_access_token).not_to be_valid + expect(personal_access_token.errors[:expires_at].first).to eq("can't be blank") + end end end @@ -146,13 +171,48 @@ end end - context 'when the instance does not enforce a PAT expiry setting' do - before do - stub_ee_application_setting(max_personal_access_token_lifetime: nil) + context 'when default_pat_expiration feature flag is true' do + context 'when the instance does not enforce a PAT expiry setting' do + before do + stub_ee_application_setting(max_personal_access_token_lifetime: nil) + end + + it_behaves_like 'PAT expiry rules are not enforced' do + let(:max_expiration_date) { instance_level_max_expiration_date } + end + + it 'defaults to PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS' do + freeze_time do + personal_access_token.expires_at = nil + + expect(personal_access_token).to be_valid + expect(personal_access_token.expires_at).to eq( + PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now.to_date + ) + end + end end + end - it_behaves_like 'PAT expiry rules are not enforced' do - let(:max_expiration_date) { instance_level_max_expiration_date } + context 'when default_pat_expiration feature flag is false' do + context 'when the instance does not enforce a PAT expiry setting' do + before do + stub_ee_application_setting(max_personal_access_token_lifetime: nil) + stub_feature_flags(default_pat_expiration: false) + end + + it_behaves_like 'PAT expiry rules are not enforced' do + let(:max_expiration_date) { instance_level_max_expiration_date } + end + + it 'does not have a default' do + freeze_time do + personal_access_token.expires_at = nil + + expect(personal_access_token).to be_valid + expect(personal_access_token.expires_at).to eq(nil) + end + end end end end diff --git a/ee/spec/support/shared_examples/features/credentials_inventory_shared_examples.rb b/ee/spec/support/shared_examples/features/credentials_inventory_shared_examples.rb index ec05c646d724377ce9653b347bdd51356de58050..7ca429ca4a243dcd7491b45d58f97b569d471363 100644 --- a/ee/spec/support/shared_examples/features/credentials_inventory_shared_examples.rb +++ b/ee/spec/support/shared_examples/features/credentials_inventory_shared_examples.rb @@ -6,25 +6,46 @@ let_it_be(:user) { defined?(managed_user) ? managed_user : create(:user, name: 'abc') } context 'when a personal access token is active' do - before_all do - create(:personal_access_token, - user: user, - created_at: '2019-12-10', - updated_at: '2020-06-22', - expires_at: nil) - end + context 'when default_pat_expiration feature flag is true' do + before do + create(:personal_access_token, + user: user, + created_at: '2019-12-10', + updated_at: '2020-06-22', + expires_at: nil) + visit credentials_path + end - before do - visit credentials_path + it 'shows the details', :aggregate_failures do + expect(first_row.text).to include('abc') + expect(first_row.text).to include('api') + expect(first_row.text).to include('2019-12-10') + expect(first_row.text).to include(PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS + .days.from_now.strftime("%Y-%m-%d")) + expect(first_row.text).not_to include('2020-06-22') + expect(first_row).to have_selector('a.btn', text: 'Revoke') + end end - it 'shows the details', :aggregate_failures do - expect(first_row.text).to include('abc') - expect(first_row.text).to include('api') - expect(first_row.text).to include('2019-12-10') - expect(first_row.text).to include('Never') - expect(first_row.text).not_to include('2020-06-22') - expect(first_row).to have_selector('a.btn', text: 'Revoke') + context 'when default_pat_expiration feature flag is false' do + before do + stub_feature_flags(default_pat_expiration: false) + create(:personal_access_token, + user: user, + created_at: '2019-12-10', + updated_at: '2020-06-22', + expires_at: nil) + visit credentials_path + end + + it 'shows the details', :aggregate_failures do + expect(first_row.text).to include('abc') + expect(first_row.text).to include('api') + expect(first_row.text).to include('2019-12-10') + expect(first_row.text).to include('Never') + expect(first_row.text).not_to include('2020-06-22') + expect(first_row).to have_selector('a.btn', text: 'Revoke') + end end end @@ -59,7 +80,7 @@ user: user, created_at: '2019-12-10', updated_at: '2020-06-22', - expires_at: nil) + expires_at: Time.zone.now) end before do diff --git a/ee/spec/views/shared/credentials_inventory/_expiry_date.html.haml_spec.rb b/ee/spec/views/shared/credentials_inventory/_expiry_date.html.haml_spec.rb index 0a7ef92d6527bf1d1ef05770d12fd7a33f469465..7df14ce62de81c0d59c94ed803b36ed7ab44c0dc 100644 --- a/ee/spec/views/shared/credentials_inventory/_expiry_date.html.haml_spec.rb +++ b/ee/spec/views/shared/credentials_inventory/_expiry_date.html.haml_spec.rb @@ -20,8 +20,27 @@ context 'when an expirable credential is used' do let_it_be(:credential) { create(:personal_access_token, user: user, expires_at: nil) } - it 'shows "Never" when not expirable' do - expect(rendered).to have_text('Never') + context 'when default_pat_expiration feature flag is true' do + it 'shows default expiry date in case expires_at is nil' do + expect(rendered).to have_text( + PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS + .days + .from_now.strftime("%Y-%m-%d") + ) + end + end + + context 'when default_pat_expiration feature flag is false' do + before do + stub_feature_flags(default_pat_expiration: false) + credential.expires_at = nil + end + + it 'shows "Never" when not expirable' do + render 'shared/credentials_inventory/expiry_date', credential: credential + + expect(rendered).to have_text("Never") + end end context 'and is not expired' do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 669e97cf0e7f65b03d5f04be7073bbf88fe1a264..5bcd1233b755f40e48ca0bd31c707490b533e3de 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -53917,6 +53917,9 @@ msgstr "" msgid "must contain only a discord user ID." msgstr "" +msgid "must expire in 365 days" +msgstr "" + msgid "must have a repository" msgstr "" diff --git a/spec/lib/api/entities/personal_access_token_spec.rb b/spec/lib/api/entities/personal_access_token_spec.rb index fd3c53a21b474ebc5275a88b2b098f82ebb99a70..7f79cc80573215de19974a509671345079d2a9b5 100644 --- a/spec/lib/api/entities/personal_access_token_spec.rb +++ b/spec/lib/api/entities/personal_access_token_spec.rb @@ -19,7 +19,7 @@ user_id: user.id, last_used_at: nil, active: true, - expires_at: nil + expires_at: token.expires_at.iso8601 }) end end diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index 45b8de509fcef7706af546767bd3ebb64e20c78c..bd6a7c156c4121f6ca758a7c42764daa3765cbe5 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -267,6 +267,41 @@ expect(personal_access_token).not_to be_valid expect(personal_access_token.errors[:scopes].first).to eq "can only contain available scopes" end + + context 'validates expires_at' do + let(:max_expiration_date) { described_class::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now } + + context 'when default_pat_expiration feature flag is true' do + context 'when expires_in is less than MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS days' do + it 'is valid' do + personal_access_token.expires_at = max_expiration_date - 1.day + + expect(personal_access_token).to be_valid + end + end + + context 'when expires_in is more than MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS days' do + it 'is invalid' do + personal_access_token.expires_at = max_expiration_date + 1.day + + expect(personal_access_token).not_to be_valid + expect(personal_access_token.errors[:expires_at].first).to eq('must expire in 365 days') + end + end + end + + context 'when default_pat_expiration feature flag is false' do + before do + stub_feature_flags(default_pat_expiration: false) + end + + it 'allows any expires_at value' do + personal_access_token.expires_at = max_expiration_date + 1.day + + expect(personal_access_token).to be_valid + end + end + end end describe 'scopes' do @@ -289,7 +324,7 @@ let_it_be(:revoked_token) { create(:personal_access_token, revoked: true) } let_it_be(:valid_token_and_notified) { create(:personal_access_token, expires_at: 2.days.from_now, expire_notification_delivered: true) } let_it_be(:valid_token) { create(:personal_access_token, expires_at: 2.days.from_now) } - let_it_be(:long_expiry_token) { create(:personal_access_token, expires_at: '999999-12-31'.to_date) } + let_it_be(:long_expiry_token) { create(:personal_access_token, expires_at: described_class::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now) } context 'in one day' do it "doesn't have any tokens" do @@ -427,4 +462,36 @@ end end end + + describe '#expires_at=' do + let(:personal_access_token) { described_class.new } + + context 'when default_pat_expiration feature flag is true' do + context 'expires_at set to empty value' do + [nil, ""].each do |expires_in_value| + it 'defaults to PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS' do + personal_access_token.expires_at = expires_in_value + + freeze_time do + expect(personal_access_token.expires_at).to eq( + PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now.to_date + ) + end + end + end + end + end + + context 'when default_pat_expiration feature flag is false' do + before do + stub_feature_flags(default_pat_expiration: false) + end + + it 'does not set a default' do + personal_access_token.expires_at = nil + + expect(personal_access_token.expires_at).to eq(nil) + end + end + end end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index dff41c4c4773b7b573c72bf6671a5e4c78417000..6414b1efe6a7cb2548af1846eab40b5f882aa14d 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -10,6 +10,9 @@ let_it_be(:project, reload: true) { create(:project, :repository, :wiki_repo) } let_it_be(:personal_snippet) { create(:personal_snippet, :repository, author: user) } let_it_be(:project_snippet) { create(:project_snippet, :repository, author: user, project: project) } + let_it_be(:max_pat_access_token_lifetime) do + PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now.to_date.freeze + end let(:key) { create(:key, user: user) } let(:secret_token) { Gitlab::Shell.secret_token } @@ -194,39 +197,68 @@ def perform_request(headers: gitlab_shell_internal_api_request_header) expect(json_response['message']).to match(/\AInvalid scope: 'badscope'. Valid scopes are: /) end - it 'returns a token without expiry when the expires_at parameter is missing' do - token_size = (PersonalAccessToken.token_prefix || '').size + 20 + it 'returns a token with expiry when it receives a valid expires_at parameter' do + freeze_time do + token_size = (PersonalAccessToken.token_prefix || '').size + 20 + + post api('/internal/personal_access_token'), + params: { + key_id: key.id, + name: 'newtoken', + scopes: %w(read_api read_repository), + expires_at: max_pat_access_token_lifetime + }, + headers: gitlab_shell_internal_api_request_header - post api('/internal/personal_access_token'), - params: { - key_id: key.id, - name: 'newtoken', - scopes: %w(read_api read_repository) - }, - headers: gitlab_shell_internal_api_request_header + expect(json_response['success']).to be_truthy + expect(json_response['token']).to match(/\A\S{#{token_size}}\z/) + expect(json_response['scopes']).to match_array(%w(read_api read_repository)) + expect(json_response['expires_at']).to eq(max_pat_access_token_lifetime.iso8601) + end + end + + context 'when default_pat_expiration feature flag is true' do + it 'returns token with expiry as PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS' do + freeze_time do + token_size = (PersonalAccessToken.token_prefix || '').size + 20 - expect(json_response['success']).to be_truthy - expect(json_response['token']).to match(/\A\S{#{token_size}}\z/) - expect(json_response['scopes']).to match_array(%w(read_api read_repository)) - expect(json_response['expires_at']).to be_nil + post api('/internal/personal_access_token'), + params: { + key_id: key.id, + name: 'newtoken', + scopes: %w(read_api read_repository) + }, + headers: gitlab_shell_internal_api_request_header + + expect(json_response['success']).to be_truthy + expect(json_response['token']).to match(/\A\S{#{token_size}}\z/) + expect(json_response['scopes']).to match_array(%w(read_api read_repository)) + expect(json_response['expires_at']).to eq(max_pat_access_token_lifetime.iso8601) + end + end end - it 'returns a token with expiry when it receives a valid expires_at parameter' do - token_size = (PersonalAccessToken.token_prefix || '').size + 20 + context 'when default_pat_expiration feature flag is false' do + before do + stub_feature_flags(default_pat_expiration: false) + end - post api('/internal/personal_access_token'), - params: { - key_id: key.id, - name: 'newtoken', - scopes: %w(read_api read_repository), - expires_at: '9001-11-17' - }, - headers: gitlab_shell_internal_api_request_header + it 'uses nil expiration value' do + token_size = (PersonalAccessToken.token_prefix || '').size + 20 + + post api('/internal/personal_access_token'), + params: { + key_id: key.id, + name: 'newtoken', + scopes: %w(read_api read_repository) + }, + headers: gitlab_shell_internal_api_request_header - expect(json_response['success']).to be_truthy - expect(json_response['token']).to match(/\A\S{#{token_size}}\z/) - expect(json_response['scopes']).to match_array(%w(read_api read_repository)) - expect(json_response['expires_at']).to eq('9001-11-17') + expect(json_response['success']).to be_truthy + expect(json_response['token']).to match(/\A\S{#{token_size}}\z/) + expect(json_response['scopes']).to match_array(%w(read_api read_repository)) + expect(json_response['expires_at']).to be_nil + end end end diff --git a/spec/requests/api/resource_access_tokens_spec.rb b/spec/requests/api/resource_access_tokens_spec.rb index a13aa48a497aa5b2e72510644c5ba238caa6e30d..ce05fa2b383185832a2f65fb7219abc3a70f74fc 100644 --- a/spec/requests/api/resource_access_tokens_spec.rb +++ b/spec/requests/api/resource_access_tokens_spec.rb @@ -336,13 +336,33 @@ context "when 'expires_at' is not set" do let(:expires_at) { nil } - it "creates a #{source_type} access token with the params", :aggregate_failures do - create_token + context 'when default_pat_expiration feature flag is true' do + it "creates a #{source_type} access token with the default expires_at value", :aggregate_failures do + freeze_time do + create_token + expires_at = PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now + + expect(response).to have_gitlab_http_status(:created) + expect(json_response["name"]).to eq("test") + expect(json_response["scopes"]).to eq(["api"]) + expect(json_response["expires_at"]).to eq(expires_at.to_date.iso8601) + end + end + end - expect(response).to have_gitlab_http_status(:created) - expect(json_response["name"]).to eq("test") - expect(json_response["scopes"]).to eq(["api"]) - expect(json_response["expires_at"]).to eq(nil) + context 'when default_pat_expiration feature flag is false' do + before do + stub_feature_flags(default_pat_expiration: false) + end + + it "creates a #{source_type} access token with the params", :aggregate_failures do + create_token + + expect(response).to have_gitlab_http_status(:created) + expect(json_response["name"]).to eq("test") + expect(json_response["scopes"]).to eq(["api"]) + expect(json_response["expires_at"]).to eq(nil) + end end end diff --git a/spec/serializers/access_token_entity_base_spec.rb b/spec/serializers/access_token_entity_base_spec.rb index e14a07a346ae48744e21e820e9616fd60c7f6b97..8a92a53d0c1cb417b3bbfe5429eba598295c1ef8 100644 --- a/spec/serializers/access_token_entity_base_spec.rb +++ b/spec/serializers/access_token_entity_base_spec.rb @@ -16,7 +16,7 @@ revoked: false, created_at: token.created_at, scopes: token.scopes, - expires_at: nil, + expires_at: token.expires_at.iso8601, expired: false, expires_soon: false )) diff --git a/spec/services/resource_access_tokens/create_service_spec.rb b/spec/services/resource_access_tokens/create_service_spec.rb index 464fc18f1f07f5414f9c971f45b4d71e94268918..59d582f038a14283ddf1f30af317cc0d86c7ed71 100644 --- a/spec/services/resource_access_tokens/create_service_spec.rb +++ b/spec/services/resource_access_tokens/create_service_spec.rb @@ -9,6 +9,9 @@ let_it_be(:project) { create(:project, :private) } let_it_be(:group) { create(:group, :private) } let_it_be(:params) { {} } + let_it_be(:max_pat_access_token_lifetime) do + PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now.to_date.freeze + end before do stub_config_setting(host: 'example.com') @@ -185,20 +188,51 @@ context 'expires_at' do context 'when no expiration value is passed' do - it 'uses nil expiration value' do - response = subject - access_token = response.payload[:access_token] + context 'when default_pat_expiration feature flag is true' do + it 'defaults to PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS' do + freeze_time do + response = subject + access_token = response.payload[:access_token] + + expect(access_token.expires_at).to eq( + max_pat_access_token_lifetime.to_date + ) + end + end - expect(access_token.expires_at).to eq(nil) + context 'expiry of the project bot member' do + it 'project bot membership does not expire' do + response = subject + access_token = response.payload[:access_token] + project_bot = access_token.user + + expect(resource.members.find_by(user_id: project_bot.id).expires_at).to eq( + max_pat_access_token_lifetime.to_date + ) + end + end end - context 'expiry of the project bot member' do - it 'project bot membership does not expire' do + context 'when default_pat_expiration feature flag is false' do + before do + stub_feature_flags(default_pat_expiration: false) + end + + it 'uses nil expiration value' do response = subject access_token = response.payload[:access_token] - project_bot = access_token.user - expect(resource.members.find_by(user_id: project_bot.id).expires_at).to eq(nil) + expect(access_token.expires_at).to eq(nil) + end + + context 'expiry of the project bot member' do + it 'project bot membership expires' do + response = subject + access_token = response.payload[:access_token] + project_bot = access_token.user + + expect(resource.members.find_by(user_id: project_bot.id).expires_at).to eq(nil) + end end end end @@ -219,7 +253,7 @@ access_token = response.payload[:access_token] project_bot = access_token.user - expect(resource.members.find_by(user_id: project_bot.id).expires_at).to eq(params[:expires_at]) + expect(resource.members.find_by(user_id: project_bot.id).expires_at).to eq(access_token.expires_at) end end end