1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-04 18:53:20 +00:00

[PM-2207], [PM-1245], [PM-3302] Make browser login, lock, and 2fa components handle configurable redirect routes (#5989)

* Initial work

* Added lock and login redirect and added functionality to abort when in login or locked state

* uncommented cipher row

* added query params to logi component

* Proof of concept for change detection fix

* Remove leftover comment

* Refactored message listener observable to handle angular change detection

* cleanup and removed unused references

* Refactored the connect method be seperating to the pop out logic to a seperate method

* Added comment to explain code change on the message listener

* Removed unused types

* Initial work

* Added lock and login redirect and added functionality to abort when in login or locked state

* uncommented cipher row

* added query params to logi component

* Proof of concept for change detection fix

* Remove leftover comment

* Refactored message listener observable to handle angular change detection

* cleanup and removed unused references

* Refactored the connect method be seperating to the pop out logic to a seperate method

* Added comment to explain code change on the message listener

* Removed unused types

* Added full synce service to the fido2 authenticator to ensure the full sync is completed before getting all decrypted ciphers

* Added full synce service to the fido2 authenticator to ensure the full sync is completed before getting all decrypted ciphers

* Code cleanup to remove sessionId from login component

* Refactored components to make the redirectUrl more generic, fixed code review comments

* Commented out ensureUnlockedVault for this PR

* Fixed destroy subject inheritance issue on the login componenet

* Fixed lock component error

* Added function to run inside angular zone

* Merged branch with master and fixed conflicts

* Changed redirect logic on login and 2fa to use callbacks

* fixed pr comments

* Updated the messageListener observable version to use same logic from the callback version and added comment on the callback version

* Refactored fido2 popup to use auth guard when routing to component, added BrowserRouterService to track previous page and route using that

* Updated components to use browserRouterService for routing to previous page

* Removed auth status reference from browser-fido2-user-interface service

* Removed activated route from lock component

* Removed route in base class constructor

* removed unused comments and method

* refactored router service to not store on the disk

* [PM-3783] feat: patch `chrome.runtime.onMessage` event listeners

(cherry picked from commit 2ca241a0d4)

* Fixed PR comments

* Fixed PR comments

* Revert "[PM-3783] feat: patch `chrome.runtime.onMessage` event listeners"

This reverts commit ed6a713688.

---------

Co-authored-by: Thomas Rittson <trittson@bitwarden.com>
Co-authored-by: Andreas Coroiu <andreas.coroiu@gmail.com>
This commit is contained in:
SmithThe4th
2023-09-12 17:19:16 -04:00
committed by GitHub
parent be620a935d
commit dbbbae2f52
15 changed files with 247 additions and 43 deletions

View File

@@ -0,0 +1,35 @@
import { inject } from "@angular/core";
import {
ActivatedRouteSnapshot,
CanActivateFn,
Router,
RouterStateSnapshot,
} from "@angular/router";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { BrowserRouterService } from "../../platform/popup/services/browser-router.service";
export const fido2AuthGuard: CanActivateFn = async (
route: ActivatedRouteSnapshot,
state: RouterStateSnapshot
) => {
const routerService = inject(BrowserRouterService);
const authService = inject(AuthService);
const router = inject(Router);
const authStatus = await authService.getAuthStatus();
if (authStatus === AuthenticationStatus.LoggedOut) {
routerService.setPreviousUrl(state.url);
return router.createUrlTree(["/home"], { queryParams: route.queryParams });
}
if (authStatus === AuthenticationStatus.Locked) {
routerService.setPreviousUrl(state.url);
return router.createUrlTree(["/lock"], { queryParams: route.queryParams });
}
return true;
};

View File

@@ -22,6 +22,7 @@ import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/pass
import { DialogService } from "@bitwarden/components";
import { BiometricErrors, BiometricErrorTypes } from "../../models/biometricErrors";
import { BrowserRouterService } from "../../platform/popup/services/browser-router.service";
@Component({
selector: "app-lock",
@@ -52,7 +53,8 @@ export class LockComponent extends BaseLockComponent {
private authService: AuthService,
dialogService: DialogService,
deviceTrustCryptoService: DeviceTrustCryptoServiceAbstraction,
userVerificationService: UserVerificationService
userVerificationService: UserVerificationService,
private routerService: BrowserRouterService
) {
super(
router,
@@ -76,6 +78,15 @@ export class LockComponent extends BaseLockComponent {
);
this.successRoute = "/tabs/current";
this.isInitialLockScreen = (window as any).previousPopupUrl == null;
super.onSuccessfulSubmit = async () => {
const previousUrl = this.routerService.getPreviousUrl();
if (previousUrl) {
this.router.navigateByUrl(previousUrl);
} else {
this.router.navigate([this.successRoute]);
}
};
}
async ngOnInit() {

View File

@@ -19,6 +19,7 @@ import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/ge
import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction";
import { flagEnabled } from "../../platform/flags";
import { BrowserRouterService } from "../../platform/popup/services/browser-router.service";
@Component({
selector: "app-login",
@@ -26,6 +27,7 @@ import { flagEnabled } from "../../platform/flags";
})
export class LoginComponent extends BaseLoginComponent {
showPasswordless = false;
constructor(
devicesApiService: DevicesApiServiceAbstraction,
appIdService: AppIdService,
@@ -43,7 +45,8 @@ export class LoginComponent extends BaseLoginComponent {
formBuilder: FormBuilder,
formValidationErrorService: FormValidationErrorsService,
route: ActivatedRoute,
loginService: LoginService
loginService: LoginService,
private routerService: BrowserRouterService
) {
super(
devicesApiService,
@@ -66,7 +69,19 @@ export class LoginComponent extends BaseLoginComponent {
super.onSuccessfulLogin = async () => {
await syncService.fullSync(true);
};
super.successRoute = "/tabs/vault";
super.onSuccessfulLoginNavigate = async () => {
const previousUrl = this.routerService.getPreviousUrl();
if (previousUrl) {
this.router.navigateByUrl(previousUrl);
} else {
this.router.navigate([this.successRoute]);
}
};
this.showPasswordless = flagEnabled("showPasswordless");
if (this.showPasswordless) {

View File

@@ -22,6 +22,7 @@ import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.serv
import { DialogService } from "@bitwarden/components";
import { BrowserApi } from "../../platform/browser/browser-api";
import { BrowserRouterService } from "../../platform/popup/services/browser-router.service";
import { PopupUtilsService } from "../../popup/services/popup-utils.service";
const BroadcasterSubscriptionId = "TwoFactorComponent";
@@ -52,6 +53,7 @@ export class TwoFactorComponent extends BaseTwoFactorComponent {
loginService: LoginService,
configService: ConfigServiceAbstraction,
private dialogService: DialogService,
private routerService: BrowserRouterService,
@Inject(WINDOW) protected win: Window
) {
super(
@@ -83,6 +85,17 @@ export class TwoFactorComponent extends BaseTwoFactorComponent {
};
super.successRoute = "/tabs/vault";
super.onSuccessfulLoginNavigate = async () => {
const previousUrl = this.routerService.getPreviousUrl();
if (previousUrl) {
this.router.navigateByUrl(previousUrl);
} else {
this.router.navigate([this.successRoute]);
}
};
// FIXME: Chromium 110 has broken WebAuthn support in extensions via an iframe
this.webAuthnNewTab = true;
}

View File

@@ -560,6 +560,7 @@ export default class MainBackground {
this.fido2AuthenticatorService = new Fido2AuthenticatorService(
this.cipherService,
this.fido2UserInterfaceService,
this.syncService,
this.logService
);
this.fido2ClientService = new Fido2ClientService(

View File

@@ -249,8 +249,12 @@ export class BrowserApi {
static messageListener$() {
return new Observable<unknown>((subscriber) => {
const handler = (message: unknown) => subscriber.next(message);
chrome.runtime.onMessage.addListener(handler);
const handler = (message: unknown) => {
subscriber.next(message);
};
BrowserApi.messageListener("message", handler);
return () => chrome.runtime.onMessage.removeListener(handler);
});
}

View File

@@ -0,0 +1,37 @@
import { Injectable } from "@angular/core";
import { ActivatedRouteSnapshot, NavigationEnd, Router } from "@angular/router";
import { filter } from "rxjs";
@Injectable({
providedIn: "root",
})
export class BrowserRouterService {
private previousUrl?: string = undefined;
constructor(router: Router) {
router.events
.pipe(filter((e) => e instanceof NavigationEnd))
.subscribe((event: NavigationEnd) => {
const state: ActivatedRouteSnapshot = router.routerState.snapshot.root;
let child = state.firstChild;
while (child.firstChild) {
child = child.firstChild;
}
const updateUrl = !child?.data?.doNotSaveUrl ?? true;
if (updateUrl) {
this.setPreviousUrl(event.url);
}
});
}
getPreviousUrl() {
return this.previousUrl;
}
setPreviousUrl(url: string) {
this.previousUrl = url;
}
}

View File

@@ -11,6 +11,7 @@ import {
import { canAccessFeature } from "@bitwarden/angular/guard/feature-flag.guard";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { fido2AuthGuard } from "../auth/guards/fido2-auth.guard";
import { EnvironmentComponent } from "../auth/popup/environment.component";
import { HintComponent } from "../auth/popup/hint.component";
import { HomeComponent } from "../auth/popup/home.component";
@@ -72,17 +73,19 @@ const routes: Routes = [
path: "home",
component: HomeComponent,
canActivate: [UnauthGuard],
data: { state: "home" },
data: { state: "home", doNotSaveUrl: true },
},
{
path: "fido2",
component: Fido2Component,
canActivate: [fido2AuthGuard],
data: { state: "fido2" },
},
{
path: "login",
component: LoginComponent,
canActivate: [UnauthGuard],
data: { state: "login" },
data: { state: "login", doNotSaveUrl: true },
},
{
path: "login-with-device",
@@ -100,13 +103,13 @@ const routes: Routes = [
path: "lock",
component: LockComponent,
canActivate: [lockGuard()],
data: { state: "lock" },
data: { state: "lock", doNotSaveUrl: true },
},
{
path: "2fa",
component: TwoFactorComponent,
canActivate: [UnauthGuard],
data: { state: "2fa" },
data: { state: "2fa", doNotSaveUrl: true },
},
{
path: "2fa-options",

View File

@@ -4,10 +4,14 @@ import {
filter,
firstValueFrom,
fromEvent,
merge,
Observable,
Subject,
switchMap,
take,
takeUntil,
throwError,
fromEventPattern,
} from "rxjs";
import { Utils } from "@bitwarden/common/platform/misc/utils";
@@ -121,6 +125,8 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi
private messages$ = (BrowserApi.messageListener$() as Observable<BrowserFido2Message>).pipe(
filter((msg) => msg.sessionId === this.sessionId)
);
private windowClosed$: Observable<number>;
private tabClosed$: Observable<number>;
private connected$ = new BehaviorSubject(false);
private destroy$ = new Subject<void>();
private popout?: Popout;
@@ -162,12 +168,20 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi
.subscribe((msg) => {
if (msg.type === "AbortResponse") {
this.close();
this.abortController.abort(
msg.fallbackRequested ? UserRequestedFallbackAbortReason : undefined
);
this.abort(msg.fallbackRequested);
}
});
this.windowClosed$ = fromEventPattern(
(handler: any) => chrome.windows.onRemoved.addListener(handler),
(handler: any) => chrome.windows.onRemoved.removeListener(handler)
);
this.tabClosed$ = fromEventPattern(
(handler: any) => chrome.tabs.onRemoved.addListener(handler),
(handler: any) => chrome.tabs.onRemoved.removeListener(handler)
);
BrowserFido2UserInterfaceSession.sendMessage({
type: "NewSessionCreatedRequest",
sessionId,
@@ -230,6 +244,10 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi
await this.receive("AbortResponse");
}
async ensureUnlockedVault(): Promise<void> {
await this.connect();
}
async informCredentialNotFound(): Promise<void> {
const data: BrowserFido2Message = {
type: "InformCredentialNotFoundRequest",
@@ -248,6 +266,10 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi
this.destroy$.complete();
}
private async abort(fallback = false) {
this.abortController.abort(fallback ? UserRequestedFallbackAbortReason : undefined);
}
private async send(msg: BrowserFido2Message): Promise<void> {
if (!this.connected$.value) {
await this.connect();
@@ -279,16 +301,51 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi
throw new Error("Cannot re-open closed session");
}
const queryParams = new URLSearchParams({ sessionId: this.sessionId }).toString();
// create promise first to avoid race condition where the popout opens before we start listening
const connectPromise = firstValueFrom(
this.connected$.pipe(filter((connected) => connected === true))
);
this.popout = await this.popupUtilsService.popOut(
null,
`popup/index.html?uilocation=popout#/fido2?${queryParams}`,
{ center: true }
merge(
this.connected$.pipe(filter((connected) => connected === true)),
fromEvent(this.abortController.signal, "abort").pipe(
switchMap(() => throwError(() => new SessionClosedError()))
)
)
);
this.popout = await this.generatePopOut();
if (this.popout.type === "window") {
const popoutWindow = this.popout;
this.windowClosed$
.pipe(
filter((windowId) => popoutWindow.window.id === windowId),
takeUntil(this.destroy$)
)
.subscribe(() => {
this.close();
this.abort();
});
} else if (this.popout.type === "tab") {
const popoutTab = this.popout;
this.tabClosed$
.pipe(
filter((tabId) => popoutTab.tab.id === tabId),
takeUntil(this.destroy$)
)
.subscribe(() => {
this.close();
this.abort();
});
}
await connectPromise;
}
private async generatePopOut() {
const queryParams = new URLSearchParams({ sessionId: this.sessionId });
return this.popupUtilsService.popOut(
null,
`popup/index.html?uilocation=popout#/fido2?${queryParams.toString()}`,
{ center: true }
);
}
}

View File

@@ -1,4 +1,4 @@
import { Component, HostListener, OnDestroy, OnInit } from "@angular/core";
import { Component, OnDestroy, OnInit } from "@angular/core";
import { ActivatedRoute } from "@angular/router";
import {
BehaviorSubject,
@@ -154,7 +154,6 @@ export class Fido2Component implements OnInit, OnDestroy {
window.close();
}
@HostListener("window:unload")
unload(fallback = false) {
this.send({
sessionId: this.sessionId,

View File

@@ -1,7 +1,7 @@
import { Component, NgZone, OnDestroy, OnInit } from "@angular/core";
import { Component, NgZone, OnInit } from "@angular/core";
import { FormBuilder } from "@angular/forms";
import { ActivatedRoute, Router } from "@angular/router";
import { Subject, takeUntil } from "rxjs";
import { takeUntil } from "rxjs";
import { first } from "rxjs/operators";
import { LoginComponent as BaseLoginComponent } from "@bitwarden/angular/auth/components/login.component";
@@ -33,14 +33,13 @@ import { RouterService, StateService } from "../../core";
selector: "app-login",
templateUrl: "login.component.html",
})
export class LoginComponent extends BaseLoginComponent implements OnInit, OnDestroy {
// eslint-disable-next-line rxjs-angular/prefer-takeuntil
export class LoginComponent extends BaseLoginComponent implements OnInit {
showResetPasswordAutoEnrollWarning = false;
enforcedPasswordPolicyOptions: MasterPasswordPolicyOptions;
policies: ListResponse<PolicyResponse>;
showPasswordless = false;
private destroy$ = new Subject<void>();
constructor(
devicesApiService: DevicesApiServiceAbstraction,
appIdService: AppIdService,
@@ -145,11 +144,6 @@ export class LoginComponent extends BaseLoginComponent implements OnInit, OnDest
}
}
ngOnDestroy(): void {
this.destroy$.next();
this.destroy$.complete();
}
async goAfterLogIn() {
const masterPassword = this.formGroup.value.masterPassword;

View File

@@ -1,7 +1,8 @@
import { Directive, ElementRef, NgZone, OnInit, ViewChild } from "@angular/core";
import { Directive, ElementRef, NgZone, OnDestroy, OnInit, ViewChild } from "@angular/core";
import { FormBuilder, Validators } from "@angular/forms";
import { ActivatedRoute, Router } from "@angular/router";
import { take } from "rxjs/operators";
import { Subject } from "rxjs";
import { take, takeUntil } from "rxjs/operators";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { DevicesApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices-api.service.abstraction";
@@ -27,7 +28,7 @@ import {
import { CaptchaProtectedComponent } from "./captcha-protected.component";
@Directive()
export class LoginComponent extends CaptchaProtectedComponent implements OnInit {
export class LoginComponent extends CaptchaProtectedComponent implements OnInit, OnDestroy {
@ViewChild("masterPasswordInput", { static: true }) masterPasswordInput: ElementRef;
showPassword = false;
@@ -53,6 +54,8 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit
protected successRoute = "vault";
protected forcePasswordResetRoute = "update-temp-password";
protected destroy$ = new Subject<void>();
get loggedEmail() {
return this.formGroup.value.email;
}
@@ -83,14 +86,17 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit
}
async ngOnInit() {
this.route?.queryParams.subscribe((params) => {
if (params != null) {
const queryParamsEmail = params["email"];
if (queryParamsEmail != null && queryParamsEmail.indexOf("@") > -1) {
this.formGroup.get("email").setValue(queryParamsEmail);
this.loginService.setEmail(queryParamsEmail);
this.paramEmailSet = true;
}
this.route?.queryParams.pipe(takeUntil(this.destroy$)).subscribe((params) => {
if (!params) {
return;
}
const queryParamsEmail = params.email;
if (queryParamsEmail != null && queryParamsEmail.indexOf("@") > -1) {
this.formGroup.get("email").setValue(queryParamsEmail);
this.loginService.setEmail(queryParamsEmail);
this.paramEmailSet = true;
}
});
let email = this.loginService.getEmail();
@@ -109,6 +115,11 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit
this.formGroup.get("rememberEmail")?.setValue(rememberEmail);
}
ngOnDestroy() {
this.destroy$.next();
this.destroy$.complete();
}
async submit(showToast = true) {
const data = this.formGroup.value;

View File

@@ -28,6 +28,7 @@ export abstract class Fido2UserInterfaceSession {
params: NewCredentialParams,
abortController?: AbortController
) => Promise<{ cipherId: string; userVerified: boolean }>;
ensureUnlockedVault: () => Promise<void>;
informExcludedCredential: (
existingCipherIds: string[],
abortController?: AbortController

View File

@@ -14,6 +14,7 @@ import {
Fido2UserInterfaceSession,
NewCredentialParams,
} from "../../abstractions/fido2/fido2-user-interface.service.abstraction";
import { SyncService } from "../../abstractions/sync/sync.service.abstraction";
import { CipherType } from "../../enums/cipher-type";
import { Cipher } from "../../models/domain/cipher";
import { CipherView } from "../../models/view/cipher.view";
@@ -31,6 +32,7 @@ describe("FidoAuthenticatorService", () => {
let cipherService!: MockProxy<CipherService>;
let userInterface!: MockProxy<Fido2UserInterfaceService>;
let userInterfaceSession!: MockProxy<Fido2UserInterfaceSession>;
let syncService!: MockProxy<SyncService>;
let authenticator!: Fido2AuthenticatorService;
beforeEach(async () => {
@@ -38,7 +40,8 @@ describe("FidoAuthenticatorService", () => {
userInterface = mock<Fido2UserInterfaceService>();
userInterfaceSession = mock<Fido2UserInterfaceSession>();
userInterface.newSession.mockResolvedValue(userInterfaceSession);
authenticator = new Fido2AuthenticatorService(cipherService, userInterface);
syncService = mock<SyncService>();
authenticator = new Fido2AuthenticatorService(cipherService, userInterface, syncService);
});
describe("makeCredential", () => {

View File

@@ -13,6 +13,7 @@ import {
PublicKeyCredentialDescriptor,
} from "../../abstractions/fido2/fido2-authenticator.service.abstraction";
import { Fido2UserInterfaceService } from "../../abstractions/fido2/fido2-user-interface.service.abstraction";
import { SyncService } from "../../abstractions/sync/sync.service.abstraction";
import { CipherType } from "../../enums/cipher-type";
import { CipherView } from "../../models/view/cipher.view";
import { Fido2KeyView } from "../../models/view/fido2-key.view";
@@ -37,6 +38,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
constructor(
private cipherService: CipherService,
private userInterface: Fido2UserInterfaceService,
private syncService: SyncService,
private logService?: LogService
) {}
async makeCredential(
@@ -81,6 +83,8 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown);
}
await userInterfaceSession.ensureUnlockedVault();
const existingCipherIds = await this.findExcludedCredentials(
params.excludeCredentialDescriptorList
);
@@ -173,7 +177,6 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
params.fallbackSupported,
abortController
);
try {
if (
params.requireUserVerification != undefined &&
@@ -188,6 +191,8 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
}
let cipherOptions: CipherView[];
await userInterfaceSession.ensureUnlockedVault();
if (params.allowCredentialDescriptorList?.length > 0) {
cipherOptions = await this.findCredentialsById(
params.allowCredentialDescriptorList,
@@ -293,6 +298,11 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
return [];
}
//ensure full sync has completed before getting the ciphers
if ((await this.syncService.getLastSync()) == null) {
await this.syncService.fullSync(false);
}
const ciphers = await this.cipherService.getAllDecrypted();
return ciphers
.filter(
@@ -323,6 +333,11 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
return [];
}
//ensure full sync has completed before getting the ciphers
if ((await this.syncService.getLastSync()) == null) {
await this.syncService.fullSync(false);
}
const ciphers = await this.cipherService.getAllDecrypted();
return ciphers.filter(
(cipher) =>
@@ -335,6 +350,11 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
}
private async findCredentialsByRp(rpId: string): Promise<CipherView[]> {
//ensure full sync has completed before getting the ciphers
if ((await this.syncService.getLastSync()) == null) {
await this.syncService.fullSync(false);
}
const ciphers = await this.cipherService.getAllDecrypted();
return ciphers.filter(
(cipher) =>