From f982855258ea7e95ad2a1f293ad1e3a680a94417 Mon Sep 17 00:00:00 2001 From: Nicolas Meienberger Date: Tue, 2 Jun 2026 18:00:06 +0200 Subject: [PATCH] fix: block login for 2fa users with un-verified passkeys --- .../alternative-sign-in-section.tsx | 35 +++++++++++ .../components/passkey-sign-in-button.tsx | 39 +++++++++++++ .../auth/routes/__tests__/login.test.tsx | 40 ++++++++++++- app/client/modules/auth/routes/login.tsx | 43 +++++++------- .../sso/components/sso-login-buttons.tsx | 54 +++++++++++++++++ .../sso/components/sso-login-section.tsx | 58 ------------------- app/server/lib/auth.ts | 25 +++++++- app/server/lib/functions/login-options.ts | 6 ++ .../modules/auth/__tests__/helpers.test.ts | 50 +++++++++++++++- app/server/modules/auth/helpers.ts | 13 +++++ 10 files changed, 278 insertions(+), 85 deletions(-) create mode 100644 app/client/modules/auth/components/alternative-sign-in-section.tsx create mode 100644 app/client/modules/auth/components/passkey-sign-in-button.tsx create mode 100644 app/client/modules/sso/components/sso-login-buttons.tsx delete mode 100644 app/client/modules/sso/components/sso-login-section.tsx create mode 100644 app/server/lib/functions/login-options.ts diff --git a/app/client/modules/auth/components/alternative-sign-in-section.tsx b/app/client/modules/auth/components/alternative-sign-in-section.tsx new file mode 100644 index 00000000..ca4f97bc --- /dev/null +++ b/app/client/modules/auth/components/alternative-sign-in-section.tsx @@ -0,0 +1,35 @@ +import { useSuspenseQuery } from "@tanstack/react-query"; +import { useServerFn } from "@tanstack/react-start"; +import { getPublicSsoProvidersOptions } from "~/client/api-client/@tanstack/react-query.gen"; +import { SsoLoginButtons } from "~/client/modules/sso/components/sso-login-buttons"; +import { getLoginOptions } from "~/server/lib/functions/login-options"; +import { PasskeySignInButton } from "./passkey-sign-in-button"; + +type AlternativeSignInSectionProps = { + onPasskeySignIn: () => Promise; +}; + +export function AlternativeSignInSection({ onPasskeySignIn }: AlternativeSignInSectionProps) { + const getOptions = useServerFn(getLoginOptions); + const { data: ssoProviders } = useSuspenseQuery({ + ...getPublicSsoProvidersOptions(), + }); + const { data: loginOptions } = useSuspenseQuery({ + queryKey: ["login-options"], + queryFn: getOptions, + }); + + if (ssoProviders.providers.length === 0 && !loginOptions.hasPasskeySignIn) { + return null; + } + + return ( +
+

Alternative Sign-in

+
+ {loginOptions.hasPasskeySignIn && } + +
+
+ ); +} diff --git a/app/client/modules/auth/components/passkey-sign-in-button.tsx b/app/client/modules/auth/components/passkey-sign-in-button.tsx new file mode 100644 index 00000000..eda2abe6 --- /dev/null +++ b/app/client/modules/auth/components/passkey-sign-in-button.tsx @@ -0,0 +1,39 @@ +import { useMutation } from "@tanstack/react-query"; +import { Fingerprint } from "lucide-react"; +import { toast } from "sonner"; +import { Button } from "~/client/components/ui/button"; +import { authClient } from "~/client/lib/auth-client"; +import { logger } from "~/client/lib/logger"; + +type PasskeySignInButtonProps = { + onSignIn: () => Promise; +}; + +export function PasskeySignInButton({ onSignIn }: PasskeySignInButtonProps) { + const passkeyLoginMutation = useMutation({ + mutationFn: async () => { + const { error } = await authClient.signIn.passkey(); + if (error) throw error; + + await onSignIn(); + }, + onError: (error) => { + logger.error(error); + toast.error("Passkey login failed", { description: error.message }); + }, + }); + + return ( + + ); +} diff --git a/app/client/modules/auth/routes/__tests__/login.test.tsx b/app/client/modules/auth/routes/__tests__/login.test.tsx index 13c18da4..597db17b 100644 --- a/app/client/modules/auth/routes/__tests__/login.test.tsx +++ b/app/client/modules/auth/routes/__tests__/login.test.tsx @@ -2,6 +2,10 @@ import { afterEach, describe, expect, test, vi } from "vitest"; import { HttpResponse, http, server } from "~/test/msw/server"; import { cleanup, render, screen } from "~/test/test-utils"; +const { mockGetLoginOptions } = vi.hoisted(() => ({ + mockGetLoginOptions: vi.fn(async () => ({ hasPasskeySignIn: false })), +})); + vi.mock("@tanstack/react-router", async (importOriginal) => { const actual = await importOriginal(); @@ -11,6 +15,19 @@ vi.mock("@tanstack/react-router", async (importOriginal) => { }; }); +vi.mock("@tanstack/react-start", async (importOriginal) => { + const actual = await importOriginal(); + + return { + ...actual, + useServerFn: (fn: unknown) => fn, + }; +}); + +vi.mock("~/server/lib/functions/login-options", () => ({ + getLoginOptions: mockGetLoginOptions, +})); + import { LoginPage } from "../login"; const inviteOnlyMessage = "Access is invite-only. Ask an organization admin to send you an invitation before signing in with SSO."; @@ -29,6 +46,8 @@ const mockSsoProvidersRequest = ( }; afterEach(() => { + mockGetLoginOptions.mockClear(); + mockGetLoginOptions.mockResolvedValue({ hasPasskeySignIn: false }); cleanup(); }); @@ -90,11 +109,30 @@ describe("LoginPage", () => { expect(screen.queryByText(inviteOnlyMessage)).toBeNull(); }); - test("renders available SSO providers from the real SSO section", async () => { + test("renders available SSO providers from the alternative sign-in section", async () => { mockSsoProvidersRequest([{ providerId: "acme", organizationSlug: "acme-org" }]); render(, { withSuspense: true }); expect(await screen.findByRole("button", { name: "Log in with acme" })).toBeTruthy(); }); + + test("renders passkey sign-in when an active user has a passkey", async () => { + mockSsoProvidersRequest(); + mockGetLoginOptions.mockResolvedValue({ hasPasskeySignIn: true }); + + render(, { withSuspense: true }); + + expect(await screen.findByRole("button", { name: "Sign in with passkey" })).toBeTruthy(); + }); + + test("hides alternative sign-in when no SSO providers or passkeys are available", async () => { + mockSsoProvidersRequest(); + + render(, { withSuspense: true }); + + await screen.findByText("Login to your account"); + expect(screen.queryByText("Alternative Sign-in")).toBeNull(); + expect(screen.queryByRole("button", { name: "Sign in with passkey" })).toBeNull(); + }); }); diff --git a/app/client/modules/auth/routes/login.tsx b/app/client/modules/auth/routes/login.tsx index 30a261e3..ae5f64cf 100644 --- a/app/client/modules/auth/routes/login.tsx +++ b/app/client/modules/auth/routes/login.tsx @@ -1,5 +1,5 @@ import { zodResolver } from "@hookform/resolvers/zod"; -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useState } from "react"; import { useForm } from "react-hook-form"; import { toast } from "sonner"; import { AuthLayout } from "~/client/components/auth-layout"; @@ -16,7 +16,7 @@ import { ResetPasswordDialog } from "../components/reset-password-dialog"; import { useNavigate } from "@tanstack/react-router"; import { normalizeUsername } from "~/lib/username"; import { cn } from "~/client/lib/utils"; -import { SsoLoginSection } from "~/client/modules/sso/components/sso-login-section"; +import { AlternativeSignInSection } from "../components/alternative-sign-in-section"; import { z } from "zod"; const loginSchema = z.object({ @@ -50,6 +50,20 @@ export function LoginPage({ error }: LoginPageProps = {}) { const errorCode = decodeLoginError(error); const errorDescription = errorCode ? getLoginErrorDescription(errorCode) : null; + const navigateAfterLogin = useCallback(async () => { + const session = await authClient.getSession(); + + if ( + session.data?.user && + !session.data.user.hasDownloadedResticPassword && + !hasSkippedRecoveryKeyDownload(session.data.user.id) + ) { + void navigate({ to: "/download-recovery-key" }); + } else { + void navigate({ to: "/volumes" }); + } + }, [navigate]); + useEffect(() => { const autoSignIn = async () => { if ( @@ -63,25 +77,13 @@ export function LoginPage({ error }: LoginPageProps = {}) { await authClient.signIn.passkey({ autoFill: true, fetchOptions: { - onResponse: async () => { - const session = await authClient.getSession(); - - if ( - session.data?.user && - !session.data.user.hasDownloadedResticPassword && - !hasSkippedRecoveryKeyDownload(session.data.user.id) - ) { - void navigate({ to: "/download-recovery-key" }); - } else { - void navigate({ to: "/volumes" }); - } - }, + onResponse: navigateAfterLogin, }, }); }; void autoSignIn(); - }, [navigate]); + }, [navigateAfterLogin]); const form = useForm({ resolver: zodResolver(loginSchema), @@ -116,12 +118,7 @@ export function LoginPage({ error }: LoginPageProps = {}) { return; } - const d = await authClient.getSession(); - if (data.user && !d.data?.user.hasDownloadedResticPassword && !hasSkippedRecoveryKeyDownload(data.user.id)) { - void navigate({ to: "/download-recovery-key" }); - } else { - void navigate({ to: "/volumes" }); - } + await navigateAfterLogin(); }; const handleVerify2FA = async () => { @@ -305,7 +302,7 @@ export function LoginPage({ error }: LoginPageProps = {}) { - + diff --git a/app/client/modules/sso/components/sso-login-buttons.tsx b/app/client/modules/sso/components/sso-login-buttons.tsx new file mode 100644 index 00000000..4ac38838 --- /dev/null +++ b/app/client/modules/sso/components/sso-login-buttons.tsx @@ -0,0 +1,54 @@ +import { useMutation } from "@tanstack/react-query"; +import { toast } from "sonner"; +import { Button } from "~/client/components/ui/button"; +import { authClient } from "~/client/lib/auth-client"; +import { logger } from "~/client/lib/logger"; + +type SsoProvider = { + providerId: string; +}; + +type SsoLoginButtonsProps = { + providers: SsoProvider[]; +}; + +export function SsoLoginButtons({ providers }: SsoLoginButtonsProps) { + const ssoLoginMutation = useMutation({ + mutationFn: async (providerId: string) => { + const callbackPath = "/login"; + const { data, error } = await authClient.signIn.sso({ + providerId: providerId, + callbackURL: callbackPath, + errorCallbackURL: "/api/v1/auth/login-error", + }); + if (error) throw error; + + return data; + }, + onSuccess: (data) => { + window.location.href = data.url; + }, + onError: (error) => { + logger.error(error); + toast.error("SSO Login failed", { description: error.message }); + }, + }); + + return ( + <> + {providers.map((provider) => ( + + ))} + + ); +} diff --git a/app/client/modules/sso/components/sso-login-section.tsx b/app/client/modules/sso/components/sso-login-section.tsx deleted file mode 100644 index e4896d23..00000000 --- a/app/client/modules/sso/components/sso-login-section.tsx +++ /dev/null @@ -1,58 +0,0 @@ -import { useMutation, useSuspenseQuery } from "@tanstack/react-query"; -import { toast } from "sonner"; -import { Button } from "~/client/components/ui/button"; -import { authClient } from "~/client/lib/auth-client"; -import { logger } from "~/client/lib/logger"; -import { getPublicSsoProvidersOptions } from "~/client/api-client/@tanstack/react-query.gen"; - -export function SsoLoginSection() { - const { data: ssoProviders } = useSuspenseQuery({ - ...getPublicSsoProvidersOptions(), - }); - - const ssoLoginMutation = useMutation({ - mutationFn: async (providerId: string) => { - const callbackPath = "/login"; - const { data, error } = await authClient.signIn.sso({ - providerId: providerId, - callbackURL: callbackPath, - errorCallbackURL: "/api/v1/auth/login-error", - }); - if (error) throw error; - - return data; - }, - onSuccess: (data) => { - window.location.href = data.url; - }, - onError: (error) => { - logger.error(error); - toast.error("SSO Login failed", { description: error.message }); - }, - }); - - if (ssoProviders.providers.length === 0) { - return null; - } - - return ( -
-

Alternative Sign-in

-
- {ssoProviders.providers.map((provider) => ( - - ))} -
-
- ); -} diff --git a/app/server/lib/auth.ts b/app/server/lib/auth.ts index 8327eb07..e01c8eac 100644 --- a/app/server/lib/auth.ts +++ b/app/server/lib/auth.ts @@ -10,8 +10,10 @@ import { drizzleAdapter } from "better-auth/adapters/drizzle"; import { admin, twoFactor, username, organization, testUtils } from "better-auth/plugins"; import { passkey } from "@better-auth/passkey"; import { createAuthMiddleware } from "better-auth/api"; +import { eq } from "drizzle-orm"; import { config } from "../core/config"; import { db } from "../db/db"; +import { passkey as passkeyTable, usersTable } from "../db/schema"; import { cryptoUtils } from "../utils/crypto"; import { authService } from "../modules/auth/auth.service"; import { tanstackStartCookies } from "better-auth/tanstack-start"; @@ -172,8 +174,29 @@ export const auth = betterAuth({ }, }), passkey({ - rpID: new URL(config.baseUrl).hostname, + rpID: "zerobyte.localhost", rpName: "Zerobyte", + authentication: { + afterVerification: async ({ verification, clientData }) => { + if (verification.authenticationInfo.userVerified) { + return; + } + + const [user] = await db + .select({ twoFactorEnabled: usersTable.twoFactorEnabled }) + .from(passkeyTable) + .innerJoin(usersTable, eq(passkeyTable.userId, usersTable.id)) + .where(eq(passkeyTable.credentialID, clientData.id)) + .limit(1); + + if (user?.twoFactorEnabled) { + throw new APIError("UNAUTHORIZED", { + message: + "This passkey cannot securely validate your identiy and since you have 2FA enabled, Zerobyte cannot validate the login request", + }); + } + }, + }, }), tanstackStartCookies(), ...(process.env.NODE_ENV === "test" ? [testUtils()] : []), diff --git a/app/server/lib/functions/login-options.ts b/app/server/lib/functions/login-options.ts new file mode 100644 index 00000000..66c98a6f --- /dev/null +++ b/app/server/lib/functions/login-options.ts @@ -0,0 +1,6 @@ +import { createServerFn } from "@tanstack/react-start"; +import { hasActivePasskeyUser } from "~/server/modules/auth/helpers"; + +export const getLoginOptions = createServerFn({ method: "GET" }).handler(async () => ({ + hasPasskeySignIn: await hasActivePasskeyUser(), +})); diff --git a/app/server/modules/auth/__tests__/helpers.test.ts b/app/server/modules/auth/__tests__/helpers.test.ts index 23e0a500..63fda9a6 100644 --- a/app/server/modules/auth/__tests__/helpers.test.ts +++ b/app/server/modules/auth/__tests__/helpers.test.ts @@ -1,8 +1,9 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; +import { eq } from "drizzle-orm"; import { db } from "~/server/db/db"; -import { account, usersTable } from "~/server/db/schema"; +import { account, passkey, usersTable } from "~/server/db/schema"; import { createUser, randomId, randomSlug } from "~/test/helpers/user-org"; -import { userHasCredentialPassword, verifyUserPassword } from "../helpers"; +import { hasActivePasskeyUser, userHasCredentialPassword, verifyUserPassword } from "../helpers"; const { verifyPassword } = vi.hoisted(() => ({ verifyPassword: vi.fn(async ({ hash }: { hash: string }) => hash === "credential-password-hash"), @@ -30,9 +31,24 @@ async function createAccount({ }); } +async function createPasskey(userId: string) { + await db.insert(passkey).values({ + id: randomId(), + name: "Test passkey", + publicKey: randomSlug("public-key"), + userId, + credentialID: randomSlug("credential"), + counter: 0, + deviceType: "singleDevice", + backedUp: false, + transports: "internal", + }); +} + describe("verifyUserPassword", () => { beforeEach(async () => { verifyPassword.mockClear(); + await db.delete(passkey); await db.delete(account); await db.delete(usersTable); }); @@ -64,6 +80,7 @@ describe("verifyUserPassword", () => { describe("userHasCredentialPassword", () => { beforeEach(async () => { + await db.delete(passkey); await db.delete(account); await db.delete(usersTable); }); @@ -89,3 +106,32 @@ describe("userHasCredentialPassword", () => { await expect(userHasCredentialPassword(userId)).resolves.toBe(false); }); }); + +describe("hasActivePasskeyUser", () => { + beforeEach(async () => { + await db.delete(passkey); + await db.delete(account); + await db.delete(usersTable); + }); + + test("returns true when a non-banned user has a passkey", async () => { + const userId = await createUser(`${randomSlug("user")}@example.com`); + await createPasskey(userId); + + await expect(hasActivePasskeyUser()).resolves.toBe(true); + }); + + test("returns false when only banned users have passkeys", async () => { + const userId = await createUser(`${randomSlug("user")}@example.com`); + await db.update(usersTable).set({ banned: true }).where(eq(usersTable.id, userId)); + await createPasskey(userId); + + await expect(hasActivePasskeyUser()).resolves.toBe(false); + }); + + test("returns false when no users have passkeys", async () => { + await createUser(`${randomSlug("user")}@example.com`); + + await expect(hasActivePasskeyUser()).resolves.toBe(false); + }); +}); diff --git a/app/server/modules/auth/helpers.ts b/app/server/modules/auth/helpers.ts index 4f440683..ad3d2c3d 100644 --- a/app/server/modules/auth/helpers.ts +++ b/app/server/modules/auth/helpers.ts @@ -1,5 +1,7 @@ +import { eq } from "drizzle-orm"; import { verifyPassword } from "better-auth/crypto"; import { db } from "~/server/db/db"; +import { passkey, usersTable } from "~/server/db/schema"; type PasswordVerificationBody = { userId: string; @@ -31,3 +33,14 @@ export const userHasCredentialPassword = async (userId: string) => { return Boolean(userAccount?.password); }; + +export const hasActivePasskeyUser = async () => { + const [user] = await db + .select({ id: usersTable.id }) + .from(passkey) + .innerJoin(usersTable, eq(passkey.userId, usersTable.id)) + .where(eq(usersTable.banned, false)) + .limit(1); + + return Boolean(user); +};