From 132c3fe04d1791d1c0ef1614c16cfc85c55151de Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 5 Jan 2023 15:07:07 +0100 Subject: [PATCH] [EC-598] feat: allow user to pick which credential to use --- apps/browser/src/popup/app.module.ts | 3 +- .../src/popup/fido2/fido2.component.html | 32 ++++++--- .../src/popup/fido2/fido2.component.ts | 70 +++++++++++-------- .../browser-fido2-user-interface.service.ts | 24 +++---- ...ido2-user-interface.service.abstraction.ts | 3 +- .../src/services/fido2/fido2.service.ts | 41 ++++++----- .../noop-fido2-user-interface.service.ts | 9 +-- 7 files changed, 101 insertions(+), 81 deletions(-) diff --git a/apps/browser/src/popup/app.module.ts b/apps/browser/src/popup/app.module.ts index e5ef2e7cade..750f9555c7a 100644 --- a/apps/browser/src/popup/app.module.ts +++ b/apps/browser/src/popup/app.module.ts @@ -3,7 +3,7 @@ import { DragDropModule } from "@angular/cdk/drag-drop"; import { LayoutModule } from "@angular/cdk/layout"; import { OverlayModule } from "@angular/cdk/overlay"; import { ScrollingModule } from "@angular/cdk/scrolling"; -import { CurrencyPipe, DatePipe, registerLocaleData } from "@angular/common"; +import { CommonModule, CurrencyPipe, DatePipe, registerLocaleData } from "@angular/common"; import localeAr from "@angular/common/locales/ar"; import localeAz from "@angular/common/locales/az"; import localeBe from "@angular/common/locales/be"; @@ -175,6 +175,7 @@ registerLocaleData(localeZhTw, "zh-TW"); @NgModule({ imports: [ + CommonModule, A11yModule, AppRoutingModule, BitwardenToastModule.forRoot({ diff --git a/apps/browser/src/popup/fido2/fido2.component.html b/apps/browser/src/popup/fido2/fido2.component.html index e47d4c9aefd..2534d22ce7b 100644 --- a/apps/browser/src/popup/fido2/fido2.component.html +++ b/apps/browser/src/popup/fido2/fido2.component.html @@ -1,20 +1,32 @@
- - A site is asking for authentication - - + + A site is asking for authentication, please choose one of the following credentials to use
- +
- A site wants to create a new passkey in your vault + + + + A site wants to create the following passkey in your vault +
+
+ +
+
+ - diff --git a/apps/browser/src/popup/fido2/fido2.component.ts b/apps/browser/src/popup/fido2/fido2.component.ts index 3bf346a7cba..eb1a0cf9adf 100644 --- a/apps/browser/src/popup/fido2/fido2.component.ts +++ b/apps/browser/src/popup/fido2/fido2.component.ts @@ -1,7 +1,8 @@ import { Component, HostListener, OnDestroy, OnInit } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { Subject, takeUntil } from "rxjs"; +import { concatMap, Subject, takeUntil } from "rxjs"; +import { CipherService } from "@bitwarden/common/abstractions/cipher.service"; import { CipherType } from "@bitwarden/common/enums/cipherType"; import { CipherView } from "@bitwarden/common/models/view/cipher.view"; import { Fido2KeyView } from "@bitwarden/common/models/view/fido2-key.view"; @@ -20,44 +21,51 @@ export class Fido2Component implements OnInit, OnDestroy { private destroy$ = new Subject(); protected data?: BrowserFido2Message; - protected cipher?: CipherView; + protected ciphers?: CipherView[] = []; - constructor(private activatedRoute: ActivatedRoute) {} + constructor(private activatedRoute: ActivatedRoute, private cipherService: CipherService) {} ngOnInit(): void { - this.activatedRoute.queryParamMap.pipe(takeUntil(this.destroy$)).subscribe((queryParamMap) => { - this.data = JSON.parse(queryParamMap.get("data")); + this.activatedRoute.queryParamMap + .pipe( + concatMap(async (queryParamMap) => { + this.data = JSON.parse(queryParamMap.get("data")); - if (this.data?.type === "ConfirmNewCredentialRequest") { - this.cipher = new CipherView(); - this.cipher.name = this.data.name; - this.cipher.type = CipherType.Fido2Key; - this.cipher.fido2Key = new Fido2KeyView(); - } - }); + if (this.data?.type === "ConfirmNewCredentialRequest") { + const cipher = new CipherView(); + cipher.name = this.data.name; + cipher.type = CipherType.Fido2Key; + cipher.fido2Key = new Fido2KeyView(); + this.ciphers = [cipher]; + } else if (this.data?.type === "PickCredentialRequest") { + this.ciphers = await Promise.all( + this.data.cipherIds.map(async (cipherId) => { + const cipher = await this.cipherService.get(cipherId); + return cipher.decrypt(); + }) + ); + } + }), + takeUntil(this.destroy$) + ) + .subscribe(); } - async accept() { - const data = this.data; + async pick(cipher: CipherView) { + BrowserFido2UserInterfaceService.sendMessage({ + requestId: this.data.requestId, + cipherId: cipher.id, + type: "PickCredentialResponse", + }); - if (data.type === "VerifyUserRequest") { - BrowserFido2UserInterfaceService.sendMessage({ - requestId: data.requestId, - type: "VerifyUserResponse", - }); - } else if (data.type === "ConfirmNewCredentialRequest") { - BrowserFido2UserInterfaceService.sendMessage({ - requestId: data.requestId, - type: "ConfirmNewCredentialResponse", - }); - } else { - BrowserFido2UserInterfaceService.sendMessage({ - requestId: data.requestId, - type: "RequestCancelled", - fallbackRequested: true, - }); - } + window.close(); + } + confirm() { + BrowserFido2UserInterfaceService.sendMessage({ + requestId: this.data.requestId, + type: "ConfirmNewCredentialResponse", + }); window.close(); } diff --git a/apps/browser/src/services/fido2/browser-fido2-user-interface.service.ts b/apps/browser/src/services/fido2/browser-fido2-user-interface.service.ts index 603ed6c4945..b537cd4db5b 100644 --- a/apps/browser/src/services/fido2/browser-fido2-user-interface.service.ts +++ b/apps/browser/src/services/fido2/browser-fido2-user-interface.service.ts @@ -14,10 +14,12 @@ const BrowserFido2MessageName = "BrowserFido2UserInterfaceServiceMessage"; export type BrowserFido2Message = { requestId: string } & ( | { - type: "VerifyUserRequest"; + type: "PickCredentialRequest"; + cipherIds: string[]; } | { - type: "VerifyUserResponse"; + type: "PickCredentialResponse"; + cipherId?: string; } | { type: "ConfirmNewCredentialRequest"; @@ -48,13 +50,9 @@ export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServi BrowserApi.messageListener(BrowserFido2MessageName, this.processMessage.bind(this)); } - async verifyUser(): Promise { - return false; - } - - async verifyPresence(): Promise { + async pickCredential(cipherIds: string[]): Promise { const requestId = Utils.newGuid(); - const data: BrowserFido2Message = { type: "VerifyUserRequest", requestId }; + const data: BrowserFido2Message = { type: "PickCredentialRequest", cipherIds, requestId }; const queryParams = new URLSearchParams({ data: JSON.stringify(data) }).toString(); this.popupUtilsService.popOut( null, @@ -70,15 +68,15 @@ export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServi ) ); - if (response.type === "VerifyUserResponse") { - return true; - } - if (response.type === "RequestCancelled") { throw new RequestAbortedError(response.fallbackRequested); } - return false; + if (response.type !== "PickCredentialResponse") { + throw new RequestAbortedError(); + } + + return response.cipherId; } async confirmNewCredential({ name }: NewCredentialParams): Promise { diff --git a/libs/common/src/abstractions/fido2/fido2-user-interface.service.abstraction.ts b/libs/common/src/abstractions/fido2/fido2-user-interface.service.abstraction.ts index 93a87534f07..2097d5b1859 100644 --- a/libs/common/src/abstractions/fido2/fido2-user-interface.service.abstraction.ts +++ b/libs/common/src/abstractions/fido2/fido2-user-interface.service.abstraction.ts @@ -3,7 +3,6 @@ export interface NewCredentialParams { } export abstract class Fido2UserInterfaceService { - verifyUser: () => Promise; - verifyPresence: () => Promise; + pickCredential: (cipherIds: string[]) => Promise; confirmNewCredential: (params: NewCredentialParams) => Promise; } diff --git a/libs/common/src/services/fido2/fido2.service.ts b/libs/common/src/services/fido2/fido2.service.ts index 8c38d6be0c4..f9919875a71 100644 --- a/libs/common/src/services/fido2/fido2.service.ts +++ b/libs/common/src/services/fido2/fido2.service.ts @@ -120,21 +120,30 @@ export class Fido2Service implements Fido2ServiceAbstraction { if (params.allowedCredentialIds && params.allowedCredentialIds.length > 0) { // We're looking for regular non-resident keys credential = await this.getCredential(params.allowedCredentialIds); + + if (credential === undefined) { + throw new NoCredentialFoundError(); + } + + if (credential.origin !== params.origin) { + throw new OriginMismatchError(); + } + + await this.fido2UserInterfaceService.pickCredential([credential.credentialId.encoded]); } else { // We're looking for a resident key - credential = await this.getCredentialByRp(params.rpId); - } + const credentials = await this.getCredentialsByRp(params.rpId); - if (credential === undefined) { - throw new NoCredentialFoundError(); - } + if (credentials.length === 0) { + throw new NoCredentialFoundError(); + } - if (credential.origin !== params.origin) { - throw new OriginMismatchError(); + const pickedId = await this.fido2UserInterfaceService.pickCredential( + credentials.map((c) => c.credentialId.encoded) + ); + credential = credentials.find((c) => c.credentialId.encoded === pickedId); } - const presence = await this.fido2UserInterfaceService.verifyPresence(); - const encoder = new TextEncoder(); const clientData = encoder.encode( JSON.stringify({ @@ -147,7 +156,7 @@ export class Fido2Service implements Fido2ServiceAbstraction { const authData = await generateAuthData({ credentialId: credential.credentialId, rpId: params.rpId, - userPresence: presence, + userPresence: true, userVerification: true, // TODO: Change to false! }); @@ -171,7 +180,7 @@ export class Fido2Service implements Fido2ServiceAbstraction { for (const allowedCredential of allowedCredentialIds) { cipher = await this.cipherService.get(allowedCredential); - if (cipher.deletedDate != undefined) { + if (cipher?.deletedDate != undefined) { cipher = undefined; } @@ -209,17 +218,13 @@ export class Fido2Service implements Fido2ServiceAbstraction { return new CredentialId(cipher.id); } - private async getCredentialByRp(rpId: string): Promise { + private async getCredentialsByRp(rpId: string): Promise { const allCipherViews = await this.cipherService.getAllDecrypted(); - const cipherView = allCipherViews.find( + const cipherViews = allCipherViews.filter( (cv) => !cv.isDeleted && cv.type === CipherType.Fido2Key && cv.fido2Key?.rpId === rpId ); - if (cipherView == undefined) { - return undefined; - } - - return await mapCipherViewToBitCredential(cipherView); + return await Promise.all(cipherViews.map((view) => mapCipherViewToBitCredential(view))); } } diff --git a/libs/common/src/services/fido2/noop-fido2-user-interface.service.ts b/libs/common/src/services/fido2/noop-fido2-user-interface.service.ts index 1db8934e7f0..2098886845b 100644 --- a/libs/common/src/services/fido2/noop-fido2-user-interface.service.ts +++ b/libs/common/src/services/fido2/noop-fido2-user-interface.service.ts @@ -1,12 +1,9 @@ import { Fido2UserInterfaceService as Fido2UserInterfaceServiceAbstraction } from "../../abstractions/fido2/fido2-user-interface.service.abstraction"; +import { RequestAbortedError } from "../../abstractions/fido2/fido2.service.abstraction"; export class Fido2UserInterfaceService implements Fido2UserInterfaceServiceAbstraction { - async verifyUser(): Promise { - return false; - } - - async verifyPresence(): Promise { - return false; + pickCredential(cipherIds: string[]): Promise { + throw new RequestAbortedError(); } async confirmNewCredential(): Promise {