-
-
Notifications
You must be signed in to change notification settings - Fork 373
fix: allow soft-deleted users to re-authenticate #177
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?
Conversation
Fixes Issue openclaw#32 where users who soft-deleted their accounts were unable to sign back in because the re-auth logic was only triggering when an existingUserId was passed by the auth provider, which doesn't happen during a standard fresh login flow.
|
@tanujbhaud 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
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: convex/auth.ts
Line: 1:3
Comment:
**Soft-delete bypass**
If this change allows re-authentication without requiring `existingUserId`, it likely makes the "reactivate" path depend on whatever identifier is present in the fresh-login payload. That can accidentally merge/attach a login to the wrong soft-deleted account if the identifier isn’t stable/unique across providers or isn’t validated against the identity provider’s subject. Please ensure reactivation only happens when you can prove the incoming identity matches the soft-deleted user (e.g., provider+subject match) and otherwise create a new user.
How can I resolve this? If you propose a fix, please make it concise. |
|
Resolved: Added a check to ensure matches before reactivation, preventing potential soft-delete bypass. |
|
Resolved: Added a check to ensure 'existingUserId' matches 'userId' before reactivation, preventing potential soft-delete bypass. |
|
Fixed the logic error: now allowing re-authentication when |
Fixes Issue #32 where users who soft-deleted their accounts were unable to sign back in. The re-auth logic was strictly looking for an existingUserId flag which wasn't consistently provided during standard fresh login flows.
Greptile Overview
Greptile Summary
This PR updates the authentication logic to allow soft-deleted users to sign back in by relaxing the requirement for an
existingUserIdflag during re-authentication, and adjusts tests accordingly to assert ondeletedAtdirectly.The change affects the core auth flow in
convex/auth.tsand its corresponding tests inconvex/auth.test.ts, aiming to unblock standard “fresh login” flows that don’t always provideexistingUserIdwhile still supporting account reactivation.Confidence Score: 2/5
existingUserIdcan be correct, but it raises the risk of reactivating or linking the wrong account unless the code strictly matches incoming identities (provider + subject) to the soft-deleted user record. Without verifying that invariant, this can become an account-takeover class bug in certain provider/identifier scenarios.(4/5) You can add custom instructions or style guidelines for the agent here!
Context used:
dashboard- AGENTS.md (source)