From f0cdcccf8114799a2f3581e01f33e2395cff7ecc Mon Sep 17 00:00:00 2001 From: SmithThe4th Date: Tue, 12 Dec 2023 13:49:24 -0500 Subject: [PATCH 1/9] [PM-4012] Vault Timing out on Chrome and Edge breaks passkeys until page is reloaded (#6845) * changed content script injection strategy * added persistent connection and reinjection of the content script * cleanup resources on disconnect * cleanup resources on disconnect * concluded messanger event listeners cleanup and added unit tests * Switched to use browser api add listener instead of navtive apis * renamed cleanup to destroy and added reconnect and disconnect command functions * refactored to use foreach and check for only https urls * refactored the content script to only load the page script if it currently doesn't extist of the page, and if it does sends a reconnect command to the page-script to replace the native webauthn methods * updated unit test * removed memoized logic * moved the send disconect command to the messenger * updated unit test * test messenger handler * [PM-4012] fix: add `senderId` to messenger * destroy pending requets * cleaned up page script and terminated pending request * fixed cannot read properties of undefined * rearranged functions, renamed misspelled words, and created test * mocked EventTarget as there are issues on jest for listeners getting the events * Return fall back error instead * Update apps/browser/src/vault/fido2/content/content-script.ts Co-authored-by: Cesar Gonzalez * Update apps/browser/src/vault/fido2/content/messaging/messenger.ts Co-authored-by: Cesar Gonzalez * removed whitespace --------- Co-authored-by: Andreas Coroiu Co-authored-by: Cesar Gonzalez --- .../browser/src/background/main.background.ts | 7 + .../src/background/runtime.background.ts | 5 + apps/browser/src/manifest.json | 5 +- .../src/vault/fido2/content/content-script.ts | 40 ++++-- .../vault/fido2/content/messaging/message.ts | 12 ++ .../fido2/content/messaging/messenger.spec.ts | 99 +++++++++++-- .../fido2/content/messaging/messenger.ts | 135 ++++++++++++------ .../src/vault/fido2/content/page-script.ts | 50 ++++++- ...ger-fido2-content-script-injection.spec.ts | 16 +++ .../trigger-fido2-content-script-injection.ts | 3 + .../services/abstractions/fido2.service.ts | 4 + .../src/vault/services/fido2.service.spec.ts | 35 +++++ .../src/vault/services/fido2.service.ts | 33 +++++ apps/browser/webpack.config.js | 2 + 14 files changed, 378 insertions(+), 68 deletions(-) create mode 100644 apps/browser/src/vault/fido2/content/trigger-fido2-content-script-injection.spec.ts create mode 100644 apps/browser/src/vault/fido2/content/trigger-fido2-content-script-injection.ts create mode 100644 apps/browser/src/vault/services/abstractions/fido2.service.ts create mode 100644 apps/browser/src/vault/services/fido2.service.spec.ts create mode 100644 apps/browser/src/vault/services/fido2.service.ts diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index ec08fef4e22..2c1573d4e9b 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -156,7 +156,9 @@ import { BrowserSendService } from "../services/browser-send.service"; import { BrowserSettingsService } from "../services/browser-settings.service"; import VaultTimeoutService from "../services/vault-timeout/vault-timeout.service"; import { BrowserFido2UserInterfaceService } from "../vault/fido2/browser-fido2-user-interface.service"; +import { Fido2Service as Fido2ServiceAbstraction } from "../vault/services/abstractions/fido2.service"; import { BrowserFolderService } from "../vault/services/browser-folder.service"; +import Fido2Service from "../vault/services/fido2.service"; import { VaultFilterService } from "../vault/services/vault-filter.service"; import CommandsBackground from "./commands.background"; @@ -232,6 +234,7 @@ export default class MainBackground { authRequestCryptoService: AuthRequestCryptoServiceAbstraction; accountService: AccountServiceAbstraction; globalStateProvider: GlobalStateProvider; + fido2Service: Fido2ServiceAbstraction; // Passed to the popup for Safari to workaround issues with theming, downloading, etc. backgroundWindow = window; @@ -597,6 +600,7 @@ export default class MainBackground { this.messagingService, ); + this.fido2Service = new Fido2Service(); this.fido2UserInterfaceService = new BrowserFido2UserInterfaceService(this.authService); this.fido2AuthenticatorService = new Fido2AuthenticatorService( this.cipherService, @@ -645,6 +649,7 @@ export default class MainBackground { this.messagingService, this.logService, this.configService, + this.fido2Service, ); this.nativeMessagingBackground = new NativeMessagingBackground( this.cryptoService, @@ -778,6 +783,8 @@ export default class MainBackground { await this.idleBackground.init(); await this.webRequestBackground.init(); + await this.fido2Service.init(); + if (this.platformUtilsService.isFirefox() && !this.isPrivateMode) { // Set Private Mode windows to the default icon - they do not share state with the background page const privateWindows = await BrowserApi.getPrivateModeWindows(); diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index 434fd078347..fcaefc7c5e2 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -20,6 +20,7 @@ import { BrowserStateService } from "../platform/services/abstractions/browser-s import { BrowserEnvironmentService } from "../platform/services/browser-environment.service"; import BrowserPlatformUtilsService from "../platform/services/browser-platform-utils.service"; import { AbortManager } from "../vault/background/abort-manager"; +import { Fido2Service } from "../vault/services/abstractions/fido2.service"; import MainBackground from "./main.background"; @@ -42,6 +43,7 @@ export default class RuntimeBackground { private messagingService: MessagingService, private logService: LogService, private configService: ConfigServiceAbstraction, + private fido2Service: Fido2Service, ) { // onInstalled listener must be wired up before anything else, so we do it in the ctor chrome.runtime.onInstalled.addListener((details: any) => { @@ -257,6 +259,9 @@ export default class RuntimeBackground { case "getClickedElementResponse": this.platformUtilsService.copyToClipboard(msg.identifier, { window: window }); break; + case "triggerFido2ContentScriptInjection": + await this.fido2Service.injectFido2ContentScripts(sender); + break; case "fido2AbortRequest": this.abortManager.abort(msg.abortedRequestId); break; diff --git a/apps/browser/src/manifest.json b/apps/browser/src/manifest.json index 2bd032495c4..aa71d648e8e 100644 --- a/apps/browser/src/manifest.json +++ b/apps/browser/src/manifest.json @@ -17,7 +17,10 @@ "content_scripts": [ { "all_frames": true, - "js": ["content/trigger-autofill-script-injection.js", "content/fido2/content-script.js"], + "js": [ + "content/trigger-autofill-script-injection.js", + "content/fido2/trigger-fido2-content-script-injection.js" + ], "matches": ["http://*/*", "https://*/*", "file:///*"], "run_at": "document_start" }, diff --git a/apps/browser/src/vault/fido2/content/content-script.ts b/apps/browser/src/vault/fido2/content/content-script.ts index 3fe3e814481..54d18f2c8f4 100644 --- a/apps/browser/src/vault/fido2/content/content-script.ts +++ b/apps/browser/src/vault/fido2/content/content-script.ts @@ -61,12 +61,28 @@ async function isLocationBitwardenVault(activeUserSettings: Record) return window.location.origin === activeUserSettings.serverConfig.environment.vault; } -function initializeFido2ContentScript() { - const s = document.createElement("script"); - s.src = chrome.runtime.getURL("content/fido2/page-script.js"); - (document.head || document.documentElement).appendChild(s); +const messenger = Messenger.forDOMCommunication(window); - const messenger = Messenger.forDOMCommunication(window); +function injectPageScript() { + // Locate an existing page-script on the page + const existingPageScript = document.getElementById("bw-fido2-page-script"); + + // Inject the page-script if it doesn't exist + if (!existingPageScript) { + const s = document.createElement("script"); + s.src = chrome.runtime.getURL("content/fido2/page-script.js"); + s.id = "bw-fido2-page-script"; + (document.head || document.documentElement).appendChild(s); + + return; + } + + // If the page-script already exists, send a reconnect message to the page-script + messenger.sendReconnectCommand(); +} + +function initializeFido2ContentScript() { + injectPageScript(); messenger.handler = async (message, abortController) => { const requestId = Date.now().toString(); @@ -78,7 +94,7 @@ function initializeFido2ContentScript() { abortController.signal.addEventListener("abort", abortHandler); if (message.type === MessageType.CredentialCreationRequest) { - return new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { const data: CreateCredentialParams = { ...message.data, origin: window.location.origin, @@ -92,7 +108,7 @@ function initializeFido2ContentScript() { requestId: requestId, }, (response) => { - if (response.error !== undefined) { + if (response && response.error !== undefined) { return reject(response.error); } @@ -106,7 +122,7 @@ function initializeFido2ContentScript() { } if (message.type === MessageType.CredentialGetRequest) { - return new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { const data: AssertCredentialParams = { ...message.data, origin: window.location.origin, @@ -120,7 +136,7 @@ function initializeFido2ContentScript() { requestId: requestId, }, (response) => { - if (response.error !== undefined) { + if (response && response.error !== undefined) { return reject(response.error); } @@ -155,6 +171,12 @@ async function run() { } initializeFido2ContentScript(); + + const port = chrome.runtime.connect({ name: "fido2ContentScriptReady" }); + port.onDisconnect.addListener(() => { + // Cleanup the messenger and remove the event listener + messenger.destroy(); + }); } run(); diff --git a/apps/browser/src/vault/fido2/content/messaging/message.ts b/apps/browser/src/vault/fido2/content/messaging/message.ts index e516dd9b37a..b803b97f92e 100644 --- a/apps/browser/src/vault/fido2/content/messaging/message.ts +++ b/apps/browser/src/vault/fido2/content/messaging/message.ts @@ -11,6 +11,8 @@ export enum MessageType { CredentialGetRequest, CredentialGetResponse, AbortRequest, + DisconnectRequest, + ReconnectRequest, AbortResponse, ErrorResponse, } @@ -60,6 +62,14 @@ export type AbortRequest = { abortedRequestId: string; }; +export type DisconnectRequest = { + type: MessageType.DisconnectRequest; +}; + +export type ReconnectRequest = { + type: MessageType.ReconnectRequest; +}; + export type ErrorResponse = { type: MessageType.ErrorResponse; error: string; @@ -76,5 +86,7 @@ export type Message = | CredentialGetRequest | CredentialGetResponse | AbortRequest + | DisconnectRequest + | ReconnectRequest | AbortResponse | ErrorResponse; diff --git a/apps/browser/src/vault/fido2/content/messaging/messenger.spec.ts b/apps/browser/src/vault/fido2/content/messaging/messenger.spec.ts index 02e0f944974..4b108a5581c 100644 --- a/apps/browser/src/vault/fido2/content/messaging/messenger.spec.ts +++ b/apps/browser/src/vault/fido2/content/messaging/messenger.spec.ts @@ -12,6 +12,12 @@ describe("Messenger", () => { beforeEach(() => { // jest does not support MessageChannel window.MessageChannel = MockMessageChannel as any; + Object.defineProperty(window, "location", { + value: { + origin: "https://bitwarden.com", + }, + writable: true, + }); const channelPair = new TestChannelPair(); messengerA = new Messenger(channelPair.channelA); @@ -27,7 +33,7 @@ describe("Messenger", () => { const request = createRequest(); messengerA.request(request); - const received = handlerB.recieve(); + const received = handlerB.receive(); expect(received.length).toBe(1); expect(received[0].message).toMatchObject(request); @@ -37,7 +43,7 @@ describe("Messenger", () => { const request = createRequest(); const response = createResponse(); const requestPromise = messengerA.request(request); - const received = handlerB.recieve(); + const received = handlerB.receive(); received[0].respond(response); const returned = await requestPromise; @@ -49,7 +55,7 @@ describe("Messenger", () => { const request = createRequest(); const error = new Error("Test error"); const requestPromise = messengerA.request(request); - const received = handlerB.recieve(); + const received = handlerB.receive(); received[0].reject(error); @@ -61,10 +67,60 @@ describe("Messenger", () => { messengerA.request(createRequest(), abortController); abortController.abort(); - const received = handlerB.recieve(); + const received = handlerB.receive(); expect(received[0].abortController.signal.aborted).toBe(true); }); + + describe("destroy", () => { + beforeEach(() => { + /** + * In Jest's jsdom environment, there is an issue where event listeners are not + * triggered upon dispatching an event. This is a workaround to mock the EventTarget + */ + window.EventTarget = MockEventTarget as any; + }); + + it("should remove the message event listener", async () => { + const channelPair = new TestChannelPair(); + const addEventListenerSpy = jest.spyOn(channelPair.channelA, "addEventListener"); + const removeEventListenerSpy = jest.spyOn(channelPair.channelA, "removeEventListener"); + messengerA = new Messenger(channelPair.channelA); + jest + .spyOn(messengerA as any, "sendDisconnectCommand") + .mockImplementation(() => Promise.resolve()); + + expect(addEventListenerSpy).toHaveBeenCalled(); + + await messengerA.destroy(); + + expect(removeEventListenerSpy).toHaveBeenCalled(); + }); + + it("should dispatch the destroy event on messenger destruction", async () => { + const request = createRequest(); + messengerA.request(request); + + const dispatchEventSpy = jest.spyOn((messengerA as any).onDestroy, "dispatchEvent"); + messengerA.destroy(); + + expect(dispatchEventSpy).toHaveBeenCalledWith(expect.any(Event)); + }); + + it("should trigger onDestroyListener when the destroy event is dispatched", async () => { + const request = createRequest(); + messengerA.request(request); + + const onDestroyListener = jest.fn(); + (messengerA as any).onDestroy.addEventListener("destroy", onDestroyListener); + messengerA.destroy(); + + expect(onDestroyListener).toHaveBeenCalled(); + const eventArg = onDestroyListener.mock.calls[0][0]; + expect(eventArg).toBeInstanceOf(Event); + expect(eventArg.type).toBe("destroy"); + }); + }); }); type TestMessage = MessageWithMetadata & { testId: string }; @@ -86,11 +142,13 @@ class TestChannelPair { this.channelA = { addEventListener: (listener) => (broadcastChannel.port1.onmessage = listener), + removeEventListener: () => (broadcastChannel.port1.onmessage = null), postMessage: (message, port) => broadcastChannel.port1.postMessage(message, port), }; this.channelB = { addEventListener: (listener) => (broadcastChannel.port2.onmessage = listener), + removeEventListener: () => (broadcastChannel.port1.onmessage = null), postMessage: (message, port) => broadcastChannel.port2.postMessage(message, port), }; } @@ -102,7 +160,7 @@ class TestMessageHandler { abortController?: AbortController, ) => Promise; - private recievedMessages: { + private receivedMessages: { message: TestMessage; respond: (response: TestMessage) => void; reject: (error: Error) => void; @@ -112,7 +170,7 @@ class TestMessageHandler { constructor() { this.handler = (message, abortController) => new Promise((resolve, reject) => { - this.recievedMessages.push({ + this.receivedMessages.push({ message, abortController, respond: (response) => resolve(response), @@ -121,9 +179,9 @@ class TestMessageHandler { }); } - recieve() { - const received = this.recievedMessages; - this.recievedMessages = []; + receive() { + const received = this.receivedMessages; + this.receivedMessages = []; return received; } } @@ -144,7 +202,11 @@ class MockMessagePort { postMessage(message: T, port?: MessagePort) { this.remotePort.onmessage( - new MessageEvent("message", { data: message, ports: port ? [port] : [] }), + new MessageEvent("message", { + data: message, + ports: port ? [port] : [], + origin: "https://bitwarden.com", + }), ); } @@ -152,3 +214,20 @@ class MockMessagePort { // Do nothing } } + +class MockEventTarget { + listeners: Record = {}; + + addEventListener(type: string, callback: EventListener) { + this.listeners[type] = this.listeners[type] || []; + this.listeners[type].push(callback); + } + + dispatchEvent(event: Event) { + (this.listeners[event.type] || []).forEach((callback) => callback(event)); + } + + removeEventListener(type: string, callback: EventListener) { + this.listeners[type] = (this.listeners[type] || []).filter((listener) => listener !== callback); + } +} diff --git a/apps/browser/src/vault/fido2/content/messaging/messenger.ts b/apps/browser/src/vault/fido2/content/messaging/messenger.ts index b69f6ac076f..cc29282227f 100644 --- a/apps/browser/src/vault/fido2/content/messaging/messenger.ts +++ b/apps/browser/src/vault/fido2/content/messaging/messenger.ts @@ -1,3 +1,5 @@ +import { FallbackRequestedError } from "@bitwarden/common/vault/abstractions/fido2/fido2-client.service.abstraction"; + import { Message, MessageType } from "./message"; const SENDER = "bitwarden-webauthn"; @@ -6,15 +8,16 @@ type PostMessageFunction = (message: MessageWithMetadata, remotePort: MessagePor export type Channel = { addEventListener: (listener: (message: MessageEvent) => void) => void; + removeEventListener: (listener: (message: MessageEvent) => void) => void; postMessage: PostMessageFunction; }; -export type Metadata = { SENDER: typeof SENDER }; +export type Metadata = { SENDER: typeof SENDER; senderId: string }; export type MessageWithMetadata = Message & Metadata; type Handler = ( message: MessageWithMetadata, abortController?: AbortController, -) => Promise; +) => void | Promise; /** * A class that handles communication between the page and content script. It converts @@ -22,6 +25,9 @@ type Handler = ( * handling aborts and exceptions across separate execution contexts. */ export class Messenger { + private messageEventListener: (event: MessageEvent) => void | null = null; + private onDestroy = new EventTarget(); + /** * Creates a messenger that uses the browser's `window.postMessage` API to initiate * requests in the content script. Every request will then create it's own @@ -35,14 +41,8 @@ export class Messenger { return new Messenger({ postMessage: (message, port) => window.postMessage(message, windowOrigin, [port]), - addEventListener: (listener) => - window.addEventListener("message", (event: MessageEvent) => { - if (event.origin !== windowOrigin) { - return; - } - - listener(event as MessageEvent); - }), + addEventListener: (listener) => window.addEventListener("message", listener), + removeEventListener: (listener) => window.removeEventListener("message", listener), }); } @@ -53,38 +53,11 @@ export class Messenger { */ handler?: Handler; + private messengerId = this.generateUniqueId(); + constructor(private broadcastChannel: Channel) { - this.broadcastChannel.addEventListener(async (event) => { - if (this.handler === undefined) { - return; - } - - const message = event.data; - const port = event.ports?.[0]; - if (message?.SENDER !== SENDER || message == null || port == null) { - return; - } - - const abortController = new AbortController(); - port.onmessage = (event: MessageEvent) => { - if (event.data.type === MessageType.AbortRequest) { - abortController.abort(); - } - }; - - try { - const handlerResponse = await this.handler(message, abortController); - port.postMessage({ ...handlerResponse, SENDER }); - } catch (error) { - port.postMessage({ - SENDER, - type: MessageType.ErrorResponse, - error: JSON.stringify(error, Object.getOwnPropertyNames(error)), - }); - } finally { - port.close(); - } - }); + this.messageEventListener = this.createMessageEventListener(); + this.broadcastChannel.addEventListener(this.messageEventListener); } /** @@ -111,7 +84,10 @@ export class Messenger { }); abortController?.signal.addEventListener("abort", abortListener); - this.broadcastChannel.postMessage({ ...request, SENDER }, remotePort); + this.broadcastChannel.postMessage( + { ...request, SENDER, senderId: this.messengerId }, + remotePort, + ); const response = await promise; abortController?.signal.removeEventListener("abort", abortListener); @@ -127,4 +103,79 @@ export class Messenger { localPort.close(); } } + + private createMessageEventListener() { + return async (event: MessageEvent) => { + const windowOrigin = window.location.origin; + if (event.origin !== windowOrigin || !this.handler) { + return; + } + + const message = event.data; + const port = event.ports?.[0]; + if ( + message?.SENDER !== SENDER || + message.senderId == this.messengerId || + message == null || + port == null + ) { + return; + } + + const abortController = new AbortController(); + port.onmessage = (event: MessageEvent) => { + if (event.data.type === MessageType.AbortRequest) { + abortController.abort(); + } + }; + + let onDestroyListener; + const destroyPromise: Promise = new Promise((_, reject) => { + onDestroyListener = () => reject(new FallbackRequestedError()); + this.onDestroy.addEventListener("destroy", onDestroyListener); + }); + + try { + const handlerResponse = await Promise.race([ + this.handler(message, abortController), + destroyPromise, + ]); + port.postMessage({ ...handlerResponse, SENDER }); + } catch (error) { + port.postMessage({ + SENDER, + type: MessageType.ErrorResponse, + error: JSON.stringify(error, Object.getOwnPropertyNames(error)), + }); + } finally { + this.onDestroy.removeEventListener("destroy", onDestroyListener); + port.close(); + } + }; + } + + /** + * Cleans up the messenger by removing the message event listener + */ + async destroy() { + this.onDestroy.dispatchEvent(new Event("destroy")); + + if (this.messageEventListener) { + await this.sendDisconnectCommand(); + this.broadcastChannel.removeEventListener(this.messageEventListener); + this.messageEventListener = null; + } + } + + async sendReconnectCommand() { + await this.request({ type: MessageType.ReconnectRequest }); + } + + private async sendDisconnectCommand() { + await this.request({ type: MessageType.DisconnectRequest }); + } + + private generateUniqueId() { + return Date.now().toString(36) + Math.random().toString(36).substring(2); + } } diff --git a/apps/browser/src/vault/fido2/content/page-script.ts b/apps/browser/src/vault/fido2/content/page-script.ts index 9a3a74bed1b..b489fd0f3eb 100644 --- a/apps/browser/src/vault/fido2/content/page-script.ts +++ b/apps/browser/src/vault/fido2/content/page-script.ts @@ -53,10 +53,21 @@ const browserCredentials = { }; const messenger = ((window as any).messenger = Messenger.forDOMCommunication(window)); -navigator.credentials.create = async ( + +navigator.credentials.create = createWebAuthnCredential; +navigator.credentials.get = getWebAuthnCredential; + +/** + * Creates a new webauthn credential. + * + * @param options Options for creating new credentials. + * @param abortController Abort controller to abort the request if needed. + * @returns Promise that resolves to the new credential object. + */ +async function createWebAuthnCredential( options?: CredentialCreationOptions, abortController?: AbortController, -): Promise => { +): Promise { if (!isWebauthnCall(options)) { return await browserCredentials.create(options); } @@ -88,12 +99,19 @@ navigator.credentials.create = async ( throw error; } -}; +} -navigator.credentials.get = async ( +/** + * Retrieves a webauthn credential. + * + * @param options Options for creating new credentials. + * @param abortController Abort controller to abort the request if needed. + * @returns Promise that resolves to the new credential object. + */ +async function getWebAuthnCredential( options?: CredentialRequestOptions, abortController?: AbortController, -): Promise => { +): Promise { if (!isWebauthnCall(options)) { return await browserCredentials.get(options); } @@ -126,7 +144,7 @@ navigator.credentials.get = async ( throw error; } -}; +} function isWebauthnCall(options?: CredentialCreationOptions | CredentialRequestOptions) { return options && "publicKey" in options; @@ -174,3 +192,23 @@ async function waitForFocus(fallbackWait = 500, timeout = 5 * 60 * 1000) { window.clearTimeout(timeoutId); } } + +/** + * Sets up a listener to handle cleanup or reconnection when the extension's + * context changes due to being reloaded or unloaded. + */ +messenger.handler = (message, abortController) => { + const type = message.type; + + // Handle cleanup for disconnect request + if (type === MessageType.DisconnectRequest && browserNativeWebauthnSupport) { + navigator.credentials.create = browserCredentials.create; + navigator.credentials.get = browserCredentials.get; + } + + // Handle reinitialization for reconnect request + if (type === MessageType.ReconnectRequest && browserNativeWebauthnSupport) { + navigator.credentials.create = createWebAuthnCredential; + navigator.credentials.get = getWebAuthnCredential; + } +}; diff --git a/apps/browser/src/vault/fido2/content/trigger-fido2-content-script-injection.spec.ts b/apps/browser/src/vault/fido2/content/trigger-fido2-content-script-injection.spec.ts new file mode 100644 index 00000000000..8f4efe03306 --- /dev/null +++ b/apps/browser/src/vault/fido2/content/trigger-fido2-content-script-injection.spec.ts @@ -0,0 +1,16 @@ +describe("TriggerFido2ContentScriptInjection", () => { + afterEach(() => { + jest.resetModules(); + jest.clearAllMocks(); + }); + + describe("init", () => { + it("sends a message to the extension background", () => { + require("../content/trigger-fido2-content-script-injection"); + + expect(chrome.runtime.sendMessage).toHaveBeenCalledWith({ + command: "triggerFido2ContentScriptInjection", + }); + }); + }); +}); diff --git a/apps/browser/src/vault/fido2/content/trigger-fido2-content-script-injection.ts b/apps/browser/src/vault/fido2/content/trigger-fido2-content-script-injection.ts new file mode 100644 index 00000000000..5b902472818 --- /dev/null +++ b/apps/browser/src/vault/fido2/content/trigger-fido2-content-script-injection.ts @@ -0,0 +1,3 @@ +(function () { + chrome.runtime.sendMessage({ command: "triggerFido2ContentScriptInjection" }); +})(); diff --git a/apps/browser/src/vault/services/abstractions/fido2.service.ts b/apps/browser/src/vault/services/abstractions/fido2.service.ts new file mode 100644 index 00000000000..138b538b150 --- /dev/null +++ b/apps/browser/src/vault/services/abstractions/fido2.service.ts @@ -0,0 +1,4 @@ +export abstract class Fido2Service { + init: () => Promise; + injectFido2ContentScripts: (sender: chrome.runtime.MessageSender) => Promise; +} diff --git a/apps/browser/src/vault/services/fido2.service.spec.ts b/apps/browser/src/vault/services/fido2.service.spec.ts new file mode 100644 index 00000000000..1db2bdfb77d --- /dev/null +++ b/apps/browser/src/vault/services/fido2.service.spec.ts @@ -0,0 +1,35 @@ +import { BrowserApi } from "../../platform/browser/browser-api"; + +import Fido2Service from "./fido2.service"; + +describe("Fido2Service", () => { + let fido2Service: Fido2Service; + let tabMock: chrome.tabs.Tab; + let sender: chrome.runtime.MessageSender; + + beforeEach(() => { + fido2Service = new Fido2Service(); + tabMock = { id: 123, url: "https://bitwarden.com" } as chrome.tabs.Tab; + sender = { tab: tabMock }; + jest.spyOn(BrowserApi, "executeScriptInTab").mockImplementation(); + }); + + afterEach(() => { + jest.resetModules(); + jest.clearAllMocks(); + }); + + describe("injectFido2ContentScripts", () => { + const fido2ContentScript = "content/fido2/content-script.js"; + const defaultExecuteScriptOptions = { runAt: "document_start" }; + + it("accepts an extension message sender and injects the fido2 scripts into the tab of the sender", async () => { + await fido2Service.injectFido2ContentScripts(sender); + + expect(BrowserApi.executeScriptInTab).toHaveBeenCalledWith(tabMock.id, { + file: fido2ContentScript, + ...defaultExecuteScriptOptions, + }); + }); + }); +}); diff --git a/apps/browser/src/vault/services/fido2.service.ts b/apps/browser/src/vault/services/fido2.service.ts new file mode 100644 index 00000000000..a2b21f56514 --- /dev/null +++ b/apps/browser/src/vault/services/fido2.service.ts @@ -0,0 +1,33 @@ +import { BrowserApi } from "../../platform/browser/browser-api"; + +import { Fido2Service as Fido2ServiceInterface } from "./abstractions/fido2.service"; + +export default class Fido2Service implements Fido2ServiceInterface { + async init() { + const tabs = await BrowserApi.tabsQuery({}); + tabs.forEach((tab) => { + if (tab.url?.startsWith("https")) { + this.injectFido2ContentScripts({ tab } as chrome.runtime.MessageSender); + } + }); + + BrowserApi.addListener(chrome.runtime.onConnect, (port) => { + if (port.name === "fido2ContentScriptReady") { + port.postMessage({ command: "fido2ContentScriptInit" }); + } + }); + } + + /** + * Injects the FIDO2 content script into the current tab. + * @param {chrome.runtime.MessageSender} sender + * @returns {Promise} + */ + async injectFido2ContentScripts(sender: chrome.runtime.MessageSender): Promise { + await BrowserApi.executeScriptInTab(sender.tab.id, { + file: "content/fido2/content-script.js", + frameId: sender.frameId, + runAt: "document_start", + }); + } +} diff --git a/apps/browser/webpack.config.js b/apps/browser/webpack.config.js index 23a24b91e64..f5342f74925 100644 --- a/apps/browser/webpack.config.js +++ b/apps/browser/webpack.config.js @@ -171,6 +171,8 @@ const mainConfig = { "content/notificationBar": "./src/autofill/content/notification-bar.ts", "content/contextMenuHandler": "./src/autofill/content/context-menu-handler.ts", "content/message_handler": "./src/autofill/content/message_handler.ts", + "content/fido2/trigger-fido2-content-script-injection": + "./src/vault/fido2/content/trigger-fido2-content-script-injection.ts", "content/fido2/content-script": "./src/vault/fido2/content/content-script.ts", "content/fido2/page-script": "./src/vault/fido2/content/page-script.ts", "notification/bar": "./src/autofill/notification/bar.ts", From bb096724b24ee6e31fdd5b21555fd216450cc362 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 12 Dec 2023 10:59:03 -0800 Subject: [PATCH 2/9] update account switcher styling for all themes (#7182) --- .../account-switcher.component.html | 6 +++--- .../account-switching/account.component.html | 19 ++++++++++------- apps/browser/src/popup/scss/box.scss | 21 +++++++++++++++++++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/apps/browser/src/auth/popup/account-switching/account-switcher.component.html b/apps/browser/src/auth/popup/account-switching/account-switcher.component.html index f7421870143..364ee07fba2 100644 --- a/apps/browser/src/auth/popup/account-switching/account-switcher.component.html +++ b/apps/browser/src/auth/popup/account-switching/account-switcher.component.html @@ -40,7 +40,7 @@
diff --git a/apps/browser/src/popup/scss/box.scss b/apps/browser/src/popup/scss/box.scss index c026c780889..d98c29176da 100644 --- a/apps/browser/src/popup/scss/box.scss +++ b/apps/browser/src/popup/scss/box.scss @@ -767,3 +767,24 @@ form { } } } + +.account-switcher-row { + @include themify($themes) { + color: themed("textColor"); + background-color: themed("boxBackgroundColor"); + } + + &:hover, + &:focus, + &.active { + @include themify($themes) { + background-color: themed("listItemBackgroundHoverColor"); + } + } + + &-details { + @include themify($themes) { + color: themed("mutedColor"); + } + } +} From 9c1169d035db0c7afb7fe9be9e9d94cbb7efe594 Mon Sep 17 00:00:00 2001 From: Joseph Flinn <58369717+joseph-flinn@users.noreply.github.com> Date: Tue, 12 Dec 2023 11:57:50 -0800 Subject: [PATCH 3/9] Update master to main in workflows (#7151) --- .github/workflows/auto-branch-updater.yml | 2 +- .github/workflows/build-browser.yml | 8 ++++---- .github/workflows/build-cli.yml | 4 ++-- .github/workflows/build-desktop.yml | 10 +++++----- .github/workflows/build-web.yml | 12 ++++++------ .github/workflows/crowdin-pull.yml | 2 +- .github/workflows/deploy-eu-prod-web.yml | 4 ++-- .github/workflows/deploy-eu-qa-web.yml | 4 ++-- .github/workflows/label-issue-pull-request.yml | 4 ++-- .github/workflows/release-browser.yml | 4 ++-- .github/workflows/release-cli.yml | 8 ++++---- .github/workflows/release-desktop-beta.yml | 12 ++++++------ .github/workflows/release-desktop.yml | 6 +++--- .github/workflows/release-web.yml | 8 ++++---- .github/workflows/stale-bot.yml | 2 +- .github/workflows/version-bump.yml | 2 +- 16 files changed, 46 insertions(+), 46 deletions(-) diff --git a/.github/workflows/auto-branch-updater.yml b/.github/workflows/auto-branch-updater.yml index fee2ad958f2..8eb9ac19c8b 100644 --- a/.github/workflows/auto-branch-updater.yml +++ b/.github/workflows/auto-branch-updater.yml @@ -4,7 +4,7 @@ name: Auto Update Branch on: push: branches: - - 'master' + - 'main' - 'rc' paths: - 'apps/web/**' diff --git a/.github/workflows/build-browser.yml b/.github/workflows/build-browser.yml index f16021570a9..d5df9772e12 100644 --- a/.github/workflows/build-browser.yml +++ b/.github/workflows/build-browser.yml @@ -14,7 +14,7 @@ on: - '!*.txt' push: branches: - - 'master' + - 'main' - 'rc' - 'hotfix-rc-browser' paths: @@ -351,7 +351,7 @@ jobs: crowdin-push: name: Crowdin Push - if: github.ref == 'refs/heads/master' + if: github.ref == 'refs/heads/main' runs-on: ubuntu-22.04 needs: - build @@ -380,7 +380,7 @@ jobs: CROWDIN_PROJECT_ID: "268134" with: config: apps/browser/crowdin.yml - crowdin_branch_name: master + crowdin_branch_name: main upload_sources: true upload_translations: false @@ -397,7 +397,7 @@ jobs: - crowdin-push steps: - name: Check if any job failed - if: ${{ (github.ref == 'refs/heads/master') || (github.ref == 'refs/heads/rc') }} + if: ${{ (github.ref == 'refs/heads/main') || (github.ref == 'refs/heads/rc') }} env: CLOC_STATUS: ${{ needs.cloc.result }} SETUP_STATUS: ${{ needs.setup.result }} diff --git a/.github/workflows/build-cli.yml b/.github/workflows/build-cli.yml index e5e2aeaa88f..d6f089501a1 100644 --- a/.github/workflows/build-cli.yml +++ b/.github/workflows/build-cli.yml @@ -15,7 +15,7 @@ on: - '.github/workflows/build-cli.yml' push: branches: - - 'master' + - 'main' - 'rc' - 'hotfix-rc-cli' paths: @@ -379,7 +379,7 @@ jobs: steps: - name: Check if any job failed working-directory: ${{ github.workspace }} - if: ${{ (github.ref == 'refs/heads/master') || (github.ref == 'refs/heads/rc') }} + if: ${{ (github.ref == 'refs/heads/main') || (github.ref == 'refs/heads/rc') }} env: CLOC_STATUS: ${{ needs.cloc.result }} SETUP_STATUS: ${{ needs.setup.result }} diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index b75e8457ea9..87c3a664786 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -15,7 +15,7 @@ on: - '.github/workflows/build-desktop.yml' push: branches: - - 'master' + - 'main' - 'rc' - 'hotfix-rc-desktop' paths: @@ -973,7 +973,7 @@ jobs: APPLE_ID_USERNAME: ${{ secrets.APPLE_ID_USERNAME }} APPLE_ID_PASSWORD: ${{ secrets.APPLE_ID_PASSWORD }} if: | - (github.ref == 'refs/heads/master' + (github.ref == 'refs/heads/main' && needs.setup.outputs.rc_branch_exists == 0 && needs.setup.outputs.hotfix_branch_exists == 0) || (github.ref == 'refs/heads/rc' && needs.setup.outputs.hotfix_branch_exists == 0) @@ -1154,7 +1154,7 @@ jobs: crowdin-push: name: Crowdin Push - if: github.ref == 'refs/heads/master' + if: github.ref == 'refs/heads/main' needs: - linux - windows @@ -1185,7 +1185,7 @@ jobs: CROWDIN_PROJECT_ID: "299360" with: config: apps/desktop/crowdin.yml - crowdin_branch_name: master + crowdin_branch_name: main upload_sources: true upload_translations: false @@ -1207,7 +1207,7 @@ jobs: - crowdin-push steps: - name: Check if any job failed - if: ${{ (github.ref == 'refs/heads/master') || (github.ref == 'refs/heads/rc') }} + if: ${{ (github.ref == 'refs/heads/main') || (github.ref == 'refs/heads/rc') }} env: CLOC_STATUS: ${{ needs.cloc.result }} ELECTRON_VERIFY_STATUS: ${{ needs.electron-verify.result }} diff --git a/.github/workflows/build-web.yml b/.github/workflows/build-web.yml index b6681c0dac8..154e2574f48 100644 --- a/.github/workflows/build-web.yml +++ b/.github/workflows/build-web.yml @@ -15,7 +15,7 @@ on: - '.github/workflows/build-web.yml' push: branches: - - 'master' + - 'main' - 'rc' - 'hotfix-rc-web' paths: @@ -170,7 +170,7 @@ jobs: - name: Check Branch to Publish env: - PUBLISH_BRANCHES: "master,rc,hotfix-rc-web" + PUBLISH_BRANCHES: "main,rc,hotfix-rc-web" id: publish-branch-check run: | IFS="," read -a publish_branches <<< $PUBLISH_BRANCHES @@ -218,7 +218,7 @@ jobs: IMAGE_TAG=$(echo "${GITHUB_REF:11}" | sed "s#/#-#g") fi - if [[ "$IMAGE_TAG" == "master" ]]; then + if [[ "$IMAGE_TAG" == "main" ]]; then IMAGE_TAG=dev fi @@ -259,7 +259,7 @@ jobs: crowdin-push: name: Crowdin Push - if: github.ref == 'refs/heads/master' + if: github.ref == 'refs/heads/main' needs: build-artifacts runs-on: ubuntu-22.04 steps: @@ -286,7 +286,7 @@ jobs: CROWDIN_PROJECT_ID: "308189" with: config: apps/web/crowdin.yml - crowdin_branch_name: master + crowdin_branch_name: main upload_sources: true upload_translations: false @@ -303,7 +303,7 @@ jobs: - crowdin-push steps: - name: Check if any job failed - if: ${{ (github.ref == 'refs/heads/master') || (github.ref == 'refs/heads/rc') }} + if: ${{ (github.ref == 'refs/heads/main') || (github.ref == 'refs/heads/rc') }} env: CLOC_STATUS: ${{ needs.cloc.result }} SETUP_STATUS: ${{ needs.setup.result }} diff --git a/.github/workflows/crowdin-pull.yml b/.github/workflows/crowdin-pull.yml index 5ffedc267fe..3600f048c23 100644 --- a/.github/workflows/crowdin-pull.yml +++ b/.github/workflows/crowdin-pull.yml @@ -45,7 +45,7 @@ jobs: CROWDIN_PROJECT_ID: ${{ matrix.crowdin_project_id }} with: config: crowdin.yml - crowdin_branch_name: master + crowdin_branch_name: main upload_sources: false upload_translations: false download_translations: true diff --git a/.github/workflows/deploy-eu-prod-web.yml b/.github/workflows/deploy-eu-prod-web.yml index 2c7835caef8..5eb8ace1edf 100644 --- a/.github/workflows/deploy-eu-prod-web.yml +++ b/.github/workflows/deploy-eu-prod-web.yml @@ -5,10 +5,10 @@ on: workflow_dispatch: inputs: tag: - description: "Branch name to deploy (examples: 'master', 'feature/sm')" + description: "Branch name to deploy (examples: 'main', 'feature/sm')" required: true type: string - default: master + default: main jobs: azure-deploy: diff --git a/.github/workflows/deploy-eu-qa-web.yml b/.github/workflows/deploy-eu-qa-web.yml index 9fa50f3ba3c..e0291063eed 100644 --- a/.github/workflows/deploy-eu-qa-web.yml +++ b/.github/workflows/deploy-eu-qa-web.yml @@ -5,10 +5,10 @@ on: workflow_dispatch: inputs: tag: - description: "Branch name to deploy (examples: 'master', 'feature/sm')" + description: "Branch name to deploy (examples: 'main', 'feature/sm')" required: true type: string - default: master + default: main jobs: notify-start: diff --git a/.github/workflows/label-issue-pull-request.yml b/.github/workflows/label-issue-pull-request.yml index d0aad2f2b25..e52bba36d63 100644 --- a/.github/workflows/label-issue-pull-request.yml +++ b/.github/workflows/label-issue-pull-request.yml @@ -1,5 +1,5 @@ # Runs creation of Pull Requests -# If the PR destination branch is master, add a needs-qa label unless created by renovate[bot] +# If the PR destination branch is main, add a needs-qa label unless created by renovate[bot] --- name: Label Issue Pull Request @@ -10,7 +10,7 @@ on: paths-ignore: - .github/workflows/** # We don't need QA on workflow changes branches: - - 'master' # We only want to check when PRs target master + - 'main' # We only want to check when PRs target main jobs: add-needs-qa-label: diff --git a/.github/workflows/release-browser.yml b/.github/workflows/release-browser.yml index dd17bcbcec6..ad6a8fa782a 100644 --- a/.github/workflows/release-browser.yml +++ b/.github/workflows/release-browser.yml @@ -114,13 +114,13 @@ jobs: dist-firefox-*.zip, dist-edge-*.zip' - - name: Dry Run - Download latest master build artifacts + - name: Dry Run - Download latest build artifacts if: ${{ github.event.inputs.release_type == 'Dry Run' }} uses: bitwarden/gh-actions/download-artifacts@main with: workflow: build-browser.yml workflow_conclusion: success - branch: master + branch: main artifacts: 'browser-source-*.zip, dist-chrome-*.zip, dist-opera-*.zip, diff --git a/.github/workflows/release-cli.yml b/.github/workflows/release-cli.yml index 194b802961f..6c4092fd303 100644 --- a/.github/workflows/release-cli.yml +++ b/.github/workflows/release-cli.yml @@ -92,7 +92,7 @@ jobs: workflow: build-cli.yml path: apps/cli workflow_conclusion: success - branch: master + branch: main - name: Create release if: ${{ github.event.inputs.release_type != 'Dry Run' }} @@ -175,7 +175,7 @@ jobs: workflow: build-cli.yml path: apps/cli workflow_conclusion: success - branch: master + branch: main artifacts: bw_${{ env._PKG_VERSION }}_amd64.snap - name: Publish Snap & logout @@ -235,7 +235,7 @@ jobs: workflow: build-cli.yml path: apps/cli/dist workflow_conclusion: success - branch: master + branch: main artifacts: bitwarden-cli.${{ env._PKG_VERSION }}.nupkg - name: Push to Chocolatey @@ -285,7 +285,7 @@ jobs: workflow: build-cli.yml path: apps/cli/build workflow_conclusion: success - branch: master + branch: main artifacts: bitwarden-cli-${{ env._PKG_VERSION }}-npm-build.zip - name: Setup NPM diff --git a/.github/workflows/release-desktop-beta.yml b/.github/workflows/release-desktop-beta.yml index fb9c710a5f8..8330e71c72f 100644 --- a/.github/workflows/release-desktop-beta.yml +++ b/.github/workflows/release-desktop-beta.yml @@ -28,9 +28,9 @@ jobs: - name: Branch check run: | - if [[ "$GITHUB_REF" != "refs/heads/master" ]] && [[ "$GITHUB_REF" != "refs/heads/rc" ]] && [[ "$GITHUB_REF" != "refs/heads/hotfix-rc" ]]; then + if [[ "$GITHUB_REF" != "refs/heads/main" ]] && [[ "$GITHUB_REF" != "refs/heads/rc" ]] && [[ "$GITHUB_REF" != "refs/heads/hotfix-rc" ]]; then echo "===================================" - echo "[!] Can only release from the 'master', 'rc' or 'hotfix-rc' branches" + echo "[!] Can only release from the 'main', 'rc' or 'hotfix-rc' branches" echo "===================================" exit 1 fi @@ -661,13 +661,13 @@ jobs: branch: rc path: ${{ github.workspace }}/browser-build-artifacts - - name: Download artifact from master + - name: Download artifacts from main if: ${{ github.ref != 'refs/heads/rc' && github.ref != 'refs/heads/hotfix-rc' }} uses: dawidd6/action-download-artifact@246dbf436b23d7c49e21a7ab8204ca9ecd1fe615 # v2.27.0 with: workflow: build-browser.yml workflow_conclusion: success - branch: master + branch: main path: ${{ github.workspace }}/browser-build-artifacts - name: Unzip Safari artifact @@ -859,13 +859,13 @@ jobs: branch: rc path: ${{ github.workspace }}/browser-build-artifacts - - name: Download artifact from master + - name: Download artifact from main if: ${{ github.ref != 'refs/heads/rc' && github.ref != 'refs/heads/hotfix-rc' }} uses: dawidd6/action-download-artifact@246dbf436b23d7c49e21a7ab8204ca9ecd1fe615 # v2.27.0 with: workflow: build-browser.yml workflow_conclusion: success - branch: master + branch: main path: ${{ github.workspace }}/browser-build-artifacts - name: Unzip Safari artifact diff --git a/.github/workflows/release-desktop.yml b/.github/workflows/release-desktop.yml index 31008a67817..4ffdc92fc26 100644 --- a/.github/workflows/release-desktop.yml +++ b/.github/workflows/release-desktop.yml @@ -136,7 +136,7 @@ jobs: with: workflow: build-desktop.yml workflow_conclusion: success - branch: master + branch: main path: apps/desktop/artifacts - name: Rename .pkg to .pkg.archive @@ -291,7 +291,7 @@ jobs: with: workflow: build-desktop.yml workflow_conclusion: success - branch: master + branch: main artifacts: bitwarden_${{ env._PKG_VERSION }}_amd64.snap path: apps/desktop/dist @@ -359,7 +359,7 @@ jobs: with: workflow: build-desktop.yml workflow_conclusion: success - branch: master + branch: main artifacts: bitwarden.${{ env._PKG_VERSION }}.nupkg path: apps/desktop/dist diff --git a/.github/workflows/release-web.yml b/.github/workflows/release-web.yml index c946f1294ac..a084ec1f02d 100644 --- a/.github/workflows/release-web.yml +++ b/.github/workflows/release-web.yml @@ -159,7 +159,7 @@ jobs: workflow: build-web.yml path: assets workflow_conclusion: success - branch: master + branch: main artifacts: web-*-cloud-COMMERCIAL.zip - name: Unzip build asset @@ -196,12 +196,12 @@ jobs: gh pr create --title "Deploy v${_RELEASE_VERSION} to GitHub Pages" \ --draft \ --body "Deploying v${_RELEASE_VERSION}" \ - --base master \ + --base main \ --head "${_BRANCH}" else gh pr create --title "Deploy v${_RELEASE_VERSION} to GitHub Pages" \ --body "Deploying v${_RELEASE_VERSION}" \ - --base master \ + --base main \ --head "${_BRANCH}" fi @@ -243,7 +243,7 @@ jobs: workflow: build-web.yml path: apps/web/artifacts workflow_conclusion: success - branch: master + branch: main artifacts: "web-*-selfhosted-COMMERCIAL.zip, web-*-selfhosted-open-source.zip" diff --git a/.github/workflows/stale-bot.yml b/.github/workflows/stale-bot.yml index 98f3b9d1722..1bd058b94b9 100644 --- a/.github/workflows/stale-bot.yml +++ b/.github/workflows/stale-bot.yml @@ -27,4 +27,4 @@ jobs: If you’re still working on this, please respond here after you’ve made the changes we’ve requested and our team will re-open it for further review. - Please make sure to resolve any conflicts with the master branch before requesting another review. + Please make sure to resolve any conflicts with the main branch before requesting another review. diff --git a/.github/workflows/version-bump.yml b/.github/workflows/version-bump.yml index 9a78f995e83..8e126e1da60 100644 --- a/.github/workflows/version-bump.yml +++ b/.github/workflows/version-bump.yml @@ -286,7 +286,7 @@ jobs: TITLE: "Bump ${{ steps.create-branch.outputs.client }} version to ${{ inputs.version_number }}" run: | PR_URL=$(gh pr create --title "$TITLE" \ - --base "master" \ + --base "main" \ --head "$PR_BRANCH" \ --label "version update" \ --label "automated pr" \ From 74fb4bce3477e63f3e5507c86a5cb4f7aa162aa1 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 12 Dec 2023 14:40:14 -0600 Subject: [PATCH 4/9] [deps] AC: Update core-js to v3.34.0 (#7164) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index ce82ae54db9..a5c0e66eb6d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -37,7 +37,7 @@ "bufferutil": "4.0.8", "chalk": "4.1.2", "commander": "7.2.0", - "core-js": "3.32.0", + "core-js": "3.34.0", "duo_web_sdk": "github:duosecurity/duo_web_sdk", "form-data": "4.0.0", "https-proxy-agent": "5.0.1", @@ -18880,9 +18880,9 @@ } }, "node_modules/core-js": { - "version": "3.32.0", - "resolved": "https://registry.npmjs.org/core-js/-/core-js-3.32.0.tgz", - "integrity": "sha512-rd4rYZNlF3WuoYuRIDEmbR/ga9CeuWX9U05umAvgrrZoHY4Z++cp/xwPQMvUpBB4Ag6J8KfD80G0zwCyaSxDww==", + "version": "3.34.0", + "resolved": "https://registry.npmjs.org/core-js/-/core-js-3.34.0.tgz", + "integrity": "sha512-aDdvlDder8QmY91H88GzNi9EtQi2TjvQhpCX6B1v/dAZHU1AuLgHvRh54RiOerpEhEW46Tkf+vgAViB/CWC0ag==", "hasInstallScript": true, "funding": { "type": "opencollective", diff --git a/package.json b/package.json index 42a7c06f2d0..448024f5a0b 100644 --- a/package.json +++ b/package.json @@ -169,7 +169,7 @@ "bufferutil": "4.0.8", "chalk": "4.1.2", "commander": "7.2.0", - "core-js": "3.32.0", + "core-js": "3.34.0", "duo_web_sdk": "github:duosecurity/duo_web_sdk", "form-data": "4.0.0", "https-proxy-agent": "5.0.1", From 2934ca6b7a4f7f4e797701bf7369396467219704 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 12 Dec 2023 14:49:35 -0600 Subject: [PATCH 5/9] [deps] AC: Update url to v0.11.3 (#7163) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package-lock.json | 10 +++++----- package.json | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index a5c0e66eb6d..5dc37bc9cbd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -175,7 +175,7 @@ "tsconfig-paths-webpack-plugin": "4.1.0", "type-fest": "2.19.0", "typescript": "4.9.5", - "url": "0.11.1", + "url": "0.11.3", "util": "0.12.5", "wait-on": "7.2.0", "webpack": "5.89.0", @@ -39535,13 +39535,13 @@ "dev": true }, "node_modules/url": { - "version": "0.11.1", - "resolved": "https://registry.npmjs.org/url/-/url-0.11.1.tgz", - "integrity": "sha512-rWS3H04/+mzzJkv0eZ7vEDGiQbgquI1fGfOad6zKvgYQi1SzMmhl7c/DdRGxhaWrVH6z0qWITo8rpnxK/RfEhA==", + "version": "0.11.3", + "resolved": "https://registry.npmjs.org/url/-/url-0.11.3.tgz", + "integrity": "sha512-6hxOLGfZASQK/cijlZnZJTq8OXAkt/3YGfQX45vvMYXpZoo8NdWZcY73K108Jf759lS1Bv/8wXnHDTSz17dSRw==", "dev": true, "dependencies": { "punycode": "^1.4.1", - "qs": "^6.11.0" + "qs": "^6.11.2" } }, "node_modules/url-parse": { diff --git a/package.json b/package.json index 448024f5a0b..237e081afe1 100644 --- a/package.json +++ b/package.json @@ -138,7 +138,7 @@ "tsconfig-paths-webpack-plugin": "4.1.0", "type-fest": "2.19.0", "typescript": "4.9.5", - "url": "0.11.1", + "url": "0.11.3", "util": "0.12.5", "wait-on": "7.2.0", "webpack": "5.89.0", From 8c17f3ff23eb9cd61783c00178cdcfdc5ee0fd6c Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 12 Dec 2023 14:58:19 -0600 Subject: [PATCH 6/9] [deps] AC: Update postcss to v8.4.32 (#7162) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package-lock.json | 56 ++++++++++++++++++++++++++++++++++++++++++----- package.json | 2 +- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5dc37bc9cbd..91509883339 100644 --- a/package-lock.json +++ b/package-lock.json @@ -155,7 +155,7 @@ "mini-css-extract-plugin": "2.7.6", "node-ipc": "9.2.1", "pkg": "5.8.1", - "postcss": "8.4.31", + "postcss": "8.4.32", "postcss-loader": "7.3.3", "prettier": "3.1.1", "prettier-plugin-tailwindcss": "0.5.9", @@ -893,6 +893,34 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/@angular-devkit/build-angular/node_modules/postcss": { + "version": "8.4.31", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.4.31.tgz", + "integrity": "sha512-PS08Iboia9mts/2ygV3eLpY5ghnUcfLV/EXTOW1E2qYxJKGGBUtNjN76FYHnMs36RmARn41bC0AZmn+rR0OVpQ==", + "dev": true, + "funding": [ + { + "type": "opencollective", + "url": "https://opencollective.com/postcss/" + }, + { + "type": "tidelift", + "url": "https://tidelift.com/funding/github/npm/postcss" + }, + { + "type": "github", + "url": "https://github.com/sponsors/ai" + } + ], + "dependencies": { + "nanoid": "^3.3.6", + "picocolors": "^1.0.0", + "source-map-js": "^1.0.2" + }, + "engines": { + "node": "^10 || ^12 || >=14" + } + }, "node_modules/@angular-devkit/build-angular/node_modules/postcss-loader": { "version": "7.0.2", "resolved": "https://registry.npmjs.org/postcss-loader/-/postcss-loader-7.0.2.tgz", @@ -33230,9 +33258,9 @@ } }, "node_modules/postcss": { - "version": "8.4.31", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.4.31.tgz", - "integrity": "sha512-PS08Iboia9mts/2ygV3eLpY5ghnUcfLV/EXTOW1E2qYxJKGGBUtNjN76FYHnMs36RmARn41bC0AZmn+rR0OVpQ==", + "version": "8.4.32", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.4.32.tgz", + "integrity": "sha512-D/kj5JNu6oo2EIy+XL/26JEDTlIbB8hw85G8StOE6L74RQAVVP5rej6wxCNqyMbR4RkPfqvezVbPw81Ngd6Kcw==", "dev": true, "funding": [ { @@ -33249,7 +33277,7 @@ } ], "dependencies": { - "nanoid": "^3.3.6", + "nanoid": "^3.3.7", "picocolors": "^1.0.0", "source-map-js": "^1.0.2" }, @@ -33486,6 +33514,24 @@ "integrity": "sha512-1NNCs6uurfkVbeXG4S8JFT9t19m45ICnif8zWLd5oPSZ50QnwMfK+H3jv408d4jw/7Bttv5axS5IiHoLaVNHeQ==", "dev": true }, + "node_modules/postcss/node_modules/nanoid": { + "version": "3.3.7", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.7.tgz", + "integrity": "sha512-eSRppjcPIatRIMC1U6UngP8XFcz8MQWGQdt1MTBQ7NaAmvXDfvNxbvWV3x2y6CdEUciCSsDHDQZbhYaB8QEo2g==", + "dev": true, + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/ai" + } + ], + "bin": { + "nanoid": "bin/nanoid.cjs" + }, + "engines": { + "node": "^10 || ^12 || ^13.7 || ^14 || >=15.0.1" + } + }, "node_modules/prebuild-install": { "version": "7.1.1", "resolved": "https://registry.npmjs.org/prebuild-install/-/prebuild-install-7.1.1.tgz", diff --git a/package.json b/package.json index 237e081afe1..30c50e8f499 100644 --- a/package.json +++ b/package.json @@ -118,7 +118,7 @@ "mini-css-extract-plugin": "2.7.6", "node-ipc": "9.2.1", "pkg": "5.8.1", - "postcss": "8.4.31", + "postcss": "8.4.32", "postcss-loader": "7.3.3", "prettier": "3.1.1", "prettier-plugin-tailwindcss": "0.5.9", From 270af43d52cd2dd1197fd26e77f56e310cf8f71b Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 12 Dec 2023 15:06:44 -0600 Subject: [PATCH 7/9] [deps] AC: Update sass to v1.69.5 (#7165) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 91509883339..1fdd961d5d0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -165,7 +165,7 @@ "regedit": "^3.0.3", "remark-gfm": "3.0.1", "rimraf": "5.0.5", - "sass": "1.65.1", + "sass": "1.69.5", "sass-loader": "13.3.2", "storybook": "7.3.0", "style-loader": "3.3.3", @@ -35690,9 +35690,9 @@ } }, "node_modules/sass": { - "version": "1.65.1", - "resolved": "https://registry.npmjs.org/sass/-/sass-1.65.1.tgz", - "integrity": "sha512-9DINwtHmA41SEd36eVPQ9BJKpn7eKDQmUHmpI0y5Zv2Rcorrh0zS+cFrt050hdNbmmCNKTW3hV5mWfuegNRsEA==", + "version": "1.69.5", + "resolved": "https://registry.npmjs.org/sass/-/sass-1.69.5.tgz", + "integrity": "sha512-qg2+UCJibLr2LCVOt3OlPhr/dqVHWOa9XtZf2OjbLs/T4VPSJ00udtgJxH3neXZm+QqX8B+3cU7RaLqp1iVfcQ==", "dev": true, "dependencies": { "chokidar": ">=3.0.0 <4.0.0", diff --git a/package.json b/package.json index 30c50e8f499..586612bcb9f 100644 --- a/package.json +++ b/package.json @@ -128,7 +128,7 @@ "regedit": "^3.0.3", "remark-gfm": "3.0.1", "rimraf": "5.0.5", - "sass": "1.65.1", + "sass": "1.69.5", "sass-loader": "13.3.2", "storybook": "7.3.0", "style-loader": "3.3.3", From bfa76885ac0de0b1c6b9baba9c7acc52254ec6f3 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Tue, 12 Dec 2023 18:14:34 -0500 Subject: [PATCH 8/9] [PM-4107] Only call config on successful sync (#7149) --- apps/desktop/src/app/app.component.ts | 6 ++++-- apps/web/src/app/app.component.ts | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 8e5feac57c3..39a5c185fde 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -232,8 +232,10 @@ export class AppComponent implements OnInit, OnDestroy { case "syncStarted": break; case "syncCompleted": - await this.updateAppMenu(); - this.configService.triggerServerConfigFetch(); + if (message.successfully) { + this.updateAppMenu(); + this.configService.triggerServerConfigFetch(); + } break; case "openSettings": await this.openModal(SettingsComponent, this.settingsRef); diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 4109a8849fa..1f7d8cc2f1c 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -134,7 +134,9 @@ export class AppComponent implements OnDestroy, OnInit { case "syncStarted": break; case "syncCompleted": - this.configService.triggerServerConfigFetch(); + if (message.successfully) { + this.configService.triggerServerConfigFetch(); + } break; case "upgradeOrganization": { const upgradeConfirmed = await this.dialogService.openSimpleDialog({ From df406a9862b09e344a88b63ccc35c63560b8a4f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=9C=A8=20Audrey=20=E2=9C=A8?= Date: Tue, 12 Dec 2023 19:17:20 -0500 Subject: [PATCH 9/9] [PM-252] fix inconsistent generator configuration behavior (#6755) * decompose password generator policy enforcement * integrate new logic with UI * improve UX of minimum password length * improve password generator policy options documentation * initialize min length to default minimum length boundary * reset form value on input to prevent UI desync from model --------- Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> --- apps/browser/src/_locales/en/messages.json | 3 + apps/browser/src/popup/scss/box.scss | 3 +- .../popup/generator/generator.component.html | 28 +- .../src/app/tools/generator.component.html | 24 +- apps/desktop/src/locales/en/messages.json | 3 + apps/desktop/src/scss/box.scss | 3 +- .../src/app/tools/generator.component.html | 31 +- apps/web/src/locales/en/messages.json | 3 + .../components/generator.component.ts | 54 +- .../password-generator-policy-options.ts | 65 +- ...phrase-generator-options-evaluator.spec.ts | 220 ++++++ .../passphrase-generator-options-evaluator.ts | 105 +++ .../password/password-generation.service.ts | 209 +----- ...ssword-generator-options-evaluator.spec.ts | 703 ++++++++++++++++++ .../password-generator-options-evaluator.ts | 179 +++++ .../password/password-generator-options.ts | 100 ++- 16 files changed, 1532 insertions(+), 201 deletions(-) create mode 100644 libs/common/src/tools/generator/password/passphrase-generator-options-evaluator.spec.ts create mode 100644 libs/common/src/tools/generator/password/passphrase-generator-options-evaluator.ts create mode 100644 libs/common/src/tools/generator/password/password-generator-options-evaluator.spec.ts create mode 100644 libs/common/src/tools/generator/password/password-generator-options-evaluator.ts diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index a6aff9e455b..39b772221a4 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -268,6 +268,9 @@ "length": { "message": "Length" }, + "passwordMinLength": { + "message": "Minimum password length" + }, "uppercase": { "message": "Uppercase (A-Z)" }, diff --git a/apps/browser/src/popup/scss/box.scss b/apps/browser/src/popup/scss/box.scss index d98c29176da..179673279e6 100644 --- a/apps/browser/src/popup/scss/box.scss +++ b/apps/browser/src/popup/scss/box.scss @@ -341,7 +341,8 @@ } } - .img-right { + .img-right, + .txt-right { float: right; margin-left: 10px; } diff --git a/apps/browser/src/tools/popup/generator/generator.component.html b/apps/browser/src/tools/popup/generator/generator.component.html index 5f722b661ce..3e6dfe0747f 100644 --- a/apps/browser/src/tools/popup/generator/generator.component.html +++ b/apps/browser/src/tools/popup/generator/generator.component.html @@ -176,7 +176,7 @@
+
+ {{ "passwordMinLength" | i18n }} + + {{ passwordOptionsMinLengthForReader$ | async }} + + {{ passwordOptions.minLength }} +
@@ -232,10 +244,10 @@
@@ -249,8 +261,8 @@ type="number" min="0" max="9" - (change)="savePasswordOptions()" [(ngModel)]="passwordOptions.minNumber" + (input)="onPasswordOptionsMinNumberInput($event)" />
@@ -260,8 +272,8 @@ type="number" min="0" max="9" - (change)="savePasswordOptions()" [(ngModel)]="passwordOptions.minSpecial" + (input)="onPasswordOptionsMinSpecialInput($event)" />
diff --git a/apps/desktop/src/app/tools/generator.component.html b/apps/desktop/src/app/tools/generator.component.html index 0c66ebde805..aa77aafc146 100644 --- a/apps/desktop/src/app/tools/generator.component.html +++ b/apps/desktop/src/app/tools/generator.component.html @@ -200,7 +200,7 @@
+
+ {{ "passwordMinLength" | i18n }} + {{ passwordOptions.minLength }} + + {{ passwordOptionsMinLengthForReader$ | async }} + +
@@ -258,7 +271,8 @@ type="checkbox" (change)="savePasswordOptions()" [disabled]="enforcedPasswordPolicyOptions?.useSpecial" - [(ngModel)]="passwordOptions.special" + [ngModel]="passwordOptions.special" + (ngModelChange)="setPasswordOptionsSpecial($event)" attr.aria-label="{{ 'specialCharacters' | i18n }}" /> @@ -275,6 +289,7 @@ max="9" (change)="savePasswordOptions()" [(ngModel)]="passwordOptions.minNumber" + (input)="onPasswordOptionsMinNumberInput($event)" />
@@ -286,6 +301,7 @@ max="9" (change)="savePasswordOptions()" [(ngModel)]="passwordOptions.minSpecial" + (input)="onPasswordOptionsMinSpecialInput($event)" />
diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index 28051428e7d..ca7fdf4d603 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -403,6 +403,9 @@ "length": { "message": "Length" }, + "passwordMinLength": { + "message": "Minimum password length" + }, "uppercase": { "message": "Uppercase (A-Z)" }, diff --git a/apps/desktop/src/scss/box.scss b/apps/desktop/src/scss/box.scss index 9466c89a7ff..0e89e9fd74e 100644 --- a/apps/desktop/src/scss/box.scss +++ b/apps/desktop/src/scss/box.scss @@ -217,7 +217,8 @@ } } - .img-right { + .img-right, + .txt-right { float: right; margin-left: 10px; } diff --git a/apps/web/src/app/tools/generator.component.html b/apps/web/src/app/tools/generator.component.html index 2e6d6d0effd..3a079bcac5a 100644 --- a/apps/web/src/app/tools/generator.component.html +++ b/apps/web/src/app/tools/generator.component.html @@ -109,13 +109,31 @@ id="length" class="form-control" type="number" - min="5" + [min]="passwordOptions.minLength" max="128" [(ngModel)]="passwordOptions.length" (blur)="savePasswordOptions()" (change)="lengthChanged()" />
+
+ + + + {{ passwordOptionsMinLengthForReader$ | async }} + +
@@ -137,8 +155,8 @@ type="number" min="0" max="9" - (blur)="savePasswordOptions()" [(ngModel)]="passwordOptions.minSpecial" + (input)="onPasswordOptionsMinSpecialInput($event)" (change)="minSpecialChanged()" /> @@ -175,7 +193,8 @@ class="form-check-input" type="checkbox" (change)="savePasswordOptions()" - [(ngModel)]="passwordOptions.number" + [ngModel]="passwordOptions.number" + (ngModelChange)="setPasswordOptionsNumber($event)" [disabled]="enforcedPasswordPolicyOptions?.useNumbers" attr.aria-label="{{ 'numbers' | i18n }}" /> @@ -186,8 +205,8 @@ id="special" class="form-check-input" type="checkbox" - (change)="savePasswordOptions()" - [(ngModel)]="passwordOptions.special" + [ngModel]="passwordOptions.special" + (ngModelChange)="setPasswordOptionsSpecial($event)" [disabled]="enforcedPasswordPolicyOptions?.useSpecial" attr.aria-label="{{ 'specialCharacters' | i18n }}" /> diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 236ba76cfe2..0ec3b27cc38 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -1150,6 +1150,9 @@ "length": { "message": "Length" }, + "passwordMinLength": { + "message": "Minimum password length" + }, "uppercase": { "message": "Uppercase (A-Z)", "description": "Include uppercase letters in the password generator." diff --git a/libs/angular/src/tools/generator/components/generator.component.ts b/libs/angular/src/tools/generator/components/generator.component.ts index 36af4868b0c..35ec0b2c5a2 100644 --- a/libs/angular/src/tools/generator/components/generator.component.ts +++ b/libs/angular/src/tools/generator/components/generator.component.ts @@ -1,6 +1,7 @@ import { Directive, EventEmitter, Input, OnInit, Output } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { first } from "rxjs/operators"; +import { BehaviorSubject } from "rxjs"; +import { debounceTime, first, map } from "rxjs/operators"; import { PasswordGeneratorPolicyOptions } from "@bitwarden/common/admin-console/models/domain/password-generator-policy-options"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -12,6 +13,7 @@ import { PasswordGenerationServiceAbstraction, PasswordGeneratorOptions, } from "@bitwarden/common/tools/generator/password"; +import { DefaultBoundaries } from "@bitwarden/common/tools/generator/password/password-generator-options-evaluator"; import { UsernameGenerationServiceAbstraction, UsernameGeneratorOptions, @@ -40,6 +42,16 @@ export class GeneratorComponent implements OnInit { enforcedPasswordPolicyOptions: PasswordGeneratorPolicyOptions; usernameWebsite: string = null; + // update screen reader minimum password length with 500ms debounce + // so that the user isn't flooded with status updates + private _passwordOptionsMinLengthForReader = new BehaviorSubject( + DefaultBoundaries.length.min, + ); + protected passwordOptionsMinLengthForReader$ = this._passwordOptionsMinLengthForReader.pipe( + map((val) => val || DefaultBoundaries.length.min), + debounceTime(500), + ); + constructor( protected passwordGenerationService: PasswordGenerationServiceAbstraction, protected usernameGenerationService: UsernameGenerationServiceAbstraction, @@ -144,6 +156,44 @@ export class GeneratorComponent implements OnInit { await this.passwordGenerationService.addHistory(this.password); } + async onPasswordOptionsMinNumberInput($event: Event) { + // `savePasswordOptions()` replaces the null + this.passwordOptions.number = null; + + await this.savePasswordOptions(); + + // fixes UI desync that occurs when minNumber has a fixed value + // that is reset through normalization + ($event.target as HTMLInputElement).value = `${this.passwordOptions.minNumber}`; + } + + async setPasswordOptionsNumber($event: boolean) { + this.passwordOptions.number = $event; + // `savePasswordOptions()` replaces the null + this.passwordOptions.minNumber = null; + + await this.savePasswordOptions(); + } + + async onPasswordOptionsMinSpecialInput($event: Event) { + // `savePasswordOptions()` replaces the null + this.passwordOptions.special = null; + + await this.savePasswordOptions(); + + // fixes UI desync that occurs when minSpecial has a fixed value + // that is reset through normalization + ($event.target as HTMLInputElement).value = `${this.passwordOptions.minSpecial}`; + } + + async setPasswordOptionsSpecial($event: boolean) { + this.passwordOptions.special = $event; + // `savePasswordOptions()` replaces the null + this.passwordOptions.minSpecial = null; + + await this.savePasswordOptions(); + } + async sliderInput() { this.normalizePasswordOptions(); this.password = await this.passwordGenerationService.generatePassword(this.passwordOptions); @@ -240,6 +290,8 @@ export class GeneratorComponent implements OnInit { this.passwordOptions, this.enforcedPasswordPolicyOptions, ); + + this._passwordOptionsMinLengthForReader.next(this.passwordOptions.minLength); } private async initForwardOptions() { diff --git a/libs/common/src/admin-console/models/domain/password-generator-policy-options.ts b/libs/common/src/admin-console/models/domain/password-generator-policy-options.ts index 858b9fd8829..9d2e7eadd59 100644 --- a/libs/common/src/admin-console/models/domain/password-generator-policy-options.ts +++ b/libs/common/src/admin-console/models/domain/password-generator-policy-options.ts @@ -1,18 +1,73 @@ import Domain from "../../../platform/models/domain/domain-base"; +/** Enterprise policy for the password generator. + * @see PolicyType.PasswordGenerator + */ export class PasswordGeneratorPolicyOptions extends Domain { - defaultType = ""; + /** The default kind of credential to generate */ + defaultType: "password" | "passphrase" | "" = ""; + + /** The minimum length of generated passwords. + * When this is less than or equal to zero, it is ignored. + * If this is less than the total number of characters required by + * the policy's other settings, then it is ignored. + * This field is not used for passphrases. + */ minLength = 0; + + /** When this is true, an uppercase character must be part of + * the generated password. + * This field is not used for passphrases. + */ useUppercase = false; + + /** When this is true, a lowercase character must be part of + * the generated password. This field is not used for passphrases. + */ useLowercase = false; + + /** When this is true, at least one digit must be part of the generated + * password. This field is not used for passphrases. + */ useNumbers = false; + + /** The quantity of digits to include in the generated password. + * When this is less than or equal to zero, it is ignored. + * This field is not used for passphrases. + */ numberCount = 0; + + /** When this is true, at least one digit must be part of the generated + * password. This field is not used for passphrases. + */ useSpecial = false; + + /** The quantity of special characters to include in the generated + * password. When this is less than or equal to zero, it is ignored. + * This field is not used for passphrases. + */ specialCount = 0; + + /** The minimum number of words required by generated passphrases. + * This field is not used for passwords. + */ minNumberWords = 0; + + /** When this is true, the first letter of each word in the passphrase + * is capitalized. This field is not used for passwords. + */ capitalize = false; + + /** When this is true, a number is included within the passphrase. + * This field is not used for passwords. + */ includeNumber = false; + /** Checks whether the policy affects the password generator. + * @returns True if at least one password or passphrase requirement has been set. + * If it returns False, then no requirements have been set and the policy should + * not be enforced. + */ inEffect() { return ( this.defaultType !== "" || @@ -28,4 +83,12 @@ export class PasswordGeneratorPolicyOptions extends Domain { this.includeNumber ); } + + /** Creates a copy of the policy. + */ + clone() { + const policy = new PasswordGeneratorPolicyOptions(); + Object.assign(policy, this); + return policy; + } } diff --git a/libs/common/src/tools/generator/password/passphrase-generator-options-evaluator.spec.ts b/libs/common/src/tools/generator/password/passphrase-generator-options-evaluator.spec.ts new file mode 100644 index 00000000000..c72a190f799 --- /dev/null +++ b/libs/common/src/tools/generator/password/passphrase-generator-options-evaluator.spec.ts @@ -0,0 +1,220 @@ +import { PasswordGeneratorPolicyOptions } from "../../../admin-console/models/domain/password-generator-policy-options"; + +import { + DefaultBoundaries, + PassphraseGeneratorOptionsEvaluator, +} from "./passphrase-generator-options-evaluator"; +import { PassphraseGenerationOptions } from "./password-generator-options"; + +describe("Password generator options builder", () => { + describe("constructor()", () => { + it("should set the policy object to a copy of the input policy", () => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.minLength = 10; // arbitrary change for deep equality check + + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + + expect(builder.policy).toEqual(policy); + expect(builder.policy).not.toBe(policy); + }); + + it("should set default boundaries when a default policy is used", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + + expect(builder.numWords).toEqual(DefaultBoundaries.numWords); + }); + + it.each([1, 2])( + "should use the default word boundaries when they are greater than `policy.minNumberWords` (= %i)", + (minNumberWords) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.minNumberWords = minNumberWords; + + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + + expect(builder.numWords).toEqual(DefaultBoundaries.numWords); + }, + ); + + it.each([8, 12, 18])( + "should use `policy.minNumberWords` (= %i) when it is greater than the default minimum words", + (minNumberWords) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.minNumberWords = minNumberWords; + + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + + expect(builder.numWords.min).toEqual(minNumberWords); + expect(builder.numWords.max).toEqual(DefaultBoundaries.numWords.max); + }, + ); + + it.each([150, 300, 9000])( + "should use `policy.minNumberWords` (= %i) when it is greater than the default boundaries", + (minNumberWords) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.minNumberWords = minNumberWords; + + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + + expect(builder.numWords.min).toEqual(minNumberWords); + expect(builder.numWords.max).toEqual(minNumberWords); + }, + ); + }); + + describe("applyPolicy(options)", () => { + // All tests should freeze the options to ensure they are not modified + + it("should set `capitalize` to `false` when the policy does not override it", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({}); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.capitalize).toBe(false); + }); + + it("should set `capitalize` to `true` when the policy overrides it", () => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.capitalize = true; + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ capitalize: false }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.capitalize).toBe(true); + }); + + it("should set `includeNumber` to false when the policy does not override it", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({}); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.includeNumber).toBe(false); + }); + + it("should set `includeNumber` to true when the policy overrides it", () => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.includeNumber = true; + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ includeNumber: false }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.includeNumber).toBe(true); + }); + + it("should set `numWords` to the minimum value when it isn't supplied", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({}); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.numWords).toBe(builder.numWords.min); + }); + + it.each([1, 2])( + "should set `numWords` (= %i) to the minimum value when it is less than the minimum", + (numWords) => { + expect(numWords).toBeLessThan(DefaultBoundaries.numWords.min); + + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ numWords }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.numWords).toBe(builder.numWords.min); + }, + ); + + it.each([3, 8, 18, 20])( + "should set `numWords` (= %i) to the input value when it is within the boundaries", + (numWords) => { + expect(numWords).toBeGreaterThanOrEqual(DefaultBoundaries.numWords.min); + expect(numWords).toBeLessThanOrEqual(DefaultBoundaries.numWords.max); + + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ numWords }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.numWords).toBe(numWords); + }, + ); + + it.each([21, 30, 50, 100])( + "should set `numWords` (= %i) to the maximum value when it is greater than the maximum", + (numWords) => { + expect(numWords).toBeGreaterThan(DefaultBoundaries.numWords.max); + + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ numWords }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.numWords).toBe(builder.numWords.max); + }, + ); + + it("should preserve unknown properties", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ + unknown: "property", + another: "unknown property", + }) as PassphraseGenerationOptions; + + const sanitizedOptions: any = builder.applyPolicy(options); + + expect(sanitizedOptions.unknown).toEqual("property"); + expect(sanitizedOptions.another).toEqual("unknown property"); + }); + }); + + describe("sanitize(options)", () => { + // All tests should freeze the options to ensure they are not modified + + it("should return the input options without altering them", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ wordSeparator: "%" }); + + const sanitizedOptions = builder.sanitize(options); + + expect(sanitizedOptions).toEqual(options); + }); + + it("should set `wordSeparator` to '-' when it isn't supplied and there is no policy override", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({}); + + const sanitizedOptions = builder.sanitize(options); + + expect(sanitizedOptions.wordSeparator).toEqual("-"); + }); + + it("should preserve unknown properties", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PassphraseGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ + unknown: "property", + another: "unknown property", + }) as PassphraseGenerationOptions; + + const sanitizedOptions: any = builder.sanitize(options); + + expect(sanitizedOptions.unknown).toEqual("property"); + expect(sanitizedOptions.another).toEqual("unknown property"); + }); + }); +}); diff --git a/libs/common/src/tools/generator/password/passphrase-generator-options-evaluator.ts b/libs/common/src/tools/generator/password/passphrase-generator-options-evaluator.ts new file mode 100644 index 00000000000..8cfdce4b094 --- /dev/null +++ b/libs/common/src/tools/generator/password/passphrase-generator-options-evaluator.ts @@ -0,0 +1,105 @@ +import { PasswordGeneratorPolicyOptions } from "../../../admin-console/models/domain/password-generator-policy-options"; + +import { PassphraseGenerationOptions } from "./password-generator-options"; + +type Boundary = { + readonly min: number; + readonly max: number; +}; + +function initializeBoundaries() { + const numWords = Object.freeze({ + min: 3, + max: 20, + }); + + return Object.freeze({ + numWords, + }); +} + +/** Immutable default boundaries for passphrase generation. + * These are used when the policy does not override a value. + */ +export const DefaultBoundaries = initializeBoundaries(); + +/** Enforces policy for passphrase generation options. + */ +export class PassphraseGeneratorOptionsEvaluator { + // This design is not ideal, but it is a step towards a more robust passphrase + // generator. Ideally, `sanitize` would be implemented on an options class, + // and `applyPolicy` would be implemented on a policy class, "mise en place". + // + // The current design of the passphrase generator, unfortunately, would require + // a substantial rewrite to make this feasible. Hopefully this change can be + // applied when the passphrase generator is ported to rust. + + /** Policy applied by the evaluator. + */ + readonly policy: PasswordGeneratorPolicyOptions; + + /** Boundaries for the number of words allowed in the password. + */ + readonly numWords: Boundary; + + /** Instantiates the evaluator. + * @param policy The policy applied by the evaluator. When this conflicts with + * the defaults, the policy takes precedence. + */ + constructor(policy: PasswordGeneratorPolicyOptions) { + function createBoundary(value: number, defaultBoundary: Boundary): Boundary { + const boundary = { + min: Math.max(defaultBoundary.min, value), + max: Math.max(defaultBoundary.max, value), + }; + + return boundary; + } + + this.policy = policy.clone(); + this.numWords = createBoundary(policy.minNumberWords, DefaultBoundaries.numWords); + } + + /** Apply policy to the input options. + * @param options The options to build from. These options are not altered. + * @returns A new password generation request with policy applied. + */ + applyPolicy(options: PassphraseGenerationOptions): PassphraseGenerationOptions { + function fitToBounds(value: number, boundaries: Boundary) { + const { min, max } = boundaries; + + const withUpperBound = Math.min(value ?? boundaries.min, max); + const withLowerBound = Math.max(withUpperBound, min); + + return withLowerBound; + } + + // apply policy overrides + const capitalize = this.policy.capitalize || options.capitalize || false; + const includeNumber = this.policy.includeNumber || options.includeNumber || false; + + // apply boundaries + const numWords = fitToBounds(options.numWords, this.numWords); + + return { + ...options, + numWords, + capitalize, + includeNumber, + }; + } + + /** Ensures internal options consistency. + * @param options The options to cascade. These options are not altered. + * @returns A passphrase generation request with cascade applied. + */ + sanitize(options: PassphraseGenerationOptions): PassphraseGenerationOptions { + // ensure words are separated by a single character + const wordSeparator = options.wordSeparator?.[0] ?? "-"; + + return { + ...options, + wordSeparator, + }; + } +} diff --git a/libs/common/src/tools/generator/password/password-generation.service.ts b/libs/common/src/tools/generator/password/password-generation.service.ts index aa77502f758..e497126cd30 100644 --- a/libs/common/src/tools/generator/password/password-generation.service.ts +++ b/libs/common/src/tools/generator/password/password-generation.service.ts @@ -7,11 +7,14 @@ import { EFFLongWordList } from "../../../platform/misc/wordlist"; import { EncString } from "../../../platform/models/domain/enc-string"; import { GeneratedPasswordHistory } from "./generated-password-history"; +import { PassphraseGeneratorOptionsEvaluator } from "./passphrase-generator-options-evaluator"; import { PasswordGenerationServiceAbstraction } from "./password-generation.service.abstraction"; import { PasswordGeneratorOptions } from "./password-generator-options"; +import { PasswordGeneratorOptionsEvaluator } from "./password-generator-options-evaluator"; const DefaultOptions: PasswordGeneratorOptions = { length: 14, + minLength: 5, ambiguous: false, number: true, minNumber: 1, @@ -28,6 +31,8 @@ const DefaultOptions: PasswordGeneratorOptions = { includeNumber: false, }; +const DefaultPolicy = new PasswordGeneratorPolicyOptions(); + const MaxPasswordsInHistory = 100; export class PasswordGenerationService implements PasswordGenerationServiceAbstraction { @@ -38,20 +43,12 @@ export class PasswordGenerationService implements PasswordGenerationServiceAbstr ) {} async generatePassword(options: PasswordGeneratorOptions): Promise { - // overload defaults with given options - const o = Object.assign({}, DefaultOptions, options); - - if (o.type === "passphrase") { - return this.generatePassphrase(options); + if ((options.type ?? DefaultOptions.type) === "passphrase") { + return this.generatePassphrase({ ...DefaultOptions, ...options }); } - // sanitize - this.sanitizePasswordLength(o, true); - - const minLength: number = o.minUppercase + o.minLowercase + o.minNumber + o.minSpecial; - if (o.length < minLength) { - o.length = minLength; - } + const evaluator = new PasswordGeneratorOptionsEvaluator(DefaultPolicy); + const o = evaluator.sanitize({ ...DefaultOptions, ...options }); const positions: string[] = []; if (o.lowercase && o.minLowercase > 0) { @@ -144,7 +141,8 @@ export class PasswordGenerationService implements PasswordGenerationServiceAbstr } async generatePassphrase(options: PasswordGeneratorOptions): Promise { - const o = Object.assign({}, DefaultOptions, options); + const evaluator = new PassphraseGeneratorOptionsEvaluator(DefaultPolicy); + const o = evaluator.sanitize({ ...DefaultOptions, ...options }); if (o.numWords == null || o.numWords <= 2) { o.numWords = DefaultOptions.numWords; @@ -192,65 +190,25 @@ export class PasswordGenerationService implements PasswordGenerationServiceAbstr async enforcePasswordGeneratorPoliciesOnOptions( options: PasswordGeneratorOptions, ): Promise<[PasswordGeneratorOptions, PasswordGeneratorPolicyOptions]> { - let enforcedPolicyOptions = await this.getPasswordGeneratorPolicyOptions(); - if (enforcedPolicyOptions != null) { - if (options.length < enforcedPolicyOptions.minLength) { - options.length = enforcedPolicyOptions.minLength; - } + let policy = await this.getPasswordGeneratorPolicyOptions(); + policy = policy ?? new PasswordGeneratorPolicyOptions(); - if (enforcedPolicyOptions.useUppercase) { - options.uppercase = true; - } - - if (enforcedPolicyOptions.useLowercase) { - options.lowercase = true; - } - - if (enforcedPolicyOptions.useNumbers) { - options.number = true; - } - - if (options.minNumber < enforcedPolicyOptions.numberCount) { - options.minNumber = enforcedPolicyOptions.numberCount; - } - - if (enforcedPolicyOptions.useSpecial) { - options.special = true; - } - - if (options.minSpecial < enforcedPolicyOptions.specialCount) { - options.minSpecial = enforcedPolicyOptions.specialCount; - } - - // Must normalize these fields because the receiving call expects all options to pass the current rules - if (options.minSpecial + options.minNumber > options.length) { - options.minSpecial = options.length - options.minNumber; - } - - if (options.numWords < enforcedPolicyOptions.minNumberWords) { - options.numWords = enforcedPolicyOptions.minNumberWords; - } - - if (enforcedPolicyOptions.capitalize) { - options.capitalize = true; - } - - if (enforcedPolicyOptions.includeNumber) { - options.includeNumber = true; - } - - // Force default type if password/passphrase selected via policy - if ( - enforcedPolicyOptions.defaultType === "password" || - enforcedPolicyOptions.defaultType === "passphrase" - ) { - options.type = enforcedPolicyOptions.defaultType; - } - } else { - // UI layer expects an instantiated object to prevent more explicit null checks - enforcedPolicyOptions = new PasswordGeneratorPolicyOptions(); + // Force default type if password/passphrase selected via policy + if (policy.defaultType === "password" || policy.defaultType === "passphrase") { + options.type = policy.defaultType; } - return [options, enforcedPolicyOptions]; + + const evaluator = options.type + ? new PasswordGeneratorOptionsEvaluator(policy) + : new PassphraseGeneratorOptionsEvaluator(policy); + + // Ensure the options to pass the current rules + const withPolicy = evaluator.applyPolicy(options); + const sanitized = evaluator.sanitize(withPolicy); + + // callers assume this function updates the options parameter + const result = Object.assign(options, sanitized); + return [result, policy]; } async getPasswordGeneratorPolicyOptions(): Promise { @@ -389,62 +347,17 @@ export class PasswordGenerationService implements PasswordGenerationServiceAbstr options: PasswordGeneratorOptions, enforcedPolicyOptions: PasswordGeneratorPolicyOptions, ) { - options.minLowercase = 0; - options.minUppercase = 0; + const evaluator = options.type + ? new PasswordGeneratorOptionsEvaluator(enforcedPolicyOptions) + : new PassphraseGeneratorOptionsEvaluator(enforcedPolicyOptions); - if (!options.length || options.length < 5) { - options.length = 5; - } else if (options.length > 128) { - options.length = 128; - } + const evaluatedOptions = evaluator.applyPolicy(options); + const santizedOptions = evaluator.sanitize(evaluatedOptions); - if (options.length < enforcedPolicyOptions.minLength) { - options.length = enforcedPolicyOptions.minLength; - } + // callers assume this function updates the options parameter + Object.assign(options, santizedOptions); - if (!options.minNumber) { - options.minNumber = 0; - } else if (options.minNumber > options.length) { - options.minNumber = options.length; - } else if (options.minNumber > 9) { - options.minNumber = 9; - } - - if (options.minNumber < enforcedPolicyOptions.numberCount) { - options.minNumber = enforcedPolicyOptions.numberCount; - } - - if (!options.minSpecial) { - options.minSpecial = 0; - } else if (options.minSpecial > options.length) { - options.minSpecial = options.length; - } else if (options.minSpecial > 9) { - options.minSpecial = 9; - } - - if (options.minSpecial < enforcedPolicyOptions.specialCount) { - options.minSpecial = enforcedPolicyOptions.specialCount; - } - - if (options.minSpecial + options.minNumber > options.length) { - options.minSpecial = options.length - options.minNumber; - } - - if (options.numWords == null || options.length < 3) { - options.numWords = 3; - } else if (options.numWords > 20) { - options.numWords = 20; - } - - if (options.numWords < enforcedPolicyOptions.minNumberWords) { - options.numWords = enforcedPolicyOptions.minNumberWords; - } - - if (options.wordSeparator != null && options.wordSeparator.length > 1) { - options.wordSeparator = options.wordSeparator[0]; - } - - this.sanitizePasswordLength(options, false); + return options; } private capitalize(str: string) { @@ -505,54 +418,4 @@ export class PasswordGenerationService implements PasswordGenerationServiceAbstr [array[i], array[j]] = [array[j], array[i]]; } } - - private sanitizePasswordLength(options: any, forGeneration: boolean) { - let minUppercaseCalc = 0; - let minLowercaseCalc = 0; - let minNumberCalc: number = options.minNumber; - let minSpecialCalc: number = options.minSpecial; - - if (options.uppercase && options.minUppercase <= 0) { - minUppercaseCalc = 1; - } else if (!options.uppercase) { - minUppercaseCalc = 0; - } - - if (options.lowercase && options.minLowercase <= 0) { - minLowercaseCalc = 1; - } else if (!options.lowercase) { - minLowercaseCalc = 0; - } - - if (options.number && options.minNumber <= 0) { - minNumberCalc = 1; - } else if (!options.number) { - minNumberCalc = 0; - } - - if (options.special && options.minSpecial <= 0) { - minSpecialCalc = 1; - } else if (!options.special) { - minSpecialCalc = 0; - } - - // This should never happen but is a final safety net - if (!options.length || options.length < 1) { - options.length = 10; - } - - const minLength: number = minUppercaseCalc + minLowercaseCalc + minNumberCalc + minSpecialCalc; - // Normalize and Generation both require this modification - if (options.length < minLength) { - options.length = minLength; - } - - // Apply other changes if the options object passed in is for generation - if (forGeneration) { - options.minUppercase = minUppercaseCalc; - options.minLowercase = minLowercaseCalc; - options.minNumber = minNumberCalc; - options.minSpecial = minSpecialCalc; - } - } } diff --git a/libs/common/src/tools/generator/password/password-generator-options-evaluator.spec.ts b/libs/common/src/tools/generator/password/password-generator-options-evaluator.spec.ts new file mode 100644 index 00000000000..e1ca854eb90 --- /dev/null +++ b/libs/common/src/tools/generator/password/password-generator-options-evaluator.spec.ts @@ -0,0 +1,703 @@ +import { PasswordGeneratorPolicyOptions } from "../../../admin-console/models/domain/password-generator-policy-options"; + +import { PasswordGenerationOptions } from "./password-generator-options"; +import { + DefaultBoundaries, + PasswordGeneratorOptionsEvaluator, +} from "./password-generator-options-evaluator"; + +describe("Password generator options builder", () => { + const defaultOptions = Object.freeze({ minLength: 0 }); + + describe("constructor()", () => { + it("should set the policy object to a copy of the input policy", () => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.minLength = 10; // arbitrary change for deep equality check + + const builder = new PasswordGeneratorOptionsEvaluator(policy); + + expect(builder.policy).toEqual(policy); + expect(builder.policy).not.toBe(policy); + }); + + it("should set default boundaries when a default policy is used", () => { + const policy = new PasswordGeneratorPolicyOptions(); + + const builder = new PasswordGeneratorOptionsEvaluator(policy); + + expect(builder.length).toEqual(DefaultBoundaries.length); + expect(builder.minDigits).toEqual(DefaultBoundaries.minDigits); + expect(builder.minSpecialCharacters).toEqual(DefaultBoundaries.minSpecialCharacters); + }); + + it.each([1, 2, 3, 4])( + "should use the default length boundaries when they are greater than `policy.minLength` (= %i)", + (minLength) => { + expect(minLength).toBeLessThan(DefaultBoundaries.length.min); + + const policy = new PasswordGeneratorPolicyOptions(); + policy.minLength = minLength; + + const builder = new PasswordGeneratorOptionsEvaluator(policy); + + expect(builder.length).toEqual(DefaultBoundaries.length); + }, + ); + + it.each([8, 20, 100])( + "should use `policy.minLength` (= %i) when it is greater than the default minimum length", + (expectedLength) => { + expect(expectedLength).toBeGreaterThan(DefaultBoundaries.length.min); + expect(expectedLength).toBeLessThanOrEqual(DefaultBoundaries.length.max); + + const policy = new PasswordGeneratorPolicyOptions(); + policy.minLength = expectedLength; + + const builder = new PasswordGeneratorOptionsEvaluator(policy); + + expect(builder.length.min).toEqual(expectedLength); + expect(builder.length.max).toEqual(DefaultBoundaries.length.max); + }, + ); + + it.each([150, 300, 9000])( + "should use `policy.minLength` (= %i) when it is greater than the default boundaries", + (expectedLength) => { + expect(expectedLength).toBeGreaterThan(DefaultBoundaries.length.max); + + const policy = new PasswordGeneratorPolicyOptions(); + policy.minLength = expectedLength; + + const builder = new PasswordGeneratorOptionsEvaluator(policy); + + expect(builder.length.min).toEqual(expectedLength); + expect(builder.length.max).toEqual(expectedLength); + }, + ); + + it.each([3, 5, 8, 9])( + "should use `policy.numberCount` (= %i) when it is greater than the default minimum digits", + (expectedMinDigits) => { + expect(expectedMinDigits).toBeGreaterThan(DefaultBoundaries.minDigits.min); + expect(expectedMinDigits).toBeLessThanOrEqual(DefaultBoundaries.minDigits.max); + + const policy = new PasswordGeneratorPolicyOptions(); + policy.numberCount = expectedMinDigits; + + const builder = new PasswordGeneratorOptionsEvaluator(policy); + + expect(builder.minDigits.min).toEqual(expectedMinDigits); + expect(builder.minDigits.max).toEqual(DefaultBoundaries.minDigits.max); + }, + ); + + it.each([10, 20, 400])( + "should use `policy.numberCount` (= %i) when it is greater than the default digit boundaries", + (expectedMinDigits) => { + expect(expectedMinDigits).toBeGreaterThan(DefaultBoundaries.minDigits.max); + + const policy = new PasswordGeneratorPolicyOptions(); + policy.numberCount = expectedMinDigits; + + const builder = new PasswordGeneratorOptionsEvaluator(policy); + + expect(builder.minDigits.min).toEqual(expectedMinDigits); + expect(builder.minDigits.max).toEqual(expectedMinDigits); + }, + ); + + it.each([2, 4, 6])( + "should use `policy.specialCount` (= %i) when it is greater than the default minimum special characters", + (expectedSpecialCharacters) => { + expect(expectedSpecialCharacters).toBeGreaterThan( + DefaultBoundaries.minSpecialCharacters.min, + ); + expect(expectedSpecialCharacters).toBeLessThanOrEqual( + DefaultBoundaries.minSpecialCharacters.max, + ); + + const policy = new PasswordGeneratorPolicyOptions(); + policy.specialCount = expectedSpecialCharacters; + + const builder = new PasswordGeneratorOptionsEvaluator(policy); + + expect(builder.minSpecialCharacters.min).toEqual(expectedSpecialCharacters); + expect(builder.minSpecialCharacters.max).toEqual( + DefaultBoundaries.minSpecialCharacters.max, + ); + }, + ); + + it.each([10, 20, 400])( + "should use `policy.specialCount` (= %i) when it is greater than the default special characters boundaries", + (expectedSpecialCharacters) => { + expect(expectedSpecialCharacters).toBeGreaterThan( + DefaultBoundaries.minSpecialCharacters.max, + ); + + const policy = new PasswordGeneratorPolicyOptions(); + policy.specialCount = expectedSpecialCharacters; + + const builder = new PasswordGeneratorOptionsEvaluator(policy); + + expect(builder.minSpecialCharacters.min).toEqual(expectedSpecialCharacters); + expect(builder.minSpecialCharacters.max).toEqual(expectedSpecialCharacters); + }, + ); + + it.each([ + [8, 6, 2], + [6, 2, 4], + [16, 8, 8], + ])( + "should ensure the minimum length (= %i) is at least the sum of minimums (= %i + %i)", + (expectedLength, numberCount, specialCount) => { + expect(expectedLength).toBeGreaterThanOrEqual(DefaultBoundaries.length.min); + + const policy = new PasswordGeneratorPolicyOptions(); + policy.numberCount = numberCount; + policy.specialCount = specialCount; + + const builder = new PasswordGeneratorOptionsEvaluator(policy); + + expect(builder.length.min).toBeGreaterThanOrEqual(expectedLength); + }, + ); + }); + + describe("applyPolicy(options)", () => { + // All tests should freeze the options to ensure they are not modified + + it.each([ + [false, false], + [true, true], + [false, undefined], + ])( + "should set `options.uppercase` to '%s' when `policy.useUppercase` is false and `options.uppercase` is '%s'", + (expectedUppercase, uppercase) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.useUppercase = false; + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, uppercase }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.uppercase).toEqual(expectedUppercase); + }, + ); + + it.each([false, true, undefined])( + "should set `options.uppercase` (= %s) to true when `policy.useUppercase` is true", + (uppercase) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.useUppercase = true; + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, uppercase }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.uppercase).toEqual(true); + }, + ); + + it.each([ + [false, false], + [true, true], + [false, undefined], + ])( + "should set `options.lowercase` to '%s' when `policy.useLowercase` is false and `options.lowercase` is '%s'", + (expectedLowercase, lowercase) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.useLowercase = false; + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, lowercase }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.lowercase).toEqual(expectedLowercase); + }, + ); + + it.each([false, true, undefined])( + "should set `options.lowercase` (= %s) to true when `policy.useLowercase` is true", + (lowercase) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.useLowercase = true; + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, lowercase }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.lowercase).toEqual(true); + }, + ); + + it.each([ + [false, false], + [true, true], + [false, undefined], + ])( + "should set `options.number` to '%s' when `policy.useNumbers` is false and `options.number` is '%s'", + (expectedNumber, number) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.useNumbers = false; + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, number }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.number).toEqual(expectedNumber); + }, + ); + + it.each([false, true, undefined])( + "should set `options.number` (= %s) to true when `policy.useNumbers` is true", + (number) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.useNumbers = true; + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, number }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.number).toEqual(true); + }, + ); + + it.each([ + [false, false], + [true, true], + [false, undefined], + ])( + "should set `options.special` to '%s' when `policy.useSpecial` is false and `options.special` is '%s'", + (expectedSpecial, special) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.useSpecial = false; + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, special }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.special).toEqual(expectedSpecial); + }, + ); + + it.each([false, true, undefined])( + "should set `options.special` (= %s) to true when `policy.useSpecial` is true", + (special) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.useSpecial = true; + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, special }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.special).toEqual(true); + }, + ); + + it.each([1, 2, 3, 4])( + "should set `options.length` (= %i) to the minimum it is less than the minimum length", + (length) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + expect(length).toBeLessThan(builder.length.min); + + const options = Object.freeze({ ...defaultOptions, length }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.length).toEqual(builder.length.min); + }, + ); + + it.each([5, 10, 50, 100, 128])( + "should not change `options.length` (= %i) when it is within the boundaries", + (length) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + expect(length).toBeGreaterThanOrEqual(builder.length.min); + expect(length).toBeLessThanOrEqual(builder.length.max); + + const options = Object.freeze({ ...defaultOptions, length }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.length).toEqual(length); + }, + ); + + it.each([129, 500, 9000])( + "should set `options.length` (= %i) to the maximum length when it is exceeded", + (length) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + expect(length).toBeGreaterThan(builder.length.max); + + const options = Object.freeze({ ...defaultOptions, length }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.length).toEqual(builder.length.max); + }, + ); + + it.each([ + [true, 1], + [true, 3], + [true, 600], + [false, 0], + [false, -2], + [false, -600], + ])( + "should set `options.number === %s` when `options.minNumber` (= %i) is set to a value greater than 0", + (expectedNumber, minNumber) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, minNumber }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.number).toEqual(expectedNumber); + }, + ); + + it("should set `options.minNumber` to the minimum value when `options.number` is true", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, number: true }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.minNumber).toEqual(builder.minDigits.min); + }); + + it("should set `options.minNumber` to 0 when `options.number` is false", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, number: false }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.minNumber).toEqual(0); + }); + + it.each([1, 2, 3, 4])( + "should set `options.minNumber` (= %i) to the minimum it is less than the minimum number", + (minNumber) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.numberCount = 5; // arbitrary value greater than minNumber + expect(minNumber).toBeLessThan(policy.numberCount); + + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, minNumber }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.minNumber).toEqual(builder.minDigits.min); + }, + ); + + it.each([1, 3, 5, 7, 9])( + "should not change `options.minNumber` (= %i) when it is within the boundaries", + (minNumber) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + expect(minNumber).toBeGreaterThanOrEqual(builder.minDigits.min); + expect(minNumber).toBeLessThanOrEqual(builder.minDigits.max); + + const options = Object.freeze({ ...defaultOptions, minNumber }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.minNumber).toEqual(minNumber); + }, + ); + + it.each([10, 20, 400])( + "should set `options.minNumber` (= %i) to the maximum digit boundary when it is exceeded", + (minNumber) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + expect(minNumber).toBeGreaterThan(builder.minDigits.max); + + const options = Object.freeze({ ...defaultOptions, minNumber }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.minNumber).toEqual(builder.minDigits.max); + }, + ); + + it.each([ + [true, 1], + [true, 3], + [true, 600], + [false, 0], + [false, -2], + [false, -600], + ])( + "should set `options.special === %s` when `options.minSpecial` (= %i) is set to a value greater than 0", + (expectedSpecial, minSpecial) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, minSpecial }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.special).toEqual(expectedSpecial); + }, + ); + + it("should set `options.minSpecial` to the minimum value when `options.special` is true", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, special: true }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.minSpecial).toEqual(builder.minDigits.min); + }); + + it("should set `options.minSpecial` to 0 when `options.special` is false", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, special: false }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.minSpecial).toEqual(0); + }); + + it.each([1, 2, 3, 4])( + "should set `options.minSpecial` (= %i) to the minimum it is less than the minimum special characters", + (minSpecial) => { + const policy = new PasswordGeneratorPolicyOptions(); + policy.specialCount = 5; // arbitrary value greater than minSpecial + expect(minSpecial).toBeLessThan(policy.specialCount); + + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ ...defaultOptions, minSpecial }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.minSpecial).toEqual(builder.minSpecialCharacters.min); + }, + ); + + it.each([1, 3, 5, 7, 9])( + "should not change `options.minSpecial` (= %i) when it is within the boundaries", + (minSpecial) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + expect(minSpecial).toBeGreaterThanOrEqual(builder.minSpecialCharacters.min); + expect(minSpecial).toBeLessThanOrEqual(builder.minSpecialCharacters.max); + + const options = Object.freeze({ ...defaultOptions, minSpecial }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.minSpecial).toEqual(minSpecial); + }, + ); + + it.each([10, 20, 400])( + "should set `options.minSpecial` (= %i) to the maximum special character boundary when it is exceeded", + (minSpecial) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + expect(minSpecial).toBeGreaterThan(builder.minSpecialCharacters.max); + + const options = Object.freeze({ ...defaultOptions, minSpecial }); + + const sanitizedOptions = builder.applyPolicy(options); + + expect(sanitizedOptions.minSpecial).toEqual(builder.minSpecialCharacters.max); + }, + ); + + it("should preserve unknown properties", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ + unknown: "property", + another: "unknown property", + }) as PasswordGenerationOptions; + + const sanitizedOptions: any = builder.applyPolicy(options); + + expect(sanitizedOptions.unknown).toEqual("property"); + expect(sanitizedOptions.another).toEqual("unknown property"); + }); + }); + + describe("sanitize(options)", () => { + // All tests should freeze the options to ensure they are not modified + + it.each([ + [1, true], + [0, false], + ])( + "should output `options.minLowercase === %i` when `options.lowercase` is %s", + (expectedMinLowercase, lowercase) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ lowercase, ...defaultOptions }); + + const actual = builder.sanitize(options); + + expect(actual.minLowercase).toEqual(expectedMinLowercase); + }, + ); + + it.each([ + [1, true], + [0, false], + ])( + "should output `options.minUppercase === %i` when `options.uppercase` is %s", + (expectedMinUppercase, uppercase) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ uppercase, ...defaultOptions }); + + const actual = builder.sanitize(options); + + expect(actual.minUppercase).toEqual(expectedMinUppercase); + }, + ); + + it.each([ + [1, true], + [0, false], + ])( + "should output `options.minNumber === %i` when `options.number` is %s and `options.minNumber` is not set", + (expectedMinNumber, number) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ number, ...defaultOptions }); + + const actual = builder.sanitize(options); + + expect(actual.minNumber).toEqual(expectedMinNumber); + }, + ); + + it.each([ + [true, 3], + [true, 2], + [true, 1], + [false, 0], + ])( + "should output `options.number === %s` when `options.minNumber` is %i and `options.number` is not set", + (expectedNumber, minNumber) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ minNumber, ...defaultOptions }); + + const actual = builder.sanitize(options); + + expect(actual.number).toEqual(expectedNumber); + }, + ); + + it.each([ + [true, 1], + [false, 0], + ])( + "should output `options.minSpecial === %i` when `options.special` is %s and `options.minSpecial` is not set", + (special, expectedMinSpecial) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ special, ...defaultOptions }); + + const actual = builder.sanitize(options); + + expect(actual.minSpecial).toEqual(expectedMinSpecial); + }, + ); + + it.each([ + [3, true], + [2, true], + [1, true], + [0, false], + ])( + "should output `options.special === %s` when `options.minSpecial` is %i and `options.special` is not set", + (minSpecial, expectedSpecial) => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ minSpecial, ...defaultOptions }); + + const actual = builder.sanitize(options); + + expect(actual.special).toEqual(expectedSpecial); + }, + ); + + it.each([ + [0, 0, 0, 0], + [1, 1, 0, 0], + [0, 0, 1, 1], + [1, 1, 1, 1], + ])( + "should set `options.minLength` to the minimum boundary when the sum of minimums (%i + %i + %i + %i) is less than the default minimum length.", + (minLowercase, minUppercase, minNumber, minSpecial) => { + const sumOfMinimums = minLowercase + minUppercase + minNumber + minSpecial; + expect(sumOfMinimums).toBeLessThan(DefaultBoundaries.length.min); + + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ + minLowercase, + minUppercase, + minNumber, + minSpecial, + ...defaultOptions, + }); + + const actual = builder.sanitize(options); + + expect(actual.minLength).toEqual(builder.length.min); + }, + ); + + it.each([ + [12, 3, 3, 3, 3], + [8, 2, 2, 2, 2], + [9, 3, 3, 3, 0], + ])( + "should set `options.minLength === %i` to the sum of minimums (%i + %i + %i + %i) when the sum is at least the default minimum length.", + (expectedMinLength, minLowercase, minUppercase, minNumber, minSpecial) => { + expect(expectedMinLength).toBeGreaterThanOrEqual(DefaultBoundaries.length.min); + + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ + minLowercase, + minUppercase, + minNumber, + minSpecial, + ...defaultOptions, + }); + + const actual = builder.sanitize(options); + + expect(actual.minLength).toEqual(expectedMinLength); + }, + ); + + it("should preserve unknown properties", () => { + const policy = new PasswordGeneratorPolicyOptions(); + const builder = new PasswordGeneratorOptionsEvaluator(policy); + const options = Object.freeze({ + unknown: "property", + another: "unknown property", + }) as PasswordGenerationOptions; + + const sanitizedOptions: any = builder.sanitize(options); + + expect(sanitizedOptions.unknown).toEqual("property"); + expect(sanitizedOptions.another).toEqual("unknown property"); + }); + }); +}); diff --git a/libs/common/src/tools/generator/password/password-generator-options-evaluator.ts b/libs/common/src/tools/generator/password/password-generator-options-evaluator.ts new file mode 100644 index 00000000000..0b3aae57ab8 --- /dev/null +++ b/libs/common/src/tools/generator/password/password-generator-options-evaluator.ts @@ -0,0 +1,179 @@ +import { PasswordGeneratorPolicyOptions } from "../../../admin-console/models/domain/password-generator-policy-options"; + +import { PasswordGenerationOptions } from "./password-generator-options"; + +function initializeBoundaries() { + const length = Object.freeze({ + min: 5, + max: 128, + }); + + const minDigits = Object.freeze({ + min: 0, + max: 9, + }); + + const minSpecialCharacters = Object.freeze({ + min: 0, + max: 9, + }); + + return Object.freeze({ + length, + minDigits, + minSpecialCharacters, + }); +} + +/** Immutable default boundaries for password generation. + * These are used when the policy does not override a value. + */ +export const DefaultBoundaries = initializeBoundaries(); + +type Boundary = { + readonly min: number; + readonly max: number; +}; + +/** Enforces policy for password generation. + */ +export class PasswordGeneratorOptionsEvaluator { + // This design is not ideal, but it is a step towards a more robust password + // generator. Ideally, `sanitize` would be implemented on an options class, + // and `applyPolicy` would be implemented on a policy class, "mise en place". + // + // The current design of the password generator, unfortunately, would require + // a substantial rewrite to make this feasible. Hopefully this change can be + // applied when the password generator is ported to rust. + + /** Boundaries for the password length. This is always large enough + * to accommodate the minimum number of digits and special characters. + */ + readonly length: Boundary; + + /** Boundaries for the minimum number of digits allowed in the password. + */ + readonly minDigits: Boundary; + + /** Boundaries for the minimum number of special characters allowed + * in the password. + */ + readonly minSpecialCharacters: Boundary; + + /** Policy applied by the evaluator. + */ + readonly policy: PasswordGeneratorPolicyOptions; + + /** Instantiates the evaluator. + * @param policy The policy applied by the evaluator. When this conflicts with + * the defaults, the policy takes precedence. + */ + constructor(policy: PasswordGeneratorPolicyOptions) { + function createBoundary(value: number, defaultBoundary: Boundary): Boundary { + const boundary = { + min: Math.max(defaultBoundary.min, value), + max: Math.max(defaultBoundary.max, value), + }; + + return boundary; + } + + this.policy = policy.clone(); + this.minDigits = createBoundary(policy.numberCount, DefaultBoundaries.minDigits); + this.minSpecialCharacters = createBoundary( + policy.specialCount, + DefaultBoundaries.minSpecialCharacters, + ); + + // the overall length should be at least as long as the sum of the minimums + const minConsistentLength = this.minDigits.min + this.minSpecialCharacters.min; + const minPolicyLength = policy.minLength > 0 ? policy.minLength : DefaultBoundaries.length.min; + const minLength = Math.max(minPolicyLength, minConsistentLength, DefaultBoundaries.length.min); + + this.length = { + min: minLength, + max: Math.max(DefaultBoundaries.length.max, minLength), + }; + } + + /** Apply policy to a set of options. + * @param options The options to build from. These options are not altered. + * @returns A complete password generation request with policy applied. + * @remarks This method only applies policy overrides. + * Pass the result to `sanitize` to ensure consistency. + */ + applyPolicy(options: PasswordGenerationOptions): PasswordGenerationOptions { + function fitToBounds(value: number, boundaries: Boundary) { + const { min, max } = boundaries; + + const withUpperBound = Math.min(value || 0, max); + const withLowerBound = Math.max(withUpperBound, min); + + return withLowerBound; + } + + // apply policy overrides + const uppercase = this.policy.useUppercase || options.uppercase || false; + const lowercase = this.policy.useLowercase || options.lowercase || false; + + // these overrides can cascade numeric fields to boolean fields + const number = this.policy.useNumbers || options.number || options.minNumber > 0; + const special = this.policy.useSpecial || options.special || options.minSpecial > 0; + + // apply boundaries; the boundaries can cascade boolean fields to numeric fields + const length = fitToBounds(options.length, this.length); + const minNumber = fitToBounds(options.minNumber, this.minDigits); + const minSpecial = fitToBounds(options.minSpecial, this.minSpecialCharacters); + + return { + ...options, + length, + uppercase, + lowercase, + number, + minNumber, + special, + minSpecial, + }; + } + + /** Ensures internal options consistency. + * @param options The options to cascade. These options are not altered. + * @returns A new password generation request with cascade applied. + * @remarks This method fills null and undefined values by looking at + * pairs of flags and values (e.g. `number` and `minNumber`). If the flag + * and value are inconsistent, the flag cascades to the value. + */ + sanitize(options: PasswordGenerationOptions): PasswordGenerationOptions { + function cascade(enabled: boolean, value: number): [boolean, number] { + const enabledResult = enabled ?? value > 0; + const valueResult = enabledResult ? value || 1 : 0; + + return [enabledResult, valueResult]; + } + + const [lowercase, minLowercase] = cascade(options.lowercase, options.minLowercase); + const [uppercase, minUppercase] = cascade(options.uppercase, options.minUppercase); + const [number, minNumber] = cascade(options.number, options.minNumber); + const [special, minSpecial] = cascade(options.special, options.minSpecial); + + // minimums can only increase the length + const minConsistentLength = minLowercase + minUppercase + minNumber + minSpecial; + const minLength = Math.max(minConsistentLength, this.length.min); + const length = Math.max(options.length ?? minLength, minLength); + + return { + ...options, + length, + minLength, + lowercase, + minLowercase, + uppercase, + minUppercase, + number, + minNumber, + special, + minSpecial, + }; + } +} diff --git a/libs/common/src/tools/generator/password/password-generator-options.ts b/libs/common/src/tools/generator/password/password-generator-options.ts index 287a91659e4..0f55f8cbc7c 100644 --- a/libs/common/src/tools/generator/password/password-generator-options.ts +++ b/libs/common/src/tools/generator/password/password-generator-options.ts @@ -1,17 +1,105 @@ -export type PasswordGeneratorOptions = { +/** Request format for credential generation. + * This type includes all properties suitable for reactive data binding. + */ +export type PasswordGeneratorOptions = PasswordGenerationOptions & + PassphraseGenerationOptions & { + /** The algorithm to use for credential generation. + * Properties on @see PasswordGenerationOptions should be processed + * only when `type === "password"`. + * Properties on @see PassphraseGenerationOptions should be processed + * only when `type === "passphrase"`. + */ + type?: "password" | "passphrase"; + }; + +/** Request format for password credential generation. + * All members of this type may be `undefined` when the user is + * generating a passphrase. + */ +export type PasswordGenerationOptions = { + /** The length of the password selected by the user */ length?: number; + + /** The minimum length of the password. This defaults to 5, and increases + * to ensure `minLength` is at least as large as the sum of the other minimums. + */ + minLength?: number; + + /** `true` when ambiguous characters may be included in the output. + * `false` when ambiguous characters should not be included in the output. + */ ambiguous?: boolean; + + /** `true` when uppercase ASCII characters should be included in the output + * This value defaults to `false. + */ uppercase?: boolean; + + /** The minimum number of uppercase characters to include in the output. + * The value is ignored when `uppercase` is `false`. + * The value defaults to 1 when `uppercase` is `true`. + */ minUppercase?: number; + + /** `true` when lowercase ASCII characters should be included in the output. + * This value defaults to `false`. + */ lowercase?: boolean; + + /** The minimum number of lowercase characters to include in the output. + * The value defaults to 1 when `lowercase` is `true`. + * The value defaults to 0 when `lowercase` is `false`. + */ minLowercase?: number; + + /** Whether or not to include ASCII digits in the output + * This value defaults to `true` when `minNumber` is at least 1. + * This value defaults to `false` when `minNumber` is less than 1. + */ number?: boolean; + + /** The minimum number of digits to include in the output. + * The value defaults to 1 when `number` is `true`. + * The value defaults to 0 when `number` is `false`. + */ minNumber?: number; + + /** Whether or not to include special characters in the output. + * This value defaults to `true` when `minSpecial` is at least 1. + * This value defaults to `false` when `minSpecial` is less than 1. + */ special?: boolean; + + /** The minimum number of special characters to include in the output. + * This value defaults to 1 when `special` is `true`. + * This value defaults to 0 when `special` is `false`. + */ minSpecial?: number; - numWords?: number; - wordSeparator?: string; - capitalize?: boolean; - includeNumber?: boolean; - type?: "password" | "passphrase"; +}; + +/** Request format for passphrase credential generation. + * The members of this type may be `undefined` when the user is + * generating a password. + */ +export type PassphraseGenerationOptions = { + /** The number of words to include in the passphrase. + * This value defaults to 4. + */ + numWords?: number; + + /** The ASCII separator character to use between words in the passphrase. + * This value defaults to a dash. + * If multiple characters appear in the string, only the first character is used. + */ + wordSeparator?: string; + + /** `true` when the first character of every word should be capitalized. + * This value defaults to `false`. + */ + capitalize?: boolean; + + /** `true` when a number should be included in the passphrase. + * This value defaults to `false`. + */ + includeNumber?: boolean; };