-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
PM-1391-Added previous-url to global-state (#5733)
* added previous-url to global-state * updated storage of previousUrl for SSO/MFA flows * revert file changes * added post login routing * Clear PreviousUrl from storage on new Login * Components do not call StateService anymore * removed needed query params * refactored components to use RouterService * fixed build error * fixed mfa component * updated logic for previous Url * removed unneeded base implementation * Added state call for Redirect Guard * Fixed test cases * Remove routing service calls * renamed global field, changed routing to guard * reverting constructor changes and git lint issue * fixing constructor ordering * fixing diffs to be clearer on actual cahnges. * addressing accepting emergency access case * refactor and add locked state logic * refactor name of guard to be more clear * Added comments and tests * comments + support lock page deep linking + code ownership * readability updates * Combined guards and specs updated routing * Update oss-routing.module.ts * fixed stroybook build
- Loading branch information
1 parent
a6e3d4d
commit f1691a5
Showing
14 changed files
with
321 additions
and
90 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| import { Component } from "@angular/core"; | ||
| import { TestBed } from "@angular/core/testing"; | ||
| import { Router, provideRouter } from "@angular/router"; | ||
| import { RouterTestingHarness } from "@angular/router/testing"; | ||
| import { MockProxy, mock } from "jest-mock-extended"; | ||
|
|
||
| import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; | ||
| import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; | ||
|
|
||
| import { RouterService } from "../../core/router.service"; | ||
|
|
||
| import { deepLinkGuard } from "./deep-link.guard"; | ||
|
|
||
| @Component({ | ||
| template: "", | ||
| }) | ||
| export class GuardedRouteTestComponent {} | ||
|
|
||
| @Component({ | ||
| template: "", | ||
| }) | ||
| export class LockTestComponent {} | ||
|
|
||
| @Component({ | ||
| template: "", | ||
| }) | ||
| export class RedirectTestComponent {} | ||
|
|
||
| /** | ||
| * We are assuming the guard is always being called. We are creating routes using the | ||
| * RouterTestingHarness. | ||
| * | ||
| * when persisting a URL to storage we don't care wether or not the user is locked or logged out. | ||
| * We only care about where the user is going, and has been. | ||
| * | ||
| * We are testing the activatedComponent because we are testing that the guard redirects when a user is | ||
| * unlocked. | ||
| */ | ||
| describe("Deep Link Guard", () => { | ||
| let authService: MockProxy<AuthService>; | ||
| let routerService: MockProxy<RouterService>; | ||
| let routerHarness: RouterTestingHarness; | ||
|
|
||
| beforeEach(async () => { | ||
| authService = mock<AuthService>(); | ||
| routerService = mock<RouterService>(); | ||
| TestBed.configureTestingModule({ | ||
| providers: [ | ||
| { provide: AuthService, useValue: authService }, | ||
| { provide: RouterService, useValue: routerService }, | ||
| provideRouter([ | ||
| { | ||
| path: "guarded-route", | ||
| component: GuardedRouteTestComponent, | ||
| canActivate: [deepLinkGuard()], | ||
| }, | ||
| { | ||
| path: "lock-route", | ||
| component: LockTestComponent, | ||
| canActivate: [deepLinkGuard()], | ||
| }, | ||
| { | ||
| path: "redirect-route", | ||
| component: RedirectTestComponent, | ||
| }, | ||
| ]), | ||
| ], | ||
| }); | ||
|
|
||
| routerHarness = await RouterTestingHarness.create(); | ||
| }); | ||
|
|
||
| // Story: User's vault times out | ||
| it('should persist routerService.previousUrl when routerService.previousUrl does not contain "lock"', async () => { | ||
| // Arrange | ||
| authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Locked); | ||
| routerService.getPreviousUrl.mockReturnValue("/previous-url"); | ||
|
|
||
| // Act | ||
| await routerHarness.navigateByUrl("/lock-route"); | ||
|
|
||
| // Assert | ||
| expect(routerService.persistLoginRedirectUrl).toHaveBeenCalledWith("/previous-url"); | ||
| }); | ||
|
|
||
| // Story: User's vault times out and previousUrl contains "lock" | ||
| it('should not persist routerService.previousUrl when routerService.previousUrl contains "lock"', async () => { | ||
| // Arrange | ||
| authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Locked); | ||
| routerService.getPreviousUrl.mockReturnValue("/lock"); | ||
|
|
||
| // Act | ||
| await routerHarness.navigateByUrl("/lock-route"); | ||
|
|
||
| // Assert | ||
| expect(routerService.persistLoginRedirectUrl).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| // Story: User's vault times out and previousUrl is undefined | ||
| it("should not persist routerService.previousUrl when routerService.previousUrl is undefined", async () => { | ||
| // Arrange | ||
| authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Locked); | ||
| routerService.getPreviousUrl.mockReturnValue(undefined); | ||
|
|
||
| // Act | ||
| await routerHarness.navigateByUrl("/lock-route"); | ||
|
|
||
| // Assert | ||
| expect(routerService.persistLoginRedirectUrl).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| // Story: User tries to deep link to a guarded route and is logged out | ||
| it('should persist currentUrl when currentUrl does not contain "lock"', async () => { | ||
| // Arrange | ||
| authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.LoggedOut); | ||
|
|
||
| // Act | ||
| await routerHarness.navigateByUrl("/guarded-route?item=123"); | ||
|
|
||
| // Assert | ||
| expect(routerService.persistLoginRedirectUrl).toHaveBeenCalledWith("/guarded-route?item=123"); | ||
| }); | ||
|
|
||
| // Story: User tries to deep link to "lock" | ||
| it('should not persist currentUrl if the currentUrl contains "lock"', async () => { | ||
| // Arrange | ||
| authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.LoggedOut); | ||
|
|
||
| // Act | ||
| await routerHarness.navigateByUrl("/lock-route"); | ||
|
|
||
| // Assert | ||
| expect(routerService.persistLoginRedirectUrl).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| // Story: User tries to deep link to a guarded route from the lock page | ||
| it("should persist currentUrl over previousUrl", async () => { | ||
| // Arrange | ||
| authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Locked); | ||
| routerService.getPreviousUrl.mockReturnValue("/previous-url"); | ||
|
|
||
| // Act | ||
| await routerHarness.navigateByUrl("/guarded-route?item=123"); | ||
|
|
||
| // Assert | ||
| expect(routerService.persistLoginRedirectUrl).toHaveBeenCalledWith("/guarded-route?item=123"); | ||
| }); | ||
|
|
||
| // Story: user tries to deep link and is unlocked | ||
| it("should not persist any URL if the user is unlocked", async () => { | ||
| // Arrange | ||
| authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Unlocked); | ||
|
|
||
| // Act | ||
| await routerHarness.navigateByUrl("/guarded-route"); | ||
|
|
||
| // Assert | ||
| expect(routerService.persistLoginRedirectUrl).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| // Story: User is redirected | ||
| it("should redirect user", async () => { | ||
| // Arrange | ||
| authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Unlocked); | ||
| routerService.getAndClearLoginRedirectUrl.mockResolvedValue("/redirect-route"); | ||
|
|
||
| // Act | ||
| const activatedComponent = await routerHarness.navigateByUrl("/guarded-route"); | ||
|
|
||
| // Assert | ||
| expect(routerService.persistLoginRedirectUrl).not.toHaveBeenCalled(); | ||
| expect(TestBed.inject(Router).url).toEqual("/redirect-route"); | ||
| expect(activatedComponent).toBeInstanceOf(RedirectTestComponent); | ||
| }); | ||
|
|
||
| // Story: User is not redirected | ||
| it("should not redirect user", async () => { | ||
| // Arrange | ||
| authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Unlocked); | ||
| routerService.getAndClearLoginRedirectUrl.mockResolvedValue(""); | ||
|
|
||
| // Act | ||
| const activatedComponent = await routerHarness.navigateByUrl("/guarded-route"); | ||
|
|
||
| // Assert | ||
| expect(routerService.persistLoginRedirectUrl).not.toHaveBeenCalled(); | ||
| expect(TestBed.inject(Router).url).toEqual("/guarded-route"); | ||
| expect(activatedComponent).toBeInstanceOf(GuardedRouteTestComponent); | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import { inject } from "@angular/core"; | ||
| import { CanActivateFn, Router } from "@angular/router"; | ||
|
|
||
| import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; | ||
| import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; | ||
| import { Utils } from "@bitwarden/common/platform/misc/utils"; | ||
|
|
||
| import { RouterService } from "../../core/router.service"; | ||
|
|
||
| /** | ||
| * Guard to persist and apply deep links to handle users who are not unlocked. | ||
| * @returns returns true. If user is not Unlocked will store URL to state for redirect once | ||
| * user is unlocked/Authenticated. | ||
| */ | ||
| export function deepLinkGuard(): CanActivateFn { | ||
| return async (route, routerState) => { | ||
| // Inject Services | ||
| const authService = inject(AuthService); | ||
| const router = inject(Router); | ||
| const routerService = inject(RouterService); | ||
|
|
||
| // Fetch State | ||
| const currentUrl = routerState.url; | ||
| const transientPreviousUrl = routerService.getPreviousUrl(); | ||
| const authStatus = await authService.getAuthStatus(); | ||
|
|
||
| // Evaluate State | ||
| /** before anything else, check if the user is already unlocked. */ | ||
| if (authStatus === AuthenticationStatus.Unlocked) { | ||
| const persistedPreLoginUrl = await routerService.getAndClearLoginRedirectUrl(); | ||
| if (!Utils.isNullOrEmpty(persistedPreLoginUrl)) { | ||
| return router.navigateByUrl(persistedPreLoginUrl); | ||
| } | ||
| return true; | ||
| } | ||
| /** | ||
| * At this point the user is either `locked` or `loggedOut`, it doesn't matter. | ||
| * We opt to persist the currentUrl over the transient previousUrl. This supports | ||
| * the case where a user is locked out of their vault and they deep link from | ||
| * the "lock" page. | ||
| * | ||
| * When the user is locked out of their vault the currentUrl contains "lock" so it will | ||
| * not be persisted, the previousUrl will be persisted instead. | ||
| */ | ||
| if (isValidUrl(currentUrl)) { | ||
| await routerService.persistLoginRedirectUrl(currentUrl); | ||
| } else if (isValidUrl(transientPreviousUrl)) { | ||
| await routerService.persistLoginRedirectUrl(transientPreviousUrl); | ||
| } | ||
| return true; | ||
| }; | ||
|
|
||
| function isValidUrl(url: string | null | undefined): boolean { | ||
| return !Utils.isNullOrEmpty(url) && !url?.toLocaleLowerCase().includes("lock"); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.