Merge branch '376474-oauth-application-secret-prefix' into 'master'

Prefixes OAuth Application Secrets with gloas

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/131601



Merged-by: Bojan Marjanovic <bmarjanovic@gitlab.com>
Approved-by: Lorenz van Herwaarden <lvanherwaarden@gitlab.com>
Approved-by: Aakriti Gupta <agupta@gitlab.com>
Approved-by: Bojan Marjanovic <bmarjanovic@gitlab.com>
Reviewed-by: Aakriti Gupta <agupta@gitlab.com>
Reviewed-by: Nick Malcolm <nmalcolm@gitlab.com>
Reviewed-by: Bojan Marjanovic <bmarjanovic@gitlab.com>
Co-authored-by: Nick Malcolm <nmalcolm@gitlab.com>
Co-authored-by: Aakriti Gupta <agupta@gitlab.com>

app/assets/javascripts/lib/utils/secret_detection.js

@@ -24,6 +24,10 @@ export const containsSensitiveToken = (message) => {
       name: 'Feed Token',
       regex: 'feed_token=((glft-)?[0-9a-zA-Z_-]{20}|glft-[a-h0-9]+-[0-9]+_)',
     },
+    {
+      name: 'GitLab OAuth Application Secret',
+      regex: `gloas-[0-9a-zA-Z_-]{64}`,
+    },
   ];
 
   for (const rule of sensitiveDataPatterns) {

config/initializers/doorkeeper.rb

@@ -124,4 +124,8 @@
   # 2 hours in seconds
   # This is also the database default value
   access_token_expires_in 7200
+
+  # Use a custom class for generating the application secret.
+  # https://doorkeeper.gitbook.io/guides/configuration/other-configurations#custom-application-secret-generator
+  application_secret_generator 'Gitlab::DoorkeeperSecretStoring::Token::UniqueApplicationToken'
 end

lib/gitlab/doorkeeper_secret_storing/token/unique_application_token.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module DoorkeeperSecretStoring
+    module Token
+      class UniqueApplicationToken
+        # Acronym for 'GitLab OAuth Application Secret'
+        OAUTH_APPLICATION_SECRET_PREFIX_FORMAT = "gloas-%{token}"
+
+        # Maintains compatibility with ::Doorkeeper::OAuth::Helpers::UniqueToken
+        # Returns a secure random token, prefixed with a GitLab identifier.
+        def self.generate(*)
+          format(OAUTH_APPLICATION_SECRET_PREFIX_FORMAT, token: SecureRandom.hex(32))
+        end
+      end
+    end
+  end
+end

spec/controllers/oauth/applications_controller_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Oauth::ApplicationsController do
+RSpec.describe Oauth::ApplicationsController, feature_category: :system_access do
   let(:user) { create(:user) }
   let(:application) { create(:oauth_application, owner: user) }
 
@@ -86,10 +86,10 @@
       it_behaves_like 'redirects to login page when the user is not signed in'
       it_behaves_like 'redirects to 2fa setup page when the user requires it'
 
-      it 'returns the secret in json format' do
+      it 'returns the prefixed secret in json format' do
         subject
 
-        expect(json_response['secret']).not_to be_nil
+        expect(json_response['secret']).to match(/gloas-\h{64}/)
       end
 
       context 'when renew fails' do
@@ -153,6 +153,15 @@
         expect(response).to render_template :show
       end
 
+      context 'the secret' do
+        render_views
+
+        it 'is in the response' do
+          subject
+          expect(response.body).to match(/gloas-\h{64}/)
+        end
+      end
+
       it 'redirects back to profile page if OAuth applications are disabled' do
         disable_user_oauth
 

spec/frontend/lib/utils/secret_detection_spec.js

@@ -28,6 +28,7 @@ describe('containsSensitiveToken', () => {
       'token: feed_token=ABCDEFGHIJKLMNOPQRSTUVWXYZ',
       'token: feed_token=glft-ABCDEFGHIJKLMNOPQRSTUVWXYZ',
       'token: feed_token=glft-a8cc74ccb0de004d09a968705ba49099229b288b3de43f26c473a9d8d7fb7693-1234',
+      'token: gloas-a8cc74ccb0de004d09a968705ba49099229b288b3de43f26c473a9d8d7fb7693',
       'https://example.com/feed?feed_token=123456789_abcdefghij',
       'glpat-1234567890 and feed_token=ABCDEFGHIJKLMNOPQRSTUVWXYZ',
     ];

spec/models/doorkeeper/application_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Doorkeeper::Application, type: :model, feature_category: :system_access do
+  let(:application) { create(:oauth_application) }
+
+  it 'uses a prefixed secret' do
+    expect(application.plaintext_secret).to match(/gloas-\h{64}/)
+  end
+end

spec/models/oauth_access_token_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe OauthAccessToken do
+RSpec.describe OauthAccessToken, feature_category: :system_access do
   let(:app_one) { create(:oauth_application) }
   let(:app_two) { create(:oauth_application) }
   let(:app_three) { create(:oauth_application) }
@@ -23,6 +23,10 @@
   end
 
   describe 'Doorkeeper secret storing' do
+    it 'does not have a prefix' do
+      expect(token.plaintext_token).not_to start_with('gl')
+    end
+
     it 'stores the token in hashed format' do
       expect(token.token).not_to eq(token.plaintext_token)
     end