feat(ui): add OIDC ConfigureSSO wizard skeleton - #9200
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 0ae71fb The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
NicolasLopes7
marked this pull request as ready for review
July 20, 2026 20:29
Contributor
API Changes Report
Summary
@clerk/sharedCurrent version: 4.25.8 Subpath
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds experimental custom OIDC enterprise connection support to Organization Profile SSO configuration, including provider types, OAuth endpoint serialization, feature-gated selection and routing, a three-step configuration wizard, validation, localization resources, shared mode controls, tests, and release metadata. Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/ui/src/components/ConfigureSSO/domain/organizationEnterpriseConnection.ts`:
- Line 16: Update isOidcProvider to require the `oidc_` prefix, matching the
OidcProviderType contract and excluding bare or malformed values such as `oidc`
and `oidcfoo`; add a regression test covering these unsupported inputs and
preserving valid OIDC values.
In `@packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/index.tsx`:
- Around line 90-96: Localize the unsupported-provider fallback in the
ConfigureStep component by adding typed
configureSSO.configureStep.unsupportedProvider resource keys for the title and
description, then render both through localizationKeys. Follow the existing OIDC
localization contract and keep the terminal UI on the same localization path
instead of using English literals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 7c43c83d-8790-40f9-a66e-d611f185371a
📒 Files selected for processing (14)
.changeset/cool-dragons-march.md.changeset/spotty-items-dream.mdpackages/localizations/src/en-US.tspackages/shared/src/types/localization.tspackages/ui/src/components/ConfigureSSO/domain/__tests__/organizationEnterpriseConnection.test.tspackages/ui/src/components/ConfigureSSO/domain/organizationEnterpriseConnection.tspackages/ui/src/components/ConfigureSSO/hooks/__tests__/useOrganizationEnterpriseConnection.test.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/__tests__/ConfigureStep.test.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/index.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/OidcCustomConfigureSteps.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/index.tsxpackages/ui/src/components/ConfigureSSO/steps/SelectProviderStep.tsxpackages/ui/src/components/ConfigureSSO/steps/__tests__/SelectProviderStep.test.tsxpackages/ui/src/components/ConfigureSSO/types.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
9 tasks
dstaley
approved these changes
Jul 24, 2026
dstaley
left a comment
dstaley
left a comment
Member
There was a problem hiding this comment.
lgtm! I think a human-review of all the comments would be a good thing to do. As-is they feel too jargon-y, and my confidence in them is low because they feel too AI written. If they're accurate that's great, but I think we can probably rewrite them to be more clear to others who eventually encounter this code.
Member
|
@NicolasLopes7 before merging please run |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcCredentialsStep.tsx (1)
22-36: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winClient secret must be re-entered on every visit, even when already configured.
Unlike
SamlCustomConfigureSteps.tsx's certificate step (which tracksexistingCertPresentso users aren't forced to re-upload an existing certificate),canSubmithere always requires a freshclientSecret, with no path to keep a previously-saved secret. This blocks users from re-savingclientIdalone or simply continuing through an already-configured connection.If intentional (secrets always required fresh), consider ignoring; otherwise consider an
existingSecretPresent-style flag mirroring the SAML pattern.🤖 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 `@packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcCredentialsStep.tsx` around lines 22 - 36, The OIDC credentials flow always requires a newly entered client secret, preventing reuse of an already-configured connection. Update OidcCredentialsStep’s initialization and canSubmit logic to track whether an existing client secret is present, mirroring the existingCertPresent pattern in the SAML certificate step, and allow submission when the saved secret is available even if the secret field remains blank.packages/ui/src/components/ConfigureSSO/domain/__tests__/organizationEnterpriseConnection.test.ts (1)
96-99: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd direct
isOidcProviderboundary tests.
This case only checks thatprovideris copied through verbatim; it doesn’t exercise the exactoidc_/oauth_custom_prefix check. Add focused cases foroidc,oidcfoo, andoauth_customrejection, plusoidc_x/oauth_custom_xacceptance.🤖 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 `@packages/ui/src/components/ConfigureSSO/domain/__tests__/organizationEnterpriseConnection.test.ts` around lines 96 - 99, Add focused boundary tests for isOidcProvider covering rejection of exact “oidc”, “oidcfoo”, and “oauth_custom” values, and acceptance of “oidc_x” and “oauth_custom_x”. Keep the existing provider passthrough test unchanged and assert the predicate’s result directly rather than only checking derive().provider.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 20-36: Remove the entire ephemeral claude-mem context block from
AGENTS.md, including its status, live activity URL, and session-specific
instructions. Leave only stable repository-owned guidance in the committed file,
or omit the generated section entirely.
---
Nitpick comments:
In
`@packages/ui/src/components/ConfigureSSO/domain/__tests__/organizationEnterpriseConnection.test.ts`:
- Around line 96-99: Add focused boundary tests for isOidcProvider covering
rejection of exact “oidc”, “oidcfoo”, and “oauth_custom” values, and acceptance
of “oidc_x” and “oauth_custom_x”. Keep the existing provider passthrough test
unchanged and assert the predicate’s result directly rather than only checking
derive().provider.
In
`@packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcCredentialsStep.tsx`:
- Around line 22-36: The OIDC credentials flow always requires a newly entered
client secret, preventing reuse of an already-configured connection. Update
OidcCredentialsStep’s initialization and canSubmit logic to track whether an
existing client secret is present, mirroring the existingCertPresent pattern in
the SAML certificate step, and allow submission when the saved secret is
available even if the secret field remains blank.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 775effe0-af57-487e-bc39-2f4f166b3d01
📒 Files selected for processing (72)
AGENTS.mdpackages/clerk-js/src/core/resources/EnterpriseConnection.tspackages/localizations/src/ar-SA.tspackages/localizations/src/be-BY.tspackages/localizations/src/bg-BG.tspackages/localizations/src/bn-IN.tspackages/localizations/src/ca-ES.tspackages/localizations/src/cs-CZ.tspackages/localizations/src/da-DK.tspackages/localizations/src/de-DE.tspackages/localizations/src/el-GR.tspackages/localizations/src/en-GB.tspackages/localizations/src/en-US.tspackages/localizations/src/es-CR.tspackages/localizations/src/es-ES.tspackages/localizations/src/es-MX.tspackages/localizations/src/es-UY.tspackages/localizations/src/fa-IR.tspackages/localizations/src/fi-FI.tspackages/localizations/src/fr-FR.tspackages/localizations/src/he-IL.tspackages/localizations/src/hi-IN.tspackages/localizations/src/hr-HR.tspackages/localizations/src/hu-HU.tspackages/localizations/src/id-ID.tspackages/localizations/src/is-IS.tspackages/localizations/src/it-IT.tspackages/localizations/src/ja-JP.tspackages/localizations/src/kk-KZ.tspackages/localizations/src/ko-KR.tspackages/localizations/src/mn-MN.tspackages/localizations/src/ms-MY.tspackages/localizations/src/nb-NO.tspackages/localizations/src/nl-BE.tspackages/localizations/src/nl-NL.tspackages/localizations/src/pl-PL.tspackages/localizations/src/pt-BR.tspackages/localizations/src/pt-PT.tspackages/localizations/src/ro-RO.tspackages/localizations/src/ru-RU.tspackages/localizations/src/sk-SK.tspackages/localizations/src/sr-RS.tspackages/localizations/src/sv-SE.tspackages/localizations/src/ta-IN.tspackages/localizations/src/te-IN.tspackages/localizations/src/th-TH.tspackages/localizations/src/tr-TR.tspackages/localizations/src/uk-UA.tspackages/localizations/src/vi-VN.tspackages/localizations/src/zh-CN.tspackages/localizations/src/zh-TW.tspackages/shared/src/types/elementIds.tspackages/shared/src/types/enterpriseConnection.tspackages/shared/src/types/localization.tspackages/ui/src/components/ConfigureSSO/domain/__tests__/organizationEnterpriseConnection.test.tspackages/ui/src/components/ConfigureSSO/domain/organizationEnterpriseConnection.tspackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/__tests__/ConfigureStep.test.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/index.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/OidcCustomConfigureSteps.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcCredentialsStep.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcEndpointsConfigurationForm.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcEndpointsStep.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcRedirectUriStep.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/SamlCustomConfigureSteps.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/SamlGoogleConfigureSteps.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/SamlMicrosoftConfigureSteps.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/SamlOktaConfigureSteps.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/shared/IdentityProviderConfigurationForm.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/shared/IdentityProviderConfigurationModes.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/shared/ActiveConnectionAlert.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/shared/IdentityProviderConfigurationModes.tsxpackages/ui/src/components/ConfigureSSO/types.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
💤 Files with no reviewable changes (1)
- packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/shared/IdentityProviderConfigurationModes.tsx
🚧 Files skipped from review as they are similar to previous changes (43)
- packages/localizations/src/fr-FR.ts
- packages/localizations/src/es-UY.ts
- packages/localizations/src/ro-RO.ts
- packages/localizations/src/sk-SK.ts
- packages/localizations/src/ja-JP.ts
- packages/localizations/src/sv-SE.ts
- packages/localizations/src/he-IL.ts
- packages/localizations/src/da-DK.ts
- packages/localizations/src/en-GB.ts
- packages/localizations/src/hr-HR.ts
- packages/ui/src/components/ConfigureSSO/types.ts
- packages/localizations/src/pt-BR.ts
- packages/localizations/src/id-ID.ts
- packages/localizations/src/it-IT.ts
- packages/localizations/src/fa-IR.ts
- packages/localizations/src/ms-MY.ts
- packages/localizations/src/es-CR.ts
- packages/localizations/src/nb-NO.ts
- packages/localizations/src/hi-IN.ts
- packages/localizations/src/pl-PL.ts
- packages/localizations/src/bn-IN.ts
- packages/localizations/src/kk-KZ.ts
- packages/localizations/src/te-IN.ts
- packages/localizations/src/nl-BE.ts
- packages/localizations/src/mn-MN.ts
- packages/localizations/src/hu-HU.ts
- packages/localizations/src/ta-IN.ts
- packages/localizations/src/zh-TW.ts
- packages/localizations/src/pt-PT.ts
- packages/localizations/src/uk-UA.ts
- packages/localizations/src/fi-FI.ts
- packages/localizations/src/ca-ES.ts
- packages/ui/src/components/ConfigureSSO/domain/organizationEnterpriseConnection.ts
- packages/localizations/src/es-MX.ts
- packages/localizations/src/es-ES.ts
- packages/localizations/src/en-US.ts
- packages/localizations/src/nl-NL.ts
- packages/localizations/src/el-GR.ts
- packages/localizations/src/be-BY.ts
- packages/localizations/src/th-TH.ts
- packages/localizations/src/bg-BG.ts
- packages/localizations/src/cs-CZ.ts
- packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/index.tsx
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcRedirectUriStep.tsx (1)
170-173: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
isDisabled={isLastStep}on Continue is alwaysfalsehere — likely copy-paste leftover.This step (
redirect-uri) is always the first of three fixedOIDC_STEPS, soisLastStepnever evaluatestruefor this component; the binding has no effect and reads as if this could be the wizard's final step, which is misleading.OidcCredentialsStep/OidcEndpointsStepdon't useisLastStepfor their Continue button.♻️ Proposed cleanup
- <Step.Footer.Continue - onClick={() => goNext()} - isDisabled={isLastStep} - /> + <Step.Footer.Continue onClick={() => goNext()} />🤖 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 `@packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcRedirectUriStep.tsx` around lines 170 - 173, Remove the redundant isDisabled={isLastStep} prop from the Step.Footer.Continue component in OidcRedirectUriStep, leaving its onClick behavior unchanged.
🤖 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
`@packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcEndpointsStep.tsx`:
- Around line 97-109: Update the non-discovery payload in the OidcEndpointsStep
updateConnection flow to include userInfoUrl only when
userInfoUrlField.value.trim() is non-empty; preserve the existing required
authUrl and tokenUrl fields and discoveryUrl behavior.
In `@packages/ui/src/components/ConfigureSSO/steps/SelectProviderStep.tsx`:
- Around line 86-93: The providerGroups filtering in SelectProviderStep must
retain the OIDC group when currentCard represents an existing oidc_custom
connection, even if isOIDCFlowEnabled is false, so the selected radio remains
visible. Update the condition near providerGroups to account for the existing
selected provider while preserving the flag-based hiding behavior for new OIDC
selections, and add a regression test covering the flag-off existing OIDC
connection case.
---
Nitpick comments:
In
`@packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcRedirectUriStep.tsx`:
- Around line 170-173: Remove the redundant isDisabled={isLastStep} prop from
the Step.Footer.Continue component in OidcRedirectUriStep, leaving its onClick
behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: fb2289aa-89ba-4454-b5db-ce19d2b15cac
📒 Files selected for processing (76)
.changeset/cool-dragons-march.mdpackages/clerk-js/src/core/resources/EnterpriseConnection.tspackages/localizations/src/ar-SA.tspackages/localizations/src/be-BY.tspackages/localizations/src/bg-BG.tspackages/localizations/src/bn-IN.tspackages/localizations/src/ca-ES.tspackages/localizations/src/cs-CZ.tspackages/localizations/src/da-DK.tspackages/localizations/src/de-DE.tspackages/localizations/src/el-GR.tspackages/localizations/src/en-GB.tspackages/localizations/src/en-US.tspackages/localizations/src/es-CR.tspackages/localizations/src/es-ES.tspackages/localizations/src/es-MX.tspackages/localizations/src/es-UY.tspackages/localizations/src/fa-IR.tspackages/localizations/src/fi-FI.tspackages/localizations/src/fr-FR.tspackages/localizations/src/he-IL.tspackages/localizations/src/hi-IN.tspackages/localizations/src/hr-HR.tspackages/localizations/src/hu-HU.tspackages/localizations/src/id-ID.tspackages/localizations/src/is-IS.tspackages/localizations/src/it-IT.tspackages/localizations/src/ja-JP.tspackages/localizations/src/kk-KZ.tspackages/localizations/src/ko-KR.tspackages/localizations/src/mn-MN.tspackages/localizations/src/ms-MY.tspackages/localizations/src/nb-NO.tspackages/localizations/src/nl-BE.tspackages/localizations/src/nl-NL.tspackages/localizations/src/pl-PL.tspackages/localizations/src/pt-BR.tspackages/localizations/src/pt-PT.tspackages/localizations/src/ro-RO.tspackages/localizations/src/ru-RU.tspackages/localizations/src/sk-SK.tspackages/localizations/src/sr-RS.tspackages/localizations/src/sv-SE.tspackages/localizations/src/ta-IN.tspackages/localizations/src/te-IN.tspackages/localizations/src/th-TH.tspackages/localizations/src/tr-TR.tspackages/localizations/src/uk-UA.tspackages/localizations/src/vi-VN.tspackages/localizations/src/zh-CN.tspackages/localizations/src/zh-TW.tspackages/shared/src/types/elementIds.tspackages/shared/src/types/enterpriseConnection.tspackages/shared/src/types/localization.tspackages/ui/src/components/ConfigureSSO/domain/__tests__/organizationEnterpriseConnection.test.tspackages/ui/src/components/ConfigureSSO/domain/organizationEnterpriseConnection.tspackages/ui/src/components/ConfigureSSO/hooks/__tests__/useOrganizationEnterpriseConnection.test.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/__tests__/ConfigureStep.test.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/index.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/OidcCustomConfigureSteps.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/index.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcCredentialsStep.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcEndpointsConfigurationForm.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcEndpointsStep.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcRedirectUriStep.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/SamlCustomConfigureSteps.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/SamlGoogleConfigureSteps.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/SamlMicrosoftConfigureSteps.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/SamlOktaConfigureSteps.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/shared/IdentityProviderConfigurationForm.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/shared/IdentityProviderConfigurationModes.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/shared/ActiveConnectionAlert.tsxpackages/ui/src/components/ConfigureSSO/steps/ConfigureStep/shared/IdentityProviderConfigurationModes.tsxpackages/ui/src/components/ConfigureSSO/steps/SelectProviderStep.tsxpackages/ui/src/components/ConfigureSSO/steps/__tests__/SelectProviderStep.test.tsxpackages/ui/src/components/ConfigureSSO/types.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
💤 Files with no reviewable changes (1)
- packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/saml/shared/IdentityProviderConfigurationModes.tsx
Comment on lines
+97
to
+109
| try { | ||
| await updateConnection( | ||
| enterpriseConnection.id, | ||
| mode === 'discoveryUrl' | ||
| ? { oidc: { discoveryUrl: discoveryUrlField.value.trim() } } | ||
| : { | ||
| oidc: { | ||
| authUrl: authUrlField.value.trim(), | ||
| tokenUrl: tokenUrlField.value.trim(), | ||
| userInfoUrl: userInfoUrlField.value.trim(), | ||
| }, | ||
| }, | ||
| ); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Avoid sending an empty userInfoUrl when the optional field is blank.
userInfoUrl is optional (per the OIDC provider contract), but the payload always includes userInfoUrl: userInfoUrlField.value.trim() even when the field is empty. Consider omitting the key entirely when blank so an empty string isn't persisted as an explicit value.
🔧 Suggested fix
await updateConnection(
enterpriseConnection.id,
mode === 'discoveryUrl'
? { oidc: { discoveryUrl: discoveryUrlField.value.trim() } }
: {
oidc: {
authUrl: authUrlField.value.trim(),
tokenUrl: tokenUrlField.value.trim(),
- userInfoUrl: userInfoUrlField.value.trim(),
+ ...(userInfoUrlField.value.trim() ? { userInfoUrl: userInfoUrlField.value.trim() } : {}),
},
},
);📝 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
| try { | |
| await updateConnection( | |
| enterpriseConnection.id, | |
| mode === 'discoveryUrl' | |
| ? { oidc: { discoveryUrl: discoveryUrlField.value.trim() } } | |
| : { | |
| oidc: { | |
| authUrl: authUrlField.value.trim(), | |
| tokenUrl: tokenUrlField.value.trim(), | |
| userInfoUrl: userInfoUrlField.value.trim(), | |
| }, | |
| }, | |
| ); | |
| try { | |
| await updateConnection( | |
| enterpriseConnection.id, | |
| mode === 'discoveryUrl' | |
| ? { oidc: { discoveryUrl: discoveryUrlField.value.trim() } } | |
| : { | |
| oidc: { | |
| authUrl: authUrlField.value.trim(), | |
| tokenUrl: tokenUrlField.value.trim(), | |
| ...(userInfoUrlField.value.trim() ? { userInfoUrl: userInfoUrlField.value.trim() } : {}), | |
| }, | |
| }, | |
| ); |
🤖 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
`@packages/ui/src/components/ConfigureSSO/steps/ConfigureStep/oidc/shared/OidcEndpointsStep.tsx`
around lines 97 - 109, Update the non-discovery payload in the OidcEndpointsStep
updateConnection flow to include userInfoUrl only when
userInfoUrlField.value.trim() is non-empty; preserve the existing required
authUrl and tokenUrl fields and discoveryUrl behavior.
Comment on lines
+86
to
+93
| const providerGroups = React.useMemo( | ||
| () => PROVIDER_GROUPS.filter(group => group.id !== 'oidc' || isOIDCFlowEnabled), | ||
| [isOIDCFlowEnabled], | ||
| ); | ||
|
|
||
| const currentCard = c.provider ? toProviderCard(c.provider) : null; | ||
|
|
||
| const [selected, setSelected] = React.useState<ProviderType | null>(currentCard); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Existing OIDC connections desync the selected radio when the flag is off.
providerGroups unconditionally hides the 'oidc' group when isOIDCFlowEnabled is false, but currentCard/selected can still resolve to 'oidc_custom' for an org that already has an OIDC/OAuth-custom connection (these can be provisioned outside this UI, per the OAuth/OIDC provider-prefix contract used elsewhere in this PR). The result: the radiogroup renders with no visibly checked option even though a connection is actively configured — confusing, though "Continue" still functions since selected === currentCard.
🐛 Proposed fix
- const providerGroups = React.useMemo(
- () => PROVIDER_GROUPS.filter(group => group.id !== 'oidc' || isOIDCFlowEnabled),
- [isOIDCFlowEnabled],
- );
-
- const currentCard = c.provider ? toProviderCard(c.provider) : null;
+ const currentCard = c.provider ? toProviderCard(c.provider) : null;
+
+ const providerGroups = React.useMemo(
+ () => PROVIDER_GROUPS.filter(group => group.id !== 'oidc' || isOIDCFlowEnabled || currentCard === 'oidc_custom'),
+ [isOIDCFlowEnabled, currentCard],
+ );Also worth a regression test alongside the existing "OIDC provider (experimental flag)" tests in SelectProviderStep.test.tsx for "flag off + existing OIDC connection → tile still shown as selected."
Also applies to: 169-169
🤖 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 `@packages/ui/src/components/ConfigureSSO/steps/SelectProviderStep.tsx` around
lines 86 - 93, The providerGroups filtering in SelectProviderStep must retain
the OIDC group when currentCard represents an existing oidc_custom connection,
even if isOIDCFlowEnabled is false, so the selected radio remains visible.
Update the condition near providerGroups to account for the existing selected
provider while preserving the flag-based hiding behavior for new OIDC
selections, and add a regression test covering the flag-off existing OIDC
connection case.
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
2 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.
Summary
Testing
CleanShot.2026-07-20.at.17.28.42.mp4