From 052b3be2eb30c47086ec5aa5319c969133874819 Mon Sep 17 00:00:00 2001 From: Jake Fink Date: Wed, 3 Jul 2024 09:53:40 -0400 Subject: [PATCH 1/9] [PM-7972] Account switching integration with "remember email" functionality (#9750) * add account switching logic to login email service * enforce boolean and fix desktop account switcher order --- .../browser/src/background/main.background.ts | 6 - .../app/layout/account-switcher.component.ts | 5 +- .../src/auth/components/login.component.ts | 2 +- .../src/services/jslib-services.module.ts | 2 +- .../abstractions/login-email.service.ts | 24 +-- .../login-email/login-email.service.spec.ts | 150 ++++++++++++++++++ .../login-email/login-email.service.ts | 68 ++++++-- 7 files changed, 225 insertions(+), 32 deletions(-) create mode 100644 libs/auth/src/common/services/login-email/login-email.service.spec.ts diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 7e2b802d346..8de3014fb2c 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -8,7 +8,6 @@ import { AuthRequestServiceAbstraction, AuthRequestService, LoginEmailServiceAbstraction, - LoginEmailService, LogoutReason, } from "@bitwarden/auth/common"; import { ApiService as ApiServiceAbstraction } from "@bitwarden/common/abstractions/api.service"; @@ -708,8 +707,6 @@ export default class MainBackground { this.stateProvider, ); - this.loginEmailService = new LoginEmailService(this.stateProvider); - this.ssoLoginService = new SsoLoginService(this.stateProvider); this.userVerificationApiService = new UserVerificationApiService(this.apiService); @@ -1255,9 +1252,6 @@ export default class MainBackground { clearCaches(); if (userId == null) { - this.loginEmailService.setRememberEmail(false); - await this.loginEmailService.saveEmailSettings(); - await this.refreshBadge(); await this.refreshMenu(); await this.overlayBackground?.updateOverlayCiphers(); // null in popup only contexts diff --git a/apps/desktop/src/app/layout/account-switcher.component.ts b/apps/desktop/src/app/layout/account-switcher.component.ts index 92cfebfd605..ff27dacd963 100644 --- a/apps/desktop/src/app/layout/account-switcher.component.ts +++ b/apps/desktop/src/app/layout/account-switcher.component.ts @@ -165,13 +165,10 @@ export class AccountSwitcherComponent { async addAccount() { this.close(); - this.loginEmailService.setRememberEmail(false); - await this.loginEmailService.saveEmailSettings(); - - await this.router.navigate(["/login"]); const activeAccount = await firstValueFrom(this.accountService.activeAccount$); await this.stateService.clearDecryptedData(activeAccount?.id as UserId); await this.accountService.switchAccount(null); + await this.router.navigate(["/login"]); } private async createInactiveAccounts(baseAccounts: { diff --git a/libs/angular/src/auth/components/login.component.ts b/libs/angular/src/auth/components/login.component.ts index 4466a106a56..0d3c3a87071 100644 --- a/libs/angular/src/auth/components/login.component.ts +++ b/libs/angular/src/auth/components/login.component.ts @@ -126,7 +126,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit, let rememberEmail = this.loginEmailService.getRememberEmail(); - if (rememberEmail == null) { + if (!rememberEmail) { rememberEmail = (await firstValueFrom(this.loginEmailService.storedEmail$)) != null; } diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 86ccce4e004..880488cfcfb 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -970,7 +970,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: LoginEmailServiceAbstraction, useClass: LoginEmailService, - deps: [StateProvider], + deps: [AccountServiceAbstraction, AuthServiceAbstraction, StateProvider], }), safeProvider({ provide: OrgDomainInternalServiceAbstraction, diff --git a/libs/auth/src/common/abstractions/login-email.service.ts b/libs/auth/src/common/abstractions/login-email.service.ts index 89165af5431..d4fbbaff840 100644 --- a/libs/auth/src/common/abstractions/login-email.service.ts +++ b/libs/auth/src/common/abstractions/login-email.service.ts @@ -2,36 +2,40 @@ import { Observable } from "rxjs"; export abstract class LoginEmailServiceAbstraction { /** - * An observable that monitors the storedEmail + * An observable that monitors the storedEmail on disk. + * This will return null if an account is being added. */ - storedEmail$: Observable; + storedEmail$: Observable; /** - * Gets the current email being used in the login process. + * Gets the current email being used in the login process from memory. * @returns A string of the email. */ getEmail: () => string; /** - * Sets the current email being used in the login process. + * Sets the current email being used in the login process in memory. * @param email The email to be set. */ setEmail: (email: string) => void; /** - * Gets whether or not the email should be stored on disk. + * Gets from memory whether or not the email should be stored on disk when `saveEmailSettings` is called. * @returns A boolean stating whether or not the email should be stored on disk. */ getRememberEmail: () => boolean; /** - * Sets whether or not the email should be stored on disk. + * Sets in memory whether or not the email should be stored on disk when + * `saveEmailSettings` is called. */ setRememberEmail: (value: boolean) => void; /** - * Sets the email and rememberEmail properties to null. + * Sets the email and rememberEmail properties in memory to null. */ clearValues: () => void; /** - * - If rememberEmail is true, sets the storedEmail on disk to the current email. - * - If rememberEmail is false, sets the storedEmail on disk to null. - * - Then sets the email and rememberEmail properties to null. + * Saves or clears the email on disk + * - If an account is being added, only changes the stored email when rememberEmail is true. + * - If rememberEmail is true, sets the email on disk to the current email. + * - If rememberEmail is false, sets the email on disk to null. + * Always clears the email and rememberEmail properties from memory. * @returns A promise that resolves once the email settings are saved. */ saveEmailSettings: () => Promise; diff --git a/libs/auth/src/common/services/login-email/login-email.service.spec.ts b/libs/auth/src/common/services/login-email/login-email.service.spec.ts new file mode 100644 index 00000000000..190c9ecf16e --- /dev/null +++ b/libs/auth/src/common/services/login-email/login-email.service.spec.ts @@ -0,0 +1,150 @@ +import { MockProxy, mock } from "jest-mock-extended"; +import { BehaviorSubject, firstValueFrom } from "rxjs"; + +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { + FakeAccountService, + FakeGlobalState, + FakeStateProvider, + mockAccountServiceWith, +} from "@bitwarden/common/spec"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { LoginEmailService, STORED_EMAIL } from "./login-email.service"; + +describe("LoginEmailService", () => { + let sut: LoginEmailService; + + let accountService: FakeAccountService; + let authService: MockProxy; + let stateProvider: FakeStateProvider; + + const userId = "USER_ID" as UserId; + let storedEmailState: FakeGlobalState; + let mockAuthStatuses$: BehaviorSubject>; + + beforeEach(() => { + accountService = mockAccountServiceWith(userId); + authService = mock(); + stateProvider = new FakeStateProvider(accountService); + + storedEmailState = stateProvider.global.getFake(STORED_EMAIL); + + mockAuthStatuses$ = new BehaviorSubject>({}); + authService.authStatuses$ = mockAuthStatuses$; + + sut = new LoginEmailService(accountService, authService, stateProvider); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe("storedEmail$", () => { + it("returns the stored email when not adding an account", async () => { + sut.setEmail("userEmail@bitwarden.com"); + sut.setRememberEmail(true); + await sut.saveEmailSettings(); + + const result = await firstValueFrom(sut.storedEmail$); + + expect(result).toEqual("userEmail@bitwarden.com"); + }); + + it("returns the stored email when not adding an account and the user has just logged in", async () => { + sut.setEmail("userEmail@bitwarden.com"); + sut.setRememberEmail(true); + await sut.saveEmailSettings(); + + mockAuthStatuses$.next({ [userId]: AuthenticationStatus.Unlocked }); + // account service already initialized with userId as active user + + const result = await firstValueFrom(sut.storedEmail$); + + expect(result).toEqual("userEmail@bitwarden.com"); + }); + + it("returns null when adding an account", async () => { + sut.setEmail("userEmail@bitwarden.com"); + sut.setRememberEmail(true); + await sut.saveEmailSettings(); + + mockAuthStatuses$.next({ + [userId]: AuthenticationStatus.Unlocked, + ["OtherUserId" as UserId]: AuthenticationStatus.Locked, + }); + + const result = await firstValueFrom(sut.storedEmail$); + + expect(result).toBeNull(); + }); + }); + + describe("saveEmailSettings", () => { + it("saves the email when not adding an account", async () => { + sut.setEmail("userEmail@bitwarden.com"); + sut.setRememberEmail(true); + await sut.saveEmailSettings(); + + const result = await firstValueFrom(storedEmailState.state$); + + expect(result).toEqual("userEmail@bitwarden.com"); + }); + + it("clears the email when not adding an account and rememberEmail is false", async () => { + storedEmailState.stateSubject.next("initialEmail@bitwarden.com"); + + sut.setEmail("userEmail@bitwarden.com"); + sut.setRememberEmail(false); + await sut.saveEmailSettings(); + + const result = await firstValueFrom(storedEmailState.state$); + + expect(result).toBeNull(); + }); + + it("saves the email when adding an account", async () => { + mockAuthStatuses$.next({ + [userId]: AuthenticationStatus.Unlocked, + ["OtherUserId" as UserId]: AuthenticationStatus.Locked, + }); + + sut.setEmail("userEmail@bitwarden.com"); + sut.setRememberEmail(true); + await sut.saveEmailSettings(); + + const result = await firstValueFrom(storedEmailState.state$); + + expect(result).toEqual("userEmail@bitwarden.com"); + }); + + it("does not clear the email when adding an account and rememberEmail is false", async () => { + storedEmailState.stateSubject.next("initialEmail@bitwarden.com"); + + mockAuthStatuses$.next({ + [userId]: AuthenticationStatus.Unlocked, + ["OtherUserId" as UserId]: AuthenticationStatus.Locked, + }); + + sut.setEmail("userEmail@bitwarden.com"); + sut.setRememberEmail(false); + await sut.saveEmailSettings(); + + const result = await firstValueFrom(storedEmailState.state$); + + // result should not be null + expect(result).toEqual("initialEmail@bitwarden.com"); + }); + + it("clears the email and rememberEmail after saving", async () => { + sut.setEmail("userEmail@bitwarden.com"); + sut.setRememberEmail(true); + await sut.saveEmailSettings(); + + const result = sut.getEmail(); + + expect(result).toBeNull(); + }); + }); +}); diff --git a/libs/auth/src/common/services/login-email/login-email.service.ts b/libs/auth/src/common/services/login-email/login-email.service.ts index 171af07430e..3babc6f43cc 100644 --- a/libs/auth/src/common/services/login-email/login-email.service.ts +++ b/libs/auth/src/common/services/login-email/login-email.service.ts @@ -1,4 +1,8 @@ -import { Observable } from "rxjs"; +import { Observable, firstValueFrom, switchMap } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { GlobalState, @@ -8,20 +12,49 @@ import { } from "../../../../../common/src/platform/state"; import { LoginEmailServiceAbstraction } from "../../abstractions/login-email.service"; -const STORED_EMAIL = new KeyDefinition(LOGIN_EMAIL_DISK, "storedEmail", { +export const STORED_EMAIL = new KeyDefinition(LOGIN_EMAIL_DISK, "storedEmail", { deserializer: (value: string) => value, }); export class LoginEmailService implements LoginEmailServiceAbstraction { - private email: string; + private email: string | null; private rememberEmail: boolean; - private readonly storedEmailState: GlobalState; - storedEmail$: Observable; + // True if an account is currently being added through account switching + private readonly addingAccount$: Observable; - constructor(private stateProvider: StateProvider) { + private readonly storedEmailState: GlobalState; + storedEmail$: Observable; + + constructor( + private accountService: AccountService, + private authService: AuthService, + private stateProvider: StateProvider, + ) { this.storedEmailState = this.stateProvider.getGlobal(STORED_EMAIL); - this.storedEmail$ = this.storedEmailState.state$; + + // In order to determine if an account is being added, we check if any account is not logged out + this.addingAccount$ = this.authService.authStatuses$.pipe( + switchMap(async (statuses) => { + // We don't want to consider the active account since it may have just changed auth status to logged in + // which would make this observable think an account is being added + const activeUser = await firstValueFrom(this.accountService.activeAccount$); + if (activeUser) { + delete statuses[activeUser.id]; + } + return Object.values(statuses).some((status) => status !== AuthenticationStatus.LoggedOut); + }), + ); + + this.storedEmail$ = this.storedEmailState.state$.pipe( + switchMap(async (storedEmail) => { + // When adding an account, we don't show the stored email + if (await firstValueFrom(this.addingAccount$)) { + return null; + } + return storedEmail; + }), + ); } getEmail() { @@ -37,16 +70,31 @@ export class LoginEmailService implements LoginEmailServiceAbstraction { } setRememberEmail(value: boolean) { - this.rememberEmail = value; + this.rememberEmail = value ?? false; } clearValues() { this.email = null; - this.rememberEmail = null; + this.rememberEmail = false; } async saveEmailSettings() { - await this.storedEmailState.update(() => (this.rememberEmail ? this.email : null)); + const addingAccount = await firstValueFrom(this.addingAccount$); + await this.storedEmailState.update((storedEmail) => { + // If we're adding an account, only overwrite the stored email when rememberEmail is true + if (addingAccount) { + if (this.rememberEmail) { + return this.email; + } + return storedEmail; + } + + // Logging in with rememberEmail set to false will clear the stored email + if (this.rememberEmail) { + return this.email; + } + return null; + }); this.clearValues(); } } From 9d060be48ca5a9db50e9a62d77469f94256161ae Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 3 Jul 2024 16:06:55 +0200 Subject: [PATCH 2/9] [PM-9442] Add tests for undefined state values and proper emulation of ElectronStorageService in tests (#9910) * fix: handle undefined value in migration 66 * fix: the if-statement was typo * feat: duplicate error behavior in fake storage service * feat: fix all migrations that were setting undefined values * feat: add test for disabled fingrint in migration 66 * fix: default single user state saving undefined value to state * revert: awaiting floating promise gonna fix this in a separate PR * Revert "feat: fix all migrations that were setting undefined values" This reverts commit 034713256cee9a8e164295c88157fe33d8372c81. * feat: automatically convert save to remove * Revert "fix: default single user state saving undefined value to state" This reverts commit 6c36da6ba52f6886d0de2b502b3aaff7f122c3a7. --- .../services/electron-storage.service.ts | 5 +++ libs/common/spec/fake-storage.service.ts | 41 ++++++++++++++++++- .../66-move-final-desktop-settings.spec.ts | 39 ++++++++++++++++++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/platform/services/electron-storage.service.ts b/apps/desktop/src/platform/services/electron-storage.service.ts index dc145fc2f7e..3fa9b2220c5 100644 --- a/apps/desktop/src/platform/services/electron-storage.service.ts +++ b/apps/desktop/src/platform/services/electron-storage.service.ts @@ -82,9 +82,14 @@ export class ElectronStorageService implements AbstractStorageService { } save(key: string, obj: unknown): Promise { + if (obj === undefined) { + return this.remove(key); + } + if (obj instanceof Set) { obj = Array.from(obj); } + this.store.set(key, obj); this.updatesSubject.next({ key, updateType: "save" }); return Promise.resolve(); diff --git a/libs/common/spec/fake-storage.service.ts b/libs/common/spec/fake-storage.service.ts index f4817ff5817..c6d989c5abf 100644 --- a/libs/common/spec/fake-storage.service.ts +++ b/libs/common/spec/fake-storage.service.ts @@ -8,6 +8,8 @@ import { } from "../src/platform/abstractions/storage.service"; import { StorageOptions } from "../src/platform/models/domain/storage-options"; +const INTERNAL_KEY = "__internal__"; + export class FakeStorageService implements AbstractStorageService, ObservableStorageService { private store: Record; private updatesSubject = new Subject(); @@ -63,13 +65,32 @@ export class FakeStorageService implements AbstractStorageService, ObservableSto this.mock.has(key, options); return Promise.resolve(this.store[key] != null); } - save(key: string, obj: T, options?: StorageOptions): Promise { + async save(key: string, obj: T, options?: StorageOptions): Promise { + // These exceptions are copied from https://github.com/sindresorhus/conf/blob/608adb0c46fb1680ddbd9833043478367a64c120/source/index.ts#L193-L203 + // which is a library that is used by `ElectronStorageService`. We add them here to ensure that the behavior in our testing mirrors the real world. + if (typeof key !== "string" && typeof key !== "object") { + throw new TypeError( + `Expected \`key\` to be of type \`string\` or \`object\`, got ${typeof key}`, + ); + } + + // We don't throw this error because ElectronStorageService automatically detects this case + // and calls `delete()` instead of `set()`. + // if (typeof key !== "object" && obj === undefined) { + // throw new TypeError("Use `delete()` to clear values"); + // } + + if (this._containsReservedKey(key)) { + throw new TypeError( + `Please don't use the ${INTERNAL_KEY} key, as it's used to manage this module internal operations.`, + ); + } + // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises this.mock.save(key, obj, options); this.store[key] = obj; this.updatesSubject.next({ key: key, updateType: "save" }); - return Promise.resolve(); } remove(key: string, options?: StorageOptions): Promise { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. @@ -79,4 +100,20 @@ export class FakeStorageService implements AbstractStorageService, ObservableSto this.updatesSubject.next({ key: key, updateType: "remove" }); return Promise.resolve(); } + + private _containsReservedKey(key: string | Partial): boolean { + if (typeof key === "object") { + const firsKey = Object.keys(key)[0]; + + if (firsKey === INTERNAL_KEY) { + return true; + } + } + + if (typeof key !== "string") { + return false; + } + + return false; + } } diff --git a/libs/common/src/state-migrations/migrations/66-move-final-desktop-settings.spec.ts b/libs/common/src/state-migrations/migrations/66-move-final-desktop-settings.spec.ts index 19be45640fb..5140260f729 100644 --- a/libs/common/src/state-migrations/migrations/66-move-final-desktop-settings.spec.ts +++ b/libs/common/src/state-migrations/migrations/66-move-final-desktop-settings.spec.ts @@ -92,6 +92,45 @@ describe("MoveDesktopSettings", () => { global_desktopSettings_browserIntegrationFingerprintEnabled: false, }, }, + { + it: "migrates browser integration without fingerprint enabled", + preMigration: { + global_account_accounts: { + user1: {}, + otherUser: {}, + }, + user1: { + settings: { + minimizeOnCopyToClipboard: false, + }, + }, + otherUser: { + settings: { + random: "stuff", + }, + }, + global: { + enableBrowserIntegration: true, + }, + }, + postMigration: { + global_account_accounts: { + user1: {}, + otherUser: {}, + }, + global: {}, + user1: { + settings: {}, + }, + otherUser: { + settings: { + random: "stuff", + }, + }, + global_desktopSettings_browserIntegrationEnabled: true, + user_user1_desktopSettings_minimizeOnCopy: false, + }, + }, { it: "does not move non-existant values", preMigration: { From d4eeeb8ed2879c3c9e5e5e2f7ae5ccbd4063a8a4 Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Wed, 3 Jul 2024 10:33:43 -0400 Subject: [PATCH 3/9] [AC-2805] Consolidated Billing UI Updates (#9893) * Add empty state for invoices * Make cards on create client dialog tabbable * Add space in $ / month per member * Mute text, remove (Monthly) and right align menu on clients table * Made used seats account for all users and fixed column sort for used/remaining * Resize pricing cards * Rename assignedSeats to occupiedSeats --- apps/web/src/locales/en/messages.json | 12 ++++--- .../providers/providers.module.ts | 2 +- .../create-client-dialog.component.html | 5 +-- .../clients/manage-clients.component.html | 16 ++++----- .../clients/manage-clients.component.ts | 2 ++ .../src/app/billing/providers/index.ts | 1 + libs/angular/src/billing/components/index.ts | 1 + .../invoices/invoices.component.html | 3 ++ .../invoices/no-invoices.component.ts | 36 +++++++++++++++++++ libs/angular/src/jslib.module.ts | 4 +++ .../provider-organization.response.ts | 4 +++ 11 files changed, 71 insertions(+), 15 deletions(-) create mode 100644 libs/angular/src/billing/components/invoices/no-invoices.component.ts diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 8462697973c..60b5b313ac0 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -8511,14 +8511,18 @@ "downloadCSV": { "message": "Download CSV" }, - "billingHistoryDescription": { - "message": "Download a CSV to obtain client details for each billing date. Prorated charges are not included in the CSV and may vary from the linked invoice. For the most accurate billing details, refer to your monthly invoices.", - "description": "A paragraph on the Billing History page of the Provider Portal letting users know they can download a CSV report for their invoices that does not include prorations." - }, "monthlySubscriptionUserSeatsMessage": { "message": "Adjustments to your subscription will result in prorated charges to your billing totals on your next billing period. " }, "annualSubscriptionUserSeatsMessage": { "message": "Adjustments to your subscription will result in prorated charges on a monthly billing cycle. " + }, + "billingHistoryDescription": { + "message": "Download a CSV to obtain client details for each billing date. Prorated charges are not included in the CSV and may vary from the linked invoice. For the most accurate billing details, refer to your monthly invoices.", + "description": "A paragraph on the Billing History page of the Provider Portal letting users know they can download a CSV report for their invoices that does not include prorations." + }, + "noInvoicesToList": { + "message": "There are no invoices to list", + "description": "A paragraph on the Billing History page of the Provider Portal letting users know they can download a CSV report for their invoices that does not include prorations." } } diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts index 46adf44d5a3..d17c973181f 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts @@ -19,8 +19,8 @@ import { ProviderPaymentMethodComponent, ProviderSelectPaymentMethodDialogComponent, ProviderSubscriptionComponent, + ProviderSubscriptionStatusComponent, } from "../../billing/providers"; -import { ProviderSubscriptionStatusComponent } from "../../billing/providers/subscription/provider-subscription-status.component"; import { AddOrganizationComponent } from "./clients/add-organization.component"; import { ClientsComponent } from "./clients/clients.component"; diff --git a/bitwarden_license/bit-web/src/app/billing/providers/clients/create-client-dialog.component.html b/bitwarden_license/bit-web/src/app/billing/providers/clients/create-client-dialog.component.html index 8a22e964d43..c4b5ec40462 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/clients/create-client-dialog.component.html +++ b/bitwarden_license/bit-web/src/app/billing/providers/clients/create-client-dialog.component.html @@ -17,6 +17,7 @@ *ngFor="let planCard of planCards" [ngClass]="getPlanCardContainerClasses(planCard.selected)" (click)="selectPlan(planCard.name)" + tabindex="0" >
{{ "selected" | i18n }}
-
+

{{ planCard.name }}

{{ planCard.cost | currency: "$" }} - /{{ "monthPerMember" | i18n }} + / {{ "monthPerMember" | i18n }}
diff --git a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.html b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.html index 2468f9df1af..caf07e49734 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.html +++ b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.html @@ -28,8 +28,8 @@ {{ "client" | i18n }} {{ "assigned" | i18n }} - {{ "used" | i18n }} - {{ "remaining" | i18n }} + {{ "used" | i18n }} + {{ "remaining" | i18n }} {{ "billingPlan" | i18n }} @@ -47,18 +47,18 @@ - {{ client.seats }} + {{ client.seats }} - {{ client.userCount }} + {{ client.assignedSeats }} - {{ client.seats - client.userCount }} + {{ client.remainingSeats }} - - {{ client.plan }} + + {{ removeMonthly(client.plan) }} - + + + + + + {{ "loading" | i18n }} + + +

{{ "noGroupsInList" | i18n }}

+ + + + + + + + + + {{ "name" | i18n }} + {{ "collections" | i18n }} + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/apps/web/src/app/admin-console/organizations/manage/new-groups.component.ts b/apps/web/src/app/admin-console/organizations/manage/new-groups.component.ts new file mode 100644 index 00000000000..e5e99333e64 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/manage/new-groups.component.ts @@ -0,0 +1,255 @@ +import { Component } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; +import { FormControl } from "@angular/forms"; +import { ActivatedRoute } from "@angular/router"; +import { + BehaviorSubject, + combineLatest, + concatMap, + from, + lastValueFrom, + map, + switchMap, + tap, +} from "rxjs"; +import { debounceTime, first } from "rxjs/operators"; + +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { ListResponse } from "@bitwarden/common/models/response/list.response"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service"; +import { CollectionData } from "@bitwarden/common/vault/models/data/collection.data"; +import { Collection } from "@bitwarden/common/vault/models/domain/collection"; +import { + CollectionDetailsResponse, + CollectionResponse, +} from "@bitwarden/common/vault/models/response/collection.response"; +import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view"; +import { DialogService, TableDataSource, ToastService } from "@bitwarden/components"; + +import { InternalGroupService as GroupService, GroupView } from "../core"; + +import { + GroupAddEditDialogResultType, + GroupAddEditTabType, + openGroupAddEditDialog, +} from "./group-add-edit.component"; + +type GroupDetailsRow = { + /** + * Details used for displaying group information + */ + details: GroupView; + + /** + * True if the group is selected in the table + */ + checked?: boolean; + + /** + * A list of collection names the group has access to + */ + collectionNames?: string[]; +}; + +/** + * Custom filter predicate that filters the groups table by id and name only. + * This is required because the default implementation searches by all properties, which can unintentionally match + * with members' names (who are assigned to the group) or collection names (which the group has access to). + */ +const groupsFilter = (filter: string) => { + const transformedFilter = filter.trim().toLowerCase(); + return (data: GroupDetailsRow) => { + const group = data.details; + + return ( + group.id.toLowerCase().indexOf(transformedFilter) != -1 || + group.name.toLowerCase().indexOf(transformedFilter) != -1 + ); + }; +}; + +@Component({ + templateUrl: "new-groups.component.html", +}) +export class NewGroupsComponent { + loading = true; + organizationId: string; + + protected dataSource = new TableDataSource(); + protected searchControl = new FormControl(""); + + // Fixed sizes used for cdkVirtualScroll + protected rowHeight = 46; + protected rowHeightClass = `tw-h-[46px]`; + + protected ModalTabType = GroupAddEditTabType; + private refreshGroups$ = new BehaviorSubject(null); + + constructor( + private apiService: ApiService, + private groupService: GroupService, + private route: ActivatedRoute, + private i18nService: I18nService, + private dialogService: DialogService, + private logService: LogService, + private collectionService: CollectionService, + private toastService: ToastService, + ) { + this.route.params + .pipe( + tap((params) => (this.organizationId = params.organizationId)), + switchMap(() => + combineLatest([ + // collectionMap + from(this.apiService.getCollections(this.organizationId)).pipe( + concatMap((response) => this.toCollectionMap(response)), + ), + // groups + this.refreshGroups$.pipe( + switchMap(() => this.groupService.getAll(this.organizationId)), + ), + ]), + ), + map(([collectionMap, groups]) => { + return groups.map((g) => ({ + id: g.id, + name: g.name, + details: g, + checked: false, + collectionNames: g.collections + .map((c) => collectionMap[c.id]?.name) + .sort(this.i18nService.collator?.compare), + })); + }), + takeUntilDestroyed(), + ) + .subscribe((groups) => { + this.dataSource.data = groups; + this.loading = false; + }); + + // Connect the search input to the table dataSource filter input + this.searchControl.valueChanges + .pipe(debounceTime(200), takeUntilDestroyed()) + .subscribe((v) => (this.dataSource.filter = groupsFilter(v))); + + this.route.queryParams.pipe(first(), takeUntilDestroyed()).subscribe((qParams) => { + this.searchControl.setValue(qParams.search); + }); + } + + async edit( + group: GroupDetailsRow, + startingTabIndex: GroupAddEditTabType = GroupAddEditTabType.Info, + ) { + const dialogRef = openGroupAddEditDialog(this.dialogService, { + data: { + initialTab: startingTabIndex, + organizationId: this.organizationId, + groupId: group != null ? group.details.id : null, + }, + }); + + const result = await lastValueFrom(dialogRef.closed); + + if (result == GroupAddEditDialogResultType.Saved) { + this.refreshGroups$.next(); + } else if (result == GroupAddEditDialogResultType.Deleted) { + this.removeGroup(group); + } + } + + async add() { + await this.edit(null); + } + + async delete(groupRow: GroupDetailsRow) { + const confirmed = await this.dialogService.openSimpleDialog({ + title: groupRow.details.name, + content: { key: "deleteGroupConfirmation" }, + type: "warning", + }); + if (!confirmed) { + return false; + } + + try { + await this.groupService.delete(this.organizationId, groupRow.details.id); + this.toastService.showToast({ + variant: "success", + title: null, + message: this.i18nService.t("deletedGroupId", groupRow.details.name), + }); + this.removeGroup(groupRow); + } catch (e) { + this.logService.error(e); + } + } + + async deleteAllSelected() { + const groupsToDelete = this.dataSource.data.filter((g) => g.checked); + + if (groupsToDelete.length == 0) { + return; + } + + const deleteMessage = groupsToDelete.map((g) => g.details.name).join(", "); + const confirmed = await this.dialogService.openSimpleDialog({ + title: { + key: "deleteMultipleGroupsConfirmation", + placeholders: [groupsToDelete.length.toString()], + }, + content: deleteMessage, + type: "warning", + }); + if (!confirmed) { + return false; + } + + try { + await this.groupService.deleteMany( + this.organizationId, + groupsToDelete.map((g) => g.details.id), + ); + this.toastService.showToast({ + variant: "success", + title: null, + message: this.i18nService.t("deletedManyGroups", groupsToDelete.length.toString()), + }); + + groupsToDelete.forEach((g) => this.removeGroup(g)); + } catch (e) { + this.logService.error(e); + } + } + + check(groupRow: GroupDetailsRow) { + groupRow.checked = !groupRow.checked; + } + + toggleAllVisible(event: Event) { + this.dataSource.filteredData.forEach( + (g) => (g.checked = (event.target as HTMLInputElement).checked), + ); + } + + private removeGroup(groupRow: GroupDetailsRow) { + // Assign a new array to dataSource.data to trigger the setters and update the table + this.dataSource.data = this.dataSource.data.filter((g) => g !== groupRow); + } + + private async toCollectionMap(response: ListResponse) { + const collections = response.data.map( + (r) => new Collection(new CollectionData(r as CollectionDetailsResponse)), + ); + const decryptedCollections = await this.collectionService.decryptMany(collections); + + // Convert to an object using collection Ids as keys for faster name lookups + const collectionMap: Record = {}; + decryptedCollections.forEach((c) => (collectionMap[c.id] = c)); + + return collectionMap; + } +} diff --git a/apps/web/src/app/admin-console/organizations/organization-routing.module.ts b/apps/web/src/app/admin-console/organizations/organization-routing.module.ts index 2959601c10a..7427bbb481d 100644 --- a/apps/web/src/app/admin-console/organizations/organization-routing.module.ts +++ b/apps/web/src/app/admin-console/organizations/organization-routing.module.ts @@ -2,6 +2,7 @@ import { NgModule } from "@angular/core"; import { RouterModule, Routes } from "@angular/router"; import { AuthGuard } from "@bitwarden/angular/auth/guards"; +import { featureFlaggedRoute } from "@bitwarden/angular/platform/utils/feature-flagged-route"; import { canAccessOrgAdmin, canAccessGroupsTab, @@ -11,11 +12,13 @@ import { canAccessSettingsTab, } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { organizationPermissionsGuard } from "../../admin-console/organizations/guards/org-permissions.guard"; import { organizationRedirectGuard } from "../../admin-console/organizations/guards/org-redirect.guard"; import { OrganizationLayoutComponent } from "../../admin-console/organizations/layouts/organization-layout.component"; import { GroupsComponent } from "../../admin-console/organizations/manage/groups.component"; +import { NewGroupsComponent } from "../../admin-console/organizations/manage/new-groups.component"; import { deepLinkGuard } from "../../auth/guards/deep-link.guard"; import { VaultModule } from "../../vault/org-vault/vault.module"; @@ -46,14 +49,18 @@ const routes: Routes = [ path: "members", loadChildren: () => import("./members").then((m) => m.MembersModule), }, - { - path: "groups", - component: GroupsComponent, - canActivate: [organizationPermissionsGuard(canAccessGroupsTab)], - data: { - titleId: "groups", + ...featureFlaggedRoute({ + defaultComponent: GroupsComponent, + flaggedComponent: NewGroupsComponent, + featureFlag: FeatureFlag.GroupsComponentRefactor, + routeOptions: { + path: "groups", + canActivate: [organizationPermissionsGuard(canAccessGroupsTab)], + data: { + titleId: "groups", + }, }, - }, + }), { path: "reporting", loadChildren: () => diff --git a/apps/web/src/app/admin-console/organizations/organization.module.ts b/apps/web/src/app/admin-console/organizations/organization.module.ts index 29a7139231e..79f3a8e5f7d 100644 --- a/apps/web/src/app/admin-console/organizations/organization.module.ts +++ b/apps/web/src/app/admin-console/organizations/organization.module.ts @@ -1,3 +1,4 @@ +import { ScrollingModule } from "@angular/cdk/scrolling"; import { NgModule } from "@angular/core"; import { LooseComponentsModule } from "../../shared"; @@ -5,6 +6,7 @@ import { LooseComponentsModule } from "../../shared"; import { CoreOrganizationModule } from "./core"; import { GroupAddEditComponent } from "./manage/group-add-edit.component"; import { GroupsComponent } from "./manage/groups.component"; +import { NewGroupsComponent } from "./manage/new-groups.component"; import { OrganizationsRoutingModule } from "./organization-routing.module"; import { SharedOrganizationModule } from "./shared"; import { AccessSelectorModule } from "./shared/components/access-selector"; @@ -16,7 +18,8 @@ import { AccessSelectorModule } from "./shared/components/access-selector"; CoreOrganizationModule, OrganizationsRoutingModule, LooseComponentsModule, + ScrollingModule, ], - declarations: [GroupsComponent, GroupAddEditComponent], + declarations: [GroupsComponent, NewGroupsComponent, GroupAddEditComponent], }) export class OrganizationModule {} diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 759868eef77..2ed70efb81d 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -21,6 +21,7 @@ export enum FeatureFlag { InlineMenuFieldQualification = "inline-menu-field-qualification", MemberAccessReport = "ac-2059-member-access-report", EnableTimeThreshold = "PM-5864-dollar-threshold", + GroupsComponentRefactor = "groups-component-refactor", } export type AllowedFeatureFlagTypes = boolean | number | string; @@ -52,6 +53,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.InlineMenuFieldQualification]: FALSE, [FeatureFlag.MemberAccessReport]: FALSE, [FeatureFlag.EnableTimeThreshold]: FALSE, + [FeatureFlag.GroupsComponentRefactor]: FALSE, } satisfies Record; export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue; From cc31d9637897463e26a460b4f7f3fefae6237ad6 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 4 Jul 2024 09:02:18 +0200 Subject: [PATCH 9/9] [PM-9441] Catch and log exceptions during migration (#9905) * feat: catch and log exceptions during migration * Revert "feat: catch and log exceptions during migration" This reverts commit d68733b7e58120298974b350e496bb3e0c9af0d2. * feat: use log service to log migration errors --- apps/desktop/src/main.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 80dbf40cb87..f2295f2cdd8 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -291,8 +291,7 @@ export class Main { }); }, (e: any) => { - // eslint-disable-next-line - console.error(e); + this.logService.error("Error while running migrations:", e); }, ); }