Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 12 additions & 1 deletion src/pages/Profile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import { type ChangeEvent, type FormEvent, useEffect, useState } from "react";
import toast from "react-hot-toast";
import InputField from "../components/InputField";
import { BASE_URL } from "../config";
import { base64urlToUint8Array, bufferToBase64url } from "../utils/webauthn";
import {
base64urlToUint8Array,
bufferToBase64url,
validateWebAuthnOptionsRpId,
} from "../utils/webauthn";

interface PasskeyAuthenticator {
credentialID?: string;
Expand Down Expand Up @@ -109,6 +113,13 @@ const Profile = () => {
user: { id: string; [key: string]: unknown };
};

const rpIdValidation = validateWebAuthnOptionsRpId(options);
if (!rpIdValidation.isValid) {
console.error("Invalid WebAuthn register RP ID", rpIdValidation);
setMessage(rpIdValidation.message || "Invalid passkey configuration");
return;
}

const publicKey: PublicKeyCredentialCreationOptions = {
...options,
challenge: base64urlToUint8Array(options.challenge).slice(0)
Expand Down
25 changes: 25 additions & 0 deletions src/pages/Signin.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,34 @@ describe("Signin – passkey tab", () => {
);
});

it("fails fast when the backend returns a URL-shaped RP ID", async () => {
vi.spyOn(globalThis, "fetch").mockResolvedValueOnce({
ok: true,
json: async () => ({
challenge: "dGVzdC1jaGFsbGVuZ2U",
rpId: "https://rc-store.benhalverson.dev",
}),
} as Response);

const toast = await import("react-hot-toast");
const user = await switchToPasskeyTab();

await user.click(screen.getByRole("button", { name: /passkey login/i }));

await waitFor(() =>
expect(toast.default.error).toHaveBeenCalledWith(
expect.stringContaining("Passkeys are unavailable on this deployment"),
expect.anything(),
),
);

expect(navigator.credentials.get).not.toHaveBeenCalled();
});

it("sends correct payload to verify-authentication and navigates on success", async () => {
const fakeOptions = {
challenge: "dGVzdC1jaGFsbGVuZ2U",
rpId: "localhost",
allowCredentials: [{ id: "Y3JlZC1pZA", type: "public-key" }],
timeout: 60000,
};
Expand Down
12 changes: 11 additions & 1 deletion src/pages/Signin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import { useNavigate } from "react-router-dom";
import { z } from "zod";
import { BASE_URL } from "../config";
import { useAuth } from "../context/AuthContext";
import { base64urlToUint8Array, bufferToBase64url } from "../utils/webauthn";
import {
base64urlToUint8Array,
bufferToBase64url,
validateWebAuthnOptionsRpId,
} from "../utils/webauthn";

const Signin = () => {
const navigate = useNavigate();
Expand Down Expand Up @@ -99,6 +103,12 @@ const Signin = () => {
}>;
};

const rpIdValidation = validateWebAuthnOptionsRpId(rawOptions);
if (!rpIdValidation.isValid) {
console.error("Invalid WebAuthn authenticate RP ID", rpIdValidation);
throw new Error(rpIdValidation.message);
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rpIdValidation.message is optional in the return type, so throwing new Error(rpIdValidation.message) can result in an undefined/empty error message being shown to users. Consider adding a fallback message at this call site, or make the validator return type guarantee message when isValid is false.

Suggested change
throw new Error(rpIdValidation.message);
throw new Error(
rpIdValidation.message || "Invalid WebAuthn authenticate RP ID",
);

Copilot uses AI. Check for mistakes.
}

const publicKey: PublicKeyCredentialRequestOptions = {
...rawOptions,
challenge: base64urlToUint8Array(
Expand Down
52 changes: 52 additions & 0 deletions src/pages/Signup.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ function renderSignup() {
}

describe("Signup", () => {
beforeEach(() => {
Object.defineProperty(globalThis.navigator, "credentials", {
value: { create: vi.fn() },
configurable: true,
writable: true,
});
});

afterEach(() => {
vi.restoreAllMocks();
mockFetchUser.mockReset();
Expand Down Expand Up @@ -126,4 +134,48 @@ describe("Signup", () => {
),
);
});

it("fails fast when register options include a URL-shaped RP ID", async () => {
vi.spyOn(globalThis, "fetch")
.mockResolvedValueOnce({
ok: true,
json: async () => ({ success: true }),
} as Response)
.mockResolvedValueOnce({
ok: true,
json: async () => ({
challenge: "dGVzdC1jaGFsbGVuZ2U",
user: {
id: "dXNlci0xMjM",
name: "user@example.com",
displayName: "User Example",
},
rp: { id: "https://rc-store.benhalverson.dev" },
}),
} as Response);
mockFetchUser.mockResolvedValueOnce({ email: "user@example.com" });

const toast = await import("react-hot-toast");

renderSignup();
const user = userEvent.setup();

await user.type(screen.getByLabelText(/email/i), "user@example.com");
await user.type(screen.getByLabelText(/password/i), "password123");
await user.click(screen.getByRole("button", { name: /^sign up$/i }));
await screen.findByRole("button", { name: /add passkey to my account/i });

await user.click(
screen.getByRole("button", { name: /add passkey to my account/i }),
);

await waitFor(() =>
expect(toast.default.error).toHaveBeenCalledWith(
expect.stringContaining("Passkeys are unavailable on this deployment"),
expect.anything(),
),
);

expect(navigator.credentials.create).not.toHaveBeenCalled();
});
});
13 changes: 12 additions & 1 deletion src/pages/Signup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import { useNavigate } from "react-router-dom";
import { z } from "zod";
import { BASE_URL } from "../config";
import { useAuth } from "../context/AuthContext";
import { base64urlToUint8Array, bufferToBase64url } from "../utils/webauthn";
import {
base64urlToUint8Array,
bufferToBase64url,
validateWebAuthnOptionsRpId,
} from "../utils/webauthn";

const schema = z.object({
email: z.email({ error: "Invalid email" }),
Expand Down Expand Up @@ -108,9 +112,16 @@ const Signup = () => {
const rawOptions = (await optionsRes.json()) as {
challenge: string;
user: { id: string; name: string; displayName: string };
rp?: { id?: string; name?: string };
[key: string]: unknown;
};

const rpIdValidation = validateWebAuthnOptionsRpId(rawOptions);
if (!rpIdValidation.isValid) {
console.error("Invalid WebAuthn register RP ID", rpIdValidation);
throw new Error(rpIdValidation.message);
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rpIdValidation.message is typed as optional, so new Error(rpIdValidation.message) can produce an Error with an undefined message (which would surface to the user as an unhelpful toast). Either provide a fallback message here or adjust the validation result type so message is required when isValid is false.

Suggested change
throw new Error(rpIdValidation.message);
throw new Error(
rpIdValidation.message ?? "Invalid WebAuthn register RP ID",
);

Copilot uses AI. Check for mistakes.
}

const options = {
...rawOptions,
challenge: base64urlToUint8Array(rawOptions.challenge).slice(0)
Expand Down
67 changes: 67 additions & 0 deletions src/utils/webauthn.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import {
formatWebAuthnRpIdError,
getWebAuthnRpId,
validateWebAuthnOptionsRpId,
validateWebAuthnRpId,
} from "./webauthn";

describe("WebAuthn RP ID validation", () => {
it("accepts an RP ID that matches the current hostname", () => {
expect(
validateWebAuthnRpId(
"rc-store.benhalverson.dev",
"rc-store.benhalverson.dev",
),
).toMatchObject({ isValid: true, rpId: "rc-store.benhalverson.dev" });
});

it("accepts a parent-domain RP ID", () => {
expect(
validateWebAuthnRpId("benhalverson.dev", "rc-store.benhalverson.dev"),
).toMatchObject({ isValid: true, rpId: "benhalverson.dev" });
});

it("rejects localhost when the current host is deployed", () => {
expect(
validateWebAuthnRpId("localhost", "rc-store.benhalverson.dev"),
).toMatchObject({
isValid: false,
rpId: "localhost",
message: formatWebAuthnRpIdError({
rpId: "localhost",
currentHostname: "rc-store.benhalverson.dev",
}),
});
});

it("rejects URL-shaped RP IDs", () => {
expect(
validateWebAuthnRpId(
"https://rc-store.benhalverson.dev",
"rc-store.benhalverson.dev",
),
).toMatchObject({
isValid: false,
rpId: "https://rc-store.benhalverson.dev",
});
});

it("extracts RP ID from either auth or registration payloads", () => {
expect(getWebAuthnRpId({ rpId: "example.com" })).toBe("example.com");
expect(getWebAuthnRpId({ rp: { id: "example.com" } })).toBe(
"example.com",
);
});

it("validates nested registration RP IDs", () => {
expect(
validateWebAuthnOptionsRpId(
{ rp: { id: "https://rc-store.benhalverson.dev" } },
"rc-store.benhalverson.dev",
),
).toMatchObject({
isValid: false,
rpId: "https://rc-store.benhalverson.dev",
});
});
});
102 changes: 102 additions & 0 deletions src/utils/webauthn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,105 @@ export const bufferToBase64url = (buffer: ArrayBuffer): string => {
.replace(/\//g, "_")
.replace(/=/g, "");
};

interface WebAuthnRpEntity {
id?: string;
}

interface WebAuthnRpIdCarrier {
rpId?: string;
rp?: WebAuthnRpEntity;
}

interface WebAuthnRpIdValidationResult {
isValid: boolean;
message?: string;
rpId?: string;
currentHostname: string;
}
Comment on lines +29 to +34
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WebAuthnRpIdValidationResult marks message/rpId as optional, but the implementation always provides them for invalid results. This type is easy to misuse (e.g., showing an undefined error message) and doesn’t encode the invariant that message exists when isValid is false. Consider changing this to a discriminated union where the isValid: false branch requires message and rpId.

Suggested change
interface WebAuthnRpIdValidationResult {
isValid: boolean;
message?: string;
rpId?: string;
currentHostname: string;
}
type WebAuthnRpIdValidationResult =
| {
isValid: true;
currentHostname: string;
rpId?: string;
message?: undefined;
}
| {
isValid: false;
currentHostname: string;
rpId: string;
message: string;
};

Copilot uses AI. Check for mistakes.

const getCurrentHostname = () =>
typeof window === "undefined" ? "" : window.location.hostname.toLowerCase();

const hasInvalidRpIdSyntax = (rpId: string) =>
rpId.includes("://") ||
rpId.includes("/") ||
rpId.includes("?") ||
rpId.includes("#") ||
rpId.includes(":");

export const getWebAuthnRpId = ({ rpId, rp }: WebAuthnRpIdCarrier) => {
if (typeof rpId === "string" && rpId.trim()) {
return rpId.trim().toLowerCase();
}

if (typeof rp?.id === "string" && rp.id.trim()) {
return rp.id.trim().toLowerCase();
}

return undefined;
};

export const formatWebAuthnRpIdError = ({
rpId,
currentHostname,
}: {
rpId: string;
currentHostname: string;
}) =>
`Passkeys are unavailable on this deployment. The server returned RP ID "${rpId}" for "${currentHostname}". Expected the RP ID to match the current hostname or one of its parent domains.`;

export const validateWebAuthnRpId = (
rpId: string | undefined,
currentHostname = getCurrentHostname(),
): WebAuthnRpIdValidationResult => {
if (!rpId) {
return { isValid: true, currentHostname };
}

const normalizedRpId = rpId.trim().toLowerCase();
const normalizedHostname = currentHostname.trim().toLowerCase();
Comment on lines +71 to +76
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When rpId is undefined, the function returns { currentHostname } without normalizing it, but all other branches return a trimmed/lowercased hostname. This makes the result shape inconsistent for callers that log/display currentHostname. Consider normalizing currentHostname up front and always returning the normalized value.

Suggested change
if (!rpId) {
return { isValid: true, currentHostname };
}
const normalizedRpId = rpId.trim().toLowerCase();
const normalizedHostname = currentHostname.trim().toLowerCase();
const normalizedHostname = currentHostname.trim().toLowerCase();
if (!rpId) {
return { isValid: true, currentHostname: normalizedHostname };
}
const normalizedRpId = rpId.trim().toLowerCase();

Copilot uses AI. Check for mistakes.

if (!normalizedRpId) {
return { isValid: true, currentHostname: normalizedHostname };
}

if (hasInvalidRpIdSyntax(normalizedRpId)) {
return {
isValid: false,
rpId: normalizedRpId,
currentHostname: normalizedHostname,
message: formatWebAuthnRpIdError({
rpId: normalizedRpId,
currentHostname: normalizedHostname,
}),
};
}

const matchesCurrentHost =
normalizedHostname === normalizedRpId ||
normalizedHostname.endsWith(`.${normalizedRpId}`);

if (!matchesCurrentHost) {
return {
isValid: false,
rpId: normalizedRpId,
currentHostname: normalizedHostname,
message: formatWebAuthnRpIdError({
rpId: normalizedRpId,
currentHostname: normalizedHostname,
}),
};
}

return {
isValid: true,
rpId: normalizedRpId,
currentHostname: normalizedHostname,
};
};

export const validateWebAuthnOptionsRpId = (
options: WebAuthnRpIdCarrier,
currentHostname = getCurrentHostname(),
) => validateWebAuthnRpId(getWebAuthnRpId(options), currentHostname);