Skip to content

Commit 9c93c05

Browse files
committed
feat(auth): support IdP-initiated SAML and harden the SSO completion flow
Follow-up to the SP-initiated SAML fix (#405), three changes: - IdP-initiated SSO: an Okta dashboard-tile launch posts an assertion with no RelayState. Rather than rejecting it, the ACS now picks the enabled SAML config whose certificate validates the assertion's signature (the stored issuer is the SP entity id, not a reliable IdP identifier) and defaults the post-login destination to /. - Enforce 2FA for SAML logins: the completion route now reads force2FAAllLogins and stamps the same twoFactorRequired / twoFactorSetupRequired claims the NextAuth jwt callback sets at sign-in, so SAML sessions hit the existing middleware 2FA gate instead of bypassing it. - Single-use completion token: the token handed to /api/auth/saml/complete rides in a redirect URL, so it now registers a single-use jti in Valkey (deleted on consume) and its lifetime is cut to two minutes — a replayed link is rejected.
1 parent 210f5b1 commit 9c93c05

6 files changed

Lines changed: 301 additions & 55 deletions

File tree

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

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ vi.mock("~/lib/valkey", () => ({ default: null })); // RelayState via signed tok
1313

1414
vi.mock("~/server/db", () => ({
1515
db: {
16-
samlConfiguration: { findUnique: vi.fn() },
16+
samlConfiguration: { findUnique: vi.fn(), findMany: vi.fn() },
1717
user: { findUnique: vi.fn(), update: vi.fn(), create: vi.fn() },
1818
account: { upsert: vi.fn() },
1919
roles: { findFirst: vi.fn() },
@@ -59,9 +59,10 @@ function relayFor(providerId: string, callbackUrl = "/dash") {
5959
describe("POST /api/auth/callback/saml — ACS validator", () => {
6060
beforeEach(() => {
6161
vi.clearAllMocks();
62+
(db.samlConfiguration.findMany as any).mockResolvedValue([]);
6263
});
6364

64-
it("returns 400 when RelayState is missing or invalid (Bug 4 regression)", async () => {
65+
it("returns 400 when there is no RelayState and no enabled config validates it", async () => {
6566
const res = await POST(makeReq(null));
6667
expect(res.status).toBe(400);
6768
const body = await res.json();
@@ -115,4 +116,46 @@ describe("POST /api/auth/callback/saml — ACS validator", () => {
115116
const res = await POST(makeReq(relayFor("nope")));
116117
expect(res.status).toBe(404);
117118
});
119+
120+
it("supports IdP-initiated login (no RelayState) via the config that validates", async () => {
121+
(db.samlConfiguration.findMany as any).mockResolvedValue([
122+
{
123+
id: "cfg",
124+
entryPoint: "e",
125+
cert: "c",
126+
issuer: "i",
127+
attributeMapping: {},
128+
autoProvisionUsers: false,
129+
provider: { name: "okta", enabled: true },
130+
},
131+
]);
132+
validateSAMLResponse.mockResolvedValue({
133+
email: "carol@example.com",
134+
nameID: "carol",
135+
});
136+
(db.user.findUnique as any).mockResolvedValue({
137+
id: "user_8",
138+
email: "carol@example.com",
139+
name: "Carol",
140+
authMethod: "SSO",
141+
externalId: "carol",
142+
});
143+
(db.account.upsert as any).mockResolvedValue({});
144+
145+
const res = await POST(makeReq(null)); // no RelayState = IdP-initiated
146+
147+
// Picked the config by validating the assertion (findMany over enabled
148+
// providers), not by a RelayState-carried id.
149+
expect(db.samlConfiguration.findMany).toHaveBeenCalled();
150+
expect(db.samlConfiguration.findUnique).not.toHaveBeenCalled();
151+
152+
const location = res.headers.get("location")!;
153+
expect(
154+
location.startsWith(
155+
"https://app.example.com/api/auth/saml/complete?token="
156+
)
157+
).toBe(true);
158+
// Post-login destination defaults to /.
159+
expect(location).toContain("callbackUrl=%2F");
160+
});
118161
});

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

Lines changed: 82 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,43 @@ import { isEmailDomainAllowed } from "~/lib/utils/email-domain-validation";
1414
import { db } from "~/server/db";
1515
import { createSAMLClient, validateSAMLResponse } from "~/server/saml-provider";
1616

17+
/**
18+
* Resolve an IdP-initiated SAMLResponse (no RelayState — e.g. an Okta dashboard
19+
* tile) to the enabled SAML config whose IdP signed it. The stored issuer field
20+
* is the SP entity id, not a reliable IdP identifier, so we let cryptographic
21+
* validation pick the config: the assertion only validates against the cert of
22+
* the IdP that actually issued it. Returns null if none validate it.
23+
*/
24+
async function resolveIdpInitiatedConfig(samlResponse: FormDataEntryValue) {
25+
const configs = await db.samlConfiguration.findMany({
26+
where: { provider: { enabled: true } },
27+
include: { provider: true },
28+
});
29+
30+
for (const samlConfig of configs) {
31+
try {
32+
const samlClient = await createSAMLClient({
33+
name: samlConfig.provider.name,
34+
entryPoint: samlConfig.entryPoint,
35+
cert: samlConfig.cert,
36+
issuer: samlConfig.issuer,
37+
});
38+
const profile = await validateSAMLResponse(samlClient, {
39+
SAMLResponse: samlResponse,
40+
});
41+
return { samlConfig, profile };
42+
} catch {
43+
// Not this IdP (or the assertion is invalid) — try the next config.
44+
}
45+
}
46+
47+
return null;
48+
}
49+
50+
type ResolvedSaml = NonNullable<
51+
Awaited<ReturnType<typeof resolveIdpInitiatedConfig>>
52+
>;
53+
1754
/**
1855
* SAML Assertion Consumer Service (ACS).
1956
*
@@ -54,48 +91,57 @@ export async function POST(request: NextRequest) {
5491
);
5592
}
5693

57-
// Recover the provider and destination from RelayState (the IdP echoes it
58-
// back here; same-site cookies are not sent on this cross-site POST). The
59-
// token is single-use and short-lived, which is what guards this endpoint.
94+
// Recover the provider and destination from RelayState. The IdP echoes it
95+
// back on this cross-site POST (where same-site cookies are not sent); the
96+
// token is single-use and short-lived, which guards SP-initiated logins.
6097
const relay = await consumeSamlRelayState(
6198
typeof relayState === "string" ? relayState : null
6299
);
63100

64-
if (!relay) {
65-
return NextResponse.json(
66-
{ error: "Invalid or expired SAML request" },
67-
{ status: 400 }
68-
);
69-
}
70-
71-
const providerId = relay.providerId;
72-
const callbackUrl = sanitizeCallbackUrl(relay.callbackUrl);
101+
let samlConfig: ResolvedSaml["samlConfig"];
102+
let profile: ResolvedSaml["profile"];
103+
let callbackUrl = "/";
73104

74-
// Fetch SAML configuration. RelayState carries the SsoProvider id, which is
75-
// the unique foreign key on SamlConfiguration (not its own id).
76-
const samlConfig = await db.samlConfiguration.findUnique({
77-
where: { providerId },
78-
include: { provider: true },
79-
});
105+
if (relay) {
106+
// SP-initiated: RelayState carries the SsoProvider id, which is the unique
107+
// foreign key on SamlConfiguration (not its own id).
108+
callbackUrl = sanitizeCallbackUrl(relay.callbackUrl);
109+
const found = await db.samlConfiguration.findUnique({
110+
where: { providerId: relay.providerId },
111+
include: { provider: true },
112+
});
80113

81-
if (!samlConfig || !samlConfig.provider.enabled) {
82-
return NextResponse.json(
83-
{ error: "SAML provider not found or disabled" },
84-
{ status: 404 }
85-
);
86-
}
114+
if (!found || !found.provider.enabled) {
115+
return NextResponse.json(
116+
{ error: "SAML provider not found or disabled" },
117+
{ status: 404 }
118+
);
119+
}
87120

88-
// Create SAML client and validate response
89-
const samlClient = await createSAMLClient({
90-
name: samlConfig.provider.name,
91-
entryPoint: samlConfig.entryPoint,
92-
cert: samlConfig.cert,
93-
issuer: samlConfig.issuer,
94-
});
121+
const samlClient = await createSAMLClient({
122+
name: found.provider.name,
123+
entryPoint: found.entryPoint,
124+
cert: found.cert,
125+
issuer: found.issuer,
126+
});
95127

96-
const profile = await validateSAMLResponse(samlClient, {
97-
SAMLResponse: samlResponse,
98-
});
128+
samlConfig = found;
129+
profile = await validateSAMLResponse(samlClient, {
130+
SAMLResponse: samlResponse,
131+
});
132+
} else {
133+
// IdP-initiated (no RelayState — e.g. an Okta dashboard tile): pick the
134+
// enabled config whose cert validates this assertion's signature.
135+
const resolved = await resolveIdpInitiatedConfig(samlResponse);
136+
if (!resolved) {
137+
return NextResponse.json(
138+
{ error: "Invalid or expired SAML request" },
139+
{ status: 400 }
140+
);
141+
}
142+
samlConfig = resolved.samlConfig;
143+
profile = resolved.profile;
144+
}
99145

100146
// Validate SAML response timestamps if available
101147
if (profile.notBefore || profile.notOnOrAfter) {
@@ -269,8 +315,8 @@ export async function POST(request: NextRequest) {
269315
},
270316
});
271317

272-
// Create a temporary session token for secure user info transfer
273-
const tempToken = createTempSessionToken({
318+
// Create a one-time session token for secure user info transfer.
319+
const tempToken = await createTempSessionToken({
274320
userId: user.id,
275321
provider: `saml-${samlConfig.provider.name}`,
276322
email: user.email,

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

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ vi.mock("next/headers", () => ({
2121
}));
2222

2323
vi.mock("~/server/db", () => ({
24-
db: { user: { findUnique: vi.fn() } },
24+
db: {
25+
user: { findUnique: vi.fn() },
26+
registrationSettings: { findFirst: vi.fn() },
27+
},
2528
}));
2629

2730
import { db } from "~/server/db";
@@ -35,6 +38,7 @@ const user = {
3538
isApi: false,
3639
passwordChangedAt: null,
3740
mustChangePassword: false,
41+
twoFactorEnabled: false,
3842
};
3943

4044
function makeReq(token: string, callbackUrl = "/dashboard") {
@@ -58,6 +62,7 @@ describe("GET /api/auth/saml/complete — post-Okta session handoff", () => {
5862
cookieJar.clear();
5963
vi.clearAllMocks();
6064
(db.user.findUnique as any).mockResolvedValue(user);
65+
(db.registrationSettings.findFirst as any).mockResolvedValue(null);
6166
});
6267

6368
afterEach(() => {
@@ -99,6 +104,40 @@ describe("GET /api/auth/saml/complete — post-Okta session handoff", () => {
99104
).toBeTruthy();
100105
});
101106

107+
it("stamps 2FA-required when force2FAAllLogins is on and the user has 2FA", async () => {
108+
(db.registrationSettings.findFirst as any).mockResolvedValue({
109+
force2FAAllLogins: true,
110+
});
111+
(db.user.findUnique as any).mockResolvedValue({
112+
...user,
113+
twoFactorEnabled: true,
114+
});
115+
116+
await GET(makeReq(tempToken()));
117+
118+
const decoded = await decode({
119+
token: cookieJar.get("next-auth.session-token")!.value,
120+
secret: SECRET,
121+
});
122+
expect(decoded?.twoFactorRequired).toBe(true);
123+
expect(decoded?.twoFactorVerified).toBe(false);
124+
});
125+
126+
it("stamps 2FA-setup-required when force2FA is on but the user has no 2FA", async () => {
127+
(db.registrationSettings.findFirst as any).mockResolvedValue({
128+
force2FAAllLogins: true,
129+
});
130+
// user.twoFactorEnabled is false by default
131+
132+
await GET(makeReq(tempToken()));
133+
134+
const decoded = await decode({
135+
token: cookieJar.get("next-auth.session-token")!.value,
136+
secret: SECRET,
137+
});
138+
expect(decoded?.twoFactorSetupRequired).toBe(true);
139+
});
140+
102141
it("rejects an invalid/expired token with 401", async () => {
103142
const res = await GET(makeReq("not-a-valid-jwt"));
104143
expect(res.status).toBe(401);

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

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
import jwt from "jsonwebtoken";
21
import { encode } from "next-auth/jwt";
32
import { cookies } from "next/headers";
43
import { NextRequest, NextResponse } from "next/server";
5-
import { getAppBaseUrl } from "~/lib/auth-security";
4+
import { consumeTempSessionToken, getAppBaseUrl } from "~/lib/auth-security";
65
import { db } from "~/server/db";
76

87
// SAML completion handler - creates NextAuth session
@@ -16,14 +15,10 @@ export async function GET(request: NextRequest) {
1615
return NextResponse.json({ error: "Token is required" }, { status: 400 });
1716
}
1817

19-
// Verify the JWT token
20-
let tokenData: any;
21-
try {
22-
tokenData = jwt.verify(
23-
token,
24-
process.env.NEXTAUTH_SECRET || "development-secret"
25-
);
26-
} catch {
18+
// Verify and consume the one-time token (single-use via Valkey when
19+
// present, so the token in the redirect URL can't be replayed).
20+
const tokenData = await consumeTempSessionToken(token);
21+
if (!tokenData) {
2722
return NextResponse.json(
2823
{ error: "Invalid or expired token" },
2924
{ status: 401 }
@@ -41,13 +36,34 @@ export async function GET(request: NextRequest) {
4136
isApi: true,
4237
passwordChangedAt: true,
4338
mustChangePassword: true,
39+
twoFactorEnabled: true,
4440
},
4541
});
4642

4743
if (!user) {
4844
return NextResponse.json({ error: "User not found" }, { status: 404 });
4945
}
5046

47+
// Mirror NextAuth's force-2FA-for-SSO gate (the jwt callback in server/auth):
48+
// this flow mints the session directly, so without stamping the same flags
49+
// here a SAML login would bypass an enforced 2FA policy.
50+
const twoFactorClaims: {
51+
twoFactorRequired?: boolean;
52+
twoFactorVerified?: boolean;
53+
twoFactorSetupRequired?: boolean;
54+
} = {};
55+
const registrationSettings = await db.registrationSettings.findFirst({
56+
select: { force2FAAllLogins: true },
57+
});
58+
if (registrationSettings?.force2FAAllLogins) {
59+
if (user.twoFactorEnabled) {
60+
twoFactorClaims.twoFactorRequired = true;
61+
twoFactorClaims.twoFactorVerified = false;
62+
} else {
63+
twoFactorClaims.twoFactorSetupRequired = true;
64+
}
65+
}
66+
5167
// Create NextAuth JWT session token. Seed the same fields the NextAuth `jwt`
5268
// callback sets at sign-in so middleware (which only decodes the token, and
5369
// never runs the callback) sees the user's access on the very first request.
@@ -61,6 +77,7 @@ export async function GET(request: NextRequest) {
6177
isApi: user.isApi,
6278
passwordChangedAt: user.passwordChangedAt?.toISOString() ?? null,
6379
mustChangePassword: user.mustChangePassword ?? false,
80+
...twoFactorClaims,
6481
},
6582
secret: process.env.NEXTAUTH_SECRET || "development-secret",
6683
});

0 commit comments

Comments
 (0)