-
Notifications
You must be signed in to change notification settings - Fork 658
Add PUT /api/v1/trusted_publishing/tokens
API endpoint
#11131
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
b758acc
diesel_helpers: Adjust `lower()` fn to also accept `NULL`
Turbo87 7e5faac
trustpub: Implement `load_jwks()` fn
Turbo87 5ebab45
trustpub: Add `GITHUB_ISSUER_URL` constant
Turbo87 ecf5455
trustpub: Add `OidcKeyStore` trait
Turbo87 a09aa72
trustpub: Implement `OidcKeyStore` trait
Turbo87 972d3bf
trustpub: Add mock `OidcKeyStore` implementation
Turbo87 f5e8363
trustpub: Add RSA keys for testing purposes
Turbo87 8d16fa0
trustpub: Add `MockOidcKeyStore::with_test_key()` fn
Turbo87 2c7096b
trustpub: Implement `extract_workflow_filename()` fn
Turbo87 9f64605
trustpub: Implement `UnverifiedClaims::decode()` fn
Turbo87 0fd9bc9
trustpub: Implement `GitHubClaims` struct
Turbo87 4139030
trustpub: Implement `FullGitHubClaims` struct for testing purposes
Turbo87 f2aaf16
trustpub: Implement `AccessToken` struct
Turbo87 03dd068
config: Add `TRUSTPUB_AUDIENCE` setting
Turbo87 bbdadbe
App: Add `oidc_key_stores` hashmap
Turbo87 98e79e4
AppBuilder: Add `trustpub_providers()` fn
Turbo87 322da35
bin/server: Use `TRUSTPUB_PROVIDERS` env var to configure Trusted Pub…
Turbo87 3d12eb1
tests/TestAppBuilder: Add `with_oidc_keystore()` fn
Turbo87 2c8c845
Implement `PUT /api/v1/trusted_publishing/tokens` API endpoint
Turbo87 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
config: Add
TRUSTPUB_AUDIENCE
setting
This defaults to the domain name (crates.io / staging.crates.io) and controls the expected `aud` claim of the OIDC JWT in the Trusted Publishing token exchange.
- Loading branch information
commit 03dd0684fa50f3fec0fcb8c08cac12f31b7dde5a
There are no files selected for viewing
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
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
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.
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.
this allows us to configure the expected
aud
claim, if necessary. it defaults to the domain name (crates.io / staging.crates.io).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.
Nice! This may or may not be applicable in your case, but noting: PyPI intentionally chose not to use the domain here, since we thought we might want to change the authentication and/or exchange subdomain at some point. So PyPI uses
pypi
and TestPyPI usestestpypi
.(This probably doesn't matter in practice, just flagging as a very small difference! In practice, any identifier that serves as a domain separator is fine, DNS name or whatever else 🙂)
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.
Yeah, this reminds me that we might encounter a
.io
ccTLD issue in the future, even though we still have about 5 years left. It seems that not using the domain in the same way PyPI does makes some sense here, but I guess sticking with.io
even if it gets removed probably won't be a big deal.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.
Yeah, the potential
.io
transition would be a good reason to avoid using a DNS name as the "domain separator" here. But you're also 100% right that usingaud: crates.io
will keep on working just fine even if the Crates domain isn't that, since it's solely a mutually agreed audience of arbitrary structure 🙂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.
aud: crates
seems more confusing and unspecific to me thanaud: crates.io
.aud: crates-io
has basically the same problem asaud: crates.io
. sinceaud: crates.io
would keep working even if we actually lost the ccTLD at some point I don't think it's worth introducing another name for it. if we had to migrate we could still allow both ccTLDs for the migration period.