Skip to content

OIDC macaroon minting for GitHub #11272

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
Merged
merged 112 commits into from
Nov 15, 2022
Merged

OIDC macaroon minting for GitHub #11272

merged 112 commits into from
Nov 15, 2022

Conversation

woodruffw
Copy link
Member

@woodruffw woodruffw commented Apr 27, 2022 •
edited
Loading

This adds an API route for converting a JWT into a temporary API token.

Very WIP.

TODO:

  • Actually create the macaroon
  • Local testing
  • Fill in test coverage
  • Feature gate with the other OIDC functionality

Needs #11218.

Closes #10970.

@woodruffw
warehouse: make Macaroon <-> User optional
bf7d11f 
Macaroons were previously strongly associated with individual users.

This was the correct association for user-minted tokens, but not
generally. In particular, ODIC-minted tokens are associated with
projects themselves and not the user who registered the OIDC provider.
@woodruffw
Merge branch 'main' into tob-oidc-macaroon-minting
bdeba08
@woodruffw
Merge remote-tracking branch 'upstream/main' into tob-oidc-macaroon-m…
0144c24 
…inting
@woodruffw
migrations: rebase
2319bb7
@woodruffw
warehouse: oidc minting boilerplate
456a787
@woodruffw
Merge remote-tracking branch 'upstream/main' into tob-oidc-macaroon-m…
aec2e71 
…inting
@woodruffw
oidc/views: more boilerplate
9afe54d
@woodruffw
dev, tests, warehouse: NullOIDCProvider
49c392d
@woodruffw
Merge remote-tracking branch 'upstream/main' into tob-oidc-macaroon-m…
be55ca5 
…inting
@woodruffw woodruffw mentioned this pull request Apr 27, 2022
Merged
@woodruffw woodruffw requested review from di and ewdurbin May 11, 2022 19:30
@woodruffw
Copy link
Member Author

woodruffw commented May 11, 2022 •
edited
Loading

This is ready for an initial review. I'll leave some comments flagging some things in particular.

@alex
Copy link
Member

alex commented Nov 2, 2022

Is including a link to/ID for the specific run practical? That'd give a greater degree of auditability.

@woodruffw
Copy link
Member Author

Is including a link to/ID for the specific run practical? That'd give a greater degree of auditability.

Yeah, it should be (not with state on the OIDC provider itself, but with state from the JWT.) Let me see if I can figure out a way to pull that out.

@woodruffw
tests: lint
5f269b9 
Signed-off-by: William Woodruff <william@trailofbits.com>
@di
Copy link
Member

di commented Nov 2, 2022

Is including a link to/ID for the specific run practical? That'd give a greater degree of auditability.

Yeah, would be good to include this with the publishing event/journal, and we can figure out how to externalize it in a followup PR.

@woodruffw
tests, warehouse: expose verified claims with the provider
8e3d5b1 
We can eventually use this to put more information in the token
creation event, for nicer renderings.

Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
Copy link
Member Author

Yeah, would be good to include this with the publishing event/journal, and we can figure out how to externalize it in a followup PR.

Did this with 8e3d5b1 -- nothing is exposed yet, but we now have access to the claims set that the verified JWT contained.

@woodruffw
Merge remote-tracking branch 'upstream/main' into tob-oidc-macaroon-m…
b9363e6 
…inting
@woodruffw
publishing: put each publisher's URL in the table
98f4400 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
warehouse: make translations
7a42cee 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw requested a review from di November 3, 2022 00:52
@woodruffw woodruffw mentioned this pull request Nov 3, 2022
Closed
di
di reviewed Nov 6, 2022
View reviewed changes
@woodruffw
oidc: refactor claim handling
f48da18 
Tests not updated, yet.

Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
Merge remote-tracking branch 'upstream/main' into tob-oidc-macaroon-m…
171100c 
…inting
@woodruffw
tests, warehouse: update tests, bring coverage back up
eec686e 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
Merge remote-tracking branch 'upstream/main' into tob-oidc-macaroon-m…
2ee080a 
…inting
@woodruffw woodruffw requested a review from di November 7, 2022 19:31
@woodruffw
Copy link
Member Author

This should be up-to-date and ready to go again.

@woodruffw
warehouse, tests: use provider URL in macaroon comment
667c9fc 
Signed-off-by: William Woodruff <william@trailofbits.com>
@di di mentioned this pull request Nov 11, 2022
Closed
di
di approved these changes Nov 15, 2022
View reviewed changes
Copy link
Member

@di di left a comment

Choose a reason for hiding this comment

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

LGTM. Can you bring this up to date so we can merge? I don't have the ability to do so.

@woodruffw
Copy link
Member Author

LGTM. Can you bring this up to date so we can merge? I don't have the ability to do so.

Done. I also need to rebase the migration, one moment...

@woodruffw
warehouse/migrations: rebase
9c53f2b 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
Copy link
Member Author

Okay, should be good to go.

@di di merged commit 4841c5b into pypi:main Nov 15, 2022
@woodruffw woodruffw deleted the tob-oidc-macaroon-minting branch November 15, 2022 18:51
@woodruffw
Copy link
Member Author

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@di di di approved these changes

@ewdurbin ewdurbin Awaiting requested review from ewdurbin

@dstufft dstufft Awaiting requested review from dstufft

Assignees

@woodruffw woodruffw

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Routes and endpoints for JWT consumption
4 participants
@woodruffw @di @alex @dstufft