feat(flex): integrate new cedarling flow on Admin UI#2425
Conversation
mo-auto
added
comp-admin-ui
📝 WalkthroughWalkthroughIntegrates Cedarling resource-scoped authorization across the Admin UI: adds CEDAR resource scopes/constants/types, rewrites useCedarling with caching and batch authorization, fetches/stores policyStoreJson at startup, updates many components/metadata to use resourceKey-based gating, and adds supporting utilities, hooks, and types. Changes
Sequence Diagram(s)sequenceDiagram
participant AppInit
participant AuthProvider
participant Backend
participant Redux
participant Cedarling
participant UI as Component
AppInit->>AuthProvider: start
AuthProvider->>Backend: fetchApiAccessToken(ujwt)
Backend-->>AuthProvider: apiAccessToken
AuthProvider->>Backend: fetchPolicyStore(access_token)
Backend-->>AuthProvider: policyStoreJson
AuthProvider->>Redux: dispatch(setPolicyStoreJson)
Redux->>Cedarling: store policyStoreJson
AppInit->>Cedarling: initialize with policyStoreJson
UI->>Cedarling: authorizeHelper(scopes[])
Cedarling->>Cedarling: dedupe & build requests
alt cache hit
Cedarling-->>UI: cached decision
else
Cedarling->>Cedarling: evaluate policies, cache result
Cedarling-->>UI: decision
end
UI->>UI: render guarded by permissions
sequenceDiagram
participant Component
participant useCedarling
participant authorizeHelper
participant CedarEngine
participant Redux
Component->>useCedarling: obtain helpers
Component->>authorizeHelper: batch(ResourceScopeEntry[])
authorizeHelper->>authorizeHelper: dedupe(resourceId,action)
loop unique checks
authorizeHelper->>CedarEngine: build request (cedar_entity_mapping)
CedarEngine->>CedarEngine: check cache
alt hit
CedarEngine-->>authorizeHelper: cached result
else
CedarEngine->>CedarEngine: evaluate against policyStore
CedarEngine-->>authorizeHelper: result
authorizeHelper->>Redux: optionally persist permissions
end
end
authorizeHelper-->>Component: AuthorizationResult[]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
…g disable buttons to removing the buttons in SAML which dont have the permissions
faisalsiddique4400
requested review from
duttarnab and
syntrydy
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (19)
admin-ui/plugins/schema/components/Person/AttributeListPage.tsx (1)
221-276: Permission-awareattributeActionswiring is correct; somedisabledflags are redundantThe
attributeActionsmemo correctly:
- Builds actions conditionally based on
canReadAttributes,canWriteAttributes, andcanDeleteAttributes.- Uses stable callbacks (
handleGoToAttribute*,handleAttribueDelete,DeleteOutlinedIcon).- Includes the right dependencies so actions recompute when permissions or translations change.
Two minor nits:
- Inside the guarded blocks,
disabled: !canWriteAttributesanddisabled: !canDeleteAttributesare redundant, because the action is only pushed when the correspondingcan*is true. You could safely omit thosedisabledprops without behavior change.- The explicit generic on
useMemois correct but verbose; optionally, you could pull the action type into atype AttributeAction = Action<JansAttribute> | ((rowData: JansAttribute) => Action<JansAttribute>)alias to improve readability.No functional issues here.
admin-ui/plugins/auth-server/components/Message/MessageForm.js (1)
49-82: Add UI-level authorization checks before form submission to improve user experience.The form fields are unconditionally enabled (
isDisabled = false), allowing users without write permissions to complete the entire form. WhilebuildPayloadonly constructs the action payload without permission validation, and authorization is ultimately enforced by the backend (sagas catch 401 errors), this creates poor UX—users discover authorization errors only after submission.Options:
- Check permissions before allowing form submission and dispatch
- Ensure backend returns and displays clear authorization error messages to users
- Confirm this authorization delegation pattern is intentional and consistent across all Admin UI configuration forms
admin-ui/plugins/fido/components/Fido.tsx (2)
68-122: Good defense‑in‑depth on writes; audit logging remains robustEarly‑returning from the submit handler when
canWriteFidois false adds a solid safety net on top of the readOnly UI, preventing accidental writes if the UI is ever mis‑configured. The audit logging (withconsole.errorin the catch) also aligns with prior guidance to keep full error details for unexpected failures. Based on learnings.If you ever want clearer feedback for edge cases (e.g., if a write is somehow attempted without permission), you could emit an explicit “not authorized” toast before the
return, but this is strictly optional given the current UI gating.
129-160: readOnly propagation to Static/Dynamic configuration looks correctPassing
readOnly={!canWriteFido}into bothStaticConfigurationandDynamicConfigurationand includingcanWriteFidoin thetabToShowdependencies ensures tab content re-renders correctly when permissions change. The write‑gating at this level looks coherent with the shared Cedarling model.If you later need stricter “view‑only” behavior, consider also propagating a readOnly flag down into lower‑level input components so the fields themselves become visually disabled when write permission is missing.
admin-ui/plugins/fido/components/DynamicConfiguration.tsx (1)
18-65: readOnly handling correctly blocks dynamic FIDO updatesThe
readOnlyprop is consistently honored across the dynamic configuration flow: Formik’sonSubmit, the internal submit handler, the form’s submit event, the Apply button, and the commit dialog all short‑circuit whenreadOnlyis true, so no mutations can slip through.If read‑only mode is intended to be strictly “view only,” you may also want to propagate
readOnly/disableddown into the underlyingGluu*inputs and toggles so users can’t even make temporary local edits that they can’t save anyway.Also applies to: 318-331
admin-ui/plugins/fido/components/StaticConfiguration.tsx (1)
35-67: Static configuration readOnly flow is consistent with dynamic configThe static configuration path mirrors the dynamic one: Formik
onSubmit,submitForm, and the form submit handler are all guarded byreadOnly, the Apply button is hidden, and the commit dialog is not rendered in read‑only mode. This gives you robust protection against unintended writes when the user lacks FIDO write permission.
- As with the dynamic form, consider propagating a disabled/readOnly flag into the
Gluu*field components if you want a fully non‑editable UI in read‑only mode.- If Cedarling becomes the single source of truth for permissions everywhere, you might eventually revisit whether
feature={adminUiFeatures.fido_configuration_write}onGluuCommitDialogis still needed, to avoid mixing permission systems.Also applies to: 132-141, 378-397
admin-ui/app/components/Sidebar/types.ts (1)
2-10: Consider narrowingresourceKeytype to match ADMIN_UI_RESOURCES
resourceKeyis currently a plainstring, but it looks like it should line up with your Cedarling resource identifiers (e.g., keys ofADMIN_UI_RESOURCES). If you have a shared union type for those (such asAdminUiFeatureResourceorkeyof typeof ADMIN_UI_RESOURCES), using it here would catch typos at compile time while keeping the field optional.cedarlingPassedas an optional boolean flag is fine as-is.admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.js (1)
76-97: Fix TDZ:handleGoToAuthNEditPageis used in deps before it is declared.
handleGoToAuthNEditPageis referenced in theuseEffectdependency array (Line 96) before itsconst ... = useCallback(...)declaration (Line 129). In JavaScript this hits the temporal dead zone and can throw aReferenceErrorat render time, which matches the BiomenoInvalidUseBeforeDeclarationerror.Move the
useCallbackabove the effect (or otherwise ensure it’s declared first) so the identifier is initialized before use:- // Actions as state that will rebuild when permissions change - useEffect(() => { - const newActions = [] - - if (canWriteAuthN) { - newActions.push((rowData) => { - return { - icon: 'edit', - iconProps: { - id: 'editAutN' + rowData.inum, - style: { color: customColors.darkGray }, - }, - tooltip: `${t('messages.edit_authn')}`, - onClick: (event, rowData) => handleGoToAuthNEditPage(rowData), - disabled: !canWriteAuthN, - } - }) - } - - setMyActions(newActions) - }, [canWriteAuthN, t, handleGoToAuthNEditPage]) - - const handleGoToAuthNEditPage = useCallback( - (row) => { - dispatch(setCurrentItem({ item: row })) - return navigate(`/auth-server/authn/edit/:` + row.inum) - }, - [dispatch, navigate], - ) + const handleGoToAuthNEditPage = useCallback( + (row) => { + dispatch(setCurrentItem({ item: row })) + return navigate(`/auth-server/authn/edit/:` + row.inum) + }, + [dispatch, navigate], + ) + + // Actions as state that will rebuild when permissions change + useEffect(() => { + const newActions = [] + + if (canWriteAuthN) { + newActions.push((rowData) => ({ + icon: 'edit', + iconProps: { + id: 'editAutN' + rowData.inum, + style: { color: customColors.darkGray }, + }, + tooltip: `${t('messages.edit_authn')}`, + onClick: (event, rowData) => handleGoToAuthNEditPage(rowData), + disabled: !canWriteAuthN, + })) + } + + setMyActions(newActions) + }, [canWriteAuthN, t, handleGoToAuthNEditPage])Also applies to: 129-135
admin-ui/plugins/services/Components/Configuration/SqlListPage.js (1)
92-152: Resolve TDZ: SQL action handlers are used before they are declared.In the actions
useEffectdependency array (Lines 143‑152),handleGoToSqlAddPage,handleGoToSqlEditPage, andhandleSqlDeleteare referenced before theirconst ... = useCallback(...)declarations (Lines 154‑169). This is a temporal dead zone issue and matches the BiomenoInvalidUseBeforeDeclarationerrors; it can throw at render time.Reorder so the callbacks are declared before the effect:
- // Build actions only when permissions change - useEffect(() => { - const actions = [] - - if (canWriteSql) { - actions.push({ - icon: 'add', - tooltip: `${t('tooltips.add_sql')}`, - iconProps: { color: 'primary', style: { color: customColors.lightBlue } }, - isFreeAction: true, - onClick: handleGoToSqlAddPage, - }) - actions.push((rowData) => ({ - icon: 'edit', - iconProps: { - id: 'editSql' + rowData.configId, - style: { color: customColors.darkGray }, - }, - tooltip: `${t('tooltips.edit_sql')}`, - onClick: (event, rowData) => handleGoToSqlEditPage(rowData), - disabled: !canWriteSql, - })) - } - - if (canReadSql) { - actions.push({ - icon: 'refresh', - tooltip: `${t('tooltips.refresh_data')}`, - iconProps: { color: 'primary', style: { color: customColors.lightBlue } }, - isFreeAction: true, - onClick: () => { - dispatch(getSqlConfig()) - }, - }) - } - - if (canDeleteSql) { - actions.push((rowData) => ({ - icon: () => <DeleteOutlined />, - iconProps: { - color: 'secondary', - id: 'deleteSql' + rowData.configId, - style: { color: customColors.darkGray }, - }, - tooltip: `${t('tooltips.delete_record')}`, - onClick: (event, rowData) => handleSqlDelete(rowData), - disabled: !canDeleteSql, - })) - } - - setMyActions(actions) - }, [ - canReadSql, - canWriteSql, - canDeleteSql, - t, - dispatch, - handleGoToSqlAddPage, - handleGoToSqlEditPage, - handleSqlDelete, - ]) - - const handleGoToSqlEditPage = useCallback( + const handleGoToSqlEditPage = useCallback( (row) => { dispatch(setCurrentItem(row)) navigate(`/config/sql/edit/:` + row.configId) }, [dispatch, navigate], ) @@ const handleGoToSqlAddPage = useCallback(() => { navigate('/config/sql/new') }, [navigate]) - const onDeletionConfirmed = useCallback( + const onDeletionConfirmed = useCallback( (message) => { buildPayload(userAction, message, item.configId) dispatch(deleteSql(item.configId)) navigate('/config/sql') toggle() }, [item.configId, dispatch, navigate], ) + + // Build actions only when permissions change + useEffect(() => { + const actions = [] + + if (canWriteSql) { + actions.push({ + icon: 'add', + tooltip: `${t('tooltips.add_sql')}`, + iconProps: { color: 'primary', style: { color: customColors.lightBlue } }, + isFreeAction: true, + onClick: handleGoToSqlAddPage, + }) + actions.push((rowData) => ({ + icon: 'edit', + iconProps: { + id: 'editSql' + rowData.configId, + style: { color: customColors.darkGray }, + }, + tooltip: `${t('tooltips.edit_sql')}`, + onClick: (event, rowData) => handleGoToSqlEditPage(rowData), + disabled: !canWriteSql, + })) + } + + if (canReadSql) { + actions.push({ + icon: 'refresh', + tooltip: `${t('tooltips.refresh_data')}`, + iconProps: { color: 'primary', style: { color: customColors.lightBlue } }, + isFreeAction: true, + onClick: () => { + dispatch(getSqlConfig()) + }, + }) + } + + if (canDeleteSql) { + actions.push((rowData) => ({ + icon: () => <DeleteOutlined />, + iconProps: { + color: 'secondary', + id: 'deleteSql' + rowData.configId, + style: { color: customColors.darkGray }, + }, + tooltip: `${t('tooltips.delete_record')}`, + onClick: (event, rowData) => handleSqlDelete(rowData), + disabled: !canDeleteSql, + })) + } + + setMyActions(actions) + }, [ + canReadSql, + canWriteSql, + canDeleteSql, + t, + dispatch, + handleGoToSqlAddPage, + handleGoToSqlEditPage, + handleSqlDelete, + ])Also applies to: 154-179
admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
100-106: Fix patchHandler’s double state update and array mutation
patchHandlercurrently:
- Calls
setPatcheswith a functional update and then immediately mutates the stalepatchesarray and callssetPatchesagain.- This risks duplicate entries and relies on a stale snapshot of
patches.You can simplify to a single, immutable update that also keeps
operationsin sync:- const patchHandler = (patch) => { - setPatches((existingPatches) => [...existingPatches, patch]) - const newPatches = patches - newPatches.push(patch) - setPatches(newPatches) - setOperations(newPatches.concat(put)) - } + const patchHandler = (patch) => { + setPatches((existingPatches) => { + const nextPatches = [...existingPatches, patch] + setOperations(nextPatches.concat(put)) + return nextPatches + }) + }admin-ui/plugins/auth-server/components/Agama/AgamaAliasListPage.js (1)
69-98: Fix use-before-declaration ofhandleEditin the actions effectThe temporal dead zone (TDZ) issue is confirmed and critical.
handleEditis referenced in theuseEffectdependency array (line 98) before itsconstdeclaration at line 140. This throws a ReferenceError at first render.The proposed fix correctly reorders the code: move
handleEditdeclaration (currently at line 140) before theuseEffectthat builds actions (currently at lines 69–98). This preserves the safe dependency chain:formik(line 104) →handleSubmit(line 115) →handleEdit(line 140) →useEffect(moves after line 140).Apply the reordering as suggested in the review.
admin-ui/plugins/services/Components/Configuration/LdapListPage.js (2)
120-183: Simplify actiondisabledflags gated bycan*LdapInside the actions effect, you already wrap action creation in
if (canWriteLdap)/if (canDeleteLdap). Within those branches,disabled: !canWriteLdapanddisabled: !canDeleteLdapwill always evaluate tofalse, so they don’t actually gate anything.You can safely drop these
disabledproperties (or, if you want row-level gating later, base them on row data instead) to reduce confusion:- onClick: (event, rowData) => handleGoToLdapEditPage(rowData), - disabled: !canWriteLdap, + onClick: (event, rowData) => handleGoToLdapEditPage(rowData), ... - onClick: (event, rowData) => handleLdapDelete(rowData), - disabled: !canDeleteLdap, + onClick: (event, rowData) => handleLdapDelete(rowData),
196-204: Attach built audit payload to the dispatched delete actionbuildPayload populates userAction.action_message and userAction.action_data, but LdapListPage dispatches only { configId: item.configId }, so LdapSaga’s addAdditionalData will not see the userAction and the audit will miss message/action_data — include the built userAction in the dispatched payload (e.g. dispatch(deleteLdap({ configId: item.configId, action: userAction }))) or change buildPayload to write into the payload shape the saga expects. File: admin-ui/plugins/services/Components/Configuration/LdapListPage.js (lines ~196–204).
admin-ui/app/routes/Apps/Gluu/GluuAppSidebar.tsx (2)
81-87: DerivingisReadyfrompluginMenus.lengthcan cause an infinite loader
isReadyis computed aspluginMenus.length > 0, and the sidebar only exits the loading state whenisReadyis true. If, after processing and authorization, there are legitimately no menu items (e.g., all filtered out due to health status or permissions, orprocessMenusreturns an empty array),pluginMenusstays empty and:
isReadynever becomes true,- the loader (
<GluuLoader blocking />) is shown indefinitely,- the wave/footer uses the “fixed” loading style.
This conflates “finished loading, but there are no menus” with “still loading”.
Consider decoupling readiness from the number of menu items, e.g. by tracking a separate boolean that is set once
loadMenusfinishes (even if it yields an empty array):- const [pluginMenus, setPluginMenus] = useState<PluginMenu[]>([]) - const didAnimateMenusRef = useRef<boolean>(false) - const isReady = pluginMenus.length > 0 + const [pluginMenus, setPluginMenus] = useState<PluginMenu[]>([]) + const didAnimateMenusRef = useRef<boolean>(false) + const [isReady, setIsReady] = useState(false) ... const loadMenus = async () => { try { const filteredMenus = await filterMenuItems(await memoizedFilteredMenus) setPluginMenus(filteredMenus) } finally { if (!didAnimateMenusRef.current) { didAnimateMenusRef.current = true } + setIsReady(true) } }This way the UI can render an “empty menu” state without being stuck in a perpetual loading indicator.
Also applies to: 161-170, 183-235, 237-241
148-159: Avoid asyncuseMemofor loading menus
memoizedFilteredMenusis defined as an asyncuseMemo, returning aPromise<PluginMenu[]>. While this works because you laterawait memoizedFilteredMenus, usinguseMemo(async () => …)is an anti-pattern:useMemois designed for synchronous calculations and will re-run in strict mode, potentially re-invokingprocessMenusmore than expected.A clearer pattern is to keep
useMemosynchronous and move the async work entirely intoloadMenus(or a dedicateduseEffect), e.g.:// Keep any cheap, sync derivations in useMemo if needed const menuHealthFilter = useMemo( () => (menus: PluginMenu[]) => !fetchedServersLength ? [] : menus.filter((menu) => { const healthKey = VISIBILITY_CONDITIONS[menu.path || ''] return healthKey ? health?.[healthKey] === 'Running' : true }), [health, fetchedServersLength], ) // Then in loadMenus: const loadMenus = useCallback(async () => { try { const menus = await processMenus() const filteredByHealth = menuHealthFilter(menus) const filteredMenus = await filterMenuItems(filteredByHealth) setPluginMenus(filteredMenus) } finally { // readiness handling... } }, [menuHealthFilter, filterMenuItems])This avoids async
useMemosemantics and makes the dataflow easier to follow.admin-ui/plugins/auth-server/components/Clients/ClientListPage.js (1)
198-214: Consider gating initial client fetch on read permission for consistencyThe initial
useEffectthat callsdispatch(getOpenidClients({ action: options }))runs regardless ofcanReadClient. For unauthorized users, this may still trigger backend calls that will likely return 403s, while the UI is hidden byGluuViewWrapper.This is not a functional bug, but for consistency with newer patterns (e.g., SAML identities list), you might consider conditioning the fetch on
canReadClientonce permissions are initialized, to avoid unnecessary calls and align behavior.admin-ui/plugins/admin/components/Webhook/WebhookListPage.js (1)
92-175: TDZ bug:navigateToAddPageandnavigateToEditPageused before declaration in effect dependenciesThe useEffect at line 93 references
navigateToAddPageandnavigateToEditPagein its dependency array (lines 169–170), but both are declared asconstarrow functions later at lines 225 and 230. During component render, the dependency array is evaluated synchronously when the effect hook is called, before those declarations are initialized, violating the temporal dead zone and causing aReferenceErrorat runtime.Move both callback declarations above the actions-building useEffect:
const [limit, setLimit] = useState(10) const [pattern, setPattern] = useState(null) + const navigateToAddPage = useCallback(() => { + dispatch(setSelectedWebhook({})) + navigate('/adm/webhook/add') + }, [dispatch, navigate]) + + const navigateToEditPage = useCallback( + (data) => { + dispatch(setSelectedWebhook(data)) + navigate(`/adm/webhook/edit/${data.inum}`) + }, + [dispatch, navigate], + ) + // Initialize Cedar permissions useEffect(() => {And remove the old declarations at lines 225–234.
admin-ui/plugins/auth-server/components/Scopes/ScopeListPage.tsx (2)
48-77: Scopes Cedarling integration is coherent; consider surfacing authorization errors if neededUsing
ADMIN_UI_RESOURCES.ScopesplusCEDAR_RESOURCE_SCOPESandauthorizeHelper(scopesScopes)is a clean way to prime Cedarling and then derivecanReadScopes/canWriteScopes/canDeleteScopesviahasCedar*Permission. Dependencies onauthorizeHelperandscopesScopesmean permissions are recomputed when Cedarling initializes, which avoids “stuck unauthorized” states.Right now any errors returned from
authorizeHelperare ignored; if you ever need better debuggability, you could optionally handle the returned results (e.g., log or toast the first failing entry), but that’s not required for correctness.Also applies to: 115-137
220-225: Actions and delete dialog correctly respect Cedarling permissions (minor redundancy only)The edit/add/delete actions are only created when
canWriteScopes/canDeleteScopesare truthy, and the delete dialog is additionally gated bycanDeleteScopes && item, so unauthorized users can neither trigger actions nor see the confirmation dialog. Thedisabled: !canWriteScopes/disabled: !canDeleteScopesflags are redundant given the surroundingif (canWriteScopes)/if (canDeleteScopes)guards but harmless; you could drop thedisabledproperties or, if you want to show disabled icons, always render actions and rely solely ondisabled.Also applies to: 257-311, 482-491
admin-ui/plugins/schema/components/Person/AttributeListPage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpEditPage.tsx
Outdated
Show resolved
Hide resolved
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/plugins/admin/components/Assets/JansAssetListPage.tsx (1)
190-202: Fix useEffect dependencies to match effect body usage.The effect body uses the memoized flags
canReadAssets,canWriteAssets, andcanDeleteAssets(lines 124, 155, 175), but the dependency array lists the hook functionshasCedarReadPermission,hasCedarWritePermission,hasCedarDeletePermission(lines 197-199) instead. Additionally,cedarPermissions(line 191) is listed but never used in the effect body.Hook functions are stable references that don't change between renders, so listing them as dependencies has no effect. The dependencies should include the memoized flags that are actually used.
Apply this diff to correct the dependencies:
}, [ - cedarPermissions, + canReadAssets, + canWriteAssets, + canDeleteAssets, limit, pattern, t, navigateToAddPage, navigateToEditPage, - hasCedarReadPermission, - hasCedarWritePermission, - hasCedarDeletePermission, handleOptionsChange, dispatch, ])
♻️ Duplicate comments (1)
admin-ui/plugins/admin/components/Assets/JansAssetListPage.tsx (1)
36-44: Typo fixed, but add null-safety forassetScopes.The typo from the previous review (
Assests→Assets) has been corrected on line 42. However,assetScopeson line 43 still lacks the null-safety fallback suggested in the prior review. IfCEDAR_RESOURCE_SCOPES[assetsResourceId]is undefined,authorizeHelper(assetScopes)on line 99 will receiveundefined, potentially causing runtime errors.Apply this diff to add null-safety:
const assetScopes = useMemo(() => CEDAR_RESOURCE_SCOPES[assetsResourceId], [assetsResourceId]) + const assetScopes = useMemo( + () => CEDAR_RESOURCE_SCOPES[assetsResourceId] ?? [], + [assetsResourceId], + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/admin/components/Assets/JansAssetListPage.tsx(7 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
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.
📚 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/admin/components/Assets/JansAssetListPage.tsx
📚 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/admin/components/Assets/JansAssetListPage.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/admin/components/Assets/JansAssetListPage.tsx (2)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(16-257)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)
There was a problem hiding this comment.
Actionable comments posted: 5
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/admin/components/Assets/JansAssetListPage.tsx (1)
121-202: TightenmyActionseffect dependencies to match the new permission flags.Inside the
useEffectthat buildsactions, the logic now relies oncanReadAssets,canWriteAssets, andcanDeleteAssets, but the dependency array still includeshasCedarReadPermission,hasCedarWritePermission, andhasCedarDeletePermissioninstead of the derived flags.You can simplify and make intent clearer by depending on the
can*Assetsbooleans instead of the raw helpers:- useEffect(() => { + useEffect(() => { const actions: Action<Document>[] = [] // ... uses canReadAssets / canWriteAssets / canDeleteAssets ... setMyActions(actions) - }, [ - cedarPermissions, - limit, - pattern, - t, - navigateToAddPage, - navigateToEditPage, - hasCedarReadPermission, - hasCedarWritePermission, - hasCedarDeletePermission, - handleOptionsChange, - dispatch, - ]) + }, [ + cedarPermissions, + limit, + pattern, + t, + navigateToAddPage, + navigateToEditPage, + canReadAssets, + canWriteAssets, + canDeleteAssets, + handleOptionsChange, + dispatch, + ])This keeps the effect tied to the actual booleans that drive the action set.
♻️ Duplicate comments (9)
admin-ui/plugins/fido/components/Fido.tsx (1)
167-171: Past review concern has been resolved.The component now correctly wraps content with
GluuViewWrapper canShow={canReadFido}, ensuring unauthorized users see the standard "Missing required permission" message instead of an indefinite loader. This addresses the previous review comment.admin-ui/app/cedarling/utility/resources.ts (1)
39-44: Past review concern has been addressed.The
buildCedarPermissionKeyfunction correctly acceptsAdminUiFeatureResource(not a plainstring) and returns the precise template literal type`${AdminUiFeatureResource}::${CedarAction}`. This provides compile-time guarantees for valid resource IDs and permission keys as requested in the previous review.admin-ui/app/routes/Apps/Gluu/GluuDialog.tsx (1)
32-46: Past review concern has been resolved.The defensive pattern has been correctly implemented with
webhookScopesdefaulting to|| []and the authorization effect properly guarded with a length check. This ensures the dialog degrades gracefully when scopes are missing.admin-ui/app/utils/AppAuthProvider.tsx (1)
175-186: Past review concern about type annotation has been resolved.The type annotation at line 177 has been correctly updated to
{ access_token: string }, matching the runtime shape returned byfetchApiAccessToken. The single-argument call at line 175 is safe sincefetchApiAccessTokenprovides a default value (permissionTag || []) for the second parameter.admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (1)
50-64: Past review concern has been resolved.The defensive pattern has been correctly implemented with
webhookScopesdefaulting to|| []and the authorization call properly guarded with a length check. This matches the pattern in GluuDialog.tsx and ensures graceful handling when scopes are missing.admin-ui/plugins/user-management/components/UserList.tsx (1)
76-89: Apply defensive fallback pattern for consistency with codebase conventions.While the code is functional, it inconsistently omits the
|| []fallback that has been adopted across newer components (SsaListPage at line 62, AgamaListPage at line 85, GluuDialog at line 34, and GluuCommitDialog at line 52). Additionally,usersResourceIddoesn't need memoization sinceADMIN_UI_RESOURCES.Usersis a static constant.Apply this refactor to align with current codebase patterns:
- const usersResourceId = useMemo(() => ADMIN_UI_RESOURCES.Users, []) - const usersScopes = useMemo(() => CEDAR_RESOURCE_SCOPES[usersResourceId], [usersResourceId]) + const usersResourceId = ADMIN_UI_RESOURCES.Users + const usersScopes = useMemo( + () => CEDAR_RESOURCE_SCOPES[usersResourceId] || [], + [usersResourceId], + )This removes unnecessary memoization of a static string and adds the defensive
|| []safeguard.admin-ui/plugins/schema/components/Person/AttributeListPage.tsx (1)
116-129: Optional: dropuseMemofor static attribute resource constants.Since
ADMIN_UI_RESOURCESandCEDAR_RESOURCE_SCOPESare static, you can simplify this block by using plain constants instead ofuseMemo:- const attributeResourceId = useMemo(() => ADMIN_UI_RESOURCES.Attributes, []) - const attributeScopes = useMemo(() => CEDAR_RESOURCE_SCOPES[ADMIN_UI_RESOURCES.Attributes], []) + const attributeResourceId = ADMIN_UI_RESOURCES.Attributes + const attributeScopes = CEDAR_RESOURCE_SCOPES[ADMIN_UI_RESOURCES.Attributes]Behavior remains the same but reduces hook noise and makes the resource wiring a bit easier to read.
admin-ui/app/cedarling/hooks/useCedarling.ts (2)
36-54: Action inference heuristic is reasonable; consider tightening case‑insensitive setThe
executeUrlsset plusgetActionLabelFromUrlheuristic is a reasonable way to inferCedarActionfrom permission URLs. To make the special‑case matching more robust against future constant changes, you could normalize the set contents as well:- const executeUrls = new Set([SSA_ADMIN, SSA_DEVELOPER, SCIM_BULK, REVOKE_SESSION, OPENID]) + const executeUrls = new Set( + [SSA_ADMIN, SSA_DEVELOPER, SCIM_BULK, REVOKE_SESSION, OPENID].map((u) => u.toLowerCase()), + )This keeps the
executeUrls.has(lowerUrl)check reliably case‑insensitive even if new constants are not already lower‑case.
55-61: Cache key usage is sound; confirm legacy fallback strategyThe
buildAuthorizationRequest/getCachedDecisionByActionpattern correctly reusesbuildCedarPermissionKey(resourceId, action)and safely short‑circuits oncachedDecisionbefore touchingrequest, which avoids unnecessary network calls.In
getCachedPermission, the preference for the explicit'resource::read'key with a fallback topermissionsByResourceId[resourceId]is a good bridge for legacy cache entries; just ensure this fallback is intentionally kept for backward compatibility and plan to remove it once all entries are normalized to the composite key form.Also applies to: 97-114
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (19)
admin-ui/app/cedarling/constants/resourceScopes.ts(1 hunks)admin-ui/app/cedarling/hooks/useCedarling.ts(6 hunks)admin-ui/app/cedarling/utility/resources.ts(1 hunks)admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx(4 hunks)admin-ui/app/routes/Apps/Gluu/GluuDialog.tsx(3 hunks)admin-ui/app/routes/License/LicenseDetailsPage.tsx(6 hunks)admin-ui/app/utils/AppAuthProvider.tsx(2 hunks)admin-ui/plugins/admin/components/Assets/JansAssetListPage.tsx(7 hunks)admin-ui/plugins/auth-server/components/Agama/AgamaListPage.js(9 hunks)admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.js(3 hunks)admin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsx(5 hunks)admin-ui/plugins/auth-server/components/Ssa/SsaListPage.tsx(8 hunks)admin-ui/plugins/fido/components/Fido.tsx(7 hunks)admin-ui/plugins/jans-lock/components/JansLockConfiguration.tsx(2 hunks)admin-ui/plugins/saml/components/SamlIdentityList.js(8 hunks)admin-ui/plugins/saml/components/TrustRelationshipList.js(6 hunks)admin-ui/plugins/schema/components/Person/AttributeListPage.tsx(9 hunks)admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpEditPage.tsx(6 hunks)admin-ui/plugins/user-management/components/UserList.tsx(9 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
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.
📚 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/app/utils/AppAuthProvider.tsxadmin-ui/plugins/schema/components/Person/AttributeListPage.tsxadmin-ui/plugins/jans-lock/components/JansLockConfiguration.tsxadmin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsxadmin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.jsadmin-ui/plugins/user-management/components/UserList.tsxadmin-ui/plugins/saml/components/SamlIdentityList.jsadmin-ui/plugins/admin/components/Assets/JansAssetListPage.tsxadmin-ui/app/cedarling/constants/resourceScopes.tsadmin-ui/plugins/fido/components/Fido.tsxadmin-ui/plugins/auth-server/components/Agama/AgamaListPage.jsadmin-ui/app/cedarling/hooks/useCedarling.tsadmin-ui/app/routes/Apps/Gluu/GluuDialog.tsxadmin-ui/app/routes/License/LicenseDetailsPage.tsxadmin-ui/app/cedarling/utility/resources.ts
📚 Learning: 2025-11-07T12:17:39.857Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
Applied to files:
admin-ui/plugins/schema/components/Person/AttributeListPage.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.jsadmin-ui/plugins/user-management/components/UserList.tsxadmin-ui/app/cedarling/constants/resourceScopes.tsadmin-ui/app/routes/Apps/Gluu/GluuDialog.tsxadmin-ui/app/cedarling/utility/resources.ts
📚 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/jans-lock/components/JansLockConfiguration.tsxadmin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsxadmin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.jsadmin-ui/plugins/user-management/components/UserList.tsxadmin-ui/plugins/admin/components/Assets/JansAssetListPage.tsxadmin-ui/plugins/auth-server/components/Agama/AgamaListPage.jsadmin-ui/app/cedarling/hooks/useCedarling.tsadmin-ui/app/routes/Apps/Gluu/GluuDialog.tsx
📚 Learning: 2025-11-11T15:17:34.651Z
Learnt from: devrimyatar
Repo: GluuFederation/flex PR: 2440
File: flex-linux-setup/flex_linux_setup/flex_setup.py:519-519
Timestamp: 2025-11-11T15:17:34.651Z
Learning: In flex-linux-setup/flex_linux_setup/flex_setup.py, the webhook LDIF (aui_webhook.ldif) should only be imported during fresh Admin UI installations, not during updates. The import is correctly guarded by the check `client_check_result['2001.'] == -1` which ensures it only runs when the Admin UI Web Client doesn't already exist. Docker update workflows differ from Linux update workflows in this regard.
Applied to files:
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsxadmin-ui/app/routes/Apps/Gluu/GluuDialog.tsx
📚 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/components/Ssa/SsaListPage.tsx
🧬 Code graph analysis (19)
admin-ui/app/utils/AppAuthProvider.tsx (1)
admin-ui/app/redux/api/backend-api.ts (2)
fetchApiAccessToken(72-83)fetchPolicyStore(95-98)
admin-ui/plugins/schema/components/Person/AttributeListPage.tsx (3)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)
admin-ui/plugins/jans-lock/components/JansLockConfiguration.tsx (3)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (2)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)
admin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsx (3)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.js (3)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)
admin-ui/plugins/user-management/components/UserList.tsx (3)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)
admin-ui/plugins/saml/components/SamlIdentityList.js (3)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)
admin-ui/plugins/admin/components/Assets/JansAssetListPage.tsx (3)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)
admin-ui/app/cedarling/constants/resourceScopes.ts (3)
admin-ui/app/cedarling/types/index.ts (2)
AdminUiFeatureResource(198-225)ResourceScopeEntry(88-91)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/utils/PermChecker.ts (72)
STAT_READ(106-106)STAT_JANS_READ(107-107)LICENSE_DETAILS_READ(33-33)LICENSE_DETAILS_WRITE(35-35)ASSETS_READ(131-131)ASSETS_WRITE(132-132)ASSETS_DELETE(133-133)WEBHOOK_READ(128-128)WEBHOOK_WRITE(129-129)WEBHOOK_DELETE(130-130)SCRIPT_READ(61-61)SCRIPT_WRITE(62-62)SCRIPT_DELETE(63-63)CACHE_READ(88-88)CACHE_WRITE(89-89)CACHE_DELETE(90-90)PERSISTENCE_DETAIL(104-104)SQL_READ(100-100)SQL_WRITE(101-101)SQL_DELETE(102-102)LDAP_READ(92-92)LDAP_WRITE(93-93)LDAP_DELETE(94-94)JANS_LOCK_READ(85-85)JANS_LOCK_WRITE(86-86)FIDO_READ(81-81)FIDO_WRITE(82-82)FIDO_DELETE(83-83)SMTP_READ(65-65)SMTP_WRITE(66-66)SMTP_DELETE(67-67)SCIM_CONFIG_READ(123-123)SCIM_CONFIG_WRITE(124-124)USER_READ(109-109)USER_WRITE(110-110)USER_DELETE(111-111)SAML_READ(51-51)SAML_WRITE(52-52)SAML_DELETE(53-53)SAML_TR_READ(55-55)SAML_TR_WRITE(56-56)SAML_CONFIG_READ(58-58)SAML_CONFIG_WRITE(59-59)ATTRIBUTE_READ(7-7)ATTRIBUTE_WRITE(8-8)ATTRIBUTE_DELETE(9-9)SSA_PORTAL(41-41)SSA_ADMIN(42-42)SSA_DELETE(43-43)SCOPE_READ(37-37)SCOPE_WRITE(38-38)SCOPE_DELETE(39-39)SESSION_READ(117-117)SESSION_DELETE(118-118)ACR_READ(69-69)ACR_WRITE(70-70)AGAMA_READ(113-113)AGAMA_WRITE(114-114)AGAMA_DELETE(115-115)CLIENT_READ(11-11)CLIENT_WRITE(12-12)CLIENT_DELETE(13-13)PROPERTIES_WRITE(4-4)API_CONFIG_READ(135-135)API_CONFIG_WRITE(136-136)LOGGING_READ(73-73)LOGGING_WRITE(74-74)JWKS_READ(77-77)JWKS_WRITE(78-78)ROLE_READ(15-15)PERMISSION_READ(19-20)MAPPING_READ(26-27)
admin-ui/plugins/fido/components/Fido.tsx (3)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)
admin-ui/plugins/auth-server/components/Agama/AgamaListPage.js (3)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)
admin-ui/app/cedarling/hooks/useCedarling.ts (5)
admin-ui/app/cedarling/types/index.ts (5)
UseCedarlingReturn(93-102)CedarAction(229-229)AdminUiFeatureResource(198-225)ResourceScopeEntry(88-91)AuthorizationResult(54-58)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDARLING_CONSTANTS(215-218)admin-ui/app/utils/PermChecker.ts (5)
SSA_ADMIN(42-42)SSA_DEVELOPER(44-44)SCIM_BULK(49-49)REVOKE_SESSION(46-46)OPENID(47-47)admin-ui/app/cedarling/utility/resources.ts (1)
buildCedarPermissionKey(39-44)admin-ui/app/cedarling/client/CedarlingClient.ts (1)
cedarlingClient(56-59)
admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpEditPage.tsx (4)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)admin-ui/plugins/smtp-management/redux/types/SmtpApi.type.ts (1)
SmtpTest(18-22)
admin-ui/app/routes/Apps/Gluu/GluuDialog.tsx (3)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)
admin-ui/app/routes/License/LicenseDetailsPage.tsx (5)
admin-ui/app/redux/features/licenseDetailsSlice.ts (1)
LicenseDetailsState(21-24)admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/routes/License/types.ts (1)
LicenseField(1-5)admin-ui/app/utils/Util.ts (1)
formatDate(53-64)
admin-ui/app/cedarling/utility/resources.ts (2)
admin-ui/app/cedarling/types/index.ts (3)
AdminUiFeatureResource(198-225)ApiPermissionType(227-227)CedarAction(229-229)admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (1)
apiPermissions(126-126)
admin-ui/plugins/saml/components/TrustRelationshipList.js (5)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/plugins/saml/components/SamlIdentityList.js (10)
useCedarling(40-45)theme(51-51)selectedTheme(52-52)themeColors(53-53)samlResourceId(61-61)samlScopes(62-62)dispatch(58-58)tableActions(169-233)handleGoToEditPage(86-88)handleGoToAddPage(90-92)admin-ui/plugins/user-management/types/CommonTypes.ts (1)
ThemeContext(13-17)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)
admin-ui/plugins/auth-server/components/Ssa/SsaListPage.tsx (3)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(17-260)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(79-213)
🔇 Additional comments (14)
admin-ui/plugins/auth-server/components/Ssa/SsaListPage.tsx (1)
61-79: LGTM! Defensive pattern correctly implemented.The Cedar permission wiring correctly uses the defensive
|| []fallback forssaScopesand properly guards theauthorizeHelpercall. This aligns with the pattern established across the codebase.admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpEditPage.tsx (1)
94-108: Cedar-based SMTP read/write gating and readOnly wiring look correctThe SMTP page now cleanly uses
hasCedarReadPermission/hasCedarWritePermissionfor the SMTP resource, pre-warms permissions viaauthorizeHelper(smtpScopes), gateshandleSubmitandhandleTestSmtpwith an earlyif (!canWriteSmtp) return, and drives bothGluuViewWrapper canShow={canReadSmtp}andreadOnly={!canWriteSmtp}onSmtpForm. This aligns with the Cedar patterns used elsewhere and should correctly prevent unauthorized writes while still showing the UI for authorized readers.Also applies to: 149-191, 198-219
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.js (1)
26-45: Logging Cedar permissions now use strict booleans and correctly gate data fetch & UI
canReadLogging/canWriteLoggingare now strict booleans,authorizeHelper(loggingScopes)is called once with a scopes-length guard, andgetLoggingConfig()is dispatched only whencanReadLogging === true. The view and commit footer are gated by these flags, which aligns with the intended Cedar pattern and removes the previous cedarPermissions no-op wiring.Also applies to: 51-62, 116-121, 206-218
admin-ui/app/routes/License/LicenseDetailsPage.tsx (1)
25-48: License Cedar write gating and typed license fields look goodThe license details page now:
- Uses a typed selector (
LicenseDetailsState) for{ item, loading }.- Derives
licenseResourceId/licenseScopesand pre-warms Cedar viaauthorizeHelper(licenseScopes).- Computes
canWriteLicensefromhasCedarWritePermission(licenseResourceId)and uses it solely to gate the reset button and commit dialog.- Structures
licenseFieldsas a typedLicenseField[]and centralizes rendering throughrenderLicenseField, including formatted validity dates viaformatDate.This is a clean, type-safe integration of Cedar write permissions without altering the existing read behavior.
Also applies to: 67-99, 144-168
admin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsx (1)
33-35: Session delete/revoke flows are correctly gated by Cedar delete permissionThe sessions list now derives
sessionResourceId/sessionScopes, pre-warms Cedar withauthorizeHelper(sessionScopes)(guarded on scopes length), and useshasCedarDeletePermission(sessionResourceId)to drivecanDeleteSession. That flag:
- Disables the delete action via
disabled: !canDeleteSession,- Controls whether the delete action is added to
myActions, and- Gates the revoke-username UI (
{canDeleteSession && (...)}).This ensures delete and revoke operations are only available when Cedar has explicitly granted the delete permission, while leaving the base listing behavior unchanged.
Also applies to: 54-56, 172-181, 183-188, 203-225, 555-595
admin-ui/plugins/saml/components/SamlIdentityList.js (1)
40-45: Cedar-based permission wiring and action gating look consistent.Using
samlResourceId/samlScopes,authorizeHelper, and the derivedcanReadIdentities/canWriteIdentities/canDeleteIdentitiesflags aligns with the new resource-scoped model. Table actions, view wrapper (canShow={canReadIdentities}), and delete dialog gating bycanDeleteIdentitiesare all coherent with how other Cedar-aware pages are structured.Also applies to: 61-80, 169-223, 237-274
admin-ui/plugins/admin/components/Assets/JansAssetListPage.tsx (1)
30-43: Assets page Cedar integration and gating look correct.
assetsResourceId/assetScopeswiring,authorizeHelper(assetScopes)in the init effect, and the derivedcanReadAssets/canWriteAssets/canDeleteAssetsbooleans are consistent with the new Cedar model. UsingcanReadAssetsforGluuViewWrapperand gating actions withcan*Assetsis a good improvement over the previous ad‑hoc checks.Also applies to: 96-107, 108-119, 155-187, 271-331
admin-ui/plugins/saml/components/TrustRelationshipList.js (1)
20-27: TrustRelationship Cedar integration and action list refactor look solid.
samlResourceId/samlScopesandauthorizeHelper(samlScopes)correctly initialize Cedar permissions for SAML.canReadTrustRelationships/canWriteTrustRelationshipsare used consistently to gate fetching, table actions, view wrapper, and the delete dialog.- The rewritten
tableActionsnow pushes each action in separate calls, eliminating the earlier nestedactions.pushbug and keeping the array clean.Also applies to: 40-60, 82-127, 131-163
admin-ui/app/cedarling/constants/resourceScopes.ts (2)
1-213: Resource–scope map is well-typed and complete.
CEDAR_RESOURCE_SCOPESnow usesRecord<AdminUiFeatureResource, ResourceScopeEntry[]>with exhaustive entries for allADMIN_UI_RESOURCESkeys, and each resource is mapped to the expected permission constants. This addresses the earlier risk of missing mappings returningundefinedat runtime and gives a single, type-safe source of truth for Cedar resource scopes.
215-218: CEDARLING_CONSTANTS definition matches hook usage.Defining
ACTION_TYPEandRESOURCE_TYPEhere and consuming them viaCEDARLING_CONSTANTSinuseCedarlingkeeps Cedar action/resource identifiers centralized and easy to update.admin-ui/plugins/schema/components/Person/AttributeListPage.tsx (1)
61-68: Attribute list Cedar permissions and action gating are consistent with the new model.
attributeResourceId/attributeScopescorrectly point atADMIN_UI_RESOURCES.Attributesand its scopes.authorizeHelper(attributeScopes)is invoked once to populate Cedar decisions.canRead/Write/DeleteAttributesare used coherently to:
- gate the view (
GluuViewWrapper canShow={canReadAttributes}),- control which row actions are available in
attributeActions, and- guard the delete dialog via
canDeleteAttributes.This keeps attribute listing, editing, and deletion aligned with Cedarling-driven authorization.
Also applies to: 91-99, 116-129, 223-278, 367-501
admin-ui/app/cedarling/hooks/useCedarling.ts (3)
3-35: Redux wiring for Cedarling permissions looks consistentImports, token extraction from
authReducer, and thecedarPermissionsselector (includingpermissions,initialized, andisInitializing) are consistent with the new Cedarling-centric state model; no issues spotted here.
199-221:authorizeHelperfan‑out logic is correct and resilientThe
authorizeHelperutility cleanly composes multipleauthorizecalls, short‑circuits on an empty input array, and usesPromise.allSettledto ensure one failing authorization does not prevent others from returning structuredAuthorizationResults. The mapping of rejected promises into{ isAuthorized: false, error: ... }keeps the return type consistent for callers.
116-123: I need to verify one more detail: whether the line 204mappattern is wrapped byauthorizeHelper, and whether any code actually passes multi-element arrays toauthorize().
The
authorizefunction accepts an array by design but intentionally processes only the first element. All verified call sites in the codebase already follow this pattern correctly:
- Single-element calls:
UiPermListPage.js,MappingPage.js,MappingItem.js,UiRoleListPage.js,GluuAppSidebar.tsx- Multi-entry fan-out:
useCedarling.ts:204usesresourceScopes.map((entry) => authorize([entry])), wrapping each entry individuallyThe API design is intentional. There are no silent failures or ignored entries—callers already pass single-element arrays or fan-out multi-entry checks via the
mappattern, which pairs withauthorizeHelperfor aggregated results (as mentioned in your original review). No changes are required.Likely an incorrect or invalid review comment.
|
…tion/flex into 2321-cedarling-integration
There was a problem hiding this comment.
Actionable comments posted: 4
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/auth-server/plugin-metadata.js (1)
220-223: Add missing resourceKey to ReportPage route.The ReportPage route at lines 220-223 lacks a
resourceKeyfield, while all 17 other routes in this file include one. TheReportsconstant does not exist inADMIN_UI_RESOURCES(defined inadmin-ui/app/cedarling/utility/resources.ts). You must either:
- Add a new
Reportsconstant toADMIN_UI_RESOURCESand useresourceKey: ADMIN_UI_RESOURCES.Reports, or- Use an existing appropriate constant such as
ADMIN_UI_RESOURCES.Dashboard, or- Use
CEDARLING_BYPASSif this route should bypass Cedarling authorization checks.admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx (1)
103-125: Simplify change‑diff construction and avoid heavyunknowncasting aroundeditLoggingConfigThe
mergedValues/changedFieldsResultlogic is correct, but the series ofas unknown as Record<string, unknown>casts and the manual remap intochangedFields: Record<string, { oldValue; newValue }>add noise and weaken type safety. Similarly, retypingeditLoggingConfigviaas unknown as (payload: EditLoggingPayload) => { … }sidelines the slice’s actual typing.Two suggestions:
Narrow the typings around diffing so you don’t need
unknown:
- Introduce a helper alias like
type LoggingRecord = Record<keyof Logging, unknown>inutils.ts.- Call
getMergedValues<LoggingRecord>/getChangedFields<LoggingRecord>and keep the return type, instead of down‑ and up‑casting throughunknownand rebuilding the map withObject.keys+ null checks.Fix the action creator typing at the source:
- Update the
editLoggingConfigcreator inloggingSliceso its payload type matches{ data: Record<string, string>; otherFields: { userMessage: string; changedFields: … } }.- Then you can call
dispatch(editLoggingConfig({ data: opts, otherFields: { userMessage, changedFields } }))directly without theas unknown asoverride.This will make the submit path more readable and prevent future payload shape mistakes from slipping past the compiler.
Also applies to: 138-159
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx(4 hunks)admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx(6 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.tsx(1 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.tsx(1 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkActions.ts(1 hunks)admin-ui/plugins/auth-server/plugin-metadata.js(5 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
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.
📚 Learning: 2025-11-07T12:17:39.857Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsxadmin-ui/plugins/auth-server/plugin-metadata.js
📚 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/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsxadmin-ui/plugins/auth-server/plugin-metadata.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/components/Configuration/Defaults/LoggingPage.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
📚 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/plugin-metadata.js
🧬 Code graph analysis (3)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (1)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/configApiTypes.ts (1)
JsonPatch(3-3)
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx (1)
admin-ui/plugins/auth-server/components/Configuration/Defaults/utils.ts (4)
LOG_LEVELS(1-1)LOG_LAYOUTS(2-2)getMergedValues(49-54)getChangedFields(56-69)
admin-ui/plugins/auth-server/plugin-metadata.js (1)
admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)
🔇 Additional comments (11)
admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkActions.ts (1)
5-15: Question: Why was JSDoc removed in a Cedarling integration PR?The AI summary indicates that JSDoc comments describing the
useJwkActionshook were removed, but no functional changes were made to this file. This documentation removal seems unrelated to the PR's objective of integrating Cedarling flow.Removing documentation generally reduces code maintainability. If this is incidental cleanup, consider:
- Keeping the documentation unless it's outdated or incorrect
- Moving documentation-only changes to a separate cleanup PR
Could you clarify the rationale for removing the JSDoc in this PR?
admin-ui/plugins/auth-server/plugin-metadata.js (2)
61-61: LGTM: Import enables resource-scoped authorization.The import of
ADMIN_UI_RESOURCEScorrectly supports the new Cedarling-based permission checks throughout the plugin metadata.
84-136: Excellent: Resource keys are consistently applied, and previous feedback addressed.The
resourceKeyfields have been systematically added to all menu items and routes, correctly mapping them to their correspondingADMIN_UI_RESOURCESconstants. Notably, the Agama route at line 228 now includesresourceKey: ADMIN_UI_RESOURCES.Authentication, which addresses the previous review comment about consistency with other Authentication-related routes.Also applies to: 151-252
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.tsx (1)
61-61: Documentation cleanup - no functional changes.The removal of the explanatory comment is a minor documentation cleanup with no impact on functionality. The component implementation remains unchanged.
admin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.tsx (3)
1-11: Cedarling authorization imports added correctly.The new imports for React hooks (
useMemo,useEffect) and Cedarling utilities are appropriate for the authorization integration. The imports follow consistent patterns seen across the PR's Cedarling migration.
32-44: No issues found - GluuViewWrapper correctly handles unauthorized access.The verification confirms that
GluuViewWrapperappropriately handles the unauthorized case. WhencanShowis false or null, it renders a localized permission error message (t('messages.missing_required_permission')) withrole="alert"for accessibility, rather than a blank page. The implementation in KeysPage.tsx correctly uses this wrapper to gate access based on thecanReadKeyspermission.
13-27: No issues found—authorization logic is properly optimized.Verification confirms that
authorizeHelperis correctly wrapped withuseCallbackand has appropriate dependencies:[authorize, getActionLabelFromUrl]. This ensures the function remains stable across renders, preventing unnecessary re-executions of theuseEffectin KeysPage. The dependency array in the component'suseEffectis correct.admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (3)
1-1: LGTM: Import additions support the Cedarling integration.The added imports (
useMemo,CEDAR_RESOURCE_SCOPES,ADMIN_UI_RESOURCES) are all utilized in the new authorization flow.Also applies to: 7-8
45-48: No changes required - memoization is correctly implemented.Verification confirms that
hasCedarWritePermissionis properly wrapped inuseCallbackwith appropriate dependencies in theuseCedarlinghook. The function is stable across renders, andconfigApiResourceIdis a constant reference. The memoization setup inApiConfigForm.tsx(lines 45–48) is correct and will effectively prevent unnecessary re-computations.
98-105: Authorization checks verified—no issues found.All resource and scope definitions are properly configured:
ADMIN_UI_RESOURCES.ConfigApiConfigurationis defined ingit/admin-ui/app/cedarling/utility/resources.tsCEDAR_RESOURCE_SCOPEShas the correct entry with bothAPI_CONFIG_READandAPI_CONFIG_WRITEpermissionscanWriteConfigApicorrectly useshasCedarWritePermission(configApiResourceId)to control the rendering of bothGluuCommitFooterandGluuCommitDialogThe conditional rendering implementation is sound and consistent with the authorization framework.
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx (1)
50-71: Cedarling permission wiring and gated fetch look solidThe
loggingResourceId+loggingScopessetup,authorizeHelper(loggingScopes)effect, and thecanReadLogging/canWriteLoggingflags (used both to gategetLoggingConfig()and to driveGluuViewWrapper/ footer visibility) form a clean, deterministic authorization flow and address earlier feedback about fetching only when read access is granted. No changes needed here.Also applies to: 76-87, 166-170
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
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/auth-server/components/Configuration/Defaults/LoggingPage.tsx (1)
95-122: changedFields computation is correct but can be simplifiedFunctionally this is fine—
getMergedValuesfollowed bygetChangedFieldsagainstlocalLogginggives you a precise diff for audit. The extrachangedFieldsrebuild viaObject.keys(changedFieldsResult)is redundant, sincegetChangedFieldsalready omits unchanged fields; you could instead castchangedFieldsResultto the concreteRecord<string, { oldValue; newValue }>type to avoid the second loop and branch.
♻️ Duplicate comments (2)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (2)
50-54: Authorization initialization pattern is correct.The guard condition
if (configApiScopes && configApiScopes.length > 0)(line 51) ensuresauthorizeHelperis only called when scopes are available, which matches the pattern used in other components and prevents unnecessary calls with empty arrays.
39-43: Consider optimizing constant resource ID usage.The
configApiResourceIdis derived from a constant and never changes, so including it in theuseMemodependency array (line 42) causes unnecessary dependency tracking. Consider movingconfigApiResourceIdto module scope as a constant.Apply this diff to move the constant outside the component:
+const CONFIG_API_RESOURCE_ID = ADMIN_UI_RESOURCES.ConfigApiConfiguration + const ApiConfigForm: React.FC<ApiConfigFormProps> = ({ configuration, onSubmit }) => { const { authorizeHelper, hasCedarWritePermission } = useCedarling() const navigate = useNavigate() const [modal, setModal] = useState(false) const [patches, setPatches] = useState<JsonPatch[]>([]) const operations = patches - const configApiResourceId = ADMIN_UI_RESOURCES.ConfigApiConfiguration const configApiScopes = useMemo( - () => CEDAR_RESOURCE_SCOPES[configApiResourceId] || [], - [configApiResourceId], + () => CEDAR_RESOURCE_SCOPES[CONFIG_API_RESOURCE_ID] || [], + [], ) const canWriteConfigApi = useMemo( - () => hasCedarWritePermission(configApiResourceId), - [hasCedarWritePermission, configApiResourceId], + () => hasCedarWritePermission(CONFIG_API_RESOURCE_ID), + [hasCedarWritePermission], )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx(4 hunks)admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx(6 hunks)admin-ui/plugins/auth-server/redux/features/types/loggingTypes.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
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.
📚 Learning: 2025-11-07T12:17:39.857Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
📚 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/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
📚 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/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
📚 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/components/Configuration/Defaults/LoggingPage.tsx
📚 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/components/Configuration/Defaults/LoggingPage.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx (2)
admin-ui/plugins/auth-server/redux/features/types/loggingTypes.ts (1)
RootState(10-12)admin-ui/plugins/auth-server/components/Configuration/Defaults/utils.ts (4)
LOG_LEVELS(1-1)LOG_LAYOUTS(2-2)getMergedValues(49-54)getChangedFields(56-69)
🔇 Additional comments (5)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (1)
100-116: LGTM! Conditional rendering based on write permissions.The footer and dialog components are correctly gated by
canWriteConfigApi, ensuring only authorized users can access write operations. The initial undefined state properly hides these components until authorization completes.admin-ui/plugins/auth-server/redux/features/types/loggingTypes.ts (2)
1-1: LGTM!The type-only import is appropriate and follows TypeScript best practices.
4-7: LGTM!The interface follows standard Redux state patterns with appropriate nullable typing for the logging configuration and a loading flag for async operations.
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx (2)
33-45: Good use of canonicalRootStateand Redux wiringImporting
RootStatefromloggingTypesand using typeduseSelectorforloggingandloadingkeeps this page aligned with the slice/store definition and addresses the previous drift risk from localRootStatedeclarations.
42-79: Cedar permission gating and logging fetch flow look solidDeriving
canReadLogging/canWriteLoggingas strict booleans and splitting permission initialization (authorizeHelper(loggingScopes)) from thegetLoggingConfigfetch gated oncanReadLoggingmakes the flow clearer and ensures the API is only called when the user has read access, matching the Cedarling patterns used elsewhere.
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
Show resolved
Hide resolved
admin-ui/plugins/auth-server/redux/features/types/loggingTypes.ts
Outdated
Show resolved
Hide resolved
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/auth-server/components/Configuration/ConfigPage.js (1)
84-84: Remove dead code.This
useEffecthas an empty body and serves no purpose.Apply this diff:
- useEffect(() => {}, [cedarPermissions])
♻️ Duplicate comments (2)
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx (1)
256-266: Add explicitonBackhandler to navigate to auth-server configuration context.The
GluuFormFootercurrently hasshowBack={true}but noonBackhandler, so it defaults tonavigateToHome()(dashboard). Based on recent learnings (PRs #2418, #2415), the Back button should navigate to the appropriate context—in this case, the auth-server configuration area—rather than the dashboard. This keeps users within the auth-server plugin context when they dismiss the form.Add an
onBackprop toGluuFormFooter(line 256-266) with a handler that navigates to the relevant auth-server configuration landing point instead of the default dashboard.admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (1)
34-48: Avoid memoizinghasCedarWritePermissionresult to prevent stale permissions.
canWriteConfigApiis memoized fromhasCedarWritePermission(configApiResourceId):const canWriteConfigApi = useMemo( () => hasCedarWritePermission(configApiResourceId), [hasCedarWritePermission, configApiResourceId], )If
hasCedarWritePermissionis a stable callback from the Cedarling context (common pattern), its identity will not change when authorization state updates, so thisuseMemomay cache the initial permission and never recompute, causing the footer and dialog (gated bycanWriteConfigApi) to stay hidden even after authorization completes.Given the low cost of the check and the importance of correct permission gating, it’s safer to drop
useMemohere and compute on each render:- const canWriteConfigApi = useMemo( - () => hasCedarWritePermission(configApiResourceId), - [hasCedarWritePermission, configApiResourceId], - ) + const canWriteConfigApi = hasCedarWritePermission(configApiResourceId)Optionally (as previously suggested), you can also hoist the resource ID to a module‑level constant to avoid recreating it and simplify memo dependencies, for example:
// At module scope const CONFIG_API_RESOURCE_ID = ADMIN_UI_RESOURCES.ConfigApiConfiguration // Inside the component const configApiScopes = useMemo( () => CEDAR_RESOURCE_SCOPES[CONFIG_API_RESOURCE_ID] || [], [], ) const canWriteConfigApi = hasCedarWritePermission(CONFIG_API_RESOURCE_ID)Also applies to: 76-111
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
admin-ui/app/helpers/index.js(1 hunks)admin-ui/app/helpers/navigation.ts(1 hunks)admin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsx(2 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx(5 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js(6 hunks)admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx(6 hunks)admin-ui/plugins/auth-server/redux/features/types/loggingTypes.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
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.
📚 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/components/Configuration/ConfigPage.jsadmin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
📚 Learning: 2025-11-07T12:17:39.857Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/ConfigPage.jsadmin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
📚 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/components/Configuration/ConfigPage.jsadmin-ui/app/helpers/navigation.tsadmin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/ConfigPage.jsadmin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
📚 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/components/Configuration/ConfigPage.jsadmin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
📚 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/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsxadmin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
🧬 Code graph analysis (5)
admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (4)
admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(16-268)admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(8-21)admin-ui/app/cedarling/utility/resources.ts (1)
ADMIN_UI_RESOURCES(5-33)admin-ui/app/cedarling/constants/resourceScopes.ts (1)
CEDAR_RESOURCE_SCOPES(80-215)
admin-ui/app/helpers/navigation.ts (2)
admin-ui/plugins/auth-server/components/Configuration/Keys/constants.ts (1)
ROUTES(3-5)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
useAppNavigation(29-29)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (2)
admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (2)
useCedarling(28-28)useAppNavigation(29-29)admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(8-21)
admin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsx (2)
admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(8-21)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
useAppNavigation(29-29)
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx (2)
admin-ui/plugins/auth-server/redux/features/types/loggingTypes.ts (2)
LoggingModuleRootState(10-12)EditLoggingPayload(15-21)admin-ui/plugins/auth-server/components/Configuration/Defaults/utils.ts (4)
LOG_LEVELS(1-1)LOG_LAYOUTS(2-2)getMergedValues(49-54)getChangedFields(56-69)
🔇 Additional comments (9)
admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (3)
1-1: LGTM! Clean migration to Cedarling resource-scoped authorization.The refactoring correctly:
- Derives
propertiesResourceIdfromADMIN_UI_RESOURCES.AuthenticationServerConfiguration- Looks up scopes from
CEDAR_RESOURCE_SCOPES- Computes
canWritePropertiesviahasCedarWritePermission- Memoizes derived values with appropriate dependencies
This pattern aligns with the broader Cedarling integration across the Admin UI.
Also applies to: 11-14, 25-25, 28-29, 41-50
69-70: LGTM! Batch authorization implemented correctly.The
authorizeHelpercall properly checks permissions forpropertiesScopes, and the dependency array is accurate.
139-139: LGTM! Navigation and rendering guards updated correctly.The Back button now navigates to the home dashboard via
navigateToHome(), which is appropriate for a configuration page. The rendering guards forGluuCommitFooterandGluuCommitDialognow correctly usecanWritePropertiesderived from the Cedarling permission check.Also applies to: 216-223, 227-234
admin-ui/plugins/auth-server/redux/features/types/loggingTypes.ts (1)
1-21: LGTM! Clean type definitions that support the Redux-based logging flow.The type definitions are well-structured and the naming (especially
LoggingModuleRootState) properly scopes them to the logging feature, avoiding confusion with the global Redux root state. TheEditLoggingPayloadinterface correctly matches the payload structure used in the LoggingPage component.admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx (1)
44-79: LGTM! Cedar permissions and data fetching are properly implemented.The permission flags are correctly coerced to strict booleans with
!!, and the effects are properly split with the data fetch (getLoggingConfig) gated behind thecanReadLoggingpermission. This ensures users without read access won't trigger unnecessary API calls.admin-ui/app/helpers/navigation.ts (1)
1-21: Centralized navigation helper is correctly implemented.
useAppNavigationcleanly wrapsuseNavigate, memoizesnavigateToHome, and centralizes the/home/dashboardroute for reuse across the app.admin-ui/app/helpers/index.js (1)
1-2: Barrel export for navigation helpers is appropriate.Re-exporting
./navigationfrom the helpers index simplifies imports and keeps navigation utilities discoverable alongside other helpers.admin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsx (1)
7-81: Back button behavior now correctly routes via centralized navigation.Using
useAppNavigation().navigateToHomeas the fallback whenonBackis absent ensures Gluu form footers consistently go to/home/dashboardinstead of relying on history, aligning with the documented requirement for this component. Based on learningsadmin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (1)
50-55: Guarded authorization effect matches the Cedarling usage pattern.Wrapping
authorizeHelper(configApiScopes)in a length check ensures the helper is only invoked when there are scopes to authorize and aligns this component with the pattern used elsewhere.
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx(5 hunks)admin-ui/plugins/auth-server/components/Configuration/Defaults/utils.ts(2 hunks)admin-ui/plugins/auth-server/redux/features/loggingSlice.js(1 hunks)admin-ui/plugins/auth-server/redux/features/types/loggingTypes.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
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.
📚 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/components/Configuration/Defaults/LoggingPage.tsx
📚 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/components/Configuration/Defaults/LoggingPage.tsx
📚 Learning: 2025-11-07T12:17:39.857Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
📚 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/components/Configuration/Defaults/LoggingPage.tsx
📚 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/components/Configuration/Defaults/LoggingPage.tsx
🧬 Code graph analysis (2)
admin-ui/plugins/auth-server/components/Configuration/Defaults/utils.ts (1)
admin-ui/plugins/auth-server/redux/features/types/loggingTypes.ts (1)
ChangedFields(3-5)
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx (2)
admin-ui/plugins/auth-server/redux/features/types/loggingTypes.ts (3)
ChangedFields(3-5)LoggingModuleRootState(14-16)EditLoggingPayload(18-24)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (2)
useCedarling(28-28)dispatch(35-35)
🔇 Additional comments (3)
admin-ui/plugins/auth-server/redux/features/loggingSlice.js (1)
22-24: Reducer signature change is safe and improves action compatibilityAccepting an
_actionparameter while keeping the body the same preserves behavior and makes the slice action creator compatible with payload‑carrying calls from sagas/components. Underscoring the unused param keeps lint noise down.admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx (2)
45-81: Permission gating and fetch flow look solidDeriving
loggingResourceIdfromADMIN_UI_RESOURCES, computingcanReadLogging/canWriteLoggingas strict booleans, callingauthorizeHelper(loggingScopes)once, and only dispatchinggetLoggingConfig()whencanReadLoggingis true is a clean pattern. It avoids unnecessary API calls for unauthorized users and keeps the view/commit controls neatly permission‑gated.
97-132: Commit payload assembly is now strongly typed and guardedUsing the
localLoggingnull‑check,getMergedValues+getChangedFields, and a typedEditLoggingPayloadforeditLoggingConfigremoves the brittle casts from earlier iterations and makes the audit payload shape explicit. ThependingValues+ commit dialog flow also keeps side effects well‑contained.
admin-ui/plugins/auth-server/components/Configuration/Defaults/LoggingPage.tsx
Show resolved
Hide resolved
5 participants
close #2321
Summary by CodeRabbit
New Features
Improvements
Localization
✏️ Tip: You can customize this high-level summary in your review settings.