feature/sso-01-schema #1377
feature/sso-01-schema #1377
Conversation
Consolidates 12 incremental SSO migrations (20251224022658 through 20260106000000) into a single comprehensive migration. Schema includes: - Tables: org_saml_connections, saml_domain_mappings, sso_audit_logs - Functions: check_org_sso_configured, lookup_sso_provider_*, auto_join_* - Triggers: auto_join_sso_user_trigger, check_sso_domain_on_signup_trigger - RLS policies for all tables - Indexes for performance - Single SSO per org constraint (UNIQUE org_id, entity_id) - auto_join_enabled flag for controlling enrollment This is PR #1 of the SSO feature split (schema foundation only). No backend endpoints, no frontend, no tests included yet. Related: feature/sso-saml-authentication
|
Warning Rate limit exceeded@jokabuyasina has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 23 minutes and 21 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds SSO/SAML database schema, functions, triggers, RLS, grants, and cron integration for audit-log cleanup; plus small frontend TypeScript suppressions and a UsageCard prop/data normalization change. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (signup/update)
participant Auth as auth.users
participant Trigger as DB Trigger
participant Lookup as lookup_sso_provider_by_domain()
participant DomainMap as saml_domain_mappings
participant OrgConn as org_saml_connections
participant AutoEnroll as auto_enroll_sso_user()
participant OrgUsers as org_users
participant Audit as sso_audit_logs
User->>Auth: INSERT/UPDATE user row (email)
Auth->>Trigger: BEFORE/AFTER trigger fires
Trigger->>Lookup: lookup_sso_provider_by_domain(email)
Lookup->>DomainMap: query domain mappings
Lookup->>OrgConn: fetch SSO connection/provider
Lookup-->>Trigger: return provider_id / org info
Trigger->>AutoEnroll: auto_enroll_sso_user(user_id,email,provider_id)
AutoEnroll->>OrgUsers: create membership/auto-join
AutoEnroll->>Audit: insert audit entry
AutoEnroll-->>Trigger: return enrollment result
Trigger->>Audit: log enforcement/validation events
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
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. 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: 5
🤖 Fix all issues with AI agents
In @supabase/migrations/20260107000000_sso_saml_complete.sql:
- Around line 998-1002: Both functions auto_enroll_sso_user and
auto_join_user_to_orgs_by_email are callable by authenticated users but
currently trust the p_user_id and p_email params; add authorization and email
validation at the start of each function by rejecting calls where p_user_id !=
auth.uid() and where p_email does not equal the email fetched from auth.users
for p_user_id, raising an exception on mismatch so callers cannot enroll or join
other users by spoofing ids/emails.
- Around line 489-528: The domain-mapping auto-enroll loop selects orgs from
public.orgs joined to public.saml_domain_mappings without checking the org-level
auto_join_enabled flag; update the FOR v_org IN SELECT to only return orgs with
o.auto_join_enabled = true (or add an equivalent WHERE clause) so the INSERT
into public.org_users and subsequent INSERT into public.sso_audit_logs only run
for orgs that have auto-join enabled; keep the rest of the checks (sdm.verified,
NOT EXISTS org_users) intact and reference the same symbols (public.orgs o,
public.saml_domain_mappings sdm, v_org.id, p_user_id, v_domain) when adding the
condition.
- Around line 956-961: The policy "System can insert audit logs" on table
public.sso_audit_logs is overly permissive because WITH CHECK (true) lets any
authenticated user insert arbitrary audit entries; remove this CREATE POLICY
block entirely (since your SECURITY DEFINER functions already bypass RLS) or
replace it with a restrictive policy that only allows the service role (e.g.,
check current_setting('jwt.claims.role') = 'service' or use the service_role
database user) to INSERT; update or drop the policy declaration for "System can
insert audit logs" accordingly.
🧹 Nitpick comments (4)
supabase/migrations/20260107000000_sso_saml_complete.sql (4)
283-298: Inconsistency betweenorg_has_sso_configuredandcheck_org_sso_configured.These two functions have nearly identical purposes but differ in behavior:
Function SECURITY DEFINERFilters enabled=truecheck_org_sso_configured(Line 235)✅ ✅ org_has_sso_configured(Line 284)❌ ❌ Without
SECURITY DEFINER,org_has_sso_configuredwill be subject to RLS policies, potentially returning inconsistent results. Consider:
- Adding
SECURITY DEFINERandSET search_path = publicfor consistency.- Clarifying the use case difference in comments, or consolidating into one function.
305-349: AddSTABLEvolatility marker for query optimization.
lookup_sso_provider_by_domainonly performs SELECT operations but lacks theSTABLEvolatility marker (compare tolookup_sso_provider_for_emailat Line 357 which has it). AddingSTABLEallows PostgreSQL to optimize repeated calls within the same query.♻️ Proposed fix
CREATE OR REPLACE FUNCTION public.lookup_sso_provider_by_domain( p_email text ) RETURNS TABLE ( provider_id uuid, entity_id text, org_id uuid, org_name text, provider_name text, metadata_url text, enabled boolean ) LANGUAGE plpgsql +STABLE SECURITY DEFINER SET search_path = public AS $$
740-774: Audit log insertion in BEFORE trigger may create orphaned records.The audit log INSERT (Lines 740-755, 757-773) occurs inside a
BEFORE INSERT OR UPDATEtrigger. If the main table INSERT/UPDATE subsequently fails (e.g., due to unique constraint violations onorg_idorentity_id), the audit log record will still be committed in the same transaction and then rolled back.However, there's a subtler issue: if any other trigger or constraint fails after this one, you could have inconsistent audit state during debugging.
Consider moving audit logging to an
AFTERtrigger or using a separate deferred mechanism to ensure audit records only persist for successful operations.
970-973: Verify intent:anonhas SELECT grant but no RLS policy fororg_saml_connections.Line 971 grants
SELECTtoanononorg_saml_connections, but the only SELECT RLS policy (Lines 878-890) requiresauthenticatedrole with org membership.This means:
anonusers will get an empty result set (RLS denies all rows).- The
lookup_sso_provider_by_domainfunction (granted toanon) usesSECURITY DEFINERso it bypasses RLS anyway.The grant to
anonappears unnecessary unless there's an intended use case. Consider removing it for clarity:-GRANT SELECT ON public.org_saml_connections TO authenticated, anon; +GRANT SELECT ON public.org_saml_connections TO authenticated;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supabase/migrations/20260107000000_sso_saml_complete.sql
🧰 Additional context used
📓 Path-based instructions (3)
supabase/migrations/**/*.sql
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database migrations must be created with
supabase migration new <feature_slug>and never modify previously committed migrations
Files:
supabase/migrations/20260107000000_sso_saml_complete.sql
**/{migrations,tests,__tests__}/**/*.{sql,ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
Always cover database changes with Postgres-level tests and complement them with end-to-end tests for affected user flows
Files:
supabase/migrations/20260107000000_sso_saml_complete.sql
supabase/migrations/*.sql
📄 CodeRabbit inference engine (AGENTS.md)
supabase/migrations/*.sql: When creating schema changes, usesupabase migration new <feature_slug>to create a single migration file 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 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 needed
Files:
supabase/migrations/20260107000000_sso_saml_complete.sql
🧠 Learnings (5)
📚 Learning: 2025-12-24T14:11:10.256Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:409-539
Timestamp: 2025-12-24T14:11:10.256Z
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/20260107000000_sso_saml_complete.sql
📚 Learning: 2025-12-27T03:51:23.575Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T03:51:23.575Z
Learning: Applies to supabase/migrations/*.sql : A migration that introduces a new table may include seed inserts for that table, treating seeding as part of the current feature and not modifying previously committed migrations
Applied to files:
supabase/migrations/20260107000000_sso_saml_complete.sql
📚 Learning: 2025-12-27T03:51:23.575Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T03:51:23.575Z
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/20260107000000_sso_saml_complete.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/20260107000000_sso_saml_complete.sql
📚 Learning: 2025-12-25T11:22:13.039Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:85-96
Timestamp: 2025-12-25T11:22:13.039Z
Learning: In SQL migrations under the repository (e.g., supabase/migrations), enforce that when an org has enforcing_2fa=true, all users (including super_admins) must have 2FA enabled before accessing any org functions, including check_org_members_2fa_enabled. Do not grant admin exceptions to 2FA requirements. This ensures consistent security enforcement across all org-related operations; implement this rule within relevant migrations and associated stored procedures/tests.
Applied to files:
supabase/migrations/20260107000000_sso_saml_complete.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 (5)
supabase/migrations/20260107000000_sso_saml_complete.sql (5)
80-101: Verify domain uniqueness constraint is intentional.The
saml_domain_mappingstable has a unique constraint on(domain, sso_connection_id)but not ondomainalone. This allows the same domain (e.g.,example.com) to be mapped to multiple SSO connections across different organizations.If a single domain should only ever belong to one organization, consider adding a unique constraint on
domain:CONSTRAINT saml_domain_mappings_domain_unique UNIQUE(domain)If multi-org domain sharing is intentional (e.g., for testing or subsidiaries), the current design is acceptable but the
prioritycolumn becomes critical for disambiguation.
149-195: Well-designed indexes with appropriate partial filtering.The indexes are properly designed with:
- Partial indexes using
WHEREclauses to reduce index size and improve performance.- Compound indexes for common query patterns (e.g.,
user_id, timestamp DESC).- The
idx_sso_audit_failuresindex for monitoring failed login attempts by IP is a good security practice.
690-699: Potential timing issue: queryingauth.identitiesin BEFORE INSERT trigger.This trigger runs
BEFORE INSERT ON auth.users, but Lines 691-694 queryauth.identities WHERE user_id = NEW.id. At this point, the user row doesn't exist yet, and the identity may not exist either depending on Supabase's internal ordering.This could cause SSO signups to be incorrectly blocked if the identity record is inserted after the user record.
Consider verifying this works correctly with Supabase's auth flow, or alternatively check for SSO indicators in the
NEW.raw_app_meta_dataorNEW.raw_user_meta_datafields instead:-- Alternative: Check metadata for SSO provider info IF NEW.raw_app_meta_data->>'provider' IS NOT NULL AND NEW.raw_app_meta_data->>'provider' != 'email' THEN RETURN NEW; END IF;
786-821: Trigger ordering is correct.The trigger setup properly sequences:
check_sso_domain_on_signup_trigger(BEFORE INSERT) - validates SSO requirements- User INSERT completes
auto_join_user_to_orgs_on_create(AFTER INSERT) - performs auto-enrollmentThe explicit
DROP TRIGGER IF EXISTSstatements ensure idempotent migration runs.
1-14: Missing Postgres-level tests for this migration.Per the coding guidelines, database changes should be covered with Postgres-level tests. The PR objectives note that tests are not included in this PR.
Consider adding tests that verify:
- RLS policies correctly restrict access.
- Auto-enrollment logic respects
auto_join_enabledflag.- Domain lookup functions return correct results.
- SSO enforcement trigger blocks non-SSO signups for configured domains.
- Audit logs are created for expected events.
These tests will help prevent regressions as the SSO feature evolves.
jokabuyasina
changed the title
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @SSO_PR_SPLIT_PLAN.md:
- Around line 22-24: Update the migration filename reference in
SSO_PR_SPLIT_PLAN.md from "supabase/migrations/20260107_sso_saml_complete.sql"
to the actual file name
"supabase/migrations/20260107210800_sso_saml_complete.sql" so the plan matches
the repository; search for the string "20260107_sso_saml_complete.sql" and
replace it with "20260107210800_sso_saml_complete.sql" wherever it appears
(e.g., the referenced line in SSO_PR_SPLIT_PLAN.md).
In @supabase/migrations/20260107210800_sso_saml_complete.sql:
- Around line 998-1002: Add caller-identity validation to both
auto_enroll_sso_user and auto_join_user_to_orgs_by_email: at the start of each
function check that p_user_id equals auth.uid() and raise an exception if not,
and then verify p_email matches the email stored for that user in auth.users
(SELECT email FROM auth.users WHERE id = p_user_id) and raise an exception on
mismatch; place these checks before any enrollment or organization-joining logic
so authenticated users cannot pass spoofed p_user_id or p_email values.
- Around line 956-961: The INSERT policy "System can insert audit logs" on table
sso_audit_logs is too permissive because WITH CHECK (true) allows any
authenticated user to insert audit records; remove this policy entirely (since
the SECURITY DEFINER audit functions already bypass RLS) or change the TO clause
to a restricted role (e.g., service_role) so only the service account can INSERT
into sso_audit_logs.
- Around line 1004-1009: The GRANTs are giving authenticated role EXECUTE on
internal trigger functions; remove or replace grants that grant EXECUTE on
public.trigger_auto_join_on_user_create and
public.trigger_auto_join_on_user_update to authenticated and instead ensure only
postgres and supabase_auth_admin (or no public grant) have EXECUTE; update the
migration to revoke or omit EXECUTE for authenticated and explicitly grant
EXECUTE only to postgres and supabase_auth_admin for these trigger functions so
they can only be invoked by the DB trigger context.
🧹 Nitpick comments (3)
supabase/migrations/20260107210800_sso_saml_complete.sql (2)
283-299: Consider addingSECURITY DEFINERfor consistency.The
org_has_sso_configuredfunction lacksSECURITY DEFINERandSET search_path = publicwhich the other helper functions have. While this function only performs an EXISTS check and doesn't expose sensitive data, adding these for consistency would align with the other SSO functions.♻️ Suggested consistency fix
CREATE OR REPLACE FUNCTION public.org_has_sso_configured(p_org_id uuid) RETURNS boolean LANGUAGE plpgsql STABLE +SECURITY DEFINER +SET search_path = public AS $$
305-349: AddSTABLEvolatility marker for query optimization.The
lookup_sso_provider_by_domainfunction performs only SELECT operations but lacks theSTABLEvolatility marker that the similarlookup_sso_provider_for_emailfunction has. This can prevent PostgreSQL from optimizing repeated calls within the same statement.♻️ Proposed fix
CREATE OR REPLACE FUNCTION public.lookup_sso_provider_by_domain( p_email text ) RETURNS TABLE ( provider_id uuid, entity_id text, org_id uuid, org_name text, provider_name text, metadata_url text, enabled boolean ) LANGUAGE plpgsql +STABLE SECURITY DEFINER SET search_path = publicSSO_PR_SPLIT_PLAN.md (1)
22-24: Add language specifiers to fenced code blocks.Several code blocks are missing language specifiers, which prevents proper syntax highlighting. These blocks contain file paths, so use a generic identifier or
text.📝 Example fix for the first block
-``` +```text supabase/migrations/20260107210800_sso_saml_complete.sql</details> Also applies to: 88-104, 164-174, 236-242, 285-296 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 7618e97b7a1752e527ee872a9dac64f1a5d76d53 and ce2d0ce3bd183b0aea945dec604e55596f3667dc. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `SSO_PR_SPLIT_PLAN.md` * `supabase/migrations/20260107210800_sso_saml_complete.sql` </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (3)</summary> <details> <summary>supabase/migrations/**/*.sql</summary> **📄 CodeRabbit inference engine (.github/copilot-instructions.md)** > Database migrations must be created with `supabase migration new <feature_slug>` and never modify previously committed migrations Files: - `supabase/migrations/20260107210800_sso_saml_complete.sql` </details> <details> <summary>**/{migrations,tests,__tests__}/**/*.{sql,ts,js}</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > Always cover database changes with Postgres-level tests and complement them with end-to-end tests for affected user flows Files: - `supabase/migrations/20260107210800_sso_saml_complete.sql` </details> <details> <summary>supabase/migrations/*.sql</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > `supabase/migrations/*.sql`: When creating schema changes, use `supabase migration new <feature_slug>` to create a single migration file 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 seeding as part of the current feature and not modifying previously committed migrations > Do not create new cron jobs; instead update the `process_all_cron_tasks` function in a new migration file to add your job if needed Files: - `supabase/migrations/20260107210800_sso_saml_complete.sql` </details> </details><details> <summary>🧠 Learnings (5)</summary> <details> <summary>📚 Learning: 2025-12-24T14:11:10.256Z</summary>Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:409-539
Timestamp: 2025-12-24T14:11:10.256Z
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/20260107210800_sso_saml_complete.sql` </details> <details> <summary>📚 Learning: 2025-12-27T03:51:23.575Z</summary>Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T03:51:23.575Z
Learning: Applies to supabase/migrations/*.sql : A migration that introduces a new table may include seed inserts for that table, treating seeding as part of the current feature and not modifying previously committed migrations**Applied to files:** - `supabase/migrations/20260107210800_sso_saml_complete.sql` </details> <details> <summary>📚 Learning: 2025-12-27T03:51:23.575Z</summary>Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T03:51:23.575Z
Learning: Applies to supabase/seed.sql : Updatesupabase/seed.sqlto back new or evolved tests; keep fixtures focused on current behavior while leaving committed migrations unchanged**Applied to files:** - `supabase/migrations/20260107210800_sso_saml_complete.sql` </details> <details> <summary>📚 Learning: 2025-12-23T02:53:12.055Z</summary>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} : Checkc.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints**Applied to files:** - `supabase/migrations/20260107210800_sso_saml_complete.sql` </details> <details> <summary>📚 Learning: 2025-12-25T11:22:13.039Z</summary>Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:85-96
Timestamp: 2025-12-25T11:22:13.039Z
Learning: In SQL migrations under the repository (e.g., supabase/migrations), enforce that when an org has enforcing_2fa=true, all users (including super_admins) must have 2FA enabled before accessing any org functions, including check_org_members_2fa_enabled. Do not grant admin exceptions to 2FA requirements. This ensures consistent security enforcement across all org-related operations; implement this rule within relevant migrations and associated stored procedures/tests.**Applied to files:** - `supabase/migrations/20260107210800_sso_saml_complete.sql` </details> </details><details> <summary>🪛 LanguageTool</summary> <details> <summary>SSO_PR_SPLIT_PLAN.md</summary> [style] ~10-~10: ‘mixed together’ might be wordy. Consider a shorter alternative. Context: ...scripts, and infrastructure changes all mixed together ## Split Strategy (5 PRs, Sequential L... (EN_WORDINESS_PREMIUM_MIXED_TOGETHER) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>SSO_PR_SPLIT_PLAN.md</summary> 22-22: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 88-88: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 164-164: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 236-236: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 285-285: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>⏰ 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)</summary> * GitHub Check: Run tests </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>supabase/migrations/20260107210800_sso_saml_complete.sql (3)</summary><blockquote> `20-59`: **Well-structured SSO configuration table with appropriate constraints.** The `org_saml_connections` table correctly enforces: - Single SSO per organization via `UNIQUE(org_id)` - Unique entity IDs via `UNIQUE(entity_id)` - Metadata availability via CHECK constraint - `auto_join_enabled` flag for controlling automatic enrollment --- `149-196`: **Good use of partial indexes for performance optimization.** The indexes are well-designed: - Partial indexes on `enabled = true` and `verified = true` for common lookup patterns - Composite indexes for audit log queries - `idx_sso_audit_failures` for security monitoring --- `786-822`: **Trigger setup is well-structured.** Good approach: - Dropping existing triggers ensures clean state for idempotent migrations - BEFORE INSERT for SSO enforcement catches invalid signups before row creation - AFTER INSERT/UPDATE for auto-join happens after the user row exists </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
- Add auth.uid() validation to auto_enroll_sso_user and auto_join_user_to_orgs_by_email - Add email verification against auth.users to prevent user spoofing - Add auto_join_enabled check to domain-mapping loop (join org_saml_connections) - Remove overly permissive audit log INSERT policy (SECURITY DEFINER functions bypass RLS)
Join org_saml_connections via sso_connection_id foreign key instead of org_id to check auto_join_enabled on the specific SSO connection referenced by the domain mapping, not just any connection for the org.
…ants - Remove auth.identities check from BEFORE INSERT trigger (identities don't exist yet at that point, created AFTER user insert) - Rely exclusively on metadata-based SSO validation (sso_provider_id in raw_user_meta_data) - Remove EXECUTE grants to authenticated role for trigger functions (trigger_auto_join_on_user_create, trigger_auto_join_on_user_update) - These should only be callable by postgres/supabase_auth_admin in trigger context, not by regular authenticated users - Remove unused v_provider_count variable
- Add cleanup_old_sso_audit_logs() function to delete logs >90 days
- Anonymize email addresses for deleted users (user_id IS NULL)
- Register daily cron task at 3 AM UTC for automated cleanup
- Addresses GDPR/CCPA data retention requirements
- Email anonymization format: deleted-user-{uuid}@anonymized.local
…scan pages - Move getDaysInCurrentMonth call from prop default to computed property to avoid initialization issues - Add @ts-expect-error for packages without TypeScript definitions (qrcode, @capacitor/barcode-scanner) - Remove unused MeteredData import from stripe_event.ts
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @src/components/dashboard/UsageCard.vue:
- Around line 24-27: The computed properties lastDayEvolution and hasData still
reference props.data and will break when the prop is undefined; update both to
read from dataArray.value (or safely guard with dataArray.value ?? []) and
adjust logic accordingly (e.g., use (dataArray.value?.length ?? 0) for counts
and access last elements via dataArray.value[dataArray.value.length - 1]) so
they no longer assume props.data exists.
In @supabase/migrations/20260107210800_sso_saml_complete.sql:
- Around line 475-558: The auth.uid() check in auto_join_user_to_orgs_by_email
currently rejects legitimate service_role or admin-triggered calls; update the
authorization guard to allow the operation when either auth.uid() = p_user_id OR
auth.role() = 'service_role' (or another trusted role used by your system)
instead of only exact uid equality, so triggers and service-role operations can
succeed while still preventing arbitrary users from joining other accounts.
- Around line 739-796: The validate_sso_configuration() trigger function must be
declared SECURITY DEFINER and explicitly set a safe search_path so its INSERTs
into sso_audit_logs succeed despite RLS and missing grants for the invoking
user; update the function declaration to include SECURITY DEFINER and add a SET
search_path = public (or a list like public, pg_temp) at the top of the function
body, and ensure the definer role that owns the function has INSERT rights on
public.sso_audit_logs so the audit INSERTs emitted during org_saml_connections
INSERT/UPDATE will succeed.
In @supabase/migrations/20260108052411_add_sso_audit_cleanup_cron.sql:
- Around line 4-27: Remove the INSERT into public.cron_tasks and instead create
a new migration that updates the process_all_cron_tasks() function to invoke
public.cleanup_old_sso_audit_logs() on the desired schedule; specifically, open
or recreate process_all_cron_tasks() and add a conditional that checks the
current hour/minute (e.g., hour == 3 and minute window < 5) and PERFORM
public.cleanup_old_sso_audit_logs(); ensure you do not add cron_tasks rows and
only modify process_all_cron_tasks() in the new migration.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/BundlePreviewFrame.vuesrc/components/dashboard/UsageCard.vuesrc/pages/scan.vuesupabase/functions/_backend/utils/stripe_event.tssupabase/migrations/20260107210800_sso_saml_complete.sqlsupabase/migrations/20260108052411_add_sso_audit_cleanup_cron.sql
✅ Files skipped from review due to trivial changes (1)
- src/components/BundlePreviewFrame.vue
🧰 Additional context used
📓 Path-based instructions (17)
supabase/migrations/**/*.sql
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database migrations must be created with
supabase migration new <feature_slug>and never modify previously committed migrations
supabase/migrations/**/*.sql: When a feature requires schema changes, create a single migration file with the Supabase CLI (supabase 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, but treat that seeding as part of the current feature and do not modify 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 needed.
Files:
supabase/migrations/20260108052411_add_sso_audit_cleanup_cron.sqlsupabase/migrations/20260107210800_sso_saml_complete.sql
src/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.vue: Use Vue 3 with Composition API and<script setup>syntax for frontend components
Style components using TailwindCSS with DaisyUI components
src/**/*.vue: Use Vue 3<script setup>syntax exclusively for all Vue component scripts
Use Tailwind utility classes for layout and spacing in Vue components
Use DaisyUI components (d-btn,d-input,d-card) for interactive elements in Vue components
Use Konsta components ONLY for safe area helpers (top/bottom insets) in Vue components; avoid other uses
UseuseRoute()fromvue-routerto access route parameters anduseRouter()for programmatic navigation in Vue components
Files:
src/pages/scan.vuesrc/components/dashboard/UsageCard.vue
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use single quotes and no semicolons per @antfu/eslint-config
Files:
src/pages/scan.vuesupabase/functions/_backend/utils/stripe_event.tssrc/components/dashboard/UsageCard.vue
src/pages/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
Use file-based routing with unplugin-vue-router for frontend pages
Frontend file-based routing uses
src/pages/directory structure; routes auto-generate withunplugin-vue-routerand types are available insrc/typed-router.d.ts
Files:
src/pages/scan.vue
{capacitor.config.{ts,js},src/**/*.{ts,tsx,vue}}
📄 CodeRabbit inference engine (CLAUDE.md)
Mobile apps should use Capacitor with app ID
ee.forgr.capacitor_gofor native mobile functionality
Files:
src/pages/scan.vuesrc/components/dashboard/UsageCard.vue
src/**/*.{ts,tsx,vue,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
~/alias for imports fromsrc/directory in frontend TypeScript and Vue components
Files:
src/pages/scan.vuesrc/components/dashboard/UsageCard.vue
src/**/*.{vue,ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Frontend ESLint must pass before commit; run
bun lint:fixto auto-fix issues in frontend files
Files:
src/pages/scan.vuesrc/components/dashboard/UsageCard.vue
**/{src,app}/{components,views,pages}/**/*.{vue,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
The web client is built with Vue.js and Tailwind CSS; lean on utility classes and composition-friendly patterns rather than bespoke CSS.
Files:
src/pages/scan.vuesrc/components/dashboard/UsageCard.vue
**/{src,app}/{components,views,pages}/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Use DaisyUI (
d-prefixed classes) for buttons, inputs, and other interactive primitives to keep behavior and spacing consistent.
Files:
src/pages/scan.vuesrc/components/dashboard/UsageCard.vue
**/{src,app}/**/*.{vue,ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Konsta components are reserved for the safe area helpers. Avoid importing
konstaanywhere else in the app.
Files:
src/pages/scan.vuesrc/components/dashboard/UsageCard.vue
**/{src,app}/**/*.{css,scss,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Capgo's design uses deep slate bases with the 'Extract' azure highlight (
--color-azure-500: #119eff) and soft radii; mirror the palette fromsrc/styles/style.css(e.g.,--color-primary-500: #515271) when introducing new UI.
Files:
src/pages/scan.vuesrc/components/dashboard/UsageCard.vue
supabase/functions/_backend/**
📄 CodeRabbit inference engine (CLAUDE.md)
Backend logic should be organized in
supabase/functions/_backend/with subdirectories for plugins, private endpoints, public endpoints, triggers, and utilities
Files:
supabase/functions/_backend/utils/stripe_event.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript strict mode with path aliases mapping
~/tosrc/
Files:
supabase/functions/_backend/utils/stripe_event.ts
supabase/functions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Supabase Edge Functions use Deno runtime
Files:
supabase/functions/_backend/utils/stripe_event.ts
supabase/functions/_backend/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
supabase/functions/_backend/**/*.{ts,js}: Backend code must be placed insupabase/functions/_backend/as shared code deployed to Cloudflare Workers (API/Plugin/Files workers), Supabase Edge Functions, and other platforms
UsecreateHonofromutils/hono.tsfor all Hono framework application initialization and routing
All database operations must usegetPgClient()orgetDrizzleClient()fromutils/pg.tsfor PostgreSQL access during active migration to Cloudflare D1
All Hono endpoint handlers must acceptContext<MiddlewareKeyVariables>and usec.get('requestId'),c.get('apikey'), andc.get('auth')for request context
Use structured logging withcloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging
UsemiddlewareAPISecretfor internal API endpoints andmiddlewareKeyfor external API keys; validate againstowner_orgin theapikeystable
Checkc.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints
Use Drizzle ORM query patterns withschemafrompostgress_schema.tsfor all database operations; usealiasV2()for self-joins or multiple table references
Files:
supabase/functions/_backend/utils/stripe_event.ts
supabase/functions/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend ESLint must pass before commit; run
bun lint:backendfor backend files
Files:
supabase/functions/_backend/utils/stripe_event.ts
src/components/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
Reusable Vue components should be organized in
src/components/directory
Files:
src/components/dashboard/UsageCard.vue
🧠 Learnings (16)
📚 Learning: 2026-01-08T00:40:00.524Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to supabase/migrations/**/*.sql : Do not create new cron jobs; instead update the `process_all_cron_tasks` function in a new migration file to add your job if needed.
Applied to files:
supabase/migrations/20260108052411_add_sso_audit_cleanup_cron.sql
📚 Learning: 2026-01-08T00:40:00.524Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to supabase/migrations/**/*.sql : A migration that introduces a new table may include seed inserts for that table, but treat that seeding as part of the current feature and do not modify previously committed migrations.
Applied to files:
supabase/migrations/20260108052411_add_sso_audit_cleanup_cron.sqlsupabase/migrations/20260107210800_sso_saml_complete.sql
📚 Learning: 2026-01-08T00:40:00.524Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to supabase/seed.sql : Updating `supabase/seed.sql` to back new or evolved tests is expected; keep fixtures focused on current behavior while leaving committed migrations unchanged.
Applied to files:
supabase/migrations/20260108052411_add_sso_audit_cleanup_cron.sql
📚 Learning: 2026-01-08T00:40:00.524Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to supabase/migrations/**/*.sql : When a feature requires schema changes, create a single migration file with the Supabase CLI (`supabase migration new <feature_slug>`) and keep editing that file until the feature ships; never edit previously committed migrations.
Applied to files:
supabase/migrations/20260108052411_add_sso_audit_cleanup_cron.sqlsupabase/migrations/20260107210800_sso_saml_complete.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/20260108052411_add_sso_audit_cleanup_cron.sql
📚 Learning: 2025-12-25T11:22:13.039Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:85-96
Timestamp: 2025-12-25T11:22:13.039Z
Learning: In SQL migrations under the repository (e.g., supabase/migrations), enforce that when an org has enforcing_2fa=true, all users (including super_admins) must have 2FA enabled before accessing any org functions, including check_org_members_2fa_enabled. Do not grant admin exceptions to 2FA requirements. This ensures consistent security enforcement across all org-related operations; implement this rule within relevant migrations and associated stored procedures/tests.
Applied to files:
supabase/migrations/20260108052411_add_sso_audit_cleanup_cron.sqlsupabase/migrations/20260107210800_sso_saml_complete.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 **/*.{ts,tsx} : Use TypeScript strict mode with path aliases mapping `~/` to `src/`
Applied to files:
src/pages/scan.vue
📚 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 src/**/*.vue : Use Vue 3 with Composition API and `<script setup>` syntax for frontend components
Applied to files:
src/pages/scan.vue
📚 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 src/**/*.vue : Use Vue 3 `<script setup>` syntax exclusively for all Vue component scripts
Applied to files:
src/pages/scan.vue
📚 Learning: 2025-12-23T01:19:04.593Z
Learnt from: riderx
Repo: Cap-go/capgo PR: 1297
File: src/components/dashboard/DeploymentBanner.vue:77-79
Timestamp: 2025-12-23T01:19:04.593Z
Learning: In the Cap-go codebase, ensure that app permission checks never include the role 'owner'. App-level permissions should be based on the user_min_right enum with values: read, upload, write, admin, super_admin (and NOT owner). This pattern applies across Vue components that perform permission checks; if you find a check referencing 'owner' for app-level access, replace it with the appropriate user_min_right value and keep organization-level owner handling in organization.ts.
Applied to files:
src/pages/scan.vuesrc/components/dashboard/UsageCard.vue
📚 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 `createHono` from `utils/hono.ts` for all Hono framework application initialization and routing
Applied to files:
supabase/functions/_backend/utils/stripe_event.ts
📚 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 Hono endpoint handlers must accept `Context<MiddlewareKeyVariables>` and use `c.get('requestId')`, `c.get('apikey')`, and `c.get('auth')` for request context
Applied to files:
supabase/functions/_backend/utils/stripe_event.ts
📚 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/**/*.ts : Supabase Edge Functions use Deno runtime
Applied to files:
supabase/functions/_backend/utils/stripe_event.ts
📚 Learning: 2025-12-24T14:11:10.256Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:409-539
Timestamp: 2025-12-24T14:11:10.256Z
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/20260107210800_sso_saml_complete.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/20260107210800_sso_saml_complete.sql
📚 Learning: 2026-01-08T00:40:00.524Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to **/{api,server,src/server,backend}/**/*.{ts,tsx,js} : Never use the Supabase admin SDK (with service key) for user-facing APIs. Always use the client SDK with user authentication so RLS policies are enforced. The admin SDK should only be used for internal operations (triggers, CRON jobs, etc.), and when unavoidable, sanitize all user inputs carefully to prevent PostgREST query injection.
Applied to files:
supabase/migrations/20260107210800_sso_saml_complete.sql
🔇 Additional comments (15)
supabase/functions/_backend/utils/stripe_event.ts (1)
2-2: LGTM! Good cleanup of unused import.Removing the unused
MeteredDataimport improves code maintainability.src/pages/scan.vue (1)
3-4: LGTM! Proper TypeScript error suppression.Using
@ts-expect-errorwith a clear explanation is the appropriate way to handle imports from packages without TypeScript definitions. This directive will fail if the underlying issue is resolved, preventing stale suppressions.src/components/dashboard/UsageCard.vue (2)
42-47: LGTM!The
dataArraycomputed properly normalizes the optional data prop, providing a consistent array to downstream logic.
49-64: LGTM!The
totalcomputed correctly usesdataArray.value, ensuring consistent behavior with the new data normalization pattern.supabase/migrations/20260108052411_add_sso_audit_cleanup_cron.sql (1)
29-29: LGTM: Good documentation of retention policy.The comment clearly documents the audit log retention policy (90+ days) and PII anonymization behavior, which is important for compliance.
supabase/migrations/20260107210800_sso_saml_complete.sql (10)
20-142: LGTM: Well-designed schema with proper constraints.The three tables (org_saml_connections, saml_domain_mappings, sso_audit_logs) are well-structured with:
- Appropriate foreign key references with ON DELETE CASCADE
- Proper constraints (UNIQUE on org_id and entity_id to enforce single SSO per org)
- CHECK constraint ensuring metadata_url or metadata_xml is provided
- Good audit fields (created_at, updated_at, created_by)
- Comprehensive documentation via COMMENT statements
154-196: LGTM: Comprehensive index strategy.The indexes are well-designed with:
- Partial indexes on filtered columns (enabled, verified) to reduce index size
- Covering common query patterns (user + time, org + time, event + time)
- Specialized index for security monitoring (failed logins by IP)
- Indexes on foreign key columns for join performance
202-298: LGTM: Secure helper functions with proper definer rights.The helper functions are well-implemented with:
- Appropriate use of SECURITY DEFINER for functions that need elevated privileges
- Proper
SET search_path = publicto prevent search path attacks- Good error handling (checking for NULL/empty domains)
- Fallback logic in get_sso_provider_id_for_user to check both metadata fields
305-385: LGTM: Secure SSO provider lookup implementation.The lookup functions properly:
- Use SECURITY DEFINER with search_path protection
- Validate domain extraction from email
- Check both verified status and enabled status
- Order by priority DESC for multiple providers
- Handle NULL/empty domain cases
565-645: Conditional approval: Trigger logic is sound, but depends on fixing called functions.The trigger functions themselves are well-implemented with:
- Proper SECURITY DEFINER and search_path settings
- Appropriate extraction of email from metadata/email field
- Good optimization (update trigger only processes on relevant field changes)
- Proper SSO provider ID lookup and fallback logic
However, these functions call
auto_join_user_to_orgs_by_email, which has the critical auth.uid() check issue flagged in previous comments. The triggers will fail until that issue is resolved.
654-730: LGTM: Proper SSO enforcement with metadata-based bypass.The enforcement logic correctly:
- Blocks password-based signup for SSO-required domains
- Allows SSO authentication by checking for sso_provider_id in user metadata
- Validates the provider ID matches the domain's verified SSO configuration
- Includes proper error handling for invalid UUID formats
- Documents why auth.identities cannot be checked in BEFORE INSERT context
805-840: LGTM: Proper trigger creation with idempotent drops.The trigger creation is well-handled with:
- DROP IF EXISTS for idempotency
- Appropriate trigger timing (BEFORE for validation/enforcement, AFTER for enrollment)
- Clear naming conventions
- Good documentation via COMMENT statements
846-973: LGTM: Comprehensive and secure RLS policies.The RLS policies are well-designed with:
- Proper use of check_min_rights for role-based access control
- Appropriate read access for org members (without write access)
- Necessary anon access to verified domain mappings for SSO detection
- User access to own audit logs
- Admin access to org audit logs
- Good security principle: no direct INSERT policy (relies on SECURITY DEFINER functions)
Note: The comment at lines 974-975 assumes all audit log inserts are from SECURITY DEFINER functions, but
validate_sso_configurationis missing that modifier (flagged separately).
982-1033: LGTM: Appropriate grants with proper separation of concerns.The grants are well-organized:
- Public lookup/check functions available to anon for SSO detection
- Auto-enrollment functions restricted to authenticated users
- Trigger functions properly restricted to postgres and supabase_auth_admin
- Good documentation about trigger function restrictions
Note: The auto-enrollment functions (lines 1013-1016) are granted to authenticated, which necessitates the auth.uid() checks in those functions. However, this creates a conflict when those functions are also called from triggers (flagged separately).
1040-1071: LGTM: Good PII compliance implementation.The cleanup function properly:
- Uses SECURITY DEFINER for elevated privileges
- Implements 90-day retention policy
- Anonymizes PII for deleted users
- Uses RAISE NOTICE for logging/monitoring
- Properly granted to service_role for cron execution
The anonymization pattern
'deleted-user-' || id::text || '@anonymized.local'ensures uniqueness while removing PII.💡 Optional enhancement
Consider also anonymizing emails for very old logs (even if user still exists) to minimize PII retention:
-- Additionally anonymize emails in logs older than 180 days (even if user exists) UPDATE public.sso_audit_logs SET email = 'aged-log-' || id::text || '@anonymized.local' WHERE timestamp < NOW() - INTERVAL '180 days' AND email IS NOT NULL AND email NOT LIKE '%@anonymized.local';
supabase/migrations/20260108052411_add_sso_audit_cleanup_cron.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/components/dashboard/UsageCard.vue:
- Line 112: The return is using a `.value` accessor on a local const `dataArray`
(not a ref), causing a runtime error; change the call to pass the array directly
by removing `.value` so `sumValues` receives `dataArray` (i.e., replace the
current `return sumValues(dataArray.value)` with a direct call to
`sumValues(dataArray)`).
- Around line 58-59: There is a duplicate, incomplete computed named total (uses
dataArray.value.some(...)) that causes a syntax/ESLint parse error; remove the
faulty duplicate definition so only the complete computed total (the one
implemented around the later block handling lines 106-122) remains, ensuring any
references to dataArray stay with that single total implementation and no
leftover partial declaration is left in the file.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/BundlePreviewFrame.vuesrc/components/dashboard/UsageCard.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/BundlePreviewFrame.vue
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.vue: Use Vue 3 with Composition API and<script setup>syntax for frontend components
Style components using TailwindCSS with DaisyUI components
src/**/*.vue: Use Vue 3<script setup>syntax exclusively for all Vue component scripts
Use Tailwind utility classes for layout and spacing in Vue components
Use DaisyUI components (d-btn,d-input,d-card) for interactive elements in Vue components
Use Konsta components ONLY for safe area helpers (top/bottom insets) in Vue components; avoid other uses
UseuseRoute()fromvue-routerto access route parameters anduseRouter()for programmatic navigation in Vue components
Files:
src/components/dashboard/UsageCard.vue
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use single quotes and no semicolons per @antfu/eslint-config
Files:
src/components/dashboard/UsageCard.vue
src/components/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
Reusable Vue components should be organized in
src/components/directory
Files:
src/components/dashboard/UsageCard.vue
{capacitor.config.{ts,js},src/**/*.{ts,tsx,vue}}
📄 CodeRabbit inference engine (CLAUDE.md)
Mobile apps should use Capacitor with app ID
ee.forgr.capacitor_gofor native mobile functionality
Files:
src/components/dashboard/UsageCard.vue
src/**/*.{ts,tsx,vue,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
~/alias for imports fromsrc/directory in frontend TypeScript and Vue components
Files:
src/components/dashboard/UsageCard.vue
src/**/*.{vue,ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Frontend ESLint must pass before commit; run
bun lint:fixto auto-fix issues in frontend files
Files:
src/components/dashboard/UsageCard.vue
**/{src,app}/{components,views,pages}/**/*.{vue,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
The web client is built with Vue.js and Tailwind CSS; lean on utility classes and composition-friendly patterns rather than bespoke CSS.
Files:
src/components/dashboard/UsageCard.vue
**/{src,app}/{components,views,pages}/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Use DaisyUI (
d-prefixed classes) for buttons, inputs, and other interactive primitives to keep behavior and spacing consistent.
Files:
src/components/dashboard/UsageCard.vue
**/{src,app}/**/*.{vue,ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Konsta components are reserved for the safe area helpers. Avoid importing
konstaanywhere else in the app.
Files:
src/components/dashboard/UsageCard.vue
**/{src,app}/**/*.{css,scss,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Capgo's design uses deep slate bases with the 'Extract' azure highlight (
--color-azure-500: #119eff) and soft radii; mirror the palette fromsrc/styles/style.css(e.g.,--color-primary-500: #515271) when introducing new UI.
Files:
src/components/dashboard/UsageCard.vue
🧠 Learnings (1)
📚 Learning: 2025-12-23T01:19:04.593Z
Learnt from: riderx
Repo: Cap-go/capgo PR: 1297
File: src/components/dashboard/DeploymentBanner.vue:77-79
Timestamp: 2025-12-23T01:19:04.593Z
Learning: In the Cap-go codebase, ensure that app permission checks never include the role 'owner'. App-level permissions should be based on the user_min_right enum with values: read, upload, write, admin, super_admin (and NOT owner). This pattern applies across Vue components that perform permission checks; if you find a check referencing 'owner' for app-level access, replace it with the appropriate user_min_right value and keep organization-level owner handling in organization.ts.
Applied to files:
src/components/dashboard/UsageCard.vue
🪛 GitHub Actions: Run tests
src/components/dashboard/UsageCard.vue
[error] 147-147: ESLint parsing error: '}' expected. Lint command failed with exit code 1.
🔇 Additional comments (2)
src/components/dashboard/UsageCard.vue (2)
35-35: LGTM: Prop default simplification is safe.The change from a function default to
undefinedis appropriate since thedataArraycomputed (lines 51-56) handles the undefined case by returning a normalized array.
51-56: Remove unused computed property.The
dataArraycomputed is defined but only referenced in the incompletetotaldefinition at line 59 (which should be removed). After fixing the duplicatetotalissue, this becomes dead code.🗑️ Remove the unused computed
-const dataArray = computed(() => { - if (!props.data || props.data.length === 0) { - return Array.from({ length: getDaysInCurrentMonth() }).fill(undefined) as number[] - } - return props.data as number[] -}) -Likely an incorrect or invalid review comment.
…_tasks - Remove INSERT into cron_tasks table per Capgo convention - Add SSO audit cleanup logic directly to process_all_cron_tasks() function - Runs daily at 3:00 AM UTC (hour == 3, minute < 5) - PERFORM cleanup_old_sso_audit_logs() with error handling - Follows existing cron pattern for time-based conditional execution
- Add SECURITY DEFINER to validate_sso_configuration() function - Set search_path = public, pg_temp for safe function execution - Grant INSERT on sso_audit_logs to postgres role for trigger function - Ensures audit log INSERTs succeed despite RLS during org_saml_connections INSERT/UPDATE - Follows same pattern as auto_enroll_sso_user and auto_join_user_to_orgs_by_email
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @supabase/migrations/20260107210800_sso_saml_complete.sql:
- Around line 410-419: The authorization check comparing p_user_id to auth.uid()
in auto_enroll_sso_user and auto_join_user_to_orgs_by_email blocks trigger-based
auto-enrollment because auth.uid() is NULL during the trigger on user creation;
update those functions to bypass the auth.uid() check when invoked from the
trigger by detecting trigger context (e.g., check TG_OP = 'INSERT' or accept and
check an internal boolean flag like p_internal_call) so direct user calls still
validate p_user_id against auth.uid() but trigger-invoked calls (from
trigger_auto_join_on_user_create) are allowed to proceed.
In
@supabase/migrations/20260108204828_add_sso_audit_cleanup_to_cron_processor.sql:
- Around line 96-99: The dynamic EXECUTE 'SELECT ' || task.target in the CASE
for task.task_type = 'function' should be guarded by an explicit whitelist:
before calling EXECUTE, check task.target against a set/array of allowed
fully-qualified function names (e.g., 'public.cleanup_old_sso_audit_logs',
'public.cleanup_queue_messages', etc.); if it is in the whitelist, run the
EXECUTE, otherwise RAISE WARNING (or skip) and do not execute the value; update
the CASE branch handling in the cron task processor to perform this validation
against the whitelist for defense-in-depth while keeping RLS intact.
🧹 Nitpick comments (1)
supabase/migrations/20260107210800_sso_saml_complete.sql (1)
284-296: Consider adding SECURITY DEFINER and search_path for consistency.Unlike similar helper functions (e.g.,
check_org_sso_configuredat line 235), this function lacksSECURITY DEFINERandSET search_path. While not a security issue for a simple existence check, adding these for consistency would align with the project's patterns.♻️ Suggested refinement
CREATE OR REPLACE FUNCTION public.org_has_sso_configured(p_org_id uuid) RETURNS boolean LANGUAGE plpgsql STABLE +SECURITY DEFINER +SET search_path = public AS $$
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
supabase/migrations/20260107210800_sso_saml_complete.sqlsupabase/migrations/20260108052411_add_sso_audit_cleanup_cron.sqlsupabase/migrations/20260108204828_add_sso_audit_cleanup_to_cron_processor.sql
✅ Files skipped from review due to trivial changes (1)
- supabase/migrations/20260108052411_add_sso_audit_cleanup_cron.sql
🧰 Additional context used
📓 Path-based instructions (1)
supabase/migrations/**/*.sql
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database migrations must be created with
supabase migration new <feature_slug>and never modify previously committed migrations
supabase/migrations/**/*.sql: When a feature requires schema changes, create a single migration file with the Supabase CLI (supabase 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, but treat that seeding as part of the current feature and do not modify 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 needed.
Files:
supabase/migrations/20260108204828_add_sso_audit_cleanup_to_cron_processor.sqlsupabase/migrations/20260107210800_sso_saml_complete.sql
🧠 Learnings (8)
📚 Learning: 2026-01-08T00:40:00.524Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to supabase/migrations/**/*.sql : Do not create new cron jobs; instead update the `process_all_cron_tasks` function in a new migration file to add your job if needed.
Applied to files:
supabase/migrations/20260108204828_add_sso_audit_cleanup_to_cron_processor.sql
📚 Learning: 2026-01-08T00:40:00.524Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to supabase/migrations/**/*.sql : When a feature requires schema changes, create a single migration file with the Supabase CLI (`supabase migration new <feature_slug>`) and keep editing that file until the feature ships; never edit previously committed migrations.
Applied to files:
supabase/migrations/20260108204828_add_sso_audit_cleanup_to_cron_processor.sql
📚 Learning: 2026-01-08T00:40:00.524Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to supabase/migrations/**/*.sql : A migration that introduces a new table may include seed inserts for that table, but treat that seeding as part of the current feature and do not modify previously committed migrations.
Applied to files:
supabase/migrations/20260108204828_add_sso_audit_cleanup_to_cron_processor.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/20260108204828_add_sso_audit_cleanup_to_cron_processor.sql
📚 Learning: 2025-12-25T11:22:13.039Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:85-96
Timestamp: 2025-12-25T11:22:13.039Z
Learning: In SQL migrations under the repository (e.g., supabase/migrations), enforce that when an org has enforcing_2fa=true, all users (including super_admins) must have 2FA enabled before accessing any org functions, including check_org_members_2fa_enabled. Do not grant admin exceptions to 2FA requirements. This ensures consistent security enforcement across all org-related operations; implement this rule within relevant migrations and associated stored procedures/tests.
Applied to files:
supabase/migrations/20260108204828_add_sso_audit_cleanup_to_cron_processor.sqlsupabase/migrations/20260107210800_sso_saml_complete.sql
📚 Learning: 2025-12-24T14:11:10.256Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:409-539
Timestamp: 2025-12-24T14:11:10.256Z
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/20260107210800_sso_saml_complete.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/20260107210800_sso_saml_complete.sql
📚 Learning: 2026-01-08T00:40:00.524Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to **/{api,server,src/server,backend}/**/*.{ts,tsx,js} : Never use the Supabase admin SDK (with service key) for user-facing APIs. Always use the client SDK with user authentication so RLS policies are enforced. The admin SDK should only be used for internal operations (triggers, CRON jobs, etc.), and when unavoidable, sanitize all user inputs carefully to prevent PostgREST query injection.
Applied to files:
supabase/migrations/20260107210800_sso_saml_complete.sql
🔇 Additional comments (10)
supabase/migrations/20260108204828_add_sso_audit_cleanup_to_cron_processor.sql (2)
19-30: LGTM - Advisory lock pattern is correctly implemented.The non-blocking advisory lock with proper cleanup in both normal and exception paths ensures only one instance runs at a time.
40-50: LGTM - SSO cleanup integration follows project conventions.The cleanup is embedded directly in
process_all_cron_tasks()as per the coding guidelines, with proper error handling that logs warnings but doesn't break other scheduled tasks.supabase/migrations/20260107210800_sso_saml_complete.sql (8)
20-59: LGTM - Well-structured table with proper constraints.The unique constraints on
org_idandentity_idcorrectly enforce single SSO per organization and prevent entity ID conflicts across organizations.
80-101: Verify: Same domain can map to multiple SSO connections.The constraint
UNIQUE(domain, sso_connection_id)allows the same domain to be claimed by multiple organizations. While the lookup functions handle this via priority ordering, this could lead to domain hijacking where Organization B claims a domain already used by Organization A.Consider adding a global unique constraint on
domainif each domain should belong to exactly one organization:-CONSTRAINT saml_domain_mappings_domain_connection_unique UNIQUE(domain, sso_connection_id) +CONSTRAINT saml_domain_mappings_domain_unique UNIQUE(domain)
112-148: LGTM - Audit log table with appropriate retention handling.The
ON DELETE SET NULLforuser_idpreserves audit records when users are deleted, and the companioncleanup_old_sso_audit_logs()function properly anonymizes PII for compliance.
149-196: LGTM - Thoughtful index design.The partial indexes (e.g.,
WHERE enabled = true,WHERE verified = true) optimize common query patterns while minimizing storage. The failure monitoring index supports security analysis use cases.
654-730: LGTM - SSO enforcement with appropriate metadata bypass.The function correctly handles the chicken-and-egg problem with
auth.identitiesby relying on metadata-based validation. The custom error code and hint provide clear guidance to users.
956-978: LGTM - Properly restrictive audit log policies.The removal of the direct INSERT policy is correct since all audit log insertions happen through
SECURITY DEFINERfunctions that bypass RLS, preventing unauthorized log injection.
1023-1037: LGTM - Trigger function grants are correctly restricted.Trigger functions are appropriately granted only to
postgresandsupabase_auth_admin, preventing direct invocation by authenticated users. Based on learnings, this follows the pattern of not exposing internal functions to end users.
1045-1076: LGTM - Proper PII handling for GDPR/CCPA compliance.The 90-day retention with email anonymization for deleted users provides appropriate compliance controls. The anonymization pattern is irreversible while maintaining record uniqueness.
supabase/migrations/20260108204828_add_sso_audit_cleanup_to_cron_processor.sql
Show resolved
Hide resolved
- Split auto_enroll_sso_user into internal and public versions - internal_auto_enroll_sso_user: No auth checks, for triggers - public.auto_enroll_sso_user: Validates auth.uid() and email - Update auto_join_user_to_orgs_by_email to use internal version - Allow service_role to bypass auth checks for trigger operations - Fix UsageCard.vue computed properties to use dataArray.value instead of props.data - Add proper undefined guards to lastDayEvolution and hasData
…mputed - Add org_saml_connections, saml_domain_mappings, sso_audit_logs to postgres_schema.ts - Fix duplicate and incomplete computed properties in UsageCard.vue - Fix dataArray.value references in UsageCard - All TypeScript errors resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/dashboard/UsageCard.vue (1)
106-122: Fix incorrect.valueaccess on local variable.Line 107 defines
dataArrayas a local variable assigned fromeffectiveData.value(already dereferenced), but line 112 attempts to accessdataArray.value. SincedataArrayis not a ref,.valuewould be undefined, causing incorrect total calculations.🐛 Proposed fix
Option 1 (Recommended): Remove
.valuefrom line 112const total = computed(() => { const dataArray = effectiveData.value const hasData = dataArray.some(val => val !== undefined) const sumValues = (values: number[]) => values.reduce((acc, val) => (typeof val === 'number' ? acc + val : acc), 0) if (hasData) { - return sumValues(dataArray.value) + return sumValues(dataArray) } if (effectiveDataByApp.value && Object.keys(effectiveDataByApp.value).length > 0) { return Object.values(effectiveDataByApp.value).reduce((totalSum, appValues: any) => { return totalSum + sumValues(appValues) }, 0) } return 0 })Option 2: Keep effectiveData as a ref and use
.valueconsistentlyconst total = computed(() => { - const dataArray = effectiveData.value + const dataArray = effectiveData - const hasData = dataArray.some(val => val !== undefined) + const hasData = dataArray.value.some(val => val !== undefined) const sumValues = (values: number[]) => values.reduce((acc, val) => (typeof val === 'number' ? acc + val : acc), 0) if (hasData) { return sumValues(dataArray.value) } // ... rest unchanged })Note: Also consider renaming the local variable to avoid shadowing the outer
dataArraycomputed property (e.g.,localDataoreffectiveArray).
🤖 Fix all issues with AI agents
In @src/components/dashboard/UsageCard.vue:
- Around line 51-56: The computed dataArray currently casts a fallback
Array.from(...).fill(undefined) to number[] which is incorrect; change the type
assertion to (number | undefined)[] (or produce a numeric fallback like fill(0)
if you truly need all numbers) and ensure any consumers handle undefined values
accordingly; update the computed return to use Array.from({ length:
getDaysInCurrentMonth() }).fill(undefined) as (number | undefined)[] (or replace
.fill(undefined) with .fill(0) and keep number[]) and adjust any usage sites
that assume strictly number[].
🧹 Nitpick comments (4)
supabase/migrations/20260107210800_sso_saml_complete.sql (4)
112-142: Consider adding a dedicated index for timestamp-based cleanup.The
cleanup_old_sso_audit_logsfunction deletes rows wheretimestamp < NOW() - INTERVAL '90 days'. Current indexes are all compound (e.g.,user_id, timestamp DESC), which won't efficiently support this cleanup query.Add index for cleanup query performance
CREATE INDEX IF NOT EXISTS idx_sso_audit_failures ON public.sso_audit_logs (ip_address, timestamp DESC) WHERE event_type = 'login_failed'; +-- Index for daily cleanup job +CREATE INDEX IF NOT EXISTS idx_sso_audit_timestamp ON public.sso_audit_logs (timestamp);
284-296: AddSET search_pathfor consistency and security hygiene.The
org_has_sso_configuredfunction is missingSET search_path = public. While it's notSECURITY DEFINER, addingSET search_pathis a best practice to prevent search_path manipulation attacks and ensures consistent behavior.Add SET search_path
CREATE OR REPLACE FUNCTION public.org_has_sso_configured(p_org_id uuid) RETURNS boolean LANGUAGE plpgsql STABLE +SET search_path = public AS $$ BEGIN RETURN EXISTS ( SELECT 1 FROM public.org_saml_connections WHERE org_id = p_org_id ); END; $$;
515-519: Fragile authorization bypass for trigger context.The condition
auth.jwt() ->> 'role' != 'service_role'relies on NULL semantics for the trigger bypass to work. In trigger context:
auth.uid()returns NULLauth.jwt()returns NULLNULL ->> 'role'= NULLNULL != 'service_role'= NULL (not TRUE)TRUE AND NULL= NULL, so exception won't raiseWhile this works, it's fragile and unclear. Consider an explicit NULL check for robustness:
Make trigger bypass explicit
-- Authorization: allow if user_id matches OR caller has service_role privileges - IF p_user_id != auth.uid() AND auth.jwt() ->> 'role' != 'service_role' THEN + -- In trigger context, auth.uid() and auth.jwt() are NULL, so we allow those calls + IF auth.uid() IS NOT NULL AND p_user_id != auth.uid() THEN RAISE EXCEPTION 'Unauthorized: cannot join other users to orgs (user_id mismatch)'; END IF;
923-936: Consider restricting SSO config visibility to admins.The "Org members can read SSO status" policy grants SELECT to users with
readrights, which exposes potentially sensitive fields likemetadata_url,metadata_xml, andcurrent_certificateto all org members.Consider either:
- Restricting this policy to
adminlevel, or- Creating a view that exposes only non-sensitive fields (
enabled,verified,provider_name) to regular members
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/dashboard/UsageCard.vuesupabase/migrations/20260107210800_sso_saml_complete.sql
🧰 Additional context used
📓 Path-based instructions (11)
supabase/migrations/**/*.sql
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database migrations must be created with
supabase migration new <feature_slug>and never modify previously committed migrations
supabase/migrations/**/*.sql: When a feature requires schema changes, create a single migration file with the Supabase CLI (supabase 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, but treat that seeding as part of the current feature and do not modify 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 needed.
Files:
supabase/migrations/20260107210800_sso_saml_complete.sql
src/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.vue: Use Vue 3 with Composition API and<script setup>syntax for frontend components
Style components using TailwindCSS with DaisyUI components
src/**/*.vue: Use Vue 3<script setup>syntax exclusively for all Vue component scripts
Use Tailwind utility classes for layout and spacing in Vue components
Use DaisyUI components (d-btn,d-input,d-card) for interactive elements in Vue components
Use Konsta components ONLY for safe area helpers (top/bottom insets) in Vue components; avoid other uses
UseuseRoute()fromvue-routerto access route parameters anduseRouter()for programmatic navigation in Vue components
Files:
src/components/dashboard/UsageCard.vue
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use single quotes and no semicolons per @antfu/eslint-config
Files:
src/components/dashboard/UsageCard.vue
src/components/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
Reusable Vue components should be organized in
src/components/directory
Files:
src/components/dashboard/UsageCard.vue
{capacitor.config.{ts,js},src/**/*.{ts,tsx,vue}}
📄 CodeRabbit inference engine (CLAUDE.md)
Mobile apps should use Capacitor with app ID
ee.forgr.capacitor_gofor native mobile functionality
Files:
src/components/dashboard/UsageCard.vue
src/**/*.{ts,tsx,vue,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
~/alias for imports fromsrc/directory in frontend TypeScript and Vue components
Files:
src/components/dashboard/UsageCard.vue
src/**/*.{vue,ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Frontend ESLint must pass before commit; run
bun lint:fixto auto-fix issues in frontend files
Files:
src/components/dashboard/UsageCard.vue
**/{src,app}/{components,views,pages}/**/*.{vue,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
The web client is built with Vue.js and Tailwind CSS; lean on utility classes and composition-friendly patterns rather than bespoke CSS.
Files:
src/components/dashboard/UsageCard.vue
**/{src,app}/{components,views,pages}/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Use DaisyUI (
d-prefixed classes) for buttons, inputs, and other interactive primitives to keep behavior and spacing consistent.
Files:
src/components/dashboard/UsageCard.vue
**/{src,app}/**/*.{vue,ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Konsta components are reserved for the safe area helpers. Avoid importing
konstaanywhere else in the app.
Files:
src/components/dashboard/UsageCard.vue
**/{src,app}/**/*.{css,scss,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Capgo's design uses deep slate bases with the 'Extract' azure highlight (
--color-azure-500: #119eff) and soft radii; mirror the palette fromsrc/styles/style.css(e.g.,--color-primary-500: #515271) when introducing new UI.
Files:
src/components/dashboard/UsageCard.vue
🧠 Learnings (5)
📚 Learning: 2025-12-24T14:11:10.256Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:409-539
Timestamp: 2025-12-24T14:11:10.256Z
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/20260107210800_sso_saml_complete.sql
📚 Learning: 2025-12-25T11:22:13.039Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:85-96
Timestamp: 2025-12-25T11:22:13.039Z
Learning: In SQL migrations under the repository (e.g., supabase/migrations), enforce that when an org has enforcing_2fa=true, all users (including super_admins) must have 2FA enabled before accessing any org functions, including check_org_members_2fa_enabled. Do not grant admin exceptions to 2FA requirements. This ensures consistent security enforcement across all org-related operations; implement this rule within relevant migrations and associated stored procedures/tests.
Applied to files:
supabase/migrations/20260107210800_sso_saml_complete.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/20260107210800_sso_saml_complete.sql
📚 Learning: 2026-01-08T00:40:00.524Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to **/{api,server,src/server,backend}/**/*.{ts,tsx,js} : Never use the Supabase admin SDK (with service key) for user-facing APIs. Always use the client SDK with user authentication so RLS policies are enforced. The admin SDK should only be used for internal operations (triggers, CRON jobs, etc.), and when unavoidable, sanitize all user inputs carefully to prevent PostgREST query injection.
Applied to files:
supabase/migrations/20260107210800_sso_saml_complete.sql
📚 Learning: 2025-12-23T01:19:04.593Z
Learnt from: riderx
Repo: Cap-go/capgo PR: 1297
File: src/components/dashboard/DeploymentBanner.vue:77-79
Timestamp: 2025-12-23T01:19:04.593Z
Learning: In the Cap-go codebase, ensure that app permission checks never include the role 'owner'. App-level permissions should be based on the user_min_right enum with values: read, upload, write, admin, super_admin (and NOT owner). This pattern applies across Vue components that perform permission checks; if you find a check referencing 'owner' for app-level access, replace it with the appropriate user_min_right value and keep organization-level owner handling in organization.ts.
Applied to files:
src/components/dashboard/UsageCard.vue
🔇 Additional comments (14)
src/components/dashboard/UsageCard.vue (2)
33-36: LGTM!Changing the default from a function returning an array to
undefinedcorrectly enables the demo-mode detection logic and works well with the newdataArraycomputed property.
124-146: LGTM!Both
lastDayEvolutionandhasDatacomputed properties correctly usedataArray.valueto reference the outer computed property, with appropriate guards (nullish coalescing and optional chaining). The logic properly handles undefined values and edge cases.supabase/migrations/20260107210800_sso_saml_complete.sql (12)
20-59: LGTM: Well-designed table with proper constraints.The
org_saml_connectionstable has appropriate constraints:
UNIQUE(org_id)enforces single SSO per orgUNIQUE(entity_id)prevents IdP reuse across orgsCHECKconstraint ensures metadata availability- Proper FK cascades for cleanup
80-101: Verify intent: Same domain can map to multiple SSO providers.The
UNIQUE(domain, sso_connection_id)constraint allows the same email domain to be configured for multiple organizations' SSO connections. While this enables multi-tenant scenarios (e.g.,@company.comused by multiple orgs), it could lead to ambiguous auto-join behavior if not carefully managed.The
priorityfield andORDER BY sdm.priority DESCin lookup functions help, but consider if a globally unique domain constraint (UNIQUE(domain)) would better match your security model.
393-404: Good: Internal function correctly omits public grants.The
internal_auto_enroll_sso_userfunction correctly:
- Has no
auth.uid()check (appropriate for trigger context)- Is not granted to
authenticatedoranonroles- Is only called from other
SECURITY DEFINERfunctionsThis follows the pattern from learnings where inner functions with user_id parameters should not be directly callable by users.
466-496: LGTM: Proper authorization checks in public function.Good security design:
- Validates
p_user_id = auth.uid()to prevent enrolling other users- Cross-checks
p_emailagainstauth.users.emailto prevent spoofing- Delegates to internal function after validation
591-620: LGTM: Trigger function correctly extracts SSO context.The trigger properly:
- Extracts email from metadata or direct field
- Attempts SSO provider lookup from user metadata first
- Falls back to domain-based lookup
- Delegates to auto-join function
625-671: LGTM: Update trigger has proper change detection.Good design:
- Uses
IS NOT DISTINCT FROMfor NULL-safe comparison- Only triggers on relevant metadata changes
- Prevents duplicate enrollments with existence check
680-756: LGTM: SSO enforcement with proper bypass mechanism.Well-designed trigger that:
- Allows SSO-authenticated users via metadata check
- Uses exception handling for safe UUID parsing
- Provides clear error message with custom ERRCODE for frontend detection
- Documents why
auth.identitiescheck isn't possible
765-824: LGTM: Comprehensive validation with audit logging.The trigger properly validates configuration and logs all changes. The metadata check is redundant with the table's CHECK constraint but provides a clearer error message.
832-867: LGTM: Triggers properly ordered and idempotent.
BEFORE INSERTfor SSO enforcement (can reject)AFTER INSERT/UPDATEfor auto-join (side effects)BEFORE INSERT OR UPDATEfor validation- Idempotent with
DROP IF EXISTSpattern
1009-1047: Grants correctly follow security patterns.Good adherence to the learned pattern:
internal_auto_enroll_sso_userhas no grants (internal only)- Public wrapper functions with auth checks granted to
authenticated- Trigger functions restricted to
postgresandsupabase_auth_admin- Lookup functions granted to
anonfor pre-auth SSO detectionBased on learnings, inner functions with user_id parameters should not be granted to anon/authenticated roles.
117-118: PII (email) logged in audit table - addressed by cleanup but verify necessity.Emails are logged in
sso_audit_logsfor audit purposes. While thecleanup_old_sso_audit_logsfunction handles retention (90 days) and anonymization for deleted users, consider ifuser_idalone would suffice for successfulauto_join_successevents, reducing PII storage.Current approach is acceptable given the cleanup function handles GDPR/CCPA requirements.
1071-1102: LGTM: Proper cleanup function for GDPR/CCPA compliance.Good implementation:
- 90-day retention policy with hard delete
- Email anonymization for deleted users preserves audit trail without PII
SECURITY DEFINERappropriate for privileged operationservice_rolegrant for cron execution- Cron integration correctly updates
process_all_cron_tasksinstead of creating a standalone cron job, per coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @supabase/functions/_backend/utils/postgres_schema.ts:
- Around line 140-158: Add the missing composite UNIQUE constraint on (org_id,
entity_id) for the org_saml_connections table by using the pgTable
second-argument config callback: define a unique index/constraint (e.g.,
unique_org_entity) referencing table.org_id and table.entity_id so the Drizzle
schema enforces a single SSO per organization; update the org_saml_connections
pgTable definition to include this composite unique constraint in its table
config.
🧹 Nitpick comments (3)
src/components/dashboard/UsageCard.vue (2)
33-36: Consider specifying the array element type.The
dataprop is typed as a genericArraywithout specifying element types. While this works, explicitly typing it asArray<number | undefined>would improve type safety and align with the actual usage pattern where the array can contain undefined values.♻️ Suggested type improvement
data: { - type: Array, + type: Array as PropType<Array<number | undefined>>, default: undefined, },Note: You'll need to import
PropTypefrom 'vue':import { computed, type PropType } from 'vue'
51-56: Type assertion masks array of undefined values.Line 53 casts an array filled with
undefinedasnumber[], which is technically incorrect. While the downstream code correctly handles undefined values (lines 62, 106, 128, 144), the type system is being misled.♻️ More accurate type assertion
const dataArray = computed(() => { if (!props.data || props.data.length === 0) { - return Array.from({ length: getDaysInCurrentMonth() }).fill(undefined) as number[] + return Array.from({ length: getDaysInCurrentMonth() }).fill(undefined) as (number | undefined)[] } - return props.data as number[] + return props.data as (number | undefined)[] })supabase/functions/_backend/utils/postgres_schema.ts (1)
154-154: Consider usingjsonbtype for JSON fields.The
attribute_mapping(line 154) andmetadata(line 188) fields usetexttype with JSON string defaults. Using Drizzle'sjsonbtype would provide better type safety, query capabilities (GIN indexes), and native JSON operations in PostgreSQL.♻️ Proposed refactor to use jsonb
Import jsonb type at the top:
import { bigint, boolean, integer, jsonb, pgEnum, pgTable, serial, text, timestamp, uuid, varchar } from 'drizzle-orm/pg-core'Then update the fields:
attribute_mapping: text('attribute_mapping').default('{}'), + attribute_mapping: jsonb('attribute_mapping').default({}),metadata: text('metadata').default('{}'), + metadata: jsonb('metadata').default({}),Also applies to: 188-188
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/dashboard/UsageCard.vuesupabase/functions/_backend/utils/postgres_schema.ts
🧰 Additional context used
📓 Path-based instructions (15)
src/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.vue: Use Vue 3 with Composition API and<script setup>syntax for frontend components
Style components using TailwindCSS with DaisyUI components
src/**/*.vue: Use Vue 3<script setup>syntax exclusively for all Vue component scripts
Use Tailwind utility classes for layout and spacing in Vue components
Use DaisyUI components (d-btn,d-input,d-card) for interactive elements in Vue components
Use Konsta components ONLY for safe area helpers (top/bottom insets) in Vue components; avoid other uses
UseuseRoute()fromvue-routerto access route parameters anduseRouter()for programmatic navigation in Vue components
Files:
src/components/dashboard/UsageCard.vue
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use single quotes and no semicolons per @antfu/eslint-config
Files:
src/components/dashboard/UsageCard.vuesupabase/functions/_backend/utils/postgres_schema.ts
src/components/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
Reusable Vue components should be organized in
src/components/directory
Files:
src/components/dashboard/UsageCard.vue
{capacitor.config.{ts,js},src/**/*.{ts,tsx,vue}}
📄 CodeRabbit inference engine (CLAUDE.md)
Mobile apps should use Capacitor with app ID
ee.forgr.capacitor_gofor native mobile functionality
Files:
src/components/dashboard/UsageCard.vue
src/**/*.{ts,tsx,vue,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
~/alias for imports fromsrc/directory in frontend TypeScript and Vue components
Files:
src/components/dashboard/UsageCard.vue
src/**/*.{vue,ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Frontend ESLint must pass before commit; run
bun lint:fixto auto-fix issues in frontend files
Files:
src/components/dashboard/UsageCard.vue
**/{src,app}/{components,views,pages}/**/*.{vue,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
The web client is built with Vue.js and Tailwind CSS; lean on utility classes and composition-friendly patterns rather than bespoke CSS.
Files:
src/components/dashboard/UsageCard.vue
**/{src,app}/{components,views,pages}/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Use DaisyUI (
d-prefixed classes) for buttons, inputs, and other interactive primitives to keep behavior and spacing consistent.
Files:
src/components/dashboard/UsageCard.vue
**/{src,app}/**/*.{vue,ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Konsta components are reserved for the safe area helpers. Avoid importing
konstaanywhere else in the app.
Files:
src/components/dashboard/UsageCard.vue
**/{src,app}/**/*.{css,scss,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Capgo's design uses deep slate bases with the 'Extract' azure highlight (
--color-azure-500: #119eff) and soft radii; mirror the palette fromsrc/styles/style.css(e.g.,--color-primary-500: #515271) when introducing new UI.
Files:
src/components/dashboard/UsageCard.vue
supabase/functions/_backend/**
📄 CodeRabbit inference engine (CLAUDE.md)
Backend logic should be organized in
supabase/functions/_backend/with subdirectories for plugins, private endpoints, public endpoints, triggers, and utilities
Files:
supabase/functions/_backend/utils/postgres_schema.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript strict mode with path aliases mapping
~/tosrc/
Files:
supabase/functions/_backend/utils/postgres_schema.ts
supabase/functions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Supabase Edge Functions use Deno runtime
Files:
supabase/functions/_backend/utils/postgres_schema.ts
supabase/functions/_backend/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
supabase/functions/_backend/**/*.{ts,js}: Backend code must be placed insupabase/functions/_backend/as shared code deployed to Cloudflare Workers (API/Plugin/Files workers), Supabase Edge Functions, and other platforms
UsecreateHonofromutils/hono.tsfor all Hono framework application initialization and routing
All database operations must usegetPgClient()orgetDrizzleClient()fromutils/pg.tsfor PostgreSQL access during active migration to Cloudflare D1
All Hono endpoint handlers must acceptContext<MiddlewareKeyVariables>and usec.get('requestId'),c.get('apikey'), andc.get('auth')for request context
Use structured logging withcloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging
UsemiddlewareAPISecretfor internal API endpoints andmiddlewareKeyfor external API keys; validate againstowner_orgin theapikeystable
Checkc.get('auth')?.authTypeto determine authentication type ('apikey' vs 'jwt') in backend endpoints
Use Drizzle ORM query patterns withschemafrompostgress_schema.tsfor all database operations; usealiasV2()for self-joins or multiple table references
Files:
supabase/functions/_backend/utils/postgres_schema.ts
supabase/functions/**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Backend ESLint must pass before commit; run
bun lint:backendfor backend files
Files:
supabase/functions/_backend/utils/postgres_schema.ts
🧠 Learnings (9)
📓 Common learnings
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
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
📚 Learning: 2025-12-23T01:19:04.593Z
Learnt from: riderx
Repo: Cap-go/capgo PR: 1297
File: src/components/dashboard/DeploymentBanner.vue:77-79
Timestamp: 2025-12-23T01:19:04.593Z
Learning: In the Cap-go codebase, ensure that app permission checks never include the role 'owner'. App-level permissions should be based on the user_min_right enum with values: read, upload, write, admin, super_admin (and NOT owner). This pattern applies across Vue components that perform permission checks; if you find a check referencing 'owner' for app-level access, replace it with the appropriate user_min_right value and keep organization-level owner handling in organization.ts.
Applied to files:
src/components/dashboard/UsageCard.vue
📚 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/functions/_backend/utils/postgres_schema.ts
📚 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/functions/_backend/utils/postgres_schema.ts
📚 Learning: 2026-01-08T00:40:00.524Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to supabase/migrations/**/*.sql : A migration that introduces a new table may include seed inserts for that table, but treat that seeding as part of the current feature and do not modify previously committed migrations.
Applied to files:
supabase/functions/_backend/utils/postgres_schema.ts
📚 Learning: 2026-01-08T00:40:00.524Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T00:40:00.524Z
Learning: Applies to supabase/seed.sql : Updating `supabase/seed.sql` to back new or evolved tests is expected; keep fixtures focused on current behavior while leaving committed migrations unchanged.
Applied to files:
supabase/functions/_backend/utils/postgres_schema.ts
📚 Learning: 2025-12-24T14:11:10.256Z
Learnt from: WcaleNieWolny
Repo: Cap-go/capgo PR: 1300
File: supabase/migrations/20251224103713_2fa_enforcement.sql:409-539
Timestamp: 2025-12-24T14:11:10.256Z
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/functions/_backend/utils/postgres_schema.ts
📚 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/functions/_backend/utils/postgres_schema.ts
📚 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/functions/_backend/utils/postgres_schema.ts
🔇 Additional comments (8)
src/components/dashboard/UsageCard.vue (6)
59-68: LGTM! Good defensive coding.The refactoring to use
dataArray.value ?? []is correct and adds proper null safety. The logic for detecting real data (checking both the main array and per-app data) is sound.
87-92: Consistent refactoring with good null safety.The update to use
dataArray.value?.length ?? 0maintains consistency with the new normalization approach and handles edge cases properly.
100-100: Clean refactoring to use normalized data.The change to use
dataArray.valueis correct and maintains the demo/real data switching logic while working with the normalized data structure.
104-120: Correct logic with proper undefined handling.The total computation properly leverages
effectiveData.value(which usesdataArray) and includes appropriate checks for undefined values. The dual path for direct data vs per-app data is handled correctly.
122-142: Robust evolution calculation with proper data handling.The refactoring to use
dataArray.value ?? []is correct. The filtering of undefined values (line 128) and the edge case handling for empty arrays (line 130) and zero division (line 137) demonstrate thorough defensive coding.
144-144: Excellent defensive coding pattern.The use of
dataArray.value?.length ?? 0combined with the check for defined values is a robust pattern that handles all edge cases (null, undefined, empty array, arrays with only undefined values).supabase/functions/_backend/utils/postgres_schema.ts (2)
160-170: The Drizzle schema definition doesn't explicitly show the constraint, but the actual database migration defines a composite unique constraint:UNIQUE(domain, sso_connection_id). This allows the same domain to map to multiple SSO connections by design, supporting the documented purpose of "Maps email domains to SSO providers for auto-join" (plural providers). No action needed.Likely an incorrect or invalid review comment.
140-170: No action needed. Foreign key constraints are properly defined in the migration filesupabase/migrations/20260107210800_sso_saml_complete.sqlwith appropriate cascading operations:
org_saml_connections.org_id→orgs.idON DELETE CASCADE (line 22)org_saml_connections.created_by→auth.users.id(line 51)saml_domain_mappings.org_id→orgs.idON DELETE CASCADE (line 85)saml_domain_mappings.sso_connection_id→org_saml_connections.idON DELETE CASCADE (line 86)The Drizzle schema intentionally excludes FK definitions; constraints are managed in migrations as part of the codebase pattern.
Likely an incorrect or invalid review comment.
- Whitelist allowed functions: cleanup_old_sso_audit_logs, cleanup_queue_messages, cleanup_old_audit_logs, process_stats_queue, send_stats_email - Prevents arbitrary function execution via cron_tasks table - Raises warning for unauthorized functions
- Remove unused useDashboardAppsStore import - Remove unused hasData computed property - Fix blank line before closing script tag
- Update sumValues to properly handle (number | undefined)[] with explicit return type - Add type narrowing for calculateDemoEvolution call - Add hasChartData computed property for template usage - Add back @ts-expect-error directives for packages without TypeScript definitions - Type totalSum explicitly in reduce callback
|
Consolidated SSO SAML schema migration.
Schema includes:
This is PR #1 of the SSO feature split (schema foundation only). No backend endpoints, no frontend, no tests included yet.
Related: feature/sso-saml-authentication
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Chores
Security
✏️ Tip: You can customize this high-level summary in your review settings.