test(auth): end-to-end journeys for OTP sign-in, stubbed and against a real backend - #9
Merged
Merged
Conversation
…a real backend Twelve journeys under e2e/auth.spec.ts covering the sign-in state machine: the happy path through to /chat, a malformed number never reaching the server, the server's 422 winning over the client's guess, 429 carrying the user forward rather than blocking them, a wrong code leaving no session, the resend countdown, and expired or unreadable tokens being discarded instead of trusted. Plus e2e/live/, one journey with nothing stubbed. The code cannot be read back from the API — PasswordlessAuth keeps it in the backend's memory and there is deliberately no route that returns it — but both send paths compose it into messages.body before the message reaches Gupshup, so the test reads it there. Every server-side step stays real without a test-only door into production code. The live suite is opt-in (yarn e2e:live) and excluded from CI, which has neither Postgres nor Phoenix. It runs serially with retries off: the backend throttles a phone to one OTP per 30s, so a retry would fail on the throttle rather than on the defect. It seeds its own contacts because glific#5710 creates them with no consent and no session, so a first-time number is deliberately sent nothing until #5713. Also drops the README's claim that the login OTP is 9999, which #5710 removed.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
The live journey talked to the database directly: pg as a devDependency, an E2E_DATABASE_URL, and SQL against contacts and messages. That couples the widget's test harness to the backend's schema and reaches past every authorization check in the product. Both things it needed are on the staff GraphQL API. optinContact plus updateContact seed a messageable contact, and the messages query reads the body carrying the code — the OTP is legible there exactly as it is to an operator watching the conversation. So the suite now signs in as staff and touches nothing an operator could not. bspStatus is forced to SESSION_AND_HSM after optinContact, which leaves it at HSM. The HSM branch needs an approved verify_otp template that a dev backend will not have; SESSION_AND_HSM routes the send down create_and_send_otp_session_message/2 instead — plain text, no template, no Meta approval. The staff credentials have no defaults. A wrong guess surfaces as a 401 midway through a journey, which reads as the feature being broken rather than the harness being unconfigured.
rvignesh89
reviewed
Sep 8, 2026
Galaxy S8 and S24 instead of a Pixel: Samsung is what this audience actually holds, and the pair brackets the range — the S8 is the small, old, still-common end and the S24 the current one. Adds a Firefox project too, so Gecko is not unrepresented. Firefox takes the descriptor's user agent and viewport but ignores isMobile and hasTouch — maxTouchPoints reads 0 against Chromium's 1 — so that project is Gecko at a phone-sized viewport, not a phone. Said so in the config rather than let someone read it as touch coverage. Six projects now, 126 tests, 41s. CI installs firefox alongside chromium and webkit.
rvignesh89
approved these changes
Sep 8, 2026
AmishaBisht
added a commit
that referenced
this pull request
Sep 8, 2026
* feat(auth): real OTP validation, honest errors, and resend Part of glific/glific#5662. The login screen was built against a prototype backend that accepted a hardcoded "9999" and never sent anything. The real endpoints send a six-digit code over WhatsApp and answer with distinct statuses, so the widget now speaks that contract. * Removes the "Prototype: use 9999" hint, the visible half of the bypass the issue requires gone. * Validates the phone before spending a request, and normalises it — spaces, dashes and brackets are stripped, so "91 98201-98765" reaches the server as "919820198765". This is a courtesy pre-check only; the server still decides what a valid number is. * The OTP field takes exactly six digits, which is what PasswordlessAuth mints. A shorter entry could only ever come back 401. * 429 and 404 stopped rendering as "Invalid OTP" and "Unable to send OTP". Each status now maps to copy that says what actually happened, with 422 and 429 passing the server's own message through since it already carries the format hint and the wait. * Adds a resend with a countdown matched to the server's throttle window, and an explanation that the code arrives on WhatsApp. request-otp answers the same way for a number it has never seen as for one it knows, so nothing can tell the user their number was wrong — this is the only recourse they get, and it is why the copy is not optional polish. Login.test.tsx goes from 3 to 14 cases, covering each failure status, the countdown, and that the bypass copy is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JqHf9PqoGCW27fcNbQnnT * fix(config): derive the backend origin from the hostname instead of baking it in VITE_GLIFIC_API_URL was a build-time constant, and the backend origin is not a build-time fact. One build serves every organisation — theming is applied at runtime precisely so a colour change is not a redeploy — so web.tap.glific.com and web.staging.glific.com are the same bundle and must reach different APIs. A baked-in URL can only ever be right for one of them. The origin is now resolved in this order: 1. VITE_GLIFIC_API_URL / VITE_WEB_SOCKET when set. Explicit config always wins. 2. Otherwise derived from the hostname, swapping the leading `web` label for `api`: web.staging.glific.com -> api.staging.glific.com. 3. Otherwise same-origin relative paths. Step 3 is local dev, where Vite proxies /api and /web_socket and there is no cross-origin request to make. Step 1 is what Vercel preview deployments need. A hostname like glific-web-channel-git-main-glific.vercel.app carries no organisation information whatsoever, so there is nothing to derive and both variables have to be set on the Preview environment. Deliberately, an unconfigured preview falls through to same-origin paths that hit Vercel and fail loudly, rather than guessing an origin and quietly pointing a preview at a real org's API. Two details that are easy to get wrong and are pinned by tests. The first label must be exactly "web", or webhooks.glific.com resolves to api.glific.com. And the socket needs a full ws(s) URL once it is cross-origin — the phoenix client derives ws://host from window.location for a path-only endpoint, which is correct only same-origin. config.test.ts covers the derivation as a pure function and then re-imports the module under a stubbed location to prove the exported endpoints actually use it, since a correct helper that nothing calls would otherwise look fine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JqHf9PqoGCW27fcNbQnnT * feat(auth): let the user go back and correct their number The OTP step told the user to "check that you entered the number your WhatsApp account uses" and then gave them no way to do it. Short of reloading the page, a mistyped digit was a dead end — and because request-otp answers identically for a number it has never seen as for one it knows, a typo is indistinguishable from a code that is merely slow to arrive. That advice was the only recourse on offer, so it needed to be actionable. "Use a different number" sits directly under the line naming the number it changes. It pre-fills what was typed rather than clearing the field, since the user is almost always correcting one digit rather than starting again, and it drops the code they may have entered for the number they are leaving so it cannot be submitted against a different one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JqHf9PqoGCW27fcNbQnnT * feat(auth): silent token refresh, expiry-aware restore, and 429 as "already sent" Three changes, all following from the backend token now living one hour rather than twenty-four. **A throttled request advances instead of stalling.** A 429 from request-otp does not mean sending failed — it means a code was already sent and the user asked again too soon. Keeping them on the phone step left a live code sitting in their WhatsApp with nowhere to type it. The 429 path now advances exactly as a 200 does and shows the server's wait as a notice rather than an error, because it is information about a code they have, not a failure about one they do not. Every other status still holds them on the phone step. **A stored token is only honoured while it is alive.** The route guards asked whether a token existed, so an expired one sent the user to /chat to fail at the socket join — a chat that never connects rather than a login screen. Both guards now ask whether the session is live and clear a dead one on the way past. decodeTokenExpiry reads the exp claim for scheduling only; it verifies nothing, and a token whose exp cannot be read counts as signed out. **The session refreshes itself.** useSessionRefresh renews once the token has ten minutes left, checking every thirty seconds and again whenever the page regains focus. The focus trigger is not redundant: a backgrounded tab has its timers throttled hard, so a user returning after an hour needs a token immediately rather than up to thirty seconds later — by which point the socket has already tried to reconnect with a dead one. Both triggers can fire in the same tick, so an in-flight guard stops the second renewal sending a token the first has already replaced. Only a refusal ends the session. A 5xx or a request that never got an answer leaves it alone and retries, because signing someone out because their wifi dropped is the same bad outcome as signing them out for a dropped packet. Also switches the phoenix socket's params from a captured object to a function. Phoenix re-evaluates params on every connect attempt, so a static token meant any auto-reconnect after a renewal retried forever with the replaced token and surfaced as a permanent "Reconnecting…" rather than an auth failure. This was survivable at a 24 hour TTL and is not at one hour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JqHf9PqoGCW27fcNbQnnT * refactor: cut comments that restate the code Comments narrating what the line below does are noise. Kept only those carrying something the code cannot: why an approach beat the obvious alternative, a hazard invisible locally, or a constraint imposed elsewhere — the phoenix params function, the exactly-"web" hostname label, the 429 meaning a code was already sent. config.ts was 55% comments and webChannelAuth.ts 30%; both are now around 20%, with no change in behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JqHf9PqoGCW27fcNbQnnT * test(config): assert the derived host is the api host, not the console Carried over from main's fix/derive-api-host, which this branch's own config rewrite would otherwise have dropped: the assertion that names *why* the api. prefix matters, rather than only that the mapping produces it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JqHf9PqoGCW27fcNbQnnT * test(auth): end-to-end journeys for OTP sign-in, stubbed and against a real backend (#9) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Amisha Bisht <amishabisht33@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Playwright coverage for the OTP sign-in flow in #4 / glific#5710. Stacked on
web-channel-otp-authso the journeys run against the real login UI rather than the prototype one onmain.Two suites
e2e/auth.spec.ts— 12 journeys, run in CI. The sign-in state machine as the browser sees it, with the endpoints stubbed:/loginwith no session, and the message says nothing about whye2e/live/otp.spec.ts— 3 journeys, opt-in,yarn e2e:live. Nothing stubbed: a real request to/api/v1/web_channel/request-otp, a code minted byGlific.OTP, a real message row, a real signed JWT. Excluded from CI, which has neither Postgres nor Phoenix.How the test receives the code
It can't come from the API.
Glific.OTPkeeps the code inPasswordlessAuth's in-memory store inside the backend's BEAM, and there is deliberately no route that hands it back — adding one, even env-gated, is the kind of thing that survives to production.Both send paths compose the code into
messages.bodybefore the message goes near Gupshup (Glific.Messages.create_and_send_otp_session_message/2, and the HSM path via parameter substitution). So the test reads it from the row. Every server-side step stays real; the only hop not covered is Gupshup handing the message to WhatsApp, which needs a real handset and a human reading it, and so stays a manual acceptance check.Notes for review
can_send_message_to?/1and is sent nothing at all — by design, until #5713 adds the exemption. Without seeding, the journey would assert on a message that is never composed.pgas a devDependency (live suite only) and ayarn e2e:livescript.9999— #5710 removed that bypass.clock.fastForwardfires each timer at most once, so a one-second countdown only ticks once.clock.runForfires every tick — and it needs the countdown on screen first, or the jump lands before React has mounted the interval and skips nothing.Verified locally: typecheck, lint, 100 unit tests, 84 e2e across all four device projects, five consecutive full runs with no flakes.