diff --git a/apps/login/src/lib/server/idp-intent.test.ts b/apps/login/src/lib/server/idp-intent.test.ts index c6e7721bc..fae6bd432 100644 --- a/apps/login/src/lib/server/idp-intent.test.ts +++ b/apps/login/src/lib/server/idp-intent.test.ts @@ -24,6 +24,8 @@ vi.mock("../zitadel", () => ({ addHuman: vi.fn(), getLoginSettings: vi.fn(), getOrgsByDomain: vi.fn(), + getActiveIdentityProviders: vi.fn(), + getUserByID: vi.fn(), getDefaultOrg: vi.fn(), })); @@ -47,6 +49,8 @@ describe("processIDPCallback", () => { let mockAddHuman: any; let mockGetLoginSettings: any; let mockGetOrgsByDomain: any; + let mockGetActiveIdentityProviders: any; + let mockGetUserByID: any; let mockGetDefaultOrg: any; let mockCreateNewSessionFromIdpIntent: any; @@ -117,6 +121,8 @@ describe("processIDPCallback", () => { addHuman, getLoginSettings, getOrgsByDomain, + getActiveIdentityProviders, + getUserByID, getDefaultOrg, } = await import("../zitadel"); const { createNewSessionFromIdpIntent } = await import("./idp"); @@ -132,6 +138,8 @@ describe("processIDPCallback", () => { mockAddHuman = vi.mocked(addHuman); mockGetLoginSettings = vi.mocked(getLoginSettings); mockGetOrgsByDomain = vi.mocked(getOrgsByDomain); + mockGetActiveIdentityProviders = vi.mocked(getActiveIdentityProviders); + mockGetUserByID = vi.mocked(getUserByID); mockGetDefaultOrg = vi.mocked(getDefaultOrg); mockCreateNewSessionFromIdpIntent = vi.mocked(createNewSessionFromIdpIntent); @@ -145,6 +153,20 @@ describe("processIDPCallback", () => { mockCreateNewSessionFromIdpIntent.mockResolvedValue({ redirect: "https://app.example.com/success", }); + + // Default mocks for validation functions + mockGetUserByID.mockResolvedValue({ + userId: "user123", + details: { + resourceOwner: "org123", + }, + }); + mockGetLoginSettings.mockResolvedValue({ + allowExternalIdp: true, + }); + mockGetActiveIdentityProviders.mockResolvedValue({ + identityProviders: [{ id: "idp123", name: "Test IDP" }], + }); }); afterEach(() => { @@ -400,13 +422,19 @@ describe("processIDPCallback", () => { options: { ...defaultIdp.config.options, autoLinking: AutoLinkingOption.EMAIL, + isLinkingAllowed: true, }, }, }); }); test("should auto-link user by email and create session", async () => { - const foundUser = { userId: "found123" }; + const foundUser = { + userId: "found123", + details: { + resourceOwner: "org123", + }, + }; mockListUsers.mockResolvedValue({ result: [foundUser], }); @@ -474,6 +502,7 @@ describe("processIDPCallback", () => { options: { ...defaultIdp.config.options, autoLinking: AutoLinkingOption.USERNAME, + isLinkingAllowed: true, }, }, }); @@ -481,7 +510,14 @@ describe("processIDPCallback", () => { test("should auto-link user by username", async () => { mockListUsers.mockResolvedValue({ - result: [{ userId: "found123" }], + result: [ + { + userId: "found123", + details: { + resourceOwner: "org123", + }, + }, + ], }); const result = await processIDPCallback(defaultParams); @@ -772,12 +808,20 @@ describe("processIDPCallback", () => { options: { ...defaultIdp.config.options, autoLinking: AutoLinkingOption.EMAIL, + isLinkingAllowed: true, isAutoCreation: true, }, }, }); mockListUsers.mockResolvedValue({ - result: [{ userId: "found123" }], + result: [ + { + userId: "found123", + details: { + resourceOwner: "org123", + }, + }, + ], }); await processIDPCallback(defaultParams); @@ -828,3 +872,228 @@ describe("processIDPCallback", () => { }); }); }); + +describe("validateIDPLinkingPermissions", () => { + let mockGetLoginSettings: any; + let mockGetActiveIdentityProviders: any; + let validateIDPLinkingPermissions: any; + + beforeEach(async () => { + vi.clearAllMocks(); + + // Import mocked modules + const { getLoginSettings, getActiveIdentityProviders } = await import("../zitadel"); + const { validateIDPLinkingPermissions: validate } = await import("./idp-intent"); + + mockGetLoginSettings = vi.mocked(getLoginSettings); + mockGetActiveIdentityProviders = vi.mocked(getActiveIdentityProviders); + validateIDPLinkingPermissions = validate; + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe("Organization login settings validation", () => { + test("should return false when allowExternalIdp is false", async () => { + mockGetLoginSettings.mockResolvedValue({ + allowExternalIdp: false, + }); + + const result = await validateIDPLinkingPermissions({ + serviceUrl: "https://api.example.com", + userOrganizationId: "org123", + idpId: "idp123", + }); + + expect(result).toBe(false); + expect(mockGetLoginSettings).toHaveBeenCalledWith({ + serviceUrl: "https://api.example.com", + organization: "org123", + }); + expect(mockGetActiveIdentityProviders).not.toHaveBeenCalled(); + }); + + test("should return false when login settings are undefined", async () => { + mockGetLoginSettings.mockResolvedValue(undefined); + + const result = await validateIDPLinkingPermissions({ + serviceUrl: "https://api.example.com", + userOrganizationId: "org123", + idpId: "idp123", + }); + + expect(result).toBe(false); + }); + + test("should return false when allowExternalIdp is missing", async () => { + mockGetLoginSettings.mockResolvedValue({}); + + const result = await validateIDPLinkingPermissions({ + serviceUrl: "https://api.example.com", + userOrganizationId: "org123", + idpId: "idp123", + }); + + expect(result).toBe(false); + }); + }); + + describe("Active IDP validation", () => { + test("should return false when IDP is not in active list", async () => { + mockGetLoginSettings.mockResolvedValue({ + allowExternalIdp: true, + }); + mockGetActiveIdentityProviders.mockResolvedValue({ + identityProviders: [ + { id: "idp456", name: "Other IDP" }, + { id: "idp789", name: "Another IDP" }, + ], + }); + + const result = await validateIDPLinkingPermissions({ + serviceUrl: "https://api.example.com", + userOrganizationId: "org123", + idpId: "idp123", + }); + + expect(result).toBe(false); + expect(mockGetActiveIdentityProviders).toHaveBeenCalledWith({ + serviceUrl: "https://api.example.com", + orgId: "org123", + linking_allowed: true, + }); + }); + + test("should return false when identityProviders list is empty", async () => { + mockGetLoginSettings.mockResolvedValue({ + allowExternalIdp: true, + }); + mockGetActiveIdentityProviders.mockResolvedValue({ + identityProviders: [], + }); + + const result = await validateIDPLinkingPermissions({ + serviceUrl: "https://api.example.com", + userOrganizationId: "org123", + idpId: "idp123", + }); + + expect(result).toBe(false); + }); + + test("should return false when identityProviders is undefined", async () => { + mockGetLoginSettings.mockResolvedValue({ + allowExternalIdp: true, + }); + mockGetActiveIdentityProviders.mockResolvedValue({}); + + const result = await validateIDPLinkingPermissions({ + serviceUrl: "https://api.example.com", + userOrganizationId: "org123", + idpId: "idp123", + }); + + expect(result).toBe(false); + }); + }); + + describe("Successful validation", () => { + test("should return true when all validations pass", async () => { + mockGetLoginSettings.mockResolvedValue({ + allowExternalIdp: true, + }); + mockGetActiveIdentityProviders.mockResolvedValue({ + identityProviders: [ + { id: "idp123", name: "Target IDP" }, + { id: "idp456", name: "Other IDP" }, + ], + }); + + const result = await validateIDPLinkingPermissions({ + serviceUrl: "https://api.example.com", + userOrganizationId: "org123", + idpId: "idp123", + }); + + expect(result).toBe(true); + expect(mockGetLoginSettings).toHaveBeenCalledWith({ + serviceUrl: "https://api.example.com", + organization: "org123", + }); + expect(mockGetActiveIdentityProviders).toHaveBeenCalledWith({ + serviceUrl: "https://api.example.com", + orgId: "org123", + linking_allowed: true, + }); + }); + + test("should find IDP in middle of list", async () => { + mockGetLoginSettings.mockResolvedValue({ + allowExternalIdp: true, + }); + mockGetActiveIdentityProviders.mockResolvedValue({ + identityProviders: [ + { id: "idp111", name: "IDP 1" }, + { id: "idp222", name: "IDP 2" }, + { id: "idp123", name: "Target IDP" }, + { id: "idp333", name: "IDP 3" }, + ], + }); + + const result = await validateIDPLinkingPermissions({ + serviceUrl: "https://api.example.com", + userOrganizationId: "org123", + idpId: "idp123", + }); + + expect(result).toBe(true); + }); + }); + + describe("Edge cases", () => { + test("should handle getLoginSettings throwing an error", async () => { + mockGetLoginSettings.mockRejectedValue(new Error("Network error")); + + await expect( + validateIDPLinkingPermissions({ + serviceUrl: "https://api.example.com", + userOrganizationId: "org123", + idpId: "idp123", + }), + ).rejects.toThrow("Network error"); + }); + + test("should handle getActiveIdentityProviders throwing an error", async () => { + mockGetLoginSettings.mockResolvedValue({ + allowExternalIdp: true, + }); + mockGetActiveIdentityProviders.mockRejectedValue(new Error("Network error")); + + await expect( + validateIDPLinkingPermissions({ + serviceUrl: "https://api.example.com", + userOrganizationId: "org123", + idpId: "idp123", + }), + ).rejects.toThrow("Network error"); + }); + + test("should perform case-sensitive IDP ID comparison", async () => { + mockGetLoginSettings.mockResolvedValue({ + allowExternalIdp: true, + }); + mockGetActiveIdentityProviders.mockResolvedValue({ + identityProviders: [{ id: "IDP123", name: "Target IDP" }], + }); + + const result = await validateIDPLinkingPermissions({ + serviceUrl: "https://api.example.com", + userOrganizationId: "org123", + idpId: "idp123", + }); + + expect(result).toBe(false); + }); + }); +}); diff --git a/apps/login/src/lib/server/idp-intent.ts b/apps/login/src/lib/server/idp-intent.ts index 7de6dd22f..02eb82d0a 100644 --- a/apps/login/src/lib/server/idp-intent.ts +++ b/apps/login/src/lib/server/idp-intent.ts @@ -10,6 +10,8 @@ import { addHuman, getLoginSettings, getOrgsByDomain, + getActiveIdentityProviders, + getUserByID, getDefaultOrg, } from "@/lib/zitadel"; import { headers } from "next/headers"; @@ -63,6 +65,48 @@ async function resolveOrganizationForUser({ return defaultOrg?.id; } +/** + * Validates if IDP linking is allowed for a user's organization. + * Checks: + * 1. Organization allows external IDP login (allowExternalIdp) + * 2. The specific IDP is activated for the organization + * + */ +export async function validateIDPLinkingPermissions({ + serviceUrl, + userOrganizationId, + idpId, +}: { + serviceUrl: string; + userOrganizationId: string; + idpId: string; +}): Promise { + // Check organization login settings + const loginSettings = await getLoginSettings({ + serviceUrl, + organization: userOrganizationId, + }); + + if (!loginSettings?.allowExternalIdp) { + return false; + } + + // Check if the IDP is activated for the organization and allows linking + const activeIDPs = await getActiveIdentityProviders({ + serviceUrl, + orgId: userOrganizationId, + linking_allowed: true, + }); + + const isIDPActive = activeIDPs.identityProviders?.some((idp) => idp.id === idpId); + + if (!isIDPActive) { + return false; + } + + return true; +} + /** * Server action to process IDP callback and handle ALL business logic. * This action: @@ -208,12 +252,34 @@ export async function processIDPCallback({ // ============================================ if (link && userId) { if (!options?.isLinkingAllowed) { - console.error("[IDP Process] Linking not allowed"); + console.error("[IDP Process] Linking not allowed by IDP configuration"); const params = buildRedirectParams(); return { redirect: `/idp/${provider}/linking-failed?${params}&error=linking_not_allowed` }; } try { + // Get user to retrieve their organization + const targetUser = await getUserByID({ serviceUrl, userId }); + + if (!targetUser || !targetUser.details?.resourceOwner) { + console.error("[IDP Process] User not found or missing organization"); + const params = buildRedirectParams(); + return { redirect: `/idp/${provider}/linking-failed?${params}&error=user_not_found` }; + } + + // Validate IDP linking permissions + const isAllowed = await validateIDPLinkingPermissions({ + serviceUrl, + userOrganizationId: targetUser.details.resourceOwner, + idpId: idpInformation.idpId, + }); + + if (!isAllowed) { + console.error("[IDP Process] IDP linking validation failed"); + const params = buildRedirectParams(); + return { redirect: `/idp/${provider}/linking-failed?${params}&error=validation_failed` }; + } + await addIDPLink({ serviceUrl, idp: { @@ -286,6 +352,25 @@ export async function processIDPCallback({ if (foundUser) { try { + if (!foundUser.details?.resourceOwner) { + console.error("[IDP Process] Found user missing organization information"); + const params = buildRedirectParams(); + return { redirect: `/idp/${provider}/linking-failed?${params}&error=missing_organization` }; + } + + // Validate IDP linking permissions + const isAllowed = await validateIDPLinkingPermissions({ + serviceUrl, + userOrganizationId: foundUser.details.resourceOwner, + idpId: idpInformation.idpId, + }); + + if (!isAllowed) { + console.error("[IDP Process] Auto-linking validation failed"); + const params = buildRedirectParams(); + return { redirect: `/idp/${provider}/linking-failed?${params}&error=validation_failed` }; + } + await addIDPLink({ serviceUrl, idp: { diff --git a/internal/api/ui/login/external_provider_handler.go b/internal/api/ui/login/external_provider_handler.go index 04414c286..36018e421 100644 --- a/internal/api/ui/login/external_provider_handler.go +++ b/internal/api/ui/login/external_provider_handler.go @@ -521,8 +521,10 @@ func (l *Login) handleExternalUserAuthenticated( // checkAutoLinking checks if a user with the provided information (username or email) already exists within ZITADEL. // The decision, which information will be checked is based on the IdP template option. -// The function returns a boolean whether a user was found or not. +// The function returns a boolean whether a user was found or not, resp. if it was linked. // If single a user was found, it will be automatically linked. +// Before the actual linking, it will check if the user's organization allows external IdP and +// has activated the correspond IdP. func (l *Login) checkAutoLinking(r *http.Request, authReq *domain.AuthRequest, provider *query.IDPTemplate, externalUser *domain.ExternalUser, human *domain.Human) (bool, error) { queries := make([]query.SearchQuery, 0, 2) switch provider.AutoLinking { @@ -539,10 +541,7 @@ func (l *Login) checkAutoLinking(r *http.Request, authReq *domain.AuthRequest, p if err != nil { return false, nil } - if err = l.autoLinkUser(r, authReq, user); err != nil { - return false, err - } - return true, nil + return l.autoLinkUser(r, authReq, user) } // If a specific org has been requested, we'll check the username (org policy (suffixed or not) is already applied) // against usernames (of that org). @@ -571,21 +570,36 @@ func (l *Login) checkAutoLinking(r *http.Request, authReq *domain.AuthRequest, p if err != nil { return false, nil } - if err = l.autoLinkUser(r, authReq, user); err != nil { - return false, err - } - return true, nil + return l.autoLinkUser(r, authReq, user) } -func (l *Login) autoLinkUser(r *http.Request, authReq *domain.AuthRequest, user *query.NotifyUser) error { +func (l *Login) checkAutoLinkingAllowedForUserAndIdP(r *http.Request, authReq *domain.AuthRequest, user *query.NotifyUser) (bool, error) { + policy, err := l.getLoginPolicy(r, user.ResourceOwner) + if err != nil { + return false, err + } + if !policy.AllowExternalIDPs { + return false, nil + } + return slices.ContainsFunc(policy.IDPLinks, func(link *query.IDPLoginPolicyLink) bool { + return link.IDPID == authReq.SelectedIDPConfigID + }), nil +} + +// autoLink user will link the external user to the found user in case the user's organization +// has the corresponding IdP activated. In case it doesn't, the function returns false and no error. +func (l *Login) autoLinkUser(r *http.Request, authReq *domain.AuthRequest, user *query.NotifyUser) (bool, error) { + if allowed, err := l.checkAutoLinkingAllowedForUserAndIdP(r, authReq, user); err != nil || !allowed { + return false, nil + } if err := l.authRepo.SelectUser(r.Context(), authReq.ID, user.ID, authReq.AgentID, false); err != nil { - return err + return false, err } if err := l.authRepo.LinkExternalUsers(r.Context(), authReq.ID, authReq.AgentID, domain.BrowserInfoFromRequest(r)); err != nil { - return err + return false, err } authReq.UserID = user.ID - return nil + return true, nil } // createOrLinkUser is called if an externalAuthentication couldn't find a corresponding externalID