mirror of
https://github.com/zitadel/zitadel.git
synced 2026-11-03 20:56:50 -06: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.
(cherry picked from commit a331e5b7ba)
This commit is contained in:
1 parent
a9311b8c70
commit
af3a9b2bae
2 files changed
+87
-1
No files matched your search
@@ -546,6 +546,81 @@ describe("sendLoginname", () => {
|
||||
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: "newuser@enterprise.com",
|
||||
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: "newuser@company.com",
|
||||
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 () => {
|
||||
mockGetLoginSettings.mockResolvedValue({
|
||||
allowRegister: false,
|
||||
|
||||
@@ -562,8 +562,19 @@ export async function sendLoginname(command: SendLoginnameCommand) {
|
||||
// and must not prevent authentication via an external IdP.
|
||||
// Fixes: https://github.com/zitadel/zitadel/issues/12021
|
||||
// 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);
|
||||
if (resp) {
|
||||
logger.debug("Redirecting to IDP", { organization: discoveredOrganization });
|
||||
|
||||
Reference in new issue
Block a user