Skip to content

Prefixes OAuth Application Secrets with gloas

Nick Malcolm requested to merge 376474-oauth-application-secret-prefix into master

What does this MR do and why?

Prefixes OAuth Application Secrets with gloas-: GitLab OAuth Application Secret

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 OAuth Application Secrets. It also updates our frontend secret detection which helps prevent users from leaking tokens via Issue / MR comments.

Note that OAuth Applications are distinct from OAuth Access Tokens, which remain unchanged (with a spec to confirm). Learn more about OAuth Applications at https://docs.gitlab.com/ee/integration/oauth_provider.html

Resolves #376747 (closed)

Following the pattern of glpat for PATs (#342327 (closed)), gltt for trigger tokens (!95968 (merged)), glft for feed tokens (!123424 (merged)).


Note that this prefix, gloas-, for a client secret should be fine:

     VSCHAR     = %x20-7E

...

A.2.  "client_secret" Syntax

   The "client_secret" element is defined in Section 2.3.1:

     client-secret = *VSCHAR

https://www.rfc-editor.org/rfc/rfc6749#appendix-A.2

20 is a space, through to 7E which is a ~. https://www.asciitable.com/

Screenshots or screen recordings

Before After
before Screenshot_2023-09-14_at_8.45.19_AM
Screenshot_2023-09-14_at_8.46.21_AM
Screenshot_2023-09-14_at_8.47.34_AM

How to set up and validate locally

  1. Go to Preferences > Applications
  2. Generate a new OAuth Application
    • Name: any
    • Callback URL: http://localhost
  3. Note the secret
  4. Go to any issue, and make a comment containing "gloas-AAAAAAAA5e7b0af568e17c0d68be1e5b482059a516dcfb307ddc1fff66783489"
  5. Check out this branch
  6. gdk restart rails-web
  7. Repeat steps above
    • note the prefixed secret on create
    • note the warning when commenting a secret

Other things to try: creating a Group or Instance OAuth app, renewing the secret. All work the same as a personal OAuth Application.

How to validate the end-to-end flow

As an alternative to creating a real OAuth app we can use the steps from !91501 (merged):

  1. Go to Pereferences -> Applications and create a new application.
    • Hint: You can use something like https://google.com as your Redirect URI for testing purposes.
    • Rest of the steps assume that url and 'read_user' scope but it's not explicitly necessary.
  2. Build an authorization code URL: https://gdk.test:3443/oauth/authorize?client_id=<client_id>&redirect_uri=https://google.com&response_type=code&state=STATE&scope=read_user replacing the client_id value with the Application ID.
  3. Paste the above URL in a browser where you're already signed in to GDK. This should ask if you want to authorize the application and once authorized will redirect you to https://google.com with a code param appended. Grab this code.
  4. Back in the Rails console, build a parameter string (press enter after to set the variable) - parameters = 'client_id=<client_id>&client_secret=<secret>&code=<code_from_previous_step>&grant_type=authorization_code&redirect_uri=https://google.com'
  5. Execute the request to get a token - response = RestClient.post 'https://gdk.test:3443/oauth/token', parameters
  6. Get the response body which should include an OAuth access token. This confirms the OAuth application token request cycle worked correctly - response.body
  7. Use the access_token value from the response body to authenticate to the API (exit the Rails console first, or use another terminal window): curl "https://gdk.test:3443/api/v4/user?access_token=<access_token>"
 % curl "https://gdk.test:3443/api/v4/user?access_token=4137bfcTHIS_IS_FROM_GDK6afc828"
{"id":22,"username":"nm","name":"Nick Malcolm","state":"active"...

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

TODO

  • Do an end-to-end test of a newly created OAuth App, if a spec doesn't exist for that already. This will validate the fairly safe assumption that clients don't fail on the new secrets which now start with gloas- (i.e. non hex chars). I'm not entirely sure how to test this.

Related to #376474

Edited by Nick Malcolm

Merge request reports