Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-14366] Deprecated active user state from billing state service #12273

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
79f0f64
Updated billing state provider to not rely on ActiveUserStateProvider
cturnbull-bitwarden Dec 4, 2024
056fbfe
Updated usages
cturnbull-bitwarden Dec 4, 2024
e943dc9
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Dec 6, 2024
79e02e1
Resolved browser build
cturnbull-bitwarden Dec 6, 2024
5f5d38e
Resolved web build
cturnbull-bitwarden Dec 6, 2024
4643a64
Resolved CLI build
cturnbull-bitwarden Dec 6, 2024
e9d9ec9
resolved desktop build
cturnbull-bitwarden Dec 6, 2024
8232a72
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Dec 6, 2024
55e6f24
Update apps/cli/src/tools/send/commands/create.command.ts
cturnbull-bitwarden Dec 10, 2024
39c8a0d
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Dec 16, 2024
f9f4ceb
Move subscription visibility logic from component to service
cturnbull-bitwarden Dec 16, 2024
ad0c18e
Resolved unit test failures. Using existing userIds where present
cturnbull-bitwarden Dec 16, 2024
bc9dc37
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Dec 16, 2024
62662e3
Simplified activeUserId access
cturnbull-bitwarden Dec 16, 2024
85af1e6
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Dec 17, 2024
a9d072c
Resolved typescript strict errors
cturnbull-bitwarden Dec 17, 2024
5f03ba7
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Dec 18, 2024
6a69690
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Dec 18, 2024
d6c1494
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Dec 20, 2024
e87e543
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Dec 23, 2024
c84e4ee
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Dec 23, 2024
f8976e3
Resolved broken unit test
cturnbull-bitwarden Dec 23, 2024
bc0ab99
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Jan 6, 2025
cdb83c3
Resolved ts strict error
cturnbull-bitwarden Jan 6, 2025
bc195b3
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Jan 6, 2025
07807f7
Merge branch 'main' into billing/PM-14366/deprectete-active-user-statโ€ฆ
cturnbull-bitwarden Jan 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { mock, MockProxy } from "jest-mock-extended";
import { of } from "rxjs";

import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { NOOP_COMMAND_SUFFIX } from "@bitwarden/common/autofill/constants";
import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import { UserId } from "@bitwarden/common/types/guid";
import { CipherType } from "@bitwarden/common/vault/enums";
import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
Expand All @@ -19,6 +21,7 @@ describe("context-menu", () => {
let i18nService: MockProxy<I18nService>;
let logService: MockProxy<LogService>;
let billingAccountProfileStateService: MockProxy<BillingAccountProfileStateService>;
let accountService: MockProxy<AccountService>;

let removeAllSpy: jest.SpyInstance<void, [callback?: () => void]>;
let createSpy: jest.SpyInstance<
Expand All @@ -34,6 +37,7 @@ describe("context-menu", () => {
i18nService = mock();
logService = mock();
billingAccountProfileStateService = mock();
accountService = mock();

removeAllSpy = jest
.spyOn(chrome.contextMenus, "removeAll")
Expand All @@ -53,8 +57,15 @@ describe("context-menu", () => {
i18nService,
logService,
billingAccountProfileStateService,
accountService,
);
autofillSettingsService.enableContextMenu$ = of(true);
accountService.activeAccount$ = of({
id: "userId" as UserId,
email: "",
emailVerified: false,
name: undefined,
});
});

afterEach(() => jest.resetAllMocks());
Expand All @@ -69,15 +80,15 @@ describe("context-menu", () => {
});

it("has menu enabled, but does not have premium", async () => {
billingAccountProfileStateService.hasPremiumFromAnySource$ = of(false);
billingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(false));

const createdMenu = await sut.init();
expect(createdMenu).toBeTruthy();
expect(createSpy).toHaveBeenCalledTimes(10);
});

it("has menu enabled and has premium", async () => {
billingAccountProfileStateService.hasPremiumFromAnySource$ = of(true);
billingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(true));

const createdMenu = await sut.init();
expect(createdMenu).toBeTruthy();
Expand Down Expand Up @@ -131,16 +142,15 @@ describe("context-menu", () => {
});

it("create entry for each cipher piece", async () => {
billingAccountProfileStateService.hasPremiumFromAnySource$ = of(true);
billingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(true));

await sut.loadOptions("TEST_TITLE", "1", createCipher());

// One for autofill, copy username, copy password, and copy totp code
expect(createSpy).toHaveBeenCalledTimes(4);
});

it("creates a login/unlock item for each context menu action option when user is not authenticated", async () => {
billingAccountProfileStateService.hasPremiumFromAnySource$ = of(true);
billingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(true));

await sut.loadOptions("TEST_TITLE", "NOOP");

Expand Down
15 changes: 10 additions & 5 deletions apps/browser/src/autofill/browser/main-context-menu-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// @ts-strict-ignore
import { firstValueFrom } from "rxjs";

import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import {
AUTOFILL_CARD_ID,
AUTOFILL_ID,
Expand Down Expand Up @@ -149,6 +150,7 @@ export class MainContextMenuHandler {
private i18nService: I18nService,
private logService: LogService,
private billingAccountProfileStateService: BillingAccountProfileStateService,
private accountService: AccountService,
) {}

/**
Expand All @@ -168,11 +170,13 @@ export class MainContextMenuHandler {
this.initRunning = true;

try {
const account = await firstValueFrom(this.accountService.activeAccount$);
const hasPremium = await firstValueFrom(
this.billingAccountProfileStateService.hasPremiumFromAnySource$(account.id),
);
Comment on lines +174 to +176
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oo, moving this out of the loop was a good idea; thanks ๐Ÿ‘


for (const options of this.initContextMenuItems) {
if (
options.checkPremiumAccess &&
!(await firstValueFrom(this.billingAccountProfileStateService.hasPremiumFromAnySource$))
) {
if (options.checkPremiumAccess && !hasPremium) {
continue;
}

Expand Down Expand Up @@ -267,8 +271,9 @@ export class MainContextMenuHandler {
await createChildItem(COPY_USERNAME_ID);
}

const account = await firstValueFrom(this.accountService.activeAccount$);
const canAccessPremium = await firstValueFrom(
this.billingAccountProfileStateService.hasPremiumFromAnySource$,
this.billingAccountProfileStateService.hasPremiumFromAnySource$(account.id),
);
if (canAccessPremium && (!cipher || !Utils.isNullOrEmpty(cipher.login?.totp))) {
await createChildItem(COPY_VERIFICATION_CODE_ID);
Expand Down
22 changes: 16 additions & 6 deletions apps/browser/src/autofill/services/autofill.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { mock, mockReset, MockProxy } from "jest-mock-extended";
import { mock, MockProxy, mockReset } from "jest-mock-extended";
import { BehaviorSubject, of, Subject } from "rxjs";

import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
Expand Down Expand Up @@ -723,7 +723,9 @@ describe("AutofillService", () => {

it("throws an error if an autofill did not occur for any of the passed pages", async () => {
autofillOptions.tab.url = "https://a-different-url.com";
billingAccountProfileStateService.hasPremiumFromAnySource$ = of(true);
jest
.spyOn(billingAccountProfileStateService, "hasPremiumFromAnySource$")
.mockImplementation(() => of(true));

try {
await autofillService.doAutoFill(autofillOptions);
Expand Down Expand Up @@ -905,7 +907,9 @@ describe("AutofillService", () => {
it("returns a TOTP value", async () => {
const totpCode = "123456";
autofillOptions.cipher.login.totp = "totp";
billingAccountProfileStateService.hasPremiumFromAnySource$ = of(true);
jest
.spyOn(billingAccountProfileStateService, "hasPremiumFromAnySource$")
.mockImplementation(() => of(true));
jest.spyOn(autofillService, "getShouldAutoCopyTotp").mockResolvedValue(true);
jest.spyOn(totpService, "getCode").mockResolvedValue(totpCode);

Expand All @@ -918,7 +922,9 @@ describe("AutofillService", () => {

it("does not return a TOTP value if the user does not have premium features", async () => {
autofillOptions.cipher.login.totp = "totp";
billingAccountProfileStateService.hasPremiumFromAnySource$ = of(false);
jest
.spyOn(billingAccountProfileStateService, "hasPremiumFromAnySource$")
.mockImplementation(() => of(false));
jest.spyOn(autofillService, "getShouldAutoCopyTotp").mockResolvedValue(true);

const autofillResult = await autofillService.doAutoFill(autofillOptions);
Expand Down Expand Up @@ -952,7 +958,9 @@ describe("AutofillService", () => {
it("returns a null value if the user cannot access premium and the organization does not use TOTP", async () => {
autofillOptions.cipher.login.totp = "totp";
autofillOptions.cipher.organizationUseTotp = false;
billingAccountProfileStateService.hasPremiumFromAnySource$ = of(false);
jest
.spyOn(billingAccountProfileStateService, "hasPremiumFromAnySource$")
.mockImplementation(() => of(false));

const autofillResult = await autofillService.doAutoFill(autofillOptions);

Expand All @@ -962,7 +970,9 @@ describe("AutofillService", () => {
it("returns a null value if the user has disabled `auto TOTP copy`", async () => {
autofillOptions.cipher.login.totp = "totp";
autofillOptions.cipher.organizationUseTotp = true;
billingAccountProfileStateService.hasPremiumFromAnySource$ = of(true);
jest
.spyOn(billingAccountProfileStateService, "hasPremiumFromAnySource$")
.mockImplementation(() => of(true));
jest.spyOn(autofillService, "getShouldAutoCopyTotp").mockResolvedValue(false);
jest.spyOn(totpService, "getCode");

Expand Down
3 changes: 2 additions & 1 deletion apps/browser/src/autofill/services/autofill.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,8 +416,9 @@ export default class AutofillService implements AutofillServiceInterface {

let totp: string | null = null;

const activeAccount = await firstValueFrom(this.accountService.activeAccount$);
const canAccessPremium = await firstValueFrom(
this.billingAccountProfileStateService.hasPremiumFromAnySource$,
this.billingAccountProfileStateService.hasPremiumFromAnySource$(activeAccount.id),
);
const defaultUriMatch = await this.getDefaultUriMatchStrategy();

Expand Down
3 changes: 3 additions & 0 deletions apps/browser/src/background/main.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,8 @@ export default class MainBackground {

this.billingAccountProfileStateService = new DefaultBillingAccountProfileStateService(
this.stateProvider,
this.platformUtilsService,
this.apiService,
);

this.ssoLoginService = new SsoLoginService(this.stateProvider);
Expand Down Expand Up @@ -1224,6 +1226,7 @@ export default class MainBackground {
this.i18nService,
this.logService,
this.billingAccountProfileStateService,
this.accountService,
);

this.cipherContextMenuHandler = new CipherContextMenuHandler(
Expand Down
5 changes: 4 additions & 1 deletion apps/browser/src/background/runtime.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,11 @@ export default class RuntimeBackground {
return await this.configService.getFeatureFlag(FeatureFlag.InlineMenuFieldQualification);
}
case "getUserPremiumStatus": {
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
const result = await firstValueFrom(
this.billingAccountProfileStateService.hasPremiumFromAnySource$,
this.billingAccountProfileStateService.hasPremiumFromAnySource$(activeUserId),
);
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { RouterModule } from "@angular/router";
import { JslibModule } from "@bitwarden/angular/jslib.module";
import { PremiumComponent as BasePremiumComponent } from "@bitwarden/angular/vault/components/premium.component";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
Expand Down Expand Up @@ -56,6 +57,7 @@ export class PremiumV2Component extends BasePremiumComponent {
dialogService: DialogService,
environmentService: EnvironmentService,
billingAccountProfileStateService: BillingAccountProfileStateService,
accountService: AccountService,
) {
super(
i18nService,
Expand All @@ -66,6 +68,7 @@ export class PremiumV2Component extends BasePremiumComponent {
dialogService,
environmentService,
billingAccountProfileStateService,
accountService,
);

// Support old price string. Can be removed in future once all translations are properly updated.
Expand Down
12 changes: 11 additions & 1 deletion apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,17 @@ describe("SendV2Component", () => {
CurrentAccountComponent,
],
providers: [
{ provide: AccountService, useValue: mock<AccountService>() },
{
provide: AccountService,
useValue: {
activeAccount$: of({
id: "123",
email: "[email protected]",
emailVerified: true,
name: "Test User",
}),
},
},
{ provide: AuthService, useValue: mock<AuthService>() },
{ provide: AvatarService, useValue: mock<AvatarService>() },
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { CommonModule } from "@angular/common";
import { Component } from "@angular/core";
import { RouterModule } from "@angular/router";
import { Observable, firstValueFrom } from "rxjs";
import { Observable, firstValueFrom, of, switchMap } from "rxjs";

import { JslibModule } from "@bitwarden/angular/jslib.module";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { DialogService, ItemModule } from "@bitwarden/components";
Expand Down Expand Up @@ -36,12 +37,19 @@ export class MoreFromBitwardenPageV2Component {

constructor(
private dialogService: DialogService,
billingAccountProfileStateService: BillingAccountProfileStateService,
private billingAccountProfileStateService: BillingAccountProfileStateService,
private environmentService: EnvironmentService,
private organizationService: OrganizationService,
private familiesPolicyService: FamiliesPolicyService,
private accountService: AccountService,
) {
this.canAccessPremium$ = billingAccountProfileStateService.hasPremiumFromAnySource$;
this.canAccessPremium$ = this.accountService.activeAccount$.pipe(
switchMap((account) =>
account
? this.billingAccountProfileStateService.hasPremiumFromAnySource$(account.id)
: of(false),
),
);
this.familySponsorshipAvailable$ = this.organizationService.familySponsorshipAvailable$;
this.hasSingleEnterpriseOrg$ = this.familiesPolicyService.hasSingleEnterpriseOrg$();
this.isFreeFamilyPolicyEnabled$ = this.familiesPolicyService.isFreeFamilyPolicyEnabled$();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ComponentFixture, TestBed } from "@angular/core/testing";
import { Router } from "@angular/router";
import { RouterTestingModule } from "@angular/router/testing";
import { BehaviorSubject } from "rxjs";
import { BehaviorSubject, of } from "rxjs";

import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
Expand All @@ -10,7 +10,6 @@ import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abs
import { ProductTierType } from "@bitwarden/common/billing/enums";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec";
import { CipherId, UserId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherType } from "@bitwarden/common/vault/enums";
Expand Down Expand Up @@ -55,14 +54,22 @@ describe("OpenAttachmentsComponent", () => {
const showFilePopoutMessage = jest.fn().mockReturnValue(false);

const mockUserId = Utils.newGuid() as UserId;
const accountService: FakeAccountService = mockAccountServiceWith(mockUserId);
const accountService = {
activeAccount$: of({
id: mockUserId,
email: "[email protected]",
emailVerified: true,
name: "Test User",
}),
};

beforeEach(async () => {
openCurrentPagePopout.mockClear();
getCipher.mockClear();
showToast.mockClear();
getOrganization.mockClear();
showFilePopoutMessage.mockClear();
hasPremiumFromAnySource$.next(true);

await TestBed.configureTestingModule({
imports: [OpenAttachmentsComponent, RouterTestingModule],
Expand Down Expand Up @@ -96,7 +103,7 @@ describe("OpenAttachmentsComponent", () => {
}).compileComponents();
});

beforeEach(() => {
beforeEach(async () => {
fixture = TestBed.createComponent(OpenAttachmentsComponent);
component = fixture.componentInstance;
component.cipherId = "5555-444-3333" as CipherId;
Expand All @@ -107,7 +114,7 @@ describe("OpenAttachmentsComponent", () => {

it("opens attachments in new popout", async () => {
showFilePopoutMessage.mockReturnValue(true);

component.canAccessAttachments = true;
await component.ngOnInit();

await component.openAttachments();
Expand All @@ -120,7 +127,7 @@ describe("OpenAttachmentsComponent", () => {

it("opens attachments in same window", async () => {
showFilePopoutMessage.mockReturnValue(false);

component.canAccessAttachments = true;
await component.ngOnInit();

await component.openAttachments();
Expand Down
Loading
Loading