Merge branch 'feat/token-prefix' into 'master'

feat: add token prefix

See merge request gitlab-org/gitlab!20968

app/helpers/application_settings_helper.rb

@@ -255,6 +255,7 @@ module ApplicationSettingsHelper
       :password_authentication_enabled_for_git,
       :performance_bar_allowed_group_path,
       :performance_bar_enabled,
+      :personal_access_token_prefix,
       :kroki_enabled,
       :kroki_url,
       :plantuml_enabled,

app/models/application_setting.rb

@@ -249,6 +249,12 @@ class ApplicationSetting < ApplicationRecord
 
   validates :user_default_internal_regex, js_regex: true, allow_nil: true
 
+  validates :personal_access_token_prefix,
+            format: { with: /\A[a-zA-Z0-9_+=\/@:.-]+\z/,
+                      message: _("can contain only letters of the Base64 alphabet (RFC4648) with the addition of '@', ':' and '.'") },
+            length: { maximum: 20, message: _('is too long (maximum is %{count} characters)') },
+            allow_blank: true
+
   validates :commit_email_hostname, format: { with: /\A[^@]+\z/ }
 
   validates :archive_builds_in_seconds,

app/models/application_setting_implementation.rb

@@ -104,6 +104,7 @@ module ApplicationSettingImplementation
         password_authentication_enabled_for_git: true,
         password_authentication_enabled_for_web: Settings.gitlab['signin_enabled'],
         performance_bar_allowed_group_id: nil,
+        personal_access_token_prefix: nil,
         plantuml_enabled: false,
         plantuml_url: nil,
         polling_interval_multiplier: 1,

app/models/concerns/token_authenticatable.rb

@@ -57,6 +57,13 @@ module TokenAuthenticatable
         token = read_attribute(token_field)
         token.present? && ActiveSupport::SecurityUtils.secure_compare(other_token, token)
       end
+
+      # Base strategy delegates to this method for formatting a token before
+      # calling set_token. Can be overridden in models to e.g. add a prefix
+      # to the tokens
+      mod.define_method("format_#{token_field}") do |token|
+        token
+      end
     end
 
     def token_authenticatable_module

app/models/concerns/token_authenticatable_strategies/base.rb

@@ -18,10 +18,15 @@ module TokenAuthenticatableStrategies
       raise NotImplementedError
     end
 
-    def set_token(instance)
+    def set_token(instance, token)
       raise NotImplementedError
     end
 
+    # Default implementation returns the token as-is
+    def format_token(instance, token)
+      instance.send("format_#{@token_field}", token) # rubocop:disable GitlabSecurity/PublicSend
+    end
+
     def ensure_token(instance)
       write_new_token(instance) unless token_set?(instance)
       get_token(instance)
@@ -57,7 +62,8 @@ module TokenAuthenticatableStrategies
 
     def write_new_token(instance)
       new_token = generate_available_token
-      set_token(instance, new_token)
+      formatted_token = format_token(instance, new_token)
+      set_token(instance, formatted_token)
     end
 
     def unique

app/models/personal_access_token.rb

@@ -9,7 +9,9 @@ class PersonalAccessToken < ApplicationRecord
   add_authentication_token_field :token, digest: true
 
   REDIS_EXPIRY_TIME = 3.minutes
-  TOKEN_LENGTH = 20
+
+  # PATs are 20 characters + optional configurable settings prefix (0..20)
+  TOKEN_LENGTH_RANGE = (20..40).freeze
 
   serialize :scopes, Array # rubocop:disable Cop/ActiveRecordSerialize
 
@@ -77,6 +79,15 @@ class PersonalAccessToken < ApplicationRecord
     )
   end
 
+  def self.token_prefix
+    Gitlab::CurrentSettings.current_application_settings.personal_access_token_prefix
+  end
+
+  override :format_token
+  def format_token(token)
+    "#{self.class.token_prefix}#{token}"
+  end
+
   protected
 
   def validate_scopes

app/models/user.rb

@@ -1665,7 +1665,7 @@ class User < ApplicationRecord
     save
   end
 
-  # each existing user needs to have an `feed_token`.
+  # each existing user needs to have a `feed_token`.
   # we do this on read since migrating all existing users is not a feasible
   # solution.
   def feed_token

app/views/admin/application_settings/_account_and_limit.html.haml

@@ -51,6 +51,9 @@
         = _('Specify an e-mail address regex pattern to identify default internal users.')
         = link_to _('More information'), help_page_path('user/permissions', anchor: 'setting-new-users-to-external'),
           target: '_blank'
+    .form-group
+      = f.label :personal_access_token_prefix, _('Personal Access Token prefix'), class: 'label-light'
+      = f.text_field :personal_access_token_prefix, placeholder: _('Max 20 characters'), class: 'form-control'
     .form-group
       = f.label :user_show_add_ssh_key_message, _('Prompt users to upload SSH keys'), class: 'label-bold'
       .form-check

changelogs/unreleased/feat-token-prefix.yml

+---
+title: Configurable personal access token prefix
+merge_request: 20968
+author: 'Max Wittig & Diego Louzán'
+type: added

db/migrate/20201119133534_add_personal_access_token_prefix_to_application_setting.rb

+# frozen_string_literal: true
+
+class AddPersonalAccessTokenPrefixToApplicationSetting < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  # rubocop:disable Migration/AddLimitToTextColumns
+  # limit is added in 20201119133604_add_text_limit_to_application_setting_personal_access_token_prefix
+  def change
+    add_column :application_settings, :personal_access_token_prefix, :text
+  end
+  # rubocop:enable Migration/AddLimitToTextColumns
+end

db/migrate/20201119133604_add_text_limit_to_application_setting_personal_access_token_prefix.rb

+# frozen_string_literal: true
+
+class AddTextLimitToApplicationSettingPersonalAccessTokenPrefix < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_text_limit :application_settings, :personal_access_token_prefix, 20
+  end
+
+  def down
+    remove_text_limit :application_settings, :personal_access_token_prefix
+  end
+end

db/schema_migrations/20201119133534

+6c8fc7904f50a792e10b5f1b0abe90ba21b1bdfd47430b3caa0df870c0a24079
\ No newline at end of file

db/schema_migrations/20201119133604

+bfb8ac3b697675bd4fca53273c6c6feb2f7a5659cbdaf57b9b4adb3e189b74ad
\ No newline at end of file

db/structure.sql

@@ -9372,11 +9372,13 @@ CREATE TABLE application_settings (
     secret_detection_revocation_token_types_url text,
     cloud_license_enabled boolean DEFAULT false NOT NULL,
     disable_feed_token boolean DEFAULT false NOT NULL,
+    personal_access_token_prefix text,
     CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
     CONSTRAINT check_17d9558205 CHECK ((char_length((kroki_url)::text) <= 1024)),
     CONSTRAINT check_2dba05b802 CHECK ((char_length(gitpod_url) <= 255)),
     CONSTRAINT check_51700b31b5 CHECK ((char_length(default_branch_name) <= 255)),
     CONSTRAINT check_57123c9593 CHECK ((char_length(help_page_documentation_base_url) <= 255)),
+    CONSTRAINT check_718b4458ae CHECK ((char_length(personal_access_token_prefix) <= 20)),
     CONSTRAINT check_85a39b68ff CHECK ((char_length(encrypted_ci_jwt_signing_key_iv) <= 255)),
     CONSTRAINT check_9a719834eb CHECK ((char_length(secret_detection_token_revocation_url) <= 255)),
     CONSTRAINT check_9c6c447a13 CHECK ((char_length(maintenance_mode_message) <= 255)),

doc/api/settings.md

@@ -82,7 +82,8 @@ Example response:
   "issues_create_limit": 300,
   "raw_blob_request_limit": 300,
   "wiki_page_max_content_bytes": 52428800,
-   "require_admin_approval_after_user_signup": false
+  "require_admin_approval_after_user_signup": false,
+  "personal_access_token_prefix": "GL-"
 }
 ```
 
@@ -174,7 +175,8 @@ Example response:
   "issues_create_limit": 300,
   "raw_blob_request_limit": 300,
   "wiki_page_max_content_bytes": 52428800,
-  "require_admin_approval_after_user_signup": false
+  "require_admin_approval_after_user_signup": false,
+  "personal_access_token_prefix": "GL-"
 }
 ```
 
@@ -318,6 +320,7 @@ listed in the descriptions of the relevant settings.
 | `performance_bar_allowed_group_id`       | string           | no                                   | (Deprecated: Use `performance_bar_allowed_group_path` instead) Path of the group that is allowed to toggle the performance bar. |
 | `performance_bar_allowed_group_path`     | string           | no                                   | Path of the group that is allowed to toggle the performance bar. |
 | `performance_bar_enabled`                | boolean          | no                                   | (Deprecated: Pass `performance_bar_allowed_group_path: nil` instead) Allow enabling the performance bar. |
+| `personal_access_token_prefix`           | string           | no                                   | Prefix for all generated personal access tokens. |
 | `plantuml_enabled`                       | boolean          | no                                   | (**If enabled, requires:** `plantuml_url`) Enable PlantUML integration. Default is `false`. |
 | `plantuml_url`                           | string           | required by: `plantuml_enabled`      | The PlantUML instance URL for integration. |
 | `polling_interval_multiplier`            | decimal          | no                                   | Interval multiplier used by endpoints that perform polling. Set to `0` to disable polling. |

doc/user/admin_area/settings/account_and_limit_settings.md

@@ -35,6 +35,25 @@ If you choose a size larger than what is currently configured for the web server
 you will likely get errors. See the [troubleshooting section](#troubleshooting) for more
 details.
 
+## Personal Access Token prefix
+
+You can set a global prefix for all generated Personal Access Tokens.
+
+A prefix can help you identify PATs visually, as well as with automation tools.
+
+### Setting a prefix
+
+Only a GitLab administrator can set the prefix, which is a global setting applied
+to any PAT generated in the system by any user:
+
+1. Navigate to **Admin Area > Settings > General**.
+1. Expand the **Account and limit** section.
+1. Fill in the **Personal Access Token prefix** field.
+1. Click **Save changes**.
+
+It is also possible to configure the prefix via the [settings API](../../../api/settings.md)
+using the `personal_access_token_prefix` field.
+
 ## Repository size limit **(STARTER ONLY)**
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/740) in [GitLab Enterprise Edition 8.12](https://about.gitlab.com/releases/2016/09/22/gitlab-8-12-released/#limit-project-size-ee).

ee/spec/requests/api/internal/base_spec.rb

@@ -342,6 +342,7 @@ RSpec.describe API::Internal::Base do
 
       it 'returns a valid token when the expiry date does not exceed the max token lifetime' do
         expires_at = instance_level_max_personal_access_token_lifetime.days.from_now.to_date.to_s
+        token_size = (PersonalAccessToken.token_prefix || '').size + 20
 
         post api('/internal/personal_access_token'),
              params: {
@@ -354,7 +355,7 @@ RSpec.describe API::Internal::Base do
 
         aggregate_failures do
           expect(json_response['success']).to eq(true)
-          expect(json_response['token']).to match(/\A\S{20}\z/)
+          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(expires_at)
         end

lib/api/settings.rb

@@ -103,6 +103,7 @@ module API
       optional :performance_bar_allowed_group_id, type: String, desc: 'Deprecated: Use :performance_bar_allowed_group_path instead. Path of the group that is allowed to toggle the performance bar.' # support legacy names, can be removed in v6
       optional :performance_bar_allowed_group_path, type: String, desc: 'Path of the group that is allowed to toggle the performance bar.'
       optional :performance_bar_enabled, type: String, desc: 'Deprecated: Pass `performance_bar_allowed_group_path: nil` instead. Allow enabling the performance.' # support legacy names, can be removed in v6
+      optional :personal_access_token_prefix, type: String, desc: 'Prefix to prepend to all personal access tokens'
       optional :kroki_enabled, type: Boolean, desc: 'Enable Kroki'
       given kroki_enabled: ->(val) { val } do
         requires :kroki_url, type: String, desc: 'The Kroki server URL'

lib/gitlab/auth/auth_finders.rb

@@ -194,6 +194,10 @@ module Gitlab
 
       def access_token
         strong_memoize(:access_token) do
+          # The token can be a PAT or an OAuth (doorkeeper) token
+          # It is also possible that a PAT is encapsulated in a `Bearer` OAuth token
+          # (e.g. NPM client registry auth), this case will be properly handled
+          # by find_personal_access_token
           find_oauth_access_token || find_personal_access_token
         end
       end
@@ -237,7 +241,7 @@ module Gitlab
       end
 
       def matches_personal_access_token_length?(token)
-        token.length == PersonalAccessToken::TOKEN_LENGTH
+        PersonalAccessToken::TOKEN_LENGTH_RANGE.include?(token.length)
       end
 
       # Check if the request is GET/HEAD, or if CSRF token is valid.

locale/gitlab.pot

@@ -16876,6 +16876,9 @@ msgstr ""
 msgid "Max 100,000 events"
 msgstr ""
 
+msgid "Max 20 characters"
+msgstr ""
+
 msgid "Max Group Export Download requests per minute per user"
 msgstr ""
 
@@ -20173,6 +20176,9 @@ msgstr ""
 msgid "Personal Access Token"
 msgstr ""
 
+msgid "Personal Access Token prefix"
+msgstr ""
+
 msgid "Personal project creation is not allowed. Please contact your administrator with questions"
 msgstr ""
 
@@ -32331,6 +32337,9 @@ msgstr ""
 msgid "by"
 msgstr ""
 
+msgid "can contain only letters of the Base64 alphabet (RFC4648) with the addition of '@', ':' and '.'"
+msgstr ""
+
 msgid "cannot be a date in the past"
 msgstr ""
 

spec/controllers/admin/application_settings_controller_spec.rb

@@ -157,6 +157,44 @@ RSpec.describe Admin::ApplicationSettingsController do
       expect(ApplicationSetting.current.default_branch_name).to eq("example_branch_name")
     end
 
+    context "personal access token prefix settings" do
+      let(:application_settings) { ApplicationSetting.current }
+
+      shared_examples "accepts prefix setting" do |prefix|
+        it "updates personal_access_token_prefix setting" do
+          put :update, params: { application_setting: { personal_access_token_prefix: prefix } }
+
+          expect(response).to redirect_to(general_admin_application_settings_path)
+          expect(application_settings.reload.personal_access_token_prefix).to eq(prefix)
+        end
+      end
+
+      shared_examples "rejects prefix setting" do |prefix|
+        it "does not update personal_access_token_prefix setting" do
+          put :update, params: { application_setting: { personal_access_token_prefix: prefix } }
+
+          expect(response).not_to redirect_to(general_admin_application_settings_path)
+          expect(application_settings.reload.personal_access_token_prefix).not_to eq(prefix)
+        end
+      end
+
+      context "with valid prefix" do
+        include_examples("accepts prefix setting", "a_prefix@")
+      end
+
+      context "with blank prefix" do
+        include_examples("accepts prefix setting", "")
+      end
+
+      context "with too long prefix" do
+        include_examples("rejects prefix setting", "a_prefix@" * 10)
+      end
+
+      context "with invalid characters prefix" do
+        include_examples("rejects prefix setting", "a_préfixñ:")
+      end
+    end
+
     context 'external policy classification settings' do
       let(:settings) do
         {

spec/factories/personal_access_tokens.rb

@@ -26,5 +26,9 @@ FactoryBot.define do
     trait :invalid do
       token_digest { nil }
     end
+
+    trait :no_prefix do
+      after(:build) { |personal_access_token| personal_access_token.set_token(Devise.friendly_token) }
+    end
   end
 end

spec/lib/gitlab/auth/auth_finders_spec.rb

@@ -384,6 +384,16 @@ RSpec.describe Gitlab::Auth::AuthFinders do
 
         expect { find_personal_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
       end
+
+      context 'when using a non-prefixed access token' do
+        let(:personal_access_token) { create(:personal_access_token, :no_prefix, user: user) }
+
+        it 'returns user' do
+          set_header('HTTP_AUTHORIZATION', "Bearer #{personal_access_token.token}")
+
+          expect(find_user_from_access_token).to eq user
+        end
+      end
     end
   end
 

spec/models/concerns/token_authenticatable_spec.rb

@@ -105,8 +105,8 @@ RSpec.describe PersonalAccessToken, 'TokenAuthenticatable' do
     it 'sets new token' do
       subject
 
-      expect(personal_access_token.token).to eq(token_value)
-      expect(personal_access_token.token_digest).to eq(Gitlab::CryptoHelper.sha256(token_value))
+      expect(personal_access_token.token).to eq("#{PersonalAccessToken.token_prefix}#{token_value}")
+      expect(personal_access_token.token_digest).to eq(Gitlab::CryptoHelper.sha256("#{PersonalAccessToken.token_prefix}#{token_value}"))
     end
   end
 

spec/requests/api/internal/base_spec.rb

@@ -220,6 +220,8 @@ RSpec.describe API::Internal::Base do
     end
 
     it 'returns a token without expiry when the expires_at parameter is missing' do
+      token_size = (PersonalAccessToken.token_prefix || '').size + 20
+
       post api('/internal/personal_access_token'),
            params: {
              secret_token: secret_token,
@@ -229,12 +231,14 @@ RSpec.describe API::Internal::Base do
            }
 
       expect(json_response['success']).to be_truthy
-      expect(json_response['token']).to match(/\A\S{20}\z/)
+      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
 
     it 'returns a token with expiry when it receives a valid expires_at parameter' do
+      token_size = (PersonalAccessToken.token_prefix || '').size + 20
+
       post api('/internal/personal_access_token'),
            params: {
              secret_token: secret_token,
@@ -245,7 +249,7 @@ RSpec.describe API::Internal::Base do
            }
 
       expect(json_response['success']).to be_truthy
-      expect(json_response['token']).to match(/\A\S{20}\z/)
+      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')
     end

spec/requests/api/settings_spec.rb

@@ -43,6 +43,7 @@ RSpec.describe API::Settings, 'Settings' do
       expect(json_response['spam_check_endpoint_url']).to be_nil
       expect(json_response['wiki_page_max_content_bytes']).to be_a(Integer)
       expect(json_response['require_admin_approval_after_user_signup']).to eq(true)
+      expect(json_response['personal_access_token_prefix']).to be_nil
     end
   end
 
@@ -122,7 +123,8 @@ RSpec.describe API::Settings, 'Settings' do
             spam_check_endpoint_url: 'https://example.com/spam_check',
             disabled_oauth_sign_in_sources: 'unknown',
             import_sources: 'github,bitbucket',
-            wiki_page_max_content_bytes: 12345
+            wiki_page_max_content_bytes: 12345,
+            personal_access_token_prefix: "GL-"
           }
 
         expect(response).to have_gitlab_http_status(:ok)
@@ -166,6 +168,7 @@ RSpec.describe API::Settings, 'Settings' do
         expect(json_response['disabled_oauth_sign_in_sources']).to eq([])
         expect(json_response['import_sources']).to match_array(%w(github bitbucket))
         expect(json_response['wiki_page_max_content_bytes']).to eq(12345)
+        expect(json_response['personal_access_token_prefix']).to eq("GL-")
       end
     end
 
@@ -451,5 +454,25 @@ RSpec.describe API::Settings, 'Settings' do
         expect(json_response['error']).to eq('spam_check_endpoint_url is missing')
       end
     end
+
+    context "personal access token prefix settings" do
+      context "handles validation errors" do
+        it "fails to update the settings with too long prefix" do
+          put api("/application/settings", admin), params: { personal_access_token_prefix: "prefix" * 10 }
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          message = json_response["message"]
+          expect(message["personal_access_token_prefix"]).to include(a_string_matching("is too long"))
+        end
+
+        it "fails to update the settings with invalid characters in the prefix" do
+          put api("/application/settings", admin), params: { personal_access_token_prefix: "éñ" }
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          message = json_response["message"]
+          expect(message["personal_access_token_prefix"]).to include(a_string_matching("can contain only letters of the Base64 alphabet"))
+        end
+      end
+    end
   end
 end