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
-
-
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
-
-
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
-
-
-
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
-
-
-
EE MR -
Recheck migrations on PG -
Recheck migrations on MySQL
-
-
Wait for merge -
After Merge -
Create issue / MR to add documentation around scopes
-
-
Edited by 🤖 GitLab Bot 🤖