Skip to content
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: allow fallback when using JWT #958

Merged
merged 1 commit into from
Apr 24, 2022
Merged

feat: allow fallback when using JWT #958

merged 1 commit into from
Apr 24, 2022

Conversation

jnodorp-jaconi
Copy link
Contributor

We try to use the jwt authenticator alongside the bearer_token authenticator. Our application receives requests with different token type, both in the Authorization header. The bearer_token tokens are not JWTs.

Currently this is not possible, as the JWT authenticator declares itself responsible as long as any token is provided at the correct location.

With the proposed change, the JWT authenticator will only declare itself responsible if the token at least looks like a JWT (<header>.<payload>.<signature>). That way, a subsequent authenticator can attempt authentication with the same token.

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • 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.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

@jnodorp-jaconi jnodorp-jaconi requested a review from aeneasr as a code owner April 22, 2022 09:35
@codecov
Copy link

codecov bot commented Apr 22, 2022

Codecov Report

Merging #958 (45c57fb) into master (0a52541) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #958      +/-   ##
==========================================
+ Coverage   65.96%   65.98%   +0.01%     
==========================================
  Files         106      106              
  Lines        4716     4718       +2     
==========================================
+ Hits         3111     3113       +2     
  Misses       1324     1324              
  Partials      281      281              
Impacted Files Coverage Δ
internal/httpclient/models/generic_error.go 0.00% <ø> (ø)
...ernal/httpclient/models/health_not_ready_status.go 0.00% <ø> (ø)
internal/httpclient/models/json_web_key.go 0.00% <ø> (ø)
internal/httpclient/models/json_web_key_set.go 0.00% <ø> (ø)
internal/httpclient/models/rule_handler.go 0.00% <ø> (ø)
internal/httpclient/models/rule_match.go 0.00% <ø> (ø)
internal/httpclient/models/upstream.go 0.00% <ø> (ø)
internal/httpclient/models/version.go 0.00% <ø> (ø)
pipeline/authn/authenticator_jwt.go 83.05% <100.00%> (+0.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84a0fe0...45c57fb. Read the comment docs.

Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea!

@aeneasr aeneasr merged commit 6959524 into ory:master Apr 24, 2022
@jnodorp-jaconi jnodorp-jaconi deleted the fallback-tokens branch April 26, 2022 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants