fix(auth): improve security key login flow.

- Fix login to verify password before showing security key prompt
- Add proper 2FA verification flow when deleting security keys

Previously, users with security keys would see the security key prompt
even if they entered an incorrect password. Now the password is verified
first. Additionally, security key deletion now properly handles 2FA
verification when enabled.
This commit is contained in:
Adrian Astles 2025-07-07 17:48:23 +08:00
parent 813992141a
commit f0a1c10ec5
4 changed files with 294 additions and 155 deletions

View file

@ -92,22 +92,6 @@ export async function login(
const existingUser = existingUserRes[0];
// Check if user has security keys registered
const userSecurityKeys = await db
.select()
.from(securityKeys)
.where(eq(securityKeys.userId, existingUser.userId));
if (userSecurityKeys.length > 0) {
return response<{ useSecurityKey: boolean }>(res, {
data: { useSecurityKey: true },
success: true,
error: false,
message: "Please use your security key to sign in",
status: HttpCode.UNAUTHORIZED
});
}
const validPassword = await verifyPassword(
password,
existingUser.passwordHash!
@ -126,6 +110,22 @@ export async function login(
);
}
// Check if user has security keys registered
const userSecurityKeys = await db
.select()
.from(securityKeys)
.where(eq(securityKeys.userId, existingUser.userId));
if (userSecurityKeys.length > 0) {
return response<LoginResponse>(res, {
data: { useSecurityKey: true },
success: true,
error: false,
message: "Security key authentication required",
status: HttpCode.OK
});
}
if (existingUser.twoFactorEnabled) {
if (!code) {
return response<{ codeRequested: boolean }>(res, {

View file

@ -30,6 +30,7 @@ import config from "@server/lib/config";
import { UserType } from "@server/types/UserTypes";
import { verifyPassword } from "@server/auth/password";
import { unauthorized } from "@server/auth/unauthorizedResponse";
import { verifyTotpCode } from "@server/auth/totp";
// The RP ID is the domain name of your application
const rpID = (() => {
@ -120,7 +121,8 @@ export const verifyAuthenticationBody = z.object({
}).strict();
export const deleteSecurityKeyBody = z.object({
password: z.string().min(1)
password: z.string().min(1),
code: z.string().optional()
}).strict();
export async function startRegistration(
@ -159,17 +161,6 @@ export async function startRegistration(
return next(unauthorized());
}
// If user has 2FA enabled, require a code
if (user.twoFactorEnabled) {
return response<{ codeRequested: boolean }>(res, {
data: { codeRequested: true },
success: true,
error: false,
message: "Two-factor authentication required",
status: HttpCode.ACCEPTED
});
}
// Get existing security keys for user
const existingSecurityKeys = await db
.select()
@ -373,7 +364,7 @@ export async function deleteSecurityKey(
);
}
const { password } = parsedBody.data;
const { password, code } = parsedBody.data;
// Only allow internal users to use security keys
if (user.type !== UserType.Internal) {
@ -392,15 +383,37 @@ export async function deleteSecurityKey(
return next(unauthorized());
}
// If user has 2FA enabled, require a code
// If user has 2FA enabled, require and verify the code
if (user.twoFactorEnabled) {
return response<{ codeRequested: boolean }>(res, {
data: { codeRequested: true },
success: true,
error: false,
message: "Two-factor authentication required",
status: HttpCode.ACCEPTED
});
if (!code) {
return response<{ codeRequested: boolean }>(res, {
data: { codeRequested: true },
success: true,
error: false,
message: "Two-factor authentication required",
status: HttpCode.ACCEPTED
});
}
const validOTP = await verifyTotpCode(
code,
user.twoFactorSecret!,
user.userId
);
if (!validOTP) {
if (config.getRawConfig().app.log_failed_attempts) {
logger.info(
`Two-factor code incorrect. Email: ${user.email}. IP: ${req.ip}.`
);
}
return next(
createHttpError(
HttpCode.UNAUTHORIZED,
"The two-factor code you entered is incorrect"
)
);
}
}
await db

View file

@ -98,108 +98,12 @@ export default function LoginForm({ redirect, onLogin, idps }: LoginFormProps) {
async function initiateSecurityKeyAuth() {
setShowSecurityKeyPrompt(true);
setError(null);
await loginWithSecurityKey();
setShowSecurityKeyPrompt(false);
}
async function onSubmit(values: any) {
const { email, password } = form.getValues();
const { code } = mfaForm.getValues();
setLoading(true);
setError(null);
try {
const res = await api.post<AxiosResponse<LoginResponse>>("/auth/login", {
email,
password,
code
});
if (res) {
setError(null);
const data = res.data.data;
if (data?.useSecurityKey) {
setShowSecurityKeyPrompt(true);
return;
}
if (data?.codeRequested) {
setMfaRequested(true);
setLoading(false);
mfaForm.reset();
return;
}
if (data?.emailVerificationRequired) {
if (redirect) {
router.push(`/auth/verify-email?redirect=${redirect}`);
} else {
router.push("/auth/verify-email");
}
return;
}
if (onLogin) {
await onLogin();
}
}
} catch (e) {
console.error(e);
const errorMessage = formatAxiosError(e, t('loginError'));
if (errorMessage.includes("Please use your security key")) {
await initiateSecurityKeyAuth();
return;
}
setError(errorMessage);
}
setLoading(false);
}
async function loginWithIdp(idpId: number) {
try {
const res = await api.post<AxiosResponse<GenerateOidcUrlResponse>>(
`/auth/idp/${idpId}/oidc/generate-url`,
{
redirectUrl: redirect || "/"
}
);
console.log(res);
if (!res) {
setError(t('loginError'));
return;
}
const data = res.data.data;
window.location.href = data.redirectUrl;
} catch (e) {
console.error(formatAxiosError(e));
}
}
async function loginWithSecurityKey() {
try {
// Check browser compatibility first
if (!window.PublicKeyCredential) {
setError(t('securityKeyBrowserNotSupported', {
defaultValue: "Your browser doesn't support security keys. Please use a modern browser like Chrome, Firefox, or Safari."
}));
return;
}
setLoading(true);
setError(null);
const email = form.getValues().email;
// Start WebAuthn authentication
const startRes = await api.post("/auth/security-key/authenticate/start", {
email: email || undefined
});
// Start WebAuthn authentication without email
const startRes = await api.post("/auth/security-key/authenticate/start", {});
if (!startRes) {
setError(t('securityKeyAuthError', {
@ -250,12 +154,104 @@ export default function LoginForm({ redirect, onLogin, idps }: LoginFormProps) {
defaultValue: "There was a problem using your security key. Please try again."
}));
}
throw error; // Re-throw to be caught by outer catch
}
} catch (e) {
console.error(formatAxiosError(e));
} catch (e: any) {
if (e.isAxiosError) {
setError(formatAxiosError(e, t('securityKeyAuthError', {
defaultValue: "Failed to authenticate with security key"
})));
} else {
console.error(e);
setError(e.message || t('securityKeyAuthError', {
defaultValue: "Failed to authenticate with security key"
}));
}
} finally {
setLoading(false);
setShowSecurityKeyPrompt(false);
}
}
async function onSubmit(values: any) {
const { email, password } = form.getValues();
const { code } = mfaForm.getValues();
setLoading(true);
setError(null);
setShowSecurityKeyPrompt(false);
try {
const res = await api.post<AxiosResponse<LoginResponse>>("/auth/login", {
email,
password,
code
});
const data = res.data.data;
if (data?.useSecurityKey) {
await initiateSecurityKeyAuth();
return;
}
if (data?.codeRequested) {
setMfaRequested(true);
setLoading(false);
mfaForm.reset();
return;
}
if (data?.emailVerificationRequired) {
if (redirect) {
router.push(`/auth/verify-email?redirect=${redirect}`);
} else {
router.push("/auth/verify-email");
}
return;
}
if (onLogin) {
await onLogin();
}
} catch (e: any) {
if (e.isAxiosError) {
const errorMessage = formatAxiosError(e, t('loginError', {
defaultValue: "Failed to log in"
}));
setError(errorMessage);
return;
} else {
console.error(e);
setError(e.message || t('loginError', {
defaultValue: "Failed to log in"
}));
return;
}
} finally {
setLoading(false);
}
}
async function loginWithIdp(idpId: number) {
try {
const res = await api.post<AxiosResponse<GenerateOidcUrlResponse>>(
`/auth/idp/${idpId}/oidc/generate-url`,
{
redirectUrl: redirect || "/"
}
);
console.log(res);
if (!res) {
setError(t('loginError'));
return;
}
const data = res.data.data;
window.location.href = data.redirectUrl;
} catch (e) {
console.error(formatAxiosError(e));
}
}
@ -321,6 +317,16 @@ export default function LoginForm({ redirect, onLogin, idps }: LoginFormProps) {
</Link>
</div>
</div>
<div className="flex flex-col space-y-2">
<Button type="submit" disabled={loading}>
{loading ? t('idpConnectingToProcess', {
defaultValue: "Connecting..."
}) : t('login', {
defaultValue: "Log in"
})}
</Button>
</div>
</form>
</Form>
</>
@ -415,17 +421,6 @@ export default function LoginForm({ redirect, onLogin, idps }: LoginFormProps) {
{!mfaRequested && (
<>
<Button
type="submit"
form="form"
className="w-full"
loading={loading}
disabled={loading || showSecurityKeyPrompt}
>
<LockIcon className="w-4 h-4 mr-2" />
{t('login')}
</Button>
<Button
type="button"
variant="outline"

View file

@ -57,6 +57,7 @@ type RegisterFormValues = {
type DeleteFormValues = {
password: string;
code?: string;
};
type FieldProps = {
@ -77,6 +78,10 @@ export default function SecurityKeyForm({ open, setOpen }: SecurityKeyFormProps)
const [isRegistering, setIsRegistering] = useState(false);
const [showRegisterDialog, setShowRegisterDialog] = useState(false);
const [selectedSecurityKey, setSelectedSecurityKey] = useState<DeleteSecurityKeyData | null>(null);
const [show2FADialog, setShow2FADialog] = useState(false);
const [deleteInProgress, setDeleteInProgress] = useState(false);
const [pendingDeleteCredentialId, setPendingDeleteCredentialId] = useState<string | null>(null);
const [pendingDeletePassword, setPendingDeletePassword] = useState<string | null>(null);
useEffect(() => {
loadSecurityKeys();
@ -89,6 +94,7 @@ export default function SecurityKeyForm({ open, setOpen }: SecurityKeyFormProps)
const deleteSchema = z.object({
password: z.string().min(1, { message: t('passwordRequired') }),
code: z.string().optional()
});
const registerForm = useForm<RegisterFormValues>({
@ -103,6 +109,7 @@ export default function SecurityKeyForm({ open, setOpen }: SecurityKeyFormProps)
resolver: zodResolver(deleteSchema),
defaultValues: {
password: "",
code: ""
},
});
@ -216,13 +223,23 @@ export default function SecurityKeyForm({ open, setOpen }: SecurityKeyFormProps)
if (!selectedSecurityKey) return;
try {
setDeleteInProgress(true);
const encodedCredentialId = encodeURIComponent(selectedSecurityKey.credentialId);
await api.delete(`/auth/security-key/${encodedCredentialId}`, {
const response = await api.delete(`/auth/security-key/${encodedCredentialId}`, {
data: {
password: values.password,
code: values.code
}
});
// If 2FA is required
if (response.status === 202 && response.data.data.codeRequested) {
setPendingDeleteCredentialId(encodedCredentialId);
setPendingDeletePassword(values.password);
setShow2FADialog(true);
return;
}
toast({
description: t('securityKeyRemoveSuccess')
});
@ -235,6 +252,40 @@ export default function SecurityKeyForm({ open, setOpen }: SecurityKeyFormProps)
variant: "destructive",
description: formatAxiosError(error, t('securityKeyRemoveError')),
});
} finally {
setDeleteInProgress(false);
}
};
const handle2FASubmit = async (values: DeleteFormValues) => {
if (!pendingDeleteCredentialId || !pendingDeletePassword) return;
try {
setDeleteInProgress(true);
await api.delete(`/auth/security-key/${pendingDeleteCredentialId}`, {
data: {
password: pendingDeletePassword,
code: values.code
}
});
toast({
description: t('securityKeyRemoveSuccess')
});
deleteForm.reset();
setSelectedSecurityKey(null);
setShow2FADialog(false);
setPendingDeleteCredentialId(null);
setPendingDeletePassword(null);
await loadSecurityKeys();
} catch (error) {
toast({
variant: "destructive",
description: formatAxiosError(error, t('securityKeyRemoveError')),
});
} finally {
setDeleteInProgress(false);
}
};
@ -421,11 +472,15 @@ export default function SecurityKeyForm({ open, setOpen }: SecurityKeyFormProps)
<FormField
control={deleteForm.control}
name="password"
render={({ field }: FieldProps) => (
render={({ field }) => (
<FormItem>
<FormLabel>{t('password')}</FormLabel>
<FormControl>
<Input {...field} type="password" />
<Input
{...field}
type="password"
disabled={deleteInProgress}
/>
</FormControl>
<FormMessage />
</FormItem>
@ -440,11 +495,87 @@ export default function SecurityKeyForm({ open, setOpen }: SecurityKeyFormProps)
deleteForm.reset();
setSelectedSecurityKey(null);
}}
disabled={deleteInProgress}
>
{t('cancel')}
</Button>
<Button type="submit" className="bg-destructive text-destructive-foreground hover:bg-destructive/90">
{t('securityKeyRemove')}
<Button
type="submit"
className="bg-destructive text-destructive-foreground hover:bg-destructive/90"
disabled={deleteInProgress}
>
{deleteInProgress ? (
<>
<Loader2 className="mr-2 h-4 w-4 animate-spin" />
{t('securityKeyRemoving')}
</>
) : (
t('securityKeyRemove')
)}
</Button>
</DialogFooter>
</form>
</Form>
</DialogContent>
</Dialog>
<Dialog open={show2FADialog} onOpenChange={(open) => !open && setShow2FADialog(false)}>
<DialogContent className="sm:max-w-[400px]">
<DialogHeader>
<DialogTitle>Two-Factor Authentication Required</DialogTitle>
<DialogDescription>
Please enter your two-factor authentication code to remove the security key
</DialogDescription>
</DialogHeader>
<Form {...deleteForm}>
<form onSubmit={deleteForm.handleSubmit(handle2FASubmit)} className="space-y-4">
<FormField
control={deleteForm.control}
name="code"
render={({ field }) => (
<FormItem>
<FormLabel>Two-Factor Code</FormLabel>
<FormControl>
<Input
{...field}
type="text"
placeholder="Enter your 6-digit code"
disabled={deleteInProgress}
/>
</FormControl>
<FormMessage />
</FormItem>
)}
/>
<DialogFooter>
<Button
type="button"
className="border border-input bg-transparent text-foreground hover:bg-accent hover:text-accent-foreground"
onClick={() => {
deleteForm.reset();
setShow2FADialog(false);
setPendingDeleteCredentialId(null);
setPendingDeletePassword(null);
}}
disabled={deleteInProgress}
>
{t('cancel')}
</Button>
<Button
type="submit"
className="bg-destructive text-destructive-foreground hover:bg-destructive/90"
disabled={deleteInProgress}
>
{deleteInProgress ? (
<>
<Loader2 className="mr-2 h-4 w-4 animate-spin" />
{t('securityKeyRemoving')}
</>
) : (
t('securityKeyRemove')
)}
</Button>
</DialogFooter>
</form>