Merge pull request #320 from zitadel/qa

fix: Finish IDP Signup for OIDC flow

README.md

@@ -60,9 +60,9 @@ You can already use the current state, and extend it with your needs.
   - [x] GitLab
   - [x] GitLab Enterprise
   - [x] Azure
-  - [ ] Apple
+  - [x] Apple
   - [x] Generic OIDC
-  - [ ] Generic OAuth
+  - [x] Generic OAuth
   - [ ] Generic JWT
   - [ ] LDAP
   - [ ] SAML SP

apps/login/readme.md

@@ -396,3 +396,5 @@ Timebased features like the multifactor init prompt or password expiry, are not
 - Login Settings: multifactor init prompt
 - forceMFA on login settings is not checked for IDPs
 - disablePhone / disableEmail from loginSettings will be implemented right after https://github.com/zitadel/zitadel/issues/9016 is merged
+
+Also note that IDP logins are considered as valid MFA. An additional MFA check will be implemented in future if enforced.

apps/login/src/app/(login)/idp/[provider]/success/page.tsx

@@ -3,18 +3,28 @@ import { DynamicTheme } from "@/components/dynamic-theme";
 import { IdpSignin } from "@/components/idp-signin";
 import { idpTypeToIdentityProviderType, PROVIDER_MAPPING } from "@/lib/idp";
 import {
+  addHuman,
   addIDPLink,
-  createUser,
   getBrandingSettings,
   getIDPByID,
+  getLoginSettings,
+  getOrgsByDomain,
   listUsers,
   retrieveIDPIntent,
 } from "@/lib/zitadel";
+import { create } from "@zitadel/client";
 import { AutoLinkingOption } from "@zitadel/proto/zitadel/idp/v2/idp_pb";
+import { OrganizationSchema } from "@zitadel/proto/zitadel/object/v2/object_pb";
 import { BrandingSettings } from "@zitadel/proto/zitadel/settings/v2/branding_settings_pb";
+import {
+  AddHumanUserRequest,
+  AddHumanUserRequestSchema,
+} from "@zitadel/proto/zitadel/user/v2/user_service_pb";
 import { getLocale, getTranslations } from "next-intl/server";
 
-async function loginFailed(branding?: BrandingSettings) {
+const ORG_SUFFIX_REGEX = /(?<=@)(.+)/;
+
+async function loginFailed(branding?: BrandingSettings, error: string = "") {
   const locale = getLocale();
   const t = await getTranslations({ locale, namespace: "idp" });
 
@@ -22,13 +32,67 @@ async function loginFailed(branding?: BrandingSettings) {
     <DynamicTheme branding={branding}>
       <div className="flex flex-col items-center space-y-4">
         <h1>{t("loginError.title")}</h1>
+        <p className="ztdl-p">{t("loginError.description")}</p>
+        {error && (
           <div className="w-full">
-          {<Alert type={AlertType.ALERT}>{t("loginError.title")}</Alert>}
+            {<Alert type={AlertType.ALERT}>{error}</Alert>}
           </div>
+        )}
       </div>
     </DynamicTheme>
   );
 }
+
+async function loginSuccess(
+  userId: string,
+  idpIntent: { idpIntentId: string; idpIntentToken: string },
+  authRequestId?: string,
+  branding?: BrandingSettings,
+) {
+  const locale = getLocale();
+  const t = await getTranslations({ locale, namespace: "idp" });
+
+  return (
+    <DynamicTheme branding={branding}>
+      <div className="flex flex-col items-center space-y-4">
+        <h1>{t("loginSuccess.title")}</h1>
+        <p className="ztdl-p">{t("loginSuccess.description")}</p>
+
+        <IdpSignin
+          userId={userId}
+          idpIntent={idpIntent}
+          authRequestId={authRequestId}
+        />
+      </div>
+    </DynamicTheme>
+  );
+}
+
+async function linkingSuccess(
+  userId: string,
+  idpIntent: { idpIntentId: string; idpIntentToken: string },
+  authRequestId?: string,
+  branding?: BrandingSettings,
+) {
+  const locale = getLocale();
+  const t = await getTranslations({ locale, namespace: "idp" });
+
+  return (
+    <DynamicTheme branding={branding}>
+      <div className="flex flex-col items-center space-y-4">
+        <h1>{t("linkingSuccess.title")}</h1>
+        <p className="ztdl-p">{t("linkingSuccess.description")}</p>
+
+        <IdpSignin
+          userId={userId}
+          idpIntent={idpIntent}
+          authRequestId={authRequestId}
+        />
+      </div>
+    </DynamicTheme>
+  );
+}
+
 export default async function Page(props: {
   searchParams: Promise<Record<string | number | symbol, string | undefined>>;
   params: Promise<{ provider: string }>;
@@ -43,7 +107,7 @@ export default async function Page(props: {
   const branding = await getBrandingSettings(organization);
 
   if (!provider || !id || !token) {
-    return loginFailed(branding);
+    return loginFailed(branding, "IDP context missing");
   }
 
   const intent = await retrieveIDPIntent(id, token);
@@ -54,24 +118,16 @@ export default async function Page(props: {
   if (userId && !link) {
     // TODO: update user if idp.options.isAutoUpdate is true
 
-    return (
-      <DynamicTheme branding={branding}>
-        <div className="flex flex-col items-center space-y-4">
-          <h1>{t("loginSuccess.title")}</h1>
-          <div>{t("loginSuccess.description")}</div>
-
-          <IdpSignin
-            userId={userId}
-            idpIntent={{ idpIntentId: id, idpIntentToken: token }}
-            authRequestId={authRequestId}
-          />
-        </div>
-      </DynamicTheme>
+    return loginSuccess(
+      userId,
+      { idpIntentId: id, idpIntentToken: token },
+      authRequestId,
+      branding,
     );
   }
 
   if (!idpInformation) {
-    return loginFailed(branding);
+    return loginFailed(branding, "IDP information missing");
   }
 
   const idp = await getIDPByID(idpInformation.idpId);
@@ -135,28 +191,65 @@ export default async function Page(props: {
       });
 
       if (idpLink) {
-        return (
-          // TODO: possibily login user now
-          <DynamicTheme branding={branding}>
-            <div className="flex flex-col items-center space-y-4">
-              <h1>{t("linkingSuccess.title")}</h1>
-              <div>{t("linkingSuccess.description")}</div>
-            </div>
-          </DynamicTheme>
+        return linkingSuccess(
+          foundUser.userId,
+          { idpIntentId: id, idpIntentToken: token },
+          authRequestId,
+          branding,
         );
       }
     }
   }
 
   if (options?.isCreationAllowed && options.isAutoCreation) {
-    const newUser = await createUser(providerType, idpInformation);
+    let orgToRegisterOn: string | undefined = organization;
+
+    let userData: AddHumanUserRequest =
+      PROVIDER_MAPPING[providerType](idpInformation);
+
+    if (
+      !orgToRegisterOn &&
+      userData.username && // username or email?
+      ORG_SUFFIX_REGEX.test(userData.username)
+    ) {
+      const matched = ORG_SUFFIX_REGEX.exec(userData.username);
+      const suffix = matched?.[1] ?? "";
+
+      // this just returns orgs where the suffix is set as primary domain
+      const orgs = await getOrgsByDomain(suffix);
+      const orgToCheckForDiscovery =
+        orgs.result && orgs.result.length === 1 ? orgs.result[0].id : undefined;
+
+      const orgLoginSettings = await getLoginSettings(orgToCheckForDiscovery);
+      if (orgLoginSettings?.allowDomainDiscovery) {
+        orgToRegisterOn = orgToCheckForDiscovery;
+      }
+    }
+
+    if (orgToRegisterOn) {
+      const organizationSchema = create(OrganizationSchema, {
+        org: { case: "orgId", value: orgToRegisterOn },
+      });
+
+      userData = create(AddHumanUserRequestSchema, {
+        ...userData,
+        organization: organizationSchema,
+      });
+    }
+
+    const newUser = await addHuman(userData);
 
     if (newUser) {
       return (
         <DynamicTheme branding={branding}>
           <div className="flex flex-col items-center space-y-4">
             <h1>{t("registerSuccess.title")}</h1>
-            <div>{t("registerSuccess.description")}</div>
+            <p className="ztdl-p">{t("registerSuccess.description")}</p>
+            <IdpSignin
+              userId={newUser.userId}
+              idpIntent={{ idpIntentId: id, idpIntentToken: token }}
+              authRequestId={authRequestId}
+            />
           </div>
         </DynamicTheme>
       );
@@ -164,5 +257,5 @@ export default async function Page(props: {
   }
 
   // return login failed if no linking or creation is allowed and no user was found
-  return loginFailed;
+  return loginFailed(branding, "No user found");
 }

apps/login/src/app/(login)/loginname/page.tsx

@@ -61,7 +61,7 @@ export default async function Page(props: {
             <SignInWithIdp
               identityProviders={identityProviders}
               authRequestId={authRequestId}
-              organization={organization ?? defaultOrganization} // use the organization from the searchParams here otherwise fallback to the default organization
+              organization={organization}
             ></SignInWithIdp>
           )}
         </UsernameForm>

apps/login/src/app/login/route.ts

@@ -109,9 +109,10 @@ async function isSessionValid(session: Session): Promise<boolean> {
       const otpSms = session.factors.otpSms?.verifiedAt;
       const totp = session.factors.totp?.verifiedAt;
       const webAuthN = session.factors.webAuthN?.verifiedAt;
+      const idp = session.factors.intent?.verifiedAt; // TODO: forceMFA should not consider this as valid factor
 
       // must have one single check
-      mfaValid = !!(otpEmail || otpSms || totp || webAuthN);
+      mfaValid = !!(otpEmail || otpSms || totp || webAuthN || idp);
       if (!mfaValid) {
         console.warn("Session has no valid multifactor", session.factors);
       }
@@ -207,8 +208,11 @@ export async function GET(request: NextRequest) {
 
       const isValid = await isSessionValid(selectedSession);
 
+      console.log("Session is valid:", isValid);
+
       if (!isValid && selectedSession.factors?.user) {
-        // if the session is not valid anymore, we need to redirect the user to re-authenticate
+        // if the session is not valid anymore, we need to redirect the user to re-authenticate /
+        // TODO: handle IDP intent direcly if available
         const command: SendLoginnameCommand = {
           loginName: selectedSession.factors.user?.loginName,
           organization: selectedSession.factors?.user?.organizationId,

apps/login/src/components/idp-signin.tsx

@@ -21,7 +21,7 @@ export function IdpSignin({
   idpIntent: { idpIntentId, idpIntentToken },
   authRequestId,
 }: Props) {
-  const [loading, setLoading] = useState(false);
+  const [loading, setLoading] = useState(true);
   const [error, setError] = useState<string | null>(null);
 
   const router = useRouter();
@@ -55,8 +55,8 @@ export function IdpSignin({
   }, []);
 
   return (
-    <div className="flex items-center justify-center">
-      {loading && <Spinner />}
+    <div className="flex items-center justify-center py-4">
+      {loading && <Spinner className="h-5 w-5" />}
       {error && (
         <div className="py-4">
           <Alert>{error}</Alert>

apps/login/src/lib/server/cookie.ts

@@ -91,47 +91,44 @@ export async function createSessionForIdpAndUpdateCookie(
     lifetime,
   );
 
-  if (createdSession) {
-    return getSession({
+  if (!createdSession) {
+    throw "Could not create session";
+  }
+
+  const { session } = await getSession({
     sessionId: createdSession.sessionId,
     sessionToken: createdSession.sessionToken,
-    }).then((response) => {
-      if (response?.session && response.session?.factors?.user?.loginName) {
+  });
+
+  if (!session || !session.factors?.user?.loginName) {
+    throw "Could not retrieve session";
+  }
+
   const sessionCookie: CustomCookieData = {
     id: createdSession.sessionId,
     token: createdSession.sessionToken,
-          creationTs: response.session.creationDate
-            ? `${timestampMs(response.session.creationDate)}`
+    creationTs: session.creationDate
+      ? `${timestampMs(session.creationDate)}`
       : "",
-          expirationTs: response.session.expirationDate
-            ? `${timestampMs(response.session.expirationDate)}`
+    expirationTs: session.expirationDate
+      ? `${timestampMs(session.expirationDate)}`
       : "",
-          changeTs: response.session.changeDate
-            ? `${timestampMs(response.session.changeDate)}`
-            : "",
-          loginName: response.session.factors.user.loginName ?? "",
-          organization: response.session.factors.user.organizationId ?? "",
+    changeTs: session.changeDate ? `${timestampMs(session.changeDate)}` : "",
+    loginName: session.factors.user.loginName ?? "",
+    organization: session.factors.user.organizationId ?? "",
   };
 
   if (authRequestId) {
     sessionCookie.authRequestId = authRequestId;
   }
 
-        if (response.session.factors.user.organizationId) {
-          sessionCookie.organization =
-            response.session.factors.user.organizationId;
+  if (session.factors.user.organizationId) {
+    sessionCookie.organization = session.factors.user.organizationId;
   }
 
   return addSessionToCookie(sessionCookie).then(() => {
-          return response.session as Session;
+    return session as Session;
   });
-      } else {
-        throw "could not get session or session does not have loginName";
-      }
-    });
-  } else {
-    throw "Could not create session";
-  }
 }
 
 export type SessionWithChallenges = Session & {

apps/login/src/lib/zitadel.ts

@@ -10,8 +10,8 @@ import {
 import { createServerTransport } from "@zitadel/node";
 import { RequestChallenges } from "@zitadel/proto/zitadel/session/v2/challenge_pb";
 import { Checks } from "@zitadel/proto/zitadel/session/v2/session_service_pb";
-import { IDPInformation } from "@zitadel/proto/zitadel/user/v2/idp_pb";
 import {
+  AddHumanUserRequest,
   RetrieveIdentityProviderIntentRequest,
   SetPasswordRequest,
   SetPasswordRequestSchema,
@@ -23,7 +23,6 @@ import { create, Duration } from "@zitadel/client";
 import { TextQueryMethod } from "@zitadel/proto/zitadel/object/v2/object_pb";
 import { CreateCallbackRequest } from "@zitadel/proto/zitadel/oidc/v2/oidc_service_pb";
 import { Organization } from "@zitadel/proto/zitadel/org/v2/org_pb";
-import { IdentityProviderType } from "@zitadel/proto/zitadel/settings/v2/login_settings_pb";
 import type { RedirectURLsJson } from "@zitadel/proto/zitadel/user/v2/idp_pb";
 import {
   NotificationType,
@@ -39,7 +38,6 @@ import {
   UserState,
 } from "@zitadel/proto/zitadel/user/v2/user_pb";
 import { unstable_cacheLife as cacheLife } from "next/cache";
-import { PROVIDER_MAPPING } from "./idp";
 
 const transport = createServerTransport(
   process.env.ZITADEL_SERVICE_USER_TOKEN!,
@@ -249,6 +247,10 @@ export async function addHumanUser({
   });
 }
 
+export async function addHuman(request: AddHumanUserRequest) {
+  return userService.addHumanUser(request);
+}
+
 export async function verifyTOTPRegistration(code: string, userId: string) {
   return userService.verifyTOTPRegistration({ code, userId }, {});
 }
@@ -487,14 +489,6 @@ export function addIDPLink(
   );
 }
 
-export function createUser(
-  provider: IdentityProviderType,
-  info: IDPInformation,
-) {
-  const userData = PROVIDER_MAPPING[provider](info);
-  return userService.addHumanUser(userData, {});
-}
-
 /**
  *
  * @param userId the id of the user where the email should be set