[PM-29127] Improve subdomain parsing for fido2 (#18383)

* Add check and test for empty inputs into isValidRpId

* Ensure the origin's scheme is https

* Improve parsing and validation of rpId

* Move https requirement check further down as we accept http for localhost

* Add documentation

* Remove ts-strict-ignore

* ts-strict: Fix possibly null on parsedOrigin.hostname

---------

Co-authored-by: Daniel James Smith <djsmith85@users.noreply.github.com>

libs/common/src/platform/services/fido2/domain-utils.spec.ts

@@ -2,6 +2,18 @@ import { isValidRpId } from "./domain-utils";
 
 // Spec: If options.rp.id is not a registrable domain suffix of and is not equal to effectiveDomain, return a DOMException whose name is "SecurityError", and terminate this algorithm.
 describe("validateRpId", () => {
+  it("should not be valid when rpId is null", () => {
+    const origin = "example.com";
+
+    expect(isValidRpId(null, origin)).toBe(false);
+  });
+
+  it("should not be valid when origin is null", () => {
+    const rpId = "example.com";
+
+    expect(isValidRpId(rpId, null)).toBe(false);
+  });
+
   it("should not be valid when rpId is more specific than origin", () => {
     const rpId = "sub.login.bitwarden.com";
     const origin = "https://login.bitwarden.com:1337";
@@ -25,7 +37,7 @@ describe("validateRpId", () => {
 
   it("should not be valid when rpId and origin are both different TLD", () => {
     const rpId = "bitwarden";
-    const origin = "localhost";
+    const origin = "https://localhost";
 
     expect(isValidRpId(rpId, origin)).toBe(false);
   });
@@ -34,14 +46,14 @@ describe("validateRpId", () => {
   // adding support for ip-addresses and other TLDs
   it("should not be valid when rpId and origin are both the same TLD", () => {
     const rpId = "bitwarden";
-    const origin = "bitwarden";
+    const origin = "https://bitwarden";
 
     expect(isValidRpId(rpId, origin)).toBe(false);
   });
 
   it("should not be valid when rpId and origin are ip-addresses", () => {
     const rpId = "127.0.0.1";
-    const origin = "127.0.0.1";
+    const origin = "https://127.0.0.1";
 
     expect(isValidRpId(rpId, origin)).toBe(false);
   });
@@ -80,4 +92,11 @@ describe("validateRpId", () => {
 
     expect(isValidRpId(rpId, origin)).toBe(true);
   });
+
+  it("should not be valid for a partial match of a subdomain", () => {
+    const rpId = "accounts.example.com";
+    const origin = "https://evilaccounts.example.com";
+
+    expect(isValidRpId(rpId, origin)).toBe(false);
+  });
 });

libs/common/src/platform/services/fido2/domain-utils.ts

@@ -1,17 +1,78 @@
-// FIXME: Update this file to be type safe and remove this and next line
-// @ts-strict-ignore
 import { parse } from "tldts";
 
+/**
+ * Validates whether a Relying Party ID (rpId) is valid for a given origin according to WebAuthn specifications.
+ *
+ * The validation enforces the following rules:
+ * - The origin must use the HTTPS scheme
+ * - Both rpId and origin must be valid domain names (not IP addresses)
+ * - Both must have the same registrable domain (e.g., example.com)
+ * - The origin must either exactly match the rpId or be a subdomain of it
+ * - Single-label domains are rejected unless they are 'localhost'
+ * - Localhost is always valid when both rpId and origin are localhost
+ *
+ * @param rpId - The Relying Party identifier to validate
+ * @param origin - The origin URL to validate against (must start with https://)
+ * @returns `true` if the rpId is valid for the given origin, `false` otherwise
+ *
+ */
 export function isValidRpId(rpId: string, origin: string) {
+  if (!rpId || !origin) {
+    return false;
+  }
+
   const parsedOrigin = parse(origin, { allowPrivateDomains: true });
   const parsedRpId = parse(rpId, { allowPrivateDomains: true });
 
-  return (
-    (parsedOrigin.domain == null &&
-      parsedOrigin.hostname == parsedRpId.hostname &&
-      parsedOrigin.hostname == "localhost") ||
-    (parsedOrigin.domain != null &&
-      parsedOrigin.domain == parsedRpId.domain &&
-      parsedOrigin.subdomain.endsWith(parsedRpId.subdomain))
-  );
+  if (!parsedRpId || !parsedOrigin) {
+    return false;
+  }
+
+  // Special case: localhost is always valid when both match
+  if (parsedRpId.hostname === "localhost" && parsedOrigin.hostname === "localhost") {
+    return true;
+  }
+
+  // The origin's scheme must be https.
+  if (!origin.startsWith("https://")) {
+    return false;
+  }
+
+  // Reject IP addresses (both must be domain names)
+  if (parsedRpId.isIp || parsedOrigin.isIp) {
+    return false;
+  }
+
+  // Reject single-label domains (TLDs) unless it's localhost
+  // This ensures we have proper domains like "example.com" not just "example"
+  if (rpId !== "localhost" && !rpId.includes(".")) {
+    return false;
+  }
+
+  if (
+    parsedOrigin.hostname != null &&
+    parsedOrigin.hostname !== "localhost" &&
+    !parsedOrigin.hostname.includes(".")
+  ) {
+    return false;
+  }
+
+  // The registrable domains must match
+  // This ensures a.example.com and b.example.com share base domain
+  if (parsedRpId.domain !== parsedOrigin.domain) {
+    return false;
+  }
+
+  // Check exact match
+  if (parsedOrigin.hostname === rpId) {
+    return true;
+  }
+
+  // Check if origin is a subdomain of rpId
+  // This prevents "evilaccounts.example.com" from matching "accounts.example.com"
+  if (parsedOrigin.hostname != null && parsedOrigin.hostname.endsWith("." + rpId)) {
+    return true;
+  }
+
+  return false;
 }