[Backend] Fix Discord account linking's dead ends: raw JSON error pages and the wrong "expired" DM on conflict #392
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Impact: HIGH
Found in the August 2026 session lifecycle review (#319).
What the user experiences
Account linking is a dead end from both directions. On the web side,
Profile.jsxtells the user a linking flow exists without saying what it is or naming the command — a player learns they need to link only reactively, after a vote is silently discarded, via a DM that is itself swallowed if their DMs are closed. When a player does click the link from Discord and the token has expired or been consumed, the browser shows a raw JSON error body — no page, no instruction. Worse: if the Discord account is already bound to another Quest Board account, the browser again gets a raw JSON409, and the bot's background poller then seeslinked=Falseforever and DMs the player the wrong message — "expired" — sending them into a loop re-running/linkon a conflict/linkcan never resolve. The only escape,/unlinkon the other account, is never mentioned to them.Evidence
webapp/frontend/src/pages/Profile.jsx:725— names that a linking flow exists, not what it is or which command starts it.webapp/backend/app/routers/auth.py:312-317— expired/consumed token returnsHTTPException(400, "Link token is invalid or expired")as raw JSON to the browser.webapp/backend/app/routers/auth.py:325-331— a Discord ID already bound to another account returns a raw409, again as JSON to the browser with no page.bot/questboard_bot/cogs/linking.py:141-142— the bot's 30-second poller seeslinked=Falseafter a conflict and DMs the "expired" message, which is the wrong diagnosis for a conflict/linkcannot fix.bot/questboard_bot/cogs/voting.py:270-277— the reactive "you need to link" DM after a failed vote is itself swallowed when the user's DMs are closed.bot/questboard_bot/cogs/notifications.py:506-512—session_vote_reminderlists unlinked players by name under "Not linked in Discord" without ever saying "run /link".Why it matters for a hosted product
A customer who hits the conflict case is stuck in an unrecoverable loop with no message anywhere telling them the actual fix (
/unlinkon the other account) — this is a dead end the product itself created and cannot signal a way out of.Proposed fix
Serve a real, styled page from
webapp/backend/app/routers/auth.py:312-331instead of raw JSON — one that explains what happened and what to do next, distinguishing "expired" from "already linked to another account". Teach the bot's linking poller to distinguish those two cases and DM the correct one, naming/unlinkon the conflicting account as the resolution. HaveProfile.jsxname the actual command (/link) and show the instruction, not just imply a flow exists. This is the audit's P7 and P29.Acceptance criteria
auth.py's expired/consumed-token path renders a real page, not raw JSON.auth.py's already-linked-to-another-account path renders a distinct real page, not raw JSON./unlinkas the fix for the latter.Profile.jsxnames/linkas the command and shows the instruction directly, rather than only implying a flow exists.Picking this up as v4.3.0 phase 5, lane B (#514), last on the branch. Decisions: the two browser dead ends become real pages with the exact next step on each (run
/linkagain vs sign in as the other account and/unlink); the link-status endpoint the poller reads gains an additivestate(pending|linked|expired|conflict) so the bot sends the right DM; closed DMs fall back to a self-deleting channel mention instead of silence; the vote reminder tells unlinked players to run/link; the Profile page names the command.Done in PR #521 (merged); ships with v4.3.0.
Both dead ends now render real pages (
app/auth/link_pages.py), keeping 400 and 409: "This link has expired → run/linkagain", and "This Discord account is already linked → sign in as that account, run/unlink, then link again". Small self-contained pages rather than an SPA redirect, which would need a route, a query-parameter vocabulary and a component for a page nobody should see twice, and would bounce an unauthenticated reader to a login they do not need.Worth knowing: those pages would have rendered unstyled. The backend sets
default-src 'none'on every response,style-srcfalls back to it, and nginx adds its own CSP to proxied/auth/responses, so the browser enforces the intersection.SecurityHeadersMiddlewarenow emitsdefault-src 'none'; style-src 'unsafe-inline'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'fortext/htmlresponses only; scripts, images and connections stay denied, the pages contain no user-controlled text, and a test asserts JSON endpoints keep the strict policy verbatim.The poller's wrong message needed backend state:
/auth/linkrecords the outcome in Redis and/api/bot/link-status/{token}gains an additivestate(pending | linked | expired | conflict). The bot branches on it and speaks as soon as the outcome is known rather than waiting out the ten minutes, so a conflict is reported within 30 seconds, names/unlink, and no longer masquerades as an expiry.Closed DMs no longer swallow anything: the linking poller and the pre-vote link prompt fall back to a short self-deleting channel mention. The vote reminder's "Not linked in Discord" list now says to run
/link, andProfile.jsxnames the command with a two-line instruction instead of alluding to "the bot's account-linking flow".No contract bump (all additive). Backend tests cover both pages and all four
statevalues; bot tests cover each poller branch and both DM fallbacks; a frontend test covers the Profile copy.