bharos commented on code in PR #10437:
URL: https://github.com/apache/gravitino/pull/10437#discussion_r2943019441
##########
web-v2/web/src/lib/auth/providers/oidc.js:
##########
@@ -67,17 +67,17 @@ export class OidcOAuthProvider extends BaseOAuthProvider {
let user = await this.userManager.getUser()
if (user && !user.expired) {
- // For JWKS validation, we need the ID token (JWT format), not the
access token
- return user.id_token || user.access_token
+ // Use access token for API requests per OAuth2 spec
+ return user.access_token || user.id_token
Review Comment:
can this reversal of order cause any issues with other OAuth providers (like
Azure AD), or will it work seamlessly
On thinking about this, I feel logically this is correct. as we should use
access_token to access gravitino which is a resource server acc. to oauth. so
probably this should be ok..
Concern I had is from oauth spec:
https://oauth.net/id-tokens-vs-access-tokens/
The access_token is always not JWT, but our validator expects JWT token.
But in most cases I think it should work. we can revisit if we see any problems
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
