fix: prevent 500 error when logging in with + in email (Fixes #548)#666
fix: prevent 500 error when logging in with + in email (Fixes #548)#666lb1192176991-lab wants to merge 3 commits into
Conversation
|
@lb1192176991-lab is attempting to deploy a commit to the ritesh Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 50cbeb8a54
ℹ️ 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 { data, error } = await supabase.auth.signInWithPassword({ | ||
| email, | ||
| email: safeEmail, | ||
| password, |
There was a problem hiding this comment.
For addresses containing +, normalizeEmail turns user+tag@example.com into user%2Btag@example.com, and this value is passed directly to the Supabase JS client. These calls send JSON, so Supabase treats the percent sequence as part of the email rather than decoding it; after the backend mirror succeeds, signInWithPassword still looks up the wrong address and the login fails. The same safeEmail is also used for OTP and signup, so only the backend transport should receive any encoded value while Supabase auth should receive the original email string.
Useful? React with 👍 / 👎.
| @router.post("/logout") | ||
| async def auth_logout(request: Request, response: Response): | ||
| # Invalidate the session server-side before clearing cookies | ||
| token = extract_token(request) | ||
| if token: | ||
| try: | ||
| client = _anon_supabase() | ||
| client.auth.sign_out(token) | ||
| except Exception: | ||
| pass # Still clear cookies even if server-side invalidation fails | ||
| async def auth_logout(response: Response): | ||
| _clear_session_cookies(response) |
There was a problem hiding this comment.
When /auth/logout is used for the HttpOnly-cookie session, this now only deletes browser cookies and no longer extracts the current token to call client.auth.sign_out(token). In any context where the token is still available outside the cookie jar (for example the existing bearer fallback in extract_token, mobile/native clients, or a copied refresh/access token), the Supabase session remains valid after logout instead of being invalidated server-side as before.
Useful? React with 👍 / 👎.
… Supabase JS client Two fixes per review feedback: 1. Supabase JS client uses JSON transport, so + should not be %2B-encoded when passed to signInWithPassword/signUp/signInWithOtp/verifyOtp. Backend mirror (mirrorBackendAuth) still uses URL-encoded form and continues to receive safeEmail (%2B). Raw email passed to all Supabase JS SDK calls. 2. Restore server-side session revocation via client.auth.sign_out(token) in /auth/logout that was accidentally removed during refactoring. Token extraction still supports both HttpOnly cookies and Authorization bearer header.
|
Good point. Updated.
|
|
Superseded by a new PR that applies the fix with better separation of concerns. The frontend-only approach in this PR is sound but being consolidated. |
1 participant
What
When a user attempts to log in with an email containing a
+character (e.g.user+test@example.com, a valid RFC 5321 email sub-addressing format), Supabase auth returns a 500 error and the user cannot access the dashboard.Root cause: The
+character is URL-encoded by intermediate layers and decoded as a space (user+test@example.comtouser test@example.com, which is an invalid email that Supabase rejects with a 500.Changes:
Backend (backend/auth_cookie.py)
urllib.parse.unquote()to decode any URL-encoded characters in the email before passing it to Supabase's sign_in_with_password()Frontend (Frontend/src/store/authStore.js)
+as%2Bbefore all Supabase auth calls (login, signup, magic link, OTP verification)+character passes through URL/form-encoding layers without being corruptedWhy
Email sub-addressing with
+is widely used (Gmail, Outlook, Fastmail all support it). Users who use this feature to organize their email get a 500 error and are completely blocked from logging in.Testing
user+test@example.com-> backend decodes percent-encoding -> Supabase receives correct emailnormal@example.com-> no percent-encoding, unquote is a no-op -> unchangeduser+helpdesk@gmail.com-> same normalization applieduser+test@example.com-> frontend normalizeEmail applies%2Bencoding