gada121982 commented on code in PR #10437:
URL: https://github.com/apache/gravitino/pull/10437#discussion_r2944173152
##########
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:
Agree with your analysis. Gravitino validators (`StaticSignKeyValidator`,
`JwksTokenValidator`) both explicitly require JWT format — opaque tokens fail
at parse stage regardless of which token the frontend sends. With
Keycloak/Azure AD, `access_token` is always JWT. The `|| id_token` fallback
covers edge cases where `access_token` might be absent. For providers that
issue opaque access tokens (e.g., Google), Gravitino server-side would need an
introspection-based validator — that's a separate concern from this PR.
##########
web-v2/web/src/lib/store/auth/index.js:
##########
@@ -102,8 +102,32 @@ export const logoutAction =
createAsyncThunk('auth/logoutAction', async ({ route
try {
const provider = await oauthProviderFactory.getProvider()
if (provider) {
+ // For OIDC providers, use signoutRedirect to end Keycloak session
+ if (provider.getUserManager) {
+ const userManager = provider.getUserManager()
+ if (userManager) {
+ // Clear legacy auth tokens before redirect
+ localStorage.removeItem('accessToken')
+ localStorage.removeItem('authParams')
+ localStorage.removeItem('expiredIn')
+ localStorage.removeItem('isIdle')
+ localStorage.removeItem('version')
+
+ dispatch(clearIntervalId())
+ dispatch(setAuthToken(''))
+
+ // Redirect to IdP logout endpoint — this will navigate away from
the app
+ // signoutRedirect() must be called before clearAuthData() because
it needs
+ // the stored id_token for the id_token_hint parameter in
RP-initiated logout
+ await userManager.signoutRedirect()
+
+ await provider.clearAuthData()
+
+ return { token: null }
+ }
+ }
+
await provider.clearAuthData()
Review Comment:
Done — added `return { token: null }` in 40423476a.
--
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]
