Skip to content

Commit 6354940

Browse files
committed
fix: handle sendLoginname failures in OIDC re-auth flow
When a session has expired during the OIDC login callback, loginWithOIDCAndSession calls sendLoginname to redirect the user to re-authenticate. sendLoginname can throw (e.g. "Could not get domain", "Could not find identity provider", or a propagated ConnectError), and that call was not wrapped in a try/catch, unlike the createCallback call just below it. The unhandled rejection propagated out of the route handler, where Next.js redacts server error messages in production, so Sentry only ever saw a bare digest with no context. Wrap the call in try/catch, matching the same pattern already used in apps/login/src/app/login/route.ts, and report the real error to Sentry with tags/extra so future alerts carry useful context. The console.error line intentionally logs only the error message and correlating IDs (authRequest, sessionId), not the full error object, to keep pod logs terse; the full error still reaches Sentry via captureException. Add regression tests covering: the failure no longer throws, the failure is reported to Sentry, and the existing redirect-on-success behavior is unchanged.
1 parent b1fc0ff commit 6354940

2 files changed

Lines changed: 113 additions & 12 deletions

File tree

apps/login/src/lib/oidc.test.ts

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import { Session } from "@zitadel/proto/zitadel/session/v2/session_pb";
2+
import { NextRequest } from "next/server";
3+
import { beforeEach, describe, expect, test, vi } from "vitest";
4+
5+
vi.mock("@/lib/server/loginname", () => ({
6+
sendLoginname: vi.fn(),
7+
}));
8+
vi.mock("@/lib/zitadel", () => ({
9+
createCallback: vi.fn(),
10+
getLoginSettings: vi.fn(),
11+
listAuthenticationMethodTypes: vi.fn(),
12+
}));
13+
vi.mock("@sentry/nextjs", () => ({
14+
captureException: vi.fn(),
15+
captureMessage: vi.fn(),
16+
}));
17+
vi.mock("./session", () => ({
18+
isSessionValid: vi.fn(),
19+
}));
20+
vi.mock("./verify-helper", () => ({
21+
checkMFAFactors: vi.fn(),
22+
}));
23+
24+
import { sendLoginname } from "@/lib/server/loginname";
25+
import * as Sentry from "@sentry/nextjs";
26+
import { loginWithOIDCAndSession } from "./oidc";
27+
import { isSessionValid } from "./session";
28+
29+
const expiredSession = {
30+
id: "session-1",
31+
factors: {
32+
user: {
33+
id: "user-1",
34+
loginName: "user@example.com",
35+
organizationId: "org-1",
36+
},
37+
// no intent factor: user did not authenticate via an IDP
38+
},
39+
} as unknown as Session;
40+
41+
const baseArgs = {
42+
serviceUrl: "https://zitadel.example.com",
43+
authRequest: "V2_authrequest-1",
44+
sessionId: "session-1",
45+
sessions: [expiredSession],
46+
sessionCookies: [],
47+
request: new NextRequest("https://auth.example.com/oidc/login"),
48+
};
49+
50+
describe("loginWithOIDCAndSession", () => {
51+
beforeEach(() => {
52+
vi.mocked(isSessionValid).mockResolvedValue(false);
53+
vi.mocked(sendLoginname).mockReset();
54+
vi.mocked(Sentry.captureException).mockReset();
55+
});
56+
57+
test("does not throw when sendLoginname rejects for an expired session", async () => {
58+
vi.mocked(sendLoginname).mockRejectedValue(
59+
new Error("Could not get domain"),
60+
);
61+
62+
await expect(loginWithOIDCAndSession(baseArgs)).resolves.not.toThrow();
63+
});
64+
65+
test("reports the sendLoginname failure to Sentry instead of swallowing it silently", async () => {
66+
const error = new Error("Could not get domain");
67+
vi.mocked(sendLoginname).mockRejectedValue(error);
68+
69+
await loginWithOIDCAndSession(baseArgs);
70+
71+
expect(Sentry.captureException).toHaveBeenCalledWith(
72+
error,
73+
expect.objectContaining({
74+
tags: { flow: "oidc", stage: "reauth_sendLoginname" },
75+
}),
76+
);
77+
});
78+
79+
test("still redirects when sendLoginname resolves with a redirect", async () => {
80+
vi.mocked(sendLoginname).mockResolvedValue({
81+
redirect: "/loginname?requestId=oidc_V2_authrequest-1",
82+
});
83+
84+
const res = await loginWithOIDCAndSession(baseArgs);
85+
86+
expect(res?.status).toBe(307);
87+
expect(res?.headers.get("location")).toContain("/loginname");
88+
expect(Sentry.captureException).not.toHaveBeenCalled();
89+
});
90+
});

apps/login/src/lib/oidc.ts

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
getLoginSettings,
66
listAuthenticationMethodTypes,
77
} from "@/lib/zitadel";
8+
import * as Sentry from "@sentry/nextjs";
89
import { create } from "@zitadel/client";
910
import {
1011
CreateCallbackRequestSchema,
@@ -86,19 +87,29 @@ export async function loginWithOIDCAndSession({
8687
requestId: `oidc_${authRequest}`,
8788
};
8889

89-
const res = await sendLoginname(command);
90-
91-
if (res && "redirect" in res && res?.redirect) {
92-
// Check if the redirect URL is already a full URL
93-
if (
94-
res.redirect.startsWith("http://") ||
95-
res.redirect.startsWith("https://")
96-
) {
97-
return NextResponse.redirect(res.redirect);
98-
} else {
99-
const absoluteUrl = constructUrl(request, res.redirect);
100-
return NextResponse.redirect(absoluteUrl.toString());
90+
try {
91+
const res = await sendLoginname(command);
92+
93+
if (res && "redirect" in res && res?.redirect) {
94+
// Check if the redirect URL is already a full URL
95+
if (
96+
res.redirect.startsWith("http://") ||
97+
res.redirect.startsWith("https://")
98+
) {
99+
return NextResponse.redirect(res.redirect);
100+
} else {
101+
const absoluteUrl = constructUrl(request, res.redirect);
102+
return NextResponse.redirect(absoluteUrl.toString());
103+
}
101104
}
105+
} catch (error) {
106+
console.error(
107+
`Failed to execute sendLoginname (authRequest=${authRequest}, sessionId=${sessionId}): ${error instanceof Error ? error.message : error}`,
108+
);
109+
Sentry.captureException(error, {
110+
tags: { flow: "oidc", stage: "reauth_sendLoginname" },
111+
extra: { authRequest, sessionId },
112+
});
102113
}
103114
}
104115

0 commit comments

Comments
 (0)