Skip to content

Commit ab75960

Browse files
authored
fix(auth): reject replayed SAML assertions (#407)
Accepting IdP-initiated (no-RelayState) POSTs in #406 opened a replay vector: any captured assertion — including an SP-initiated one with its RelayState stripped — could be re-POSTed and accepted until it expired, since the only remaining bound was its notBefore/notOnOrAfter window. The ACS now records each validated assertion by its signed ID in Valkey for the remainder of its validity window and rejects a second use — the same single-use pattern as the relay state and completion token. The ID is the dedup key (not a byte hash) because it lives inside the signed element: an attacker can't change it without the IdP's key, whereas the surrounding bytes can be mutated (re-canonicalization, namespace/attribute reordering, the unsigned Response wrapper) while keeping a valid signature. Without Valkey the assertion's expiry window remains the only bound.
1 parent 94aa3d8 commit ab75960

4 files changed

Lines changed: 146 additions & 0 deletions

File tree

testplanit/app/api/auth/callback/saml/route.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ vi.mock("~/lib/utils/email-domain-validation", () => ({
3434
isEmailDomainAllowed: vi.fn(async () => true),
3535
}));
3636

37+
// Override only the assertion replay guard; everything else stays real.
38+
const { registerSamlAssertion } = vi.hoisted(() => ({
39+
registerSamlAssertion: vi.fn(async () => true),
40+
}));
41+
vi.mock("~/lib/auth-security", async (importActual) => {
42+
const actual = await importActual<typeof import("~/lib/auth-security")>();
43+
return { ...actual, registerSamlAssertion };
44+
});
45+
3746
import { db } from "~/server/db";
3847
import { POST } from "./route";
3948

@@ -111,6 +120,29 @@ describe("POST /api/auth/callback/saml — ACS validator", () => {
111120
expect(location).not.toContain("/api/auth/callback/saml?token=");
112121
});
113122

123+
it("rejects a replayed assertion with 400 (single-use)", async () => {
124+
(db.samlConfiguration.findUnique as any).mockResolvedValue({
125+
id: "cfg",
126+
entryPoint: "e",
127+
cert: "c",
128+
issuer: "i",
129+
attributeMapping: {},
130+
autoProvisionUsers: false,
131+
provider: { name: "okta", enabled: true },
132+
});
133+
validateSAMLResponse.mockResolvedValue({
134+
email: "bob@example.com",
135+
nameID: "bob",
136+
});
137+
registerSamlAssertion.mockResolvedValueOnce(false); // already seen
138+
139+
const res = await POST(makeReq(relayFor("ssoprovider_9")));
140+
141+
expect(res.status).toBe(400);
142+
const body = await res.json();
143+
expect(body.error).toMatch(/already been used/i);
144+
});
145+
114146
it("returns 404 when the provider in RelayState is unknown", async () => {
115147
(db.samlConfiguration.findUnique as any).mockResolvedValue(null);
116148
const res = await POST(makeReq(relayFor("nope")));

testplanit/app/api/auth/callback/saml/route.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
createTempSessionToken,
77
getAppBaseUrl,
88
getSecurityHeaders,
9+
registerSamlAssertion,
910
sanitizeCallbackUrl,
1011
validateSAMLTimestamp,
1112
} from "~/lib/auth-security";
@@ -158,6 +159,41 @@ export async function POST(request: NextRequest) {
158159
}
159160
}
160161

162+
// Reject replayed assertions: a validated assertion is single-use within its
163+
// validity window. This closes the replay vector opened by accepting
164+
// IdP-initiated (no-RelayState) POSTs — without it any captured assertion
165+
// could be re-POSTed until it expires.
166+
const notOnOrAfterMs = profile.notOnOrAfter
167+
? new Date(profile.notOnOrAfter as string).getTime()
168+
: 0;
169+
const assertionTtlSeconds =
170+
notOnOrAfterMs > Date.now()
171+
? (notOnOrAfterMs - Date.now()) / 1000 + 60
172+
: 300;
173+
// Dedup on the signed assertion ID — the replay-stable anchor (it's inside
174+
// the signed element, unlike the surrounding bytes). Fall back to the
175+
// validated assertion XML, then the raw response, only if the ID is absent.
176+
const getAssertionXml = (
177+
profile as unknown as { getAssertionXml?: () => string }
178+
).getAssertionXml;
179+
const assertionXml = getAssertionXml ? getAssertionXml() : "";
180+
const assertionId =
181+
assertionXml.match(
182+
/<(?:\w+:)?Assertion\b[^>]*\bID=["']([^"']+)["']/
183+
)?.[1] ||
184+
assertionXml ||
185+
(samlResponse as string);
186+
const isFreshAssertion = await registerSamlAssertion(
187+
assertionId,
188+
assertionTtlSeconds
189+
);
190+
if (!isFreshAssertion) {
191+
return NextResponse.json(
192+
{ error: "SAML response has already been used" },
193+
{ status: 400 }
194+
);
195+
}
196+
161197
// Extract user attributes based on mapping
162198
const attributeMapping = samlConfig.attributeMapping as any;
163199
const email = profile[attributeMapping.email || "email"] as string;

testplanit/lib/auth-security.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,53 @@ describe("createSamlRelayState / consumeSamlRelayState", () => {
458458
});
459459
});
460460

461+
describe("registerSamlAssertion (single-use)", () => {
462+
const SECRET = "test-secret-key-at-least-32-chars-long";
463+
464+
it("Valkey-backed: first use returns true, replay returns false", async () => {
465+
const store = new Map<string, string>();
466+
const fakeRedis = {
467+
set: vi.fn(
468+
async (
469+
k: string,
470+
_v: string,
471+
_ex: string,
472+
_ttl: number,
473+
nx?: string
474+
) => {
475+
if (nx === "NX" && store.has(k)) return null;
476+
store.set(k, "1");
477+
return "OK";
478+
}
479+
),
480+
};
481+
482+
vi.resetModules();
483+
process.env.NEXTAUTH_SECRET = SECRET;
484+
vi.doMock("./valkey", () => ({ default: fakeRedis }));
485+
const { registerSamlAssertion } = await import("./auth-security");
486+
487+
expect(await registerSamlAssertion("_assertion-id-1", 300)).toBe(true);
488+
expect(await registerSamlAssertion("_assertion-id-1", 300)).toBe(false);
489+
// A different assertion is tracked independently.
490+
expect(await registerSamlAssertion("_assertion-id-2", 300)).toBe(true);
491+
492+
vi.doUnmock("./valkey");
493+
});
494+
495+
it("without Valkey: returns true (no cross-pod dedup)", async () => {
496+
vi.resetModules();
497+
process.env.NEXTAUTH_SECRET = SECRET;
498+
vi.doMock("./valkey", () => ({ default: null }));
499+
const { registerSamlAssertion } = await import("./auth-security");
500+
501+
expect(await registerSamlAssertion("_assertion-id-3", 300)).toBe(true);
502+
expect(await registerSamlAssertion("_assertion-id-3", 300)).toBe(true);
503+
504+
vi.doUnmock("./valkey");
505+
});
506+
});
507+
461508
describe("getSecureCookieOptions", () => {
462509
it("should return httpOnly as true", () => {
463510
const options = getSecureCookieOptions();

testplanit/lib/auth-security.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,37 @@ export async function consumeSamlRelayState(
222222
}
223223
}
224224

225+
const SAML_ASSERTION_PREFIX = "saml:assertion:";
226+
227+
/**
228+
* Enforce one-time use of a validated SAML assertion. The IdP-initiated ACS
229+
* accepts assertions with no RelayState, so without this an attacker could
230+
* replay a captured assertion (SP- or IdP-issued) within its validity window.
231+
*
232+
* The caller passes the signed assertion's ID — the replay-stable anchor. The
233+
* ID lives inside the signed element, so an attacker can't change it without
234+
* the IdP's key; the surrounding bytes can be mutated (re-canonicalization,
235+
* namespace/attribute reordering, the unsigned Response wrapper) while keeping
236+
* a valid signature, which is why a byte hash is not a safe dedup key. The ID
237+
* is hashed here only to bound the Valkey key length/charset. Returns true on
238+
* first use, false if already seen; needs Valkey for the cross-pod check
239+
* (without it the assertion's expiry window is the only bound).
240+
*/
241+
export async function registerSamlAssertion(
242+
assertionId: string,
243+
ttlSeconds: number
244+
): Promise<boolean> {
245+
if (!valkeyConnection) return true;
246+
const result = await valkeyConnection.set(
247+
`${SAML_ASSERTION_PREFIX}${hashData(assertionId)}`,
248+
"1",
249+
"EX",
250+
Math.max(1, Math.ceil(ttlSeconds)),
251+
"NX"
252+
);
253+
return result !== null;
254+
}
255+
225256
// Rate limiting configuration
226257
export interface RateLimitConfig {
227258
windowMs: number;

0 commit comments

Comments
 (0)