fix(admin-ui): prevent crashes in Aliases, Settings and Built-In authentication components #2597
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…defined loading state (#2580)
Contributor
📝 WalkthroughWalkthroughRefactors Agama alias UI to a Formik modal flow, adds ACR mapping helpers and validation, enhances JSON-config audit enrichment in the saga, adds localization keys, and updates AuthN/AUTH UI/types and slice state for save error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User (UI)"
participant UI as "AgamaAliasListPage"
participant Saga as "JsonConfigSaga"
participant API as "JansConfigApi"
participant Audit as "auditHelpers.enhanceJsonConfigAuditPayload"
participant Store as "Redux Store / Toast"
User->>UI: Add/Edit/Delete ACR mapping (via modal)
UI->>Saga: dispatch patchJsonConfig(actionPayload)
Saga->>API: call patchJsonConfig API (payload)
API-->>Saga: returns success/error
Saga->>Audit: enhanceJsonConfigAuditPayload(payload, resource)
Audit-->>Saga: enhancedPayload (with action_data/deletedMapping)
Saga->>Store: addAdditionalData(enhancedPayload, actionType)
Saga->>Store: postUserAction / updateToast / navigate
Store-->>User: toast / audit recorded / navigation result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mo-auto
added
comp-admin-ui
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@admin-ui/plugins/auth-server/components/Agama/AgamaAliasListPage.tsx`:
- Around line 291-305: The custom pagination in tableComponents is
non-functional because TablePagination uses page={0} and no-op handlers; either
remove the Pagination override so MaterialTable handles paging automatically, or
implement real pagination state: add React state (e.g., page and rowsPerPage)
and handlers (onPageChange, onRowsPerPageChange) and use those values in
TablePagination and to slice listData (or pass paginated data into the table);
update references to tableComponents, TablePagination, DEFAULT_PAGE_SIZE and
listData to wire the new state/handlers into the component.
- Around line 88-122: The catch block in handleSubmitCallback only
console.errors the exception and gives no user feedback; update the catch to
surface a user-facing error (use the app's notification/toast utility, e.g.,
showToast/notifyError) and include the error message, and do not clear UI state
(setIsEdit, setSelectedRow, setShowAddModal) on failure; also consider recording
a failed audit via logAcrMappingUpdate or dispatching an error action with
dispatch(patchJsonConfig) so failures are tracked. Ensure the notification shows
a clear message like "Error submitting ACR mapping" plus error details and keep
console.error for debugging.
- Around line 162-169: The use of setTimeout(..., 0) in handleCancel is a
fragile microtask workaround; remove the timeout and call setShowAddModal(false)
directly after the other state updates (formik.resetForm, setIsEdit(false),
setSelectedRow(null)). If you truly need to defer closing to the next tick,
replace setTimeout with a microtask via Promise.resolve().then(() =>
setShowAddModal(false)) or use ReactDOM.flushSync/startTransition explicitly and
add a short comment explaining why; update the handleCancel function (reference:
handleCancel, formik.resetForm, setIsEdit, setSelectedRow, setShowAddModal)
accordingly.
- Around line 106-107: The dispatched patchJsonConfig result is not awaited so
subsequent audit logging and modal closing may run before the patch completes;
update the handler that calls buildPayload(userAction, 'changes',
toActionData(postBody)) followed by dispatch(patchJsonConfig({ action:
userAction })) to await the dispatched async thunk
(dispatch(patchJsonConfig(...))) and handle success/failure (e.g., try/catch or
checking the returned fulfilled/rejected result) before running the audit log
and closing the modal so those actions only occur on successful completion.
In `@admin-ui/plugins/auth-server/components/Agama/helper/utils.ts`:
- Around line 12-15: Remove the redundant type cast in the mapping operation:
inside the Object.entries(acrMappings).map callback (the arrow function
producing objects with mapping and source), replace the unnecessary "value as
string" with just "value" so the returned object uses source: value; this keeps
types intact when acrMappings is a Record<string, string> and eliminates the
extraneous cast in the function handling entries.
- Around line 33-46: buildAcrMappingDeletePayload currently duplicates
buildAcrMappingPayload; either consolidate them into a single helper or make
delete semantics explicit: update buildAcrMappingDeletePayload to return an
AgamaJsonPatchRequestBody that uses op: 'remove' with path '/acrMappings' when
mappings is empty (or when you detect deletion intent), otherwise use 'replace'
(or 'add' if existingMappings is undefined) — alternatively delete this function
and call buildAcrMappingPayload from callers, keeping only one function
(buildAcrMappingPayload) that documents and handles add/replace/remove behavior
for '/acrMappings'.
- Around line 73-75: The function toActionData currently force-casts
AgamaJsonPatchRequestBody to ActionData which is unsafe because
AgamaJsonPatchRequestBody contains a requestBody: AgamaJsonPatch[] structure
while ActionData expects primitives; update toActionData to either (a)
map/extract primitive values from payload.requestBody into the exact ActionData
shape (iterate payload.requestBody entries, convert/flatten op/path/value to the
required primitive fields) or (b) expand/adjust the ActionData type to include
the AgamaJsonPatch[] shape if that is intended; ensure you remove the "as
ActionData" assertion and perform a proper transformation/validation inside
toActionData so callers receive a correctly typed object.
In `@admin-ui/plugins/auth-server/components/Agama/helper/validations.ts`:
- Around line 4-8: The validation schema in getAcrMappingValidationSchema uses
translated field keys but hardcodes the English string "is Required!"; update
the required messages for source and mapping to be fully localized by calling
the translation function for the whole message (e.g. use a parameterized key
like t('messages.field_required', { field: t('fields.source') }) or separate
keys such as t('errors.source_required')) instead of concatenating
`${t('fields.source')} is Required!`, and apply the same change for mapping
inside the Yup.object<AcrMappingFormValues> schema.
In `@admin-ui/plugins/auth-server/components/Agama/types/agamaTypes.ts`:
- Around line 52-63: Remove the unused index signature from the
AgamaJsonPatchRequestBody interface to tighten typing: modify
AgamaJsonPatchRequestBody so it only declares requestBody: AgamaJsonPatch[] (no
[key: string] signature). Update/verify callers buildAcrMappingPayload and
buildAcrMappingDeletePayload still match the narrowed type (they already only
set requestBody) and run TypeScript checks to ensure no other code relied on the
broader index signature.
Contributor
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/auth-server/components/AuthN/AuthNListPage.tsx (1)
92-125: Guard blocks LDAP entries from being edited despite being designed as editable.
mapLdapToAuthNItemcorrectly omitsinum(LDAP configs useconfigIdinstead), but the guard inhandleGoToAuthNEditPageblocks all rows withoutinum, including LDAP entries. The edit page has explicit handling forname === 'default_ldap_password'to process LDAP updates, confirming they should be editable. The error message "built_in_acr_cannot_be_edited" is also misleading since LDAP entries aren't built-in ACRs. This prevents users from editing LDAP configurations in the UI.Recommend either: (1) move the guard check before
setCurrentItemto avoid partial state updates, (2) disable the edit action for rows withoutinumrather than showing a failed click, or (3) mapconfigIdas the edit identifier for LDAP items.🛠️ Suggested fix (disable edit action for rows without inum)
- const handleGoToAuthNEditPage = useCallback( - (row: AuthNItem) => { - setCurrentItem(row) - if (!row.inum) { - dispatch(updateToast(true, 'error', t('messages.built_in_acr_cannot_be_edited'))) - return - } - - return navigateToRoute(ROUTES.AUTH_SERVER_AUTHN_EDIT(row.inum)) - }, - [setCurrentItem, navigateToRoute, dispatch, t], - ) + const handleGoToAuthNEditPage = useCallback( + (row: AuthNItem) => { + if (!row.inum) { + dispatch(updateToast(true, 'error', t('messages.built_in_acr_cannot_be_edited'))) + return + } + setCurrentItem(row) + return navigateToRoute(ROUTES.AUTH_SERVER_AUTHN_EDIT(row.inum)) + }, + [setCurrentItem, navigateToRoute, dispatch, t], + ) ... - newActions.push((rowData: AuthNItem) => ({ + newActions.push((rowData: AuthNItem) => ({ icon: 'edit', iconProps: { id: 'editAutN' + (rowData.inum || rowData.acrName), style: { color: customColors.darkGray }, }, + disabled: !rowData.inum, - tooltip: `${t('messages.edit_authn')}`, + tooltip: rowData.inum + ? `${t('messages.edit_authn')}` + : `${t('messages.built_in_acr_cannot_be_edited')}`, onClick: (_event: React.MouseEvent, rowDataClicked: AuthNItem | AuthNItem[]) => { if (!Array.isArray(rowDataClicked)) { handleGoToAuthNEditPage(rowDataClicked) } }, }))
🤖 Fix all issues with AI agents
In `@admin-ui/plugins/auth-server/components/Agama/AgamaAliasListPage.tsx`:
- Around line 122-129: The current check treats configuration?.acrMappings ===
undefined as an error, which can be a valid empty state; change the logic to
rely on an explicit success/error flag instead of inferring from data presence:
update the reducer to expose a clear status (e.g., configurationError or
configurationSaveStatus) or return a promise/result from the save action, then
in AgamaAliasListPage replace the condition using configuration?.acrMappings
with that explicit flag (use wasLoading && !isLoading && !configurationError to
clear UI state, and use wasLoading && !isLoading && configurationError to
dispatch updateToast). Reference the existing symbols wasLoading, isLoading,
configuration, and dispatch(updateToast(...)) when making the change.
- Around line 157-164: Add a brief explanatory comment above the useEffect in
AgamaAliasListPage clarifying why eslint's exhaustive-deps is disabled: note
that formik and initialFormValues are intentionally omitted from the dependency
array to avoid infinite re-renders caused by Formik's changing object
references, and that the effect should only run when showAddModal changes; keep
the existing behavior (resetForm, setIsEdit, setSelectedRow) and retain the
eslint-disable-line after this explanatory comment.
- Around line 253-255: The delete handler currently closes the modal and clears
itemToDelete immediately after dispatching patchJsonConfig, which prevents retry
if the server delete fails; remove the immediate calls to setDeleteModal(false)
and setItemToDelete(null) from the delete flow (the block that calls
dispatch(patchJsonConfig({ action: userAction }))). Instead, update the existing
loading-state effect (the useEffect that reacts to loading/success/failure for
add/edit) to also clear the delete state on successful completion by calling
setDeleteModal(false) and setItemToDelete(null) when the saga reports success;
this keeps delete consistent with add/edit and preserves the modal if the
operation errors.
In `@admin-ui/plugins/auth-server/redux/sagas/JsonConfigSaga.js`:
- Around line 43-46: The deletion detection only checks
payload?.action?.action_data?.deletedMapping so JSON Patch "remove" ops in
payload.action.action_data.requestBody are misclassified; update the
isDeleteOperation logic in JsonConfigSaga.js to also scan
payload.action.action_data.requestBody (an array of patch objects) for any entry
with op === 'remove' and treat that as a deletion, then call
addAdditionalData(audit, actionType, JSON_CONFIG, enhancedPayload) with
actionType set to DELETION when either deletedMapping is truthy or a requestBody
patch with op 'remove' is found (references: enhanceJsonConfigAuditPayload,
JSON_CONFIG, addAdditionalData).
In `@admin-ui/plugins/auth-server/redux/utils/auditHelpers.ts`:
- Around line 23-103: The helpers
createAddAuditDetails/createReplaceAuditDetails/createDeleteAuditDetails
populate auditMessage but it is never used by enhanceJsonConfigAuditPayload;
update enhanceJsonConfigAuditPayload to use the AuditDetails.auditMessage value
when composing action_message (fall back to existing message if absent) so the
derived message isn't dead code, and ensure the function consumes the
Omit<AuditDetails,'performedOn'> returned by those helpers; alternatively remove
auditMessage from those helpers if you prefer not to surface it—pick one
approach and apply the same fix for the other similar block referenced (lines
~152-193).
♻️ Duplicate comments (2)
admin-ui/plugins/auth-server/components/Agama/helper/utils.ts (1)
83-85: Avoid unsafe double-cast intoActionData.Line 83-85 force-casts
AgamaJsonPatchRequestBodytoActionData, masking a structural mismatch. Please map to the exactActionDatashape or extendActionDatato accept patch payloads.admin-ui/plugins/auth-server/components/Agama/AgamaAliasListPage.tsx (1)
100-101: Dispatched action result is not awaited.The
dispatch(patchJsonConfig(...))call isn't awaited, so subsequent code doesn't know the operation's outcome. While the loading-state effect (lines 117-130) handles modal closure on completion, this pattern has limitations:
- Cannot provide operation-specific error handling at this call site
- Multiple rapid submissions could cause race conditions with the loading state
Consider awaiting the dispatch and checking the result:
♻️ Suggested improvement
- buildPayload(userAction, 'changes', toActionData(postBody)) - dispatch(patchJsonConfig({ action: userAction })) + buildPayload(userAction, 'changes', toActionData(postBody)) + const result = await dispatch(patchJsonConfig({ action: userAction })) + if (patchJsonConfig.rejected.match(result)) { + throw new Error(result.payload?.message || 'Failed to update ACR mapping') + }
admin-ui/plugins/auth-server/components/Agama/AgamaAliasListPage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Agama/AgamaAliasListPage.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/auth-server/redux/features/jsonConfigSlice.js (1)
14-26: ResetsaveErroron fetch to avoid stale error state.If a previous save failed,
saveErrorstays true through subsequentgetJsonConfigcycles, which can trigger spurious error handling in the UI. Clear it when a fresh fetch starts.🐛 Proposed fix
getJsonConfig: (state) => { state.loading = true + state.saveError = false },
♻️ Duplicate comments (1)
admin-ui/plugins/auth-server/components/Agama/AgamaAliasListPage.tsx (1)
227-255: Delete modal closes before the save completes.Line 252–254 closes the delete dialog immediately after dispatch; if the patch fails, the user loses context and can’t retry. Align delete behavior with the add/edit flow by closing only on successful completion.
♻️ Suggested fix
- dispatch(patchJsonConfig({ action: userAction })) - setDeleteModal(false) - setItemToDelete(null) + dispatch(patchJsonConfig({ action: userAction }))Then close on success in the loading/saveError effect:
if (wasLoading && !isLoading && !saveError) { setIsEdit(false) setSelectedRow(null) setShowAddModal(false) + setDeleteModal(false) + setItemToDelete(null) }
duttarnab
reviewed
Jan 21, 2026
Contributor
duttarnab
left a comment
duttarnab
left a comment
There was a problem hiding this comment.
- Error alert is shown on editing Built-in entry
faisalsiddique4400
changed the title
|
Contributor
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 (3)
admin-ui/plugins/auth-server/components/AuthN/AuthNEditPage.tsx (1)
138-151: Consider logging caught errors for debugging.The catch blocks at lines 138 and 171 discard the error details. While the user sees a warning toast, capturing the error for logging would aid debugging.
♻️ Optional improvement
- } catch { + } catch (error) { + console.warn('Failed to set default ACR after LDAP save:', error) dispatch(Also applies to: 171-184
admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.tsx (1)
78-87: Remove module-scope constant from dependency arrays.
authNResourceIdis a module-level constant that never changes. Including it in useMemo dependency arrays (lines 78, 82, 86) is unnecessary and triggers ESLint warnings. The dependencies should only include values that can change.♻️ Suggested fix
- const authNScopes = useMemo(() => CEDAR_RESOURCE_SCOPES[authNResourceId] || [], [authNResourceId]) + const authNScopes = useMemo(() => CEDAR_RESOURCE_SCOPES[authNResourceId] || [], []) const canReadAuthN = useMemo( () => hasCedarReadPermission(authNResourceId), - [hasCedarReadPermission, authNResourceId], + [hasCedarReadPermission], ) const canWriteAuthN = useMemo( () => hasCedarWritePermission(authNResourceId), - [hasCedarWritePermission, authNResourceId], + [hasCedarWritePermission], )admin-ui/plugins/auth-server/components/AuthN/AuthNForm.tsx (1)
394-407: Duplicate error display for servers field.
GluuTypeAheadreceivesshowErroranderrorMessageprops (lines 401-402), which likely renders the error internally. The additional error div at lines 405-407 may cause the error message to appear twice.🐛 Suggested fix - remove duplicate error display
showError={!!(formik.errors.servers && formik.touched.servers)} errorMessage={formik.errors.servers as string} /> - - {formik.errors.servers && formik.touched.servers ? ( - <div style={{ color: customColors.accentRed }}>{formik.errors.servers}</div> - ) : null}
🤖 Fix all issues with AI agents
In `@admin-ui/plugins/auth-server/components/AuthN/AuthNEditPage.tsx`:
- Around line 104-113: The simple_password_auth branch currently wraps the
entire putAcrsMutation.mutateAsync call in a try/catch and calls handleError,
while LDAP/Script use inner try/catch with primary mutations relying on onError
callbacks; make the error flow consistent by either (A) removing the outer
try/catch around putAcrsMutation.mutateAsync in the simple_password_auth branch
and using the same onError-based primary mutation pattern as the LDAP/Script
branches, or (B) extract a shared helper that performs the ACR update and
invokes handleSuccess/handleError uniformly and call that helper from the
simple_password_auth, LDAP and Script flows; reference
putAcrsMutation.mutateAsync, isDefaultAuthNMethod, handleSuccess, and
handleError when making the change so all branches follow the same error
handling approach.
In `@admin-ui/plugins/auth-server/components/AuthN/AuthNForm.tsx`:
- Around line 81-82: In AuthNForm (AuthNForm.tsx) there's a copy-paste bug where
remotePrimaryKey is initialized from item?.localPrimaryKey causing both fields
to be identical; change the initialization of remotePrimaryKey (in the object
that sets form initial values using item) to an empty string (remotePrimaryKey:
'') instead of item?.localPrimaryKey so remotePrimaryKey is blank when AuthNItem
has no such property.
In `@admin-ui/plugins/auth-server/components/AuthN/helper/validations.ts`:
- Around line 5-7: Extract the hardcoded provider-name strings used in the
type-detection logic (currently used in isBuiltIn, isLdap, isScript in
validations.ts) into shared constants (e.g., AUTHN_SIMPLE_PASSWORD,
AUTHN_DEFAULT_LDAP, AUTHN_SCRIPT) and replace the inline literals in
validations.ts, AuthNForm.tsx and AuthNListPage.tsx to import and use these
constants; ensure the constants are exported from a single module and update any
comparisons or type checks to reference the new constant names so duplication
and typo risk are eliminated.
Comment on lines
+104
to
113
| try { | ||
| const acrData: AuthenticationMethod = isDefaultAuthNMethod(data.defaultAuthNMethod) | ||
| ? { defaultAcr: 'simple_password_auth' } | ||
| : { defaultAcr: '' } | ||
| await putAcrsMutation.mutateAsync({ data: acrData }) | ||
| handleSuccess() | ||
| } catch (error) { | ||
| handleError(error as Error) | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent error handling pattern for simple_password_auth.
The simple_password_auth branch wraps the entire mutation in a try-catch and calls handleError, while the LDAP and Script branches only wrap the secondary ACR mutation in inner try-catch blocks (relying on onError callbacks for the primary mutations). This inconsistency makes error flows harder to follow.
Consider aligning with the pattern used for LDAP/Script, or extracting common error handling logic.
🤖 Prompt for AI Agents
In `@admin-ui/plugins/auth-server/components/AuthN/AuthNEditPage.tsx` around lines
104 - 113, The simple_password_auth branch currently wraps the entire
putAcrsMutation.mutateAsync call in a try/catch and calls handleError, while
LDAP/Script use inner try/catch with primary mutations relying on onError
callbacks; make the error flow consistent by either (A) removing the outer
try/catch around putAcrsMutation.mutateAsync in the simple_password_auth branch
and using the same onError-based primary mutation pattern as the LDAP/Script
branches, or (B) extract a shared helper that performs the ACR update and
invokes handleSuccess/handleError uniformly and call that helper from the
simple_password_auth, LDAP and Script flows; reference
putAcrsMutation.mutateAsync, isDefaultAuthNMethod, handleSuccess, and
handleError when making the change so all branches follow the same error
handling approach.
Comment on lines
+81
to
+82
| remotePrimaryKey: item?.localPrimaryKey || '', | ||
| localPrimaryKey: item?.localPrimaryKey || '', |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "remotePrimaryKey" --type=tsRepository: GluuFederation/flex
Length of output: 728
🏁 Script executed:
rg -n "interface AuthNItem|type AuthNItem" --type=ts -A 20Repository: GluuFederation/flex
Length of output: 8405
🏁 Script executed:
cat -n admin-ui/plugins/auth-server/components/AuthN/AuthNForm.tsx | sed -n '60,95p'Repository: GluuFederation/flex
Length of output: 1624
🏁 Script executed:
cat -n admin-ui/plugins/auth-server/components/AuthN/AuthNForm.tsx | sed -n '360,385p'Repository: GluuFederation/flex
Length of output: 1139
Fix copy-paste error: remotePrimaryKey should not be initialized from localPrimaryKey.
Line 81 sets remotePrimaryKey: item?.localPrimaryKey which causes both remotePrimaryKey and localPrimaryKey form fields to be initialized with identical values. Since the AuthNItem interface contains only localPrimaryKey (not remotePrimaryKey), the field should be initialized to an empty string instead:
remotePrimaryKey: '',
🤖 Prompt for AI Agents
In `@admin-ui/plugins/auth-server/components/AuthN/AuthNForm.tsx` around lines 81
- 82, In AuthNForm (AuthNForm.tsx) there's a copy-paste bug where
remotePrimaryKey is initialized from item?.localPrimaryKey causing both fields
to be identical; change the initialization of remotePrimaryKey (in the object
that sets form initial values using item) to an empty string (remotePrimaryKey:
'') instead of item?.localPrimaryKey so remotePrimaryKey is blank when AuthNItem
has no such property.
Comment on lines
+5
to
+7
| const isBuiltIn = item?.name === 'simple_password_auth' | ||
| const isLdap = item?.name === 'default_ldap_password' | ||
| const isScript = item?.name === 'myAuthnScript' |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting magic strings to constants.
The type detection relies on hardcoded strings ('simple_password_auth', 'default_ldap_password', 'myAuthnScript'). These same strings appear in AuthNForm.tsx and AuthNListPage.tsx. Extracting them to shared constants would reduce duplication and prevent typo-related bugs.
♻️ Suggested refactor
+// In a shared constants file or at the top of this file
+export const AUTHN_TYPES = {
+ BUILT_IN: 'simple_password_auth',
+ LDAP: 'default_ldap_password',
+ SCRIPT: 'myAuthnScript',
+} as const
+
export const getAuthNValidationSchema = (item: AuthNItem | null): Yup.ObjectSchema<any> => {
- const isBuiltIn = item?.name === 'simple_password_auth'
- const isLdap = item?.name === 'default_ldap_password'
- const isScript = item?.name === 'myAuthnScript'
+ const isBuiltIn = item?.name === AUTHN_TYPES.BUILT_IN
+ const isLdap = item?.name === AUTHN_TYPES.LDAP
+ const isScript = item?.name === AUTHN_TYPES.SCRIPT📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const isBuiltIn = item?.name === 'simple_password_auth' | |
| const isLdap = item?.name === 'default_ldap_password' | |
| const isScript = item?.name === 'myAuthnScript' | |
| export const AUTHN_TYPES = { | |
| BUILT_IN: 'simple_password_auth', | |
| LDAP: 'default_ldap_password', | |
| SCRIPT: 'myAuthnScript', | |
| } as const | |
| export const getAuthNValidationSchema = (item: AuthNItem | null): Yup.ObjectSchema<any> => { | |
| const isBuiltIn = item?.name === AUTHN_TYPES.BUILT_IN | |
| const isLdap = item?.name === AUTHN_TYPES.LDAP | |
| const isScript = item?.name === AUTHN_TYPES.SCRIPT |
🤖 Prompt for AI Agents
In `@admin-ui/plugins/auth-server/components/AuthN/helper/validations.ts` around
lines 5 - 7, Extract the hardcoded provider-name strings used in the
type-detection logic (currently used in isBuiltIn, isLdap, isScript in
validations.ts) into shared constants (e.g., AUTHN_SIMPLE_PASSWORD,
AUTHN_DEFAULT_LDAP, AUTHN_SCRIPT) and replace the inline literals in
validations.ts, AuthNForm.tsx and AuthNListPage.tsx to import and use these
constants; ensure the constants are exported from a single module and update any
comparisons or type checks to reference the new constant names so duplication
and typo risk are eliminated.
duttarnab
approved these changes
Jan 21, 2026
moabu
approved these changes
Jan 21, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix(admin-ui): prevent crashes in Aliases and Built-In authentication components (#2596)
Summary
The Admin UI was crashing in the Authentication section under specific user actions. The issue affected both the Aliases and Built-In components, blocking normal configuration workflows.
Problem Details
+button did not trigger the add form and resulted in a crash.Root Cause
Fix Details
Result
+button in Aliases now correctly opens the add form.Additional Notes
This fix improves overall stability in the Auth Server Authentication section and aligns with ongoing efforts to harden Admin UI components against runtime failures.
🔗 Ticket
Closes: #2596
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.