-
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
fix tls with jwt authn usecase #1856
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1856 +/- ##
==========================================
+ Coverage 65.87% 66.14% +0.26%
==========================================
Files 86 86
Lines 12924 12867 -57
==========================================
- Hits 8514 8511 -3
+ Misses 3866 3827 -39
+ Partials 544 529 -15
|
f9d8c61
to
61cfb8b
Compare
can you add test case for this? |
9f52017
to
c39ccc5
Compare
/retest |
1 similar comment
/retest |
27de98e
to
df38ab4
Compare
/retest |
@zirain somehow |
seems related to #1859 |
/retest |
/retest |
Signed-off-by: tanujd11 <[email protected]>
Signed-off-by: tanujd11 <[email protected]>
Signed-off-by: tanujd11 <[email protected]>
Signed-off-by: tanujd11 <[email protected]>
58a8c76
to
8a61129
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.
LGTM, thanks !
What type of PR is this?
Bug Fix
What this PR does / why we need it:
HTTPS and JWT authentication was not working in tandem because for some reason we were getting the information for JWTRequirement from HCM in DefaultFilterChain. DefaultFilterChain is not populated in case of HTTPS.
The question is irRoute already have all the information about JWT Requirement as the HCM of listener is also populated from route only which only checks if the listener has JWT Filter on the route and patch listener. Then why reverse check is done if httpfilter has the JWT requirements?
Which issue(s) this PR fixes:
Fixes #1768