mirror of
https://github.com/bitwarden/browser
synced 2025-12-10 21:33:27 +00:00
fix(Multi-Account-Logout: [Auth/PM-19555] Fix multi account logout on lock screens not redirecting properly (#14630)
* PM-19555 - LogoutService - build abstraction, default, and extension service and register with service modules * PM-19555 - Lock Comp - use logoutService * PM-19555 - LoginDecryptionOptions - Use logout service which removed need for extension-login-decryption-options.service * PM-19555 - AccountSwitcher logic update - (1) Use logout service + redirect guard routing (2) Remove logout method from account switcher service (3) use new NewActiveUser type * PM-19555 - Extension - Acct Switcher comp - clean up TODOs * PM-19555 - Add TODOs for remaining tech debt * PM-19555 - Add tests for new logout services. * PM-19555 - Extension - LoginInitiated - show acct switcher b/c user is AuthN * PM-19555 - Add TODO to replace LogoutCallback with LogoutService * PM-19555 WIP * PM-19555 - Extension App Comp - account switching to account in TDE locked state works now. * PM-19555 - Extension App Comp - add docs * PM-19555 - Extension App Comp - add early return * PM-19555 - Desktop App Comp - add handling for TDE lock case to switch account logic. * PM-19555 - Extension - Account Component - if account unlocked go to vault * PM-19555 - Per PR feedback, clean up unnecessary nullish coalescing operator. * PM-19555 - Extension - AppComponent - fix everHadUserKey merge issue * PM-19555 - PR feedback - refactor switchAccount and locked message handling on browser & desktop to require user id. I audited all callsites for both to ensure this *shouldn't* error.
This commit is contained in:
@@ -42,6 +42,7 @@ import {
|
||||
AuthRequestServiceAbstraction,
|
||||
DefaultAuthRequestApiService,
|
||||
DefaultLoginSuccessHandlerService,
|
||||
DefaultLogoutService,
|
||||
InternalUserDecryptionOptionsServiceAbstraction,
|
||||
LoginApprovalComponentServiceAbstraction,
|
||||
LoginEmailService,
|
||||
@@ -50,6 +51,7 @@ import {
|
||||
LoginStrategyServiceAbstraction,
|
||||
LoginSuccessHandlerService,
|
||||
LogoutReason,
|
||||
LogoutService,
|
||||
PinService,
|
||||
PinServiceAbstraction,
|
||||
UserDecryptionOptionsService,
|
||||
@@ -405,6 +407,7 @@ const safeProviders: SafeProvider[] = [
|
||||
provide: STATE_FACTORY,
|
||||
useValue: new StateFactory(GlobalState, Account),
|
||||
}),
|
||||
// TODO: PM-21212 - Deprecate LogoutCallback in favor of LogoutService
|
||||
safeProvider({
|
||||
provide: LOGOUT_CALLBACK,
|
||||
useFactory:
|
||||
@@ -1540,6 +1543,11 @@ const safeProviders: SafeProvider[] = [
|
||||
useClass: MasterPasswordApiService,
|
||||
deps: [ApiServiceAbstraction, LogService],
|
||||
}),
|
||||
safeProvider({
|
||||
provide: LogoutService,
|
||||
useClass: DefaultLogoutService,
|
||||
deps: [MessagingServiceAbstraction],
|
||||
}),
|
||||
safeProvider({
|
||||
provide: DocumentLangSetter,
|
||||
useClass: DocumentLangSetter,
|
||||
|
||||
@@ -10,6 +10,7 @@ import { catchError, defer, firstValueFrom, from, map, of, switchMap, throwError
|
||||
import { JslibModule } from "@bitwarden/angular/jslib.module";
|
||||
import {
|
||||
LoginEmailServiceAbstraction,
|
||||
LogoutService,
|
||||
UserDecryptionOptions,
|
||||
UserDecryptionOptionsServiceAbstraction,
|
||||
} from "@bitwarden/auth/common";
|
||||
@@ -109,6 +110,7 @@ export class LoginDecryptionOptionsComponent implements OnInit {
|
||||
private toastService: ToastService,
|
||||
private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction,
|
||||
private validationService: ValidationService,
|
||||
private logoutService: LogoutService,
|
||||
) {
|
||||
this.clientType = this.platformUtilsService.getClientType();
|
||||
}
|
||||
@@ -156,19 +158,17 @@ export class LoginDecryptionOptionsComponent implements OnInit {
|
||||
}
|
||||
|
||||
private async handleMissingEmail() {
|
||||
// TODO: PM-15174 - the solution for this bug will allow us to show the toast on app re-init after
|
||||
// the user has been logged out and the process reload has occurred.
|
||||
this.toastService.showToast({
|
||||
variant: "error",
|
||||
title: null,
|
||||
message: this.i18nService.t("activeUserEmailNotFoundLoggingYouOut"),
|
||||
});
|
||||
|
||||
setTimeout(async () => {
|
||||
// We can't simply redirect to `/login` because the user is authed and the unauthGuard
|
||||
// will prevent navigation. We must logout the user first via messagingService, which
|
||||
// redirects to `/`, which will be handled by the redirectGuard to navigate the user to `/login`.
|
||||
// The timeout just gives the user a chance to see the error toast before process reload runs on logout.
|
||||
await this.loginDecryptionOptionsService.logOut();
|
||||
}, 5000);
|
||||
await this.logoutService.logout(this.activeAccountId);
|
||||
// navigate to root so redirect guard can properly route next active user or null user to correct page
|
||||
await this.router.navigate(["/"]);
|
||||
}
|
||||
|
||||
private observeAndPersistRememberDeviceValueChanges() {
|
||||
@@ -312,7 +312,9 @@ export class LoginDecryptionOptionsComponent implements OnInit {
|
||||
|
||||
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
|
||||
if (confirmed) {
|
||||
this.messagingService.send("logout", { userId: userId });
|
||||
await this.logoutService.logout(userId);
|
||||
// navigate to root so redirect guard can properly route next active user or null user to correct page
|
||||
await this.router.navigate(["/"]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,8 +3,4 @@ export abstract class LoginDecryptionOptionsService {
|
||||
* Handles client-specific logic that runs after a user was successfully created
|
||||
*/
|
||||
abstract handleCreateUserSuccess(): Promise<void | null>;
|
||||
/**
|
||||
* Logs the user out
|
||||
*/
|
||||
abstract logOut(): Promise<void>;
|
||||
}
|
||||
|
||||
@@ -6,3 +6,4 @@ export * from "./user-decryption-options.service.abstraction";
|
||||
export * from "./auth-request.service.abstraction";
|
||||
export * from "./login-approval-component.service.abstraction";
|
||||
export * from "./login-success-handler.service";
|
||||
export * from "./logout.service";
|
||||
|
||||
19
libs/auth/src/common/abstractions/logout.service.ts
Normal file
19
libs/auth/src/common/abstractions/logout.service.ts
Normal file
@@ -0,0 +1,19 @@
|
||||
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
|
||||
import { LogoutReason } from "../types";
|
||||
|
||||
export interface NewActiveUser {
|
||||
userId: UserId;
|
||||
authenticationStatus: AuthenticationStatus;
|
||||
}
|
||||
|
||||
export abstract class LogoutService {
|
||||
/**
|
||||
* Logs out the user.
|
||||
* @param userId The user id.
|
||||
* @param logoutReason The optional reason for logging out.
|
||||
* @returns The new active user or undefined if there isn't a new active account.
|
||||
*/
|
||||
abstract logout(userId: UserId, logoutReason?: LogoutReason): Promise<NewActiveUser | undefined>;
|
||||
}
|
||||
@@ -7,3 +7,4 @@ export * from "./auth-request/auth-request-api.service";
|
||||
export * from "./accounts/lock.service";
|
||||
export * from "./login-success-handler/default-login-success-handler.service";
|
||||
export * from "./sso-redirect/sso-url.service";
|
||||
export * from "./logout/default-logout.service";
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
import { MockProxy, mock } from "jest-mock-extended";
|
||||
|
||||
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
|
||||
import { LogoutService } from "../../abstractions";
|
||||
import { LogoutReason } from "../../types";
|
||||
|
||||
import { DefaultLogoutService } from "./default-logout.service";
|
||||
|
||||
describe("DefaultLogoutService", () => {
|
||||
let logoutService: LogoutService;
|
||||
let messagingService: MockProxy<MessagingService>;
|
||||
|
||||
beforeEach(() => {
|
||||
messagingService = mock<MessagingService>();
|
||||
logoutService = new DefaultLogoutService(messagingService);
|
||||
});
|
||||
|
||||
it("instantiates", () => {
|
||||
expect(logoutService).not.toBeFalsy();
|
||||
});
|
||||
|
||||
describe("logout", () => {
|
||||
it("sends logout message without a logout reason when not provided", async () => {
|
||||
const userId = "1" as UserId;
|
||||
|
||||
await logoutService.logout(userId);
|
||||
|
||||
expect(messagingService.send).toHaveBeenCalledWith("logout", { userId });
|
||||
});
|
||||
|
||||
it("sends logout message with a logout reason when provided", async () => {
|
||||
const userId = "1" as UserId;
|
||||
const logoutReason: LogoutReason = "vaultTimeout";
|
||||
await logoutService.logout(userId, logoutReason);
|
||||
expect(messagingService.send).toHaveBeenCalledWith("logout", { userId, logoutReason });
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,14 @@
|
||||
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
|
||||
import { LogoutService, NewActiveUser } from "../../abstractions/logout.service";
|
||||
import { LogoutReason } from "../../types";
|
||||
|
||||
export class DefaultLogoutService implements LogoutService {
|
||||
constructor(protected messagingService: MessagingService) {}
|
||||
async logout(userId: UserId, logoutReason?: LogoutReason): Promise<NewActiveUser | undefined> {
|
||||
this.messagingService.send("logout", { userId, logoutReason });
|
||||
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
@@ -89,9 +89,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
|
||||
) {
|
||||
this.supportsDeviceTrust$ = this.userDecryptionOptionsService.userDecryptionOptions$.pipe(
|
||||
map((options) => {
|
||||
// TODO: Eslint upgrade. Please resolve this since the ?? does nothing
|
||||
// eslint-disable-next-line no-constant-binary-expression
|
||||
return options?.trustedDeviceOption != null ?? false;
|
||||
return options?.trustedDeviceOption != null;
|
||||
}),
|
||||
);
|
||||
}
|
||||
@@ -99,9 +97,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
|
||||
supportsDeviceTrustByUserId$(userId: UserId): Observable<boolean> {
|
||||
return this.userDecryptionOptionsService.userDecryptionOptionsById$(userId).pipe(
|
||||
map((options) => {
|
||||
// TODO: Eslint upgrade. Please resolve this since the ?? does nothing
|
||||
// eslint-disable-next-line no-constant-binary-expression
|
||||
return options?.trustedDeviceOption != null ?? false;
|
||||
return options?.trustedDeviceOption != null;
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -17,7 +17,7 @@ import {
|
||||
|
||||
import { JslibModule } from "@bitwarden/angular/jslib.module";
|
||||
import { AnonLayoutWrapperDataService } from "@bitwarden/auth/angular";
|
||||
import { PinServiceAbstraction } from "@bitwarden/auth/common";
|
||||
import { LogoutService, PinServiceAbstraction } from "@bitwarden/auth/common";
|
||||
import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
|
||||
import { MasterPasswordPolicyOptions } from "@bitwarden/common/admin-console/models/domain/master-password-policy-options";
|
||||
import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
|
||||
@@ -156,6 +156,7 @@ export class LockComponent implements OnInit, OnDestroy {
|
||||
private userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService,
|
||||
|
||||
private biometricService: BiometricsService,
|
||||
private logoutService: LogoutService,
|
||||
|
||||
private lockComponentService: LockComponentService,
|
||||
private anonLayoutWrapperDataService: AnonLayoutWrapperDataService,
|
||||
@@ -353,7 +354,9 @@ export class LockComponent implements OnInit, OnDestroy {
|
||||
});
|
||||
|
||||
if (confirmed && this.activeAccount != null) {
|
||||
this.messagingService.send("logout", { userId: this.activeAccount.id });
|
||||
await this.logoutService.logout(this.activeAccount.id);
|
||||
// navigate to root so redirect guard can properly route next active user or null user to correct page
|
||||
await this.router.navigate(["/"]);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user