Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts the password-compliance validation endpoint to verify organization membership without triggering RBAC/password-policy logic, addressing lockouts caused by recent RBAC enforcement.
Changes:
- Replace the direct
rbac_check_permissionRPC call with a membership-only check viaget_user_org_idsusing the authenticated Supabase client. - Add explicit error handling and logging for failures in the
get_user_org_idsRPC and return clear 500/403 error responses for membership lookup failure and non-membership.
| // Verify user is a member of this organization without enforcing password policy checks | ||
| const { data: userOrgIds, error: userOrgIdsError } = await supabase | ||
| .rpc('get_user_org_ids') | ||
|
|
||
| if (userOrgIdsError) { | ||
| cloudlog({ requestId: c.get('requestId'), context: 'validate_password_compliance - org membership lookup failed', error: userOrgIdsError.message }) | ||
| return quickError(500, 'org_membership_lookup_failed', 'Failed to verify organization membership', { error: userOrgIdsError.message }) | ||
| } | ||
|
|
||
| if (accessError || !hasOrgAccess) { | ||
| const hasOrgAccess = (userOrgIds ?? []).some(row => row.org_id === body.org_id) | ||
| if (!hasOrgAccess) { |
There was a problem hiding this comment.
The new organization membership check using get_user_org_ids (including the 500 org_membership_lookup_failed path and the 403 not_member path) introduces important authorization behavior that doesn’t appear to be covered by existing password-policy.test.ts tests, which currently focus on body validation, missing policy, invalid credentials, and generic success/failure cases. To avoid regressions here, please add tests that exercise (1) a user who is not a member of org_id receiving the not_member 403 response and (2) the membership RPC failing (e.g., simulated/Postgrest error) returning the org_membership_lookup_failed 500 response, in line with the patterns used for other error-path tests in this file.
![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.
Reviewed commit: 5ce4a49478
ℹ️ 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".
| // Verify user is a member of this organization without enforcing password policy checks | ||
| const { data: userOrgIds, error: userOrgIdsError } = await supabase | ||
| .rpc('get_user_org_ids') | ||
|
|
||
| if (userOrgIdsError) { |
There was a problem hiding this comment.
Switching from rbac_check_permission('org.read') to get_user_org_ids() widens access to anyone with any role binding under the org, including app/channel‑scoped roles that do not grant org.read (see role definitions and get_user_org_ids unioning app/channel bindings in supabase/migrations/20251222140030_rbac_system.sql around lines 2065–2103). That means users who were previously blocked can now fetch org password policy details and upsert compliance for the org, which is a permission regression if org‑level reads were intended to be restricted. Consider preserving the org.read check or filtering out app/channel‑only memberships here.
Useful? React with 👍 / 👎.
|
Warning Rate limit exceeded
⌛ 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. 📝 WalkthroughWalkthroughAdds a TypeScript helper that checks org.read via a new RPC and uses it in the password-compliance handler (returns 500 on lookup error, 403 on denial), introduces three DB PL/pgSQL RBAC functions for password-policy-agnostic checks, and adds tests for the new behavior. Changes
Sequence Diagram(s)sequenceDiagram
%% colors: rgba(0,123,255,0.5)
participant Client as Client
participant Handler as ValidatePasswordCompliance
participant TS as checkOrgReadAccess
participant RPC as Postgres RPC
participant DB as Database
Client->>Handler: POST /private/validate_password_compliance (user, orgId, ...)
Handler->>TS: call checkOrgReadAccess(supabase, orgId, requestId)
TS->>RPC: rpc('rbac_check_permission_no_password_policy', {p_org_id: orgId, ...})
RPC->>DB: execute rbac_check_permission_no_password_policy
DB-->>RPC: boolean result or error
RPC-->>TS: { data: true|false|null, error?: message }
TS-->>Handler: { allowed: boolean, error?: string }
alt error
Handler-->>Client: 500 { error: "org_membership_lookup_failed" }
else not allowed
Handler-->>Client: 403 { error: "not_member" }
else allowed
Handler->>Handler: continue password policy validation & upsert compliance record
Handler-->>Client: 200/validation response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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: 4
🤖 Fix all issues with AI agents
In `@supabase/functions/_backend/private/validate_password_compliance.ts`:
- Line 14: Replace the type alias OrgReadAccessResult with an equivalent
interface to satisfy the ts/consistent-type-definitions lint rule: change "type
OrgReadAccessResult = { allowed: boolean, error?: string }" to "interface
OrgReadAccessResult { allowed: boolean; error?: string }" and ensure any usages
of OrgReadAccessResult remain unchanged.
In `@supabase/migrations/20260201042609_fix_password_policy_org_read_gate.sql`:
- Around line 103-147: The RBAC path uses a direct lookup on public.apikeys
(SELECT rbac_id ... WHERE key = p_apikey) which misses hashed API keys; replace
that query with a call to public.find_apikey_by_value(p_apikey) so
v_apikey_principal is resolved the same way as earlier (use SELECT rbac_id INTO
v_apikey_principal FROM public.find_apikey_by_value(p_apikey) LIMIT 1), keeping
the rest of the RBAC check (public.rbac_has_permission,
public.rbac_principal_apikey(), v_apikey_principal) unchanged.
In `@tests/password-policy.test.ts`:
- Around line 4-5: The imports are out of order for the
perfectionist/sort-imports rule: move the parent-directory import for
checkOrgReadAccess (from
../supabase/functions/_backend/private/validate_password_compliance.ts) above
the local relative import of BASE_URL, getSupabaseClient, headers, TEST_EMAIL,
USER_EMAIL, USER_ID, USER_ID_2 (from ./test-utils.ts) so that ../ imports come
before ./ imports; update the import block in tests/password-policy.test.ts
accordingly.
- Around line 362-370: Add a Postgres-level test file under supabase/tests/ that
exercises the rbac_check_permission_no_password_policy function (which
checkOrgReadAccess calls) following the pattern in
supabase/tests/40_test_password_policy_enforcement.sql: create fixture
users/orgs/members, grant the relevant role/permissions, then assert the
function returns true for a known org member with sufficient permissions and
false for a non-member or a user without permissions; include setup/teardown SQL
and explicit SELECT assertions for both positive and negative cases so database
behavior is validated end-to-end.
🧹 Nitpick comments (1)
tests/password-policy.test.ts (1)
303-370: Switch the new tests toit.concurrent.Update both new tests to run concurrently for CI parallelism.
♻️ Suggested refactor
- it('reject request when user is not a member of the org', async () => { + it.concurrent('reject request when user is not a member of the org', async () => { @@ - it('returns error when membership RPC fails', async () => { + it.concurrent('returns error when membership RPC fails', async () => {As per coding guidelines: Use
it.concurrent()instead ofit()to run tests in parallel within the same file to maximize parallelism in CI/CD.
supabase/functions/_backend/private/validate_password_compliance.ts
Outdated
Show resolved
Hide resolved
| -- Resolve user from API key when needed (handles hashed keys too). | ||
| IF v_effective_user_id IS NULL AND p_apikey IS NOT NULL THEN | ||
| SELECT user_id INTO v_effective_user_id | ||
| FROM public.find_apikey_by_value(p_apikey) | ||
| LIMIT 1; | ||
| END IF; | ||
|
|
||
| -- Enforce 2FA if the org requires it. | ||
| IF v_effective_org_id IS NOT NULL THEN | ||
| SELECT enforcing_2fa INTO v_org_enforcing_2fa | ||
| FROM public.orgs | ||
| WHERE id = v_effective_org_id; | ||
|
|
||
| IF v_org_enforcing_2fa = true AND (v_effective_user_id IS NULL OR NOT public.has_2fa_enabled(v_effective_user_id)) THEN | ||
| PERFORM public.pg_log('deny: RBAC_CHECK_PERM_2FA_ENFORCEMENT', jsonb_build_object( | ||
| 'permission', p_permission_key, | ||
| 'org_id', v_effective_org_id, | ||
| 'app_id', p_app_id, | ||
| 'channel_id', p_channel_id, | ||
| 'user_id', v_effective_user_id, | ||
| 'has_apikey', p_apikey IS NOT NULL | ||
| )); | ||
| RETURN false; | ||
| END IF; | ||
| END IF; | ||
|
|
||
| -- Check if RBAC is enabled for this org | ||
| v_use_rbac := public.rbac_is_enabled_for_org(v_effective_org_id); | ||
|
|
||
| IF v_use_rbac THEN | ||
| -- RBAC path: Check user permission directly | ||
| IF v_effective_user_id IS NOT NULL THEN | ||
| v_allowed := public.rbac_has_permission(public.rbac_principal_user(), v_effective_user_id, p_permission_key, v_effective_org_id, p_app_id, p_channel_id); | ||
| END IF; | ||
|
|
||
| -- If user doesn't have permission, check apikey permission | ||
| IF NOT v_allowed AND p_apikey IS NOT NULL THEN | ||
| SELECT rbac_id INTO v_apikey_principal | ||
| FROM public.apikeys | ||
| WHERE key = p_apikey | ||
| LIMIT 1; | ||
|
|
||
| IF v_apikey_principal IS NOT NULL THEN | ||
| v_allowed := public.rbac_has_permission(public.rbac_principal_apikey(), v_apikey_principal, p_permission_key, v_effective_org_id, p_app_id, p_channel_id); | ||
| END IF; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the migration file structure and lines around 103-147
fd "20260201042609_fix_password_policy_org_read_gate.sql" -t fRepository: Cap-go/capgo
Length of output: 128
🏁 Script executed:
# Check the full migration file to understand context
cat -n supabase/migrations/20260201042609_fix_password_policy_org_read_gate.sql | head -200Repository: Cap-go/capgo
Length of output: 9832
🏁 Script executed:
# Search for find_apikey_by_value implementation
rg -n "find_apikey_by_value" --type sql -B 5 -A 15Repository: Cap-go/capgo
Length of output: 50368
🏁 Script executed:
# Search for rbac_check_permission_direct and how it handles API keys
rg -n "rbac_check_permission_direct" --type sql -B 5 -A 30Repository: Cap-go/capgo
Length of output: 50369
🏁 Script executed:
# Check how apikeys table is defined and if keys are hashed
rg -n "CREATE TABLE.*apikeys|ALTER TABLE.*apikeys" --type sql -B 2 -A 20Repository: Cap-go/capgo
Length of output: 25526
🏁 Script executed:
# Search for all API key related queries to identify patterns
rg -n "FROM public.apikeys|FROM apikeys" --type sql -B 2 -A 5Repository: Cap-go/capgo
Length of output: 33227
Use find_apikey_by_value() for API key principal lookup in RBAC path.
Lines 140–143 use a direct WHERE key = p_apikey lookup that only matches plain-text API keys. This creates a parity gap: lines 104–107 correctly use find_apikey_by_value() to resolve the user from hashed keys, but the subsequent RBAC principal lookup will fail silently for hashed keys. Replace the direct query with find_apikey_by_value():
Suggested fix
-- Instead of:
SELECT rbac_id INTO v_apikey_principal
FROM public.apikeys
WHERE key = p_apikey
LIMIT 1;
-- Use:
SELECT rbac_id INTO v_apikey_principal
FROM public.find_apikey_by_value(p_apikey)
LIMIT 1;🤖 Prompt for AI Agents
In `@supabase/migrations/20260201042609_fix_password_policy_org_read_gate.sql`
around lines 103 - 147, The RBAC path uses a direct lookup on public.apikeys
(SELECT rbac_id ... WHERE key = p_apikey) which misses hashed API keys; replace
that query with a call to public.find_apikey_by_value(p_apikey) so
v_apikey_principal is resolved the same way as earlier (use SELECT rbac_id INTO
v_apikey_principal FROM public.find_apikey_by_value(p_apikey) LIMIT 1), keeping
the rest of the RBAC check (public.rbac_has_permission,
public.rbac_principal_apikey(), v_apikey_principal) unchanged.
| import { BASE_URL, getSupabaseClient, headers, TEST_EMAIL, USER_EMAIL, USER_ID, USER_ID_2 } from './test-utils.ts' | ||
| import { checkOrgReadAccess } from '../supabase/functions/_backend/private/validate_password_compliance.ts' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -20 tests/password-policy.test.tsRepository: Cap-go/capgo
Length of output: 931
🏁 Script executed:
wc -l tests/password-policy.test.tsRepository: Cap-go/capgo
Length of output: 89
🏁 Script executed:
sed -n '1,10p' tests/password-policy.test.ts | cat -nRepository: Cap-go/capgo
Length of output: 612
🏁 Script executed:
# Search for it() and it.concurrent() usage in the file
rg "^\s*(it|describe)\(" tests/password-policy.test.ts -nRepository: Cap-go/capgo
Length of output: 1867
🏁 Script executed:
# Check lines 303-350 and 362-370 mentioned in scratchpad
sed -n '303,350p' tests/password-policy.test.ts | cat -nRepository: Cap-go/capgo
Length of output: 1995
🏁 Script executed:
sed -n '362,370p' tests/password-policy.test.ts | cat -nRepository: Cap-go/capgo
Length of output: 444
🏁 Script executed:
# Check for ESLint configuration files
find . -maxdepth 3 -type f \( -name ".eslintrc*" -o -name "eslint.config.*" \) | head -20Repository: Cap-go/capgo
Length of output: 74
🏁 Script executed:
# Check if perfectionist is configured
rg "perfectionist" . --type json --type js --type yaml -i | head -20Repository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
# Check package.json for eslint and perfectionist dependencies
rg '"eslint|perfectionist"' package.jsonRepository: Cap-go/capgo
Length of output: 281
🏁 Script executed:
# Verify all test patterns in the file to assess it.concurrent usage comprehensively
rg "^\s+it\(" tests/password-policy.test.ts -cRepository: Cap-go/capgo
Length of output: 58
🏁 Script executed:
# Count it.concurrent usage
rg "^\s+it\.concurrent\(" tests/password-policy.test.ts -cRepository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
cat eslint.config.jsRepository: Cap-go/capgo
Length of output: 744
🏁 Script executed:
# Check if perfectionist package exists in node_modules
find node_modules -maxdepth 1 -type d -name "*perfectionist*" 2>/dev/nullRepository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
# Alternative: check dependencies more comprehensively
grep -i perfectionist package.json package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null | head -10Repository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
# Check what `@antfu/eslint-config` includes
cat package.json | grep -A 5 -B 5 "@antfu/eslint-config"Repository: Cap-go/capgo
Length of output: 373
🏁 Script executed:
# Verify import sorting rule is actually enforced by checking antfu documentation context
# Also check if there are any test-specific eslint overrides
rg "test|spec" eslint.config.jsRepository: Cap-go/capgo
Length of output: 85
🏁 Script executed:
# Examine the actual test file structure to understand if it.concurrent is truly required
# Check if any other test files in the project use it.concurrent
find tests -name "*.test.ts" -o -name "*.test.js" | head -5Repository: Cap-go/capgo
Length of output: 194
🏁 Script executed:
# Check one other test file to see if it.concurrent is used elsewhere
rg "it\.concurrent" tests/ --type ts --type js -lRepository: Cap-go/capgo
Length of output: 229
🏁 Script executed:
# Check `@antfu/eslint-config` documentation/details - does it include perfectionist?
# Look at node_modules to see what's actually installed
ls -la node_modules | grep -i "perfectionist\|antfu" | head -10Repository: Cap-go/capgo
Length of output: 114
🏁 Script executed:
# Check if `@antfu/eslint-config` brings in perfectionist automatically
rg "perfectionist" node_modules/@antfu/eslint-config --type json -l 2>/dev/null | head -5Repository: Cap-go/capgo
Length of output: 102
🏁 Script executed:
# Verify the actual ESLint rule that would apply to import sorting in `@antfu` config
# Check if there's any documentation or config about import/sort rules
python3 << 'EOF'
import json
# Try to load package.json of `@antfu/eslint-config` if available
try:
with open('node_modules/@antfu/eslint-config/package.json', 'r') as f:
data = json.load(f)
deps = data.get('dependencies', {})
if 'perfectionist' in str(deps).lower() or 'eslint-plugin-perfectionist' in str(deps).lower():
print("Found perfectionist in dependencies:", deps)
else:
print("Checking if perfectionist is mentioned:")
for key, value in deps.items():
if 'perfect' in key.lower():
print(f" {key}: {value}")
except FileNotFoundError:
print("@antfu/eslint-config package.json not found in node_modules")
except Exception as e:
print(f"Error: {e}")
EOFRepository: Cap-go/capgo
Length of output: 1335
🏁 Script executed:
# Check a few other test files to verify if they consistently use it() or it.concurrent()
for file in tests/admin-credits.test.ts tests/apikeys.test.ts tests/device.test.ts; do
echo "=== $file ==="
rg "^\s+it\(" "$file" -c || echo "0"
doneRepository: Cap-go/capgo
Length of output: 158
🏁 Script executed:
# Check if coding_guidelines are enforced somewhere in the repo (CI, pre-commit, etc.)
rg "it\.concurrent" . --type ts --type js --type md -i -l | grep -E "(README|CONTRIBUTING|\.github|\.husky)" | head -10Repository: Cap-go/capgo
Length of output: 38
Reorder imports to fix perfectionist/sort-imports rule.
The validate_password_compliance import must come before test-utils.ts since parent directory imports (../) should be sorted before relative imports (./).
🔧 Suggested fix
-import { BASE_URL, getSupabaseClient, headers, TEST_EMAIL, USER_EMAIL, USER_ID, USER_ID_2 } from './test-utils.ts'
-import { checkOrgReadAccess } from '../supabase/functions/_backend/private/validate_password_compliance.ts'
+import { checkOrgReadAccess } from '../supabase/functions/_backend/private/validate_password_compliance.ts'
+import { BASE_URL, getSupabaseClient, headers, TEST_EMAIL, USER_EMAIL, USER_ID, USER_ID_2 } from './test-utils.ts'🧰 Tools
🪛 ESLint
[error] 5-5: Expected "../supabase/functions/_backend/private/validate_password_compliance.ts" to come before "./test-utils.ts".
(perfectionist/sort-imports)
🤖 Prompt for AI Agents
In `@tests/password-policy.test.ts` around lines 4 - 5, The imports are out of
order for the perfectionist/sort-imports rule: move the parent-directory import
for checkOrgReadAccess (from
../supabase/functions/_backend/private/validate_password_compliance.ts) above
the local relative import of BASE_URL, getSupabaseClient, headers, TEST_EMAIL,
USER_EMAIL, USER_ID, USER_ID_2 (from ./test-utils.ts) so that ../ imports come
before ./ imports; update the import block in tests/password-policy.test.ts
accordingly.
| describe('checkOrgReadAccess', () => { | ||
| it('returns error when membership RPC fails', async () => { | ||
| const result = await checkOrgReadAccess({ | ||
| rpc: async () => ({ data: null, error: { message: 'boom' } }), | ||
| }, ORG_ID, 'req-test') | ||
|
|
||
| expect(result.allowed).toBe(false) | ||
| expect(result.error).toBe('boom') | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the password-policy test file and understand its structure
find . -name "password-policy.test.ts" -o -name "*password*policy*" -type f | head -20Repository: Cap-go/capgo
Length of output: 437
🏁 Script executed:
# Look for migration files that might introduce RBAC functions
find . -path "*/migrations/*" -type f | grep -i rbac | head -20Repository: Cap-go/capgo
Length of output: 299
🏁 Script executed:
# Search for any migration files related to password policy
find . -path "*/migrations/*" -type f | head -20Repository: Cap-go/capgo
Length of output: 1291
🏁 Script executed:
# Search for RBAC-related functions in the codebase
rg "rbac_check_permission" --type ts --type sql -l | head -20Repository: Cap-go/capgo
Length of output: 654
🏁 Script executed:
# Read the password-policy test file around lines 362-370
cat -n tests/password-policy.test.ts | sed -n '355,375p'Repository: Cap-go/capgo
Length of output: 791
🏁 Script executed:
# Check the migration files to understand what RBAC functions were introduced
cat supabase/migrations/20251228100000_password_policy_enforcement.sql | head -100Repository: Cap-go/capgo
Length of output: 4718
🏁 Script executed:
# Check for existing Postgres-level tests for password policy
ls -la supabase/tests/ | grep -i passwordRepository: Cap-go/capgo
Length of output: 235
🏁 Script executed:
# Check the second password policy migration
cat supabase/migrations/20251229100000_fix_check_org_members_password_policy_service_role.sql | head -150Repository: Cap-go/capgo
Length of output: 2471
🏁 Script executed:
# Check the third password policy migration
cat supabase/migrations/20260201042609_fix_password_policy_org_read_gate.sql | head -150Repository: Cap-go/capgo
Length of output: 6116
🏁 Script executed:
# Read the existing Postgres-level test file for password policy
cat supabase/tests/40_test_password_policy_enforcement.sql | head -200Repository: Cap-go/capgo
Length of output: 7170
🏁 Script executed:
# Continue reading the rest of the password policy test file
tail -n +200 supabase/tests/40_test_password_policy_enforcement.sql | head -200Repository: Cap-go/capgo
Length of output: 6950
🏁 Script executed:
# Search for tests of rbac_check_permission functions
rg "rbac_check_permission" supabase/tests/ --type sqlRepository: Cap-go/capgo
Length of output: 1349
🏁 Script executed:
# Check if rbac_check_permission_no_password_policy function exists
rg "rbac_check_permission_no_password_policy" --type sqlRepository: Cap-go/capgo
Length of output: 785
🏁 Script executed:
# Count total tests in the password policy test file
wc -l supabase/tests/40_test_password_policy_enforcement.sqlRepository: Cap-go/capgo
Length of output: 114
🏁 Script executed:
# Search for tests of the no_password_policy RBAC functions
rg "rbac_check_permission.*no_password_policy" supabase/tests/ --type sql -A 5Repository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
# Check the full password-policy.test.ts file to understand what's being tested
wc -l tests/password-policy.test.tsRepository: Cap-go/capgo
Length of output: 89
🏁 Script executed:
# Look at the checkOrgReadAccess function implementation
rg "checkOrgReadAccess" tests/ --type ts -B 5 -A 15Repository: Cap-go/capgo
Length of output: 2863
🏁 Script executed:
# Search for the function definition being tested
rg "function checkOrgReadAccess|export.*checkOrgReadAccess|const checkOrgReadAccess" --type ts -A 20Repository: Cap-go/capgo
Length of output: 2198
🏁 Script executed:
# Check if there are any tests at all for rbac_check_permission_no_password_policy
rg "rbac_check_permission_no_password_policy" . --type ts --type sqlRepository: Cap-go/capgo
Length of output: 954
🏁 Script executed:
# Look at the test setup and see what tests exist for password policy
grep -n "describe\|it(" tests/password-policy.test.ts | head -40Repository: Cap-go/capgo
Length of output: 1936
🏁 Script executed:
# Read the validate_password_compliance file to understand what functions are being tested
cat supabase/functions/_backend/private/validate_password_compliance.ts | head -80Repository: Cap-go/capgo
Length of output: 2648
🏁 Script executed:
# Check if there are any tests in the SQL test files that test rbac_check_permission_no_password_policy
rg "rbac_check_permission_no_password_policy|rbac_check_permission_direct_no_password_policy" supabase/tests/ --type sqlRepository: Cap-go/capgo
Length of output: 38
🏁 Script executed:
# Look at the password-policy.test.ts file more thoroughly - specifically tests around line 362-370 and beyond
sed -n '362,416p' tests/password-policy.test.tsRepository: Cap-go/capgo
Length of output: 1789
🏁 Script executed:
# Check if the test file has integration tests that actually call the RPC functions
grep -n "rpc\|RPC" tests/password-policy.test.ts | head -20Repository: Cap-go/capgo
Length of output: 1035
Add Postgres-level SQL tests for rbac_check_permission_no_password_policy.
The function is called by checkOrgReadAccess but only has a unit test with a stubbed RPC (lines 365). Add a Postgres-level test (in supabase/tests/) that validates the actual database behavior: verify that a known org member with sufficient permissions returns true and a non-member or user without permissions returns false. Follow the pattern from supabase/tests/40_test_password_policy_enforcement.sql.
As per coding guidelines: Always cover database changes with Postgres-level tests and complement them with end-to-end tests for affected user flows.
🤖 Prompt for AI Agents
In `@tests/password-policy.test.ts` around lines 362 - 370, Add a Postgres-level
test file under supabase/tests/ that exercises the
rbac_check_permission_no_password_policy function (which checkOrgReadAccess
calls) following the pattern in
supabase/tests/40_test_password_policy_enforcement.sql: create fixture
users/orgs/members, grant the relevant role/permissions, then assert the
function returns true for a known org member with sufficient permissions and
false for a non-member or a user without permissions; include setup/teardown SQL
and explicit SELECT assertions for both positive and negative cases so database
behavior is validated end-to-end.
|
Summary (AI generated)
Motivation (AI generated)
RBAC permission checks now enforce password policy, which blocked the verification flow and caused false "not member" errors.
Business Impact (AI generated)
Restores password verification for orgs with enforced password policies without widening org-level access.
Test plan (AI generated)
not_member403.Screenshots (AI generated)
Checklist (AI generated)
bun run lint:backend && bun run lint.accordingly.
my tests
Generated with AI
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.