oathkeeper icon indicating copy to clipboard operation
oathkeeper copied to clipboard

feat: introduce auth scheme and jumping to next authentication

Open Marlinc opened this issue 2 years ago • 3 comments

Some (legacy) application don't use Bearer as auth scheme in the Authorization 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 to Bearer when the header is set to Authorization. The second change is that the session store endpoint can now return a HTTP 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

  • [x] I have read the contributing guidelines.
  • [ ] I have referenced an issue containing the design document if my change introduces a new feature.
  • [x] I am following the contributing code guidelines.
  • [x] I have read the security policy.
  • [x] I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security. vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
  • [x] I have added tests that prove my fix is effective or that my feature works.
  • [ ] I have added or changed the documentation.

Further Comments

Marlinc avatar Jul 04 '22 13:07 Marlinc

Codecov Report

Merging #982 (2e345a4) into master (1738e61) will decrease coverage by 11.47%. The diff coverage is 100.00%.

:exclamation: Current head 2e345a4 differs from pull request most recent head 64e332f. Consider uploading reports for the commit 64e332f to get more accurate results

@@             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     
Impacted Files Coverage Δ
helper/bearer.go 100.00% <100.00%> (ø)
pipeline/authn/authenticator_cookie_session.go 83.33% <100.00%> (+4.08%) :arrow_up:
metrics/prometheus.go 93.33% <0.00%> (-2.42%) :arrow_down:
rule/fetcher_default.go 76.65% <0.00%> (-1.92%) :arrow_down:
credentials/fetcher_default.go 64.89% <0.00%> (-1.07%) :arrow_down:
pipeline/mutate/mutator_hydrator.go 65.09% <0.00%> (-1.00%) :arrow_down:
metrics/middleware.go 97.36% <0.00%> (-0.51%) :arrow_down:
driver/registry_memory.go 89.91% <0.00%> (-0.18%) :arrow_down:
cmd/serve.go 100.00% <0.00%> (ø)
driver/health/event_manager_default.go 94.11% <0.00%> (ø)
... and 44 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Jul 04 '22 13:07 codecov[bot]

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 04 '22 14:07 CLAassistant

This would potentially be solved by: https://github.com/ory/oathkeeper/pull/949

aeneasr avatar Sep 10 '22 08:09 aeneasr