From 2f47add6f157db355603c072fbde9cd4881b4ad0 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Tue, 22 Jul 2025 19:08:09 -0500 Subject: [PATCH] [PM-23596] Redirect to `/setup-extension` (#15641) * remove current redirection from auth code * update timeouts of the web browser interaction * add guard for setup-extension page * decrease timeout to 25ms * avoid redirection for mobile users + add tests * add tests * condense variables * catch error from profile fetch --------- Co-authored-by: Shane Melton --- .../web-registration-finish.service.spec.ts | 22 --- .../web-registration-finish.service.ts | 15 -- apps/web/src/app/core/core.module.ts | 1 - apps/web/src/app/oss-routing.module.ts | 2 + .../add-extension-later-dialog.component.html | 8 +- ...d-extension-later-dialog.component.spec.ts | 14 ++ .../add-extension-later-dialog.component.ts | 17 ++- .../setup-extension.component.spec.ts | 16 +++ .../setup-extension.component.ts | 30 +++- .../setup-extension-redirect.guard.spec.ts | 132 ++++++++++++++++++ .../guards/setup-extension-redirect.guard.ts | 109 +++++++++++++++ .../web-browser-interaction.service.spec.ts | 6 +- .../web-browser-interaction.service.ts | 17 ++- .../default-registration-finish.service.ts | 4 - .../registration-finish.component.ts | 3 +- .../registration-finish.service.ts | 5 - .../src/platform/state/state-definitions.ts | 7 + 17 files changed, 347 insertions(+), 61 deletions(-) create mode 100644 apps/web/src/app/vault/guards/setup-extension-redirect.guard.spec.ts create mode 100644 apps/web/src/app/vault/guards/setup-extension-redirect.guard.ts diff --git a/apps/web/src/app/auth/core/services/registration/web-registration-finish.service.spec.ts b/apps/web/src/app/auth/core/services/registration/web-registration-finish.service.spec.ts index 69a2f27a322..845df89622b 100644 --- a/apps/web/src/app/auth/core/services/registration/web-registration-finish.service.spec.ts +++ b/apps/web/src/app/auth/core/services/registration/web-registration-finish.service.spec.ts @@ -12,7 +12,6 @@ import { AccountApiService } from "@bitwarden/common/auth/abstractions/account-a import { OrganizationInvite } from "@bitwarden/common/auth/services/organization-invite/organization-invite"; import { OrganizationInviteService } from "@bitwarden/common/auth/services/organization-invite/organization-invite.service"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { CsprngArray } from "@bitwarden/common/types/csprng"; @@ -30,7 +29,6 @@ describe("WebRegistrationFinishService", () => { let policyApiService: MockProxy; let logService: MockProxy; let policyService: MockProxy; - let configService: MockProxy; beforeEach(() => { keyService = mock(); @@ -39,7 +37,6 @@ describe("WebRegistrationFinishService", () => { policyApiService = mock(); logService = mock(); policyService = mock(); - configService = mock(); service = new WebRegistrationFinishService( keyService, @@ -48,7 +45,6 @@ describe("WebRegistrationFinishService", () => { policyApiService, logService, policyService, - configService, ); }); @@ -414,22 +410,4 @@ describe("WebRegistrationFinishService", () => { ); }); }); - - describe("determineLoginSuccessRoute", () => { - it("returns /setup-extension when the end user activation feature flag is enabled", async () => { - configService.getFeatureFlag.mockResolvedValue(true); - - const result = await service.determineLoginSuccessRoute(); - - expect(result).toBe("/setup-extension"); - }); - - it("returns /vault when the end user activation feature flag is disabled", async () => { - configService.getFeatureFlag.mockResolvedValue(false); - - const result = await service.determineLoginSuccessRoute(); - - expect(result).toBe("/vault"); - }); - }); }); diff --git a/apps/web/src/app/auth/core/services/registration/web-registration-finish.service.ts b/apps/web/src/app/auth/core/services/registration/web-registration-finish.service.ts index a3774e87db8..a9eba08be8c 100644 --- a/apps/web/src/app/auth/core/services/registration/web-registration-finish.service.ts +++ b/apps/web/src/app/auth/core/services/registration/web-registration-finish.service.ts @@ -14,12 +14,10 @@ import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; import { AccountApiService } from "@bitwarden/common/auth/abstractions/account-api.service"; import { RegisterFinishRequest } from "@bitwarden/common/auth/models/request/registration/register-finish.request"; import { OrganizationInviteService } from "@bitwarden/common/auth/services/organization-invite/organization-invite.service"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { EncryptedString, EncString, } from "@bitwarden/common/key-management/crypto/models/enc-string"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { KeyService } from "@bitwarden/key-management"; @@ -34,7 +32,6 @@ export class WebRegistrationFinishService private policyApiService: PolicyApiServiceAbstraction, private logService: LogService, private policyService: PolicyService, - private configService: ConfigService, ) { super(keyService, accountApiService); } @@ -79,18 +76,6 @@ export class WebRegistrationFinishService return masterPasswordPolicyOpts; } - override async determineLoginSuccessRoute(): Promise { - const endUserActivationFlagEnabled = await this.configService.getFeatureFlag( - FeatureFlag.PM19315EndUserActivationMvp, - ); - - if (endUserActivationFlagEnabled) { - return "/setup-extension"; - } else { - return super.determineLoginSuccessRoute(); - } - } - // Note: the org invite token and email verification are mutually exclusive. Only one will be present. override async buildRegisterRequest( email: string, diff --git a/apps/web/src/app/core/core.module.ts b/apps/web/src/app/core/core.module.ts index d98a2ee8cf2..7fe8ef4c79f 100644 --- a/apps/web/src/app/core/core.module.ts +++ b/apps/web/src/app/core/core.module.ts @@ -264,7 +264,6 @@ const safeProviders: SafeProvider[] = [ PolicyApiServiceAbstraction, LogService, PolicyService, - ConfigService, ], }), safeProvider({ diff --git a/apps/web/src/app/oss-routing.module.ts b/apps/web/src/app/oss-routing.module.ts index 8a2270113a9..1fb19757d60 100644 --- a/apps/web/src/app/oss-routing.module.ts +++ b/apps/web/src/app/oss-routing.module.ts @@ -83,6 +83,7 @@ import { SendComponent } from "./tools/send/send.component"; import { BrowserExtensionPromptInstallComponent } from "./vault/components/browser-extension-prompt/browser-extension-prompt-install.component"; import { BrowserExtensionPromptComponent } from "./vault/components/browser-extension-prompt/browser-extension-prompt.component"; import { SetupExtensionComponent } from "./vault/components/setup-extension/setup-extension.component"; +import { setupExtensionRedirectGuard } from "./vault/guards/setup-extension-redirect.guard"; import { VaultModule } from "./vault/individual-vault/vault.module"; const routes: Routes = [ @@ -628,6 +629,7 @@ const routes: Routes = [ children: [ { path: "vault", + canActivate: [setupExtensionRedirectGuard], loadChildren: () => VaultModule, }, { diff --git a/apps/web/src/app/vault/components/setup-extension/add-extension-later-dialog.component.html b/apps/web/src/app/vault/components/setup-extension/add-extension-later-dialog.component.html index df1786e227e..560bd5fd464 100644 --- a/apps/web/src/app/vault/components/setup-extension/add-extension-later-dialog.component.html +++ b/apps/web/src/app/vault/components/setup-extension/add-extension-later-dialog.component.html @@ -18,7 +18,13 @@ > {{ "getTheExtension" | i18n }} - + {{ "skipToWebApp" | i18n }} diff --git a/apps/web/src/app/vault/components/setup-extension/add-extension-later-dialog.component.spec.ts b/apps/web/src/app/vault/components/setup-extension/add-extension-later-dialog.component.spec.ts index d34dba737dd..a5d5ec4b939 100644 --- a/apps/web/src/app/vault/components/setup-extension/add-extension-later-dialog.component.spec.ts +++ b/apps/web/src/app/vault/components/setup-extension/add-extension-later-dialog.component.spec.ts @@ -1,3 +1,4 @@ +import { DialogRef } from "@angular/cdk/dialog"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; import { provideNoopAnimations } from "@angular/platform-browser/animations"; @@ -5,20 +6,26 @@ import { RouterModule } from "@angular/router"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { DIALOG_DATA } from "@bitwarden/components"; import { AddExtensionLaterDialogComponent } from "./add-extension-later-dialog.component"; describe("AddExtensionLaterDialogComponent", () => { let fixture: ComponentFixture; const getDevice = jest.fn().mockReturnValue(null); + const onDismiss = jest.fn(); beforeEach(async () => { + onDismiss.mockClear(); + await TestBed.configureTestingModule({ imports: [AddExtensionLaterDialogComponent, RouterModule.forRoot([])], providers: [ provideNoopAnimations(), { provide: PlatformUtilsService, useValue: { getDevice } }, { provide: I18nService, useValue: { t: (key: string) => key } }, + { provide: DialogRef, useValue: { close: jest.fn() } }, + { provide: DIALOG_DATA, useValue: { onDismiss } }, ], }).compileComponents(); @@ -39,4 +46,11 @@ describe("AddExtensionLaterDialogComponent", () => { expect(skipLink.attributes.href).toBe("/vault"); }); + + it('invokes `onDismiss` when "Skip to Web App" is clicked', () => { + const skipLink = fixture.debugElement.queryAll(By.css("a[bitButton]"))[1]; + skipLink.triggerEventHandler("click", {}); + + expect(onDismiss).toHaveBeenCalled(); + }); }); diff --git a/apps/web/src/app/vault/components/setup-extension/add-extension-later-dialog.component.ts b/apps/web/src/app/vault/components/setup-extension/add-extension-later-dialog.component.ts index 3324cb8b1b0..5f4e3f586f5 100644 --- a/apps/web/src/app/vault/components/setup-extension/add-extension-later-dialog.component.ts +++ b/apps/web/src/app/vault/components/setup-extension/add-extension-later-dialog.component.ts @@ -4,7 +4,17 @@ import { RouterModule } from "@angular/router"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { getWebStoreUrl } from "@bitwarden/common/vault/utils/get-web-store-url"; -import { ButtonComponent, DialogModule, TypographyModule } from "@bitwarden/components"; +import { + ButtonComponent, + DIALOG_DATA, + DialogModule, + TypographyModule, +} from "@bitwarden/components"; + +export type AddExtensionLaterDialogData = { + /** Method invoked when the dialog is dismissed */ + onDismiss: () => void; +}; @Component({ selector: "vault-add-extension-later-dialog", @@ -13,6 +23,7 @@ import { ButtonComponent, DialogModule, TypographyModule } from "@bitwarden/comp }) export class AddExtensionLaterDialogComponent implements OnInit { private platformUtilsService = inject(PlatformUtilsService); + private data: AddExtensionLaterDialogData = inject(DIALOG_DATA); /** Download Url for the extension based on the browser */ protected webStoreUrl: string = ""; @@ -20,4 +31,8 @@ export class AddExtensionLaterDialogComponent implements OnInit { ngOnInit(): void { this.webStoreUrl = getWebStoreUrl(this.platformUtilsService.getDevice()); } + + async dismissExtensionPage() { + this.data.onDismiss(); + } } diff --git a/apps/web/src/app/vault/components/setup-extension/setup-extension.component.spec.ts b/apps/web/src/app/vault/components/setup-extension/setup-extension.component.spec.ts index 752e2c8d4a6..e824cd92f37 100644 --- a/apps/web/src/app/vault/components/setup-extension/setup-extension.component.spec.ts +++ b/apps/web/src/app/vault/components/setup-extension/setup-extension.component.spec.ts @@ -3,12 +3,14 @@ import { By } from "@angular/platform-browser"; import { Router, RouterModule } from "@angular/router"; import { BehaviorSubject } from "rxjs"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { DeviceType } from "@bitwarden/common/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 { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { StateProvider } from "@bitwarden/common/platform/state"; import { WebBrowserInteractionService } from "../../services/web-browser-interaction.service"; @@ -21,11 +23,13 @@ describe("SetupExtensionComponent", () => { const getFeatureFlag = jest.fn().mockResolvedValue(false); const navigate = jest.fn().mockResolvedValue(true); const openExtension = jest.fn().mockResolvedValue(true); + const update = jest.fn().mockResolvedValue(true); const extensionInstalled$ = new BehaviorSubject(null); beforeEach(async () => { navigate.mockClear(); openExtension.mockClear(); + update.mockClear(); getFeatureFlag.mockClear().mockResolvedValue(true); window.matchMedia = jest.fn().mockReturnValue(false); @@ -36,6 +40,14 @@ describe("SetupExtensionComponent", () => { { provide: ConfigService, useValue: { getFeatureFlag } }, { provide: WebBrowserInteractionService, useValue: { extensionInstalled$, openExtension } }, { provide: PlatformUtilsService, useValue: { getDevice: () => DeviceType.UnknownBrowser } }, + { + provide: AccountService, + useValue: { activeAccount$: new BehaviorSubject({ account: { id: "account-id" } }) }, + }, + { + provide: StateProvider, + useValue: { getUser: () => ({ update }) }, + }, ], }).compileComponents(); @@ -120,6 +132,10 @@ describe("SetupExtensionComponent", () => { expect(openExtension).toHaveBeenCalled(); }); + + it("dismisses the extension page", () => { + expect(update).toHaveBeenCalledTimes(1); + }); }); }); }); diff --git a/apps/web/src/app/vault/components/setup-extension/setup-extension.component.ts b/apps/web/src/app/vault/components/setup-extension/setup-extension.component.ts index 9ee8e189627..14770ca5d6c 100644 --- a/apps/web/src/app/vault/components/setup-extension/setup-extension.component.ts +++ b/apps/web/src/app/vault/components/setup-extension/setup-extension.component.ts @@ -2,13 +2,16 @@ import { DOCUMENT, NgIf } from "@angular/common"; import { Component, DestroyRef, inject, OnDestroy, OnInit } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { Router, RouterModule } from "@angular/router"; -import { pairwise, startWith } from "rxjs"; +import { firstValueFrom, pairwise, startWith } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { StateProvider } from "@bitwarden/common/platform/state"; import { UnionOfValues } from "@bitwarden/common/vault/types/union-of-values"; import { getWebStoreUrl } from "@bitwarden/common/vault/utils/get-web-store-url"; import { @@ -20,9 +23,13 @@ import { } from "@bitwarden/components"; import { VaultIcons } from "@bitwarden/vault"; +import { SETUP_EXTENSION_DISMISSED } from "../../guards/setup-extension-redirect.guard"; import { WebBrowserInteractionService } from "../../services/web-browser-interaction.service"; -import { AddExtensionLaterDialogComponent } from "./add-extension-later-dialog.component"; +import { + AddExtensionLaterDialogComponent, + AddExtensionLaterDialogData, +} from "./add-extension-later-dialog.component"; import { AddExtensionVideosComponent } from "./add-extension-videos.component"; const SetupExtensionState = { @@ -53,6 +60,8 @@ export class SetupExtensionComponent implements OnInit, OnDestroy { private destroyRef = inject(DestroyRef); private platformUtilsService = inject(PlatformUtilsService); private dialogService = inject(DialogService); + private stateProvider = inject(StateProvider); + private accountService = inject(AccountService); private document = inject(DOCUMENT); protected SetupExtensionState = SetupExtensionState; @@ -96,6 +105,7 @@ export class SetupExtensionComponent implements OnInit, OnDestroy { // Extension was not installed and now it is, show success state if (previousState === false && currentState) { this.dialogRef?.close(); + void this.dismissExtensionPage(); this.state = SetupExtensionState.Success; } @@ -125,17 +135,31 @@ export class SetupExtensionComponent implements OnInit, OnDestroy { const isMobile = Utils.isMobileBrowser; if (!isFeatureEnabled || isMobile) { + await this.dismissExtensionPage(); await this.router.navigate(["/vault"]); } } /** Opens the add extension later dialog */ addItLater() { - this.dialogRef = this.dialogService.open(AddExtensionLaterDialogComponent); + this.dialogRef = this.dialogService.open( + AddExtensionLaterDialogComponent, + { + data: { + onDismiss: this.dismissExtensionPage.bind(this), + }, + }, + ); } /** Opens the browser extension */ openExtension() { void this.webBrowserExtensionInteractionService.openExtension(); } + + /** Update local state to never show this page again. */ + private async dismissExtensionPage() { + const accountId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + void this.stateProvider.getUser(accountId, SETUP_EXTENSION_DISMISSED).update(() => true); + } } diff --git a/apps/web/src/app/vault/guards/setup-extension-redirect.guard.spec.ts b/apps/web/src/app/vault/guards/setup-extension-redirect.guard.spec.ts new file mode 100644 index 00000000000..e6fc03fd844 --- /dev/null +++ b/apps/web/src/app/vault/guards/setup-extension-redirect.guard.spec.ts @@ -0,0 +1,132 @@ +import { TestBed } from "@angular/core/testing"; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from "@angular/router"; +import { BehaviorSubject } from "rxjs"; + +import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service"; +import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { StateProvider } from "@bitwarden/common/platform/state"; + +import { WebBrowserInteractionService } from "../services/web-browser-interaction.service"; + +import { setupExtensionRedirectGuard } from "./setup-extension-redirect.guard"; + +describe("setupExtensionRedirectGuard", () => { + const _state = Object.freeze({}) as RouterStateSnapshot; + const emptyRoute = Object.freeze({ queryParams: {} }) as ActivatedRouteSnapshot; + const seventeenDaysAgo = new Date(); + seventeenDaysAgo.setDate(seventeenDaysAgo.getDate() - 17); + + const account = { + id: "account-id", + } as unknown as Account; + + const activeAccount$ = new BehaviorSubject(account); + const extensionInstalled$ = new BehaviorSubject(false); + const state$ = new BehaviorSubject(false); + const createUrlTree = jest.fn(); + const getFeatureFlag = jest.fn().mockImplementation((key) => { + if (key === FeatureFlag.PM19315EndUserActivationMvp) { + return Promise.resolve(true); + } + + return Promise.resolve(false); + }); + const getProfileCreationDate = jest.fn().mockResolvedValue(seventeenDaysAgo); + + beforeEach(() => { + Utils.isMobileBrowser = false; + + getFeatureFlag.mockClear(); + getProfileCreationDate.mockClear(); + createUrlTree.mockClear(); + + TestBed.configureTestingModule({ + providers: [ + { provide: Router, useValue: { createUrlTree } }, + { provide: ConfigService, useValue: { getFeatureFlag } }, + { provide: AccountService, useValue: { activeAccount$ } }, + { provide: StateProvider, useValue: { getUser: () => ({ state$ }) } }, + { provide: WebBrowserInteractionService, useValue: { extensionInstalled$ } }, + { + provide: VaultProfileService, + useValue: { getProfileCreationDate }, + }, + ], + }); + }); + + function setupExtensionGuard(route?: ActivatedRouteSnapshot) { + // Run the guard within injection context so `inject` works as you'd expect + // Pass state object to make TypeScript happy + return TestBed.runInInjectionContext(async () => + setupExtensionRedirectGuard(route ?? emptyRoute, _state), + ); + } + + it("returns `true` when the profile was created more than 30 days ago", async () => { + const thirtyOneDaysAgo = new Date(); + thirtyOneDaysAgo.setDate(thirtyOneDaysAgo.getDate() - 31); + + getProfileCreationDate.mockResolvedValueOnce(thirtyOneDaysAgo); + + expect(await setupExtensionGuard()).toBe(true); + }); + + it("returns `true` when the profile check fails", async () => { + getProfileCreationDate.mockRejectedValueOnce(new Error("Profile check failed")); + + expect(await setupExtensionGuard()).toBe(true); + }); + + it("returns `true` when the feature flag is disabled", async () => { + getFeatureFlag.mockResolvedValueOnce(false); + + expect(await setupExtensionGuard()).toBe(true); + }); + + it("returns `true` when the user is on a mobile device", async () => { + Utils.isMobileBrowser = true; + + expect(await setupExtensionGuard()).toBe(true); + }); + + it("returns `true` when the user has dismissed the extension page", async () => { + state$.next(true); + + expect(await setupExtensionGuard()).toBe(true); + }); + + it("returns `true` when the user has the extension installed", async () => { + state$.next(false); + extensionInstalled$.next(true); + + expect(await setupExtensionGuard()).toBe(true); + }); + + it('redirects the user to "/setup-extension" when all criteria do not pass', async () => { + state$.next(false); + extensionInstalled$.next(false); + + await setupExtensionGuard(); + + expect(createUrlTree).toHaveBeenCalledWith(["/setup-extension"]); + }); + + describe("missing current account", () => { + afterAll(() => { + // reset `activeAccount$` observable + activeAccount$.next(account); + }); + + it("redirects to login when account is missing", async () => { + activeAccount$.next(null); + + await setupExtensionGuard(); + + expect(createUrlTree).toHaveBeenCalledWith(["/login"]); + }); + }); +}); diff --git a/apps/web/src/app/vault/guards/setup-extension-redirect.guard.ts b/apps/web/src/app/vault/guards/setup-extension-redirect.guard.ts new file mode 100644 index 00000000000..983fd8ed0aa --- /dev/null +++ b/apps/web/src/app/vault/guards/setup-extension-redirect.guard.ts @@ -0,0 +1,109 @@ +import { inject } from "@angular/core"; +import { CanActivateFn, Router } from "@angular/router"; +import { firstValueFrom, map } from "rxjs"; + +import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { + SETUP_EXTENSION_DISMISSED_DISK, + StateProvider, + UserKeyDefinition, +} from "@bitwarden/common/platform/state"; + +import { WebBrowserInteractionService } from "../services/web-browser-interaction.service"; + +export const SETUP_EXTENSION_DISMISSED = new UserKeyDefinition( + SETUP_EXTENSION_DISMISSED_DISK, + "setupExtensionDismissed", + { + deserializer: (dismissed) => dismissed, + clearOn: [], + }, +); + +export const setupExtensionRedirectGuard: CanActivateFn = async () => { + const router = inject(Router); + const configService = inject(ConfigService); + const accountService = inject(AccountService); + const vaultProfileService = inject(VaultProfileService); + const stateProvider = inject(StateProvider); + const webBrowserInteractionService = inject(WebBrowserInteractionService); + + const isMobile = Utils.isMobileBrowser; + + const endUserFeatureEnabled = await configService.getFeatureFlag( + FeatureFlag.PM19315EndUserActivationMvp, + ); + + // The extension page isn't applicable for mobile users, do not redirect them. + // Include before any other checks to avoid unnecessary processing. + if (!endUserFeatureEnabled || isMobile) { + return true; + } + + const currentAcct = await firstValueFrom(accountService.activeAccount$); + + if (!currentAcct) { + return router.createUrlTree(["/login"]); + } + + const hasExtensionInstalledPromise = firstValueFrom( + webBrowserInteractionService.extensionInstalled$, + ); + + const dismissedExtensionPage = await firstValueFrom( + stateProvider + .getUser(currentAcct.id, SETUP_EXTENSION_DISMISSED) + .state$.pipe(map((dismissed) => dismissed ?? false)), + ); + + const isProfileOlderThan30Days = await profileIsOlderThan30Days( + vaultProfileService, + currentAcct.id, + ).catch( + () => + // If the call for the profile fails for any reason, do not block the user + true, + ); + + if (dismissedExtensionPage || isProfileOlderThan30Days) { + return true; + } + + // Checking for the extension is a more expensive operation, do it last to avoid unnecessary delays. + const hasExtensionInstalled = await hasExtensionInstalledPromise; + + if (hasExtensionInstalled) { + return true; + } + + return router.createUrlTree(["/setup-extension"]); +}; + +/** Returns true when the user's profile is older than 30 days */ +async function profileIsOlderThan30Days( + vaultProfileService: VaultProfileService, + userId: string, +): Promise { + const creationDate = await vaultProfileService.getProfileCreationDate(userId); + return isMoreThan30DaysAgo(creationDate); +} + +/** Returns the true when the date given is older than 30 days */ +function isMoreThan30DaysAgo(date?: string | Date): boolean { + if (!date) { + return false; + } + + const inputDate = new Date(date).getTime(); + const today = new Date().getTime(); + + const differenceInMS = today - inputDate; + const msInADay = 1000 * 60 * 60 * 24; + const differenceInDays = Math.round(differenceInMS / msInADay); + + return differenceInDays > 30; +} diff --git a/apps/web/src/app/vault/services/web-browser-interaction.service.spec.ts b/apps/web/src/app/vault/services/web-browser-interaction.service.spec.ts index fef5d45e8c3..bfbfb0fb676 100644 --- a/apps/web/src/app/vault/services/web-browser-interaction.service.spec.ts +++ b/apps/web/src/app/vault/services/web-browser-interaction.service.spec.ts @@ -38,7 +38,7 @@ describe("WebBrowserInteractionService", () => { expect(installed).toBe(false); }); - tick(1500); + tick(150); })); it("returns true when the extension is installed", (done) => { @@ -58,13 +58,13 @@ describe("WebBrowserInteractionService", () => { }); // initial timeout, should emit false - tick(1500); + tick(26); expect(results[0]).toBe(false); tick(2500); // then emit `HasBwInstalled` dispatchEvent(VaultMessages.HasBwInstalled); - tick(); + tick(26); expect(results[1]).toBe(true); })); }); diff --git a/apps/web/src/app/vault/services/web-browser-interaction.service.ts b/apps/web/src/app/vault/services/web-browser-interaction.service.ts index f1005ef6dc9..1f91942591b 100644 --- a/apps/web/src/app/vault/services/web-browser-interaction.service.ts +++ b/apps/web/src/app/vault/services/web-browser-interaction.service.ts @@ -21,10 +21,19 @@ import { ExtensionPageUrls } from "@bitwarden/common/vault/enums"; import { VaultMessages } from "@bitwarden/common/vault/enums/vault-messages.enum"; /** - * The amount of time in milliseconds to wait for a response from the browser extension. + * The amount of time in milliseconds to wait for a response from the browser extension. A longer duration is + * used to allow for the extension to open and then emit to the message. * NOTE: This value isn't computed by any means, it is just a reasonable timeout for the extension to respond. */ -const MESSAGE_RESPONSE_TIMEOUT_MS = 1500; +const OPEN_RESPONSE_TIMEOUT_MS = 1500; + +/** + * Timeout for checking if the extension is installed. + * + * A shorter timeout is used to avoid waiting for too long for the extension. The listener for + * checking the installation runs in the background scripts so the response should be relatively quick. + */ +const CHECK_FOR_EXTENSION_TIMEOUT_MS = 25; @Injectable({ providedIn: "root", @@ -63,7 +72,7 @@ export class WebBrowserInteractionService { filter((event) => event.data.command === VaultMessages.PopupOpened), map(() => true), ), - timer(MESSAGE_RESPONSE_TIMEOUT_MS).pipe(map(() => false)), + timer(OPEN_RESPONSE_TIMEOUT_MS).pipe(map(() => false)), ) .pipe(take(1)) .subscribe((didOpen) => { @@ -85,7 +94,7 @@ export class WebBrowserInteractionService { filter((event) => event.data.command === VaultMessages.HasBwInstalled), map(() => true), ), - timer(MESSAGE_RESPONSE_TIMEOUT_MS).pipe(map(() => false)), + timer(CHECK_FOR_EXTENSION_TIMEOUT_MS).pipe(map(() => false)), ).pipe( tap({ subscribe: () => { diff --git a/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.ts b/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.ts index b51f45f1b27..2bef5670ac3 100644 --- a/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.ts +++ b/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.ts @@ -28,10 +28,6 @@ export class DefaultRegistrationFinishService implements RegistrationFinishServi return null; } - determineLoginSuccessRoute(): Promise { - return Promise.resolve("/vault"); - } - async finishRegistration( email: string, passwordInputResult: PasswordInputResult, diff --git a/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts b/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts index 1d1a2d8f892..dac62f039ee 100644 --- a/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts +++ b/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts @@ -204,8 +204,7 @@ export class RegistrationFinishComponent implements OnInit, OnDestroy { await this.loginSuccessHandlerService.run(authenticationResult.userId); - const successRoute = await this.registrationFinishService.determineLoginSuccessRoute(); - await this.router.navigate([successRoute]); + await this.router.navigate(["/vault"]); } catch (e) { // If login errors, redirect to login page per product. Don't show error this.logService.error("Error logging in after registration: ", e.message); diff --git a/libs/auth/src/angular/registration/registration-finish/registration-finish.service.ts b/libs/auth/src/angular/registration/registration-finish/registration-finish.service.ts index 523a4c79c54..5f3c04e5155 100644 --- a/libs/auth/src/angular/registration/registration-finish/registration-finish.service.ts +++ b/libs/auth/src/angular/registration/registration-finish/registration-finish.service.ts @@ -16,11 +16,6 @@ export abstract class RegistrationFinishService { */ abstract getMasterPasswordPolicyOptsFromOrgInvite(): Promise; - /** - * Returns the route the user is redirected to after a successful login. - */ - abstract determineLoginSuccessRoute(): Promise; - /** * Finishes the registration process by creating a new user account. * diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index 93c489a343e..a1c3ee35c5c 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -202,6 +202,13 @@ export const SECURITY_TASKS_DISK = new StateDefinition("securityTasks", "disk"); export const AT_RISK_PASSWORDS_PAGE_DISK = new StateDefinition("atRiskPasswordsPage", "disk"); export const NOTIFICATION_DISK = new StateDefinition("notifications", "disk"); export const NUDGES_DISK = new StateDefinition("nudges", "disk", { web: "disk-local" }); +export const SETUP_EXTENSION_DISMISSED_DISK = new StateDefinition( + "setupExtensionDismissed", + "disk", + { + web: "disk-local", + }, +); export const VAULT_BROWSER_INTRO_CAROUSEL = new StateDefinition( "vaultBrowserIntroCarousel", "disk",