From 391e5dc81add462526b8be7b552742675c93b67d Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Thu, 12 Oct 2023 22:03:30 -0400 Subject: [PATCH 1/9] remove listeners for safari --- apps/browser/src/platform/browser/browser-api.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/browser/src/platform/browser/browser-api.ts b/apps/browser/src/platform/browser/browser-api.ts index a09c63e18ab..13081765c3f 100644 --- a/apps/browser/src/platform/browser/browser-api.ts +++ b/apps/browser/src/platform/browser/browser-api.ts @@ -274,7 +274,16 @@ export class BrowserApi { BrowserApi.messageListener("message", handler); - return () => chrome.runtime.onMessage.removeListener(handler); + return () => { + chrome.runtime.onMessage.removeListener(handler); + + if (BrowserApi.isSafariApi) { + const index = BrowserApi.registeredMessageListeners.indexOf(handler); + if (index !== -1) { + BrowserApi.registeredMessageListeners.splice(index, 1); + } + } + }; }); } From cabd855c54d8d0cf3dd75575bbde43becb618ade Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Thu, 12 Oct 2023 22:04:46 -0400 Subject: [PATCH 2/9] removed unused i18n tokens --- apps/browser/src/_locales/en/messages.json | 4 ++-- apps/desktop/src/locales/en/messages.json | 6 ------ apps/web/src/locales/en/messages.json | 6 ------ 3 files changed, 2 insertions(+), 14 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 48d69ace9d2..cad4ff91c35 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -2503,8 +2503,8 @@ "choosePasskey": { "message": "Choose a login to save this passkey to" }, - "fido2Item": { - "message": "Fido2 Item" + "passkeyItem": { + "message": "Passkey Item" }, "overwritePasskey": { "message": "Overwrite passkey?" diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index b829a0234be..52fdbf1b560 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -2419,12 +2419,6 @@ "typePasskey": { "message": "Passkey" }, - "application": { - "message": "Application" - }, - "duplicatePasskey": { - "message": "A passkey with this ID already exists in this organization." - }, "passkeyNotCopied": { "message": "Passkey will not be copied" }, diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 64ba1a224d9..e2942b49774 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -7269,12 +7269,6 @@ "typePasskey": { "message": "Passkey" }, - "application": { - "message": "Application" - }, - "duplicatePasskey": { - "message": "A passkey with this ID already exists in this organization." - }, "passkeyNotCopied": { "message": "Passkey will not be copied" }, From 35e91931529407d1cc5c2768fc26616cfc287ea3 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Thu, 12 Oct 2023 22:05:40 -0400 Subject: [PATCH 3/9] changed link to button for accessibilty purposes --- .../fido2/fido2-use-browser-link.component.html | 4 ++-- .../src/vault/popup/components/fido2/fido2.component.html | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/browser/src/vault/popup/components/fido2/fido2-use-browser-link.component.html b/apps/browser/src/vault/popup/components/fido2/fido2-use-browser-link.component.html index 886ec3247d9..3e71675aa2c 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2-use-browser-link.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2-use-browser-link.component.html @@ -1,5 +1,5 @@
- +
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 5ce39c89c9c..0f298b67fb6 100644 --- a/apps/browser/src/vault/popup/components/fido2/fido2.component.html +++ b/apps/browser/src/vault/popup/components/fido2/fido2.component.html @@ -53,7 +53,7 @@ *ngFor="let cipherItem of displayedCiphers" [cipher]="cipherItem" [isSearching]="searchPending" - title="{{ 'fido2Item' | i18n }}" + title="{{ 'passkeyItem' | i18n }}" (onSelected)="selectedPasskey($event)" [isSelected]="cipher === cipherItem" > @@ -108,7 +108,7 @@ @@ -131,9 +131,9 @@
- +
From 186eb392fcd5bcb7f6b633328703d610f508d36b Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Fri, 13 Oct 2023 12:11:59 -0400 Subject: [PATCH 4/9] Fix potential reference error by restoring the typeof check for chrome --- apps/browser/src/popup/services/popup-utils.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/browser/src/popup/services/popup-utils.service.ts b/apps/browser/src/popup/services/popup-utils.service.ts index 9d424586b20..b5a5a058171 100644 --- a/apps/browser/src/popup/services/popup-utils.service.ts +++ b/apps/browser/src/popup/services/popup-utils.service.ts @@ -64,7 +64,7 @@ export class PopupUtilsService { href = win.location.href; } - if (chrome?.windows?.create != null) { + if (typeof chrome !== "undefined" && chrome?.windows?.create != null) { if (href.indexOf("?uilocation=") > -1) { href = href .replace("uilocation=popup", "uilocation=popout") From ce5fc9c278b67df3ca2afc28e181d94f22fbc667 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Fri, 13 Oct 2023 18:10:23 -0400 Subject: [PATCH 5/9] added fromNullable to reduces repetitive logic --- .../models/export/fido2-credential.export.ts | 23 +++++++++---------- .../src/platform/models/domain/enc-string.ts | 4 ++++ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/libs/common/src/models/export/fido2-credential.export.ts b/libs/common/src/models/export/fido2-credential.export.ts index 67f7b7b4408..24d08dc94a6 100644 --- a/libs/common/src/models/export/fido2-credential.export.ts +++ b/libs/common/src/models/export/fido2-credential.export.ts @@ -37,18 +37,17 @@ export class Fido2CredentialExport { } static toDomain(req: Fido2CredentialExport, domain = new Fido2Credential()) { - domain.credentialId = req.credentialId != null ? new EncString(req.credentialId) : null; - domain.keyType = req.keyType != null ? new EncString(req.keyType) : null; - domain.keyAlgorithm = req.keyAlgorithm != null ? new EncString(req.keyAlgorithm) : null; - domain.keyCurve = req.keyCurve != null ? new EncString(req.keyCurve) : null; - domain.keyValue = req.keyValue != null ? new EncString(req.keyValue) : null; - domain.rpId = req.rpId != null ? new EncString(req.rpId) : null; - domain.userHandle = req.userHandle != null ? new EncString(req.userHandle) : null; - domain.counter = req.counter != null ? new EncString(req.counter) : null; - domain.rpName = req.rpName != null ? new EncString(req.rpName) : null; - domain.userDisplayName = - req.userDisplayName != null ? new EncString(req.userDisplayName) : null; - domain.discoverable = req.discoverable != null ? new EncString(req.discoverable) : null; + domain.credentialId = EncString.fromNullable(req.credentialId); + domain.keyType = EncString.fromNullable(req.keyType); + domain.keyAlgorithm = EncString.fromNullable(req.keyAlgorithm); + domain.keyCurve = EncString.fromNullable(req.keyCurve); + domain.keyValue = EncString.fromNullable(req.keyValue); + domain.rpId = EncString.fromNullable(req.rpId); + domain.userHandle = EncString.fromNullable(req.userHandle); + domain.counter = EncString.fromNullable(req.counter); + domain.rpName = EncString.fromNullable(req.rpName); + domain.userDisplayName = EncString.fromNullable(req.userDisplayName); + domain.discoverable = EncString.fromNullable(req.discoverable); domain.creationDate = req.creationDate; return domain; } diff --git a/libs/common/src/platform/models/domain/enc-string.ts b/libs/common/src/platform/models/domain/enc-string.ts index 60b59fa4d8c..65313e94d84 100644 --- a/libs/common/src/platform/models/domain/enc-string.ts +++ b/libs/common/src/platform/models/domain/enc-string.ts @@ -43,6 +43,10 @@ export class EncString implements Encrypted { return this.encryptedString; } + static fromNullable(val: string | null) { + return val ? new EncString(val) : null; + } + static fromJSON(obj: Jsonify): EncString { if (obj == null) { return null; From 6270592bb1bd9a4f53134f73cd41f5380de924ed Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Mon, 16 Oct 2023 09:43:58 -0400 Subject: [PATCH 6/9] Revert "added fromNullable to reduces repetitive logic" This reverts commit ce5fc9c278b67df3ca2afc28e181d94f22fbc667. --- .../models/export/fido2-credential.export.ts | 23 ++++++++++--------- .../src/platform/models/domain/enc-string.ts | 4 ---- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/libs/common/src/models/export/fido2-credential.export.ts b/libs/common/src/models/export/fido2-credential.export.ts index 24d08dc94a6..67f7b7b4408 100644 --- a/libs/common/src/models/export/fido2-credential.export.ts +++ b/libs/common/src/models/export/fido2-credential.export.ts @@ -37,17 +37,18 @@ export class Fido2CredentialExport { } static toDomain(req: Fido2CredentialExport, domain = new Fido2Credential()) { - domain.credentialId = EncString.fromNullable(req.credentialId); - domain.keyType = EncString.fromNullable(req.keyType); - domain.keyAlgorithm = EncString.fromNullable(req.keyAlgorithm); - domain.keyCurve = EncString.fromNullable(req.keyCurve); - domain.keyValue = EncString.fromNullable(req.keyValue); - domain.rpId = EncString.fromNullable(req.rpId); - domain.userHandle = EncString.fromNullable(req.userHandle); - domain.counter = EncString.fromNullable(req.counter); - domain.rpName = EncString.fromNullable(req.rpName); - domain.userDisplayName = EncString.fromNullable(req.userDisplayName); - domain.discoverable = EncString.fromNullable(req.discoverable); + domain.credentialId = req.credentialId != null ? new EncString(req.credentialId) : null; + domain.keyType = req.keyType != null ? new EncString(req.keyType) : null; + domain.keyAlgorithm = req.keyAlgorithm != null ? new EncString(req.keyAlgorithm) : null; + domain.keyCurve = req.keyCurve != null ? new EncString(req.keyCurve) : null; + domain.keyValue = req.keyValue != null ? new EncString(req.keyValue) : null; + domain.rpId = req.rpId != null ? new EncString(req.rpId) : null; + domain.userHandle = req.userHandle != null ? new EncString(req.userHandle) : null; + domain.counter = req.counter != null ? new EncString(req.counter) : null; + domain.rpName = req.rpName != null ? new EncString(req.rpName) : null; + domain.userDisplayName = + req.userDisplayName != null ? new EncString(req.userDisplayName) : null; + domain.discoverable = req.discoverable != null ? new EncString(req.discoverable) : null; domain.creationDate = req.creationDate; return domain; } diff --git a/libs/common/src/platform/models/domain/enc-string.ts b/libs/common/src/platform/models/domain/enc-string.ts index 65313e94d84..60b59fa4d8c 100644 --- a/libs/common/src/platform/models/domain/enc-string.ts +++ b/libs/common/src/platform/models/domain/enc-string.ts @@ -43,10 +43,6 @@ export class EncString implements Encrypted { return this.encryptedString; } - static fromNullable(val: string | null) { - return val ? new EncString(val) : null; - } - static fromJSON(obj: Jsonify): EncString { if (obj == null) { return null; From 59e355367dea6db5f273460498bb53f5b876531f Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Mon, 16 Oct 2023 12:04:19 -0400 Subject: [PATCH 7/9] Added js docs to fido2credential export --- .../models/export/fido2-credential.export.ts | 24 +++++++++++++++++++ libs/common/src/models/export/login.export.ts | 3 ++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/libs/common/src/models/export/fido2-credential.export.ts b/libs/common/src/models/export/fido2-credential.export.ts index 67f7b7b4408..6ceb6850fe5 100644 --- a/libs/common/src/models/export/fido2-credential.export.ts +++ b/libs/common/src/models/export/fido2-credential.export.ts @@ -2,7 +2,14 @@ import { EncString } from "../../platform/models/domain/enc-string"; import { Fido2Credential } from "../../vault/models/domain/fido2-credential"; import { Fido2CredentialView } from "../../vault/models/view/fido2-credential.view"; +/** + * Represents format of Fido2 Credentials in JSON exports. + */ export class Fido2CredentialExport { + /** + * Generates a template for Fido2CredentialExport + * @returns Instance of Fido2CredentialExport with predefined values. + */ static template(): Fido2CredentialExport { const req = new Fido2CredentialExport(); req.credentialId = "keyId"; @@ -20,6 +27,12 @@ export class Fido2CredentialExport { return req; } + /** + * Converts a Fido2CredentialExport object to its view representation. + * @param req - The Fido2CredentialExport object to be converted. + * @param view - (Optional) The Fido2CredentialView object to popualte with Fido2CredentialExport data + * @returns Fido2CredentialView + */ static toView(req: Fido2CredentialExport, view = new Fido2CredentialView()) { view.credentialId = req.credentialId; view.keyType = req.keyType as "public-key"; @@ -36,6 +49,12 @@ export class Fido2CredentialExport { return view; } + /** + * Converts a Fido2CredentialExport object to its domain representation. + * @param req - The Fido2CredentialExport object to be converted. + * @param domain - (Optional) The Fido2Credential object to popualte with Fido2CredentialExport data + * @returns Fido2Credential + */ static toDomain(req: Fido2CredentialExport, domain = new Fido2Credential()) { domain.credentialId = req.credentialId != null ? new EncString(req.credentialId) : null; domain.keyType = req.keyType != null ? new EncString(req.keyType) : null; @@ -66,6 +85,11 @@ export class Fido2CredentialExport { discoverable: string; creationDate: Date; + /** + * Constructs a new Fid2CredentialExport instance + * @param o - The credential to populate the new instance. + * Optional and can be either Fido2CredentialView or Fido2Credential + */ constructor(o?: Fido2CredentialView | Fido2Credential) { if (o == null) { return; diff --git a/libs/common/src/models/export/login.export.ts b/libs/common/src/models/export/login.export.ts index c64d30b5b67..6e14fbd809e 100644 --- a/libs/common/src/models/export/login.export.ts +++ b/libs/common/src/models/export/login.export.ts @@ -36,7 +36,8 @@ export class LoginExport { domain.username = req.username != null ? new EncString(req.username) : null; domain.password = req.password != null ? new EncString(req.password) : null; domain.totp = req.totp != null ? new EncString(req.totp) : null; - //left out fido2Credential for now + // Fido2credentials are currently not supported for imports. + return domain; } From 148fe087b8d334e5bafa7282df948479947514cf Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Mon, 16 Oct 2023 17:36:03 -0400 Subject: [PATCH 8/9] refined jsdocs comments --- .../src/models/export/fido2-credential.export.ts | 10 +++++----- libs/common/src/models/export/login.export.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libs/common/src/models/export/fido2-credential.export.ts b/libs/common/src/models/export/fido2-credential.export.ts index 6ceb6850fe5..258699c8daf 100644 --- a/libs/common/src/models/export/fido2-credential.export.ts +++ b/libs/common/src/models/export/fido2-credential.export.ts @@ -31,7 +31,7 @@ export class Fido2CredentialExport { * Converts a Fido2CredentialExport object to its view representation. * @param req - The Fido2CredentialExport object to be converted. * @param view - (Optional) The Fido2CredentialView object to popualte with Fido2CredentialExport data - * @returns Fido2CredentialView + * @returns Fido2CredentialView - The populated view, or a new instance if none was provided. */ static toView(req: Fido2CredentialExport, view = new Fido2CredentialView()) { view.credentialId = req.credentialId; @@ -53,7 +53,7 @@ export class Fido2CredentialExport { * Converts a Fido2CredentialExport object to its domain representation. * @param req - The Fido2CredentialExport object to be converted. * @param domain - (Optional) The Fido2Credential object to popualte with Fido2CredentialExport data - * @returns Fido2Credential + * @returns Fido2Credential - The populated domain, or a new instance if none was provided. */ static toDomain(req: Fido2CredentialExport, domain = new Fido2Credential()) { domain.credentialId = req.credentialId != null ? new EncString(req.credentialId) : null; @@ -86,9 +86,9 @@ export class Fido2CredentialExport { creationDate: Date; /** - * Constructs a new Fid2CredentialExport instance - * @param o - The credential to populate the new instance. - * Optional and can be either Fido2CredentialView or Fido2Credential + * Constructs a new Fid2CredentialExport instance. + * + * @param o - The credential storing the data being exported. When not provided, an empty export is created instead. */ constructor(o?: Fido2CredentialView | Fido2Credential) { if (o == null) { diff --git a/libs/common/src/models/export/login.export.ts b/libs/common/src/models/export/login.export.ts index 6e14fbd809e..a5d9348c2ca 100644 --- a/libs/common/src/models/export/login.export.ts +++ b/libs/common/src/models/export/login.export.ts @@ -36,7 +36,7 @@ export class LoginExport { domain.username = req.username != null ? new EncString(req.username) : null; domain.password = req.password != null ? new EncString(req.password) : null; domain.totp = req.totp != null ? new EncString(req.totp) : null; - // Fido2credentials are currently not supported for imports. + // Fido2credentials are currently not supported for exports. return domain; } From ae1fe10d2df7b8b81909f121da699a12329ffb99 Mon Sep 17 00:00:00 2001 From: gbubemismith Date: Mon, 16 Oct 2023 20:48:34 -0400 Subject: [PATCH 9/9] added documentation to fido2 auth guard --- apps/browser/src/auth/guards/fido2-auth.guard.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/browser/src/auth/guards/fido2-auth.guard.ts b/apps/browser/src/auth/guards/fido2-auth.guard.ts index dfe70c6f0b3..7ff0060663d 100644 --- a/apps/browser/src/auth/guards/fido2-auth.guard.ts +++ b/apps/browser/src/auth/guards/fido2-auth.guard.ts @@ -11,6 +11,10 @@ import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authenticatio import { BrowserRouterService } from "../../platform/popup/services/browser-router.service"; +/** + * This guard verifies the user's authetication status. + * If "Locked", it saves the intended route in memory and redirects to the lock screen. Otherwise, the intended route is allowed. + */ export const fido2AuthGuard: CanActivateFn = async ( route: ActivatedRouteSnapshot, state: RouterStateSnapshot