-
Notifications
You must be signed in to change notification settings - Fork 137
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
Extract additional claims from github-workflow token #306
Conversation
Thanks for your comment @nsmith5, I will add the extensions and the docs. |
With reusable github-workflows the "job_workflow_ref" will reference the shared workflow instead the actual calling workflow. Fixes #305 Signed-off-by: Christian Kotzbauer <git@ckotzbauer.de>
@nsmith5 I added the new claims to the signing-cert and the docs now. |
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, great work! I'm not a maintainer so can't approve the workflow / merge, but this looks good to me
cc @mattmoor for 👀 |
cc @dlorenc to merge/deploy once CI passes. |
The code looks good to me; one thought though: should we consider using a separate child OID for GitHub-specific fields rather than keeping them all at the same depth? If we add information from other CI/CD systems it might make sense to separate them, but I'm curious to hear what others think. |
I'd be fine with that. I think we can nest them arbitrarily long. |
@dlorenc @bobcallaway Should the nesting be part of this PR? If yes, how is it supposed to look like? |
I'm fine either way since we already have some un-nested Github OIDs. We could start nesting other providers going forward. Wdyt @bobcallaway? |
this PR is fine as is, we can nest them in a separate one. |
With reusable github-workflows the "job_workflow_ref" will reference the shared workflow instead the actual calling workflow.
Summary
This adds the additional infos
repository
,workflow
andref
from the Github-Workflow OIDC token as described in #305. According to the Github-Docs I added therepository
field instead of therepo
.Ticket Link
Fixes #305
Release Note