mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-30 02:52:16 +00:00
fix(login): Prevent IDP auto-creation failure when name fields are missing (#11070)
# Which Problems Are Solved
When using IDP auto-creation, the addHuman() call would fail if the IDP
didn't provide required profile fields (givenName or familyName),
resulting in a poor user experience.
# How the Problems Are Solved
Added validation before auto-creation to check if required profile
fields are present. If givenName or familyName is missing, users are now
redirected to the complete-registration page where they can manually
provide the missing information.
- Added profile field validation in CASE 4 (auto-creation) of
processIDPCallback
- Redirect to /idp/{provider}/complete-registration when required fields
are missing
- Pre-fill any available user data in the registration form
---------
Co-authored-by: David Skewis <[email protected]>
Co-authored-by: Florian Forster <[email protected]>
This commit is contained in:
co-authored by
David Skewis
Florian Forster
parent
f96080a4c1
commit
14874d6546
@@ -736,6 +736,121 @@ describe("processIDPCallback", () => {
|
||||
|
||||
expect(result.error).toBe("Session error");
|
||||
});
|
||||
|
||||
test("should redirect to complete registration when givenName is missing", async () => {
|
||||
mockRetrieveIDPIntent.mockResolvedValue({
|
||||
...defaultIntent,
|
||||
userId: undefined,
|
||||
addHumanUser: {
|
||||
...defaultIntent.addHumanUser,
|
||||
profile: {
|
||||
givenName: "",
|
||||
familyName: "User",
|
||||
displayName: "User",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = await processIDPCallback(defaultParams);
|
||||
|
||||
expect(mockAddHuman).not.toHaveBeenCalled();
|
||||
expect(result.redirect).toContain("/idp/google/complete-registration");
|
||||
expect(result.redirect).toContain("id=intent123");
|
||||
expect(result.redirect).toContain("token=token123");
|
||||
expect(result.redirect).toContain("organization=org123");
|
||||
expect(result.redirect).toContain("familyName=User");
|
||||
});
|
||||
|
||||
test("should redirect to complete registration when familyName is missing", async () => {
|
||||
mockRetrieveIDPIntent.mockResolvedValue({
|
||||
...defaultIntent,
|
||||
userId: undefined,
|
||||
addHumanUser: {
|
||||
...defaultIntent.addHumanUser,
|
||||
profile: {
|
||||
givenName: "Test",
|
||||
familyName: "",
|
||||
displayName: "Test",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = await processIDPCallback(defaultParams);
|
||||
|
||||
expect(mockAddHuman).not.toHaveBeenCalled();
|
||||
expect(result.redirect).toContain("/idp/google/complete-registration");
|
||||
expect(result.redirect).toContain("id=intent123");
|
||||
expect(result.redirect).toContain("token=token123");
|
||||
expect(result.redirect).toContain("organization=org123");
|
||||
expect(result.redirect).toContain("givenName=Test");
|
||||
});
|
||||
|
||||
test("should redirect to complete registration when both givenName and familyName are missing", async () => {
|
||||
mockRetrieveIDPIntent.mockResolvedValue({
|
||||
...defaultIntent,
|
||||
userId: undefined,
|
||||
addHumanUser: {
|
||||
...defaultIntent.addHumanUser,
|
||||
profile: {
|
||||
givenName: "",
|
||||
familyName: "",
|
||||
displayName: "",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = await processIDPCallback(defaultParams);
|
||||
|
||||
expect(mockAddHuman).not.toHaveBeenCalled();
|
||||
expect(result.redirect).toContain("/idp/google/complete-registration");
|
||||
expect(result.redirect).toContain("id=intent123");
|
||||
expect(result.redirect).toContain("token=token123");
|
||||
expect(result.redirect).toContain("organization=org123");
|
||||
});
|
||||
|
||||
test("should redirect to complete registration when profile is undefined", async () => {
|
||||
mockRetrieveIDPIntent.mockResolvedValue({
|
||||
...defaultIntent,
|
||||
userId: undefined,
|
||||
addHumanUser: {
|
||||
...defaultIntent.addHumanUser,
|
||||
profile: undefined,
|
||||
},
|
||||
});
|
||||
|
||||
const result = await processIDPCallback(defaultParams);
|
||||
|
||||
expect(mockAddHuman).not.toHaveBeenCalled();
|
||||
expect(result.redirect).toContain("/idp/google/complete-registration");
|
||||
expect(result.redirect).toContain("id=intent123");
|
||||
expect(result.redirect).toContain("token=token123");
|
||||
});
|
||||
|
||||
test("should use fallback organization when required fields missing and no org provided", async () => {
|
||||
mockRetrieveIDPIntent.mockResolvedValue({
|
||||
...defaultIntent,
|
||||
userId: undefined,
|
||||
addHumanUser: {
|
||||
...defaultIntent.addHumanUser,
|
||||
profile: {
|
||||
givenName: "",
|
||||
familyName: "User",
|
||||
displayName: "User",
|
||||
},
|
||||
},
|
||||
});
|
||||
mockGetDefaultOrg.mockResolvedValue({ id: "default-org" });
|
||||
|
||||
const result = await processIDPCallback({
|
||||
...defaultParams,
|
||||
organization: undefined,
|
||||
});
|
||||
|
||||
expect(mockGetDefaultOrg).toHaveBeenCalled();
|
||||
expect(mockAddHuman).not.toHaveBeenCalled();
|
||||
expect(result.redirect).toContain("/idp/google/complete-registration");
|
||||
expect(result.redirect).toContain("organization=default-org");
|
||||
});
|
||||
});
|
||||
|
||||
describe("CASE 5: Manual user creation allowed", () => {
|
||||
|
||||
@@ -452,7 +452,7 @@ async function handleAutoLinking(ctx: IDPHandlerContext): Promise<IDPHandlerResu
|
||||
*/
|
||||
async function handleAutoCreation(ctx: IDPHandlerContext): Promise<IDPHandlerResult> {
|
||||
const { options, intent, serviceConfig, buildRedirectParams, t } = ctx;
|
||||
const { addHumanUser } = intent;
|
||||
const { addHumanUser, idpInformation } = intent;
|
||||
const { organization, provider } = ctx.params;
|
||||
|
||||
if (options?.isAutoCreation && addHumanUser) {
|
||||
@@ -468,6 +468,32 @@ async function handleAutoCreation(ctx: IDPHandlerContext): Promise<IDPHandlerRes
|
||||
return { redirect: `/idp/${provider}/failure?${params}&error=no_organization_context` };
|
||||
}
|
||||
|
||||
// Check if required profile fields are present
|
||||
if (!addHumanUser.profile?.givenName || !addHumanUser.profile?.familyName) {
|
||||
logger.info("Missing required profile fields (givenName or familyName), redirecting to complete registration");
|
||||
|
||||
if (!idpInformation!.userId) {
|
||||
logger.error("IDP intent missing userId, cannot redirect to complete registration");
|
||||
const params = buildRedirectParams();
|
||||
return { redirect: `/idp/${provider}/failure?${params}&error=missing_idp_user_info` };
|
||||
}
|
||||
|
||||
const params = buildRedirectParams(
|
||||
{
|
||||
organization: orgToRegisterOn,
|
||||
idpId: idpInformation!.idpId,
|
||||
idpUserId: idpInformation!.userId,
|
||||
idpUserName: idpInformation!.userName || "",
|
||||
// User data for pre-filling form
|
||||
givenName: addHumanUser.profile?.givenName || "",
|
||||
familyName: addHumanUser.profile?.familyName || "",
|
||||
email: addHumanUser.email?.email || "",
|
||||
},
|
||||
true,
|
||||
);
|
||||
return { redirect: `/idp/${provider}/complete-registration?${params}` };
|
||||
}
|
||||
|
||||
const organizationSchema = create(OrganizationSchema, {
|
||||
org: { case: "orgId", value: orgToRegisterOn },
|
||||
});
|
||||
@@ -542,11 +568,17 @@ async function handleManualCreation(ctx: IDPHandlerContext): Promise<IDPHandlerR
|
||||
// Store user data for manual registration form
|
||||
// Note: includeToken=true because the session hasn't been created yet
|
||||
// The token will be needed when registerUserAndLinkToIDP creates the session
|
||||
if (!idpInformation!.userId) {
|
||||
logger.error("IDP intent missing userId, cannot redirect to complete registration");
|
||||
const params = buildRedirectParams();
|
||||
return { redirect: `/idp/${provider}/failure?${params}&error=missing_idp_user_info` };
|
||||
}
|
||||
|
||||
const params = buildRedirectParams(
|
||||
{
|
||||
organization: orgToRegisterOn,
|
||||
idpId: idpInformation!.idpId,
|
||||
idpUserId: idpInformation!.userId || "",
|
||||
idpUserId: idpInformation!.userId,
|
||||
idpUserName: idpInformation!.userName || "",
|
||||
// User data for pre-filling form
|
||||
givenName: addHumanUser.profile?.givenName || "",
|
||||
|
||||
Reference in New Issue
Block a user