fix: block login for 2fa users with un-verified passkeys
This commit is contained in:
parent
648ccae5fc
commit
f982855258
10 changed files with 278 additions and 85 deletions
|
|
@ -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<void>;
|
||||
};
|
||||
|
||||
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 (
|
||||
<div className="pt-4 border-t border-border/60 space-y-3">
|
||||
<p className="text-sm font-medium">Alternative Sign-in</p>
|
||||
<div className="flex flex-col gap-2">
|
||||
{loginOptions.hasPasskeySignIn && <PasskeySignInButton onSignIn={onPasskeySignIn} />}
|
||||
<SsoLoginButtons providers={ssoProviders.providers} />
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
|
@ -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<void>;
|
||||
};
|
||||
|
||||
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 (
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
className="w-full"
|
||||
loading={passkeyLoginMutation.isPending}
|
||||
disabled={passkeyLoginMutation.isPending}
|
||||
onClick={() => passkeyLoginMutation.mutate()}
|
||||
>
|
||||
<Fingerprint className="h-4 w-4 mr-2" />
|
||||
Sign in with passkey
|
||||
</Button>
|
||||
);
|
||||
}
|
||||
|
|
@ -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<typeof import("@tanstack/react-router")>();
|
||||
|
||||
|
|
@ -11,6 +15,19 @@ vi.mock("@tanstack/react-router", async (importOriginal) => {
|
|||
};
|
||||
});
|
||||
|
||||
vi.mock("@tanstack/react-start", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("@tanstack/react-start")>();
|
||||
|
||||
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(<LoginPage />, { 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(<LoginPage />, { 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(<LoginPage />, { 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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<LoginFormValues>({
|
||||
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 = {}) {
|
|||
</form>
|
||||
</Form>
|
||||
|
||||
<SsoLoginSection />
|
||||
<AlternativeSignInSection onPasskeySignIn={navigateAfterLogin} />
|
||||
|
||||
<ResetPasswordDialog open={showResetDialog} onOpenChange={setShowResetDialog} />
|
||||
</AuthLayout>
|
||||
|
|
|
|||
54
app/client/modules/sso/components/sso-login-buttons.tsx
Normal file
54
app/client/modules/sso/components/sso-login-buttons.tsx
Normal file
|
|
@ -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) => (
|
||||
<Button
|
||||
key={provider.providerId}
|
||||
type="button"
|
||||
variant="outline"
|
||||
className="w-full"
|
||||
loading={ssoLoginMutation.isPending}
|
||||
disabled={ssoLoginMutation.isPending}
|
||||
onClick={() => ssoLoginMutation.mutate(provider.providerId)}
|
||||
>
|
||||
Log in with {provider.providerId}
|
||||
</Button>
|
||||
))}
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
|
@ -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 (
|
||||
<div className="pt-4 border-t border-border/60 space-y-3">
|
||||
<p className="text-sm font-medium">Alternative Sign-in</p>
|
||||
<div className="flex flex-col gap-2">
|
||||
{ssoProviders.providers.map((provider) => (
|
||||
<Button
|
||||
key={provider.providerId}
|
||||
type="button"
|
||||
variant="outline"
|
||||
className="w-full"
|
||||
loading={ssoLoginMutation.isPending}
|
||||
disabled={ssoLoginMutation.isPending}
|
||||
onClick={() => ssoLoginMutation.mutate(provider.providerId)}
|
||||
>
|
||||
Log in with {provider.providerId}
|
||||
</Button>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
|
@ -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()] : []),
|
||||
|
|
|
|||
6
app/server/lib/functions/login-options.ts
Normal file
6
app/server/lib/functions/login-options.ts
Normal file
|
|
@ -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(),
|
||||
}));
|
||||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
};
|
||||
|
|
|
|||
Loading…
Reference in a new issue