-
Notifications
You must be signed in to change notification settings - Fork 359
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
Extend ID_TOKEN mutator with custom claims #243
Conversation
pipeline/mutate/mutator_id_token.go
Outdated
@@ -41,23 +44,29 @@ type MutatorIDTokenRegistry interface { | |||
} | |||
|
|||
type CredentialsIDTokenConfig struct { | |||
Audience []string `json:"aud"` | |||
Audience []string `json:"aud"` |
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.
I guess the question is if we want to keep this or not. As now we can use the claims portion instead.
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.
True. Do you want me to delete it?
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, I think that makes sense - but we have to document that in the upgrade guide .)
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.
Sure! I'm on it!
19af64c
to
4826011
Compare
If you need help with the e2e tests lmk |
pipeline/mutate/mutator_id_token.go
Outdated
templateID := rl.GetID() | ||
tmpl = a.t.Lookup(templateID) | ||
if tmpl == nil { | ||
tmpl, err = a.t.New(templateID).Parse(string(config)) |
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 should be Prase(string(CredentialsIDTokenConfig.Claims)
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.
Wait, are you sure? I thought we execute the template first to avoid those parsing errors!
f3f3ac2
to
92aa903
Compare
Thank you so much for your work - I've made some changes to this PR and merged it with #246 :) |
Related issue
#228
Proposed changes
Checklist
vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.