Skip to content

Commit

Permalink
Merge pull request #320 from zitadel/qa
Browse files Browse the repository at this point in the history
fix: Finish IDP Signup for OIDC flow
  • Loading branch information
peintnermax authored Dec 17, 2024
2 parents 188eb42 + 0fa5752 commit 0f4d31e
Show file tree
Hide file tree
Showing 8 changed files with 177 additions and 87 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions apps/login/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
155 changes: 124 additions & 31 deletions apps/login/src/app/(login)/idp/[provider]/success/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,96 @@ 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" });

return (
<DynamicTheme branding={branding}>
<div className="flex flex-col items-center space-y-4">
<h1>{t("loginError.title")}</h1>
<div className="w-full">
{<Alert type={AlertType.ALERT}>{t("loginError.title")}</Alert>}
</div>
<p className="ztdl-p">{t("loginError.description")}</p>
{error && (
<div className="w-full">
{<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 }>;
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -135,34 +191,71 @@ 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>
);
}
}

// return login failed if no linking or creation is allowed and no user was found
return loginFailed;
return loginFailed(branding, "No user found");
}
2 changes: 1 addition & 1 deletion apps/login/src/app/(login)/loginname/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
8 changes: 6 additions & 2 deletions apps/login/src/app/login/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions apps/login/src/components/idp-signin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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>
Expand Down
71 changes: 34 additions & 37 deletions apps/login/src/lib/server/cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,47 +91,44 @@ export async function createSessionForIdpAndUpdateCookie(
lifetime,
);

if (createdSession) {
return getSession({
sessionId: createdSession.sessionId,
sessionToken: createdSession.sessionToken,
}).then((response) => {
if (response?.session && response.session?.factors?.user?.loginName) {
const sessionCookie: CustomCookieData = {
id: createdSession.sessionId,
token: createdSession.sessionToken,
creationTs: response.session.creationDate
? `${timestampMs(response.session.creationDate)}`
: "",
expirationTs: response.session.expirationDate
? `${timestampMs(response.session.expirationDate)}`
: "",
changeTs: response.session.changeDate
? `${timestampMs(response.session.changeDate)}`
: "",
loginName: response.session.factors.user.loginName ?? "",
organization: response.session.factors.user.organizationId ?? "",
};
if (!createdSession) {
throw "Could not create session";
}

if (authRequestId) {
sessionCookie.authRequestId = authRequestId;
}
const { session } = await getSession({
sessionId: createdSession.sessionId,
sessionToken: createdSession.sessionToken,
});

if (response.session.factors.user.organizationId) {
sessionCookie.organization =
response.session.factors.user.organizationId;
}
if (!session || !session.factors?.user?.loginName) {
throw "Could not retrieve session";
}

return addSessionToCookie(sessionCookie).then(() => {
return response.session as Session;
});
} else {
throw "could not get session or session does not have loginName";
}
});
} else {
throw "Could not create session";
const sessionCookie: CustomCookieData = {
id: createdSession.sessionId,
token: createdSession.sessionToken,
creationTs: session.creationDate
? `${timestampMs(session.creationDate)}`
: "",
expirationTs: session.expirationDate
? `${timestampMs(session.expirationDate)}`
: "",
changeTs: session.changeDate ? `${timestampMs(session.changeDate)}` : "",
loginName: session.factors.user.loginName ?? "",
organization: session.factors.user.organizationId ?? "",
};

if (authRequestId) {
sessionCookie.authRequestId = authRequestId;
}

if (session.factors.user.organizationId) {
sessionCookie.organization = session.factors.user.organizationId;
}

return addSessionToCookie(sessionCookie).then(() => {
return session as Session;
});
}

export type SessionWithChallenges = Session & {
Expand Down
Loading

0 comments on commit 0f4d31e

Please sign in to comment.