Merge branch 'EC-598-beeep-properly-store-passkeys-in-bitwarden' into PM-2207

libs/angular/src/auth/components/environment-selector.component.html

@@ -81,7 +81,7 @@
         ></i>
         <i
           class="bwi bwi-fw bwi-md bwi-pencil-square"
-          style="padding-bottom: 1px; margin-right: 5px"
+          style="padding-bottom: 1px"
           aria-hidden="true"
         ></i>
         <span>{{ "selfHosted" | i18n }}</span>

libs/angular/src/services/jslib-services.module.ts

@@ -572,8 +572,6 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction";
         LogService,
         OrganizationServiceAbstraction,
         CryptoFunctionServiceAbstraction,
-        SyncNotifierServiceAbstraction,
-        MessagingServiceAbstraction,
         LOGOUT_CALLBACK,
       ],
     },

libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts

@@ -6,7 +6,7 @@ import { OrganizationData } from "../../models/data/organization.data";
 import { Organization } from "../../models/domain/organization";
 
 export function canAccessVaultTab(org: Organization): boolean {
-  return org.canViewAssignedCollections || org.canViewAllCollections || org.canManageGroups;
+  return org.canViewAssignedCollections || org.canViewAllCollections;
 }
 
 export function canAccessSettingsTab(org: Organization): boolean {

libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts

@@ -266,11 +266,11 @@ describe("SsoLogInStrategy", () => {
   describe("Key Connector", () => {
     let tokenResponse: IdentityTokenResponse;
     beforeEach(() => {
-      tokenResponse = identityTokenResponseFactory();
+      tokenResponse = identityTokenResponseFactory(null, { HasMasterPassword: false });
       tokenResponse.keyConnectorUrl = keyConnectorUrl;
     });
 
-    it("gets and sets the master key if Key Connector is enabled", async () => {
+    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;
@@ -283,7 +283,7 @@ describe("SsoLogInStrategy", () => {
       expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl);
     });
 
-    it("converts new SSO user to Key Connector on first login", async () => {
+    it("converts new SSO user with no master password to Key Connector on first login", async () => {
       tokenResponse.key = null;
 
       apiService.postIdentityToken.mockResolvedValue(tokenResponse);
@@ -296,7 +296,7 @@ describe("SsoLogInStrategy", () => {
       );
     });
 
-    it("decrypts and sets the user key if Key Connector is enabled", async () => {
+    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

libs/common/src/auth/login-strategies/sso-login.strategy.ts

@@ -77,19 +77,50 @@ export class SsoLogInStrategy extends LogInStrategy {
   }
 
   protected override async setMasterKey(tokenResponse: IdentityTokenResponse) {
-    // TODO: discuss how this is no longer true with TDE
-    // eventually we’ll need to support migration of existing TDE users to Key Connector
-    const newSsoUser = tokenResponse.key == null;
-
-    if (tokenResponse.keyConnectorUrl != null) {
-      if (!newSsoUser) {
-        await this.keyConnectorService.setMasterKeyFromUrl(tokenResponse.keyConnectorUrl);
-      } else {
+    // The only way we can be setting a master key at this point is if we are using Key Connector.
+    // First, check to make sure that we should do so based on the token response.
+    if (this.shouldSetMasterKeyFromKeyConnector(tokenResponse)) {
+      // If we're here, we know that the user should use Key Connector (they have a KeyConnectorUrl) and does not have a master password.
+      // We can now check the key on the token response to see whether they are a brand new user or an existing user.
+      // The presence of a masterKeyEncryptedUserKey indicates that the user has already been provisioned in Key Connector.
+      const newSsoUser = tokenResponse.key == null;
+      if (newSsoUser) {
         await this.keyConnectorService.convertNewSsoUserToKeyConnector(tokenResponse, this.orgId);
+      } else {
+        const keyConnectorUrl = this.getKeyConnectorUrl(tokenResponse);
+        await this.keyConnectorService.setMasterKeyFromUrl(keyConnectorUrl);
       }
     }
   }
 
+  /**
+   * Determines if it is possible set the `masterKey` from Key Connector.
+   * @param tokenResponse
+   * @returns `true` if the master key can be set from Key Connector, `false` otherwise
+   */
+  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;
+
+    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;
+  }
+
+  private getKeyConnectorUrl(tokenResponse: IdentityTokenResponse): string {
+    // TODO: remove tokenResponse.keyConnectorUrl reference after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537)
+    const userDecryptionOptions = tokenResponse?.userDecryptionOptions;
+    return (
+      tokenResponse.keyConnectorUrl ?? userDecryptionOptions?.keyConnectorOption?.keyConnectorUrl
+    );
+  }
+
   // TODO: future passkey login strategy will need to support setting user key (decrypting via TDE or admin approval request)
   // so might be worth moving this logic to a common place (base login strategy or a separate service?)
   protected override async setUserKey(tokenResponse: IdentityTokenResponse): Promise<void> {
@@ -117,9 +148,8 @@ export class SsoLogInStrategy extends LogInStrategy {
         await this.trySetUserKeyWithDeviceKey(tokenResponse);
       }
     } else if (
-      // TODO: remove tokenResponse.keyConnectorUrl when it's deprecated
       masterKeyEncryptedUserKey != null &&
-      (tokenResponse.keyConnectorUrl || userDecryptionOptions?.keyConnectorOption?.keyConnectorUrl)
+      this.getKeyConnectorUrl(tokenResponse) != null
     ) {
       // Key connector enabled for user
       await this.trySetUserKeyWithMasterKey();
@@ -208,12 +238,15 @@ export class SsoLogInStrategy extends LogInStrategy {
   private async trySetUserKeyWithMasterKey(): Promise<void> {
     const masterKey = await this.cryptoService.getMasterKey();
 
+    // There is a scenario in which the master key is not set here. That will occur if the user
+    // has a master password and is using Key Connector. In that case, we cannot set the master key
+    // because the user hasn't entered their master password yet.
+    // Instead, we'll return here and let the migration to Key Connector handle setting the master key.
     if (!masterKey) {
-      throw new Error("Master key not found");
+      return;
     }
 
     const userKey = await this.cryptoService.decryptUserKeyWithMasterKey(masterKey);
-
     await this.cryptoService.setUserKey(userKey);
   }
 

libs/common/src/auth/services/key-connector.service.ts

@@ -24,7 +24,7 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
     private logService: LogService,
     private organizationService: OrganizationService,
     private cryptoFunctionService: CryptoFunctionService,
-    private logoutCallback: (expired: boolean, userId?: string) => void
+    private logoutCallback: (expired: boolean, userId?: string) => Promise<void>
   ) {}
 
   setUsesKeyConnector(usesKeyConnector: boolean) {
@@ -84,7 +84,15 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
   }
 
   async convertNewSsoUserToKeyConnector(tokenResponse: IdentityTokenResponse, orgId: string) {
-    const { kdf, kdfIterations, kdfMemory, kdfParallelism, keyConnectorUrl } = tokenResponse;
+    // TODO: Remove after tokenResponse.keyConnectorUrl is deprecated in 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537)
+    const {
+      kdf,
+      kdfIterations,
+      kdfMemory,
+      kdfParallelism,
+      keyConnectorUrl: legacyKeyConnectorUrl,
+      userDecryptionOptions,
+    } = tokenResponse;
     const password = await this.cryptoFunctionService.randomBytes(64);
     const kdfConfig = new KdfConfig(kdfIterations, kdfMemory, kdfParallelism);
 
@@ -104,6 +112,8 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
     const [pubKey, privKey] = await this.cryptoService.makeKeyPair();
 
     try {
+      const keyConnectorUrl =
+        legacyKeyConnectorUrl ?? userDecryptionOptions?.keyConnectorOption?.keyConnectorUrl;
       await this.apiService.postUserKeyToKeyConnector(keyConnectorUrl, keyConnectorRequest);
     } catch (e) {
       this.handleKeyConnectorError(e);

libs/components/src/multi-select/multi-select.component.html

@@ -11,7 +11,6 @@
   notFoundText="{{ 'multiSelectNotFound' | i18n }}"
   clearAllText="{{ 'multiSelectClearAll' | i18n }}"
   [multiple]="true"
-  [selectOnTab]="true"
   [closeOnSelect]="false"
   (close)="onDropdownClosed()"
   [disabled]="disabled"

libs/components/src/multi-select/multi-select.component.ts

@@ -75,12 +75,6 @@ export class MultiSelectComponent implements OnInit, BitFormFieldControl, Contro
       return false;
     }
 
-    if (this.select.isOpen && event.key === "Enter" && !hasModifierKey(event)) {
-      this.select.close();
-      event.preventDefault();
-      return false;
-    }
-
     if (this.select.isOpen && event.key === "Escape" && !hasModifierKey(event)) {
       this.selectedItems = [];
       this.select.close();

libs/importer/spec/psono-json-importer.spec.ts

@@ -11,6 +11,7 @@ import { EnvVariablesData } from "./test-data/psono-json/environment-variables";
 import { FoldersTestData } from "./test-data/psono-json/folders";
 import { GPGData } from "./test-data/psono-json/gpg";
 import { NotesData } from "./test-data/psono-json/notes";
+import { ReducedWebsiteLoginsData } from "./test-data/psono-json/reduced-website-logins";
 import { TOTPData } from "./test-data/psono-json/totp";
 import { WebsiteLoginsData } from "./test-data/psono-json/website-logins";
 
@@ -23,6 +24,7 @@ function validateCustomField(
   expect(fields).not.toBeUndefined();
   const customField = fields.find((f) => f.name === fieldName);
   expect(customField).not.toBeNull();
+  expect(customField).not.toBeUndefined();
 
   expect(customField.value).toEqual(expectedValue);
   expect(customField.type).toEqual(fieldType);
@@ -38,6 +40,7 @@ describe("PSONO JSON Importer", () => {
   const FoldersTestDataJson = JSON.stringify(FoldersTestData);
   const GPGDataJson = JSON.stringify(GPGData);
   const EnvVariablesDataJson = JSON.stringify(EnvVariablesData);
+  const ReducedWebsiteLoginsDataJson = JSON.stringify(ReducedWebsiteLoginsData);
 
   it("should parse Website/Password data", async () => {
     const importer = new PsonoJsonImporter();
@@ -64,6 +67,23 @@ describe("PSONO JSON Importer", () => {
     validateCustomField(cipher.fields, "callback_user", "callbackUser");
     validateCustomField(cipher.fields, "callback_pass", "callbackPassword");
   });
+  it("should parse Website/Password data with missing fields", async () => {
+    const importer = new PsonoJsonImporter();
+    const result = await importer.parse(ReducedWebsiteLoginsDataJson);
+    expect(result != null).toBe(true);
+
+    const cipher = result.ciphers.shift();
+
+    expect(cipher.type).toEqual(CipherType.Login);
+    expect(cipher.name).toEqual("export_website_1");
+    expect(cipher.login.username).toEqual("username123");
+    expect(cipher.login.password).toEqual("password123");
+    expect(cipher.login.uris).toEqual(null);
+
+    expect(cipher.fields.length).toBe(2);
+    validateCustomField(cipher.fields, "create_date", "2022-09-10T23:05:02.351417Z");
+    validateCustomField(cipher.fields, "write_date", "2022-09-10T23:05:02.351583Z");
+  });
 
   it("should parse Application Password data", async () => {
     const importer = new PsonoJsonImporter();

libs/importer/spec/test-data/psono-json/reduced-website-logins.ts

@@ -0,0 +1,21 @@
+import { PsonoJsonExport } from "../../../src/importers/psono/psono-json-types";
+
+export const ReducedWebsiteLoginsData: PsonoJsonExport = {
+  folders: [],
+  items: [
+    {
+      type: "website_password",
+      name: "export_website_name",
+      website_password_password: "password123",
+      website_password_username: "username123",
+      website_password_notes: "",
+      website_password_url: "",
+      website_password_title: "export_website_1",
+      create_date: "2022-09-10T23:05:02.351417Z",
+      write_date: "2022-09-10T23:05:02.351583Z",
+      callback_url: "",
+      callback_user: "",
+      callback_pass: "",
+    },
+  ],
+};

libs/importer/src/importers/psono/psono-json-importer.ts

@@ -131,7 +131,7 @@ export class PsonoJsonImporter extends BaseImporter implements Importer {
     this.processKvp(
       cipher,
       "website_password_auto_submit",
-      entry.website_password_auto_submit.toString(),
+      entry.website_password_auto_submit?.toString(),
       FieldType.Boolean
     );
 

libs/importer/src/importers/psono/psono-json-types.ts

@@ -38,15 +38,15 @@ export type PsonoEntryTypes =
 
 export interface WebsitePasswordEntry extends RecordBase {
   type: "website_password";
-  autosubmit: boolean;
-  urlfilter: string;
+  autosubmit?: boolean;
+  urlfilter?: string;
   website_password_title: string;
   website_password_url: string;
   website_password_username: string;
   website_password_password: string;
   website_password_notes: string;
-  website_password_auto_submit: boolean;
-  website_password_url_filter: string;
+  website_password_auto_submit?: boolean;
+  website_password_url_filter?: string;
 }
 
 export interface PsonoEntry {