feat: implement SSO activation and enforcement workflow#1725
Merged
Conversation
Add 38 SSO-related translation keys across all supported languages:
- continue-with-sso (login flow)
- sso-configuration, sso-add-provider, sso-new-provider, etc. (configuration)
- sso-domain, sso-metadata-url (form fields)
- sso-dns-verification-* (DNS verification flow)
- sso-status-* (status badges)
- sso-error-* (error messages)
- sso-no-providers (empty states)
Languages: en, de, es, fr, hi, id, it, ja, ko, pl, pt-br, ru, tr, vi, zh-cn
All translations follow semantic grouping with alphabetical ordering within groups.
{domain} placeholder preserved in sso-delete-confirm for all languages.
Generated with AI
Fix 4 issues identified during manual QA verification: 1. SSO redirect preserves 'to' query parameter (login.vue) - handleSsoLogin() now appends ?to= param to /sso-callback redirect - Ensures users land on originally requested protected route 2. Public route mismatch resolved (sso-enforcement.ts) - Changed /forgot-password → /forgot_password in PUBLIC_ROUTES - Matches actual route definition 3. Auto-provisioning integrated (sso-callback.vue) - Import and call useSSOProvisioning() after code exchange - Ensures user provisioned on first SSO login 4. Test API compatibility (sso*.test.ts) - Replace it.concurrent() → it() (Vitest doesn't support concurrent) - All 7 SSO tests now pass All changes verified with lint, typecheck, and test suite. Generated with AI
Enable SSO provider CRUD testing locally without real Supabase Management API token. Mock mode activates when SUPABASE_MANAGEMENT_API_TOKEN is missing or placeholder. Generates fake provider IDs and returns realistic mock responses for all CRUD operations. Production behavior unchanged - real API calls when tokens are configured. - Add isLocalDevMode() detection for missing/placeholder tokens - Add generateMockProviderId() for unique mock IDs - Add mockManagementAPICall() with POST/GET/PATCH/DELETE mocks - Update .env with SSO Management API env var documentation - Document mock mode behavior in learnings notepad Fixes #issue-local-dev-sso-500-error
- Replace recentlyCreatedProvider with pendingVerificationProvider - Show DNS instructions for ANY pending_verification provider, not just newly created - Prioritize recently created provider first, then any pending provider - Block now persists across page refreshes until DNS is verified - Fixes issue where DNS instructions disappeared after page reload
The check-domain endpoint is called during login flow when users are unauthenticated. Removed middlewareAuth and added SECURITY DEFINER to the SQL function to allow anonymous access while maintaining security through RLS. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Enhanced SSO provider management: - Added status field to update schema - Implemented status transition validation (pending_verification → verified → active → disabled) - Preserved dns_verification_token in POST response for UI display This enables the SSO activation workflow for Enterprise customers. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Added comprehensive SSO configuration UI: - Copy buttons for DNS TXT and SAML metadata - Add Provider button in empty state - Activate/Deactivate/Enforce SSO action buttons - Collapsible SAML metadata section (closed by default) - Enterprise plan gating for all SSO features Enables full SSO lifecycle management for Enterprise organizations. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Added English translation keys for SSO management: - Activate/deactivate/reactivate actions - Enforce SSO toggle and tooltip - Success/error messages for all operations Supports the new SSO activation UI. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
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:
📝 WalkthroughWalkthroughImplements end-to-end SSO: adds frontend UI/components and composables, refactors login into a multi-step SSO-aware flow, introduces many backend serverless SSO endpoints and utilities, DB migrations/types for sso_providers, tests, i18n entries across locales, and related env/config updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Frontend as App (Login UI)
participant Backend as API (/private/sso)
participant DB as Database
participant Auth as Supabase Auth / Mgmt
participant IdP as Identity Provider
User->>Browser: enter email
Browser->>Frontend: submit email
Frontend->>Backend: POST /private/sso/check-domain { email }
Backend->>DB: call check_domain_sso(domain)
DB-->>Backend: { has_sso, provider_id, org_id }
Backend-->>Frontend: { has_sso }
alt has_sso == true
Frontend->>Auth: signInWithSSO(domain)
Auth-->>IdP: redirect to IdP
IdP-->>Auth: SAML assertion
Auth->>Frontend: redirect to /sso-callback
Frontend->>Backend: (optional) POST /private/sso/provision-user
Backend->>DB: insert org_user if needed
else
Frontend->>Auth: signInWithPassword
end
Frontend->>User: navigate to dashboard
sequenceDiagram
participant Admin
participant UI as SsoConfiguration
participant Backend as API (/private/sso/providers)
participant Mgmt as Supabase Management API
participant DB as Database
participant DNS as Cloudflare DoH
Admin->>UI: create provider (domain, metadata)
UI->>Backend: POST /private/sso/providers
Backend->>Mgmt: create provider on Supabase Management API
Mgmt-->>Backend: { provider_id }
Backend->>DB: insert sso_providers (pending_verification + dns token)
Backend-->>UI: provider created, show DNS TXT to add
Admin->>DNS: add TXT record _capgo-sso.domain
UI->>Backend: POST /private/sso/verify-dns { provider_id }
Backend->>DNS: verifyDnsTxtRecord(domain, token)
DNS-->>Backend: records & verified
Backend->>DB: update sso_providers status → verified
Backend-->>UI: verification success
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/login.vue (1)
481-516:⚠️ Potential issue | 🟠 MajorDon’t force SSO-only login just because a provider exists.
On Line 481 onward,
hasSsohides the password form entirely. That blocks password login even when SSO exists but enforcement is disabled.💡 Minimal fallback option
<div v-if="hasSso" class="space-y-5"> @@ <div class="text-center"> + <p + class="font-medium text-orange-500 transition-all duration-200 cursor-pointer hover:text-orange-600 hover:underline" + `@click`="hasSso = false" + > + {{ t('sso-dismiss') }} + </p> <p class="font-medium text-orange-500 transition-all duration-200 cursor-pointer hover:text-orange-600 hover:underline" `@click`="goBackToEmail()"> ← {{ t('go-back') }} </p> </div> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/login.vue` around lines 481 - 516, The template currently uses v-if="hasSso" to completely hide the password form when any SSO provider exists; change the condition so the password form is only hidden when SSO is actually enforced (e.g., use v-if="hasSso && isSsoEnforced" or similar boolean), and for non-enforced SSO render the SSO prompt (with handleSsoLogin and goBackToEmail) above or alongside the password FormKit (handlePasswordSubmit) so users can still sign in with a password when enforcement is disabled. Ensure you add or use the enforcement flag (isSsoEnforced / ssoEnforced) in the component state/props and update the v-if/v-else logic accordingly.
🟡 Minor comments (8)
supabase/functions/.env.example-35-35 (1)
35-35:⚠️ Potential issue | 🟡 MinorRemove the extra blank line flagged by dotenv-linter.
Line 35 triggers
ExtraBlankLine; trimming it will keep.env.examplelint-clean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/.env.example` at line 35, Remove the extra blank/empty line in the .env.example file (the stray extra blank line flagged by dotenv-linter) so there are no consecutive blank lines; ensure the file ends with a single newline character and re-run dotenv-linter to confirm the ExtraBlankLine warning is resolved..sisyphus/notepads/enterprise-sso/decisions.md-20-20 (1)
20-20:⚠️ Potential issue | 🟡 MinorClarify token exposure policy to avoid contradictory guidance.
Line 20 says
dns_verification_tokenis not returned by list endpoints, while the PR objective says token preservation in responses. Please make this explicit (e.g., “preserved only for specific provider/detail responses”) so future changes don’t regress behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.sisyphus/notepads/enterprise-sso/decisions.md at line 20, The current note contradicts itself about dns_verification_token exposure—update the decision text to explicitly state the token exposure policy: that dns_verification_token is not returned by LIST endpoints but is preserved and returned only in specific endpoints (e.g., provider-specific or detail GET responses) as described in the PR objective; reference the token name dns_verification_token and the LIST endpoint wording so reviewers can verify consistency and add a short example sentence like “dns_verification_token: not returned in LIST responses; returned only in provider/detail GET responses” to prevent future regressions.messages/tr.json-1357-1357 (1)
1357-1357:⚠️ Potential issue | 🟡 MinorFix typo in SSO configuration description.
Line 1357 uses “Teklu”; this should be “Tekli” for correct Turkish wording.
✏️ Proposed fix
- "sso-configuration-description": "Kuruluşunuz için SAML 2.0 Teklu Oturum Açma özelliğini yapılandırın", + "sso-configuration-description": "Kuruluşunuz için SAML 2.0 Tekli Oturum Açma özelliğini yapılandırın",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@messages/tr.json` at line 1357, The string value for the localization key "sso-configuration-description" contains a typo ("Teklu")—update the value to use the correct Turkish word "Tekli" so the entry reads: Kuruluşunuz için SAML 2.0 Tekli Oturum Açma özelliğini yapılandırın; locate the key "sso-configuration-description" in the messages/tr.json and replace only the misspelled word.messages/pt-br.json-1383-1383 (1)
1383-1383:⚠️ Potential issue | 🟡 MinorCorrect PT-BR article typo in SSO empty-state description.
Line 1383 uses “un provedor”; in PT-BR this should be “um provedor”.
✏️ Proposed fix
- "sso-no-providers-description": "Adicione un provedor SSO para habilitar o Logon Único em sua organização", + "sso-no-providers-description": "Adicione um provedor SSO para habilitar o Logon Único em sua organização",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@messages/pt-br.json` at line 1383, Fix the PT-BR string for key "sso-no-providers-description": change the article "un" to the correct Brazilian Portuguese "um" so the value reads "Adicione um provedor SSO para habilitar o Logon Único em sua organização"..sisyphus/notepads/enterprise-sso/learnings.md-627-627 (1)
627-627:⚠️ Potential issue | 🟡 MinorFix list indentation to satisfy markdownlint MD005.
Line 627 has one extra leading space compared to sibling list items.
🔧 Suggested fix
- - Impact: public-route allowlist is inconsistent; current behavior is mostly masked by `!session` early-return, but route config is incorrect. + - Impact: public-route allowlist is inconsistent; current behavior is mostly masked by `!session` early-return, but route config is incorrect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.sisyphus/notepads/enterprise-sso/learnings.md at line 627, The list item "Impact: public-route allowlist is inconsistent; current behavior is mostly masked by `!session` early-return, but route config is incorrect." has one extra leading space breaking MD005; edit the Markdown so this list line's indentation matches its sibling list items (remove the extra leading space) to restore consistent list indentation and re-run markdownlint to verify the MD005 warning is resolved.supabase/functions/_backend/private/sso/check-domain.ts-26-29 (1)
26-29:⚠️ Potential issue | 🟡 MinorNormalize extracted domain before lookup.
Line 26 uses raw domain casing from the email. Mixed-case domains can miss SSO matches on case-sensitive comparisons.
🔧 Suggested fix
- const domain = email.split('@')[1] + const domain = email.split('@')[1]?.trim().toLowerCase() if (!domain) { return quickError(400, 'invalid_email', 'Email must contain a domain') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/private/sso/check-domain.ts` around lines 26 - 29, The extracted domain (variable domain from email.split in check-domain) must be normalized before SSO lookup to avoid case-sensitive misses; trim whitespace and convert to lowercase (and apply any IDN/punycode normalization if your app supports internationalized domains) and then use that normalized value in your lookup logic instead of the raw domain; keep the same invalid-domain early return using quickError('invalid_email') when no domain exists..sisyphus/plans/enterprise-sso.md-345-356 (1)
345-356:⚠️ Potential issue | 🟡 MinorTask 3 endpoint auth requirements conflict with themselves.
This section says to use
middlewareAuthand also says the endpoint is public for login and should not require user permission checks. Please resolve this contradiction so implementation guidance is unambiguous.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.sisyphus/plans/enterprise-sso.md around lines 345 - 356, The auth instructions for POST /private/sso/check-domain conflict: remove the requirement to enforce authenticated user permissions and instead make the endpoint publicly callable for login flows; implement it using the optional/lenient auth middleware (e.g., middlewareAuthOptional or equivalent) or no auth middleware while retaining any global rate-limiting/abuse protections, ensure you do not perform user permission checks in the handler for check-domain, and keep the sensitive provider fields omitted when returning has_sso/provider_id/org_id..sisyphus/plans/enterprise-sso.md-130-171 (1)
130-171:⚠️ Potential issue | 🟡 MinorAdd language identifiers to fenced code blocks (MD040).
Multiple fenced blocks are missing language tags (for example
text,bash,sql), which triggers markdownlint warnings.Also applies to: 269-284, 328-335, 375-393, 434-449, 504-523, 570-580, 623-641, 680-694, 742-761, 807-822, 864-871, 915-932, 981-998, 1037-1045, 1084-1099, 1141-1158, 1198-1205, 1250-1257, 1298-1305, 1344-1351, 1395-1403
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.sisyphus/plans/enterprise-sso.md around lines 130 - 171, The markdown has multiple fenced code blocks without language identifiers (MD040); update each triple-backtick block (e.g., the Wave plan block and the other blocks called out) to include an appropriate language tag such as text, bash, or sql so markdownlint stops reporting MD040; search for all fenced blocks in enterprise-sso.md (including the Wave 1..FINAL plan block and the other ranges noted) and add the correct language token for each block based on its contents (use "text" for plain ASCII diagrams, "bash" for shell snippets, "sql" for queries, etc.).
🧹 Nitpick comments (10)
supabase/functions/_backend/private/sso/verify-dns.ts (1)
19-22: Add explicitauthTypehandling before continuing request flow.Line 19 only checks presence of auth. Explicit branching on JWT vs API key keeps this endpoint aligned with your backend auth contract and avoids ambiguous behavior across callers.
Based on learnings: Check
c.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/private/sso/verify-dns.ts` around lines 19 - 22, The current code only checks for presence of auth via c.get('auth') before continuing; update the handler to read c.get('auth')?.authType and explicitly branch on allowed types ('apikey' and 'jwt'), e.g., allow request to proceed for 'apikey' and 'jwt' and call quickError(401, 'not_authorized', 'Not authorized') for missing or any other authType; reference the existing auth retrieval (c.get('auth')) and the quickError helper when implementing this explicit branching so the endpoint enforces the backend auth contract.src/auto-imports.d.ts (1)
248-249: Use~/alias for the new composable import types.These new type imports use relative paths on Line 248 and Line 249, which conflicts with the frontend import-path guideline.
Suggested patch
- const useSSOProvisioning: typeof import('./composables/useSSOProvisioning').useSSOProvisioning - const useSSORouting: typeof import('./composables/useSSORouting').useSSORouting + const useSSOProvisioning: typeof import('~/composables/useSSOProvisioning').useSSOProvisioning + const useSSORouting: typeof import('~/composables/useSSORouting').useSSORoutingAs per coding guidelines "Use path alias
~/to referencesrc/directory in imports".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/auto-imports.d.ts` around lines 248 - 249, The generated type imports for useSSOProvisioning and useSSORouting currently use relative paths; update their import type references to use the project path alias by changing the module specifiers to start with "~/", e.g. import types from "~/composables/useSSOProvisioning" and "~/composables/useSSORouting" so the declarations for useSSOProvisioning and useSSORouting in auto-imports.d.ts follow the frontend import-path guideline.src/pages/settings/organization/Security.vue (1)
1402-1407: Hardcoded strings should use i18n for consistency.The SSO Configuration section title and description are hardcoded in English, while other sections on this page use the
t()function for internationalization. Consider using i18n keys for consistency with the rest of the codebase.♻️ Proposed fix to use i18n
<h3 class="text-lg font-semibold dark:text-white text-slate-800"> - SSO Configuration + {{ t('sso-configuration') }} </h3> <p class="mt-1 text-sm text-slate-600 dark:text-slate-400"> - Configure SAML 2.0 Single Sign-On for your organization. Requires Enterprise plan. + {{ t('sso-configuration-description') }} </p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/settings/organization/Security.vue` around lines 1402 - 1407, Replace the hardcoded English strings in the SSO section by using the i18n translation function: update the <h3> element content ("SSO Configuration") and the following <p> description ("Configure SAML 2.0 Single Sign-On for your organization. Requires Enterprise plan.") to call t() with new i18n keys (e.g. t('settings.sso.title') and t('settings.sso.description')); add those keys to the locale files with the corresponding English text and any other locales, and keep the existing class attributes and markup (the <h3> and <p> in Security.vue) untouched aside from replacing the inner text with the t() calls.tests/sso.test.ts (1)
50-117: Consider usingit.concurrent()for parallel test execution.Per coding guidelines, tests within the same file should use
it.concurrent()for parallel execution. Since these tests use isolated test data (unique org ID, customer ID), they should be safe to run concurrently.♻️ Proposed fix to use concurrent tests
describe('[POST] /private/sso/check-domain', () => { - it('should return has_sso=false for non-SSO domain', async () => { + it.concurrent('should return has_sso=false for non-SSO domain', async () => { // ... }) - it('should return 400 for invalid email', async () => { + it.concurrent('should return 400 for invalid email', async () => { // ... }) }) describe('[POST] /private/sso/check-enforcement', () => { - it('should return allowed=true for password auth when no SSO is configured', async () => { + it.concurrent('should return allowed=true for password auth when no SSO is configured', async () => { // ... }) }) describe('[GET] /private/sso/providers/:orgId', () => { - it('should return empty array for org with no SSO providers', async () => { + it.concurrent('should return empty array for org with no SSO providers', async () => { // ... }) - it('should return 401 without authentication', async () => { + it.concurrent('should return 401 without authentication', async () => { // ... }) })As per coding guidelines: "ALL test files run in parallel; use
it.concurrent()instead ofit()to run tests within the same file in parallel for faster CI/CD".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/sso.test.ts` around lines 50 - 117, The tests in this file use synchronous it() calls but should use Jest's it.concurrent() to allow parallel execution; update each test invocation (e.g., the cases with titles "should return has_sso=false for non-SSO domain", "should return 400 for invalid email", "should return allowed=true for password auth when no SSO is configured", "should return empty array for org with no SSO providers", and "should return 401 without authentication") to use it.concurrent(), keeping their bodies unchanged (they call fetchWithRetry/getEndpointUrl and reference SSO_TEST_ORG_ID and authHeaders), so tests remain isolated but run in parallel.tests/sso-verify-dns.test.ts (1)
12-36: Consider usingit.concurrent()for parallel test execution.Per coding guidelines, test files run in parallel, and
it.concurrent()should be used instead ofit()to run tests within the same file in parallel for faster CI/CD.♻️ Proposed fix to use concurrent tests
describe('[POST] /private/sso/verify-dns', () => { - it('should return 404 for non-existent provider', async () => { + it.concurrent('should return 404 for non-existent provider', async () => { const response = await fetchWithRetry(getEndpointUrl('/private/sso/verify-dns'), { method: 'POST', headers: authHeaders, body: JSON.stringify({ provider_id: randomUUID() }), }) expect(response.status).toBe(404) const data = await response.json() as { error: string } expect(data.error).toBe('provider_not_found') }) - it('should return 401 without authentication', async () => { + it.concurrent('should return 401 without authentication', async () => { const response = await fetchWithRetry(getEndpointUrl('/private/sso/verify-dns'), { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify({ provider_id: randomUUID() }), }) expect(response.status).toBe(401) const data = await response.json() as { error: string } expect(data.error).toBe('no_jwt_apikey_or_subkey') }) })As per coding guidelines: "ALL test files run in parallel; use
it.concurrent()instead ofit()to run tests within the same file in parallel for faster CI/CD".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/sso-verify-dns.test.ts` around lines 12 - 36, Replace the two serial tests in this file so they run concurrently: change the calls to it() for the tests named 'should return 404 for non-existent provider' and 'should return 401 without authentication' to use it.concurrent(), leaving the test bodies (fetchWithRetry, getEndpointUrl, authHeaders, assertions) unchanged so the same setup and assertions are executed in parallel.playwright/e2e/sso-login.spec.ts (1)
32-32: Fragile selector for back button.The back button is selected using
page.locator('p').filter({ hasText: '←' }), which is fragile and inconsistent with thedata-testattribute pattern used for other elements in this test. If the arrow character changes or the element type changes, this test will break silently.♻️ Proposed fix to use data-test attribute
- await page.locator('p').filter({ hasText: '←' }).first().click() + await page.locator('[data-test="back-to-email"]').click()This requires adding the
data-test="back-to-email"attribute to the back button in the login component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playwright/e2e/sso-login.spec.ts` at line 32, Replace the fragile arrow-based locator ("page.locator('p').filter({ hasText: '←' }).first()") with a stable data-test selector and add data-test="back-to-email" to the back button in the login component; update the test to click the locator that targets [data-test="back-to-email"] (use the same selector pattern as other tests) so the test no longer depends on the arrow character or element type.src/pages/sso-callback.vue (1)
58-82: Hardcoded strings should use i18n.Multiple strings in the template are hardcoded in English. For consistency with the rest of the codebase and to support internationalization, these should use the
t()function.♻️ Proposed fix to use i18n
<h1 class="text-2xl font-bold text-gray-900 dark:text-white"> - SSO Authentication + {{ t('sso-authentication') }} </h1> <div v-if="isLoading" class="mt-6 space-y-4"> <!-- loader --> <p class="text-gray-700 dark:text-gray-300"> - Completing sign in... + {{ t('sso-completing-sign-in') }} </p> </div> <div v-else class="mt-6 space-y-4"> <p class="font-medium text-red-600"> {{ errorMessage }} </p> <p class="text-gray-700 dark:text-gray-300"> - Please try again or contact your administrator. + {{ t('sso-error-contact-admin') }} </p> <router-link to="/login" class="..." > - Back to Login + {{ t('back-to-login') }} </router-link> </div>You'll also need to add
const { t } = useI18n()to the script setup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sso-callback.vue` around lines 58 - 82, Replace hardcoded English strings in the SSO callback template with i18n calls and add the i18n composable in the script setup: add const { t } = useI18n() in the script setup, then change "SSO Authentication" to t('sso.title'), "Completing sign in..." to t('sso.completingSignIn'), the static "Please try again or contact your administrator." to t('sso.retryOrContactAdmin'), and the router-link text "Back to Login" to t('auth.backToLogin'); keep the dynamic errorMessage binding as-is (it may contain server text) but wrap any other static labels around it with t(...) as needed so all fixed strings use the i18n keys (e.g., sso.title, sso.completingSignIn, sso.retryOrContactAdmin, auth.backToLogin).supabase/functions/_backend/private/sso/check-enforcement.ts (1)
29-30: Usemessage(notcontext) in structured backend logs.Several
cloudlog(...)calls usecontextinstead of the requiredmessagefield shape, which breaks log consistency and filtering.As per coding guidelines: “Use structured logging with
cloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging.”Also applies to: 44-45, 52-53, 58-59, 73-74, 79-80, 92-93, 100-101, 105-106, 109-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/private/sso/check-enforcement.ts` around lines 29 - 30, Replace the incorrect `context` field in all cloudlog calls in check-enforcement.ts with the required `message` field and ensure the requestId is taken via c.get('requestId'); e.g., update each cloudlog call (the ones shown around the return lines and at the other listed locations) to use cloudlog({ requestId: c.get('requestId'), message: 'check_enforcement - SSO auth always allowed', email }) and similarly change the message text for the other cloudlog invocations (lines referenced in the comment) so logs follow the structured shape.src/pages/login.vue (1)
445-447: Use DaisyUI interactive components for the new auth actions.The newly added action controls are plain
<button>elements; project guidelines require DaisyUI interactive components.As per coding guidelines: “Use DaisyUI components (
d-btn,d-input,d-card) for interactive elements in Vue components.”Also applies to: 497-505, 543-547, 617-621
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/login.vue` around lines 445 - 447, The template uses plain <button> elements for auth actions (seen around the v-if="!isDomainChecking" submit button and also at the other noted ranges) but project guidelines require DaisyUI interactive components; replace each plain button with the DaisyUI component (d-btn) while preserving the same bindings and attributes (v-if/v-else, type="submit", data-test values, click handlers, disabled states and any reactive classes like isDomainChecking) and map existing class-based styles to the appropriate d-btn props or classes to keep visual/behavioral parity; do this for the button at v-if="!isDomainChecking" and the other occurrences referenced (lines ~497-505, ~543-547, ~617-621).supabase/functions/_backend/private/sso/providers.ts (1)
70-74: Branch explicitly onauthTypein these handlers.These handlers only check that
authexists. Add an explicitc.get('auth')?.authTypebranch so JWT/API key flows are intentionally handled.As per coding guidelines: “Check
c.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints.”Also applies to: 123-126, 151-154, 238-241
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/private/sso/providers.ts` around lines 70 - 74, The current handlers only verify presence of auth but must branch on c.get('auth')?.authType; update the app.post('/') handler (and the other POST handlers marked in the comment) to read const auth = c.get('auth') and then switch or if/else on auth?.authType (e.g., 'apikey' vs 'jwt'), explicitly allowing the intended flows and calling quickError(401, ...) when the authType is missing or not permitted; ensure the branching logic is applied in the same way in the other handlers referenced so API-key and JWT flows are handled intentionally.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings.local.json:
- Line 14: Replace the overly broad permission entry "Bash(grep:*)" with
least-privilege patterns: find the "Bash(grep:*)" entry and restrict it to the
exact file paths or regex patterns your code needs (for example, specific log or
config directories) or explicit commands rather than a wildcard; update the
allowlist to one or more scoped entries like Bash(grep:<allowed-path-or-regex>)
that only match the expected targets and remove the global wildcard.
- Around line 15-18: Remove the machine-specific absolute paths in
.claude/settings.local.json by replacing entries like "Bash(git -C
/Users/jordanlorho/Capgo/capgo log --oneline -15)" and the other "git -C
/Users/jordanlorho/Capgo/capgo ..." commands with repo-relative or generic
commands (for example "Bash(git -C . log --oneline -15)" or simply "Bash(git log
--oneline -15)"), or remove them entirely and keep truly local
permissions/config out of version control; ensure the changed strings in the
file match the originals so you can find and update each offending entry.
In @.sisyphus/boulder.json:
- Around line 2-8: The committed .sisyphus/boulder.json currently contains
machine-specific and sensitive data: the absolute path in the active_plan value
and raw session IDs in session_ids (and optionally a precise started_at
timestamp). Update the artifact to sanitize these fields: replace active_plan's
absolute path with a portable value (e.g., just the filename "enterprise-sso.md"
or null), remove or redact session_ids (set to an empty array or a placeholder
like ["REDACTED"]), and if desired clear or normalize started_at (set to null or
a non-identifying timestamp); ensure you modify the active_plan, session_ids,
and started_at keys in .sisyphus/boulder.json so no local paths or raw session
tokens are committed.
In `@src/components/organizations/SsoConfiguration.vue`:
- Around line 343-570: Replace raw HTML form controls and buttons with the
project's DaisyUI primitives: swap <input v-model="newDomain"> and <input
v-model="newMetadataUrl"> for d-input components (preserve v-model,
type/placeholder, :disabled="isSubmitting" and classes/aria attributes), wrap
provider blocks and the DNS-verification panel in d-card, and replace all
<button> elements (create, cancel, verifyDns, updateProviderStatus,
toggleEnforceSso checkbox wrapper, deleteProvider, dismiss) with d-btn (preserve
disabled state checks like :disabled="isSubmitting" or :disabled="isVerifying
=== provider.id", click handlers addProvider, verifyDns(provider.id),
updateProviderStatus(provider.id, ...), toggleEnforceSso(provider),
deleteProvider(provider), recentlyCreatedId = null, and keep Spinner/Icon
components and title/tooltip text). Ensure d-input/d-btn support the same
classes/props (loading state, size, icon slots) so existing bindings (newDomain,
newMetadataUrl, isSubmitting, isVerifying, pendingVerificationProvider,
providers) and accessibility attributes remain unchanged.
- Around line 329-574: The component currently renders SSO controls without
verifying the stricter backend permission; add a frontend check for the
org.manage_sso permission and gate UI/actions accordingly: create a
computed/prop (e.g., hasManageSso or canManageSso) and use it to wrap or guard
the Add Provider form (showAddForm), pendingVerificationProvider actions
(verifyDns), provider action buttons (calls to addProvider,
updateProviderStatus, deleteProvider, toggleEnforceSso) and the DNS copy/verify
controls so users without org.manage_sso cannot see or interact with these
controls; alternatively, update the parent Security.vue permission gate to check
for org.manage_sso instead of org.update_settings so SsoConfiguration.vue is
never mounted for insufficient-permission users. Include references to methods
addProvider, verifyDns, updateProviderStatus, deleteProvider, toggleEnforceSso
and reactive state pendingVerificationProvider/showAddForm when implementing the
guard.
In `@src/composables/useSSOProvisioning.ts`:
- Around line 70-93: The code in provisionUser currently only logs and returns
when data.has_sso && data.org_id is true, so it never actually provisions org
membership; replace the early return with a call to the backend provisioning
endpoint (using session.access_token) to pre-link the user to the returned
org_id/provider_id. Specifically, in the block where you inspect data.has_sso,
call a POST (or the existing server route for SSO provisioning) with body {
email, org_id: data.org_id, provider_id: data.provider_id } and await the
response, handle non-ok errors (log and continue/fail gracefully), and only
return after the provisioning request completes successfully or has been
handled; keep using the same headers pattern as the check-domain request and
ensure provisionUser processes the provisioning result instead of just logging.
In `@src/modules/sso-enforcement.ts`:
- Around line 84-94: The cache is being marked valid even when the enforcement
HTTP response is not ok; change the logic in the SSO enforcement flow so
setCacheValid() is only called when response.ok is true AND the parsed
SsoEnforcementResponse has data.allowed === true. Locate the block that checks
response.ok and data.allowed (uses response.ok, SsoEnforcementResponse,
clearSsoEnforcementCache(), supabase.auth.signOut(),
next('/login?sso_required=true')), and move the setCacheValid() call inside the
branch where response.ok && data.allowed are both satisfied so failed or non-OK
responses never set the cache.
In `@src/pages/sso-callback.vue`:
- Around line 40-41: The code uses route.query.to directly in router.replace
(redirectTo and router.replace) causing an open redirect; validate and sanitize
the parameter before using it: only accept internal paths (e.g., ensure
redirectTo is a relative path starting with a single '/' and not starting with
'//' or containing a scheme like 'http:'), or match it against a whitelist of
allowed paths, and fall back to '/dashboard' if validation fails. Update the
logic around route.query.to and router.replace to perform this check and only
call router.replace with a validated path.
In `@supabase/functions/_backend/private/sso/check-enforcement.ts`:
- Around line 83-106: Remove the special-case super-admin bypass in the SSO
enforcement flow: delete the isSuperAdmin check and the early return that
returns c.json({ allowed: true }) (the block referencing isSuperAdmin, cloudlog
'super admin bypass', and that return). Keep the existing role query and its
error handling (roleData/roleError and the PGRST116 check) but ensure that after
determining roleData there is no conditional that exempts org_super_admin from
SSO enforcement; always log the enforced SSO outcome and return c.json({
allowed: false, reason: 'sso_enforced' }) when SSO is enforced.
In `@supabase/functions/_backend/private/sso/prelink.ts`:
- Around line 66-72: The handler currently uses the service-role client via
supabaseAdmin(c) (see variable admin and calls to .from('sso_providers') and
later admin.auth.api.listUsers / admin.auth.getUserById), which bypasses RLS;
replace those database reads with the authenticated client (use the
request-bound Supabase client — e.g., the same client used for other user
requests) so the .from('sso_providers').select(...).eq('id',
provider_id).single() call is executed under RLS and not via supabaseAdmin(c).
For auth.admin-only operations (listUsers, getUserById) do not call admin.auth
from this user-facing endpoint; instead move those actions behind a separate
internal endpoint authenticated by an API secret or perform them only in backend
processes that use the service role. Ensure all references to admin for DB reads
are swapped to the authenticated client and that only the isolated internal
route uses supabaseAdmin for privileged auth operations.
In `@supabase/functions/_backend/private/sso/providers.ts`:
- Around line 262-273: The current flow deletes the database row before calling
deleteSSOProvider which risks cross-system inconsistency if external deletion
fails; change the order so you first call deleteSSOProvider(c,
provider.provider_id) (only when provider.provider_id exists), handle and
surface any error from that call (use quickError to return a 500 and include the
external error), and only after successful external deletion perform the
supabase .from('sso_providers').delete().eq('id', id) and then handle
deleteError as before; ensure you reference deleteSSOProvider,
provider.provider_id, id, quickError and the supabase delete call when making
the change.
- Around line 94-115: The external provider (created via createSSOProvider) can
be left orphaned if the DB insert (via
supabaseWithAuth().from('sso_providers').insert(...)) fails; modify the flow so
that if the insert returns error or no data you call the management API to
delete/rollback the created provider (e.g., a deleteSSOProvider or
managementProvider.delete using managementProvider.id) before calling
quickError, and ensure this cleanup runs in the error branch (and in any thrown
exception path) so the external provider is removed on DB failure; reference
createSSOProvider, managementProvider.id,
supabaseWithAuth(...).from('sso_providers').insert(...),
generateDnsVerificationToken and quickError when adding the rollback.
In `@supabase/functions/_backend/private/sso/verify-dns.ts`:
- Around line 64-70: The update always sets status: 'verified' which can
downgrade an already-active provider; modify the logic in verify-dns so you
first read the current row (from sso_providers by provider_id) or check its
status before updating, and only set status: 'verified' when the existing status
is not 'active' (but always update dns_verified_at). Use the same supabase
client and provider_id symbol to locate the code that currently calls .update({
status: 'verified', dns_verified_at: ... }). Ensure the update either omits the
status field if current status === 'active' or explicitly preserves the existing
status when performing the .update.
In `@supabase/functions/_backend/utils/dns-verification.ts`:
- Around line 31-36: The code allows an empty expectedToken to pass because
''.includes('') is true; update the validation to reject empty/whitespace tokens
and require exact matches when checking TXT records: ensure expectedToken is a
non-empty string (e.g., typeof expectedToken === 'string' &&
expectedToken.trim().length > 0) and replace loose checks that use
includes(expectedToken) with strict equality or a token-exact comparison (e.g.,
txtRecord === expectedToken or txtRecord.split('"').join('') === expectedToken)
wherever expectedToken is compared (the checks around expectedToken at the
initial validation block and the comparison at the TXT answers processing, lines
referencing expectedToken).
- Around line 46-51: The Cloudflare DoH fetch in dns-verification.ts currently
has no timeout; wrap the request in an AbortController, start a timer (e.g.,
3–5s) that calls controller.abort(), pass signal to the existing fetch call that
creates `response`, and clear the timer after fetch completes; also catch the
abort/timeout error (AbortError) and surface a clear, descriptive error so the
caller can handle timeouts from the DNS-over-HTTPS request.
In `@supabase/functions/_backend/utils/supabase-management.ts`:
- Around line 34-39: The current isLocalDevMode in supabase-management.ts treats
missing SUPABASE_MANAGEMENT_API_TOKEN or SUPABASE_PROJECT_REF as local-dev and
enables mock mode; change this to "fail closed": use getEnv to read token and
projectRef and if either is missing or empty throw a clear error (e.g., throw
new Error(...)) so misconfiguration surfaces, but still allow explicit local-dev
values to enable mock mode (keep the token === 'local-dev-token' || projectRef
=== 'local-dev-ref' check to return true); otherwise return false. Target the
isLocalDevMode function and getEnv usage when making this change.
In `@supabase/migrations/20260223000001_add_sso_providers.sql`:
- Around line 123-137: Make domain case-insensitive by using the citext type for
public.sso_providers.domain (or alternatively enforce uniqueness via a UNIQUE
index on lower(domain)); change the column definition from "domain text NOT NULL
UNIQUE" to "domain citext NOT NULL UNIQUE" (ensure the citext extension is
created earlier in the migration) and remove the redundant non-unique index
CREATE INDEX idx_sso_providers_domain ON public.sso_providers (domain) so you
only have the case-insensitive unique constraint on domain.
---
Outside diff comments:
In `@src/pages/login.vue`:
- Around line 481-516: The template currently uses v-if="hasSso" to completely
hide the password form when any SSO provider exists; change the condition so the
password form is only hidden when SSO is actually enforced (e.g., use
v-if="hasSso && isSsoEnforced" or similar boolean), and for non-enforced SSO
render the SSO prompt (with handleSsoLogin and goBackToEmail) above or alongside
the password FormKit (handlePasswordSubmit) so users can still sign in with a
password when enforcement is disabled. Ensure you add or use the enforcement
flag (isSsoEnforced / ssoEnforced) in the component state/props and update the
v-if/v-else logic accordingly.
---
Minor comments:
In @.sisyphus/notepads/enterprise-sso/decisions.md:
- Line 20: The current note contradicts itself about dns_verification_token
exposure—update the decision text to explicitly state the token exposure policy:
that dns_verification_token is not returned by LIST endpoints but is preserved
and returned only in specific endpoints (e.g., provider-specific or detail GET
responses) as described in the PR objective; reference the token name
dns_verification_token and the LIST endpoint wording so reviewers can verify
consistency and add a short example sentence like “dns_verification_token: not
returned in LIST responses; returned only in provider/detail GET responses” to
prevent future regressions.
In @.sisyphus/notepads/enterprise-sso/learnings.md:
- Line 627: The list item "Impact: public-route allowlist is inconsistent;
current behavior is mostly masked by `!session` early-return, but route config
is incorrect." has one extra leading space breaking MD005; edit the Markdown so
this list line's indentation matches its sibling list items (remove the extra
leading space) to restore consistent list indentation and re-run markdownlint to
verify the MD005 warning is resolved.
In @.sisyphus/plans/enterprise-sso.md:
- Around line 345-356: The auth instructions for POST /private/sso/check-domain
conflict: remove the requirement to enforce authenticated user permissions and
instead make the endpoint publicly callable for login flows; implement it using
the optional/lenient auth middleware (e.g., middlewareAuthOptional or
equivalent) or no auth middleware while retaining any global rate-limiting/abuse
protections, ensure you do not perform user permission checks in the handler for
check-domain, and keep the sensitive provider fields omitted when returning
has_sso/provider_id/org_id.
- Around line 130-171: The markdown has multiple fenced code blocks without
language identifiers (MD040); update each triple-backtick block (e.g., the Wave
plan block and the other blocks called out) to include an appropriate language
tag such as text, bash, or sql so markdownlint stops reporting MD040; search for
all fenced blocks in enterprise-sso.md (including the Wave 1..FINAL plan block
and the other ranges noted) and add the correct language token for each block
based on its contents (use "text" for plain ASCII diagrams, "bash" for shell
snippets, "sql" for queries, etc.).
In `@messages/pt-br.json`:
- Line 1383: Fix the PT-BR string for key "sso-no-providers-description": change
the article "un" to the correct Brazilian Portuguese "um" so the value reads
"Adicione um provedor SSO para habilitar o Logon Único em sua organização".
In `@messages/tr.json`:
- Line 1357: The string value for the localization key
"sso-configuration-description" contains a typo ("Teklu")—update the value to
use the correct Turkish word "Tekli" so the entry reads: Kuruluşunuz için SAML
2.0 Tekli Oturum Açma özelliğini yapılandırın; locate the key
"sso-configuration-description" in the messages/tr.json and replace only the
misspelled word.
In `@supabase/functions/_backend/private/sso/check-domain.ts`:
- Around line 26-29: The extracted domain (variable domain from email.split in
check-domain) must be normalized before SSO lookup to avoid case-sensitive
misses; trim whitespace and convert to lowercase (and apply any IDN/punycode
normalization if your app supports internationalized domains) and then use that
normalized value in your lookup logic instead of the raw domain; keep the same
invalid-domain early return using quickError('invalid_email') when no domain
exists.
In `@supabase/functions/.env.example`:
- Line 35: Remove the extra blank/empty line in the .env.example file (the stray
extra blank line flagged by dotenv-linter) so there are no consecutive blank
lines; ensure the file ends with a single newline character and re-run
dotenv-linter to confirm the ExtraBlankLine warning is resolved.
---
Nitpick comments:
In `@playwright/e2e/sso-login.spec.ts`:
- Line 32: Replace the fragile arrow-based locator ("page.locator('p').filter({
hasText: '←' }).first()") with a stable data-test selector and add
data-test="back-to-email" to the back button in the login component; update the
test to click the locator that targets [data-test="back-to-email"] (use the same
selector pattern as other tests) so the test no longer depends on the arrow
character or element type.
In `@src/auto-imports.d.ts`:
- Around line 248-249: The generated type imports for useSSOProvisioning and
useSSORouting currently use relative paths; update their import type references
to use the project path alias by changing the module specifiers to start with
"~/", e.g. import types from "~/composables/useSSOProvisioning" and
"~/composables/useSSORouting" so the declarations for useSSOProvisioning and
useSSORouting in auto-imports.d.ts follow the frontend import-path guideline.
In `@src/pages/login.vue`:
- Around line 445-447: The template uses plain <button> elements for auth
actions (seen around the v-if="!isDomainChecking" submit button and also at the
other noted ranges) but project guidelines require DaisyUI interactive
components; replace each plain button with the DaisyUI component (d-btn) while
preserving the same bindings and attributes (v-if/v-else, type="submit",
data-test values, click handlers, disabled states and any reactive classes like
isDomainChecking) and map existing class-based styles to the appropriate d-btn
props or classes to keep visual/behavioral parity; do this for the button at
v-if="!isDomainChecking" and the other occurrences referenced (lines ~497-505,
~543-547, ~617-621).
In `@src/pages/settings/organization/Security.vue`:
- Around line 1402-1407: Replace the hardcoded English strings in the SSO
section by using the i18n translation function: update the <h3> element content
("SSO Configuration") and the following <p> description ("Configure SAML 2.0
Single Sign-On for your organization. Requires Enterprise plan.") to call t()
with new i18n keys (e.g. t('settings.sso.title') and
t('settings.sso.description')); add those keys to the locale files with the
corresponding English text and any other locales, and keep the existing class
attributes and markup (the <h3> and <p> in Security.vue) untouched aside from
replacing the inner text with the t() calls.
In `@src/pages/sso-callback.vue`:
- Around line 58-82: Replace hardcoded English strings in the SSO callback
template with i18n calls and add the i18n composable in the script setup: add
const { t } = useI18n() in the script setup, then change "SSO Authentication" to
t('sso.title'), "Completing sign in..." to t('sso.completingSignIn'), the static
"Please try again or contact your administrator." to
t('sso.retryOrContactAdmin'), and the router-link text "Back to Login" to
t('auth.backToLogin'); keep the dynamic errorMessage binding as-is (it may
contain server text) but wrap any other static labels around it with t(...) as
needed so all fixed strings use the i18n keys (e.g., sso.title,
sso.completingSignIn, sso.retryOrContactAdmin, auth.backToLogin).
In `@supabase/functions/_backend/private/sso/check-enforcement.ts`:
- Around line 29-30: Replace the incorrect `context` field in all cloudlog calls
in check-enforcement.ts with the required `message` field and ensure the
requestId is taken via c.get('requestId'); e.g., update each cloudlog call (the
ones shown around the return lines and at the other listed locations) to use
cloudlog({ requestId: c.get('requestId'), message: 'check_enforcement - SSO auth
always allowed', email }) and similarly change the message text for the other
cloudlog invocations (lines referenced in the comment) so logs follow the
structured shape.
In `@supabase/functions/_backend/private/sso/providers.ts`:
- Around line 70-74: The current handlers only verify presence of auth but must
branch on c.get('auth')?.authType; update the app.post('/') handler (and the
other POST handlers marked in the comment) to read const auth = c.get('auth')
and then switch or if/else on auth?.authType (e.g., 'apikey' vs 'jwt'),
explicitly allowing the intended flows and calling quickError(401, ...) when the
authType is missing or not permitted; ensure the branching logic is applied in
the same way in the other handlers referenced so API-key and JWT flows are
handled intentionally.
In `@supabase/functions/_backend/private/sso/verify-dns.ts`:
- Around line 19-22: The current code only checks for presence of auth via
c.get('auth') before continuing; update the handler to read
c.get('auth')?.authType and explicitly branch on allowed types ('apikey' and
'jwt'), e.g., allow request to proceed for 'apikey' and 'jwt' and call
quickError(401, 'not_authorized', 'Not authorized') for missing or any other
authType; reference the existing auth retrieval (c.get('auth')) and the
quickError helper when implementing this explicit branching so the endpoint
enforces the backend auth contract.
In `@tests/sso-verify-dns.test.ts`:
- Around line 12-36: Replace the two serial tests in this file so they run
concurrently: change the calls to it() for the tests named 'should return 404
for non-existent provider' and 'should return 401 without authentication' to use
it.concurrent(), leaving the test bodies (fetchWithRetry, getEndpointUrl,
authHeaders, assertions) unchanged so the same setup and assertions are executed
in parallel.
In `@tests/sso.test.ts`:
- Around line 50-117: The tests in this file use synchronous it() calls but
should use Jest's it.concurrent() to allow parallel execution; update each test
invocation (e.g., the cases with titles "should return has_sso=false for non-SSO
domain", "should return 400 for invalid email", "should return allowed=true for
password auth when no SSO is configured", "should return empty array for org
with no SSO providers", and "should return 401 without authentication") to use
it.concurrent(), keeping their bodies unchanged (they call
fetchWithRetry/getEndpointUrl and reference SSO_TEST_ORG_ID and authHeaders), so
tests remain isolated but run in parallel.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (59)
.claude/settings.local.json.sisyphus/boulder.json.sisyphus/evidence/task-1-migration-applies.txt.sisyphus/evidence/task-17-env-vars.txt.sisyphus/evidence/task-2-module-exports.txt.sisyphus/evidence/task-3-no-sso.txt.sisyphus/evidence/task-4-dns-fail.txt.sisyphus/evidence/task-5-create-provider.txt.sisyphus/evidence/task-6-prelink-uuid.txt.sisyphus/evidence/task-7-enforced-block.txt.sisyphus/evidence/task-8-enterprise-pass.txt.sisyphus/notepads/enterprise-sso/decisions.md.sisyphus/notepads/enterprise-sso/issues.md.sisyphus/notepads/enterprise-sso/learnings.md.sisyphus/plans/enterprise-sso.mdmessages/de.jsonmessages/en.jsonmessages/es.jsonmessages/fr.jsonmessages/hi.jsonmessages/id.jsonmessages/it.jsonmessages/ja.jsonmessages/ko.jsonmessages/pl.jsonmessages/pt-br.jsonmessages/ru.jsonmessages/tr.jsonmessages/vi.jsonmessages/zh-cn.jsonplaywright/e2e/sso-login.spec.tssrc/auto-imports.d.tssrc/components.d.tssrc/components/organizations/SsoConfiguration.vuesrc/composables/useSSOProvisioning.tssrc/composables/useSSORouting.tssrc/modules/sso-enforcement.tssrc/pages/login.vuesrc/pages/settings/organization/Security.vuesrc/pages/sso-callback.vuesrc/route-map.d.tssrc/types/supabase.types.tssupabase/functions/.envsupabase/functions/.env.examplesupabase/functions/_backend/private/sso/check-domain.tssupabase/functions/_backend/private/sso/check-enforcement.tssupabase/functions/_backend/private/sso/prelink.tssupabase/functions/_backend/private/sso/providers.tssupabase/functions/_backend/private/sso/verify-dns.tssupabase/functions/_backend/utils/dns-verification.tssupabase/functions/_backend/utils/plan-gating.tssupabase/functions/_backend/utils/supabase-management.tssupabase/functions/_backend/utils/supabase.types.tssupabase/functions/private/index.tssupabase/migrations/20260223000001_add_sso_providers.sqlsupabase/migrations/20260228154639_fix_check_domain_sso_security.sqlsupabase/seed.sqltests/sso-verify-dns.test.tstests/sso.test.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/functions/_backend/private/sso/verify-dns.ts`:
- Around line 34-41: The code in verify-dns.ts uses supabaseWithAuth
(supabaseWithAuth(...)) to query and update the sso_providers table; replace
those calls with the approved PostgreSQL client by calling getPgClient() or
getDrizzleClient() from utils/pg.ts and reimplement the read and update logic
(the select/.single() read for provider by provider_id and subsequent update
statements) using parameterized pg queries or Drizzle methods so all DB access
(reads/updates against sso_providers using provider_id, dns_verification_token,
status, etc.) uses the pg/drizzle client instead of supabaseWithAuth. Ensure you
preserve the same error handling and return values when translating the supabase
.from(...).select(...) and update calls to the new client.
- Around line 78-81: There is a duplicate/out-of-scope check for updateError
after the block where updateError is declared; remove the dangling if
(updateError) { cloudlog(...); quickError(...) } that appears after the closing
brace of the provider.status === 'pending_verification' block so only the
original check inside that block remains (leave the existing cloudlog(...) and
quickError(...) within the if (provider.status === 'pending_verification')
branch and delete the extra check referencing updateError outside that scope).
Prevent enabling SSO enforcement when the provider status is not 'active'. Auto-reset enforce_sso to false when provider is disabled to prevent stale enforcement blocking users.
Replace the errors array (containing raw user IDs) in the response with an error_count. User IDs are now only logged server-side via cloudlogErr, preventing information leakage in API responses.
Replace raw pg client for provider lookup with supabaseAdmin as defense-in-depth. Raw pg is still used for the atomic conditional status update where it is needed.
Add ALLOWED_ATTRIBUTE_KEYS whitelist (email, first_name, last_name, display_name, groups, role, phone) and enforce a 256-char max on values. Reject unknown keys and empty values to prevent injection of arbitrary SAML attribute mappings. Security: M2 finding from SSO audit.
Apply sanitizeProvider() to the POST creation response. The function was already used on GET and PATCH responses but the POST handler returned raw data including the dns_verification_token secret. Security: L1 finding from SSO audit.
Add rate limiting (10 requests/minute per IP) to the unauthenticated check-domain endpoint to prevent mass domain enumeration. Uses the existing CacheHelper + getClientIP pattern from rate_limit.ts. Returns 429 when limit is exceeded. Security: M4 finding from SSO audit.
Show SSO config only for Enterprise plan orgs. Non-Enterprise orgs see a placeholder with an upgrade button linking to the plans page.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
supabase/functions/_backend/private/sso/prelink-internal.ts (1)
158-163: Remove redundantnormalizedDomainredeclaration.
normalizedDomainis already defined and normalized at line 102. This inner redeclaration from the rawdomainvariable is unnecessary and slightly inconsistent (it omits the.trim()call).♻️ Suggested fix
for (const user of users) { const userEmail = user.email?.toLowerCase() ?? '' - const normalizedDomain = domain.toLowerCase() if (!userEmail.endsWith(`@${normalizedDomain}`)) { continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/private/sso/prelink-internal.ts` around lines 158 - 163, In the for-loop over users, remove the inner redeclaration "const normalizedDomain = domain.toLowerCase()" and instead use the previously computed normalizedDomain (the one normalized with .trim() earlier) when checking userEmail.endsWith; this eliminates redundancy and ensures the same trimmed/lowercased domain value is used consistently in the userEmail check inside the loop.supabase/functions/_backend/private/sso/providers.ts (1)
139-141: Use structured backend logging for rollback failures.These
console.errorcalls drop request context. Please log cleanup failures via structured backend logging withrequestId.🔧 Suggested fix
-import { version } from '../../utils/version.ts' +import { cloudlogErr } from '../../utils/logging.ts' +import { version } from '../../utils/version.ts' @@ await deleteSSOProvider(c, managementProvider.id).catch((cleanupError) => { - console.error('Failed to cleanup external SSO provider after DB insert failure:', cleanupError) + cloudlogErr({ + requestId: c.get('requestId'), + message: 'Failed to cleanup external SSO provider after DB insert failure', + providerId: managementProvider.id, + error: cleanupError instanceof Error ? cleanupError.message : String(cleanupError), + }) }) @@ await deleteSSOProvider(c, managementProvider.id).catch((cleanupError) => { - console.error('Failed to cleanup external SSO provider after exception:', cleanupError) + cloudlogErr({ + requestId: c.get('requestId'), + message: 'Failed to cleanup external SSO provider after exception', + providerId: managementProvider.id, + error: cleanupError instanceof Error ? cleanupError.message : String(cleanupError), + }) })As per coding guidelines: "Use structured logging with
cloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging."Also applies to: 149-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/private/sso/providers.ts` around lines 139 - 141, Replace the console.error rollback logs with structured backend logging using cloudlog and the requestId from the context: when catching errors from deleteSSOProvider(c, managementProvider.id) (and the similar handler around the other cleanup at lines referenced), call cloudlog({ requestId: c.get('requestId'), message: 'Failed to cleanup external SSO provider after DB insert failure', err: cleanupError }) (or equivalent structured payload) so the cleanup failure preserves request context and error details; update both occurrences that currently use console.error to use cloudlog with c.get('requestId').
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/composables/useSSOProvisioning.ts`:
- Around line 73-76: When the provisionResponse is not ok in
useSSOProvisioning.ts (the block handling /private/sso/provision-user), set the
composable's error.value to the parsed error (e.g., error.value = errorData || {
message: 'Provisioning failed', status: provisionResponse.status }) instead of
only console.error, and ensure you abort further success handling (return or
throw) so callers see the failure; apply the same change to the similar failure
handling around the provisionResponse at lines 82-84 so both failure branches
populate error.value with useful details.
- Around line 43-60: The current early return in useSSOProvisioning checks
org_membership from supabase (org_users query -> orgMembership) and returns
whenever the user belongs to any org, which is too broad; remove that
short-circuit (do not return when orgMembership is truthy) or refine it to only
skip when orgMembership.org_id matches the specific SSO target org id if you
have a resolved target id, and always proceed to call the backend provisioning
endpoint (the existing provisioning API call) so the server can decide
membership for the resolved provider org; update the logic around the supabase
.from('org_users') query, orgMembership handling, and the place where the
provisioning request is made in useSSOProvisioning accordingly.
In `@src/modules/sso-enforcement.ts`:
- Around line 89-99: The guard currently only enforces SSO when response.ok is
true, letting 4xx/5xx responses pass; update the check around fetch response
handling in the SSO enforcement function so that any non-ok response
(response.ok === false) is treated like a failure: call
clearSsoEnforcementCache(), await supabase.auth.signOut(), and return
next('/login?sso_required=true') instead of falling through; keep the existing
behavior of setCacheValid(userId) only when response.ok and data.allowed are
true and apply the same non-ok handling for the other similar response branch
that uses the same logic (refer to response, SsoEnforcementResponse,
setCacheValid, clearSsoEnforcementCache, supabase.auth.signOut, and next).
In `@src/pages/settings/organization/Security.vue`:
- Around line 1418-1422: Replace the hardcoded English UI strings in
Security.vue (e.g., "SSO Configuration", "Configure SAML 2.0 Single Sign-On for
your organization." and the CTA/labels around the subsequent blocks) with i18n
lookups using the t(...) helper (for example: t('settings.security.ssoTitle'),
t('settings.security.ssoDescription'), etc.), update the template bindings where
those strings are used (headers, paragraph text and CTA/button labels), and add
corresponding keys and translations to the locale JSON/YAML files so the new
keys exist for all supported languages.
In `@supabase/functions/_backend/private/sso/check-domain.ts`:
- Around line 22-23: The branch in check-domain.ts that returns false when
getClientIP() yields 'unknown' bypasses your rate-limiting; instead of returning
early, treat 'unknown' as a valid identifier for throttling (e.g., set ip =
'unknown' and proceed) or derive a fallback key (like a normalized Origin/Host
header or a global "unknown" bucket) so the request still passes through your
existing rate-limiting/throttling logic; update the code around getClientIP() in
check-domain.ts to remove the early return and ensure the rate limiter receives
a concrete key for the IP/fallback case.
In `@supabase/functions/_backend/private/sso/check-enforcement.ts`:
- Around line 17-35: The endpoint currently trusts client-supplied body fields
(email, auth_type) which allows spoofing; change the logic in check-enforcement
to ignore these request body fields for enforcement decisions and instead derive
the authenticated user’s attributes (email and auth_type) from the server-side
auth/session context (e.g., the authenticated user object available on c,
c.auth, or c.get('user')) before running validation or enforcement; keep
parseBody and bodySchema for optional input validation if needed but do not use
parsed email/auth_type for allow/deny decisions—use the server-derived values,
log them via requestId/cloudlog, and handle missing server-side identity with a
clear error path.
In `@supabase/functions/_backend/private/sso/prelink-internal.ts`:
- Around line 66-74: The fetch call in prelink-internal.ts currently has no
timeout and can hang; wrap the DELETE request to the GoTrue service in an
AbortController: create an AbortController, pass controller.signal to fetch (the
existing fetch invocation), set a short timeout (e.g., 5s) with setTimeout that
calls controller.abort(), and clear that timeout after fetch completes; also
catch the abort error in the surrounding try/catch so aborted requests are
treated as a timeout case rather than indefinite hangs.
- Around line 95-131: The request extracts org_id but never verifies it against
the provider; update the provider verification to ensure the provider belongs to
that org by including org_id in the lookup and/or validating the returned
provider's org_id: when calling
supabaseAdmin(...).from('sso_providers').select(...).eq('id', provider_id) add a
filter for org_id (e.g., .eq('org_id', org_id)) or retain the existing query and
after obtaining providerCheck assert providerCheck.org_id === org_id, logging a
clear error with requestId and returning a quickError like 'provider_not_found'
or 'org_mismatch' if it doesn't match; keep the existing domain/status checks
afterwards.
In `@supabase/functions/_backend/private/sso/provision-user.ts`:
- Around line 14-24: The auth block currently only verifies presence of auth and
userId but doesn't ensure the auth type is JWT; update the handler to explicitly
check auth.authType === 'jwt' (using c.get('auth')?.authType or the existing
auth variable) and return quickError(401, 'not_authorized', 'Not authorized') or
a similar 401 when the auth type is not 'jwt' before any provisioning logic
(i.e., immediately after acquiring auth and before using userId/requestId in
this file's provisioning code such as in provision-user.ts).
- Around line 92-103: The insert into org_users currently treats any insertError
as fatal; update the error handling around (admin as
any).from('org_users').insert(...) so that if insertError indicates a
unique-constraint/duplicate-membership (e.g., SQLSTATE '23505' or message
containing 'duplicate' depending on Supabase error shape) you treat it as a
successful idempotent outcome (do not call quickError), otherwise log with
cloudlogErr({ requestId, message: 'Failed to insert user into org_users',
userId, orgId: provider.org_id, error: insertError }) and return quickError(500,
'provision_failed', 'Failed to provision user to organization') as before; keep
the same context (userId, provider.org_id) in logs.
---
Nitpick comments:
In `@supabase/functions/_backend/private/sso/prelink-internal.ts`:
- Around line 158-163: In the for-loop over users, remove the inner
redeclaration "const normalizedDomain = domain.toLowerCase()" and instead use
the previously computed normalizedDomain (the one normalized with .trim()
earlier) when checking userEmail.endsWith; this eliminates redundancy and
ensures the same trimmed/lowercased domain value is used consistently in the
userEmail check inside the loop.
In `@supabase/functions/_backend/private/sso/providers.ts`:
- Around line 139-141: Replace the console.error rollback logs with structured
backend logging using cloudlog and the requestId from the context: when catching
errors from deleteSSOProvider(c, managementProvider.id) (and the similar handler
around the other cleanup at lines referenced), call cloudlog({ requestId:
c.get('requestId'), message: 'Failed to cleanup external SSO provider after DB
insert failure', err: cleanupError }) (or equivalent structured payload) so the
cleanup failure preserves request context and error details; update both
occurrences that currently use console.error to use cloudlog with
c.get('requestId').
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/composables/useSSOProvisioning.tssrc/modules/sso-enforcement.tssrc/pages/settings/organization/Security.vuesupabase/functions/_backend/private/sso/check-domain.tssupabase/functions/_backend/private/sso/check-enforcement.tssupabase/functions/_backend/private/sso/prelink-internal.tssupabase/functions/_backend/private/sso/providers.tssupabase/functions/_backend/private/sso/provision-user.tssupabase/functions/_backend/private/sso/verify-dns.tssupabase/functions/_backend/utils/supabase-management.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- supabase/functions/_backend/private/sso/verify-dns.ts
…oning The client-side check returned early if the user belonged to ANY org, preventing the server from adding them to the SSO provider's specific org. Now always calls the backend, which checks membership against the resolved target org and handles idempotency.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/composables/useSSOProvisioning.ts (1)
57-68:⚠️ Potential issue | 🟡 MinorSurface provisioning failures via
error.valueand stop success flow.
/private/sso/provision-userfailures are only logged right now, so callers can’t react to a failed auto-provision attempt.💡 Suggested change
if (!provisionResponse.ok) { const errorData = await provisionResponse.json().catch(() => ({ error: 'Unknown error' })) console.error('SSO provisioning: provision request failed', provisionResponse.status, errorData) + error.value = typeof errorData?.error === 'string' + ? errorData.error + : 'Failed to provision organization membership' + return } @@ catch (provisionError) { console.error('SSO provisioning: provision request error', provisionError) + error.value = 'Failed to provision organization membership' + return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/useSSOProvisioning.ts` around lines 57 - 68, The provisioning branch currently only logs failures; update the error handling in the provision flow so callers can react: when provisionResponse.ok is false, set the composable's error.value to a meaningful object/string (using the parsed errorData) and abort the success path (return or throw) so provisionData isn't treated as success; likewise, in the catch block assign error.value = provisionError (or a normalized message) before rethrowing or returning. Locate and modify the logic around provisionResponse, provisionData, and provisionError in useSSOProvisioning to ensure error.value is populated on failures and the success branch is not executed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/composables/useSSOProvisioning.ts`:
- Around line 31-35: The current users pre-check in useSSOProvisioning aborts
provisioning on userError (it logs and returns), which blocks valid SSO logins
on transient DB/RLS errors; change the logic so that when userError is truthy
you log the error (keep the console.error line), set a non-fatal message if
desired (e.g. error.value = 'Failed to verify user account' or clear it) but do
NOT return—allow the code to continue to the server-side provisioning path (keep
the subsequent server call/invoke path intact), referencing the existing
userError check and error.value assignment to locate where to remove the early
return and make the check non-blocking.
---
Duplicate comments:
In `@src/composables/useSSOProvisioning.ts`:
- Around line 57-68: The provisioning branch currently only logs failures;
update the error handling in the provision flow so callers can react: when
provisionResponse.ok is false, set the composable's error.value to a meaningful
object/string (using the parsed errorData) and abort the success path (return or
throw) so provisionData isn't treated as success; likewise, in the catch block
assign error.value = provisionError (or a normalized message) before rethrowing
or returning. Locate and modify the logic around provisionResponse,
provisionData, and provisionError in useSSOProvisioning to ensure error.value is
populated on failures and the success branch is not executed.
Both failure branches (HTTP error and network error) now set error.value with a meaningful message instead of only logging to console. HTTP failures also return early to prevent success handling.
Previously 4xx/5xx responses from check-enforcement fell through to next(), letting the user proceed. Now any non-ok HTTP status clears cache, signs user out, and redirects to login.
Replace 4 hardcoded English strings in the SSO section of Security.vue with vue-i18n t() calls. Add corresponding keys (sso-configuration, sso-configuration-description, sso-enterprise-upgrade-description, sso-upgrade-to-enterprise) to all 15 locale JSON files with English fallback values for non-English locales.
When getClientIP() returns 'unknown', the rate limiter was returning false (not limited), allowing unlimited requests from clients that strip or spoof IP headers. Now 'unknown' is used as the rate-limit cache key, so all such requests share a single throttled bucket.
…nstraint violation
… don't abort provisioning
…ctivation # Conflicts: # messages/de.json # messages/en.json # messages/es.json # messages/fr.json # messages/hi.json # messages/id.json # messages/it.json # messages/ja.json # messages/ko.json # messages/pl.json # messages/pt-br.json # messages/ru.json # messages/tr.json # messages/vi.json # messages/zh-cn.json # src/pages/login.vue # supabase/functions/_backend/utils/hono.ts
Backend: ENABLE_SSO env var controls /sso/* routes via Hono middleware in both Supabase functions and Cloudflare Workers. Returns 404 when off. Frontend: VITE_ENABLE_SSO controls SSO enforcement module, domain checking on login, SSO config section in Security settings, and SSO callback page. All skip gracefully when disabled.
… column - Add sso_enabled boolean (default false) to orgs table - Update check_domain_sso RPC to check orgs.sso_enabled - Update get_orgs_v7 to return sso_enabled for frontend - Remove global SSO middleware from private/index.ts and cloudflare workers - Remove ENABLE_SSO env var from .env.example and supabase.ts - Frontend uses currentOrganization.sso_enabled instead of global ref - SSO provider creation checks org-level sso_enabled flag - Update SSO tests with sso_enabled: true on test org - Add sso_enabled to TypeScript types
|
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
1 participant
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 (AI generated)
Motivation (AI generated)
Enterprise customers need a complete SSO lifecycle management workflow. Previously, SSO providers could be created but there was no way to:
Additionally, the `/check-domain` endpoint required authentication, which prevented it from being called during the login flow when users are unauthenticated.
Business Impact (AI generated)
This enables Capgo to offer a complete Enterprise SSO solution, which:
Expected impact: Increased Enterprise plan adoption and reduced time-to-activation for SSO customers.
Test Plan (AI generated)
Backend Tests
Frontend Tests
Integration Tests
Generated with AI
Summary by CodeRabbit
New Features
Localization
Tests