Skip to content

Replace one-time captain links with reusable team access codes - #263

Open
diese-tech wants to merge 3 commits into
mainfrom
claude/player-draft-room-access-sewt0x
Open

Replace one-time captain links with reusable team access codes#263
diese-tech wants to merge 3 commits into
mainfrom
claude/player-draft-room-access-sewt0x

Conversation

@diese-tech

Copy link
Copy Markdown
Owner

Why

Captains could not get into the draft room when the player draft was hosted. Three defects in the one-time link flow combined to cause it:

  1. Redemption destroyed the credential. consumeCaptainToken deleted the row on exchange. Open the link on a phone and then a laptop → dead. Worse, if the browser dropped the session cookie (Safari ITP, the in-app browsers in Discord/Slack), the token was burned with nothing to show for it and the captain was locked out permanently.
  2. Reissuing was impossible. captain_tokens has unique (draft_room_id, org_id), but the admin UI offered "Generate another link". The second insert violated the constraint and returned 500 Failed to generate access link. The all-seat path threw unhandled.
  3. One cookie, one seat. The session cookie held a single roomId|orgId pair, so joining a second room silently evicted the first, and two captains sharing a machine logged each other out.

There was also no recovery path: nothing in the draft room let a captain get back in, and admins could not re-read a link once it scrolled away.

What changed

Captains now type a short code into the draft room itself instead of following a link.

CodesH7K2-QM4X, Crockford base32 (no I/L/O/U), with lookalike folding so h7k2-qm4x and H7KZ QM4X resolve identically. Reusable and re-readable: any device, any number of times. A captain who loses their session just re-enters the same code, no admin action needed.

Captain UI — the dead-end "Spectator mode" now carries a join panel, plus a Leave control in the header for shared or borrowed devices.

Admin UI — every seat's live code is listed with copy / copy-all (the latter produces a paste-ready block with the room URL). Rotate invalidates the previous code instantly — the revocation path one-time links never actually had.

Multi-seat cookie — the seat cookie now holds a room → org map, so seats in several rooms coexist. v1 cookies still verify and upgrade to v2 on the next join, so sessions issued before this change survive.

Notes

  • No schema change. Backed by the existing captain_tokens table, whose unique (draft_room_id, org_id) already models one credential per seat. supabase/migrations/README.md forbids new shared-schema migrations in this repo; npm run check:db-contract still verifies clean.
  • Legacy ?token= links keep working until they expire, and are no longer consumed on first use.
  • Issuing uses delete-then-insert rather than upsert, so a rotation collapses any duplicate legacy rows and cannot collide with the unique constraint.
  • Auditing on the join path is non-blocking — writeAuditLog rethrows on DB error, and joining is the one draft-day path that must not fail for a reason unrelated to the captain's code.

Security trade-off

This reverses the one-time-use hardening recorded as SEC-06. A leaked code stays usable until it expires or is rotated, so a code should be treated like a password. Mitigations:

  • redemption is rate-limited per client (reusing the existing shared limiter: 10 attempts / 15 min), and every failed attempt is written to the audit log;
  • Rotate code invalidates the prior code for that seat immediately;
  • codes are scoped to a single organization in a single room, enforced server-side on the pick and shortlist APIs; and
  • Leave releases a seat from a shared device.

Worth a look from whoever owns that decision — the trade-off is documented in docs/draft-platform-guide.md, including guidance to rotate if a code is suspected to have spread beyond the intended captain and backup owner.

Verification

  • npm test — 788 passed, 26 skipped, 0 failures
  • npm run lint — 0 errors (12 warnings, all pre-existing)
  • npx tsc --noEmit — clean
  • npm run check:db-contract — verifies db-v1.17.0
  • npm run build compiles and typechecks; it then fails collecting page data for /teams/[teamId] because this container has no Supabase env. Confirmed identical on a clean main build — pre-existing, not from this change.

New coverage: access-code normalize/format/generate, the join route (reuse across devices, wrong-room codes, throttling and its reset, audit resilience), multi-seat and legacy-v1 cookie behaviour, and the code issue/redeem/list data layer.

🤖 Generated with Claude Code

https://claude.ai/code/session_014qa7rba4XGU3QuFqG1Tcjo


Generated by Claude Code

Captains could not get into the draft room on draft day. Three defects
in the one-time link flow combined to cause it:

- Redemption deleted the token (consumeCaptainToken), so a link opened
  on a second device, or in a browser that dropped the session cookie,
  was dead with nothing to show for it.
- captain_tokens has unique (draft_room_id, org_id), but the admin UI
  offered "Generate another link". The second insert violated the
  constraint and returned 500; the all-seat path threw unhandled.
- The session cookie held a single roomId|orgId pair, so joining a
  second room silently evicted the first seat.

Captains now type a short code (H7K2-QM4X) into the draft room itself:

- Codes are reusable and re-readable. Any device, any number of times;
  a captain who loses their session just re-enters the same code.
- Crockford base32 alphabet (no I/L/O/U) with lookalike folding, so
  "h7k2-qm4x" and "H7KZ QM4X" resolve the same way.
- The draft board shows a join panel instead of a dead-end "Spectator
  mode", plus a Leave control for shared devices.
- Admin panel lists every seat's live code with copy/copy-all, and
  Rotate invalidates the previous code instantly — the revocation path
  one-time links never actually had.
- The seat cookie holds a map of room -> org, so multiple seats coexist.
  v1 cookies still verify and upgrade on next join.

Backed by the existing captain_tokens table; no schema change, per
supabase/migrations/README.md. Legacy ?token= links stay redeemable
until they expire and are no longer consumed on first use.

This reverses the one-time-use hardening in SEC-06. A leaked code is
usable until rotated, mitigated by per-client rate limiting, audited
join attempts, instant rotation, and per-room/per-org scoping. The
trade-off is documented in docs/draft-platform-guide.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qa7rba4XGU3QuFqG1Tcjo
@vercel

vercel Bot commented Aug 19, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
sal-draft-league Ready Ready Preview Aug 19, 2026 7:27am

dependency-audit started failing on high severity: nanoid <3.3.18 can
loop indefinitely in custom generators when size is zero. It reaches the
tree transitively via @tailwindcss/postcss -> postcss, which this repo
already pins through overrides, so the advisory fires without any
dependency change on our side.

Adds a nanoid override alongside the existing postcss/sharp/minimatch
pins. npm audit reports 0 vulnerabilities for both the production and
full trees afterwards.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qa7rba4XGU3QuFqG1Tcjo

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 52eccdca81

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/lib/draft-data.ts Outdated
Comment on lines +349 to +354
const { error: deleteError } = await supabase
.from("captain_tokens")
.delete()
.eq("draft_room_id", draftRoomId)
.eq("org_id", orgId);
if (deleteError) throw deleteError;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the prior code when rotation insertion fails

When the delete succeeds but the subsequent insert fails—for example because of a transient database error or a generated-code primary-key collision—the team's previously valid credential has already been destroyed. The endpoint then returns 500 while the admin UI continues displaying the now-invalid old code, potentially locking the captain out during a live draft. Perform the replacement atomically, such as with a transaction or a safe upsert, so a failed rotation leaves the prior code valid.

Useful? React with 👍 / 👎.

Comment thread src/lib/captain-auth.ts
Comment on lines +164 to +166
export function getCaptainSessionFromRequest(request: NextRequest, draftRoomId: string): CaptainSession | null {
const orgId = getCaptainSeatsFromRequest(request)[draftRoomId];
return orgId ? { draftRoomId, orgId } : null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Revoke existing sessions when rotating a leaked code

If someone redeems a leaked code before an admin rotates it, the resulting signed cookie continues authorizing picks and shortlist operations for up to 30 days because session validation checks only the cookie's room/org mapping and never checks the current credential. Rotation therefore prevents only future redemption and does not remove an intruder who already joined, despite being presented as the response to suspected sharing. Include a credential version or similarly revocable value in the session and validate it against the current seat credential.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, and it means the guide I wrote overclaimed. Fixed the documentation in f27f5cd rather than the mechanism, and I want to be explicit about why.

Implementing this properly is a security-model change rather than a patch: the seat cookie needs a third payload version carrying a credential fingerprint, a policy decision on what happens to the v1/v2 sessions already in the wild (grandfather them, or force every captain to re-enter their code on deploy), and a live credential check on the authenticated routes. Worth noting the check only needs to sit on pick and shortlist — the draft board itself is public, so a stale session on the read path gains nothing a spectator does not already have. That keeps the 3-second poll free of an extra query, which matters on draft day.

I have deliberately not bundled that into this PR. It arrives on the back of a draft where captains could not get into the room at all, so the priority is the access path being reliable; adding per-request credential validation to the same change widens the blast radius of the thing meant to fix reliability. Raising it with the repo owner as follow-up.

In the meantime the guide no longer presents rotation as the answer to an active intruder. It now states that rotation stops future redemption only, and points at pausing the room with admin-made picks, or voiding and replacing it, as the actual remedy.

One scope note on the threat model: a code only reaches an attacker if a captain forwards it, and it is scoped to one org in one room. The realistic failure here is a captain sharing their code with a teammate, not a hostile takeover.


Generated by Claude Code

Two findings from review on the access-code work.

Rotation was delete-then-insert. If the delete landed and the insert
then failed, the seat was left with no credential at all and the
captain locked out mid-draft — a worse version of the bug this feature
exists to fix. Replaced with a single upsert on the seat's
unique (draft_room_id, org_id) constraint, so a failed rotation leaves
the previous code untouched and the admin can retry.

The guide also claimed rotation was the answer to a suspected leak.
Sessions are validated by signature alone and are never re-checked
against the current credential, so rotation stops future redemption but
does not evict anyone who already joined. Documented what rotation
actually does, and what to do instead when someone is genuinely in a
seat they should not have.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qa7rba4XGU3QuFqG1Tcjo

Copy link
Copy Markdown
Owner Author

e2e check is cancelled for an infrastructure reason, not a test failure

The e2e-tests job on f27f5cd shows cancelled. It ran for six hours and was killed by the GitHub job timeout without executing a single test. It stalled in the Install Playwright browsers step:

Ign:2 http://azure.archive.ubuntu.com/ubuntu noble InRelease
...
2026-08-19T07:29:49Z  Get:5 https://archive.ubuntu.com/ubuntu noble-security InRelease
2026-08-19T13:29:33Z  ##[error]The operation was canceled.
Terminate orphan process: pid (2292) (npm exec playwright install --with-deps chromium)

apt-get hung against unreachable Ubuntu mirrors inside playwright install --with-deps. Re-running the job should clear it; I do not have permission to trigger a re-run from here (403 Resource not accessible by integration).

Verified the suite locally instead

343 passed, 0 failed against this branch.

One caveat on how that was obtained, since it is not a plain npm run test:e2e: the sandbox ships Chromium build 1194 while the pinned @playwright/test wants 1234, so an unmodified run fails every test at browserType.launch: Executable doesn't exist. I pointed Playwright at the Chromium that exists via a throwaway config and re-ran. That override is local-only and is not part of this branch — nothing in the diff touches playwright.config.ts or any CI workflow.

Everything else on this commit

Check Result
build
lint-and-typecheck
unit-tests ✅ (788 passed)
integration-tests
dependency-audit
secret-scan
lighthouse
Vercel preview ✅ deployed

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants