Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(loginname page): refactor user discovery search #313

Merged
merged 25 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions acceptance/docker-compose.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
services:
zitadel:
user: "${ZITADEL_DEV_UID}"
image: "${ZITADEL_IMAGE:-ghcr.io/zitadel/zitadel:v2.65.0}"
image: "${ZITADEL_IMAGE:-ghcr.io/zitadel/zitadel:v2.67.1}"
peintnermax marked this conversation as resolved.
Show resolved Hide resolved
command: 'start-from-init --masterkey "MasterkeyNeedsToHave32Characters" --tlsMode disabled --config /zitadel.yaml --steps /zitadel.yaml'
ports:
- "8080:8080"
Expand All @@ -22,7 +22,7 @@ services:
- POSTGRES_HOST_AUTH_METHOD=trust
command: postgres -c shared_preload_libraries=pg_stat_statements -c pg_stat_statements.track=all -c shared_buffers=1GB -c work_mem=16MB -c effective_io_concurrency=100 -c wal_level=minimal -c archive_mode=off -c max_wal_senders=0
healthcheck:
test: [ "CMD-SHELL", "pg_isready" ]
test: ["CMD-SHELL", "pg_isready"]
interval: "10s"
timeout: "30s"
retries: 5
Expand Down
1 change: 0 additions & 1 deletion apps/login/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,5 @@ Timebased features like the multifactor init prompt or password expiry, are not
- Password Expiry Settings
- 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.
5 changes: 5 additions & 0 deletions apps/login/src/app/(login)/loginname/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export default async function Page(props: {
const loginName = searchParams?.loginName;
const authRequestId = searchParams?.authRequestId;
const organization = searchParams?.organization;
const suffix = searchParams?.suffix;
const submit: boolean = searchParams?.submit === "true";

let defaultOrganization;
Expand All @@ -34,6 +35,8 @@ export default async function Page(props: {
organization ?? defaultOrganization,
);

const contextLoginSettings = await getLoginSettings(organization);

const identityProviders = await getActiveIdentityProviders(
organization ?? defaultOrganization,
).then((resp) => {
Expand All @@ -54,6 +57,8 @@ export default async function Page(props: {
loginName={loginName}
authRequestId={authRequestId}
organization={organization} // stick to "organization" as we still want to do user discovery based on the searchParams not the default organization, later the organization is determined by the found user
loginSettings={contextLoginSettings}
suffix={suffix}
submit={submit}
allowRegister={!!loginSettings?.allowRegister}
>
Expand Down
5 changes: 5 additions & 0 deletions apps/login/src/app/login/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ export async function GET(request: NextRequest) {
const { authRequest } = await getAuthRequest({ authRequestId });

let organization = "";
let suffix = "";
let idpId = "";

if (authRequest?.scope) {
Expand All @@ -326,6 +327,7 @@ export async function GET(request: NextRequest) {
const orgs = await getOrgsByDomain(orgDomain);
if (orgs.result && orgs.result.length === 1) {
organization = orgs.result[0].id ?? "";
suffix = orgDomain;
}
}
}
Expand Down Expand Up @@ -448,6 +450,9 @@ export async function GET(request: NextRequest) {
if (organization) {
loginNameUrl.searchParams.set("organization", organization);
}
if (suffix) {
loginNameUrl.searchParams.set("suffix", suffix);
}
return NextResponse.redirect(loginNameUrl);
} else if (authRequest.prompt.includes(Prompt.NONE)) {
/**
Expand Down
10 changes: 9 additions & 1 deletion apps/login/src/components/input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export type TextInputProps = DetailedHTMLProps<
HTMLInputElement
> & {
label: string;
suffix?: string;
placeholder?: string;
defaultValue?: string;
error?: string | ReactNode;
Expand Down Expand Up @@ -45,6 +46,7 @@ export const TextInput = forwardRef<HTMLInputElement, TextInputProps>(
label,
placeholder,
defaultValue,
suffix,
required = false,
error,
disabled,
Expand All @@ -56,7 +58,7 @@ export const TextInput = forwardRef<HTMLInputElement, TextInputProps>(
ref,
) => {
return (
<label className="flex flex-col text-12px text-input-light-label dark:text-input-dark-label">
<label className="relative flex flex-col text-12px text-input-light-label dark:text-input-dark-label">
<span
className={`leading-3 mb-1 ${
error ? "text-warn-light-500 dark:text-warn-dark-500" : ""
Expand All @@ -78,6 +80,12 @@ export const TextInput = forwardRef<HTMLInputElement, TextInputProps>(
{...props}
/>

{suffix && (
<span className="z-30 absolute right-[3px] bottom-[22px] transform translate-y-1/2 bg-background-light-500 dark:bg-background-dark-500 p-2 rounded-sm">
@{suffix}
</span>
)}

<div className="leading-14.5px h-14.5px text-warn-light-500 dark:text-warn-dark-500 flex flex-row items-center text-12px">
<span>{error ? error : " "}</span>
</div>
Expand Down
21 changes: 20 additions & 1 deletion apps/login/src/components/username-form.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"use client";

import { sendLoginname } from "@/lib/server/loginname";
import { LoginSettings } from "@zitadel/proto/zitadel/settings/v2/login_settings_pb";
import { useTranslations } from "next-intl";
import { useRouter } from "next/navigation";
import { ReactNode, useEffect, useState } from "react";
Expand All @@ -18,7 +19,9 @@ type Inputs = {
type Props = {
loginName: string | undefined;
authRequestId: string | undefined;
loginSettings: LoginSettings | undefined;
organization?: string;
suffix?: string;
submit: boolean;
allowRegister: boolean;
children?: ReactNode;
Expand All @@ -28,6 +31,8 @@ export function UsernameForm({
loginName,
authRequestId,
organization,
suffix,
loginSettings,
submit,
allowRegister,
children,
Expand All @@ -52,6 +57,7 @@ export function UsernameForm({
loginName: values.loginName,
organization,
authRequestId,
suffix,
})
.catch(() => {
setError("An internal error occurred");
Expand Down Expand Up @@ -80,15 +86,28 @@ export function UsernameForm({
}
}, []);

let inputLabel = "Loginname";
if (
loginSettings?.disableLoginWithEmail &&
loginSettings?.disableLoginWithPhone
) {
inputLabel = "Username";
} else if (loginSettings?.disableLoginWithEmail) {
inputLabel = "Username or phone number";
} else if (loginSettings?.disableLoginWithPhone) {
inputLabel = "Username or email";
}

return (
<form className="w-full">
<div className="">
<TextInput
type="text"
autoComplete="username"
{...register("loginName", { required: "This field is required" })}
label="Loginname"
label={inputLabel}
data-testid="username-text-input"
suffix={suffix}
/>
{allowRegister && (
<button
Expand Down
109 changes: 79 additions & 30 deletions apps/login/src/lib/server/loginname.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
getOrgsByDomain,
listAuthenticationMethodTypes,
listIDPLinks,
listUsers,
searchUsers,
SearchUsersCommand,
startIdentityProviderFlow,
} from "../zitadel";
import { createSessionAndUpdateCookie } from "./cookie";
Expand All @@ -25,26 +26,36 @@ export type SendLoginnameCommand = {
loginName: string;
authRequestId?: string;
organization?: string;
suffix?: string;
};

const ORG_SUFFIX_REGEX = /(?<=@)(.+)/;

export async function sendLoginname(command: SendLoginnameCommand) {
const users = await listUsers({
loginName: command.loginName,
const loginSettingsByContext = await getLoginSettings(command.organization);

if (!loginSettingsByContext) {
return { error: "Could not get login settings" };
}

let searchUsersRequest: SearchUsersCommand = {
searchValue: command.loginName,
organizationId: command.organization,
});
loginSettings: loginSettingsByContext,
suffix: command.suffix,
};

const searchResult = await searchUsers(searchUsersRequest);

if ("error" in searchResult && searchResult.error) {
return searchResult;
}

const loginSettings = await getLoginSettings(command.organization);
if (!("result" in searchResult)) {
return { error: "Could not search users" };
}

const potentialUsers = users.result.filter((u) => {
const human = u.type.case === "human" ? u.type.value : undefined;
return loginSettings?.disableLoginWithEmail
? human?.email?.isVerified && human?.email?.email !== command.loginName
: loginSettings?.disableLoginWithPhone
? human?.phone?.isVerified && human?.phone?.phone !== command.loginName
: true;
});
const { result: potentialUsers } = searchResult;

const redirectUserToSingleIDPIfAvailable = async () => {
const identityProviders = await getActiveIdentityProviders(
Expand Down Expand Up @@ -145,9 +156,50 @@ export async function sendLoginname(command: SendLoginnameCommand) {
}
};

if (potentialUsers.length == 1 && potentialUsers[0].userId) {
if (potentialUsers.length > 1) {
return { error: "More than one user found. Provide a unique identifier." };
} else if (potentialUsers.length == 1 && potentialUsers[0].userId) {
const user = potentialUsers[0];
const userId = potentialUsers[0].userId;

const userLoginSettings = await getLoginSettings(
user.details?.resourceOwner,
);

// compare with the concatenated suffix when set
const concatLoginname = command.suffix
? `${command.loginName}@${command.suffix}`
: command.loginName;

const humanUser =
potentialUsers[0].type.case === "human"
? potentialUsers[0].type.value
: undefined;

// recheck login settings after user discovery, as the search might have been done without org scope
if (
userLoginSettings?.disableLoginWithEmail &&
userLoginSettings?.disableLoginWithPhone
) {
if (user.preferredLoginName !== concatLoginname) {
return { error: "User not found in the system!" };
}
} else if (userLoginSettings?.disableLoginWithEmail) {
if (
user.preferredLoginName !== concatLoginname ||
humanUser?.phone?.phone !== command.loginName
) {
return { error: "User not found in the system!" };
}
} else if (userLoginSettings?.disableLoginWithPhone) {
if (
user.preferredLoginName !== concatLoginname ||
humanUser?.email?.email !== command.loginName
) {
return { error: "User not found in the system!" };
}
}

const checks = create(ChecksSchema, {
user: { search: { case: "userId", value: userId } },
});
Expand All @@ -163,7 +215,7 @@ export async function sendLoginname(command: SendLoginnameCommand) {
}

// TODO: check if handling of userstate INITIAL is needed
if (potentialUsers[0].state === UserState.INITIAL) {
if (user.state === UserState.INITIAL) {
return { error: "Initial User not supported" };
}

Expand All @@ -173,11 +225,6 @@ export async function sendLoginname(command: SendLoginnameCommand) {

// this can be expected to be an invite as users created in console have a password set.
if (!methods.authMethodTypes || !methods.authMethodTypes.length) {
const humanUser =
potentialUsers[0].type.case === "human"
? potentialUsers[0].type.value
: undefined;

// redirect to /verify invite if no auth method is set and email is not verified
const inviteCheck = checkInvite(
session,
Expand Down Expand Up @@ -213,7 +260,7 @@ export async function sendLoginname(command: SendLoginnameCommand) {
const method = methods.authMethodTypes[0];
switch (method) {
case AuthenticationMethodType.PASSWORD: // user has only password as auth method
if (!loginSettings?.allowUsernamePassword) {
if (!userLoginSettings?.allowUsernamePassword) {
return {
error:
"Username Password not allowed! Contact your administrator for more information.",
Expand All @@ -240,7 +287,7 @@ export async function sendLoginname(command: SendLoginnameCommand) {
};

case AuthenticationMethodType.PASSKEY: // AuthenticationMethodType.AUTHENTICATION_METHOD_TYPE_PASSKEY
if (loginSettings?.passkeysType === PasskeysType.NOT_ALLOWED) {
if (userLoginSettings?.passkeysType === PasskeysType.NOT_ALLOWED) {
return {
error:
"Passkeys not allowed! Contact your administrator for more information.",
Expand Down Expand Up @@ -303,22 +350,24 @@ export async function sendLoginname(command: SendLoginnameCommand) {
}
}

// user not found, check if register is enabled on organization
if (loginSettings?.allowRegister && !loginSettings?.allowUsernamePassword) {
// TODO: do we need to handle login hints for IDPs here?
// user not found, check if register is enabled on instance / organization context
if (
loginSettingsByContext?.allowRegister &&
!loginSettingsByContext?.allowUsernamePassword
) {
const resp = await redirectUserToSingleIDPIfAvailable();
if (resp) {
return resp;
}
return { error: "User not found in the system" };
} else if (
loginSettings?.allowRegister &&
loginSettings?.allowUsernamePassword
loginSettingsByContext?.allowRegister &&
loginSettingsByContext?.allowUsernamePassword
) {
let orgToRegisterOn: string | undefined = command.organization;

if (
!loginSettings?.ignoreUnknownUsernames &&
!loginSettingsByContext?.ignoreUnknownUsernames &&
!orgToRegisterOn &&
command.loginName &&
ORG_SUFFIX_REGEX.test(command.loginName)
Expand All @@ -338,7 +387,7 @@ export async function sendLoginname(command: SendLoginnameCommand) {
}

// do not register user if ignoreUnknownUsernames is set
if (orgToRegisterOn && !loginSettings?.ignoreUnknownUsernames) {
if (orgToRegisterOn && !loginSettingsByContext?.ignoreUnknownUsernames) {
const params = new URLSearchParams({ organization: orgToRegisterOn });

if (command.authRequestId) {
Expand All @@ -353,7 +402,7 @@ export async function sendLoginname(command: SendLoginnameCommand) {
}
}

if (loginSettings?.ignoreUnknownUsernames) {
if (loginSettingsByContext?.ignoreUnknownUsernames) {
const paramsPasswordDefault = new URLSearchParams({
loginName: command.loginName,
});
Expand Down
Loading
Loading