mirror of
https://github.com/zitadel/zitadel.git
synced 2025-12-02 12:32:24 +00:00
# Which Problems Are Solved When users authenticate via IDP (Identity Provider) without explicit organization context, the flow could fail or create users without proper organization assignment. This occurred when: - No organization parameter was provided in the IDP callback - Domain discovery didn't find a matching organization - OIDC requests didn't include organization scopes # How the Problems Are Solved Implemented a fallback mechanism that ensures organization context is always available: - Centralized organization resolution in `resolveOrganizationForUser()` - First: Use explicitly provided organization - Second: Attempt domain discovery from username - Third: Fallback to default organization (NEW) - Explicit error handling: Users are never created without organization context. If no organization can be determined (including no default org), the flow fails gracefully with a clear error message. - Applied to both creation flows: - CASE 4: Auto-creation of users - CASE 5: Manual user registration Co-authored-by: Ramon <mail@conblem.me>
This commit is contained in:
@@ -24,6 +24,7 @@ vi.mock("../zitadel", () => ({
|
||||
addHuman: vi.fn(),
|
||||
getLoginSettings: vi.fn(),
|
||||
getOrgsByDomain: vi.fn(),
|
||||
getDefaultOrg: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("./idp", () => ({
|
||||
@@ -46,6 +47,7 @@ describe("processIDPCallback", () => {
|
||||
let mockAddHuman: any;
|
||||
let mockGetLoginSettings: any;
|
||||
let mockGetOrgsByDomain: any;
|
||||
let mockGetDefaultOrg: any;
|
||||
let mockCreateNewSessionFromIdpIntent: any;
|
||||
|
||||
const defaultParams = {
|
||||
@@ -104,6 +106,7 @@ describe("processIDPCallback", () => {
|
||||
addHuman,
|
||||
getLoginSettings,
|
||||
getOrgsByDomain,
|
||||
getDefaultOrg,
|
||||
} = await import("../zitadel");
|
||||
const { createNewSessionFromIdpIntent } = await import("./idp");
|
||||
|
||||
@@ -118,6 +121,7 @@ describe("processIDPCallback", () => {
|
||||
mockAddHuman = vi.mocked(addHuman);
|
||||
mockGetLoginSettings = vi.mocked(getLoginSettings);
|
||||
mockGetOrgsByDomain = vi.mocked(getOrgsByDomain);
|
||||
mockGetDefaultOrg = vi.mocked(getDefaultOrg);
|
||||
mockCreateNewSessionFromIdpIntent = vi.mocked(createNewSessionFromIdpIntent);
|
||||
|
||||
// Default mock implementations
|
||||
@@ -564,7 +568,8 @@ describe("processIDPCallback", () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("should create user without organization when not resolved", async () => {
|
||||
test("should fallback to default organization when not resolved", async () => {
|
||||
mockGetDefaultOrg.mockResolvedValue({ id: "default-org" });
|
||||
mockAddHuman.mockResolvedValue({ userId: "newuser123" });
|
||||
|
||||
await processIDPCallback({
|
||||
@@ -572,14 +577,33 @@ describe("processIDPCallback", () => {
|
||||
organization: undefined,
|
||||
});
|
||||
|
||||
expect(mockGetDefaultOrg).toHaveBeenCalledWith({
|
||||
serviceUrl: "https://api.example.com",
|
||||
});
|
||||
expect(mockAddHuman).toHaveBeenCalledWith({
|
||||
serviceUrl: "https://api.example.com",
|
||||
request: expect.not.objectContaining({
|
||||
organization: expect.anything(),
|
||||
request: expect.objectContaining({
|
||||
organization: expect.objectContaining({
|
||||
org: { case: "orgId", value: "default-org" },
|
||||
}),
|
||||
}),
|
||||
});
|
||||
});
|
||||
|
||||
test("should return error when no organization context can be determined", async () => {
|
||||
mockGetDefaultOrg.mockResolvedValue(null);
|
||||
|
||||
const result = await processIDPCallback({
|
||||
...defaultParams,
|
||||
organization: undefined,
|
||||
});
|
||||
|
||||
expect(mockGetDefaultOrg).toHaveBeenCalled();
|
||||
expect(mockAddHuman).not.toHaveBeenCalled();
|
||||
expect(result.redirect).toContain("/idp/google/failure");
|
||||
expect(result.redirect).toContain("error=no_organization_context");
|
||||
});
|
||||
|
||||
test("should return error redirect when user creation fails", async () => {
|
||||
mockAddHuman.mockRejectedValue(new Error("Creation failed"));
|
||||
|
||||
@@ -634,12 +658,28 @@ describe("processIDPCallback", () => {
|
||||
expect(result.redirect).toContain("email=test%40example.com");
|
||||
});
|
||||
|
||||
test("should redirect to registration failed when organization cannot be resolved", async () => {
|
||||
test("should fallback to default organization for registration", async () => {
|
||||
mockGetDefaultOrg.mockResolvedValue({ id: "default-org" });
|
||||
|
||||
const result = await processIDPCallback({
|
||||
...defaultParams,
|
||||
organization: undefined,
|
||||
});
|
||||
|
||||
expect(mockGetDefaultOrg).toHaveBeenCalled();
|
||||
expect(result.redirect).toContain("/idp/google/complete-registration");
|
||||
expect(result.redirect).toContain("organization=default-org");
|
||||
});
|
||||
|
||||
test("should redirect to registration failed when no organization context available", async () => {
|
||||
mockGetDefaultOrg.mockResolvedValue(null);
|
||||
|
||||
const result = await processIDPCallback({
|
||||
...defaultParams,
|
||||
organization: undefined,
|
||||
});
|
||||
|
||||
expect(mockGetDefaultOrg).toHaveBeenCalled();
|
||||
expect(result.redirect).toContain("/idp/google/registration-failed");
|
||||
expect(result.redirect).toContain("id=intent123");
|
||||
});
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
addHuman,
|
||||
getLoginSettings,
|
||||
getOrgsByDomain,
|
||||
getDefaultOrg,
|
||||
} from "@/lib/zitadel";
|
||||
import { headers } from "next/headers";
|
||||
import { create } from "@zitadel/client";
|
||||
@@ -56,7 +57,10 @@ async function resolveOrganizationForUser({
|
||||
}
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
|
||||
// Fallback to default organization if no org was resolved through discovery
|
||||
const defaultOrg = await getDefaultOrg({ serviceUrl });
|
||||
return defaultOrg?.id;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -333,20 +337,21 @@ export async function processIDPCallback({
|
||||
serviceUrl,
|
||||
});
|
||||
|
||||
let addHumanUserWithOrganization: AddHumanUserRequest;
|
||||
if (orgToRegisterOn) {
|
||||
const organizationSchema = create(OrganizationSchema, {
|
||||
org: { case: "orgId", value: orgToRegisterOn },
|
||||
});
|
||||
|
||||
addHumanUserWithOrganization = create(AddHumanUserRequestSchema, {
|
||||
...addHumanUser,
|
||||
organization: organizationSchema,
|
||||
});
|
||||
} else {
|
||||
addHumanUserWithOrganization = create(AddHumanUserRequestSchema, addHumanUser);
|
||||
if (!orgToRegisterOn) {
|
||||
console.error("[IDP Process] Could not determine organization for auto-creation (no default org available)");
|
||||
const params = buildRedirectParams();
|
||||
return { redirect: `/idp/${provider}/failure?${params}&error=no_organization_context` };
|
||||
}
|
||||
|
||||
const organizationSchema = create(OrganizationSchema, {
|
||||
org: { case: "orgId", value: orgToRegisterOn },
|
||||
});
|
||||
|
||||
const addHumanUserWithOrganization = create(AddHumanUserRequestSchema, {
|
||||
...addHumanUser,
|
||||
organization: organizationSchema,
|
||||
});
|
||||
|
||||
try {
|
||||
const newUser = await addHuman({
|
||||
serviceUrl,
|
||||
@@ -394,7 +399,7 @@ export async function processIDPCallback({
|
||||
});
|
||||
|
||||
if (!orgToRegisterOn) {
|
||||
console.error("[IDP Process] Could not determine organization for registration");
|
||||
console.error("[IDP Process] Could not determine organization for registration (no default org available)");
|
||||
const params = buildRedirectParams();
|
||||
return { redirect: `/idp/${provider}/registration-failed?${params}` };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user