-
-
Notifications
You must be signed in to change notification settings - Fork 362
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
feat: introduce auth scheme and jumping to next authentication #982
base: master
Are you sure you want to change the base?
Conversation
f64c7a0
to
fb908e1
Compare
Codecov Report
@@ Coverage Diff @@
## master #982 +/- ##
===========================================
- Coverage 77.79% 66.31% -11.48%
===========================================
Files 81 107 +26
Lines 3873 4771 +898
===========================================
+ Hits 3013 3164 +151
- Misses 587 1325 +738
- Partials 273 282 +9
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
41d2dc6
to
0a4adec
Compare
0a4adec
to
2e345a4
Compare
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.
Thank you @Marlinc and sorry. for the late review! I'm wondering if it would make more sense to use a generic authenticator in such a case? The bearer token authenticator is supposed to solve https://datatracker.ietf.org/doc/html/rfc6750 which is expecting the Bearer
to be in the header. If that's not the case, wouldn't it be a different type of authentication (same mechanism maybe, but a different HTTP auth mechanism).
We could add another authenticator that handles this (e.g. a more generic one)
This would potentially be solved by: #949 |
2e345a4
to
64e332f
Compare
Some (legacy) application don't use
Bearer
as auth scheme in theAuthorization
header, in the current implementation this means Oathkeeper cannot be used to handle these legacy applications. The bearer token (and cookie) authenticator currently also don't support handling multiple authentication configurations (for example an access token in a header but also in a query parameter).This change adds an optional
auth_scheme
option to the bearer token authenticator that allows changing the scheme it accepts, it also supports setting an empty scheme for applications that provide a token directly in a header. For compatibility it defaults toBearer
when the header is set toAuthorization
. The second change is that the session store endpoint can now return aHTTP 406 Not Acceptable
response to tell Oathkeeper to jump to the next authentication rule.This changes were previously discussed on Slack.
I will add some documentation too about these new features.
Related issue(s)
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security. vulnerability, I
confirm that I got green light (please contact
security@ory.sh) from the maintainers to push
the changes.
works.
Further Comments