feat(admin-ui): sessions not revoked after password/role/ user activation status change #2576
Conversation
…s change Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
📝 WalkthroughWalkthroughAdds Admin UI session revocation on user updates, changes logout route to Changes
Sequence Diagram(s)sequenceDiagram
participant AdminUI as Admin UI
participant Backend as Backend API
participant SessionAPI as Session API
participant Axios as AXIOS_INSTANCE
AdminUI->>Backend: PATCH /user (change password/role/status)
Backend-->>AdminUI: 200 OK
AdminUI->>AdminUI: revokeSessionMutation.mutateAsync()
AdminUI->>SessionAPI: DELETE /app/admin-ui/oauth2/session/{user_dn} (via AXIOS_INSTANCE)
SessionAPI-->>AdminUI: 204 No Content
Note over Backend,Axios: Later request returns 403
AdminUI->>Backend: GET /protected
Backend-->>Axios: 403 Forbidden (response)
Axios->>Backend: POST /app/admin-ui/oauth2/api-protection-token (fetch token)
Backend-->>Axios: 200 { access_token }
Axios->>SessionAPI: DELETE /app/admin-ui/oauth2/session (with token)
SessionAPI-->>Axios: 204 No Content
Axios-->>AdminUI: trigger redirect to /admin/logout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mo-auto
added
comp-admin-ui
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
admin-ui/plugins/user-management/components/PasswordChangeModal.tsx (2)
138-148: Missing error handling for session revocation calls.If
revokeSessionMutationorAXIOS_INSTANCE.deletefails after the password change has already succeeded, the unhandled rejection will propagate. Consider wrapping these calls in a try/catch similar to the audit logging pattern, or at least logging failures without blocking the success flow.Proposed fix
await changePasswordMutation.mutateAsync({ inum: userDetails.inum, data: patchOperations, }) - // Revoke user session after password change - await revokeSessionMutation.mutateAsync({ userDn: userDetails?.dn || '' }) - // Additional safeguard to delete Admin UI sessions - await AXIOS_INSTANCE.delete( - `/app/admin-ui/oauth2/session/${encodeURIComponent(userDetails?.dn || '')}`, - ) + // Revoke user session after password change (best-effort) + try { + await revokeSessionMutation.mutateAsync({ userDn: userDetails?.dn || '' }) + await AXIOS_INSTANCE.delete( + `/app/admin-ui/oauth2/session/${encodeURIComponent(userDetails?.dn || '')}`, + ) + } catch (error) { + console.error('Failed to revoke user session:', error) + }
150-150: MissingrevokeSessionMutationin dependency array.
revokeSessionMutationis used insidesubmitChangePasswordbut is not listed in the dependency array. This violates React hooks exhaustive-deps rules and could lead to stale closures.Proposed fix
- [userDetails, password, changePasswordMutation], + [userDetails, password, changePasswordMutation, revokeSessionMutation],admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
110-139: Race condition: session revocation may execute before user update completes or succeeds.
updateUserMutation.mutate()at line 110 is non-blocking, so execution immediately proceeds to the session revocation logic. This means:
- Sessions could be revoked while the user update is still in-flight
- If the user update fails (triggers
onError), sessions would already be revokedThe session revocation should only occur after the user update succeeds. Consider using
mutateAsyncwith proper sequencing, or moving the session revocation logic into theonSuccesscallback.Proposed fix using mutateAsync
- updateUserMutation.mutate({ + await updateUserMutation.mutateAsync({ data: { inum: userDetails?.inum, ...standardFieldValues, customAttributes, dn: userDetails?.dn || '', ...(persistenceType === 'ldap' && { customObjectClasses: ['top', 'jansPerson', 'jansCustomPerson'], }), modifiedFields: Object.keys(modifiedFields).map((key) => ({ [key]: modifiedFields[key], })), performedOn: { user_inum: userDetails?.inum, userId: userDetails?.displayName, }, action_message: userMessage, } as CustomUser, }) const anyKeyPresent = revokeSessionWhenFieldsModifiedInUserForm.some((key) => Object.prototype.hasOwnProperty.call(modifiedFields, key), ) if (anyKeyPresent) { - // Revoke user session after password change - await revokeSessionMutation.mutateAsync({ userDn: userDetails?.dn || '' }) - // Additional safeguard to delete Admin UI sessions - await AXIOS_INSTANCE.delete( - `/app/admin-ui/oauth2/session/${encodeURIComponent(userDetails?.dn || '')}`, - ) + // Revoke user session after successful update (best-effort) + try { + await revokeSessionMutation.mutateAsync({ userDn: userDetails?.dn || '' }) + await AXIOS_INSTANCE.delete( + `/app/admin-ui/oauth2/session/${encodeURIComponent(userDetails?.dn || '')}`, + ) + } catch (error) { + console.error('Failed to revoke user session:', error) + } }Note: Using
mutateAsyncmeans theonSuccess/onErrorcallbacks will still fire, but exceptions will also propagate. The navigation inonSuccesswill execute, then the session revocation will run. You may want to consolidate the post-success logic.
🤖 Fix all issues with AI agents
In @admin-ui/plugins/user-management/components/UserEditPage.tsx:
- Line 141: submitData uses revokeSessionMutation but the hook's dependency
array only lists [personAttributes, userDetails, persistenceType,
standardFields, updateUserMutation]; add revokeSessionMutation to that
dependency array so the hook complies with exhaustive-deps (or alternatively
memoize/referentially-stabilize revokeSessionMutation before using it). Update
the dependency array referenced in the useCallback/useEffect that defines
submitData to include revokeSessionMutation.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/plugins/user-management/components/PasswordChangeModal.tsxadmin-ui/plugins/user-management/components/UserEditPage.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.
Applied to files:
admin-ui/plugins/user-management/components/UserEditPage.tsx
🧬 Code graph analysis (2)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
admin-ui/api-client.ts (1)
AXIOS_INSTANCE(9-9)
admin-ui/plugins/user-management/components/PasswordChangeModal.tsx (1)
admin-ui/api-client.ts (1)
AXIOS_INSTANCE(9-9)
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
110-142: Race condition: session revocation runs before user update completes.
updateUserMutation.mutate()is fire-and-forget and not awaited, so the session revocation logic (lines 134-141) executes immediately—potentially before the update completes or even if it fails. The comment says "after successful update" but the code doesn't wait for success.Compare with
PasswordChangeModal.tsxwhich correctly usesawait changePasswordMutation.mutateAsync()before revoking.🐛 Proposed fix: use mutateAsync and await before revoking
- updateUserMutation.mutate({ + await updateUserMutation.mutateAsync({ data: { inum: userDetails?.inum, ...standardFieldValues, customAttributes, dn: userDetails?.dn || '', ...(persistenceType === 'ldap' && { customObjectClasses: ['top', 'jansPerson', 'jansCustomPerson'], }), modifiedFields: Object.keys(modifiedFields).map((key) => ({ [key]: modifiedFields[key], })), performedOn: { user_inum: userDetails?.inum, userId: userDetails?.displayName, }, action_message: userMessage, } as CustomUser, })Note: This changes control flow—you'll need to handle errors appropriately and may need to adjust the
onSuccesscallback to not perform navigation/toast if that logic moves inline.admin-ui/plugins/user-management/components/PasswordChangeModal.tsx (1)
153-153: MissingrevokeSessionMutationin dependency array.
revokeSessionMutationis used on line 140 but is not listed in theuseCallbackdependencies.Proposed fix
- [userDetails, password, changePasswordMutation], + [userDetails, password, changePasswordMutation, revokeSessionMutation],
🤖 Fix all issues with AI agents
In @admin-ui/plugins/user-management/components/UserEditPage.tsx:
- Line 144: The useCallback that captures revokeSessionMutation has an
incomplete dependency array; add revokeSessionMutation to the dependency array
(alongside personAttributes, userDetails, persistenceType, standardFields,
updateUserMutation) so the callback updates when revokeSessionMutation changes
and prevents stale closures.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/plugins/user-management/components/PasswordChangeModal.tsxadmin-ui/plugins/user-management/components/UserEditPage.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.
Applied to files:
admin-ui/plugins/user-management/components/UserEditPage.tsx
🧬 Code graph analysis (2)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
admin-ui/api-client.ts (1)
AXIOS_INSTANCE(9-9)
admin-ui/plugins/user-management/components/PasswordChangeModal.tsx (1)
admin-ui/api-client.ts (1)
AXIOS_INSTANCE(9-9)
🔇 Additional comments (1)
admin-ui/plugins/user-management/components/PasswordChangeModal.tsx (1)
134-146: Session revocation implementation looks correct.The use of
await changePasswordMutation.mutateAsync()before the session revocation ensures sessions are only revoked after a successful password change. The try/catch with error logging is appropriate.
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
110-142: Race condition: Session revocation runs before user update completes.
updateUserMutation.mutate()on line 110 is non-blocking and returns immediately. The session revocation logic (lines 134-141) executes regardless of whether the user update succeeds or fails. This contradicts the comment on line 133 and could revoke sessions even when the update fails.Move the revocation logic inside the
onSuccesscallback, or switch tomutateAsync:🐛 Proposed fix using mutateAsync
- updateUserMutation.mutate({ + try { + await updateUserMutation.mutateAsync({ data: { inum: userDetails?.inum, ...standardFieldValues, customAttributes, dn: userDetails?.dn || '', ...(persistenceType === 'ldap' && { customObjectClasses: ['top', 'jansPerson', 'jansCustomPerson'], }), modifiedFields: Object.keys(modifiedFields).map((key) => ({ [key]: modifiedFields[key], })), performedOn: { user_inum: userDetails?.inum, userId: userDetails?.displayName, }, action_message: userMessage, } as CustomUser, - }) + }) + } catch { + // Error already handled by onError callback + return + } + const anyKeyPresent = revokeSessionWhenFieldsModifiedInUserForm.some((key) => Object.prototype.hasOwnProperty.call(modifiedFields, key), ) if (anyKeyPresent) {
🤖 Fix all issues with AI agents
In @admin-ui/plugins/user-management/components/UserEditPage.tsx:
- Around line 139-141: The catch block in UserEditPage.tsx that currently logs
"Failed to revoke user session" should also notify the user via a UI toast; keep
the console.error but add a warning toast (using your app's notification helper
such as useToast/enqueueSnackbar/toast.warn) that shows a concise message like
"Could not revoke user session" and include the error message/details for
debugging; update the catch in the revoke session logic that contains the
"Failed to revoke user session" log to call the toast API with the message and
error, and ensure the notification is user-facing and styled as a warning.
- Around line 134-141: Guard against calling revokeSessionMutation.mutateAsync
or AXIOS_INSTANCE.delete with an empty DN by extracting const userDn =
userDetails?.dn and wrapping the revocation logic in if (userDn) { ... } in
UserEditPage (and mirror the same change in PasswordChangeModal); also review
the two calls—revokeSessionMutation.mutateAsync(...) and
AXIOS_INSTANCE.delete(...)—target the same endpoint (same path), so either
remove the redundant direct AXIOS call and rely on the mutation, or keep both
but add a clarifying comment explaining why both are required (e.g., one updates
client cache/state while the other forces server-side deletion).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/user-management/components/UserEditPage.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
admin-ui/api-client.ts (1)
AXIOS_INSTANCE(9-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sonar scan (flex-linux-setup)
🔇 Additional comments (2)
admin-ui/plugins/user-management/components/UserEditPage.tsx (2)
31-31: LGTM!Import is correctly added for the session revocation HTTP call.
144-151: LGTM!Dependency array correctly includes
revokeSessionMutationsince it's used inside the callback.
…s change Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
110-146: Race condition: session revoked before user update completes.
updateUserMutation.mutate()is fire-and-forget. The session revocation at lines 139-141 executes immediately, not after the update succeeds. If the update fails, the user's session is still revoked—contradicting the intent to revoke "after successful update."Either await the mutation or move the revocation into
onSuccess:Option 1: Use mutateAsync and await
- updateUserMutation.mutate({ + try { + await updateUserMutation.mutateAsync({ data: { inum: userDetails?.inum, ...standardFieldValues, customAttributes, dn: userDetails?.dn || '', ...(persistenceType === 'ldap' && { customObjectClasses: ['top', 'jansPerson', 'jansCustomPerson'], }), modifiedFields: Object.keys(modifiedFields).map((key) => ({ [key]: modifiedFields[key], })), performedOn: { user_inum: userDetails?.inum, userId: userDetails?.displayName, }, action_message: userMessage, } as CustomUser, - }) + }) + } catch { + // Error already handled by onError callback + return + } const anyKeyPresent = revokeSessionWhenFieldsModifiedInUserForm.some((key) =>Option 2: Move revocation to onSuccess callback
Pass the revocation logic to
usePutUser'sonSuccess, ensuring it only runs after a successful update. This requires restructuring to passmodifiedFieldscontext to the callback.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/user-management/components/UserEditPage.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/plugins/user-management/components/UserEditPage.tsx (2)
admin-ui/api-client.ts (1)
AXIOS_INSTANCE(9-9)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
dispatch(37-37)
🔇 Additional comments (3)
admin-ui/plugins/user-management/components/UserEditPage.tsx (3)
31-31: LGTM!Import correctly references the shared Axios instance for the session revocation API call.
148-155: LGTM!Dependency array now correctly includes
revokeSessionMutation, addressing the stale closure concern from previous reviews.
140-141: Verify if both revocation calls are necessary.
revokeSessionMutation.mutateAsyncand the directAXIOS_INSTANCE.deletecall appear to target the same session endpoint. If redundant, consolidate to avoid double network calls; if intentional (e.g., one manages React Query cache), add a clarifying comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…s change Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
110-145: Session revocation executes before update confirmation—race condition.
updateUserMutation.mutate()is fire-and-forget; the revocation logic on lines 129-145 runs immediately without waiting for the mutation to succeed. If the update fails, the user's session is still revoked, contradicting the PR objective.Move the revocation logic into the
onSuccesscallback, or switch tomutateAsyncand await it:🐛 Proposed fix using mutateAsync
- updateUserMutation.mutate({ + try { + await updateUserMutation.mutateAsync({ data: { inum: userDetails?.inum, ...standardFieldValues, customAttributes, dn: userDetails?.dn || '', ...(persistenceType === 'ldap' && { customObjectClasses: ['top', 'jansPerson', 'jansCustomPerson'], }), modifiedFields: Object.keys(modifiedFields).map((key) => ({ [key]: modifiedFields[key], })), performedOn: { user_inum: userDetails?.inum, userId: userDetails?.displayName, }, action_message: userMessage, } as CustomUser, - }) + }) + } catch { + // Error already handled by mutation's onError callback + return + } + const anyKeyPresent = revokeSessionWhenFieldsModifiedInUserForm.some((key) =>Alternatively, move the revocation logic into the existing
onSuccesscallback at lines 79-85, passingmodifiedFieldsvia closure or mutation variables.
♻️ Duplicate comments (1)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
142-144: Warning toast for session revocation failure appears to be missing.A previous review suggested adding a user-facing toast when session revocation fails, which was marked as addressed. However, the current code only logs to console. Silent failure may leave sessions active when administrators expect them revoked.
♻️ Proposed fix to add warning toast
} catch (error) { console.error('Failed to revoke user session:', error) + dispatch(updateToast(true, 'warning', t('messages.session_revocation_failed'))) }
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/user-management/components/UserEditPage.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
admin-ui/api-client.ts (1)
AXIOS_INSTANCE(9-9)
🔇 Additional comments (3)
admin-ui/plugins/user-management/components/UserEditPage.tsx (3)
31-31: LGTM!The import correctly references the shared Axios instance for making authenticated API calls.
147-154: LGTM!The dependency array now correctly includes
revokeSessionMutation, addressing the previous stale closure concern. All dependencies used within the callback are properly listed.
140-141: Clarify or consolidate the duplicate session revocation calls.Lines 140–141 make two sequential API calls in the same error handler:
revokeSessionMutation.mutateAsync({ userDn })(from JansConfigApi)AXIOS_INSTANCE.delete('/app/admin-ui/oauth2/session/{userDn}')This pattern is also repeated in PasswordChangeModal.tsx. If both target the same endpoint, remove the redundant call. If they serve different purposes (e.g., revoking Jans Auth Server sessions vs. Admin UI sessions), add a clarifying comment to explain the distinction.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…s change Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
…s change Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
admin-ui/plugins/auth-server/redux/sagas/SessionSaga.js (1)
30-30: Remove console.log before merging.Debug logging statements should be removed from production code.
🧹 Remove debug log
- console.log('SessionSaga: API response data:', data)admin-ui/plugins/saml/redux/sagas/SamlSaga.ts (1)
388-399: Rename function to match its new purpose and consider using centralized helper.The function name
handleFourZeroOneErroris now misleading since it handles 406 errors viaisFourZeroSixError. Additionally, other saga files (e.g.,WebhookSaga.ts,CustomScriptSaga.ts) use the centralizedredirectToLogout()helper fromSagaUtilsfor consistency.♻️ Suggested refactor
-function* handleFourZeroOneError( +function* handleSessionExpiredError( error: Error | { status?: number } | null, ): Generator<SelectEffect | PutEffect, void, string> { - const errorWithStatus = - error && typeof error === 'object' && 'status' in error - ? { status: (error as { status?: number }).status } - : null - if (isFourZeroSixError(errorWithStatus)) { + if (isFourZeroSixError(error)) { // Session expired - redirect to login window.location.href = '/admin/logout' } }Also update all call sites from
handleFourZeroOneErrortohandleSessionExpiredError.Note: The
isFourZeroSixErrorutility already handles bothAxiosErrorLikeandDirectStatusErrorshapes, so the manual status extraction may be unnecessary. However, verify this doesn't break error objects that don't conform to those shapes.admin-ui/app/redux/sagas/HealthSaga.ts (1)
51-53: Missing 406 error handling ingetHealthServerStatus.The
getHealthStatusfunction handles 406 errors with logout redirect, butgetHealthServerStatusonly dispatchesnullresponse without checking for session expiry. If both endpoints can return 406 on revoked sessions, this saga should also redirect.🔧 Proposed fix
} catch (e) { yield put(getHealthServerStatusResponse(null)) + if (isFourZeroSixError(e)) { + yield* redirectToLogout() + } }admin-ui/plugins/admin/redux/sagas/AssetSaga.ts (1)
56-65: Consider extracting repeated error handling pattern.The 406 error check with logout redirect is repeated 6 times in this file and across many other saga files. Consider extracting this into a reusable utility:
// In SagaUtils or TokenController export function* handleSessionError(e: unknown): SagaIterator<boolean> { if (isFourZeroSixError(e)) { yield* redirectToLogout() return true } return false }Usage would simplify each catch block:
if (yield* handleSessionError(e)) return return eThis would reduce duplication and make future changes to session error handling easier to maintain.
🤖 Fix all issues with AI agents
In `@admin-ui/api-client.ts`:
- Around line 36-47: The response interceptor on AXIOS_INSTANCE currently
contains an outdated inline comment and duplicates logout redirects with
saga-level handlers; remove the orphaned comment "OR call API logout explicitly
(see below)" from the interceptor, then pick one redirect strategy and
consolidate: either delete per-saga redirects (e.g., the window.location.href
assignments in JsonConfigSaga.js and replace any saga-level 406 handling with
relying on the AXIOS_INSTANCE interceptor), or remove this AXIOS_INSTANCE
interceptor and have sagas call redirectToLogout() centrally; ensure you update
references to AXIOS_INSTANCE.interceptors.response.use, JsonConfigSaga (the 406
handling locations), and the redirectToLogout utility accordingly so only one
redirect path remains.
In `@admin-ui/app/redux/sagas/AuthSaga.ts`:
- Around line 139-142: The catch block that checks isFourZeroSixError(error) and
calls redirectToLogout() is missing a return, causing inconsistent control flow
with other handlers; update the AuthSaga catch block so that after calling
yield* redirectToLogout() you immediately return to stop further execution
(match the pattern used in the other catch blocks that call redirectToLogout()),
referencing the isFourZeroSixError check and the redirectToLogout() generator to
locate the exact spot to change.
In `@admin-ui/app/redux/sagas/HealthSaga.ts`:
- Around line 35-38: The error handler in HealthSaga currently calls
window.location.href = '/admin/logout' when isFourZeroSixError(e) is true;
replace this direct navigation with the centralized redirectToLogout() utility
and add an import for redirectToLogout at the top of HealthSaga (to match the
pattern used in LicenseDetailsSaga). Locate the isFourZeroSixError check in
HealthSaga and change the body to call redirectToLogout(), ensuring no other
logic is altered.
In `@admin-ui/app/redux/sagas/LicenseDetailsSaga.ts`:
- Line 11: Replace the direct use of window.location.href = '/admin/logout' in
LicenseDetailsSaga with the centralized redirectToLogout() utility: add
redirectToLogout to the existing import from 'Redux/sagas/SagaUtils' (alongside
initAudit) and update every error handler that currently sets
window.location.href (the handlers around the sections flagged) to call
redirectToLogout() instead, preserving any surrounding logging or cleanup logic.
In `@admin-ui/app/redux/sagas/LockSaga.ts`:
- Around line 25-28: Replace the direct window.location.href = '/admin/logout'
in the LockSaga error handler with the centralized redirectToLogout() helper for
consistency: import or reference redirectToLogout where LockSaga handles
isFourZeroSixError(e) and call redirectToLogout() instead of assigning
window.location.href, matching the pattern used in WebhookSaga and other sagas.
In `@admin-ui/app/redux/sagas/OidcDiscoverySaga.ts`:
- Around line 23-26: Replace the direct redirect in OidcDiscoverySaga: import
the existing redirectToLogout helper and call redirectToLogout() in the
isFourZeroSixError(e) branch instead of assigning window.location.href =
'/admin/logout' so the logout path goes through the audit-logging helper; update
the top-level imports to include redirectToLogout and remove the direct
window.location.href usage inside the isFourZeroSixError handler.
In `@admin-ui/app/redux/sagas/ProfileDetailsSaga.ts`:
- Around line 50-55: The catch block in ProfileDetailsSaga uses an unnecessary
cast `e as { status?: number }` when calling isFourZeroSixError which narrows
the checked shapes and can miss Axios-style errors; update the catch so it calls
isFourZeroSixError(e) with the raw error (no cast), keep the existing behavior
of dispatching setUserProfileDetails(null) and calling yield* redirectToLogout()
on a 406, and ensure the surrounding code (catch block in the saga that
references setUserProfileDetails, isFourZeroSixError, and redirectToLogout)
compiles after removing the cast.
In `@admin-ui/plugins/admin/redux/sagas/ApiRoleSaga.js`:
- Around line 37-40: Replace the direct logout navigation in ApiRoleSaga.js
where you check isFourZeroSixError(e) and call window.location.href =
'/admin/logout' with the shared redirectToLogout() helper (import it from
Redux/sagas/SagaUtils alongside initAudit if not already imported); update every
occurrence of that pattern in this file (the other isFourZeroSixError branches)
so all sagas use redirectToLogout() for consistent logout handling.
In `@admin-ui/plugins/admin/redux/sagas/MappingSaga.js`:
- Around line 38-41: The repeated session-expired handling in MappingSaga.js
should use the centralized redirect helper: import redirectToLogout from
'Redux/sagas/SagaUtils' at the top and, inside each saga function (fetchMapping,
updateMapping, addMapping, deleteMapping), replace the window.location.href =
'/admin/logout' branch (the isFourZeroSix(e) check) with a call to
redirectToLogout() so all logout redirections are consistent and centralized.
In `@admin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.js`:
- Line 10: Remove the dead empty import "import {} from
'../../common/Constants'" in JsonConfigSaga.js; delete that import statement
entirely and, if any constants from '../../common/Constants' are actually needed
in this file, replace the empty import with the correct named imports (e.g.,
import { SOME_CONST } from '../../common/Constants') so you don't reintroduce
unused imports.
- Around line 34-37: JsonConfigSaga.js handles 406 by directly setting
window.location.href which bypasses centralized logout logic; import and call
the existing saga helper redirectToLogout() instead of window.location.href (use
yield* redirectToLogout() so it runs in generator context), and update both
occurrences (the earlier catch and the later block noted) to use the helper;
keep the isFourZeroSixError checks if you still need to detect 406, but replace
the direct assignment with yield* redirectToLogout() for consistency with
AttributesSaga/ProfileDetailsSaga and the global interceptor.
In `@admin-ui/plugins/auth-server/redux/sagas/MessageSaga.js`:
- Around line 58-61: The catch blocks in MessageSaga.js call
window.location.href directly when isFourZeroSixError(e) is true, bypassing
audit logging; import and use the existing redirectToLogout helper instead: add
redirectToLogout to the import list where other auth helpers are pulled in (line
~11) and replace all window.location.href = '/admin/logout' occurrences with a
call to redirectToLogout() so logout redirection goes through the centralized
audit-logging flow.
In `@admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js`:
- Around line 41-44: Replace direct redirects in OAuthScopeSaga.js with the
shared redirectToLogout generator from SagaUtils.ts: add redirectToLogout to the
existing import that already brings in initAudit, then in all catch blocks that
currently test isFourZeroSixError(e) and call window.location.href =
'/admin/logout', yield call(redirectToLogout) (or yield* redirectToLogout if
appropriate to its generator signature) so audit events are emitted before
navigation. Ensure every occurrence of window.location.href = '/admin/logout' in
this file is replaced and that isFourZeroSixError(e) branches delegate to
redirectToLogout for consistent logging.
In `@admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js`:
- Around line 35-38: The saga currently handles 406 session-expired cases by
directly setting window.location.href = '/admin/logout' inside UMAResourceSaga;
replace those direct assignments (both occurrences near where
isFourZeroSixError(e) is checked) with the centralized saga helper by importing
and calling yield* redirectToLogout() from Redux/sagas/SagaUtils (ensure the
redirectToLogout import is added if missing) so logout behavior is consistent
and testable across InitSaga/AuthSaga and this UMAResourceSaga.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (27)
admin-ui/api-client.tsadmin-ui/app/redux/sagas/AttributesSaga.tsadmin-ui/app/redux/sagas/AuthSaga.tsadmin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/app/redux/sagas/InitSaga.tsadmin-ui/app/redux/sagas/LicenseDetailsSaga.tsadmin-ui/app/redux/sagas/LockSaga.tsadmin-ui/app/redux/sagas/MauSaga.tsadmin-ui/app/redux/sagas/OidcDiscoverySaga.tsadmin-ui/app/redux/sagas/ProfileDetailsSaga.tsadmin-ui/app/redux/sagas/SagaUtils.tsadmin-ui/app/utilities.tsxadmin-ui/app/utils/TokenController.tsadmin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.jsadmin-ui/plugins/admin/redux/sagas/ApiRoleSaga.jsadmin-ui/plugins/admin/redux/sagas/AssetSaga.tsadmin-ui/plugins/admin/redux/sagas/CustomScriptSaga.tsadmin-ui/plugins/admin/redux/sagas/MappingSaga.jsadmin-ui/plugins/admin/redux/sagas/WebhookSaga.tsadmin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.jsadmin-ui/plugins/auth-server/redux/sagas/MessageSaga.jsadmin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.jsadmin-ui/plugins/auth-server/redux/sagas/OIDCSaga.jsadmin-ui/plugins/auth-server/redux/sagas/SessionSaga.jsadmin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.jsadmin-ui/plugins/saml/components/WebsiteSsoServiceProviderForm.tsxadmin-ui/plugins/saml/redux/sagas/SamlSaga.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js
📚 Learning: 2026-01-06T13:45:19.278Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts:4-101
Timestamp: 2026-01-06T13:45:19.278Z
Learning: In admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts, ensure Yup schema marks fields as nullable when the backend DTOs permit nulls and do not mark them as required. Align frontend validation with backend validation to prevent conflicts when editing existing configurations with null values; apply this pattern consistently to other Admin UI validation files where the backend is the source of truth.
Applied to files:
admin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/app/redux/sagas/ProfileDetailsSaga.tsadmin-ui/plugins/admin/redux/sagas/AssetSaga.tsadmin-ui/app/utils/TokenController.tsadmin-ui/app/redux/sagas/InitSaga.tsadmin-ui/app/redux/sagas/LicenseDetailsSaga.tsadmin-ui/api-client.tsadmin-ui/app/redux/sagas/OidcDiscoverySaga.tsadmin-ui/app/redux/sagas/AuthSaga.tsadmin-ui/plugins/admin/redux/sagas/WebhookSaga.tsadmin-ui/app/redux/sagas/MauSaga.tsadmin-ui/app/redux/sagas/SagaUtils.tsadmin-ui/plugins/admin/redux/sagas/CustomScriptSaga.tsadmin-ui/plugins/saml/redux/sagas/SamlSaga.tsadmin-ui/app/redux/sagas/LockSaga.tsadmin-ui/app/redux/sagas/AttributesSaga.ts
📚 Learning: 2026-01-07T09:56:06.244Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx:320-322
Timestamp: 2026-01-07T09:56:06.244Z
Learning: In the admin-ui Config API Properties forms (ApiConfigForm component in admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx), the error display pattern using `String(fieldError)` for top-level properties is acceptable because the DTO structure and validation schema design ensure that top-level errors are string messages, not nested error objects. Nested field validation is handled within JsonPropertyBuilderConfigApi components themselves.
Applied to files:
admin-ui/plugins/saml/components/WebsiteSsoServiceProviderForm.tsx
🧬 Code graph analysis (24)
admin-ui/plugins/auth-server/redux/sagas/MessageSaga.js (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/plugins/auth-server/redux/sagas/OIDCSaga.js (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.js (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/app/redux/sagas/HealthSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/app/redux/sagas/ProfileDetailsSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/plugins/admin/redux/sagas/AssetSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/app/redux/sagas/InitSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/app/redux/sagas/LicenseDetailsSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/app/redux/sagas/OidcDiscoverySaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/app/redux/sagas/AuthSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/plugins/saml/components/WebsiteSsoServiceProviderForm.tsx (1)
admin-ui/plugins/auth-server/components/Message/MessageForm.js (1)
formik(37-47)
admin-ui/plugins/admin/redux/sagas/ApiRoleSaga.js (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/plugins/admin/redux/sagas/WebhookSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/app/redux/sagas/MauSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/app/redux/sagas/SagaUtils.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/plugins/saml/redux/sagas/SamlSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/plugins/auth-server/redux/sagas/SessionSaga.js (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/app/redux/sagas/LockSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/plugins/admin/redux/sagas/MappingSaga.js (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
admin-ui/app/redux/sagas/AttributesSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)
🔇 Additional comments (30)
admin-ui/plugins/saml/components/WebsiteSsoServiceProviderForm.tsx (1)
484-487: Clarify intent: AI summary inconsistent with actual code.The AI summary states this change "removes the requirement that the field be touched," but the code still includes
&& formik.touched.samlMetadata?.nameIDPolicyFormat. The current implementation correctly follows the same validation pattern used by other fields in this form (e.g.,name,displayName,entityId).Additionally, this change appears unrelated to the PR's objective of session revocation after password/role/status changes. Please verify whether this change was intentional or accidentally included.
admin-ui/app/redux/sagas/AttributesSaga.ts (1)
32-37: LGTM - Consistent error handling pattern.The error handling correctly uses
isFourZeroSixErrorand delegates toredirectToLogout()viayield*, which is the proper pattern for saga delegation. This is consistent with the centralized logout handling approach.admin-ui/app/utilities.tsx (1)
8-8: LGTM - Minor formatting change.Trivial semicolon addition with no functional impact.
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js (1)
7-7: Import update looks correct.The import now includes
isFourZeroSixErrorto align with the new session expiration handling pattern.admin-ui/app/redux/sagas/SagaUtils.ts (2)
3-3: LGTM! Central utility updated correctly.The
redirectToLogouthelper now redirects to/admin/logoutand includes audit logging viaauditLogoutLogs. This is the correct centralized approach that other saga files should use.Also applies to: 23-26
42-45: Error handling updated correctly.The
handleResponseErrorutility now checks for 406 errors and delegates toredirectToLogout, maintaining the audit trail.admin-ui/plugins/auth-server/redux/sagas/MessageSaga.js (1)
2-2: Import update is correct.The import now uses
isFourZeroSixErrorfor the updated error handling pattern.admin-ui/plugins/auth-server/redux/sagas/SessionSaga.js (3)
2-2: LGTM! Correct import pattern.This file correctly imports both
isFourZeroSixErrorandredirectToLogout, using the centralized helper for session expiry handling.Also applies to: 9-9
37-40: Good pattern: UsingredirectToLogouthelper.This is the correct implementation. The
redirectToLogouthelper ensures audit logs are recorded before redirecting. Other saga files should follow this pattern.
35-35: Remove console.error or convert to structured logging.Consider using a proper logging mechanism instead of
console.errorfor production error tracking.⛔ Skipped due to learnings
Learnt from: kdhttps Repo: GluuFederation/flex PR: 2378 File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109 Timestamp: 2025-11-10T14:18:58.310Z Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.admin-ui/app/redux/sagas/OidcDiscoverySaga.ts (1)
4-4: Import update is correct.The import now uses
isFourZeroSixErrorfor the updated error handling pattern.admin-ui/app/redux/sagas/InitSaga.ts (1)
3-3: LGTM! Consistent error handling migration.The switch from
isFourZeroOneErrortoisFourZeroSixErroris applied consistently across all four saga functions (getScripts,getClients,getScopes,getAttributes). The use ofyield* redirectToLogout()helper ensures centralized logout behavior.Also applies to: 50-53, 68-71, 85-88, 102-105
admin-ui/app/redux/sagas/AuthSaga.ts (1)
26-27: LGTM! Consistent use of centralized logout helper.The migration to
isFourZeroSixErroris correctly applied, and usingyield* redirectToLogout()ensures logout behavior is centralized and maintainable.Also applies to: 72-75, 96-99, 166-169
admin-ui/plugins/saml/redux/sagas/SamlSaga.ts (1)
8-11: LGTM on import update.The import correctly switches to
isFourZeroSixErrorto handle the new 406-based session expiration logic.admin-ui/plugins/admin/redux/sagas/MappingSaga.js (1)
11-11: LGTM on import update.The import correctly switches to
isFourZeroSixErrorfor the new 406-based session expiration detection.admin-ui/app/redux/sagas/LockSaga.ts (1)
3-3: LGTM on import update.The import correctly switches to
isFourZeroSixErrorfor 406-based session expiration detection.admin-ui/plugins/admin/redux/sagas/WebhookSaga.ts (2)
31-31: LGTM on import update.The import correctly switches to
isFourZeroSixErrorfor 406-based session expiration detection.
79-82: LGTM on error handling updates.All saga functions consistently use
isFourZeroSixErrorfor session expiration detection and properly delegate toredirectToLogout()helper. This is the recommended pattern for the codebase.Also applies to: 110-113, 142-145, 174-177, 202-205, 234-237, 269-272, 356-359
admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts (2)
16-16: LGTM on import update.The import correctly switches to
isFourZeroSixErrorfor 406-based session expiration detection.
66-69: LGTM on error handling updates.All saga functions consistently use
isFourZeroSixErrorfor session expiration detection and properly delegate toredirectToLogout()helper. The pattern is clean and maintainable.Also applies to: 91-94, 121-124, 151-154, 177-180, 209-212
admin-ui/plugins/auth-server/redux/sagas/OIDCSaga.js (1)
2-2: LGTM! Consistent error handling migration to 406.The migration from
isFourZeroOneErrortoisFourZeroSixErroris applied consistently across all saga error handlers in this file. The use ofredirectToLogout()utility maintains a clean, centralized logout pattern.Also applies to: 49-52, 70-73, 94-97, 117-120, 143-146, 159-162
admin-ui/app/redux/sagas/MauSaga.ts (1)
3-3: LGTM! Consistent with the centralized logout pattern.The error handling correctly uses
isFourZeroSixErrorand theredirectToLogout()utility, maintaining consistency with other sagas.Also applies to: 29-32
admin-ui/app/utils/TokenController.ts (1)
35-40: Implementation is correct and aligned with backend contract.The function properly checks for HTTP 406 in both Axios and direct error shapes. The 406 status is intentionally used by the backend to signal forced session invalidation after sensitive admin changes (password, role, activation status), as evidenced by the recent feature commit and consistent usage throughout the codebase for triggering logout behavior.
admin-ui/plugins/admin/redux/sagas/AssetSaga.ts (7)
16-16: Import change aligns with session revocation strategy.The switch from
isFourZeroOneErrortoisFourZeroSixErroris consistent with the broader PR changes to detect revoked sessions via HTTP 406 responses.
56-65: Error handling correctly triggers logout on session revocation.The pattern is correct: error toast is shown, state is updated to null, then 406 detection redirects to logout. The early return prevents the error from being returned after redirect.
80-89: Consistent error handling pattern applied.
104-113: Consistent error handling pattern applied.
130-139: Consistent error handling pattern applied.
153-162: Consistent error handling pattern applied.
180-189: Consistent error handling pattern applied.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…s change Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
duttarnab
changed the title
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
admin-ui/app/redux/sagas/LockSaga.ts (1)
17-29: Unusedauditvariable.The
auditvariable is initialized on line 18 but never used within the function. Either utilize it for audit logging (e.g., in the catch block or after successful API calls) or remove theinitAuditcall if auditing isn't needed here.🔧 Option 1: Remove unused initAudit call
export function* getLockMau({ payload }) { - const audit = yield* initAudit() try { const lockapi = yield* newFunction() const data = yield call(lockapi.getLockMau, payload) yield put(getLockStatusResponse({ data })) } catch (e) { yield put(getLockStatusResponse(null)) if (isFourZeroSixError(e)) { // Session expired - redirect to login yield* redirectToLogout() } } }🔧 Option 2: Use audit for logging (if auditing is required)
export function* getLockMau({ payload }) { const audit = yield* initAudit() try { const lockapi = yield* newFunction() const data = yield call(lockapi.getLockMau, payload) yield put(getLockStatusResponse({ data })) + // Add audit logging on success if needed } catch (e) { yield put(getLockStatusResponse(null)) if (isFourZeroSixError(e)) { // Session expired - redirect to login yield* redirectToLogout() + } else { + // Consider logging other errors using audit } } }admin-ui/app/redux/sagas/HealthSaga.ts (1)
51-53: Missing 406 error handling ingetHealthServerStatus.
getHealthStatushandles 406 errors withredirectToLogout(), butgetHealthServerStatussilently swallows all errors. For consistency and proper session expiry handling, consider adding the same 406 check here.♻️ Proposed fix
} catch (e) { yield put(getHealthServerStatusResponse(null)) + if (isFourZeroSixError(e)) { + yield* redirectToLogout() + return + } }admin-ui/plugins/saml/redux/sagas/SamlSaga.ts (1)
388-399: Function name is misleading and missing return for early exit.The function is named
handleFourZeroOneErrorbut now checks for 406 errors viaisFourZeroSixError. Additionally, since this helper doesn't return a value or signal to callers that a redirect occurred, the calling code (e.g., lines 110-112) will continue to executeerrorToastafterredirectToLogout()is triggered, which is inconsistent with other sagas wherereturnprevents further execution.Consider either:
- Renaming to
handleFourZeroSixErrorand having it return a boolean so callers can early-exit- Inlining the check in each catch block (as done in other sagas) for consistent control flow
♻️ Option 1: Return boolean to signal redirect
-function* handleFourZeroOneError( +function* handleFourZeroSixError( error: Error | { status?: number } | null, -): Generator<SelectEffect | PutEffect, void, string> { +): Generator<SelectEffect | PutEffect, boolean, string> { const errorWithStatus = error && typeof error === 'object' && 'status' in error ? { status: (error as { status?: number }).status } : null if (isFourZeroSixError(errorWithStatus)) { // Session expired - redirect to login yield* redirectToLogout() + return true } + return false }Then in callers:
} catch (e) { yield put(getSamlConfigurationResponse(null)) const error = e instanceof Error ? e : new Error(String(e)) - yield* handleFourZeroOneError(error) - yield* errorToast({ error }) + const redirected = yield* handleFourZeroSixError(error) + if (redirected) return + yield* errorToast({ error }) return error }
🤖 Fix all issues with AI agents
In `@admin-ui/api-client.ts`:
- Around line 36-45: The interceptor in AXIOS_INSTANCE.interceptors.response.use
currently does a direct window.location.href redirect on 406, which duplicates
saga-level handling (redirectToLogout() used in JsonConfigSaga.js and
MappingSaga.js) and can preempt auditLogoutLogs; change the interceptor to
delegate to the existing saga-level flow by invoking the same redirectToLogout()
path (or dispatching the action that triggers auditLogoutLogs) instead of
directly setting window.location.href, ensuring you import or dispatch the same
redirect/audit function used by the sagas so auditLogoutLogs runs before
navigation.
In `@admin-ui/app/redux/sagas/LicenseDetailsSaga.ts`:
- Around line 63-66: Both getLicenseDetailsWorker and updateLicenseDetailsWorker
catch blocks call yield* redirectToLogout() when isFourZeroSixError(e) is true
but do not return afterward; update each handler to immediately return after
yield* redirectToLogout() (i.e., change "yield* redirectToLogout()" to "yield*
redirectToLogout(); return" or add a bare return on the next line) so execution
stops and no further error handling runs.
- Around line 45-48: In the catch block inside LicenseDetailsSaga.ts, after
calling yield* redirectToLogout() when isFourZeroSixError(error) is true, add a
return statement so execution stops (i.e., replace or follow the yield*
redirectToLogout() line with yield* redirectToLogout(); return;). This matches
the pattern used in MessageSaga, AuthSaga, and ApiRoleSaga and prevents any
further code from running if session-expired handling is triggered.
In `@admin-ui/app/redux/sagas/OidcDiscoverySaga.ts`:
- Line 7: Remove the unused import "initAudit" from the import statement in
OidcDiscoverySaga.ts; update the import line that currently reads import {
initAudit, redirectToLogout } from '../sagas/SagaUtils' to only import the used
symbol(s) (e.g., redirectToLogout) so "initAudit" is no longer imported.
In `@admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js`:
- Around line 56-59: The four saga handlers getPermission, addPermission,
editPermission, and deletePermission call yield* redirectToLogout() on a 406
error but do not return immediately; add a return after each yield*
redirectToLogout() so execution stops (matching getPermissions) to prevent
subsequent code (like returning the error) from running before navigation
completes.
In `@admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js`:
- Around line 50-56: The catch block in deleteUMAResourceById in
UMAResourceSaga.js doesn't log the caught error like getUMAResourcesByClient
does; add the same error logging (e.g., console.log(e) or the project's standard
logger) at the start of the catch so the error is recorded, then continue to
yield put(deleteUMAResourceResponse(null)) and keep the existing
isFourZeroSixError(e) check/redirectToLogout call.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
admin-ui/api-client.tsadmin-ui/app/redux/sagas/AuthSaga.tsadmin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/app/redux/sagas/LicenseDetailsSaga.tsadmin-ui/app/redux/sagas/LockSaga.tsadmin-ui/app/redux/sagas/OidcDiscoverySaga.tsadmin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.jsadmin-ui/plugins/admin/redux/sagas/ApiRoleSaga.jsadmin-ui/plugins/admin/redux/sagas/MappingSaga.jsadmin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.jsadmin-ui/plugins/auth-server/redux/sagas/MessageSaga.jsadmin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.jsadmin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.jsadmin-ui/plugins/saml/redux/sagas/SamlSaga.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/app/redux/sagas/OidcDiscoverySaga.tsadmin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/plugins/auth-server/redux/sagas/MessageSaga.jsadmin-ui/app/redux/sagas/LockSaga.tsadmin-ui/app/redux/sagas/AuthSaga.tsadmin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.jsadmin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.jsadmin-ui/app/redux/sagas/LicenseDetailsSaga.tsadmin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.jsadmin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.jsadmin-ui/plugins/admin/redux/sagas/ApiRoleSaga.js
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.
Applied to files:
admin-ui/app/redux/sagas/OidcDiscoverySaga.tsadmin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/plugins/auth-server/redux/sagas/MessageSaga.jsadmin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.jsadmin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.jsadmin-ui/app/redux/sagas/LicenseDetailsSaga.tsadmin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.jsadmin-ui/plugins/admin/redux/sagas/ApiRoleSaga.js
📚 Learning: 2026-01-06T13:45:19.278Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts:4-101
Timestamp: 2026-01-06T13:45:19.278Z
Learning: In admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts, ensure Yup schema marks fields as nullable when the backend DTOs permit nulls and do not mark them as required. Align frontend validation with backend validation to prevent conflicts when editing existing configurations with null values; apply this pattern consistently to other Admin UI validation files where the backend is the source of truth.
Applied to files:
admin-ui/app/redux/sagas/OidcDiscoverySaga.tsadmin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/app/redux/sagas/LockSaga.tsadmin-ui/plugins/saml/redux/sagas/SamlSaga.tsadmin-ui/app/redux/sagas/AuthSaga.tsadmin-ui/api-client.tsadmin-ui/app/redux/sagas/LicenseDetailsSaga.ts
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/auth-server/redux/sagas/MessageSaga.jsadmin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/app/redux/sagas/AuthSaga.ts
🧬 Code graph analysis (13)
admin-ui/plugins/admin/redux/sagas/MappingSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/OidcDiscoverySaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/HealthSaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/auth-server/redux/sagas/MessageSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/LockSaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/saml/redux/sagas/SamlSaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/AuthSaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/LicenseDetailsSaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/admin/redux/sagas/ApiRoleSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroSixError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: label PR
🔇 Additional comments (24)
admin-ui/app/redux/sagas/LockSaga.ts (2)
3-6: LGTM! Imports correctly updated for the session expiry handling migration.The imports align with the codebase patterns shown in
SagaUtils.tsandTokenController.ts.
25-28: Error handling correctly migrated to useisFourZeroSixErrorandredirectToLogout().The changes align with the PR objective of handling session invalidation on 406 responses and are consistent with patterns in other sagas (
WebhookSaga.ts, etc.). The past review feedback has been addressed.admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js (3)
7-10: Imports correctly updated to support new session-expiry handling.The imports are properly consolidated:
isFourZeroSixErrorreplaces the former 401-based check to align with the new 406-based session expiry detection, andredirectToLogoutis imported alongsideinitAuditfrom SagaUtils.
40-44: Error handling pattern correctly implemented.This catch block now properly uses
redirectToLogout()withyield*delegation, which ensures audit logging occurs before redirect. The earlyreturncorrectly prevents further saga execution after triggering navigation. This pattern addresses the previous review feedback.
56-60: Consistent error handling across all saga functions.All seven saga functions now follow the same pattern for session expiry handling:
- Check for 406 error with
isFourZeroSixError(e)- Delegate to
redirectToLogout()generator for audit logging and redirect- Early
returnto prevent further executionThe implementation is consistent and aligns with the broader PR changes for session invalidation on 406 responses.
Also applies to: 76-80, 96-100, 119-123, 143-147, 166-170
admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js (2)
1-13: LGTM - Imports correctly updated for new session expiry handling.The switch from
isFourZeroOneErrortoisFourZeroSixErroraligns with the session revocation changes, and the addition ofredirectToLogoutaddresses the previous review feedback about using centralized helpers.
32-39: LGTM - Error handling correctly updated.The error handling now properly uses
isFourZeroSixErrorand delegates toredirectToLogout()withyield*, which ensures the audit log is recorded before redirecting. This addresses the previous review feedback.admin-ui/app/redux/sagas/HealthSaga.ts (1)
35-37: LGTM! Correct adoption ofredirectToLogout()helper.The error handling now properly uses
isFourZeroSixErrorand the centralizedredirectToLogout()utility, which ensures audit logging occurs before the redirect.admin-ui/app/redux/sagas/OidcDiscoverySaga.ts (1)
24-27: LGTM! Properly usesredirectToLogout()helper.The error handling correctly uses the centralized
redirectToLogout()utility with properreturnto exit the generator.admin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.js (2)
33-39: LGTM! Correct 406 error handling pattern.The error handling properly distinguishes between 406 (session expired) and other errors, using
redirectToLogout()for the former while still returning other errors to the caller.
56-62: LGTM! Consistent error handling inpatchJsonConfig.Same correct pattern applied here - 406 triggers logout redirect, other errors are returned.
admin-ui/plugins/admin/redux/sagas/MappingSaga.js (2)
38-42: LGTM! ConsistentredirectToLogout()usage across all mapping sagas.All four saga functions (
fetchMapping,updateMapping,addMapping,deleteMapping) now use the centralizedredirectToLogout()helper for 406 errors, providing consistent audit logging and redirect behavior.
59-63: LGTM! Uniform error handling pattern.The remaining sagas follow the same correct pattern for session expiry handling.
Also applies to: 79-83, 100-104
admin-ui/plugins/auth-server/redux/sagas/MessageSaga.js (3)
1-17: Imports correctly updated for session-expiry handling.The import changes properly bring in
isFourZeroSixErrorandredirectToLogout, aligning with the codebase-wide pattern for handling 406 session-expiry errors with audit logging.
59-63: Correct session-expiry handling pattern applied.The error handling follows the established pattern: checking for 406 errors via
isFourZeroSixError, using the centralizedredirectToLogout()helper (which includes audit logging), and returning to prevent further execution. This addresses the previous review feedback about using the helper instead of directwindow.location.href.
81-85: Consistent session-expiry handling across all saga functions.All catch blocks (
editMessageConfig,putConfigMessagePostgres,putConfigMessageRedis) correctly implement the same pattern withisFourZeroSixError,yield* redirectToLogout(), andreturnfor early exit.Also applies to: 102-106, 123-127
admin-ui/app/redux/sagas/AuthSaga.ts (2)
26-27: Imports properly updated for 406-based session handling.The import of
isFourZeroSixErrorfromTokenControllerandredirectToLogoutfromSagaUtilsaligns with the PR-wide pattern.
72-75: Session-expiry handling is now consistent across all catch blocks.All error handlers correctly implement:
- Check with
isFourZeroSixError(error)- Redirect via
yield* redirectToLogout()- Early
returnto prevent further executionThe previously missing
returnstatement ingetAPIAccessTokenWorker(line 141) has been added, resolving the control flow inconsistency flagged in past reviews.Also applies to: 96-99, 139-142, 167-170
admin-ui/plugins/saml/redux/sagas/SamlSaga.ts (1)
5-11: Imports correctly updated for session-expiry handling.The imports properly bring in
isFourZeroSixErrorandredirectToLogout, aligning with the codebase-wide pattern.admin-ui/app/redux/sagas/LicenseDetailsSaga.ts (1)
11-13: Imports correctly updated for session-expiry handling.The imports properly bring in
redirectToLogoutfromSagaUtilsandisFourZeroSixErrorfromTokenController, addressing the previous review feedback about using the centralized utility.admin-ui/plugins/admin/redux/sagas/ApiRoleSaga.js (2)
12-17: Imports correctly updated for session-expiry handling.The imports properly bring in
isFourZeroSixErrorfromTokenControllerandredirectToLogoutfromSagaUtils, addressing the previous review feedback.
37-41: Consistent session-expiry handling across all saga functions.All five CRUD operations (
getRoles,getRole,addRole,editRole,deleteRole) correctly implement the pattern:
- Check with
isFourZeroSixError(e)- Redirect via
yield* redirectToLogout()- Early
returnto prevent further executionThis addresses the previous review feedback about using the centralized
redirectToLogout()helper instead of directwindow.location.hrefassignment.Also applies to: 55-59, 75-79, 96-100, 118-122
admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (2)
13-17: LGTM! Imports correctly updated for the new session-expiry handling.The migration from
isFourZeroOneErrortoisFourZeroSixErrorand the addition ofredirectToLogoutfromSagaUtilsaligns with the broader codebase refactoring for session invalidation.
38-42: Good pattern with early return after redirect.This correctly exits the generator after initiating the logout redirect, preventing any subsequent code from executing.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
admin-ui/plugins/auth-server/redux/sagas/SessionSaga.js (1)
30-30: Remove debug console.log statement.This
console.log('SessionSaga: API response data:', data)appears to be debug logging that should not be in production code, especially since it may log sensitive session data.🧹 Proposed fix
- console.log('SessionSaga: API response data:', data)admin-ui/app/redux/sagas/HealthSaga.ts (1)
51-53: Add 403 error handling togetHealthServerStatusto matchgetHealthStatus.The
getHealthStatusfunction (lines 35-37) explicitly checks for 403 errors and callsredirectToLogout()when a session expires. However,getHealthServerStatus(lines 51-53) lacks this check. Since both functions are health-related endpoints using session-based authentication (both selectauthReducer.issuerand initialize audit), they should handle session expiration consistently.Add the 403 error check to the catch block in
getHealthServerStatus:Suggested fix
} catch (e) { yield put(getHealthServerStatusResponse(null)) + if (isFourZeroThreeError(e)) { + yield* redirectToLogout() + } }Both required utilities (
isFourZeroThreeErrorandredirectToLogout) are already imported at the top of the file.admin-ui/plugins/auth-server/redux/sagas/OIDCSaga.js (1)
129-147: Prevent logout on insufficient scope errors.The
getOpenidClientTokensfunction checks for insufficient scope 403 errors and displays a custom error message (lines 131-138), but then unconditionally redirects to logout viaisFourZeroThreeError(error)(line 143) since both conditions match status 403. Users with insufficient token read permissions will see the custom error toast briefly before being logged out, which is likely unintended for a permission-based error.Apply the proposed fix to skip logout when the 403 is specifically an insufficient scope error:
🔧 Proposed fix
yield put(updateToast(true, 'error', errorMessage)) yield put(getTokenByClientResponse(null)) - if (isFourZeroThreeError(error)) { + if (isFourZeroThreeError(error) && !isInsufficientScope) { yield* redirectToLogout() return }admin-ui/plugins/saml/redux/sagas/SamlSaga.ts (1)
388-399: Function name is inconsistent with its behavior.The function is named
handleFourZeroOneErrorbut now checks for 403 errors usingisFourZeroThreeError. Consider renaming tohandleFourZeroThreeErroror a more generic name likehandleSessionExpiredErrorto accurately reflect its purpose.♻️ Suggested rename
-function* handleFourZeroOneError( +function* handleSessionExpiredError( error: Error | { status?: number } | null, ): Generator<SelectEffect | PutEffect, void, string> { const errorWithStatus = error && typeof error === 'object' && 'status' in error ? { status: (error as { status?: number }).status } : null if (isFourZeroThreeError(errorWithStatus)) { // Session expired - redirect to login yield* redirectToLogout() } }Then update all call sites from
handleFourZeroOneErrortohandleSessionExpiredError.admin-ui/plugins/admin/redux/sagas/ApiRoleSaga.js (1)
45-60: ImportgetRoleResponsefromapiRoleSlice- it is used but not imported.The
getRolesaga usesgetRoleResponseon lines 51 and 54, but this action is not included in the import statement fromapiRoleSlice. This will cause a ReferenceError at runtime.🐛 Proposed fix
import { getRoles as getRolesAction, getRolesResponse, + getRoleResponse, addRoleResponse, editRoleResponse, deleteRoleResponse, } from 'Plugins/admin/redux/features/apiRoleSlice'
🤖 Fix all issues with AI agents
In `@admin-ui/api-client.ts`:
- Around line 36-45: The interceptor on AXIOS_INSTANCE currently does a
synchronous window.location.href redirect on 403 which bypasses saga-level audit
work; update the interceptor to dispatch the auditLogoutLogs action before
redirecting instead of directly changing location (import the app store and the
auditLogoutLogs action and call store.dispatch(auditLogoutLogs(...)) when
error.response?.status === 403), then perform the redirect (or call
redirectToLogout() if that helper safely performs the audit and redirect without
circular imports); ensure you remove duplicate saga-level redirects
(redirectToLogout calls) or guard them so audit runs only once (reference
AXIOS_INSTANCE.interceptors.response.use, isFourZeroThreeError,
redirectToLogout, and auditLogoutLogs).
In `@admin-ui/app/redux/sagas/HealthSaga.ts`:
- Around line 33-39: In the catch block handling errors in the saga, after
calling redirectToLogout() (used when isFourZeroThreeError(e) is true), add a
return so execution stops and matches other sagas' behavior; update the catch to
call yield* redirectToLogout() followed by an immediate return to prevent
further execution after dispatching getHealthStatusResponse(null).
In `@admin-ui/app/redux/sagas/LockSaga.ts`:
- Around line 23-29: In LockSaga's catch block where you handle errors (yield
put(getLockStatusResponse(null))), ensure you follow the same control flow
pattern as other sagas by returning immediately after yielding
redirectToLogout(): i.e., after the if (isFourZeroThreeError(e)) { yield*
redirectToLogout(); } add a return so no further saga code can accidentally run
if code is added later; this targets the isFourZeroThreeError check and
redirectToLogout invocation in the catch branch of LockSaga.
In `@admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js`:
- Around line 142-147: In the isFourZeroThreeError branch inside
ApiPermissionSaga, add an immediate return after the yield* redirectToLogout()
call (i.e., call yield* redirectToLogout(); then return) so execution does not
continue to the later return e; this ensures redirectToLogout() stops further
handling for 403 cases.
- Around line 103-108: In the catch block handling isFourZeroThreeError(e)
inside ApiPermissionSaga.js, add an immediate return after calling yield*
redirectToLogout() (e.g., yield* redirectToLogout(); return) so execution stops
and does not continue past the redirect; reference the isFourZeroThreeError
check and the redirectToLogout generator to locate where to add the return.
In `@admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js`:
- Around line 35-39: Both getUMAResourcesByClient and deleteUMAResourceById call
yield* redirectToLogout() when isFourZeroThreeError(e) is true but fail to stop
execution; add an early return immediately after the yield* redirectToLogout()
call in each saga so the generator exits and no further code runs after the
logout redirect.
♻️ Duplicate comments (5)
admin-ui/app/redux/sagas/ProfileDetailsSaga.ts (1)
52-55: Unnecessary type cast reduces error detection coverage.The cast
e as { status?: number }only covers theDirectStatusErrorshape, butisFourZeroThreeErroralso checks forAxiosErrorLike(withresponse.status). For consistency with other saga files that pass the error without casting (e.g.,OidcDiscoverySaga.ts), consider removing the cast:Suggested fix
} catch (e) { yield put(setUserProfileDetails(null)) - if (isFourZeroThreeError(e as { status?: number })) { + if (isFourZeroThreeError(e)) { yield* redirectToLogout() return }admin-ui/app/redux/sagas/OidcDiscoverySaga.ts (1)
7-7: Remove unusedinitAuditimport.
initAuditis imported but never used in this file.Proposed fix
-import { initAudit, redirectToLogout } from '../sagas/SagaUtils' +import { redirectToLogout } from '../sagas/SagaUtils'admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (2)
56-60: MissingreturnafterredirectToLogout()— code continues execution.Unlike
getPermissions(line 41), this handler lacks areturnstatement afterredirectToLogout(). Sincewindow.location.hrefassignment doesn't immediately halt JavaScript execution, the code will continue and may execute unintended operations before navigation completes.Proposed fix
if (isFourZeroThreeError(e)) { // Session expired - redirect to login yield* redirectToLogout() + return }
78-83: MissingreturnafterredirectToLogout().Same issue — add
returnafterredirectToLogout()to preventreturn efrom executing.Proposed fix
if (isFourZeroThreeError(e)) { // Session expired - redirect to login yield* redirectToLogout() + return } - return eadmin-ui/app/redux/sagas/LicenseDetailsSaga.ts (1)
45-49: Missingreturnstatements afterredirectToLogout()in all three catch blocks.Unlike other saga files in this PR (
MappingSaga.js,MessageSaga.js,JsonConfigSaga.js,OIDCSaga.js) which includereturnstatements afterredirectToLogout(), all three error handlers in this file are missing them. This inconsistency could cause unintended behavior if code is added after the if-block, and deviates from the established pattern.🔧 Proposed fix for all three catch blocks
if (isFourZeroThreeError(error)) { // Session expired - redirect to login yield* redirectToLogout() + return }Apply to
resetLicenseConfigWorker(line 47),getLicenseDetailsWorker(line 65), andupdateLicenseDetailsWorker(line 82).Also applies to: 63-67, 80-84
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (25)
admin-ui/api-client.tsadmin-ui/app/redux/sagas/AttributesSaga.tsadmin-ui/app/redux/sagas/AuthSaga.tsadmin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/app/redux/sagas/InitSaga.tsadmin-ui/app/redux/sagas/LicenseDetailsSaga.tsadmin-ui/app/redux/sagas/LockSaga.tsadmin-ui/app/redux/sagas/MauSaga.tsadmin-ui/app/redux/sagas/OidcDiscoverySaga.tsadmin-ui/app/redux/sagas/ProfileDetailsSaga.tsadmin-ui/app/redux/sagas/SagaUtils.tsadmin-ui/app/utils/TokenController.tsadmin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.jsadmin-ui/plugins/admin/redux/sagas/ApiRoleSaga.jsadmin-ui/plugins/admin/redux/sagas/AssetSaga.tsadmin-ui/plugins/admin/redux/sagas/CustomScriptSaga.tsadmin-ui/plugins/admin/redux/sagas/MappingSaga.jsadmin-ui/plugins/admin/redux/sagas/WebhookSaga.tsadmin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.jsadmin-ui/plugins/auth-server/redux/sagas/MessageSaga.jsadmin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.jsadmin-ui/plugins/auth-server/redux/sagas/OIDCSaga.jsadmin-ui/plugins/auth-server/redux/sagas/SessionSaga.jsadmin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.jsadmin-ui/plugins/saml/redux/sagas/SamlSaga.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2026-01-06T13:45:19.278Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts:4-101
Timestamp: 2026-01-06T13:45:19.278Z
Learning: In admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts, ensure Yup schema marks fields as nullable when the backend DTOs permit nulls and do not mark them as required. Align frontend validation with backend validation to prevent conflicts when editing existing configurations with null values; apply this pattern consistently to other Admin UI validation files where the backend is the source of truth.
Applied to files:
admin-ui/app/redux/sagas/SagaUtils.tsadmin-ui/plugins/admin/redux/sagas/CustomScriptSaga.tsadmin-ui/api-client.tsadmin-ui/app/utils/TokenController.tsadmin-ui/app/redux/sagas/LockSaga.tsadmin-ui/app/redux/sagas/InitSaga.tsadmin-ui/app/redux/sagas/OidcDiscoverySaga.tsadmin-ui/app/redux/sagas/MauSaga.tsadmin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/app/redux/sagas/ProfileDetailsSaga.tsadmin-ui/plugins/saml/redux/sagas/SamlSaga.tsadmin-ui/app/redux/sagas/AuthSaga.tsadmin-ui/app/redux/sagas/AttributesSaga.tsadmin-ui/plugins/admin/redux/sagas/AssetSaga.tsadmin-ui/plugins/admin/redux/sagas/WebhookSaga.tsadmin-ui/app/redux/sagas/LicenseDetailsSaga.ts
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.jsadmin-ui/app/redux/sagas/LockSaga.tsadmin-ui/plugins/admin/redux/sagas/ApiRoleSaga.jsadmin-ui/app/redux/sagas/OidcDiscoverySaga.tsadmin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.jsadmin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.jsadmin-ui/plugins/auth-server/redux/sagas/MessageSaga.jsadmin-ui/app/redux/sagas/AuthSaga.tsadmin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.jsadmin-ui/app/redux/sagas/LicenseDetailsSaga.ts
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.
Applied to files:
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.jsadmin-ui/plugins/admin/redux/sagas/ApiRoleSaga.jsadmin-ui/app/redux/sagas/OidcDiscoverySaga.tsadmin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.jsadmin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.jsadmin-ui/plugins/auth-server/redux/sagas/MessageSaga.jsadmin-ui/app/redux/sagas/AuthSaga.tsadmin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.jsadmin-ui/app/redux/sagas/LicenseDetailsSaga.ts
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.jsadmin-ui/plugins/auth-server/redux/sagas/MessageSaga.js
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.jsadmin-ui/app/redux/sagas/AuthSaga.tsadmin-ui/app/redux/sagas/LicenseDetailsSaga.ts
🧬 Code graph analysis (23)
admin-ui/app/redux/sagas/SagaUtils.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)
admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/LockSaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/auth-server/redux/sagas/OIDCSaga.js (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)
admin-ui/app/redux/sagas/InitSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)
admin-ui/plugins/admin/redux/sagas/ApiRoleSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/OidcDiscoverySaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/auth-server/redux/sagas/SessionSaga.js (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)
admin-ui/plugins/admin/redux/sagas/MappingSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/MauSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)
admin-ui/app/redux/sagas/HealthSaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/ProfileDetailsSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)
admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/auth-server/redux/sagas/MessageSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/saml/redux/sagas/SamlSaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/AuthSaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/AttributesSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)
admin-ui/plugins/admin/redux/sagas/AssetSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)
admin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/plugins/admin/redux/sagas/WebhookSaga.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)
admin-ui/app/redux/sagas/LicenseDetailsSaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
🔇 Additional comments (38)
admin-ui/app/utils/TokenController.ts (1)
35-40: LGTM - Consistent rename and appropriate status code.The function rename from
isFourZeroOneErrortoisFourZeroThreeErrorwith the corresponding status code change to 403 is appropriate. HTTP 403 Forbidden correctly signals that the server understood the request but refuses to authorize it (e.g., session invalidated by admin action), distinguishing it from 401 Unauthorized (missing/invalid credentials).admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts (2)
16-16: LGTM - Import updated correctly.The import change from
isFourZeroOneErrortoisFourZeroThreeErroraligns with the renamed function inTokenController.ts.
66-70: Consistent error handling pattern across all saga functions.All six saga catch blocks (
getCustomScripts,getScriptsByType,addScript,editScript,deleteScript,getScriptTypes) correctly useisFourZeroThreeErrorandredirectToLogout(). The pattern ensures audit logging viaauditLogoutLogsbefore redirect.However, this saga-level handling becomes redundant if the global
AXIOS_INSTANCEinterceptor inapi-client.tsremains, as it redirects before these catch blocks execute. See the related comment onapi-client.tsfor resolution options.admin-ui/app/redux/sagas/InitSaga.ts (2)
3-3: LGTM - Import updated correctly.
50-53: Consistent 403 error handling across all saga functions.All four sagas (
getScripts,getClients,getScopes,getAttributes) correctly checkisFourZeroThreeError(e)and callredirectToLogout()on session expiry. The implementation is consistent with the pattern used across other saga files in this PR.Also applies to: 68-71, 85-88, 102-105
admin-ui/app/redux/sagas/SagaUtils.ts (3)
3-3: LGTM - Import updated correctly.
23-26: LGTM - Logout path and audit logging order are correct.The
redirectToLogoutfunction correctly:
- Dispatches
auditLogoutLogsaction first (line 24) to record the session expiry- Then redirects to
/admin/logout(line 25)This ensures audit logging completes before navigation, unlike the direct redirect in the
AXIOS_INSTANCEinterceptor.
42-45: LGTM - handleResponseError updated consistently.The utility function now uses
isFourZeroThreeErrorand delegates toredirectToLogout(), ensuring the audit logging path is followed.admin-ui/app/redux/sagas/AttributesSaga.ts (1)
32-40: LGTM!The error handling correctly uses
isFourZeroThreeErrorto detect session invalidation and properly callsredirectToLogout()with an earlyreturnto prevent further execution. The pattern is consistent with other sagas in the codebase.admin-ui/plugins/auth-server/redux/sagas/SessionSaga.js (2)
1-9: LGTM on import changes.The imports are correctly updated to use
isFourZeroThreeErrorandredirectToLogout, aligning with the new session invalidation strategy.
34-42: Error handling is consistent across all saga functions.All four saga functions (
getSessions,searchSessions,revokeSessionByUserDn,deleteSessionById) correctly implement the 403 error check withredirectToLogout()and early return pattern.admin-ui/app/redux/sagas/AuthSaga.ts (5)
26-27: LGTM on import changes.The imports correctly bring in
isFourZeroThreeErrorandredirectToLogoutfor the updated session invalidation handling.
70-78: LGTM!Error handling in
getOAuth2ConfigWorkercorrectly checks for 403 errors and redirects with an early return.
94-104: LGTM!Error handling in
putConfigWorkercorrectly implements the 403 check pattern withredirectToLogout()and early return, while preserving the existing toast and error propagation logic.
130-143: LGTM!The
getAPIAccessTokenWorkercatch block now includes thereturnstatement afterredirectToLogout(), addressing the previous review feedback and maintaining consistency with other error handlers.
163-172: LGTM!Error handling in
createAdminUiSessionWorkerfollows the established pattern correctly, with 403 check, redirect, and early return before the fallback error response.admin-ui/plugins/admin/redux/sagas/WebhookSaga.ts (1)
31-31: LGTM! Consistent migration to 403-based session expiry handling.The import and error handling updates are correctly applied across all 8 saga handlers (
getWebhooks,createWebhook,deleteWebhook,updateWebhook,getFeatures,getFeaturesByWebhookId,getWebhooksByFeatureId,triggerWebhook). Each catch block properly:
- Checks for 403 errors using
isFourZeroThreeError- Uses
yield* redirectToLogout()for consistent audit logging- Returns early to prevent further execution
This aligns with the PR objective to invalidate sessions on user credential/role changes.
Also applies to: 79-84
admin-ui/app/redux/sagas/OidcDiscoverySaga.ts (1)
24-28: LGTM! Error handling correctly updated.The migration to
isFourZeroThreeErrorwithredirectToLogout()and early return is properly implemented, ensuring consistent session expiry handling with audit logging.admin-ui/app/redux/sagas/MauSaga.ts (1)
3-3: LGTM! Consistent error handling migration.The import and error check are correctly updated to use
isFourZeroThreeErrorwith properredirectToLogout()and early return.Also applies to: 29-33
admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (1)
38-41: Correct pattern — this handler properly returns after redirect.This
getPermissionshandler demonstrates the correct pattern withreturnafterredirectToLogout(). The four other handlers should follow this same pattern.admin-ui/plugins/admin/redux/sagas/MappingSaga.js (3)
11-16: LGTM! Imports are correctly updated.The imports for
isFourZeroThreeErrorandredirectToLogoutare properly configured, aligning with the centralized session expiration handling pattern.
38-44: LGTM! Consistent error handling pattern.The
fetchMappingsaga correctly implements the session expiration flow: checks for 403 error, yields to the centralized logout helper with audit logging, and returns early to prevent further execution.
59-65: LGTM! All saga error handlers follow the same pattern.The
updateMapping,addMapping, anddeleteMappingsagas consistently implement the session expiration handling with proper early returns afterredirectToLogout().Also applies to: 79-85, 100-106
admin-ui/plugins/auth-server/redux/sagas/MessageSaga.js (2)
2-15: LGTM! Imports correctly configured for session handling.The imports for
isFourZeroThreeErrorandredirectToLogoutare properly set up.
59-63: LGTM! All four saga error handlers implement consistent session expiration flow.Each catch block in
getConfigMessage,editMessageConfig,putConfigMessagePostgres, andputConfigMessageRedisproperly checks for 403 errors and uses the centralized logout mechanism with early returns.Also applies to: 81-85, 102-106, 123-127
admin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.js (2)
8-10: LGTM! Imports properly updated.The imports for
isFourZeroThreeErrorandredirectToLogoutare correctly configured.
33-39: LGTM! Error handling correctly implemented.Both
getJsonConfigandpatchJsonConfigsagas properly handle session expiration with the centralized logout flow. Thereturn eafter the if-block correctly returns the error for non-403 cases.Also applies to: 56-62
admin-ui/plugins/auth-server/redux/sagas/OIDCSaga.js (2)
2-2: LGTM! Imports correctly configured.The imports for
isFourZeroThreeErrorandredirectToLogoutare properly set up.Also applies to: 20-20
49-53: LGTM! Consistent error handling across most sagas.The sagas
getOauthOpenidClients,addNewClient,editAClient,deleteAClient, anddeleteClientTokenall correctly implement the session expiration flow with early returns.Also applies to: 70-75, 94-99, 117-121, 159-163
admin-ui/app/redux/sagas/LicenseDetailsSaga.ts (1)
11-13: LGTM! Imports correctly configured.The imports for
redirectToLogoutandisFourZeroThreeErrorare properly set up.admin-ui/plugins/saml/redux/sagas/SamlSaga.ts (1)
5-11: Import updates look correct.The imports correctly bring in
redirectToLogoutfromSagaUtilsandisFourZeroThreeErrorfromTokenController, aligning with the PR's goal to standardize session expiry handling using HTTP 403.admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js (2)
7-10: Imports correctly updated.The imports for
isFourZeroThreeErrorandredirectToLogoutare properly added, enabling centralized session expiry handling with audit logging.
40-44: Consistent error handling pattern across all saga functions.All catch blocks correctly:
- Check for 403 errors using
isFourZeroThreeError(e)- Use
yield* redirectToLogout()for centralized logout with audit logging- Include
returnstatements to prevent further execution after redirectThis addresses the previous review feedback and aligns with the PR objective.
Also applies to: 56-60, 76-80, 96-100, 119-123, 143-147, 166-170
admin-ui/plugins/admin/redux/sagas/ApiRoleSaga.js (2)
12-17: Imports correctly updated for session expiry handling.The
isFourZeroThreeErrorandredirectToLogoutimports are properly added.
37-41: Error handling pattern is consistent across all saga functions.All catch blocks correctly use
isFourZeroThreeError(e),yield* redirectToLogout(), and includereturnstatements. This aligns with the centralized logout approach used across other sagas.Also applies to: 75-79, 96-100, 118-122
admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js (1)
2-13: Imports correctly updated.The imports for
isFourZeroThreeErrorandredirectToLogoutare properly added.admin-ui/plugins/admin/redux/sagas/AssetSaga.ts (2)
16-20: Imports correctly updated.The imports for
isFourZeroThreeErrorandredirectToLogoutare properly structured.
60-63: Consistent and complete error handling across all asset saga functions.All catch blocks correctly:
- Display error toast before checking session expiry
- Dispatch response action to update state
- Check for 403 errors using
isFourZeroThreeError(e)- Use
yield* redirectToLogout()for centralized logout- Include
returnstatements to halt execution after redirectThis is well-structured and consistent with the PR's session revocation objectives.
Also applies to: 84-87, 108-111, 134-137, 157-160, 184-187
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/app/redux/sagas/HealthSaga.ts (1)
43-55: Inconsistent error handling—getHealthServerStatusis missing 403 handling.
getHealthStatushandles 403 errors by redirecting to logout, butgetHealthServerStatusdoes not. Both sagas make authenticated API calls that could return 403 when the session is revoked. For consistency and complete session invalidation coverage, add the same handling here.♻️ Proposed fix
export function* getHealthServerStatus({ payload }) { const audit = yield* initAudit() try { payload = payload || { action: {} } addAdditionalData(audit, 'FETCH', 'Health', payload) const healthApi = yield* newStatusFunction() const data = yield call(healthApi.getHealthServerStatus, payload.action.action_data) yield put(getHealthServerStatusResponse({ data })) yield call(postUserAction, audit) } catch (e) { yield put(getHealthServerStatusResponse(null)) + if (isFourZeroThreeError(e)) { + // Session expired - redirect to login + yield* redirectToLogout() + return + } } }
🤖 Fix all issues with AI agents
In `@admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js`:
- Around line 54-60: In the getPermission saga, after catching errors and
handling the 403 branch, add an explicit return immediately after the yield*
redirectToLogout() call so execution exits consistently (similar to
getPermissions and editPermission); locate the catch block that calls yield
put(getPermissionResponse(null)) and isFourZeroThreeError(e) and insert a return
following yield* redirectToLogout() to ensure no further code runs.
♻️ Duplicate comments (3)
admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js (1)
35-38: AddreturnafterredirectToLogout()to stop the saga flow.
Without an early return at Line 37 and Line 55, the generator may continue executing after the redirect.Suggested fix
if (isFourZeroThreeError(e)) { // Session expired - redirect to login yield* redirectToLogout() + return }if (isFourZeroThreeError(e)) { // Session expired - redirect to login yield* redirectToLogout() + return }Also applies to: 53-55
admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (2)
78-83: MissingreturnafterredirectToLogout()- code continues execution.After
redirectToLogout()is called, execution continues toreturn eon line 82. This is inconsistent withgetPermissions(line 41) andeditPermission(line 106) which properly return after the redirect.Proposed fix
if (isFourZeroThreeError(e)) { // Session expired - redirect to login yield* redirectToLogout() + return } - return e } }
143-148: MissingreturnafterredirectToLogout()- code continues execution.Same issue as
addPermission— afterredirectToLogout()is called, execution continues toreturn eon line 147. This is inconsistent with the pattern established ingetPermissionsandeditPermission.Proposed fix
if (isFourZeroThreeError(e)) { // Session expired - redirect to login yield* redirectToLogout() + return } - return e } }
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
admin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/app/redux/sagas/LockSaga.tsadmin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.jsadmin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.jsadmin-ui/app/redux/sagas/LockSaga.tsadmin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.
Applied to files:
admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.jsadmin-ui/app/redux/sagas/HealthSaga.tsadmin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js
📚 Learning: 2026-01-06T13:45:19.278Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts:4-101
Timestamp: 2026-01-06T13:45:19.278Z
Learning: In admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts, ensure Yup schema marks fields as nullable when the backend DTOs permit nulls and do not mark them as required. Align frontend validation with backend validation to prevent conflicts when editing existing configurations with null values; apply this pattern consistently to other Admin UI validation files where the backend is the source of truth.
Applied to files:
admin-ui/app/redux/sagas/LockSaga.tsadmin-ui/app/redux/sagas/HealthSaga.ts
🧬 Code graph analysis (3)
admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/LockSaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
admin-ui/app/redux/sagas/HealthSaga.ts (2)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)admin-ui/app/redux/sagas/SagaUtils.ts (1)
redirectToLogout(23-26)
🔇 Additional comments (9)
admin-ui/plugins/auth-server/redux/sagas/UMAResourceSaga.js (2)
2-2: Looks good: aligned with 403 handling and centralized logout.Also applies to: 13-13
51-51: Consistent error logging in delete flow.admin-ui/app/redux/sagas/LockSaga.ts (2)
3-6: Consistent 403 handling imports.The new predicate and logout helper align this saga with the centralized session-expiry flow and
/admin/logoutpath.
25-28: Clean 403 redirect flow.Redirecting via
redirectToLogout()and returning immediately keeps control flow consistent with other sagas.admin-ui/app/redux/sagas/HealthSaga.ts (2)
3-3: LGTM! Imports correctly updated for 403-based session handling.The imports now correctly pull
isFourZeroThreeErrorfor the new 403-based session validation andredirectToLogoutfor centralized logout handling.Also applies to: 9-9
33-40: LGTM! Error handling follows the centralized logout pattern.The 403 error handling now correctly uses the
redirectToLogout()utility and includes thereturnstatement to prevent further execution. This aligns with the broader session-revocation strategy in the PR.admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (3)
13-17: LGTM!The import changes correctly bring in
isFourZeroThreeErrorfor the new 403-based session expiry detection andredirectToLogoutfor centralized logout handling.
36-44: LGTM!The error handling in
getPermissionscorrectly usesisFourZeroThreeError, callsredirectToLogout(), and includes areturnstatement to prevent further execution.
99-109: LGTM!The error handling in
editPermissionnow correctly includes areturnstatement afterredirectToLogout(), preventing continued execution.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| } catch (e) { | ||
| yield put(getPermissionResponse(null)) | ||
| if (isFourZeroOneError(e)) { | ||
| if (isFourZeroThreeError(e)) { | ||
| // Session expired - redirect to login | ||
| window.location.href = '/logout' | ||
| yield* redirectToLogout() | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Acceptable, but consider adding explicit return for consistency.
The getPermission function doesn't have code after the if block, so execution naturally stops. However, adding an explicit return after redirectToLogout() would improve consistency with getPermissions and editPermission.
🤖 Prompt for AI Agents
In `@admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js` around lines 54 -
60, In the getPermission saga, after catching errors and handling the 403
branch, add an explicit return immediately after the yield* redirectToLogout()
call so execution exits consistently (similar to getPermissions and
editPermission); locate the catch block that calls yield
put(getPermissionResponse(null)) and isFourZeroThreeError(e) and insert a return
following yield* redirectToLogout() to ensure no further code runs.
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@admin-ui/app/redux/sagas/SagaUtils.ts`:
- Around line 25-27: Update the cookie-clear assignment in SagaUtils.ts where
document.cookie is set for 'admin_ui_session_id' to include the Secure and
SameSite attributes (e.g., add "; Secure; SameSite=Strict") along with the
existing "max-age=0; path=/". Locate the document.cookie assignment (the lines
that currently read document.cookie = 'admin_ui_session_id=; max-age=0; path=/'
and window.location.href = '/admin/logout') and modify the cookie string to
append the Secure and SameSite attributes so the cookie is cleared with the same
security flags it was set with.
♻️ Duplicate comments (1)
admin-ui/api-client.ts (1)
36-47: Dual 403 handling bypasses audit logging.This interceptor redirects immediately on 403, but sagas throughout the codebase also check
isFourZeroThreeError(e)and callredirectToLogout(), which dispatchesauditLogoutLogsbefore redirecting. This interceptor bypasses that audit action by redirecting synchronously.Additionally, after setting
window.location.href,Promise.reject(error)still propagates to saga catch blocks, which may attempt redundant handling on a page that's already navigating away.Consider either:
- Remove this interceptor and rely solely on saga-level handling (ensures audit logging), or
- Dispatch the audit action here before redirecting, then remove saga-level 403 handling
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/api-client.tsadmin-ui/app/redux/sagas/SagaUtils.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-06T13:45:19.278Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts:4-101
Timestamp: 2026-01-06T13:45:19.278Z
Learning: In admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts, ensure Yup schema marks fields as nullable when the backend DTOs permit nulls and do not mark them as required. Align frontend validation with backend validation to prevent conflicts when editing existing configurations with null values; apply this pattern consistently to other Admin UI validation files where the backend is the source of truth.
Applied to files:
admin-ui/app/redux/sagas/SagaUtils.tsadmin-ui/api-client.ts
🧬 Code graph analysis (1)
admin-ui/app/redux/sagas/SagaUtils.ts (1)
admin-ui/app/utils/TokenController.ts (1)
isFourZeroThreeError(35-40)
🪛 GitHub Check: CodeQL
admin-ui/app/redux/sagas/SagaUtils.ts
[warning] 26-26: Clear text transmission of sensitive cookie
Sensitive cookie sent without enforcing SSL encryption.
admin-ui/api-client.ts
[warning] 41-41: Clear text transmission of sensitive cookie
Sensitive cookie sent without enforcing SSL encryption.
🔇 Additional comments (3)
admin-ui/app/redux/sagas/SagaUtils.ts (2)
3-3: Transition from 401 to 403 error handling looks correct.The import change aligns with the broader refactoring to use 403 (Forbidden) instead of 401 (Unauthorized) for triggering session invalidation when user credentials/roles/status change.
44-47: No action required—403 handling is intentionally designed for session invalidation.The change to
isFourZeroThreeErroris consistent with existing codebase patterns. The framework already has dedicated 403 handling inapi-client.tsthat removes session cookies, and all saga usages treat 403 as "Session expired." The backend architecture appears intentional in using 403 for session-related errors rather than routine authorization failures.admin-ui/api-client.ts (1)
39-46: Cookie clearing approach is correct; suggested Secure/SameSite attributes are unnecessary and add no benefit.The current cookie clearing statement
document.cookie = 'admin_ui_session_id=; max-age=0; path=/'is correct per HTTP cookie standards. Browser cookie deletion requires only matching the cookie name, path, and domain—not the Secure, HttpOnly, or SameSite attributes. Adding these attributes to the clear statement provides no additional security or functionality.If the backend sets the
admin_ui_session_idcookie with additional attributes (e.g.,Domain,Secure), ensure the clear statement matches the same path and domain. If the cookie is flagged asHttpOnly, note that JavaScript cannot clear it; only a server-side Set-Cookie response can remove HttpOnly cookies.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@admin-ui/api-client.ts`:
- Around line 41-42: Remove the dead commented-out cookie removal line in
admin-ui/api-client.ts: delete the commented line
"//Cookies.remove('admin_ui_session_id', { path: '/' })" (and the nearby
"//remove session cookie" comment if you prefer) so only active code remains;
this targets the unused Cookies.remove reference and cleans up noise in the
api-client.ts file.
- Around line 37-50: The 403 response interceptor
(AXIOS_INSTANCE.interceptors.response.use) lacks try/catch and bypasses saga
audit logging; wrap the async block that calls fetchApiTokenWithDefaultScopes()
and deleteAdminUiSession() in a try/catch and ensure a finally that always
triggers the logout redirect (window.location.href = '/admin/logout') so
failures don’t prevent redirect and the original error is still rethrown;
additionally, instead of manually deleting session and redirecting here, call or
dispatch the existing redirectToLogout()/auditLogoutLogs flow (or invoke the
same auditLogoutLogs action) so audit trails remain consistent with saga-level
handling and errors are logged in the catch before rethrowing the original
error.
In `@admin-ui/app/redux/sagas/SagaUtils.ts`:
- Around line 24-30: The redirectToLogout saga currently lets errors from
fetchApiTokenWithDefaultScopes or deleteAdminUiSession abort the generator and
prevent navigation; wrap the calls in a try/catch/finally inside
redirectToLogout so any thrown error is caught (yield put an error/audit action
with the error details, e.g. auditLogoutLogs or a new auditLogoutError) and then
always perform the redirect in the finally block (window.location.href =
'/admin/logout'), ensuring response?.access_token is still used safely when
present.
| AXIOS_INSTANCE.interceptors.response.use( | ||
| (response) => response, | ||
| async (error) => { | ||
| if (error.response?.status === 403) { | ||
| //remove session cookie | ||
| //Cookies.remove('admin_ui_session_id', { path: '/' }) | ||
| const response = await fetchApiTokenWithDefaultScopes() | ||
| await deleteAdminUiSession(response?.access_token) | ||
| window.location.href = '/admin/logout' | ||
| } | ||
|
|
||
| return Promise.reject(error) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Missing error handling in 403 interceptor can cause silent failures or unexpected behavior.
The async operations fetchApiTokenWithDefaultScopes() and deleteAdminUiSession() can throw, but there's no try-catch. If either fails:
- The redirect to
/admin/logoutwon't execute - The original error rejection may behave unexpectedly
- User gets stuck in an inconsistent state
Additionally, as noted in previous reviews, this interceptor bypasses the auditLogoutLogs action that redirectToLogout() in SagaUtils dispatches, creating inconsistent audit trails.
Proposed fix with error handling
AXIOS_INSTANCE.interceptors.response.use(
(response) => response,
async (error) => {
if (error.response?.status === 403) {
- //remove session cookie
- //Cookies.remove('admin_ui_session_id', { path: '/' })
- const response = await fetchApiTokenWithDefaultScopes()
- await deleteAdminUiSession(response?.access_token)
- window.location.href = '/admin/logout'
+ try {
+ const response = await fetchApiTokenWithDefaultScopes()
+ await deleteAdminUiSession(response?.access_token)
+ } catch (cleanupError) {
+ console.error('Failed to clean up session on 403:', cleanupError)
+ }
+ window.location.href = '/admin/logout'
}
return Promise.reject(error)
},
)Consider also consolidating 403 handling to avoid the dual-path issue flagged in previous reviews—either handle it solely here (and dispatch audit logs from the interceptor) or remove this interceptor and rely on saga-level handling.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| AXIOS_INSTANCE.interceptors.response.use( | |
| (response) => response, | |
| async (error) => { | |
| if (error.response?.status === 403) { | |
| //remove session cookie | |
| //Cookies.remove('admin_ui_session_id', { path: '/' }) | |
| const response = await fetchApiTokenWithDefaultScopes() | |
| await deleteAdminUiSession(response?.access_token) | |
| window.location.href = '/admin/logout' | |
| } | |
| return Promise.reject(error) | |
| }, | |
| ) | |
| AXIOS_INSTANCE.interceptors.response.use( | |
| (response) => response, | |
| async (error) => { | |
| if (error.response?.status === 403) { | |
| try { | |
| const response = await fetchApiTokenWithDefaultScopes() | |
| await deleteAdminUiSession(response?.access_token) | |
| } catch (cleanupError) { | |
| console.error('Failed to clean up session on 403:', cleanupError) | |
| } | |
| window.location.href = '/admin/logout' | |
| } | |
| return Promise.reject(error) | |
| }, | |
| ) |
🤖 Prompt for AI Agents
In `@admin-ui/api-client.ts` around lines 37 - 50, The 403 response interceptor
(AXIOS_INSTANCE.interceptors.response.use) lacks try/catch and bypasses saga
audit logging; wrap the async block that calls fetchApiTokenWithDefaultScopes()
and deleteAdminUiSession() in a try/catch and ensure a finally that always
triggers the logout redirect (window.location.href = '/admin/logout') so
failures don’t prevent redirect and the original error is still rethrown;
additionally, instead of manually deleting session and redirecting here, call or
dispatch the existing redirectToLogout()/auditLogoutLogs flow (or invoke the
same auditLogoutLogs action) so audit trails remain consistent with saga-level
handling and errors are logged in the catch before rethrowing the original
error.
| export function* redirectToLogout(message = 'Session expired'): Generator<any, void, any> { | ||
| yield put(auditLogoutLogs({ message })) | ||
| window.location.href = '/logout' | ||
|
|
||
| const response = yield call(fetchApiTokenWithDefaultScopes) | ||
| yield call(deleteAdminUiSession, response?.access_token) | ||
| window.location.href = '/admin/logout' | ||
| } |
There was a problem hiding this comment.
Missing error handling in redirectToLogout can leave users stuck.
If fetchApiTokenWithDefaultScopes or deleteAdminUiSession throws an error, the saga will terminate without reaching the redirect, leaving the user in an inconsistent state with an expired/revoked session but no navigation to the login page.
Proposed fix with error handling
export function* redirectToLogout(message = 'Session expired'): Generator<any, void, any> {
yield put(auditLogoutLogs({ message }))
- const response = yield call(fetchApiTokenWithDefaultScopes)
- yield call(deleteAdminUiSession, response?.access_token)
+ try {
+ const response = yield call(fetchApiTokenWithDefaultScopes)
+ yield call(deleteAdminUiSession, response?.access_token)
+ } catch (error) {
+ console.error('Failed to clean up session during logout:', error)
+ }
window.location.href = '/admin/logout'
}This ensures the user is always redirected to the logout page, even if session cleanup fails server-side.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function* redirectToLogout(message = 'Session expired'): Generator<any, void, any> { | |
| yield put(auditLogoutLogs({ message })) | |
| window.location.href = '/logout' | |
| const response = yield call(fetchApiTokenWithDefaultScopes) | |
| yield call(deleteAdminUiSession, response?.access_token) | |
| window.location.href = '/admin/logout' | |
| } | |
| export function* redirectToLogout(message = 'Session expired'): Generator<any, void, any> { | |
| yield put(auditLogoutLogs({ message })) | |
| try { | |
| const response = yield call(fetchApiTokenWithDefaultScopes) | |
| yield call(deleteAdminUiSession, response?.access_token) | |
| } catch (error) { | |
| console.error('Failed to clean up session during logout:', error) | |
| } | |
| window.location.href = '/admin/logout' | |
| } |
🤖 Prompt for AI Agents
In `@admin-ui/app/redux/sagas/SagaUtils.ts` around lines 24 - 30, The
redirectToLogout saga currently lets errors from fetchApiTokenWithDefaultScopes
or deleteAdminUiSession abort the generator and prevent navigation; wrap the
calls in a try/catch/finally inside redirectToLogout so any thrown error is
caught (yield put an error/audit action with the error details, e.g.
auditLogoutLogs or a new auditLogoutError) and then always perform the redirect
in the finally block (window.location.href = '/admin/logout'), ensuring
response?.access_token is still used safely when present.
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@admin-ui/app/redux/sagas/SagaUtils.ts`:
- Line 6: Remove the unused import fetchApiTokenWithDefaultScopes from the
import list at the top of SagaUtils.ts and delete the associated commented-out
usage line that references fetchApiTokenWithDefaultScopes (the commented line
around the saga where fetchApiTokenWithDefaultScopes is mentioned), leaving only
the needed import deleteAdminUiSession; this cleans up the import statement and
removes dead commented code.
♻️ Duplicate comments (3)
admin-ui/app/redux/sagas/SagaUtils.ts (1)
24-30: Missing error handling leaves users stuck if session deletion fails.If
deleteAdminUiSessionthrows, the generator terminates before the redirect, leaving the user with an expired/revoked session but no navigation to the login page.Proposed fix
export function* redirectToLogout(message = 'Session expired'): Generator<any, void, any> { yield put(auditLogoutLogs({ message })) - //const response = yield call(fetchApiTokenWithDefaultScopes) - yield call(deleteAdminUiSession) + try { + yield call(deleteAdminUiSession) + } catch (error) { + console.error('Failed to clean up session during logout:', error) + } window.location.href = '/admin/logout' }admin-ui/api-client.ts (2)
37-45: Ensure logout redirect always happens even if session cleanup fails.
deleteAdminUiSession()can throw; the currentawaitmeans a failure prevents redirect, leaving the user in a stuck state. Wrap cleanup intry/catchand redirect infinally.💡 Suggested fix
if (error.response?.status === 403) { - //remove session cookie - //Cookies.remove('admin_ui_session_id', { path: '/' }) - //const response = await fetchApiTokenWithDefaultScopes() - await deleteAdminUiSession() - window.location.href = '/admin/logout' + try { + await deleteAdminUiSession() + } catch (cleanupError) { + console.error('Failed to delete Admin UI session on 403:', cleanupError) + } finally { + window.location.href = '/admin/logout' + } }
41-43: Remove commented-out dead code.The commented cleanup lines add noise and aren’t used.
🧹 Cleanup
- //remove session cookie - //Cookies.remove('admin_ui_session_id', { path: '/' }) - //const response = await fetchApiTokenWithDefaultScopes()
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@admin-ui/api-client.ts`:
- Line 41: The comment above deleteAdminUiSession in admin-ui/api-client.ts is
misleading — it says "remove session cookie" but the function issues a backend
DELETE request to /app/admin-ui/oauth2/session; update the comment to accurately
describe the action (e.g., "call backend to delete admin UI session" or remove
the comment entirely) and ensure it references the DELETE API call in
deleteAdminUiSession so future readers understand it is a server-side session
deletion, not a local cookie removal.
- Around line 39-47: The error handler passed to the API client currently awaits
deleteAdminUiSession() directly, which can throw and prevent the redirect; wrap
the call to deleteAdminUiSession() in a try/catch (or use a finally) inside the
async (error) => { ... } handler so that window.location.href = '/admin/logout'
always runs regardless of deleteAdminUiSession() success, and in the catch log
the caught error (e.g., console.error) for debugging while still proceeding to
redirect.
♻️ Duplicate comments (1)
admin-ui/app/redux/sagas/SagaUtils.ts (1)
24-28: Ensure logout redirect still happens if session deletion fails.
deleteAdminUiSessionthrows on failure; without a guard, the generator can abort and skip the redirect, leaving users stranded after a revoked session. Wrap the call and always redirect.✅ Proposed fix
export function* redirectToLogout(message = 'Session expired'): Generator<any, void, any> { yield put(auditLogoutLogs({ message })) - yield call(deleteAdminUiSession) - window.location.href = '/admin/logout' + try { + yield call(deleteAdminUiSession) + } catch (error) { + console.error('Failed to clean up Admin UI session during logout:', error) + } finally { + window.location.href = '/admin/logout' + } }
| async (error) => { | ||
| if (error.response?.status === 403) { | ||
| //remove session cookie | ||
| await deleteAdminUiSession() | ||
| window.location.href = '/admin/logout' | ||
| } | ||
|
|
||
| return Promise.reject(error) | ||
| }, |
There was a problem hiding this comment.
Missing error handling will prevent logout redirect on cleanup failure.
If deleteAdminUiSession() throws (likely when the session is already invalidated server-side), the redirect to /admin/logout won't execute, leaving the user stuck in an inconsistent state. The redirect should always happen regardless of cleanup success.
Proposed fix
async (error) => {
if (error.response?.status === 403) {
- //remove session cookie
- await deleteAdminUiSession()
+ try {
+ await deleteAdminUiSession()
+ } catch (cleanupError) {
+ console.error('Failed to clean up session on 403:', cleanupError)
+ }
window.location.href = '/admin/logout'
}
return Promise.reject(error)
},Based on learnings, console.error in the catch block helps print the whole error object for debugging unexpected errors.
🤖 Prompt for AI Agents
In `@admin-ui/api-client.ts` around lines 39 - 47, The error handler passed to the
API client currently awaits deleteAdminUiSession() directly, which can throw and
prevent the redirect; wrap the call to deleteAdminUiSession() in a try/catch (or
use a finally) inside the async (error) => { ... } handler so that
window.location.href = '/admin/logout' always runs regardless of
deleteAdminUiSession() success, and in the catch log the caught error (e.g.,
console.error) for debugging while still proceeding to redirect.
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
|
|
closing this Pr as there is too much commit history |
closes #2543
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.