Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Better upload errors when using API tokens #11885

Merged
merged 11 commits into from
Jul 27, 2022
Merged

Better upload errors when using API tokens #11885

merged 11 commits into from
Jul 27, 2022

Conversation

woodruffw
Copy link
Member

This propagates the actual validation errors that occur during Macaroon authn/authz, allowing us to return a more meaningful error message than just InvalidMacaroonError(...).

Example:

$ twine upload --config-file pypirc -r localhost dist/*
Uploading distributions to http://localhost/legacy/
Uploading fakepkg-0.0.2.tar.gz
100% ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 4.0/4.0 kB • 00:00 • ?
WARNING  Error during upload. Retry with the --verbose option for more details.
ERROR    HTTPError: 403 Forbidden from http://localhost/legacy/
         Invalid API Token: project-scoped token is not valid for project 'fakepkg'

Fixes #10821.
Fixes #9018.

woodruffw added 2 commits July 19, 2022 11:36
@woodruffw
warehouse/macaroons: propagate errors nicely
8f8b7a3 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
tests: update tests for improved macaroons errors
5395ddc 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw requested a review from a team as a code owner July 19, 2022 15:39
@woodruffw woodruffw requested a review from dstufft July 19, 2022 15:39
@woodruffw woodruffw self-assigned this Jul 19, 2022
This was referenced Jul 19, 2022
Open
Merged
Closed
Closed
@woodruffw woodruffw requested a review from di July 19, 2022 16:21
@dstufft dstufft mentioned this pull request Jul 21, 2022
Merged
woodruffw added 3 commits July 21, 2022 11:51
@woodruffw
Merge remote-tracking branch 'origin/main' into tob-better-macaroon-e…
6e49225 
…rrors
@woodruffw
macaroons/caveats: don't stringify generic exceptions
6826cf9 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
tests: update caveats tests
369a68d 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw requested a review from dstufft July 21, 2022 16:30
@woodruffw
Copy link
Member Author

I think this should be good to go, assuming we still want to merge it (instead of kicking the can for this down to Caveats v2).

dstufft
dstufft approved these changes Jul 27, 2022
View reviewed changes
Copy link
Member

@dstufft dstufft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a solid improvement to me, even if caveats v2 reworks this, it seems reasonable to land this now.

@dstufft dstufft merged commit eba0a21 into pypi:main Jul 27, 2022
@woodruffw woodruffw deleted the tob-better-macaroon-errors branch July 27, 2022 15:52
woodruffw added a commit to trail-of-forks/warehouse that referenced this pull request Jul 27, 2022
@woodruffw
Better upload errors when using API tokens (pypi#11885)
1a837df
SamirPS pushed a commit to SamirPS/warehouse that referenced this pull request Aug 30, 2022
@woodruffw @SamirPS
Better upload errors when using API tokens (pypi#11885)
466933e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@merwok merwok merwok left review comments

@dstufft dstufft dstufft approved these changes

@di di Awaiting requested review from di

Assignees

@woodruffw woodruffw

Labels
APIs/feeds usability
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

InvalidMacaroonError not very helpful Improve "invalid macaroon signature" error
3 participants
@woodruffw @dstufft @merwok