diff --git a/apps/login/src/app/(login)/authenticator/set/page.tsx b/apps/login/src/app/(login)/authenticator/set/page.tsx index bb4eb0050..d0559b015 100644 --- a/apps/login/src/app/(login)/authenticator/set/page.tsx +++ b/apps/login/src/app/(login)/authenticator/set/page.tsx @@ -186,6 +186,7 @@ export default async function Page(props: { searchParams: Promise )} diff --git a/apps/login/src/app/(login)/loginname/page.tsx b/apps/login/src/app/(login)/loginname/page.tsx index 1cf4960a2..7b12c1e4f 100644 --- a/apps/login/src/app/(login)/loginname/page.tsx +++ b/apps/login/src/app/(login)/loginname/page.tsx @@ -23,6 +23,11 @@ export default async function Page(props: { searchParams: Promise diff --git a/apps/login/src/components/sign-in-with-idp.tsx b/apps/login/src/components/sign-in-with-idp.tsx index e3e8566c0..360d674a6 100644 --- a/apps/login/src/components/sign-in-with-idp.tsx +++ b/apps/login/src/components/sign-in-with-idp.tsx @@ -23,6 +23,8 @@ export interface SignInWithIDPProps { organization?: string; sessionId?: string; postErrorRedirectUrl?: string; + /** Forwarded to the IdP as login_hint so the user does not have to type the identifier again. */ + loginHint?: string; showLabel?: boolean; } @@ -32,6 +34,7 @@ export function SignInWithIdp({ organization, sessionId, postErrorRedirectUrl, + loginHint, showLabel = true, }: Readonly) { const [state, action, _isPending] = useActionState(redirectToIdp, {}); @@ -64,6 +67,7 @@ export function SignInWithIdp({ {sessionId && } {postErrorRedirectUrl && } + {loginHint && } ) : null; diff --git a/apps/login/src/lib/server/flow-initiation.test.ts b/apps/login/src/lib/server/flow-initiation.test.ts index c4eea5a05..7991e548a 100644 --- a/apps/login/src/lib/server/flow-initiation.test.ts +++ b/apps/login/src/lib/server/flow-initiation.test.ts @@ -1061,4 +1061,34 @@ describe("handleOIDCFlowInitiation — idp scope (urn:zitadel:iam:org:idp:id)", expect(html).toContain('action="https://adfs.example.com/adfs/ls"'); expect(html).toContain('name="SAMLRequest"'); }); + + test("should pass the login_hint of the auth request on to the scoped IdP", async () => { + const { IdentityProviderType } = await import("@zitadel/proto/zitadel/settings/v2/login_settings_pb"); + + mockGetAuthRequest.mockResolvedValue({ + authRequest: { + id: "abc123", + uiLocales: [], + scope: ["urn:zitadel:iam:org:idp:id:idp-1"], + prompt: [], + loginHint: "user@example.com", + }, + }); + + mockGetActiveIdentityProviders.mockResolvedValue({ + identityProviders: [{ id: "idp-1", type: IdentityProviderType.AZURE_AD }], + }); + + mockStartIdentityProviderFlow.mockResolvedValue({ + url: "https://login.microsoftonline.com/tenant/oauth2/v2.0/authorize?client_id=xyz", + }); + + await handleOIDCFlowInitiation(makeBaseParams({ sessions: [] })); + + expect(mockStartIdentityProviderFlow).toHaveBeenCalledWith( + expect.objectContaining({ + urls: expect.objectContaining({ loginHint: "user@example.com" }), + }), + ); + }); }); diff --git a/apps/login/src/lib/server/flow-initiation.ts b/apps/login/src/lib/server/flow-initiation.ts index e0c758045..b1d752cbd 100644 --- a/apps/login/src/lib/server/flow-initiation.ts +++ b/apps/login/src/lib/server/flow-initiation.ts @@ -329,6 +329,7 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr urls: { successUrl: constructUrl(request, `/idp/${provider}/process?${params.toString()}`).toString(), failureUrl: constructUrl(request, `/idp/${provider}/failure?${params.toString()}`).toString(), + loginHint: authRequest.loginHint, }, }); diff --git a/apps/login/src/lib/server/idp.test.ts b/apps/login/src/lib/server/idp.test.ts index 7769400b3..a1f4b9a88 100644 --- a/apps/login/src/lib/server/idp.test.ts +++ b/apps/login/src/lib/server/idp.test.ts @@ -269,6 +269,55 @@ describe("redirectToIdp", () => { }); }); + describe("loginHint parameter handling", () => { + test("should forward the loginHint to the IDP flow when provided", async () => { + const formData = new FormData(); + formData.append("id", "idp123"); + formData.append("provider", "google"); + formData.append("loginHint", "user@example.com"); + + mockStartIdentityProviderFlow.mockResolvedValue({ url: "https://idp.example.com/auth" }); + + try { + await redirectToIdp(undefined, formData); + } catch (error: any) { + // Redirect throws in tests + expect(error.message).toContain("REDIRECT:"); + } + + expect(mockStartIdentityProviderFlow).toHaveBeenCalledWith( + expect.objectContaining({ + idpId: "idp123", + urls: expect.objectContaining({ loginHint: "user@example.com" }), + }), + ); + }); + + test("should not set a loginHint when it is missing or empty", async () => { + for (const loginHint of [undefined, ""]) { + mockStartIdentityProviderFlow.mockReset(); + mockStartIdentityProviderFlow.mockResolvedValue({ url: "https://idp.example.com/auth" }); + + const formData = new FormData(); + formData.append("id", "idp123"); + formData.append("provider", "google"); + if (loginHint !== undefined) { + formData.append("loginHint", loginHint); + } + + try { + await redirectToIdp(undefined, formData); + } catch (error: any) { + // Redirect throws in tests + expect(error.message).toContain("REDIRECT:"); + } + + const callArgs = mockStartIdentityProviderFlow.mock.calls[0][0]; + expect(callArgs.urls.loginHint).toBeUndefined(); + } + }); + }); + describe("General redirect behavior", () => { test("should return error when IDP flow returns null", async () => { const formData = new FormData(); diff --git a/apps/login/src/lib/server/idp.ts b/apps/login/src/lib/server/idp.ts index ea9e9aece..a043e6bcf 100644 --- a/apps/login/src/lib/server/idp.ts +++ b/apps/login/src/lib/server/idp.ts @@ -35,6 +35,7 @@ export async function redirectToIdp(prevState: RedirectToIdpState, formData: For const idpId = formData.get("id") as string; const provider = formData.get("provider") as string; const postErrorRedirectUrl = formData.get("postErrorRedirectUrl") as string; + const loginHint = formData.get("loginHint") as string; if (sessionId) { try { @@ -72,6 +73,7 @@ export async function redirectToIdp(prevState: RedirectToIdpState, formData: For idpId, successUrl: `/idp/${provider}/process?` + params.toString(), failureUrl: `/idp/${provider}/failure?` + params.toString(), + loginHint: loginHint || undefined, }); if (!response) { @@ -99,6 +101,7 @@ export type StartIDPFlowCommand = { idpId: string; successUrl: string; failureUrl: string; + loginHint?: string; }; async function startIDPFlow(command: StartIDPFlowCommand) { @@ -110,6 +113,7 @@ async function startIDPFlow(command: StartIDPFlowCommand) { urls: { successUrl: `${command.host.includes("localhost") ? "http://" : "https://"}${command.host}${basePath}${command.successUrl}`, failureUrl: `${command.host.includes("localhost") ? "http://" : "https://"}${command.host}${basePath}${command.failureUrl}`, + loginHint: command.loginHint, }, }); diff --git a/apps/login/src/lib/server/loginname.test.ts b/apps/login/src/lib/server/loginname.test.ts index a4281847c..a2c4583cb 100644 --- a/apps/login/src/lib/server/loginname.test.ts +++ b/apps/login/src/lib/server/loginname.test.ts @@ -279,6 +279,70 @@ describe("sendLoginname", () => { }); }); + test("should pass the login name to the user's IDP as a login hint", async () => { + mockGetLoginSettings.mockResolvedValue({ allowLocalAuthentication: false }); + mockListAuthenticationMethodTypes.mockResolvedValue({ + authMethodTypes: [AuthenticationMethodType.PASSWORD], + }); + mockListIDPLinks.mockResolvedValue({ + result: [{ idpId: "idp123" }], + }); + mockStartIdentityProviderFlow.mockResolvedValue({ url: "https://idp.example.com/auth" }); + + await sendLoginname({ + loginName: "user@example.com", + }); + + expect(mockStartIdentityProviderFlow).toHaveBeenCalledWith( + expect.objectContaining({ + urls: expect.objectContaining({ loginHint: "user@example.com" }), + }), + ); + }); + + test("should prefer the username known by the linked IDP over the login name as a login hint", async () => { + mockGetLoginSettings.mockResolvedValue({ allowLocalAuthentication: false }); + mockListAuthenticationMethodTypes.mockResolvedValue({ + authMethodTypes: [AuthenticationMethodType.PASSWORD], + }); + mockListIDPLinks.mockResolvedValue({ + result: [{ idpId: "idp123", userName: "user.name@idp.example.com" }], + }); + mockStartIdentityProviderFlow.mockResolvedValue({ url: "https://idp.example.com/auth" }); + + await sendLoginname({ + loginName: "user@example.com", + }); + + expect(mockStartIdentityProviderFlow).toHaveBeenCalledWith( + expect.objectContaining({ + urls: expect.objectContaining({ loginHint: "user.name@idp.example.com" }), + }), + ); + }); + + test("should add the suffix back to the login hint when the user only typed the local part", async () => { + mockGetLoginSettings.mockResolvedValue({ allowLocalAuthentication: false }); + mockListAuthenticationMethodTypes.mockResolvedValue({ + authMethodTypes: [AuthenticationMethodType.PASSWORD], + }); + mockListIDPLinks.mockResolvedValue({ + result: [{ idpId: "idp123" }], + }); + mockStartIdentityProviderFlow.mockResolvedValue({ url: "https://idp.example.com/auth" }); + + await sendLoginname({ + loginName: "user", + suffix: "example.com", + }); + + expect(mockStartIdentityProviderFlow).toHaveBeenCalledWith( + expect.objectContaining({ + urls: expect.objectContaining({ loginHint: "user@example.com" }), + }), + ); + }); + test("should return error when password not allowed and no IDP links available", async () => { mockGetLoginSettings.mockResolvedValue({ allowLocalAuthentication: false }); mockListAuthenticationMethodTypes.mockResolvedValue({ @@ -319,6 +383,29 @@ describe("sendLoginname", () => { }); }); + test("should pass the login name to the organization's single active IDP as a login hint", async () => { + mockGetLoginSettings.mockResolvedValue({ allowLocalAuthentication: false }); + mockListAuthenticationMethodTypes.mockResolvedValue({ + authMethodTypes: [AuthenticationMethodType.PASSWORD], + }); + mockListIDPLinks.mockResolvedValue({ result: [] }); + mockGetActiveIdentityProviders.mockResolvedValue({ + identityProviders: [{ id: "org-idp-123", type: 0, options: { isAutoCreation: true } }], + }); + mockIdpTypeToSlug.mockReturnValue("google"); + mockStartIdentityProviderFlow.mockResolvedValue({ url: "https://org-idp.example.com/auth" }); + + await sendLoginname({ + loginName: "user@example.com", + }); + + expect(mockStartIdentityProviderFlow).toHaveBeenCalledWith( + expect.objectContaining({ + urls: expect.objectContaining({ loginHint: "user@example.com" }), + }), + ); + }); + test("should redirect to passkey when user has only passkey method and it's allowed", async () => { mockGetLoginSettings.mockResolvedValue({ passkeysType: PasskeysType.ALLOWED, allowLocalAuthentication: true }); mockListAuthenticationMethodTypes.mockResolvedValue({ diff --git a/apps/login/src/lib/server/loginname.ts b/apps/login/src/lib/server/loginname.ts index 1ece81a02..d6a7bd02f 100644 --- a/apps/login/src/lib/server/loginname.ts +++ b/apps/login/src/lib/server/loginname.ts @@ -115,6 +115,10 @@ export async function sendLoginname(command: SendLoginnameCommand) { return { error: t("errors.userNotFound") }; }; + // With an org domain suffix the user types only the local part, so put the + // login name back together for the checks below and for the IdP login hint. + const concatLoginname = command.suffix ? `${command.loginName}@${command.suffix}` : command.loginName; + const redirectUserToIDP = async (userId?: string, organization?: string) => { // If userId is provided, check for user-specific IDP links first let identityProviders: IDPLink[] = []; @@ -165,6 +169,7 @@ export async function sendLoginname(command: SendLoginnameCommand) { failureUrl: `${host.includes("localhost") ? "http://" : "https://"}${host}${basePath}/idp/${provider}/failure?` + new URLSearchParams(params), + loginHint: concatLoginname, }, }); @@ -224,6 +229,9 @@ export async function sendLoginname(command: SendLoginnameCommand) { failureUrl: `${host.includes("localhost") ? "http://" : "https://"}${host}${basePath}/idp/${provider}/failure?` + new URLSearchParams(params), + // The user is already linked to this IdP, so prefer the username the + // IdP knows the user by over the ZITADEL login name (same as Login V1). + loginHint: identityProviders[0].userName || concatLoginname, }, }); @@ -251,9 +259,6 @@ export async function sendLoginname(command: SendLoginnameCommand) { const userLoginSettings = await getLoginSettings({ serviceConfig, organization: user.details?.resourceOwner }); - // compare with the concatenated suffix when set - const concatLoginname = command.suffix ? `${command.loginName}@${command.suffix}` : command.loginName; - const humanUser = users[0].type.case === "human" ? users[0].type.value : undefined; // recheck login settings after user discovery, as the search might have been done without org scope diff --git a/apps/login/src/lib/zitadel.test.ts b/apps/login/src/lib/zitadel.test.ts new file mode 100644 index 000000000..6aa7c22e7 --- /dev/null +++ b/apps/login/src/lib/zitadel.test.ts @@ -0,0 +1,75 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { startIdentityProviderFlow } from "./zitadel"; + +vi.mock("./service", () => ({ + createServiceForHost: vi.fn(), +})); + +vi.mock("next-intl/server", () => ({ + getTranslations: vi.fn(), +})); + +vi.mock("./fingerprint", () => ({ + getUserAgent: vi.fn(), +})); + +describe("startIdentityProviderFlow", () => { + const serviceConfig = { baseUrl: "https://api.example.com" } as any; + let startIdentityProviderIntent: ReturnType; + + beforeEach(async () => { + vi.clearAllMocks(); + const { createServiceForHost } = await import("./service"); + startIdentityProviderIntent = vi.fn().mockResolvedValue({ + nextStep: { case: "authUrl", value: "https://idp.example.com/auth" }, + }); + vi.mocked(createServiceForHost).mockResolvedValue({ startIdentityProviderIntent } as any); + }); + + test("forwards the login hint to the API", async () => { + const result = await startIdentityProviderFlow({ + serviceConfig, + idpId: "idp123", + urls: { successUrl: "https://login/success", failureUrl: "https://login/failure", loginHint: "user@example.com" }, + }); + + expect(result).toEqual({ url: "https://idp.example.com/auth" }); + expect(startIdentityProviderIntent).toHaveBeenCalledWith({ + idpId: "idp123", + content: { + case: "urls", + value: { successUrl: "https://login/success", failureUrl: "https://login/failure", loginHint: "user@example.com" }, + }, + }); + }); + + test("drops a login hint the API would reject instead of failing the IdP flow", async () => { + const loginHint = "a".repeat(201); + + const result = await startIdentityProviderFlow({ + serviceConfig, + idpId: "idp123", + urls: { successUrl: "https://login/success", failureUrl: "https://login/failure", loginHint }, + }); + + expect(result).toEqual({ url: "https://idp.example.com/auth" }); + expect(startIdentityProviderIntent).toHaveBeenCalledWith({ + idpId: "idp123", + content: { + case: "urls", + value: { successUrl: "https://login/success", failureUrl: "https://login/failure" }, + }, + }); + }); + + test("does not set an empty login hint", async () => { + await startIdentityProviderFlow({ + serviceConfig, + idpId: "idp123", + urls: { successUrl: "https://login/success", failureUrl: "https://login/failure", loginHint: "" }, + }); + + const value = startIdentityProviderIntent.mock.calls[0][0].content.value; + expect(value).not.toHaveProperty("loginHint"); + }); +}); diff --git a/apps/login/src/lib/zitadel.ts b/apps/login/src/lib/zitadel.ts index a4ab2bf2b..2ac5e2252 100644 --- a/apps/login/src/lib/zitadel.ts +++ b/apps/login/src/lib/zitadel.ts @@ -925,6 +925,9 @@ export async function getOrgsByDomain({ serviceConfig, domain }: WithServiceConf ); } +// RedirectURLs.login_hint is validated with max_len 200 by the API. +const MAX_LOGIN_HINT_LENGTH = 200; + export async function startIdentityProviderFlow({ serviceConfig, idpId, @@ -939,12 +942,19 @@ export async function startIdentityProviderFlow({ publicHost: "", }); + // The login hint only improves the UX at the IdP, so a hint the API would + // reject (e.g. an overlong login_hint sent by the RP) is dropped instead of + // failing the whole IdP flow. + const { loginHint, ...redirectUrls } = urls; + const content: RedirectURLsJson = + loginHint && loginHint.length <= MAX_LOGIN_HINT_LENGTH ? { ...redirectUrls, loginHint } : redirectUrls; + return userService .startIdentityProviderIntent({ idpId, content: { case: "urls", - value: urls, + value: content, }, }) .then(async (resp) => {