Enhance CORS management with support to configure regexes - #4833
Merged
Conversation
NipuniBhagya
requested review from
DonOmalVindula,
brionmario and
jeradrutnam
as code owners
August 11, 2026 09:49
NipuniBhagya
added
Type/Improvement
trigger-pr-builder
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change replaces string-based CORS origins with typed exact-origin and regex rows. It adds shared validation, settings editing, application-creation integration, localized messages, typed merge handling, and end-to-end regex coverage. ChangesTyped CORS origin workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend/apps/console/src/features/applications/pages/ApplicationCreatePage.tsx`:
- Around line 665-673: Update the CORS persistence flow around
updateCorsConfig.mutate and the ApplicationCreatePage form surface to handle
mutation failures inline instead of relying on hook-level failure toasts.
Preserve the created application and keep the form/error state visible with an
actionable retry path until the CORS update succeeds; only continue the existing
success navigation after the mutation completes successfully.
In `@frontend/packages/configure-settings/src/hooks/useAllowedOriginsDraft.ts`:
- Around line 80-88: Update the dynamic validation translation in computeIssues,
specifically the resolve helper, to map each validation code to an appropriate
fallback string and pass that fallback as the positional argument to t. Preserve
the existing settings:cors.validation.${code} key and the returned
errors/warnings structure.
In `@frontend/packages/configure-settings/src/utils/isRegexAnchored.ts`:
- Around line 19-20: Update the end-anchor detection in isRegexAnchored so a
trailing `$` is accepted only when it has an even number of preceding
backslashes, while preserving the existing \z check with equivalent escaping
awareness. Add coverage for escaped and unescaped end anchors, including
patterns such as ^foo\$ and valid anchored endings.
In `@tests/e2e/pages/settings/settings.page.ts`:
- Around line 60-63: Update rowFor to locate the input by its value using
Playwright’s value-oriented locator API rather than interpolating value into a
CSS selector. Preserve matching for both exact origins and regex patterns,
including values containing backslashes, so hasCustomOrigin and
removeAllowedOrigin continue finding persisted rows.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b0aed382-3979-4123-b194-e9b14f5076a0
📒 Files selected for processing (35)
frontend/apps/console/src/features/applications/components/create-application/ConfigureDetails.tsxfrontend/apps/console/src/features/applications/components/create-application/ConfigureRedirectUris.tsxfrontend/apps/console/src/features/applications/components/create-application/CorsOriginsEditor.tsxfrontend/apps/console/src/features/applications/components/create-application/__tests__/ConfigureRedirectUris.test.tsxfrontend/apps/console/src/features/applications/components/create-application/__tests__/CorsOriginsEditor.test.tsxfrontend/apps/console/src/features/applications/contexts/ApplicationCreate/ApplicationCreateContext.tsxfrontend/apps/console/src/features/applications/contexts/ApplicationCreate/ApplicationCreateProvider.tsxfrontend/apps/console/src/features/applications/pages/ApplicationCreatePage.tsxfrontend/apps/console/src/features/applications/utils/__tests__/hasInvalidCorsRows.test.tsfrontend/apps/console/src/features/applications/utils/__tests__/mergeCorsOrigins.test.tsfrontend/apps/console/src/features/applications/utils/hasInvalidCorsRows.tsfrontend/apps/console/src/features/applications/utils/mergeCorsOrigins.tsfrontend/packages/configure-settings/src/components/cors/AllowedOriginRow.tsxfrontend/packages/configure-settings/src/components/cors/CorsSection.tsxfrontend/packages/configure-settings/src/components/cors/OriginRow.tsxfrontend/packages/configure-settings/src/components/cors/__tests__/AllowedOriginRow.test.tsxfrontend/packages/configure-settings/src/components/cors/__tests__/CorsSection.test.tsxfrontend/packages/configure-settings/src/hooks/__tests__/useAllowedOriginsDraft.test.tsfrontend/packages/configure-settings/src/hooks/useAllowedOriginsDraft.tsfrontend/packages/configure-settings/src/index.tsfrontend/packages/configure-settings/src/models/allowedOriginRow.tsfrontend/packages/configure-settings/src/utils/__tests__/allowedOriginRows.test.tsfrontend/packages/configure-settings/src/utils/__tests__/baselineKey.test.tsfrontend/packages/configure-settings/src/utils/__tests__/isRegexAnchored.test.tsfrontend/packages/configure-settings/src/utils/__tests__/normalizedNonEmpty.test.tsfrontend/packages/configure-settings/src/utils/__tests__/validateAllowedOriginRows.test.tsfrontend/packages/configure-settings/src/utils/allowedOriginRows.tsfrontend/packages/configure-settings/src/utils/baselineKey.tsfrontend/packages/configure-settings/src/utils/isRegexAnchored.tsfrontend/packages/configure-settings/src/utils/nonEmptyRowKeys.tsfrontend/packages/configure-settings/src/utils/normalizedNonEmpty.tsfrontend/packages/configure-settings/src/utils/validateAllowedOriginRows.tsfrontend/packages/i18n/src/locales/en-US.tstests/e2e/pages/settings/settings.page.tstests/e2e/tests/settings/cors-allowed-origins.spec.ts
💤 Files with no reviewable changes (3)
- frontend/packages/configure-settings/src/utils/normalizedNonEmpty.ts
- frontend/packages/configure-settings/src/utils/tests/normalizedNonEmpty.test.ts
- frontend/packages/configure-settings/src/components/cors/OriginRow.tsx
Comment on lines
665
to
673
| const corsAdditions = corsOrigins.filter((row) => !isRowEmpty(row)); | ||
| if (selectedTemplateConfig?.capabilities?.cors && corsAdditions.length > 0) { | ||
| updateCorsConfig.mutate({ | ||
| data: mergeCorsOrigins( | ||
| corsConfigData?.writable.allowedOrigins ?? [], | ||
| corsConfigData?.readOnly.allowedOrigins ?? [], | ||
| validCorsAdditions, | ||
| corsAdditions, | ||
| ), | ||
| }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve a recovery path when CORS persistence fails.
Line 667 starts the CORS mutation without an error handler. The success path then navigates away. If this write fails, the application is created but its configured CORS entries are lost without an actionable retry path. Keep a retryable error state visible until the CORS update succeeds.
As per coding guidelines, render mutation failures inline where the form has a natural surface and do not use hook-level failure toasts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/apps/console/src/features/applications/pages/ApplicationCreatePage.tsx`
around lines 665 - 673, Update the CORS persistence flow around
updateCorsConfig.mutate and the ApplicationCreatePage form surface to handle
mutation failures inline instead of relying on hook-level failure toasts.
Preserve the created application and keep the form/error state visible with an
actionable retry path until the CORS update succeeds; only continue the existing
success navigation after the mutation completes successfully.
Source: Coding guidelines
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
NipuniBhagya
added a commit
to NipuniBhagya/thunderid
that referenced
this pull request
Aug 11, 2026
Refs thunder-id#4833 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NipuniBhagya
force-pushed
the
allowed-origin-fix
branch
from
August 11, 2026 11:29
75e0360 to
cf7d4cf
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend/packages/configure-settings/src/components/cors/__tests__/CorsSection.test.tsx`:
- Line 21: Remove the duplicate updateState declaration in the CorsSection test
module, retaining a single binding with its existing {isError, error} type and
initial value so the test compiles.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 14454b8d-3d70-4e80-9693-6777e019b5a6
📒 Files selected for processing (15)
frontend/apps/console/src/features/applications/components/create-application/CorsOriginsEditor.tsxfrontend/apps/console/src/features/applications/components/create-application/__tests__/ConfigureDetails.test.tsxfrontend/apps/console/src/features/applications/components/create-application/__tests__/CorsOriginsEditor.test.tsxfrontend/apps/console/src/features/applications/pages/ApplicationCreatePage.tsxfrontend/packages/configure-settings/src/components/cors/AllowedOriginRow.tsxfrontend/packages/configure-settings/src/components/cors/__tests__/AllowedOriginRow.test.tsxfrontend/packages/configure-settings/src/components/cors/__tests__/CorsSection.test.tsxfrontend/packages/configure-settings/src/hooks/__tests__/useAllowedOriginsDraft.test.tsfrontend/packages/configure-settings/src/hooks/useAllowedOriginsDraft.tsfrontend/packages/configure-settings/src/index.tsfrontend/packages/configure-settings/src/utils/__tests__/isRegexAnchored.test.tsfrontend/packages/configure-settings/src/utils/isRegexAnchored.tsfrontend/packages/configure-settings/src/utils/validateAllowedOriginRows.tsfrontend/packages/i18n/src/locales/en-US.tstests/e2e/pages/settings/settings.page.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- frontend/apps/console/src/features/applications/components/create-application/tests/CorsOriginsEditor.test.tsx
- frontend/apps/console/src/features/applications/pages/ApplicationCreatePage.tsx
- frontend/packages/i18n/src/locales/en-US.ts
- frontend/packages/configure-settings/src/components/cors/AllowedOriginRow.tsx
- frontend/packages/configure-settings/src/utils/validateAllowedOriginRows.ts
- frontend/packages/configure-settings/src/hooks/useAllowedOriginsDraft.ts
- frontend/packages/configure-settings/src/index.ts
- tests/e2e/pages/settings/settings.page.ts
| const mockMutate = vi.fn(); | ||
| const mockReset = vi.fn(); | ||
| // The save-failure surface reads the mutation's own error state, so each test declares it. | ||
| let updateState: {isError: boolean; error: Error | null} = {isError: false, error: null}; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the duplicate updateState declaration.
The file declares the same let updateState binding twice. TypeScript cannot compile this test module.
Proposed fix
let updateState: {isError: boolean; error: Error | null} = {isError: false, error: null};
-let updateState: {isError: boolean; error: Error | null} = {isError: false, error: null};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let updateState: {isError: boolean; error: Error | null} = {isError: false, error: null}; | |
| let updateState: {isError: boolean; error: Error | null} = {isError: false, error: null}; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/packages/configure-settings/src/components/cors/__tests__/CorsSection.test.tsx`
at line 21, Remove the duplicate updateState declaration in the CorsSection test
module, retaining a single binding with its existing {isError, error} type and
initial value so the test compiles.
NipuniBhagya
added a commit
to NipuniBhagya/thunderid
that referenced
this pull request
Aug 11, 2026
Refs thunder-id#4833 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NipuniBhagya
force-pushed
the
allowed-origin-fix
branch
from
August 11, 2026 11:53
cf7d4cf to
60bb32e
Compare
NipuniBhagya
added a commit
to NipuniBhagya/thunderid
that referenced
this pull request
Aug 11, 2026
Refs thunder-id#4833 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NipuniBhagya
force-pushed
the
allowed-origin-fix
branch
from
August 11, 2026 12:10
60bb32e to
107054c
Compare
Refs thunder-id#4833 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NipuniBhagya
force-pushed
the
allowed-origin-fix
branch
from
August 11, 2026 12:35
107054c to
ea3b99c
Compare
brionmario
approved these changes
Aug 11, 2026
brionmario
enabled auto-merge
August 11, 2026 12:44
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 11, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 11, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 11, 2026
KashiwalHarsh
pushed a commit
to Infosys/thunder-id
that referenced
this pull request
Aug 14, 2026
Refs thunder-id#4833 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Harsh Kashiwal <harsh.kashiwal@infosys.com>
lashinijay
pushed a commit
to lashinijay/thunderid
that referenced
this pull request
Sep 3, 2026
Refs thunder-id#4833 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This pull request refactors the CORS Allowed Origins editor in the application creation flow to improve validation, user experience, and code maintainability. The main change is replacing the generic URI list editor with a dedicated
CorsOriginsEditorcomponent, which supports both exact origins and regular expressions, and aligns validation and messaging with the application's settings page. The update also changes the internal representation of CORS origins from plain strings to structured objects, ensuring more robust handling throughout the UI and tests.Preview
Approach
CORS Allowed Origins Editor Improvements:
CorsOriginsEditorcomponent, which provides a better user experience for editing CORS allowed origins by supporting both exact origins and regex patterns, and by surfacing consistent validation messages shared with the settings page.AllowedOriginDraftRowobjects instead of plain strings for CORS origins, ensuring each entry explicitly states its type and value. (Fa9d7fb2L189, Fa9d7fb2L212, Fa9d7fb2L340)Validation and Readiness Logic:
ConfigureDetails.tsxto block progression if any CORS origin entry is invalid, preventing silent submission failures and improving feedback to the user. [1] [2]Testing Updates:
Code Cleanup and Consistency:
Dependency and Utility Updates:
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
New Features
Bug Fixes