mirror of
https://github.com/bitwarden/browser
synced 2025-12-06 00:13:28 +00:00
[PM-20540] Deep-link refactor to fix SSO deep links (#14587)
* PM-20540 - TwoFactorAuthComponent - Refactor determineDefaultSuccessRoute to rely on user's auth status as the loginStrategyService's state is cleared after successful AuthN * PM-20540 - DeepLinkGuard - Refactor to exempt login-initiated so that TDE + unlock with MP + deep link works. * doc: Add documentation and change folder structure. * test: add test for new excluded route. --------- Co-authored-by: Jared Snider <jsnider@bitwarden.com>
This commit is contained in:
@@ -14,7 +14,7 @@ import {
|
||||
} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
|
||||
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
|
||||
|
||||
import { deepLinkGuard } from "../../auth/guards/deep-link.guard";
|
||||
import { deepLinkGuard } from "../../auth/guards/deep-link/deep-link.guard";
|
||||
|
||||
import { VaultModule } from "./collections/vault.module";
|
||||
import { isEnterpriseOrgGuard } from "./guards/is-enterprise-org.guard";
|
||||
|
||||
@@ -1,58 +0,0 @@
|
||||
// FIXME: Update this file to be type safe and remove this and next line
|
||||
// @ts-strict-ignore
|
||||
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");
|
||||
}
|
||||
}
|
||||
@@ -7,7 +7,7 @@ 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 { RouterService } from "../../../core/router.service";
|
||||
|
||||
import { deepLinkGuard } from "./deep-link.guard";
|
||||
|
||||
@@ -99,6 +99,18 @@ describe("Deep Link Guard", () => {
|
||||
expect(routerService.persistLoginRedirectUrl).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not persist routerService.previousUrl when routerService.previousUrl contains "login-initiated"', async () => {
|
||||
// Arrange
|
||||
authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Locked);
|
||||
routerService.getPreviousUrl.mockReturnValue("/login-initiated");
|
||||
|
||||
// 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
|
||||
99
apps/web/src/app/auth/guards/deep-link/deep-link.guard.ts
Normal file
99
apps/web/src/app/auth/guards/deep-link/deep-link.guard.ts
Normal file
@@ -0,0 +1,99 @@
|
||||
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: string | undefined =
|
||||
await routerService.getAndClearLoginRedirectUrl();
|
||||
if (persistedPreLoginUrl === undefined) {
|
||||
// Url us undefined, so there is nothing to navigate to.
|
||||
return true;
|
||||
}
|
||||
// Check if the url is empty or null
|
||||
if (!Utils.isNullOrEmpty(persistedPreLoginUrl)) {
|
||||
// const urlTree: string | UrlTree = 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) && transientPreviousUrl !== undefined) {
|
||||
await routerService.persistLoginRedirectUrl(transientPreviousUrl);
|
||||
}
|
||||
return true;
|
||||
};
|
||||
|
||||
/**
|
||||
* Check if the URL is valid for deep linking. A valid url is described as not including
|
||||
* "lock" or "login-initiated". Valid urls are only urls that are not part of login or
|
||||
* decryption flows.
|
||||
* We ignore the "lock" url because standard SSO flows will send users to the lock component.
|
||||
* We ignore "login-initiated" because TDE users decrypting with master passwords are
|
||||
* sent to the lock component.
|
||||
* @param url The URL to check.
|
||||
* @returns True if the URL is valid, false otherwise.
|
||||
*/
|
||||
function isValidUrl(url: string | null | undefined): boolean {
|
||||
if (url === undefined || url === null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (Utils.isNullOrEmpty(url)) {
|
||||
return false;
|
||||
}
|
||||
const lowerCaseUrl: string = url.toLocaleLowerCase();
|
||||
|
||||
/**
|
||||
* "Login-initiated" ignored because it is used for TDE users decrypting from a new device. A TDE user
|
||||
* can opt to decrypt using their password. Decrypting with a password will send the user to the lock component,
|
||||
* which is protected by the deep link guard. We don't persist the `login-initiated` url because it is not a
|
||||
* valid deep-link. We don't want users to be sent to the login-initiated url when they are unlocked.
|
||||
* If we did navigate to the login-initiated url, the user would get caught by the TDE Guard and be sent
|
||||
* to the vault and not the intended deep link.
|
||||
*
|
||||
* "Lock" is ignored because users cannot deep-link to the lock component if they are already unlocked.
|
||||
* Users logging in with SSO will be sent to the lock component after they are authenticated with their IdP.
|
||||
* SSO users would be navigated to the "lock" component loop if we persisted the "lock" url.
|
||||
*/
|
||||
|
||||
if (lowerCaseUrl.includes("/login-initiated") || lowerCaseUrl.includes("/lock")) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
}
|
||||
23
apps/web/src/app/auth/guards/deep-link/readme.md
Normal file
23
apps/web/src/app/auth/guards/deep-link/readme.md
Normal file
@@ -0,0 +1,23 @@
|
||||
# Deep-link Guard
|
||||
|
||||
The `deep-link.guard.ts` supports users who are trying to access a protected route from an unauthenticated or locked state.
|
||||
|
||||
This guard will persist the protected URL to session state when a user is either unauthenticated or in an encrypted/locked state. This allows users to have multiple tabs of the application running simultaneously without interfering with 'previousUrl` functionality.
|
||||
|
||||
Writing to session state allows users who are authenticating through SSO to be routed to their identity provider and back without losing the protected route they were trying to access in the first place.
|
||||
|
||||
The deep link guard will not persist Urls that are in the middle of authentication or decryption. SSO users will sometimes have to decrypt their vault after a successful authentication. This is why we do not persist the `/lock` route.
|
||||
|
||||
## General operation
|
||||
|
||||
The `deep-link.guard.ts` will always return true. The `deep-link.guard.ts` will only persist a URL if the user is in an unauthenticated or locked state. The URL cannot contain `/lock` or `/login-initiated`. The persisted URL is cleared from state when it is read.
|
||||
|
||||
## Routes to protect
|
||||
|
||||
The deep link guards should be used on routes where a user will be navigated to a protected route but may not be authenticated, decrypted, or have an account.
|
||||
|
||||
A use cases is the `emergency-access` route which is a link that is sent to the user's email address, and in order for them to accept the request, they must first authenticate and decrypt.
|
||||
|
||||
## TDE Users decrypting/unlocking with password
|
||||
|
||||
For TDE users opting to decrypt with a password they will be routed from the `login-initiated` to the `lock` route. We ignore the `login-initiated` route for this reason allowing TDE users who decrypt/unlock with a password to still be navigated to the initial request.
|
||||
@@ -48,7 +48,7 @@ import { VerifyRecoverDeleteOrgComponent } from "./admin-console/organizations/m
|
||||
import { AcceptFamilySponsorshipComponent } from "./admin-console/organizations/sponsorships/accept-family-sponsorship.component";
|
||||
import { FamiliesForEnterpriseSetupComponent } from "./admin-console/organizations/sponsorships/families-for-enterprise-setup.component";
|
||||
import { CreateOrganizationComponent } from "./admin-console/settings/create-organization.component";
|
||||
import { deepLinkGuard } from "./auth/guards/deep-link.guard";
|
||||
import { deepLinkGuard } from "./auth/guards/deep-link/deep-link.guard";
|
||||
import { LoginViaWebAuthnComponent } from "./auth/login/login-via-webauthn/login-via-webauthn.component";
|
||||
import { AcceptOrganizationComponent } from "./auth/organization-invite/accept-organization.component";
|
||||
import { RecoverDeleteComponent } from "./auth/recover-delete.component";
|
||||
|
||||
Reference in New Issue
Block a user