Merge commit from fork

* validate IDP linking permissions * fix: check policy before linking * apply suggested review comments --------- Co-authored-by: Max Peintner <[email protected]>
2026-10-18 05:52:51 +00:00 · 2025-11-12 13:48:40 +01:00
parent 75791361f3
commit 33c51deb20
3 changed files with 385 additions and 17 deletions
@@ -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);
+    });
+  });
+});
@@ -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<boolean> {
+  // 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: {
@@ -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