From bf7aa6473ee8e0a2d5e9055c3b4ec21571f306d0 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 1 Sep 2023 13:18:20 -0700 Subject: [PATCH 1/3] [PM-1509] Accessibility for elements (#5686) * change code color to meet accessibility requirements * updates to desktop and web * adjust colors for desktop, web, and browser * update color values * switch nord color to use same as Tailwind theme * align variable names --- apps/browser/src/popup/scss/variables.scss | 12 ++++++++---- apps/desktop/src/scss/variables.scss | 8 +++++--- apps/web/src/scss/variables.scss | 6 ++++-- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/apps/browser/src/popup/scss/variables.scss b/apps/browser/src/popup/scss/variables.scss index 2cdc49cd9ef..d8891cf620b 100644 --- a/apps/browser/src/popup/scss/variables.scss +++ b/apps/browser/src/popup/scss/variables.scss @@ -43,6 +43,10 @@ $button-color: lighten($text-color, 40%); $button-color-primary: darken($brand-primary, 8%); $button-color-danger: darken($brand-danger, 10%); +$code-color: #c01176; +$code-color-dark: #f08dc7; +$code-color-nord: #dbb1d5; + $solarizedDarkBase03: #002b36; $solarizedDarkBase02: #073642; $solarizedDarkBase01: #586e75; @@ -122,7 +126,7 @@ $themes: ( // light has no hover so use same color webkitCalendarPickerHoverFilter: invert(46%) sepia(69%) saturate(6397%) hue-rotate(211deg) brightness(85%) contrast(103%), - codeColor: #e83e8c, + codeColor: $code-color, ), dark: ( textColor: #ffffff, @@ -184,7 +188,7 @@ $themes: ( hue-rotate(184deg) brightness(87%) contrast(93%), webkitCalendarPickerHoverFilter: brightness(0) saturate(100%) invert(100%) sepia(0%) saturate(0%) hue-rotate(93deg) brightness(103%) contrast(103%), - codeColor: #e83e8c, + codeColor: $code-color-dark, ), nord: ( textColor: $nord5, @@ -246,7 +250,7 @@ $themes: ( // has no hover so use same color webkitCalendarPickerHoverFilter: brightness(0) saturate(100%) invert(94%) sepia(5%) saturate(454%) hue-rotate(185deg) brightness(93%) contrast(96%), - codeColor: #e83e8c, + codeColor: $code-color-nord, ), solarizedDark: ( textColor: $solarizedDarkBase2, @@ -307,7 +311,7 @@ $themes: ( hue-rotate(138deg) brightness(92%) contrast(90%), webkitCalendarPickerHoverFilter: brightness(0) saturate(100%) invert(94%) sepia(10%) saturate(462%) hue-rotate(345deg) brightness(103%) contrast(87%), - codeColor: #e83e8c, + codeColor: $code-color-dark, ), ); diff --git a/apps/desktop/src/scss/variables.scss b/apps/desktop/src/scss/variables.scss index 3ad4c0f0754..e4a2f124768 100644 --- a/apps/desktop/src/scss/variables.scss +++ b/apps/desktop/src/scss/variables.scss @@ -41,7 +41,9 @@ $button-color: lighten($text-color, 40%); $button-color-primary: darken($brand-primary, 8%); $button-color-danger: darken($brand-danger, 10%); -$code-color: #e83e8c; +$code-color: #c01176; +$code-color-dark: #f08dc7; +$code-color-nord: #dbb1d5; $themes: ( light: ( @@ -158,7 +160,7 @@ $themes: ( accountSwitcherTextColor: #ffffff, svgSuffix: "-dark.svg", hrColor: #bac0ce, - codeColor: $code-color, + codeColor: $code-color-dark, ), nord: ( textColor: $nord5, @@ -216,7 +218,7 @@ $themes: ( accountSwitcherTextColor: $nord5, svgSuffix: "-dark.svg", hrColor: $nord4, - codeColor: $code-color, + codeColor: $code-color-nord, ), ); diff --git a/apps/web/src/scss/variables.scss b/apps/web/src/scss/variables.scss index 719f403e385..af61daff512 100644 --- a/apps/web/src/scss/variables.scss +++ b/apps/web/src/scss/variables.scss @@ -88,6 +88,7 @@ $mfaTypes: 0, 2, 3, 4, 6; $lightDangerHover: #c43421; $lightInputColor: #465057; $lightInputPlaceholderColor: #b6b8b8; +$lightCodeColor: #c01176; // Dark @@ -107,6 +108,7 @@ $darkDarkBlue1: #2f343d; $darkDarkBlue2: #1f242e; $darkInputColor: $white; $darkInputPlaceholderColor: $darkGrey1; +$darkCodeColor: #f08dc7; $themes: ( light: ( @@ -167,7 +169,7 @@ $themes: ( calloutBackground: #fafafa, calloutColor: #212529, cdkDraggingBackground: $white, - codeColor: #e83e8c, + codeColor: $lightCodeColor, dropdownBackground: $white, dropdownHover: rgba(0, 0, 0, 0.06), dropdownTextColor: $body-color, @@ -276,7 +278,7 @@ $themes: ( calloutBackground: $darkBlue2, calloutColor: $white, cdkDraggingBackground: $darkDarkBlue1, - codeColor: #e83e8c, + codeColor: $darkCodeColor, dropdownBackground: $darkDarkBlue1, dropdownHover: rgba(255, 255, 255, 0.03), dropdownTextColor: $white, From a920d62dfeb3f339a366097376c8ef52d2360471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Pereira?= Date: Mon, 4 Sep 2023 21:01:16 +0100 Subject: [PATCH 2/3] [PM-3326] [CLI] Add minNumber, minSpecial and ambiguous password generation options (#5974) * feat(cli): add minNumber option and pass to generation service * feat(cli): add minSpecial option and pass to generation service * feat(cli): add ambiguous option and pass to generation service * refactor: extract utils to convert number and string options * feat(ts): add types to utils and options * feat: validate result of parsed value in convertNumberOption util --- apps/cli/src/program.ts | 3 +++ apps/cli/src/tools/generate.command.ts | 18 ++++++++++++++---- apps/cli/src/utils.ts | 16 ++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/apps/cli/src/program.ts b/apps/cli/src/program.ts index 9eca236a3a0..8bca024b410 100644 --- a/apps/cli/src/program.ts +++ b/apps/cli/src/program.ts @@ -298,9 +298,12 @@ export class Program { .option("-p, --passphrase", "Generate a passphrase.") .option("--length ", "Length of the password.") .option("--words ", "Number of words.") + .option("--minNumber ", "Minimum number of numeric characters.") + .option("--minSpecial ", "Minimum number of special characters.") .option("--separator ", "Word separator.") .option("-c, --capitalize", "Title case passphrase.") .option("--includeNumber", "Passphrase includes number.") + .option("--ambiguous", "Avoid ambiguous characters.") .on("--help", () => { writeLn("\n Notes:"); writeLn(""); diff --git a/apps/cli/src/tools/generate.command.ts b/apps/cli/src/tools/generate.command.ts index bd9ad88a04f..30436e7db71 100644 --- a/apps/cli/src/tools/generate.command.ts +++ b/apps/cli/src/tools/generate.command.ts @@ -1,5 +1,6 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; +import { PasswordGeneratorOptions } from "@bitwarden/common/tools/generator/password/password-generator-options"; import { Response } from "../models/response"; import { StringResponse } from "../models/response/string.response"; @@ -13,7 +14,7 @@ export class GenerateCommand { async run(cmdOptions: Record): Promise { const normalizedOptions = new Options(cmdOptions); - const options = { + const options: PasswordGeneratorOptions = { uppercase: normalizedOptions.uppercase, lowercase: normalizedOptions.lowercase, number: normalizedOptions.number, @@ -24,6 +25,9 @@ export class GenerateCommand { numWords: normalizedOptions.words, capitalize: normalizedOptions.capitalize, includeNumber: normalizedOptions.includeNumber, + minNumber: normalizedOptions.minNumber, + minSpecial: normalizedOptions.minSpecial, + ambiguous: normalizedOptions.ambiguous, }; const enforcedOptions = (await this.stateService.getIsAuthenticated()) @@ -47,6 +51,9 @@ class Options { words: number; capitalize: boolean; includeNumber: boolean; + minNumber: number; + minSpecial: number; + ambiguous: boolean; constructor(passedOptions: Record) { this.uppercase = CliUtils.convertBooleanOption(passedOptions?.uppercase); @@ -55,10 +62,13 @@ class Options { this.special = CliUtils.convertBooleanOption(passedOptions?.special); this.capitalize = CliUtils.convertBooleanOption(passedOptions?.capitalize); this.includeNumber = CliUtils.convertBooleanOption(passedOptions?.includeNumber); - this.length = passedOptions?.length != null ? parseInt(passedOptions?.length, null) : 14; + this.ambiguous = CliUtils.convertBooleanOption(passedOptions?.ambiguous); + this.length = CliUtils.convertNumberOption(passedOptions?.length, 14); this.type = passedOptions?.passphrase ? "passphrase" : "password"; - this.separator = passedOptions?.separator == null ? "-" : passedOptions.separator + ""; - this.words = passedOptions?.words != null ? parseInt(passedOptions.words, null) : 3; + this.separator = CliUtils.convertStringOption(passedOptions?.separator, "-"); + this.words = CliUtils.convertNumberOption(passedOptions?.words, 3); + this.minNumber = CliUtils.convertNumberOption(passedOptions?.minNumber, 1); + this.minSpecial = CliUtils.convertNumberOption(passedOptions?.minSpecial, 1); if (!this.uppercase && !this.lowercase && !this.special && !this.number) { this.lowercase = true; diff --git a/apps/cli/src/utils.ts b/apps/cli/src/utils.ts index f8780dbec63..5d77f6d3730 100644 --- a/apps/cli/src/utils.ts +++ b/apps/cli/src/utils.ts @@ -253,4 +253,20 @@ export class CliUtils { static convertBooleanOption(optionValue: any) { return optionValue || optionValue === "" ? true : false; } + + static convertNumberOption(optionValue: any, defaultValue: number) { + try { + if (optionValue != null) { + const numVal = parseInt(optionValue); + return !Number.isNaN(numVal) ? numVal : defaultValue; + } + return defaultValue; + } catch { + return defaultValue; + } + } + + static convertStringOption(optionValue: any, defaultValue: string) { + return optionValue != null ? String(optionValue) : defaultValue; + } } From 182d5bf5ace604f93f17032b580efa399e3b862d Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Mon, 4 Sep 2023 22:07:14 -0400 Subject: [PATCH 3/3] [PM-3758] Handle user decryption options from pre-TDE server response (#6180) * Mapped pre-TDE server response to UserDecryptionOptions. * Updated logic on SsoLoginStrategy to match account. * Linting. * Adjusted tests. * Fixed tests. --- .../login-strategies/login.strategy.spec.ts | 11 +--- .../auth/login-strategies/login.strategy.ts | 4 +- .../sso-login.strategy.spec.ts | 56 ++++++++++++++++++- .../login-strategies/sso-login.strategy.ts | 24 +++++--- .../src/platform/models/domain/account.ts | 50 +++++++++++------ 5 files changed, 107 insertions(+), 38 deletions(-) diff --git a/libs/common/src/auth/login-strategies/login.strategy.spec.ts b/libs/common/src/auth/login-strategies/login.strategy.spec.ts index f7128b35dfb..735135f4061 100644 --- a/libs/common/src/auth/login-strategies/login.strategy.spec.ts +++ b/libs/common/src/auth/login-strategies/login.strategy.spec.ts @@ -41,10 +41,7 @@ import { IdentityCaptchaResponse } from "../models/response/identity-captcha.res import { IdentityTokenResponse } from "../models/response/identity-token.response"; import { IdentityTwoFactorResponse } from "../models/response/identity-two-factor.response"; import { MasterPasswordPolicyResponse } from "../models/response/master-password-policy.response"; -import { - IUserDecryptionOptionsServerResponse, - UserDecryptionOptionsResponse, -} from "../models/response/user-decryption-options/user-decryption-options.response"; +import { IUserDecryptionOptionsServerResponse } from "../models/response/user-decryption-options/user-decryption-options.response"; import { PasswordLogInStrategy } from "./password-login.strategy"; @@ -65,10 +62,6 @@ const name = "NAME"; const defaultUserDecryptionOptionsServerResponse: IUserDecryptionOptionsServerResponse = { HasMasterPassword: true, }; -const userDecryptionOptions = new UserDecryptionOptionsResponse( - defaultUserDecryptionOptionsServerResponse -); -const acctDecryptionOptions = AccountDecryptionOptions.fromResponse(userDecryptionOptions); const decodedToken = { sub: userId, @@ -197,7 +190,7 @@ describe("LogInStrategy", () => { }, }, keys: new AccountKeys(), - decryptionOptions: acctDecryptionOptions, + decryptionOptions: AccountDecryptionOptions.fromResponse(idTokenResponse), }) ); expect(messagingService.send).toHaveBeenCalledWith("loggedIn"); diff --git a/libs/common/src/auth/login-strategies/login.strategy.ts b/libs/common/src/auth/login-strategies/login.strategy.ts index 7bc83580164..6e51f215012 100644 --- a/libs/common/src/auth/login-strategies/login.strategy.ts +++ b/libs/common/src/auth/login-strategies/login.strategy.ts @@ -143,9 +143,7 @@ export abstract class LogInStrategy { }, }, keys: accountKeys, - decryptionOptions: AccountDecryptionOptions.fromResponse( - tokenResponse.userDecryptionOptions - ), + decryptionOptions: AccountDecryptionOptions.fromResponse(tokenResponse), adminAuthRequest: adminAuthRequest?.toJSON(), }) ); diff --git a/libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts b/libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts index 099a3a02a2b..f078a7b86b1 100644 --- a/libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts +++ b/libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts @@ -266,7 +266,61 @@ describe("SsoLogInStrategy", () => { describe("Key Connector", () => { let tokenResponse: IdentityTokenResponse; beforeEach(() => { - tokenResponse = identityTokenResponseFactory(null, { HasMasterPassword: false }); + tokenResponse = identityTokenResponseFactory(null, { + HasMasterPassword: false, + KeyConnectorOption: { KeyConnectorUrl: keyConnectorUrl }, + }); + tokenResponse.keyConnectorUrl = keyConnectorUrl; + }); + + it("gets and sets the master key if Key Connector is enabled and the user doesn't have a master password", async () => { + const masterKey = new SymmetricCryptoKey( + new Uint8Array(64).buffer as CsprngArray + ) as MasterKey; + + apiService.postIdentityToken.mockResolvedValue(tokenResponse); + cryptoService.getMasterKey.mockResolvedValue(masterKey); + + await ssoLogInStrategy.logIn(credentials); + + expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl); + }); + + it("converts new SSO user with no master password to Key Connector on first login", async () => { + tokenResponse.key = null; + + apiService.postIdentityToken.mockResolvedValue(tokenResponse); + + await ssoLogInStrategy.logIn(credentials); + + expect(keyConnectorService.convertNewSsoUserToKeyConnector).toHaveBeenCalledWith( + tokenResponse, + ssoOrgId + ); + }); + + it("decrypts and sets the user key if Key Connector is enabled and the user doesn't have a master password", async () => { + const userKey = new SymmetricCryptoKey(new Uint8Array(64).buffer as CsprngArray) as UserKey; + const masterKey = new SymmetricCryptoKey( + new Uint8Array(64).buffer as CsprngArray + ) as MasterKey; + + apiService.postIdentityToken.mockResolvedValue(tokenResponse); + cryptoService.getMasterKey.mockResolvedValue(masterKey); + cryptoService.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); + + await ssoLogInStrategy.logIn(credentials); + + expect(cryptoService.decryptUserKeyWithMasterKey).toHaveBeenCalledWith(masterKey); + expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey); + }); + }); + + describe("Key Connector Pre-TDE", () => { + let tokenResponse: IdentityTokenResponse; + beforeEach(() => { + tokenResponse = identityTokenResponseFactory(); + tokenResponse.userDecryptionOptions = null; tokenResponse.keyConnectorUrl = keyConnectorUrl; }); diff --git a/libs/common/src/auth/login-strategies/sso-login.strategy.ts b/libs/common/src/auth/login-strategies/sso-login.strategy.ts index 3e9a7e33f33..09dbca72fea 100644 --- a/libs/common/src/auth/login-strategies/sso-login.strategy.ts +++ b/libs/common/src/auth/login-strategies/sso-login.strategy.ts @@ -101,16 +101,22 @@ export class SsoLogInStrategy extends LogInStrategy { private shouldSetMasterKeyFromKeyConnector(tokenResponse: IdentityTokenResponse): boolean { const userDecryptionOptions = tokenResponse?.userDecryptionOptions; - // If the user has a master password, this means that they need to migrate to Key Connector, so we won't set the key here. - // We default to false here because old server versions won't have hasMasterPassword and in that case we want to rely solely on the keyConnectorUrl. - // TODO: remove null default after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537) - const userHasMasterPassword = userDecryptionOptions?.hasMasterPassword ?? false; + if (userDecryptionOptions != null) { + const userHasMasterPassword = userDecryptionOptions.hasMasterPassword; + const userHasKeyConnectorUrl = + userDecryptionOptions.keyConnectorOption?.keyConnectorUrl != null; - const keyConnectorUrl = this.getKeyConnectorUrl(tokenResponse); - - // In order for us to set the master key from Key Connector, we need to have a Key Connector URL - // and the user must not have a master password. - return keyConnectorUrl != null && !userHasMasterPassword; + // In order for us to set the master key from Key Connector, we need to have a Key Connector URL + // and the user must not have a master password. + return userHasKeyConnectorUrl && !userHasMasterPassword; + } else { + // In pre-TDE versions of the server, the userDecryptionOptions will not be present. + // In this case, we can determine if the user has a master password and has a Key Connector URL by + // just checking the keyConnectorUrl property. This is because the server short-circuits on the response + // and will not pass back the URL in the response if the user has a master password. + // TODO: remove compatibility check after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537) + return tokenResponse.keyConnectorUrl != null; + } } private getKeyConnectorUrl(tokenResponse: IdentityTokenResponse): string { diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 95a5a899129..09dc6971dcf 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -11,7 +11,7 @@ import { EnvironmentUrls } from "../../../auth/models/domain/environment-urls"; import { ForceResetPasswordReason } from "../../../auth/models/domain/force-reset-password-reason"; import { KeyConnectorUserDecryptionOption } from "../../../auth/models/domain/user-decryption-options/key-connector-user-decryption-option"; import { TrustedDeviceUserDecryptionOption } from "../../../auth/models/domain/user-decryption-options/trusted-device-user-decryption-option"; -import { UserDecryptionOptionsResponse } from "../../../auth/models/response/user-decryption-options/user-decryption-options.response"; +import { IdentityTokenResponse } from "../../../auth/models/response/identity-token.response"; import { KdfType, UriMatchType } from "../../../enums"; import { EventData } from "../../../models/data/event.data"; import { GeneratedPasswordHistory } from "../../../tools/generator/password"; @@ -311,28 +311,46 @@ export class AccountDecryptionOptions { // return this.keyConnectorOption !== null && this.keyConnectorOption !== undefined; // } - static fromResponse(response: UserDecryptionOptionsResponse): AccountDecryptionOptions { + static fromResponse(response: IdentityTokenResponse): AccountDecryptionOptions { if (response == null) { return null; } const accountDecryptionOptions = new AccountDecryptionOptions(); - accountDecryptionOptions.hasMasterPassword = response.hasMasterPassword; - if (response.trustedDeviceOption) { - accountDecryptionOptions.trustedDeviceOption = new TrustedDeviceUserDecryptionOption( - response.trustedDeviceOption.hasAdminApproval, - response.trustedDeviceOption.hasLoginApprovingDevice, - response.trustedDeviceOption.hasManageResetPasswordPermission - ); + if (response.userDecryptionOptions) { + // If the response has userDecryptionOptions, this means it's on a post-TDE server version and can interrogate + // the new decryption options. + const responseOptions = response.userDecryptionOptions; + accountDecryptionOptions.hasMasterPassword = responseOptions.hasMasterPassword; + + if (responseOptions.trustedDeviceOption) { + accountDecryptionOptions.trustedDeviceOption = new TrustedDeviceUserDecryptionOption( + responseOptions.trustedDeviceOption.hasAdminApproval, + responseOptions.trustedDeviceOption.hasLoginApprovingDevice, + responseOptions.trustedDeviceOption.hasManageResetPasswordPermission + ); + } + + if (responseOptions.keyConnectorOption) { + accountDecryptionOptions.keyConnectorOption = new KeyConnectorUserDecryptionOption( + responseOptions.keyConnectorOption.keyConnectorUrl + ); + } + } else { + // If the response does not have userDecryptionOptions, this means it's on a pre-TDE server version and so + // we must base our decryption options on the presence of the keyConnectorUrl. + // Note that the presence of keyConnectorUrl implies that the user does not have a master password, as in pre-TDE + // server versions, a master password short-circuited the addition of the keyConnectorUrl to the response. + // TODO: remove this check after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537) + const usingKeyConnector = response.keyConnectorUrl != null; + accountDecryptionOptions.hasMasterPassword = !usingKeyConnector; + if (usingKeyConnector) { + accountDecryptionOptions.keyConnectorOption = new KeyConnectorUserDecryptionOption( + response.keyConnectorUrl + ); + } } - - if (response.keyConnectorOption) { - accountDecryptionOptions.keyConnectorOption = new KeyConnectorUserDecryptionOption( - response.keyConnectorOption.keyConnectorUrl - ); - } - return accountDecryptionOptions; }