fix(server): resolve clan-tag ownership before rejoin identity updates

[evanpelle]

Problem

A pre-start page refresh restored a reserved clan tag the server had already dropped at admission. Reported in the wild: a clan's pending applicant (not yet a member) played game a1Vnq3sk wearing the clan's tag.

Mechanics of the bypass:

1. First join works correctly: resolveClanTag drops the reserved tag because the player's /users/@me clans list doesn't include it (a pending request lives in the separate clanRequests field).
2. The player refreshes while still in the lobby. Their client re-sends the tag, so the stored identity no longer matches and planJoinVerify returns verify — but join_verify only censors (it receives no account identity, so it cannot check membership) and echoes the tag back approved.
3. The Worker passed that pair straight into rejoinClient and returned — before ever reaching the resolveClanTag block, which only ran on the fresh-join path. rejoinClient applies the tag with no ownership check.

No exploit knowledge needed: set a tag you don't own, join a lobby, hit F5. (This gap was flagged as a pre-existing follow-up during the #4680 review.)

Fix

Reorder the join path so the account fetch and clan-tag ownership resolution run before any identity-carrying rejoin:

• The claims/getUserMe/allowed-flares block becomes a loadAccount() closure.
• Skipped-verify reconnects (post-start, or pre-start with unchanged identity) keep their zero-API fast path — mass reconnects at game start stay off the API — and still carry no identity update.
• Everyone else (first joins, and pre-start reconnects with a changed identity) loads the account, resolves tag ownership, then attempts the rejoin with the screened, ownership-resolved pair; a miss falls through to the fresh-join path unchanged.

Behavior notes:

• First joins make the same single getUserMe call as before, just earlier.
• Pre-start identity-changed reconnects now cost one getUserMe call (they already made a join_verify call), and now go through the allowed-flares gate and close on a getUserMe failure — consistent with how a fresh join handles the same conditions.
• Also updated the rejoinClient doc comment to state the strengthened caller contract.

Testing

• npx vitest tests/server tests/Privilege.test.ts --run — 355 tests pass. The resolveClanTag matrix in tests/Privilege.test.ts already covers the pending-member case (reserved tag, not in ownedClanTags → dropped); this PR is pure ordering in the Worker's inline ws handler, which has no test harness (same as the existing Turnstile gating there).
• npx tsc --noEmit and npm run lint clean.

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

Changes

Join and reconnect identity

Layer / File(s)	Summary
Account loading and reconnect authorization src/server/Worker.ts	Account loading now returns success or failure. Verified reconnects can skip API calls. Identity updates occur after account and clan-tag checks.
Resolved clan-tag propagation src/server/Worker.ts, src/server/GameServer.ts	Reconnects and new clients receive the resolved clan tag. rejoinClient documents the caller-side ownership check.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Worker
  participant AccountAPI
  participant GameServer
  Client->>Worker: Send join request
  Worker->>AccountAPI: Load account when required
  AccountAPI-->>Worker: Return authorization result
  Worker->>GameServer: Rejoin with screened identity and resolved clan tag
  GameServer-->>Client: Replace WebSocket

Loading

Possibly related PRs

• openfrontio/OpenFrontIO#4420: Both update Worker join and reconnect authorization handling.
• openfrontio/OpenFrontIO#4680: Both update reconnect identity and clan-tag resolution.
• openfrontio/OpenFrontIO#4778: Both update clan-tag ownership resolution and propagation.

Suggested reviewers: ryanbarlow97, zixer1, celant

Poem

Tags are checked before names take flight,
Accounts guide the join just right.
Reconnecting sockets find their way,
New clients carry tags today.
Clean flows now guard the gate.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main server fix: resolving clan-tag ownership before rejoin identity updates.
Description check	✅ Passed	The description explains the clan-tag bypass, the join-flow fix, behavior changes, and validation results.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@src/server/Worker.ts`:
- Line 562: Update the unauthorized join handling in Worker.ts so the close
reason passed to ws.close() uses translateText() instead of a hardcoded
“Unauthorized” string, and add the matching English resource entry in
resources/lang/en.json.

🪄 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: 40da452d-218d-4341-bd5a-12ce6da1a703

📥 Commits

Reviewing files that changed from the base of the PR and between 0668045 and 88f26ac.

📒 Files selected for processing (2)

• src/server/GameServer.ts
• src/server/Worker.ts

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Localize the close reason.

ws.close() sends "Unauthorized" to the client. Route this text through translateText() and add its English resource entry.

As per coding guidelines, “All user-visible text must go through translateText() and have a corresponding entry in resources/lang/en.json.”

🤖 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/server/Worker.ts` at line 562, Update the unauthorized join handling in
Worker.ts so the close reason passed to ws.close() uses translateText() instead
of a hardcoded “Unauthorized” string, and add the matching English resource
entry in resources/lang/en.json.

Source: Coding guidelines