mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-18 05:52:51 +00:00
fix(login): use correct requestId with oidc_ prefix in Prompt.LOGIN + loginHint flow (#12376)
Closes #11946 # Which Problems Are Solved - OIDC redirect is broken when `Prompt.LOGIN` + `loginHint` is used — the user lands on `/signedin` instead of being redirected to the OIDC client's callback URL (fixes #11946) # How the Problems Are Solved - In `handleOIDCFlowInitiation`, the `Prompt.LOGIN` + `loginHint` code path was passing `authRequest.id` (raw ID without `oidc_` prefix) to `sendLoginname`. Without the prefix, `completeFlowOrGetUrl` does not recognize the flow as OIDC and falls through to the "Regular flow" path, redirecting to `/signedin` instead of calling `createCallback`. Fixed by using the `requestId` parameter which already carries the `oidc_` prefix. # Additional Changes - Standardized all request ID references in `handleOIDCFlowInitiation` to consistently use the `requestId` parameter instead of reconstructing it with `` `oidc_${authRequest.id}` ``. This eliminates the inconsistency that caused the bug and prevents similar issues in the future. - Removed a redundant `if (authRequest.id)` guard in the LDAP redirect path, since `requestId` is always present as a required parameter. - Added regression tests verifying that `sendLoginname` receives the `requestId` with the `oidc_` prefix in the `Prompt.LOGIN` + `loginHint` code path.
This commit is contained in:
@@ -355,3 +355,103 @@ describe("handleOIDCFlowInitiation — org-scoped session filtering", () => {
|
||||
expect(location).toContain("/loginname");
|
||||
});
|
||||
});
|
||||
|
||||
describe("handleOIDCFlowInitiation — Prompt.LOGIN + loginHint requestId prefix", () => {
|
||||
let mockGetAuthRequest: ReturnType<typeof vi.fn>;
|
||||
let mockConstructUrl: ReturnType<typeof vi.fn>;
|
||||
let mockSendLoginname: ReturnType<typeof vi.fn>;
|
||||
|
||||
const existingSession = {
|
||||
id: "session-1",
|
||||
factors: { user: { id: "user1", organizationId: "org1", loginName: "[email protected]" } },
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.clearAllMocks();
|
||||
vi.unstubAllEnvs();
|
||||
|
||||
const zitadel = await import("@/lib/zitadel");
|
||||
const serviceUrl = await import("@/lib/service-url");
|
||||
const authUtils = await import("@/lib/auth-utils");
|
||||
const loginname = await import("@/lib/server/loginname");
|
||||
|
||||
mockGetAuthRequest = vi.mocked(zitadel.getAuthRequest);
|
||||
mockConstructUrl = vi.mocked(serviceUrl.constructUrl);
|
||||
mockSendLoginname = vi.mocked(loginname.sendLoginname);
|
||||
vi.mocked(authUtils.getValidLocaleFromUILocales).mockReturnValue(null);
|
||||
|
||||
mockConstructUrl.mockImplementation((_req: any, path: string) => {
|
||||
return new URL(`https://example.com${path}`);
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
test("should pass requestId with oidc_ prefix to sendLoginname (not authRequest.id)", async () => {
|
||||
const { Prompt } = await import("@zitadel/proto/zitadel/oidc/v2/authorization_pb");
|
||||
|
||||
mockGetAuthRequest.mockResolvedValue({
|
||||
authRequest: {
|
||||
id: "abc123",
|
||||
uiLocales: [],
|
||||
scope: [],
|
||||
prompt: [Prompt.LOGIN],
|
||||
loginHint: "[email protected]",
|
||||
},
|
||||
});
|
||||
|
||||
mockSendLoginname.mockResolvedValue({
|
||||
redirect: "/password?loginName=user%40example.com",
|
||||
});
|
||||
|
||||
await handleOIDCFlowInitiation(
|
||||
makeBaseParams({
|
||||
requestId: "oidc_abc123",
|
||||
sessions: [existingSession] as any,
|
||||
sessionCookies: [{ id: "session-1", token: "tok" }],
|
||||
}),
|
||||
);
|
||||
|
||||
expect(mockSendLoginname).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
loginName: "[email protected]",
|
||||
requestId: "oidc_abc123",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
test("should NOT pass raw authRequest.id without oidc_ prefix to sendLoginname", async () => {
|
||||
const { Prompt } = await import("@zitadel/proto/zitadel/oidc/v2/authorization_pb");
|
||||
|
||||
mockGetAuthRequest.mockResolvedValue({
|
||||
authRequest: {
|
||||
id: "abc123",
|
||||
uiLocales: [],
|
||||
scope: [],
|
||||
prompt: [Prompt.LOGIN],
|
||||
loginHint: "[email protected]",
|
||||
},
|
||||
});
|
||||
|
||||
mockSendLoginname.mockResolvedValue({
|
||||
redirect: "/password?loginName=user%40example.com",
|
||||
});
|
||||
|
||||
await handleOIDCFlowInitiation(
|
||||
makeBaseParams({
|
||||
requestId: "oidc_abc123",
|
||||
sessions: [existingSession] as any,
|
||||
sessionCookies: [{ id: "session-1", token: "tok" }],
|
||||
}),
|
||||
);
|
||||
|
||||
// Ensure the raw authRequest.id is NOT what's passed
|
||||
expect(mockSendLoginname).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
requestId: "abc123",
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -187,9 +187,7 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr
|
||||
|
||||
if (identityProviderType === IdentityProviderType.LDAP) {
|
||||
const ldapUrl = constructUrl(request, "/ldap");
|
||||
if (authRequest.id) {
|
||||
ldapUrl.searchParams.set("requestId", `oidc_${authRequest.id}`);
|
||||
}
|
||||
ldapUrl.searchParams.set("requestId", requestId);
|
||||
if (organization) {
|
||||
ldapUrl.searchParams.set("organization", organization);
|
||||
}
|
||||
@@ -274,7 +272,7 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr
|
||||
if (eligibleSessions.length === 0) {
|
||||
return gotoLoginname({
|
||||
request,
|
||||
requestId: `oidc_${authRequest.id}`,
|
||||
requestId,
|
||||
loginHint: authRequest.loginHint,
|
||||
organization,
|
||||
suffix,
|
||||
@@ -282,7 +280,7 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr
|
||||
}
|
||||
return gotoAccounts({
|
||||
request,
|
||||
requestId: `oidc_${authRequest.id}`,
|
||||
requestId,
|
||||
organization,
|
||||
});
|
||||
} else if (authRequest.prompt.includes(Prompt.LOGIN)) {
|
||||
@@ -290,7 +288,7 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr
|
||||
try {
|
||||
let command: SendLoginnameCommand = {
|
||||
loginName: authRequest.loginHint,
|
||||
requestId: authRequest.id,
|
||||
requestId: requestId,
|
||||
};
|
||||
|
||||
if (organization) {
|
||||
@@ -377,7 +375,7 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr
|
||||
if (eligibleSessions.length === 0) {
|
||||
return gotoLoginname({
|
||||
request,
|
||||
requestId: `oidc_${authRequest.id}`,
|
||||
requestId,
|
||||
loginHint: authRequest.loginHint,
|
||||
organization,
|
||||
suffix,
|
||||
@@ -385,7 +383,7 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr
|
||||
}
|
||||
return gotoAccounts({
|
||||
request,
|
||||
requestId: `oidc_${authRequest.id}`,
|
||||
requestId,
|
||||
organization,
|
||||
});
|
||||
}
|
||||
@@ -395,7 +393,7 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr
|
||||
if (!cookie || !cookie.id || !cookie.token) {
|
||||
return gotoAccounts({
|
||||
request,
|
||||
requestId: `oidc_${authRequest.id}`,
|
||||
requestId,
|
||||
organization,
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user