Require reauth before account deletion#1516
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds enforced recent reauthentication for account deletion at the database level and updates frontend flows to require and validate user credentials (email/password or current password) before initiating deletion; includes dialog handling, state cleanup, and enhanced error propagation. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Settings as Settings Page
participant DeletePage as Delete Account Page
participant Auth as Supabase Auth
participant DB as Database
User->>Settings: Click "Delete account"
Settings->>User: Show final confirmation dialog (password input)
User->>Settings: Enter password & confirm
Settings->>DeletePage: Open deletion flow (passes intent)
DeletePage->>User: Prompt for email & password (re-auth)
User->>DeletePage: Submit credentials
DeletePage->>Auth: Sign in with email/password
alt Auth success
Auth-->>DeletePage: Session established
DeletePage->>DB: Invoke public.delete_user()
DB->>Auth: auth.uid() and last_sign_in_at check
alt last_sign_in_at within 5 minutes
DB->>DB: Enqueue on_user_delete via pgmq
DB->>DB: Insert into to_delete_accounts (now +30 days)
DB->>DB: Delete apikeys for user
DB-->>DeletePage: Success / deletion scheduled
DeletePage->>Settings: Trigger navigation/reload
else last_sign_in_at stale or null
DB-->>DeletePage: Raise reauth_required
DeletePage->>User: Show invalid-auth / re-auth error
end
else Auth failure
Auth-->>DeletePage: invalid credentials
DeletePage->>User: Show invalid-auth error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/settings/account/index.vue (1)
850-856: Hardcoded English text should use i18n.The confirmation dialog contains hardcoded English strings that should use the translation function for consistency and localization support.
Proposed fix
<div class="text-base text-gray-500 dark:text-gray-400"> <p class="mb-4"> - This action cannot be undone. Your account and all associated data will be permanently deleted. + {{ t('delete-account-warning-irreversible') }} </p> <p class="font-medium text-gray-700 dark:text-gray-300"> - Your account will be deleted after 30 days + {{ t('delete-account-30-day-notice') }} </p>Add the corresponding translation keys to your i18n files.
🤖 Fix all issues with AI agents
In
`@supabase/migrations/20260127153000_require_recent_reauth_for_delete_user.sql`:
- Around line 26-46: The SELECT that populates old_record_json may yield NULL
when a user exists in auth.users but not in public.users; add an existence check
after the SELECT that tests old_record_json and aborts (e.g., RAISE EXCEPTION or
RETURN) when NULL to avoid sending a pgmq message with a null old_record. Locate
the SELECT that populates old_record_json (uses user_id_fn) and insert an IF
old_record_json IS NULL THEN ... END IF; block so the PERFORM
"pgmq"."send"('on_user_delete', ...) is only executed when old_record_json is
present; reference on_user_delete and pgmq.send in the error/early-return
handling.
🧹 Nitpick comments (2)
src/pages/delete_account.vue (2)
33-49: Minor: Redundant supabase client instantiation.
supabaseClientis created on line 33, butsupabaseis already available from line 14. Consider using the existing instance for consistency.Suggested fix
handler: async () => { - const supabaseClient = useSupabase() isLoading.value = true try {Also note:
supabaseClientis used on line 58 for the users query, butsupabasecould be used there as well.
74-77: Error detection based on message string is fragile.Using
deleteError.message?.includes('reauth_required')could fail if the error format changes or is localized. Consider checking the error code instead.Proposed fix using error code
if (deleteError) { console.error('Delete error:', deleteError) - if (deleteError.message?.includes('reauth_required')) { + if (deleteError.code === 'P0001' || deleteError.message?.includes('reauth_required')) { isLoading.value = false return setErrors('delete-account', [t('invalid-auth')], {}) }
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7ead2dfeb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const { error: signInError } = await supabase.auth.signInWithPassword({ | ||
| email: main.auth.email, | ||
| password, | ||
| }) |
There was a problem hiding this comment.
If Turnstile/captcha is enabled for password sign-in (e.g., VITE_CAPTCHA_KEY is set, as the login flow indicates), this reauth call will fail because it omits captchaToken, and performAccountDeletion will always return false, preventing account deletion in those environments. Consider wiring the same captcha token used on login (or another reauth mechanism) into this signInWithPassword call when captcha enforcement is active.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@supabase/migrations/20260127153000_require_recent_reauth_for_delete_user.sql`:
- Line 57: The jsonb_agg subquery inside the jsonb_build_object can return NULL
when no apikeys rows match; update the expression that selects
"jsonb_agg"("to_jsonb"(a.*)) from "public"."apikeys" a (used in the apikeys
field of jsonb_build_object) to wrap the aggregation with COALESCE and return an
empty JSON array (e.g., COALESCE(..., '[]'::jsonb)) so the apikeys key is always
an array rather than NULL.
- Around line 1-63: Add an explicit GRANT EXECUTE for the delete_user function:
append a statement granting EXECUTE on FUNCTION "public"."delete_user"() to the
API roles (at minimum authenticated and anon) — include service_role as in prior
migrations if present — so add: GRANT EXECUTE ON FUNCTION
"public"."delete_user"() TO authenticated, anon, service_role; to the end of the
migration.
🧹 Nitpick comments (1)
supabase/migrations/20260127153000_require_recent_reauth_for_delete_user.sql (1)
17-19: Consider handling the case where user is not found inauth.users.If
auth.uid()returns a valid UUID but no corresponding row exists inauth.users(edge case, e.g., race condition during deletion), bothuser_emailandlast_sign_in_at_tswill beNULL. The reauth check on line 22 would then raisereauth_required, which is misleading for this scenario.Proposed fix: Add explicit check
SELECT "email", "last_sign_in_at" INTO user_email, last_sign_in_at_ts FROM "auth"."users" WHERE "id" = user_id_fn; + IF user_email IS NULL THEN + RAISE EXCEPTION 'user_not_found' USING ERRCODE = 'P0002'; + END IF; + -- Require a fresh reauthentication (password confirmation)
riderx
force-pushed
the
riderx/account-delete-reauth
branch
from
90c1da8 to
4fdc510
Compare
|
* fix(auth): require reauth for account deletion * fix(db): require recent reauth for delete user * test(db): cover delete_user reauth * fix(account): harden delete reauth flow * fix(db): avoid duplicate confirmation tokens in seed
1 participant
Summary (AI generated)
Test plan (AI generated)
Screenshots (AI generated)
Checklist (AI generated)
.
accordingly.
my tests
Generated with AI
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.