From f09de3db0ed92ce55c39a50dabf2125b9d0daa1f Mon Sep 17 00:00:00 2001
From: Nick Malcolm <nmalcolm@gitlab.com>
Date: Wed, 20 Dec 2023 08:45:38 +0000
Subject: [PATCH] Add a prefix to SCIM tokens behind a feature flag

Prefixes SCIM OAuth Access Tokens with `glsoat-` following the
guidance at
https://docs.gitlab.com/ee/development/secure_coding_guidelines.html#token-prefixes.

GitLab applies a prefix to some of its generated secrets. For example, a
Personal Access Token begins with `glpat-`. This MR adds a prefix to
SCIM Tokens. It also updates our frontend secret detection which
helps prevent users from leaking tokens via Issue / MR comments.

SCIM tokens can belong to a Group, or have no Group and be an instance
token. These tokens are used to authenticate against the APIs described
at https://docs.gitlab.com/ee/development/internal_api/#group-scim-api
and
https://docs.gitlab.com/ee/development/internal_api/#instance-scim-api
respectively.

A feature flag is being used to reduce the risk of breaking third-party
integrations, which might have made assumptions about the format of
GitLab's SCIM tokens remaining static.
See https://gitlab.com/gitlab-org/gitlab/-/issues/435096#note_1691498327
for discussion.

Resolves https://gitlab.com/gitlab-org/gitlab/-/issues/435096

Changelog: changed
EE: true
---
 .gitleaksignore                                |  1 +
 .../javascripts/lib/utils/secret_detection.js  |  4 ++++
 .../development/prefix_scim_tokens.yml         |  8 ++++++++
 config/gitleaks.toml                           |  1 +
 doc/security/token_overview.md                 |  1 +
 ee/app/models/scim_oauth_access_token.rb       |  8 +++++++-
 ee/spec/models/scim_oauth_access_token_spec.rb | 18 ++++++++++++++++++
 .../lib/utils/secret_detection_spec.js         |  1 +
 8 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 config/feature_flags/development/prefix_scim_tokens.yml

diff --git a/.gitleaksignore b/.gitleaksignore
index 32906f05b2acc..d52f10ebdc40a 100644
--- a/.gitleaksignore
+++ b/.gitleaksignore
@@ -1,2 +1,3 @@
 afedb913baf4203aa688421873fdb9f94649578e:doc/api/users.md:generic-api-key:2201
 spec/frontend/lib/utils/secret_detection_spec.js:generic-api-key:34
+spec/frontend/lib/utils/secret_detection_spec.js:generic-api-key:35
diff --git a/app/assets/javascripts/lib/utils/secret_detection.js b/app/assets/javascripts/lib/utils/secret_detection.js
index 4d8612aeeff0a..92edd286c7685 100644
--- a/app/assets/javascripts/lib/utils/secret_detection.js
+++ b/app/assets/javascripts/lib/utils/secret_detection.js
@@ -32,6 +32,10 @@ export const containsSensitiveToken = (message) => {
       name: 'GitLab Deploy Token',
       regex: `gldt-[0-9a-zA-Z_-]{20}`,
     },
+    {
+      name: 'GitLab SCIM OAuth Access Token',
+      regex: `glsoat-[0-9a-zA-Z_-]{20}`,
+    },
   ];
 
   for (const rule of sensitiveDataPatterns) {
diff --git a/config/feature_flags/development/prefix_scim_tokens.yml b/config/feature_flags/development/prefix_scim_tokens.yml
new file mode 100644
index 0000000000000..297327f26d2ce
--- /dev/null
+++ b/config/feature_flags/development/prefix_scim_tokens.yml
@@ -0,0 +1,8 @@
+---
+name: prefix_scim_tokens
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139737
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/435423
+milestone: '16.8'
+type: development
+group: group::authentication
+default_enabled: false
diff --git a/config/gitleaks.toml b/config/gitleaks.toml
index a492b38fdf965..0983a34ca7105 100644
--- a/config/gitleaks.toml
+++ b/config/gitleaks.toml
@@ -13,6 +13,7 @@ path = "/gitleaks.toml"
     # spec/frontend/lib/utils/secret_detection_spec.js
     "glpat-cgyKc1k_AsnEpmP-5fRL",
     "gldt-cgyKc1k_AsnEpmP-5fRL",
+    "glsoat-cgyKc1k_AsnEpmP-5fRL",
     # spec/frontend/lib/utils/secret_detection_spec.js
     "GlPat-abcdefghijklmnopqrstuvwxyz",
     # doc/development/sec/token_revocation_api.md
diff --git a/doc/security/token_overview.md b/doc/security/token_overview.md
index 4555459e7c5ff..4498ee893a784 100644
--- a/doc/security/token_overview.md
+++ b/doc/security/token_overview.md
@@ -245,6 +245,7 @@ The following tables show the prefixes for each type of token where applicable.
 | Incoming mail token               | `glimt-`           |
 | GitLab Agent for Kubernetes token | `glagent-`         |
 | GitLab session cookies            | `_gitlab_session=` |
+| SCIM Tokens                       | `glsoat-` ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/435096) in GitLab 16.8 behind a feature flag named `prefix_scim_tokens`. Disabled by default.) |
 
 ### External system tokens
 
diff --git a/ee/app/models/scim_oauth_access_token.rb b/ee/app/models/scim_oauth_access_token.rb
index 4bea5a91b0fa0..9ecd6aa105d6f 100644
--- a/ee/app/models/scim_oauth_access_token.rb
+++ b/ee/app/models/scim_oauth_access_token.rb
@@ -3,9 +3,11 @@
 class ScimOauthAccessToken < ApplicationRecord
   include TokenAuthenticatable
 
+  TOKEN_PREFIX = 'glsoat-'
+
   belongs_to :group, optional: true
 
-  add_authentication_token_field :token, encrypted: :required
+  add_authentication_token_field :token, encrypted: :required, format_with_prefix: :prefix_for_token
 
   before_save :ensure_token
 
@@ -31,4 +33,8 @@ def self.find_for_instance
   def as_entity_json
     ScimOauthAccessTokenEntity.new(self).as_json
   end
+
+  def prefix_for_token
+    Feature.enabled?(:prefix_scim_tokens) ? TOKEN_PREFIX : ''
+  end
 end
diff --git a/ee/spec/models/scim_oauth_access_token_spec.rb b/ee/spec/models/scim_oauth_access_token_spec.rb
index 561f77d32aa2d..4a7274dcf5346 100644
--- a/ee/spec/models/scim_oauth_access_token_spec.rb
+++ b/ee/spec/models/scim_oauth_access_token_spec.rb
@@ -93,5 +93,23 @@
 
       expect(scim_token.token).to be_a(String)
     end
+
+    it 'is prefixed' do
+      scim_token = described_class.create!
+
+      expect(scim_token.token).to match(/^glsoat-[\w-]{20}$/)
+    end
+
+    context 'when feature flag is disabled' do
+      before do
+        stub_feature_flags(prefix_scim_tokens: false)
+      end
+
+      it 'is not prefixed' do
+        scim_token = described_class.create!
+
+        expect(scim_token.token).to match(/^[\w-]{20}$/)
+      end
+    end
   end
 end
diff --git a/spec/frontend/lib/utils/secret_detection_spec.js b/spec/frontend/lib/utils/secret_detection_spec.js
index a8da6e8969fa9..0d1bf1abbaab6 100644
--- a/spec/frontend/lib/utils/secret_detection_spec.js
+++ b/spec/frontend/lib/utils/secret_detection_spec.js
@@ -32,6 +32,7 @@ describe('containsSensitiveToken', () => {
       'https://example.com/feed?feed_token=123456789_abcdefghij',
       'glpat-1234567890 and feed_token=ABCDEFGHIJKLMNOPQRSTUVWXYZ',
       'token: gldt-cgyKc1k_AsnEpmP-5fRL',
+      'curl "https://gitlab.example.com/api/v4/groups/33/scim/identities" --header "PRIVATE-TOKEN: glsoat-cgyKc1k_AsnEpmP-5fRL',
     ];
 
     it.each(sensitiveMessages)('returns true for message: %s', (message) => {
-- 
GitLab