From ed236df24b93509aa8e50b7b7a4804cf95959c6e Mon Sep 17 00:00:00 2001 From: Anas Date: Fri, 3 May 2024 15:44:57 +0200 Subject: [PATCH 01/43] fix(8560): refreshing reports pages displays empty pages (#8700) --- .../tools/exposed-passwords-report.component.ts | 3 +++ .../tools/inactive-two-factor-report.component.ts | 3 +++ .../tools/reused-passwords-report.component.ts | 11 ++++++++++- .../tools/unsecured-websites-report.component.ts | 11 ++++++++++- .../tools/weak-passwords-report.component.ts | 3 +++ .../tools/reports/pages/cipher-report.component.ts | 3 +++ .../pages/exposed-passwords-report.component.spec.ts | 11 +++++++++++ .../pages/exposed-passwords-report.component.ts | 11 ++++++++++- .../inactive-two-factor-report.component.spec.ts | 11 +++++++++++ .../pages/inactive-two-factor-report.component.ts | 11 ++++++++++- .../pages/reused-passwords-report.component.spec.ts | 11 +++++++++++ .../pages/reused-passwords-report.component.ts | 11 ++++++++++- .../pages/unsecured-websites-report.component.spec.ts | 11 +++++++++++ .../pages/unsecured-websites-report.component.ts | 11 ++++++++++- .../pages/weak-passwords-report.component.spec.ts | 11 +++++++++++ .../reports/pages/weak-passwords-report.component.ts | 11 ++++++++++- 16 files changed, 137 insertions(+), 7 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/tools/exposed-passwords-report.component.ts b/apps/web/src/app/admin-console/organizations/tools/exposed-passwords-report.component.ts index d354459ee9..cab6189c45 100644 --- a/apps/web/src/app/admin-console/organizations/tools/exposed-passwords-report.component.ts +++ b/apps/web/src/app/admin-console/organizations/tools/exposed-passwords-report.component.ts @@ -6,6 +6,7 @@ import { AuditService } from "@bitwarden/common/abstractions/audit.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { PasswordRepromptService } from "@bitwarden/vault"; @@ -29,6 +30,7 @@ export class ExposedPasswordsReportComponent extends BaseExposedPasswordsReportC private route: ActivatedRoute, passwordRepromptService: PasswordRepromptService, i18nService: I18nService, + syncService: SyncService, ) { super( cipherService, @@ -37,6 +39,7 @@ export class ExposedPasswordsReportComponent extends BaseExposedPasswordsReportC modalService, passwordRepromptService, i18nService, + syncService, ); } diff --git a/apps/web/src/app/admin-console/organizations/tools/inactive-two-factor-report.component.ts b/apps/web/src/app/admin-console/organizations/tools/inactive-two-factor-report.component.ts index 67d4e963b0..abfbd45f38 100644 --- a/apps/web/src/app/admin-console/organizations/tools/inactive-two-factor-report.component.ts +++ b/apps/web/src/app/admin-console/organizations/tools/inactive-two-factor-report.component.ts @@ -6,6 +6,7 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { PasswordRepromptService } from "@bitwarden/vault"; @@ -26,6 +27,7 @@ export class InactiveTwoFactorReportComponent extends BaseInactiveTwoFactorRepor passwordRepromptService: PasswordRepromptService, organizationService: OrganizationService, i18nService: I18nService, + syncService: SyncService, ) { super( cipherService, @@ -34,6 +36,7 @@ export class InactiveTwoFactorReportComponent extends BaseInactiveTwoFactorRepor logService, passwordRepromptService, i18nService, + syncService, ); } diff --git a/apps/web/src/app/admin-console/organizations/tools/reused-passwords-report.component.ts b/apps/web/src/app/admin-console/organizations/tools/reused-passwords-report.component.ts index c8ceb023af..76d783b666 100644 --- a/apps/web/src/app/admin-console/organizations/tools/reused-passwords-report.component.ts +++ b/apps/web/src/app/admin-console/organizations/tools/reused-passwords-report.component.ts @@ -5,6 +5,7 @@ import { ModalService } from "@bitwarden/angular/services/modal.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { PasswordRepromptService } from "@bitwarden/vault"; @@ -27,8 +28,16 @@ export class ReusedPasswordsReportComponent extends BaseReusedPasswordsReportCom organizationService: OrganizationService, passwordRepromptService: PasswordRepromptService, i18nService: I18nService, + syncService: SyncService, ) { - super(cipherService, organizationService, modalService, passwordRepromptService, i18nService); + super( + cipherService, + organizationService, + modalService, + passwordRepromptService, + i18nService, + syncService, + ); } async ngOnInit() { diff --git a/apps/web/src/app/admin-console/organizations/tools/unsecured-websites-report.component.ts b/apps/web/src/app/admin-console/organizations/tools/unsecured-websites-report.component.ts index 2a905b3665..7f6f08fb96 100644 --- a/apps/web/src/app/admin-console/organizations/tools/unsecured-websites-report.component.ts +++ b/apps/web/src/app/admin-console/organizations/tools/unsecured-websites-report.component.ts @@ -5,6 +5,7 @@ import { ModalService } from "@bitwarden/angular/services/modal.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { PasswordRepromptService } from "@bitwarden/vault"; @@ -24,8 +25,16 @@ export class UnsecuredWebsitesReportComponent extends BaseUnsecuredWebsitesRepor organizationService: OrganizationService, passwordRepromptService: PasswordRepromptService, i18nService: I18nService, + syncService: SyncService, ) { - super(cipherService, organizationService, modalService, passwordRepromptService, i18nService); + super( + cipherService, + organizationService, + modalService, + passwordRepromptService, + i18nService, + syncService, + ); } async ngOnInit() { diff --git a/apps/web/src/app/admin-console/organizations/tools/weak-passwords-report.component.ts b/apps/web/src/app/admin-console/organizations/tools/weak-passwords-report.component.ts index 8820e596e3..0ac2129478 100644 --- a/apps/web/src/app/admin-console/organizations/tools/weak-passwords-report.component.ts +++ b/apps/web/src/app/admin-console/organizations/tools/weak-passwords-report.component.ts @@ -6,6 +6,7 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { PasswordRepromptService } from "@bitwarden/vault"; @@ -29,6 +30,7 @@ export class WeakPasswordsReportComponent extends BaseWeakPasswordsReportCompone organizationService: OrganizationService, passwordRepromptService: PasswordRepromptService, i18nService: I18nService, + syncService: SyncService, ) { super( cipherService, @@ -37,6 +39,7 @@ export class WeakPasswordsReportComponent extends BaseWeakPasswordsReportCompone modalService, passwordRepromptService, i18nService, + syncService, ); } diff --git a/apps/web/src/app/tools/reports/pages/cipher-report.component.ts b/apps/web/src/app/tools/reports/pages/cipher-report.component.ts index 041307122b..4e63dd5cc9 100644 --- a/apps/web/src/app/tools/reports/pages/cipher-report.component.ts +++ b/apps/web/src/app/tools/reports/pages/cipher-report.component.ts @@ -6,6 +6,7 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { PasswordRepromptService } from "@bitwarden/vault"; @@ -40,6 +41,7 @@ export class CipherReportComponent implements OnDestroy { protected passwordRepromptService: PasswordRepromptService, protected organizationService: OrganizationService, protected i18nService: I18nService, + private syncService: SyncService, ) { this.organizations$ = this.organizationService.organizations$; this.organizations$.pipe(takeUntil(this.destroyed$)).subscribe((orgs) => { @@ -106,6 +108,7 @@ export class CipherReportComponent implements OnDestroy { async load() { this.loading = true; + await this.syncService.fullSync(false); // when a user fixes an item in a report we want to persist the filter they had // if they fix the last item of that filter we will go back to the "All" filter if (this.currentFilterStatus) { diff --git a/apps/web/src/app/tools/reports/pages/exposed-passwords-report.component.spec.ts b/apps/web/src/app/tools/reports/pages/exposed-passwords-report.component.spec.ts index 7b73ad8305..07dc218bd6 100644 --- a/apps/web/src/app/tools/reports/pages/exposed-passwords-report.component.spec.ts +++ b/apps/web/src/app/tools/reports/pages/exposed-passwords-report.component.spec.ts @@ -9,6 +9,7 @@ import { AuditService } from "@bitwarden/common/abstractions/audit.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { PasswordRepromptService } from "@bitwarden/vault"; import { ExposedPasswordsReportComponent } from "./exposed-passwords-report.component"; @@ -19,8 +20,10 @@ describe("ExposedPasswordsReportComponent", () => { let fixture: ComponentFixture; let auditService: MockProxy; let organizationService: MockProxy; + let syncServiceMock: MockProxy; beforeEach(() => { + syncServiceMock = mock(); auditService = mock(); organizationService = mock(); organizationService.organizations$ = of([]); @@ -49,6 +52,10 @@ describe("ExposedPasswordsReportComponent", () => { provide: PasswordRepromptService, useValue: mock(), }, + { + provide: SyncService, + useValue: syncServiceMock, + }, { provide: I18nService, useValue: mock(), @@ -82,4 +89,8 @@ describe("ExposedPasswordsReportComponent", () => { expect(component.ciphers[1].id).toEqual(expectedIdTwo); expect(component.ciphers[1].edit).toEqual(true); }); + + it("should call fullSync method of syncService", () => { + expect(syncServiceMock.fullSync).toHaveBeenCalledWith(false); + }); }); diff --git a/apps/web/src/app/tools/reports/pages/exposed-passwords-report.component.ts b/apps/web/src/app/tools/reports/pages/exposed-passwords-report.component.ts index 39414487d7..cabc7bdfa1 100644 --- a/apps/web/src/app/tools/reports/pages/exposed-passwords-report.component.ts +++ b/apps/web/src/app/tools/reports/pages/exposed-passwords-report.component.ts @@ -5,6 +5,7 @@ import { AuditService } from "@bitwarden/common/abstractions/audit.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { PasswordRepromptService } from "@bitwarden/vault"; @@ -26,8 +27,16 @@ export class ExposedPasswordsReportComponent extends CipherReportComponent imple modalService: ModalService, passwordRepromptService: PasswordRepromptService, i18nService: I18nService, + syncService: SyncService, ) { - super(cipherService, modalService, passwordRepromptService, organizationService, i18nService); + super( + cipherService, + modalService, + passwordRepromptService, + organizationService, + i18nService, + syncService, + ); } async ngOnInit() { diff --git a/apps/web/src/app/tools/reports/pages/inactive-two-factor-report.component.spec.ts b/apps/web/src/app/tools/reports/pages/inactive-two-factor-report.component.spec.ts index 528f6306e0..80760eb5de 100644 --- a/apps/web/src/app/tools/reports/pages/inactive-two-factor-report.component.spec.ts +++ b/apps/web/src/app/tools/reports/pages/inactive-two-factor-report.component.spec.ts @@ -9,6 +9,7 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { PasswordRepromptService } from "@bitwarden/vault"; import { InactiveTwoFactorReportComponent } from "./inactive-two-factor-report.component"; @@ -18,10 +19,12 @@ describe("InactiveTwoFactorReportComponent", () => { let component: InactiveTwoFactorReportComponent; let fixture: ComponentFixture; let organizationService: MockProxy; + let syncServiceMock: MockProxy; beforeEach(() => { organizationService = mock(); organizationService.organizations$ = of([]); + syncServiceMock = mock(); // 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 TestBed.configureTestingModule({ @@ -47,6 +50,10 @@ describe("InactiveTwoFactorReportComponent", () => { provide: PasswordRepromptService, useValue: mock(), }, + { + provide: SyncService, + useValue: syncServiceMock, + }, { provide: I18nService, useValue: mock(), @@ -87,4 +94,8 @@ describe("InactiveTwoFactorReportComponent", () => { expect(component.ciphers[1].id).toEqual(expectedIdTwo); expect(component.ciphers[1].edit).toEqual(true); }); + + it("should call fullSync method of syncService", () => { + expect(syncServiceMock.fullSync).toHaveBeenCalledWith(false); + }); }); diff --git a/apps/web/src/app/tools/reports/pages/inactive-two-factor-report.component.ts b/apps/web/src/app/tools/reports/pages/inactive-two-factor-report.component.ts index 956607c8fb..5cfe2cd1a9 100644 --- a/apps/web/src/app/tools/reports/pages/inactive-two-factor-report.component.ts +++ b/apps/web/src/app/tools/reports/pages/inactive-two-factor-report.component.ts @@ -6,6 +6,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { PasswordRepromptService } from "@bitwarden/vault"; @@ -28,8 +29,16 @@ export class InactiveTwoFactorReportComponent extends CipherReportComponent impl private logService: LogService, passwordRepromptService: PasswordRepromptService, i18nService: I18nService, + syncService: SyncService, ) { - super(cipherService, modalService, passwordRepromptService, organizationService, i18nService); + super( + cipherService, + modalService, + passwordRepromptService, + organizationService, + i18nService, + syncService, + ); } async ngOnInit() { diff --git a/apps/web/src/app/tools/reports/pages/reused-passwords-report.component.spec.ts b/apps/web/src/app/tools/reports/pages/reused-passwords-report.component.spec.ts index 29e20c11af..9d16bbb1c6 100644 --- a/apps/web/src/app/tools/reports/pages/reused-passwords-report.component.spec.ts +++ b/apps/web/src/app/tools/reports/pages/reused-passwords-report.component.spec.ts @@ -8,6 +8,7 @@ import { ModalService } from "@bitwarden/angular/services/modal.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { PasswordRepromptService } from "@bitwarden/vault"; import { cipherData } from "./reports-ciphers.mock"; @@ -17,10 +18,12 @@ describe("ReusedPasswordsReportComponent", () => { let component: ReusedPasswordsReportComponent; let fixture: ComponentFixture; let organizationService: MockProxy; + let syncServiceMock: MockProxy; beforeEach(() => { organizationService = mock(); organizationService.organizations$ = of([]); + syncServiceMock = mock(); // 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 TestBed.configureTestingModule({ @@ -42,6 +45,10 @@ describe("ReusedPasswordsReportComponent", () => { provide: PasswordRepromptService, useValue: mock(), }, + { + provide: SyncService, + useValue: syncServiceMock, + }, { provide: I18nService, useValue: mock(), @@ -73,4 +80,8 @@ describe("ReusedPasswordsReportComponent", () => { expect(component.ciphers[1].id).toEqual(expectedIdTwo); expect(component.ciphers[1].edit).toEqual(true); }); + + it("should call fullSync method of syncService", () => { + expect(syncServiceMock.fullSync).toHaveBeenCalledWith(false); + }); }); diff --git a/apps/web/src/app/tools/reports/pages/reused-passwords-report.component.ts b/apps/web/src/app/tools/reports/pages/reused-passwords-report.component.ts index cbc2ea11b5..70cb2ed69b 100644 --- a/apps/web/src/app/tools/reports/pages/reused-passwords-report.component.ts +++ b/apps/web/src/app/tools/reports/pages/reused-passwords-report.component.ts @@ -4,6 +4,7 @@ import { ModalService } from "@bitwarden/angular/services/modal.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { PasswordRepromptService } from "@bitwarden/vault"; @@ -24,8 +25,16 @@ export class ReusedPasswordsReportComponent extends CipherReportComponent implem modalService: ModalService, passwordRepromptService: PasswordRepromptService, i18nService: I18nService, + syncService: SyncService, ) { - super(cipherService, modalService, passwordRepromptService, organizationService, i18nService); + super( + cipherService, + modalService, + passwordRepromptService, + organizationService, + i18nService, + syncService, + ); } async ngOnInit() { diff --git a/apps/web/src/app/tools/reports/pages/unsecured-websites-report.component.spec.ts b/apps/web/src/app/tools/reports/pages/unsecured-websites-report.component.spec.ts index 3b7c6d350f..e616d1f21e 100644 --- a/apps/web/src/app/tools/reports/pages/unsecured-websites-report.component.spec.ts +++ b/apps/web/src/app/tools/reports/pages/unsecured-websites-report.component.spec.ts @@ -8,6 +8,7 @@ import { ModalService } from "@bitwarden/angular/services/modal.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { PasswordRepromptService } from "@bitwarden/vault"; import { cipherData } from "./reports-ciphers.mock"; @@ -17,10 +18,12 @@ describe("UnsecuredWebsitesReportComponent", () => { let component: UnsecuredWebsitesReportComponent; let fixture: ComponentFixture; let organizationService: MockProxy; + let syncServiceMock: MockProxy; beforeEach(() => { organizationService = mock(); organizationService.organizations$ = of([]); + syncServiceMock = mock(); // 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 TestBed.configureTestingModule({ @@ -42,6 +45,10 @@ describe("UnsecuredWebsitesReportComponent", () => { provide: PasswordRepromptService, useValue: mock(), }, + { + provide: SyncService, + useValue: syncServiceMock, + }, { provide: I18nService, useValue: mock(), @@ -73,4 +80,8 @@ describe("UnsecuredWebsitesReportComponent", () => { expect(component.ciphers[1].id).toEqual(expectedIdTwo); expect(component.ciphers[1].edit).toEqual(true); }); + + it("should call fullSync method of syncService", () => { + expect(syncServiceMock.fullSync).toHaveBeenCalledWith(false); + }); }); diff --git a/apps/web/src/app/tools/reports/pages/unsecured-websites-report.component.ts b/apps/web/src/app/tools/reports/pages/unsecured-websites-report.component.ts index 769eb058cd..0a8023c303 100644 --- a/apps/web/src/app/tools/reports/pages/unsecured-websites-report.component.ts +++ b/apps/web/src/app/tools/reports/pages/unsecured-websites-report.component.ts @@ -4,6 +4,7 @@ import { ModalService } from "@bitwarden/angular/services/modal.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { CipherType } from "@bitwarden/common/vault/enums"; import { PasswordRepromptService } from "@bitwarden/vault"; @@ -22,8 +23,16 @@ export class UnsecuredWebsitesReportComponent extends CipherReportComponent impl modalService: ModalService, passwordRepromptService: PasswordRepromptService, i18nService: I18nService, + syncService: SyncService, ) { - super(cipherService, modalService, passwordRepromptService, organizationService, i18nService); + super( + cipherService, + modalService, + passwordRepromptService, + organizationService, + i18nService, + syncService, + ); } async ngOnInit() { diff --git a/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.spec.ts b/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.spec.ts index dbc367b108..bcace60ac0 100644 --- a/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.spec.ts +++ b/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.spec.ts @@ -9,6 +9,7 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { PasswordRepromptService } from "@bitwarden/vault"; import { cipherData } from "./reports-ciphers.mock"; @@ -19,8 +20,10 @@ describe("WeakPasswordsReportComponent", () => { let fixture: ComponentFixture; let passwordStrengthService: MockProxy; let organizationService: MockProxy; + let syncServiceMock: MockProxy; beforeEach(() => { + syncServiceMock = mock(); passwordStrengthService = mock(); organizationService = mock(); organizationService.organizations$ = of([]); @@ -49,6 +52,10 @@ describe("WeakPasswordsReportComponent", () => { provide: PasswordRepromptService, useValue: mock(), }, + { + provide: SyncService, + useValue: syncServiceMock, + }, { provide: I18nService, useValue: mock(), @@ -85,4 +92,8 @@ describe("WeakPasswordsReportComponent", () => { expect(component.ciphers[1].id).toEqual(expectedIdTwo); expect(component.ciphers[1].edit).toEqual(true); }); + + it("should call fullSync method of syncService", () => { + expect(syncServiceMock.fullSync).toHaveBeenCalledWith(false); + }); }); diff --git a/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.ts b/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.ts index 4d179b58f3..f33e0626ab 100644 --- a/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.ts +++ b/apps/web/src/app/tools/reports/pages/weak-passwords-report.component.ts @@ -6,6 +6,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { Utils } from "@bitwarden/common/platform/misc/utils"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { BadgeVariant } from "@bitwarden/components"; @@ -31,8 +32,16 @@ export class WeakPasswordsReportComponent extends CipherReportComponent implemen modalService: ModalService, passwordRepromptService: PasswordRepromptService, i18nService: I18nService, + syncService: SyncService, ) { - super(cipherService, modalService, passwordRepromptService, organizationService, i18nService); + super( + cipherService, + modalService, + passwordRepromptService, + organizationService, + i18nService, + syncService, + ); } async ngOnInit() { From debfe914c24d39bec799d2d7111eec7e2a43fe34 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 3 May 2024 10:47:20 -0400 Subject: [PATCH 02/43] [deps] Platform (CL): Update tailwindcss to v3.4.3 (#8736) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package-lock.json | 10 +++++----- package.json | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index ba0119ca63..b7372ce27b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -172,7 +172,7 @@ "sass-loader": "13.3.3", "storybook": "7.6.17", "style-loader": "3.3.4", - "tailwindcss": "3.4.1", + "tailwindcss": "3.4.3", "ts-jest": "29.1.2", "ts-loader": "9.5.1", "tsconfig-paths-webpack-plugin": "4.1.0", @@ -35401,9 +35401,9 @@ "dev": true }, "node_modules/tailwindcss": { - "version": "3.4.1", - "resolved": "https://registry.npmjs.org/tailwindcss/-/tailwindcss-3.4.1.tgz", - "integrity": "sha512-qAYmXRfk3ENzuPBakNK0SRrUDipP8NQnEY6772uDhflcQz5EhRdD7JNZxyrFHVQNCwULPBn6FNPp9brpO7ctcA==", + "version": "3.4.3", + "resolved": "https://registry.npmjs.org/tailwindcss/-/tailwindcss-3.4.3.tgz", + "integrity": "sha512-U7sxQk/n397Bmx4JHbJx/iSOOv5G+II3f1kpLpY2QeUv5DcPdcTsYLlusZfq1NthHS1c1cZoyFmmkex1rzke0A==", "dev": true, "dependencies": { "@alloc/quick-lru": "^5.2.0", @@ -35414,7 +35414,7 @@ "fast-glob": "^3.3.0", "glob-parent": "^6.0.2", "is-glob": "^4.0.3", - "jiti": "^1.19.1", + "jiti": "^1.21.0", "lilconfig": "^2.1.0", "micromatch": "^4.0.5", "normalize-path": "^3.0.0", diff --git a/package.json b/package.json index c73ae492f5..5aca97a6f6 100644 --- a/package.json +++ b/package.json @@ -133,7 +133,7 @@ "sass-loader": "13.3.3", "storybook": "7.6.17", "style-loader": "3.3.4", - "tailwindcss": "3.4.1", + "tailwindcss": "3.4.3", "ts-jest": "29.1.2", "ts-loader": "9.5.1", "tsconfig-paths-webpack-plugin": "4.1.0", From 69ed6ce1f5ed42eaecb38cfc46bb9e9cf6482963 Mon Sep 17 00:00:00 2001 From: Jake Fink Date: Fri, 3 May 2024 11:54:29 -0400 Subject: [PATCH 03/43] [PM-6727] Part 1: pass userId in login strategies (#9030) * add validation to initAccount * pass userId to setMasterKey * fix key connector tests --- .../auth-request-login.strategy.ts | 8 ++- .../login-strategies/login.strategy.spec.ts | 1 + .../common/login-strategies/login.strategy.ts | 5 +- .../password-login.strategy.spec.ts | 7 ++- .../password-login.strategy.ts | 53 ++++++++++--------- .../sso-login.strategy.spec.ts | 11 ++-- .../login-strategies/sso-login.strategy.ts | 18 +++---- .../user-api-login.strategy.spec.ts | 4 +- .../user-api-login.strategy.ts | 8 +-- .../webauthn-login.strategy.spec.ts | 7 ++- .../webauthn-login.strategy.ts | 6 +-- .../abstractions/key-connector.service.ts | 4 +- .../src/auth/models/domain/auth-result.ts | 2 + .../services/key-connector.service.spec.ts | 4 +- .../auth/services/key-connector.service.ts | 15 +++--- .../src/platform/services/crypto.service.ts | 13 +++++ 16 files changed, 101 insertions(+), 65 deletions(-) diff --git a/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts b/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts index a66d987984..c8acc6c24b 100644 --- a/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts @@ -117,13 +117,12 @@ export class AuthRequestLoginStrategy extends LoginStrategy { return super.logInTwoFactor(twoFactor); } - protected override async setMasterKey(response: IdentityTokenResponse) { + protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) { const authRequestCredentials = this.cache.value.authRequestCredentials; if ( authRequestCredentials.decryptedMasterKey && authRequestCredentials.decryptedMasterKeyHash ) { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; await this.masterPasswordService.setMasterKey( authRequestCredentials.decryptedMasterKey, userId, @@ -147,15 +146,14 @@ export class AuthRequestLoginStrategy extends LoginStrategy { if (authRequestCredentials.decryptedUserKey) { await this.cryptoService.setUserKey(authRequestCredentials.decryptedUserKey); } else { - await this.trySetUserKeyWithMasterKey(); + await this.trySetUserKeyWithMasterKey(userId); // Establish trust if required after setting user key await this.deviceTrustService.trustDeviceIfRequired(userId); } } - private async trySetUserKeyWithMasterKey(): Promise { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; + private async trySetUserKeyWithMasterKey(userId: UserId): Promise { const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); if (masterKey) { const userKey = await this.cryptoService.decryptUserKeyWithMasterKey(masterKey); diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index c3a8f61d78..612222c10e 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -244,6 +244,7 @@ describe("LoginStrategy", () => { const result = await passwordLoginStrategy.logIn(credentials); expect(result).toEqual({ + userId: userId, forcePasswordReset: ForceSetPasswordReason.AdminForcePasswordReset, resetMasterPassword: true, twoFactorProviders: null, diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 3a3109349e..96f7b73cab 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -241,6 +241,7 @@ export abstract class LoginStrategy { // Must come before setting keys, user key needs email to update additional keys const userId = await this.saveAccountInformation(response); + result.userId = userId; if (response.twoFactorToken != null) { // note: we can read email from access token b/c it was saved in saveAccountInformation @@ -249,7 +250,7 @@ export abstract class LoginStrategy { await this.tokenService.setTwoFactorToken(userEmail, response.twoFactorToken); } - await this.setMasterKey(response); + await this.setMasterKey(response, userId); await this.setUserKey(response, userId); await this.setPrivateKey(response); @@ -259,7 +260,7 @@ export abstract class LoginStrategy { } // The keys comes from different sources depending on the login strategy - protected abstract setMasterKey(response: IdentityTokenResponse): Promise; + protected abstract setMasterKey(response: IdentityTokenResponse, userId: UserId): Promise; protected abstract setUserKey(response: IdentityTokenResponse, userId: UserId): Promise; protected abstract setPrivateKey(response: IdentityTokenResponse): Promise; diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts index c97639f102..e85d01c5d7 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts @@ -173,8 +173,11 @@ describe("PasswordLoginStrategy", () => { localHashedPassword, userId, ); - expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(tokenResponse.key); - expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey); + expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( + tokenResponse.key, + userId, + ); + expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey, userId); expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey); }); diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.ts b/libs/auth/src/common/login-strategies/password-login.strategy.ts index d3ce8fa9e8..ec2a4850fe 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -147,6 +147,10 @@ export class PasswordLoginStrategy extends LoginStrategy { const [authResult, identityResponse] = await this.startLogIn(); + if (identityResponse instanceof IdentityCaptchaResponse) { + return authResult; + } + const masterPasswordPolicyOptions = this.getMasterPasswordPolicyOptionsFromResponse(identityResponse); @@ -157,23 +161,23 @@ export class PasswordLoginStrategy extends LoginStrategy { credentials, masterPasswordPolicyOptions, ); + if (meetsRequirements) { + return authResult; + } - if (!meetsRequirements) { - if (authResult.requiresCaptcha || authResult.requiresTwoFactor) { - // Save the flag to this strategy for later use as the master password is about to pass out of scope - this.cache.next({ - ...this.cache.value, - forcePasswordResetReason: ForceSetPasswordReason.WeakMasterPassword, - }); - } else { - // Authentication was successful, save the force update password options with the state service - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; - await this.masterPasswordService.setForceSetPasswordReason( - ForceSetPasswordReason.WeakMasterPassword, - userId, - ); - authResult.forcePasswordReset = ForceSetPasswordReason.WeakMasterPassword; - } + if (identityResponse instanceof IdentityTwoFactorResponse) { + // Save the flag to this strategy for use in 2fa login as the master password is about to pass out of scope + this.cache.next({ + ...this.cache.value, + forcePasswordResetReason: ForceSetPasswordReason.WeakMasterPassword, + }); + } else { + // Authentication was successful, save the force update password options with the state service + await this.masterPasswordService.setForceSetPasswordReason( + ForceSetPasswordReason.WeakMasterPassword, + authResult.userId, // userId is only available on successful login + ); + authResult.forcePasswordReset = ForceSetPasswordReason.WeakMasterPassword; } } return authResult; @@ -196,17 +200,18 @@ export class PasswordLoginStrategy extends LoginStrategy { !result.requiresCaptcha && forcePasswordResetReason != ForceSetPasswordReason.None ) { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; - await this.masterPasswordService.setForceSetPasswordReason(forcePasswordResetReason, userId); + await this.masterPasswordService.setForceSetPasswordReason( + forcePasswordResetReason, + result.userId, + ); result.forcePasswordReset = forcePasswordResetReason; } return result; } - protected override async setMasterKey(response: IdentityTokenResponse) { + protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) { const { masterKey, localMasterKeyHash } = this.cache.value; - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; await this.masterPasswordService.setMasterKey(masterKey, userId); await this.masterPasswordService.setMasterKeyHash(localMasterKeyHash, userId); } @@ -219,12 +224,12 @@ export class PasswordLoginStrategy extends LoginStrategy { if (this.encryptionKeyMigrationRequired(response)) { return; } - await this.cryptoService.setMasterKeyEncryptedUserKey(response.key); + await this.cryptoService.setMasterKeyEncryptedUserKey(response.key, userId); const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); if (masterKey) { const userKey = await this.cryptoService.decryptUserKeyWithMasterKey(masterKey); - await this.cryptoService.setUserKey(userKey); + await this.cryptoService.setUserKey(userKey, userId); } } @@ -239,9 +244,9 @@ export class PasswordLoginStrategy extends LoginStrategy { } private getMasterPasswordPolicyOptionsFromResponse( - response: IdentityTokenResponse | IdentityTwoFactorResponse | IdentityCaptchaResponse, + response: IdentityTokenResponse | IdentityTwoFactorResponse, ): MasterPasswordPolicyOptions { - if (response == null || response instanceof IdentityCaptchaResponse) { + if (response == null) { return null; } return MasterPasswordPolicyOptions.fromResponse(response.masterPasswordPolicy); diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts index 416e910b47..1e4d867603 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts @@ -163,7 +163,10 @@ describe("SsoLoginStrategy", () => { // Assert expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledTimes(1); - expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(tokenResponse.key); + expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( + tokenResponse.key, + userId, + ); }); describe("Trusted Device Decryption", () => { @@ -417,7 +420,7 @@ describe("SsoLoginStrategy", () => { await ssoLoginStrategy.logIn(credentials); - expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl); + expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl, userId); }); it("converts new SSO user with no master password to Key Connector on first login", async () => { @@ -430,6 +433,7 @@ describe("SsoLoginStrategy", () => { expect(keyConnectorService.convertNewSsoUserToKeyConnector).toHaveBeenCalledWith( tokenResponse, ssoOrgId, + userId, ); }); @@ -468,7 +472,7 @@ describe("SsoLoginStrategy", () => { await ssoLoginStrategy.logIn(credentials); - expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl); + expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl, userId); }); it("converts new SSO user with no master password to Key Connector on first login", async () => { @@ -481,6 +485,7 @@ describe("SsoLoginStrategy", () => { expect(keyConnectorService.convertNewSsoUserToKeyConnector).toHaveBeenCalledWith( tokenResponse, ssoOrgId, + userId, ); }); diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index ad56d1ae51..37f616c98a 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -8,6 +8,7 @@ import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-con import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service"; +import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { SsoTokenRequest } from "@bitwarden/common/auth/models/request/identity-token/sso-token.request"; import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; @@ -124,7 +125,7 @@ export class SsoLoginStrategy extends LoginStrategy { this.ssoEmail2FaSessionToken$ = this.cache.pipe(map((state) => state.ssoEmail2FaSessionToken)); } - async logIn(credentials: SsoLoginCredentials) { + async logIn(credentials: SsoLoginCredentials): Promise { const data = new SsoLoginStrategyData(); data.orgId = credentials.orgId; @@ -147,10 +148,9 @@ export class SsoLoginStrategy extends LoginStrategy { // Auth guard currently handles redirects for this. if (ssoAuthResult.forcePasswordReset == ForceSetPasswordReason.AdminForcePasswordReset) { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; await this.masterPasswordService.setForceSetPasswordReason( ssoAuthResult.forcePasswordReset, - userId, + ssoAuthResult.userId, ); } @@ -163,7 +163,7 @@ export class SsoLoginStrategy extends LoginStrategy { return ssoAuthResult; } - protected override async setMasterKey(tokenResponse: IdentityTokenResponse) { + protected override async setMasterKey(tokenResponse: IdentityTokenResponse, userId: UserId) { // 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)) { @@ -175,10 +175,11 @@ export class SsoLoginStrategy extends LoginStrategy { await this.keyConnectorService.convertNewSsoUserToKeyConnector( tokenResponse, this.cache.value.orgId, + userId, ); } else { const keyConnectorUrl = this.getKeyConnectorUrl(tokenResponse); - await this.keyConnectorService.setMasterKeyFromUrl(keyConnectorUrl); + await this.keyConnectorService.setMasterKeyFromUrl(keyConnectorUrl, userId); } } } @@ -231,7 +232,7 @@ export class SsoLoginStrategy extends LoginStrategy { if (masterKeyEncryptedUserKey) { // set the master key encrypted user key if it exists - await this.cryptoService.setMasterKeyEncryptedUserKey(masterKeyEncryptedUserKey); + await this.cryptoService.setMasterKeyEncryptedUserKey(masterKeyEncryptedUserKey, userId); } const userDecryptionOptions = tokenResponse?.userDecryptionOptions; @@ -251,7 +252,7 @@ export class SsoLoginStrategy extends LoginStrategy { this.getKeyConnectorUrl(tokenResponse) != null ) { // Key connector enabled for user - await this.trySetUserKeyWithMasterKey(); + await this.trySetUserKeyWithMasterKey(userId); } // Note: In the traditional SSO flow with MP without key connector, the lock component @@ -338,8 +339,7 @@ export class SsoLoginStrategy extends LoginStrategy { } } - private async trySetUserKeyWithMasterKey(): Promise { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; + private async trySetUserKeyWithMasterKey(userId: UserId): Promise { const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); // There is a scenario in which the master key is not set here. That will occur if the user diff --git a/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts index 130e6c2d89..03dfa4f5fa 100644 --- a/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts @@ -174,7 +174,7 @@ describe("UserApiLoginStrategy", () => { await apiLogInStrategy.logIn(credentials); - expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl); + expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl, userId); }); it("decrypts and sets the user key if Key Connector is enabled", async () => { @@ -195,6 +195,6 @@ describe("UserApiLoginStrategy", () => { await apiLogInStrategy.logIn(credentials); expect(cryptoService.decryptUserKeyWithMasterKey).toHaveBeenCalledWith(masterKey); - expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey); + expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey, userId); }); }); diff --git a/libs/auth/src/common/login-strategies/user-api-login.strategy.ts b/libs/auth/src/common/login-strategies/user-api-login.strategy.ts index d7ee6fdc4b..b9d0c9e588 100644 --- a/libs/auth/src/common/login-strategies/user-api-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/user-api-login.strategy.ts @@ -93,11 +93,11 @@ export class UserApiLoginStrategy extends LoginStrategy { return authResult; } - protected override async setMasterKey(response: IdentityTokenResponse) { + protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) { if (response.apiUseKeyConnector) { const env = await firstValueFrom(this.environmentService.environment$); const keyConnectorUrl = env.getKeyConnectorUrl(); - await this.keyConnectorService.setMasterKeyFromUrl(keyConnectorUrl); + await this.keyConnectorService.setMasterKeyFromUrl(keyConnectorUrl, userId); } } @@ -108,11 +108,10 @@ export class UserApiLoginStrategy extends LoginStrategy { await this.cryptoService.setMasterKeyEncryptedUserKey(response.key); if (response.apiUseKeyConnector) { - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); if (masterKey) { const userKey = await this.cryptoService.decryptUserKeyWithMasterKey(masterKey); - await this.cryptoService.setUserKey(userKey); + await this.cryptoService.setUserKey(userKey, userId); } } } @@ -123,6 +122,7 @@ export class UserApiLoginStrategy extends LoginStrategy { ); } + // Overridden to save client ID and secret to token service protected async saveAccountInformation(tokenResponse: IdentityTokenResponse): Promise { const userId = await super.saveAccountInformation(tokenResponse); diff --git a/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts index bbcd3bafdd..9e977193dd 100644 --- a/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts @@ -208,7 +208,10 @@ describe("WebAuthnLoginStrategy", () => { // Assert // Master key encrypted user key should be set expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledTimes(1); - expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(idTokenResponse.key); + expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( + idTokenResponse.key, + userId, + ); expect(cryptoService.decryptToBytes).toHaveBeenCalledTimes(1); expect(cryptoService.decryptToBytes).toHaveBeenCalledWith( @@ -220,7 +223,7 @@ describe("WebAuthnLoginStrategy", () => { idTokenResponse.userDecryptionOptions.webAuthnPrfOption.encryptedUserKey.encryptedString, mockPrfPrivateKey, ); - expect(cryptoService.setUserKey).toHaveBeenCalledWith(mockUserKey); + expect(cryptoService.setUserKey).toHaveBeenCalledWith(mockUserKey, userId); expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(idTokenResponse.privateKey); // Master key and private key should not be set diff --git a/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts b/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts index ac487b3a82..714edabc1e 100644 --- a/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts @@ -98,7 +98,7 @@ export class WebAuthnLoginStrategy extends LoginStrategy { throw new Error("2FA not supported yet for WebAuthn Login."); } - protected override async setMasterKey() { + protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) { return Promise.resolve(); } @@ -107,7 +107,7 @@ export class WebAuthnLoginStrategy extends LoginStrategy { if (masterKeyEncryptedUserKey) { // set the master key encrypted user key if it exists - await this.cryptoService.setMasterKeyEncryptedUserKey(masterKeyEncryptedUserKey); + await this.cryptoService.setMasterKeyEncryptedUserKey(masterKeyEncryptedUserKey, userId); } const userDecryptionOptions = idTokenResponse?.userDecryptionOptions; @@ -134,7 +134,7 @@ export class WebAuthnLoginStrategy extends LoginStrategy { ); if (userKey) { - await this.cryptoService.setUserKey(new SymmetricCryptoKey(userKey) as UserKey); + await this.cryptoService.setUserKey(new SymmetricCryptoKey(userKey) as UserKey, userId); } } } diff --git a/libs/common/src/auth/abstractions/key-connector.service.ts b/libs/common/src/auth/abstractions/key-connector.service.ts index 36f413d70c..b1b6727cd1 100644 --- a/libs/common/src/auth/abstractions/key-connector.service.ts +++ b/libs/common/src/auth/abstractions/key-connector.service.ts @@ -1,8 +1,9 @@ import { Organization } from "../../admin-console/models/domain/organization"; +import { UserId } from "../../types/guid"; import { IdentityTokenResponse } from "../models/response/identity-token.response"; export abstract class KeyConnectorService { - setMasterKeyFromUrl: (url?: string) => Promise; + setMasterKeyFromUrl: (url: string, userId: UserId) => Promise; getManagingOrganization: () => Promise; getUsesKeyConnector: () => Promise; migrateUser: () => Promise; @@ -10,6 +11,7 @@ export abstract class KeyConnectorService { convertNewSsoUserToKeyConnector: ( tokenResponse: IdentityTokenResponse, orgId: string, + userId: UserId, ) => Promise; setUsesKeyConnector: (enabled: boolean) => Promise; setConvertAccountRequired: (status: boolean) => Promise; diff --git a/libs/common/src/auth/models/domain/auth-result.ts b/libs/common/src/auth/models/domain/auth-result.ts index f45466777e..bc828d3e86 100644 --- a/libs/common/src/auth/models/domain/auth-result.ts +++ b/libs/common/src/auth/models/domain/auth-result.ts @@ -1,9 +1,11 @@ import { Utils } from "../../../platform/misc/utils"; +import { UserId } from "../../../types/guid"; import { TwoFactorProviderType } from "../../enums/two-factor-provider-type"; import { ForceSetPasswordReason } from "./force-set-password-reason"; export class AuthResult { + userId: UserId; captchaSiteKey = ""; // TODO: PM-3287 - Remove this after 3 releases of backwards compatibility. - Target release 2023.12 for removal /** diff --git a/libs/common/src/auth/services/key-connector.service.spec.ts b/libs/common/src/auth/services/key-connector.service.spec.ts index e3e5fbdbe7..0fc0267a53 100644 --- a/libs/common/src/auth/services/key-connector.service.spec.ts +++ b/libs/common/src/auth/services/key-connector.service.spec.ts @@ -215,7 +215,7 @@ describe("KeyConnectorService", () => { const masterKey = new SymmetricCryptoKey(keyArr) as MasterKey; // Act - await keyConnectorService.setMasterKeyFromUrl(url); + await keyConnectorService.setMasterKeyFromUrl(url, mockUserId); // Assert expect(apiService.getMasterKeyFromKeyConnector).toHaveBeenCalledWith(url); @@ -235,7 +235,7 @@ describe("KeyConnectorService", () => { try { // Act - await keyConnectorService.setMasterKeyFromUrl(url); + await keyConnectorService.setMasterKeyFromUrl(url, mockUserId); } catch { // Assert expect(logService.error).toHaveBeenCalledWith(error); diff --git a/libs/common/src/auth/services/key-connector.service.ts b/libs/common/src/auth/services/key-connector.service.ts index c19185ae91..5065e58c76 100644 --- a/libs/common/src/auth/services/key-connector.service.ts +++ b/libs/common/src/auth/services/key-connector.service.ts @@ -16,6 +16,7 @@ import { StateProvider, UserKeyDefinition, } from "../../platform/state"; +import { UserId } from "../../types/guid"; import { MasterKey } from "../../types/key"; import { AccountService } from "../abstractions/account.service"; import { KeyConnectorService as KeyConnectorServiceAbstraction } from "../abstractions/key-connector.service"; @@ -100,12 +101,11 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { } // TODO: UserKey should be renamed to MasterKey and typed accordingly - async setMasterKeyFromUrl(url: string) { + async setMasterKeyFromUrl(url: string, userId: UserId) { try { const masterKeyResponse = await this.apiService.getMasterKeyFromKeyConnector(url); const keyArr = Utils.fromB64ToArray(masterKeyResponse.key); const masterKey = new SymmetricCryptoKey(keyArr) as MasterKey; - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; await this.masterPasswordService.setMasterKey(masterKey, userId); } catch (e) { this.handleKeyConnectorError(e); @@ -123,7 +123,11 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { ); } - async convertNewSsoUserToKeyConnector(tokenResponse: IdentityTokenResponse, orgId: string) { + async convertNewSsoUserToKeyConnector( + tokenResponse: IdentityTokenResponse, + orgId: string, + userId: UserId, + ) { // TODO: Remove after tokenResponse.keyConnectorUrl is deprecated in 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3537) const { kdf, @@ -145,12 +149,11 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { kdfConfig, ); const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64); - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; await this.masterPasswordService.setMasterKey(masterKey, userId); const userKey = await this.cryptoService.makeUserKey(masterKey); - await this.cryptoService.setUserKey(userKey[0]); - await this.cryptoService.setMasterKeyEncryptedUserKey(userKey[1].encryptedString); + await this.cryptoService.setUserKey(userKey[0], userId); + await this.cryptoService.setMasterKeyEncryptedUserKey(userKey[1].encryptedString, userId); const [pubKey, privKey] = await this.cryptoService.makeKeyPair(); diff --git a/libs/common/src/platform/services/crypto.service.ts b/libs/common/src/platform/services/crypto.service.ts index 70f10a4b98..6a5ae71529 100644 --- a/libs/common/src/platform/services/crypto.service.ts +++ b/libs/common/src/platform/services/crypto.service.ts @@ -771,6 +771,19 @@ export class CryptoService implements CryptoServiceAbstraction { publicKey: string; privateKey: EncString; }> { + // Verify keys don't exist + const existingUserKey = await this.getUserKey(); + const existingPrivateKey = await this.getPrivateKey(); + if (existingUserKey != null || existingPrivateKey != null) { + if (existingUserKey != null) { + this.logService.error("Tried to initialize account with existing user key."); + } + if (existingPrivateKey != null) { + this.logService.error("Tried to initialize account with existing private key."); + } + throw new Error("Cannot initialize account, keys already exist."); + } + const userKey = (await this.keyGenerationService.createKey(512)) as UserKey; const [publicKey, privateKey] = await this.makeKeyPair(userKey); await this.setUserKey(userKey); From 4c860e12d7c3189ba132b74b9d807ef07214dde6 Mon Sep 17 00:00:00 2001 From: Jake Fink Date: Fri, 3 May 2024 12:23:12 -0400 Subject: [PATCH 04/43] fix init account validation (#9034) --- libs/common/src/platform/services/crypto.service.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/libs/common/src/platform/services/crypto.service.ts b/libs/common/src/platform/services/crypto.service.ts index 6a5ae71529..b90fab6b49 100644 --- a/libs/common/src/platform/services/crypto.service.ts +++ b/libs/common/src/platform/services/crypto.service.ts @@ -771,16 +771,10 @@ export class CryptoService implements CryptoServiceAbstraction { publicKey: string; privateKey: EncString; }> { - // Verify keys don't exist + // Verify user key doesn't exist const existingUserKey = await this.getUserKey(); - const existingPrivateKey = await this.getPrivateKey(); - if (existingUserKey != null || existingPrivateKey != null) { - if (existingUserKey != null) { - this.logService.error("Tried to initialize account with existing user key."); - } - if (existingPrivateKey != null) { - this.logService.error("Tried to initialize account with existing private key."); - } + if (existingUserKey != null) { + this.logService.error("Tried to initialize account with existing user key."); throw new Error("Cannot initialize account, keys already exist."); } From 0b02d2ee1c2a5ec33e3ec3c85f5298d6e4101d11 Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Fri, 3 May 2024 12:36:10 -0400 Subject: [PATCH 05/43] [AC-1970] Add billing navigation group to provider layout (#8941) * Add billing navigation item to provider layout with empty subscription page behind FF. * Fixing tests * Missed build error * Addison's feedback * Remove unused function * Missed one get$ conversion * Fixed background failure --- .../providers/clients/clients.component.ts | 25 +++----- .../providers/providers-layout.component.html | 27 +++++--- .../providers/providers-layout.component.ts | 63 ++++++++----------- .../providers/providers-routing.module.ts | 23 ++++++- .../providers/providers.module.ts | 2 + .../manage-client-organizations.component.ts | 31 ++++----- .../guards/has-consolidated-billing.guard.ts | 30 +++++++++ .../src/app/billing/providers/index.ts | 2 + .../provider-subscription.component.html | 1 + .../provider-subscription.component.ts | 7 +++ .../abstractions/provider.service.ts | 3 + .../services/provider.service.spec.ts | 19 ++++++ .../services/provider.service.ts | 8 ++- .../provider-billing.service.abstraction.ts | 25 ++++++++ 14 files changed, 181 insertions(+), 85 deletions(-) create mode 100644 bitwarden_license/bit-web/src/app/billing/providers/guards/has-consolidated-billing.guard.ts create mode 100644 bitwarden_license/bit-web/src/app/billing/providers/index.ts create mode 100644 bitwarden_license/bit-web/src/app/billing/providers/provider-subscription.component.html create mode 100644 bitwarden_license/bit-web/src/app/billing/providers/provider-subscription.component.ts create mode 100644 libs/common/src/billing/abstractions/provider-billing.service.abstraction.ts diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts index 6875c3816b..7a96bdc7c7 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts @@ -1,17 +1,17 @@ import { Component } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; -import { combineLatest, firstValueFrom, from } from "rxjs"; -import { concatMap, switchMap, takeUntil } from "rxjs/operators"; +import { firstValueFrom, from, map } from "rxjs"; +import { switchMap, takeUntil } from "rxjs/operators"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; -import { ProviderStatusType, ProviderUserType } from "@bitwarden/common/admin-console/enums"; +import { ProviderUserType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { canAccessBilling } from "@bitwarden/common/billing/abstractions/provider-billing.service.abstraction"; import { PlanType } from "@bitwarden/common/billing/enums"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; @@ -40,10 +40,6 @@ export class ClientsComponent extends BaseClientsComponent { manageOrganizations = false; showAddExisting = false; - protected consolidatedBillingEnabled$ = this.configService.getFeatureFlag$( - FeatureFlag.EnableConsolidatedBilling, - ); - constructor( private router: Router, private providerService: ProviderService, @@ -75,15 +71,10 @@ export class ClientsComponent extends BaseClientsComponent { .pipe( switchMap((params) => { this.providerId = params.providerId; - return combineLatest([ - this.providerService.get(this.providerId), - this.consolidatedBillingEnabled$, - ]).pipe( - concatMap(([provider, consolidatedBillingEnabled]) => { - if ( - consolidatedBillingEnabled && - provider.providerStatus === ProviderStatusType.Billable - ) { + return this.providerService.get$(this.providerId).pipe( + canAccessBilling(this.configService), + map((canAccessBilling) => { + if (canAccessBilling) { return from( this.router.navigate(["../manage-client-organizations"], { relativeTo: this.activatedRoute, diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.html b/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.html index 55efbe1386..a1cf2cc5aa 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.html @@ -1,5 +1,5 @@ -