fix(admin-ui): Use typescript generated client for Webhooks pages#2434
fix(admin-ui): Use typescript generated client for Webhooks pages#2434
Conversation
syntrydy
added
comp-admin-ui
📝 WalkthroughWalkthroughReplaces legacy Redux webhook implementation with a TypeScript/hook-driven system: adds WebhookDialog context/provider, hook-based trigger helpers and per-plugin hooks, typed pages/components and utilities, stricter URL validator, i18n entries; removes webhook Redux slice, API wrapper, sagas, and plugin wiring. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant List as WebhookListPage
participant Query as useGetAllWebhooks
participant API
participant Dialog as WebhookDialogContext
User->>List: open list
List->>Query: fetch(params)
Query->>API: GET /webhooks
API-->>Query: 200 + data
Query-->>List: data
User->>List: click delete
List->>Dialog: actions.setShowConfirm(true)
User->>List: confirm
List->>API: DELETE /webhooks/{id}
API-->>List: 200
List->>API: POST /audit
List->>Query: invalidate()
Query->>API: refetch
sequenceDiagram
autonumber
participant User
participant Add as WebhookAddPage
participant URLChk as WebhookURLChecker
participant Mutate as usePostWebhook
participant API
participant Audit
User->>Add: submit form
Add->>URLChk: isValid(url)
alt invalid
Add->>User: show error toast
else valid
Add->>Mutate: mutate(payload)
Mutate->>API: POST /webhooks
API-->>Mutate: 201
Mutate->>Audit: POST /audit
Mutate-->>Add: success
Add->>User: success toast & navigate
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
admin-ui/plugins/auth-server/redux/sagas/OIDCSaga.js (1)
24-35: Optional: simplify token selection helpers.You select
tokentwice viaselect. Read it once and deriveaccess_tokenlocally to reduce saga effects.function* newFunction() { - const wholeToken = yield select((state) => state.authReducer.token) - let token = null - if (wholeToken) { - token = yield select((state) => state.authReducer.token.access_token) - } + const wholeToken = yield select((state) => state.authReducer.token) + const token = wholeToken?.access_token ?? null const issuer = yield select((state) => state.authReducer.issuer) const api = new JansConfigApi.OAuthOpenIDConnectClientsApi( getClient(JansConfigApi, token, issuer), ) return new OIDCApi(api) } function* newTokenFunction() { - const wholeToken = yield select((state) => state.authReducer.token) - let token = null - if (wholeToken) { - token = yield select((state) => state.authReducer.token.access_token) - } + const wholeToken = yield select((state) => state.authReducer.token) + const token = wholeToken?.access_token ?? null const issuer = yield select((state) => state.authReducer.issuer) const api = new JansConfigApi.TokenApi(getClient(JansConfigApi, token, issuer)) return new TokenApi(api) }Also applies to: 37-46
admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts (1)
121-121: Unify saga invocation style fortriggerWebhook.Other sagas use
yield* triggerWebhook(...); here you useyield call(triggerWebhook, ...). Both work, but pick one style repo‑wide for clarity.Also applies to: 156-156
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js (1)
160-166: Fix incorrect state selector causing 401 recovery to fail.
state.s.userinfo_jwtis a typo; should usestate.authReducer.userinfo_jwtlike elsewhere.- const jwt = yield select((state) => state.s.userinfo_jwt) + const jwt = yield select((state) => state.authReducer.userinfo_jwt)admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (1)
102-110: Minor a11y: clickable icon should be a button.
<i>with onClick/onKeyDown is not keyboard-focusable. Use a<button>with proper aria.- <i - onClick={closeModal} - onKeyDown={() => {}} - style={{ color: customColors.logo }} - className="fa fa-2x fa-info fa-fw modal-icon mb-3" - role="img" - aria-hidden="true" - ></i> + <button + type="button" + onClick={closeModal} + className="btn btn-link p-0" + aria-label={t('actions.close')} + style={{ color: customColors.logo }} + > + <i className="fa fa-2x fa-info fa-fw modal-icon mb-3" aria-hidden="true"></i> + </button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (33)
admin-ui/app/context/WebhookDialogContext.tsx(1 hunks)admin-ui/app/context/WebhookDialogProvider.tsx(1 hunks)admin-ui/app/index.tsx(2 hunks)admin-ui/app/locales/en/translation.json(2 hunks)admin-ui/app/locales/es/translation.json(2 hunks)admin-ui/app/locales/fr/translation.json(2 hunks)admin-ui/app/locales/pt/translation.json(2 hunks)admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx(2 hunks)admin-ui/app/routes/Apps/Gluu/GluuDialog.tsx(1 hunks)admin-ui/app/routes/Apps/Gluu/GluuWebhookErrorDialog.tsx(2 hunks)admin-ui/app/utils/hooks/useWebhookDialogAction.tsx(3 hunks)admin-ui/plugins/admin/components/Webhook/ShortcodePopover.tsx(6 hunks)admin-ui/plugins/admin/components/Webhook/WebhookAddPage.js(0 hunks)admin-ui/plugins/admin/components/Webhook/WebhookAddPage.tsx(1 hunks)admin-ui/plugins/admin/components/Webhook/WebhookEditPage.js(0 hunks)admin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsx(1 hunks)admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx(13 hunks)admin-ui/plugins/admin/components/Webhook/WebhookListPage.js(0 hunks)admin-ui/plugins/admin/components/Webhook/WebhookListPage.tsx(1 hunks)admin-ui/plugins/admin/components/Webhook/WebhookURLChecker.js(0 hunks)admin-ui/plugins/admin/components/Webhook/WebhookURLChecker.ts(1 hunks)admin-ui/plugins/admin/components/Webhook/types/index.ts(1 hunks)admin-ui/plugins/admin/plugin-metadata.ts(1 hunks)admin-ui/plugins/admin/redux/api/WebhookApi.js(0 hunks)admin-ui/plugins/admin/redux/features/WebhookSlice.js(0 hunks)admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts(1 hunks)admin-ui/plugins/admin/redux/sagas/WebhookSaga.ts(0 hunks)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts(1 hunks)admin-ui/plugins/admin/redux/sagas/types/state.ts(0 hunks)admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js(1 hunks)admin-ui/plugins/auth-server/redux/sagas/OIDCSaga.js(1 hunks)admin-ui/plugins/saml/redux/sagas/SamlSaga.js(1 hunks)admin-ui/plugins/schema/hooks/useSchemaWebhook.ts(1 hunks)
💤 Files with no reviewable changes (8)
- admin-ui/plugins/admin/components/Webhook/WebhookURLChecker.js
- admin-ui/plugins/admin/components/Webhook/WebhookEditPage.js
- admin-ui/plugins/admin/components/Webhook/WebhookAddPage.js
- admin-ui/plugins/admin/redux/features/WebhookSlice.js
- admin-ui/plugins/admin/redux/api/WebhookApi.js
- admin-ui/plugins/admin/components/Webhook/WebhookListPage.js
- admin-ui/plugins/admin/redux/sagas/types/state.ts
- admin-ui/plugins/admin/redux/sagas/WebhookSaga.ts
🧰 Additional context used
🧠 Learnings (1)
📚 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/Webhook/WebhookListPage.tsx
🧬 Code graph analysis (13)
admin-ui/app/context/WebhookDialogProvider.tsx (1)
admin-ui/app/context/WebhookDialogContext.tsx (4)
WebhookDialogState(20-29)WebhookDialogActions(31-41)WebhookTriggerError(10-18)WebhookDialogContextValue(43-46)
admin-ui/app/routes/Apps/Gluu/GluuWebhookErrorDialog.tsx (1)
admin-ui/app/context/WebhookDialogContext.tsx (2)
useWebhookDialog(50-56)WebhookTriggerError(10-18)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (1)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)
admin-ui/app/index.tsx (1)
admin-ui/app/context/WebhookDialogProvider.tsx (1)
WebhookDialogProvider(26-81)
admin-ui/plugins/admin/components/Webhook/WebhookAddPage.tsx (4)
admin-ui/plugins/admin/redux/sagas/types/state.ts (1)
RootState(4-25)admin-ui/app/utils/TokenController.ts (1)
addAdditionalData(49-124)admin-ui/app/redux/api/backend-api.ts (1)
postUserAction(51-69)admin-ui/plugins/admin/components/Webhook/types/index.ts (2)
WebhookFormValues(29-38)WebhookEntry(18-18)
admin-ui/app/routes/Apps/Gluu/GluuDialog.tsx (1)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)
admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx (1)
admin-ui/plugins/admin/components/Webhook/types/index.ts (5)
WebhookFormProps(78-84)AuiFeature(21-21)CursorPosition(43-46)WebhookFormValues(29-38)FeatureShortcodes(60-64)
admin-ui/app/context/WebhookDialogContext.tsx (1)
admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookEntry(18-18)
admin-ui/plugins/admin/components/Webhook/WebhookListPage.tsx (7)
admin-ui/plugins/admin/redux/sagas/types/state.ts (1)
RootState(4-25)admin-ui/plugins/user-management/types/CommonTypes.ts (1)
ThemeContext(13-17)admin-ui/plugins/admin/components/Webhook/types/index.ts (3)
WebhookEntry(18-18)TableAction(99-110)WebhookTableColumn(115-122)admin-ui/app/utils/TokenController.ts (1)
addAdditionalData(49-124)admin-ui/app/redux/api/backend-api.ts (1)
postUserAction(51-69)admin-ui/app/utils/PermChecker.ts (3)
WEBHOOK_READ(128-128)WEBHOOK_WRITE(129-129)WEBHOOK_DELETE(130-130)admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx (4)
admin-ui/plugins/user-management/types/CommonTypes.ts (1)
ThemeContext(13-17)admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/app/utils/PermChecker.ts (1)
WEBHOOK_READ(128-128)admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookEntry(18-18)
admin-ui/plugins/schema/hooks/useSchemaWebhook.ts (1)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)
admin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsx (4)
admin-ui/plugins/admin/redux/sagas/types/state.ts (1)
RootState(4-25)admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookFormValues(29-38)admin-ui/app/utils/TokenController.ts (1)
addAdditionalData(49-124)admin-ui/app/redux/api/backend-api.ts (1)
postUserAction(51-69)
admin-ui/plugins/admin/components/Webhook/ShortcodePopover.tsx (1)
admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
ShortcodePopoverProps(69-73)
🪛 Biome (2.1.2)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts
[error] 15-31: This generator function doesn't contain yield.
(lint/correctness/useYield)
admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx
[error] 130-130: Do not add then to an object.
(lint/suspicious/noThenProperty)
🔇 Additional comments (7)
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js (1)
10-10: Import path migration to WebhookSagaUtils looks good.Consistent with other files.
admin-ui/app/locales/en/translation.json (2)
828-835: Added HTTP status messages — LGTM.Keys are clear and consistent with usage patterns.
884-884: New placeholderenter_header_value— LGTM.Matches header key/value pattern.
admin-ui/app/locales/es/translation.json (2)
827-834: Mensajes de estado HTTP añadidos — bien.Traducciones correctas y consistentes con EN.
877-877: Nuevo placeholderenter_header_value— bien.Coherente con
enter_header_key.admin-ui/plugins/auth-server/redux/sagas/OIDCSaga.js (1)
21-21: Import path migration verified and correct.WebhookSagaUtils module exists at
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.tsand exportstriggerWebhookas a generator function. ThePlugins/adminpath alias is properly configured and consistently used throughout the codebase.admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (1)
23-23: Provider wrapping verified — code is ready.
WebhookDialogProvideris correctly wired at the app root (admin-ui/app/index.tsx:31-33) and all usage sites align with the provider hierarchy. Hook guard in place to catch misuse. No issues found.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx(3 hunks)admin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsx(1 hunks)admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx(13 hunks)admin-ui/plugins/admin/components/Webhook/types/index.ts(1 hunks)admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts(4 hunks)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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/admin/components/Webhook/WebhookEditPage.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/admin/components/Webhook/WebhookEditPage.tsx
🧬 Code graph analysis (4)
admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts (1)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerWebhook(3-14)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx (3)
admin-ui/plugins/user-management/types/CommonTypes.ts (1)
ThemeContext(13-17)admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/app/utils/PermChecker.ts (1)
WEBHOOK_READ(128-128)
admin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsx (3)
admin-ui/plugins/admin/redux/sagas/types/state.ts (1)
RootState(4-25)admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookFormValues(31-40)admin-ui/app/utils/TokenController.ts (1)
addAdditionalData(49-124)
admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx (1)
admin-ui/plugins/admin/components/Webhook/types/index.ts (6)
WebhookFormProps(80-86)AuiFeature(23-23)CursorPosition(45-48)WebhookFormValues(31-40)KeyValuePair(22-22)FeatureShortcodes(62-66)
🪛 Biome (2.1.2)
admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx
[error] 132-132: Do not add then to an object.
(lint/suspicious/noThenProperty)
🔇 Additional comments (11)
admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts (2)
38-38: Import path update is correct.The import has been successfully updated to reference
WebhookSagaUtils, aligning with the webhook migration strategy.
121-121: Invocation style now consistent with other sagas.All three call sites correctly use
yield*to delegate to thetriggerWebhookgenerator, addressing the inconsistency flagged in the previous review.Also applies to: 156-156, 182-184
admin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsx (3)
39-66: LGTM! Well-structured error handling.The
getErrorMessagefunction provides comprehensive HTTP status code mapping with i18n support, covering all major error scenarios (400, 401, 403, 404, 409, 5xx).
86-92: Deep-link support is now properly implemented.The component now fetches the webhook by ID (lines 86-90) and falls back to location state if available (line 92). This addresses the previous concern about deep links and refresh workflows being broken.
201-225: LGTM! Render logic is well-structured.The component properly handles loading states and conditionally renders the form only when data is available.
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx (1)
37-76: LGTM! Context-based data fetching is properly implemented.The hook successfully migrates from Redux to context-based state management, with proper conditional fetching based on feature, modal state, and permissions.
admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
1-124: LGTM! Type definitions are comprehensive and correct.The file properly imports React types (line 6), addressing the previous review concern about missing React namespace. All interfaces are well-documented and provide strong typing for webhook components.
admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx (4)
57-72: LGTM! Feature synchronization properly implemented.The
useEffecthook now correctly synchronizesselectedFeatureswith loaded feature data, addressing the previous concern about features being stripped when editing. The effect properly waits forloadingFeaturesto complete and includes helpful logging for missing features.
130-133: Static analysis false positive - Yup API usage is correct.The Biome linter flagged
.then()at line 132 as potentially problematic, but this is Yup's documented API for conditional validation (.when().then()). This is correct usage and should be ignored.
258-275: Cursor tracking is complex but appears necessary.The debounced cursor position tracking (using timeouts) enables accurate shortcode insertion at the cursor position. While complex, this pattern is appropriate for handling cursor position across multiple events (
onChangeandonFocus).
352-376: LGTM! Editor cursor calculation is correctly implemented.The cursor position calculation correctly accounts for multi-line content by summing line lengths and adding column offsets. The debounced update pattern prevents performance issues.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx(3 hunks)admin-ui/plugins/admin/components/Webhook/WebhookAddPage.tsx(1 hunks)admin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsx(1 hunks)admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx(13 hunks)admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts(4 hunks)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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/admin/components/Webhook/WebhookEditPage.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/admin/components/Webhook/WebhookEditPage.tsx
🧬 Code graph analysis (5)
admin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsx (4)
admin-ui/plugins/admin/redux/sagas/types/state.ts (1)
RootState(4-25)admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookFormValues(31-40)admin-ui/app/utils/TokenController.ts (1)
addAdditionalData(49-124)admin-ui/app/redux/api/backend-api.ts (1)
postUserAction(51-69)
admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts (1)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerWebhook(1-6)
admin-ui/plugins/admin/components/Webhook/WebhookAddPage.tsx (4)
admin-ui/plugins/admin/redux/sagas/types/state.ts (1)
RootState(4-25)admin-ui/app/utils/TokenController.ts (1)
addAdditionalData(49-124)admin-ui/app/redux/api/backend-api.ts (1)
postUserAction(51-69)admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookFormValues(31-40)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx (3)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/app/utils/PermChecker.ts (1)
WEBHOOK_READ(128-128)admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookEntry(20-20)
admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx (1)
admin-ui/plugins/admin/components/Webhook/types/index.ts (5)
WebhookFormProps(80-86)AuiFeature(23-23)CursorPosition(45-48)WebhookFormValues(31-40)FeatureShortcodes(62-66)
🪛 Biome (2.1.2)
admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx
[error] 132-132: Do not add then to an object.
(lint/suspicious/noThenProperty)
🔇 Additional comments (2)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
1-6: Stub looks good as a synchronous warning
Thanks for inlining the warning and keeping the call signature intact. Forwarders can keep invoking this stub without saga helpers now.admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts (1)
121-184: Direct invocation fits the new stub
Swapping out theyielded call for a plain function call now thattriggerWebhookjust logs keeps the saga tidy and avoids unnecessary effects.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx(3 hunks)admin-ui/plugins/admin/components/Webhook/WebhookAddPage.tsx(1 hunks)admin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsx(1 hunks)admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx(13 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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/admin/components/Webhook/WebhookEditPage.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/admin/components/Webhook/WebhookEditPage.tsx
📚 Learning: 2025-11-10T14:18:58.282Z
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.282Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx
🧬 Code graph analysis (4)
admin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsx (4)
admin-ui/plugins/admin/redux/sagas/types/state.ts (1)
RootState(4-25)admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookFormValues(31-40)admin-ui/app/utils/TokenController.ts (1)
addAdditionalData(49-124)admin-ui/app/redux/api/backend-api.ts (1)
postUserAction(51-69)
admin-ui/plugins/admin/components/Webhook/WebhookAddPage.tsx (4)
admin-ui/plugins/admin/redux/sagas/types/state.ts (1)
RootState(4-25)admin-ui/app/utils/TokenController.ts (1)
addAdditionalData(49-124)admin-ui/app/redux/api/backend-api.ts (1)
postUserAction(51-69)admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookFormValues(31-40)
admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx (1)
admin-ui/plugins/admin/components/Webhook/types/index.ts (6)
WebhookFormProps(80-86)AuiFeature(23-23)CursorPosition(45-48)WebhookFormValues(31-40)KeyValuePair(22-22)FeatureShortcodes(62-66)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx (4)
admin-ui/plugins/user-management/types/CommonTypes.ts (1)
ThemeContext(13-17)admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/app/utils/PermChecker.ts (1)
WEBHOOK_READ(128-128)admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookEntry(20-20)
🪛 Biome (2.1.2)
admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx
[error] 132-132: Do not add then to an object.
(lint/suspicious/noThenProperty)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx (1)
40-49: React Query fetch is not properly gated – moveenabledto query options.The current structure passes
enabled: shouldFetchWebhooksinside thequeryobject, which sends it as an API query parameter (?enabled=true/false) rather than gating the React Query fetch itself. This means:
- When
featureis empty or permissions are missing, the hook still executes and calls the API with invalid parameters- The API receives
?enabled=falsewhen the modal is closed, potentially filtering out enabled webhooks incorrectly- This can break the auto-open logic that depends on seeing enabled webhooks in the cached data
The typical orval-generated hook structure accepts React Query options separately. Apply a fix like this:
const { data: webhooksData, isLoading: isFetchingWebhooks } = useGetWebhooksByFeatureId( feature ?? '', { query: { - enabled: shouldFetchWebhooks, + enabled: true, }, + request: { + enabled: shouldFetchWebhooks, + }, }, )Or if your generated hook uses a different structure:
const { data: webhooksData, isLoading: isFetchingWebhooks } = useGetWebhooksByFeatureId( feature ?? '', - { - query: { - enabled: shouldFetchWebhooks, - }, - }, + { query: { enabled: true } }, + { enabled: shouldFetchWebhooks }, )Verify the exact option structure for your generated
useGetWebhooksByFeatureIdhook and gate the fetch using the React Queryenabledoption (oftenrequest.enabledor a third parameter), not the APIqueryparameter.Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-10T14:18:58.282Z
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.282Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/app/utils/hooks/useWebhookDialogAction.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/app/utils/hooks/useWebhookDialogAction.tsx
🧬 Code graph analysis (1)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx (2)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/app/utils/PermChecker.ts (1)
WEBHOOK_READ(128-128)
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-10T14:18:58.282Z
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.282Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/app/utils/hooks/useWebhookDialogAction.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/app/utils/hooks/useWebhookDialogAction.tsx
🧬 Code graph analysis (1)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx (4)
admin-ui/plugins/user-management/types/CommonTypes.ts (1)
ThemeContext(13-17)admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/app/utils/PermChecker.ts (1)
WEBHOOK_READ(128-128)admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookEntry(20-20)
🔇 Additional comments (3)
admin-ui/app/utils/hooks/useWebhookDialogAction.tsx (3)
56-68: LGTM! Data and loading state synchronization.The effects correctly synchronize fetched webhook data and loading state with the context. The else branch at lines 60-61 properly clears stale data when
webhooksDatabecomes undefined, addressing the concern from past reviews.
100-197: LGTM! Modal component implementation.The
webhookTriggerModalcomponent correctly renders the modal with proper loading states, permission checks, and user actions. The modal visibility guard at line 108 and the cleanup callbacks at lines 103 and 113 are properly implemented.
41-49: Verify theenabledparameter placement inuseGetWebhooksByFeatureIdcall.The
enabled: shouldFetchWebhooksis placed inside thequeryobject rather than a separatequeryOptionsobject. In standard React Query,enabledcontrols query execution and should be in query options, not URL parameters. However, sinceuseGetWebhooksByFeatureIdis from the externalJansConfigApipackage, it may use a custom API structure.To verify this is correct:
- Check the generated hook definition and its options signature
- Inspect browser network tab when
featureis empty orhasCedarPermission(WEBHOOK_READ)returns false—confirm the fetch is not executed (not sent with?enabled=false)- Review Orval configuration to understand how this hook was generated
If the API expects
enabledin aqueryOptionsobject instead, apply the diff from the original comment.
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
admin-ui/app/locales/en/translation.json(3 hunks)admin-ui/app/locales/es/translation.json(3 hunks)admin-ui/app/locales/fr/translation.json(3 hunks)admin-ui/app/locales/pt/translation.json(3 hunks)admin-ui/plugins/admin/components/Webhook/ShortcodePopover.tsx(6 hunks)admin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsx(1 hunks)admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx(14 hunks)admin-ui/plugins/admin/components/Webhook/types/index.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
📚 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/admin/components/Webhook/WebhookEditPage.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/admin/components/Webhook/WebhookEditPage.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/plugins/admin/components/Webhook/WebhookEditPage.tsx
🧬 Code graph analysis (3)
admin-ui/plugins/admin/components/Webhook/WebhookEditPage.tsx (4)
admin-ui/plugins/admin/redux/sagas/types/state.ts (1)
RootState(4-25)admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookFormValues(21-30)admin-ui/app/utils/TokenController.ts (1)
addAdditionalData(49-124)admin-ui/app/redux/api/backend-api.ts (1)
postUserAction(51-69)
admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx (1)
admin-ui/plugins/admin/components/Webhook/types/index.ts (6)
WebhookFormProps(55-61)AuiFeature(16-16)CursorPosition(32-35)WebhookFormValues(21-30)KeyValuePair(15-15)FeatureShortcodes(43-47)
admin-ui/plugins/admin/components/Webhook/ShortcodePopover.tsx (1)
admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
ShortcodePopoverProps(49-53)
🪛 Biome (2.1.2)
admin-ui/plugins/admin/components/Webhook/WebhookForm.tsx
[error] 143-143: Do not add then to an object.
(lint/suspicious/noThenProperty)
🔇 Additional comments (8)
admin-ui/app/locales/fr/translation.json (1)
711-711: Translations for webhook UI features and HTTP errors added correctly.The French translations for the new webhook feature validation and HTTP error handling are properly formatted and semantically appropriate. The entries follow the existing structure with correct JSON syntax.
Also applies to: 766-772, 817-817
admin-ui/app/locales/en/translation.json (1)
742-742: Translations for webhook UI features and HTTP errors added correctly.The English translations for the new webhook feature validation and HTTP error handling are properly formatted and semantically appropriate. All new keys align with their French counterparts and follow the existing structure with correct JSON syntax.
Also applies to: 829-835, 885-885
admin-ui/app/locales/es/translation.json (3)
742-742: Verifyaui_feature_ids_erroris used in webhook feature selection validation.This error message validates that at least one admin UI feature is selected. Ensure this validation is enforced in the corresponding UI component during webhook configuration.
828-834: HTTP error messages added for API response handling.Standard HTTP error messages (400, 401, 403, 404, 409, 500) are now translatable. Verify these keys are consistently referenced throughout the codebase when handling webhook API errors.
878-878: Add corresponding English and French translations forenter_header_valueplaceholder.This placeholder is used for HTTP header value input in webhook configuration. Ensure this key exists in all other locale files (en, fr, pt) to maintain parity across languages.
admin-ui/app/locales/pt/translation.json (3)
693-693: Validateaui_feature_ids_errormessage consistency across all locales.Cross-reference this key with the English (
en), French (fr), and Spanish (es) translation files to ensure the error message is present in all locale files and conveys the same validation requirement.
761-767: Standard HTTP error status codes translated for consistent error handling.These messages (bad_request through server_error) provide localized feedback for common HTTP errors. Confirm these keys are used uniformly across webhook error handling logic.
807-807: Header value input placeholder added for webhook HTTP headers configuration.This placeholder guides users to input custom HTTP header values. Ensure the corresponding field component in the webhook UI displays this placeholder text when applicable.
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (4)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
18-41: Past review comment is outdated.The previous review suggested converting
triggerWebhookto a plain function because it was a "deprecation stub" that only logged a warning. However, the current implementation (lines 18-41) is a fully functional generator that:
- Validates payload
- Gets API instance from Redux state
- Builds webhook request
- Calls the actual API endpoint
- Handles errors
This is NOT a deprecation stub, so the suggestion to remove the generator pattern does not apply to this implementation.
admin-ui/plugins/admin/hooks/useScriptWebhook.ts (1)
14-31: Mirror the typed response fix from useOidcWebhookThis block repeats the
unknown/as never[]pattern flagged inuseOidcWebhook(Lines 14‑31). Please apply the same typed response handling there here as well so the script webhook flow keeps its type guarantees and clears stale errors.admin-ui/plugins/saml/hooks/useSamlWebhook.ts (1)
14-31: Mirror the typed response fix from useOidcWebhookSame issue here: the hook discards the generated response type and pushes
never[]into context. Please reuse the typed handling described in theuseOidcWebhookcomment (Lines 14‑31) so the SAML webhook path stays type-safe and flushes errors on success.admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts (1)
14-31: Mirror the typed response fix from useOidcWebhookThe scope hook repeats the
unknown/never[]casts. Please apply the same typed response cleanup fromuseOidcWebhook(Lines 14‑31) so we keep the generated typings end-to-end.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
admin-ui/plugins/admin/hooks/useScriptWebhook.ts(1 hunks)admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts(4 hunks)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts(1 hunks)admin-ui/plugins/auth-server/hooks/useOidcWebhook.ts(1 hunks)admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts(1 hunks)admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js(4 hunks)admin-ui/plugins/auth-server/redux/sagas/OIDCSaga.js(4 hunks)admin-ui/plugins/saml/hooks/useSamlWebhook.ts(1 hunks)admin-ui/plugins/saml/redux/sagas/SamlSaga.js(8 hunks)admin-ui/plugins/user-management/components/UserAddPage.tsx(2 hunks)admin-ui/plugins/user-management/components/UserEditPage.tsx(2 hunks)admin-ui/plugins/user-management/components/UserForm.tsx(4 hunks)admin-ui/plugins/user-management/components/UserList.tsx(2 hunks)admin-ui/plugins/user-management/hooks/useUserWebhook.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.
Applied to files:
admin-ui/plugins/user-management/components/UserForm.tsxadmin-ui/plugins/user-management/components/UserEditPage.tsxadmin-ui/plugins/user-management/components/UserAddPage.tsx
🧬 Code graph analysis (14)
admin-ui/plugins/user-management/hooks/useUserWebhook.ts (3)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(8-27)admin-ui/plugins/user-management/types/UserApiTypes.ts (1)
CustomUser(5-5)
admin-ui/plugins/auth-server/hooks/useOidcWebhook.ts (1)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js (1)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerScopeWebhook(54-63)
admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts (2)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerScopeWebhook(54-63)
admin-ui/plugins/user-management/components/UserForm.tsx (2)
admin-ui/plugins/user-management/hooks/useUserWebhook.ts (1)
useUserWebhook(8-77)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(8-27)
admin-ui/plugins/auth-server/redux/sagas/OIDCSaga.js (1)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerOidcClientWebhook(43-52)
admin-ui/plugins/saml/redux/sagas/SamlSaga.js (1)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerSamlWebhook(76-84)
admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts (1)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerScriptWebhook(65-74)
admin-ui/plugins/user-management/components/UserEditPage.tsx (2)
admin-ui/plugins/user-management/hooks/useUserWebhook.ts (1)
useUserWebhook(8-77)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(8-27)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (3)
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js (4)
token(26-26)issuer(27-27)JansConfigApi(12-12)api(28-28)admin-ui/plugins/auth-server/redux/sagas/OIDCSaga.js (12)
token(26-26)token(39-39)issuer(30-30)issuer(43-43)JansConfigApi(19-19)api(31-33)api(44-44)api(72-72)api(97-97)api(120-120)api(141-141)api(151-151)admin-ui/plugins/saml/redux/sagas/SamlSaga.js (19)
token(28-28)token(35-35)token(42-42)token(104-104)token(147-147)token(175-175)token(225-225)issuer(29-29)issuer(36-36)issuer(43-43)JansConfigApi(25-25)api(30-30)api(37-37)api(44-44)api(51-51)api(66-66)api(82-82)api(105-105)api(131-131)
admin-ui/plugins/user-management/components/UserList.tsx (3)
admin-ui/plugins/user-management/hooks/useUserWebhook.ts (1)
useUserWebhook(8-77)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(8-27)admin-ui/plugins/user-management/types/UserApiTypes.ts (1)
CustomUser(5-5)
admin-ui/plugins/admin/hooks/useScriptWebhook.ts (2)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerScriptWebhook(65-74)
admin-ui/plugins/user-management/components/UserAddPage.tsx (2)
admin-ui/plugins/user-management/hooks/useUserWebhook.ts (1)
useUserWebhook(8-77)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(8-27)
admin-ui/plugins/saml/hooks/useSamlWebhook.ts (2)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerSamlWebhook(76-84)
🔇 Additional comments (13)
admin-ui/plugins/user-management/components/UserForm.tsx (2)
29-29: LGTM: Clean hook-based webhook integration.The migration from direct import to the
useUserWebhookhook is well-implemented. The hook properly encapsulates webhook triggering logic and state management, and the integration into the password change flow is clean.Also applies to: 37-37, 374-374
46-46: LGTM: Webhook trigger properly threaded through password change flow.The
triggerWebhookparameter is correctly passed tousePasswordChangeand invoked after successful password updates. The callback is properly memoized in the hook's dependencies.Also applies to: 53-53, 417-417
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
23-23: LGTM: Consistent webhook hook integration.The migration to
useUserWebhookfollows the same pattern as UserForm.tsx. The webhook is properly triggered after successful user updates with the correct data type.Also applies to: 31-31, 61-61
admin-ui/plugins/admin/redux/sagas/CustomScriptSaga.ts (1)
38-38: LGTM: Correct generator delegation and webhook utility integration.The migration to
triggerScriptWebhookis properly implemented:
- Uses
yield*for generator delegation (consistent with other sagas)- Correctly passes
isDelete: truefor delete operations- Import path aligns with the new WebhookSagaUtils module
Also applies to: 121-121, 156-156, 182-185
admin-ui/plugins/user-management/components/UserAddPage.tsx (1)
18-18: LGTM: Webhook trigger correctly integrated in user creation flow.The hook-based webhook trigger is properly invoked after successful user creation. The fire-and-forget semantics are appropriate here since the webhook trigger is handled asynchronously by the mutation's callbacks, and user creation success is not dependent on webhook delivery.
Also applies to: 25-25, 34-34
admin-ui/plugins/auth-server/redux/sagas/OAuthScopeSaga.js (1)
10-10: LGTM: Consistent saga webhook migration.The migration to
triggerScopeWebhookis correctly implemented with proper generator delegation usingyield*and theisDelete: trueflag for delete operations.Also applies to: 112-112, 135-135, 158-161
admin-ui/plugins/saml/redux/sagas/SamlSaga.js (1)
23-23: LGTM: SAML webhook migration with proper feature identification.The migration to
triggerSamlWebhookcorrectly distinguishes between SAML configuration and IDP operations through thefeatureIdparameter. All invocations use proper generator delegation withyield*.Also applies to: 86-89, 110-113, 155-158, 183-186, 208-211, 232-235, 258-261
admin-ui/plugins/user-management/components/UserList.tsx (1)
49-49: LGTM: Defensive webhook triggering with proper null check.The null check before calling
triggerUserWebhookis good defensive programming that prevents potential runtime errors. The type cast toCustomUseris safe given the data structure.Also applies to: 55-55, 123-125
admin-ui/plugins/auth-server/redux/sagas/OIDCSaga.js (1)
21-21: LGTM: OIDC client webhook migration correctly implemented.The migration to
triggerOidcClientWebhookmaintains the correct payload structure for each operation type and properly uses generator delegation withyield*. TheisDelete: trueflag is correctly added for delete operations.Also applies to: 77-77, 102-102, 125-127
admin-ui/plugins/user-management/hooks/useUserWebhook.ts (2)
1-6: Imports look appropriate.The imports correctly include React hooks, the generated API client types, webhook dialog context, and Redux utilities needed for this functionality.
52-54: No issues found. The type assertion is consistent with API contract.The
shortcodeValueMaptype assertionRecord<string, { [key: string]: unknown }>is used identically across all 8 webhook implementations in the codebase (SAML, user management, schema, scripts, scopes, OIDC, and saga utilities). This pattern originates from generated code (Orval-based API client) and represents the actual OpenAPI specification contract. Since the type is generated and consistent across the codebase, the code correctly implements the API requirements and cannot be changed without modifying the underlying API specification.Likely an incorrect or invalid review comment.
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (2)
1-3: Imports are appropriate.The imports correctly include Redux-Saga effects and the generated JansConfigApi client with necessary types.
32-34: Verify intended behavior of webhookId extraction and type assertion.The inconsistency between this file and
useUserWebhook.tsis less clear-cut than stated:
useUserWebhook.tsusesinumonly (user entities)WebhookSagaUtils.tsusesinum || dn(feature values)useSamlWebhook.tsalso usesinum || dn(configuration entities)This pattern suggests different entity types intentionally have different fallback logic—not an error. Verify that feature values can indeed have
dnas a fallback identifier.Regarding the type assertion
as Record<string, { [key: string]: unknown }>: The externalShortCodeRequesttype fromJansConfigApilibrary cannot be inspected here. This assertion appears consistently across all webhook files. Confirm whether changing it toRecord<string, unknown>(as suggested) matches the actual library interface requirements.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
admin-ui/plugins/user-management/hooks/useUserWebhook.ts (1)
11-86: Webhook hook follows the same duplicated pattern.This hook duplicates the same structure as
useScriptWebhookanduseScopeWebhook. See the refactoring suggestion in useScriptWebhook.ts for how to extract shared logic.admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts (1)
10-85: Webhook hook follows the same duplicated pattern.This hook duplicates the same structure as
useScriptWebhookanduseUserWebhook. See the refactoring suggestion in useScriptWebhook.ts for how to extract shared logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
admin-ui/app/locales/en/translation.json(4 hunks)admin-ui/app/locales/es/translation.json(4 hunks)admin-ui/app/locales/fr/translation.json(4 hunks)admin-ui/app/locales/pt/translation.json(3 hunks)admin-ui/plugins/admin/constants/webhookFeatures.ts(1 hunks)admin-ui/plugins/admin/constants/webhookTypes.ts(1 hunks)admin-ui/plugins/admin/hooks/useScriptWebhook.ts(1 hunks)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts(1 hunks)admin-ui/plugins/auth-server/hooks/useOidcWebhook.ts(1 hunks)admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts(1 hunks)admin-ui/plugins/saml/hooks/useSamlWebhook.ts(1 hunks)admin-ui/plugins/saml/redux/sagas/SamlSaga.js(8 hunks)admin-ui/plugins/schema/hooks/useSchemaWebhook.ts(1 hunks)admin-ui/plugins/user-management/hooks/useUserWebhook.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 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/redux/sagas/WebhookSagaUtils.ts
🧬 Code graph analysis (8)
admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts (3)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/admin/constants/webhookTypes.ts (1)
WebhookTriggerResponse(11-13)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerScopeWebhook(74-83)
admin-ui/plugins/schema/hooks/useSchemaWebhook.ts (2)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/admin/constants/webhookTypes.ts (1)
WebhookTriggerResponse(11-13)
admin-ui/plugins/user-management/hooks/useUserWebhook.ts (4)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/admin/constants/webhookTypes.ts (1)
WebhookTriggerResponse(11-13)admin-ui/plugins/user-management/types/UserApiTypes.ts (1)
CustomUser(5-5)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
admin-ui/plugins/saml/hooks/useSamlWebhook.ts (3)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/admin/constants/webhookTypes.ts (1)
WebhookTriggerResponse(11-13)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerSamlWebhook(98-108)
admin-ui/plugins/auth-server/hooks/useOidcWebhook.ts (2)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/admin/constants/webhookTypes.ts (1)
WebhookTriggerResponse(11-13)
admin-ui/plugins/saml/redux/sagas/SamlSaga.js (2)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerSamlWebhook(98-108)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
admin-ui/plugins/admin/hooks/useScriptWebhook.ts (3)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/admin/constants/webhookTypes.ts (1)
WebhookTriggerResponse(11-13)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerScriptWebhook(85-96)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (3)
admin-ui/plugins/admin/redux/sagas/types/state.ts (1)
RootState(4-25)admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (1)
errorMessage(100-100)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
🔇 Additional comments (15)
admin-ui/plugins/admin/constants/webhookTypes.ts (1)
1-13: No issues found. The manual types correctly fill a gap in the generated client.The WebhookTriggerResponse interface aligns with PR objectives: the generated JansConfigApi provides the useTriggerWebhook hook, but does not export the response type. The manual type definition correctly types the API response structure, as confirmed by error handling code that references the same
response?.body?.responseMessagestructure.The implementation is consistent: ShortCodeRequest is imported from the generated JansConfigApi, while WebhookTriggerResponse provides the missing response type. This dual approach—using generated types where available and creating manual types for gaps—aligns with the PR's objective of transitioning to TypeScript-generated client types.
admin-ui/app/locales/fr/translation.json (4)
670-673: Webhook trigger message translations are complete and well-localized.The four new webhook-related messages (trigger_success, trigger_error, trigger_failed, trigger_invalid_data) follow the existing French localization conventions and are appropriately detailed to inform users about webhook execution outcomes. Their placement in the messages section aligns with the webhook feature's integration into the new TypeScript-generated client architecture.
715-715: aui_feature_ids_error message is correctly placed and localized.This validation message for the Admin UI feature IDs field is properly positioned within the messages section and maintains semantic clarity about the requirement for at least one feature selection.
770-776: HTTP status code translations and device deletion message are comprehensive and consistently formatted.The addition of HTTP error status translations (bad_request, unauthorized, forbidden, not_found, conflict, server_error) alongside the device_deleted_successfully message provides complete coverage for common API response scenarios. These align well with the migration to a TypeScript-generated client that explicitly handles various error states.
821-821: Header value placeholder translation is appropriately concise.The enter_header_value placeholder in the placeholders section correctly maintains brevity while conveying the input requirement for webhook HTTP headers.
admin-ui/app/locales/es/translation.json (5)
734-737: Spanish webhook trigger translations are complete and properly localized.The four webhook outcome messages are idiomatically appropriate in Spanish and provide clear user feedback for webhook execution states. The terminology (activaron, salió mal, etc.) aligns with common UI/API feedback patterns in Spanish.
746-746: aui_feature_ids_error validation message is appropriately Spanish-localized.The message correctly conveys the requirement to select at least one Admin UI feature and maintains consistency with the French translation's semantic intent.
770-776: HTTP status codes and device deletion translations provide comprehensive API error coverage.The parallel additions of HTTP error translations and device_deleted_successfully message mirror the French file structure and ensure consistent error handling feedback across both locales. Translations maintain appropriate formality and clarity for API/system messages.
882-882: Header value placeholder is appropriately concise in Spanish.The enter_header_value translation in the placeholders section correctly conveys the input requirement for webhook HTTP header values while maintaining brevity.
670-880: Verify i18n key coverage across all supported locales.Both French and Spanish files show consistent additions of 13 new translation keys (webhook triggers, HTTP status codes, feature validation, device deletion, and header placeholder). Per the AI summary, parallel translations should exist in English, Portuguese, and other locales. Ensure these keys are systematically added across all language files to prevent missing translations at runtime.
To verify complete i18n coverage, confirm that all 13 new keys have been added to every locale file mentioned in the PR (en, fr, es, pt, and any others supported by the Admin UI).
admin-ui/app/locales/pt/translation.json (2)
690-693: LGTM: Webhook translation keys properly added.The Portuguese translations for webhook trigger messages are correctly added and align with the keys used in the new webhook hooks.
697-697: LGTM: Feature IDs error message added.The translation for the admin UI feature IDs error is properly included.
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (3)
13-24: LGTM: API instance creation is properly implemented.The generator correctly retrieves authentication state and constructs a typed JansConfigApi instance.
26-59: LGTM: Webhook triggering logic is well-structured.The implementation properly:
- Validates payload before triggering
- Constructs ShortCodeRequest with webhookId fallback (inum or dn)
- Handles errors with detailed message extraction and toast notifications
- Rethrows errors for upstream handling
Based on learnings: The
console.errorin the catch block (line 53) is appropriate for debugging unexpected errors in this saga context.
61-108: LGTM: Webhook wrapper functions are clean.The specialized webhook functions properly delegate to the base
triggerWebhookwith appropriate feature IDs from constants, maintaining consistency across the codebase.
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
admin-ui/plugins/admin/constants/webhookTypes.ts(1 hunks)admin-ui/plugins/admin/hooks/useScriptWebhook.ts(1 hunks)admin-ui/plugins/admin/hooks/useWebhookTrigger.ts(1 hunks)admin-ui/plugins/auth-server/hooks/useOidcWebhook.ts(1 hunks)admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts(1 hunks)admin-ui/plugins/saml/hooks/useSamlWebhook.ts(1 hunks)admin-ui/plugins/schema/hooks/useSchemaWebhook.ts(1 hunks)admin-ui/plugins/user-management/hooks/useUserWebhook.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🧬 Code graph analysis (7)
admin-ui/plugins/admin/hooks/useScriptWebhook.ts (2)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(16-103)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerScriptWebhook(85-96)
admin-ui/plugins/auth-server/hooks/useOidcWebhook.ts (1)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(16-103)
admin-ui/plugins/saml/hooks/useSamlWebhook.ts (2)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(16-103)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerSamlWebhook(98-108)
admin-ui/plugins/schema/hooks/useSchemaWebhook.ts (1)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(16-103)
admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts (2)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(16-103)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerScopeWebhook(74-83)
admin-ui/plugins/user-management/hooks/useUserWebhook.ts (4)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(16-103)admin-ui/plugins/user-management/types/UserApiTypes.ts (1)
CustomUser(5-5)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(8-27)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (3)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/admin/constants/webhookTypes.ts (1)
WebhookTriggerResponse(13-15)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
🔇 Additional comments (1)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
23-97: Core webhook trigger logic is sound.The implementation properly:
- Validates the webhook ID before proceeding
- Constructs the correct payload envelope (
createdFeatureValue/deletedFeatureValue)- Manages UI state (loading, errors, modals) via context
- Provides user feedback via toasts
- Cleans up progress state on settlement
This refactoring successfully centralizes the webhook trigger logic that was previously scattered across Redux sagas.
| export function useScopeWebhook() { | ||
| const trigger = useWebhookTrigger<Record<string, unknown>>({ | ||
| extractId: (scope) => scope.inum as string | undefined, | ||
| idFieldName: 'scope.inum', | ||
| }) | ||
|
|
||
| const triggerScopeWebhook = useCallback( | ||
| (scope: Record<string, unknown>, featureId: 'scopes_write' | 'scopes_delete'): void => { | ||
| trigger(scope, featureId) | ||
| }, | ||
| [trigger], | ||
| ) | ||
|
|
||
| return { triggerScopeWebhook } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Pattern duplication across webhook hooks.
This hook is nearly identical to useOidcWebhook.ts, differing only in the entity type and feature IDs. While the current abstraction via useWebhookTrigger is good, consider whether a factory function or additional layer could reduce this boilerplate if the pattern repeats across many entity types (scripts, SAML, users, etc.).
🤖 Prompt for AI Agents
admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts lines 4-18: this hook
duplicates the same pattern used for other entity webhook hooks; refactor by
creating a small generic factory (e.g., createEntityWebhookHook or
useEntityWebhook) that accepts parameters: entity name, extractId function,
idFieldName and allowed feature IDs, and returns a hook that wires up
useWebhookTrigger and exposes trigger<Entity>Webhook; replace this file to call
the factory with extractId = scope => scope.inum as string|undefined,
idFieldName = 'scope.inum' and featureIds = ['scopes_write','scopes_delete'] so
the implementation is one line and the boilerplate is removed.
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/schema/hooks/useSchemaWebhook.ts (1)
1-22: LGTM! Consider adding feature ID constant for consistency.The hook correctly uses the generated
Partial<JansAttribute>type and follows the established pattern. The implementation is clean and type-safe.The hardcoded
'attributes_write'string (line 16) is inconsistent with other hooks that useWEBHOOK_FEATURE_IDSconstants. If this feature ID isn't already defined inadmin-ui/plugins/admin/constants/webhookFeatures.ts, consider adding it:// In webhookFeatures.ts export const WEBHOOK_FEATURE_IDS = { // ... existing IDs ATTRIBUTES_WRITE: 'attributes_write', } as constThen use it here:
import { useCallback } from 'react' import type { JansAttribute } from 'JansConfigApi' import { useWebhookTrigger } from 'Plugins/admin/hooks/useWebhookTrigger' +import { WEBHOOK_FEATURE_IDS } from 'Plugins/admin/constants/webhookFeatures' /** * Hook for triggering webhooks related to schema/attribute operations */ export function useSchemaWebhook() { const trigger = useWebhookTrigger<Partial<JansAttribute>>({ extractId: (attribute) => attribute.inum, idFieldName: 'attribute.inum', }) const triggerAttributeWebhook = useCallback( (attribute: Partial<JansAttribute>): void => { - trigger(attribute, 'attributes_write') + trigger(attribute, WEBHOOK_FEATURE_IDS.ATTRIBUTES_WRITE) }, [trigger], ) return { triggerAttributeWebhook } }This maintains a single source of truth for feature IDs and prevents typos.
♻️ Duplicate comments (1)
admin-ui/plugins/auth-server/hooks/useOidcWebhook.ts (1)
1-19: LGTM! Consider using WEBHOOK_FEATURE_IDS constants for consistency.The hook correctly uses the generated
Clienttype and the implementation is clean and type-safe.For consistency with other hooks in the codebase (e.g.,
useUserWebhook,useSamlWebhook,useScriptWebhook), consider importing and using theWEBHOOK_FEATURE_IDSconstants:import { useCallback } from 'react' import type { Client } from 'JansConfigApi' import { useWebhookTrigger } from 'Plugins/admin/hooks/useWebhookTrigger' +import { WEBHOOK_FEATURE_IDS } from 'Plugins/admin/constants/webhookFeatures' export function useOidcWebhook() { const trigger = useWebhookTrigger<Client>({ extractId: (client) => client.inum, idFieldName: 'client.inum', }) const triggerOidcWebhook = useCallback( - (client: Client, featureId: 'oidc_clients_write' | 'oidc_clients_delete'): void => { + (client: Client, featureId: typeof WEBHOOK_FEATURE_IDS.OIDC_CLIENTS_WRITE | typeof WEBHOOK_FEATURE_IDS.OIDC_CLIENTS_DELETE): void => { trigger(client, featureId) }, [trigger], ) return { triggerOidcWebhook } }This prevents typos and keeps the feature IDs synchronized with the central definition.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
admin-ui/plugins/admin/hooks/useScriptWebhook.ts(1 hunks)admin-ui/plugins/admin/hooks/useWebhookTrigger.ts(1 hunks)admin-ui/plugins/auth-server/hooks/useOidcWebhook.ts(1 hunks)admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts(1 hunks)admin-ui/plugins/saml/hooks/useSamlWebhook.ts(1 hunks)admin-ui/plugins/schema/hooks/useSchemaWebhook.ts(1 hunks)admin-ui/plugins/user-management/hooks/useUserWebhook.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
📚 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/plugins/saml/hooks/useSamlWebhook.tsadmin-ui/plugins/admin/hooks/useScriptWebhook.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/admin/hooks/useWebhookTrigger.ts
🧬 Code graph analysis (7)
admin-ui/plugins/schema/hooks/useSchemaWebhook.ts (1)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(16-98)
admin-ui/plugins/auth-server/hooks/useOidcWebhook.ts (1)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(16-98)
admin-ui/plugins/saml/hooks/useSamlWebhook.ts (3)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(16-98)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerSamlWebhook(98-108)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts (2)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(16-98)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerScopeWebhook(74-83)
admin-ui/plugins/admin/hooks/useScriptWebhook.ts (3)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(16-98)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerScriptWebhook(85-96)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (2)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/admin/constants/webhookTypes.ts (1)
WebhookTriggerResponse(13-15)
admin-ui/plugins/user-management/hooks/useUserWebhook.ts (4)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(16-98)admin-ui/plugins/user-management/types/UserApiTypes.ts (1)
CustomUser(5-5)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(8-27)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
🔇 Additional comments (6)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
1-98: LGTM!The hook implementation is well-designed and serves as a solid foundation for all webhook triggers. Key strengths:
- Proper error typing with
ApiError- Comprehensive mutation lifecycle handling (onSuccess, onError, onSettled)
- Correct state management via
WebhookDialogContext- Smart payload construction with
createdFeatureValue/deletedFeatureValuebased on action type- Complete dependency array capturing all closure values
- Early validation with user-friendly error messages
The generic constraint
T extends { inum?: string; dn?: string }appropriately supports the various entity types across the codebase.admin-ui/plugins/user-management/hooks/useUserWebhook.ts (1)
1-20: LGTM!Excellent implementation that follows all best practices:
- Uses the generated
Partial<CustomUser>type for type safety- Leverages
WEBHOOK_FEATURE_IDSconstant for consistency- Clean, straightforward code with no unnecessary type casts
- Correct dependency array
This hook serves as a good reference implementation for the webhook trigger pattern.
admin-ui/plugins/saml/hooks/useSamlWebhook.ts (2)
11-21: LGTM on the implementation pattern!The hook correctly:
- Uses
WEBHOOK_FEATURE_IDSconstants for type-safe feature identification- Properly handles the dual-ID scenario (inum or dn) for SAML entities
- Follows the established callback pattern with correct dependencies
6-9: Ignore this review comment; SAML types are not available in JansConfigApi.The suggestion assumes SAML-specific types like
SamlTrustRelationshipexist in JansConfigApi, but they don't. Searches across the codebase found no SAML type exports—while other entities (User, Scope, Client) have corresponding JansConfigApi types, SAML does not. Additionally,useSamlWebhookis currently unused in the codebase. The current implementation usingRecord<string, unknown>is appropriate until SAML types are generated and exported from JansConfigApi.Likely an incorrect or invalid review comment.
admin-ui/plugins/admin/hooks/useScriptWebhook.ts (2)
11-21: LGTM on the implementation pattern!The hook correctly:
- Uses
WEBHOOK_FEATURE_IDSconstants for type-safe feature identification- Follows the established callback pattern with correct dependencies
- Properly handles both write and delete operations
6-9: The generated API type definitions are not accessible in the repository for verification.The
jans_config_api_orvaldirectory (which would contain generated types) is not present—it appears to be generated at build time. Additionally, the localjans_config_apipackage is not accessible for inspection.Without access to the generated
JansConfigApitype exports, I cannot definitively verify whether it exports aScriptorCustomScripttype that could replaceRecord<string, unknown>.However, based on the code structure:
useWebhookTrigger<T>is designed as a reusable generic utility constrained to{ inum?: string; dn?: string }- Using
Record<string, unknown>with the type cast onscript.inumis appropriate for this generic design- Applying a specific
CustomScriptItemtype would break the hook's reusability across different entity typesThe suggestion may be valid if
JansConfigApiexports appropriate types post-generation, or it may misunderstand the intentional generic design of the hook.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
29-29: Specify concrete yielded value type in generator.The
anytype in the Generator's third type parameter (yielded value) reduces type safety. Consider specifying the actual types yielded bygetApiInstanceandcall:-}): Generator<CallEffect | SelectEffect | PutEffect, void, any> { +}): Generator<CallEffect | SelectEffect | PutEffect, void, JansConfigApi.WebhooksApi> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 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/redux/sagas/WebhookSagaUtils.ts
🧬 Code graph analysis (1)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (3)
admin-ui/plugins/admin/redux/sagas/types/state.ts (1)
RootState(4-25)admin-ui/app/redux/api/base.ts (1)
getClient(23-44)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
🔇 Additional comments (4)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (4)
11-13: Good use of type safety.The
WebhookPayloadinterface properly usesunknowninstead ofanyfor type safety.
71-80: Consistent and correct implementation.The function follows the same pattern as other webhook triggers and correctly uses the feature ID constants.
82-93: Consistent webhook trigger implementation.The function correctly implements the webhook trigger pattern for custom scripts.
95-105: Excellent type constraint for feature ID.The union type constraint on lines 100-102 ensures only valid SAML feature IDs can be passed, providing strong compile-time safety. This pattern effectively prevents misuse.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 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/redux/sagas/WebhookSagaUtils.ts
🧬 Code graph analysis (1)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (4)
admin-ui/plugins/admin/redux/sagas/types/state.ts (1)
RootState(4-25)admin-ui/app/redux/api/base.ts (1)
getClient(23-44)admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (1)
errorMessage(100-100)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
🔇 Additional comments (4)
admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (4)
1-14: LGTM! Imports and interface definition are well-structured.The imports use consistent ES6 style, proper TypeScript types are imported, and the
WebhookPayloadinterface correctly usesRecord<string, unknown>instead ofany. All previous concerns about import style and type safety have been addressed.
16-22: LGTM! API instance creation is properly typed.The function correctly uses
RootStatefor state typing and returns a concreteJansConfigApi.AdminUIWebhooksApitype. Previous concerns about usinganyhave been fully addressed.
118-128: LGTM! SAML webhook trigger is cleanly implemented.The function correctly delegates to
triggerWebhookwith proper type constraints onfeatureId. Unlike the other wrapper functions, it doesn't need payload conversion, which is appropriate for the SAML use case.
66-116: I need to search for the actual call sites of the other two webhook functions to confirm the calling pattern is consistent.Let me search the codebase directly for the remaining saga files:Based on the information gathered from the script execution, I have enough evidence to verify the review comment. The shell script output showed that in
CustomScriptSaga.ts, whentriggerScriptWebhookis called withisDelete: true, the payload still usescreatedFeatureValuefield. This pattern allows the saga functions to correctly handle the conversion. The original review comment's concern about the "suspicious" fallback logic appears to be unfounded—the fallback pattern is actually intentional and correct.Resolve the review comment—the payload conversion pattern is correct, not suspicious.
The original concern about lines 77-80, 93-95, and 111-113 misidentifies the intended behavior. Evidence from
CustomScriptSaga.tsshows that callers consistently pass webhook data in thecreatedFeatureValuefield for all operations (both create and delete). The saga functions then correctly convert this to the appropriate target field based on the operation type:
- For delete:
{ deletedFeatureValue: payload.createdFeatureValue || payload.deletedFeatureValue }- For create:
{ createdFeatureValue: payload.createdFeatureValue || payload.deletedFeatureValue }This pattern is semantically correct because it reflects how the callers invoke these functions. The fallback (
|| payload.deletedFeatureValue) provides defensive programming to handle potential edge cases, not a bug. No changes are required.
duttarnab
left a comment
duttarnab
left a comment
There was a problem hiding this comment.
- the user-payload is pssed as short-code even if no short code added as parameter in the webhook
- the webhook should be triggered for the selected feature event listed in drop-down. I test for scopes it is not working.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
admin-ui/plugins/auth-server/hooks/useOidcWebhook.ts (1)
9-9: Use WEBHOOK_FEATURE_IDS constants for consistency.Unlike
useScopeWebhook,useSamlWebhook,useScriptWebhook, anduseUserWebhook, this hook uses string literal types instead of deriving types from theWEBHOOK_FEATURE_IDSconstants. This creates inconsistency and risks typos.Apply this diff to align with the other hooks:
import { useCallback } from 'react' import type { Client } from 'JansConfigApi' import { useWebhookTrigger } from 'Plugins/admin/hooks/useWebhookTrigger' +import { WEBHOOK_FEATURE_IDS } from 'Plugins/admin/constants/webhookFeatures' export function useOidcWebhook() { const trigger = useWebhookTrigger() const triggerOidcWebhook = useCallback( - (client: Client, featureId: 'oidc_clients_write' | 'oidc_clients_delete'): void => { + ( + client: Client, + featureId: + | typeof WEBHOOK_FEATURE_IDS.OIDC_CLIENTS_WRITE + | typeof WEBHOOK_FEATURE_IDS.OIDC_CLIENTS_DELETE, + ): void => { trigger(client, featureId) }, [trigger], ) return { triggerOidcWebhook } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
admin-ui/plugins/admin/helper/webhookUtils.ts(1 hunks)admin-ui/plugins/admin/hooks/useScriptWebhook.ts(1 hunks)admin-ui/plugins/admin/hooks/useWebhookTrigger.ts(1 hunks)admin-ui/plugins/auth-server/hooks/useOidcWebhook.ts(1 hunks)admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts(1 hunks)admin-ui/plugins/saml/hooks/useSamlWebhook.ts(1 hunks)admin-ui/plugins/schema/hooks/useSchemaWebhook.ts(1 hunks)admin-ui/plugins/user-management/hooks/useUserWebhook.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
📚 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/plugins/admin/hooks/useScriptWebhook.tsadmin-ui/plugins/saml/hooks/useSamlWebhook.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/admin/hooks/useWebhookTrigger.ts
🧬 Code graph analysis (8)
admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts (2)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(12-108)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
admin-ui/plugins/admin/hooks/useScriptWebhook.ts (3)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(12-108)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerScriptWebhook(100-116)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
admin-ui/plugins/saml/hooks/useSamlWebhook.ts (3)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(12-108)admin-ui/plugins/admin/redux/sagas/WebhookSagaUtils.ts (1)
triggerSamlWebhook(118-128)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
admin-ui/plugins/schema/hooks/useSchemaWebhook.ts (1)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(12-108)
admin-ui/plugins/user-management/hooks/useUserWebhook.ts (4)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(12-108)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(8-27)admin-ui/plugins/user-management/types/UserApiTypes.ts (1)
CustomUser(5-5)admin-ui/plugins/admin/constants/webhookFeatures.ts (1)
WEBHOOK_FEATURE_IDS(1-11)
admin-ui/plugins/admin/helper/webhookUtils.ts (2)
admin-ui/plugins/admin/helper/utils.js (1)
getNestedValue(5-7)admin-ui/plugins/admin/components/Webhook/types/index.ts (1)
WebhookEntry(13-13)
admin-ui/plugins/auth-server/hooks/useOidcWebhook.ts (1)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
useWebhookTrigger(12-108)
admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (3)
admin-ui/app/context/WebhookDialogContext.tsx (1)
useWebhookDialog(50-56)admin-ui/plugins/admin/constants/webhookTypes.ts (1)
WebhookTriggerResponse(13-15)admin-ui/plugins/admin/helper/webhookUtils.ts (1)
webhookOutputObject(14-63)
🪛 ast-grep (0.39.9)
admin-ui/plugins/admin/helper/webhookUtils.ts
[warning] 30-30: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\{${key}\\}, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 47-47: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\{${placeholderKey}\\}, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
admin-ui/plugins/schema/hooks/useSchemaWebhook.ts (1)
1-16: LGTM! Clean migration to hook-based webhook triggering.The refactor from Redux dispatch to
useWebhookTriggeris well-executed. The hook now uses the typedJansAttributedirectly without unnecessary casting, maintains correct dependencies, and follows the established pattern used by other feature-specific webhook hooks in this PR.admin-ui/plugins/auth-server/hooks/useScopeWebhook.ts (1)
1-20: LGTM! Excellent type safety and consistency.This hook correctly:
- Uses the generated
Scopetype instead of genericRecord<string, unknown>- References
WEBHOOK_FEATURE_IDSconstants for feature ID types- Follows the established pattern for feature-specific webhook hooks
- Maintains proper
useCallbackdependenciesadmin-ui/plugins/admin/hooks/useScriptWebhook.ts (1)
1-21: LGTM! Clean implementation following established pattern.This hook correctly uses
WEBHOOK_FEATURE_IDSconstants, maintains proper dependencies inuseCallback, and follows the same pattern as the other feature-specific webhook hooks introduced in this PR.admin-ui/plugins/user-management/hooks/useUserWebhook.ts (1)
1-17: LGTM! Well-typed and follows established pattern.This hook correctly uses the generated
CustomUsertype, references the centralizedWEBHOOK_FEATURE_IDS.USERS_EDITconstant, and maintains properuseCallbackdependencies. The implementation is clean and consistent with the other feature-specific webhook hooks.admin-ui/plugins/admin/hooks/useWebhookTrigger.ts (1)
1-108: LGTM! Solid central implementation with appropriate handling of API mismatches.This hook serves as the core webhook triggering mechanism for all feature-specific hooks. The implementation is well-structured:
- Properly manages UI state (loading, errors, dialogs) via
WebhookDialogContext- Handles success/error cases with appropriate user feedback via toasts
- Fetches and filters enabled webhooks before triggering
- Uses
webhookOutputObjectto construct proper payloads- Cleans up state consistently in
onSettledand error paths- Maintains complete dependency array
[dispatch, t, actions, triggerWebhookMutation]The multiple
as anycasts are documented as necessary due to OpenAPI spec mismatches and are acceptable given the runtime validation and error handling in place.admin-ui/plugins/saml/hooks/useSamlWebhook.ts (1)
1-21: Verify webhook template compatibility with the new payload structure.The migration from Redux sagas to hooks changes the webhook payload structure:
- Old saga approach (lines 12-14 in WebhookSagaUtils.ts): wraps entities under
createdFeatureValue/deletedFeatureValuekeys- New hook approach (useSamlWebhook.ts): passes entities directly as flat objects to
webhookOutputObjectThis means webhook templates referencing
{createdFeatureValue.displayName}will break with the new structure, which extracts fields directly as{displayName}.Webhook configurations are likely stored in the database (not found in version control), so manual verification is required:
- Check existing webhook URL templates and request bodies for references to
createdFeatureValue.*ordeletedFeatureValue.*patterns- If found, confirm they have been updated to use flat field references (e.g.,
{displayName}instead of{createdFeatureValue.displayName})- Or, verify migrations were applied to transform stored templates
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/admin/helper/webhookUtils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sonar scan (admin-ui)
🔇 Additional comments (3)
admin-ui/plugins/admin/helper/webhookUtils.ts (3)
1-2: LGTM!The imports are appropriate for the file's functionality.
14-33: LGTM! RegEx injection issue from previous review has been resolved.The URL placeholder processing logic is correct. The use of
replaceAllinstead of the RegExp constructor eliminates the regex injection risk flagged in the previous review.
58-62: LGTM!The return structure correctly maps to the ShortCodeRequest type, and the empty string fallback for webhookId is appropriate.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/admin/helper/webhookUtils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🧬 Code graph analysis (1)
admin-ui/plugins/admin/helper/webhookUtils.ts (1)
admin-ui/plugins/admin/helper/utils.js (1)
getNestedValue(5-7)
🔇 Additional comments (1)
admin-ui/plugins/admin/helper/webhookUtils.ts (1)
62-65: No action required – empty webhookId is intentional and properly handled.The empty string fallback for missing
webhook.inumis a deliberate pattern in the codebase. The type definitions explicitly markwebhookIdas optional (webhookId?: string), confirming that empty values are valid. This same fallback pattern is consistently used elsewhere (e.g.,WebhookSagaUtils.ts:42), and downstream consumers handle empty strings without validation errors, indicating the design is intentional and working as expected.
| const getNestedValue = (obj: Record<string, any>, path: string): any => { | ||
| return path.split('.').reduce((acc, part) => (acc != null ? acc[part] : undefined), obj) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider consolidating duplicate getNestedValue implementation.
This helper function is duplicated in admin-ui/plugins/admin/helper/utils.js (lines 4-6), though the version in utils.js still uses the old acc && acc[part] pattern that was corrected here. Consider either importing from a shared location or updating the utils.js version to match this corrected implementation to avoid divergence.
🤖 Prompt for AI Agents
In admin-ui/plugins/admin/helper/webhookUtils.ts around lines 4 to 6, there is a
corrected getNestedValue implementation using safe null checks; the review notes
a duplicate older implementation in admin-ui/plugins/admin/helper/utils.js
(lines 4-6). Replace the duplicate in utils.js with a single shared
implementation: either extract getNestedValue into a new shared helper module
(e.g., admin-ui/plugins/admin/helper/shared.ts) and import it from both
webhookUtils.ts and utils.js, or update utils.js to use the exact null-safe
reduce implementation shown here (path.split('.').reduce((acc, part) => (acc !=
null ? acc[part] : undefined), obj)); ensure both files reference the same
function and remove the old acc && acc[part] pattern to avoid divergence.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
admin-ui/plugins/admin/helper/webhookUtils.ts (1)
4-6: Consolidate the duplicate getNestedValue implementation.This helper is duplicated in
admin-ui/plugins/admin/helper/utils.js(lines 4-6) with an older, less safe pattern (acc && acc[part]). To prevent divergence and improve maintainability, consider extracting this into a shared helper module or updating utils.js to import from here.Based on past review comments and relevant code snippets.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/admin/helper/webhookUtils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🧬 Code graph analysis (1)
admin-ui/plugins/admin/helper/webhookUtils.ts (1)
admin-ui/plugins/admin/helper/utils.js (1)
getNestedValue(5-7)
🔇 Additional comments (3)
admin-ui/plugins/admin/helper/webhookUtils.ts (3)
1-2: LGTM: Imports align with PR objectives.The imports correctly use TypeScript-generated types from JansConfigApi, which aligns with the PR's goal of replacing manually created types with the generated client.
36-51: Multiple placeholder handling correctly implemented.The accumulation pattern using
updatedValuecorrectly handles multiple placeholders in a single field, fixing the critical issue from previous reviews.Note: The check at line 49
if (webhook.httpRequestBody)is redundant since we're already inside the block that verified this (lines 30-32). While harmless, it can be removed for cleaner code.Optional simplification:
}) - if (webhook.httpRequestBody) { - ;(webhook.httpRequestBody as Record<string, any>)[key] = updatedValue - } + ;(webhook.httpRequestBody as Record<string, any>)[key] = updatedValue }
56-60:inumis the correct field name and empty string fallback is acceptable.After verification:
- WebhookEntry is imported from JansConfigApi, confirming
inumis the standard identifier field in the Jans webhook system- The empty string fallback is appropriate because
inumis a required identifier for WebhookEntry objects managed by the Jans API; no errors elsewhere in the codebase suggest missinginumvalues- The code pattern aligns with similar optional field handling (e.g.,
webhook.url || '')No changes needed.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/admin/helper/webhookUtils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🧬 Code graph analysis (1)
admin-ui/plugins/admin/helper/webhookUtils.ts (1)
admin-ui/plugins/admin/helper/utils.js (1)
getNestedValue(5-7)
🔇 Additional comments (2)
admin-ui/plugins/admin/helper/webhookUtils.ts (2)
8-75: Excellent implementation - previous issues have been addressed.The webhook placeholder replacement logic is well-structured and correctly handles:
- Deep cloning to avoid mutation
- Nested path access via dot notation
- Complex value serialization with appropriate warnings
- Multiple placeholders within a single field (fixed via accumulator pattern)
- Safe string replacement using
replaceAll()instead of dynamic RegExpThis aligns well with the PR objective of using TypeScript-generated client types (
WebhookEntry,ShortCodeRequest).
70-74: Verify that empty webhookId can be safely used with the webhook API.Line 71 defaults
webhookIdto empty string whenwebhook.inumis missing. While this is consistent with other parts of the codebase (e.g., WebhookSagaUtils.ts:42), no validation guards exist to confirm the webhook triggering API accepts empty string IDs. Check the webhook service or OpenAPI specification to verify this is a supported behavior.
| if (value !== undefined) { | ||
| if (typeof value === 'object' && value !== null) { | ||
| console.warn( | ||
| `Placeholder {${key}} resolved to a complex value; using JSON.stringify. Consider using a primitive field.`, | ||
| ) | ||
| } | ||
| const stringValue = | ||
| typeof value === 'object' && value !== null ? JSON.stringify(value) : String(value) | ||
| shortcodeValueMap[key] = value | ||
| webhook.url = webhook.url?.replaceAll(`{${key}}`, stringValue) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting duplicate complex value stringification logic.
The logic for handling complex values (lines 24-30) is duplicated at lines 52-58. Extracting this into a small helper function would improve maintainability.
Apply this refactor to reduce duplication:
+const stringifyPlaceholderValue = (value: any, key: string): string => {
+ if (typeof value === 'object' && value !== null) {
+ console.warn(
+ `Placeholder {${key}} resolved to a complex value; using JSON.stringify. Consider using a primitive field.`,
+ )
+ return JSON.stringify(value)
+ }
+ return String(value)
+}
+
export const webhookOutputObject = (
enabledFeatureWebhooks: WebhookEntry[],
createdFeatureValue: Record<string, any>,Then use it in both locations:
if (value !== undefined) {
- if (typeof value === 'object' && value !== null) {
- console.warn(
- `Placeholder {${key}} resolved to a complex value; using JSON.stringify. Consider using a primitive field.`,
- )
- }
- const stringValue =
- typeof value === 'object' && value !== null ? JSON.stringify(value) : String(value)
+ const stringValue = stringifyPlaceholderValue(value, key)
shortcodeValueMap[key] = value
webhook.url = webhook.url?.replaceAll(`{${key}}`, stringValue)
}And at lines 52-59:
if (value !== undefined) {
- if (typeof value === 'object' && value !== null) {
- console.warn(
- `Placeholder {${placeholderKey}} resolved to a complex value; using JSON.stringify. Consider using a primitive field.`,
- )
- }
- const stringValue =
- typeof value === 'object' && value !== null ? JSON.stringify(value) : String(value)
+ const stringValue = stringifyPlaceholderValue(value, placeholderKey)
updatedValue = updatedValue.replaceAll(`{${placeholderKey}}`, stringValue)
shortcodeValueMap[placeholderKey] = value
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In admin-ui/plugins/admin/helper/webhookUtils.ts around lines 23-33 (and
similarly at 52-58), the complex-value handling and JSON.stringify +
console.warn logic is duplicated; extract that logic into a small helper (e.g.,
stringifyPlaceholderValue(value, key)) that returns the stringified value and
optionally the original value or a tuple, move the console.warn into the helper
so warnings are centralized, then replace both duplicated blocks to call the
helper, assign shortcodeValueMap[key] = originalValue, and use the helper's
string result when calling webhook.url.replaceAll.
duttarnab
left a comment
duttarnab
left a comment
There was a problem hiding this comment.
Please take look on following docs which will give you idea how webhooks should work. https://docs.gluu.org/stable/admin/admin-ui/webhooks/
Review comments:
- On creating webhooks we select
Admin UI Featurefield. Suppose we selectedScopes - Add/Editin this field then it means the webhook should be triggered on creating or Editing a scope. I added a webhook by selectingScopes - Add/Editbut it was not triggered on creating new scope .
- Webhook fired from Admin UI also has its audit log. I not able to see /audit executed for webhook.
syntrydy
changed the title
4 participants
#2433
Closes #2433
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores