Skip to content
Snippets Groups Projects
Verified Commit cce2adbf authored by Stan Hu's avatar Stan Hu Committed by GitLab
Browse files

Merge branch 'sh-add-require-pat-expiry-option-17-1' into '17-1-stable-ee'

Add require_personal_access_token_expiry application setting

See merge request !161388



Merged-by: default avatarStan Hu <stanhu@gmail.com>
parents b6871a56 3b5090c7
No related branches found
No related tags found
No related merge requests found
Pipeline #1397538965 failed
Pipeline: E2E Omnibus GitLab EE

#1397560518

    Pipeline: Ruby 3.1.5 as-if-foss

    #1397544367

      Showing
      with 216 additions and 118 deletions
      ......@@ -36,6 +36,8 @@ def tokens_app_data
      end
      def expires_at_field_data
      return {} unless Gitlab::CurrentSettings.require_personal_access_token_expiry?
      {
      min_date: 1.day.from_now.iso8601
      }
      ......
      ......@@ -8,6 +8,7 @@ module ApplicationSettingsHelper
      :password_authentication_enabled_for_web?,
      :akismet_enabled?,
      :spam_check_endpoint_enabled?,
      :require_personal_access_token_expiry?,
      to: :'Gitlab::CurrentSettings.current_application_settings'
      def user_oauth_applications?
      ......@@ -523,7 +524,8 @@ def visible_attributes
      :security_txt_content,
      :allow_project_creation_for_guest_and_below,
      :downstream_pipeline_trigger_limit_per_project_user_sha,
      :asciidoc_max_includes
      :asciidoc_max_includes,
      :require_personal_access_token_expiry
      ].tap do |settings|
      unless Gitlab.com?
      settings << :deactivate_dormant_users
      ......
      ......@@ -285,7 +285,8 @@ def defaults # rubocop:disable Metrics/AbcSize
      user_contributed_projects_api_limit: 100,
      user_projects_api_limit: 300,
      user_starred_projects_api_limit: 100,
      nuget_skip_metadata_url_validation: false
      nuget_skip_metadata_url_validation: false,
      require_personal_access_token_expiry: true
      }.tap do |hsh|
      hsh.merge!(non_production_defaults) unless Rails.env.production?
      end
      ......
      ......@@ -104,7 +104,7 @@ def prefix_from_application_current_settings
      end
      def allow_expires_at_to_be_empty?
      false
      !Gitlab::CurrentSettings.require_personal_access_token_expiry?
      end
      def expires_at_before_instance_max_expiry_date
      ......
      ......@@ -41,7 +41,11 @@ def personal_access_token_params
      end
      def pat_expiration
      params[:expires_at].presence || max_expiry_date
      return params[:expires_at] if params[:expires_at].present?
      return max_expiry_date if Gitlab::CurrentSettings.require_personal_access_token_expiry?
      nil
      end
      def max_expiry_date
      ......
      ......@@ -4,13 +4,15 @@ module PersonalAccessTokens
      class RotateService
      EXPIRATION_PERIOD = 1.week
      def initialize(current_user, token, resource = nil)
      def initialize(current_user, token, resource = nil, params = {})
      @current_user = current_user
      @token = token
      @resource = resource
      @params = params.dup
      @target_user = token.user
      end
      def execute(params = {})
      def execute
      return error_response(_('token already revoked')) if token.revoked?
      response = ServiceResponse.success
      ......@@ -21,7 +23,7 @@ def execute(params = {})
      raise ActiveRecord::Rollback
      end
      response = create_access_token(params)
      response = create_access_token
      raise ActiveRecord::Rollback unless response.success?
      end
      ......@@ -31,16 +33,14 @@ def execute(params = {})
      private
      attr_reader :current_user, :token, :resource
      def create_access_token(params)
      target_user = token.user
      attr_reader :current_user, :token, :resource, :params, :target_user
      def create_access_token
      unless valid_access_level?
      return error_response(_('Not eligible to rotate token with access level higher than the user'))
      end
      new_token = target_user.personal_access_tokens.create(create_token_params(token, params))
      new_token = target_user.personal_access_tokens.create(create_token_params)
      if new_token.persisted?
      update_bot_membership(target_user, new_token.expires_at)
      ......@@ -61,10 +61,12 @@ def update_bot_membership(target_user, expires_at)
      target_user.members.update(expires_at: expires_at)
      end
      def expires_at(params)
      return params[:expires_at] if params[:expires_at]
      def expires_at
      return params[:expires_at] if params[:expires_at].present?
      return default_expiration_date if Gitlab::CurrentSettings.require_personal_access_token_expiry?
      params[:expires_at] || EXPIRATION_PERIOD.from_now.to_date
      nil
      end
      def success_response(new_token)
      ......@@ -75,12 +77,16 @@ def error_response(message)
      ServiceResponse.error(message: message)
      end
      def create_token_params(token, params)
      def create_token_params
      { name: token.name,
      previous_personal_access_token_id: token.id,
      impersonation: token.impersonation,
      scopes: token.scopes,
      expires_at: expires_at(params) }
      expires_at: expires_at }
      end
      def default_expiration_date
      EXPIRATION_PERIOD.from_now.to_date
      end
      end
      end
      ......
      ......@@ -113,7 +113,13 @@ def create_membership(resource, user, access_level)
      end
      def pat_expiration
      params[:expires_at].presence || PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now
      return params[:expires_at] if params[:expires_at].present?
      if Gitlab::CurrentSettings.require_personal_access_token_expiry?
      return PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now
      end
      nil
      end
      def log_event(token)
      ......
      ......@@ -29,6 +29,7 @@
      = f.gitlab_ui_checkbox_component :remember_me_enabled, _('Allow users to extend their session'), help_text: _("Users can select 'Remember me' on sign-in to keep their session active beyond the session duration. %{link_start}Learn more%{link_end}.").html_safe % { link_start: remember_me_help_link_start, link_end: '</a>'.html_safe }
      = render_if_exists 'admin/application_settings/git_two_factor_session_expiry', form: f
      = render 'admin/application_settings/require_personal_access_token_expiry', form: f
      = render_if_exists 'admin/application_settings/personal_access_token_expiration_policy', form: f
      = render_if_exists 'admin/application_settings/service_access_tokens_expiration_enforced', form: f
      = render_if_exists 'admin/application_settings/ssh_key_expiration_policy', form: f
      ......
      - help_link = help_page_path('user/profile/personal_access_tokens', anchor: 'create-a-service-account-personal-access-token-with-no-expiry-date')
      - help_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: help_link }
      - help_text = s_('AccessTokens|When enabled, a user will be required to enter in an expiration date when creating an access token. Changes will not affect existing token expiration dates. This setting overrides the group-level %{link_start}service account token expiration%{link_end} setting.').html_safe % { link_start: help_link_start, link_end: '</a>'.html_safe }
      .form-group
      = form.label :require_personal_access_token_expiry, s_('AccessTokens|Personal / project / group access token expiration'), class: 'label-light'
      = form.gitlab_ui_checkbox_component :require_personal_access_token_expiry, _('Require expiration date'), help_text: help_text
      # frozen_string_literal: true
      class AddRequirePatExpiryToApplicationSettings < Gitlab::Database::Migration[2.2]
      milestone '17.2'
      def change
      add_column :application_settings, :require_personal_access_token_expiry, :boolean, default: true, null: false
      end
      end
      ce07813b40454e38bdcc467c3c313573f1944848974f39d4ceea2c299714c6ac
      \ No newline at end of file
      ......@@ -5360,6 +5360,7 @@ CREATE TABLE application_settings (
      rate_limits_unauthenticated_git_http jsonb DEFAULT '{}'::jsonb NOT NULL,
      importers jsonb DEFAULT '{}'::jsonb NOT NULL,
      security_policy_scheduled_scans_max_concurrency integer DEFAULT 100 NOT NULL,
      require_personal_access_token_expiry boolean DEFAULT true NOT NULL,
      CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
      CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)),
      CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)),
      ......@@ -200,6 +200,40 @@ To set a limit on how long these sessions are valid:
      1. Fill in the **Session duration for Git operations when 2FA is enabled (minutes)** field.
      1. Select **Save changes**.
      ## Require expiration dates for new access tokens
      DETAILS:
      **Tier:** Free, Premium, Ultimate
      **Offering:** GitLab.com, Self-managed, GitLab Dedicated
      > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/470192) in GitLab 17.3.
      Prerequisites:
      - You must be an administrator.
      You can require all new access tokens to have an expiration date.
      This setting is turned on by default and applies to:
      - Project access tokens.
      - Group access tokens.
      - Personal access tokens for non-service account users.
      For personal access tokens for service accounts, use the `service_access_tokens_expiration_enforced` setting in the [Application Settings API](../../api/settings.md).
      To require expiration dates for new access tokens:
      1. On the left sidebar, at the bottom, select **Admin**.
      1. Select **Settings > General**.
      1. Expand **Account and limit**.
      1. Select the **Personal / Project / Group access token expiration** checkbox.
      1. Select **Save changes**.
      When you require expiration dates for new access tokens:
      - Users must set an expiration date that does not exceed the allowed lifetime for new access tokens.
      - To control the maximum access token lifetime, use the [**Limit the lifetime of access tokens** setting](#limit-the-lifetime-of-access-tokens).
      ## Limit the lifetime of SSH keys
      DETAILS:
      ......
      ......@@ -129,6 +129,7 @@ Example response:
      "raw_blob_request_limit": 300,
      "wiki_page_max_content_bytes": 52428800,
      "require_admin_approval_after_user_signup": false,
      "require_personal_access_token_expiry": true,
      "personal_access_token_prefix": "glpat-",
      "rate_limiting_response_text": null,
      "keep_latest_artifact": true,
      ......@@ -298,6 +299,7 @@ Example response:
      "raw_blob_request_limit": 300,
      "wiki_page_max_content_bytes": 52428800,
      "require_admin_approval_after_user_signup": false,
      "require_personal_access_token_expiry": true,
      "personal_access_token_prefix": "glpat-",
      "rate_limiting_response_text": null,
      "keep_latest_artifact": true,
      ......@@ -356,9 +358,11 @@ Example responses:
      ## List of settings that can be accessed via API calls
      > - Fields `housekeeping_full_repack_period`, `housekeeping_gc_period`, and `housekeeping_incremental_repack_period` [deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106963) in GitLab 15.8. Use `housekeeping_optimize_repository_period` instead.
      > - Parameter `allow_account_deletion` [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/412411) in GitLab 16.1.
      > - Parameter `silent_admin_exports_enabled` [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148918) in GitLab 17.0.
      > - `housekeeping_full_repack_period`, `housekeeping_gc_period`, and `housekeeping_incremental_repack_period` [deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106963) in GitLab 15.8. Use `housekeeping_optimize_repository_period` instead.
      > - `allow_account_deletion` [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/412411) in GitLab 16.1.
      > - `allow_project_creation_for_guest_and_below` [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/134625) in GitLab 16.8.
      > - `silent_admin_exports_enabled` [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148918) in GitLab 17.0.
      > - `require_personal_access_token_expiry` [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/470192) in GitLab 17.3 and backported to GitLab 17.2.2, 17.1.4, 17.0.6, and 16.11.8.
      In general, all settings are optional. Certain settings though, if enabled,
      require other settings to be set to function properly. These requirements are
      ......@@ -606,6 +610,7 @@ listed in the descriptions of the relevant settings.
      | `repository_size_limit` | integer | no | Size limit per repository (MB). Premium and Ultimate only. |
      | `repository_storages_weighted` | hash of strings to integers | no | Hash of names of taken from `gitlab.yml` to [weights](../administration/repository_storage_paths.md#configure-where-new-repositories-are-stored). New projects are created in one of these stores, chosen by a weighted random selection. |
      | `require_admin_approval_after_user_signup` | boolean | no | When enabled, any user that signs up for an account using the registration form is placed under a **Pending approval** state and has to be explicitly [approved](../administration/moderate_users.md) by an administrator. |
      | `require_personal_access_token_expiry` | boolean | no | When enabled, users must set an expiration date when creating a group or project access token, or a personal access token owned by a non-service account. |
      | `require_two_factor_authentication` | boolean | no | (**If enabled, requires:** `two_factor_grace_period`) Require all users to set up two-factor authentication. |
      | `restricted_visibility_levels` | array of strings | no | Selected levels cannot be used by non-Administrator users for groups, projects or snippets. Can take `private`, `internal` and `public` as a parameter. Default is `null` which means there is no restriction.[Changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131203) in GitLab 16.4: cannot select levels that are set as `default_project_visibility` and `default_group_visibility`. |
      | `rsa_key_restriction` | integer | no | The minimum allowed bit length of an uploaded RSA key. Default is `0` (no restriction). `-1` disables RSA keys. |
      ......
      ......@@ -378,6 +378,8 @@ To manage this risk, when you upgrade to GitLab 16.0 and later, any
      token that does not have an expiration date automatically has an expiration
      date set at one year from the date of upgrade.
      In GitLab 17.3 and later, this automatic setting of expiry on existing tokens has been reverted, and you can [disable expiration date enforcement for new access tokens](../administration/settings/account_and_limit_settings.md#require-expiration-dates-for-new-access-tokens).
      If you are not aware of when your tokens expire because the dates have changed,
      you might have unexpected authentication failures when trying to sign into GitLab
      on that date.
      ......
      ......@@ -33,12 +33,11 @@ def send_audit_event(response)
      override :pat_expiration
      def pat_expiration
      return params[:expires_at] unless params[:expires_at].blank?
      return params[:expires_at] if params[:expires_at].present?
      return unless EE::Gitlab::PersonalAccessTokens::ServiceAccountTokenValidator.new(target_user)
      .expiry_enforced?
      return unless EE::Gitlab::PersonalAccessTokens::ServiceAccountTokenValidator.new(target_user).expiry_enforced?
      super
      max_expiry_date
      end
      override :creation_permitted?
      ......
      ......@@ -8,13 +8,21 @@ module RotateService
      private
      override :expires_at
      def expires_at(params)
      expires_at = super
      max_pat_lifetime_duration = ::Gitlab::CurrentSettings.max_personal_access_token_lifetime_from_now
      def expires_at
      return params[:expires_at] if params[:expires_at].present?
      return max_pat_lifetime_duration if max_pat_lifetime_duration && max_pat_lifetime_duration < expires_at
      return unless EE::Gitlab::PersonalAccessTokens::ServiceAccountTokenValidator.new(target_user).expiry_enforced?
      expires_at
      max_pat_lifetime_duration =
      EE::Gitlab::PersonalAccessTokens::ExpiryDateCalculator.new(target_user).max_expiry_date
      min_expiry(default_expiration_date, max_pat_lifetime_duration)
      end
      def min_expiry(expiry, limit)
      return expiry unless expiry && limit
      expiry < limit ? expiry : limit
      end
      end
      end
      ......
      ......@@ -11,7 +11,9 @@ def initialize(service_account_user)
      attr_accessor :service_account_user
      def expiry_enforced?
      return true unless License.feature_available?(:service_accounts) && service_account_user.service_account?
      unless License.feature_available?(:service_accounts) && service_account_user.service_account?
      return ::Gitlab::CurrentSettings.require_personal_access_token_expiry?
      end
      if saas?
      return true unless service_account_scoped_to_group
      ......
      ......@@ -3,6 +3,8 @@
      require 'spec_helper'
      RSpec.describe PersonalAccessToken, feature_category: :system_access do
      using RSpec::Parameterized::TableSyntax
      describe 'associations' do
      subject { create(:personal_access_token) }
      ......@@ -64,7 +66,7 @@
      let(:user) { build(:user) }
      let(:personal_access_token) { build(:personal_access_token, user: user) }
      context 'with expiration policy' do
      context 'with max token lifetime configured' do
      let(:instance_level_pat_expiration_policy) { 30 }
      let(:instance_level_max_expiration_date) { Date.current + instance_level_pat_expiration_policy }
      ......@@ -81,74 +83,6 @@
      "Expiration date must be before #{max_expiration_date}"
      )
      end
      context 'expires_at is nil' do
      before do
      personal_access_token.expires_at = nil
      end
      context 'user is not service accounts' do
      it 'is invalid' do
      expect(personal_access_token).not_to be_valid
      expect(personal_access_token.errors[:expires_at]).to include("can't be blank")
      end
      end
      context 'user is service accounts' do
      before do
      stub_licensed_features(service_accounts: true)
      end
      context 'when saas', :saas do
      let(:group) { create(:group) }
      let(:user) { create(:service_account, provisioned_by_group: group) }
      context 'when namespace enforces token expiration' do
      before do
      group.namespace_settings.update!(service_access_tokens_expiration_enforced: true)
      end
      it 'is invalid' do
      expect(personal_access_token).to be_invalid
      end
      end
      context 'when namespace does not enforce token expiration' do
      before do
      group.namespace_settings.update!(service_access_tokens_expiration_enforced: false)
      end
      it 'is invalid' do
      expect(personal_access_token).to be_valid
      end
      end
      end
      context 'when self-managed' do
      let(:user) { build(:service_account) }
      context 'when application setting does not enforce token expiration' do
      before do
      stub_ee_application_setting(service_access_tokens_expiration_enforced: false)
      end
      it 'is valid' do
      expect(personal_access_token).to be_valid
      end
      end
      context 'when application setting enforces token expiration' do
      before do
      stub_ee_application_setting(service_access_tokens_expiration_enforced: true)
      end
      it 'is invalid' do
      expect(personal_access_token).to be_invalid
      end
      end
      end
      end
      end
      end
      context 'when the feature is licensed' do
      ......@@ -189,6 +123,70 @@
      end
      end
      end
      context 'with conditional presence validation on token expiry' do
      before do
      personal_access_token.expires_at = nil
      end
      context 'user is not service accounts' do
      it 'is invalid' do
      expect(personal_access_token).not_to be_valid
      expect(personal_access_token.errors[:expires_at]).to include("can't be blank")
      end
      end
      context 'user is service accounts' do
      before do
      stub_licensed_features(service_accounts: true)
      end
      context 'for group-level service accounts token expiry setting on saas', :saas do
      let(:group) { create(:group) }
      let(:user) { create(:service_account, provisioned_by_group: group) }
      where(:require_token_expiry, :require_token_expiry_for_service_accounts, :is_valid) do
      true | true | false
      true | false | true
      false | true | false
      false | false | true
      end
      with_them do
      before do
      stub_application_setting(require_personal_access_token_expiry: require_token_expiry)
      group.namespace_settings.update!(
      service_access_tokens_expiration_enforced: require_token_expiry_for_service_accounts)
      end
      it 'validates the token' do
      expect(personal_access_token.valid?).to eq(is_valid)
      end
      end
      end
      context 'for instance-level service accounts token expiry setting' do
      let(:user) { build(:service_account) }
      where(:require_token_expiry, :require_token_expiry_for_service_accounts, :is_valid) do
      true | true | false
      true | false | true
      false | true | false
      false | false | true
      end
      with_them do
      before do
      stub_application_setting(require_personal_access_token_expiry: require_token_expiry)
      stub_ee_application_setting(
      service_access_tokens_expiration_enforced: require_token_expiry_for_service_accounts)
      end
      it 'validates the token' do
      expect(personal_access_token.valid?).to eq(is_valid)
      end
      end
      end
      end
      end
      end
      describe '.pluck_names' do
      ......
      ......@@ -3,6 +3,8 @@
      require 'spec_helper'
      RSpec.describe PersonalAccessTokens::CreateService, feature_category: :system_access do
      using RSpec::Parameterized::TableSyntax
      shared_examples_for 'an unsuccessfully created token' do
      it { expect(create_token.success?).to be false }
      it { expect(create_token.message).to eq('Not permitted to create') }
      ......@@ -111,17 +113,22 @@
      context 'when expires_at is nil' do
      let(:params) { valid_params.merge(expires_at: nil) }
      context 'when service_access_tokens_expiration_enforced is false' do
      where(:require_token_expiry, :require_token_expiry_for_service_accounts, :expires_at) do
      true | true | PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now.to_date
      true | false | nil
      false | true | PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now.to_date
      false | false | nil
      end
      with_them do
      before do
      stub_ee_application_setting(service_access_tokens_expiration_enforced: false)
      stub_application_setting(require_personal_access_token_expiry: require_token_expiry)
      stub_ee_application_setting(
      service_access_tokens_expiration_enforced: require_token_expiry_for_service_accounts)
      end
      it { expect(token.expires_at).to be_nil }
      end
      it "sets expires_at to default value when setting is true" do
      expect(token.expires_at)
      .to eq max_personal_access_token_lifetime
      it 'optionally sets token expiry based on settings' do
      expect(token.expires_at).to eq(expires_at)
      end
      end
      end
      end
      ......@@ -158,19 +165,22 @@
      let(:params) { valid_params.merge(group: group, expires_at: nil) }
      context 'when saas', :saas, :enable_admin_mode do
      context 'when service_access_tokens_expiration_enforced is false' do
      where(:require_token_expiry, :require_token_expiry_for_service_accounts, :expires_at) do
      true | true | PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now.to_date
      true | false | nil
      false | true | PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now.to_date
      false | false | nil
      end
      with_them do
      before do
      group.namespace_settings.update!(service_access_tokens_expiration_enforced: false)
      stub_application_setting(require_personal_access_token_expiry: require_token_expiry)
      group.namespace_settings.update!(
      service_access_tokens_expiration_enforced: require_token_expiry_for_service_accounts)
      end
      it { expect(create_token.payload[:personal_access_token].expires_at).to be_nil }
      end
      context 'when service_access_tokens_expiration_enforced is true' do
      it {
      expect(create_token.payload[:personal_access_token].expires_at)
      .to eq max_personal_access_token_lifetime
      }
      it 'optionally sets token expiry based on settings' do
      expect(token.expires_at).to eq(expires_at)
      end
      end
      end
      ......
      0% Loading or .
      You are about to add 0 people to the discussion. Proceed with caution.
      Finish editing this message first!
      Please register or to comment