-
Notifications
You must be signed in to change notification settings - Fork 1k
Re-add UserTokenContext
, with instance checks
#15590
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…tyPolicy.identity` (pypi#15581)" (pypi#15588)" This reverts commit 5eba9cb.
Eliminate the line that needed it.
There was another failure case in #15586 which I couldn't root-cause: "caveat must be an array", which change covers that here? |
Hmm, that happens in |
Oh, I think that error happens as a variant of #15588 (comment) when the API token is in the "legacy" format: we call Relevant bit: # Our V1 token didn't have a way to specify that a token should be
# restricted to a specific user, just that it was scoped to "the user",
# which the user was whoever the token was linked to in the database.
# Our new tokens strengthens that to validate that the linked user
# matches who it is expected to be, but since we don't have that
# data for V1 tokens, we'll just use the current user.
if permissions == "user":
request = get_current_request()
# If we don't have a current request, then we can't validate this
# token.
if request is None:
return None
# If we don't have a user associated with this request, then we
# can't validate this token.
if request.user is None:
return None
return [3, str(request.user.id)] So we'd return |
I've added some tests for the behavior here, but I'm not immediately sure how to "backstop" this -- maybe some additional |
Signed-off-by: William Woodruff <[email protected]>
This should be good to go now: I've added a layer of configurations on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@woodruffw Can you rebase & get tests passing?
Should be good now! |
* Revert "Revert "Return `Macaroon` alongside `User` in `MacaroonSecurityPolicy.identity` (pypi#15581)" (pypi#15588)" This reverts commit 5eba9cb. * warehouse: add UserTokenContext checks * warehouse: reformat, circular import * tests: coverage * tests: reformat * warehouse: remove UserTokenContext.id Eliminate the line that needed it. * accounts/utils: lintage * test_legacy: backstop behavior Signed-off-by: William Woodruff <[email protected]> * tests/unit: lintage --------- Signed-off-by: William Woodruff <[email protected]>
Reverts #15588, reviving #15581.
WIP while I add both coverage and determine a backstop test approach here.