fix(backend): prevent org existence oracle#1613
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46729ddb09
ℹ️ 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".
| const { data: org, error: orgError } = await userClient | ||
| .from('orgs') | ||
| .select('id, password_policy_config') | ||
| .eq('id', body.org_id) | ||
| .single() |
There was a problem hiding this comment.
Fetch org policy without password-policy RLS
The new userClient.from('orgs').select(...).single() lookup runs under normal org RLS, but org SELECT uses check_min_rights (supabase/schemas/prod.sql line 14619), and check_min_rights enforces password-policy compliance (supabase/schemas/prod.sql lines 1331-1343). That means a valid member who is currently non-compliant (the exact user this endpoint is meant to help) cannot read the org row, hits this branch, and gets 403 not_member instead of password_does_not_meet_policy, so they cannot complete the remediation flow. This undoes the purpose of rbac_check_permission_no_password_policy introduced in supabase/migrations/20260201042609_fix_password_policy_org_read_gate.sql.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR hardens the /private/validate_password_compliance endpoint against organization existence probing by authenticating first and normalizing responses for unknown/non-member orgs.
Changes:
- Authenticate before any org lookup and return stable
not_memberresponses for unknown/non-member org IDs. - Switch org policy lookup to use an authenticated Supabase client (instead of service-role) and keep compliance upsert via service-role.
- Update/add Vitest coverage to ensure invalid credentials do not reveal org existence.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/password-policy.test.ts | Updates expectations for non-existent orgs and adds a regression test ensuring invalid credentials don’t reveal org existence. |
| supabase/functions/_backend/private/validate_password_compliance.ts | Reorders flow to authenticate first and normalizes org-not-found vs non-member responses to avoid an existence oracle. |
| // Fetch the org's password policy (member-only). | ||
| const { data: org, error: orgError } = await userClient | ||
| .from('orgs') | ||
| .select('id, password_policy_config') | ||
| .eq('id', body.org_id) | ||
| .single() | ||
|
|
There was a problem hiding this comment.
The org password policy lookup is now done with userClient (RLS enforced). orgs SELECT is guarded by check_min_rights, which enforces user_meets_password_policy (and 2FA), so a user who is currently non-compliant may be unable to read org.password_policy_config and will get not_member, creating a circular dependency (they can’t validate to become compliant). Fetch the org’s policy using the service-role adminClient after the membership check, or expose a dedicated SECURITY DEFINER/RPC to read just the policy config for verified members without enforcing password policy.
📝 WalkthroughWalkthroughAuthentication-first flow: sign in with user credentials, create a per-user client, verify org membership, then fetch org password policy and upsert user compliance; error codes and test expectations updated accordingly. (49 words) Changes
Sequence DiagramsequenceDiagram
participant Client
participant Auth as Auth/Session
participant UserClient as User Client
participant OrgDB as Organization DB
participant AdminClient as Admin Client
Client->>Auth: Sign in (email/password)
alt invalid credentials
Auth-->>Client: 401 (invalid_credentials)
else valid credentials
Auth-->>UserClient: Session / user token
UserClient->>OrgDB: checkOrgReadAccess(org_id)
alt not a member / lookup failed
OrgDB-->>UserClient: not_member / error
UserClient-->>Client: 403 (not_member) or 500 (org_membership_lookup_failed)
else is member
UserClient->>OrgDB: fetch password_policy_config(org_id)
OrgDB-->>UserClient: policy_config
UserClient->>UserClient: validate password against policy, compute policy hash
UserClient-->>AdminClient: request upsert compliance (user_id, org_id, hash, result)
AdminClient->>OrgDB: upsert user_password_compliance
OrgDB-->>AdminClient: upsert result
AdminClient-->>Client: 200 (success) or 500 (compliance_update_failed)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
5cfa399 to
7b0192f
Compare
|
|
/tip @Judel777 $100 |
|
🎉🎈 @Judel777 has been awarded $100 by Capgo! 🎈🎊 |
Summary (AI generated)
/private/validate_password_complianceby authenticating first and returning a stablenot_memberfor non-members/non-existent orgs.check_min_rightspassword-policy enforcement loop).Motivation (AI generated)
/private/validate_password_complianceleaked org existence via status/error differences and must remain usable as the password-policy remediation path.Business Impact (AI generated)
Reduces org enumeration risk without breaking the password-policy compliance flow for legitimate users.
Test Plan (AI generated)
bun lint:backend && bun lintbunx supabase db resetSUPABASE_URL=http://127.0.0.1:54321 SUPABASE_ANON_KEY=<from bunx supabase status --output env> bun run test:backend -- tests/password-policy.test.tsGenerated with AI
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests