From 124ba75b176d7edf9e504cd6877a90c8ca52c3e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Gonc=CC=A7alves?= Date: Tue, 29 Aug 2023 17:06:31 +0100 Subject: [PATCH 01/81] PM-1235 Added component to display passkey on auth flow --- apps/browser/src/popup/app.module.ts | 2 ++ .../fido2/fido2-cipher-row.component.html | 26 +++++++++++++++++++ .../fido2/fido2-cipher-row.component.ts | 18 +++++++++++++ .../components/fido2/fido2.component.html | 16 +++--------- 4 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html create mode 100644 apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts diff --git a/apps/browser/src/popup/app.module.ts b/apps/browser/src/popup/app.module.ts index 7e4e1512844..527644635ef 100644 --- a/apps/browser/src/popup/app.module.ts +++ b/apps/browser/src/popup/app.module.ts @@ -40,6 +40,7 @@ import { SendTypeComponent } from "../tools/popup/send/send-type.component"; import { ExportComponent } from "../tools/popup/settings/export.component"; import { ActionButtonsComponent } from "../vault/popup/components/action-buttons.component"; import { CipherRowComponent } from "../vault/popup/components/cipher-row.component"; +import { Fido2CipherRowComponent } from "../vault/popup/components/fido2/fido2-cipher-row.component"; import { Fido2Component } from "../vault/popup/components/fido2/fido2.component"; import { PasswordRepromptComponent } from "../vault/popup/components/password-reprompt.component"; import { AddEditCustomFieldsComponent } from "../vault/popup/components/vault/add-edit-custom-fields.component"; @@ -115,6 +116,7 @@ import "../platform/popup/locales"; EnvironmentComponent, ExcludedDomainsComponent, ExportComponent, + Fido2CipherRowComponent, FolderAddEditComponent, FoldersComponent, VaultFilterComponent, diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html new file mode 100644 index 00000000000..22000ea7e7f --- /dev/null +++ b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html @@ -0,0 +1,26 @@ +
+
+ +
+
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts new file mode 100644 index 00000000000..343effeb5f1 --- /dev/null +++ b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts @@ -0,0 +1,18 @@ +import { Component, EventEmitter, Input, Output } from "@angular/core"; + +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; + +@Component({ + selector: "app-fido2-cipher-row", + templateUrl: "fido2-cipher-row.component.html", +}) +export class Fido2CipherRowComponent { + @Output() onSelected = new EventEmitter(); + @Input() cipher: CipherView; + @Input() last: boolean; + @Input() title: string; + + selectCipher(c: CipherView) { + this.onSelected.emit(c); + } +} diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index 0be33e50c3f..2df514ac377 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -1,6 +1,7 @@
+ Verification required by the initiating site. This feature is not yet implemented for accounts @@ -13,14 +14,13 @@ data.message.type == 'ConfirmNewNonDiscoverableCredentialRequest' " > - A site is asking for authentication, please choose one of the following credentials to use:
- + >
@@ -45,14 +45,6 @@ You do not have a matching login for this site. - - + Use browser
From 3adb863d15b4803851f8e7e76343b8dab9f58508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Gonc=CC=A7alves?= Date: Thu, 31 Aug 2023 10:40:44 +0100 Subject: [PATCH 02/81] PM-1235 Implement basic structure and behaviour of UI --- apps/browser/src/popup/scss/base.scss | 65 ++++++++++++++++--- .../fido2/fido2-cipher-row.component.html | 4 +- .../components/fido2/fido2.component.html | 49 +++++++++----- .../popup/components/fido2/fido2.component.ts | 9 ++- 4 files changed, 98 insertions(+), 29 deletions(-) diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index bef8307e57a..360197eef1c 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -622,19 +622,66 @@ app-fido2 { right: 0; bottom: 0; left: 0; - display: flex; - align-items: center; - justify-content: center; + flex-direction: column; - padding: 0 25px; + padding: 12px 24px 12px 24px; - .btn { - margin-top: 25px; + .logo-image { + margin: 0; + width: 197px; + height: 30px; + background-size: 197px 30px; + background-repeat: no-repeat; + @include themify($themes) { + background-image: url("../images/logo-" + themed("logoSuffix") + "@2x.png"); + } } - } - .box.list { - overflow-y: auto; + .auth-flow { + display: flex; + align-items: flex-start; + flex-direction: column; + margin-top: 32px; + margin-bottom: 32px; + + .subtitle { + font-family: Open Sans; + font-size: 24px; + font-style: normal; + font-weight: 600; + line-height: 32px; + } + + .box.list { + overflow-y: auto; + } + + .box-content-row { + display: flex; + justify-content: center; + align-items: center; + margin: 0px; + padding: 0px; + margin-bottom: 12px; + + .row-main { + border-radius: 6px; + padding: 5px 0px 5px 12px; + + &:focus { + @include themify($themes) { + border-left: 5px solid themed("headerInputBackgroundFocusColor"); + background-color: themed("headerBackgroundHoverColor"); + color: themed("headerColor"); + } + } + } + } + + .btn { + width: 100%; + } + } } } diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html index 22000ea7e7f..ab028afd67d 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html @@ -4,10 +4,10 @@ class="virtual-scroll-item" [ngClass]="{ 'override-last': !last }" > -
+
- A site wants to create the following passkey in your vault -
-
- +
+

Save passkey in Bitwarden?

+
+
+ +
+
- - A passkey already exists in Bitwarden for this account -
-
- +
+

A passkey already exists in Bitwarden for this account

+
+
+ +
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index 751f0aa7c69..4e33cc49779 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -126,7 +126,8 @@ export class Fido2Component implements OnInit, OnDestroy { }); } - async pick(cipher: CipherView) { + async pick() { + const cipher = this.selectedItem; const data = this.message$.value; if (data?.type === "PickCredentialRequest") { let userVerified = false; @@ -157,6 +158,12 @@ export class Fido2Component implements OnInit, OnDestroy { this.loading = true; } + selectedItem: CipherView; + + selectedPasskey(item: CipherView) { + this.selectedItem = item; + } + async confirm() { const data = this.message$.value; if (data.type !== "ConfirmNewCredentialRequest") { From dac87061d7bc9a105ecc560cfe4dbeaa0c14e783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Gonc=CC=A7alves?= Date: Thu, 31 Aug 2023 13:50:25 +0100 Subject: [PATCH 03/81] PM-1235 Added localised strings --- apps/browser/src/_locales/en/messages.json | 33 +++++++++++++++++++ .../components/fido2/fido2.component.html | 23 ++++++++----- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index b068befd0d0..99feee49053 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -2439,5 +2439,38 @@ }, "passkeyNotCopiedAlert": { "message": "The passkey will not be copied to the cloned item. Do you want to continue cloning this item?" + }, + "passkeyFeatureIsNotImplementedForAccountsWithoutMasterPassword": { + "message": "Verification required by the initiating site. This feature is not yet implemented for accounts without master password." + }, + "logInWithPasskey": { + "message": "Log in with passkey?" + }, + "savePasskeyInBitwarden": { + "message": "Save passkey in Bitwarden?" + }, + "passkeyAlreadyExistsInBitwardenForThisAccount": { + "message": "A passkey already exists in Bitwarden for this account" + }, + "noPasskeysFoundForThisApplication": { + "message": "No passkeys found for this application." + }, + "confirm": { + "message": "Confirm" + }, + "savePasskey": { + "message": "Save passkey" + }, + "viewPasskey": { + "message": "View passkey" + }, + "useBrowserName": { + "message": "Use $browserName$", + "placeholders": { + "browserName": { + "content": "$1", + "example": "Chrome" + } + } } } diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index 9ebbcb9a2dc..1518fecdf13 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -4,8 +4,7 @@
- Verification required by the initiating site. This feature is not yet implemented for accounts - without master password. + {{ "passkeyFeatureIsNotImplementedForAccountsWithoutMasterPassword" | i18n }}
-

Log in with passkey?

+

{{ "LogInWithPasskey" | i18n }}

-

Save passkey in Bitwarden?

+

{{ "SavePasskeyInBitwarden" | i18n }}

- +
-

A passkey already exists in Bitwarden for this account

+

{{ "passkeyAlreadyExistsInBitwardenForThisAccount" | i18n }}

- You do not have a matching login for this site. +
+

{{ "noPasskeysFoundForThisApplication" | i18n }}

+
- Use browser + + {{ "useBrowserName" | i18n }} Chrome +
From 7db1cf4086204c2c61cec2eab391bdd181a22309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Gonc=CC=A7alves?= Date: Thu, 31 Aug 2023 14:53:50 +0100 Subject: [PATCH 04/81] PM-1235 Improved button UI --- apps/browser/src/popup/scss/base.scss | 2 ++ .../vault/popup/components/fido2/fido2.component.html | 9 +++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index 360197eef1c..03e49526d0c 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -680,6 +680,8 @@ app-fido2 { .btn { width: 100%; + font-size: 16px; + font-weight: 600; } } } diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index 1518fecdf13..cea66e1185b 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -2,7 +2,6 @@
- {{ "passkeyFeatureIsNotImplementedForAccountsWithoutMasterPassword" | i18n }} @@ -26,7 +25,8 @@
@@ -40,7 +40,8 @@
@@ -64,7 +65,7 @@ - {{ "useBrowserName" | i18n }} Chrome + {{ "useBrowserName" | i18n }} browser
From 30d7bbbff5cc75e878ad9f79e25573e4216a45be Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Mon, 4 Sep 2023 17:40:48 -0400 Subject: [PATCH 05/81] Implemented view passkey button --- .../vault/popup/components/fido2/fido2.component.html | 6 +++++- .../vault/popup/components/fido2/fido2.component.ts | 11 ++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index cea66e1185b..d2486c049f9 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -40,7 +40,7 @@
@@ -56,6 +56,10 @@ > + diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index 4e33cc49779..d95d70535d0 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -1,5 +1,5 @@ import { Component, HostListener, OnDestroy, OnInit } from "@angular/core"; -import { ActivatedRoute } from "@angular/router"; +import { ActivatedRoute, Router } from "@angular/router"; import { BehaviorSubject, combineLatest, @@ -36,6 +36,7 @@ interface ViewData { styleUrls: [], }) export class Fido2Component implements OnInit, OnDestroy { + selectedItem: CipherView; private destroy$ = new Subject(); protected data$: Observable; @@ -46,6 +47,7 @@ export class Fido2Component implements OnInit, OnDestroy { private message$ = new BehaviorSubject(null); constructor( + private router: Router, private activatedRoute: ActivatedRoute, private cipherService: CipherService, private passwordRepromptService: PasswordRepromptService @@ -158,8 +160,6 @@ export class Fido2Component implements OnInit, OnDestroy { this.loading = true; } - selectedItem: CipherView; - selectedPasskey(item: CipherView) { this.selectedItem = item; } @@ -183,6 +183,11 @@ export class Fido2Component implements OnInit, OnDestroy { this.loading = true; } + viewPasskey() { + const cipher = this.ciphers[0]; + this.router.navigate(["/view-cipher"], { queryParams: { cipherId: cipher.id } }); + } + abort(fallback: boolean) { this.unload(fallback); window.close(); From aaafac0e81eec58b8c5603cb1c513655bab2ba49 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Mon, 4 Sep 2023 20:01:01 -0400 Subject: [PATCH 06/81] Implemented multiple matching passkeys --- apps/browser/src/_locales/en/messages.json | 3 +++ .../src/vault/popup/components/fido2/fido2.component.html | 4 ++-- .../src/vault/popup/components/fido2/fido2.component.ts | 4 ++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 99feee49053..75aaf7eb021 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -2464,6 +2464,9 @@ "viewPasskey": { "message": "View passkey" }, + "choosePasskey": { + "message": "Choose a passkey to login with" + }, "useBrowserName": { "message": "Use $browserName$", "placeholders": { diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index d2486c049f9..47c316430c0 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -13,7 +13,7 @@ " >
-

{{ "LogInWithPasskey" | i18n }}

+

{{ pickCredentialSubTitleText | i18n }}

diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index d95d70535d0..72d97b236b6 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -46,6 +46,10 @@ export class Fido2Component implements OnInit, OnDestroy { private message$ = new BehaviorSubject(null); + get pickCredentialSubTitleText(): string { + return this.ciphers.length > 1 ? "choosePasskey" : "logInWithPasskey"; + } + constructor( private router: Router, private activatedRoute: ActivatedRoute, From e39a106000caee1c8787730cc519d9451cead7cd Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Thu, 7 Sep 2023 13:43:04 -0400 Subject: [PATCH 07/81] Refactored fido2 popup to use browser popout windows service --- .../browser/src/background/main.background.ts | 5 +- .../src/background/runtime.background.ts | 4 +- .../browser-popout-window.service.ts | 8 +++ .../popup/browser-popout-window.service.ts | 37 +++++++++- apps/browser/src/popup/scss/base.scss | 4 ++ .../browser-fido2-user-interface.service.ts | 33 ++++++--- .../components/fido2/fido2.component.html | 2 +- ...fido2-authenticator.service.abstraction.ts | 2 + .../fido2/fido2-client.service.abstraction.ts | 2 + ...ido2-user-interface.service.abstraction.ts | 1 + .../fido2/fido2-authenticator.service.spec.ts | 68 ++++++++++--------- .../fido2/fido2-authenticator.service.ts | 5 +- .../fido2/fido2-client.service.spec.ts | 65 ++++++++++++------ .../services/fido2/fido2-client.service.ts | 12 ++++ 14 files changed, 175 insertions(+), 73 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 75734fcdad4..b75cded1c74 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -564,7 +564,10 @@ export default class MainBackground { this.browserPopoutWindowService = new BrowserPopoutWindowService(); this.popupUtilsService = new PopupUtilsService(this.isPrivateMode); - this.fido2UserInterfaceService = new BrowserFido2UserInterfaceService(this.popupUtilsService); + this.fido2UserInterfaceService = new BrowserFido2UserInterfaceService( + this.popupUtilsService, + this.browserPopoutWindowService + ); this.fido2AuthenticatorService = new Fido2AuthenticatorService( this.cipherService, this.fido2UserInterfaceService, diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index f8754798241..dca8aa1ee72 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -252,11 +252,11 @@ export default class RuntimeBackground { return await this.main.fido2ClientService.isFido2FeatureEnabled(); case "fido2RegisterCredentialRequest": return await this.main.fido2ClientService - .createCredential(msg.data, this.createAbortController(msg.requestId)) + .createCredential(msg.data, sender.tab, this.createAbortController(msg.requestId)) .finally(() => this.abortControllers.delete(msg.requestId)); case "fido2GetCredentialRequest": return await this.main.fido2ClientService - .assertCredential(msg.data, this.createAbortController(msg.requestId)) + .assertCredential(msg.data, sender.tab, this.createAbortController(msg.requestId)) .finally(() => this.abortControllers.delete(msg.requestId)); } } diff --git a/apps/browser/src/platform/popup/abstractions/browser-popout-window.service.ts b/apps/browser/src/platform/popup/abstractions/browser-popout-window.service.ts index 0b3f55ee990..362e17facf7 100644 --- a/apps/browser/src/platform/popup/abstractions/browser-popout-window.service.ts +++ b/apps/browser/src/platform/popup/abstractions/browser-popout-window.service.ts @@ -10,6 +10,14 @@ interface BrowserPopoutWindowService { } ): Promise; closePasswordRepromptPrompt(): Promise; + openFido2Popout( + senderWindowId: number, + promptData: { + sessionId: string; + senderTabId: number; + } + ): Promise; + closeFido2Popout(): Promise; } export { BrowserPopoutWindowService }; diff --git a/apps/browser/src/platform/popup/browser-popout-window.service.ts b/apps/browser/src/platform/popup/browser-popout-window.service.ts index 95be15cc20d..f4bc4e54935 100644 --- a/apps/browser/src/platform/popup/browser-popout-window.service.ts +++ b/apps/browser/src/platform/popup/browser-popout-window.service.ts @@ -52,24 +52,55 @@ class BrowserPopoutWindowService implements BrowserPopupWindowServiceInterface { await this.closeSingleActionPopout("passwordReprompt"); } + async openFido2Popout( + senderWindowId: number, + { + sessionId, + senderTabId, + }: { + sessionId: string; + senderTabId: number; + } + ): Promise { + await this.closeFido2Popout(); + + const promptWindowPath = + "popup/index.html#/fido2" + + "?uilocation=popout" + + `&sessionId=${sessionId}` + + `&senderTabId=${senderTabId}`; + + await this.openSingleActionPopout(senderWindowId, promptWindowPath, "fido2Popout", { + width: 200, + height: 500, + }); + } + + async closeFido2Popout(): Promise { + await this.closeSingleActionPopout("fido2Popout"); + } + private async openSingleActionPopout( senderWindowId: number, popupWindowURL: string, - singleActionPopoutKey: string + singleActionPopoutKey: string, + customDimensions: { width?: number; height?: number } = {} ) { const senderWindow = senderWindowId && (await BrowserApi.getWindow(senderWindowId)); const url = chrome.extension.getURL(popupWindowURL); const offsetRight = 15; const offsetTop = 90; - const popupWidth = this.defaultPopoutWindowOptions.width; + // Use custom dimensions if provided, otherwise use default + const popupWidth = customDimensions.width || this.defaultPopoutWindowOptions.width; const windowOptions = senderWindow ? { ...this.defaultPopoutWindowOptions, + ...customDimensions, url, left: senderWindow.left + senderWindow.width - popupWidth - offsetRight, top: senderWindow.top + offsetTop, } - : { ...this.defaultPopoutWindowOptions, url }; + : { ...this.defaultPopoutWindowOptions, ...customDimensions, url }; const popupWindow = await BrowserApi.createWindow(windowOptions); diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index 03e49526d0c..3c7b945b51d 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -656,6 +656,10 @@ app-fido2 { overflow-y: auto; } + .box-content { + max-height: 145px; + } + .box-content-row { display: flex; justify-content: center; 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 2c2901c7d53..258e2bb95db 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 @@ -20,7 +20,8 @@ import { } from "@bitwarden/common/vault/abstractions/fido2/fido2-user-interface.service.abstraction"; import { BrowserApi } from "../../platform/browser/browser-api"; -import { Popout, PopupUtilsService } from "../../popup/services/popup-utils.service"; +import { BrowserPopoutWindowService } from "../../platform/popup/abstractions/browser-popout-window.service"; +import { PopupUtilsService } from "../../popup/services/popup-utils.service"; const BrowserFido2MessageName = "BrowserFido2UserInterfaceServiceMessage"; @@ -97,15 +98,21 @@ export type BrowserFido2Message = { sessionId: string } & ( ); export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServiceAbstraction { - constructor(private popupUtilsService: PopupUtilsService) {} + constructor( + private popupUtilsService: PopupUtilsService, + private browserPopoutWindowService: BrowserPopoutWindowService + ) {} async newSession( fallbackSupported: boolean, + tab: chrome.tabs.Tab, abortController?: AbortController ): Promise { return await BrowserFido2UserInterfaceSession.create( this.popupUtilsService, + this.browserPopoutWindowService, fallbackSupported, + tab, abortController ); } @@ -114,12 +121,16 @@ export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServi export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSession { static async create( popupUtilsService: PopupUtilsService, + browserPopoutWindowService: BrowserPopoutWindowService, fallbackSupported: boolean, + tab: chrome.tabs.Tab, abortController?: AbortController ): Promise { return new BrowserFido2UserInterfaceSession( popupUtilsService, + browserPopoutWindowService, fallbackSupported, + tab, abortController ); } @@ -134,11 +145,12 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi ); private connected$ = new BehaviorSubject(false); private destroy$ = new Subject(); - private popout?: Popout; private constructor( private readonly popupUtilsService: PopupUtilsService, + private readonly browserPopoutWindowService: BrowserPopoutWindowService, private readonly fallbackSupported: boolean, + private readonly tab: chrome.tabs.Tab, readonly abortController = new AbortController(), readonly sessionId = Utils.newGuid() ) { @@ -273,7 +285,7 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi } async close() { - this.popupUtilsService.closePopOut(this.popout); + this.browserPopoutWindowService.closeFido2Popout(); this.closed = true; this.destroy$.next(); this.destroy$.complete(); @@ -309,17 +321,16 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi if (this.closed) { throw new Error("Cannot re-open closed session"); } - - const queryParams = new URLSearchParams({ sessionId: this.sessionId }).toString(); // create promise first to avoid race condition where the popout opens before we start listening const connectPromise = firstValueFrom( this.connected$.pipe(filter((connected) => connected === true)) ); - this.popout = await this.popupUtilsService.popOut( - null, - `popup/index.html?uilocation=popout#/fido2?${queryParams}`, - { center: true } - ); + + await this.browserPopoutWindowService.openFido2Popout(this.tab.windowId, { + sessionId: this.sessionId, + senderTabId: this.tab.id, + }); + await connectPromise; } } diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index 47c316430c0..225d45315f7 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -68,7 +68,7 @@
- + {{ "useBrowserName" | i18n }} browser
diff --git a/libs/common/src/vault/abstractions/fido2/fido2-authenticator.service.abstraction.ts b/libs/common/src/vault/abstractions/fido2/fido2-authenticator.service.abstraction.ts index 3e842700bc3..50ceb7b2193 100644 --- a/libs/common/src/vault/abstractions/fido2/fido2-authenticator.service.abstraction.ts +++ b/libs/common/src/vault/abstractions/fido2/fido2-authenticator.service.abstraction.ts @@ -6,6 +6,7 @@ export abstract class Fido2AuthenticatorService { **/ makeCredential: ( params: Fido2AuthenticatorMakeCredentialsParams, + tab: chrome.tabs.Tab, abortController?: AbortController ) => Promise; @@ -14,6 +15,7 @@ export abstract class Fido2AuthenticatorService { */ getAssertion: ( params: Fido2AuthenticatorGetAssertionParams, + tab: chrome.tabs.Tab, abortController?: AbortController ) => Promise; } diff --git a/libs/common/src/vault/abstractions/fido2/fido2-client.service.abstraction.ts b/libs/common/src/vault/abstractions/fido2/fido2-client.service.abstraction.ts index f888b5d6892..abada959a91 100644 --- a/libs/common/src/vault/abstractions/fido2/fido2-client.service.abstraction.ts +++ b/libs/common/src/vault/abstractions/fido2/fido2-client.service.abstraction.ts @@ -5,10 +5,12 @@ export type UserVerification = "discouraged" | "preferred" | "required"; export abstract class Fido2ClientService { createCredential: ( params: CreateCredentialParams, + tab: chrome.tabs.Tab, abortController?: AbortController ) => Promise; assertCredential: ( params: AssertCredentialParams, + tab: chrome.tabs.Tab, abortController?: AbortController ) => Promise; isFido2FeatureEnabled: () => Promise; diff --git a/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts b/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts index 3d0db42d3c1..ee0fd3d0396 100644 --- a/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts +++ b/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts @@ -12,6 +12,7 @@ export interface PickCredentialParams { export abstract class Fido2UserInterfaceService { newSession: ( fallbackSupported: boolean, + tab: chrome.tabs.Tab, abortController?: AbortController ) => Promise; } diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts index 92898df9522..5e74a4e9eeb 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts @@ -31,6 +31,7 @@ describe("FidoAuthenticatorService", () => { let userInterface!: MockProxy; let userInterfaceSession!: MockProxy; let authenticator!: Fido2AuthenticatorService; + let tab!: any; beforeEach(async () => { cipherService = mock(); @@ -38,6 +39,7 @@ describe("FidoAuthenticatorService", () => { userInterfaceSession = mock(); userInterface.newSession.mockResolvedValue(userInterfaceSession); authenticator = new Fido2AuthenticatorService(cipherService, userInterface); + tab = { id: 123, windowId: 456 }; }); describe("makeCredential", () => { @@ -51,19 +53,19 @@ describe("FidoAuthenticatorService", () => { // Spec: Check if at least one of the specified combinations of PublicKeyCredentialType and cryptographic parameters in credTypesAndPubKeyAlgs is supported. If not, return an error code equivalent to "NotSupportedError" and terminate the operation. it("should throw error when input does not contain any supported algorithms", async () => { const result = async () => - await authenticator.makeCredential(invalidParams.unsupportedAlgorithm); + await authenticator.makeCredential(invalidParams.unsupportedAlgorithm, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotSupported); }); it("should throw error when requireResidentKey has invalid value", async () => { - const result = async () => await authenticator.makeCredential(invalidParams.invalidRk); + const result = async () => await authenticator.makeCredential(invalidParams.invalidRk, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Unknown); }); it("should throw error when requireUserVerification has invalid value", async () => { - const result = async () => await authenticator.makeCredential(invalidParams.invalidUv); + const result = async () => await authenticator.makeCredential(invalidParams.invalidUv, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Unknown); }); @@ -76,7 +78,7 @@ describe("FidoAuthenticatorService", () => { it.skip("should throw error if requireUserVerification is set to true", async () => { const params = await createParams({ requireUserVerification: true }); - const result = async () => await authenticator.makeCredential(params); + const result = async () => await authenticator.makeCredential(params, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Constraint); }); @@ -90,7 +92,7 @@ describe("FidoAuthenticatorService", () => { for (const p of Object.values(invalidParams)) { try { - await authenticator.makeCredential(p); + await authenticator.makeCredential(p, tab); // eslint-disable-next-line no-empty } catch {} } @@ -131,7 +133,7 @@ describe("FidoAuthenticatorService", () => { userInterfaceSession.informExcludedCredential.mockResolvedValue(); try { - await authenticator.makeCredential(params); + await authenticator.makeCredential(params, tab); // eslint-disable-next-line no-empty } catch {} @@ -142,7 +144,7 @@ describe("FidoAuthenticatorService", () => { it("should throw error", async () => { userInterfaceSession.informExcludedCredential.mockResolvedValue(); - const result = async () => await authenticator.makeCredential(params); + const result = async () => await authenticator.makeCredential(params, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed); }); @@ -153,7 +155,7 @@ describe("FidoAuthenticatorService", () => { excludedCipher.organizationId = "someOrganizationId"; try { - await authenticator.makeCredential(params); + await authenticator.makeCredential(params, tab); // eslint-disable-next-line no-empty } catch {} @@ -166,7 +168,7 @@ describe("FidoAuthenticatorService", () => { for (const p of Object.values(invalidParams)) { try { - await authenticator.makeCredential(p); + await authenticator.makeCredential(p, tab); // eslint-disable-next-line no-empty } catch {} } @@ -205,7 +207,7 @@ describe("FidoAuthenticatorService", () => { userInterfaceSession.informExcludedCredential.mockResolvedValue(); try { - await authenticator.makeCredential(params); + await authenticator.makeCredential(params, tab); // eslint-disable-next-line no-empty } catch {} @@ -216,7 +218,7 @@ describe("FidoAuthenticatorService", () => { it("should throw error", async () => { userInterfaceSession.informExcludedCredential.mockResolvedValue(); - const result = async () => await authenticator.makeCredential(params); + const result = async () => await authenticator.makeCredential(params, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed); }); @@ -227,7 +229,7 @@ describe("FidoAuthenticatorService", () => { excludedCipherView.organizationId = "someOrganizationId"; try { - await authenticator.makeCredential(params); + await authenticator.makeCredential(params, tab); // eslint-disable-next-line no-empty } catch {} @@ -240,7 +242,7 @@ describe("FidoAuthenticatorService", () => { for (const p of Object.values(invalidParams)) { try { - await authenticator.makeCredential(p); + await authenticator.makeCredential(p, tab); // eslint-disable-next-line no-empty } catch {} } @@ -278,7 +280,7 @@ describe("FidoAuthenticatorService", () => { return cipher; }); - await authenticator.makeCredential(params, new AbortController()); + await authenticator.makeCredential(params, tab, new AbortController()); expect(userInterfaceSession.confirmNewCredential).toHaveBeenCalledWith( { @@ -303,7 +305,7 @@ describe("FidoAuthenticatorService", () => { return cipher; }); - await authenticator.makeCredential(params); + await authenticator.makeCredential(params, tab); const saved = cipherService.encrypt.mock.lastCall?.[0]; expect(saved).toEqual( @@ -334,7 +336,7 @@ describe("FidoAuthenticatorService", () => { userVerified: false, }); - const result = async () => await authenticator.makeCredential(params); + const result = async () => await authenticator.makeCredential(params, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed); }); @@ -349,7 +351,7 @@ describe("FidoAuthenticatorService", () => { cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as Cipher); cipherService.createWithServer.mockRejectedValue(new Error("Internal error")); - const result = async () => await authenticator.makeCredential(params); + const result = async () => await authenticator.makeCredential(params, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Unknown); }); @@ -380,7 +382,7 @@ describe("FidoAuthenticatorService", () => { userVerified: userVerification, }); - await authenticator.makeCredential(params, new AbortController()); + await authenticator.makeCredential(params, tab, new AbortController()); expect(userInterfaceSession.confirmNewNonDiscoverableCredential).toHaveBeenCalledWith( { @@ -401,7 +403,7 @@ describe("FidoAuthenticatorService", () => { }); cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as Cipher); - await authenticator.makeCredential(params); + await authenticator.makeCredential(params, tab); const saved = cipherService.encrypt.mock.lastCall?.[0]; expect(saved).toEqual( @@ -435,7 +437,7 @@ describe("FidoAuthenticatorService", () => { }); const params = await createParams(); - const result = async () => await authenticator.makeCredential(params); + const result = async () => await authenticator.makeCredential(params, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed); }); @@ -450,7 +452,7 @@ describe("FidoAuthenticatorService", () => { cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as Cipher); cipherService.updateWithServer.mockRejectedValue(new Error("Internal error")); - const result = async () => await authenticator.makeCredential(params); + const result = async () => await authenticator.makeCredential(params, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Unknown); }); @@ -504,7 +506,7 @@ describe("FidoAuthenticatorService", () => { }); it("should return attestation object", async () => { - const result = await authenticator.makeCredential(params); + const result = await authenticator.makeCredential(params, tab); const attestationObject = CBOR.decode( Fido2Utils.bufferSourceToUint8Array(result.attestationObject).buffer @@ -606,7 +608,7 @@ describe("FidoAuthenticatorService", () => { describe("invalid input parameters", () => { it("should throw error when requireUserVerification has invalid value", async () => { - const result = async () => await authenticator.getAssertion(invalidParams.invalidUv); + const result = async () => await authenticator.getAssertion(invalidParams.invalidUv, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Unknown); }); @@ -619,7 +621,7 @@ describe("FidoAuthenticatorService", () => { it.skip("should throw error if requireUserVerification is set to true", async () => { const params = await createParams({ requireUserVerification: true }); - const result = async () => await authenticator.getAssertion(params); + const result = async () => await authenticator.getAssertion(params, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Constraint); }); @@ -648,7 +650,7 @@ describe("FidoAuthenticatorService", () => { userInterfaceSession.informCredentialNotFound.mockResolvedValue(); try { - await authenticator.getAssertion(params); + await authenticator.getAssertion(params, tab); // eslint-disable-next-line no-empty } catch {} @@ -663,7 +665,7 @@ describe("FidoAuthenticatorService", () => { userInterfaceSession.informCredentialNotFound.mockResolvedValue(); try { - await authenticator.getAssertion(params); + await authenticator.getAssertion(params, tab); // eslint-disable-next-line no-empty } catch {} @@ -684,7 +686,7 @@ describe("FidoAuthenticatorService", () => { /** Spec: If credentialOptions is now empty, return an error code equivalent to "NotAllowedError" and terminate the operation. */ it("should throw error", async () => { - const result = async () => await authenticator.getAssertion(params); + const result = async () => await authenticator.getAssertion(params, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed); }); @@ -726,7 +728,7 @@ describe("FidoAuthenticatorService", () => { userVerified: userVerification, }); - await authenticator.getAssertion(params); + await authenticator.getAssertion(params, tab); expect(userInterfaceSession.pickCredential).toHaveBeenCalledWith({ cipherIds: ciphers.map((c) => c.id), @@ -742,7 +744,7 @@ describe("FidoAuthenticatorService", () => { userVerified: false, }); - const result = async () => await authenticator.getAssertion(params); + const result = async () => await authenticator.getAssertion(params, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed); }); @@ -808,7 +810,7 @@ describe("FidoAuthenticatorService", () => { const encrypted = Symbol(); cipherService.encrypt.mockResolvedValue(encrypted as any); - await authenticator.getAssertion(params); + await authenticator.getAssertion(params, tab); expect(cipherService.updateWithServer).toHaveBeenCalledWith(encrypted); if (residentKey) { @@ -835,7 +837,7 @@ describe("FidoAuthenticatorService", () => { }); it("should return an assertion result", async () => { - const result = await authenticator.getAssertion(params); + const result = await authenticator.getAssertion(params, tab); const encAuthData = result.authenticatorData; const rpIdHash = encAuthData.slice(0, 32); @@ -876,7 +878,7 @@ describe("FidoAuthenticatorService", () => { for (let i = 0; i < 10; ++i) { await init(); // Reset inputs - const result = await authenticator.getAssertion(params); + const result = await authenticator.getAssertion(params, tab); const counter = result.authenticatorData.slice(33, 37); expect(counter).toEqual(new Uint8Array([0, 0, 0x23, 0x29])); // double check that the counter doesn't change @@ -893,7 +895,7 @@ describe("FidoAuthenticatorService", () => { it("should throw unkown error if creation fails", async () => { cipherService.updateWithServer.mockRejectedValue(new Error("Internal error")); - const result = async () => await authenticator.getAssertion(params); + const result = async () => await authenticator.getAssertion(params, tab); await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Unknown); }); diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts index c586b24679e..72fa61f5db7 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts @@ -41,10 +41,12 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr ) {} async makeCredential( params: Fido2AuthenticatorMakeCredentialsParams, + tab: any, abortController?: AbortController ): Promise { const userInterfaceSession = await this.userInterface.newSession( params.fallbackSupported, + tab, abortController ); @@ -212,13 +214,14 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr async getAssertion( params: Fido2AuthenticatorGetAssertionParams, + tab: any, abortController?: AbortController ): Promise { const userInterfaceSession = await this.userInterface.newSession( params.fallbackSupported, + tab, abortController ); - try { if ( params.requireUserVerification != undefined && diff --git a/libs/common/src/vault/services/fido2/fido2-client.service.spec.ts b/libs/common/src/vault/services/fido2/fido2-client.service.spec.ts index a22091625f2..290ac38c0a7 100644 --- a/libs/common/src/vault/services/fido2/fido2-client.service.spec.ts +++ b/libs/common/src/vault/services/fido2/fido2-client.service.spec.ts @@ -24,21 +24,32 @@ describe("FidoAuthenticatorService", () => { let authenticator!: MockProxy; let configService!: MockProxy; let client!: Fido2ClientService; + let tab!: any; beforeEach(async () => { authenticator = mock(); configService = mock(); client = new Fido2ClientService(authenticator, configService); configService.getFeatureFlagBool.mockResolvedValue(true); + tab = { id: 123, windowId: 456 }; }); describe("createCredential", () => { describe("input parameters validation", () => { + // Spec: If tab is null, should not call makeCredential + it("should not call makeCredential if tab is null", async () => { + const params = createParams({ sameOriginWithAncestors: false }); + + await client.createCredential(params, null); + + expect(authenticator.makeCredential).not.toHaveBeenCalled(); + }); + // Spec: If sameOriginWithAncestors is false, return a "NotAllowedError" DOMException. it("should throw error if sameOriginWithAncestors is false", async () => { const params = createParams({ sameOriginWithAncestors: false }); - const result = async () => await client.createCredential(params); + const result = async () => await client.createCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "NotAllowedError" }); @@ -49,7 +60,7 @@ describe("FidoAuthenticatorService", () => { it("should throw error if user.id is too small", async () => { const params = createParams({ user: { id: "", displayName: "name" } }); - const result = async () => await client.createCredential(params); + const result = async () => await client.createCredential(params, tab); await expect(result).rejects.toBeInstanceOf(TypeError); }); @@ -63,7 +74,7 @@ describe("FidoAuthenticatorService", () => { }, }); - const result = async () => await client.createCredential(params); + const result = async () => await client.createCredential(params, tab); await expect(result).rejects.toBeInstanceOf(TypeError); }); @@ -78,7 +89,7 @@ describe("FidoAuthenticatorService", () => { origin: "invalid-domain-name", }); - const result = async () => await client.createCredential(params); + const result = async () => await client.createCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "SecurityError" }); @@ -92,7 +103,7 @@ describe("FidoAuthenticatorService", () => { rp: { id: "bitwarden.com", name: "Bitwraden" }, }); - const result = async () => await client.createCredential(params); + const result = async () => await client.createCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "SecurityError" }); @@ -105,7 +116,7 @@ describe("FidoAuthenticatorService", () => { rp: { id: "bitwarden.com", name: "Bitwraden" }, }); - const result = async () => await client.createCredential(params); + const result = async () => await client.createCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "SecurityError" }); @@ -121,7 +132,7 @@ describe("FidoAuthenticatorService", () => { ], }); - const result = async () => await client.createCredential(params); + const result = async () => await client.createCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "NotSupportedError" }); @@ -136,7 +147,7 @@ describe("FidoAuthenticatorService", () => { const abortController = new AbortController(); abortController.abort(); - const result = async () => await client.createCredential(params, abortController); + const result = async () => await client.createCredential(params, tab, abortController); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "AbortError" }); @@ -151,7 +162,7 @@ describe("FidoAuthenticatorService", () => { }); authenticator.makeCredential.mockResolvedValue(createAuthenticatorMakeResult()); - await client.createCredential(params); + await client.createCredential(params, tab); expect(authenticator.makeCredential).toHaveBeenCalledWith( expect.objectContaining({ @@ -164,6 +175,7 @@ describe("FidoAuthenticatorService", () => { displayName: params.user.displayName, }), }), + tab, expect.anything() ); }); @@ -175,7 +187,7 @@ describe("FidoAuthenticatorService", () => { new Fido2AutenticatorError(Fido2AutenticatorErrorCode.InvalidState) ); - const result = async () => await client.createCredential(params); + const result = async () => await client.createCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "InvalidStateError" }); @@ -187,7 +199,7 @@ describe("FidoAuthenticatorService", () => { const params = createParams(); authenticator.makeCredential.mockRejectedValue(new Error("unknown error")); - const result = async () => await client.createCredential(params); + const result = async () => await client.createCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "NotAllowedError" }); @@ -198,7 +210,7 @@ describe("FidoAuthenticatorService", () => { const params = createParams(); configService.getFeatureFlagBool.mockResolvedValue(false); - const result = async () => await client.createCredential(params); + const result = async () => await client.createCredential(params, tab); const rejects = expect(result).rejects; await rejects.toThrow(FallbackRequestedError); @@ -255,7 +267,7 @@ describe("FidoAuthenticatorService", () => { origin: "invalid-domain-name", }); - const result = async () => await client.assertCredential(params); + const result = async () => await client.assertCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "SecurityError" }); @@ -269,7 +281,7 @@ describe("FidoAuthenticatorService", () => { rpId: "bitwarden.com", }); - const result = async () => await client.assertCredential(params); + const result = async () => await client.assertCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "SecurityError" }); @@ -282,7 +294,7 @@ describe("FidoAuthenticatorService", () => { rpId: "bitwarden.com", }); - const result = async () => await client.assertCredential(params); + const result = async () => await client.assertCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "SecurityError" }); @@ -297,7 +309,7 @@ describe("FidoAuthenticatorService", () => { const abortController = new AbortController(); abortController.abort(); - const result = async () => await client.assertCredential(params, abortController); + const result = async () => await client.assertCredential(params, tab, abortController); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "AbortError" }); @@ -306,6 +318,15 @@ describe("FidoAuthenticatorService", () => { }); describe("assert credential", () => { + // Spec: If tab is null, should not call getAssertion + it("should not call getAssertion if tab is null", async () => { + const params = createParams(); + + await client.assertCredential(params, null); + + expect(authenticator.makeCredential).not.toHaveBeenCalled(); + }); + // Spec: If any authenticator returns an error status equivalent to "InvalidStateError", Return a DOMException whose name is "InvalidStateError" and terminate this algorithm. it("should throw error if authenticator throws InvalidState", async () => { const params = createParams(); @@ -313,7 +334,7 @@ describe("FidoAuthenticatorService", () => { new Fido2AutenticatorError(Fido2AutenticatorErrorCode.InvalidState) ); - const result = async () => await client.assertCredential(params); + const result = async () => await client.assertCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "InvalidStateError" }); @@ -325,7 +346,7 @@ describe("FidoAuthenticatorService", () => { const params = createParams(); authenticator.getAssertion.mockRejectedValue(new Error("unknown error")); - const result = async () => await client.assertCredential(params); + const result = async () => await client.assertCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "NotAllowedError" }); @@ -336,7 +357,7 @@ describe("FidoAuthenticatorService", () => { const params = createParams(); configService.getFeatureFlagBool.mockResolvedValue(false); - const result = async () => await client.assertCredential(params); + const result = async () => await client.assertCredential(params, tab); const rejects = expect(result).rejects; await rejects.toThrow(FallbackRequestedError); @@ -356,7 +377,7 @@ describe("FidoAuthenticatorService", () => { }); authenticator.getAssertion.mockResolvedValue(createAuthenticatorAssertResult()); - await client.assertCredential(params); + await client.assertCredential(params, tab); expect(authenticator.getAssertion).toHaveBeenCalledWith( expect.objectContaining({ @@ -374,6 +395,7 @@ describe("FidoAuthenticatorService", () => { }), ], }), + tab, expect.anything() ); }); @@ -387,7 +409,7 @@ describe("FidoAuthenticatorService", () => { }); authenticator.getAssertion.mockResolvedValue(createAuthenticatorAssertResult()); - await client.assertCredential(params); + await client.assertCredential(params, tab); expect(authenticator.getAssertion).toHaveBeenCalledWith( expect.objectContaining({ @@ -395,6 +417,7 @@ describe("FidoAuthenticatorService", () => { rpId: RpId, allowCredentialDescriptorList: [], }), + tab, expect.anything() ); }); diff --git a/libs/common/src/vault/services/fido2/fido2-client.service.ts b/libs/common/src/vault/services/fido2/fido2-client.service.ts index 4733010944a..6ba9fd56a33 100644 --- a/libs/common/src/vault/services/fido2/fido2-client.service.ts +++ b/libs/common/src/vault/services/fido2/fido2-client.service.ts @@ -40,8 +40,13 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { async createCredential( params: CreateCredentialParams, + tab: chrome.tabs.Tab, abortController = new AbortController() ): Promise { + if (tab == null) { + return; + } + const enableFido2VaultCredentials = await this.isFido2FeatureEnabled(); if (!enableFido2VaultCredentials) { @@ -150,6 +155,7 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { try { makeCredentialResult = await this.authenticator.makeCredential( makeCredentialParams, + tab, abortController ); } catch (error) { @@ -191,8 +197,13 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { async assertCredential( params: AssertCredentialParams, + tab: chrome.tabs.Tab, abortController = new AbortController() ): Promise { + if (tab == null) { + return; + } + const enableFido2VaultCredentials = await this.isFido2FeatureEnabled(); if (!enableFido2VaultCredentials) { @@ -258,6 +269,7 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { try { getAssertionResult = await this.authenticator.getAssertion( getAssertionParams, + tab, abortController ); } catch (error) { From 84c8173d9e528bf494ffbf326f76aef73cc13466 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 8 Sep 2023 11:25:02 +0200 Subject: [PATCH 08/81] [PM-3807] feat: remove non-discoverable from fido2 user interface class --- .../browser-fido2-user-interface.service.ts | 32 ------------------- ...ido2-user-interface.service.abstraction.ts | 4 --- 2 files changed, 36 deletions(-) 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 2c2901c7d53..44c1fcb8e89 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 @@ -66,18 +66,6 @@ export type BrowserFido2Message = { sessionId: string } & ( type: "ConfirmNewCredentialResponse"; userVerified: boolean; } - | { - type: "ConfirmNewNonDiscoverableCredentialRequest"; - credentialName: string; - userName: string; - userVerification: boolean; - fallbackSupported: boolean; - } - | { - type: "ConfirmNewNonDiscoverableCredentialResponse"; - cipherId: string; - userVerified: boolean; - } | { type: "InformExcludedCredentialRequest"; existingCipherIds: string[]; @@ -229,26 +217,6 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi return { confirmed: true, userVerified: response.userVerified }; } - async confirmNewNonDiscoverableCredential({ - credentialName, - userName, - userVerification, - }: NewCredentialParams): Promise<{ cipherId: string; userVerified: boolean }> { - const data: BrowserFido2Message = { - type: "ConfirmNewNonDiscoverableCredentialRequest", - sessionId: this.sessionId, - credentialName, - userName, - userVerification, - fallbackSupported: this.fallbackSupported, - }; - - await this.send(data); - const response = await this.receive("ConfirmNewNonDiscoverableCredentialResponse"); - - return { cipherId: response.cipherId, userVerified: response.userVerified }; - } - async informExcludedCredential(existingCipherIds: string[]): Promise { const data: BrowserFido2Message = { type: "InformExcludedCredentialRequest", diff --git a/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts b/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts index 3d0db42d3c1..c9cbff072f1 100644 --- a/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts +++ b/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts @@ -28,10 +28,6 @@ export abstract class Fido2UserInterfaceSession { params: NewCredentialParams, abortController?: AbortController ) => Promise<{ confirmed: boolean; userVerified: boolean }>; - confirmNewNonDiscoverableCredential: ( - params: NewCredentialParams, - abortController?: AbortController - ) => Promise<{ cipherId: string; userVerified: boolean }>; informExcludedCredential: ( existingCipherIds: string[], abortController?: AbortController From 119fe9bfaa2abd71b82290bdb26ed30733fa83f7 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 8 Sep 2023 11:30:53 +0200 Subject: [PATCH 09/81] [PM-3807] feat: merge fido2 component ui --- .../browser-fido2-user-interface.service.ts | 1 + .../components/fido2/fido2.component.html | 11 +----- .../popup/components/fido2/fido2.component.ts | 37 +++---------------- 3 files changed, 7 insertions(+), 42 deletions(-) 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 44c1fcb8e89..1e0b6cfb057 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 @@ -64,6 +64,7 @@ export type BrowserFido2Message = { sessionId: string } & ( } | { type: "ConfirmNewCredentialResponse"; + cipherId: string; userVerified: boolean; } | { diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index 0be33e50c3f..378c67f10ac 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -10,7 +10,7 @@ A site is asking for authentication, please choose one of the following credentials to use: @@ -24,15 +24,6 @@
- - A site wants to create the following passkey in your vault -
-
- -
-
- - A passkey already exists in Bitwarden for this account
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index 8a847c4b923..74ed60cce17 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -16,7 +16,6 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi import { PasswordRepromptService } from "@bitwarden/common/vault/abstractions/password-reprompt.service"; import { CipherType } from "@bitwarden/common/vault/enums/cipher-type"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; -import { Fido2KeyView } from "@bitwarden/common/vault/models/view/fido2-key.view"; import { BrowserApi } from "../../../../platform/browser/browser-api"; import { @@ -80,12 +79,9 @@ export class Fido2Component implements OnInit, OnDestroy { filter((message) => message != undefined), concatMap(async (message) => { if (message.type === "ConfirmNewCredentialRequest") { - const cipher = new CipherView(); - cipher.name = message.credentialName; - cipher.type = CipherType.Fido2Key; - cipher.fido2Key = new Fido2KeyView(); - cipher.fido2Key.userDisplayName = message.userName; - this.ciphers = [cipher]; + this.ciphers = (await this.cipherService.getAllDecrypted()).filter( + (cipher) => cipher.type === CipherType.Login && !cipher.isDeleted + ); } else if (message.type === "PickCredentialRequest") { this.ciphers = await Promise.all( message.cipherIds.map(async (cipherId) => { @@ -93,10 +89,6 @@ export class Fido2Component implements OnInit, OnDestroy { return cipher.decrypt(); }) ); - } else if (message.type === "ConfirmNewNonDiscoverableCredentialRequest") { - this.ciphers = (await this.cipherService.getAllDecrypted()).filter( - (cipher) => cipher.type === CipherType.Login && !cipher.isDeleted - ); } else if (message.type === "InformExcludedCredentialRequest") { this.ciphers = await Promise.all( message.existingCipherIds.map(async (cipherId) => { @@ -140,7 +132,7 @@ export class Fido2Component implements OnInit, OnDestroy { type: "PickCredentialResponse", userVerified, }); - } else if (data?.type === "ConfirmNewNonDiscoverableCredentialRequest") { + } else if (data?.type === "ConfirmNewCredentialRequest") { let userVerified = false; if (data.userVerification) { userVerified = await this.passwordRepromptService.showPasswordPrompt(); @@ -149,7 +141,7 @@ export class Fido2Component implements OnInit, OnDestroy { this.send({ sessionId: this.sessionId, cipherId: cipher.id, - type: "ConfirmNewNonDiscoverableCredentialResponse", + type: "ConfirmNewCredentialResponse", userVerified, }); } @@ -157,25 +149,6 @@ export class Fido2Component implements OnInit, OnDestroy { this.loading = true; } - async confirm() { - const data = this.message$.value; - if (data.type !== "ConfirmNewCredentialRequest") { - return; - } - - let userVerified = false; - if (data.userVerification) { - userVerified = await this.passwordRepromptService.showPasswordPrompt(); - } - - this.send({ - sessionId: this.sessionId, - type: "ConfirmNewCredentialResponse", - userVerified, - }); - this.loading = true; - } - abort(fallback: boolean) { this.unload(fallback); window.close(); From 7effcd41899df93c4640a96a7bb6b6e06ae2a235 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 8 Sep 2023 11:33:33 +0200 Subject: [PATCH 10/81] [PM-3807] feat: return `cipherId` from user interface --- .../services/fido2/browser-fido2-user-interface.service.ts | 4 ++-- .../fido2/fido2-user-interface.service.abstraction.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) 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 1e0b6cfb057..3d57f5aba5e 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 @@ -202,7 +202,7 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi credentialName, userName, userVerification, - }: NewCredentialParams): Promise<{ confirmed: boolean; userVerified: boolean }> { + }: NewCredentialParams): Promise<{ cipherId: string; userVerified: boolean }> { const data: BrowserFido2Message = { type: "ConfirmNewCredentialRequest", sessionId: this.sessionId, @@ -215,7 +215,7 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi await this.send(data); const response = await this.receive("ConfirmNewCredentialResponse"); - return { confirmed: true, userVerified: response.userVerified }; + return { cipherId: response.cipherId, userVerified: response.userVerified }; } async informExcludedCredential(existingCipherIds: string[]): Promise { diff --git a/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts b/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts index c9cbff072f1..d5c31caae55 100644 --- a/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts +++ b/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts @@ -27,7 +27,7 @@ export abstract class Fido2UserInterfaceSession { confirmNewCredential: ( params: NewCredentialParams, abortController?: AbortController - ) => Promise<{ confirmed: boolean; userVerified: boolean }>; + ) => Promise<{ cipherId: string; userVerified: boolean }>; informExcludedCredential: ( existingCipherIds: string[], abortController?: AbortController From d074b9a0090761109aa61882f0f0c413eaadebed Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 8 Sep 2023 11:40:04 +0200 Subject: [PATCH 11/81] [PM-3807] feat: merge credential creation logic in authenticator --- .../fido2/fido2-authenticator.service.spec.ts | 253 +++++------------- .../fido2/fido2-authenticator.service.ts | 119 +++----- 2 files changed, 106 insertions(+), 266 deletions(-) diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts index 35a1442db6e..548ff6f6480 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts @@ -84,7 +84,7 @@ describe("FidoAuthenticatorService", () => { it("should not request confirmation from user", async () => { userInterfaceSession.confirmNewCredential.mockResolvedValue({ - confirmed: true, + cipherId: "75280e7e-a72e-4d6c-bf1e-d37238352f9b", userVerified: false, }); const invalidParams = await createInvalidParams(); @@ -257,109 +257,6 @@ describe("FidoAuthenticatorService", () => { }); describe("creation of discoverable credential", () => { - let params: Fido2AuthenticatorMakeCredentialsParams; - - beforeEach(async () => { - params = await createParams({ requireResidentKey: true }); - cipherService.getAllDecrypted.mockResolvedValue([]); - }); - - /** - * Spec: Collect an authorization gesture confirming user consent for creating a new credential. The prompt for the authorization gesture is shown by the authenticator if it has its own output capability. The prompt SHOULD display rpEntity.id, rpEntity.name, userEntity.name and userEntity.displayName, if possible. - * If requireUserVerification is true, the authorization gesture MUST include user verification. - * Deviation: Only `rpEntity.name` and `userEntity.name` is shown. - * */ - for (const userVerification of [true, false]) { - it(`should request confirmation from user when user verification is ${userVerification}`, async () => { - params.requireUserVerification = userVerification; - userInterfaceSession.confirmNewCredential.mockResolvedValue({ - confirmed: true, - userVerified: userVerification, - }); - cipherService.encrypt.mockResolvedValue({} as unknown as Cipher); - cipherService.createWithServer.mockImplementation(async (cipher) => { - cipher.id = Utils.newGuid(); - return cipher; - }); - - await authenticator.makeCredential(params, new AbortController()); - - expect(userInterfaceSession.confirmNewCredential).toHaveBeenCalledWith( - { - credentialName: params.rpEntity.name, - userName: params.userEntity.displayName, - userVerification, - } as NewCredentialParams, - expect.anything() - ); - }); - } - - it("should save credential to vault if request confirmed by user", async () => { - const encryptedCipher = {}; - userInterfaceSession.confirmNewCredential.mockResolvedValue({ - confirmed: true, - userVerified: false, - }); - cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as Cipher); - cipherService.createWithServer.mockImplementation(async (cipher) => { - cipher.id = Utils.newGuid(); - return cipher; - }); - - await authenticator.makeCredential(params); - - const saved = cipherService.encrypt.mock.lastCall?.[0]; - expect(saved).toEqual( - expect.objectContaining({ - type: CipherType.Fido2Key, - name: params.rpEntity.name, - - fido2Key: expect.objectContaining({ - credentialId: expect.anything(), - keyType: "public-key", - keyAlgorithm: "ECDSA", - keyCurve: "P-256", - rpId: params.rpEntity.id, - rpName: params.rpEntity.name, - userHandle: Fido2Utils.bufferToString(params.userEntity.id), - counter: 0, - userDisplayName: params.userEntity.displayName, - }), - }) - ); - expect(cipherService.createWithServer).toHaveBeenCalledWith(encryptedCipher); - }); - - /** Spec: If the user does not consent or if user verification fails, return an error code equivalent to "NotAllowedError" and terminate the operation. */ - it("should throw error if user denies creation request", async () => { - userInterfaceSession.confirmNewCredential.mockResolvedValue({ - confirmed: false, - userVerified: false, - }); - - const result = async () => await authenticator.makeCredential(params); - - await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed); - }); - - /** Spec: If any error occurred while creating the new credential object, return an error code equivalent to "UnknownError" and terminate the operation. */ - it("should throw unkown error if creation fails", async () => { - const encryptedCipher = {}; - userInterfaceSession.confirmNewCredential.mockResolvedValue({ - confirmed: true, - userVerified: false, - }); - cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as Cipher); - cipherService.createWithServer.mockRejectedValue(new Error("Internal error")); - - const result = async () => await authenticator.makeCredential(params); - - await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Unknown); - }); - }); - - describe("creation of non-discoverable credential", () => { let existingCipher: CipherView; let params: Fido2AuthenticatorMakeCredentialsParams; @@ -379,14 +276,14 @@ describe("FidoAuthenticatorService", () => { for (const userVerification of [true, false]) { it(`should request confirmation from user when user verification is ${userVerification}`, async () => { params.requireUserVerification = userVerification; - userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue({ + userInterfaceSession.confirmNewCredential.mockResolvedValue({ cipherId: existingCipher.id, userVerified: userVerification, }); await authenticator.makeCredential(params, new AbortController()); - expect(userInterfaceSession.confirmNewNonDiscoverableCredential).toHaveBeenCalledWith( + expect(userInterfaceSession.confirmNewCredential).toHaveBeenCalledWith( { credentialName: params.rpEntity.name, userName: params.userEntity.displayName, @@ -399,7 +296,7 @@ describe("FidoAuthenticatorService", () => { it("should save credential to vault if request confirmed by user", async () => { const encryptedCipher = Symbol(); - userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue({ + userInterfaceSession.confirmNewCredential.mockResolvedValue({ cipherId: existingCipher.id, userVerified: false, }); @@ -433,7 +330,7 @@ describe("FidoAuthenticatorService", () => { /** Spec: If the user does not consent or if user verification fails, return an error code equivalent to "NotAllowedError" and terminate the operation. */ it("should throw error if user denies creation request", async () => { - userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue({ + userInterfaceSession.confirmNewCredential.mockResolvedValue({ cipherId: undefined, userVerified: false, }); @@ -447,7 +344,7 @@ describe("FidoAuthenticatorService", () => { /** Spec: If any error occurred while creating the new credential object, return an error code equivalent to "UnknownError" and terminate the operation. */ it("should throw unkown error if creation fails", async () => { const encryptedCipher = Symbol(); - userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue({ + userInterfaceSession.confirmNewCredential.mockResolvedValue({ cipherId: existingCipher.id, userVerified: false, }); @@ -460,86 +357,74 @@ describe("FidoAuthenticatorService", () => { }); }); - for (const requireResidentKey of [true, false]) { - describe(`attestation of new ${ - requireResidentKey ? "discoverable" : "non-discoverable" - } credential`, () => { - const cipherId = "75280e7e-a72e-4d6c-bf1e-d37238352f9b"; - const credentialId = "52217b91-73f1-4fea-b3f2-54a7959fd5aa"; - const credentialIdBytes = new Uint8Array([ - 0x52, 0x21, 0x7b, 0x91, 0x73, 0xf1, 0x4f, 0xea, 0xb3, 0xf2, 0x54, 0xa7, 0x95, 0x9f, 0xd5, - 0xaa, - ]); - let params: Fido2AuthenticatorMakeCredentialsParams; - - beforeEach(async () => { - const cipher = createCipherView({ id: cipherId, type: CipherType.Login }); - params = await createParams({ requireResidentKey }); - userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue({ - cipherId, - userVerified: false, - }); - userInterfaceSession.confirmNewCredential.mockResolvedValue({ - confirmed: true, - userVerified: false, - }); - cipherService.get.mockImplementation(async (cipherId) => - cipherId === cipher.id ? ({ decrypt: () => cipher } as any) : undefined - ); - cipherService.getAllDecrypted.mockResolvedValue([await cipher]); - cipherService.encrypt.mockImplementation(async (cipher) => { - if (!requireResidentKey) { - cipher.login.fido2Key.credentialId = credentialId; // Replace id for testability - } else { - cipher.fido2Key.credentialId = credentialId; - } - return {} as any; - }); - cipherService.createWithServer.mockImplementation(async (cipher) => { - cipher.id = cipherId; - return cipher; - }); - cipherService.updateWithServer.mockImplementation(async (cipher) => { - cipher.id = cipherId; - return cipher; - }); + describe(`attestation of new credential`, () => { + const cipherId = "75280e7e-a72e-4d6c-bf1e-d37238352f9b"; + const credentialId = "52217b91-73f1-4fea-b3f2-54a7959fd5aa"; + const credentialIdBytes = new Uint8Array([ + 0x52, 0x21, 0x7b, 0x91, 0x73, 0xf1, 0x4f, 0xea, 0xb3, 0xf2, 0x54, 0xa7, 0x95, 0x9f, 0xd5, + 0xaa, + ]); + let params: Fido2AuthenticatorMakeCredentialsParams; + + beforeEach(async () => { + const cipher = createCipherView({ id: cipherId, type: CipherType.Login }); + params = await createParams(); + userInterfaceSession.confirmNewCredential.mockResolvedValue({ + cipherId, + userVerified: false, + }); + cipherService.get.mockImplementation(async (cipherId) => + cipherId === cipher.id ? ({ decrypt: () => cipher } as any) : undefined + ); + cipherService.getAllDecrypted.mockResolvedValue([await cipher]); + cipherService.encrypt.mockImplementation(async (cipher) => { + cipher.login.fido2Key.credentialId = credentialId; // Replace id for testability + return {} as any; }); + cipherService.createWithServer.mockImplementation(async (cipher) => { + cipher.id = cipherId; + return cipher; + }); + cipherService.updateWithServer.mockImplementation(async (cipher) => { + cipher.id = cipherId; + return cipher; + }); + }); - it("should return attestation object", async () => { - const result = await authenticator.makeCredential(params); + it("should return attestation object", async () => { + const result = await authenticator.makeCredential(params); - const attestationObject = CBOR.decode( - Fido2Utils.bufferSourceToUint8Array(result.attestationObject).buffer - ); + const attestationObject = CBOR.decode( + Fido2Utils.bufferSourceToUint8Array(result.attestationObject).buffer + ); - const encAuthData: Uint8Array = attestationObject.authData; - const rpIdHash = encAuthData.slice(0, 32); - const flags = encAuthData.slice(32, 33); - const counter = encAuthData.slice(33, 37); - const aaguid = encAuthData.slice(37, 53); - const credentialIdLength = encAuthData.slice(53, 55); - const credentialId = encAuthData.slice(55, 71); - // Unsure how to test public key - // const publicKey = encAuthData.slice(87); - - expect(encAuthData.length).toBe(71 + 77); - expect(attestationObject.fmt).toBe("none"); - expect(attestationObject.attStmt).toEqual({}); - expect(rpIdHash).toEqual( - new Uint8Array([ - 0x22, 0x6b, 0xb3, 0x92, 0x02, 0xff, 0xf9, 0x22, 0xdc, 0x74, 0x05, 0xcd, 0x28, 0xa8, - 0x34, 0x5a, 0xc4, 0xf2, 0x64, 0x51, 0xd7, 0x3d, 0x0b, 0x40, 0xef, 0xf3, 0x1d, 0xc1, - 0xd0, 0x5c, 0x3d, 0xc3, - ]) - ); - expect(flags).toEqual(new Uint8Array([0b01000001])); // UP = true, AD = true - expect(counter).toEqual(new Uint8Array([0, 0, 0, 0])); // 0 because of new counter - expect(aaguid).toEqual(AAGUID); - expect(credentialIdLength).toEqual(new Uint8Array([0, 16])); // 16 bytes because we're using GUIDs - expect(credentialId).toEqual(credentialIdBytes); - }); + const encAuthData: Uint8Array = attestationObject.authData; + const rpIdHash = encAuthData.slice(0, 32); + const flags = encAuthData.slice(32, 33); + const counter = encAuthData.slice(33, 37); + const aaguid = encAuthData.slice(37, 53); + const credentialIdLength = encAuthData.slice(53, 55); + const credentialId = encAuthData.slice(55, 71); + // Unsure how to test public key + // const publicKey = encAuthData.slice(87); + + expect(encAuthData.length).toBe(71 + 77); + expect(attestationObject.fmt).toBe("none"); + expect(attestationObject.attStmt).toEqual({}); + expect(rpIdHash).toEqual( + new Uint8Array([ + 0x22, 0x6b, 0xb3, 0x92, 0x02, 0xff, 0xf9, 0x22, 0xdc, 0x74, 0x05, 0xcd, 0x28, 0xa8, + 0x34, 0x5a, 0xc4, 0xf2, 0x64, 0x51, 0xd7, 0x3d, 0x0b, 0x40, 0xef, 0xf3, 0x1d, 0xc1, + 0xd0, 0x5c, 0x3d, 0xc3, + ]) + ); + expect(flags).toEqual(new Uint8Array([0b01000001])); // UP = true, AD = true + expect(counter).toEqual(new Uint8Array([0, 0, 0, 0])); // 0 because of new counter + expect(aaguid).toEqual(AAGUID); + expect(credentialIdLength).toEqual(new Uint8Array([0, 16])); // 16 bytes because we're using GUIDs + expect(credentialId).toEqual(credentialIdBytes); }); - } + }); async function createParams( params: Partial = {} diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts index 65802f2d28d..a411ccd4b31 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts @@ -97,90 +97,45 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr let keyPair: CryptoKeyPair; let userVerified = false; let credentialId: string; - if (params.requireResidentKey) { - const response = await userInterfaceSession.confirmNewCredential( - { - credentialName: params.rpEntity.name, - userName: params.userEntity.displayName, - userVerification: params.requireUserVerification, - }, - abortController + const response = await userInterfaceSession.confirmNewCredential( + { + credentialName: params.rpEntity.name, + userName: params.userEntity.displayName, + userVerification: params.requireUserVerification, + }, + abortController + ); + const cipherId = response.cipherId; + userVerified = response.userVerified; + + if (cipherId === undefined) { + this.logService?.warning( + `[Fido2Authenticator] Aborting because user confirmation was not recieved.` ); - const userConfirmation = response.confirmed; - userVerified = response.userVerified; - - if (!userConfirmation) { - this.logService?.warning( - `[Fido2Authenticator] Aborting because user confirmation was not recieved.` - ); - throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); - } - - if (params.requireUserVerification && !userVerified) { - this.logService?.warning( - `[Fido2Authenticator] Aborting because user verification was not successful.` - ); - throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); - } - - try { - keyPair = await createKeyPair(); - - cipher = new CipherView(); - cipher.type = CipherType.Fido2Key; - cipher.name = params.rpEntity.name; - cipher.fido2Key = fido2Key = await createKeyView(params, keyPair.privateKey); - const encrypted = await this.cipherService.encrypt(cipher); - await this.cipherService.createWithServer(encrypted); // encrypted.id is assigned inside here - cipher.id = encrypted.id; - credentialId = cipher.fido2Key.credentialId; - } catch (error) { - this.logService?.error( - `[Fido2Authenticator] Aborting because of unknown error when creating discoverable credential: ${error}` - ); - throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown); - } - } else { - const response = await userInterfaceSession.confirmNewNonDiscoverableCredential( - { - credentialName: params.rpEntity.name, - userName: params.userEntity.displayName, - userVerification: params.requireUserVerification, - }, - abortController + throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); + } + + if (params.requireUserVerification && !userVerified) { + this.logService?.warning( + `[Fido2Authenticator] Aborting because user verification was unsuccessful.` + ); + throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); + } + + try { + keyPair = await createKeyPair(); + + const encrypted = await this.cipherService.get(cipherId); + cipher = await encrypted.decrypt(); + cipher.login.fido2Key = fido2Key = await createKeyView(params, keyPair.privateKey); + const reencrypted = await this.cipherService.encrypt(cipher); + await this.cipherService.updateWithServer(reencrypted); + credentialId = cipher.login.fido2Key.credentialId; + } catch (error) { + this.logService?.error( + `[Fido2Authenticator] Aborting because of unknown error when creating credential: ${error}` ); - const cipherId = response.cipherId; - userVerified = response.userVerified; - - if (cipherId === undefined) { - this.logService?.warning( - `[Fido2Authenticator] Aborting because user confirmation was not recieved.` - ); - throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); - } - - if (params.requireUserVerification && !userVerified) { - this.logService?.warning( - `[Fido2Authenticator] Aborting because user verification was unsuccessful.` - ); - throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); - } - - try { - keyPair = await createKeyPair(); - - const encrypted = await this.cipherService.get(cipherId); - cipher = await encrypted.decrypt(); - cipher.login.fido2Key = fido2Key = await createKeyView(params, keyPair.privateKey); - const reencrypted = await this.cipherService.encrypt(cipher); - await this.cipherService.updateWithServer(reencrypted); - credentialId = cipher.login.fido2Key.credentialId; - } catch (error) { - this.logService?.error( - `[Fido2Authenticator] Aborting because of unknown error when creating non-discoverable credential: ${error}` - ); - throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown); - } + throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown); } const authData = await generateAuthData({ From 0cf7641c12b9f9dbdee1a19a4766d369e2992e59 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 8 Sep 2023 11:41:41 +0200 Subject: [PATCH 12/81] [PM-3807] feat: merge credential assertion logic in authenticator --- .../fido2/fido2-authenticator.service.spec.ts | 247 ++++++++---------- .../fido2/fido2-authenticator.service.ts | 18 +- 2 files changed, 118 insertions(+), 147 deletions(-) diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts index 548ff6f6480..74e674f9c55 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts @@ -256,7 +256,7 @@ describe("FidoAuthenticatorService", () => { ); }); - describe("creation of discoverable credential", () => { + describe("credential creation", () => { let existingCipher: CipherView; let params: Fido2AuthenticatorMakeCredentialsParams; @@ -586,7 +586,7 @@ describe("FidoAuthenticatorService", () => { { credentialId: credentialIds[0], rpId: RpId } ), await createCipherView( - { type: CipherType.Fido2Key }, + { type: CipherType.Login }, { credentialId: credentialIds[1], rpId: RpId } ), ]; @@ -631,157 +631,128 @@ describe("FidoAuthenticatorService", () => { }); }); - for (const residentKey of [true, false]) { - describe(`assertion of ${ - residentKey ? "discoverable" : "non-discoverable" - } credential`, () => { - let keyPair: CryptoKeyPair; - let credentialIds: string[]; - let selectedCredentialId: string; - let ciphers: CipherView[]; - let fido2Keys: Fido2KeyView[]; - let params: Fido2AuthenticatorGetAssertionParams; - - const init = async () => { - keyPair = await createKeyPair(); - credentialIds = [Utils.newGuid(), Utils.newGuid()]; - const keyValue = Fido2Utils.bufferToString( - await crypto.subtle.exportKey("pkcs8", keyPair.privateKey) - ); - if (residentKey) { - ciphers = credentialIds.map((id) => - createCipherView( - { type: CipherType.Fido2Key }, - { credentialId: id, rpId: RpId, counter: 9000, keyValue } - ) - ); - fido2Keys = ciphers.map((c) => c.fido2Key); - selectedCredentialId = credentialIds[0]; - params = await createParams({ - allowCredentialDescriptorList: undefined, - rpId: RpId, - }); - } else { - ciphers = credentialIds.map((id) => - createCipherView( - { type: CipherType.Login }, - { credentialId: id, rpId: RpId, counter: 9000 } - ) - ); - fido2Keys = ciphers.map((c) => c.login.fido2Key); - selectedCredentialId = credentialIds[0]; - params = await createParams({ - allowCredentialDescriptorList: credentialIds.map((credentialId) => ({ - id: guidToRawFormat(credentialId), - type: "public-key", - })), - rpId: RpId, - }); - } - cipherService.getAllDecrypted.mockResolvedValue(ciphers); - userInterfaceSession.pickCredential.mockResolvedValue({ - cipherId: ciphers[0].id, - userVerified: false, - }); - }; - beforeEach(init); + describe("assertion of credential", () => { + let keyPair: CryptoKeyPair; + let credentialIds: string[]; + let selectedCredentialId: string; + let ciphers: CipherView[]; + let fido2Keys: Fido2KeyView[]; + let params: Fido2AuthenticatorGetAssertionParams; - /** Spec: Increment the credential associated signature counter */ - it("should increment counter", async () => { - const encrypted = Symbol(); - cipherService.encrypt.mockResolvedValue(encrypted as any); + const init = async () => { + keyPair = await createKeyPair(); + credentialIds = [Utils.newGuid(), Utils.newGuid()]; + const keyValue = Fido2Utils.bufferToString( + await crypto.subtle.exportKey("pkcs8", keyPair.privateKey) + ); + ciphers = credentialIds.map((id) => + createCipherView( + { type: CipherType.Login }, + { credentialId: id, rpId: RpId, counter: 9000, keyValue } + ) + ); + fido2Keys = ciphers.map((c) => c.login.fido2Key); + selectedCredentialId = credentialIds[0]; + params = await createParams({ + allowCredentialDescriptorList: credentialIds.map((credentialId) => ({ + id: guidToRawFormat(credentialId), + type: "public-key", + })), + rpId: RpId, + }); + cipherService.getAllDecrypted.mockResolvedValue(ciphers); + userInterfaceSession.pickCredential.mockResolvedValue({ + cipherId: ciphers[0].id, + userVerified: false, + }); + }; + beforeEach(init); - await authenticator.getAssertion(params); + /** Spec: Increment the credential associated signature counter */ + it("should increment counter", async () => { + const encrypted = Symbol(); + cipherService.encrypt.mockResolvedValue(encrypted as any); - expect(cipherService.updateWithServer).toHaveBeenCalledWith(encrypted); - if (residentKey) { - expect(cipherService.encrypt).toHaveBeenCalledWith( - expect.objectContaining({ - id: ciphers[0].id, - fido2Key: expect.objectContaining({ - counter: 9001, - }), - }) - ); - } else { - expect(cipherService.encrypt).toHaveBeenCalledWith( - expect.objectContaining({ - id: ciphers[0].id, - login: expect.objectContaining({ - fido2Key: expect.objectContaining({ - counter: 9001, - }), - }), - }) - ); - } - }); + await authenticator.getAssertion(params); - it("should return an assertion result", async () => { - const result = await authenticator.getAssertion(params); + expect(cipherService.updateWithServer).toHaveBeenCalledWith(encrypted); - const encAuthData = result.authenticatorData; - const rpIdHash = encAuthData.slice(0, 32); - const flags = encAuthData.slice(32, 33); - const counter = encAuthData.slice(33, 37); + expect(cipherService.encrypt).toHaveBeenCalledWith( + expect.objectContaining({ + id: ciphers[0].id, + login: expect.objectContaining({ + fido2Key: expect.objectContaining({ + counter: 9001, + }), + }), + }) + ); + }); - expect(result.selectedCredential.id).toEqual(guidToRawFormat(selectedCredentialId)); - expect(result.selectedCredential.userHandle).toEqual( - Fido2Utils.stringToBuffer(fido2Keys[0].userHandle) - ); - expect(rpIdHash).toEqual( - new Uint8Array([ - 0x22, 0x6b, 0xb3, 0x92, 0x02, 0xff, 0xf9, 0x22, 0xdc, 0x74, 0x05, 0xcd, 0x28, 0xa8, - 0x34, 0x5a, 0xc4, 0xf2, 0x64, 0x51, 0xd7, 0x3d, 0x0b, 0x40, 0xef, 0xf3, 0x1d, 0xc1, - 0xd0, 0x5c, 0x3d, 0xc3, - ]) - ); - expect(flags).toEqual(new Uint8Array([0b00000001])); // UP = true - expect(counter).toEqual(new Uint8Array([0, 0, 0x23, 0x29])); // 9001 in hex - - // Verify signature - // TODO: Cannot verify signature because it has been converted into DER format - // const sigBase = new Uint8Array([ - // ...result.authenticatorData, - // ...Fido2Utils.bufferSourceToUint8Array(params.hash), - // ]); - // const isValidSignature = await crypto.subtle.verify( - // { name: "ECDSA", hash: { name: "SHA-256" } }, - // keyPair.publicKey, - // result.signature, - // sigBase - // ); - // expect(isValidSignature).toBe(true); - }); + it("should return an assertion result", async () => { + const result = await authenticator.getAssertion(params); + + const encAuthData = result.authenticatorData; + const rpIdHash = encAuthData.slice(0, 32); + const flags = encAuthData.slice(32, 33); + const counter = encAuthData.slice(33, 37); + + expect(result.selectedCredential.id).toEqual(guidToRawFormat(selectedCredentialId)); + expect(result.selectedCredential.userHandle).toEqual( + Fido2Utils.stringToBuffer(fido2Keys[0].userHandle) + ); + expect(rpIdHash).toEqual( + new Uint8Array([ + 0x22, 0x6b, 0xb3, 0x92, 0x02, 0xff, 0xf9, 0x22, 0xdc, 0x74, 0x05, 0xcd, 0x28, 0xa8, + 0x34, 0x5a, 0xc4, 0xf2, 0x64, 0x51, 0xd7, 0x3d, 0x0b, 0x40, 0xef, 0xf3, 0x1d, 0xc1, + 0xd0, 0x5c, 0x3d, 0xc3, + ]) + ); + expect(flags).toEqual(new Uint8Array([0b00000001])); // UP = true + expect(counter).toEqual(new Uint8Array([0, 0, 0x23, 0x29])); // 9001 in hex + + // Verify signature + // TODO: Cannot verify signature because it has been converted into DER format + // const sigBase = new Uint8Array([ + // ...result.authenticatorData, + // ...Fido2Utils.bufferSourceToUint8Array(params.hash), + // ]); + // const isValidSignature = await crypto.subtle.verify( + // { name: "ECDSA", hash: { name: "SHA-256" } }, + // keyPair.publicKey, + // result.signature, + // sigBase + // ); + // expect(isValidSignature).toBe(true); + }); - it("should always generate unique signatures even if the input is the same", async () => { - const signatures = new Set(); + it("should always generate unique signatures even if the input is the same", async () => { + const signatures = new Set(); - for (let i = 0; i < 10; ++i) { - await init(); // Reset inputs - const result = await authenticator.getAssertion(params); + for (let i = 0; i < 10; ++i) { + await init(); // Reset inputs + const result = await authenticator.getAssertion(params); - const counter = result.authenticatorData.slice(33, 37); - expect(counter).toEqual(new Uint8Array([0, 0, 0x23, 0x29])); // double check that the counter doesn't change + const counter = result.authenticatorData.slice(33, 37); + expect(counter).toEqual(new Uint8Array([0, 0, 0x23, 0x29])); // double check that the counter doesn't change - const signature = Fido2Utils.bufferToString(result.signature); - if (signatures.has(signature)) { - throw new Error("Found duplicate signature"); - } - signatures.add(signature); + const signature = Fido2Utils.bufferToString(result.signature); + if (signatures.has(signature)) { + throw new Error("Found duplicate signature"); } - }); + signatures.add(signature); + } + }); - /** Spec: If any error occurred while generating the assertion signature, return an error code equivalent to "UnknownError" and terminate the operation. */ - it("should throw unkown error if creation fails", async () => { - cipherService.updateWithServer.mockRejectedValue(new Error("Internal error")); + /** Spec: If any error occurred while generating the assertion signature, return an error code equivalent to "UnknownError" and terminate the operation. */ + it("should throw unkown error if creation fails", async () => { + cipherService.updateWithServer.mockRejectedValue(new Error("Internal error")); - const result = async () => await authenticator.getAssertion(params); + const result = async () => await authenticator.getAssertion(params); - await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Unknown); - }); + await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Unknown); }); - } + }); async function createParams( params: Partial = {} diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts index a411ccd4b31..b945fcc972c 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts @@ -329,14 +329,11 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr const ciphers = await this.cipherService.getAllDecrypted(); return ciphers.filter( (cipher) => - (!cipher.isDeleted && - cipher.type === CipherType.Login && - cipher.login.fido2Key != undefined && - cipher.login.fido2Key.rpId === rpId && - ids.includes(cipher.login.fido2Key.credentialId)) || - (cipher.type === CipherType.Fido2Key && - cipher.fido2Key.rpId === rpId && - ids.includes(cipher.fido2Key.credentialId)) + !cipher.isDeleted && + cipher.type === CipherType.Login && + cipher.login.fido2Key != undefined && + cipher.login.fido2Key.rpId === rpId && + ids.includes(cipher.login.fido2Key.credentialId) ); } @@ -344,7 +341,10 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr const ciphers = await this.cipherService.getAllDecrypted(); return ciphers.filter( (cipher) => - !cipher.isDeleted && cipher.type === CipherType.Fido2Key && cipher.fido2Key.rpId === rpId + !cipher.isDeleted && + cipher.type === CipherType.Login && + cipher.login.fido2Key != undefined && + cipher.login.fido2Key.rpId === rpId ); } } From b7472d41ac8aa3c60eb4dc413658b78fffc5f5e6 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Sat, 9 Sep 2023 12:06:11 -0400 Subject: [PATCH 13/81] updated test cases and services using the config service --- .../src/vault/services/fido2/fido2-client.service.spec.ts | 8 ++++---- .../src/vault/services/fido2/fido2-client.service.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libs/common/src/vault/services/fido2/fido2-client.service.spec.ts b/libs/common/src/vault/services/fido2/fido2-client.service.spec.ts index 83c0f7dfec3..26357517f7e 100644 --- a/libs/common/src/vault/services/fido2/fido2-client.service.spec.ts +++ b/libs/common/src/vault/services/fido2/fido2-client.service.spec.ts @@ -31,7 +31,7 @@ describe("FidoAuthenticatorService", () => { authenticator = mock(); configService = mock(); client = new Fido2ClientService(authenticator, configService); - configService.getFeatureFlagBool.mockResolvedValue(true); + configService.getFeatureFlag.mockResolvedValue(true); tab = { id: 123, windowId: 456 }; }); @@ -209,7 +209,7 @@ describe("FidoAuthenticatorService", () => { it("should throw FallbackRequestedError if feature flag is not enabled", async () => { const params = createParams(); - configService.getFeatureFlagBool.mockResolvedValue(false); + configService.getFeatureFlag.mockResolvedValue(false); const result = async () => await client.createCredential(params, tab); @@ -356,7 +356,7 @@ describe("FidoAuthenticatorService", () => { it("should throw FallbackRequestedError if feature flag is not enabled", async () => { const params = createParams(); - configService.getFeatureFlagBool.mockResolvedValue(false); + configService.getFeatureFlag.mockResolvedValue(false); const result = async () => await client.assertCredential(params, tab); @@ -369,7 +369,7 @@ describe("FidoAuthenticatorService", () => { const params = createParams(); params.sameOriginWithAncestors = false; // Simulating the falsey value - const result = async () => await client.assertCredential(params); + const result = async () => await client.assertCredential(params, tab); const rejects = expect(result).rejects; await rejects.toMatchObject({ name: "NotAllowedError" }); diff --git a/libs/common/src/vault/services/fido2/fido2-client.service.ts b/libs/common/src/vault/services/fido2/fido2-client.service.ts index f1f2516f509..8f500495cd2 100644 --- a/libs/common/src/vault/services/fido2/fido2-client.service.ts +++ b/libs/common/src/vault/services/fido2/fido2-client.service.ts @@ -35,7 +35,7 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { ) {} async isFido2FeatureEnabled(): Promise { - return await this.configService.getFeatureFlagBool(FeatureFlag.Fido2VaultCredentials); + return await this.configService.getFeatureFlag(FeatureFlag.Fido2VaultCredentials); } async createCredential( From 28e669dd90c2bd3164ebdbfb6d0311cc9532934d Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 7 Sep 2023 13:51:25 +0200 Subject: [PATCH 14/81] [PM-3807] feat: add `discoverable` property to fido2keys --- .../fido2-authenticator.service.abstraction.ts | 2 +- libs/common/src/vault/api/fido2-key.api.ts | 2 ++ .../src/vault/models/data/fido2-key.data.ts | 2 ++ .../src/vault/models/domain/fido2-key.spec.ts | 9 ++++++++- libs/common/src/vault/models/domain/fido2-key.ts | 16 ++++++++++++++++ .../src/vault/models/view/fido2-key.view.ts | 1 + 6 files changed, 30 insertions(+), 2 deletions(-) diff --git a/libs/common/src/vault/abstractions/fido2/fido2-authenticator.service.abstraction.ts b/libs/common/src/vault/abstractions/fido2/fido2-authenticator.service.abstraction.ts index 3e842700bc3..521c332344c 100644 --- a/libs/common/src/vault/abstractions/fido2/fido2-authenticator.service.abstraction.ts +++ b/libs/common/src/vault/abstractions/fido2/fido2-authenticator.service.abstraction.ts @@ -81,7 +81,7 @@ export interface Fido2AuthenticatorMakeCredentialsParams { }; /** A Boolean value that indicates that individually-identifying attestation MAY be returned by the authenticator. */ enterpriseAttestationPossible?: boolean; // Ignored by bitwarden at the moment - /** The effective resident key requirement for credential creation, a Boolean value determined by the client. */ + /** The effective resident key requirement for credential creation, a Boolean value determined by the client. Resident is synonymous with discoverable. */ requireResidentKey: boolean; requireUserVerification: boolean; /** Forwarded to user interface */ diff --git a/libs/common/src/vault/api/fido2-key.api.ts b/libs/common/src/vault/api/fido2-key.api.ts index c98a06f1d15..bd5b09dc117 100644 --- a/libs/common/src/vault/api/fido2-key.api.ts +++ b/libs/common/src/vault/api/fido2-key.api.ts @@ -11,6 +11,7 @@ export class Fido2KeyApi extends BaseResponse { counter: string; rpName: string; userDisplayName: string; + discoverable: string; constructor(data: any = null) { super(data); @@ -28,5 +29,6 @@ export class Fido2KeyApi extends BaseResponse { this.counter = this.getResponseProperty("Counter"); this.rpName = this.getResponseProperty("RpName"); this.userDisplayName = this.getResponseProperty("UserDisplayName"); + this.discoverable = this.getResponseProperty("discoverable"); } } diff --git a/libs/common/src/vault/models/data/fido2-key.data.ts b/libs/common/src/vault/models/data/fido2-key.data.ts index 45d2feadf4f..6f0c49f3e8d 100644 --- a/libs/common/src/vault/models/data/fido2-key.data.ts +++ b/libs/common/src/vault/models/data/fido2-key.data.ts @@ -11,6 +11,7 @@ export class Fido2KeyData { counter: string; rpName: string; userDisplayName: string; + discoverable: string; constructor(data?: Fido2KeyApi) { if (data == null) { @@ -27,5 +28,6 @@ export class Fido2KeyData { this.counter = data.counter; this.rpName = data.rpName; this.userDisplayName = data.userDisplayName; + this.discoverable = data.discoverable; } } diff --git a/libs/common/src/vault/models/domain/fido2-key.spec.ts b/libs/common/src/vault/models/domain/fido2-key.spec.ts index 6804963fbe1..365dbb1c6e1 100644 --- a/libs/common/src/vault/models/domain/fido2-key.spec.ts +++ b/libs/common/src/vault/models/domain/fido2-key.spec.ts @@ -22,6 +22,7 @@ describe("Fido2Key", () => { rpName: null, userDisplayName: null, counter: null, + discoverable: null, }); }); @@ -37,6 +38,7 @@ describe("Fido2Key", () => { counter: "counter", rpName: "rpName", userDisplayName: "userDisplayName", + discoverable: "discoverable", }; const fido2Key = new Fido2Key(data); @@ -51,6 +53,7 @@ describe("Fido2Key", () => { counter: { encryptedString: "counter", encryptionType: 0 }, rpName: { encryptedString: "rpName", encryptionType: 0 }, userDisplayName: { encryptedString: "userDisplayName", encryptionType: 0 }, + discoverable: { encryptedString: "discoverable", encryptionType: 0 }, }); }); @@ -76,6 +79,7 @@ describe("Fido2Key", () => { fido2Key.counter = mockEnc("2"); fido2Key.rpName = mockEnc("rpName"); fido2Key.userDisplayName = mockEnc("userDisplayName"); + fido2Key.discoverable = mockEnc("true"); const fido2KeyView = await fido2Key.decrypt(null); @@ -90,6 +94,7 @@ describe("Fido2Key", () => { rpName: "rpName", userDisplayName: "userDisplayName", counter: 2, + discoverable: true, }); }); }); @@ -104,9 +109,10 @@ describe("Fido2Key", () => { keyValue: "keyValue", rpId: "rpId", userHandle: "userHandle", - counter: "counter", + counter: "2", rpName: "rpName", userDisplayName: "userDisplayName", + discoverable: "true", }; const fido2Key = new Fido2Key(data); @@ -129,6 +135,7 @@ describe("Fido2Key", () => { fido2Key.counter = createEncryptedEncString("2"); fido2Key.rpName = createEncryptedEncString("rpName"); fido2Key.userDisplayName = createEncryptedEncString("userDisplayName"); + fido2Key.discoverable = createEncryptedEncString("discoverable"); const json = JSON.stringify(fido2Key); const result = Fido2Key.fromJSON(JSON.parse(json)); diff --git a/libs/common/src/vault/models/domain/fido2-key.ts b/libs/common/src/vault/models/domain/fido2-key.ts index e03920f3d68..61abd655ee2 100644 --- a/libs/common/src/vault/models/domain/fido2-key.ts +++ b/libs/common/src/vault/models/domain/fido2-key.ts @@ -17,6 +17,7 @@ export class Fido2Key extends Domain { counter: EncString; rpName: EncString; userDisplayName: EncString; + discoverable: EncString; constructor(obj?: Fido2KeyData) { super(); @@ -38,6 +39,7 @@ export class Fido2Key extends Domain { counter: null, rpName: null, userDisplayName: null, + discoverable: null, }, [] ); @@ -56,6 +58,7 @@ export class Fido2Key extends Domain { userHandle: null, rpName: null, userDisplayName: null, + discoverable: null, }, orgId, encKey @@ -72,6 +75,16 @@ export class Fido2Key extends Domain { // Counter will end up as NaN if this fails view.counter = parseInt(counter); + const { discoverable } = await this.decryptObj( + { discoverable: "" }, + { + discoverable: null, + }, + orgId, + encKey + ); + view.discoverable = discoverable === "true"; + return view; } @@ -88,6 +101,7 @@ export class Fido2Key extends Domain { counter: null, rpName: null, userDisplayName: null, + discoverable: null, }); return i; } @@ -107,6 +121,7 @@ export class Fido2Key extends Domain { const counter = EncString.fromJSON(obj.counter); const rpName = EncString.fromJSON(obj.rpName); const userDisplayName = EncString.fromJSON(obj.userDisplayName); + const discoverable = EncString.fromJSON(obj.discoverable); return Object.assign(new Fido2Key(), obj, { credentialId, @@ -119,6 +134,7 @@ export class Fido2Key extends Domain { counter, rpName, userDisplayName, + discoverable, }); } } diff --git a/libs/common/src/vault/models/view/fido2-key.view.ts b/libs/common/src/vault/models/view/fido2-key.view.ts index 6fec5dbf25d..8af29af70d3 100644 --- a/libs/common/src/vault/models/view/fido2-key.view.ts +++ b/libs/common/src/vault/models/view/fido2-key.view.ts @@ -13,6 +13,7 @@ export class Fido2KeyView extends ItemView { counter: number; rpName: string; userDisplayName: string; + discoverable: boolean; get subTitle(): string { return this.userDisplayName; From c8bae9cac1cf848160e265b2823a74432c1a055d Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 7 Sep 2023 14:14:19 +0200 Subject: [PATCH 15/81] [PM-3807] feat: assign discoverable property during creation --- .../vault/services/fido2/fido2-authenticator.service.spec.ts | 4 +++- .../src/vault/services/fido2/fido2-authenticator.service.ts | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts index 74e674f9c55..e16aa4d80c8 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts @@ -262,7 +262,7 @@ describe("FidoAuthenticatorService", () => { beforeEach(async () => { existingCipher = createCipherView({ type: CipherType.Login }); - params = await createParams(); + params = await createParams({ requireResidentKey: false }); cipherService.get.mockImplementation(async (id) => id === existingCipher.id ? ({ decrypt: () => existingCipher } as any) : undefined ); @@ -321,6 +321,7 @@ describe("FidoAuthenticatorService", () => { userHandle: Fido2Utils.bufferToString(params.userEntity.id), counter: 0, userDisplayName: params.userEntity.displayName, + discoverable: false, }), }), }) @@ -797,6 +798,7 @@ function createCipherView( fido2KeyView.userHandle = fido2Key.userHandle ?? Fido2Utils.bufferToString(randomBytes(16)); fido2KeyView.keyAlgorithm = fido2Key.keyAlgorithm ?? "ECDSA"; fido2KeyView.keyCurve = fido2Key.keyCurve ?? "P-256"; + fido2KeyView.discoverable = true; fido2KeyView.keyValue = fido2KeyView.keyValue ?? "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgTC-7XDZipXbaVBlnkjlBgO16ZmqBZWejK2iYo6lV0dehRANCAASOcM2WduNq1DriRYN7ZekvZz-bRhA-qNT4v0fbp5suUFJyWmgOQ0bybZcLXHaerK5Ep1JiSrQcewtQNgLtry7f"; diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts index b945fcc972c..8f5cae64df7 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts @@ -380,6 +380,7 @@ async function createKeyView( fido2Key.counter = 0; fido2Key.rpName = params.rpEntity.name; fido2Key.userDisplayName = params.userEntity.displayName; + fido2Key.discoverable = params.requireResidentKey; return fido2Key; } From 6cee3b83f5aa78e94530b62e569e7cbbfde7ca4e Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 11 Sep 2023 10:34:25 +0200 Subject: [PATCH 16/81] [PM-3807] feat: save discoverable field to server --- libs/common/src/vault/api/fido2-key.api.ts | 2 +- libs/common/src/vault/models/request/cipher.request.ts | 8 ++++++++ libs/common/src/vault/services/cipher.service.ts | 8 ++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/libs/common/src/vault/api/fido2-key.api.ts b/libs/common/src/vault/api/fido2-key.api.ts index bd5b09dc117..0d3f425bd94 100644 --- a/libs/common/src/vault/api/fido2-key.api.ts +++ b/libs/common/src/vault/api/fido2-key.api.ts @@ -29,6 +29,6 @@ export class Fido2KeyApi extends BaseResponse { this.counter = this.getResponseProperty("Counter"); this.rpName = this.getResponseProperty("RpName"); this.userDisplayName = this.getResponseProperty("UserDisplayName"); - this.discoverable = this.getResponseProperty("discoverable"); + this.discoverable = this.getResponseProperty("Discoverable"); } } diff --git a/libs/common/src/vault/models/request/cipher.request.ts b/libs/common/src/vault/models/request/cipher.request.ts index 5ca6cfc7817..108e326a326 100644 --- a/libs/common/src/vault/models/request/cipher.request.ts +++ b/libs/common/src/vault/models/request/cipher.request.ts @@ -104,6 +104,10 @@ export class CipherRequest { cipher.login.fido2Key.userDisplayName != null ? cipher.login.fido2Key.userDisplayName.encryptedString : null; + this.login.fido2Key.discoverable = + cipher.login.fido2Key.discoverable != null + ? cipher.login.fido2Key.discoverable.encryptedString + : null; } break; case CipherType.SecureNote: @@ -197,6 +201,10 @@ export class CipherRequest { cipher.fido2Key.userDisplayName != null ? cipher.fido2Key.userDisplayName.encryptedString : null; + this.fido2Key.discoverable = + cipher.fido2Key.discoverable != null + ? cipher.fido2Key.discoverable.encryptedString + : null; break; default: break; diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 66479f561ad..58530fbd19e 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -1112,6 +1112,10 @@ export class CipherService implements CipherServiceAbstraction { String(model.login.fido2Key.counter), key ); + cipher.login.fido2Key.discoverable = await this.cryptoService.encrypt( + String(model.login.fido2Key.discoverable), + key + ); } return; case CipherType.SecureNote: @@ -1185,6 +1189,10 @@ export class CipherService implements CipherServiceAbstraction { String(model.fido2Key.counter), key ); + cipher.fido2Key.discoverable = await this.cryptoService.encrypt( + String(model.fido2Key.discoverable), + key + ); break; default: throw new Error("Unknown cipher type."); From 84c43beab050226679932df5227d510a9f8f8618 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 11 Sep 2023 11:41:26 +0200 Subject: [PATCH 17/81] [PM-3807] feat: filter credentials by rpId AND discoverable --- .../fido2/fido2-authenticator.service.spec.ts | 36 +++++++++++++++++-- .../fido2/fido2-authenticator.service.ts | 5 ++- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts index e16aa4d80c8..4e9f928e548 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts @@ -584,11 +584,11 @@ describe("FidoAuthenticatorService", () => { ciphers = [ await createCipherView( { type: CipherType.Login }, - { credentialId: credentialIds[0], rpId: RpId } + { credentialId: credentialIds[0], rpId: RpId, discoverable: false } ), await createCipherView( { type: CipherType.Login }, - { credentialId: credentialIds[1], rpId: RpId } + { credentialId: credentialIds[1], rpId: RpId, discoverable: true } ), ]; params = await createParams({ @@ -601,6 +601,36 @@ describe("FidoAuthenticatorService", () => { cipherService.getAllDecrypted.mockResolvedValue(ciphers); }); + it("should ask for all credentials in list when `params` contains allowedCredentials list", async () => { + userInterfaceSession.pickCredential.mockResolvedValue({ + cipherId: ciphers[0].id, + userVerified: false, + }); + + await authenticator.getAssertion(params); + + expect(userInterfaceSession.pickCredential).toHaveBeenCalledWith({ + cipherIds: ciphers.map((c) => c.id), + userVerification: false, + }); + }); + + it("should only ask for discoverable credentials matched by rpId when params does not contains allowedCredentials list", async () => { + params.allowCredentialDescriptorList = undefined; + const discoverableCiphers = ciphers.filter((c) => c.login.fido2Key.discoverable); + userInterfaceSession.pickCredential.mockResolvedValue({ + cipherId: discoverableCiphers[0].id, + userVerified: false, + }); + + await authenticator.getAssertion(params); + + expect(userInterfaceSession.pickCredential).toHaveBeenCalledWith({ + cipherIds: [discoverableCiphers[0].id], + userVerification: false, + }); + }); + for (const userVerification of [true, false]) { /** Spec: Prompt the user to select a public key credential source selectedCredential from credentialOptions. */ it(`should request confirmation from user when user verification is ${userVerification}`, async () => { @@ -798,7 +828,7 @@ function createCipherView( fido2KeyView.userHandle = fido2Key.userHandle ?? Fido2Utils.bufferToString(randomBytes(16)); fido2KeyView.keyAlgorithm = fido2Key.keyAlgorithm ?? "ECDSA"; fido2KeyView.keyCurve = fido2Key.keyCurve ?? "P-256"; - fido2KeyView.discoverable = true; + fido2KeyView.discoverable = fido2Key.discoverable ?? true; fido2KeyView.keyValue = fido2KeyView.keyValue ?? "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgTC-7XDZipXbaVBlnkjlBgO16ZmqBZWejK2iYo6lV0dehRANCAASOcM2WduNq1DriRYN7ZekvZz-bRhA-qNT4v0fbp5suUFJyWmgOQ0bybZcLXHaerK5Ep1JiSrQcewtQNgLtry7f"; diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts index 8f5cae64df7..9a92a51aee9 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts @@ -188,8 +188,6 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr } let cipherOptions: CipherView[]; - - // eslint-disable-next-line no-empty if (params.allowCredentialDescriptorList?.length > 0) { cipherOptions = await this.findCredentialsById( params.allowCredentialDescriptorList, @@ -344,7 +342,8 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr !cipher.isDeleted && cipher.type === CipherType.Login && cipher.login.fido2Key != undefined && - cipher.login.fido2Key.rpId === rpId + cipher.login.fido2Key.rpId === rpId && + cipher.login.fido2Key.discoverable ); } } From 85a3903dc1ae14dacae147d9bc40b60646ee6bca Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 11 Sep 2023 13:18:29 +0200 Subject: [PATCH 18/81] [PM-3807] chore: remove discoverable tests which are no longer needed --- .../fido2/fido2-authenticator.service.spec.ts | 79 +------------------ 1 file changed, 1 insertion(+), 78 deletions(-) diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts index 4e9f928e548..d0e57f8cc46 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts @@ -101,7 +101,7 @@ describe("FidoAuthenticatorService", () => { describe.skip("when extensions parameter is present", () => undefined); - describe("vault contains excluded non-discoverable credential", () => { + describe("vault contains excluded credential", () => { let excludedCipher: CipherView; let params: Fido2AuthenticatorMakeCredentialsParams; @@ -179,83 +179,6 @@ describe("FidoAuthenticatorService", () => { ); }); - describe("vault contains excluded discoverable credential", () => { - let excludedCipherView: CipherView; - let params: Fido2AuthenticatorMakeCredentialsParams; - - beforeEach(async () => { - excludedCipherView = createCipherView(); - params = await createParams({ - excludeCredentialDescriptorList: [ - { - id: guidToRawFormat(excludedCipherView.fido2Key.credentialId), - type: "public-key", - }, - ], - }); - cipherService.get.mockImplementation(async (id) => - id === excludedCipherView.id - ? ({ decrypt: async () => excludedCipherView } as any) - : undefined - ); - cipherService.getAllDecrypted.mockResolvedValue([excludedCipherView]); - }); - - /** - * Spec: collect an authorization gesture confirming user consent for creating a new credential. - * Deviation: Consent is not asked and the user is simply informed of the situation. - **/ - it("should inform user", async () => { - userInterfaceSession.informExcludedCredential.mockResolvedValue(); - - try { - await authenticator.makeCredential(params); - // eslint-disable-next-line no-empty - } catch {} - - expect(userInterfaceSession.informExcludedCredential).toHaveBeenCalled(); - }); - - /** Spec: return an error code equivalent to "NotAllowedError" and terminate the operation. */ - it("should throw error", async () => { - userInterfaceSession.informExcludedCredential.mockResolvedValue(); - - const result = async () => await authenticator.makeCredential(params); - - await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed); - }); - - /** Devation: Organization ciphers are not checked against excluded credentials, even if the user has access to them. */ - it("should not inform user of duplication when the excluded credential belongs to an organization", async () => { - userInterfaceSession.informExcludedCredential.mockResolvedValue(); - excludedCipherView.organizationId = "someOrganizationId"; - - try { - await authenticator.makeCredential(params); - // eslint-disable-next-line no-empty - } catch {} - - expect(userInterfaceSession.informExcludedCredential).not.toHaveBeenCalled(); - }); - - it("should not inform user of duplication when input data does not pass checks", async () => { - userInterfaceSession.informExcludedCredential.mockResolvedValue(); - const invalidParams = await createInvalidParams(); - - for (const p of Object.values(invalidParams)) { - try { - await authenticator.makeCredential(p); - // eslint-disable-next-line no-empty - } catch {} - } - expect(userInterfaceSession.informExcludedCredential).not.toHaveBeenCalled(); - }); - - it.todo( - "should not throw error if the excluded credential has been marked as deleted in the vault" - ); - }); - describe("credential creation", () => { let existingCipher: CipherView; let params: Fido2AuthenticatorMakeCredentialsParams; From afae855b05869b9cda961cabdf0a88ae1ab74217 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 11 Sep 2023 13:38:26 +0200 Subject: [PATCH 19/81] [PM-3807] chore: remove all logic for handling standalone Fido2Key View and components will be cleaned up as part of UI tickets --- .../src/vault/models/data/cipher.data.ts | 7 ---- libs/common/src/vault/models/domain/cipher.ts | 14 ------- .../vault/models/request/cipher.request.ts | 38 ------------------- .../vault/models/response/cipher.response.ts | 7 ---- .../src/vault/models/view/cipher.view.ts | 5 --- .../src/vault/services/cipher.service.ts | 28 -------------- .../fido2/fido2-authenticator.service.spec.ts | 4 +- .../fido2/fido2-authenticator.service.ts | 7 ++-- 8 files changed, 5 insertions(+), 105 deletions(-) diff --git a/libs/common/src/vault/models/data/cipher.data.ts b/libs/common/src/vault/models/data/cipher.data.ts index 1c995b32aea..2f83ee194b4 100644 --- a/libs/common/src/vault/models/data/cipher.data.ts +++ b/libs/common/src/vault/models/data/cipher.data.ts @@ -4,7 +4,6 @@ import { CipherResponse } from "../response/cipher.response"; import { AttachmentData } from "./attachment.data"; import { CardData } from "./card.data"; -import { Fido2KeyData } from "./fido2-key.data"; import { FieldData } from "./field.data"; import { IdentityData } from "./identity.data"; import { LoginData } from "./login.data"; @@ -27,7 +26,6 @@ export class CipherData { secureNote?: SecureNoteData; card?: CardData; identity?: IdentityData; - fido2Key?: Fido2KeyData; fields?: FieldData[]; attachments?: AttachmentData[]; passwordHistory?: PasswordHistoryData[]; @@ -60,8 +58,6 @@ export class CipherData { switch (this.type) { case CipherType.Login: this.login = new LoginData(response.login); - this.fido2Key = - response.fido2Key != undefined ? new Fido2KeyData(response.fido2Key) : undefined; break; case CipherType.SecureNote: this.secureNote = new SecureNoteData(response.secureNote); @@ -72,9 +68,6 @@ export class CipherData { case CipherType.Identity: this.identity = new IdentityData(response.identity); break; - case CipherType.Fido2Key: - this.fido2Key = new Fido2KeyData(response.fido2Key); - break; default: break; } diff --git a/libs/common/src/vault/models/domain/cipher.ts b/libs/common/src/vault/models/domain/cipher.ts index ec64d40ee55..f85b6cb45c9 100644 --- a/libs/common/src/vault/models/domain/cipher.ts +++ b/libs/common/src/vault/models/domain/cipher.ts @@ -13,7 +13,6 @@ import { CipherView } from "../view/cipher.view"; import { Attachment } from "./attachment"; import { Card } from "./card"; -import { Fido2Key } from "./fido2-key"; import { Field } from "./field"; import { Identity } from "./identity"; import { Login } from "./login"; @@ -39,7 +38,6 @@ export class Cipher extends Domain implements Decryptable { identity: Identity; card: Card; secureNote: SecureNote; - fido2Key: Fido2Key; attachments: Attachment[]; fields: Field[]; passwordHistory: Password[]; @@ -96,9 +94,6 @@ export class Cipher extends Domain implements Decryptable { case CipherType.Identity: this.identity = new Identity(obj.identity); break; - case CipherType.Fido2Key: - this.fido2Key = new Fido2Key(obj.fido2Key); - break; default: break; } @@ -148,9 +143,6 @@ export class Cipher extends Domain implements Decryptable { case CipherType.Identity: model.identity = await this.identity.decrypt(this.organizationId, encKey); break; - case CipherType.Fido2Key: - model.fido2Key = await this.fido2Key.decrypt(this.organizationId, encKey); - break; default: break; } @@ -236,9 +228,6 @@ export class Cipher extends Domain implements Decryptable { case CipherType.Identity: c.identity = this.identity.toIdentityData(); break; - case CipherType.Fido2Key: - c.fido2Key = this.fido2Key.toFido2KeyData(); - break; default: break; } @@ -292,9 +281,6 @@ export class Cipher extends Domain implements Decryptable { case CipherType.SecureNote: domain.secureNote = SecureNote.fromJSON(obj.secureNote); break; - case CipherType.Fido2Key: - domain.fido2Key = Fido2Key.fromJSON(obj.fido2Key); - break; default: break; } diff --git a/libs/common/src/vault/models/request/cipher.request.ts b/libs/common/src/vault/models/request/cipher.request.ts index 108e326a326..8c27ec597b5 100644 --- a/libs/common/src/vault/models/request/cipher.request.ts +++ b/libs/common/src/vault/models/request/cipher.request.ts @@ -23,7 +23,6 @@ export class CipherRequest { secureNote: SecureNoteApi; card: CardApi; identity: IdentityApi; - fido2Key: Fido2KeyApi; fields: FieldApi[]; passwordHistory: PasswordHistoryRequest[]; // Deprecated, remove at some point and rename attachments2 to attachments @@ -169,43 +168,6 @@ export class CipherRequest { ? cipher.identity.licenseNumber.encryptedString : null; break; - case CipherType.Fido2Key: - this.fido2Key = new Fido2KeyApi(); - this.fido2Key.credentialId = - cipher.fido2Key.credentialId != null - ? cipher.fido2Key.credentialId.encryptedString - : null; - this.fido2Key.keyType = - cipher.fido2Key.keyType != null - ? (cipher.fido2Key.keyType.encryptedString as "public-key") - : null; - this.fido2Key.keyAlgorithm = - cipher.fido2Key.keyAlgorithm != null - ? (cipher.fido2Key.keyAlgorithm.encryptedString as "ECDSA") - : null; - this.fido2Key.keyCurve = - cipher.fido2Key.keyCurve != null - ? (cipher.fido2Key.keyCurve.encryptedString as "P-256") - : null; - this.fido2Key.keyValue = - cipher.fido2Key.keyValue != null ? cipher.fido2Key.keyValue.encryptedString : null; - this.fido2Key.rpId = - cipher.fido2Key.rpId != null ? cipher.fido2Key.rpId.encryptedString : null; - this.fido2Key.rpName = - cipher.fido2Key.rpName != null ? cipher.fido2Key.rpName.encryptedString : null; - this.fido2Key.counter = - cipher.fido2Key.counter != null ? cipher.fido2Key.counter.encryptedString : null; - this.fido2Key.userHandle = - cipher.fido2Key.userHandle != null ? cipher.fido2Key.userHandle.encryptedString : null; - this.fido2Key.userDisplayName = - cipher.fido2Key.userDisplayName != null - ? cipher.fido2Key.userDisplayName.encryptedString - : null; - this.fido2Key.discoverable = - cipher.fido2Key.discoverable != null - ? cipher.fido2Key.discoverable.encryptedString - : null; - break; default: break; } diff --git a/libs/common/src/vault/models/response/cipher.response.ts b/libs/common/src/vault/models/response/cipher.response.ts index 52d35708a8f..71e43373775 100644 --- a/libs/common/src/vault/models/response/cipher.response.ts +++ b/libs/common/src/vault/models/response/cipher.response.ts @@ -4,7 +4,6 @@ import { IdentityApi } from "../../../models/api/identity.api"; import { LoginApi } from "../../../models/api/login.api"; import { SecureNoteApi } from "../../../models/api/secure-note.api"; import { BaseResponse } from "../../../models/response/base.response"; -import { Fido2KeyApi } from "../../api/fido2-key.api"; import { CipherRepromptType } from "../../enums/cipher-reprompt-type"; import { AttachmentResponse } from "./attachment.response"; @@ -22,7 +21,6 @@ export class CipherResponse extends BaseResponse { card: CardApi; identity: IdentityApi; secureNote: SecureNoteApi; - fido2Key: Fido2KeyApi; favorite: boolean; edit: boolean; viewPassword: boolean; @@ -76,11 +74,6 @@ export class CipherResponse extends BaseResponse { this.secureNote = new SecureNoteApi(secureNote); } - const fido2Key = this.getResponseProperty("Fido2Key"); - if (fido2Key != null) { - this.fido2Key = new Fido2KeyApi(fido2Key); - } - const fields = this.getResponseProperty("Fields"); if (fields != null) { this.fields = fields.map((f: any) => new FieldApi(f)); diff --git a/libs/common/src/vault/models/view/cipher.view.ts b/libs/common/src/vault/models/view/cipher.view.ts index 608ce4571e6..4b342fd57fb 100644 --- a/libs/common/src/vault/models/view/cipher.view.ts +++ b/libs/common/src/vault/models/view/cipher.view.ts @@ -78,8 +78,6 @@ export class CipherView implements View, InitializerMetadata { return this.card; case CipherType.Identity: return this.identity; - case CipherType.Fido2Key: - return this.fido2Key; default: break; } @@ -174,9 +172,6 @@ export class CipherView implements View, InitializerMetadata { case CipherType.SecureNote: view.secureNote = SecureNoteView.fromJSON(obj.secureNote); break; - case CipherType.Fido2Key: - view.fido2Key = Fido2KeyView.fromJSON(obj.fido2Key); - break; default: break; } diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 58530fbd19e..e1801c47899 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -1166,34 +1166,6 @@ export class CipherService implements CipherServiceAbstraction { key ); return; - case CipherType.Fido2Key: - cipher.fido2Key = new Fido2Key(); - await this.encryptObjProperty( - model.fido2Key, - cipher.fido2Key, - { - credentialId: null, - keyType: null, - keyAlgorithm: null, - keyCurve: null, - keyValue: null, - rpId: null, - rpName: null, - userHandle: null, - userDisplayName: null, - origin: null, - }, - key - ); - cipher.fido2Key.counter = await this.cryptoService.encrypt( - String(model.fido2Key.counter), - key - ); - cipher.fido2Key.discoverable = await this.cryptoService.encrypt( - String(model.fido2Key.discoverable), - key - ); - break; default: throw new Error("Unknown cipher type."); } diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts index d0e57f8cc46..c7fb3f53244 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts @@ -422,7 +422,7 @@ describe("FidoAuthenticatorService", () => { * Spec: If requireUserVerification is true and the authenticator cannot perform user verification, return an error code equivalent to "ConstraintError" and terminate the operation. * Deviation: User verification is checked before checking for excluded credentials **/ - /** TODO: This test should only be activated if we disable support for user verification */ + /** NOTE: This test should only be activated if we disable support for user verification */ it.skip("should throw error if requireUserVerification is set to true", async () => { const params = await createParams({ requireUserVerification: true }); @@ -741,7 +741,7 @@ function createCipherView( ): CipherView { const cipher = new CipherView(); cipher.id = data.id ?? Utils.newGuid(); - cipher.type = data.type ?? CipherType.Fido2Key; + cipher.type = CipherType.Login; cipher.localData = {}; const fido2KeyView = new Fido2KeyView(); diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts index 9a92a51aee9..30f40158f27 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts @@ -299,10 +299,9 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr (cipher) => !cipher.isDeleted && cipher.organizationId == undefined && - ((cipher.type === CipherType.Fido2Key && ids.includes(cipher.fido2Key.credentialId)) || - (cipher.type === CipherType.Login && - cipher.login.fido2Key != undefined && - ids.includes(cipher.login.fido2Key.credentialId))) + cipher.type === CipherType.Login && + cipher.login.fido2Key != undefined && + ids.includes(cipher.login.fido2Key.credentialId) ) .map((cipher) => cipher.id); } From a6d4db9afce66ddf19785be5f269aa9cc302a124 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 11 Sep 2023 13:48:35 +0200 Subject: [PATCH 20/81] [PM-3807] fix: add missing discoverable property handling to tests --- libs/common/src/vault/models/domain/login.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libs/common/src/vault/models/domain/login.spec.ts b/libs/common/src/vault/models/domain/login.spec.ts index 30b3077cb32..f2ec3dbf965 100644 --- a/libs/common/src/vault/models/domain/login.spec.ts +++ b/libs/common/src/vault/models/domain/login.spec.ts @@ -123,6 +123,7 @@ describe("Login DTO", () => { counter: "counter" as EncryptedString, rpName: "rpName" as EncryptedString, userDisplayName: "userDisplayName" as EncryptedString, + discoverable: "discoverable" as EncryptedString, }, }); @@ -143,6 +144,7 @@ describe("Login DTO", () => { counter: "counter_fromJSON", rpName: "rpName_fromJSON", userDisplayName: "userDisplayName_fromJSON", + discoverable: "discoverable_fromJSON", }, }); expect(actual).toBeInstanceOf(Login); From eb1f7ee100f774d30624b08c43eabed118af92d1 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Fri, 15 Sep 2023 09:57:40 -0400 Subject: [PATCH 21/81] updated locales with new text --- apps/browser/src/_locales/en/messages.json | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 0e8ed6d1df6..c4a80c1087b 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -2449,23 +2449,35 @@ "savePasskeyInBitwarden": { "message": "Save passkey in Bitwarden?" }, - "passkeyAlreadyExistsInBitwardenForThisAccount": { - "message": "A passkey already exists in Bitwarden for this account" + "passkeyAlreadyExists": { + "message": "A passkey already exists for this application." }, "noPasskeysFoundForThisApplication": { "message": "No passkeys found for this application." }, + "noMatchingPasskeyLogin": { + "message": "You do not have a matching login for this site." + }, "confirm": { "message": "Confirm" }, "savePasskey": { "message": "Save passkey" }, - "viewPasskey": { - "message": "View passkey" + "savePasskeyNewLogin": { + "message": "Save passkey as new login" }, "choosePasskey": { - "message": "Choose a passkey to login with" + "message": "Choose a login to save this passkey to" + }, + "fido2Item": { + "message": "Fido2 Item" + }, + "overwritePasskey": { + "message": "Overwrite passkey?" + }, + "overwritePasskeyAlert": { + "message": "This item already contains a passkey. Are you sure you want to overwrite the current passkey?" }, "useBrowserName": { "message": "Use $browserName$", From 4c73595f9a9bdcbb778c7935c464a9698532bac6 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Fri, 15 Sep 2023 09:58:52 -0400 Subject: [PATCH 22/81] Updated popout windows service to use defined type for custom width and height --- .../platform/popup/browser-popout-window.service.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/browser/src/platform/popup/browser-popout-window.service.ts b/apps/browser/src/platform/popup/browser-popout-window.service.ts index 3bde8540972..6bc80c3da9c 100644 --- a/apps/browser/src/platform/popup/browser-popout-window.service.ts +++ b/apps/browser/src/platform/popup/browser-popout-window.service.ts @@ -81,23 +81,23 @@ class BrowserPopoutWindowService implements BrowserPopupWindowServiceInterface { senderWindowId: number, popupWindowURL: string, singleActionPopoutKey: string, - customDimensions: { width?: number; height?: number } = {} + options: chrome.windows.CreateData = {} ) { const senderWindow = senderWindowId && (await BrowserApi.getWindow(senderWindowId)); const url = chrome.extension.getURL(popupWindowURL); const offsetRight = 15; const offsetTop = 90; - // Use custom dimensions if provided, otherwise use default - const popupWidth = customDimensions.width || this.defaultPopoutWindowOptions.width; + /// Use overrides in `options` if provided, otherwise use default + const popupWidth = options?.width || this.defaultPopoutWindowOptions.width; const windowOptions = senderWindow ? { ...this.defaultPopoutWindowOptions, - ...customDimensions, - url, left: senderWindow.left + senderWindow.width - popupWidth - offsetRight, top: senderWindow.top + offsetTop, + ...options, + url, } - : { ...this.defaultPopoutWindowOptions, ...customDimensions, url }; + : { ...this.defaultPopoutWindowOptions, url, ...options }; const popupWindow = await BrowserApi.createWindow(windowOptions); From 824e1ddd9d25d4748c9ac0b553e7c2a7d0fff05b Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Fri, 15 Sep 2023 10:00:17 -0400 Subject: [PATCH 23/81] Update on unifying auth flow ui to align with architecture changes --- apps/browser/src/popup/scss/base.scss | 120 +++++++++++- .../browser-fido2-user-interface.service.ts | 3 + .../fido2/fido2-cipher-row.component.html | 2 + .../fido2/fido2-cipher-row.component.ts | 21 +- .../components/fido2/fido2.component.html | 58 +++++- .../popup/components/fido2/fido2.component.ts | 184 ++++++++++++++++-- .../components/vault/add-edit.component.html | 2 +- .../components/vault/view.component.html | 2 +- ...ido2-user-interface.service.abstraction.ts | 1 + .../fido2/fido2-authenticator.service.ts | 1 + 10 files changed, 356 insertions(+), 38 deletions(-) diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index 3c7b945b51d..7a1466b5195 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -626,14 +626,95 @@ app-fido2 { flex-direction: column; padding: 12px 24px 12px 24px; - .logo-image { - margin: 0; - width: 197px; - height: 30px; - background-size: 197px 30px; - background-repeat: no-repeat; - @include themify($themes) { - background-image: url("../images/logo-" + themed("logoSuffix") + "@2x.png"); + .auth-header { + display: flex; + justify-content: space-between; + + .left, + .right { + flex: 1; + display: flex; + min-width: -webkit-min-content; /* Workaround to Chrome bug */ + } + + .left { + align-items: center; + padding-right: 10px; + } + + .right { + justify-content: flex-end; + align-items: center; + padding-left: 10px; + } + + .search { + padding: 7px 10px; + width: 100%; + text-align: left; + position: relative; + display: flex; + + .bwi { + position: absolute; + top: 15px; + left: 20px; + + @include themify($themes) { + color: themed("labelColor"); + } + } + + input { + width: 100%; + margin: 0; + border: none; + padding: 5px 10px 5px 30px; + border-radius: $border-radius; + + &:focus { + border-radius: $border-radius; + outline: none; + } + + /** make the cancel button visible in both dark/light themes **/ + &[type="search"]::-webkit-search-cancel-button { + -webkit-appearance: none; + appearance: none; + height: 15px; + width: 15px; + background-repeat: no-repeat; + mask-image: url("../images/close-button-white.svg"); + -webkit-mask-image: url("../images/close-button-white.svg"); + @include themify($themes) { + background-color: themed("headerInputColor"); + } + } + } + } + + .left + .search, + .left + .sr-only + .search { + padding-left: 0; + + .bwi { + left: 10px; + } + } + + .search + .right { + margin-left: -10px; + } + + .logo-image { + margin: 0; + width: 197px; + height: 30px; + background-size: 197px 30px; + background-repeat: no-repeat; + @include themify($themes) { + background-image: url("../images/logo-" + themed("logoSuffix") + "@2x.png"); + } } } @@ -657,7 +738,19 @@ app-fido2 { } .box-content { - max-height: 145px; + max-height: 140px; + } + + @media screen and (min-height: 501px) and (max-height: 600px) { + .box-content { + max-height: 200px; + } + } + + @media screen and (min-height: 601px) { + .box-content { + max-height: 260px; + } } .box-content-row { @@ -674,6 +767,7 @@ app-fido2 { &:focus { @include themify($themes) { + outline: none; border-left: 5px solid themed("headerInputBackgroundFocusColor"); background-color: themed("headerBackgroundHoverColor"); color: themed("headerColor"); @@ -682,6 +776,14 @@ app-fido2 { } } + .box-content-row.selected-item { + @include themify($themes) { + border-left: 5px solid themed("headerInputBackgroundFocusColor"); + background-color: themed("headerBackgroundHoverColor"); + color: themed("headerColor"); + } + } + .btn { width: 100%; font-size: 16px; diff --git a/apps/browser/src/vault/fido2/browser-fido2-user-interface.service.ts b/apps/browser/src/vault/fido2/browser-fido2-user-interface.service.ts index 224a76bf993..f925009c7f5 100644 --- a/apps/browser/src/vault/fido2/browser-fido2-user-interface.service.ts +++ b/apps/browser/src/vault/fido2/browser-fido2-user-interface.service.ts @@ -60,6 +60,7 @@ export type BrowserFido2Message = { sessionId: string } & ( type: "ConfirmNewCredentialRequest"; credentialName: string; userName: string; + discoverable: boolean; userVerification: boolean; fallbackSupported: boolean; } @@ -212,6 +213,7 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi credentialName, userName, userVerification, + discoverable, }: NewCredentialParams): Promise<{ cipherId: string; userVerified: boolean }> { const data: BrowserFido2Message = { type: "ConfirmNewCredentialRequest", @@ -219,6 +221,7 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi credentialName, userName, userVerification, + discoverable, fallbackSupported: this.fallbackSupported, }; diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html index ab028afd67d..a02dc704083 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html @@ -8,6 +8,8 @@ +
+ + {{ "passkeyFeatureIsNotImplementedForAccountsWithoutMasterPassword" | i18n }} @@ -13,19 +35,35 @@ " >
-

{{ pickCredentialSubTitleText | i18n }}

+

+ {{ + (displayedCiphers.length > 0 + ? getCredentialSubTitleText(data.message.type) + : "noMatchingPasskeyLogin" + ) | i18n + }} +

-
@@ -33,17 +71,17 @@
-

{{ "passkeyAlreadyExistsInBitwardenForThisAccount" | i18n }}

+

{{ "passkeyAlreadyExists" | i18n }}

diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index 4f31734b39a..b6c8f9014e1 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -1,4 +1,4 @@ -import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Component, NgZone, OnDestroy, OnInit } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; import { BehaviorSubject, @@ -12,10 +12,23 @@ import { takeUntil, } from "rxjs"; +import { SearchService } from "@bitwarden/common/abstractions/search.service"; +import { SettingsService } from "@bitwarden/common/abstractions/settings.service"; +import { SecureNoteType } from "@bitwarden/common/enums"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { PasswordRepromptService } from "@bitwarden/common/vault/abstractions/password-reprompt.service"; +import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type"; import { CipherType } from "@bitwarden/common/vault/enums/cipher-type"; +import { CardView } from "@bitwarden/common/vault/models/view/card.view"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { IdentityView } from "@bitwarden/common/vault/models/view/identity.view"; +import { LoginUriView } from "@bitwarden/common/vault/models/view/login-uri.view"; +import { LoginView } from "@bitwarden/common/vault/models/view/login.view"; +import { SecureNoteView } from "@bitwarden/common/vault/models/view/secure-note.view"; +import { DialogService } from "@bitwarden/components"; import { BrowserApi } from "../../../../platform/browser/browser-api"; import { @@ -35,34 +48,55 @@ interface ViewData { styleUrls: [], }) export class Fido2Component implements OnInit, OnDestroy { - selectedItem: CipherView; + cipher: CipherView; + searchTypeSearch = false; + searchPending = false; + searchText: string; + url: string; + hostname: string; + private destroy$ = new Subject(); + private hasSearched = false; + private searchTimeout: any = null; + private hasLoadedAllCiphers = false; protected data$: Observable; protected sessionId?: string; protected ciphers?: CipherView[] = []; + protected displayedCiphers?: CipherView[] = []; protected loading = false; private message$ = new BehaviorSubject(null); - get pickCredentialSubTitleText(): string { - return this.ciphers.length > 1 ? "choosePasskey" : "logInWithPasskey"; - } - constructor( private router: Router, private activatedRoute: ActivatedRoute, private cipherService: CipherService, - private passwordRepromptService: PasswordRepromptService + private passwordRepromptService: PasswordRepromptService, + private platformUtilsService: PlatformUtilsService, + private settingsService: SettingsService, + private searchService: SearchService, + private ngZone: NgZone, + private logService: LogService, + private dialogService: DialogService ) {} ngOnInit(): void { + this.searchTypeSearch = !this.platformUtilsService.isSafari(); + const sessionId$ = this.activatedRoute.queryParamMap.pipe( take(1), map((queryParamMap) => queryParamMap.get("sessionId")) ); - combineLatest([sessionId$, BrowserApi.messageListener$() as Observable]) + // TODO: Remove on Andreas ngZone monkey patch has been merged + const messageListener$ = new Observable((subscriber) => { + const handler = (message: unknown) => this.ngZone.run(() => subscriber.next(message)); // <-- the magic is here + chrome.runtime.onMessage.addListener(handler); + return () => chrome.runtime.onMessage.removeListener(handler); + }) as Observable; + + combineLatest([sessionId$, messageListener$]) .pipe(takeUntil(this.destroy$)) .subscribe(([sessionId, message]) => { this.sessionId = sessionId; @@ -85,9 +119,21 @@ export class Fido2Component implements OnInit, OnDestroy { filter((message) => message != undefined), concatMap(async (message) => { if (message.type === "ConfirmNewCredentialRequest") { + const activeTabs = await BrowserApi.getActiveTabs(); + this.url = activeTabs[0].url; + const equivalentDomains = this.settingsService.getEquivalentDomains(this.url); + this.ciphers = (await this.cipherService.getAllDecrypted()).filter( (cipher) => cipher.type === CipherType.Login && !cipher.isDeleted ); + + this.displayedCiphers = this.ciphers.filter((cipher) => + cipher.login.matchesUri(this.url, equivalentDomains) + ); + + if (this.displayedCiphers.length > 0) { + this.selectedPasskey(this.displayedCiphers[0]); + } } else if (message.type === "PickCredentialRequest") { this.ciphers = await Promise.all( message.cipherIds.map(async (cipherId) => { @@ -95,6 +141,8 @@ export class Fido2Component implements OnInit, OnDestroy { return cipher.decrypt(); }) ); + + this.displayedCiphers = [...this.ciphers]; } else if (message.type === "InformExcludedCredentialRequest") { this.ciphers = await Promise.all( message.existingCipherIds.map(async (cipherId) => { @@ -102,6 +150,12 @@ export class Fido2Component implements OnInit, OnDestroy { return cipher.decrypt(); }) ); + + this.displayedCiphers = [...this.ciphers]; + + if (this.displayedCiphers.length > 0) { + this.selectedPasskey(this.displayedCiphers[0]); + } } return { @@ -124,9 +178,8 @@ export class Fido2Component implements OnInit, OnDestroy { }); } - async pick() { + async submit() { const data = this.message$.value; - const cipher = this.selectedItem; if (data?.type === "PickCredentialRequest") { let userVerified = false; if (data.userVerification) { @@ -135,19 +188,36 @@ export class Fido2Component implements OnInit, OnDestroy { this.send({ sessionId: this.sessionId, - cipherId: cipher.id, + cipherId: this.cipher.id, type: "PickCredentialResponse", userVerified, }); } else if (data?.type === "ConfirmNewCredentialRequest") { let userVerified = false; + + if (this.cipher.login.fido2Key) { + const confirmed = await this.dialogService.openSimpleDialog({ + title: { key: "overwritePasskey" }, + content: { key: "overwritePasskeyAlert" }, + type: "info", + }); + + if (!confirmed) { + return false; + } + } + if (data.userVerification) { userVerified = await this.passwordRepromptService.showPasswordPrompt(); } + if (!this.cipher) { + await this.createNewCipher(); + } + this.send({ sessionId: this.sessionId, - cipherId: cipher.id, + cipherId: this.cipher.id, type: "ConfirmNewCredentialResponse", userVerified, }); @@ -156,13 +226,97 @@ export class Fido2Component implements OnInit, OnDestroy { this.loading = true; } + getCredentialSubTitleText(messageType: string): string { + return messageType == "ConfirmNewCredentialRequest" ? "choosePasskey" : "logInWithPasskey"; + } + + getCredentialButtonText(messageType: string): string { + return messageType == "ConfirmNewCredentialRequest" ? "savePasskey" : "choosePasskey"; + } + selectedPasskey(item: CipherView) { - this.selectedItem = item; + this.cipher = item; } viewPasskey() { - const cipher = this.ciphers[0]; - this.router.navigate(["/view-cipher"], { queryParams: { cipherId: cipher.id } }); + this.router.navigate(["/view-cipher"], { + queryParams: { cipherId: this.cipher.id, uilocation: "popout" }, + }); + } + + addCipher() { + this.router.navigate(["/add-cipher"], { + queryParams: { + name: this.hostname, + uri: this.url, + }, + }); + } + + buildCipher() { + this.cipher = new CipherView(); + this.cipher.name = Utils.getHostname(this.url); + this.cipher.type = CipherType.Login; + this.cipher.login = new LoginView(); + this.cipher.login.uris = [new LoginUriView()]; + this.cipher.login.uris[0].uri = this.url; + this.cipher.card = new CardView(); + this.cipher.identity = new IdentityView(); + this.cipher.secureNote = new SecureNoteView(); + this.cipher.secureNote.type = SecureNoteType.Generic; + this.cipher.reprompt = CipherRepromptType.None; + } + + async createNewCipher() { + this.buildCipher(); + const cipher = await this.cipherService.encrypt(this.cipher); + try { + await this.cipherService.createWithServer(cipher); + this.cipher.id = cipher.id; + } catch (e) { + this.logService.error(e); + } + } + + async loadLoginCiphers() { + this.ciphers = (await this.cipherService.getAllDecrypted()).filter( + (cipher) => cipher.type === CipherType.Login && !cipher.isDeleted + ); + if (!this.hasLoadedAllCiphers) { + this.hasLoadedAllCiphers = !this.searchService.isSearchable(this.searchText); + } + await this.search(null); + } + + async search(timeout: number = null) { + this.searchPending = false; + if (this.searchTimeout != null) { + clearTimeout(this.searchTimeout); + } + + if (timeout == null) { + this.hasSearched = this.searchService.isSearchable(this.searchText); + this.displayedCiphers = await this.searchService.searchCiphers( + this.searchText, + null, + this.ciphers + ); + return; + } + this.searchPending = true; + this.searchTimeout = setTimeout(async () => { + this.hasSearched = this.searchService.isSearchable(this.searchText); + if (!this.hasLoadedAllCiphers && !this.hasSearched) { + await this.loadLoginCiphers(); + } else { + this.displayedCiphers = await this.searchService.searchCiphers( + this.searchText, + null, + this.ciphers + ); + } + this.searchPending = false; + }, timeout); } abort(fallback: boolean) { diff --git a/apps/browser/src/vault/popup/components/vault/add-edit.component.html b/apps/browser/src/vault/popup/components/vault/add-edit.component.html index f88e2b8ee6a..1a9ce0d806d 100644 --- a/apps/browser/src/vault/popup/components/vault/add-edit.component.html +++ b/apps/browser/src/vault/popup/components/vault/add-edit.component.html @@ -138,7 +138,7 @@

{{ "typePasskey" | i18n }} - {{ "passkeyTwoStepLogin" | i18n }} + {{ "dateCreated" | i18n }} {{ cipher.creationDate | date : "short" }}

diff --git a/apps/browser/src/vault/popup/components/vault/view.component.html b/apps/browser/src/vault/popup/components/vault/view.component.html index 0748a23b8d4..f20a7bb3206 100644 --- a/apps/browser/src/vault/popup/components/vault/view.component.html +++ b/apps/browser/src/vault/popup/components/vault/view.component.html @@ -207,7 +207,7 @@

{{ "typePasskey" | i18n }} - {{ "passkeyTwoStepLogin" | i18n }} + {{ "dateCreated" | i18n }} {{ cipher.creationDate | date : "short" }}
diff --git a/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts b/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts index c246b7b8f1b..2f8f1e4408f 100644 --- a/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts +++ b/libs/common/src/vault/abstractions/fido2/fido2-user-interface.service.abstraction.ts @@ -2,6 +2,7 @@ export interface NewCredentialParams { credentialName: string; userName: string; userVerification: boolean; + discoverable: boolean; } export interface PickCredentialParams { diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts index e088f5a5088..41346dc292a 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts @@ -108,6 +108,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr credentialName: params.rpEntity.name, userName: params.userEntity.displayName, userVerification: params.requireUserVerification, + discoverable: params.requireResidentKey, }, abortController ); From cdb6e093d17472aef550cbbd35484e8d49dd2816 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Fri, 15 Sep 2023 14:20:07 -0400 Subject: [PATCH 24/81] Moved click event --- .../popup/components/fido2/fido2-cipher-row.component.html | 5 +++-- .../popup/components/fido2/fido2-cipher-row.component.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html index a02dc704083..a4a9254ea3d 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html @@ -4,11 +4,12 @@ class="virtual-scroll-item" [ngClass]="{ 'override-last': !last }" > -
+
-
+ +
+ +
+ + + +
+ +
+
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index d6857bc6fc2..e4cfd192527 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -219,7 +219,27 @@ export class Fido2Component implements OnInit, OnDestroy { userVerified = await this.passwordRepromptService.showPasswordPrompt(); } - if (!this.cipher) { + this.send({ + sessionId: this.sessionId, + cipherId: this.cipher.id, + type: "ConfirmNewCredentialResponse", + userVerified, + }); + } + + this.loading = true; + } + + //TODO: Confirm if search field should allowed when a passkey already exists + async saveNewLogin() { + const data = this.message$.value; + if (data?.type === "ConfirmNewCredentialRequest") { + let userVerified = false; + if (data.userVerification) { + userVerified = await this.passwordRepromptService.showPasswordPrompt(); + } + + if (userVerified) { await this.createNewCipher(); } From 1dff16041035022f0ab4a3021eb61de2d1d075b4 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Sun, 17 Sep 2023 22:44:26 -0400 Subject: [PATCH 30/81] Allow fido2 pop out close wehn cancle is clicked on add edit component --- .../popup/components/fido2/fido2.component.ts | 4 +++- .../components/vault/add-edit.component.ts | 18 +++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index e4cfd192527..33dda3a94ee 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -279,8 +279,10 @@ export class Fido2Component implements OnInit, OnDestroy { addCipher() { this.router.navigate(["/add-cipher"], { queryParams: { - name: this.hostname, + name: Utils.getHostname(this.url), uri: this.url, + uilocation: "popout", + senderTabId: this.senderTabId, }, }); } diff --git a/apps/browser/src/vault/popup/components/vault/add-edit.component.ts b/apps/browser/src/vault/popup/components/vault/add-edit.component.ts index 6148e31dd6d..324149225d0 100644 --- a/apps/browser/src/vault/popup/components/vault/add-edit.component.ts +++ b/apps/browser/src/vault/popup/components/vault/add-edit.component.ts @@ -1,7 +1,7 @@ import { Location } from "@angular/common"; import { Component } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; -import { first } from "rxjs/operators"; +import { first, takeUntil } from "rxjs/operators"; import { AddEditComponent as BaseAddEditComponent } from "@bitwarden/angular/vault/components/add-edit.component"; import { AuditService } from "@bitwarden/common/abstractions/audit.service"; @@ -35,6 +35,9 @@ export class AddEditComponent extends BaseAddEditComponent { showAttachments = true; openAttachmentsInPopup: boolean; showAutoFillOnPageLoadOptions: boolean; + inPopout = false; + senderTabId?: number; + uilocation?: "popout" | "popup" | "sidebar" | "tab"; constructor( cipherService: CipherService, @@ -79,6 +82,13 @@ export class AddEditComponent extends BaseAddEditComponent { async ngOnInit() { await super.ngOnInit(); + this.route.queryParams.pipe(takeUntil(this.destroy$)).subscribe((value) => { + this.senderTabId = parseInt(value?.senderTabId, 10) || undefined; + this.uilocation = value?.uilocation; + }); + + this.inPopout = this.uilocation === "popout" || this.popupUtilsService.inPopout(window); + // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe this.route.queryParams.pipe(first()).subscribe(async (params) => { if (params.cipherId) { @@ -199,6 +209,12 @@ export class AddEditComponent extends BaseAddEditComponent { return; } + if (this.inPopout && this.senderTabId) { + BrowserApi.focusTab(this.senderTabId); + window.close(); + return; + } + this.location.back(); } From 28021be5587ea19c92a3eee718cfd5a6f653c9d8 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Mon, 18 Sep 2023 08:34:27 -0400 Subject: [PATCH 31/81] Removed makshift run in angular zone --- .../vault/popup/components/fido2/fido2.component.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index 33dda3a94ee..f7ae72db5ab 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -1,4 +1,4 @@ -import { Component, NgZone, OnDestroy, OnInit } from "@angular/core"; +import { Component, OnDestroy, OnInit } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; import { BehaviorSubject, @@ -77,7 +77,6 @@ export class Fido2Component implements OnInit, OnDestroy { private platformUtilsService: PlatformUtilsService, private settingsService: SettingsService, private searchService: SearchService, - private ngZone: NgZone, private logService: LogService, private dialogService: DialogService ) {} @@ -93,14 +92,7 @@ export class Fido2Component implements OnInit, OnDestroy { })) ); - // TODO: Remove on Andreas ngZone monkey patch has been merged - const messageListener$ = new Observable((subscriber) => { - const handler = (message: unknown) => this.ngZone.run(() => subscriber.next(message)); // <-- the magic is here - chrome.runtime.onMessage.addListener(handler); - return () => chrome.runtime.onMessage.removeListener(handler); - }) as Observable; - - combineLatest([queryParams$, messageListener$]) + combineLatest([queryParams$, BrowserApi.messageListener$() as Observable]) .pipe(takeUntil(this.destroy$)) .subscribe(([queryParams, message]) => { this.sessionId = queryParams.sessionId; From 5de3f5212cf49c90fd95f8fd2a789207cde7d847 Mon Sep 17 00:00:00 2001 From: jng Date: Mon, 18 Sep 2023 17:14:28 -0400 Subject: [PATCH 32/81] created focus directive to target first element in ngFor for displayed ciphers in fido2 --- apps/browser/src/popup/app.module.ts | 2 ++ .../fido2/fido2-cipher-row.component.html | 4 ++-- .../fido2/fido2-cipher-row.component.ts | 23 ++++--------------- .../components/fido2/fido2.component.html | 5 ++-- .../popup/components/fido2/fido2.component.ts | 1 + .../popup/components/fido2/focus.directive.ts | 19 +++++++++++++++ .../popup/components/fido2/focus.module.ts | 9 ++++++++ 7 files changed, 40 insertions(+), 23 deletions(-) create mode 100644 apps/browser/src/vault/popup/components/fido2/focus.directive.ts create mode 100644 apps/browser/src/vault/popup/components/fido2/focus.module.ts diff --git a/apps/browser/src/popup/app.module.ts b/apps/browser/src/popup/app.module.ts index c4a3b9c5b73..e6ab3127a6e 100644 --- a/apps/browser/src/popup/app.module.ts +++ b/apps/browser/src/popup/app.module.ts @@ -41,6 +41,7 @@ import { ActionButtonsComponent } from "../vault/popup/components/action-buttons import { CipherRowComponent } from "../vault/popup/components/cipher-row.component"; import { Fido2CipherRowComponent } from "../vault/popup/components/fido2/fido2-cipher-row.component"; import { Fido2Component } from "../vault/popup/components/fido2/fido2.component"; +import { FocusModule } from "../vault/popup/components/fido2/focus.module"; import { PasswordRepromptComponent } from "../vault/popup/components/password-reprompt.component"; import { AddEditCustomFieldsComponent } from "../vault/popup/components/vault/add-edit-custom-fields.component"; import { AddEditComponent } from "../vault/popup/components/vault/add-edit.component"; @@ -99,6 +100,7 @@ import "../platform/popup/locales"; ScrollingModule, ServicesModule, DialogModule, + FocusModule, ], declarations: [ ActionButtonsComponent, diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html index a4a9254ea3d..eefc124011c 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html @@ -9,11 +9,11 @@ type="button" (click)="selectCipher(cipher)" tabindex="0" - #cipherRow - id="fido2CipherRow-{{ rowIndex }}" appStopClick title="{{ title }} - {{ cipher.name }}" class="row-main" + [appFocusOnLoad]="isFirst" + [isSearching]="isSearching" >
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts index 5064b5ce723..0d5d112a616 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts @@ -1,12 +1,4 @@ -import { - AfterViewInit, - Component, - ElementRef, - EventEmitter, - Input, - Output, - ViewChild, -} from "@angular/core"; +import { Component, EventEmitter, Input, Output } from "@angular/core"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -14,22 +6,15 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; selector: "app-fido2-cipher-row", templateUrl: "fido2-cipher-row.component.html", }) -export class Fido2CipherRowComponent implements AfterViewInit { - @ViewChild("cipherRow") cipherRow: ElementRef; - +export class Fido2CipherRowComponent { @Output() onSelected = new EventEmitter(); @Input() cipher: CipherView; @Input() last: boolean; @Input() title: string; - @Input() rowIndex: number; + @Input() isFirst: boolean; + @Input() isSearching: boolean; selectCipher(c: CipherView) { this.onSelected.emit(c); } - - ngAfterViewInit(): void { - if (this.cipherRow.nativeElement.id === "fido2CipherRow-0") { - this.cipherRow.nativeElement.focus(); - } - } } diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index acf2b3ba0bb..2af14ae01a3 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -48,9 +48,10 @@
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index f7ae72db5ab..de1df224a97 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -340,6 +340,7 @@ export class Fido2Component implements OnInit, OnDestroy { null, this.ciphers ); + this.selectedPasskey(this.displayedCiphers[0]); } this.searchPending = false; }, timeout); diff --git a/apps/browser/src/vault/popup/components/fido2/focus.directive.ts b/apps/browser/src/vault/popup/components/fido2/focus.directive.ts new file mode 100644 index 00000000000..bdfc67f3490 --- /dev/null +++ b/apps/browser/src/vault/popup/components/fido2/focus.directive.ts @@ -0,0 +1,19 @@ +import { Directive, ElementRef, Input, OnChanges, SimpleChanges } from "@angular/core"; + +@Directive({ + selector: "[appFocusOnLoad]", +}) +export class FocusOnLoadDirective implements OnChanges { + @Input() appFocusOnLoad: boolean; + @Input() isSearching?: boolean = false; + + constructor(private el: ElementRef) {} + + ngOnChanges(changes: SimpleChanges): void { + if (this.appFocusOnLoad && !this.isSearching) { + setTimeout(() => { + this.el.nativeElement.focus(); + }, 500); + } + } +} diff --git a/apps/browser/src/vault/popup/components/fido2/focus.module.ts b/apps/browser/src/vault/popup/components/fido2/focus.module.ts new file mode 100644 index 00000000000..ff0aa89bb09 --- /dev/null +++ b/apps/browser/src/vault/popup/components/fido2/focus.module.ts @@ -0,0 +1,9 @@ +import { NgModule } from "@angular/core"; + +import { FocusOnLoadDirective } from "./focus.directive"; + +@NgModule({ + declarations: [FocusOnLoadDirective], + exports: [FocusOnLoadDirective], +}) +export class FocusModule {} From d3f0caa875520bbddf31443d99494ebca26a7373 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Mon, 18 Sep 2023 22:35:25 -0400 Subject: [PATCH 33/81] Refactored to use switch statement and added condtional on search and add div --- .../components/fido2/fido2.component.html | 43 +++++----- .../popup/components/fido2/fido2.component.ts | 79 ++++++++++--------- 2 files changed, 67 insertions(+), 55 deletions(-) diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index 2af14ae01a3..efb3bba21c2 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -3,25 +3,32 @@
- -
-
- - -
-
- +
+ +
+ + +
+
+ +
+
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index de1df224a97..d3ae17e8245 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -118,43 +118,49 @@ export class Fido2Component implements OnInit, OnDestroy { this.data$ = this.message$.pipe( filter((message) => message != undefined), concatMap(async (message) => { - if (message.type === "ConfirmNewCredentialRequest") { - const activeTabs = await BrowserApi.getActiveTabs(); - this.url = activeTabs[0].url; - const equivalentDomains = this.settingsService.getEquivalentDomains(this.url); - - this.ciphers = (await this.cipherService.getAllDecrypted()).filter( - (cipher) => cipher.type === CipherType.Login && !cipher.isDeleted - ); - - this.displayedCiphers = this.ciphers.filter((cipher) => - cipher.login.matchesUri(this.url, equivalentDomains) - ); + switch (message.type) { + case "ConfirmNewCredentialRequest": { + const activeTabs = await BrowserApi.getActiveTabs(); + this.url = activeTabs[0].url; + const equivalentDomains = this.settingsService.getEquivalentDomains(this.url); + + this.ciphers = (await this.cipherService.getAllDecrypted()).filter( + (cipher) => cipher.type === CipherType.Login && !cipher.isDeleted + ); + this.displayedCiphers = this.ciphers.filter((cipher) => + cipher.login.matchesUri(this.url, equivalentDomains) + ); + + if (this.displayedCiphers.length > 0) { + this.selectedPasskey(this.displayedCiphers[0]); + } + break; + } - if (this.displayedCiphers.length > 0) { - this.selectedPasskey(this.displayedCiphers[0]); + case "PickCredentialRequest": { + this.ciphers = await Promise.all( + message.cipherIds.map(async (cipherId) => { + const cipher = await this.cipherService.get(cipherId); + return cipher.decrypt(); + }) + ); + this.displayedCiphers = [...this.ciphers]; + break; } - } else if (message.type === "PickCredentialRequest") { - this.ciphers = await Promise.all( - message.cipherIds.map(async (cipherId) => { - const cipher = await this.cipherService.get(cipherId); - return cipher.decrypt(); - }) - ); - - this.displayedCiphers = [...this.ciphers]; - } else if (message.type === "InformExcludedCredentialRequest") { - this.ciphers = await Promise.all( - message.existingCipherIds.map(async (cipherId) => { - const cipher = await this.cipherService.get(cipherId); - return cipher.decrypt(); - }) - ); - - this.displayedCiphers = [...this.ciphers]; - - if (this.displayedCiphers.length > 0) { - this.selectedPasskey(this.displayedCiphers[0]); + + case "InformExcludedCredentialRequest": { + this.ciphers = await Promise.all( + message.existingCipherIds.map(async (cipherId) => { + const cipher = await this.cipherService.get(cipherId); + return cipher.decrypt(); + }) + ); + this.displayedCiphers = [...this.ciphers]; + + if (this.displayedCiphers.length > 0) { + this.selectedPasskey(this.displayedCiphers[0]); + } + break; } } @@ -222,7 +228,6 @@ export class Fido2Component implements OnInit, OnDestroy { this.loading = true; } - //TODO: Confirm if search field should allowed when a passkey already exists async saveNewLogin() { const data = this.message$.value; if (data?.type === "ConfirmNewCredentialRequest") { @@ -237,7 +242,7 @@ export class Fido2Component implements OnInit, OnDestroy { this.send({ sessionId: this.sessionId, - cipherId: this.cipher.id, + cipherId: this.cipher?.id, type: "ConfirmNewCredentialResponse", userVerified, }); From da283fbba5f0d17e7284851c2989540f26234782 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Mon, 18 Sep 2023 23:29:13 -0400 Subject: [PATCH 34/81] Adjusted footer link and added more features to the login flow --- apps/browser/src/popup/scss/base.scss | 6 +++++- .../components/fido2/fido2.component.html | 20 ++++++++++--------- .../popup/components/fido2/fido2.component.ts | 2 +- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index efe4a83aa6b..1640f15d5b4 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -628,7 +628,7 @@ app-fido2 { right: 0; bottom: 0; left: 0; - + display: flex; flex-direction: column; padding: 12px 24px 12px 24px; @@ -796,6 +796,10 @@ app-fido2 { font-weight: 600; } } + + .footer-link { + margin-top: auto; + } } } diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index efb3bba21c2..a5e4d2c976d 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -5,12 +5,7 @@
- +

{{ "noPasskeysFoundForThisApplication" | i18n }}

+ - - {{ "useBrowserName" | i18n }} browser - + +
+ + {{ "useBrowserName" | i18n }} browser + +
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index d3ae17e8245..3cd123a375f 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -256,7 +256,7 @@ export class Fido2Component implements OnInit, OnDestroy { } getCredentialButtonText(messageType: string): string { - return messageType == "ConfirmNewCredentialRequest" ? "savePasskey" : "choosePasskey"; + return messageType == "ConfirmNewCredentialRequest" ? "savePasskey" : "confirm"; } selectedPasskey(item: CipherView) { From d83c3234cf2bd226c8b425f9f44b4e590c075094 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Tue, 19 Sep 2023 10:24:13 -0400 Subject: [PATCH 35/81] Added host listener to abort when window is closed --- .../src/vault/popup/components/fido2/fido2.component.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index 3cd123a375f..d78a6c3eb5c 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -1,4 +1,4 @@ -import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Component, HostListener, OnDestroy, OnInit } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; import { BehaviorSubject, @@ -356,6 +356,7 @@ export class Fido2Component implements OnInit, OnDestroy { window.close(); } + @HostListener("window:unload") unload(fallback = false) { this.send({ sessionId: this.sessionId, From 7c4c4c59d696cbd79fb96859b68722a3d588acbd Mon Sep 17 00:00:00 2001 From: jng Date: Tue, 19 Sep 2023 15:47:51 -0400 Subject: [PATCH 36/81] remove custom focus directive. instead stuck focus logic into fido2-cipher-row component --- apps/browser/src/popup/app.module.ts | 2 -- .../fido2/fido2-cipher-row.component.html | 3 +-- .../fido2/fido2-cipher-row.component.ts | 22 +++++++++++++++++-- .../popup/components/fido2/focus.directive.ts | 19 ---------------- .../popup/components/fido2/focus.module.ts | 9 -------- 5 files changed, 21 insertions(+), 34 deletions(-) delete mode 100644 apps/browser/src/vault/popup/components/fido2/focus.directive.ts delete mode 100644 apps/browser/src/vault/popup/components/fido2/focus.module.ts diff --git a/apps/browser/src/popup/app.module.ts b/apps/browser/src/popup/app.module.ts index e6ab3127a6e..c4a3b9c5b73 100644 --- a/apps/browser/src/popup/app.module.ts +++ b/apps/browser/src/popup/app.module.ts @@ -41,7 +41,6 @@ import { ActionButtonsComponent } from "../vault/popup/components/action-buttons import { CipherRowComponent } from "../vault/popup/components/cipher-row.component"; import { Fido2CipherRowComponent } from "../vault/popup/components/fido2/fido2-cipher-row.component"; import { Fido2Component } from "../vault/popup/components/fido2/fido2.component"; -import { FocusModule } from "../vault/popup/components/fido2/focus.module"; import { PasswordRepromptComponent } from "../vault/popup/components/password-reprompt.component"; import { AddEditCustomFieldsComponent } from "../vault/popup/components/vault/add-edit-custom-fields.component"; import { AddEditComponent } from "../vault/popup/components/vault/add-edit.component"; @@ -100,7 +99,6 @@ import "../platform/popup/locales"; ScrollingModule, ServicesModule, DialogModule, - FocusModule, ], declarations: [ ActionButtonsComponent, diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html index eefc124011c..d2a33325899 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html @@ -12,8 +12,7 @@ appStopClick title="{{ title }} - {{ cipher.name }}" class="row-main" - [appFocusOnLoad]="isFirst" - [isSearching]="isSearching" + #cipherRow >
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts index 0d5d112a616..ac723095b96 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts @@ -1,4 +1,13 @@ -import { Component, EventEmitter, Input, Output } from "@angular/core"; +import { + Component, + EventEmitter, + Input, + OnChanges, + Output, + SimpleChanges, + ViewChild, + ElementRef, +} from "@angular/core"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -6,7 +15,8 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; selector: "app-fido2-cipher-row", templateUrl: "fido2-cipher-row.component.html", }) -export class Fido2CipherRowComponent { +export class Fido2CipherRowComponent implements OnChanges { + @ViewChild("cipherRow") cipherRow: ElementRef; @Output() onSelected = new EventEmitter(); @Input() cipher: CipherView; @Input() last: boolean; @@ -14,6 +24,14 @@ export class Fido2CipherRowComponent { @Input() isFirst: boolean; @Input() isSearching: boolean; + ngOnChanges(changes: SimpleChanges): void { + if (this.isFirst && !this.isSearching) { + setTimeout(() => { + this.cipherRow.nativeElement.focus(); + }, 500); + } + } + selectCipher(c: CipherView) { this.onSelected.emit(c); } diff --git a/apps/browser/src/vault/popup/components/fido2/focus.directive.ts b/apps/browser/src/vault/popup/components/fido2/focus.directive.ts deleted file mode 100644 index bdfc67f3490..00000000000 --- a/apps/browser/src/vault/popup/components/fido2/focus.directive.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { Directive, ElementRef, Input, OnChanges, SimpleChanges } from "@angular/core"; - -@Directive({ - selector: "[appFocusOnLoad]", -}) -export class FocusOnLoadDirective implements OnChanges { - @Input() appFocusOnLoad: boolean; - @Input() isSearching?: boolean = false; - - constructor(private el: ElementRef) {} - - ngOnChanges(changes: SimpleChanges): void { - if (this.appFocusOnLoad && !this.isSearching) { - setTimeout(() => { - this.el.nativeElement.focus(); - }, 500); - } - } -} diff --git a/apps/browser/src/vault/popup/components/fido2/focus.module.ts b/apps/browser/src/vault/popup/components/fido2/focus.module.ts deleted file mode 100644 index ff0aa89bb09..00000000000 --- a/apps/browser/src/vault/popup/components/fido2/focus.module.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { NgModule } from "@angular/core"; - -import { FocusOnLoadDirective } from "./focus.directive"; - -@NgModule({ - declarations: [FocusOnLoadDirective], - exports: [FocusOnLoadDirective], -}) -export class FocusModule {} From ce3919af9db795c03e7a98e4914728b2a50ba827 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Tue, 19 Sep 2023 16:40:52 -0400 Subject: [PATCH 37/81] Fixed bug where close and cancel on view and add component does not abort the fido2 request --- .../components/fido2/fido2.component.html | 3 +- .../popup/components/fido2/fido2.component.ts | 2 ++ .../components/vault/add-edit.component.ts | 35 +++++++++++++++++-- .../popup/components/vault/view.component.ts | 20 +++++++++++ 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index a5e4d2c976d..1cbdcabc441 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -96,8 +96,9 @@
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index d78a6c3eb5c..c3c220f1ba0 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -269,6 +269,7 @@ export class Fido2Component implements OnInit, OnDestroy { cipherId: this.cipher.id, uilocation: "popout", senderTabId: this.senderTabId, + sessionId: this.sessionId, }, }); } @@ -280,6 +281,7 @@ export class Fido2Component implements OnInit, OnDestroy { uri: this.url, uilocation: "popout", senderTabId: this.senderTabId, + sessionId: this.sessionId, }, }); } diff --git a/apps/browser/src/vault/popup/components/vault/add-edit.component.ts b/apps/browser/src/vault/popup/components/vault/add-edit.component.ts index 324149225d0..282dc8b951c 100644 --- a/apps/browser/src/vault/popup/components/vault/add-edit.component.ts +++ b/apps/browser/src/vault/popup/components/vault/add-edit.component.ts @@ -24,6 +24,7 @@ import { DialogService } from "@bitwarden/components"; import { BrowserApi } from "../../../../platform/browser/browser-api"; import { PopupUtilsService } from "../../../../popup/services/popup-utils.service"; +import { BrowserFido2UserInterfaceSession } from "../../../fido2/browser-fido2-user-interface.service"; @Component({ selector: "app-vault-add-edit", @@ -38,6 +39,8 @@ export class AddEditComponent extends BaseAddEditComponent { inPopout = false; senderTabId?: number; uilocation?: "popout" | "popup" | "sidebar" | "tab"; + // uniquely identifies a passkey's popout window + sessionId?: string; constructor( cipherService: CipherService, @@ -85,6 +88,7 @@ export class AddEditComponent extends BaseAddEditComponent { this.route.queryParams.pipe(takeUntil(this.destroy$)).subscribe((value) => { this.senderTabId = parseInt(value?.senderTabId, 10) || undefined; this.uilocation = value?.uilocation; + this.sessionId = value?.sessionId; }); this.inPopout = this.uilocation === "popout" || this.popupUtilsService.inPopout(window); @@ -166,6 +170,10 @@ export class AddEditComponent extends BaseAddEditComponent { return false; } + if (this.inPopout && this.sessionId) { + this.abortFido2Popout(); + } + if (this.popupUtilsService.inTab(window)) { this.popupUtilsService.disableCloseTabWarning(); this.messagingService.send("closeTab", { delay: 1000 }); @@ -204,20 +212,43 @@ export class AddEditComponent extends BaseAddEditComponent { cancel() { super.cancel(); + // Would be refactored after rework is done on the windows popout service + if (this.inPopout && this.sessionId) { + this.abortFido2Popout(); + } + if (this.popupUtilsService.inTab(window)) { this.messagingService.send("closeTab"); return; } if (this.inPopout && this.senderTabId) { - BrowserApi.focusTab(this.senderTabId); - window.close(); + this.close(); return; } this.location.back(); } + // Used for closing single-action views + close() { + BrowserApi.focusTab(this.senderTabId); + window.close(); + + return; + } + + // Used for aborting Fido2 popout + abortFido2Popout() { + BrowserFido2UserInterfaceSession.sendMessage({ + sessionId: this.sessionId, + type: "AbortResponse", + fallbackRequested: false, + }); + + return; + } + async generateUsername(): Promise { const confirmed = await super.generateUsername(); if (confirmed) { diff --git a/apps/browser/src/vault/popup/components/vault/view.component.ts b/apps/browser/src/vault/popup/components/vault/view.component.ts index 73381dce4d0..4fc92bdfb70 100644 --- a/apps/browser/src/vault/popup/components/vault/view.component.ts +++ b/apps/browser/src/vault/popup/components/vault/view.component.ts @@ -28,6 +28,7 @@ import { DialogService } from "@bitwarden/components"; import { AutofillService } from "../../../../autofill/services/abstractions/autofill.service"; import { BrowserApi } from "../../../../platform/browser/browser-api"; import { PopupUtilsService } from "../../../../popup/services/popup-utils.service"; +import { BrowserFido2UserInterfaceSession } from "../../../fido2/browser-fido2-user-interface.service"; const BroadcasterSubscriptionId = "ChildViewComponent"; @@ -55,6 +56,8 @@ export class ViewComponent extends BaseViewComponent { uilocation?: "popout" | "popup" | "sidebar" | "tab"; loadPageDetailsTimeout: number; inPopout = false; + // uniquely identifies a passkey's popout window + sessionId?: string; private destroy$ = new Subject(); @@ -112,6 +115,7 @@ export class ViewComponent extends BaseViewComponent { this.loadAction = value?.action; this.senderTabId = parseInt(value?.senderTabId, 10) || undefined; this.uilocation = value?.uilocation; + this.sessionId = value?.sessionId; }); this.inPopout = this.uilocation === "popout" || this.popupUtilsService.inPopout(window); @@ -300,6 +304,11 @@ export class ViewComponent extends BaseViewComponent { } close() { + // Would be refactored after rework is done on the windows popout service + if (this.inPopout && this.sessionId) { + this.abortFido2Popout(); + } + if (this.inPopout && this.senderTabId) { BrowserApi.focusTab(this.senderTabId); window.close(); @@ -309,6 +318,17 @@ export class ViewComponent extends BaseViewComponent { this.location.back(); } + // Used for aborting Fido2 popout + abortFido2Popout() { + BrowserFido2UserInterfaceSession.sendMessage({ + sessionId: this.sessionId, + type: "AbortResponse", + fallbackRequested: false, + }); + + return; + } + private async loadPageDetails() { this.pageDetails = []; this.tab = await BrowserApi.getTabFromCurrentWindow(); From 4a949e451745e783812b5b6e52c76a5cbecaa065 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Tue, 19 Sep 2023 21:13:48 -0400 Subject: [PATCH 38/81] show info dialog when user account does not have master password --- apps/browser/src/_locales/en/messages.json | 3 + .../components/fido2/fido2.component.html | 3 - .../popup/components/fido2/fido2.component.ts | 57 +++++++++++++------ 3 files changed, 43 insertions(+), 20 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 27f253517bb..e851905dc20 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -2479,6 +2479,9 @@ "overwritePasskeyAlert": { "message": "This item already contains a passkey. Are you sure you want to overwrite the current passkey?" }, + "featureNotSupported": { + "message": "Feature not yet supported" + }, "useBrowserName": { "message": "Use $browserName$", "placeholders": { diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index 1cbdcabc441..e396a080dfd 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -26,9 +26,6 @@
- - {{ "passkeyFeatureIsNotImplementedForAccountsWithoutMasterPassword" | i18n }} - { + this.sessionId = queryParams.sessionId; + this.senderTabId = queryParams.senderTabId; + + if ( + message.type === "NewSessionCreatedRequest" && + message.sessionId !== queryParams.sessionId + ) { + this.abort(false); + return; + } - if (message.sessionId !== queryParams.sessionId) { - return; - } + if (message.sessionId !== queryParams.sessionId) { + return; + } - if (message.type === "AbortRequest") { - return this.abort(false); - } + if (message.type === "AbortRequest") { + this.abort(false); + return; + } + + // Show dialog if user account does not have master password + if (!(await this.passwordRepromptService.enabled())) { + await this.dialogService.openSimpleDialog({ + title: { key: "featureNotSupported" }, + content: { key: "passkeyFeatureIsNotImplementedForAccountsWithoutMasterPassword" }, + acceptButtonText: { key: "ok" }, + cancelButtonText: null, + type: "info", + }); + + this.abort(true); + return; + } + return message; + }), + filter((message) => !!message), + takeUntil(this.destroy$) + ) + .subscribe((message) => { this.message$.next(message); }); From d6ba70d8e2c8334dc1a68dd3f38c4df9e7bda1ef Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Wed, 20 Sep 2023 08:02:48 -0400 Subject: [PATCH 39/81] Removed PopupUtilsService --- apps/browser/src/background/main.background.ts | 2 -- .../fido2/browser-fido2-user-interface.service.ts | 10 +--------- .../vault/popup/components/fido2/fido2.component.ts | 5 ----- 3 files changed, 1 insertion(+), 16 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 12adf6aa83e..a63fb48e4ee 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -556,9 +556,7 @@ export default class MainBackground { ); this.browserPopoutWindowService = new BrowserPopoutWindowService(); - this.popupUtilsService = new PopupUtilsService(this.isPrivateMode); this.fido2UserInterfaceService = new BrowserFido2UserInterfaceService( - this.popupUtilsService, this.browserPopoutWindowService ); this.fido2AuthenticatorService = new Fido2AuthenticatorService( diff --git a/apps/browser/src/vault/fido2/browser-fido2-user-interface.service.ts b/apps/browser/src/vault/fido2/browser-fido2-user-interface.service.ts index 224a76bf993..002cb6d20a8 100644 --- a/apps/browser/src/vault/fido2/browser-fido2-user-interface.service.ts +++ b/apps/browser/src/vault/fido2/browser-fido2-user-interface.service.ts @@ -21,7 +21,6 @@ import { import { BrowserApi } from "../../platform/browser/browser-api"; import { BrowserPopoutWindowService } from "../../platform/popup/abstractions/browser-popout-window.service"; -import { PopupUtilsService } from "../../popup/services/popup-utils.service"; const BrowserFido2MessageName = "BrowserFido2UserInterfaceServiceMessage"; @@ -87,10 +86,7 @@ export type BrowserFido2Message = { sessionId: string } & ( ); export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServiceAbstraction { - constructor( - private popupUtilsService: PopupUtilsService, - private browserPopoutWindowService: BrowserPopoutWindowService - ) {} + constructor(private browserPopoutWindowService: BrowserPopoutWindowService) {} async newSession( fallbackSupported: boolean, @@ -98,7 +94,6 @@ export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServi abortController?: AbortController ): Promise { return await BrowserFido2UserInterfaceSession.create( - this.popupUtilsService, this.browserPopoutWindowService, fallbackSupported, tab, @@ -109,14 +104,12 @@ export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServi export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSession { static async create( - popupUtilsService: PopupUtilsService, browserPopoutWindowService: BrowserPopoutWindowService, fallbackSupported: boolean, tab: chrome.tabs.Tab, abortController?: AbortController ): Promise { return new BrowserFido2UserInterfaceSession( - popupUtilsService, browserPopoutWindowService, fallbackSupported, tab, @@ -136,7 +129,6 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi private destroy$ = new Subject(); private constructor( - private readonly popupUtilsService: PopupUtilsService, private readonly browserPopoutWindowService: BrowserPopoutWindowService, private readonly fallbackSupported: boolean, private readonly tab: chrome.tabs.Tab, diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index 4a9ef93d466..0adc877de7e 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -38,7 +38,6 @@ import { interface ViewData { message: BrowserFido2Message; - showUnsupportedVerification: boolean; fallbackSupported: boolean; } @@ -189,10 +188,6 @@ export class Fido2Component implements OnInit, OnDestroy { return { message, - showUnsupportedVerification: - "userVerification" in message && - message.userVerification && - !(await this.passwordRepromptService.enabled()), fallbackSupported: "fallbackSupported" in message && message.fallbackSupported, }; }), From c26c78fc1aae11aeb1fcc218bec3ced46ffc276e Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Wed, 20 Sep 2023 08:44:20 -0400 Subject: [PATCH 40/81] show info dialog when user account does not have master password --- .../src/vault/popup/components/fido2/fido2.component.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index e396a080dfd..912dca89e65 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -26,7 +26,7 @@
- + Date: Wed, 20 Sep 2023 09:16:00 -0400 Subject: [PATCH 42/81] Added comments --- .../vault/popup/components/fido2/fido2.component.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index bbfa26ab45b..83b3bb4c16f 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -96,9 +96,8 @@ export class Fido2Component implements OnInit, OnDestroy { concatMap(async ([queryParams, message]) => { this.sessionId = queryParams.sessionId; this.senderTabId = queryParams.senderTabId; - /** - * For a 'NewSessionCreatedRequest', abort if it doesn't belong to the current session. - */ + + // For a 'NewSessionCreatedRequest', abort if it doesn't belong to the current session. if ( message.type === "NewSessionCreatedRequest" && message.sessionId !== queryParams.sessionId @@ -106,9 +105,8 @@ export class Fido2Component implements OnInit, OnDestroy { this.abort(false); return; } - /** - * Ignore messages that don't belong to the current session. - */ + + // Ignore messages that don't belong to the current session. if (message.sessionId !== queryParams.sessionId) { return; } From 06cefc856f64af0709274983df6283f9147cad0e Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Wed, 20 Sep 2023 12:03:08 -0400 Subject: [PATCH 43/81] made row height consistent --- apps/browser/src/popup/scss/base.scss | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index 1640f15d5b4..b1eecaf89ec 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -780,6 +780,17 @@ app-fido2 { } } } + + .row-main-content { + display: flex; + flex-direction: column; + justify-content: center; + + .detail { + min-height: 15px; + display: block; + } + } } .box-content-row.selected-item { From 62f59de77ce8f556c4f8621392ec7682968916d2 Mon Sep 17 00:00:00 2001 From: jng Date: Wed, 20 Sep 2023 13:34:34 -0400 Subject: [PATCH 44/81] update logo to be dynamic with theme selection --- apps/browser/src/popup/images/shield+logo.svg | 3 +++ apps/browser/src/popup/images/shield.svg | 10 ++++++++++ apps/browser/src/popup/scss/base.scss | 6 ++++++ .../popup/components/fido2/fido2.component.html | 16 ++++++++++++++-- 4 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 apps/browser/src/popup/images/shield+logo.svg create mode 100644 apps/browser/src/popup/images/shield.svg diff --git a/apps/browser/src/popup/images/shield+logo.svg b/apps/browser/src/popup/images/shield+logo.svg new file mode 100644 index 00000000000..f794bae3dac --- /dev/null +++ b/apps/browser/src/popup/images/shield+logo.svg @@ -0,0 +1,3 @@ + + + diff --git a/apps/browser/src/popup/images/shield.svg b/apps/browser/src/popup/images/shield.svg new file mode 100644 index 00000000000..00453fce26e --- /dev/null +++ b/apps/browser/src/popup/images/shield.svg @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index 1640f15d5b4..8a0a2a72acc 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -722,6 +722,12 @@ app-fido2 { background-image: url("../images/logo-" + themed("logoSuffix") + "@2x.png"); } } + + #passkey-icon-wrapper { + @include themify($themes) { + fill: themed("primaryColor"); + } + } } .auth-flow { diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index 912dca89e65..1b912dd7ec3 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -3,7 +3,16 @@
-
+ + + + + + + + + +
@@ -25,7 +34,10 @@
- +
+

HELLO WORLD

+ {{ data | json }} +
Date: Wed, 20 Sep 2023 21:03:10 -0400 Subject: [PATCH 46/81] Dis some styling to align cipher items --- apps/browser/src/popup/scss/base.scss | 50 ++++--------------- .../fido2/fido2-cipher-row.component.html | 2 +- .../components/fido2/fido2.component.html | 14 ++---- 3 files changed, 15 insertions(+), 51 deletions(-) diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index 0d8831cfa3d..42a14efaf75 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -635,23 +635,15 @@ app-fido2 { .auth-header { display: flex; justify-content: space-between; - - .left, - .right { - flex: 1; - display: flex; - min-width: -webkit-min-content; /* Workaround to Chrome bug */ - } + align-items: center; .left { - align-items: center; padding-right: 10px; } .right { justify-content: flex-end; align-items: center; - padding-left: 10px; } .search { @@ -683,18 +675,12 @@ app-fido2 { outline: none; } - /** make the cancel button visible in both dark/light themes **/ &[type="search"]::-webkit-search-cancel-button { -webkit-appearance: none; appearance: none; - height: 15px; - width: 15px; background-repeat: no-repeat; - mask-image: url("../images/close-button-white.svg"); - -webkit-mask-image: url("../images/close-button-white.svg"); - @include themify($themes) { - background-color: themed("headerInputColor"); - } + mask-image: none; + -webkit-mask-image: none; } } } @@ -708,24 +694,10 @@ app-fido2 { } } - .search + .right { - margin-left: -10px; - } - - .logo-image { - margin: 0; - width: 197px; - height: 30px; - background-size: 197px 30px; - background-repeat: no-repeat; - @include themify($themes) { - background-image: url("../images/logo-" + themed("logoSuffix") + "@2x.png"); - } - } - #passkey-icon-wrapper { + padding-top: 2px; @include themify($themes) { - fill: themed("primaryColor"); + fill: themed("headerBackgroundColor"); } } } @@ -773,6 +745,10 @@ app-fido2 { padding: 0px; margin-bottom: 12px; + button { + min-height: 44px; + } + .row-main { border-radius: 6px; padding: 5px 0px 5px 12px; @@ -799,14 +775,6 @@ app-fido2 { } } - .box-content-row.selected-item { - @include themify($themes) { - border-left: 5px solid themed("headerInputBackgroundFocusColor"); - background-color: themed("headerBackgroundHoverColor"); - color: themed("headerColor"); - } - } - .btn { width: 100%; font-size: 16px; diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html index d2a33325899..7eb811cb752 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html @@ -21,7 +21,7 @@ {{ cipher.name }} - {{ cipher.subTitle }} + {{ cipher.subTitle }}
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index 1b912dd7ec3..ed1a39224e5 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -1,15 +1,14 @@ -
- - + + - - + + @@ -34,10 +33,7 @@
-
-

HELLO WORLD

- {{ data | json }} -
+ Date: Thu, 21 Sep 2023 14:15:58 -0400 Subject: [PATCH 48/81] updated flow of focus and selected items in the passkey popup --- apps/browser/src/popup/scss/base.scss | 6 ++++++ .../components/fido2/fido2-cipher-row.component.html | 2 +- .../components/fido2/fido2-cipher-row.component.ts | 12 ++++++++---- .../popup/components/fido2/fido2.component.html | 11 +++++------ .../vault/popup/components/fido2/fido2.component.ts | 2 +- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index 51a894983e1..8b1613c977e 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -754,6 +754,12 @@ app-fido2 { padding: 5px 0px 5px 12px; &:focus { + @include themify($themes) { + border: 2px solid themed("headerInputBackgroundFocusColor"); + } + } + + &.row-selected { @include themify($themes) { outline: none; border-left: 5px solid themed("headerInputBackgroundFocusColor"); diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html index 7eb811cb752..7b059895dac 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.html @@ -1,6 +1,6 @@
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts index ac723095b96..44366431d94 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2-cipher-row.component.ts @@ -21,14 +21,18 @@ export class Fido2CipherRowComponent implements OnChanges { @Input() cipher: CipherView; @Input() last: boolean; @Input() title: string; - @Input() isFirst: boolean; @Input() isSearching: boolean; + @Input() selectedCipher: CipherView; ngOnChanges(changes: SimpleChanges): void { - if (this.isFirst && !this.isSearching) { + if (this.cipher.id === this.selectedCipher.id && !this.isSearching) { setTimeout(() => { - this.cipherRow.nativeElement.focus(); - }, 500); + this.cipherRow.nativeElement.classList.add("row-selected"); + }); + } else { + setTimeout(() => { + this.cipherRow.nativeElement.classList.remove("row-selected"); + }); } } diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index ed1a39224e5..b7324b9d8b9 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -37,8 +37,8 @@
@@ -55,12 +55,12 @@
@@ -101,9 +101,8 @@
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index 83b3bb4c16f..fbca70b5a22 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -367,9 +367,9 @@ export class Fido2Component implements OnInit, OnDestroy { null, this.ciphers ); - this.selectedPasskey(this.displayedCiphers[0]); } this.searchPending = false; + this.selectedPasskey(this.displayedCiphers[0]); }, timeout); } From 2a4ad41885f1b931a24ed37bb6e1d8c818d04974 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Thu, 21 Sep 2023 23:09:41 -0400 Subject: [PATCH 49/81] Fixed bug when picking a credential --- .../src/vault/popup/components/fido2/fido2.component.html | 3 +++ .../src/vault/popup/components/fido2/fido2.component.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index b7324b9d8b9..6adcdb1d27c 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -103,6 +103,9 @@
diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts index fbca70b5a22..71ea730aea2 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.ts +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.ts @@ -169,6 +169,9 @@ export class Fido2Component implements OnInit, OnDestroy { }) ); this.displayedCiphers = [...this.ciphers]; + if (this.displayedCiphers.length > 0) { + this.selectedPasskey(this.displayedCiphers[0]); + } break; } From 5a63e7b1c1a447d52247a5e95e83b303ec5023e8 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Fri, 22 Sep 2023 14:51:05 -0400 Subject: [PATCH 50/81] Added text to lock popout screen --- apps/browser/src/_locales/en/messages.json | 3 +++ .../src/auth/popup/lock.component.html | 10 +++++++- apps/browser/src/auth/popup/lock.component.ts | 23 +++++++++++++++++-- apps/browser/src/popup/scss/base.scss | 8 +++++++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 1992fd7e5ca..628d2be48da 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -2485,6 +2485,9 @@ "searchLogins": { "message": "Search all logins" }, + "yourPasskeyIsLocked": { + "message": "Authentication required to fill passkey. Verify your identity to continue." + }, "useBrowserName": { "message": "Use $browserName$", "placeholders": { diff --git a/apps/browser/src/auth/popup/lock.component.html b/apps/browser/src/auth/popup/lock.component.html index e787e0106d1..147424cded0 100644 --- a/apps/browser/src/auth/popup/lock.component.html +++ b/apps/browser/src/auth/popup/lock.component.html @@ -62,7 +62,9 @@

@@ -87,5 +89,11 @@

{{ "awaitDesktop" | i18n }}

+ +
+ + {{ "useBrowserName" | i18n }} browser + +
diff --git a/apps/browser/src/auth/popup/lock.component.ts b/apps/browser/src/auth/popup/lock.component.ts index 0e3502dc8e5..d37054c034e 100644 --- a/apps/browser/src/auth/popup/lock.component.ts +++ b/apps/browser/src/auth/popup/lock.component.ts @@ -1,5 +1,5 @@ import { Component, NgZone } from "@angular/core"; -import { Router } from "@angular/router"; +import { ActivatedRoute, Router } from "@angular/router"; import { LockComponent as BaseLockComponent } from "@bitwarden/angular/auth/components/lock.component"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -23,6 +23,7 @@ import { DialogService } from "@bitwarden/components"; import { BiometricErrors, BiometricErrorTypes } from "../../models/biometricErrors"; import { BrowserRouterService } from "../../platform/popup/services/browser-router.service"; +import { BrowserFido2UserInterfaceSession } from "../../vault/fido2/browser-fido2-user-interface.service"; @Component({ selector: "app-lock", @@ -30,10 +31,15 @@ import { BrowserRouterService } from "../../platform/popup/services/browser-rout }) export class LockComponent extends BaseLockComponent { private isInitialLockScreen: boolean; + private sessionId?: string; // Used to uniquely identify passkeys biometricError: string; pendingBiometric = false; + get isPasskeysPopout(): boolean { + return this.sessionId != null; + } + constructor( router: Router, i18nService: I18nService, @@ -54,7 +60,8 @@ export class LockComponent extends BaseLockComponent { dialogService: DialogService, deviceTrustCryptoService: DeviceTrustCryptoServiceAbstraction, userVerificationService: UserVerificationService, - private routerService: BrowserRouterService + private routerService: BrowserRouterService, + private route: ActivatedRoute ) { super( router, @@ -91,6 +98,7 @@ export class LockComponent extends BaseLockComponent { async ngOnInit() { await super.ngOnInit(); + this.sessionId = this.route.snapshot.queryParams.sessionId; const disableAutoBiometricsPrompt = (await this.stateService.getDisableAutoBiometricsPrompt()) ?? true; @@ -131,4 +139,15 @@ export class LockComponent extends BaseLockComponent { return success; } + + // Used for aborting Fido2 popout + abortFido2Popout(fallback = false) { + BrowserFido2UserInterfaceSession.sendMessage({ + sessionId: this.sessionId, + type: "AbortResponse", + fallbackRequested: fallback, + }); + + return; + } } diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index 8b1613c977e..316e1379cd0 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -471,6 +471,14 @@ main { height: calc(100% - 99px); } } + + .footer-link { + padding: 0 10px 5px 10px; + position: fixed; + bottom: 10px; + left: 0; + right: 0; + } } .tab-page { From acdff840b062d54c997583f4471080cf84f6638c Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Fri, 22 Sep 2023 16:34:11 -0400 Subject: [PATCH 51/81] Added passkeys test to home view --- apps/browser/src/_locales/en/messages.json | 3 +++ .../src/auth/popup/home.component.html | 15 +++++++++-- apps/browser/src/auth/popup/home.component.ts | 26 +++++++++++++++++-- .../src/auth/popup/lock.component.html | 13 ++++++---- apps/browser/src/auth/popup/lock.component.ts | 1 + apps/browser/src/popup/scss/base.scss | 9 +------ apps/browser/src/popup/scss/pages.scss | 8 ++++++ 7 files changed, 58 insertions(+), 17 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 628d2be48da..e46e2e767b8 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -2488,6 +2488,9 @@ "yourPasskeyIsLocked": { "message": "Authentication required to fill passkey. Verify your identity to continue." }, + "loginToSavePasskey": { + "message": "Log in to save [access] this passkey" + }, "useBrowserName": { "message": "Use $browserName$", "placeholders": { diff --git a/apps/browser/src/auth/popup/home.component.html b/apps/browser/src/auth/popup/home.component.html index 6b42033c4bc..5d30003d56e 100644 --- a/apps/browser/src/auth/popup/home.component.html +++ b/apps/browser/src/auth/popup/home.component.html @@ -1,7 +1,9 @@
diff --git a/apps/browser/src/auth/popup/home.component.ts b/apps/browser/src/auth/popup/home.component.ts index 5dd3bdd641a..4481722e3c7 100644 --- a/apps/browser/src/auth/popup/home.component.ts +++ b/apps/browser/src/auth/popup/home.component.ts @@ -1,6 +1,6 @@ import { Component, OnDestroy, OnInit, ViewChild } from "@angular/core"; import { FormBuilder, Validators } from "@angular/forms"; -import { Router } from "@angular/router"; +import { ActivatedRoute, Router } from "@angular/router"; import { Subject, takeUntil } from "rxjs"; import { EnvironmentSelectorComponent } from "@bitwarden/angular/auth/components/environment-selector.component"; @@ -10,6 +10,8 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { BrowserFido2UserInterfaceSession } from "../../vault/fido2/browser-fido2-user-interface.service"; + @Component({ selector: "app-home", templateUrl: "home.component.html", @@ -18,6 +20,7 @@ export class HomeComponent implements OnInit, OnDestroy { @ViewChild(EnvironmentSelectorComponent, { static: true }) environmentSelector!: EnvironmentSelectorComponent; private destroyed$: Subject = new Subject(); + private sessionId?: string; // Used to uniquely identify passkeys loginInitiated = false; formGroup = this.formBuilder.group({ @@ -25,6 +28,10 @@ export class HomeComponent implements OnInit, OnDestroy { rememberEmail: [false], }); + get isPasskeysPopout(): boolean { + return this.sessionId != null; + } + constructor( protected platformUtilsService: PlatformUtilsService, private stateService: StateService, @@ -32,7 +39,8 @@ export class HomeComponent implements OnInit, OnDestroy { private router: Router, private i18nService: I18nService, private environmentService: EnvironmentService, - private loginService: LoginService + private loginService: LoginService, + private route: ActivatedRoute ) {} async ngOnInit(): Promise { @@ -60,6 +68,9 @@ export class HomeComponent implements OnInit, OnDestroy { this.setFormValues(); this.router.navigate(["environment"]); }); + + // Get's the sessionId from the query params. The sessionId is sent from the passkeys popout. + this.sessionId = this.route.snapshot.queryParams.sessionId; } ngOnDestroy(): void { @@ -91,4 +102,15 @@ export class HomeComponent implements OnInit, OnDestroy { this.loginService.setEmail(this.formGroup.value.email); this.loginService.setRememberEmail(this.formGroup.value.rememberEmail); } + + // Used for aborting Fido2 popout + abortFido2Popout(fallback = false) { + BrowserFido2UserInterfaceSession.sendMessage({ + sessionId: this.sessionId, + type: "AbortResponse", + fallbackRequested: fallback, + }); + + return; + } } diff --git a/apps/browser/src/auth/popup/lock.component.html b/apps/browser/src/auth/popup/lock.component.html index 147424cded0..3521651b173 100644 --- a/apps/browser/src/auth/popup/lock.component.html +++ b/apps/browser/src/auth/popup/lock.component.html @@ -90,10 +90,13 @@

{{ "awaitDesktop" | i18n }}

-
- - {{ "useBrowserName" | i18n }} browser - -
+ + {{ "useBrowserName" | i18n }} browser + diff --git a/apps/browser/src/auth/popup/lock.component.ts b/apps/browser/src/auth/popup/lock.component.ts index d37054c034e..aed6e843e1d 100644 --- a/apps/browser/src/auth/popup/lock.component.ts +++ b/apps/browser/src/auth/popup/lock.component.ts @@ -98,6 +98,7 @@ export class LockComponent extends BaseLockComponent { async ngOnInit() { await super.ngOnInit(); + // Get's the sessionId from the query params. The sessionId is sent from the passkeys popout. this.sessionId = this.route.snapshot.queryParams.sessionId; const disableAutoBiometricsPrompt = (await this.stateService.getDisableAutoBiometricsPrompt()) ?? true; diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index 316e1379cd0..d242e5ae3ac 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -356,6 +356,7 @@ header { } .content { + width: 100%; padding: 15px 5px; } @@ -471,14 +472,6 @@ main { height: calc(100% - 99px); } } - - .footer-link { - padding: 0 10px 5px 10px; - position: fixed; - bottom: 10px; - left: 0; - right: 0; - } } .tab-page { diff --git a/apps/browser/src/popup/scss/pages.scss b/apps/browser/src/popup/scss/pages.scss index e104570e563..4699d1f0456 100644 --- a/apps/browser/src/popup/scss/pages.scss +++ b/apps/browser/src/popup/scss/pages.scss @@ -153,6 +153,14 @@ body.body-full { margin: 15px 0 15px 0; } +.useBrowserlink { + padding: 0 10px 5px 10px; + position: fixed; + bottom: 10px; + left: 0; + right: 0; +} + app-options { .box { margin: 10px 0; From 1a6135d1f5c0b8271895664c34dead8c80d0a591 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Fri, 22 Sep 2023 19:25:26 -0400 Subject: [PATCH 52/81] changed class name --- apps/browser/src/popup/scss/base.scss | 2 +- .../popup/components/fido2/fido2.component.html | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/apps/browser/src/popup/scss/base.scss b/apps/browser/src/popup/scss/base.scss index 8b1613c977e..3fc70c28acd 100644 --- a/apps/browser/src/popup/scss/base.scss +++ b/apps/browser/src/popup/scss/base.scss @@ -788,7 +788,7 @@ app-fido2 { } } - .footer-link { + .useBrowserlink { margin-top: auto; } } diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index 6adcdb1d27c..b0cf40fa1d8 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -126,10 +126,13 @@ -
- - {{ "useBrowserName" | i18n }} browser - -
+ + {{ "useBrowserName" | i18n }} browser +

From fd54e23f7fe57a4e37091f1455f5c4f5a2ef12af Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Sun, 24 Sep 2023 01:45:01 -0400 Subject: [PATCH 53/81] Added uilocation as a query paramter to know if the user is in the popout window --- apps/browser/src/auth/popup/home.component.ts | 10 ++++++++-- apps/browser/src/auth/popup/login.component.ts | 8 ++++++++ .../src/auth/popup/two-factor-options.component.ts | 4 +++- apps/browser/src/auth/popup/two-factor.component.ts | 11 +++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/apps/browser/src/auth/popup/home.component.ts b/apps/browser/src/auth/popup/home.component.ts index 5dd3bdd641a..80b7ac71a97 100644 --- a/apps/browser/src/auth/popup/home.component.ts +++ b/apps/browser/src/auth/popup/home.component.ts @@ -1,6 +1,6 @@ import { Component, OnDestroy, OnInit, ViewChild } from "@angular/core"; import { FormBuilder, Validators } from "@angular/forms"; -import { Router } from "@angular/router"; +import { ActivatedRoute, Router } from "@angular/router"; import { Subject, takeUntil } from "rxjs"; import { EnvironmentSelectorComponent } from "@bitwarden/angular/auth/components/environment-selector.component"; @@ -30,6 +30,7 @@ export class HomeComponent implements OnInit, OnDestroy { private stateService: StateService, private formBuilder: FormBuilder, private router: Router, + private route: ActivatedRoute, private i18nService: I18nService, private environmentService: EnvironmentService, private loginService: LoginService @@ -80,7 +81,12 @@ export class HomeComponent implements OnInit, OnDestroy { this.loginService.setEmail(this.formGroup.value.email); this.loginService.setRememberEmail(this.formGroup.value.rememberEmail); - this.router.navigate(["login"], { queryParams: { email: this.formGroup.value.email } }); + this.router.navigate(["login"], { + queryParams: { + email: this.formGroup.value.email, + uilocation: this.route.snapshot.queryParams.uilocation, + }, + }); } get selfHostedDomain() { diff --git a/apps/browser/src/auth/popup/login.component.ts b/apps/browser/src/auth/popup/login.component.ts index 2dc997e657b..005779619b9 100644 --- a/apps/browser/src/auth/popup/login.component.ts +++ b/apps/browser/src/auth/popup/login.component.ts @@ -82,6 +82,14 @@ export class LoginComponent extends BaseLoginComponent { } }; + super.onSuccessfulLoginTwoFactorNavigate = async () => { + this.router.navigate([this.twoFactorRoute], { + queryParams: { + uilocation: this.route.snapshot.queryParams.uilocation, + }, + }); + }; + this.showPasswordless = flagEnabled("showPasswordless"); if (this.showPasswordless) { diff --git a/apps/browser/src/auth/popup/two-factor-options.component.ts b/apps/browser/src/auth/popup/two-factor-options.component.ts index a7e95a2a4ec..5e4c1cb782d 100644 --- a/apps/browser/src/auth/popup/two-factor-options.component.ts +++ b/apps/browser/src/auth/popup/two-factor-options.component.ts @@ -41,7 +41,9 @@ export class TwoFactorOptionsComponent extends BaseTwoFactorOptionsComponent { // SSO + 2FA in browser extension this.router.navigate(["2fa"], { queryParams: { sso: true } }); } else { - this.router.navigate(["2fa"]); + this.router.navigate(["2fa"], { + queryParams: { uilocation: this.activatedRoute.snapshot.queryParams.uilocation }, + }); } } } diff --git a/apps/browser/src/auth/popup/two-factor.component.ts b/apps/browser/src/auth/popup/two-factor.component.ts index f4d09aa26ec..33391356720 100644 --- a/apps/browser/src/auth/popup/two-factor.component.ts +++ b/apps/browser/src/auth/popup/two-factor.component.ts @@ -33,6 +33,7 @@ const BroadcasterSubscriptionId = "TwoFactorComponent"; }) export class TwoFactorComponent extends BaseTwoFactorComponent { showNewWindowMessage = false; + inPopout = false; constructor( authService: AuthService, @@ -126,9 +127,13 @@ export class TwoFactorComponent extends BaseTwoFactorComponent { document.body.classList.add("linux-webauthn"); } + // Check if we are in a popout window + this.inPopout = this.route.snapshot.queryParams.uilocation === "popout"; + if ( this.selectedProviderType === TwoFactorProviderType.Email && - this.popupUtilsService.inPopup(window) + this.popupUtilsService.inPopup(window) && + !this.inPopout ) { const confirmed = await this.dialogService.openSimpleDialog({ title: { key: "warning" }, @@ -178,7 +183,9 @@ export class TwoFactorComponent extends BaseTwoFactorComponent { // proper onSuccessfulLogin logic is executed. this.router.navigate(["2fa-options"], { queryParams: { sso: true } }); } else { - this.router.navigate(["2fa-options"]); + this.router.navigate(["2fa-options"], { + queryParams: { uilocation: this.route.snapshot.queryParams.uilocation }, + }); } } From ca048abdc83f9302acfd37b95bc23fc42ddeb749 Mon Sep 17 00:00:00 2001 From: jng Date: Mon, 25 Sep 2023 12:14:11 -0400 Subject: [PATCH 54/81] update fido2 component for dynamic subtitleText as well as additional appA11yTitle attrs --- .../components/fido2/fido2.component.html | 23 +++++++++++-------- .../popup/components/fido2/fido2.component.ts | 6 +++++ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/apps/browser/src/vault/popup/components/fido2/fido2.component.html b/apps/browser/src/vault/popup/components/fido2/fido2.component.html index b0cf40fa1d8..de2a7ecb24f 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -42,13 +42,8 @@ " >
-

- {{ - (displayedCiphers.length > 0 - ? getCredentialSubTitleText(data.message.type) - : "noMatchingPasskeyLogin" - ) | i18n - }} +

+ {{ subtitleText | i18n }}

@@ -66,7 +61,12 @@
-