From d5102f16247c3b4840db40ae2e6a990943cd2bb7 Mon Sep 17 00:00:00 2001 From: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Date: Wed, 19 Jul 2023 14:16:26 -0400 Subject: [PATCH] Auth/[pm-2759] - TDE - SSO and 2FA routing logic (#5829) * PM-2759 - SsoComp - (1) Temp remove all TDE routing logic (2) Refactor existing navigation logic via new component utility function navigateViaCallbackOrRoute * PM-2759 - SSO Component - Create test suite for logIn logic * PM-2759 - SsoComp Tests - add disclaimer regarding testing private methods and props * PM-1259 - SSO Comp - Refactor LogIn method to use functions for each navigation case for improved readability * PM-1259 - SSO Comp Tests - Add tests for error case during login + test for new handleLoginError logic * PM-2759 - SsoComp - Deprecate resetMasterPassword and replace with AccountDecryptionOptions logic + update tests * PM-2759 - SsoComp + tests - Add trusted device encryption first draft handling which has login success and force password reset handling * PM-2759 - Minor SsoComp comment and method name tweaks * PM-2759 - BaseTwoFactorComp - (1) Comment out TDE stuff for now (2) Add test suite (3) Replace global window in base comp constructor with angular injection token for window which follows best practices and allows for mocking so the comp can be unit tested * PM-2759 - Update child 2FA components to use angular injection token for window like base comp * PM-2759 - TwoFactorComp - Finish testing all logic in doSubmit * PM-2759 - TwoFactorComponent - Refactor DoSubmit method logic into multiple simple functions to make logic easier to follow * PM-2759 - Add newtrustedDeviceOption.hasManageResetPasswordPermission property to match server changes * PM-2759 - Flag AuthResult.resetMasterPassword property as deprecated * PM-2759 - SSO comp - TDE routing logic - User without MP and ResetPassword permission must set a MP * PM-2759 - Update Sso Comp tests to reflect additionally added TDE > MP set required logic (when user has no MP but they can reset other user passwords) * PM-2759 - SsoComp - Add comment explaining the happy paths better for TDE success navigation * PM-2759 - SsoComp - Refactor isTrustedDeviceEncEnabled logic into own method * PM-2759 - SsoComp - As the 2FA comp passes the org id through to each route, going to standardize on doing so across the board for now to avoid any tricky scenarios down the line where it is needed and it's not present * PM-2759 - SsoComp - Finish renaming orgIdFromState to orgIdentifier * PM-2759 - SsoComp - update tests for forcePasswordReset flows now passing orgIdentifier as query param * PM-2759 - SsoComp Tests - Export mockAcctDecryptionOpts permutations so we can share them across SsoComp and TwoFactorComp tests * PM-2759 - Refactor 2FA comp post login redirect logic to match SSO component + add TDE logic * PM-2759 - SsoComp - Refactor tests a bit for improved re-use * PM-2759 - Sso Comp tests - can't export consts from a spec file or the other spec files that import them will re-execute the whole test suite as a nested test suite. TIL. * PM-2759 - TwoFactorComp tests - All existing navigation scenarios + new TDE scenarios should now be tested. * PM-2759 - Web - 2FA comp - Fix build error b/c of renamed base comp prop (identifier --> orgIdentifier) * PM-2759 - Fix SsoLogin strategy tests b/c they were broken w/ the addition of the HasManageResetPasswordPermission prop to the TrustedDeviceOption interface * PM-2759 - Web TwoFactorComp - goAfterLogIn method must be an arrow function to inherit the parent base component scope so that important things like angular services can be defined. Web 2FA flow does not work without this being an arrow func. * PM-2759 - Fix typo * PM-2759 - SsoComp and TwoFactorComp tests - move service and other mocks into the top level before each to better ensure no crossover between test states per PR feedback * PM-2759 - SsoComp - add clarity by refactoring unclear comment * PM-2759 - SsoComp - Per excellent PR feedback, refactor if else statements to guard statements for better readability / design * PM-2759 - TwoFactorComp - Replace ifs with guard statements * PM-2759 - TwoFactorComp - add clarity to comment per PR feedback * PM-2759 - Replace use of jest.Mocked with MockProxy per PR feedback * PM-2759 - Use unknown over any per PR feedback --- .../src/auth/popup/two-factor.component.ts | 8 +- apps/desktop/src/auth/two-factor.component.ts | 8 +- apps/web/src/app/auth/two-factor.component.ts | 14 +- .../src/auth/components/sso.component.spec.ts | 552 ++++++++++++++++++ .../src/auth/components/sso.component.ts | 217 ++++--- .../components/two-factor.component.spec.ts | 443 ++++++++++++++ .../auth/components/two-factor.component.ts | 176 ++++-- .../sso-login.strategy.spec.ts | 1 + .../src/auth/models/domain/auth-result.ts | 6 + .../trusted-device-user-decryption-option.ts | 6 +- ...-device-user-decryption-option.response.ts | 5 + .../src/platform/models/domain/account.ts | 6 +- 12 files changed, 1315 insertions(+), 127 deletions(-) create mode 100644 libs/angular/src/auth/components/sso.component.spec.ts create mode 100644 libs/angular/src/auth/components/two-factor.component.spec.ts diff --git a/apps/browser/src/auth/popup/two-factor.component.ts b/apps/browser/src/auth/popup/two-factor.component.ts index 2f93dc7664..aeaaaf7474 100644 --- a/apps/browser/src/auth/popup/two-factor.component.ts +++ b/apps/browser/src/auth/popup/two-factor.component.ts @@ -1,9 +1,10 @@ -import { Component } from "@angular/core"; +import { Component, Inject } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; import { first } from "rxjs/operators"; import { TwoFactorComponent as BaseTwoFactorComponent } from "@bitwarden/angular/auth/components/two-factor.component"; import { DialogServiceAbstraction, SimpleDialogType } from "@bitwarden/angular/services/dialog"; +import { WINDOW } from "@bitwarden/angular/services/injection-tokens"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { LoginService } from "@bitwarden/common/auth/abstractions/login.service"; @@ -50,7 +51,8 @@ export class TwoFactorComponent extends BaseTwoFactorComponent { appIdService: AppIdService, loginService: LoginService, configService: ConfigServiceAbstraction, - private dialogService: DialogServiceAbstraction + private dialogService: DialogServiceAbstraction, + @Inject(WINDOW) protected win: Window ) { super( authService, @@ -58,7 +60,7 @@ export class TwoFactorComponent extends BaseTwoFactorComponent { i18nService, apiService, platformUtilsService, - window, + win, environmentService, stateService, route, diff --git a/apps/desktop/src/auth/two-factor.component.ts b/apps/desktop/src/auth/two-factor.component.ts index 93833f68aa..cf8b18a360 100644 --- a/apps/desktop/src/auth/two-factor.component.ts +++ b/apps/desktop/src/auth/two-factor.component.ts @@ -1,7 +1,8 @@ -import { Component, ViewChild, ViewContainerRef } from "@angular/core"; +import { Component, Inject, ViewChild, ViewContainerRef } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; import { TwoFactorComponent as BaseTwoFactorComponent } from "@bitwarden/angular/auth/components/two-factor.component"; +import { WINDOW } from "@bitwarden/angular/services/injection-tokens"; import { ModalService } from "@bitwarden/angular/services/modal.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; @@ -45,7 +46,8 @@ export class TwoFactorComponent extends BaseTwoFactorComponent { twoFactorService: TwoFactorService, appIdService: AppIdService, loginService: LoginService, - configService: ConfigServiceAbstraction + configService: ConfigServiceAbstraction, + @Inject(WINDOW) protected win: Window ) { super( authService, @@ -53,7 +55,7 @@ export class TwoFactorComponent extends BaseTwoFactorComponent { i18nService, apiService, platformUtilsService, - window, + win, environmentService, stateService, route, diff --git a/apps/web/src/app/auth/two-factor.component.ts b/apps/web/src/app/auth/two-factor.component.ts index a37ebeea2c..2990331761 100644 --- a/apps/web/src/app/auth/two-factor.component.ts +++ b/apps/web/src/app/auth/two-factor.component.ts @@ -1,7 +1,8 @@ -import { Component, ViewChild, ViewContainerRef } from "@angular/core"; +import { Component, Inject, ViewChild, ViewContainerRef } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; import { TwoFactorComponent as BaseTwoFactorComponent } from "@bitwarden/angular/auth/components/two-factor.component"; +import { WINDOW } from "@bitwarden/angular/services/injection-tokens"; import { ModalService } from "@bitwarden/angular/services/modal.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; @@ -44,7 +45,8 @@ export class TwoFactorComponent extends BaseTwoFactorComponent { appIdService: AppIdService, private routerService: RouterService, loginService: LoginService, - configService: ConfigServiceAbstraction + configService: ConfigServiceAbstraction, + @Inject(WINDOW) protected win: Window ) { super( authService, @@ -52,7 +54,7 @@ export class TwoFactorComponent extends BaseTwoFactorComponent { i18nService, apiService, platformUtilsService, - window, + win, environmentService, stateService, route, @@ -84,7 +86,7 @@ export class TwoFactorComponent extends BaseTwoFactorComponent { ); } - async goAfterLogIn() { + goAfterLogIn = async () => { this.loginService.clearValues(); const previousUrl = this.routerService.getPreviousUrl(); if (previousUrl) { @@ -106,9 +108,9 @@ export class TwoFactorComponent extends BaseTwoFactorComponent { this.router.navigate([this.successRoute], { queryParams: { - identifier: this.identifier, + identifier: this.orgIdentifier, }, }); } - } + }; } diff --git a/libs/angular/src/auth/components/sso.component.spec.ts b/libs/angular/src/auth/components/sso.component.spec.ts new file mode 100644 index 0000000000..d0eb11bc5e --- /dev/null +++ b/libs/angular/src/auth/components/sso.component.spec.ts @@ -0,0 +1,552 @@ +import { Component } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { ActivatedRoute, Router } from "@angular/router"; +import { MockProxy, mock } from "jest-mock-extended"; +import { Observable, of } from "rxjs"; + +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type"; +import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; +import { ForceResetPasswordReason } from "@bitwarden/common/auth/models/domain/force-reset-password-reason"; +import { KeyConnectorUserDecryptionOption } from "@bitwarden/common/auth/models/domain/user-decryption-options/key-connector-user-decryption-option"; +import { TrustedDeviceUserDecryptionOption } from "@bitwarden/common/auth/models/domain/user-decryption-options/trusted-device-user-decryption-option"; +import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction"; +import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; +import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { AccountDecryptionOptions } from "@bitwarden/common/platform/models/domain/account"; +import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; + +import { SsoComponent } from "./sso.component"; +// test component that extends the SsoComponent +@Component({}) +class TestSsoComponent extends SsoComponent {} + +interface SsoComponentProtected { + twoFactorRoute: string; + successRoute: string; + trustedDeviceEncRoute: string; + changePasswordRoute: string; + forcePasswordResetRoute: string; + logIn(code: string, codeVerifier: string, orgIdFromState: string): Promise; + handleLoginError(e: any): Promise; +} + +// The ideal scenario would be to not have to test the protected / private methods of the SsoComponent +// but that will require a refactor of the SsoComponent class which is out of scope for now. +// This test suite allows us to be sure that the new Trusted Device encryption flows + mild refactors +// of the SsoComponent don't break the existing post login flows. +describe("SsoComponent", () => { + let component: TestSsoComponent; + let _component: SsoComponentProtected; + let fixture: ComponentFixture; + + // Mock Services + let mockAuthService: MockProxy; + let mockRouter: MockProxy; + let mockI18nService: MockProxy; + + let mockQueryParams: Observable; + let mockActivatedRoute: ActivatedRoute; + + let mockStateService: MockProxy; + let mockPlatformUtilsService: MockProxy; + let mockApiService: MockProxy; + let mockCryptoFunctionService: MockProxy; + let mockEnvironmentService: MockProxy; + let mockPasswordGenerationService: MockProxy; + let mockLogService: MockProxy; + let mockConfigService: MockProxy; + + // Mock authService.logIn params + let code: string; + let codeVerifier: string; + let orgIdFromState: string; + + // Mock component callbacks + let mockOnSuccessfulLogin: jest.Mock; + let mockOnSuccessfulLoginNavigate: jest.Mock; + let mockOnSuccessfulLoginTwoFactorNavigate: jest.Mock; + let mockOnSuccessfulLoginChangePasswordNavigate: jest.Mock; + let mockOnSuccessfulLoginForceResetNavigate: jest.Mock; + + let mockAcctDecryptionOpts: { + noMasterPassword: AccountDecryptionOptions; + withMasterPassword: AccountDecryptionOptions; + withMasterPasswordAndTrustedDevice: AccountDecryptionOptions; + withMasterPasswordAndTrustedDeviceWithManageResetPassword: AccountDecryptionOptions; + withMasterPasswordAndKeyConnector: AccountDecryptionOptions; + noMasterPasswordWithTrustedDevice: AccountDecryptionOptions; + noMasterPasswordWithTrustedDeviceWithManageResetPassword: AccountDecryptionOptions; + noMasterPasswordWithKeyConnector: AccountDecryptionOptions; + }; + + beforeEach(() => { + // Mock Services + mockAuthService = mock(); + mockRouter = mock(); + mockI18nService = mock(); + + // Default mockQueryParams + mockQueryParams = of({ code: "code", state: "state" }); + // Create a custom mock for ActivatedRoute with mock queryParams + mockActivatedRoute = { + queryParams: mockQueryParams, + } as any as ActivatedRoute; + + mockStateService = mock(); + mockPlatformUtilsService = mock(); + mockApiService = mock(); + mockCryptoFunctionService = mock(); + mockEnvironmentService = mock(); + mockPasswordGenerationService = mock(); + mockLogService = mock(); + mockConfigService = mock(); + + // Mock authService.logIn params + code = "code"; + codeVerifier = "codeVerifier"; + orgIdFromState = "orgIdFromState"; + + // Mock component callbacks + mockOnSuccessfulLogin = jest.fn(); + mockOnSuccessfulLoginNavigate = jest.fn(); + mockOnSuccessfulLoginTwoFactorNavigate = jest.fn(); + mockOnSuccessfulLoginChangePasswordNavigate = jest.fn(); + mockOnSuccessfulLoginForceResetNavigate = jest.fn(); + + mockAcctDecryptionOpts = { + noMasterPassword: new AccountDecryptionOptions({ + hasMasterPassword: false, + trustedDeviceOption: undefined, + keyConnectorOption: undefined, + }), + withMasterPassword: new AccountDecryptionOptions({ + hasMasterPassword: true, + trustedDeviceOption: undefined, + keyConnectorOption: undefined, + }), + withMasterPasswordAndTrustedDevice: new AccountDecryptionOptions({ + hasMasterPassword: true, + trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false), + keyConnectorOption: undefined, + }), + withMasterPasswordAndTrustedDeviceWithManageResetPassword: new AccountDecryptionOptions({ + hasMasterPassword: true, + trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true), + keyConnectorOption: undefined, + }), + withMasterPasswordAndKeyConnector: new AccountDecryptionOptions({ + hasMasterPassword: true, + trustedDeviceOption: undefined, + keyConnectorOption: new KeyConnectorUserDecryptionOption("http://example.com"), + }), + noMasterPasswordWithTrustedDevice: new AccountDecryptionOptions({ + hasMasterPassword: false, + trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false), + keyConnectorOption: undefined, + }), + noMasterPasswordWithTrustedDeviceWithManageResetPassword: new AccountDecryptionOptions({ + hasMasterPassword: false, + trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true), + keyConnectorOption: undefined, + }), + noMasterPasswordWithKeyConnector: new AccountDecryptionOptions({ + hasMasterPassword: false, + trustedDeviceOption: undefined, + keyConnectorOption: new KeyConnectorUserDecryptionOption("http://example.com"), + }), + }; + + TestBed.configureTestingModule({ + declarations: [TestSsoComponent], + providers: [ + { provide: AuthService, useValue: mockAuthService }, + { provide: Router, useValue: mockRouter }, + { provide: I18nService, useValue: mockI18nService }, + { provide: ActivatedRoute, useValue: mockActivatedRoute }, + { provide: StateService, useValue: mockStateService }, + { provide: PlatformUtilsService, useValue: mockPlatformUtilsService }, + + { provide: ApiService, useValue: mockApiService }, + { provide: CryptoFunctionService, useValue: mockCryptoFunctionService }, + { provide: EnvironmentService, useValue: mockEnvironmentService }, + { provide: PasswordGenerationServiceAbstraction, useValue: mockPasswordGenerationService }, + + { provide: LogService, useValue: mockLogService }, + { provide: ConfigServiceAbstraction, useValue: mockConfigService }, + ], + }); + + fixture = TestBed.createComponent(TestSsoComponent); + component = fixture.componentInstance; + _component = component as any; + }); + + afterEach(() => { + // Reset all mocks after each test + jest.resetAllMocks(); + }); + + it("should create", () => { + expect(component).toBeTruthy(); + }); + + describe("navigateViaCallbackOrRoute(...)", () => { + it("calls the provided callback when callback is defined", async () => { + const callback = jest.fn().mockResolvedValue(null); + const commands = ["some", "route"]; + + await (component as any).navigateViaCallbackOrRoute(callback, commands); + + expect(callback).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).not.toHaveBeenCalled(); + }); + + it("calls router.navigate when callback is not defined", async () => { + const commands = ["some", "route"]; + + await (component as any).navigateViaCallbackOrRoute(undefined, commands); + + expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith(commands, undefined); + }); + }); + + describe("logIn(...)", () => { + describe("2FA scenarios", () => { + beforeEach(() => { + const authResult = new AuthResult(); + authResult.twoFactorProviders = new Map([[TwoFactorProviderType.Authenticator, {}]]); + + // use standard user with MP because this test is not concerned with password reset. + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.withMasterPassword + ); + + mockAuthService.logIn.mockResolvedValue(authResult); + }); + + it("calls authService.logIn and navigates to the component's defined 2FA route when the auth result requires 2FA and onSuccessfulLoginTwoFactorNavigate is not defined", async () => { + await _component.logIn(code, codeVerifier, orgIdFromState); + expect(mockAuthService.logIn).toHaveBeenCalledTimes(1); + + expect(mockOnSuccessfulLoginTwoFactorNavigate).not.toHaveBeenCalled(); + + expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith([_component.twoFactorRoute], { + queryParams: { + identifier: orgIdFromState, + sso: "true", + }, + }); + + expect(mockLogService.error).not.toHaveBeenCalled(); + }); + + it("calls onSuccessfulLoginTwoFactorNavigate instead of router.navigate when response.requiresTwoFactor is true and the callback is defined", async () => { + mockOnSuccessfulLoginTwoFactorNavigate = jest.fn().mockResolvedValue(null); + component.onSuccessfulLoginTwoFactorNavigate = mockOnSuccessfulLoginTwoFactorNavigate; + + await _component.logIn(code, codeVerifier, orgIdFromState); + + expect(mockAuthService.logIn).toHaveBeenCalledTimes(1); + expect(mockOnSuccessfulLoginTwoFactorNavigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).not.toHaveBeenCalled(); + expect(mockLogService.error).not.toHaveBeenCalled(); + }); + }); + + // Shared test helpers + const testChangePasswordOnSuccessfulLogin = () => { + it("navigates to the component's defined change password route when onSuccessfulLoginChangePasswordNavigate callback is undefined", async () => { + await _component.logIn(code, codeVerifier, orgIdFromState); + expect(mockAuthService.logIn).toHaveBeenCalledTimes(1); + + expect(mockOnSuccessfulLoginChangePasswordNavigate).not.toHaveBeenCalled(); + + expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith([_component.changePasswordRoute], { + queryParams: { + identifier: orgIdFromState, + }, + }); + + expect(mockLogService.error).not.toHaveBeenCalled(); + }); + }; + + const testOnSuccessfulLoginChangePasswordNavigate = () => { + it("calls onSuccessfulLoginChangePasswordNavigate instead of router.navigate when the callback is defined", async () => { + mockOnSuccessfulLoginChangePasswordNavigate = jest.fn().mockResolvedValue(null); + component.onSuccessfulLoginChangePasswordNavigate = + mockOnSuccessfulLoginChangePasswordNavigate; + + await _component.logIn(code, codeVerifier, orgIdFromState); + + expect(mockAuthService.logIn).toHaveBeenCalledTimes(1); + expect(mockOnSuccessfulLoginChangePasswordNavigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).not.toHaveBeenCalled(); + expect(mockLogService.error).not.toHaveBeenCalled(); + }); + }; + + const testForceResetOnSuccessfulLogin = (reasonString: string) => { + it(`navigates to the component's defined forcePasswordResetRoute when response.forcePasswordReset is ${reasonString}`, async () => { + await _component.logIn(code, codeVerifier, orgIdFromState); + + expect(mockAuthService.logIn).toHaveBeenCalledTimes(1); + + expect(mockOnSuccessfulLoginForceResetNavigate).not.toHaveBeenCalled(); + + expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith([_component.forcePasswordResetRoute], { + queryParams: { + identifier: orgIdFromState, + }, + }); + expect(mockLogService.error).not.toHaveBeenCalled(); + }); + }; + + const testOnSuccessfulLoginForceResetNavigate = (reasonString: string) => { + it(`calls onSuccessfulLoginForceResetNavigate instead of router.navigate when response.forcePasswordReset is ${reasonString} and the callback is defined`, async () => { + mockOnSuccessfulLoginForceResetNavigate = jest.fn().mockResolvedValue(null); + component.onSuccessfulLoginForceResetNavigate = mockOnSuccessfulLoginForceResetNavigate; + + await _component.logIn(code, codeVerifier, orgIdFromState); + + expect(mockAuthService.logIn).toHaveBeenCalledTimes(1); + expect(mockOnSuccessfulLoginForceResetNavigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).not.toHaveBeenCalled(); + expect(mockLogService.error).not.toHaveBeenCalled(); + }); + }; + + describe("Trusted Device Encryption scenarios", () => { + beforeEach(() => { + mockConfigService.getFeatureFlagBool.mockResolvedValue(true); // TDE enabled + }); + + describe("Given Trusted Device Encryption is enabled and user needs to set a master password", () => { + let authResult; + beforeEach(() => { + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.noMasterPasswordWithTrustedDeviceWithManageResetPassword + ); + + authResult = new AuthResult(); + mockAuthService.logIn.mockResolvedValue(authResult); + }); + + testChangePasswordOnSuccessfulLogin(); + testOnSuccessfulLoginChangePasswordNavigate(); + }); + + describe("Given Trusted Device Encryption is enabled, user doesn't need to set a MP, and forcePasswordReset is required", () => { + [ + ForceResetPasswordReason.AdminForcePasswordReset, + ForceResetPasswordReason.WeakMasterPassword, + ].forEach((forceResetPasswordReason) => { + const reasonString = ForceResetPasswordReason[forceResetPasswordReason]; + let authResult; + beforeEach(() => { + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.withMasterPasswordAndTrustedDevice + ); + + authResult = new AuthResult(); + authResult.forcePasswordReset = ForceResetPasswordReason.AdminForcePasswordReset; + mockAuthService.logIn.mockResolvedValue(authResult); + }); + + testForceResetOnSuccessfulLogin(reasonString); + testOnSuccessfulLoginForceResetNavigate(reasonString); + }); + }); + + describe("Given Trusted Device Encryption is enabled, user doesn't need to set a MP, and forcePasswordReset is not required", () => { + let authResult; + beforeEach(() => { + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.withMasterPasswordAndTrustedDevice + ); + + authResult = new AuthResult(); + authResult.forcePasswordReset = ForceResetPasswordReason.None; + mockAuthService.logIn.mockResolvedValue(authResult); + }); + + it("navigates to the component's defined trusted device encryption route when login is successful", async () => { + await _component.logIn(code, codeVerifier, orgIdFromState); + + expect(mockAuthService.logIn).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith([_component.trustedDeviceEncRoute], { + queryParams: { + identifier: orgIdFromState, + }, + }); + expect(mockLogService.error).not.toHaveBeenCalled(); + }); + }); + }); + + describe("Set Master Password scenarios", () => { + beforeEach(() => { + const authResult = new AuthResult(); + mockAuthService.logIn.mockResolvedValue(authResult); + }); + + describe("Given user needs to set a master password", () => { + beforeEach(() => { + // Only need to test the case where the user has no master password to test the primary change mp flow here + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.noMasterPassword + ); + }); + + testChangePasswordOnSuccessfulLogin(); + testOnSuccessfulLoginChangePasswordNavigate(); + }); + + it("does not navigate to the change password route when the user has key connector even if user has no master password", async () => { + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.noMasterPasswordWithKeyConnector + ); + + await _component.logIn(code, codeVerifier, orgIdFromState); + expect(mockAuthService.logIn).toHaveBeenCalledTimes(1); + + expect(mockOnSuccessfulLoginChangePasswordNavigate).not.toHaveBeenCalled(); + expect(mockRouter.navigate).not.toHaveBeenCalledWith([_component.changePasswordRoute], { + queryParams: { + identifier: orgIdFromState, + }, + }); + }); + }); + + describe("Force Master Password Reset scenarios", () => { + [ + ForceResetPasswordReason.AdminForcePasswordReset, + ForceResetPasswordReason.WeakMasterPassword, + ].forEach((forceResetPasswordReason) => { + const reasonString = ForceResetPasswordReason[forceResetPasswordReason]; + + beforeEach(() => { + // use standard user with MP because this test is not concerned with password reset. + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.withMasterPassword + ); + + const authResult = new AuthResult(); + authResult.forcePasswordReset = forceResetPasswordReason; + mockAuthService.logIn.mockResolvedValue(authResult); + }); + + testForceResetOnSuccessfulLogin(reasonString); + testOnSuccessfulLoginForceResetNavigate(reasonString); + }); + }); + + describe("Success scenarios", () => { + beforeEach(() => { + const authResult = new AuthResult(); + authResult.twoFactorProviders = null; + // use standard user with MP because this test is not concerned with password reset. + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.withMasterPassword + ); + authResult.forcePasswordReset = ForceResetPasswordReason.None; + mockAuthService.logIn.mockResolvedValue(authResult); + }); + + it("calls authService.logIn and navigates to the component's defined success route when the login is successful", async () => { + await _component.logIn(code, codeVerifier, orgIdFromState); + + expect(mockAuthService.logIn).toHaveBeenCalled(); + + expect(mockOnSuccessfulLoginNavigate).not.toHaveBeenCalled(); + expect(mockOnSuccessfulLogin).not.toHaveBeenCalled(); + + expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith([_component.successRoute], undefined); + expect(mockLogService.error).not.toHaveBeenCalled(); + }); + + it("calls onSuccessfulLogin if defined when login is successful", async () => { + mockOnSuccessfulLogin = jest.fn().mockResolvedValue(null); + component.onSuccessfulLogin = mockOnSuccessfulLogin; + + await _component.logIn(code, codeVerifier, orgIdFromState); + + expect(mockAuthService.logIn).toHaveBeenCalled(); + expect(mockOnSuccessfulLogin).toHaveBeenCalledTimes(1); + + expect(mockOnSuccessfulLoginNavigate).not.toHaveBeenCalled(); + + expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith([_component.successRoute], undefined); + + expect(mockLogService.error).not.toHaveBeenCalled(); + }); + + it("calls onSuccessfulLoginNavigate instead of router.navigate when login is successful and the callback is defined", async () => { + mockOnSuccessfulLoginNavigate = jest.fn().mockResolvedValue(null); + component.onSuccessfulLoginNavigate = mockOnSuccessfulLoginNavigate; + + await _component.logIn(code, codeVerifier, orgIdFromState); + + expect(mockAuthService.logIn).toHaveBeenCalled(); + + expect(mockOnSuccessfulLoginNavigate).toHaveBeenCalledTimes(1); + + expect(mockRouter.navigate).not.toHaveBeenCalled(); + + expect(mockLogService.error).not.toHaveBeenCalled(); + }); + }); + + describe("Error scenarios", () => { + it("calls handleLoginError when an error is thrown during logIn", async () => { + const errorMessage = "Key Connector error"; + const error = new Error(errorMessage); + mockAuthService.logIn.mockRejectedValue(error); + + const handleLoginErrorSpy = jest.spyOn(_component, "handleLoginError"); + + await _component.logIn(code, codeVerifier, orgIdFromState); + + expect(handleLoginErrorSpy).toHaveBeenCalledWith(error); + }); + }); + }); + + describe("handleLoginError(e)", () => { + it("logs the error and shows a toast when the error message is 'Key Connector error'", async () => { + const errorMessage = "Key Connector error"; + const error = new Error(errorMessage); + + mockI18nService.t.mockReturnValueOnce("ssoKeyConnectorError"); + + await _component.handleLoginError(error); + + expect(mockLogService.error).toHaveBeenCalledTimes(1); + expect(mockLogService.error).toHaveBeenCalledWith(error); + + expect(mockPlatformUtilsService.showToast).toHaveBeenCalledTimes(1); + expect(mockPlatformUtilsService.showToast).toHaveBeenCalledWith( + "error", + null, + "ssoKeyConnectorError" + ); + + expect(mockRouter.navigate).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/libs/angular/src/auth/components/sso.component.ts b/libs/angular/src/auth/components/sso.component.ts index a565c526c5..f05caafef8 100644 --- a/libs/angular/src/auth/components/sso.component.ts +++ b/libs/angular/src/auth/components/sso.component.ts @@ -1,5 +1,5 @@ import { Directive } from "@angular/core"; -import { ActivatedRoute, Router } from "@angular/router"; +import { ActivatedRoute, NavigationExtras, Router } from "@angular/router"; import { first } from "rxjs/operators"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -7,6 +7,7 @@ import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; import { ForceResetPasswordReason } from "@bitwarden/common/auth/models/domain/force-reset-password-reason"; import { SsoLogInCredentials } from "@bitwarden/common/auth/models/domain/log-in-credentials"; +import { TrustedDeviceUserDecryptionOption } from "@bitwarden/common/auth/models/domain/user-decryption-options/trusted-device-user-decryption-option"; import { SsoPreValidateResponse } from "@bitwarden/common/auth/models/response/sso-pre-validate.response"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction"; @@ -37,7 +38,6 @@ export class SsoComponent { protected successRoute = "lock"; protected trustedDeviceEncRoute = "login-initiated"; protected changePasswordRoute = "set-password"; - protected tdeLogin = "login-initiated"; protected forcePasswordResetRoute = "update-temp-password"; protected clientId: string; protected redirectUri: string; @@ -179,88 +179,169 @@ export class SsoComponent { return authorizeUrl; } - private async logIn(code: string, codeVerifier: string, orgIdFromState: string) { + private async logIn(code: string, codeVerifier: string, orgIdentifier: string) { this.loggingIn = true; try { const credentials = new SsoLogInCredentials( code, codeVerifier, this.redirectUri, - orgIdFromState + orgIdentifier ); this.formPromise = this.authService.logIn(credentials); - const response = await this.formPromise; + const authResult = await this.formPromise; - const trustedDeviceEncryptionFeatureActive = await this.configService.getFeatureFlagBool( - FeatureFlag.TrustedDeviceEncryption - ); - - const accountDecryptionOptions: AccountDecryptionOptions = + const acctDecryptionOpts: AccountDecryptionOptions = await this.stateService.getAccountDecryptionOptions(); - if (response.requiresTwoFactor) { - if (this.onSuccessfulLoginTwoFactorNavigate != null) { - await this.onSuccessfulLoginTwoFactorNavigate(); - } else { - this.router.navigate([this.twoFactorRoute], { - queryParams: { - identifier: orgIdFromState, - sso: "true", - }, - }); - } - } else if ( - trustedDeviceEncryptionFeatureActive && - accountDecryptionOptions.trustedDeviceOption !== undefined - ) { - this.router.navigate([this.trustedDeviceEncRoute], { - queryParams: { - identifier: orgIdFromState, - }, - }); - } else if (response.resetMasterPassword) { - // TODO: for TDE, we are going to deprecate using response.resetMasterPassword - // and instead rely on accountDecryptionOptions to determine if the user needs to set a password - // Users are allowed to not have a MP if TDE feature enabled + TDE configured. Otherwise, they must set a MP - // src: https://bitwarden.atlassian.net/browse/PM-2759?focusedCommentId=39438 - if (this.onSuccessfulLoginChangePasswordNavigate != null) { - await this.onSuccessfulLoginChangePasswordNavigate(); - } else { - this.router.navigate([this.changePasswordRoute], { - queryParams: { - identifier: orgIdFromState, - }, - }); - } - } else if (response.forcePasswordReset !== ForceResetPasswordReason.None) { - if (this.onSuccessfulLoginForceResetNavigate != null) { - await this.onSuccessfulLoginForceResetNavigate(); - } else { - this.router.navigate([this.forcePasswordResetRoute]); - } - } else { - if (this.onSuccessfulLogin != null) { - await this.onSuccessfulLogin(); - } - if (this.onSuccessfulLoginNavigate != null) { - await this.onSuccessfulLoginNavigate(); - } else { - this.router.navigate([this.successRoute]); - } + if (authResult.requiresTwoFactor) { + return await this.handleTwoFactorRequired(orgIdentifier); } - } catch (e) { - this.logService.error(e); - // TODO: Key Connector Service should pass this error message to the logout callback instead of displaying here - if (e.message === "Key Connector error") { - this.platformUtilsService.showToast( - "error", - null, - this.i18nService.t("ssoKeyConnectorError") + const tdeEnabled = await this.isTrustedDeviceEncEnabled( + acctDecryptionOpts.trustedDeviceOption + ); + + if (tdeEnabled) { + return await this.handleTrustedDeviceEncryptionEnabled( + authResult, + orgIdentifier, + acctDecryptionOpts ); } + + // In the standard, non TDE case, a user must set password if they don't + // have one and they aren't using key connector. + // Note: TDE & Key connector are mutually exclusive org config options. + const requireSetPassword = + !acctDecryptionOpts.hasMasterPassword && + acctDecryptionOpts.keyConnectorOption === undefined; + + if (requireSetPassword) { + // Change implies going no password -> password in this case + return await this.handleChangePasswordRequired(orgIdentifier); + } + + // Users can be forced to reset their password via an admin or org policy + // disallowing weak passwords + if (authResult.forcePasswordReset !== ForceResetPasswordReason.None) { + return await this.handleForcePasswordReset(orgIdentifier); + } + + // Standard SSO login success case + return await this.handleSuccessfulLogin(); + } catch (e) { + await this.handleLoginError(e); + } finally { + this.loggingIn = false; + } + } + + private async isTrustedDeviceEncEnabled( + trustedDeviceOption: TrustedDeviceUserDecryptionOption + ): Promise { + const trustedDeviceEncryptionFeatureActive = await this.configService.getFeatureFlagBool( + FeatureFlag.TrustedDeviceEncryption + ); + + return trustedDeviceEncryptionFeatureActive && trustedDeviceOption !== undefined; + } + + private async handleTwoFactorRequired(orgIdentifier: string) { + await this.navigateViaCallbackOrRoute( + this.onSuccessfulLoginTwoFactorNavigate, + [this.twoFactorRoute], + { + queryParams: { + identifier: orgIdentifier, + sso: "true", + }, + } + ); + } + + private async handleTrustedDeviceEncryptionEnabled( + authResult: AuthResult, + orgIdentifier: string, + acctDecryptionOpts: AccountDecryptionOptions + ): Promise { + // If user doesn't have a MP, but has reset password permission, they must set a MP + if ( + !acctDecryptionOpts.hasMasterPassword && + acctDecryptionOpts.trustedDeviceOption.hasManageResetPasswordPermission + ) { + // Change implies going no password -> password in this case + return await this.handleChangePasswordRequired(orgIdentifier); + } + + if (authResult.forcePasswordReset !== ForceResetPasswordReason.None) { + return await this.handleForcePasswordReset(orgIdentifier); + } + + // Navigate to TDE page (if user was on trusted device and TDE has decrypted + // their user key, the lock guard will redirect them to the vault) + this.router.navigate([this.trustedDeviceEncRoute], { + queryParams: { + identifier: orgIdentifier, + }, + }); + } + + private async handleChangePasswordRequired(orgIdentifier: string) { + await this.navigateViaCallbackOrRoute( + this.onSuccessfulLoginChangePasswordNavigate, + [this.changePasswordRoute], + { + queryParams: { + identifier: orgIdentifier, + }, + } + ); + } + + private async handleForcePasswordReset(orgIdentifier: string) { + await this.navigateViaCallbackOrRoute( + this.onSuccessfulLoginForceResetNavigate, + [this.forcePasswordResetRoute], + { + queryParams: { + identifier: orgIdentifier, + }, + } + ); + } + + private async handleSuccessfulLogin() { + if (this.onSuccessfulLogin != null) { + await this.onSuccessfulLogin(); + } + + await this.navigateViaCallbackOrRoute(this.onSuccessfulLoginNavigate, [this.successRoute]); + } + + private async handleLoginError(e: any) { + this.logService.error(e); + + // TODO: Key Connector Service should pass this error message to the logout callback instead of displaying here + if (e.message === "Key Connector error") { + this.platformUtilsService.showToast( + "error", + null, + this.i18nService.t("ssoKeyConnectorError") + ); + } + } + + private async navigateViaCallbackOrRoute( + callback: () => Promise, + commands: unknown[], + extras?: NavigationExtras + ): Promise { + if (callback) { + await callback(); + } else { + await this.router.navigate(commands, extras); } - this.loggingIn = false; } private getOrgIdentifierFromState(state: string): string { diff --git a/libs/angular/src/auth/components/two-factor.component.spec.ts b/libs/angular/src/auth/components/two-factor.component.spec.ts new file mode 100644 index 0000000000..847e5b3a40 --- /dev/null +++ b/libs/angular/src/auth/components/two-factor.component.spec.ts @@ -0,0 +1,443 @@ +import { Component } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { ActivatedRoute, Router, convertToParamMap } from "@angular/router"; +import { MockProxy, mock } from "jest-mock-extended"; + +// eslint-disable-next-line no-restricted-imports +import { WINDOW } from "@bitwarden/angular/services/injection-tokens"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { LoginService } from "@bitwarden/common/auth/abstractions/login.service"; +import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service"; +import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; +import { ForceResetPasswordReason } from "@bitwarden/common/auth/models/domain/force-reset-password-reason"; +import { KeyConnectorUserDecryptionOption } from "@bitwarden/common/auth/models/domain/user-decryption-options/key-connector-user-decryption-option"; +import { TrustedDeviceUserDecryptionOption } from "@bitwarden/common/auth/models/domain/user-decryption-options/trusted-device-user-decryption-option"; +import { TokenTwoFactorRequest } from "@bitwarden/common/auth/models/request/identity-token/token-two-factor.request"; +import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; +import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction"; +import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { AccountDecryptionOptions } from "@bitwarden/common/platform/models/domain/account"; + +import { TwoFactorComponent } from "./two-factor.component"; + +// test component that extends the TwoFactorComponent +@Component({}) +class TestTwoFactorComponent extends TwoFactorComponent {} + +interface TwoFactorComponentProtected { + trustedDeviceEncRoute: string; + changePasswordRoute: string; + forcePasswordResetRoute: string; + successRoute: string; +} + +describe("TwoFactorComponent", () => { + let component: TestTwoFactorComponent; + let _component: TwoFactorComponentProtected; + + let fixture: ComponentFixture; + + // Mock Services + let mockAuthService: MockProxy; + let mockRouter: MockProxy; + let mockI18nService: MockProxy; + let mockApiService: MockProxy; + let mockPlatformUtilsService: MockProxy; + let mockWin: MockProxy; + let mockEnvironmentService: MockProxy; + let mockStateService: MockProxy; + let mockLogService: MockProxy; + let mockTwoFactorService: MockProxy; + let mockAppIdService: MockProxy; + let mockLoginService: MockProxy; + let mockConfigService: MockProxy; + + let mockAcctDecryptionOpts: { + noMasterPassword: AccountDecryptionOptions; + withMasterPassword: AccountDecryptionOptions; + withMasterPasswordAndTrustedDevice: AccountDecryptionOptions; + withMasterPasswordAndTrustedDeviceWithManageResetPassword: AccountDecryptionOptions; + withMasterPasswordAndKeyConnector: AccountDecryptionOptions; + noMasterPasswordWithTrustedDevice: AccountDecryptionOptions; + noMasterPasswordWithTrustedDeviceWithManageResetPassword: AccountDecryptionOptions; + noMasterPasswordWithKeyConnector: AccountDecryptionOptions; + }; + + beforeEach(() => { + mockAuthService = mock(); + mockRouter = mock(); + mockI18nService = mock(); + mockApiService = mock(); + mockPlatformUtilsService = mock(); + mockWin = mock(); + mockEnvironmentService = mock(); + mockStateService = mock(); + mockLogService = mock(); + mockTwoFactorService = mock(); + mockAppIdService = mock(); + mockLoginService = mock(); + mockConfigService = mock(); + + mockAcctDecryptionOpts = { + noMasterPassword: new AccountDecryptionOptions({ + hasMasterPassword: false, + trustedDeviceOption: undefined, + keyConnectorOption: undefined, + }), + withMasterPassword: new AccountDecryptionOptions({ + hasMasterPassword: true, + trustedDeviceOption: undefined, + keyConnectorOption: undefined, + }), + withMasterPasswordAndTrustedDevice: new AccountDecryptionOptions({ + hasMasterPassword: true, + trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false), + keyConnectorOption: undefined, + }), + withMasterPasswordAndTrustedDeviceWithManageResetPassword: new AccountDecryptionOptions({ + hasMasterPassword: true, + trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true), + keyConnectorOption: undefined, + }), + withMasterPasswordAndKeyConnector: new AccountDecryptionOptions({ + hasMasterPassword: true, + trustedDeviceOption: undefined, + keyConnectorOption: new KeyConnectorUserDecryptionOption("http://example.com"), + }), + noMasterPasswordWithTrustedDevice: new AccountDecryptionOptions({ + hasMasterPassword: false, + trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, false), + keyConnectorOption: undefined, + }), + noMasterPasswordWithTrustedDeviceWithManageResetPassword: new AccountDecryptionOptions({ + hasMasterPassword: false, + trustedDeviceOption: new TrustedDeviceUserDecryptionOption(true, false, true), + keyConnectorOption: undefined, + }), + noMasterPasswordWithKeyConnector: new AccountDecryptionOptions({ + hasMasterPassword: false, + trustedDeviceOption: undefined, + keyConnectorOption: new KeyConnectorUserDecryptionOption("http://example.com"), + }), + }; + + TestBed.configureTestingModule({ + declarations: [TestTwoFactorComponent], + providers: [ + { provide: AuthService, useValue: mockAuthService }, + { provide: Router, useValue: mockRouter }, + { provide: I18nService, useValue: mockI18nService }, + { provide: ApiService, useValue: mockApiService }, + { provide: PlatformUtilsService, useValue: mockPlatformUtilsService }, + { provide: WINDOW, useValue: mockWin }, + { provide: EnvironmentService, useValue: mockEnvironmentService }, + { provide: StateService, useValue: mockStateService }, + { + provide: ActivatedRoute, + useValue: { + snapshot: { + // Default to standard 2FA flow - not SSO + 2FA + queryParamMap: convertToParamMap({ sso: "false" }), + }, + }, + }, + { provide: LogService, useValue: mockLogService }, + { provide: TwoFactorService, useValue: mockTwoFactorService }, + { provide: AppIdService, useValue: mockAppIdService }, + { provide: LoginService, useValue: mockLoginService }, + { provide: ConfigServiceAbstraction, useValue: mockConfigService }, + ], + }); + + fixture = TestBed.createComponent(TestTwoFactorComponent); + component = fixture.componentInstance; + _component = component as any; + }); + + afterEach(() => { + // Reset all mocks after each test + jest.resetAllMocks(); + }); + + it("should create", () => { + expect(component).toBeTruthy(); + }); + + // Shared tests + const testChangePasswordOnSuccessfulLogin = () => { + it("navigates to the component's defined change password route when user doesn't have a MP and key connector isn't enabled", async () => { + // Act + await component.doSubmit(); + + // Assert + expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith([_component.changePasswordRoute], { + queryParams: { + identifier: component.orgIdentifier, + }, + }); + }); + }; + + const testForceResetOnSuccessfulLogin = (reasonString: string) => { + it(`navigates to the component's defined forcePasswordResetRoute route when response.forcePasswordReset is ${reasonString}`, async () => { + // Act + await component.doSubmit(); + + // expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith([_component.forcePasswordResetRoute], { + queryParams: { + identifier: component.orgIdentifier, + }, + }); + }); + }; + + describe("Standard 2FA scenarios", () => { + describe("doSubmit", () => { + const token = "testToken"; + const remember = false; + const captchaToken = "testCaptchaToken"; + + beforeEach(() => { + component.token = token; + component.remember = remember; + component.captchaToken = captchaToken; + + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.withMasterPassword + ); + }); + + it("calls authService.logInTwoFactor with correct parameters when form is submitted", async () => { + // Arrange + mockAuthService.logInTwoFactor.mockResolvedValue(new AuthResult()); + + // Act + await component.doSubmit(); + + // Assert + expect(mockAuthService.logInTwoFactor).toHaveBeenCalledWith( + new TokenTwoFactorRequest(component.selectedProviderType, token, remember), + captchaToken + ); + }); + + it("should return when handleCaptchaRequired returns true", async () => { + // Arrange + const captchaSiteKey = "testCaptchaSiteKey"; + const authResult = new AuthResult(); + authResult.captchaSiteKey = captchaSiteKey; + + mockAuthService.logInTwoFactor.mockResolvedValue(authResult); + + // Note: the any casts are required b/c typescript cant recognize that + // handleCaptureRequired is a method on TwoFactorComponent b/c it is inherited + // from the CaptchaProtectedComponent + const handleCaptchaRequiredSpy = jest + .spyOn(component, "handleCaptchaRequired") + .mockReturnValue(true); + + // Act + const result = await component.doSubmit(); + + // Assert + expect(handleCaptchaRequiredSpy).toHaveBeenCalled(); + expect(result).toBeUndefined(); + }); + + it("calls onSuccessfulLogin when defined", async () => { + // Arrange + component.onSuccessfulLogin = jest.fn().mockResolvedValue(undefined); + mockAuthService.logInTwoFactor.mockResolvedValue(new AuthResult()); + + // Act + await component.doSubmit(); + + // Assert + expect(component.onSuccessfulLogin).toHaveBeenCalled(); + }); + + it("calls loginService.clearValues() when login is successful", async () => { + // Arrange + mockAuthService.logInTwoFactor.mockResolvedValue(new AuthResult()); + // spy on loginService.clearValues + const clearValuesSpy = jest.spyOn(mockLoginService, "clearValues"); + + // Act + await component.doSubmit(); + + // Assert + expect(clearValuesSpy).toHaveBeenCalled(); + }); + + describe("Set Master Password scenarios", () => { + beforeEach(() => { + const authResult = new AuthResult(); + mockAuthService.logInTwoFactor.mockResolvedValue(authResult); + }); + + describe("Given user needs to set a master password", () => { + beforeEach(() => { + // Only need to test the case where the user has no master password to test the primary change mp flow here + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.noMasterPassword + ); + }); + + testChangePasswordOnSuccessfulLogin(); + }); + + it("does not navigate to the change password route when the user has key connector even if user has no master password", async () => { + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.noMasterPasswordWithKeyConnector + ); + + await component.doSubmit(); + + expect(mockRouter.navigate).not.toHaveBeenCalledWith([_component.changePasswordRoute], { + queryParams: { + identifier: component.orgIdentifier, + }, + }); + }); + }); + + describe("Force Master Password Reset scenarios", () => { + [ + ForceResetPasswordReason.AdminForcePasswordReset, + ForceResetPasswordReason.WeakMasterPassword, + ].forEach((forceResetPasswordReason) => { + const reasonString = ForceResetPasswordReason[forceResetPasswordReason]; + + beforeEach(() => { + // use standard user with MP because this test is not concerned with password reset. + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.withMasterPassword + ); + + const authResult = new AuthResult(); + authResult.forcePasswordReset = forceResetPasswordReason; + mockAuthService.logInTwoFactor.mockResolvedValue(authResult); + }); + + testForceResetOnSuccessfulLogin(reasonString); + }); + }); + + it("calls onSuccessfulLoginNavigate when the callback is defined", async () => { + // Arrange + component.onSuccessfulLoginNavigate = jest.fn().mockResolvedValue(undefined); + mockAuthService.logInTwoFactor.mockResolvedValue(new AuthResult()); + + // Act + await component.doSubmit(); + + // Assert + expect(component.onSuccessfulLoginNavigate).toHaveBeenCalled(); + }); + + it("navigates to the component's defined success route when the login is successful and onSuccessfulLoginNavigate is undefined", async () => { + mockAuthService.logInTwoFactor.mockResolvedValue(new AuthResult()); + + // Act + await component.doSubmit(); + + // Assert + expect(component.onSuccessfulLoginNavigate).not.toBeDefined(); + + expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith([_component.successRoute], undefined); + }); + }); + }); + + describe("SSO > 2FA scenarios", () => { + beforeEach(() => { + const mockActivatedRoute = TestBed.inject(ActivatedRoute); + mockActivatedRoute.snapshot.queryParamMap.get = jest.fn().mockReturnValue("true"); + }); + + describe("doSubmit", () => { + const token = "testToken"; + const remember = false; + const captchaToken = "testCaptchaToken"; + + beforeEach(() => { + component.token = token; + component.remember = remember; + component.captchaToken = captchaToken; + }); + + describe("Trusted Device Encryption scenarios", () => { + beforeEach(() => { + mockConfigService.getFeatureFlagBool.mockResolvedValue(true); + }); + + describe("Given Trusted Device Encryption is enabled and user needs to set a master password", () => { + beforeEach(() => { + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.noMasterPasswordWithTrustedDeviceWithManageResetPassword + ); + + const authResult = new AuthResult(); + mockAuthService.logInTwoFactor.mockResolvedValue(authResult); + }); + + testChangePasswordOnSuccessfulLogin(); + }); + + describe("Given Trusted Device Encryption is enabled, user doesn't need to set a MP, and forcePasswordReset is required", () => { + [ + ForceResetPasswordReason.AdminForcePasswordReset, + ForceResetPasswordReason.WeakMasterPassword, + ].forEach((forceResetPasswordReason) => { + const reasonString = ForceResetPasswordReason[forceResetPasswordReason]; + + beforeEach(() => { + // use standard user with MP because this test is not concerned with password reset. + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.withMasterPasswordAndTrustedDevice + ); + + const authResult = new AuthResult(); + authResult.forcePasswordReset = forceResetPasswordReason; + mockAuthService.logInTwoFactor.mockResolvedValue(authResult); + }); + + testForceResetOnSuccessfulLogin(reasonString); + }); + }); + + describe("Given Trusted Device Encryption is enabled, user doesn't need to set a MP, and forcePasswordReset is not required", () => { + let authResult; + beforeEach(() => { + mockStateService.getAccountDecryptionOptions.mockResolvedValue( + mockAcctDecryptionOpts.withMasterPasswordAndTrustedDevice + ); + + authResult = new AuthResult(); + authResult.forcePasswordReset = ForceResetPasswordReason.None; + mockAuthService.logInTwoFactor.mockResolvedValue(authResult); + }); + + it("navigates to the component's defined trusted device encryption route when login is successful", async () => { + await component.doSubmit(); + + expect(mockRouter.navigate).toHaveBeenCalledTimes(1); + expect(mockRouter.navigate).toHaveBeenCalledWith([_component.trustedDeviceEncRoute], { + queryParams: { + identifier: component.orgIdentifier, + }, + }); + }); + }); + }); + }); + }); +}); diff --git a/libs/angular/src/auth/components/two-factor.component.ts b/libs/angular/src/auth/components/two-factor.component.ts index 4ae0601961..35fc7240f2 100644 --- a/libs/angular/src/auth/components/two-factor.component.ts +++ b/libs/angular/src/auth/components/two-factor.component.ts @@ -1,8 +1,10 @@ -import { Directive, OnDestroy, OnInit } from "@angular/core"; -import { ActivatedRoute, Router } from "@angular/router"; +import { Directive, Inject, OnDestroy, OnInit } from "@angular/core"; +import { ActivatedRoute, NavigationExtras, Router } from "@angular/router"; import * as DuoWebSDK from "duo_web_sdk"; import { first } from "rxjs/operators"; +// eslint-disable-next-line no-restricted-imports +import { WINDOW } from "@bitwarden/angular/services/injection-tokens"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { LoginService } from "@bitwarden/common/auth/abstractions/login.service"; @@ -10,6 +12,7 @@ import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type"; import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; import { ForceResetPasswordReason } from "@bitwarden/common/auth/models/domain/force-reset-password-reason"; +import { TrustedDeviceUserDecryptionOption } from "@bitwarden/common/auth/models/domain/user-decryption-options/trusted-device-user-decryption-option"; import { TokenTwoFactorRequest } from "@bitwarden/common/auth/models/request/identity-token/token-two-factor.request"; import { TwoFactorEmailRequest } from "@bitwarden/common/auth/models/request/two-factor-email.request"; import { TwoFactorProviders } from "@bitwarden/common/auth/services/two-factor.service"; @@ -41,13 +44,16 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI twoFactorEmail: string = null; formPromise: Promise; emailPromise: Promise; - identifier: string = null; + orgIdentifier: string = null; onSuccessfulLogin: () => Promise; onSuccessfulLoginNavigate: () => Promise; protected loginRoute = "login"; - protected successRoute = "vault"; + protected trustedDeviceEncRoute = "login-initiated"; + protected changePasswordRoute = "set-password"; + protected forcePasswordResetRoute = "update-temp-password"; + protected successRoute = "vault"; constructor( protected authService: AuthService, @@ -55,7 +61,7 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI protected i18nService: I18nService, protected apiService: ApiService, protected platformUtilsService: PlatformUtilsService, - protected win: Window, + @Inject(WINDOW) protected win: Window, protected environmentService: EnvironmentService, protected stateService: StateService, protected route: ActivatedRoute, @@ -77,7 +83,7 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI this.route.queryParams.pipe(first()).subscribe((qParams) => { if (qParams.identifier != null) { - this.identifier = qParams.identifier; + this.orgIdentifier = qParams.identifier; } }); @@ -201,55 +207,137 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI new TokenTwoFactorRequest(this.selectedProviderType, this.token, this.remember), this.captchaToken ); - const response: AuthResult = await this.formPromise; - if (this.handleCaptchaRequired(response)) { + const authResult: AuthResult = await this.formPromise; + + await this.handleLoginResponse(authResult); + } + + private async handleLoginResponse(authResult: AuthResult) { + if (this.handleCaptchaRequired(authResult)) { return; } + + // Note: calling this here is different from the SSO component. + // The SsoComponent only executes this logic as part of the handleSuccessfulLogin logic. + // We should investigate later on if we can move this down into the handleSuccessfulLogin + // without negative side effects. + await this.handleOnSuccessfulLogin(); + + this.loginService.clearValues(); + + const acctDecryptionOpts: AccountDecryptionOptions = + await this.stateService.getAccountDecryptionOptions(); + + const tdeEnabled = await this.isTrustedDeviceEncEnabled(acctDecryptionOpts.trustedDeviceOption); + + if (tdeEnabled) { + return await this.handleTrustedDeviceEncryptionEnabled( + authResult, + this.orgIdentifier, + acctDecryptionOpts + ); + } + + // User must set password if they don't have one and they aren't using either TDE or key connector. + const requireSetPassword = + !acctDecryptionOpts.hasMasterPassword && acctDecryptionOpts.keyConnectorOption === undefined; + + if (requireSetPassword) { + // Change implies going no password -> password in this case + return await this.handleChangePasswordRequired(this.orgIdentifier); + } + + // Users can be forced to reset their password via an admin or org policy + // disallowing weak passwords + if (authResult.forcePasswordReset !== ForceResetPasswordReason.None) { + return await this.handleForcePasswordReset(this.orgIdentifier); + } + + return await this.handleSuccessfulLogin(); + } + + private async handleOnSuccessfulLogin() { if (this.onSuccessfulLogin != null) { this.loginService.clearValues(); // Note: awaiting this will currently cause a hang on desktop & browser as they will wait for a full sync to complete // before nagivating to the success route. this.onSuccessfulLogin(); } - if (response.resetMasterPassword) { - // TODO: for TDE, we are going to deprecate using response.resetMasterPassword - // and instead rely on accountDecryptionOptions to determine if the user needs to set a password - // Users are allowed to not have a MP if TDE feature enabled + TDE configured. Otherwise, they must set a MP - // src: https://bitwarden.atlassian.net/browse/PM-2759?focusedCommentId=39438 - this.successRoute = "set-password"; + } + + private async isTrustedDeviceEncEnabled( + trustedDeviceOption: TrustedDeviceUserDecryptionOption + ): Promise { + const ssoTo2faFlowActive = this.route.snapshot.queryParamMap.get("sso") === "true"; + const trustedDeviceEncryptionFeatureActive = await this.configService.getFeatureFlagBool( + FeatureFlag.TrustedDeviceEncryption + ); + + return ( + ssoTo2faFlowActive && + trustedDeviceEncryptionFeatureActive && + trustedDeviceOption !== undefined + ); + } + + private async handleTrustedDeviceEncryptionEnabled( + authResult: AuthResult, + orgIdentifier: string, + acctDecryptionOpts: AccountDecryptionOptions + ): Promise { + // If user doesn't have a MP, but has reset password permission, they must set a MP + if ( + !acctDecryptionOpts.hasMasterPassword && + acctDecryptionOpts.trustedDeviceOption.hasManageResetPasswordPermission + ) { + // Change implies going no password -> password in this case + return await this.handleChangePasswordRequired(orgIdentifier); } - if (response.forcePasswordReset !== ForceResetPasswordReason.None) { - this.successRoute = "update-temp-password"; + + // Users can be forced to reset their password via an admin or org policy + // disallowing weak passwords + if (authResult.forcePasswordReset !== ForceResetPasswordReason.None) { + return await this.handleForcePasswordReset(orgIdentifier); } - if (this.onSuccessfulLoginNavigate != null) { - this.loginService.clearValues(); - // TODO: this function is defined when coming SSO with 2FA for authenticator app - // see two goAfterLogIn functions (one in web login.component.ts and one in web two factor component.ts ) - await this.onSuccessfulLoginNavigate(); + + // Navigate to TDE page (if user was on trusted device and TDE has decrypted + // their user key, the lock guard will redirect them to the vault) + this.router.navigate([this.trustedDeviceEncRoute], { + queryParams: { + identifier: orgIdentifier, + }, + }); + } + + private async handleChangePasswordRequired(orgIdentifier: string) { + await this.router.navigate([this.changePasswordRoute], { + queryParams: { + identifier: orgIdentifier, + }, + }); + } + + private async handleForcePasswordReset(orgIdentifier: string) { + this.router.navigate([this.forcePasswordResetRoute], { + queryParams: { + identifier: orgIdentifier, + }, + }); + } + + private async handleSuccessfulLogin() { + await this.navigateViaCallbackOrRoute(this.onSuccessfulLoginNavigate, [this.successRoute]); + } + + private async navigateViaCallbackOrRoute( + callback: () => Promise, + commands: unknown[], + extras?: NavigationExtras + ): Promise { + if (callback) { + await callback(); } else { - this.loginService.clearValues(); - - const ssoTo2faFlowActive = this.route.snapshot.queryParamMap.get("sso") === "true"; - const trustedDeviceEncryptionFeatureActive = await this.configService.getFeatureFlagBool( - FeatureFlag.TrustedDeviceEncryption - ); - - const accountDecryptionOptions: AccountDecryptionOptions = - await this.stateService.getAccountDecryptionOptions(); - - if ( - ssoTo2faFlowActive && - trustedDeviceEncryptionFeatureActive && - accountDecryptionOptions.trustedDeviceOption !== undefined - ) { - this.router.navigate([this.trustedDeviceEncRoute]); - } else { - this.router.navigate([this.successRoute], { - queryParams: { - identifier: this.identifier, - }, - }); - } + await this.router.navigate(commands, extras); } } diff --git a/libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts b/libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts index 29797e0e72..77c108d34f 100644 --- a/libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts +++ b/libs/common/src/auth/login-strategies/sso-login.strategy.spec.ts @@ -149,6 +149,7 @@ describe("SsoLogInStrategy", () => { TrustedDeviceOption: { HasAdminApproval: true, HasLoginApprovingDevice: true, + HasManageResetPasswordPermission: false, EncryptedPrivateKey: mockEncDevicePrivateKey, EncryptedUserKey: mockEncUserKey, }, diff --git a/libs/common/src/auth/models/domain/auth-result.ts b/libs/common/src/auth/models/domain/auth-result.ts index 7a88d490f0..460ef2b8a0 100644 --- a/libs/common/src/auth/models/domain/auth-result.ts +++ b/libs/common/src/auth/models/domain/auth-result.ts @@ -5,7 +5,13 @@ import { ForceResetPasswordReason } from "./force-reset-password-reason"; export class AuthResult { captchaSiteKey = ""; + /** + * @deprecated + * Replace with using AccountDecryptionOptions to determine if the user does + * not have a master password and is not using Key Connector. + * */ resetMasterPassword = false; + forcePasswordReset: ForceResetPasswordReason = ForceResetPasswordReason.None; twoFactorProviders: Map = null; ssoEmail2FaSessionToken?: string; diff --git a/libs/common/src/auth/models/domain/user-decryption-options/trusted-device-user-decryption-option.ts b/libs/common/src/auth/models/domain/user-decryption-options/trusted-device-user-decryption-option.ts index 9d8fc3ed5f..7d7211061f 100644 --- a/libs/common/src/auth/models/domain/user-decryption-options/trusted-device-user-decryption-option.ts +++ b/libs/common/src/auth/models/domain/user-decryption-options/trusted-device-user-decryption-option.ts @@ -1,3 +1,7 @@ export class TrustedDeviceUserDecryptionOption { - constructor(public hasAdminApproval: boolean, public hasLoginApprovingDevice: boolean) {} + constructor( + public hasAdminApproval: boolean, + public hasLoginApprovingDevice: boolean, + public hasManageResetPasswordPermission: boolean + ) {} } diff --git a/libs/common/src/auth/models/response/user-decryption-options/trusted-device-user-decryption-option.response.ts b/libs/common/src/auth/models/response/user-decryption-options/trusted-device-user-decryption-option.response.ts index af29e3b3a4..903022b074 100644 --- a/libs/common/src/auth/models/response/user-decryption-options/trusted-device-user-decryption-option.response.ts +++ b/libs/common/src/auth/models/response/user-decryption-options/trusted-device-user-decryption-option.response.ts @@ -4,6 +4,7 @@ import { EncString } from "../../../../platform/models/domain/enc-string"; export interface ITrustedDeviceUserDecryptionOptionServerResponse { HasAdminApproval: boolean; HasLoginApprovingDevice: boolean; + HasManageResetPasswordPermission: boolean; EncryptedPrivateKey?: string; EncryptedUserKey?: string; } @@ -11,6 +12,7 @@ export interface ITrustedDeviceUserDecryptionOptionServerResponse { export class TrustedDeviceUserDecryptionOptionResponse extends BaseResponse { hasAdminApproval: boolean; hasLoginApprovingDevice: boolean; + hasManageResetPasswordPermission: boolean; encryptedPrivateKey: EncString; encryptedUserKey: EncString; @@ -19,6 +21,9 @@ export class TrustedDeviceUserDecryptionOptionResponse extends BaseResponse { this.hasAdminApproval = this.getResponseProperty("HasAdminApproval"); this.hasLoginApprovingDevice = this.getResponseProperty("HasLoginApprovingDevice"); + this.hasManageResetPasswordPermission = this.getResponseProperty( + "HasManageResetPasswordPermission" + ); if (response.EncryptedPrivateKey) { this.encryptedPrivateKey = new EncString(this.getResponseProperty("EncryptedPrivateKey")); diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 6bdf47035b..b3da761f8a 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -323,7 +323,8 @@ export class AccountDecryptionOptions { if (response.trustedDeviceOption) { accountDecryptionOptions.trustedDeviceOption = new TrustedDeviceUserDecryptionOption( response.trustedDeviceOption.hasAdminApproval, - response.trustedDeviceOption.hasLoginApprovingDevice + response.trustedDeviceOption.hasLoginApprovingDevice, + response.trustedDeviceOption.hasManageResetPasswordPermission ); } @@ -346,7 +347,8 @@ export class AccountDecryptionOptions { if (obj.trustedDeviceOption) { accountDecryptionOptions.trustedDeviceOption = new TrustedDeviceUserDecryptionOption( obj.trustedDeviceOption.hasAdminApproval, - obj.trustedDeviceOption.hasLoginApprovingDevice + obj.trustedDeviceOption.hasLoginApprovingDevice, + obj.trustedDeviceOption.hasManageResetPasswordPermission ); }