mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-18 05:52:51 +00:00
fix(login): redirect unknown users to external IdP after domain discovery when enumeration protection is active (#12581)
# Which Problems Are Solved A `login_hint` that resolves an organization via domain discovery but matches no existing user is no longer automatically redirected to the organization's external IdP when enumeration protection (`ignoreUnknownUsernames`) is active on the request context. #12369 combined the unknown-user IdP redirect into a single condition, but gated it with `&& !ignoreUnknownUsernames`: ```ts if ((!effectiveLoginSettings?.allowLocalAuthentication || discoveredOrganization) && !ignoreUnknownUsernames) { ``` Since the flag is resolved from the pre-discovery context (the instance default settings — during domain discovery there is no organization context yet), enabling enumeration protection instance-wide disables the redirect even for IdP-only organizations (`allowLocalAuthentication: false`). The flow falls through to `preventUserEnumeration()` and strands the user on a decoy Password screen for an account that does not exist; the Back button re-enters the auto-submit loop and dead-ends identically. This breaks silent-SSO first logins where the external IdP is supposed to create the account on first authentication (Login V2). The guard is also counterproductive on its own terms: existing users of the same org still auto-redirect to the IdP, so the divergent treatment acts as an enumeration oracle — redirect means the user exists, password screen means it doesn't. And it protects nothing in practice, since the IdP remains reachable manually via the account switcher → login-name screen → IdP button. # How the Problems Are Solved Split the condition by what known users of the resolved organization actually experience, so unknown usernames always mimic the known-user flow: ```ts if ( !effectiveLoginSettings?.allowLocalAuthentication || (discoveredOrganization && !ignoreUnknownUsernames) ) { ``` - **Local authentication disabled (IdP-only org):** redirect unknown usernames to the single allowed IdP regardless of `ignoreUnknownUsernames`. Known users are auto-redirected there, so the identical redirect is precisely what keeps unknown usernames indistinguishable — and it restores first-login account creation via the external IdP. - **Local authentication enabled:** known (password) users see the password screen, so the decoy password screen remains the indistinguishable behavior and enumeration protection keeps gating the discovery-based IdP redirect, as introduced by #12369. Adds regression tests for both cases: domain discovery into an IdP-only org with `ignoreUnknownUsernames: true` expects the IdP redirect; discovery into a local-auth org expects the decoy `/password` redirect with no IdP flow started.
This commit is contained in:
@@ -546,6 +546,81 @@ describe("sendLoginname", () => {
|
|||||||
expect(result?.redirect).toContain("organization=org123");
|
expect(result?.redirect).toContain("organization=org123");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("should redirect to IDP via domain discovery for IdP-only org even when ignoreUnknownUsernames is true", async () => {
|
||||||
|
// Regression test: enumeration protection must not divert unknown users of
|
||||||
|
// an IdP-only org (allowLocalAuthentication: false) to the decoy password
|
||||||
|
// screen — known users of that org are auto-redirected to the IdP, so the
|
||||||
|
// redirect is the indistinguishable (and functional) behavior.
|
||||||
|
mockGetLoginSettings
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
// context (instance default) settings
|
||||||
|
ignoreUnknownUsernames: true,
|
||||||
|
allowLocalAuthentication: true,
|
||||||
|
})
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
// discovered org settings: IdP-only
|
||||||
|
allowDomainDiscovery: true,
|
||||||
|
allowRegister: false,
|
||||||
|
allowLocalAuthentication: false,
|
||||||
|
ignoreUnknownUsernames: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
mockGetOrgsByDomain.mockResolvedValue({
|
||||||
|
result: [{ id: "discovered-org-sso", name: "Enterprise Org" }],
|
||||||
|
});
|
||||||
|
|
||||||
|
mockGetActiveIdentityProviders.mockResolvedValue({
|
||||||
|
identityProviders: [{ id: "idp-entra", type: "OIDC", options: { isCreationAllowed: true } }],
|
||||||
|
});
|
||||||
|
mockStartIdentityProviderFlow.mockResolvedValue({ url: "https://entra.example.com/auth" });
|
||||||
|
|
||||||
|
const result = await sendLoginname({
|
||||||
|
loginName: "[email protected]",
|
||||||
|
requestId: "req123",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result).toEqual({ redirect: "https://entra.example.com/auth" });
|
||||||
|
|
||||||
|
expect(mockGetActiveIdentityProviders).toHaveBeenCalledWith({
|
||||||
|
serviceConfig: { baseUrl: "https://api.example.com" },
|
||||||
|
orgId: "discovered-org-sso",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test("should keep decoy password screen when ignoreUnknownUsernames is true and discovered org allows local authentication", async () => {
|
||||||
|
// With local authentication enabled, known (password) users see the
|
||||||
|
// password screen, so the decoy is what keeps unknown users
|
||||||
|
// indistinguishable — the discovery-based IdP redirect stays gated.
|
||||||
|
mockGetLoginSettings
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
// context (instance default) settings
|
||||||
|
ignoreUnknownUsernames: true,
|
||||||
|
allowLocalAuthentication: true,
|
||||||
|
})
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
// discovered org settings: local auth enabled
|
||||||
|
allowDomainDiscovery: true,
|
||||||
|
allowRegister: false,
|
||||||
|
allowLocalAuthentication: true,
|
||||||
|
ignoreUnknownUsernames: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
mockGetOrgsByDomain.mockResolvedValue({
|
||||||
|
result: [{ id: "discovered-org-local", name: "Password Org" }],
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await sendLoginname({
|
||||||
|
loginName: "[email protected]",
|
||||||
|
requestId: "req123",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result).toBeDefined();
|
||||||
|
expect(result?.redirect).toMatch(/^\/password\?/);
|
||||||
|
expect(result?.redirect).toContain("loginName=newuser%40company.com");
|
||||||
|
expect(result?.redirect).toContain("organization=discovered-org-local");
|
||||||
|
expect(mockStartIdentityProviderFlow).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
test("should return error when user not found and no registration allowed", async () => {
|
test("should return error when user not found and no registration allowed", async () => {
|
||||||
mockGetLoginSettings.mockResolvedValue({
|
mockGetLoginSettings.mockResolvedValue({
|
||||||
allowRegister: false,
|
allowRegister: false,
|
||||||
|
|||||||
@@ -562,8 +562,19 @@ export async function sendLoginname(command: SendLoginnameCommand) {
|
|||||||
// and must not prevent authentication via an external IdP.
|
// and must not prevent authentication via an external IdP.
|
||||||
// Fixes: https://github.com/zitadel/zitadel/issues/12021
|
// Fixes: https://github.com/zitadel/zitadel/issues/12021
|
||||||
// Fixes: https://github.com/zitadel/zitadel/issues/12023
|
// Fixes: https://github.com/zitadel/zitadel/issues/12023
|
||||||
|
//
|
||||||
|
// Enumeration protection (ignoreUnknownUsernames) must NOT gate the redirect
|
||||||
|
// when local authentication is disabled: in an IdP-only org, known users are
|
||||||
|
// auto-redirected to the IdP, so sending unknown usernames to the very same
|
||||||
|
// IdP is what keeps them indistinguishable. Diverting unknown users to the
|
||||||
|
// decoy password screen instead would both break silent SSO for first-time
|
||||||
|
// users (the IdP is meant to create the account on first login) and act as an
|
||||||
|
// enumeration oracle (redirect = user exists, password screen = unknown).
|
||||||
|
// Conversely, when local authentication is enabled, the decoy password screen
|
||||||
|
// is what known (password) users see, so enumeration protection keeps gating
|
||||||
|
// the discovery-based redirect there.
|
||||||
|
|
||||||
if ((!effectiveLoginSettings?.allowLocalAuthentication || discoveredOrganization) && !ignoreUnknownUsernames) {
|
if (!effectiveLoginSettings?.allowLocalAuthentication || (discoveredOrganization && !ignoreUnknownUsernames)) {
|
||||||
const resp = await redirectUserToIDP(undefined, discoveredOrganization);
|
const resp = await redirectUserToIDP(undefined, discoveredOrganization);
|
||||||
if (resp) {
|
if (resp) {
|
||||||
logger.debug("Redirecting to IDP", { organization: discoveredOrganization });
|
logger.debug("Redirecting to IDP", { organization: discoveredOrganization });
|
||||||
|
|||||||
Reference in New Issue
Block a user