diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb index 2ea1aea1f5114f45e304306afbc34152c8266148..c368e6c8ac7fd5eb66c4c543b5cb7ffaee491406 100644 --- a/app/mailers/emails/profile.rb +++ b/app/mailers/emails/profile.rb @@ -34,3 +34,5 @@ def new_gpg_key_email(gpg_key_id) # rubocop: enable CodeReuse/ActiveRecord end end + +Emails::Profile.prepend_if_ee('EE::Emails::Profile') diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb index 7ae431eaad77fa00fc8e67f8700452e1dcf39e27..770fc2b5c8fd17529776df67933f1f43bc61a4e4 100644 --- a/app/models/personal_access_token.rb +++ b/app/models/personal_access_token.rb @@ -70,3 +70,5 @@ def self.redis_shared_state_key(user_id) "gitlab:personal_access_token:#{user_id}" end end + +PersonalAccessToken.prepend_if_ee('EE::PersonalAccessToken') diff --git a/app/views/admin/application_settings/_account_and_limit.html.haml b/app/views/admin/application_settings/_account_and_limit.html.haml index 4358365504ae15cce09a1298a568e652a78e315e..6b95c0f40c5c7a3e6d8f3737b3f546b25b24aad5 100644 --- a/app/views/admin/application_settings/_account_and_limit.html.haml +++ b/app/views/admin/application_settings/_account_and_limit.html.haml @@ -23,6 +23,9 @@ = f.label :session_expire_delay, _('Session duration (minutes)'), class: 'label-light' = f.number_field :session_expire_delay, class: 'form-control' %span.form-text.text-muted#session_expire_delay_help_block= _('GitLab restart is required to apply changes') + + = render_if_exists 'admin/application_settings/personal_access_token_expiration_policy', form: f + .form-group = f.label :user_oauth_applications, _('User OAuth applications'), class: 'label-bold' .form-check diff --git a/app/views/shared/_personal_access_tokens_form.html.haml b/app/views/shared/_personal_access_tokens_form.html.haml index ca0b473addf003af1521d4a0e43ebcd51b368bce..16f8a69263520f44f16c9e643b4581d267d2fbdf 100644 --- a/app/views/shared/_personal_access_tokens_form.html.haml +++ b/app/views/shared/_personal_access_tokens_form.html.haml @@ -18,6 +18,9 @@ .form-group.col-md-6 = f.label :expires_at, _('Expires at'), class: 'label-bold' .input-icon-wrapper + + = render_if_exists 'personal_access_tokens/callout_max_personal_access_token_lifetime' + = f.text_field :expires_at, class: "datepicker form-control", placeholder: 'YYYY-MM-DD' .form-group diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index b4be61d8a3d7137c85144b93483508ada7279951..063a3cc2b5b484ee144415ea650c88e41f4d0cbc 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -123,3 +123,5 @@ - [refresh_license_compliance_checks, 2] - [design_management_new_version, 1] - [epics, 2] + - [personal_access_tokens, 1] + diff --git a/db/migrate/20190920122420_add_max_personal_access_token_lifetime_to_application_settings.rb b/db/migrate/20190920122420_add_max_personal_access_token_lifetime_to_application_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..5a6e810dedea96f8d5cd655a50fd67210fc38c4d --- /dev/null +++ b/db/migrate/20190920122420_add_max_personal_access_token_lifetime_to_application_settings.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddMaxPersonalAccessTokenLifetimeToApplicationSettings < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def change + add_column :application_settings, :max_personal_access_token_lifetime, :integer + end +end diff --git a/db/migrate/20191112105448_add_index_on_personal_access_tokens_user_id_and_expires_at.rb b/db/migrate/20191112105448_add_index_on_personal_access_tokens_user_id_and_expires_at.rb new file mode 100644 index 0000000000000000000000000000000000000000..1c1dc31ff231c8760ce9c2f9109a416ccfa9c279 --- /dev/null +++ b/db/migrate/20191112105448_add_index_on_personal_access_tokens_user_id_and_expires_at.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddIndexOnPersonalAccessTokensUserIdAndExpiresAt < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'index_pat_on_user_id_and_expires_at' + + disable_ddl_transaction! + + def up + add_concurrent_index :personal_access_tokens, [:user_id, :expires_at], name: INDEX_NAME, using: :btree + end + + def down + remove_concurrent_index_by_name :personal_access_tokens, INDEX_NAME + end +end diff --git a/db/schema.rb b/db/schema.rb index c33223f8b59cc7177f315b32d9b174274989f176..e63db981b298097049f199bb7f189c0a21319ef2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -325,6 +325,7 @@ t.string "encrypted_asset_proxy_secret_key_iv" t.string "static_objects_external_storage_url", limit: 255 t.string "static_objects_external_storage_auth_token", limit: 255 + t.integer "max_personal_access_token_lifetime" t.boolean "throttle_protected_paths_enabled", default: false, null: false t.integer "throttle_protected_paths_requests_per_period", default: 10, null: false t.integer "throttle_protected_paths_period_in_seconds", default: 60, null: false @@ -2908,6 +2909,7 @@ t.boolean "impersonation", default: false, null: false t.string "token_digest" t.index ["token_digest"], name: "index_personal_access_tokens_on_token_digest", unique: true + t.index ["user_id", "expires_at"], name: "index_pat_on_user_id_and_expires_at" t.index ["user_id"], name: "index_personal_access_tokens_on_user_id" end diff --git a/doc/api/settings.md b/doc/api/settings.md index ad9ffcbf872397995c23affbf03334fecec113fa..185cce6353ee6df9b256d233a5d51983802fcc4e 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -269,6 +269,7 @@ are listed in the descriptions of the relevant settings. | `max_artifacts_size` | integer | no | Maximum artifacts size in MB | | `max_attachment_size` | integer | no | Limit attachment size in MB | | `max_pages_size` | integer | no | Maximum size of pages repositories in MB | +| `max_personal_access_token_lifetime` | integer | no | **(ULTIMATE ONLY)** Maximum allowable lifetime for personal access tokens in days | | `metrics_enabled` | boolean | no | (**If enabled, requires:** `metrics_host`, `metrics_method_call_threshold`, `metrics_packet_size`, `metrics_pool_size`, `metrics_port`, `metrics_sample_interval` and `metrics_timeout`) Enable influxDB metrics. | | `metrics_host` | string | required by: `metrics_enabled` | InfluxDB host. | | `metrics_method_call_threshold` | integer | required by: `metrics_enabled` | A method call is only tracked when it takes longer than the given amount of milliseconds. | diff --git a/doc/user/admin_area/settings/account_and_limit_settings.md b/doc/user/admin_area/settings/account_and_limit_settings.md index e443127a8a043563587fe03319fedf455432aa71..9d82b3b4292030da4175c2904e29db5257ef4bb0 100644 --- a/doc/user/admin_area/settings/account_and_limit_settings.md +++ b/doc/user/admin_area/settings/account_and_limit_settings.md @@ -84,3 +84,35 @@ add the line below to `/etc/gitlab/gitlab.rb` before increasing the max attachme ``` nginx['client_max_body_size'] = "200m" ``` + +## Limiting lifetime of personal access tokens **(ULTIMATE ONLY)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/3649) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 12.6. + +Users can optionally specify an expiration date for +[personal access tokens](../../profile/personal_access_tokens.md). +This expiration date is not a requirement, and can be set to any arbitrary date. + +Since personal access tokens are the only token needed for programmatic access to GitLab, +organizations with security requirements may want to enforce more protection to require +regular rotation of these tokens. + +### Setting a limit + +Only a GitLab administrator can set a limit. Leaving it empty means +there are no restrictions. + +To set a limit on how long personal access tokens are valid: + +1. Navigate to **Admin Area > Settings > General**. +1. Expand the **Account and limit** section. +1. Fill in the **Maximun allowable lifetime for personal access tokens (days)** field. +1. Click **Save changes**. + +Once a lifetime for personal access tokens is set, GitLab will: + +- Apply the lifetime for new personal access tokens, and require users to set an expiration date + and a date no later than the allowed lifetime. +- After three hours, revoke old tokens with no expiration date or with a lifetime longer than the + allowed lifetime. Three hours is given to allow administrators to change the allowed lifetime, + or remove it, before revocation takes place. diff --git a/ee/app/helpers/ee/application_settings_helper.rb b/ee/app/helpers/ee/application_settings_helper.rb index 3823da8a0dff668ed1b095f2ba2b122f8efa7a38..4e270f7354693bf1a55031a87ead4054ee40c628 100644 --- a/ee/app/helpers/ee/application_settings_helper.rb +++ b/ee/app/helpers/ee/application_settings_helper.rb @@ -37,6 +37,7 @@ def visible_attributes :geo_node_allowed_ips, :help_text, :lock_memberships_to_ldap, + :max_personal_access_token_lifetime, :pseudonymizer_enabled, :repository_size_limit, :shared_runners_minutes, diff --git a/ee/app/helpers/personal_access_tokens_helper.rb b/ee/app/helpers/personal_access_tokens_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..949d7c06aee3a1cb09b31746c07481a7b890c4d9 --- /dev/null +++ b/ee/app/helpers/personal_access_tokens_helper.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module PersonalAccessTokensHelper + def personal_access_token_expiration_policy_licensed? + License.feature_available?(:personal_access_token_expiration_policy) + end + + def personal_access_token_expiration_policy_enabled? + Gitlab::CurrentSettings.max_personal_access_token_lifetime && personal_access_token_expiration_policy_licensed? + end +end diff --git a/ee/app/mailers/ee/emails/profile.rb b/ee/app/mailers/ee/emails/profile.rb new file mode 100644 index 0000000000000000000000000000000000000000..1354a4cfd339b01832ee6ba360c951550f07c86c --- /dev/null +++ b/ee/app/mailers/ee/emails/profile.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module EE + module Emails + module Profile + def policy_revoked_personal_access_tokens_email(user, revoked_token_names) + return unless user && revoked_token_names + + @user = user + @revoked_token_names = revoked_token_names + @target_url = profile_personal_access_tokens_url + + ::Gitlab::I18n.with_locale(@user.preferred_language) do + mail(to: user.notification_email, subject: subject(_("One or more of you personal access tokens were revoked"))) + end + end + end + end +end diff --git a/ee/app/models/ee/application_setting.rb b/ee/app/models/ee/application_setting.rb index bba54a8bcfc2e9a03e7fb9059a6284a2920fcae2..29a901211d000759f71867d766b088944531d760 100644 --- a/ee/app/models/ee/application_setting.rb +++ b/ee/app/models/ee/application_setting.rb @@ -60,6 +60,12 @@ module ApplicationSetting validates :required_instance_ci_template, presence: true, allow_nil: true validate :check_geo_node_allowed_ips + + validates :max_personal_access_token_lifetime, + allow_blank: true, + numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: 365 } + + after_commit :update_personal_access_tokens_lifetime, if: :saved_change_to_max_personal_access_token_lifetime? end class_methods do @@ -77,6 +83,7 @@ def defaults elasticsearch_url: ENV['ELASTIC_URL'] || 'http://localhost:9200', email_additional_text: nil, lock_memberships_to_ldap: false, + max_personal_access_token_lifetime: nil, mirror_capacity_threshold: Settings.gitlab['mirror_capacity_threshold'], mirror_max_capacity: Settings.gitlab['mirror_max_capacity'], mirror_max_delay: Settings.gitlab['mirror_max_delay'], @@ -231,8 +238,18 @@ def instance_review_permitted? users_count >= INSTANCE_REVIEW_MIN_USERS end + def max_personal_access_token_lifetime_from_now + max_personal_access_token_lifetime&.days&.from_now + end + private + def update_personal_access_tokens_lifetime + return unless max_personal_access_token_lifetime.present? && License.feature_available?(:personal_access_token_expiration_policy) + + ::PersonalAccessTokens::UpdateLifetimeService.new.execute + end + def mirror_max_delay_in_minutes ::Gitlab::Mirror.min_delay_upper_bound / 60 end diff --git a/ee/app/models/ee/personal_access_token.rb b/ee/app/models/ee/personal_access_token.rb new file mode 100644 index 0000000000000000000000000000000000000000..7e3a159e59ed13bfc00c46899f543e40e560edc6 --- /dev/null +++ b/ee/app/models/ee/personal_access_token.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module EE + # PersonalAccessToken EE mixin + # + # This module is intended to encapsulate EE-specific model logic + # and be prepended in the `PersonalAccessToken` model + module PersonalAccessToken + extend ActiveSupport::Concern + + prepended do + include ::Gitlab::Utils::StrongMemoize + include FromUnion + + scope :with_no_expires_at, -> { where(revoked: false, expires_at: nil) } + scope :with_expires_at_after, ->(max_lifetime) { where(revoked: false).where('expires_at > ?', max_lifetime) } + + with_options if: :max_personal_access_token_lifetime_enabled? do + validates :expires_at, presence: true + validate :expires_at_before_max_lifetime + end + end + + class_methods do + def pluck_names + pluck(:name) + end + + def with_invalid_expires_at(max_lifetime, limit = 1_000) + from_union( + [ + with_no_expires_at.limit(limit), + with_expires_at_after(max_lifetime).limit(limit) + ] + ) + end + end + + private + + def max_expiration_date + strong_memoize(:max_expiration_date) do + ::Gitlab::CurrentSettings.max_personal_access_token_lifetime_from_now + end + end + + def max_personal_access_token_lifetime_enabled? + max_expiration_date && License.feature_available?(:personal_access_token_expiration_policy) + end + + def expires_at_before_max_lifetime + return if expires_at.blank? + + errors.add(:expires_at, :invalid) if expires_at > max_expiration_date + end + end +end diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index a4aa40191720b63a4d3da4f290b177596aa5e404..acb03ad4c061a53a9f32f55adaff9b1a4a88d99d 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -73,6 +73,10 @@ module User scope :bots, -> { where.not(bot_type: nil) } scope :humans, -> { where(bot_type: nil) } + scope :with_invalid_expires_at_tokens, ->(expiration_date) do + where(id: ::PersonalAccessToken.with_invalid_expires_at(expiration_date).select(:user_id)) + end + accepts_nested_attributes_for :namespace enum roadmap_layout: { weeks: 1, months: 4, quarters: 12 } diff --git a/ee/app/models/license.rb b/ee/app/models/license.rb index dbf8756c9517f97daa665e17d82e749afac7508b..40f24df3f003e5b4a6c4a89abb0afbfc6ae788e3 100644 --- a/ee/app/models/license.rb +++ b/ee/app/models/license.rb @@ -113,6 +113,7 @@ class License < ApplicationRecord insights licenses_list license_management + personal_access_token_expiration_policy pod_logs prometheus_alerts pseudonymizer diff --git a/ee/app/services/personal_access_tokens/revoke_invalid_tokens.rb b/ee/app/services/personal_access_tokens/revoke_invalid_tokens.rb new file mode 100644 index 0000000000000000000000000000000000000000..7db9e7db3120b332a317a3d19c805f58852e2a5c --- /dev/null +++ b/ee/app/services/personal_access_tokens/revoke_invalid_tokens.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module PersonalAccessTokens + class RevokeInvalidTokens + def initialize(user, expiration_date) + @user = user + @expiration_date = expiration_date + end + + def execute + return unless ::Feature.enabled?(:personal_access_token_expiration_policy, default_enabled: true) + return unless expiration_date && user_affected? + + notify_user + + revoke_tokens! + end + + private + + attr_reader :user, :expiration_date + + def user_affected? + user && affected_tokens.any? + end + + def notify_user + return unless user.can?(:receive_notifications) + + mailer.policy_revoked_personal_access_tokens_email(user, affected_tokens.pluck_names).deliver_later + end + + def mailer + Notify + end + + def affected_tokens + @affected_tokens ||= user.personal_access_tokens.with_invalid_expires_at(expiration_date) + end + + def revoke_tokens! + personal_access_tokens.with_no_expires_at.update_all(revoked: true) + personal_access_tokens.with_expires_at_after(expiration_date).update_all(revoked: true) + end + + def personal_access_tokens + @personal_access_tokens ||= user.personal_access_tokens + end + end +end diff --git a/ee/app/services/personal_access_tokens/update_lifetime_service.rb b/ee/app/services/personal_access_tokens/update_lifetime_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..3a9e1e9f72214df69c0b168a7c900b1be1da6b2e --- /dev/null +++ b/ee/app/services/personal_access_tokens/update_lifetime_service.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module PersonalAccessTokens + class UpdateLifetimeService + include ExclusiveLeaseGuard + + DEFAULT_LEASE_TIMEOUT = 3.hours.to_i + + def execute + try_obtain_lease do + ::PersonalAccessTokens::PolicyWorker.perform_in(DEFAULT_LEASE_TIMEOUT) + end + end + + private + + # Used by ExclusiveLeaseGuard + def lease_timeout + DEFAULT_LEASE_TIMEOUT + end + + # Used by ExclusiveLeaseGuard + # Overriding value as we never release the lease + # before the timeout in order to prevent multiple + # PersonalAccessTokens::PolicyWorker to start in + # a short span of time + def lease_release? + false + end + end +end diff --git a/ee/app/views/admin/application_settings/_personal_access_token_expiration_policy.html.haml b/ee/app/views/admin/application_settings/_personal_access_token_expiration_policy.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..62d3611b2f24d701b91460a664660d970db36e33 --- /dev/null +++ b/ee/app/views/admin/application_settings/_personal_access_token_expiration_policy.html.haml @@ -0,0 +1,7 @@ +- return unless personal_access_token_expiration_policy_licensed? +- form = local_assigns.fetch(:form) + +.form-group + = form.label :max_personal_access_token_lifetime, _('Maximum allowable lifetime for personal access token (days)'), class: 'label-light' + = form.number_field :max_personal_access_token_lifetime, class: 'form-control input-xs' + %span.form-text.text-muted#max_personal_access_token_lifetime= _('Leave blank for no limit. Once set, existing personal access tokens may be revoked') diff --git a/ee/app/views/notify/policy_revoked_personal_access_tokens_email.html.haml b/ee/app/views/notify/policy_revoked_personal_access_tokens_email.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..d2e4ea615473ad44de07791e8ca1bceb74537987 --- /dev/null +++ b/ee/app/views/notify/policy_revoked_personal_access_tokens_email.html.haml @@ -0,0 +1,7 @@ +%p + = _('Hi %{username}!') % { username: sanitize_name(@user.name) } +%p + = n_('The following personal access token: %{token_names} was revoked, because a new policy to expire personal access tokens were set.', 'The following personal access tokens: %{token_names} were revoked, because a new policy to expire personal access tokens were set.', @revoked_token_names.size) % { token_names: @revoked_token_names.to_sentence } +%p + - pat_link_start = ''.html_safe % { url: @target_url } + = _('You can create new ones at your %{pat_link_start}Personal Access Tokens%{pat_link_end} settings').html_safe % { pat_link_start: pat_link_start, pat_link_end: ''.html_safe } diff --git a/ee/app/views/notify/policy_revoked_personal_access_tokens_email.text.erb b/ee/app/views/notify/policy_revoked_personal_access_tokens_email.text.erb new file mode 100644 index 0000000000000000000000000000000000000000..68ff25700e96b4759690206809d544dffe05eb3e --- /dev/null +++ b/ee/app/views/notify/policy_revoked_personal_access_tokens_email.text.erb @@ -0,0 +1,5 @@ +<%= _('Hi %{username}!') % { username: sanitize_name(@user.name) } %> + +<%= n_('The following personal access token: %{token_names} was revoked, because a new policy to expire personal access tokens were set.', 'The following personal access tokens: %{token_names} were revoked, because a new policy to expire personal access tokens were set.', @revoked_token_names.size) % { token_names: @revoked_token_names.to_sentence } %> + +<%= _('You can create new ones at your Personal Access Tokens settings %{pat_link}') % { pat_link: @target_url } %> \ No newline at end of file diff --git a/ee/app/views/personal_access_tokens/_callout_max_personal_access_token_lifetime.html.haml b/ee/app/views/personal_access_tokens/_callout_max_personal_access_token_lifetime.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..d087f299b908d1c1c857fd375a76fc1b267f7ad6 --- /dev/null +++ b/ee/app/views/personal_access_tokens/_callout_max_personal_access_token_lifetime.html.haml @@ -0,0 +1,4 @@ +- return unless personal_access_token_expiration_policy_enabled? + +.bs-callout.bs-callout-danger + = _('Maximum lifetime allowable for Personal Access Tokens is active, your expire date must be set before %{maximum_allowable_date}.') % { maximum_allowable_date: ::Gitlab::CurrentSettings.max_personal_access_token_lifetime_from_now } diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index 55be22a1de4f951667c545423d9d07663ff24b80..5594f1fc2dec4fe5973452f1b502bb7a6cf8d923 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -58,6 +58,8 @@ - jira_connect:jira_connect_sync_branch - jira_connect:jira_connect_sync_merge_request +- personal_access_tokens:personal_access_tokens_policy + - admin_emails - create_github_webhook - design_management_new_version diff --git a/ee/app/workers/personal_access_tokens/policy_worker.rb b/ee/app/workers/personal_access_tokens/policy_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..69390e3eda835612b0c7f14d57e4d19a5edf1aaf --- /dev/null +++ b/ee/app/workers/personal_access_tokens/policy_worker.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module PersonalAccessTokens + class PolicyWorker + include ApplicationWorker + + queue_namespace :personal_access_tokens + feature_category :authentication_and_authorization + + def perform + expiration_date = ::Gitlab::CurrentSettings.max_personal_access_token_lifetime_from_now + + return unless expiration_date + + User.with_invalid_expires_at_tokens(expiration_date).find_each do |user| + PersonalAccessTokens::RevokeInvalidTokens.new(user, expiration_date).execute + end + end + end +end diff --git a/ee/changelogs/unreleased/3649-pat-expiry-policy.yml b/ee/changelogs/unreleased/3649-pat-expiry-policy.yml new file mode 100644 index 0000000000000000000000000000000000000000..e905108740e0915f2a779e2dc7b222cbd61a85ec --- /dev/null +++ b/ee/changelogs/unreleased/3649-pat-expiry-policy.yml @@ -0,0 +1,5 @@ +--- +title: Add Personal access token expiry policy +merge_request: 17344 +author: +type: added diff --git a/ee/spec/helpers/personal_access_tokens_helper_spec.rb b/ee/spec/helpers/personal_access_tokens_helper_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b3a2d78112b48e529a60dacddc9abd8b4cce3cc8 --- /dev/null +++ b/ee/spec/helpers/personal_access_tokens_helper_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe PersonalAccessTokensHelper do + describe '#personal_access_token_expiration_policy_licensed?' do + subject { helper.personal_access_token_expiration_policy_licensed? } + + context 'when is not licensed' do + before do + stub_licensed_features(personal_access_token_expiration_policy: false) + end + + it { is_expected.to be_falsey } + end + + context 'when is licensed' do + before do + stub_licensed_features(personal_access_token_expiration_policy: true) + end + + it { is_expected.to be_truthy } + end + end + + describe '#personal_access_token_expiration_policy_enabled?' do + subject { helper.personal_access_token_expiration_policy_enabled? } + + context 'when is licensed and used' do + before do + stub_licensed_features(personal_access_token_expiration_policy: true) + stub_application_setting(max_personal_access_token_lifetime: 1) + end + + it { is_expected.to be_truthy } + end + + context 'when is not licensed' do + before do + stub_licensed_features(personal_access_token_expiration_policy: false) + end + + it { is_expected.to be_falsey } + end + + context 'when is licensed but not used' do + before do + stub_licensed_features(personal_access_token_expiration_policy: true) + stub_application_setting(max_personal_access_token_lifetime: nil) + end + + it { is_expected.to be_falsey } + end + end +end diff --git a/ee/spec/mailers/ee/emails/profile_spec.rb b/ee/spec/mailers/ee/emails/profile_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..48c4bbf35515375dcd41ee46a22553417fd24123 --- /dev/null +++ b/ee/spec/mailers/ee/emails/profile_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'email_spec' + +describe EE::Emails::Profile do + include EmailSpec::Matchers + + describe '#policy_revoked_personal_access_tokens_email' do + let_it_be(:user) { create(:user) } + let(:token_names) { %w(name1 name2) } + + subject { Notify.policy_revoked_personal_access_tokens_email(user, token_names) } + + it 'is sent to the user' do + is_expected.to deliver_to user.email + end + + it 'has the correct subject' do + is_expected.to have_subject /^One or more of you personal access tokens were revoked$/i + end + + it 'mentions the access tokens were revoke' do + is_expected.to have_body_text /The following personal access tokens: name1 and name2 were revoked/ + end + + it 'includes a link to personal access tokens page' do + is_expected.to have_body_text /#{profile_personal_access_tokens_path}/ + end + + it 'includes the email reason' do + is_expected.to have_body_text /You're receiving this email because of your account on localhost/ + end + end +end diff --git a/ee/spec/models/application_setting_spec.rb b/ee/spec/models/application_setting_spec.rb index 5bdc506106be901b0a245e120ad6f5a77f3e7e71..6a8b7ef57f3760bb039e817c9190698992460b88 100644 --- a/ee/spec/models/application_setting_spec.rb +++ b/ee/spec/models/application_setting_spec.rb @@ -46,6 +46,15 @@ it { is_expected.not_to allow_value(" ").for(:required_instance_ci_template) } it { is_expected.to allow_value("template_name").for(:required_instance_ci_template) } + it { is_expected.to allow_value(1).for(:max_personal_access_token_lifetime) } + it { is_expected.to allow_value(nil).for(:max_personal_access_token_lifetime) } + it { is_expected.to allow_value(10).for(:max_personal_access_token_lifetime) } + it { is_expected.to allow_value(365).for(:max_personal_access_token_lifetime) } + it { is_expected.not_to allow_value("value").for(:max_personal_access_token_lifetime) } + it { is_expected.not_to allow_value(2.5).for(:max_personal_access_token_lifetime) } + it { is_expected.not_to allow_value(-5).for(:max_personal_access_token_lifetime) } + it { is_expected.not_to allow_value(366).for(:max_personal_access_token_lifetime) } + describe 'when additional email text is enabled' do before do stub_licensed_features(email_additional_text: true) @@ -416,4 +425,65 @@ def expect_is_es_licensed end end end + + describe '#max_personal_access_token_lifetime_from_now' do + subject { setting.max_personal_access_token_lifetime_from_now } + + let(:days_from_now) { nil } + + before do + stub_application_setting(max_personal_access_token_lifetime: days_from_now) + end + + context 'when max_personal_access_token_lifetime is defined' do + let(:days_from_now) { 30 } + + it 'is a date time' do + expect(subject).to be_a Time + end + + it 'is in the future' do + expect(subject).to be > Time.zone.now + end + + it 'is in days_from_now' do + expect(subject.to_date - Date.today).to eq days_from_now + end + end + + context 'when max_personal_access_token_lifetime is nil' do + it 'is nil' do + expect(subject).to be_nil + end + end + end + + describe 'updates to max_personal_access_token_lifetime' do + context 'without personal_access_token_expiration_policy licensed' do + before do + stub_licensed_features(personal_access_token_expiration_policy: false) + end + + it "doesn't call the update lifetime service" do + expect(::PersonalAccessTokens::UpdateLifetimeService).not_to receive(:new) + + setting.save + end + end + + context 'with personal_access_token_expiration_policy licensed' do + before do + setting.max_personal_access_token_lifetime = 30 + stub_licensed_features(personal_access_token_expiration_policy: true) + end + + it 'executes the update lifetime service' do + expect_next_instance_of(::PersonalAccessTokens::UpdateLifetimeService) do |service| + expect(service).to receive(:execute) + end + + setting.save + end + end + end end diff --git a/ee/spec/models/ee/personal_access_token_spec.rb b/ee/spec/models/ee/personal_access_token_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b40031beb8d601789012b44bd5442da7537c7d35 --- /dev/null +++ b/ee/spec/models/ee/personal_access_token_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe PersonalAccessToken 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!(:pat) { create(:personal_access_token, expires_at: expiration_date) } + + describe 'with_expires_at_after' do + subject { described_class.with_expires_at_after(2.days.from_now) } + + let(:expiration_date) { 3.days.from_now } + + it 'includes the tokens with higher than the lifetime expires_at value' do + expect(subject).to contain_exactly(pat) + end + + it "doesn't contain expired tokens" do + expect(subject).not_to include(expired_token) + end + + it "doesn't contain tokens within the expiration time" do + expect(subject).not_to include(valid_token) + end + end + + describe 'with_no_expires_at' do + subject { described_class.with_expires_at_after(2.days.from_now) } + + let(:expiration_date) { nil } + + it 'includes the tokens with nil expires_at' do + expect(described_class.with_no_expires_at).to contain_exactly(pat) + end + + it "doesn't contain expired tokens" do + expect(subject).not_to include(expired_token) + end + + it "doesn't contain tokens within the expiration time" do + expect(subject).not_to include(valid_token) + end + end + end + + describe 'validations' do + let(:personal_access_token) { build(:personal_access_token) } + + it 'allows to define expires_at' do + personal_access_token.expires_at = 1.day.from_now + + expect(personal_access_token).to be_valid + end + + it "allows to don't define expires_at" do + personal_access_token.expires_at = nil + + expect(personal_access_token).to be_valid + end + + context 'with expiration policy' do + let(:pat_expiration_policy) { 30 } + let(:max_expiration_date) { pat_expiration_policy.days.from_now } + + before do + stub_ee_application_setting(max_personal_access_token_lifetime: pat_expiration_policy) + end + + context 'when the feature is licensed' do + before do + stub_licensed_features(personal_access_token_expiration_policy: true) + end + + it 'requires to be less or equal than the max_personal_access_token_lifetime' 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('is invalid') + 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 + + context 'when the feature is not available' do + before do + stub_licensed_features(personal_access_token_expiration_policy: false) + end + + it 'allows to be after the max_personal_access_token_lifetime' do + personal_access_token.expires_at = max_expiration_date + 1.day + + expect(personal_access_token).to be_valid + end + end + end + end + + describe '.pluck_names' do + it 'returns the names of the tokens' do + pat1 = create(:personal_access_token) + pat2 = create(:personal_access_token) + + expect(described_class.pluck_names).to contain_exactly(pat1.name, pat2.name) + end + end + + describe '.with_invalid_expires_at' do + subject { described_class.with_invalid_expires_at(2.days.from_now) } + + it 'includes the tokens with invalid expires_at' do + pat_with_no_expires_at = create(:personal_access_token, expires_at: nil) + pat_with_longer_expires_at = create(:personal_access_token, expires_at: 3.days.from_now) + + expect(subject).to contain_exactly(pat_with_no_expires_at, pat_with_longer_expires_at) + end + + it "doesn't include valid tokens" do + valid_token = create(:personal_access_token, expires_at: 1.day.from_now) + + expect(subject).not_to include(valid_token) + end + + it "doesn't include revoked tokens" do + revoked_token = create(:personal_access_token, revoked: true) + + expect(subject).not_to include(revoked_token) + end + + it "doesn't include expired tokens" do + expired_token = create(:personal_access_token, expires_at: 1.day.ago) + + expect(subject).not_to include(expired_token) + end + end +end diff --git a/ee/spec/models/user_spec.rb b/ee/spec/models/user_spec.rb index 57cfe05b824f508593ec83275b877e0736179de7..b14be6ebcc44f978a7c4b44706f4e9e0cbde0e2b 100644 --- a/ee/spec/models/user_spec.rb +++ b/ee/spec/models/user_spec.rb @@ -83,6 +83,19 @@ expect(described_class.bots).to match_array([bot]) end end + + describe 'with_invalid_expires_at_tokens' do + it 'only includes users with invalid tokens' do + valid_pat = create(:personal_access_token, expires_at: 7.days.from_now) + invalid_pat1 = create(:personal_access_token, expires_at: nil) + invalid_pat2 = create(:personal_access_token, expires_at: 20.days.from_now) + + users_with_invalid_tokens = described_class.with_invalid_expires_at_tokens(15.days.from_now) + + expect(users_with_invalid_tokens).to contain_exactly(invalid_pat1.user, invalid_pat2.user) + expect(users_with_invalid_tokens).not_to include valid_pat.user + end + end end describe '.find_by_smartcard_identity' do diff --git a/ee/spec/services/personal_access_tokens/revoke_invalid_tokens_spec.rb b/ee/spec/services/personal_access_tokens/revoke_invalid_tokens_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..82550951b2ea15dc60f4698e8a6dee565a8f2fd9 --- /dev/null +++ b/ee/spec/services/personal_access_tokens/revoke_invalid_tokens_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe PersonalAccessTokens::RevokeInvalidTokens do + subject(:service) { described_class.new(user, expiration_date) } + + describe '#execute' do + let(:expiration_date) { 10.days.from_now } + let_it_be(:user) { create(:user) } + let_it_be(:pat) { create(:personal_access_token, expires_at: 5.days.from_now, user: user) } + let_it_be(:invalid_pat1) { create(:personal_access_token, expires_at: nil, user: user) } + let_it_be(:invalid_pat2) { create(:personal_access_token, expires_at: 20.days.from_now, user: user) } + + context 'with a valid user and expiration date' do + context 'with user tokens that will be revoked' do + it 'calls mailer to send an email notifying the user' do + expect(Notify).to receive(:policy_revoked_personal_access_tokens_email).and_call_original + service.execute + end + + it "revokes invalid user's tokens" do + service.execute + + expect(pat.reload).not_to be_revoked + expect(invalid_pat1.reload).to be_revoked + expect(invalid_pat2.reload).to be_revoked + end + + context 'user optout for notifications' do + before do + allow(user).to receive(:can?).and_return(false) + end + + it "doesn't call mailer to send a notification" do + expect(Notify).not_to receive(:policy_revoked_personal_access_tokens_email) + service.execute + end + end + end + end + + context 'with no user' do + let(:user) { nil } + + it "doesn't call mailer to send an email notifying the user" do + expect(Notify).not_to receive(:policy_revoked_personal_access_tokens_email) + service.execute + end + + it "doesn't revoke user's tokens" do + expect { service.execute }.not_to change { pat.reload.revoked } + end + end + + context 'with no expiration date' do + let(:expiration_date) { nil } + + it "doesn't call mailer to send an email notifying the user" do + expect(Notify).not_to receive(:policy_revoked_personal_access_tokens_email) + service.execute + end + + it "doesn't revoke user's tokens" do + expect { service.execute }.not_to change { pat.reload.revoked } + end + end + + context 'when the feature flag for personal access token policy is disabled' do + before do + stub_feature_flags(personal_access_token_expiration_policy: false) + end + + it "doesn't call mailer to send an email notifying the user" do + expect(Notify).not_to receive(:policy_revoked_personal_access_tokens_email) + service.execute + end + + it "doesn't revoke user's tokens" do + expect { service.execute }.not_to change { pat.reload.revoked } + end + end + end +end diff --git a/ee/spec/services/personal_access_tokens/update_lifetime_service_spec.rb b/ee/spec/services/personal_access_tokens/update_lifetime_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..e8b7a0fd40770b35ace0f3faa84e5fbd5dd97571 --- /dev/null +++ b/ee/spec/services/personal_access_tokens/update_lifetime_service_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe PersonalAccessTokens::UpdateLifetimeService do + describe '#execute', :clean_gitlab_redis_shared_state do + include ExclusiveLeaseHelpers + + let(:lease_key) { 'personal_access_tokens/update_lifetime_service' } + + context 'when we can obtain the lease' do + it 'schedules the worker' do + stub_exclusive_lease(lease_key, timeout: described_class::DEFAULT_LEASE_TIMEOUT) + + expect(::PersonalAccessTokens::PolicyWorker).to receive(:perform_in).once + + subject.execute + end + end + + context "when we can't obtain the lease" do + it 'does not schedule the worker' do + stub_exclusive_lease_taken(lease_key, timeout: described_class::DEFAULT_LEASE_TIMEOUT) + + expect(::PersonalAccessTokens::PolicyWorker).not_to receive(:perform_in) + + subject.execute + end + end + end +end diff --git a/ee/spec/workers/personal_access_tokens/policy_worker_spec.rb b/ee/spec/workers/personal_access_tokens/policy_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..05b8bba40d558089e452c377d7e4129711c3be09 --- /dev/null +++ b/ee/spec/workers/personal_access_tokens/policy_worker_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe PersonalAccessTokens::PolicyWorker, type: :worker do + describe '#perform' do + let(:limit) { 7 } + let!(:pat) { create(:personal_access_token, expires_at: expire_at) } + + before do + stub_application_setting(max_personal_access_token_lifetime: limit) + end + + context "when a token doesn't have an expiration time" do + let(:expire_at) { nil } + + it 'enforces the policy on tokens' do + expect { subject.perform }.to change { pat.reload.revoked }.from(false).to(true) + end + end + + context 'when a token expires after the given time' do + let(:expire_at) { 8.days.from_now.to_date } + + it 'enforces the policy on tokens' do + expect { subject.perform }.to change { pat.reload.revoked }.from(false).to(true) + end + end + + context 'when a token is valid' do + let(:expire_at) { 5.days.from_now.to_date } + + it "doesn't revoked valid tokens" do + expect { subject.perform }.not_to change { pat.reload.revoked } + end + end + + context 'when limit is nil' do + let(:limit) { nil } + let(:expire_at) { 1.day.from_now } + + it "doesn't revoked valid tokens" do + expect { subject.perform }.not_to change { pat.reload.revoked } + end + + it "doesn't call the revoke invalid service" do + expect(PersonalAccessTokens::RevokeInvalidTokens).not_to receive(:new) + + subject.perform + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5b00a1629d53f9665cd90a4c6f0915dc986bb082..8e459266e7c6a61d542850f3556386dd9f3285d6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9107,6 +9107,9 @@ msgstr "" msgid "Helps reduce request volume for protected paths" msgstr "" +msgid "Hi %{username}!" +msgstr "" + msgid "Hide archived projects" msgstr "" @@ -10253,6 +10256,9 @@ msgstr "" msgid "Leave Admin Mode" msgstr "" +msgid "Leave blank for no limit. Once set, existing personal access tokens may be revoked" +msgstr "" + msgid "Leave edit mode? All unsaved changes will be lost." msgstr "" @@ -10743,6 +10749,9 @@ msgstr "" msgid "Max seats used" msgstr "" +msgid "Maximum allowable lifetime for personal access token (days)" +msgstr "" + msgid "Maximum artifacts size (MB)" msgstr "" @@ -10761,6 +10770,9 @@ msgstr "" msgid "Maximum job timeout has a value which could not be accepted" msgstr "" +msgid "Maximum lifetime allowable for Personal Access Tokens is active, your expire date must be set before %{maximum_allowable_date}." +msgstr "" + msgid "Maximum number of comments exceeded" msgstr "" @@ -11964,6 +11976,9 @@ msgstr[1] "" msgid "One or more groups that you don't have access to." msgstr "" +msgid "One or more of you personal access tokens were revoked" +msgstr "" + msgid "One or more of your Bitbucket projects cannot be imported into GitLab directly because they use Subversion or Mercurial for version control, rather than Git." msgstr "" @@ -17415,6 +17430,11 @@ msgstr "" msgid "The following items will be exported:" msgstr "" +msgid "The following personal access token: %{token_names} was revoked, because a new policy to expire personal access tokens were set." +msgid_plural "The following personal access tokens: %{token_names} were revoked, because a new policy to expire personal access tokens were set." +msgstr[0] "" +msgstr[1] "" + msgid "The fork relationship has been removed." msgstr "" @@ -20084,6 +20104,12 @@ msgstr "" msgid "You can create files directly in GitLab using one of the following options." msgstr "" +msgid "You can create new ones at your %{pat_link_start}Personal Access Tokens%{pat_link_end} settings" +msgstr "" + +msgid "You can create new ones at your Personal Access Tokens settings %{pat_link}" +msgstr "" + msgid "You can easily contribute to them by requesting to join these groups." msgstr ""