Skip to content

Commit 2666bfe

Browse files
committed
fix(api): reject malformed OAuth state instead of treating it as empty
consume() returned {} when the stored Valkey value failed JSON.parse or was not an object — indistinguishable from a legitimately stored empty state. Corrupted state now routes through the same null rejection path as absent/forged state. Adds the missing oauth.state.test.ts covering round-trip, replay burn, garbage, scalar, and unknown-field stripping. Audit: F003
1 parent bf69ffa commit 2666bfe

2 files changed

Lines changed: 88 additions & 3 deletions

File tree

apps/api/src/lib/oauth/oauth.state.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ class OAuthStateStore {
6868

6969
/**
7070
* Read + delete the stored state. Returns `null` if absent (expired,
71-
* forged, or already consumed). Read-and-delete makes replay
71+
* forged, or already consumed) — and equally if the stored value is
72+
* not a JSON object: corrupted state must fail the flow, not pass as
73+
* a valid state with no extras. Read-and-delete makes replay
7274
* impossible.
7375
*/
7476
async consume(state: string): Promise<IStoredState | null> {
@@ -83,7 +85,7 @@ class OAuthStateStore {
8385
const parsed: unknown = JSON.parse(raw);
8486

8587
if (parsed === null || typeof parsed !== "object") {
86-
return {};
88+
return null;
8789
}
8890

8991
const result: IStoredState = {};
@@ -98,7 +100,7 @@ class OAuthStateStore {
98100

99101
return result;
100102
} catch {
101-
return {};
103+
return null;
102104
}
103105
}
104106
}
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { afterAll, describe, expect, test } from "bun:test";
2+
import { Redis } from "ioredis";
3+
4+
import { getValkeyAppClientOptions } from "../../../src/clients/valkey";
5+
import { OAUTH_STATE_PREFIX } from "../../../src/lib/oauth/oauth.constants";
6+
import { oauthStateStore } from "../../../src/lib/oauth/oauth.state";
7+
8+
/*
9+
* Seeds raw values under the OAuth state prefix to exercise consume()'s
10+
* handling of corrupted store contents — something store() can never
11+
* produce, so it needs a direct client.
12+
*/
13+
const seedClient = new Redis(
14+
getValkeyAppClientOptions({ connectTimeout: 500 })
15+
);
16+
17+
const SEED_TTL_SECONDS = 30;
18+
19+
afterAll(async () => {
20+
try {
21+
await seedClient.quit();
22+
} catch {
23+
seedClient.disconnect();
24+
}
25+
26+
await oauthStateStore.close();
27+
});
28+
29+
describe("oauthStateStore.consume", () => {
30+
test("round-trips a stored state and burns it on first read", async () => {
31+
await oauthStateStore.store("state-roundtrip", { codeVerifier: "v" });
32+
33+
const first = await oauthStateStore.consume("state-roundtrip");
34+
35+
expect(first).toEqual({ codeVerifier: "v" });
36+
37+
const replay = await oauthStateStore.consume("state-roundtrip");
38+
39+
expect(replay).toBeNull();
40+
});
41+
42+
test("returns null for an unknown state", async () => {
43+
const got = await oauthStateStore.consume("state-never-stored");
44+
45+
expect(got).toBeNull();
46+
});
47+
48+
test("returns null when the stored value is not JSON", async () => {
49+
await seedClient.setex(
50+
`${OAUTH_STATE_PREFIX}state-garbage`,
51+
SEED_TTL_SECONDS,
52+
"not-json{"
53+
);
54+
55+
const got = await oauthStateStore.consume("state-garbage");
56+
57+
expect(got).toBeNull();
58+
});
59+
60+
test("returns null when the stored value is a JSON scalar", async () => {
61+
await seedClient.setex(
62+
`${OAUTH_STATE_PREFIX}state-scalar`,
63+
SEED_TTL_SECONDS,
64+
'"just-a-string"'
65+
);
66+
67+
const got = await oauthStateStore.consume("state-scalar");
68+
69+
expect(got).toBeNull();
70+
});
71+
72+
test("strips unknown fields from a stored object", async () => {
73+
await seedClient.setex(
74+
`${OAUTH_STATE_PREFIX}state-extra-fields`,
75+
SEED_TTL_SECONDS,
76+
JSON.stringify({ codeVerifier: "v", role: "superadmin" })
77+
);
78+
79+
const got = await oauthStateStore.consume("state-extra-fields");
80+
81+
expect(got).toEqual({ codeVerifier: "v" });
82+
});
83+
});

0 commit comments

Comments
 (0)