SSO - #3511
Conversation
move general utils to a new file. move forget business logic to ForgotView. remove onProviderClick from EmailView.
remove props that comes from baseprops and unpack them in Sefaria. move ChooseView buisness logic from AuthPage.
…ay because this div (with z index) has overridden the cursor behaviour.
📊 Code Quality Score: 63/100
Was this score accurate? 👍 Yes · 👎 No Scored by GitVelocity · How are scores calculated? |
There was a problem hiding this comment.
Pull request overview
This PR replaces the legacy Django-rendered login/register pages with a React-driven authentication experience rendered inside ReaderApp, and wires server-rendered base.html + base_props to expose SSO and reCAPTCHA configuration to the client.
Changes:
- Removed
templates/registration/login.htmlandtemplates/registration/register.html, shifting/loginand/registerto renderbase.htmlso React can take over. - Added a new client-side auth flow under
static/js/auth/(choose provider, email login/register, forgot password) and integrated it intostatic/js/ReaderApp.jsxwith history support. - Added new global CSS/assets for the auth UI and a
--header-heightCSS var to position the auth page below the header.
Reviewed changes
Copilot reviewed 24 out of 37 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/registration/register.html | Deleted legacy Django register template (migrated to React auth). |
| templates/registration/login.html | Deleted legacy Django login template (migrated to React auth). |
| templates/base.html | Loads new auth/common-component styles globally. |
| static/js/sefaria/strings.js | Adds Hebrew interface strings for the new auth UI. |
| static/js/sefaria/sefaria.js | Unpacks new base props + adds small SSO redirect helpers. |
| static/js/ReaderApp.jsx | Adds /login and /register routing to render AuthPage and store auth state in history. |
| static/js/common/Input.jsx | New design-system input (includes password reveal + inline error). |
| static/js/common/Captcha.jsx | New wrapper for rendering reCAPTCHA with a design-system error state. |
| static/js/auth/utils.js | Shared auth helpers (csrf, safeNext, error formatting, readiness polling). |
| static/js/auth/ProviderButton.jsx | Custom SSO provider button shell (Google overlay + Apple click). |
| static/js/auth/LegalText.jsx | Terms/Privacy legal copy component for auth screens. |
| static/js/auth/ForgotView.jsx | Forgot-password request form (client-side). |
| static/js/auth/ForgotSentView.jsx | Forgot-password confirmation view (client-side). |
| static/js/auth/ErrorBanner.jsx | Inline error banner with provider-action links for SSO-only accounts. |
| static/js/auth/EmailView.jsx | Email login/register form, reCAPTCHA (register), and registration analytics hooks. |
| static/js/auth/Divider.jsx | Divider (“or”) between SSO and email entry. |
| static/js/auth/ChooseView.jsx | Provider-selection view + Google GIS/Apple JS SDK initialization. |
| static/js/auth/AuthPage.jsx | Top-level auth state machine and GA4 registration funnel tracking. |
| static/js/auth/AuthCard.jsx | Presentational auth card container (heading/sub/back + content). |
| static/icons/google.svg | New Google icon for provider UI. |
| static/icons/eye.svg | New “show password” icon. |
| static/icons/eye-off.svg | New “hide password” icon. |
| static/icons/arrow-left.svg | New back-arrow icon for auth card. |
| static/icons/apple.svg | New Apple icon for provider UI. |
| static/css/header.scss | Introduces $header-height + --header-height CSS variable. |
| static/css/header.css | Compiled CSS updated for --header-height. |
| static/css/header.css.map | Updated sourcemap for compiled header CSS. |
| static/css/common-component.scss | New SCSS for shared auth/common components. |
| static/css/common-component.css | Compiled CSS for shared auth/common components. |
| static/css/common-component.css.map | Sourcemap for shared auth/common component CSS. |
| static/css/auth.scss | New SCSS for auth page/card/provider UI layout. |
| static/css/auth.css | Compiled CSS for auth UI. |
| static/css/auth.css.map | Sourcemap for compiled auth CSS. |
| sefaria/views.py | Renders base.html for login/register and adds JSON mode support for register. |
| reader/views.py | Adds googleClientId, appleClientId, recaptchaSiteKey to base_props. |
Files not reviewed (3)
- static/css/auth.css: Generated file
- static/css/common-component.css: Generated file
- static/css/header.css: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .sefaria-provider-sdk-overlay { | ||
| position: absolute; | ||
| inset: 0; | ||
| z-index: 1; | ||
| opacity: 0.0001; | ||
| overflow: hidden; | ||
| pointer-events: none; | ||
|
|
||
| > div, | ||
| iframe { | ||
| width: 100% !important; // overrides Google SDK inline style | ||
| height: 100% !important; // overrides Google SDK inline style | ||
| margin: 0 !important; // overrides Google SDK inline style | ||
| } | ||
| } |
There was a problem hiding this comment.
Confirmed — this is a genuine feature-killer, not a nit. ProviderButton gives the Google button no onClick; per the comment in ChooseView.jsx, GIS exposes no programmatic trigger, so the invisible iframe overlay receiving the click is the only activation path. pointer-events appears exactly once in auth.scss (this line), and the > div, iframe rule only overrides width/height/margin — nothing re-enables it. Also note the comment directly above claims the overlay "captures clicks correctly", which this rule prevents.
Worth flagging that even with this fixed, Google sign-in still can't work: the GIS SDK script is never loaded anywhere on the branch, and /api/auth/google/callback doesn't exist. Three independent blockers.
| .sefaria-provider-sdk-overlay { | ||
| position: absolute; | ||
| inset: 0; | ||
| z-index: 1; | ||
| opacity: 0.0001; | ||
| overflow: hidden; | ||
| pointer-events: none; | ||
| } |
| <a | ||
| className="sefaria-input-trailingLink" | ||
| href={trailingLink.href} | ||
| onClick={trailingLink.onClick} | ||
| > | ||
| {trailingLink.text} | ||
| </a> |
There was a problem hiding this comment.
Confirmed. EmailView passes passwordTrailingLink: { text, onClick } with no href, so React renders a bare <a onClick> — not in the tab order, so keyboard users cannot reach "Forgot password?" at all.
Cleanest fix given the file already has the pattern: render a <button type="button"> when href is absent, mirroring the reveal-password toggle a few lines down (which is correctly implemented).
| if (path === '/login') { | ||
| this.showAuthPage('login', params.get('next') || '/'); | ||
| return true; | ||
| } else if (path === '/register') { | ||
| this.showAuthPage('register', params.get('next') || '/'); | ||
| return true; | ||
| } |
| const res = await fetch('/api/auth/password/reset', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json', 'X-CSRFToken': csrf }, | ||
| body: JSON.stringify({ email: emailValue }), | ||
| }); | ||
| if (res.ok) { onSuccess(); } | ||
| else { | ||
| const d = await res.json().catch(() => ({})); | ||
| setError(authError(d, 'Something went wrong. Try again.')); |
| const config = { | ||
| client_id: googleClientId, | ||
| ux_mode: useRedirect ? 'redirect' : 'popup', | ||
| }; | ||
| if (useRedirect) { | ||
| config.login_uri = `${window.location.origin}/auth/google/redirect`; | ||
| } else { | ||
| config.callback = (resp) => onSSOResult('/api/auth/google/callback', { credential: resp.credential }); | ||
| } |
There was a problem hiding this comment.
Confirmed on the missing routes. Adding one you didn't catch: the Google Identity Services SDK script is never loaded anywhere on this branch (no gsi/client reference in any template or JS), so window.google never appears, whenReady times out after 8s, and the button stays permanently disabled — the flow dies before it ever reaches a callback URL. Same for Apple (appleid.auth).
| onSSOResult('/api/auth/apple/callback', { | ||
| id_token: a.id_token, first_name: n.firstName || '', last_name: n.lastName || '', email: u.email || '', | ||
| }); | ||
| }; | ||
| const onFail = (ev) => { | ||
| if (ev.detail?.error !== 'popup_closed_by_user') { | ||
| setError(authError(null, 'Something went wrong. Try again.')); | ||
| } | ||
| }; | ||
| document.addEventListener('AppleIDSignInOnSuccess', onOk); | ||
| document.addEventListener('AppleIDSignInOnFailure', onFail); | ||
| const stopWaiting = whenReady( | ||
| () => window.AppleID?.auth, | ||
| () => { | ||
| try { | ||
| window.AppleID.auth.init({ | ||
| clientId: appleClientId, | ||
| scope: 'name email', | ||
| redirectURI: `${window.location.origin}/auth/apple/redirect`, | ||
| state: ssoRedirectState, | ||
| usePopup: !useRedirect, | ||
| }); |
| * SSO uses the existing backend callbacks (/api/auth/{google,apple}/callback). Email | ||
| * login/register use JSON+session endpoints (/api/auth/login, /api/auth/register). |
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
| import {InterfaceText} from "../Misc"; | ||
|
|
||
| /** | ||
| * Divider — a horizontal rule with centered text ("or" / "או"), used between the | ||
| * SSO buttons and the email button on the auth choose screen. Figma `Form Card`. | ||
| */ | ||
| const Divider = () => ( | ||
| <div className="sefaria-divider" role="separator"><InterfaceText>or</InterfaceText></div> | ||
| ); | ||
|
|
||
| Divider.propTypes = { children: PropTypes.node }; | ||
|
|
||
| export default Divider; |
There was a problem hiding this comment.
Non-issue — I'd skip this one. PropTypes is not unused (Divider is trivial, and an extraneous propTypes entry is harmless); this is lint noise, not a defect worth a round-trip on a PR with functional blockers.
yodem
left a comment
yodem
left a comment
There was a problem hiding this comment.
Reviewed the branch and ran it locally (real DB + built SSR bundle) rather than only reading the diff. Flagging as request-changes: as it stands the auth page is non-functional and it takes existing login down with it.
Verified by running the branch
| Path | HTTP | id="s2" mount point present? |
|---|---|---|
/login |
200 | 0 — absent |
/register |
200 | 1 |
/texts |
200 | 1 |
So a fresh load of /login has no React root at all (details inline on sefaria/views.py).
The four blockers
/loginrenders a blank page —CustomLoginViewno longer goes throughrender_template().- SSR is dead site-wide —
windowis read in theReaderAppconstructor, which runs in Node. - SSO can't work for three independent reasons — the Google/Apple SDK
<script>tags are never loaded anywhere,pointer-events: noneblocks the GIS overlay, and/api/auth/*doesn't exist. - Email login POSTs to a route that doesn't exist, while this PR deletes the
login.htmlthat currently handles it. This also breakse2e-tests/global-setup.ts, which logs in through the real/loginform to mint storage state for every authenticated profile and hard-fails the suite if it can't. No tests or page objects were updated, and no tests were added — CI is green only because the auth suite doesn't run there.
Plus a P1 open redirect (inline on utils.js).
Credit where due: RTL is handled properly (logical inset-inline-* properties throughout), the password toggle is a correct type="button" with proper htmlFor/aria-invalid/role="alert" wiring, and committing the compiled .css/.css.map does match existing repo convention. Copilot's Divider.jsx propTypes nit and its doc-comment comment are noise — safe to ignore.
| class CustomLoginView(StaticViewMixin, LoginView): | ||
| class CustomLoginView(LoginView): | ||
| authentication_form = SefariaLoginForm | ||
| template_name = 'base.html' |
There was a problem hiding this comment.
P0 — this makes /login render a blank page.
Dropping StaticViewMixin and pointing template_name at base.html means this view never goes through render_template() / render_react_component(). So neither html nor renderStatic is ever set in the context — and base.html only emits the <div id="s2"> React mount point inside {% if html and not renderStatic %} / {% elif renderStatic %}.
I ran the branch against a real DB to confirm:
GET /login -> 200, id="s2" occurrences: 0 <-- no React root
GET /register -> 200, id="s2" occurrences: 1
GET /texts -> 200, id="s2" occurrences: 1
inReaderApp: true is still emitted, so client.jsx calls ReactDOM.render(component, document.getElementById('s2')) with null and throws. Any fresh load of /login (bookmark, refresh, external link, SSO redirect back) is a blank page.
register() below correctly calls render_template(...), which is why /register works — that isolates the bug to this class. Suggested fix: route CustomLoginView through render_template() with app_props the same way register() does.
There was a problem hiding this comment.
Not sure why it commented this, you use render_template on line 100?
There was a problem hiding this comment.
@yodem i ignore all your (claude) comments for now - some of them are based on the fact that there was no server side, some claim that working things are not working.
| translationLanguagePreference: props.translationLanguagePreference, | ||
| editorSaveState: 'saved', | ||
| notificationCount: props.notificationCount || 0, | ||
| showAuth: ['/login', '/register'].includes(window.location.pathname), |
There was a problem hiding this comment.
P0 — this breaks SSR for the entire site, not just the auth pages.
This reads window.location in the constructor, unguarded. master has zero window access in this constructor — this is the first. But ReaderApp is server-rendered in Node: node/server.js:93 does ReactDOMServer.renderToString(ReaderApp(props)).
I built the SSR bundle and confirmed:
- the compiled line
includes(window.location.pathname)is present in the node-targetedserver-bundle.js; - there is no
window/jsdom shim anywhere innode/or the bundle (global.window =/jsdom→ 0 matches); node -e "window.location.pathname"→ReferenceError: window is not defined.
So renderToString throws on every SSR render. It won't 500 — render_react_component catches it and silently falls back to the loading placeholder — so this degrades quietly into "SSR is just off everywhere", which is easy to miss in review and painful for SEO/TTFB.
Fix: guard with typeof window !== 'undefined', or better, derive showAuth/authFlow/authNext from props.initialPath / props, which are already passed in from Django and safe on both sides.
Separately, Copilot is right that the exact-match is wrong: urls_shared.py uses re_path(fr'^login/?$'), so /login/ and /register/ are genuinely reachable and won't match here.
| Sefaria.ssoSetRedirectState(ssoRedirectState); | ||
| } | ||
| const stopWaiting = whenReady( | ||
| () => window.google?.accounts?.id && googleBtnRef.current, |
There was a problem hiding this comment.
P0 — the Google and Apple SDKs are never loaded, so both provider buttons are permanently disabled.
This polls for window.google?.accounts?.id, and the Apple effect polls for window.AppleID?.auth. But nothing on this branch ever loads those SDKs — searching the whole branch for gsi/client, appleid.auth, accounts.google.com, appleid.cdn-apple returns zero hits in any template or JS. The base.html diff only adds CSS.
So whenReady polls 80×100ms, gives up silently after 8s, googleReady/appleReady stay false, and both ProviderButtons render permanently disabled. The whole SSO surface of the PR is dead on arrival even before the missing /api/auth/* routes.
Needs the GIS + Apple JS <script> tags added to base.html (ideally only on the auth pages, and async/defer).
Minor, same file: onSSOResult omits the X-CSRFToken header that EmailView sends. Worth fixing now so it doesn't 403 the moment the backend lands.
| z-index: 1; | ||
| opacity: 0.0001; | ||
| overflow: hidden; | ||
| pointer-events: none; |
There was a problem hiding this comment.
Confirming Copilot here — this is real, and it's a second independent reason Google sign-in can't work.
pointer-events: none on the wrapper is inherited by the GIS <iframe> inside it — the > div, iframe rule only overrides width/height/margin, and nothing re-enables pointer events (pointer-events appears exactly once in this file: here).
That matters because ProviderButton gives the Google button no onClick — per the comment in ChooseView.jsx, GIS has no programmatic trigger, so the invisible iframe overlay receiving the click is the only activation path. With pointer events off, nothing is clickable.
Note the comment two lines above says the overlay "captures clicks correctly" — which is exactly what this rule prevents. Looks like a late edit that was never re-tested.
| captchaToken.current = ''; | ||
| } | ||
| } else { | ||
| const res = await fetch('/api/auth/login', { |
There was a problem hiding this comment.
P0 — email login POSTs to a route that does not exist, and this PR deletes the page that currently handles login.
There is no /api/auth/login in sefaria/urls_shared.py (searching the branch for api/auth returns nothing). /api/login/ exists but is TokenObtainPairView — it mints JWTs and does not establish a Django session, so it isn't a drop-in.
The PR description says server endpoints are "not in this PR", but that framing only covers the SSO callbacks. This is ordinary email login, and templates/registration/login.html — which currently handles it — is deleted in this same PR. Net effect on merge: nobody can log in.
The blast radius goes past the login page: e2e-tests/global-setup.ts drives this exact form to capture storage state for every authenticated e2e profile, and explicitly hard-fails the whole suite if login doesn't complete. e2e-tests/pages/loginPage.ts also matches on getByPlaceholder('Email Address') and getByRole('button', { name: 'Login' }), whereas the new UI uses placeholder you@example.com, a button labelled "Log In", and hides the form behind a "Continue with Email" click. Those page objects need updating in this PR.
Simplest path that keeps this PR self-contained: POST form-encoded credentials to the existing /login (Django's LoginView already handles it and sets the session), the same way the register branch above reuses /register with noredirect.
| return null; | ||
| } | ||
|
|
||
| export function safeNext(next) { |
There was a problem hiding this comment.
P1 — open redirect. next is attacker-controlled (query string) and flows straight into window.location.href in both EmailView and ChooseView.
This regex only rejects a second literal forward slash, so it blocks //evil.com but passes /\evil.com — and both Chrome and Firefox normalize backslashes to forward slashes when parsing a URL, so that navigates to the protocol-relative //evil.com:
safeNext("//evil.com") // -> "/" blocked
safeNext("/\\evil.com") // -> "/\\evil.com" PASSES -> lands on https://evil.comSo sefaria.org/login?next=/%5Cevil.com drops the user on an attacker's page immediately after a legitimate login — prime phishing setup, since the URL bar showed sefaria.org the whole way through.
The old flow POSTed next back to Django, which validated it server-side. Worth noting url_has_allowed_host_and_scheme is already imported and used in sefaria/views.py (line 112) — the correct pattern is right there. Either mirror it client-side (reject any \, then new URL(next, location.origin).origin === location.origin) or let the server own the post-auth redirect.
| "First Name": "שם פרטי", | ||
| "Last Name": "שם משפחה", | ||
| "Don't have an account?": 'אין לך חשבון?', | ||
| "Already have an account?": 'יש לך חשבון?', |
There was a problem hiding this comment.
This key already exists on master and is used by the pre-existing sign-up modal in Misc.jsx. Since _i18nInterfaceStrings is one flat object literal, this later declaration silently wins — so merging this changes the Hebrew text of that unrelated modal as an unreviewed side effect.
Either drop the duplicate and reuse the existing string, or scope it under the Auth context block (the pattern already used correctly for "Sign Up"/"Log In").
While here: "First Name"/"Last Name" duplicate existing keys, "Reset Password" is unused, and the error strings routed through authError() can never match — Django has already translated those messages server-side by the time they reach Sefaria._(), which keys on English.
| <link rel="stylesheet" href="{% static 'css/themes/library-theme.css' %}"> | ||
| <link rel="stylesheet" href="{% static 'css/themes/sheets-theme.css' %}"> | ||
| <link rel="stylesheet" href="{% static 'css/common-components.css' %}"> | ||
| <link rel="stylesheet" href="{% static 'css/common-component.css' %}"> |
There was a problem hiding this comment.
Two things here:
common-component.css(singular) sits directly above the pre-existingcommon-components.css(plural), and both are now loaded globally. Near-identical names one line apart is a maintenance trap — worth renaming to something unambiguous.- This is where the missing Google/Apple SDK
<script>tags need to go (see my comment onChooseView.jsx) — without them the SSO buttons never enable.
Also: auth.css is loaded on every page sitewide for a feature used on two. Worth gating behind the auth views if that's easy.
…ding data to sailsforce.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…_type Checking localStorage.isReturningVisitor alone diverges from Sefaria.isReturningVisitor() (static/js/sefaria/sefaria.js), which also factors in sessionStorage.isNewVisitor. That caused first-session visitors to be misreported as "old" on a page reload within the same session, since markUserAsNewVisitor() sets localStorage.isReturningVisitor immediately. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
import_gravatar runs outside the transaction.atomic block (avoids holding a DB transaction during the slow Gravatar fetch/GCS upload) but only mutates the UserProfile in memory, so profile_pic_url/profile_pic_url_small were never saved. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
del request.session[...] raises if the token is already gone (e.g. session expired between GET and POST), turning a successful reset into a 500.
…register_errors str(ValidationError) formats as a repr'd list (e.g. "['msg']") when there's no error code, breaking client-side EMAIL_EXISTS_ERRORS matching and showing bracketed text to users. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- interface_language_test.py instantiated LanguageSettingsMiddleware() with no get_response, which only worked because of Django 4.2's now-removed default; pass a no-op callable instead. - SaveUserTest asserted profile.settings['interface_language'] on a MagicMock, which never round-trips through __setitem__/__getitem__; assert the __setitem__ call instead. - Google/AppleMobileTest asserted the sessionid cookie is absent after session.flush(), but Django deletes cookies by resending them empty with Max-Age=0, not by omitting them; assert the empty value instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…re signals Google's rendered button lives in a cross-origin iframe, so method_chosen/ process_started were only ever synthesized on a successful credential — closing the popup/account picker without finishing produced zero events at all, on both desktop and the mobile redirect flow. Wires up GIS's official click_listener for a real click signal (mirroring Apple's triggerApple), adds a state-driven (not fixed-timeout) popup-abandonment detector that can't be fooled by a slow backend or a long user pause, and adds a pageshow/bfcache listener to catch the mobile browser-Back abandonment path that beforeunload/popstate can't see. Also fixes a One Tap bug where a successful credential return (getDismissedReason() === 'credential_returned') was being miscoded as a failure event alongside the real success event. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
isDisplayed() reflects Google's own choice to show the One Tap widget, not a user action — recording method_chosen/process_started there counted intent for people who never touched it. Moves the funnel burst to the credential callback (the earliest real proxy for "clicked", since One Tap only offers accounts already signed into the browser and hands back a credential unconditionally on click, before our own backend's success/ failure is known). If the user never engages, nothing fires at all - not even flow_ended - since no attempt began. This also removes the concluded/markConcluded race guard entirely, since dismissal notifications no longer fire any events for it to race against. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
window doesn't exist in Node, so a direct GET to /register or /login server-rendered the plain library shell instead of AuthPage. React 16's hydrate() then failed to cleanly replace that mismatched tree client-side, leaving AuthPage's content stuck inside the stale panelContainer wrapper — dropping .sefaria-auth-page (and its background) rather than just delaying it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AuthPage offset itself below the header via a hardcoded --header-height, duplicating that push when a siteWideBanner already moved #main down in flow. Centralize the offset as --header-offset on #main, which collapses to 0 when a banner precedes it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Slot was flush against the container edge; visually it needs a 1px inset toward the start edge and 1px down to align correctly.
RegisterView's email field only checked required-ness on blur; format was left entirely to server-side rejection on submit. Now also checks the input's native type="email" validity and shows auth.invalid_email, the same key sso/views.py's password_reset_api returns for a malformed email, so the message matches server-side validation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ter navigation next was captured from currentPath() at render time in header buttons, and unconditionally from currentPath() on every fresh auth navigation - so clicking Login/Sign up while already on /login or /register baked the auth page's own URL into next, growing on every repeat. next is now always resolved centrally in ReaderApp.openURL at the moment of navigation: reused from the already-preserved authPath when already in the auth flow, otherwise read fresh from the current page.
…n mobile Previously it sat 16px past the padding boundary (40px from the true edge); now it's inset 16px from the edge itself, overlapping into the padding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Raising this as a finding rather than a change — it's your branch and your call. While adding
The last pair is the one worth a look. raise forms.ValidationError("This email address is already registered via Google Sign-In.")and the client regex-reconstructs the structure that was just discarded: 'This email address is already registered via Google Sign-In.': { code: 'sso_only_account', providers: ['google'] },The Sefaria mobile app now does the same string-match, against the same sentences. So one English sentence is load-bearing API for two clients in two repos, with no test or type that would catch a copy edit — a wording change in Notably, login already does this correctly — A possible shape, if you think it's worth it:
I had implemented this and have since removed it from #3580 — it was out of scope for a mobile fix and it's your branch's code. Happy to open it as a separate PR against 🤖 Generated with Claude Code |
reCAPTCHA always loaded in its default (English/LTR) language regardless of the page's interface language, which also threw off widget alignment on Hebrew/RTL pages where the surrounding layout is RTL but the widget itself rendered LTR. Passing hl= on the script URL (the only place reCAPTCHA's language is configurable) ties it to request.interfaceLang, fixing both the language mismatch and the alignment issue. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # build/ci/createJobFromRollout.sh # pytest.ini # reader/views.py # sefaria/system/context_processors.py # sefaria/views.py # static/js/sefaria/i18n/interface-context/en.json # static/js/sefaria/i18n/interface-context/he.json
safeNext's regex only rejected a second literal "/", letting a backslash (e.g. "/\evil.com") slip through and get normalized by the browser into a protocol-relative "//evil.com". Rewritten to resolve `next` against a placeholder base via the URL parser and compare origins, so the browser's own normalization is what decides safety instead of a hand-rolled regex. register() in sefaria/views.py echoed the client-supplied `next` back as `redirect` in its JSON response (consumed verbatim by RegisterView.jsx) with no host/scheme validation at all. Added the same url_has_allowed_host_and_scheme check CustomLogoutView already uses.
…emailing Users with SSO-only accounts got a generic reset-email response when requesting a password reset, even though they can't use a password to log in. password_reset_api now returns the same sso_only_account error as email_login. Extracted the shared check into _sso_only_account_error. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add a small sefaria-blue spinner + "Loading"/"טוען" text in the AuthCard sub area while ChooseView waits on a popup-mode Google/Apple SSO round trip, and while ForgotView/LoginView/RegisterView await their submit response. Also fix FormView's submitting flag being cleared right before a Login/Register redirect actually fires, which caused a brief flash back to the normal sub text. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Native iOS Sign in with Apple was rejected with a bare
`auth.social_signin_failed`; the server-side log showed
`apple token verification failed error=['Invalid token.']`.
Apple stamps `aud` differently per surface: the Services ID
(org.sefaria.web.signin) for the web JS SDK, but the app's bundle ID
(org.sefaria.sefariaApp) for a native iOS sign-in. Both have to be
accepted.
The settings declared that list under `APP['settings']['audience']`,
but allauth has no such setting -- it is silently ignored.
`AppleProvider.get_auds()` is literally
`self.app.client_id.split(",")`, so the allowed audiences come from a
comma-separated `client_id` and nothing else. Every native iOS token
therefore failed audience validation while web sign-in kept working.
Join the two IDs into `client_id` and drop the dead `audience` key.
Web behaviour is unchanged: `AppleOAuth2Adapter.client_class` is
`AppleOAuth2Client`, whose `get_client_id()` returns
`consumer_key.split(",")[0]`, so the Services ID stays first and
remains what the authorize redirect, the token exchange, and the
client-secret JWT's `sub` all carry. `reader/views.py` builds the web
SDK's `appleClientId` from the `APPLE_SSO_CLIENT_ID` setting directly
and is untouched. The change is strictly additive -- it only widens the
set of accepted audiences.
No mobile-client change is required.
Claude-Session: https://claude.ai/code/session_01P6SyKh9dkDroXyNma5X9UN
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Replaces Sefaria's old server-rendered login/register/password-reset templates with a single React auth experience (
AuthPage), and adds Google + Apple Sign-In (SSO) for web and mobile, built ondjango-allauth. Also includes a new sign-up funnel analytics pipeline, an account-settings UI update to reflect SSO-linked accounts, and a handful of unrelated fixes picked up along the way (SSR crash, proxy header handling, i18n).Backend:
django-allauthintegration + newssoappdjango-allauth(allauth,allauth.account,allauth.socialaccount, Google + Apple providers,allauth.headless) toINSTALLED_APPS, plus its authentication backend and account middleware (sefaria/settings.py).ssoapp (sso/adapters.py,sso/views.py,sso/urls.py):SefariaAccountAdapter— tiesemailusernames(username == email) into allauth, and honors anextredirect stashed in a short-livedsefaria_sso_nextcookie (needed because Google's redirect-mode SSO can't carrynextas a query param — thelogin_urimust be an exact-match registered redirect URI).SefariaSocialAccountAdapter— handles first-time social sign-up (creates the MongoUserProfile, assigns slug, imports Gravatar, registers the user in Salesforce CRM) and email-collision linking (disables the password on an existing email/password account when a Google/Apple sign-in claims the same email, making it SSO-only going forward).import_gravatarextracted out ofsefaria/views.pyintosso/adapters.pyso both the SSO and classic email registration paths share it.sso/urls.py:POST /api/auth/google/redirect— Google One Tap redirect-mode callback (delegates to allauth'sLoginByTokenView)POST /api/auth/google/mobile/POST /api/auth/apple/mobile— native mobile (React Native) Google/Apple sign-in; verifies the provider ID token server-side and returns SimpleJWT{access, refresh}tokensPOST /api/auth/apple/callback— Apple Sign-In popup mode (web)POST /api/auth/login— JSON email login for the new AuthPagePOST /api/auth/password/reset— JSON password-reset requestClearSsoNextCookieMiddleware— deletes thesefaria_sso_nextcookie as soon as either SSO callback view responds (success or failure) so it can't be replayed by a later, unrelated attempt.SECURE_PROXY_SSL_HEADERnow set sorequest.is_secure()(and allauth's OAuth2 callback URLs built fromrequest.build_absolute_uri()) correctly reporthttpsbehind the TLS-terminating ingress.sefaria/forms.py— registration now distinguishes "already registered via Google", "via Apple", or plain email/password when an email collision is hit.sefaria/views.py:CustomLoginView/CustomPasswordResetConfirmViewnow render the SPA shell (base.html) instead of the old dedicated templates, andCustomPasswordResetConfirmViewgained a JSON POST path (submit new password, or request a resend when the link has expired) so the flow works fully client-side.process_register_formsimplified now that Gravatar import lives insso/adapters.py.account_settingsview/template now surfacesocial_providers(fromrequest.user.socialaccount_set) so the account page shows "Google/Apple Sign-In" instead of an editable email field for SSO-only accounts, and the Google Drive connection copy was reworded for clarity.registration/login.html,registration/register.html,registration/accounts.html, and the long-deadtranslate_campaign.html./accountsview.LanguageSettingsMiddlewarerefactored to extract interface-language resolution into a shared helper (used by both the "excluded path" short-circuit and the normal path), fixing interface language on excluded paths (e.g. auth API paths) instead of hardcoding English.GOOGLE_SSO_CLIENT_ID/APPLE_SSO_*settings (local_settings_example.py, Helm chartlocal-settings-file.yaml),django-allauthadded torequirements.txt.Frontend: new
AuthPageReact experience (static/js/auth/)AuthPage.jsx— single state machine (view×flow) that swaps card content in place with no full page navigation, replacing the old/login,/register, and/password/reset/confirmserver-rendered pages. Views:choose,email(login or register),forgot,forgot-sent,reset,reset-expired,reset-success.ChooseView,LoginView,RegisterView,ForgotView,ResetView,ResetExpiredView,MessageView,ErrorBanner,AuthCard,Divider,LegalText,EmailInput,PasswordInput,ProviderButton,FormView.useSsoSignIn.jsx— shared hook wiring up Google Sign-In (button + One Tap) and Apple Sign-In JS SDKs, exposing provider-ready flags and click triggers to the views above.GoogleOneTap.jsx— renders Google's One Tap prompt globally, mounted once inReaderApp.utils.js— path/flow helpers (pathToFlow,flowToPath,nextFromPath,isAuthPath,withNext) shared betweenAuthPageandReaderApp.static/js/common/:Input.jsx,Captcha.jsx.ReaderApp.jsx— now owns auth routing: recognizes/login,/register, and reset-confirm paths, tracksshowAuth/authPath/authSourcein component state and browser history, and rendersAuthPagein place of the normal panel container when active. Also mountsGoogleOneTapand resumes any pending sign-up analytics attempt on mount.Header.jsx— login/sign-up links now route through the in-appAuthPage(via a newAuthNavLinkthat callsopenURL) instead of full-page navigation, and the nav-bar sign-up button carries adata-signup-sourcemarker for funnel attribution.auth.scss/auth.cssandcommon-component.scss/common-component.css, plus new icons (google.svg,apple.svg,eye.svg,eye-off.svg,arrow-left.svg,info-error.svg) and auth background images.base.html— exposeswindow.SefariaAuth/ passesgoogleClientId,appleClientId,recaptchaSiteKey, and reset-link state (authResetUid,authResetValid) down to the client.templates/elements/login_method_text.html— small partial used by the updated account-settings page.Sign-up funnel analytics
signupAnalytics.js/useSignUpTracking.js— new tracking layer for the sign-up funnel (method chosen, process started/ended, resuming an in-progress attempt across a redirect-based SSO round trip viaresumePendingSignUpAttempt), replacing the old inline GA4 script that used to live inregister.html.static/js/auth/tests/signupAnalytics.test.js,useSignUpTracking.test.js,utils.test.js.i18n
auth.*string keys added tostatic/js/sefaria/i18n/interface-context/en.jsonandhe.json(English + Hebrew) for every AuthPage view, replacing the old templates' hand-rolledint-en/int-hespans.Tests
sso/tests/adapters_test.py,sso/tests/views_test.py,sso/tests/middleware_test.py— new backend coverage for the adapters, allsso/views.pyendpoints, andClearSsoNextCookieMiddleware.reader/tests/password_reset_confirm_view_test.py— coverage for the new JSON reset-confirm flow.sefaria/tests/forms_test.py,sefaria/system/tests/interface_language_test.py— coverage for the updated registration-form email-collision messaging and the refactored language middleware.e2e-tests/— Playwright specs and page objects (loginPage.ts,signupPage.ts,mobileHamburgerPage.ts) updated for the newAuthPageflow; addedUser Menu/auth-page.spec.tsand updatedmobile web/auth-flow.spec.ts.Other fixes bundled into this branch
static/js/SiteWideBanner.jsx/sitewidebanner).X-Forwarded-Protobehind the TLS-terminating proxy (seeSECURE_PROXY_SSL_HEADERabove).nextinstead of always redirecting home; thesefaria_sso_nextcookie is sent asSameSite=Noneand percent-decoded correctly before use.sefaria_sso_nextcookie value.Notable dependencies/config
django-allauth[socialaccount].GOOGLE_SSO_CLIENT_ID,APPLE_SSO_CLIENT_ID,APPLE_SSO_IOS_BUNDLE_ID,APPLE_SSO_TEAM_ID,APPLE_SSO_KEY_ID,APPLE_SSO_PRIVATE_KEY— must be populated (via env vars in deployed environments) for Google/Apple sign-in to work.