-
Notifications
You must be signed in to change notification settings - Fork 365
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
Impl: JWT claim authorization #4167
Impl: JWT claim authorization #4167
Conversation
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4167 +/- ##
==========================================
+ Coverage 65.75% 65.90% +0.15%
==========================================
Files 197 197
Lines 23565 23762 +197
==========================================
+ Hits 15494 15661 +167
- Misses 6967 6986 +19
- Partials 1104 1115 +11 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
6efa728
to
f6a1c85
Compare
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
d77a4cb
to
21fd4cb
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.
one nit, but I am also thinking that which JWT this is now going to use? There is use-case for both access_token
and id_token
. Is there way to specify which one it should use?
It's up to you. A JWT provider must be defined in the JWT configuration within the same If we want to use the ID Token retreived from the OIDC, we need to modify the OAuth2 filter to allow it to add the ID Token in the headers. |
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
I can get this working with idtoken by following configuration (it verifies sub AND that I have some single group that is needed... just for testing)
great job @zhaohuabing |
|
@arkodg do you have idea who could get progress in this PR? its waiting for review |
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
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 !
Implementation for: #4009
Related issue: #3367
This PR also changes the default value of
PayloadInMetada
of the JWT filter fromJwtProvider.Issuer
toJwtProvider.Name
asJwtProvider.Issuer
is optional and we need the metadata in the authorization.