feat: add SAML SSO with automatic org auto-join #1313
Conversation
Add enterprise-exclusive SAML SSO configuration with domain-based automatic user provisioning. ## Changes - Database migration: org_sso_providers and org_domains tables with auto-join triggers - Backend API endpoints for SSO provider and domain management with DNS verification - Frontend settings page for configuring SAML and managing claimed domains - Auto-join logic: new users automatically added to orgs based on email domain match - Auto-backfill: existing users added when domain verification completes - Enterprise plan gating for all SSO features 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <[email protected]>
|
Warning Rate limit exceeded@riderx has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 23 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR introduces comprehensive SSO management functionality with translation keys, a Vue SFC for enterprise SSO configuration, backend API endpoints for SSO provider and domain management, and database schema with auto-join triggers and permission gating. No existing logic is modified; functionality is purely additive. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend as SSO<br/>Component
participant Backend as Backend<br/>API
participant Supabase as Supabase<br/>(RPC/Functions)
participant DNS as DNS<br/>Verification
User->>Frontend: View SSO Configuration
Frontend->>Backend: GET /config
Backend->>Supabase: Call get_org_sso_config()
Supabase-->>Backend: Return config + is_enterprise
Backend-->>Frontend: Config data
Frontend->>Frontend: Render provider form
User->>Frontend: Add domain
Frontend->>Backend: POST /domains
Backend->>Supabase: Call add_org_domain()
Supabase-->>Backend: Return domain_id + verification_token
Backend-->>Frontend: Domain + token
Frontend->>Frontend: Display verification instructions
User->>Frontend: Verify domain
Frontend->>Backend: POST /domains/verify
Backend->>DNS: DNS TXT lookup
DNS-->>Backend: TXT record value
alt DNS matches token
Backend->>Supabase: Call verify_org_domain()
Supabase->>Supabase: Trigger backfill_domain_users()
Supabase-->>Backend: Success
Backend-->>Frontend: Domain verified ✓
Frontend->>Frontend: Show success, enable auto-join
else DNS mismatch
Backend-->>Frontend: Verification failed
end
User->>Frontend: Configure auto-join
Frontend->>Backend: PUT /domains/settings
Backend->>Supabase: Call update_org_domain_settings()
Supabase-->>Backend: Success
Backend-->>Frontend: Settings updated
Frontend->>Frontend: Refresh domain state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Mark domain as verified (this triggers backfill) | ||
| const { data, error } = await supabaseAdmin.rpc('verify_org_domain', { | ||
| p_domain_id: domain_id, |
There was a problem hiding this comment.
The /sso/domains/verify handler calls supabaseAdmin.rpc('verify_org_domain', ...) (see sso.ts lines 423-425), but verify_org_domain enforces check_min_rights(..., auth.uid(), ...) in the migration. Service-role clients do not carry the end-user UID, so auth.uid() is null and the RPC returns NO_RIGHTS, making domain verification fail even for super_admins. Consider invoking the RPC with a client bound to the user JWT or changing the SQL function to accept a user_id parameter / bypass the auth.uid check for service-role calls.
Useful? React with 👍 / 👎.
| FROM public.users u | ||
| WHERE split_part(u.email, '@', 2) = NEW.domain |
There was a problem hiding this comment.
The backfill trigger compares split_part(u.email, '@', 2) to NEW.domain without lowercasing either side. Because email domains are case-insensitive and org_domains.domain is stored lowercased, users who register with mixed/uppercase domains (e.g., [email protected]) will not be auto-joined/backfilled. Use lower(split_part(...)) or compare lower(NEW.domain) in both auto-join and backfill triggers.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
supabase/migrations/20251228080040_sso_auto_join.sql (3)
50-56: Potential RLS policy conflict: overlapping permissions.The
FOR ALLpolicy for super_admins (Line 50-52) and theFOR SELECTpolicy for admins (Line 54-56) may result in super_admins being subject to both policies. While PostgreSQL's permissive policy model typically ORs policies together, this could lead to unexpected behavior. Consider explicitly documenting the intended access pattern or using a single policy with a combined check.
262-307: Domain validation could be stricter.The function accepts any domain string without validating format. Consider adding a regex check for valid domain format (e.g., no spaces, valid TLD pattern).
🔎 Proposed domain validation
+ -- Validate domain format + IF NOT (lower(p_domain) ~ '^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*\.[a-z]{2,}$') THEN + RETURN QUERY SELECT NULL::uuid, NULL::varchar, 'INVALID_DOMAIN_FORMAT'::text; + RETURN; + END IF; + -- Check if domain is already claimed IF EXISTS (SELECT 1 FROM public.org_domains WHERE domain = lower(p_domain)) THEN
495-523: count_domain_users returns -1 for unauthorized access.Returning -1 for permission errors is non-standard. Consider raising an exception or returning a structured result with an error code, consistent with other functions in this migration.
supabase/functions/_backend/private/sso.ts (1)
383-399: DNS resolution lacks timeout protection.
Deno.resolveDnscould hang if the DNS server is unresponsive. Consider wrapping in a timeout to prevent request hangs.🔎 Proposed fix with timeout
+ // Helper to add timeout to DNS resolution + async function resolveDnsWithTimeout(hostname: string, recordType: 'TXT', timeoutMs = 10000) { + const controller = new AbortController() + const timeoutId = setTimeout(() => controller.abort(), timeoutMs) + try { + return await Deno.resolveDns(hostname, recordType) + } finally { + clearTimeout(timeoutId) + } + } try { - const records = await Deno.resolveDns(dnsRecord, 'TXT') + const records = await resolveDnsWithTimeout(dnsRecord, 'TXT')src/pages/settings/organization/SSO.vue (3)
177-202: verifyDomain uses functions.invoke but other methods use RPC directly.This inconsistency could cause confusion. The backend has the
/domains/verifyendpoint, but domain add/delete use RPC. Consider aligning on one approach for consistency.
293-301: copyToClipboard lacks secure context check.
navigator.clipboard.writeTextrequires a secure context (HTTPS). While the catch block handles failures, a more informative error message could help debugging in non-secure contexts.
588-591: Non-null assertion on verification_token could cause runtime error.Line 590 uses
domain.verification_token!but the condition on Line 565 only checksdomain.verification_tokenis truthy. While this is safe in this context, using optional chaining or explicit null check would be more defensive.🔎 Proposed fix
- @click.stop="copyToClipboard(domain.verification_token!)" + @click.stop="copyToClipboard(domain.verification_token ?? '')"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
messages/en.jsonsrc/constants/organizationTabs.tssrc/pages/settings/organization/SSO.vuesupabase/functions/_backend/private/sso.tssupabase/functions/private/index.tssupabase/migrations/20251228080040_sso_auto_join.sql
🧰 Additional context used
📓 Path-based instructions (17)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use single quotes and no semicolons per @antfu/eslint-config
Files:
src/constants/organizationTabs.tssupabase/functions/_backend/private/sso.tssupabase/functions/private/index.tssrc/pages/settings/organization/SSO.vue
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript strict mode with path aliases mapping
~/tosrc/
Files:
src/constants/organizationTabs.tssupabase/functions/_backend/private/sso.tssupabase/functions/private/index.ts
{capacitor.config.{ts,js},src/**/*.{ts,tsx,vue}}
📄 CodeRabbit inference engine (CLAUDE.md)
Mobile apps should use Capacitor with app ID
ee.forgr.capacitor_gofor native mobile functionality
Files:
src/constants/organizationTabs.tssrc/pages/settings/organization/SSO.vue
src/**/*.{ts,tsx,vue,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
~/alias for imports fromsrc/directory in frontend TypeScript and Vue components
Files:
src/constants/organizationTabs.tssrc/pages/settings/organization/SSO.vue
src/**/*.{vue,ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Frontend ESLint must pass before commit; run
bun lint:fixto auto-fix issues in frontend files
Files:
src/constants/organizationTabs.tssrc/pages/settings/organization/SSO.vue
src/**/*.{vue,ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Konsta components are reserved for the safe area helpers; avoid importing
konstaanywhere else in the app
Files:
src/constants/organizationTabs.tssrc/pages/settings/organization/SSO.vue
supabase/functions/_backend/**
📄 CodeRabbit inference engine (CLAUDE.md)
Backend logic should be organized in
supabase/functions/_backend/with subdirectories for plugins, private endpoints, public endpoints, triggers, and utilities
Files:
supabase/functions/_backend/private/sso.ts
supabase/functions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Supabase Edge Functions use Deno runtime
Files:
supabase/functions/_backend/private/sso.tssupabase/functions/private/index.ts
supabase/functions/_backend/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
supabase/functions/_backend/**/*.{ts,js}: Backend code must be placed insupabase/functions/_backend/as shared code deployed to Cloudflare Workers (API/Plugin/Files workers), Supabase Edge Functions, and other platforms
UsecreateHonofromutils/hono.tsfor all Hono framework application initialization and routing
All database operations must usegetPgClient()orgetDrizzleClient()fromutils/pg.tsfor PostgreSQL access during active migration to Cloudflare D1
All Hono endpoint handlers must acceptContext<MiddlewareKeyVariables>and usec.get('requestId'),c.get('apikey'), andc.get('auth')for request context
Use structured logging withcloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging
UsemiddlewareAPISecretfor internal API endpoints andmiddlewareKeyfor external API keys; validate againstowner_orgin theapikeystable
Checkc.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints
Use Drizzle ORM query patterns withschemafrompostgress_schema.tsfor all database operations; usealiasV2()for self-joins or multiple table references
Files:
supabase/functions/_backend/private/sso.ts
supabase/functions/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend ESLint must pass before commit; run
bun lint:backendfor backend files
Files:
supabase/functions/_backend/private/sso.tssupabase/functions/private/index.ts
src/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.vue: Use Vue 3 with Composition API and<script setup>syntax for frontend components
Style components using TailwindCSS with DaisyUI components
src/**/*.vue: Use Vue 3<script setup>syntax exclusively for all Vue component scripts
Use Tailwind utility classes for layout and spacing in Vue components
Use DaisyUI components (d-btn,d-input,d-card) for interactive elements in Vue components
Use Konsta components ONLY for safe area helpers (top/bottom insets) in Vue components; avoid other uses
UseuseRoute()fromvue-routerto access route parameters anduseRouter()for programmatic navigation in Vue componentsUse DaisyUI (
d-prefixed classes) for buttons, inputs, and other interactive primitives to keep behavior and spacing consistent
Files:
src/pages/settings/organization/SSO.vue
src/pages/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
Use file-based routing with unplugin-vue-router for frontend pages
Frontend file-based routing uses
src/pages/directory structure; routes auto-generate withunplugin-vue-routerand types are available insrc/typed-router.d.ts
Files:
src/pages/settings/organization/SSO.vue
src/**/*.{vue,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
The web client is built with Vue.js and Tailwind CSS; lean on utility classes and composition-friendly patterns rather than bespoke CSS
Files:
src/pages/settings/organization/SSO.vue
src/**/*.{css,scss,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Mirror the Capgo design palette from
src/styles/style.css(e.g.,--color-primary-500: #515271,--color-azure-500: #119eff) when introducing new UI, using deep slate bases with the Extract azure highlight and soft radii
Files:
src/pages/settings/organization/SSO.vue
supabase/migrations/**/*.sql
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database migrations must be created with
supabase migration new <feature_slug>and never modify previously committed migrations
Files:
supabase/migrations/20251228080040_sso_auto_join.sql
**/{migrations,tests,__tests__}/**/*.{sql,ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
Always cover database changes with Postgres-level tests and complement them with end-to-end tests for affected user flows
Files:
supabase/migrations/20251228080040_sso_auto_join.sql
supabase/migrations/*.sql
📄 CodeRabbit inference engine (AGENTS.md)
supabase/migrations/*.sql: When creating schema changes, usesupabase migration new <feature_slug>to create a single migration file and keep editing that file until the feature ships; never edit previously committed migrations
A migration that introduces a new table may include seed inserts for that table, treating seeding as part of the current feature and not modifying previously committed migrations
Do not create new cron jobs; instead update theprocess_all_cron_tasksfunction in a new migration file to add your job if needed
Files:
supabase/migrations/20251228080040_sso_auto_join.sql
🧠 Learnings (13)
📚 Learning: 2025-10-30T14:58:37.007Z
Learnt from: Dalanir
Repo: Cap-go/capgo PR: 1238
File: src/layouts/settings.vue:7-13
Timestamp: 2025-10-30T14:58:37.007Z
Learning: In the Cap-go/capgo repository, heroicons are imported using the pattern `~icons/heroicons/icon-name` (e.g., `~icons/heroicons/user`, `~icons/heroicons/bell`) without requiring `/outline/` or `/solid/` path segments. Solid variants use a `-solid` suffix in the icon name itself (e.g., `~icons/heroicons/arrow-path-solid`).
Applied to files:
src/constants/organizationTabs.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `createHono` from `utils/hono.ts` for all Hono framework application initialization and routing
Applied to files:
supabase/functions/_backend/private/sso.tssupabase/functions/private/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use `middlewareAPISecret` for internal API endpoints and `middlewareKey` for external API keys; validate against `owner_org` in the `apikeys` table
Applied to files:
supabase/functions/_backend/private/sso.tssupabase/functions/private/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Check `c.get('auth')?.authType` to determine authentication type ('apikey' vs 'jwt') in backend endpoints
Applied to files:
supabase/functions/_backend/private/sso.tssupabase/functions/private/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : All Hono endpoint handlers must accept `Context<MiddlewareKeyVariables>` and use `c.get('requestId')`, `c.get('apikey')`, and `c.get('auth')` for request context
Applied to files:
supabase/functions/_backend/private/sso.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Backend code must be placed in `supabase/functions/_backend/` as shared code deployed to Cloudflare Workers (API/Plugin/Files workers), Supabase Edge Functions, and other platforms
Applied to files:
supabase/functions/_backend/private/sso.tssupabase/functions/private/index.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Applies to supabase/functions/_backend/**/*.{ts,js} : Use Drizzle ORM query patterns with `schema` from `postgress_schema.ts` for all database operations; use `aliasV2()` for self-joins or multiple table references
Applied to files:
supabase/functions/_backend/private/sso.ts
📚 Learning: 2025-12-23T02:53:12.055Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-23T02:53:12.055Z
Learning: Use shared backend code from `supabase/functions/_backend/` across all deployment platforms; never create platform-specific implementations outside this directory
Applied to files:
supabase/functions/private/index.ts
📚 Learning: 2025-12-23T01:19:04.593Z
Learnt from: riderx
Repo: Cap-go/capgo PR: 1297
File: src/components/dashboard/DeploymentBanner.vue:77-79
Timestamp: 2025-12-23T01:19:04.593Z
Learning: In the Cap-go codebase, ensure that app permission checks never include the role 'owner'. App-level permissions should be based on the user_min_right enum with values: read, upload, write, admin, super_admin (and NOT owner). This pattern applies across Vue components that perform permission checks; if you find a check referencing 'owner' for app-level access, replace it with the appropriate user_min_right value and keep organization-level owner handling in organization.ts.
Applied to files:
src/pages/settings/organization/SSO.vue
📚 Learning: 2025-12-24T14:11:10.256Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:409-539
Timestamp: 2025-12-24T14:11:10.256Z
Learning: In supabase/migrations for get_orgs_v6 and get_orgs_v7: The inner functions with user_id parameter (get_orgs_v6(uuid) and get_orgs_v7(uuid)) should NOT be granted to anon/authenticated roles as this allows any user to query other users' organizations; only the no-argument wrapper functions should be public as they perform authentication checks.
Applied to files:
supabase/migrations/20251228080040_sso_auto_join.sql
📚 Learning: 2025-12-25T11:22:13.039Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:85-96
Timestamp: 2025-12-25T11:22:13.039Z
Learning: In SQL migrations under the repository (e.g., supabase/migrations), enforce that when an org has enforcing_2fa=true, all users (including super_admins) must have 2FA enabled before accessing any org functions, including check_org_members_2fa_enabled. Do not grant admin exceptions to 2FA requirements. This ensures consistent security enforcement across all org-related operations; implement this rule within relevant migrations and associated stored procedures/tests.
Applied to files:
supabase/migrations/20251228080040_sso_auto_join.sql
📚 Learning: 2025-12-27T03:51:23.575Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T03:51:23.575Z
Learning: Applies to supabase/migrations/*.sql : A migration that introduces a new table may include seed inserts for that table, treating seeding as part of the current feature and not modifying previously committed migrations
Applied to files:
supabase/migrations/20251228080040_sso_auto_join.sql
📚 Learning: 2025-12-27T03:51:23.575Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T03:51:23.575Z
Learning: Applies to supabase/seed.sql : Update `supabase/seed.sql` to back new or evolved tests; keep fixtures focused on current behavior while leaving committed migrations unchanged
Applied to files:
supabase/migrations/20251228080040_sso_auto_join.sql
🧬 Code graph analysis (2)
src/constants/organizationTabs.ts (1)
src/components/comp_def.ts (1)
Tab(44-50)
supabase/functions/_backend/private/sso.ts (2)
supabase/functions/_backend/utils/hono.ts (5)
MiddlewareKeyVariables(27-41)useCors(43-47)simpleError(247-249)quickError(224-237)parseBody(251-262)supabase/functions/_backend/utils/hono_middleware.ts (1)
middlewareV2(172-196)
🪛 GitHub Actions: Run tests
src/pages/settings/organization/SSO.vue
[error] 20-20: ESLint: perfectionist/sort-imports - Expected "/services/supabase" to come before "/stores/organization".
[error] 78-78: ESLint: 'hasAdminPermission' is assigned a value but never used. (unused-vars)
🔇 Additional comments (9)
supabase/migrations/20251228080040_sso_auto_join.sql (3)
71-89: Enterprise check may return NULL for orgs without a plan.If an org has no associated
stripe_infoorplansrecord,v_plan_namewill be NULL, andNULL = 'Enterprise'evaluates to NULL (falsy). This is likely the intended behavior, but consider adding an explicitCOALESCEor comment to clarify.
178-216: LGTM: Proper permission checks and query structure.The function correctly validates admin rights before returning data and uses table aliases to avoid ambiguity.
340-376: verify_org_domain has authorization checks but no mechanism to prevent bypassing DNS verification.The function is currently only called from the backend after DNS verification completes. However, since it's an RPC function with only role-based checks (super_admin, enterprise org), direct calls could theoretically bypass the DNS verification step. Consider either: (1) adding a backend-only marker/documentation to prevent direct RPC calls, (2) re-verifying DNS within the function, or (3) storing the DNS verification state and validating it here.
supabase/functions/private/index.ts (1)
17-17: LGTM: SSO route correctly registered.The import and route registration follow the established pattern in this file.
Also applies to: 51-51
src/constants/organizationTabs.ts (1)
8-8: LGTM: SSO tab correctly added.The tab follows the established pattern with proper icon import (using
~icons/heroicons/keyper project convention from learnings) and consistent structure.Also applies to: 14-14
supabase/functions/_backend/private/sso.ts (2)
52-93: GET /config endpoint pattern is correct.Properly validates auth, checks admin rights, and returns enterprise status. The use of
.maybeSingle()correctly handles the case of no existing config.
22-26: Theauto_join_roleenum correctly excludessuper_admin. This is intentional design aligned with SAML SSO security best practices, which mandate that auto-provisioned users receive minimal default privileges and never auto-assign admin or privileged roles. The 'super_admin' role is reserved for explicit assignment by existing 'super_admin' users (enforced by themin_right: 'super_admin'check on the SSO endpoint itself), preventing privilege escalation through auto-join mechanisms.src/pages/settings/organization/SSO.vue (1)
318-676: LGTM: Well-structured template with proper permission gating.The template correctly gates features behind
hasSuperAdminPermissionandisEnterprisechecks, provides good UX with loading states, and follows the project's Tailwind/DaisyUI styling patterns.messages/en.json (1)
1229-1285: LGTM: Comprehensive i18n keys for SSO feature.The translation keys follow the existing naming convention (
sso-*prefix), cover all UI states (loading, errors, success), and provide clear user-facing messages. The keys align with the Vue component's usage.
| export const app = new Hono<MiddlewareKeyVariables>() | ||
|
|
||
| app.use('/*', useCors) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Missing basePath in Hono initialization.
Other backend files use createHono from utils. Using bare new Hono() may miss error handling, logging, and other middleware setup. As per coding guidelines, use createHono for all Hono app initialization.
🔎 Proposed fix
-import { Hono } from 'hono/tiny'
+import { createHono } from '../utils/hono.ts'
-export const app = new Hono<MiddlewareKeyVariables>()
-
-app.use('/*', useCors)
+export const app = createHono('sso')Note: If createHono adds CORS automatically, the explicit useCors may be redundant. Verify the behavior of createHono in utils/hono.ts.
📝 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.
| export const app = new Hono<MiddlewareKeyVariables>() | |
| app.use('/*', useCors) | |
| import { createHono } from '../utils/hono.ts' | |
| export const app = createHono('sso') | |
| app.use('/*', useCors) |
🤖 Prompt for AI Agents
In supabase/functions/_backend/private/sso.ts around lines 37 to 39, the file
instantiates Hono directly which bypasses shared setup (basePath, error
handling, logging, and common middleware); replace new Hono() with the project
helper createHono(...) from utils/hono.ts (pass the same generic
MiddlewareKeyVariables and any required basePath argument), then remove or
verify the redundant useCors registration if createHono already applies CORS;
ensure imports are updated to import createHono instead of Hono and that the
resulting app variable uses the same name and type.
| if (!verified) { | ||
| return c.json({ | ||
| success: false, | ||
| verified: false, | ||
| error: 'verification_token_mismatch', | ||
| message: `DNS TXT record found but value doesn't match. Expected: ${domainInfo.verification_token}`, | ||
| expected_record: dnsRecord, | ||
| expected_value: domainInfo.verification_token, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Token mismatch error also exposes the expected value.
Similar to the DNS lookup failure case, exposing the expected token value could allow an attacker to set up the correct record if they can intercept this response.
🔎 Proposed fix
if (!verified) {
return c.json({
success: false,
verified: false,
error: 'verification_token_mismatch',
- message: `DNS TXT record found but value doesn't match. Expected: ${domainInfo.verification_token}`,
+ message: `DNS TXT record found but value doesn't match the expected verification token.`,
expected_record: dnsRecord,
- expected_value: domainInfo.verification_token,
})
}🤖 Prompt for AI Agents
In supabase/functions/_backend/private/sso.ts around lines 412 to 421, the
verification failure response currently returns the expected token (and DNS
record) which leaks sensitive data; change the response to omit expected_value
(and remove any detailed dns/record fields), returning only a generic
error/verified flag and a non-sensitive message (e.g. "DNS TXT record found but
value does not match") so callers know verification failed without exposing the
expected token or record details.
| CREATE OR REPLACE FUNCTION public.handle_sso_auto_join() | ||
| RETURNS TRIGGER | ||
| LANGUAGE plpgsql | ||
| SECURITY DEFINER | ||
| SET search_path = '' | ||
| AS $$ | ||
| DECLARE | ||
| user_domain text; | ||
| domain_record record; | ||
| BEGIN | ||
| -- Extract domain from email | ||
| user_domain := split_part(NEW.email, '@', 2); | ||
|
|
||
| -- Find verified domain with auto-join enabled for Enterprise org | ||
| FOR domain_record IN | ||
| SELECT od.org_id, od.auto_join_role | ||
| FROM public.org_domains od | ||
| WHERE od.domain = user_domain | ||
| AND od.verified = true | ||
| AND od.auto_join_enabled = true | ||
| AND public.is_enterprise_org(od.org_id) | ||
| LOOP | ||
| -- Check if user already in org | ||
| IF NOT EXISTS ( | ||
| SELECT 1 FROM public.org_users | ||
| WHERE user_id = NEW.id AND org_id = domain_record.org_id | ||
| ) THEN | ||
| INSERT INTO public.org_users (user_id, org_id, user_right) | ||
| VALUES (NEW.id, domain_record.org_id, domain_record.auto_join_role); | ||
| END IF; | ||
| END LOOP; | ||
|
|
||
| RETURN NEW; | ||
| END; | ||
| $$; |
There was a problem hiding this comment.
Auto-join trigger lacks error handling for insert failures.
If the INSERT INTO public.org_users fails (e.g., due to constraint violations), the trigger will raise an exception and potentially block user creation. Consider wrapping in an exception handler or using INSERT ... ON CONFLICT DO NOTHING.
🔎 Proposed fix with conflict handling
-- Check if user already in org
IF NOT EXISTS (
SELECT 1 FROM public.org_users
WHERE user_id = NEW.id AND org_id = domain_record.org_id
) THEN
- INSERT INTO public.org_users (user_id, org_id, user_right)
- VALUES (NEW.id, domain_record.org_id, domain_record.auto_join_role);
+ BEGIN
+ INSERT INTO public.org_users (user_id, org_id, user_right)
+ VALUES (NEW.id, domain_record.org_id, domain_record.auto_join_role);
+ EXCEPTION WHEN unique_violation THEN
+ -- Race condition: user was added by another process
+ NULL;
+ END;
END IF;📝 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.
| CREATE OR REPLACE FUNCTION public.handle_sso_auto_join() | |
| RETURNS TRIGGER | |
| LANGUAGE plpgsql | |
| SECURITY DEFINER | |
| SET search_path = '' | |
| AS $$ | |
| DECLARE | |
| user_domain text; | |
| domain_record record; | |
| BEGIN | |
| -- Extract domain from email | |
| user_domain := split_part(NEW.email, '@', 2); | |
| -- Find verified domain with auto-join enabled for Enterprise org | |
| FOR domain_record IN | |
| SELECT od.org_id, od.auto_join_role | |
| FROM public.org_domains od | |
| WHERE od.domain = user_domain | |
| AND od.verified = true | |
| AND od.auto_join_enabled = true | |
| AND public.is_enterprise_org(od.org_id) | |
| LOOP | |
| -- Check if user already in org | |
| IF NOT EXISTS ( | |
| SELECT 1 FROM public.org_users | |
| WHERE user_id = NEW.id AND org_id = domain_record.org_id | |
| ) THEN | |
| INSERT INTO public.org_users (user_id, org_id, user_right) | |
| VALUES (NEW.id, domain_record.org_id, domain_record.auto_join_role); | |
| END IF; | |
| END LOOP; | |
| RETURN NEW; | |
| END; | |
| $$; | |
| CREATE OR REPLACE FUNCTION public.handle_sso_auto_join() | |
| RETURNS TRIGGER | |
| LANGUAGE plpgsql | |
| SECURITY DEFINER | |
| SET search_path = '' | |
| AS $$ | |
| DECLARE | |
| user_domain text; | |
| domain_record record; | |
| BEGIN | |
| -- Extract domain from email | |
| user_domain := split_part(NEW.email, '@', 2); | |
| -- Find verified domain with auto-join enabled for Enterprise org | |
| FOR domain_record IN | |
| SELECT od.org_id, od.auto_join_role | |
| FROM public.org_domains od | |
| WHERE od.domain = user_domain | |
| AND od.verified = true | |
| AND od.auto_join_enabled = true | |
| AND public.is_enterprise_org(od.org_id) | |
| LOOP | |
| -- Check if user already in org | |
| IF NOT EXISTS ( | |
| SELECT 1 FROM public.org_users | |
| WHERE user_id = NEW.id AND org_id = domain_record.org_id | |
| ) THEN | |
| BEGIN | |
| INSERT INTO public.org_users (user_id, org_id, user_right) | |
| VALUES (NEW.id, domain_record.org_id, domain_record.auto_join_role); | |
| EXCEPTION WHEN unique_violation THEN | |
| -- Race condition: user was added by another process | |
| NULL; | |
| END; | |
| END IF; | |
| END LOOP; | |
| RETURN NEW; | |
| END; | |
| $$; |
🤖 Prompt for AI Agents
In supabase/migrations/20251228080040_sso_auto_join.sql around lines 95 to 129,
the trigger function performs INSERT into public.org_users without handling
insert failures which can abort the entire transaction; modify the INSERT to be
resilient by either using INSERT ... ON CONFLICT DO NOTHING (targeting the
unique constraint on user_id, org_id) or wrap the INSERT in a BEGIN ...
EXCEPTION WHEN others THEN NULL END block to swallow expected constraint errors
and ensure the trigger does not raise and block user creation.
| CREATE OR REPLACE FUNCTION public.backfill_domain_users() | ||
| RETURNS TRIGGER | ||
| LANGUAGE plpgsql | ||
| SECURITY DEFINER | ||
| SET search_path = '' | ||
| AS $$ | ||
| BEGIN | ||
| -- Only run when verified changes from false to true | ||
| IF NEW.verified = true AND (OLD.verified = false OR OLD.verified IS NULL) THEN | ||
| -- Only backfill if org is Enterprise | ||
| IF public.is_enterprise_org(NEW.org_id) THEN | ||
| INSERT INTO public.org_users (user_id, org_id, user_right) | ||
| SELECT u.id, NEW.org_id, NEW.auto_join_role | ||
| FROM public.users u | ||
| WHERE split_part(u.email, '@', 2) = NEW.domain | ||
| AND NOT EXISTS ( | ||
| SELECT 1 FROM public.org_users ou | ||
| WHERE ou.user_id = u.id AND ou.org_id = NEW.org_id | ||
| ); | ||
| END IF; | ||
| END IF; | ||
| RETURN NEW; | ||
| END; | ||
| $$; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for batch processing patterns in migrations
rg -n -A3 'LIMIT|batch|chunk|batch_size' supabase/migrations/ --type sql | head -100Repository: Cap-go/capgo
Length of output: 9690
🏁 Script executed:
#!/bin/bash
# Check for similar trigger implementations that handle bulk operations
rg -n -B5 -A10 'TRIGGER.*INSERT.*SELECT|CREATE.*TRIGGER' supabase/migrations/ --type sql | head -150Repository: Cap-go/capgo
Length of output: 13239
🏁 Script executed:
#!/bin/bash
# Check the full migration file to understand context and any test coverage
wc -l supabase/migrations/20251228080040_sso_auto_join.sqlRepository: Cap-go/capgo
Length of output: 112
🏁 Script executed:
#!/bin/bash
# Check for tests related to SSO auto-join
find . -type f \( -name "*.test.*" -o -name "*.spec.*" -o -name "*test*" \) | xargs grep -l "sso\|auto_join\|backfill" 2>/dev/null | head -20Repository: Cap-go/capgo
Length of output: 287
🏁 Script executed:
#!/bin/bash
# Check if there are existing org_domains with many users to understand scale
rg -n "org_domains|domain" supabase/migrations/20251228080040_sso_auto_join.sql -A2 -B2 | head -50Repository: Cap-go/capgo
Length of output: 2070
🏁 Script executed:
#!/bin/bash
# Look for any queue-based alternatives or async patterns in recent migrations
rg -n "enqueue|queue|async" supabase/migrations/20251228080040_sso_auto_join.sqlRepository: Cap-go/capgo
Length of output: 38
Backfill trigger should use batching for large domains to avoid lock contention.
This trigger performs an unbounded bulk INSERT within a synchronous AFTER UPDATE transaction. The codebase establishes a clear pattern of batching for bulk operations (e.g., process_function_queue with batch sizes of 10–1000, process_channel_device_counts_queue(1000)). For Enterprise domains with many existing users, this unbounded insert could cause long-running transactions and lock contention during domain verification. Consider either deferring the backfill to an async queue job (consistent with your cron architecture) or adding a LIMIT clause to batch the inserts across multiple verification updates.
- Add pgtap SQL tests for SSO tables, triggers, and RPC functions (28 test cases) - Add Vitest E2E API tests for SSO config and domain endpoints 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Resolved conflict in organizationTabs.ts - kept both SSO and Security tabs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Resolved conflict in messages/en.json - kept both SSO translations and native dependencies translations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Fix #1: Add p_user_id parameter to RPC functions for service-role compatibility - Fix #2: Normalize email domains with lower() in auto-join and backfill triggers - Fix #3: Remove duplicate translation keys (copied-to-clipboard, copy-failed) - Fix #4: Fix import order and remove unused hasAdminPermission variable in SSO.vue - Fix #6 & #7: Remove verification token from DNS error responses for security - Fix #8: Add exception handling for unique_violation in auto-join trigger - Fix #9: Add batching (1000 users per batch) for backfill trigger to avoid lock contention Note: Comment #5 (use createHono) was not applied as all private/* modules use new Hono() consistently - createHono is for top-level function entry points. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Resolved conflict in private/index.ts - kept both SSO and validate_password_compliance routes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
@riderx this makes not a lot of sense. You always want to have the SSO provider enabled + please fix i18n |
|
I got the following error when trying to setup SSO |
| END; | ||
| $$; | ||
|
|
||
| -- Update upsert_org_sso_provider to accept optional user_id |
There was a problem hiding this comment.
I think we can drop the old function
| -- Update upsert_org_sso_provider to accept optional user_id | |
| -- Update upsert_org_sso_provider to accept optional user_id | |
| -- First drop the old 6-parameter version to avoid overloading conflict | |
| DROP FUNCTION IF EXISTS public.upsert_org_sso_provider(uuid, uuid, varchar, varchar, text, boolean); | |
| const { data, error } = await supabaseAdmin.rpc('upsert_org_sso_provider', { | ||
| p_org_id: org_id, | ||
| p_supabase_sso_provider_id: ssoConfig.supabase_sso_provider_id, |
There was a problem hiding this comment.
We can add a p_user_id
| const { data, error } = await supabaseAdmin.rpc('upsert_org_sso_provider', { | |
| p_org_id: org_id, | |
| p_supabase_sso_provider_id: ssoConfig.supabase_sso_provider_id, | |
| const { data, error } = await supabaseAdmin.rpc('upsert_org_sso_provider', { | |
| p_user_id: auth.userId, | |
| p_org_id: org_id, | |
| p_supabase_sso_provider_id: ssoConfig.supabase_sso_provider_id, |
Summary
Implement SAML SSO with automatic organization auto-join for Enterprise plan users. Admins can now configure SAML identity providers and claim email domains to automatically provision users into their organization.
Test plan
company.com) and follow DNS verification instructionsreadroleChecklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.