Skip to content

Fix password compliance validation flow#1544

Closed
riderx wants to merge 9 commits into
mainfrom
riderx/password-policy-tests
Closed

Fix password compliance validation flow#1544
riderx wants to merge 9 commits into
mainfrom
riderx/password-policy-tests

Conversation

@riderx
Copy link
Copy Markdown
Member

@riderx riderx commented Jan 30, 2026

Summary (AI generated)

  • Keep the admin client in service-role context while validating password compliance and membership.
  • Use the membership RPC to avoid policy-enforced access blocking during compliance checks.
  • Add isolated tests for compliant org members and non-members plus auth-user setup helpers.

Test plan (AI generated)

  • bun test:backend -- tests/password-policy.test.ts
  • bun typecheck

Screenshots (AI generated)

  • Not applicable (backend-only changes).

Checklist (AI generated)

  • My code follows the code style of this project and passes
    /Users/martindonadieu/conductor/workspaces/capgo/tunis/supabase/functions/_backend/utils/stats.ts
    5:191 error Expected "readDevicesCF" to come before "readDeviceVersionCountsCF" perfectionist/sort-named-imports
    9:108 error Expected "readDevicesSB" to come before "readDeviceVersionCountsSB" perfectionist/sort-named-imports

✖ 2 problems (2 errors, 0 warnings)
2 errors and 0 warnings potentially fixable with the --fix option..

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • My change has adequate E2E test coverage.
  • I have tested my code manually, and I have provided steps how to reproduce my tests.

Generated with AI

Summary by CodeRabbit

  • Bug Fixes

    • Improved password verification flow with clearer, user-facing error messages and consistent handling of verification failures.
    • Strengthened membership and authorization checks before allowing password compliance actions.
    • Added conditional re‑verification after password changes so org state refreshes only when policy compliance is confirmed.
  • Tests

    • Added extensive tests for password policy validation, membership rejection, and policy-hash consistency.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings January 30, 2026 21:33
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

Frontend adds an org-scoped password verification helper and delegates verification/reauth flows to it; backend validate_password_compliance moves to a dual-client model, tightens membership and policy checks, and improves error handling; tests add comprehensive password-policy scenarios and utilities.

Changes

Cohort / File(s) Summary
Frontend: ChangePassword
src/pages/settings/account/ChangePassword.vue
Adds validatePasswordForOrg(password, formId) helper; verifyPassword delegates to it and maps backend errors to FormKit errors; submit flow conditionally re-verifies new password against org policy when enabled; clears fields and refreshes org state on success.
Backend: Password Compliance
supabase/functions/_backend/private/validate_password_compliance.ts
Switches to dual-client pattern (service auth + admin), fetches created_by, adds public-user lookup by email, replaces RBAC call with ownership + is_member_of_org check, enforces upfront policy evaluation (passwordMeetsPolicy), retrieves policy hash via admin RPC, upserts via admin client, and expands logging/error handling.
Supabase Client Config
supabase/functions/_backend/utils/supabase.ts
Adds serviceRoleKey fallback: uses SUPABASE_SERVICE_ROLE_KEY or falls back to SUPABASE_SERVICE_KEY when constructing the admin client.
Tests: Password Policy
tests/password-policy.test.ts
Adds test utilities (createAuthUser, cleanupAuthUser, executeSQL, constants) and extensive tests covering validate_password_compliance: valid/invalid credentials, membership rejection, multiple org scenarios, policy-hash checks, and cleanup flows.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client App
    participant FE as ChangePassword.vue
    participant BE as validate_password_compliance (Function)
    participant Auth as Service Auth Client
    participant Admin as Admin Client
    participant DB as Supabase DB / RPCs

    Client->>FE: request verifyPassword(currentPassword)
    FE->>BE: POST /private/validate_password_compliance { email, orgId, password }
    BE->>Auth: signInWithPassword(email, password) (service auth client)
    Auth-->>BE: auth result / access token
    BE->>Admin: SELECT org by id (id, password_policy_config, created_by)
    Admin-->>BE: org data
    BE->>Admin: SELECT public user by email
    Admin-->>BE: public user data
    BE->>DB: RPC is_member_of_org(userId, orgId)
    DB-->>BE: membership status
    alt authorized (owner or member)
        BE->>DB: RPC passwordMeetsPolicy(password, policy)
        DB-->>BE: compliance result
        alt compliant
            BE->>Admin: upsert user_password_compliance
            Admin-->>BE: upsert result
            BE-->>FE: { ok: true }
        else not compliant
            BE-->>FE: { error: password_does_not_meet_policy }
        end
    else not authorized
        BE-->>FE: { error: not_member / 403 }
    end
    FE->>Client: show toast or surface FormKit errors based on response
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 I hopped through code to check each rule,

orgs and tokens kept my whiskers cool,
frontend asks, backend proves true,
tests hop in to sing it too,
carrots earned for security stew 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is largely incomplete. While it includes an AI-generated summary and test plan, the description lacks proper detail, context, and reasoning. The checklist shows unresolved linting errors and unchecked items, indicating the description was not thoroughly reviewed. Complete the description with detailed explanations of why changes were made, review and resolve linting errors listed in the checklist, and ensure all checklist items are properly evaluated before merging.
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix password compliance validation flow' clearly and accurately reflects the main changes: refactoring password compliance validation logic across backend and frontend components.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch riderx/password-policy-tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@supabase/functions/_backend/private/validate_password_compliance.ts`:
- Around line 119-131: Ownership and membership checks are inconsistent: isOwner
currently compares org.created_by to both membershipUserId and userId but the
membership RPC (is_member_of_org) only uses membershipUserId; update the
ownership check so it only compares org.created_by === membershipUserId (remove
the org.created_by === userId comparison) to match the org_users/public.users
schema and keep membership verification via the is_member_of_org RPC using
membershipUserId, leaving isMember and memberError handling unchanged.
🧹 Nitpick comments (3)
src/pages/settings/account/ChangePassword.vue (2)

156-158: Consider sanitizing error messages before displaying to users.

Line 157 displays result.message directly from the backend response. If the backend returns detailed error messages (e.g., database errors, stack traces), these could be exposed to users. Consider using only the translation fallback or sanitizing the message.

🛡️ Suggested fix
     else {
-      setErrors(formId, [result.message || t('verification-failed')], {})
+      setErrors(formId, [t('verification-failed')], {})
     }

278-289: Consider notifying users if compliance verification fails after password change.

If the password change succeeds but validatePasswordForOrg fails (Line 282), the user sees a success toast but may still lack organization access. The validatePasswordForOrg function does set form errors, but they may not be visible at this point in the flow. Consider showing an explicit warning if compliance verification fails after a successful password change.

tests/password-policy.test.ts (1)

406-406: Consider using it.concurrent() for parallel test execution.

As per coding guidelines, using it.concurrent() instead of it() would maximize parallelism in CI/CD. These tests are well-isolated and should be safe to run concurrently.

♻️ Example refactor
-  it('accepts valid credentials and marks compliance for org members', async () => {
+  it.concurrent('accepts valid credentials and marks compliance for org members', async () => {
-  it('rejects request when user is not a member of the org', async () => {
+  it.concurrent('rejects request when user is not a member of the org', async () => {

Based on learnings: "Use it.concurrent() instead of it() to run tests in parallel within the same file to maximize parallelism in CI/CD"

Also applies to: 481-481

Comment on lines +119 to 131
const membershipUserId = publicUser?.id ?? userId
const isOwner = org.created_by === membershipUserId || org.created_by === userId
const { data: isMember, error: memberError } = await supabaseAdmin
.rpc('is_member_of_org', { user_id: membershipUserId, org_id: body.org_id })

// Verify user has access to this organization (RBAC + legacy compatible)
const { data: hasOrgAccess, error: accessError } = await supabase
.rpc('rbac_check_permission', {
p_permission_key: 'org.read',
p_org_id: body.org_id,
})
if (memberError) {
cloudlog({ requestId: c.get('requestId'), context: 'validate_password_compliance - membership rpc failed', error: memberError.message })
return quickError(500, 'membership_check_failed', 'Failed to verify organization membership', { error: memberError.message })
}

if (accessError || !hasOrgAccess) {
if (!isOwner && !isMember) {
return quickError(403, 'not_member', 'You are not a member of this organization')
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Locate the is_member_of_org RPC implementation
rg -nA 30 'CREATE.*FUNCTION.*is_member_of_org' --glob '*.sql'

Repository: Cap-go/capgo

Length of output: 4236


🏁 Script executed:

#!/bin/bash
# Check the validate_password_compliance.ts file for publicUser context
cat -n supabase/functions/_backend/private/validate_password_compliance.ts | head -150

Repository: Cap-go/capgo

Length of output: 6757


🏁 Script executed:

#!/bin/bash
# Check if there are other calls to is_member_of_org to understand expected usage patterns
rg -n 'is_member_of_org' --glob '*.ts' supabase/functions/_backend/

Repository: Cap-go/capgo

Length of output: 373


🏁 Script executed:

#!/bin/bash
# Check the usage in get.ts to see if there's a pattern
cat -n supabase/functions/_backend/public/app/get.ts | sed -n '50,70p'

Repository: Cap-go/capgo

Length of output: 900


🏁 Script executed:

#!/bin/bash
# Understand the relationship between auth.users.id and users table.id
# Check if users table has a foreign key to auth.users
rg -A 10 'CREATE TABLE.*users' supabase/migrations/ | head -50

Repository: Cap-go/capgo

Length of output: 2920


Fix asymmetry between ownership and membership authorization checks.

The ownership check at line 120 compares org.created_by against both membershipUserId and userId, but the membership RPC at line 122 only checks membershipUserId. Since org_users.user_id references the public.users table (not auth.users), this inconsistency causes broken authorization logic: if org.created_by === userId (the auth ID), the ownership check passes, but the membership check ignores that ID entirely.

Align both checks to use only membershipUserId for consistency with the org_users table schema.

🤖 Prompt for AI Agents
In `@supabase/functions/_backend/private/validate_password_compliance.ts` around
lines 119 - 131, Ownership and membership checks are inconsistent: isOwner
currently compares org.created_by to both membershipUserId and userId but the
membership RPC (is_member_of_org) only uses membershipUserId; update the
ownership check so it only compares org.created_by === membershipUserId (remove
the org.created_by === userId comparison) to match the org_users/public.users
schema and keep membership verification via the is_member_of_org RPC using
membershipUserId, leaving isMember and memberError handling unchanged.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the password compliance validation flow by keeping the admin client in service-role context and using the is_member_of_org RPC to check membership without RLS policy interference.

Changes:

  • Replaced RBAC permission check with direct is_member_of_org RPC call to avoid RLS-related access issues
  • Introduced separate Supabase client (emptySupabase) for authentication to preserve admin client context
  • Added comprehensive test helpers (createAuthUser, cleanupAuthUser) for isolated password policy tests
  • Refactored frontend password validation into reusable validatePasswordForOrg function
  • Added service role key fallback (SUPABASE_SERVICE_KEY) for backward compatibility

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
tests/password-policy.test.ts Adds test helpers for creating auth users and two new tests: one verifying successful compliance marking for org members, another confirming rejection for non-members
supabase/functions/_backend/private/validate_password_compliance.ts Replaces authenticated client and RBAC check with admin client and is_member_of_org RPC; uses separate auth client for password verification to maintain service-role context
supabase/functions/_backend/utils/supabase.ts Adds fallback from SUPABASE_SERVICE_ROLE_KEY to SUPABASE_SERVICE_KEY for backward compatibility
src/pages/settings/account/ChangePassword.vue Extracts password validation logic into reusable validatePasswordForOrg function; adds post-password-change compliance validation

it('rejects request when user is not a member of the org', async () => {
const nonMemberOrgId = randomUUID()
const nonMemberCustomerId = `cus_pwd_non_member_${nonMemberOrgId}`
const { email, password, userId } = await createAuthUser()
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The destructuring order of the return value from createAuthUser() is inconsistent. Line 427 uses { email, userId, password } while line 484 uses { email, password, userId }. While JavaScript object destructuring is order-independent, maintaining consistent ordering improves code readability and reduces confusion. Consider using the same order (email, userId, password) that matches the return statement at line 95.

Suggested change
const { email, password, userId } = await createAuthUser()
const { email, userId, password } = await createAuthUser()

Copilot uses AI. Check for mistakes.
Comment on lines +96 to 100
const supabaseAuth = emptySupabase(c)
const { data: signInData, error: signInError } = await supabaseAuth.auth.signInWithPassword({
email: body.email,
password: body.password,
})
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sign-in operation at line 97 creates a new session for the user, which could have unintended side effects such as triggering session-related database triggers, analytics, or audit logs. Consider documenting this behavior in a comment, or investigate whether there's an alternative method to validate credentials without creating a full session (though this may not be possible with Supabase's current API).

Copilot uses AI. Check for mistakes.
Comment on lines +281 to +285
if (policyEnabled) {
const complianceOk = await validatePasswordForOrg(form.password, 'change-pass')
if (complianceOk) {
await organizationStore.fetchOrganizations()
}
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After successfully changing the password, the code validates password compliance with the new password and displays error messages to the user if validation fails. However, at this point the password has already been changed in Supabase Auth. This creates a confusing user experience where the password change succeeds but then shows an error. Consider either: (1) validating the new password against the policy before calling supabase.auth.updateUser(), or (2) silently logging compliance validation failures after password change without showing errors to the user, since the primary operation (password change) has already succeeded.

Copilot uses AI. Check for mistakes.
}
else if (result.error === 'no_policy') {
setErrors(formId, [t('verification-failed')], {})
}
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The no_policy error case is handled in the frontend but shouldn't be reachable in normal operation since the endpoint is only called when policyEnabled is true (line 281). However, having this error handling is defensive programming and not a problem. Consider adding a similar check for the not_member error case which is returned by the backend at line 130 of the validate_password_compliance endpoint, to provide users with a clearer error message when they're not a member of the organization.

Suggested change
}
}
else if (result.error === 'not_member') {
setErrors(formId, [t('not-member-of-organization')], {})
}

Copilot uses AI. Check for mistakes.
Comment on lines +119 to 130
const membershipUserId = publicUser?.id ?? userId
const isOwner = org.created_by === membershipUserId || org.created_by === userId
const { data: isMember, error: memberError } = await supabaseAdmin
.rpc('is_member_of_org', { user_id: membershipUserId, org_id: body.org_id })

// Verify user has access to this organization (RBAC + legacy compatible)
const { data: hasOrgAccess, error: accessError } = await supabase
.rpc('rbac_check_permission', {
p_permission_key: 'org.read',
p_org_id: body.org_id,
})
if (memberError) {
cloudlog({ requestId: c.get('requestId'), context: 'validate_password_compliance - membership rpc failed', error: memberError.message })
return quickError(500, 'membership_check_failed', 'Failed to verify organization membership', { error: memberError.message })
}

if (accessError || !hasOrgAccess) {
if (!isOwner && !isMember) {
return quickError(403, 'not_member', 'You are not a member of this organization')
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ownership check on line 120 uses OR logic (isOwner || isMember) at line 129, but the ownership check itself is redundant with membership. When an org is created, a trigger (generate_org_user_on_org_create) automatically adds the created_by user to org_users with super_admin rights. This means org.created_by users are always members, making the separate isOwner check unnecessary. The !isOwner && !isMember condition could be simplified to !isMember. However, keeping the explicit owner check may be defensive programming for edge cases where the trigger might fail.

Copilot uses AI. Check for mistakes.
@riderx riderx force-pushed the riderx/password-policy-tests branch from 1ef8d57 to 1d54a80 Compare January 31, 2026 15:51
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@tests/password-policy.test.ts`:
- Line 4: Tests are making raw fetch calls using BASE_URL/concatenated paths;
replace those with the test helper getEndpointUrl(path) imported from
test-utils.ts so requests route correctly in worker environments (e.g., change
fetch(`${BASE_URL}/some/path`, ...) to fetch(getEndpointUrl('/some/path'),
...)), update all similar fetches mentioned (including the ones around the other
ranges noted), and ensure the test run uses the USE_CLOUDFLARE_WORKERS=true
environment flag as required by the test guidelines.
- Around line 406-535: The tests "accepts valid credentials and marks compliance
for org members" and "rejects request when user is not a member of the org"
should be converted to run in parallel; replace the two it(...) declarations in
tests/password-policy.test.ts (the tests with descriptions starting "accepts
valid credentials..." and "rejects request when user is not a member...") with
it.concurrent(...) so they execute concurrently while keeping the same bodies
and cleanup logic.

import { afterAll, beforeAll, describe, expect, it } from 'vitest'

import { BASE_URL, getSupabaseClient, headers, TEST_EMAIL, USER_ID } from './test-utils.ts'
import { BASE_URL, executeSQL, getSupabaseClient, headers, TEST_EMAIL, USER_ID, USER_ID_2 } from './test-utils.ts'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use getEndpointUrl() for worker routing in new fetch calls.

Switch the new fetches to the test helper so they route correctly across worker environments.

Suggested update
-import { BASE_URL, executeSQL, getSupabaseClient, headers, TEST_EMAIL, USER_ID, USER_ID_2 } from './test-utils.ts'
+import { executeSQL, getEndpointUrl, getSupabaseClient, headers, TEST_EMAIL, USER_ID, USER_ID_2 } from './test-utils.ts'
-      const response = await fetch(`${BASE_URL}/private/validate_password_compliance`, {
+      const response = await fetch(getEndpointUrl('/private/validate_password_compliance'), {
         headers,
         method: 'POST',
         body: JSON.stringify({
           email,
           password,
           org_id: testOrgId,
         }),
       })
-      const response = await fetch(`${BASE_URL}/private/validate_password_compliance`, {
+      const response = await fetch(getEndpointUrl('/private/validate_password_compliance'), {
         headers,
         method: 'POST',
         body: JSON.stringify({
           email,
           password,
           org_id: nonMemberOrgId,
         }),
       })

As per coding guidelines: tests/**/*.{ts,js}: Backend tests must use helpers from tests/test-utils.ts including getEndpointUrl(path) for correct worker routing and USE_CLOUDFLARE_WORKERS=true for CF Workers testing.

Also applies to: 450-457, 514-521

🤖 Prompt for AI Agents
In `@tests/password-policy.test.ts` at line 4, Tests are making raw fetch calls
using BASE_URL/concatenated paths; replace those with the test helper
getEndpointUrl(path) imported from test-utils.ts so requests route correctly in
worker environments (e.g., change fetch(`${BASE_URL}/some/path`, ...) to
fetch(getEndpointUrl('/some/path'), ...)), update all similar fetches mentioned
(including the ones around the other ranges noted), and ensure the test run uses
the USE_CLOUDFLARE_WORKERS=true environment flag as required by the test
guidelines.

Comment on lines +406 to +535
it('accepts valid credentials and marks compliance for org members', async () => {
const testOrgId = randomUUID()
const testCustomerId = `cus_pwd_success_${testOrgId}`
const policyConfig = {
enabled: true,
min_length: 6,
require_uppercase: false,
require_number: false,
require_special: false,
}
const { error: stripeError } = await getSupabaseClient().from('stripe_info').insert({
customer_id: testCustomerId,
status: 'succeeded',
product_id: 'prod_LQIregjtNduh4q',
subscription_id: `sub_pwd_success_${testOrgId}`,
trial_at: new Date(Date.now() + 15 * 24 * 60 * 60 * 1000).toISOString(),
is_good_plan: true,
})
if (stripeError)
throw stripeError

const { email, userId, password } = await createAuthUser()

const { error: orgError } = await getSupabaseClient().from('orgs').insert({
id: testOrgId,
name: `Password policy success org ${testOrgId}`,
management_email: TEST_EMAIL,
created_by: userId,
customer_id: testCustomerId,
password_policy_config: policyConfig,
})
if (orgError)
throw orgError

const orgRows = await executeSQL('SELECT id FROM public.orgs WHERE id = $1', [testOrgId])
if (orgRows.length === 0)
throw new Error('Org was not created for password policy test')

const orgCreatedBy = await executeSQL('SELECT created_by FROM public.orgs WHERE id = $1', [testOrgId])
const orgUsers = await executeSQL('SELECT user_id FROM public.org_users WHERE user_id = $1 AND org_id = $2', [userId, testOrgId])
expect(orgCreatedBy[0]?.created_by).toBe(userId)
expect(orgUsers.length).toBeGreaterThan(0)

try {
const response = await fetch(`${BASE_URL}/private/validate_password_compliance`, {
headers,
method: 'POST',
body: JSON.stringify({
email,
password,
org_id: testOrgId,
}),
})

const responseData = await response.json() as { status?: string, error?: string, message?: string }
expect(response.status).toBe(200)
expect(responseData.status).toBe('ok')

const { data: meetsPolicy, error: meetsError } = await getSupabaseClient().rpc('user_meets_password_policy', {
user_id: userId,
org_id: testOrgId,
})

expect(meetsError).toBeNull()
expect(meetsPolicy).toBe(true)
}
finally {
await getSupabaseClient().from('user_password_compliance').delete().eq('org_id', testOrgId)
await getSupabaseClient().from('org_users').delete().eq('org_id', testOrgId)
await getSupabaseClient().from('orgs').delete().eq('id', testOrgId)
await getSupabaseClient().from('stripe_info').delete().eq('customer_id', testCustomerId)
await cleanupAuthUser(userId)
}
})

it('rejects request when user is not a member of the org', async () => {
const nonMemberOrgId = randomUUID()
const nonMemberCustomerId = `cus_pwd_non_member_${nonMemberOrgId}`
const { email, password, userId } = await createAuthUser()
const { error: stripeError } = await getSupabaseClient().from('stripe_info').insert({
customer_id: nonMemberCustomerId,
status: 'succeeded',
product_id: 'prod_LQIregjtNduh4q',
subscription_id: `sub_non_member_${nonMemberOrgId}`,
trial_at: new Date(Date.now() + 15 * 24 * 60 * 60 * 1000).toISOString(),
is_good_plan: true,
})
if (stripeError)
throw stripeError

const { error: orgError } = await getSupabaseClient().from('orgs').insert({
id: nonMemberOrgId,
name: `Non-member org ${nonMemberOrgId}`,
management_email: TEST_EMAIL,
created_by: USER_ID_2,
customer_id: nonMemberCustomerId,
password_policy_config: {
enabled: true,
min_length: 6,
require_uppercase: false,
require_number: false,
require_special: false,
},
})
if (orgError)
throw orgError

try {
const response = await fetch(`${BASE_URL}/private/validate_password_compliance`, {
headers,
method: 'POST',
body: JSON.stringify({
email,
password,
org_id: nonMemberOrgId,
}),
})

expect(response.status).toBe(403)
const responseData = await response.json() as { error?: string }
expect(responseData.error).toBe('not_member')
}
finally {
await getSupabaseClient().from('user_password_compliance').delete().eq('org_id', nonMemberOrgId)
await getSupabaseClient().from('org_users').delete().eq('org_id', nonMemberOrgId)
await getSupabaseClient().from('orgs').delete().eq('id', nonMemberOrgId)
await getSupabaseClient().from('stripe_info').delete().eq('customer_id', nonMemberCustomerId)
await cleanupAuthUser(userId)
}
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Check if the test file exists and review the structure
fd password-policy.test.ts
wc -l tests/password-policy.test.ts

Repository: Cap-go/capgo

Length of output: 119


🏁 Script executed:

# Check if Vitest is configured in the project
fd -e config -e json | grep -E "(vite|vitest)" | head -20
cat vitest.config.ts 2>/dev/null || cat vitest.config.js 2>/dev/null || echo "No vitest config found at root"

Repository: Cap-go/capgo

Length of output: 938


🏁 Script executed:

# Check package.json for Vitest dependency
cat package.json | grep -A 5 -B 5 "vitest"

Repository: Cap-go/capgo

Length of output: 2902


🏁 Script executed:

# Check the test declarations in the file
rg "^\s*(it|it\.concurrent)\(" tests/password-policy.test.ts | head -30

Repository: Cap-go/capgo

Length of output: 1444


🏁 Script executed:

# Get a more detailed view of lines around the two tests mentioned in the review
sed -n '406,415p' tests/password-policy.test.ts
sed -n '457,466p' tests/password-policy.test.ts

Repository: Cap-go/capgo

Length of output: 782


🏁 Script executed:

# Check if any tests in this file use it.concurrent() to establish pattern
rg "it\.concurrent\(" tests/password-policy.test.ts | wc -l

Repository: Cap-go/capgo

Length of output: 57


🏁 Script executed:

# Get more context around the test data creation to verify isolation
sed -n '406,475p' tests/password-policy.test.ts

Repository: Cap-go/capgo

Length of output: 2729


🏁 Script executed:

# Check if tests use any shared mutable state or globals that might prevent concurrency
rg "^(const|let|var) (TEST_EMAIL|USER_ID_2|BASE_URL|headers)" tests/password-policy.test.ts | head -20

Repository: Cap-go/capgo

Length of output: 38


🏁 Script executed:

# Check the structure of the entire test file to see test organization and potential dependencies
head -100 tests/password-policy.test.ts

Repository: Cap-go/capgo

Length of output: 2642


Use it.concurrent() for the new tests.

These tests create isolated seed data with unique IDs and properly clean up in finally blocks, meeting all requirements for parallel execution.

Suggested update
-  it('accepts valid credentials and marks compliance for org members', async () => {
+  it.concurrent('accepts valid credentials and marks compliance for org members', async () => {
     // ...
   })
-  it('rejects request when user is not a member of the org', async () => {
+  it.concurrent('rejects request when user is not a member of the org', async () => {
     // ...
   })
📝 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.

Suggested change
it('accepts valid credentials and marks compliance for org members', async () => {
const testOrgId = randomUUID()
const testCustomerId = `cus_pwd_success_${testOrgId}`
const policyConfig = {
enabled: true,
min_length: 6,
require_uppercase: false,
require_number: false,
require_special: false,
}
const { error: stripeError } = await getSupabaseClient().from('stripe_info').insert({
customer_id: testCustomerId,
status: 'succeeded',
product_id: 'prod_LQIregjtNduh4q',
subscription_id: `sub_pwd_success_${testOrgId}`,
trial_at: new Date(Date.now() + 15 * 24 * 60 * 60 * 1000).toISOString(),
is_good_plan: true,
})
if (stripeError)
throw stripeError
const { email, userId, password } = await createAuthUser()
const { error: orgError } = await getSupabaseClient().from('orgs').insert({
id: testOrgId,
name: `Password policy success org ${testOrgId}`,
management_email: TEST_EMAIL,
created_by: userId,
customer_id: testCustomerId,
password_policy_config: policyConfig,
})
if (orgError)
throw orgError
const orgRows = await executeSQL('SELECT id FROM public.orgs WHERE id = $1', [testOrgId])
if (orgRows.length === 0)
throw new Error('Org was not created for password policy test')
const orgCreatedBy = await executeSQL('SELECT created_by FROM public.orgs WHERE id = $1', [testOrgId])
const orgUsers = await executeSQL('SELECT user_id FROM public.org_users WHERE user_id = $1 AND org_id = $2', [userId, testOrgId])
expect(orgCreatedBy[0]?.created_by).toBe(userId)
expect(orgUsers.length).toBeGreaterThan(0)
try {
const response = await fetch(`${BASE_URL}/private/validate_password_compliance`, {
headers,
method: 'POST',
body: JSON.stringify({
email,
password,
org_id: testOrgId,
}),
})
const responseData = await response.json() as { status?: string, error?: string, message?: string }
expect(response.status).toBe(200)
expect(responseData.status).toBe('ok')
const { data: meetsPolicy, error: meetsError } = await getSupabaseClient().rpc('user_meets_password_policy', {
user_id: userId,
org_id: testOrgId,
})
expect(meetsError).toBeNull()
expect(meetsPolicy).toBe(true)
}
finally {
await getSupabaseClient().from('user_password_compliance').delete().eq('org_id', testOrgId)
await getSupabaseClient().from('org_users').delete().eq('org_id', testOrgId)
await getSupabaseClient().from('orgs').delete().eq('id', testOrgId)
await getSupabaseClient().from('stripe_info').delete().eq('customer_id', testCustomerId)
await cleanupAuthUser(userId)
}
})
it('rejects request when user is not a member of the org', async () => {
const nonMemberOrgId = randomUUID()
const nonMemberCustomerId = `cus_pwd_non_member_${nonMemberOrgId}`
const { email, password, userId } = await createAuthUser()
const { error: stripeError } = await getSupabaseClient().from('stripe_info').insert({
customer_id: nonMemberCustomerId,
status: 'succeeded',
product_id: 'prod_LQIregjtNduh4q',
subscription_id: `sub_non_member_${nonMemberOrgId}`,
trial_at: new Date(Date.now() + 15 * 24 * 60 * 60 * 1000).toISOString(),
is_good_plan: true,
})
if (stripeError)
throw stripeError
const { error: orgError } = await getSupabaseClient().from('orgs').insert({
id: nonMemberOrgId,
name: `Non-member org ${nonMemberOrgId}`,
management_email: TEST_EMAIL,
created_by: USER_ID_2,
customer_id: nonMemberCustomerId,
password_policy_config: {
enabled: true,
min_length: 6,
require_uppercase: false,
require_number: false,
require_special: false,
},
})
if (orgError)
throw orgError
try {
const response = await fetch(`${BASE_URL}/private/validate_password_compliance`, {
headers,
method: 'POST',
body: JSON.stringify({
email,
password,
org_id: nonMemberOrgId,
}),
})
expect(response.status).toBe(403)
const responseData = await response.json() as { error?: string }
expect(responseData.error).toBe('not_member')
}
finally {
await getSupabaseClient().from('user_password_compliance').delete().eq('org_id', nonMemberOrgId)
await getSupabaseClient().from('org_users').delete().eq('org_id', nonMemberOrgId)
await getSupabaseClient().from('orgs').delete().eq('id', nonMemberOrgId)
await getSupabaseClient().from('stripe_info').delete().eq('customer_id', nonMemberCustomerId)
await cleanupAuthUser(userId)
}
})
it.concurrent('accepts valid credentials and marks compliance for org members', async () => {
const testOrgId = randomUUID()
const testCustomerId = `cus_pwd_success_${testOrgId}`
const policyConfig = {
enabled: true,
min_length: 6,
require_uppercase: false,
require_number: false,
require_special: false,
}
const { error: stripeError } = await getSupabaseClient().from('stripe_info').insert({
customer_id: testCustomerId,
status: 'succeeded',
product_id: 'prod_LQIregjtNduh4q',
subscription_id: `sub_pwd_success_${testOrgId}`,
trial_at: new Date(Date.now() + 15 * 24 * 60 * 60 * 1000).toISOString(),
is_good_plan: true,
})
if (stripeError)
throw stripeError
const { email, userId, password } = await createAuthUser()
const { error: orgError } = await getSupabaseClient().from('orgs').insert({
id: testOrgId,
name: `Password policy success org ${testOrgId}`,
management_email: TEST_EMAIL,
created_by: userId,
customer_id: testCustomerId,
password_policy_config: policyConfig,
})
if (orgError)
throw orgError
const orgRows = await executeSQL('SELECT id FROM public.orgs WHERE id = $1', [testOrgId])
if (orgRows.length === 0)
throw new Error('Org was not created for password policy test')
const orgCreatedBy = await executeSQL('SELECT created_by FROM public.orgs WHERE id = $1', [testOrgId])
const orgUsers = await executeSQL('SELECT user_id FROM public.org_users WHERE user_id = $1 AND org_id = $2', [userId, testOrgId])
expect(orgCreatedBy[0]?.created_by).toBe(userId)
expect(orgUsers.length).toBeGreaterThan(0)
try {
const response = await fetch(`${BASE_URL}/private/validate_password_compliance`, {
headers,
method: 'POST',
body: JSON.stringify({
email,
password,
org_id: testOrgId,
}),
})
const responseData = await response.json() as { status?: string, error?: string, message?: string }
expect(response.status).toBe(200)
expect(responseData.status).toBe('ok')
const { data: meetsPolicy, error: meetsError } = await getSupabaseClient().rpc('user_meets_password_policy', {
user_id: userId,
org_id: testOrgId,
})
expect(meetsError).toBeNull()
expect(meetsPolicy).toBe(true)
}
finally {
await getSupabaseClient().from('user_password_compliance').delete().eq('org_id', testOrgId)
await getSupabaseClient().from('org_users').delete().eq('org_id', testOrgId)
await getSupabaseClient().from('orgs').delete().eq('id', testOrgId)
await getSupabaseClient().from('stripe_info').delete().eq('customer_id', testCustomerId)
await cleanupAuthUser(userId)
}
})
it.concurrent('rejects request when user is not a member of the org', async () => {
const nonMemberOrgId = randomUUID()
const nonMemberCustomerId = `cus_pwd_non_member_${nonMemberOrgId}`
const { email, password, userId } = await createAuthUser()
const { error: stripeError } = await getSupabaseClient().from('stripe_info').insert({
customer_id: nonMemberCustomerId,
status: 'succeeded',
product_id: 'prod_LQIregjtNduh4q',
subscription_id: `sub_non_member_${nonMemberOrgId}`,
trial_at: new Date(Date.now() + 15 * 24 * 60 * 60 * 1000).toISOString(),
is_good_plan: true,
})
if (stripeError)
throw stripeError
const { error: orgError } = await getSupabaseClient().from('orgs').insert({
id: nonMemberOrgId,
name: `Non-member org ${nonMemberOrgId}`,
management_email: TEST_EMAIL,
created_by: USER_ID_2,
customer_id: nonMemberCustomerId,
password_policy_config: {
enabled: true,
min_length: 6,
require_uppercase: false,
require_number: false,
require_special: false,
},
})
if (orgError)
throw orgError
try {
const response = await fetch(`${BASE_URL}/private/validate_password_compliance`, {
headers,
method: 'POST',
body: JSON.stringify({
email,
password,
org_id: nonMemberOrgId,
}),
})
expect(response.status).toBe(403)
const responseData = await response.json() as { error?: string }
expect(responseData.error).toBe('not_member')
}
finally {
await getSupabaseClient().from('user_password_compliance').delete().eq('org_id', nonMemberOrgId)
await getSupabaseClient().from('org_users').delete().eq('org_id', nonMemberOrgId)
await getSupabaseClient().from('orgs').delete().eq('id', nonMemberOrgId)
await getSupabaseClient().from('stripe_info').delete().eq('customer_id', nonMemberCustomerId)
await cleanupAuthUser(userId)
}
})
🤖 Prompt for AI Agents
In `@tests/password-policy.test.ts` around lines 406 - 535, The tests "accepts
valid credentials and marks compliance for org members" and "rejects request
when user is not a member of the org" should be converted to run in parallel;
replace the two it(...) declarations in tests/password-policy.test.ts (the tests
with descriptions starting "accepts valid credentials..." and "rejects request
when user is not a member...") with it.concurrent(...) so they execute
concurrently while keeping the same bodies and cleanup logic.

@riderx riderx force-pushed the riderx/password-policy-tests branch from 1d54a80 to 5d4b6d9 Compare February 1, 2026 03:50
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Feb 1, 2026

@riderx riderx closed this Feb 2, 2026
@riderx riderx deleted the riderx/password-policy-tests branch March 17, 2026 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants