mirror of
https://github.com/zitadel/zitadel.git
synced 2026-08-17 16:35:14 -05:00
fix(login): redirect to external IdP after domain discovery regardless of registration policy (#12369)
Closes #12021 Closes #12023 # Which Problems Are Solved When a user entered an email on the login page and domain discovery resolved the correct organization, the login flow failed to redirect to the configured external IdP. Instead it returned a "user not found" error or showed the registration page. This happened because the "user not found" decision tree in `sendLoginname` only attempted an IdP redirect when `allowLocalAuthentication` was disabled. If local auth was enabled but `allowRegister` was `false`, the IdP check was skipped entirely. # How the Problems Are Solved Combined the IdP redirect logic into a single block that triggers when either local authentication is disabled **or** domain discovery resolved an organization. The registration policy (`allowRegister`) now only affects local account creation and no longer gates external IdP authentication. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Livio Spring <9405495+livio-a@users.noreply.github.com>
This commit is contained in:
co-authored by
Copilot Autofix powered by AI
Livio Spring
parent
2b9b9856b4
commit
3e94047da7
@@ -120,6 +120,8 @@ describe("sendLoginname", () => {
|
||||
});
|
||||
// Default: org discovery returns empty result
|
||||
mockGetOrgsByDomain.mockResolvedValue({ result: [] });
|
||||
// Default: no active identity providers
|
||||
mockGetActiveIdentityProviders.mockResolvedValue({ identityProviders: [] });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -591,6 +593,121 @@ describe("sendLoginname", () => {
|
||||
expect(result).toEqual({ error: "errors.userNotFound" });
|
||||
});
|
||||
|
||||
test("should redirect to IDP when allowRegister: false, allowLocalAuthentication: true and discovered org has single active IDP", async () => {
|
||||
mockGetLoginSettings.mockResolvedValue({
|
||||
allowRegister: false,
|
||||
allowLocalAuthentication: true,
|
||||
});
|
||||
mockGetActiveIdentityProviders.mockResolvedValue({
|
||||
identityProviders: [{ id: "idp123", type: "OIDC", options: { isAutoCreation: true } }],
|
||||
});
|
||||
mockStartIdentityProviderFlow.mockResolvedValue({ url: "https://idp.example.com/auth" });
|
||||
|
||||
const result = await sendLoginname({
|
||||
loginName: "user@example.com",
|
||||
organization: "org123",
|
||||
});
|
||||
|
||||
expect(result).toEqual({ redirect: "https://idp.example.com/auth" });
|
||||
});
|
||||
|
||||
test("should redirect to IDP via domain discovery when allowRegister: false, allowLocalAuthentication: true", async () => {
|
||||
mockGetLoginSettings
|
||||
.mockResolvedValueOnce({
|
||||
allowRegister: false,
|
||||
allowLocalAuthentication: true,
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
allowDomainDiscovery: true,
|
||||
allowRegister: false,
|
||||
allowLocalAuthentication: true,
|
||||
});
|
||||
|
||||
mockGetOrgsByDomain.mockResolvedValue({
|
||||
result: [{ id: "discovered-org-789", name: "Company Org" }],
|
||||
});
|
||||
|
||||
mockGetActiveIdentityProviders.mockResolvedValue({
|
||||
identityProviders: [{ id: "idp456", type: "OIDC", options: { isAutoCreation: true } }],
|
||||
});
|
||||
mockStartIdentityProviderFlow.mockResolvedValue({ url: "https://company-idp.example.com/auth" });
|
||||
|
||||
const result = await sendLoginname({
|
||||
loginName: "user@company.com",
|
||||
requestId: "req123",
|
||||
});
|
||||
|
||||
expect(result).toEqual({ redirect: "https://company-idp.example.com/auth" });
|
||||
|
||||
expect(mockGetOrgsByDomain).toHaveBeenCalledWith({
|
||||
serviceConfig: { baseUrl: "https://api.example.com" },
|
||||
domain: "company.com",
|
||||
});
|
||||
|
||||
expect(mockGetActiveIdentityProviders).toHaveBeenCalledWith({
|
||||
serviceConfig: { baseUrl: "https://api.example.com" },
|
||||
orgId: "discovered-org-789",
|
||||
});
|
||||
});
|
||||
|
||||
test("should redirect to IDP when allowRegister: true, allowLocalAuthentication: true and discovered org has single active IDP", async () => {
|
||||
mockGetLoginSettings
|
||||
.mockResolvedValueOnce({
|
||||
allowRegister: true,
|
||||
allowLocalAuthentication: true,
|
||||
ignoreUnknownUsernames: false,
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
allowDomainDiscovery: true,
|
||||
allowRegister: true,
|
||||
allowLocalAuthentication: true,
|
||||
ignoreUnknownUsernames: false,
|
||||
});
|
||||
|
||||
mockGetOrgsByDomain.mockResolvedValue({
|
||||
result: [{ id: "discovered-org-101", name: "Example Org" }],
|
||||
});
|
||||
|
||||
mockGetActiveIdentityProviders.mockResolvedValue({
|
||||
identityProviders: [{ id: "idp789", type: "OIDC", options: { isAutoCreation: true } }],
|
||||
});
|
||||
mockStartIdentityProviderFlow.mockResolvedValue({ url: "https://example-idp.example.com/auth" });
|
||||
|
||||
const result = await sendLoginname({
|
||||
loginName: "user@example.com",
|
||||
requestId: "req123",
|
||||
});
|
||||
|
||||
expect(result).toEqual({ redirect: "https://example-idp.example.com/auth" });
|
||||
});
|
||||
|
||||
test("should return error when allowRegister: false, allowLocalAuthentication: true, discovered org but no active IDP", async () => {
|
||||
mockGetLoginSettings
|
||||
.mockResolvedValueOnce({
|
||||
allowRegister: false,
|
||||
allowLocalAuthentication: true,
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
allowDomainDiscovery: true,
|
||||
allowRegister: false,
|
||||
allowLocalAuthentication: true,
|
||||
});
|
||||
|
||||
mockGetOrgsByDomain.mockResolvedValue({
|
||||
result: [{ id: "discovered-org-empty", name: "No IDP Org" }],
|
||||
});
|
||||
|
||||
mockGetActiveIdentityProviders.mockResolvedValue({
|
||||
identityProviders: [],
|
||||
});
|
||||
|
||||
const result = await sendLoginname({
|
||||
loginName: "user@noidp.com",
|
||||
});
|
||||
|
||||
expect(result).toEqual({ error: "errors.userNotFound" });
|
||||
});
|
||||
|
||||
test("should discover organization from domain suffix when user not found without org context", async () => {
|
||||
// Mock login settings for instance level (no org context)
|
||||
mockGetLoginSettings
|
||||
|
||||
@@ -545,18 +545,29 @@ export async function sendLoginname(command: SendLoginnameCommand) {
|
||||
logger.debug("No single org found for discovery");
|
||||
}
|
||||
}
|
||||
// When a user is not found, try to redirect to an external IdP if:
|
||||
// - local authentication is disabled, OR
|
||||
// - domain discovery resolved an organization (regardless of allowRegister)
|
||||
// Registration policy (allowRegister) controls local account creation only
|
||||
// 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
|
||||
|
||||
// user not found, check if IDPs are available when local auth is not allowed
|
||||
if (!effectiveLoginSettings?.allowLocalAuthentication) {
|
||||
logger.debug("redirecting to IDP (register allowed, password not allowed)");
|
||||
if ((!effectiveLoginSettings?.allowLocalAuthentication || discoveredOrganization) && !command.ignoreUnknownUsernames) {
|
||||
const resp = await redirectUserToIDP(undefined, discoveredOrganization);
|
||||
if (resp) {
|
||||
logger.debug("Redirecting to IDP", { organization: discoveredOrganization });
|
||||
return resp;
|
||||
}
|
||||
logger.debug("IDP redirect failed, returning user not found");
|
||||
|
||||
return preventUserEnumeration(discoveredOrganization);
|
||||
} else if (effectiveLoginSettings?.allowRegister && effectiveLoginSettings?.allowLocalAuthentication) {
|
||||
// If local auth is disabled, there is no fallback — return error
|
||||
if (!effectiveLoginSettings?.allowLocalAuthentication) {
|
||||
logger.debug("IDP redirect failed and local auth not allowed, returning user not found");
|
||||
return preventUserEnumeration(discoveredOrganization);
|
||||
}
|
||||
}
|
||||
|
||||
if (effectiveLoginSettings?.allowRegister && effectiveLoginSettings?.allowLocalAuthentication) {
|
||||
logger.debug("register and password both allowed");
|
||||
// do not register user if ignoreUnknownUsernames is set
|
||||
if (discoveredOrganization && !effectiveLoginSettings?.ignoreUnknownUsernames) {
|
||||
|
||||
Reference in New Issue
Block a user