feat: 2FA enforcement - database #1300
Conversation
📝 WalkthroughWalkthroughAdds a single SQL migration implementing org-level 2FA enforcement (functions, column, grants, redaction) and five pgTAP test suites validating 2FA checks, enforcement in rights checks, org-data redaction, permission boundaries, and related utility functions. Changes
Sequence Diagram(s)(Skipped — changes are internal DB functions and tests; not a multi-component sequential flow suitable for a diagram.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-12-24T14:11:10.256ZApplied to files:
📚 Learning: 2025-12-05T17:34:36.569ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
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 (3.5.0)supabase/tests/20_test_org_management_functions.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: 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
supabase/migrations/20251224103713_2fa_enforcement.sql (2)
246-259: Consider caching the 2FA check result to avoid duplicate calls.
public.has_2fa_enabled(userid)is called twice in the CTE (lines 253 and 256). While PostgreSQL may optimize this, explicitly computing it once would be cleaner:🔎 Suggested optimization
two_fa_access AS ( SELECT o.id AS org_id, o.enforcing_2fa, + public.has_2fa_enabled(userid) AS user_has_2fa + FROM public.orgs o + JOIN public.org_users ou ON ou.user_id = userid AND o.id = ou.org_id + ), + two_fa_computed AS ( + SELECT + org_id, + enforcing_2fa, -- 2fa_has_access: true if enforcing_2fa is false OR (enforcing_2fa is true AND user has 2FA) CASE - WHEN o.enforcing_2fa = false THEN true - ELSE public.has_2fa_enabled(userid) + WHEN enforcing_2fa = false THEN true + ELSE user_has_2fa END AS "2fa_has_access", -- should_redact: true if org enforces 2FA and user doesn't have 2FA - (o.enforcing_2fa = true AND NOT public.has_2fa_enabled(userid)) AS should_redact - FROM public.orgs o - JOIN public.org_users ou ON ou.user_id = userid AND o.id = ou.org_id + (enforcing_2fa = true AND NOT user_has_2fa) AS should_redact + FROM two_fa_access )
549-581: Consider combining the existence check with the SELECT.The function checks if the org exists (line 558), then performs a separate SELECT (lines 563-565). These could be combined to reduce database round-trips:
🔎 Suggested optimization
DECLARE org_enforcing_2fa boolean; BEGIN - -- Check if org exists - IF NOT EXISTS (SELECT 1 FROM public.orgs WHERE public.orgs.id = reject_access_due_to_2fa.org_id) THEN - RETURN false; - END IF; - -- Check if org has 2FA enforcement enabled SELECT enforcing_2fa INTO org_enforcing_2fa FROM public.orgs WHERE public.orgs.id = reject_access_due_to_2fa.org_id; + -- If org doesn't exist, org_enforcing_2fa will be NULL + IF org_enforcing_2fa IS NULL THEN + RETURN false; + END IF; + -- 7.1 If a given org does not enable 2FA enforcement, return false IF org_enforcing_2fa = false THEN RETURN false; END IF;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
supabase/migrations/20251224103713_2fa_enforcement.sqlsupabase/tests/35_test_has_2fa_enabled.sqlsupabase/tests/36_test_check_org_members_2fa_enabled.sqlsupabase/tests/37_test_check_min_rights_2fa_enforcement.sqlsupabase/tests/38_test_get_orgs_v7_2fa_enforcement.sqlsupabase/tests/39_test_reject_access_due_to_2fa.sql
🧰 Additional context used
📓 Path-based instructions (1)
supabase/migrations/**/*.sql
📄 CodeRabbit inference engine (AGENTS.md)
supabase/migrations/**/*.sql: When a feature requires schema changes, create a single migration file withsupabase migration new <feature_slug>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 that 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 neededDatabase migrations must be created with
supabase migration new <feature_slug>and never modify previously committed migrations
Files:
supabase/migrations/20251224103713_2fa_enforcement.sql
🧠 Learnings (6)
📚 Learning: 2025-12-05T17:34:36.569Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
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/tests/35_test_has_2fa_enabled.sqlsupabase/tests/38_test_get_orgs_v7_2fa_enforcement.sqlsupabase/tests/36_test_check_org_members_2fa_enabled.sqlsupabase/tests/39_test_reject_access_due_to_2fa.sqlsupabase/tests/37_test_check_min_rights_2fa_enforcement.sqlsupabase/migrations/20251224103713_2fa_enforcement.sql
📚 Learning: 2025-12-05T17:34:36.569Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
Learning: Always cover database changes with Postgres-level tests and complement them with end-to-end tests for affected user flows
Applied to files:
supabase/tests/35_test_has_2fa_enabled.sqlsupabase/tests/38_test_get_orgs_v7_2fa_enforcement.sqlsupabase/tests/39_test_reject_access_due_to_2fa.sqlsupabase/tests/37_test_check_min_rights_2fa_enforcement.sql
📚 Learning: 2025-12-05T17:34:36.569Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
Learning: Applies to supabase/migrations/**/*.sql : A migration that introduces a new table may include seed inserts for that table, treating that seeding as part of the current feature and not modifying previously committed migrations
Applied to files:
supabase/migrations/20251224103713_2fa_enforcement.sql
📚 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/migrations/**/*.sql : Database migrations must be created with `supabase migration new <feature_slug>` and never modify previously committed migrations
Applied to files:
supabase/migrations/20251224103713_2fa_enforcement.sql
📚 Learning: 2025-12-05T17:34:36.569Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
Learning: Applies to supabase/migrations/**/*.sql : When a feature requires schema changes, create a single migration file with `supabase migration new <feature_slug>` and keep editing that file until the feature ships; never edit previously committed migrations
Applied to files:
supabase/migrations/20251224103713_2fa_enforcement.sql
📚 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/migrations/20251224103713_2fa_enforcement.sql
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (11)
supabase/migrations/20251224103713_2fa_enforcement.sql (6)
6-19: LGTM! Well-structured has_2fa_enabled() for session user.The function correctly:
- Uses
SECURITY DEFINERto accessauth.mfa_factors- Sets empty
search_pathto prevent search path injection- Uses
auth.uid()to get the current user securely- Checks for
verifiedstatus only
25-64: LGTM! Proper permission boundary for the user_id variant.The overloaded function correctly:
- Restricts access to
postgresandservice_roleonly- Revokes from
PUBLIC,anon, andauthenticatedbefore granting- Uses qualified column reference (
mfa.user_id) to avoid ambiguity
71-112: LGTM! Properly gated check_org_members_2fa_enabled function.Good implementation:
- Validates org existence before proceeding
- Requires
super_adminrights for access- Uses
COALESCEto handle potential NULL fromhas_2fa_enabled
118-123: LGTM! Safe schema migration with sensible defaults.The column addition uses
IF NOT EXISTSfor idempotency,NOT NULL DEFAULT falseensures backward compatibility, and the comment documents the purpose clearly.
333-396: LGTM! Well-structured wrapper with proper auth flow.The wrapper correctly:
- Prioritizes API key authentication
- Falls back to session identity
- Handles
limited_to_orgsfiltering for scoped API keys- Logs and raises exceptions for authentication failures
149-158: NULL handling for non-existent orgs is correct.When
org_iddoesn't exist,org_enforcing_2faremainsNULL, the 2FA check is skipped (sinceNULL = trueis falsy), and the permission loop also returnsfalsefor missing org_users entries. This dual safeguard is appropriate. For added clarity, consider explicitly checkingIF org_enforcing_2fa IS NOT NULL AND org_enforcing_2fa = true THENto make the intent more obvious.supabase/tests/35_test_has_2fa_enabled.sql (1)
1-157: LGTM! Comprehensive test coverage for has_2fa_enabled.The test file properly covers:
- Verified, unverified, and no MFA scenarios
- Session-based
has_2fa_enabled()for all user types- Service role access to
has_2fa_enabled(user_id)- Permission boundaries (authenticated and anon cannot call user_id variant)
- Function existence verification
supabase/tests/37_test_check_min_rights_2fa_enforcement.sql (1)
1-441: LGTM! Thorough test coverage for check_min_rights 2FA enforcement.Excellent coverage including:
- Orgs with/without 2FA enforcement
- Users with verified/unverified/no 2FA
- All permission levels (read, write, admin, super_admin)
- App-scoped permissions
- Edge cases (NULL user_id, non-existent org, user not in org)
- Verification that 2FA check precedes permission check
supabase/tests/38_test_get_orgs_v7_2fa_enforcement.sql (1)
1-479: LGTM! Comprehensive redaction testing for get_orgs_v7/v6.The test file thoroughly validates:
2fa_has_accessandenforcing_2fafield visibility- Redaction of all sensitive fields (paying, trial_left, management_email, etc.)
- Non-sensitive fields (name) remain visible to all users
- Consistency between
get_orgs_v7andget_orgs_v6redaction behavior- Real vs redacted value comparisons with actual stripe_info data
supabase/tests/39_test_reject_access_due_to_2fa.sql (1)
1-231: LGTM! Complete test coverage for reject_access_due_to_2fa.The test file covers:
- All user/org 2FA enforcement combinations
- Non-existent org returns false
- Permission boundaries (private function accessible only to service_role)
- Idempotency (multiple calls return same result)
- Function existence verification
supabase/tests/36_test_check_org_members_2fa_enabled.sql (1)
1-254: LGTM! Thorough test coverage for check_org_members_2fa_enabled.The test file validates:
- Super admin access and correct member count
- 2FA status for verified, unverified, and no MFA members
- Super admin's own 2FA status is included
- Permission denial for non-super_admin users
- Exception for non-existent org
- Function existence verification
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
supabase/migrations/20251224103713_2fa_enforcement.sql (2)
12-17: Optional: Simplify WHERE clause.The nested
SELECT auth.uid()within the WHERE clause works correctly but can be simplified for readability:RETURN EXISTS( SELECT 1 FROM auth.mfa_factors - WHERE (SELECT auth.uid()) = user_id + WHERE auth.uid() = user_id AND status = 'verified' );Both versions are functionally equivalent, but the simpler form is more direct.
577-594: Optional: Simplify conditional logic.The function correctly implements rejection logic, but the conditions can be streamlined:
🔎 Proposed refactoring
-- Check if org has 2FA enforcement enabled SELECT enforcing_2fa INTO org_enforcing_2fa FROM public.orgs WHERE public.orgs.id = reject_access_due_to_2fa.org_id; - -- 7.1 If a given org does not enable 2FA enforcement, return false - IF org_enforcing_2fa = false THEN - RETURN false; - END IF; - - -- 7.2 If a given org REQUIRES 2FA, and has_2fa_enabled(user_id) == false, return true - IF org_enforcing_2fa = true AND NOT public.has_2fa_enabled(reject_access_due_to_2fa.user_id) THEN + -- If org requires 2FA and user doesn't have it, reject access + IF COALESCE(org_enforcing_2fa, false) = true AND NOT public.has_2fa_enabled(reject_access_due_to_2fa.user_id) THEN PERFORM public.pg_log('deny: REJECT_ACCESS_DUE_TO_2FA', jsonb_build_object('org_id', org_id, 'user_id', user_id)); RETURN true; END IF; - -- 7.3 Otherwise, return false + -- Otherwise, allow access RETURN false;The
COALESCE(org_enforcing_2fa, false)handles NULL values explicitly. Both versions are functionally equivalent given theNOT NULL DEFAULT falseconstraint, but the simplified version is more direct.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supabase/migrations/20251224103713_2fa_enforcement.sql
🧰 Additional context used
📓 Path-based instructions (1)
supabase/migrations/**/*.sql
📄 CodeRabbit inference engine (AGENTS.md)
supabase/migrations/**/*.sql: When a feature requires schema changes, create a single migration file withsupabase migration new <feature_slug>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 that 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 neededDatabase migrations must be created with
supabase migration new <feature_slug>and never modify previously committed migrations
Files:
supabase/migrations/20251224103713_2fa_enforcement.sql
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
Learning: Always cover database changes with Postgres-level tests and complement them with end-to-end tests for affected user flows
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
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
📚 Learning: 2025-12-24T14:11:04.619Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:409-539
Timestamp: 2025-12-24T14:11:04.619Z
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/20251224103713_2fa_enforcement.sql
📚 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/migrations/20251224103713_2fa_enforcement.sql
📚 Learning: 2025-12-05T17:34:36.569Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
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/20251224103713_2fa_enforcement.sql
📚 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/migrations/20251224103713_2fa_enforcement.sql
📚 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 database operations must use `getPgClient()` or `getDrizzleClient()` from `utils/pg.ts` for PostgreSQL access during active migration to Cloudflare D1
Applied to files:
supabase/migrations/20251224103713_2fa_enforcement.sql
📚 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/migrations/20251224103713_2fa_enforcement.sql
📚 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/utils/postgress_schema.ts : Schema definitions must be placed in `utils/postgress_schema.ts` using Drizzle ORM and never edited in committed migration files
Applied to files:
supabase/migrations/20251224103713_2fa_enforcement.sql
📚 Learning: 2025-12-05T17:34:25.556Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-05T17:34:25.556Z
Learning: Applies to supabase/functions/_backend/** : Backend logic should be organized in `supabase/functions/_backend/` with subdirectories for plugins, private endpoints, public endpoints, triggers, and utilities
Applied to files:
supabase/migrations/20251224103713_2fa_enforcement.sql
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (6)
supabase/migrations/20251224103713_2fa_enforcement.sql (6)
118-124: LGTM!The column addition is safe:
NOT NULL DEFAULT falseensures existing orgs are not affectedIF NOT EXISTSmakes the migration idempotent- Clear documentation comment
149-158: LGTM: 2FA check correctly precedes permission checks.The 2FA enforcement logic is correctly implemented:
- Checks org-level enforcement before evaluating user permissions
- Handles NULL values safely (non-existent orgs or NULL enforcing_2fa treated as false)
- Logs denials consistently with existing patterns
Note: This enforcement applies to all functions using
check_min_rights, includingcheck_org_members_2fa_enabled(see earlier comment about potential circular dependency).
184-324: LGTM: get_orgs_v7(userid uuid) correctly implements 2FA-aware redaction.The inner function properly:
- Computes
2fa_has_accessbased on org enforcement and user 2FA status (lines 251-254)- Redacts sensitive fields (paying, trial_left, app_count, subscription dates, management_email, etc.) when
should_redact = true- Restricts access to postgres/service_role only (security fix confirmed per learning)
The redaction logic preserves enough information (org name, logo, user role) for users to see which orgs exist while hiding financial details until they enable 2FA.
338-401: LGTM: Wrapper function correctly handles authentication.The no-argument wrapper properly:
- Validates API keys and filters by
limited_to_orgswhen applicable (lines 371-388)- Falls back to session-based authentication (lines 390-397)
- Logs and raises exceptions on authentication failures
- Is correctly granted to anon/authenticated since it performs auth checks internally
416-544: LGTM: get_orgs_v6(userid uuid) correctly backports 2FA redaction.The function properly:
- Adds the same
two_fa_accessCTE and redaction logic as v7 (lines 474-524)- Maintains backward compatibility by keeping the same return schema (no enforcing_2fa/2fa_has_access fields)
- Is now correctly restricted to postgres/service_role only (security fix confirmed)
This ensures v6 users also benefit from 2FA-aware data protection.
564-615: LGTM: Rejection logic is correct and properly secured.The function correctly:
- Returns false for non-existent orgs (graceful handling)
- Returns false when 2FA is not enforced
- Returns true only when org enforces 2FA and user lacks it
- Logs rejections for audit trails
- Is properly restricted to postgres/service_role
|
Changes (AI generated)
Section 1: has_2fa_enabled functions
has_2fa_enabled()function that checks if the current authenticateduser has any verified MFA factors
has_2fa_enabled(user_id uuid)function that checks if a specificuser has any verified MFA factors
authenticatedandanonrolesuser_idversion is private and only accessible topostgresandservice_rolerolesSECURITY DEFINERto accessauth.mfa_factorstableSection 2: check_org_members_2fa_enabled function
check_org_members_2fa_enabled(org_id uuid)function that returns alist of all organization members with their 2FA status
super_adminsof the organizationuser_idand2fa_enabledcolumns for all membersrights
Section 3: Add enforcing_2fa column to orgs table
enforcing_2faboolean column topublic.orgstable withNOT NULL DEFAULT falseenabled
Section 4: Modify check_min_rights to enforce 2FA
check_min_rights(user_id uuid, org_id uuid, ...)function to check2FA enforcement rules
enforcing_2faenabled and the user doesn't have 2FAenabled, the function returns
falseand logs the denialare denied access even if they would otherwise have permissions
unchanged
Section 4.A: Create get_orgs_v7 with 2FA enforcement
get_orgs_v7(userid uuid)function based onget_orgs_v6with2FA enforcement support
enforcing_2fafield to the return table showing whether the orgrequires 2FA
2fa_has_accessfield indicating if the user has access (true if orgdoesn't enforce 2FA OR user has 2FA enabled)
paying,trial_left,can_use_more,is_canceled,app_count,subscription_start,subscription_end,management_email,is_yearly) when user doesn't have 2FA accesstwo_fa_access) to compute 2FA access status efficientlyget_orgs_v7()that handles API key andauthentication logic
Section 4.B: Modify get_orgs_v6 to prevent information leakage
get_orgs_v6(userid uuid)function to redact sensitive informationwhen users don't have 2FA access
for sensitive fields when org enforces 2FA and user doesn't have it
two_fa_accessCTE pattern asget_orgs_v7for consistencyredaction logic added
Section 5: reject_access_due_to_2fa function
reject_access_due_to_2fa(org_id uuid, user_id uuid)function thatreturns boolean
trueif org requires 2FA and user doesn't have it enabled,falseotherwise
falseif org doesn't enforce 2FA enforcementfalseif org doesn't existpostgresandservice_roleroles (not accessible to regular users)
pg_logfor audit purposesTests
35_test_has_2fa_enabled.sqlcovering bothfunction variants and permission checks
36_test_check_org_members_2fa_enabled.sqlcovering super_admin access, member 2FA status, and edge cases
37_test_check_min_rights_2fa_enforcement.sqlcovering 2FA enforcement blocking access, normal access scenarios, different
permission levels, and edge cases
38_test_get_orgs_v7_2fa_enforcement.sqlcovering field visibility, redaction verification, and comparison between
users with and without 2FA
39_test_reject_access_due_to_2fa.sqlcovering functionbehavior, permission checks, and verification that regular users cannot call
the private function
permission boundaries
Motivation
This PR is the direct resonse to part one of #1291. I strongly believe that the changes of this PR bring Capgo closer to fully implemeneting the 2FA enforcement feature. As such, I believe this PR is a high priority for the development of capgo and it should be reviewed and merged as soon as possible, so further development of the 2FA enforcement can continue.
Business impact
The feature of 2FA enforcement has been deemed important. (Medium business impact. Enterprise clients often consider 2FA enforcement to provide good operation security for their organization). However, this PR on its own does not fully implement the 2FA enforcement feature, so on its own this PR doesn't have a business impact.
Questions to be ansered during the review
Q: Should the get_orgs_v6 be modified to prevent the leak of sensitive data when 2FA enforcement is enabled?
Q: How to lint the SQL?I ran
npm run lint:sqlin 278e0d1Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
Note
Implements database-side 2FA enforcement and redaction to protect org data when users lack verified MFA.
enforcing_2facolumn toorgs; updatecheck_min_rightsto deny access when enforced andhas_2fa_enabled(user)is false (logged viapg_log).has_2fa_enabled()/has_2fa_enabled(uuid)(MFA status),check_org_members_2fa_enabled(org_id)(super_admin-only member 2FA status),reject_access_due_to_2fa(org_id, user_id)(backend-only gating helper).get_orgs_v7(userid)and wrapperget_orgs_v7()returningenforcing_2faand2fa_has_access; redact sensitive fields when access is blocked. Modifyget_orgs_v6(userid)to apply equivalent redaction.uuidvariants and backend helpers restricted topostgres/service_role.Written by Cursor Bugbot for commit 287fd7c. This will update automatically on new commits. Configure here.