Skip to content

fix: harden setup token exchange #254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
texuf wants to merge 1 commit into
base: main
Choose a base branch
from
+114 −44
Open

fix: harden setup token exchange #254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 57 additions & 38 deletions apps/server/src/auth/github.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Elysia, t } from "elysia";
import { jwt } from "@elysiajs/jwt";
import { eq, and } from "drizzle-orm";
import { eq, and, gt } from "drizzle-orm";
import { config } from "../config";
import type { Database } from "../db";
import { users, setupTokens, devices, apiKeys } from "../db/schema";
Expand Down Expand Up @@ -263,49 +263,68 @@ export const cliAuth = (db: Database) =>
.post(
"/exchange",
async ({ body, set }) => {
const [st] = await db
.select()
.from(setupTokens)
.where(and(eq(setupTokens.token, body.token), eq(setupTokens.redeemed, false)))
.limit(1);
const tokenHash = await hashToken(body.token);

if (!st || st.expiresAt < new Date()) {
set.status = 400;
return { error: "Invalid or expired setup token" };
}
return await db.transaction(async (tx) => {
const now = new Date();
const [st] = await tx
.select()
.from(setupTokens)
.where(and(eq(setupTokens.token, tokenHash), eq(setupTokens.redeemed, false)))
.limit(1);

await db.update(setupTokens).set({ redeemed: true }).where(eq(setupTokens.id, st.id));
if (!st || st.expiresAt < now) {
set.status = 400;
return { error: "Invalid or expired setup token" };
}

// Upsert by (userId, deviceName): a second sign-in on the same
// machine reuses the existing device row so device_repo_paths and
// sessions linked to that device survive sign-out/in cycles.
const [device] = await db
.insert(devices)
.values({
userId: st.userId,
deviceName: body.deviceName,
os: body.os ?? null,
lastSeenAt: new Date(),
})
.onConflictDoUpdate({
target: [devices.userId, devices.deviceName],
set: { os: body.os ?? null, lastSeenAt: new Date() },
})
.returning();
const [redeemed] = await tx
.update(setupTokens)
.set({ redeemed: true })
.where(
and(
eq(setupTokens.id, st.id),
eq(setupTokens.redeemed, false),
gt(setupTokens.expiresAt, now),
),
)
.returning();
if (!redeemed) {
set.status = 400;
return { error: "Invalid or expired setup token" };
}

// One active API key per device. Revoke any prior keys before issuing
// the new one so an old machine-captured key can't outlive a sign-in.
await db.delete(apiKeys).where(eq(apiKeys.deviceId, device.id));
// Upsert by (userId, deviceName): a second sign-in on the same
// machine reuses the existing device row so device_repo_paths and
// sessions linked to that device survive sign-out/in cycles.
const [device] = await tx
.insert(devices)
.values({
userId: st.userId,
deviceName: body.deviceName,
os: body.os ?? null,
lastSeenAt: now,
})
.onConflictDoUpdate({
target: [devices.userId, devices.deviceName],
set: { os: body.os ?? null, lastSeenAt: now },
})
.returning();

const rawKey = generateApiKey();
const keyHash = await hashToken(rawKey);
await db.insert(apiKeys).values({
userId: st.userId,
deviceId: device.id,
keyHash,
});
// One active API key per device. Revoke any prior keys before issuing
// the new one so an old machine-captured key can't outlive a sign-in.
await tx.delete(apiKeys).where(eq(apiKeys.deviceId, device.id));

return { apiKey: rawKey, deviceId: device.id };
const rawKey = generateApiKey();
const keyHash = await hashToken(rawKey);
await tx.insert(apiKeys).values({
userId: st.userId,
deviceId: device.id,
keyHash,
});

return { apiKey: rawKey, deviceId: device.id };
});
},
{
body: t.Object({
Expand Down
10 changes: 8 additions & 2 deletions apps/server/src/auth/tokens.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
/** Generate a random API key (UUID v4) */
/** Generate a random bearer token. */
export function generateOpaqueToken(bytes = 32): string {
const raw = crypto.getRandomValues(new Uint8Array(bytes));
return Buffer.from(raw).toString("base64url");
}

/** Generate a random API key. */
export function generateApiKey(): string {
return crypto.randomUUID();
return generateOpaqueToken();

@cursor cursor Bot May 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Changed generateApiKey output breaks existing UUID format test

Medium Severity

generateApiKey now returns a base64url-encoded random string via generateOpaqueToken(), but an existing test at line 96–99 of upload.test.ts asserts it matches a UUID v4 regex. That test will always fail because base64url output doesn't conform to the xxxxxxxx-xxxx-4xxx-… pattern. The test wasn't updated alongside the implementation change.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b519baa. Configure here.

}

/** SHA-256 hash a token string (for storing hashed keys/tokens) */
Expand Down
6 changes: 4 additions & 2 deletions apps/server/src/user/routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
sessions,
} from "../db/schema";
import { jwtAuth, apiKeyAuth } from "../auth/middleware";
import { generateOpaqueToken, hashToken } from "../auth/tokens";
import { authAudit } from "../auth/audit";
import { matchSessionRepo, normalizeFullName } from "../social/github-sync";
import { __clearClaimCaches } from "./claim";
Expand Down Expand Up @@ -73,12 +74,13 @@ export const userRoutes = (db: Database) =>

// POST /api/me/setup-token — generate a new setup token
.post("/setup-token", async ({ user }) => {
const token = crypto.randomUUID();
const token = generateOpaqueToken();
const expiresAt = new Date(Date.now() + 10 * 60 * 1000);
const tokenHash = await hashToken(token);

await db.insert(setupTokens).values({
userId: user.id,
token,
token: tokenHash,
expiresAt,
});

Expand Down
47 changes: 45 additions & 2 deletions apps/server/test/upload.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
describe("token utilities", () => {
it("generateApiKey returns UUID format", () => {
const key = generateApiKey();
expect(key).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i);

Check failure on line 98 in apps/server/test/upload.test.ts

View workflow job for this annotation

GitHub Actions / test

error: expect(received).toMatch(expected) 

Expected substring or pattern: /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i
Received: "16jGWqsXVtiSk7cQEC3MCm0Eby90snRJ-9SirSN3N8E"

      at <anonymous> (/home/runner/work/slashtalk/slashtalk/apps/server/test/upload.test.ts:98:17)
});

it("hashToken produces deterministic 64-char hex", async () => {
Expand Down Expand Up @@ -192,11 +192,34 @@
// ── Setup Token Exchange ─────────────────────────────────────

describe("setup token exchange", () => {
it("stores setup tokens hashed at rest", async () => {
const setupRes = await fetch(`${baseUrl}/api/me/setup-token`, {
method: "POST",
headers: { Cookie: aliceCookie },
});
expect(setupRes.status).toBe(200);
const { token } = (await setupRes.json()) as { token: string };

const [rawMatch] = await db
.select()
.from(setupTokens)
.where(eq(setupTokens.token, token))
.limit(1);
expect(rawMatch).toBeUndefined();

const [hashMatch] = await db
.select()
.from(setupTokens)
.where(eq(setupTokens.token, await hashToken(token)))
.limit(1);
expect(hashMatch).toBeTruthy();
});

it("rejects expired setup token", async () => {
// Insert an expired token directly
await db.insert(setupTokens).values({
userId: aliceUserId,
token: "expired-token-test",
token: await hashToken("expired-token-test"),
expiresAt: new Date(Date.now() - 60_000), // 1 min ago
redeemed: false,
});
Expand All @@ -217,7 +240,7 @@
it("rejects already-redeemed setup token", async () => {
await db.insert(setupTokens).values({
userId: aliceUserId,
token: "redeemed-token-test",
token: await hashToken("redeemed-token-test"),
expiresAt: new Date(Date.now() + 600_000),
redeemed: true,
});
Expand All @@ -232,6 +255,26 @@
});
expect(res.status).toBe(400);
});

it("lets only one concurrent setup-token exchange succeed", async () => {
const setupRes = await fetch(`${baseUrl}/api/me/setup-token`, {
method: "POST",
headers: { Cookie: aliceCookie },
});
expect(setupRes.status).toBe(200);
const { token } = (await setupRes.json()) as { token: string };

const exchange = (deviceName: string) =>
fetch(`${baseUrl}/v1/auth/exchange`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ token, deviceName }),
});

const results = await Promise.all([exchange("race-device-a"), exchange("race-device-b")]);
expect(results.map((res) => res.status).sort()).toEqual([200, 400]);
await Promise.all(results.map((res) => res.text()));
});
});

// ── NDJSON Ingest ────────────────────────────────────────────
Expand Down
Loading