-
-
Notifications
You must be signed in to change notification settings - Fork 274
fix: Restore soft-deleted users on re-authentication #106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: Restore soft-deleted users on re-authentication #106
Conversation
Users who deleted their account were unable to sign in again with the same GitHub account. The OAuth flow would complete but the user would remain logged out due to the `deletedAt` field being set. This fix adds a `createOrUpdateUser` callback that: 1. Detects soft-deleted users during OAuth 2. Checks audit logs to determine if user was BANNED vs SELF-DELETED 3. If banned → throws error "This account has been suspended" 4. If self-deleted → clears `deletedAt` to restore account Security: Both `deleteAccount` and `banUser` set the same `deletedAt` field. This fix ensures banned users cannot restore their accounts. Performance: The callback runs on every sign-in, but the audit log query ONLY executes for soft-deleted users (rare edge case). Normal active users just hit a single `if` check - no extra queries. When the audit log query does run, it uses the `by_target` index for efficient lookup. Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
@mkrokosz is attempting to deploy a commit to the Amantus Machina Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 file reviewed, 1 comment
| .withIndex('by_target', (q) => | ||
| q.eq('targetType', 'user').eq('targetId', userId) | ||
| ) | ||
| .filter((q) => q.eq(q.field('action'), 'user.ban')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] auditLogs.targetId is a v.string() in the schema, but this query compares it to userId (an Id<'users'>). Convex IDs are often string-like, but relying on implicit coercion makes this brittle (e.g., if the ID type changes or strict typing is enforced on the query builder).
Consider comparing against userId.toString() (or storing targetId as v.id('users') for user targets) so the index lookup is guaranteed to match.
Prompt To Fix With AI
This is a comment left during a code review.
Path: convex/auth.ts
Line: 47:50
Comment:
[P1] `auditLogs.targetId` is a `v.string()` in the schema, but this query compares it to `userId` (an `Id<'users'>`). Convex IDs are often string-like, but relying on implicit coercion makes this brittle (e.g., if the ID type changes or strict typing is enforced on the query builder).
Consider comparing against `userId.toString()` (or storing `targetId` as `v.id('users')` for user targets) so the index lookup is guaranteed to match.
How can I resolve this? If you propose a fix, please make it concise.The auditLogs.targetId field is v.string() in the schema, so explicitly convert the Id<'users'> to string to ensure type-safe comparison. Co-Authored-By: Claude Opus 4.5 <[email protected]>
ec6a17d to
9fc36ba
Compare
Summary
Users who deleted their ClawHub account cannot sign in again with the same GitHub account. This PR fixes that by restoring self-deleted accounts on re-authentication.
The Problem
When a user deletes their account:
deletedAttimestamp is set (soft-delete)When they try to sign in again:
mequery filters deleted users:if (!user || user.deletedAt) return nullThe Fix
Adds a
createOrUpdateUsercallback inconvex/auth.tsthat:deletedAtto restore accountSecurity Consideration
Both
deleteAccountandbanUserset the samedeletedAtfield. This fix ensures banned users cannot restore their accounts by re-authenticating - only self-deleted users can restore.Performance
The callback runs on every OAuth sign-in, but:
nullimmediatelyifcheck on already-loaded fieldauditLogs.by_targetFor 99% of logins (active users), there is zero performance impact. The audit log query only runs for soft-deleted users attempting to sign in, and uses the existing
by_targetindex.Testing
🤖 Generated with Claude Code
Greptile Overview
Greptile Summary
This PR adds a
callbacks.createOrUpdateUserhook inconvex/auth.tsto restore soft-deleted user records when the same GitHub account re-authenticates. The callback:nullwhen there is noexistingUserId).auditLogs.by_targetindex for auser.banrecord and blocks login with a suspension error if present; otherwise it clearsdeletedAtand updatesupdatedAtto restore the account.This aligns the auth flow with the existing
me/requireUserbehavior that filters outdeletedAtusers, while preserving bans by treating ban audit logs as the source of truth.Confidence Score: 4/5
user.banaudit logs), and avoids extra queries for normal logins. The main concern is a minor schema/type mismatch (auditLogs.targetIdisstringwhile code compares against anId<'users'>), which could make the ban check brittle depending on how IDs are serialized in practice.(2/5) Greptile learns from your feedback when you react with thumbs up/down!
Context used:
dashboard- AGENTS.md (source)