refactor(admin-ui): migrate permission gating to Cedarling usePermission + single-source action catalog#2873
Merged
Conversation
…ion + single-source action catalog (#2872) Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
|
Warning Review limit reached
More reviews will be available in 24 minutes and 39 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughMigrates Cedarling permission handling to an action/resource catalog and new hook ChangesPermission & storage migration
Sequence Diagram(s)(silently skipped because changes are broad but the main sequential flows are simple and documented in code) Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
mo-auto
added
comp-admin-ui
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 (2)
admin-ui/plugins/fido/__tests__/components/Metrics/MetricsPage.test.tsx (1)
50-52: 🧹 Nitpick | 🔵 TrivialAvoid mock-implementation leakage across tests.
In
admin-ui/plugins/fido/__tests__/components/Metrics/MetricsPage.test.tsx,beforeEachusesjest.clearAllMocks(), which clears call history but doesn’t resetusePermission.mockImplementation(...). The “missing permission” test setscanRead: false; today it’s the last test in the file, but any later tests added could inherit that override. Preferjest.resetAllMocks()inbeforeEach(or re-apply the defaultusePermissionmock inbeforeEach/afterEach).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin-ui/plugins/fido/__tests__/components/Metrics/MetricsPage.test.tsx` around lines 50 - 52, The tests currently call jest.clearAllMocks() in the beforeEach, which only clears call history and allows mock implementations like usePermission.mockImplementation(...) to leak; change the setup to use jest.resetAllMocks() in the beforeEach (or re-apply the default usePermission mock implementation there) so usePermission and other mocks are reset between tests and the "missing permission" case (canRead: false) cannot affect subsequent tests; update the beforeEach that references jest.clearAllMocks() to call jest.resetAllMocks() or re-establish the default usePermission.mockImplementation(...) so each test gets a clean mock state.admin-ui/plugins/scim/plugin-metadata.ts (1)
8-25:⚠️ Potential issue | 🟠 MajorAlign SCIM menu/route permission actions to avoid blocking READ-only users.
SCIM menu uses
action: CEDAR_ACTIONS.READ, but the SCIM route usesaction: CEDAR_ACTIONS.WRITE.ScimPagerenders behindGluuViewWrapper canShow={canReadScim}(so READ is sufficient to view), whilecanWriteScimis only passed toScimConfigurationfor write/edit behavior. If route access is gated by plugin metadata (resourceKey+action), users with only READ may be denied access when navigating to the page.Set the SCIM route action to
CEDAR_ACTIONS.READ(or otherwise make route gating match the page’s canRead/canWrite behavior).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin-ui/plugins/scim/plugin-metadata.ts` around lines 8 - 25, The route permission in pluginMetadata is too strict and prevents READ-only users from accessing ScimPage; update the routes entry for ScimPage (where component: ScimPage and path: ROUTES.SCIM_BASE) to use CEDAR_ACTIONS.READ and the same ADMIN_UI_RESOURCES.SCIM resourceKey as the menus entry so route gating matches the page's GluuViewWrapper canReadScim behavior (keep canWriteScim only for ScimConfiguration write controls).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@admin-ui/app/cedarling/types/cedarTypes.ts`:
- Line 34: Change the TokenAuthorizationRequest.action type from a generic
string to the CedarAction union so action values are validated at compile time:
update the TokenAuthorizationRequest interface (or type) to use CedarAction for
the action property, and ensure CedarAction is imported or referenced in
cedarTypes.ts (or the same module) so the compiler can enforce allowed actions;
adjust any call sites that pass plain strings to use CedarAction values or
cast/convert to CedarAction where appropriate.
In `@admin-ui/plugins/admin/helper/settings.ts`:
- Line 28: The code force-casts configData?.cedarlingLogType to CedarlingLogType
which accepts any string and prevents the fallback; update the initialization of
cedarlingLogType to validate the incoming value against the allowed
CedarlingLogType set (or CEDARLING_LOG_TYPE enum) before using it — e.g., check
if configData?.cedarlingLogType is one of Object.values(CEDARLING_LOG_TYPE) (or
use an isValidCedarlingLogType helper) and only then assign it, otherwise fall
back to CEDARLING_LOG_TYPE.OFF; apply this change where cedarlingLogType is
assigned so unexpected backend strings don’t bypass the fallback.
In `@admin-ui/plugins/admin/plugin-metadata.ts`:
- Around line 131-140: The route entries for RolePermissionMappingPage and
CedarlingConfigPage declare action: CEDAR_ACTIONS.READ but omit resourceKey,
causing a mismatch with their menu items; add resourceKey:
ADMIN_UI_RESOURCES.Security to both route objects (the ones using
ROUTES.ADMIN_MAPPING and ROUTES.ADMIN_CEDARLING_CONFIG) so route-level
authorization aligns with the menu entries and the rest of the security checks.
In `@admin-ui/plugins/auth-server/plugin-metadata.tsx`:
- Around line 189-194: The SsaAddPage route is using CEDAR_ACTIONS.READ which is
incorrect and allows read-only users to access the add page; update the route
entry that references SsaAddPage and ROUTES.AUTH_SERVER_SSA_ADD in
plugin-metadata.tsx to use CEDAR_ACTIONS.WRITE instead of CEDAR_ACTIONS.READ so
it matches other add routes (e.g., ClientAddPage, ScopeAddPage) and enforces
write permissions.
In `@admin-ui/plugins/saml/components/WebsiteSsoServiceProviderList.tsx`:
- Around line 56-59: The component currently destructures canRead and canWrite
from usePermission(ADMIN_UI_RESOURCES.SAML) and uses
canWriteWebsiteSsoServiceProviders to gate delete UI; instead destructure
canDelete from usePermission (e.g., canDelete:
canDeleteWebsiteSsoServiceProviders) and replace usages of
canWriteWebsiteSsoServiceProviders that control the delete action and delete
dialog with canDeleteWebsiteSsoServiceProviders so permission checks match
WebsiteSsoIdentityBrokeringList's canDelete behavior.
In `@admin-ui/plugins/scripts/plugin-metadata.ts`:
- Around line 39-44: The route configuration for the edit page currently grants
read-only permission; update the action for the CustomScriptEditPage route (the
entry with component: CustomScriptEditPage and path:
ROUTES.CUSTOM_SCRIPT_EDIT_TEMPLATE) to use CEDAR_ACTIONS.WRITE instead of
CEDAR_ACTIONS.READ so the edit route requires write permission consistent with
other edit routes (e.g., ClientEditPage, ScopeEditPage).
---
Outside diff comments:
In `@admin-ui/plugins/fido/__tests__/components/Metrics/MetricsPage.test.tsx`:
- Around line 50-52: The tests currently call jest.clearAllMocks() in the
beforeEach, which only clears call history and allows mock implementations like
usePermission.mockImplementation(...) to leak; change the setup to use
jest.resetAllMocks() in the beforeEach (or re-apply the default usePermission
mock implementation there) so usePermission and other mocks are reset between
tests and the "missing permission" case (canRead: false) cannot affect
subsequent tests; update the beforeEach that references jest.clearAllMocks() to
call jest.resetAllMocks() or re-establish the default
usePermission.mockImplementation(...) so each test gets a clean mock state.
In `@admin-ui/plugins/scim/plugin-metadata.ts`:
- Around line 8-25: The route permission in pluginMetadata is too strict and
prevents READ-only users from accessing ScimPage; update the routes entry for
ScimPage (where component: ScimPage and path: ROUTES.SCIM_BASE) to use
CEDAR_ACTIONS.READ and the same ADMIN_UI_RESOURCES.SCIM resourceKey as the menus
entry so route gating matches the page's GluuViewWrapper canReadScim behavior
(keep canWriteScim only for ScimConfiguration write controls).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f30840d5-0d77-41e7-89b8-253a68b958cd
📒 Files selected for processing (144)
admin-ui/__mocks__/cedarlingPermissionBridge.tsadmin-ui/app/cedarling/__tests__/constants/cedarlingConstants.test.tsadmin-ui/app/cedarling/__tests__/constants/cedarlingLogType.test.tsadmin-ui/app/cedarling/__tests__/constants/resourceScopes.test.tsadmin-ui/app/cedarling/__tests__/enums/CedarlingLogType.test.tsadmin-ui/app/cedarling/__tests__/hooks/useCedarling.test.tsadmin-ui/app/cedarling/__tests__/hooks/usePermission.test.tsadmin-ui/app/cedarling/__tests__/utility/resources.test.tsadmin-ui/app/cedarling/components/Protected.tsxadmin-ui/app/cedarling/components/index.tsadmin-ui/app/cedarling/constants/cedarlingConstants.tsadmin-ui/app/cedarling/constants/index.tsadmin-ui/app/cedarling/constants/resourceCatalog.tsadmin-ui/app/cedarling/constants/resourceScopes.tsadmin-ui/app/cedarling/enums/CedarlingLogType.tsadmin-ui/app/cedarling/hooks/index.tsadmin-ui/app/cedarling/hooks/useCedarling.tsadmin-ui/app/cedarling/hooks/usePermission.tsadmin-ui/app/cedarling/index.tsadmin-ui/app/cedarling/types/cedarTypes.tsadmin-ui/app/cedarling/types/index.tsadmin-ui/app/cedarling/utility/index.tsadmin-ui/app/cedarling/utility/resources.tsadmin-ui/app/components/App/PermissionsPolicyInitializer.tsxadmin-ui/app/components/Sidebar/types.tsadmin-ui/app/redux/features/cedarPermissionsSlice.tsadmin-ui/app/routes/Apps/Gluu/GluuAppSidebar.tsxadmin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsxadmin-ui/app/routes/Apps/Gluu/GluuDialog.tsxadmin-ui/app/routes/Apps/Gluu/GluuWebhookCommitDialog.tsxadmin-ui/app/routes/Apps/Gluu/GluuWebhookExecutionDialog.tsxadmin-ui/app/routes/Apps/Gluu/Tests/GluuAppSiderbar.test.tsxadmin-ui/app/routes/Apps/Profile/ProfilePage.tsxadmin-ui/app/routes/Dashboards/DashboardPage.tsxadmin-ui/app/routes/Dashboards/__tests__/DashboardPage.test.tsxadmin-ui/app/routes/License/LicenseDetailsPage.test.tsxadmin-ui/app/routes/License/LicenseDetailsPage.tsxadmin-ui/app/utils/AppAuthProvider.tsxadmin-ui/app/utils/PermChecker.tsadmin-ui/app/utils/auditAction.tsadmin-ui/app/utils/hooks/useWebhookDialogAction.tsxadmin-ui/app/utils/types/AuditActionTypes.tsadmin-ui/app/utils/types/index.tsadmin-ui/jest.config.tsadmin-ui/plugins/admin/__tests__/components/Asset/JansAssetListPage.test.tsxadmin-ui/plugins/admin/__tests__/components/Asset/assetTestUtils.tsxadmin-ui/plugins/admin/__tests__/components/Audit/AuditListPage.test.tsxadmin-ui/plugins/admin/__tests__/components/Cedarling/CedarlingConfigPage.test.tsxadmin-ui/plugins/admin/__tests__/components/MAU/MauPage.test.tsxadmin-ui/plugins/admin/__tests__/components/Mapping/RolePermissionMappingPage.test.tsxadmin-ui/plugins/admin/__tests__/components/Settings/SettingsPage.test.tsxadmin-ui/plugins/admin/__tests__/components/Webhook/WebhookListPage.test.tsxadmin-ui/plugins/admin/components/Assets/JansAssetAddPage.tsxadmin-ui/plugins/admin/components/Assets/JansAssetEditPage.tsxadmin-ui/plugins/admin/components/Assets/JansAssetListPage.tsxadmin-ui/plugins/admin/components/Audit/AuditListPage.tsxadmin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsxadmin-ui/plugins/admin/components/MAU/MauPage.tsxadmin-ui/plugins/admin/components/Mapping/RolePermissionMappingPage.tsxadmin-ui/plugins/admin/components/Settings/SettingsPage.tsxadmin-ui/plugins/admin/components/Webhook/WebhookAddPage.tsxadmin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsxadmin-ui/plugins/admin/components/Webhook/WebhookListPage.tsxadmin-ui/plugins/admin/helper/settings.tsadmin-ui/plugins/admin/helper/types/SettingsTypes.tsadmin-ui/plugins/admin/plugin-metadata.tsadmin-ui/plugins/auth-server/components/AuthServerProperties/__tests__/components/AuthServerPropertiesPage.test.tsxadmin-ui/plugins/auth-server/components/AuthServerProperties/components/AuthServerPropertiesPage.tsxadmin-ui/plugins/auth-server/components/Authentication/Acrs/Acrs.tsxadmin-ui/plugins/auth-server/components/Authentication/AgamaFlows/AgamaFlows.tsxadmin-ui/plugins/auth-server/components/Authentication/AgamaFlows/__tests__/helpers/agamaTestUtils.tsxadmin-ui/plugins/auth-server/components/Authentication/AgamaFlows/helper/utils.tsadmin-ui/plugins/auth-server/components/Authentication/Aliases/Aliases.tsxadmin-ui/plugins/auth-server/components/Authentication/DefaultAcr/DefaultAcr.tsxadmin-ui/plugins/auth-server/components/Authentication/__tests__/helpers/authenticationTestUtils.tsxadmin-ui/plugins/auth-server/components/Authentication/constants.tsadmin-ui/plugins/auth-server/components/ConfigApiProperties/components/ConfigApiPropertiesForm.tsxadmin-ui/plugins/auth-server/components/ConfigApiProperties/components/ConfigApiPropertiesPage.tsxadmin-ui/plugins/auth-server/components/Keys/__tests__/components/JwkItem.test.tsxadmin-ui/plugins/auth-server/components/Keys/__tests__/components/JwkListPage.test.tsxadmin-ui/plugins/auth-server/components/Keys/__tests__/components/KeysPage.test.tsxadmin-ui/plugins/auth-server/components/Keys/components/KeysPage.tsxadmin-ui/plugins/auth-server/components/Logging/__tests__/components/LoggingPage.test.tsxadmin-ui/plugins/auth-server/components/Logging/components/LoggingPage.tsxadmin-ui/plugins/auth-server/components/OidcClients/__tests__/components/ClientListPage.test.tsxadmin-ui/plugins/auth-server/components/OidcClients/components/ClientListPage.tsxadmin-ui/plugins/auth-server/components/OidcClients/components/ClientWizardForm.tsxadmin-ui/plugins/auth-server/components/Scopes/__tests__/components/ScopeListPage.test.tsxadmin-ui/plugins/auth-server/components/Scopes/components/ScopeListPage.tsxadmin-ui/plugins/auth-server/components/Sessions/__tests__/components/SessionDetailPage.test.tsxadmin-ui/plugins/auth-server/components/Sessions/__tests__/components/SessionListPage.test.tsxadmin-ui/plugins/auth-server/components/Sessions/components/SessionListPage.tsxadmin-ui/plugins/auth-server/components/Ssa/__tests__/helpers/ssaTestUtils.tsxadmin-ui/plugins/auth-server/components/Ssa/components/SsaListPage.tsxadmin-ui/plugins/auth-server/hooks/useAuthServerJsonProperties.tsadmin-ui/plugins/auth-server/plugin-metadata.tsxadmin-ui/plugins/auth-server/redux/utils/auditHelpers.tsadmin-ui/plugins/auth-server/services/jsonPropertiesService.tsadmin-ui/plugins/fido/__tests__/components/DynamicConfiguration.test.tsxadmin-ui/plugins/fido/__tests__/components/Fido.test.tsxadmin-ui/plugins/fido/__tests__/components/Metrics/MetricsPage.test.tsxadmin-ui/plugins/fido/__tests__/components/StaticConfiguration.test.tsxadmin-ui/plugins/fido/components/Configuration/Fido.tsxadmin-ui/plugins/fido/components/Metrics/MetricsPage.tsxadmin-ui/plugins/fido/plugin-metadata.tsadmin-ui/plugins/jans-lock/__tests__/components/JansLockConfiguration.test.tsxadmin-ui/plugins/jans-lock/__tests__/components/JansLockFieldRenderer.test.tsxadmin-ui/plugins/jans-lock/components/JansLock.tsxadmin-ui/plugins/jans-lock/plugin-metadata.tsadmin-ui/plugins/saml/components/SamlConfigurationForm.tsxadmin-ui/plugins/saml/components/WebsiteSsoIdentityBrokeringList.tsxadmin-ui/plugins/saml/components/WebsiteSsoServiceProviderList.tsxadmin-ui/plugins/saml/plugin-metadata.tsadmin-ui/plugins/scim/__tests__/components/ScimPage.test.tsxadmin-ui/plugins/scim/components/ScimPage.tsxadmin-ui/plugins/scim/plugin-metadata.tsadmin-ui/plugins/scripts/__tests__/components/CustomScriptAddPage.test.tsxadmin-ui/plugins/scripts/__tests__/components/CustomScriptEditPage.test.tsxadmin-ui/plugins/scripts/__tests__/components/CustomScriptForm.test.tsxadmin-ui/plugins/scripts/components/CustomScriptAddPage.tsxadmin-ui/plugins/scripts/components/CustomScriptEditPage.tsxadmin-ui/plugins/scripts/components/CustomScriptListPage.tsxadmin-ui/plugins/scripts/plugin-metadata.tsadmin-ui/plugins/services/Components/CachePage.tsxadmin-ui/plugins/services/Components/PersistenceDetail.tsxadmin-ui/plugins/services/__tests__/components/PersistenceDetail.test.tsxadmin-ui/plugins/services/plugin-metadata.tsadmin-ui/plugins/smtp/__tests__/components/SmtpEditPage.test.tsxadmin-ui/plugins/smtp/__tests__/components/SmtpForm.test.tsxadmin-ui/plugins/smtp/components/SmtpEditPage.tsxadmin-ui/plugins/smtp/plugin-metadata.tsadmin-ui/plugins/user-claims/__tests__/cedarTestHelpers.tsadmin-ui/plugins/user-claims/__tests__/components/UserClaimsAddPage.test.tsxadmin-ui/plugins/user-claims/__tests__/components/UserClaimsEditPage.test.tsxadmin-ui/plugins/user-claims/__tests__/components/UserClaimsForm.test.tsxadmin-ui/plugins/user-claims/__tests__/components/UserClaimsListPage.test.tsxadmin-ui/plugins/user-claims/components/UserClaimsAddPage.tsxadmin-ui/plugins/user-claims/components/UserClaimsEditPage.tsxadmin-ui/plugins/user-claims/components/UserClaimsListPage.tsxadmin-ui/plugins/user-claims/components/UserClaimsViewPage.tsxadmin-ui/plugins/user-claims/plugin-metadata.tsadmin-ui/plugins/user-management/__tests__/helpers/userManagementTestUtils.tsxadmin-ui/plugins/user-management/components/UserList.tsxadmin-ui/plugins/user-management/plugin-metadata.ts
💤 Files with no reviewable changes (39)
- admin-ui/plugins/auth-server/components/Authentication/constants.ts
- admin-ui/app/cedarling/enums/CedarlingLogType.ts
- admin-ui/app/cedarling/constants/resourceScopes.ts
- admin-ui/app/cedarling/tests/enums/CedarlingLogType.test.ts
- admin-ui/plugins/smtp/tests/components/SmtpEditPage.test.tsx
- admin-ui/app/routes/Dashboards/tests/DashboardPage.test.tsx
- admin-ui/plugins/admin/tests/components/Settings/SettingsPage.test.tsx
- admin-ui/plugins/admin/tests/components/Asset/assetTestUtils.tsx
- admin-ui/plugins/admin/tests/components/Mapping/RolePermissionMappingPage.test.tsx
- admin-ui/plugins/services/tests/components/PersistenceDetail.test.tsx
- admin-ui/plugins/auth-server/components/Ssa/tests/helpers/ssaTestUtils.tsx
- admin-ui/plugins/smtp/tests/components/SmtpForm.test.tsx
- admin-ui/plugins/scripts/tests/components/CustomScriptAddPage.test.tsx
- admin-ui/plugins/admin/tests/components/Audit/AuditListPage.test.tsx
- admin-ui/plugins/scripts/tests/components/CustomScriptEditPage.test.tsx
- admin-ui/plugins/admin/tests/components/Cedarling/CedarlingConfigPage.test.tsx
- admin-ui/plugins/auth-server/components/Keys/tests/components/JwkItem.test.tsx
- admin-ui/plugins/auth-server/components/Keys/tests/components/KeysPage.test.tsx
- admin-ui/app/utils/PermChecker.ts
- admin-ui/plugins/admin/tests/components/MAU/MauPage.test.tsx
- admin-ui/plugins/jans-lock/tests/components/JansLockFieldRenderer.test.tsx
- admin-ui/plugins/admin/tests/components/Webhook/WebhookListPage.test.tsx
- admin-ui/plugins/auth-server/components/AuthServerProperties/tests/components/AuthServerPropertiesPage.test.tsx
- admin-ui/plugins/auth-server/components/Sessions/tests/components/SessionDetailPage.test.tsx
- admin-ui/plugins/fido/tests/components/DynamicConfiguration.test.tsx
- admin-ui/plugins/scim/tests/components/ScimPage.test.tsx
- admin-ui/plugins/auth-server/components/Authentication/tests/helpers/authenticationTestUtils.tsx
- admin-ui/plugins/auth-server/components/Sessions/tests/components/SessionListPage.test.tsx
- admin-ui/app/routes/Apps/Gluu/Tests/GluuAppSiderbar.test.tsx
- admin-ui/plugins/scripts/tests/components/CustomScriptForm.test.tsx
- admin-ui/plugins/fido/tests/components/Fido.test.tsx
- admin-ui/plugins/user-management/tests/helpers/userManagementTestUtils.tsx
- admin-ui/app/redux/features/cedarPermissionsSlice.ts
- admin-ui/plugins/jans-lock/tests/components/JansLockConfiguration.test.tsx
- admin-ui/plugins/auth-server/components/Logging/tests/components/LoggingPage.test.tsx
- admin-ui/plugins/auth-server/components/Keys/tests/components/JwkListPage.test.tsx
- admin-ui/plugins/user-claims/tests/cedarTestHelpers.ts
- admin-ui/plugins/auth-server/components/Authentication/AgamaFlows/tests/helpers/agamaTestUtils.tsx
- admin-ui/plugins/fido/tests/components/StaticConfiguration.test.tsx
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@admin-ui/plugins/auth-server/components/Authentication/Aliases/__tests__/Aliases.test.tsx`:
- Around line 71-85: The tests for Aliases use a mocked useCedarling with
default empty acrMappings which causes no row actions to render and makes the
permission assertions vacuously pass; update both tests that mock useCedarling
(the ones changing hasCedarWritePermission and hasCedarDeletePermission) to
return a makeMockCedarling where acrMappings contains at least one row (e.g., a
simple mapping object) so the component will render row actions when permissions
allow, then keep the mocked permission functions (hasCedarWritePermission /
hasCedarDeletePermission) returning false and assert that the edit/delete action
is absent; locate mocks around useCedarling and makeMockCedarling in the Aliases
tests and add the non-empty acrMappings to the mocked cedarling return value.
In `@admin-ui/plugins/smtp/plugin-metadata.ts`:
- Around line 20-23: The route for SmtpEditPage was changed to
CEDAR_ACTIONS.READ, widening access; revert the action for the route that
references SmtpEditPage and ROUTES.SMTP_BASE back to CEDAR_ACTIONS.WRITE
(keeping ADMIN_UI_RESOURCES.SMTP as the resourceKey) so the SMTP editor keeps
its original write-level authorization enforcement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3f8eee2f-5f7f-4319-9ae0-c4374fa7aced
📒 Files selected for processing (17)
admin-ui/app/constants/ui.tsadmin-ui/plugins/admin/components/Assets/JansAssetAddPage.tsxadmin-ui/plugins/admin/components/Assets/JansAssetEditPage.tsxadmin-ui/plugins/admin/components/Webhook/WebhookAddPage.tsxadmin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsxadmin-ui/plugins/admin/helper/settings.tsadmin-ui/plugins/admin/plugin-metadata.tsadmin-ui/plugins/auth-server/components/Authentication/Aliases/Aliases.tsxadmin-ui/plugins/auth-server/components/Authentication/Aliases/__tests__/Aliases.test.tsxadmin-ui/plugins/auth-server/components/Ssa/components/SsaListPage.tsxadmin-ui/plugins/auth-server/plugin-metadata.tsxadmin-ui/plugins/fido/__tests__/components/Metrics/MetricsPage.test.tsxadmin-ui/plugins/fido/plugin-metadata.tsadmin-ui/plugins/saml/components/WebsiteSsoServiceProviderList.tsxadmin-ui/plugins/scim/plugin-metadata.tsadmin-ui/plugins/scripts/plugin-metadata.tsadmin-ui/plugins/smtp/plugin-metadata.ts
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
mo-auto
added
the
area-documentation
Contributor
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/app/routes/Pages/ByeBye.tsx (1)
26-65:⚠️ Potential issue | 🟡 MinorAdd rejection handling to
performLogout()fire-and-forget call
performLogout()is started fromuseEffectwithout handling its returned promise (Line 64). While the awaiteddeleteSession()is wrapped intry/catchand thebuildSafeNavigationUrl/buildSafeLogoutUrlhelpers returnnullon invalid inputs instead of throwing, any unexpected synchronous error insideperformLogoutwould still become an unhandled rejection and prevent the intended redirect.🔧 Suggested fix
@@ - performLogout() + void performLogout().catch((error) => { + devLogger.error('Unexpected logout failure:', error instanceof Error ? error : String(error)) + window.location.href = '/' + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin-ui/app/routes/Pages/ByeBye.tsx` around lines 26 - 65, The useEffect currently calls performLogout() without handling its returned promise; wrap the call so any rejection is caught (e.g., call performLogout().catch(...)) and log the error via devLogger.error and perform a safe fallback redirect (window.location.href = '/' or use buildSafeNavigationUrl) to ensure redirects still happen if performLogout (which uses deleteSession, buildSafeLogoutUrl, buildSafeNavigationUrl) throws synchronously or rejects; update the effect to call performLogout and handle errors instead of fire-and-forget.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@admin-ui/app/utils/storage.ts`:
- Line 3: The storage availability check in isAvailable currently reads
window.localStorage directly which can throw a SecurityError in blocked-storage
environments; wrap the access in a try/catch inside isAvailable, return false if
any exception is thrown (or if typeof window === 'undefined'), and only return
true when accessing window.localStorage succeeds and is truthy so callers never
receive an exception from this helper.
In `@admin-ui/docs/cedarling.md`:
- Around line 193-195: The import path for ADMIN_UI_RESOURCES is incorrect in
the example; update the import so ADMIN_UI_RESOURCES (and CEDAR_ACTIONS if also
exported from there) is imported from '`@/cedarling/utility`' instead of
'`@/cedarling/constants`' to match the module map; adjust the import line that
references Protected, ADMIN_UI_RESOURCES, and CEDAR_ACTIONS so consumers copying
the example will get the correct module.
---
Outside diff comments:
In `@admin-ui/app/routes/Pages/ByeBye.tsx`:
- Around line 26-65: The useEffect currently calls performLogout() without
handling its returned promise; wrap the call so any rejection is caught (e.g.,
call performLogout().catch(...)) and log the error via devLogger.error and
perform a safe fallback redirect (window.location.href = '/' or use
buildSafeNavigationUrl) to ensure redirects still happen if performLogout (which
uses deleteSession, buildSafeLogoutUrl, buildSafeNavigationUrl) throws
synchronously or rejects; update the effect to call performLogout and handle
errors instead of fire-and-forget.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 49fe3a57-e2ef-43ca-9647-5282f872ee81
📒 Files selected for processing (27)
admin-ui/app/cedarling/types/cedarTypes.tsadmin-ui/app/constants/storageKeys.tsadmin-ui/app/context/theme/themeContext.tsxadmin-ui/app/i18n.tsadmin-ui/app/layout/default.tsxadmin-ui/app/redux/features/logoutSlice.tsadmin-ui/app/redux/listeners/authListener.tsadmin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsxadmin-ui/app/routes/Apps/Gluu/LanguageMenu.tsxadmin-ui/app/routes/Apps/Gluu/ThemeDropdown.tsxadmin-ui/app/routes/Dashboards/DashboardPage.tsxadmin-ui/app/routes/Pages/ByeBye.tsxadmin-ui/app/utils/AppAuthProvider.tsxadmin-ui/app/utils/TokenController.tsadmin-ui/app/utils/pagingUtils.tsadmin-ui/app/utils/storage.tsadmin-ui/app/utils/types/AppAuthProviderTypes.tsadmin-ui/docs/auth.mdadmin-ui/docs/cedarling.mdadmin-ui/docs/onboarding.mdadmin-ui/docs/recipes.mdadmin-ui/docs/testing.mdadmin-ui/plugins/admin/__tests__/components/Asset/assetTestUtils.tsxadmin-ui/plugins/auth-server/components/Authentication/AgamaFlows/__tests__/helpers/agamaTestUtils.tsxadmin-ui/plugins/auth-server/components/Authentication/Aliases/__tests__/Aliases.test.tsxadmin-ui/plugins/auth-server/components/Authentication/__tests__/helpers/authenticationTestUtils.tsxadmin-ui/plugins/auth-server/components/Ssa/__tests__/helpers/ssaTestUtils.tsx
💤 Files with no reviewable changes (5)
- admin-ui/app/cedarling/types/cedarTypes.ts
- admin-ui/app/constants/storageKeys.ts
- admin-ui/plugins/admin/tests/components/Asset/assetTestUtils.tsx
- admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx
- admin-ui/app/redux/listeners/authListener.ts
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
duttarnab
approved these changes
Jun 10, 2026
duttarnab
requested changes
Jun 10, 2026
duttarnab
left a comment
duttarnab
left a comment
Contributor
There was a problem hiding this comment.
- I removed dashboard policy but getting following screen:
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
|
duttarnab
approved these changes
Jun 10, 2026
moabu
approved these changes
Jun 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
refactor(admin-ui): migrate permission gating to Cedarling usePermission + single-source action catalog (#2872)
Summary
This PR modernizes Admin UI authorization by replacing the legacy
PermCheckerstring-based permission flow with Cedarling-backed permission gating and a centralized action catalog.The refactor introduces a consistent authorization model through
usePermissionandProtected, eliminates scattered string-based permission checks, establishes a single source of truth for resource actions, and reorganizes the Cedarling module into a cleaner constants → types → utility architecture.This change improves type safety, maintainability, and consistency across authorization-related code while preserving existing access-control behavior.
Fix Summary
PermCheckerpermission gating with Cedarling authorizationusePermission(resource)hook exposing:canReadcanWritecanDelete<Protected>component for action-based UI renderingplugins
RESOURCE_ACTIONScatalog as the single source of truth for resource actionsCEDAR_ACTIONSconstant and replaced bare action literals throughout the codebaseCedarActionfrom the centralized action catalogCedarlingLogTypeenum withCEDARLING_LOG_TYPEconstanttypes/cedarTypes.tstypes/index.tsresourceScopes.tsresources.tsPermChecker.tsapp/utils/auditAction.tsPermCheckerTypestoAuditActionTypesVerification
passes successfully.
usePermissiontest coverage🔗 Ticket
Closes: #2872
Summary by CodeRabbit
Refactored Authorization System
Improved Storage Handling
UI Adjustments