Resolve "Add a doorkeeper scope suitable for authentication"
What does this MR do?
- Add a single new scope (in addition to the
apiscope 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
APIscope
- They have the
- Existing OAuth tokens should continue to have the
apiscope- Applications created without any scopes (as before) have
scopesset to ''. - Any tokens created for these applications have
scopesset to 'api'
- Applications created without any scopes (as before) have
- Where is
Gitlab::Authused?- 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
apiscope - How does the
sessionsAPI affect this feature?- The
sessionsAPI 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
- The
- Doorkeeper
- Test / refactor
- Move
Oauth2::AccessTokenValidationServicetoAccessTokenValidationService - Test on MySQL
- Run all migrations on a fresh database
- Write feature specs
- Run feature specs on MySQL and Postgres
- Fix build
- Move
- 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_wardenimpact 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
apiscope? - Build passing
- Extract partial from
app/views/admin/applications/show.html.hamlandapp/views/doorkeeper/applications/show.html.html - Split feature / controller specs
- Endboss
- 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
apiscope - 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
- API
- EE MR
- Recheck migrations on PG
- Recheck migrations on MySQL
- Wait for merge
- After Merge
- Create issue / MR to add documentation around scopes
- Implementation
Edited by 🤖 GitLab Bot 🤖