-
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
Add mutator for modifying authenticationSession with external API #240
Conversation
sorry - I squash-merged the other PR causing some merge conflicts |
No problem :) fortunately, there are not many conflicts |
Co-Authored-By: Jakub Kabza <[email protected]>
This looks great!! All that's left is adding docs here: https://github.com/ory/docs/blob/master/docs/oathkeeper/pipeline/mutator.md :) |
…rovide a test case for request with empty Extras field
pipeline/mutate/mutator_hydrator.go
Outdated
|
||
type externalAPIConfig struct { | ||
Url string `json:"url"` | ||
Authn *Authentication `json:"authn,omitempty"` |
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 would probably be authorization
right? Because we're modifying HTTP Authorization?
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, we are modifying HTTP Authorization
header, but it is a header used for authentication. And the config is actually specifying Authentication scheme and credentials. But if this argumentation doesn't convince you and you prefer it to be Authorization
I will not argue to change that 😄
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.
We could also just change it to auth
, then it implies both 😅
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.
Ok, changed it. Let me know what you think about it now 😄
I will adjust documentation in a second
Wohoooo! |
Related issue
#228
@aeneasr @piotrmsc
Proposed changes
As stated in the linked issue - this PR adds a mutator which make an upstream call to an API with AuthenticationSession as payload and expect AuthenticationSession as a response.
Checklist
vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
Further comments