Resolve "Add a doorkeeper scope suitable for authentication"

What does this MR do?

• Add a single new scope (in addition to the api scope we've had) - read_user
• Allow creating OAuth applications and Personal access tokens with a scope selected
• Enforce scopes in the API

What are the relevant issue numbers?

• Closes #20492 (closed)
• EE counterpart for this MR: gitlab-org/gitlab-ee!946

Tasks

• [#20492 (closed)/!5951 (merged)] Add a doorkeeper scope suitable for authentication
  • Implementation
    • Doorkeeper
      • Create application with scopes
      • Create token with scopes
      • Enforce scopes
      • Pretty names / explanations for scopes
    • Personal Access Tokens
      • Create tokens with scopes
      • Enforce scopes
      • Centralise canonical list of scopes
    • Admin::Application?
    • What happens when no scopes are selected while creating a token / application?
      • They have the API scope
    • Existing OAuth tokens should continue to have the api scope
      • Applications created without any scopes (as before) have scopes set to ''.
      • Any tokens created for these applications have scopes set to 'api'
    • Where is Gitlab::Auth used?
      • Used to authenticate Git pushes by using the token instead of a password
      • Check for the right scope during token validation
    • Existing Personal access tokens should continue to have the api scope
    • How does the sessions API affect this feature?
      • The sessions API allows clients to obtain a private token using the user's username and password
      • The API endpoint does not call authenticate!, so no scope checking occurs (as expected)
      • No changes are necessary heren
  • Test / refactor
    • Move Oauth2::AccessTokenValidationService to AccessTokenValidationService
    • Test on MySQL
    • Run all migrations on a fresh database
    • Write feature specs
    • Run feature specs on MySQL and Postgres
    • Fix build
  • Meta
    • API support added
    • Branch has no merge conflicts with master
    • Squashed related commits together
    • Added screenshots
    • CHANGELOG entry created
    • Created EE merge request
  • Follow-up
    • Verify that rebasing didn't break anything
    • Does find_user_from_warden impact this feature in any way?
    • Tests for sufficient_scope?
    • Tests for scope checks at the API level
    • Tests passing
  • Review
    • Endboss
      • What do you think about making AccessTokenValidationService a class
      • valid_api_token? (or even just api_token?) would be more explicit
      • Could we instead consider tokens with scopes == []
        • Check if migrations run before or after the new code is deployed
    • Miniboss
      • Extract the scope listing into a partial
      • What happens to existing doorkeeper tokens? Will they inherit the api scope?
      • Build passing
      • Extract partial from app/views/admin/applications/show.html.haml and app/views/doorkeeper/applications/show.html.html
      • Split feature / controller specs
  • Before Merge
    • Seed database before migrations, add applications / tokens, migrate, make sure everything is working
    • Check if we need to migrate all existing OAuth applications to use the api scope
    • Resolve conflicts
    • Recheck flows
      • API
        • Personal access token
        • OAuth application created per-user
        • OAuth application created by admin
      • Git over HTTP
        • Personal access token
        • OAuth application created per-user
        • OAuth application created by admin
    • EE MR
    • Recheck migrations on PG
    • Recheck migrations on MySQL
  • Wait for merge
  • After Merge
    • Create issue / MR to add documentation around scopes

Marked this merge request as a Work In Progress

Added 3 commits:

• 8e1d12ca - Allow creating an OAuth application with scopes.
• 6d9fdaee - Enforce OAuth scopes in the API.
• abf5618d - Allow creating personal access tokens with a scope.

Marked the task Personal Access Tokens as completed

Marked the task Create tokens with scopes as completed

Added 2 commits:

• 52667b87 - Enforce personal access token scopes in the API.
• 500f5adc - Move scopes into Gitlab::Auth.

Added 5 commits:

• 4385cbf2 - Allow creating personal access tokens with a scope.
• d904249a - Enforce personal access token scopes in the API.
• 84733751 - Move scopes into Gitlab::Auth.
• d3adf055 - Allow creating OAuth Applications with scopes from the admin panel.
• 6cd58e2e - Check scope for tokens used to authenticate git pushes over HTTP.

Marked the task Admin::Application? as completed

Marked the task Where is Gitlab::Auth used? as completed

Added 1 commit:

• a59d8900 - All existing personal access tokens need to have the api scope.

Marked the task Implementation as completed

Marked the task How does the sessions API affect this feature? as completed

Added 2 commits:

• 8779a833 - Move Oauth2::AccessTokenValidationService to AccessTokenValidationService.
• 88c84e0a - Remove unused methods in APIGuard.

@timothyandrew I'd like to suggest renaming the read_user scope to profile as defined by OpenID Connect, this will make it easier to support the full core spec later (which I'm currently looking into).

@toupeira: In my opinion, profile isn't granular enough - I would want us to (eventually) have separate scopes for read_user and write_user, for example.

I've looked at the OpenID Connect Core spec - it defines the profile, email, address, and phone scopes, which need to be accompanied by the openid scope. Since we can have multiple allowable scopes for a given resource, my preference would be to leave the read_user scope here, and add in the openid and profile scopes whenever we're implementing OpenID compliance. The presence of other scopes (like read_user and write_user) shouldn't affect the OpenID flow.

@DouweM: Do you have any thoughts here?

P.S.: I'm not very familiar with OpenID (Connect), so please feel free to correct me if I've made any mistaken assumptions.

Added 2 commits:

• 55c2d7ef - Don't use Postgres arrays to store personal access token scopes.
• 41a737ab - Add a feature spec for personal access tokens with scopes.

Added 447 commits:

• 41a737ab...7649497f - 434 commits from branch master
• 1e4b5d27 - Allow creating an OAuth application with scopes.
• d219fb10 - Enforce OAuth scopes in the API.
• 9a09ccc7 - Allow creating personal access tokens with a scope.
• 82e85f29 - Enforce personal access token scopes in the API.
• 7ec3ecf7 - Move scopes into Gitlab::Auth.
• ce5f70f4 - Allow creating OAuth Applications with scopes from the admin panel.
• f28adecd - Check scope for tokens used to authenticate git pushes over HTTP.
• d0ad7be2 - All existing personal access tokens need to have the api scope.
• 5c44c81a - Move Oauth2::AccessTokenValidationService to AccessTokenValidationService.
• f4e0b02f - Remove unused methods in APIGuard.
• 11c8ff82 - Don't use Postgres arrays to store personal access token scopes.
• bf96f136 - Add a feature spec for personal access tokens with scopes.
• 659204b5 - Fix tests.

Marked the task Move Oauth2::AccessTokenValidationService to AccessTokenValidationService as completed

Marked the task Test on MySQL as completed

Marked the task Run all migrations on a fresh database as completed

Marked the task Test / refactor as completed