feat(sso): remove sso_enabled flag, always show SSO section#2013
Conversation
SSO settings are now visible to all org members with super-admin permissions. Enterprise orgs see the configuration form directly; non-enterprise orgs see an upgrade prompt. The Capgo-managed sso_enabled column is dropped from orgs and all related DB functions are updated accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR removes the ChangesSSO Enablement Detection Refactor
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Settings UI
participant Backend as SSO Function
participant DB as Postgres (sso_providers & orgs)
participant Auth as Auth System
User->>UI: open Organization Security settings
UI->>Backend: request SSO config (requires org.update_settings)
Backend->>Auth: validate permissions & plan
Backend->>DB: query/insert into sso_providers (no orgs.sso_enabled check)
DB-->>Backend: return provider / org info
Backend-->>UI: return SSO config / upgrade prompt
UI-->>User: render SSO configuration section
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 SQLFluff (4.1.0)supabase/migrations/20260501200000_remove_sso_enabled_flag.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: Review rate limit: 3/5 reviews remaining, refill in 21 minutes and 58 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
supabase/migrations/20260501120000_remove_sso_enabled_flag.sql (1)
59-85: ⚡ Quick winSet the recreated trigger owner explicitly.
generate_org_on_user_create()is recreated asSECURITY DEFINER, but unlike the callable functions above and below it, the migration never pins its owner. Add an explicitALTER FUNCTION public.generate_org_on_user_create() OWNER TO postgres;after the definition so the definer role stays deterministic across environments.Suggested fix
CREATE OR REPLACE FUNCTION "public"."generate_org_on_user_create" () RETURNS "trigger" LANGUAGE "plpgsql" SET search_path = '' SECURITY DEFINER AS $$ ... RETURN NEW; END $$; + +ALTER FUNCTION public.generate_org_on_user_create() OWNER TO postgres;As per coding guidelines "Apply explicit REVOKE ALL and GRANT statements for function permissioning; start from deny-by-default and grant only required roles; set OWNER explicitly" and based on learnings "do not require REVOKE ALL ON FUNCTION ... FROM PUBLIC for PostgreSQL trigger functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260501120000_remove_sso_enabled_flag.sql` around lines 59 - 85, The migration recreates the SECURITY DEFINER trigger function generate_org_on_user_create but does not set an explicit owner; add a statement after the function definition that sets the function owner to postgres (e.g., run ALTER FUNCTION public.generate_org_on_user_create() OWNER TO postgres) so the definer role is deterministic across environments and matches the pattern used for other functions; ensure you place this ALTER immediately after the CREATE OR REPLACE FUNCTION block and follow existing permissioning conventions for other trigger functions.
🤖 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/migrations/20260501120000_remove_sso_enabled_flag.sql`:
- Around line 79-82: The INSERT into public.orgs is only guarded against sso:*
providers, so email-auth users on domains with active SSO (has_sso) will still
get a personal org; update the IF condition used around the INSERT (the check
that references user_provider, has_sso, and the INSERT INTO public.orgs that
sets org_record) so it also treats email logins as SSO-managed: change the
predicate to consider either sso providers or email (e.g., (user_provider ~
'^sso:' OR user_provider = 'email') AND has_sso) and keep the INSERT only when
that combined condition is false.
---
Nitpick comments:
In `@supabase/migrations/20260501120000_remove_sso_enabled_flag.sql`:
- Around line 59-85: The migration recreates the SECURITY DEFINER trigger
function generate_org_on_user_create but does not set an explicit owner; add a
statement after the function definition that sets the function owner to postgres
(e.g., run ALTER FUNCTION public.generate_org_on_user_create() OWNER TO
postgres) so the definer role is deterministic across environments and matches
the pattern used for other functions; ensure you place this ALTER immediately
after the CREATE OR REPLACE FUNCTION block and follow existing permissioning
conventions for other trigger functions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6013ddf5-40e9-4d3d-904f-a24b49c3f063
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
src/pages/settings/organization/Security.vuesrc/types/supabase.types.tssupabase/functions/_backend/private/sso/providers.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260501120000_remove_sso_enabled_flag.sqltests/organization-put-stripe-sync.unit.test.tstests/sso.test.ts
💤 Files with no reviewable changes (3)
- tests/organization-put-stripe-sync.unit.test.ts
- src/types/supabase.types.ts
- supabase/functions/_backend/utils/supabase.types.ts
…tion - Renamed 20260501120000 → 20260501200000 (must be > latest on main: 20260501162433) - Added ALTER FUNCTION generate_org_on_user_create() OWNER TO postgres Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/migrations/20260501200000_remove_sso_enabled_flag.sql`:
- Around line 79-82: The IF guard is not NULL-safe because "user_provider ~
'^sso:'" yields NULL when user_provider IS NULL, causing the whole condition to
be NULL and skipping org creation; update the condition to explicitly handle
NULL (e.g., change the IF to use an explicit NULL check or COALESCE), for
example replace the current IF with: IF NOT ((user_provider IS NOT NULL AND
user_provider ~ '^sso:') AND has_sso) THEN ... so the regex test is only
evaluated when user_provider is non-NULL and has_sso is true before skipping the
INSERT into public.orgs (the INSERT INTO public.orgs ... RETURNING * INTO
org_record remains unchanged).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6a6c1bd5-e6d4-46a1-8b93-b13d8ea03684
📒 Files selected for processing (1)
supabase/migrations/20260501200000_remove_sso_enabled_flag.sql
Per CodeRabbit review: email signups on domains with an active SSO provider are handled by provision-user.ts and should not receive an auto-created personal org, same as genuine SAML SSO logins. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ins" This reverts commit c41b125.
|
Re: CodeRabbit inline comment on Ce comportement est intentionnel et préservé volontairement. La migration originale (
La raison : un utilisateur qui s'inscrit avec email/password sur un domaine SSO (ex: Changer la condition pour inclure |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
supabase/migrations/20260501200000_remove_sso_enabled_flag.sql (1)
80-82:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the SSO login guard NULL-safe before deciding org creation.
Line 80 still has a tri-valued boolean bug: if
user_providerisNULL,user_provider ~ '^sso:'isNULL, soNOT (NULL AND has_sso)isNULL, and theIFbody does not run. That can wrongly skip org creation.Suggested fix
- IF NOT (user_provider ~ '^sso:' AND has_sso) THEN + IF NOT ((COALESCE(user_provider, '') ~ '^sso:') AND has_sso) THEN INSERT INTO public.orgs (created_by, name, management_email) values (NEW.id, format('%s organization', NEW.first_name), NEW.email) RETURNING * INTO org_record; END IF;In PostgreSQL PL/pgSQL, what does NOT (NULL AND TRUE) evaluate to, and does IF execute the THEN branch when the condition is NULL?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260501200000_remove_sso_enabled_flag.sql` around lines 80 - 82, The IF condition using "user_provider ~ '^sso:' AND has_sso" is not NULL-safe so a NULL user_provider can produce NULL and prevent the THEN block from running; update the guard to a boolean-safe form such as using COALESCE or explicit IS TRUE checks — e.g., replace the condition in the IF that references user_provider and has_sso with something like "NOT (COALESCE(user_provider, '') ~ '^sso:' AND has_sso)" or "NOT ((user_provider ~ '^sso:') IS TRUE AND has_sso IS TRUE)" so org creation executes predictably when user_provider is NULL.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@supabase/migrations/20260501200000_remove_sso_enabled_flag.sql`:
- Around line 80-82: The IF condition using "user_provider ~ '^sso:' AND
has_sso" is not NULL-safe so a NULL user_provider can produce NULL and prevent
the THEN block from running; update the guard to a boolean-safe form such as
using COALESCE or explicit IS TRUE checks — e.g., replace the condition in the
IF that references user_provider and has_sso with something like "NOT
(COALESCE(user_provider, '') ~ '^sso:' AND has_sso)" or "NOT ((user_provider ~
'^sso:') IS TRUE AND has_sso IS TRUE)" so org creation executes predictably when
user_provider is NULL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5495423d-5447-46d4-8648-29a67798b8be
📒 Files selected for processing (1)
supabase/migrations/20260501200000_remove_sso_enabled_flag.sql
|
1 participant
Summary
sso_enabled(colonne Capgo-managed surorgs) qui cachait entièrement la section SSO dans les paramètres organisationcheck_domain_sso,get_sso_enforcement_by_domain,generate_org_on_user_create,get_orgs_v7)Fichiers modifiés
Security.vue— retire&& currentOrganization?.sso_enableddu v-ifproviders.ts— retire la vérification sso_enabled (le gate requireEnterprisePlan suffit)migrations/20260501120000_remove_sso_enabled_flag.sql— migration de suppression complètesrc/types/supabase.types.ts+supabase/functions/_backend/utils/supabase.types.ts— types mis à jourtests/sso.test.ts+tests/organization-put-stripe-sync.unit.test.ts— références retiréesNote
Après cette migration, tout org avec un provider SSO actif aura SSO actif — comportement attendu puisque le gate enterprise dans providers.ts garantit qu'on ne peut créer un provider que si on est enterprise.
🤖 Generated with Claude Code
Summary by CodeRabbit