feat(steam-link): browser side of Steam account linking - #4849
Merged
Conversation
Adds the modal a player sees during Steam <-> web account linking: it names the Steam persona (from GET /auth/steam/link_ticket/:token) and the logged-in web account (from /users/@me, never the token) and requires explicit confirm before the link is committed. Extends SteamLink.ts with fetchSteamLinkTicket for the persona read, wires Main.ts to open the modal on #steam-link?token=... and to resume a stashed token after a login redirect, and mounts <steam-link-modal> in index.html so the boot hook has an element to find. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JRMRzHbZp2VhxbxqDMw4Zr
- redeemSteamLink now clears a stale JWT on 401 (logOut()), matching the convention every other authenticated call in Api.ts follows. - Guard against firing with an empty Authorization header (mirrors linkGoogle's guard in Auth.ts). - Fixed the status-mapping comment to describe current behavior (429/500/ network errors all collapse into "failed" today) rather than a future task's. - Rewrote the "treats a repeat redemption as ok" test to actually call redeemSteamLink twice; it previously called it once and could not fail for the reason its name claimed.
Adds a "Link an existing account" action to the account modal's Account tab that re-opens the Electron desktop shell's account-linking gate via window.openfrontDesktop.showLinkGate(). The gate is shown at first launch but the game will eventually run fullscreen borderless with the menu bar hidden, so a player who dismissed it (or wants to link later) needs another way back in. Absent entirely on plain web, since window.openfrontDesktop doesn't exist there. Guards specifically on showLinkGate being callable rather than a sibling property (window.openfrontDesktop.linkGate, used separately by the gate page itself) so a rename of one can't silently leave this button wired to nothing.
The desktop gate's browser-handoff can fail (wrong default browser, an odd Linux setup, Steam's overlay browser), in which case it shows an 8-character code and tells the player to enter it on the website instead. Nothing in the web client offered anywhere to type one. Reuses Task 15's confirmation modal rather than a second one: a new openForCodeEntry() entry point normalizes/validates the code client-side, then proceeds to the same ready/confirm render path the token flow uses. There is no server-side persona lookup keyed by code (only by token), so the confirm step shows the real, logged-in web account name and falls back to the existing "unknown persona" copy for the Steam side rather than inventing one. Also splits 429 out of redeemSteamLink's generic "failed" bucket into a distinct rate_limited reason with a parsed Retry-After, shared with the new redeemSteamLinkCode via a common postSteamLinkRedeem helper — the throttle refuses even a correct code/token once tripped, so collapsing it into "that was wrong" would be misleading. Existing 200/401/409/410 mappings and their tests are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JRMRzHbZp2VhxbxqDMw4Zr
… copy
Review findings on the code-entry form:
- openForCodeEntry() checked login before rendering the form, but nothing
survived the redirect to log in — a logged-out player arriving by code
was bounced to the account modal and lost the flow entirely, with no way
back except retyping a hash nothing on the page links to. Gives the
pending-link stash an explicit kind discriminator ("token" | "code_entry")
so a stashed code-entry intent can't be confused with a stashed token, and
adds resumePendingSteamLink() as the one place both get read back and
resumed after login. takePendingLink() also now degrades to null instead
of throwing on the old bare-string stash format.
- The confirm prompt's generic template ("Link Steam {persona} with account
{username}?") read as a doubled "Steam ... Steam account" whenever there
is no persona to show - always true on the code path, since no ticket
lookup exists for a code. Adds a dedicated confirm_prompt_no_persona
string instead of forcing one template to cover both cases, and removes
the now-unreferenced unknown_persona key.
- Adds Enter-to-submit on the code input.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JRMRzHbZp2VhxbxqDMw4Zr
…al dead end
Whole-branch review findings:
- The confirm prompt fell back to a placeholder noun ("your account")
whenever player.username was null - which is the default, unclaimed
state, not an edge case. That guts the confirm step's whole purpose (a
shared machine's browser could be logged into someone else's session) and
repeats the same doubled-noun copy bug just fixed on the persona side.
Follows the repo's existing username ?? publicId convention
(ApiSchemas.ts, PlayerName.ts) instead, and drops the now-unreferenced
unknown_username key.
- A refused code (the alphabet still has eye-confusable pairs like B/8,
S/5) landed on a confirm screen with only Cancel, which closes the modal
for good since Main.ts's strip() already removed #steam-link from the
URL - the same dead end this task exists to remove, one screen further
along. A code-mode refusal now returns to the code-entry form with the
draft still prefilled and the refusal explained inline, so a
mistranscribed character can be corrected and resubmitted without
reopening the modal.
- Minor: the load-error copy told a code-path player to "try again from
Steam", which is the token path's instruction. Gives the code path its
own message.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JRMRzHbZp2VhxbxqDMw4Zr
Contributor
WalkthroughThe client adds Steam account linking through desktop handoff tokens and eight-character codes. It persists pending flows across authentication redirects, retrieves ticket data, redeems links, renders localized modal states, routes URL hashes, and adds desktop entry points. ChangesSteam account linking
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DesktopBridge
participant AccountModal
participant Client
participant SteamLinkModal
participant SteamLink
DesktopBridge->>AccountModal: showLinkGate()
AccountModal->>Client: start Steam-link flow
Client->>SteamLinkModal: openWithToken or openForCodeEntry
SteamLinkModal->>SteamLink: fetch ticket or redeem token/code
SteamLink-->>SteamLinkModal: persona or redemption result
SteamLinkModal-->>Client: render confirmation, error, or success
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/client/SteamLink.ts (1)
273-282: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake the log label match the shared function.
postSteamLinkRedeemserves bothredeemSteamLinkandredeemSteamLinkCode, but both log lines sayredeemSteamLink. A failure on the fallback-code path is then logged under the token path's name. That makes triage harder. Log the function name and the body kind instead.♻️ Proposed change
+ const kind = "code" in body ? "code" : "token"; console.error( - "redeemSteamLink: request failed", + `postSteamLinkRedeem(${kind}): request failed`, response.status, response.statusText, ); return { ok: false, reason: "failed" }; } catch (e) { - console.error("redeemSteamLink: request failed", e); + console.error("postSteamLinkRedeem: request failed", e); return { ok: false, reason: "failed" }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/SteamLink.ts` around lines 273 - 282, Update the failure logs in the shared postSteamLinkRedeem flow to use the shared function name and include the request body kind, so both redeemSteamLink and redeemSteamLinkCode failures identify the correct path. Apply this consistently to the status-based and catch-block console.error calls.src/client/AccountModal.ts (1)
402-404: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHandle a rejected
showLinkGate()so a failure is not silent.
voiddiscards the promise. If the Electron IPC call rejects, the player clicks the button and nothing happens. No message appears, and the rejection surfaces only as an unhandled-rejection warning. On desktop this button is the only way back to the linking gate, so a silent failure leaves the player with no next step and leaves support with no log line.Add a
catchthat logs. The rest of this file already logs bridge and fetch failures withconsole.warn(Lines 919, 950).♻️ Proposed change
private handleShowLinkGate(): void { - void desktopLinkGateBridge()?.showLinkGate(); + desktopLinkGateBridge() + ?.showLinkGate() + .catch((err) => { + console.warn("Failed to open the desktop link gate:", err); + }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/AccountModal.ts` around lines 402 - 404, Update AccountModal.handleShowLinkGate to handle rejection from desktopLinkGateBridge()?.showLinkGate() by attaching a catch handler that logs the failure with console.warn, matching the existing bridge and fetch failure logging style in this file; preserve the current optional bridge invocation.tests/client/SteamLinkModal.test.ts (1)
116-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis test does not yet pin the "both identities loaded" safeguard.
The title says confirm stays disabled "until both have loaded". The body resolves both deferreds back to back at Line 130-131, so the only disabled assertion (Line 128) runs while neither has resolved. A regression that flips
loadStateto"ready"after only the ticket arrives would still pass this test. That safeguard is one of the stated goals of this PR, so it deserves a test that fails when it breaks.Resolve one side, assert the button is still disabled, then resolve the other.
💚 Proposed test change
expect(modal.isOpen()).toBe(true); // Still in flight: nothing to confirm yet, so confirm must be disabled. expect(confirmButton()?.disabled).toBe(true); ticket.resolve({ ok: true, personaName: "Ada" }); + // Only the Steam side has arrived. The web account is still unknown, so + // there is nothing to confirm yet. + await modal.updateComplete; + expect(confirmButton()?.disabled).toBe(true); + userMe.resolve(makeUserMe("web.1234")); await vi.waitFor(async () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/client/SteamLinkModal.test.ts` around lines 116 - 142, Strengthen the test around modal’s loading state by resolving only one of the deferred requests first and asserting confirmButton() remains disabled, then resolve the other request and retain the existing enabled assertion. Use the ticket and userMe deferreds in the test “renders both names once loaded, and disables confirm until both have loaded” to verify readiness requires both identities.src/client/SteamLinkModal.ts (1)
405-433: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the
as stringcast; let the compiler prove the persona is present.Line 430 casts
this.personaName as string. The cast is safe today only because the branch above already handledready && this.personaName === null, and because the inner ternary never evaluates the cast whenreadyisfalse. That safety depends on branch order, not on types. If someone reorders these branches later, the cast silently renders the string "null" into the confirm prompt — on the one screen whose purpose is naming the correct accounts.Bind the persona to a local first. The compiler then narrows it and no cast is needed.
♻️ Proposed refactor
const ready = this.loadState === "ready"; ... const account = ready ? (this.username ?? "") : translateText("steam_link_modal.loading_placeholder"); + const placeholder = translateText("steam_link_modal.loading_placeholder"); + const persona = ready ? this.personaName : placeholder; const prompt = - ready && this.personaName === null + persona === null ? translateText("steam_link_modal.confirm_prompt_no_persona", { username: account, }) : translateText("steam_link_modal.confirm_prompt", { - persona: ready - ? (this.personaName as string) - : translateText("steam_link_modal.loading_placeholder"), + persona, username: account, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/SteamLinkModal.ts` around lines 405 - 433, In the prompt construction around personaName, remove the `as string` cast and bind the persona value to a local after handling the ready/null case so TypeScript can narrow it to a present string. Update the ready branch in `confirm_prompt` to use that narrowed local, while preserving the loading placeholder and `confirm_prompt_no_persona` behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/client/SteamLink.test.ts`:
- Around line 184-191: Update the test case describing
resumePendingSteamLink(undefined) to state that it returns true when a pending
stash exists, and assert that return value explicitly while retaining the
no-throw and stash-consumption assertions. Use resumePendingSteamLink and
takePendingLink as the relevant symbols.
---
Nitpick comments:
In `@src/client/AccountModal.ts`:
- Around line 402-404: Update AccountModal.handleShowLinkGate to handle
rejection from desktopLinkGateBridge()?.showLinkGate() by attaching a catch
handler that logs the failure with console.warn, matching the existing bridge
and fetch failure logging style in this file; preserve the current optional
bridge invocation.
In `@src/client/SteamLink.ts`:
- Around line 273-282: Update the failure logs in the shared postSteamLinkRedeem
flow to use the shared function name and include the request body kind, so both
redeemSteamLink and redeemSteamLinkCode failures identify the correct path.
Apply this consistently to the status-based and catch-block console.error calls.
In `@src/client/SteamLinkModal.ts`:
- Around line 405-433: In the prompt construction around personaName, remove the
`as string` cast and bind the persona value to a local after handling the
ready/null case so TypeScript can narrow it to a present string. Update the
ready branch in `confirm_prompt` to use that narrowed local, while preserving
the loading placeholder and `confirm_prompt_no_persona` behavior.
In `@tests/client/SteamLinkModal.test.ts`:
- Around line 116-142: Strengthen the test around modal’s loading state by
resolving only one of the deferred requests first and asserting confirmButton()
remains disabled, then resolve the other request and retain the existing enabled
assertion. Use the ticket and userMe deferreds in the test “renders both names
once loaded, and disables confirm until both have loaded” to verify readiness
requires both identities.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 377fbca8-0a8a-4b9b-81fc-5457a614dab1
📒 Files selected for processing (9)
index.htmlresources/lang/en.jsonsrc/client/AccountModal.tssrc/client/Main.tssrc/client/SteamLink.tssrc/client/SteamLinkModal.tstests/client/AccountModal.rendering.test.tstests/client/SteamLink.test.tstests/client/SteamLinkModal.test.ts
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
Three items from the PR review: - `resumePendingSteamLink(undefined)` returns true, not false. The test title said the opposite and passed only because it never asserted the return value. The contract is "there was a stash, stop routing", not "a modal was opened" -- documented at the assertion. - Name the shared redeem helper and its payload kind in its failure logs. Both lines said `redeemSteamLink`, so a fallback-code failure logged under the token path's name. - Catch a rejected `showLinkGate()`. It is an IPC round trip that can genuinely reject; the bare `void` turned that into an unhandled rejection and a button that appears to do nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JRMRzHbZp2VhxbxqDMw4Zr
…k-browser # Conflicts: # resources/lang/en.json # src/client/Main.ts
github-project-automation
Bot
moved this from Development
to Complete
in OpenFront Release Management
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
1 participant
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.
Description:
The browser half of Steam account linking, for the upcoming Steam desktop release.
The desktop Electron build shows a gate at first launch asking whether the player already has a web OpenFront account. If they do, it mints an opaque token server-side and opens the browser at
#steam-link?token=…. This PR is what happens on this side: parse the token, show the player which two accounts are about to be linked, and redeem it on confirm. When the browser handoff fails entirely (wrong default browser, odd Linux setup, Steam's overlay browser), the desktop shows an 8-character code instead, and this PR provides the form to type it into.This is the change that makes account linking real — #4844 already removed the toast telling Steam players it was "coming in a later update".
What's here
src/client/SteamLink.ts— token parsing, the pending-link stash (consumed on read, with akinddiscriminator so a stashed token and a stashed code-entry intent can't be confused), and redemption againstPOST /auth/steam/link.src/client/SteamLinkModal.ts— the confirmation modal, in two modes (token and code entry).src/client/AccountModal.ts— a "Link an existing account" action that renders only when the Electron preload bridge is present, so plain web is untouched.resources/lang/en.jsononly. No other language file — those are Crowdin's.The confirmation step is the point
The token is opaque and carries nothing about the account. On a shared machine the browser may be logged into someone else's session, and linking is not freely reversible. So the modal shows both names and stops:
GET /auth/steam/link_ticket/:token./users/@me— never from the token, which is attacker-controllable.Confirm is disabled until both have loaded, and a partial load fails closed rather than showing a confirm button next to a blank name. Following this repo's
username ?? publicIdconvention (ApiSchemas.ts:429-430,PlayerName.ts:36) matters more here than usual:usernameis null until claimed, so without the fallback the prompt would have read "…with account your account?" for most players and identified nothing.Server contract
Fixed, and not changed by this PR:
200linked (idempotent),409with a machine-readablereasonsurfaced verbatim so each refusal gets its own message,410expired,429throttled withRetry-After(rendered as "wait", never as "wrong code" — the throttle refuses correct codes too),401→logOut().The endpoints live in the private infra repo and are not deployed yet. Nothing here breaks before they are: every failure path resolves to an error state in the modal, and the desktop gate fails open independently.
Please complete the following:
translateText(), keys added toen.jsononly, alphabetically sorted, andTranslationSystem's unused-key check is green.tests/client/SteamLink.test.ts,tests/client/SteamLinkModal.test.ts, and additions totests/client/AccountModal.rendering.test.ts.Verification
npx vitest run tests/clientplus the translation gates → 70 files / 875 tests passing, on top of currentmain.tsc --noEmit, ESLint and Prettier all clean.Known and deliberate
/linkalias for the hand-typed path.maxlength/autocapitalize/aria-label; worth a polish pass.Happy to fold either in if you'd rather they didn't land as-is.
Note on the PR gate
No linked
approvedissue — this is internal Steam-release work rather than a community contribution, so it should clear on repo permission. Glad to file an issue and relink if you'd prefer that route.