Refactor auth code #11379
Draft
Refactor auth code #11379
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Up until this point our "auth" code was scattered all over the place and mixed up authentication (who are you?) and authorization (what are you allowed to do?) in multiple places: - Whether a user account was locked was checked as part of authentication, although it is more of an authorization concern. - Whether a user has a verified email address was checked in the individual endpoints, if at all. - Whether a user is an admin was also checked in the individual endpoints that were adjusted to support admin users. - Whether a user is an owner (or team member) of a crate was also checked in the individual endpoints, in various different ways. - Whether an API token has the right scopes was also checked as part of authentication, but it is an authorization concern. This commit centralizes these checks with a three step approach: 1. In the first step the endpoint uses one of the `Credentials` structs as an axum request extractor to extract the raw credentials from the request metadata (user ID from session cookie, API token, Trusted Publishing token). 2. The credentials are used to look up a corresponding user (or Trusted Publishing token) from the database. 3. The authenticated entity is used to check for authorization of the requested operation. The second and third step are folded into one fn call (`.validate(permission)`) to make it harder to use an authenticated entity without knowing if they are authorized for the operation. While the auth code and the endpoints have changed quite a bit due to this refactoring, the test code is mostly the same except for a few minor details caused by using the request extractors now.
Turbo87
added
C-internal 🔧
Member
Author
|
@rust-lang/crates-io I've opened this as a draft since I'm mostly looking for feedback on this idea. Please take a look and let me know if this is actually an improvement or not. I'm a little worried that I've over-complicated it, but I'm not quite sure if it can be simplified in a significant way. |
5 tasks
Member
Author
|
@rust-lang/crates-io it's been almost 6 months now. does anyone have an opinion on this?! |
|
@steveklabnik @alexcrichton this is a semi-blocker for non-Github account creation |
Member
Author
no, it's not |
|
Okay, sorry then. However @carols10cents in #10611 seemed to think that it was. |
Member
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Up until this point our "auth" code was scattered all over the place and mixed up authentication (who are you?) and authorization (what are you allowed to do?) in multiple places:
This commit centralizes these checks with a three step approach:
Credentialsstructs as an axum request extractor to extract the raw credentials from the request metadata (user ID from session cookie, API token, Trusted Publishing token).The second and third step are folded into one fn call (
.validate(permission)) to make it harder to use an authenticated entity without knowing if they are authorized for the operation.While the auth code and the endpoints have changed quite a bit due to this refactoring, the test code is mostly the same except for a few minor details caused by using the request extractors now.