refactor fido2-client.service. created new errorhandling method for s…

…imilar code between create and assert
bitwarden · Aug 18, 2023 · 3115c0d · 3115c0d
1 parent d557840
commit 3115c0d
Showing 1 changed file with 44 additions and 59 deletions.
diff --git a/libs/common/src/vault/services/fido2/fido2-client.service.ts b/libs/common/src/vault/services/fido2/fido2-client.service.ts
@@ -34,49 +34,56 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
     private logService?: LogService
   ) {}
 
-  async isFido2FeatureEnabled(): Promise<boolean> {
-    return await this.configService.getFeatureFlagBool(FeatureFlag.Fido2VaultCredentials);
-  }
-
-  async createCredential(
-    params: CreateCredentialParams,
-    abortController = new AbortController()
-  ): Promise<CreateCredentialResult> {
-    const enableFido2VaultCredentials = await this.isFido2FeatureEnabled();
+  errorCheckHandler(params: any, enableFido2VaultCredentials: boolean, parsedOrigin: any) {
+    const { sameOriginWithAncestors, origin } = params;
+    const rpId = params.rpId ?? params.rp.id ?? parsedOrigin.hostname;
 
     if (!enableFido2VaultCredentials) {
       this.logService?.warning(`[Fido2Client] Fido2VaultCredential is not enabled`);
       throw new FallbackRequestedError();
     }
 
-    if (!params.sameOriginWithAncestors) {
+    if (!sameOriginWithAncestors) {
       this.logService?.warning(
-        `[Fido2Client] Invalid 'sameOriginWithAncestors' value: ${params.sameOriginWithAncestors}`
+        `[Fido2Client] Invalid 'sameOriginWithAncestors' value: ${sameOriginWithAncestors}`
       );
       throw new DOMException("Invalid 'sameOriginWithAncestors' value", "NotAllowedError");
     }
 
-    const userId = Fido2Utils.stringToBuffer(params.user.id);
-    if (userId.length < 1 || userId.length > 64) {
+    if (parsedOrigin.hostname == undefined || !origin.startsWith("https://")) {
+      this.logService?.warning(`[Fido2Client] Invalid https origin: ${origin}`);
+      throw new DOMException("'origin' is not a valid https origin", "SecurityError");
+    }
+
+    if (!isValidRpId(rpId, origin)) {
       this.logService?.warning(
-        `[Fido2Client] Invalid 'user.id' length: ${params.user.id} (${userId.length})`
+        `[Fido2Client] 'rp.id' cannot be used with the current origin: rp.id = ${rpId}; origin = ${origin}`
       );
-      throw new TypeError("Invalid 'user.id' length");
+      throw new DOMException("'rp.id' cannot be used with the current origin", "SecurityError");
     }
+  }
+
+  async isFido2FeatureEnabled(): Promise<boolean> {
+    return await this.configService.getFeatureFlagBool(FeatureFlag.Fido2VaultCredentials);
+  }
 
-    const parsedOrigin = parse(params.origin, { allowPrivateDomains: true });
+  async createCredential(
+    params: CreateCredentialParams,
+    abortController = new AbortController()
+  ): Promise<CreateCredentialResult> {
+    const { sameOriginWithAncestors, origin, user } = params;
+    const parsedOrigin = parse(origin, { allowPrivateDomains: true });
+    const enableFido2VaultCredentials = await this.isFido2FeatureEnabled();
     const rpId = params.rp.id ?? parsedOrigin.hostname;
 
-    if (parsedOrigin.hostname == undefined || !params.origin.startsWith("https://")) {
-      this.logService?.warning(`[Fido2Client] Invalid https origin: ${params.origin}`);
-      throw new DOMException("'origin' is not a valid https origin", "SecurityError");
-    }
+    this.errorCheckHandler(params, enableFido2VaultCredentials, parsedOrigin);
 
-    if (!isValidRpId(rpId, params.origin)) {
+    const userId = Fido2Utils.stringToBuffer(user.id);
+    if (userId.length < 1 || userId.length > 64) {
       this.logService?.warning(
-        `[Fido2Client] 'rp.id' cannot be used with the current origin: rp.id = ${rpId}; origin = ${params.origin}`
+        `[Fido2Client] Invalid 'user.id' length: ${user.id} (${userId.length})`
       );
-      throw new DOMException("'rp.id' cannot be used with the current origin", "SecurityError");
+      throw new TypeError("Invalid 'user.id' length");
     }
 
     let credTypesAndPubKeyAlgs: PublicKeyCredentialParam[];
@@ -102,8 +109,8 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
     const collectedClientData = {
       type: "webauthn.create",
       challenge: params.challenge,
-      origin: params.origin,
-      crossOrigin: !params.sameOriginWithAncestors,
+      origin: origin,
+      crossOrigin: !sameOriginWithAncestors,
       // tokenBinding: {} // Not currently supported
     };
     const clientDataJSON = JSON.stringify(collectedClientData);
@@ -141,8 +148,8 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
         name: params.rp.name,
       },
       userEntity: {
-        id: Fido2Utils.stringToBuffer(params.user.id),
-        displayName: params.user.displayName,
+        id: Fido2Utils.stringToBuffer(user.id),
+        displayName: user.displayName,
       },
       fallbackSupported: params.fallbackSupported,
     };
@@ -193,46 +200,24 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
     params: AssertCredentialParams,
     abortController = new AbortController()
   ): Promise<AssertCredentialResult> {
+    const { sameOriginWithAncestors, origin, userVerification } = params;
+    const parsedOrigin = parse(origin, { allowPrivateDomains: true });
+    const rpId = params.rpId ?? parsedOrigin.hostname;
     const enableFido2VaultCredentials = await this.isFido2FeatureEnabled();
 
-    if (!enableFido2VaultCredentials) {
-      this.logService?.warning(`[Fido2Client] Fido2VaultCredential is not enabled`);
-      throw new FallbackRequestedError();
-    }
+    this.errorCheckHandler(params, enableFido2VaultCredentials, parsedOrigin);
 
-    if (!params.sameOriginWithAncestors) {
-      this.logService?.warning(
-        `[Fido2Client] Invalid 'sameOriginWithAncestors' value: ${params.sameOriginWithAncestors}`
-      );
-      throw new DOMException("Invalid 'sameOriginWithAncestors' value", "NotAllowedError");
-    }
-
-    const { domain: effectiveDomain } = parse(params.origin, { allowPrivateDomains: true });
+    const { domain: effectiveDomain } = parsedOrigin;
     if (effectiveDomain == undefined) {
-      this.logService?.warning(`[Fido2Client] Invalid origin: ${params.origin}`);
+      this.logService?.warning(`[Fido2Client] Invalid origin: ${origin}`);
       throw new DOMException("'origin' is not a valid domain", "SecurityError");
     }
 
-    const parsedOrigin = parse(params.origin, { allowPrivateDomains: true });
-    const rpId = params.rpId ?? parsedOrigin.hostname;
-
-    if (parsedOrigin.hostname == undefined || !params.origin.startsWith("https://")) {
-      this.logService?.warning(`[Fido2Client] Invalid https origin: ${params.origin}`);
-      throw new DOMException("'origin' is not a valid https origin", "SecurityError");
-    }
-
-    if (!isValidRpId(rpId, params.origin)) {
-      this.logService?.warning(
-        `[Fido2Client] 'rp.id' cannot be used with the current origin: rp.id = ${rpId}; origin = ${params.origin}`
-      );
-      throw new DOMException("'rp.id' cannot be used with the current origin", "SecurityError");
-    }
-
     const collectedClientData = {
       type: "webauthn.get",
       challenge: params.challenge,
-      origin: params.origin,
-      crossOrigin: !params.sameOriginWithAncestors,
+      origin: origin,
+      crossOrigin: !sameOriginWithAncestors,
       // tokenBinding: {} // Not currently supported
     };
     const clientDataJSON = JSON.stringify(collectedClientData);
@@ -244,7 +229,7 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
       throw new DOMException(undefined, "AbortError");
     }
 
-    const timeout = setAbortTimeout(abortController, params.userVerification, params.timeout);
+    const timeout = setAbortTimeout(abortController, userVerification, params.timeout);
 
     const allowCredentialDescriptorList: PublicKeyCredentialDescriptor[] =
       params.allowedCredentialIds.map((id) => ({
@@ -254,7 +239,7 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
 
     const getAssertionParams: Fido2AuthenticatorGetAssertionParams = {
       rpId,
-      requireUserVerification: params.userVerification === "required",
+      requireUserVerification: userVerification === "required",
       hash: clientDataHash,
       allowCredentialDescriptorList,
       extensions: {},