diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 8d81b34448..03890c4d27 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -599,6 +599,9 @@ "loggedOut": { "message": "Logged out" }, + "loggedOutDesc": { + "message": "You have been logged out of your account." + }, "loginExpired": { "message": "Your login session has expired." }, @@ -1744,6 +1747,12 @@ "ok": { "message": "Ok" }, + "errorRefreshingAccessToken":{ + "message": "Access Token Refresh Error" + }, + "errorRefreshingAccessTokenDesc":{ + "message": "No refresh token or API keys found. Please try logging out and logging back in." + }, "desktopSyncVerificationTitle": { "message": "Desktop sync verification" }, diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 274649ef13..63721466f6 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -9,6 +9,7 @@ import { AuthRequestService, LoginEmailServiceAbstraction, LoginEmailService, + LogoutReason, } from "@bitwarden/auth/common"; import { ApiService as ApiServiceAbstraction } from "@bitwarden/common/abstractions/api.service"; import { AuditService as AuditServiceAbstraction } from "@bitwarden/common/abstractions/audit.service"; @@ -375,8 +376,17 @@ export default class MainBackground { } }; - const logoutCallback = async (expired: boolean, userId?: UserId) => - await this.logout(expired, userId); + const logoutCallback = async (logoutReason: LogoutReason, userId?: UserId) => + await this.logout(logoutReason, userId); + + const refreshAccessTokenErrorCallback = () => { + // Send toast to popup + this.messagingService.send("showToast", { + type: "error", + title: this.i18nService.t("errorRefreshingAccessToken"), + message: this.i18nService.t("errorRefreshingAccessTokenDesc"), + }); + }; const isDev = process.env.ENV === "development"; this.logService = new ConsoleLogService(isDev); @@ -523,6 +533,7 @@ export default class MainBackground { this.keyGenerationService, this.encryptService, this.logService, + logoutCallback, ); const migrationRunner = new MigrationRunner( @@ -608,9 +619,12 @@ export default class MainBackground { this.platformUtilsService, this.environmentService, this.appIdService, + refreshAccessTokenErrorCallback, + this.logService, + (logoutReason: LogoutReason, userId?: UserId) => this.logout(logoutReason, userId), this.vaultTimeoutSettingsService, - (expired: boolean) => this.logout(expired), ); + this.domainSettingsService = new DefaultDomainSettingsService(this.stateProvider); this.fileUploadService = new FileUploadService(this.logService); this.cipherFileUploadService = new CipherFileUploadService( @@ -1283,7 +1297,7 @@ export default class MainBackground { } } - async logout(expired: boolean, userId?: UserId) { + async logout(logoutReason: LogoutReason, userId?: UserId) { const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe( map((a) => a?.id), @@ -1349,7 +1363,7 @@ export default class MainBackground { await logoutPromise; this.messagingService.send("doneLoggingOut", { - expired: expired, + logoutReason: logoutReason, userId: userBeingLoggedOut, }); diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 7e94e84ef5..b70a5564ed 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -2,6 +2,7 @@ import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit } from "@angula import { NavigationEnd, Router, RouterOutlet } from "@angular/router"; import { Subject, takeUntil, firstValueFrom, concatMap, filter, tap } from "rxjs"; +import { LogoutReason } from "@bitwarden/auth/common"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; @@ -10,7 +11,12 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { MessageListener } from "@bitwarden/common/platform/messaging"; import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; -import { DialogService, SimpleDialogOptions, ToastService } from "@bitwarden/components"; +import { + DialogService, + SimpleDialogOptions, + ToastOptions, + ToastService, +} from "@bitwarden/components"; import { BrowserApi } from "../platform/browser/browser-api"; import { BrowserStateService } from "../platform/services/abstractions/browser-state.service"; @@ -83,13 +89,10 @@ export class AppComponent implements OnInit, OnDestroy { .pipe( tap((msg: any) => { if (msg.command === "doneLoggingOut") { + // TODO: PM-8544 - why do we call logout in the popup after receiving the doneLoggingOut message? Hasn't this already completeted logout? this.authService.logOut(async () => { - if (msg.expired) { - this.toastService.showToast({ - variant: "warning", - title: this.i18nService.t("loggedOut"), - message: this.i18nService.t("loginExpired"), - }); + if (msg.logoutReason) { + await this.displayLogoutReason(msg.logoutReason); } }); this.changeDetectorRef.detectChanges(); @@ -233,4 +236,23 @@ export class AppComponent implements OnInit, OnDestroy { this.browserSendStateService.setBrowserSendTypeComponentState(null), ]); } + + // Displaying toasts isn't super useful on the popup due to the reloads we do. + // However, it is visible for a moment on the FF sidebar logout. + private async displayLogoutReason(logoutReason: LogoutReason) { + let toastOptions: ToastOptions; + switch (logoutReason) { + case "invalidSecurityStamp": + case "sessionExpired": { + toastOptions = { + variant: "warning", + title: this.i18nService.t("loggedOut"), + message: this.i18nService.t("loginExpired"), + }; + break; + } + } + + this.toastService.showToast(toastOptions); + } } diff --git a/apps/cli/src/platform/services/node-api.service.ts b/apps/cli/src/platform/services/node-api.service.ts index 4849aef151..c480d9d1af 100644 --- a/apps/cli/src/platform/services/node-api.service.ts +++ b/apps/cli/src/platform/services/node-api.service.ts @@ -6,6 +6,7 @@ import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vaul import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { ApiService } from "@bitwarden/common/services/api.service"; @@ -21,8 +22,10 @@ export class NodeApiService extends ApiService { platformUtilsService: PlatformUtilsService, environmentService: EnvironmentService, appIdService: AppIdService, + refreshAccessTokenErrorCallback: () => Promise, + logService: LogService, + logoutCallback: () => Promise, vaultTimeoutSettingsService: VaultTimeoutSettingsService, - logoutCallback: (expired: boolean) => Promise, customUserAgent: string = null, ) { super( @@ -30,8 +33,10 @@ export class NodeApiService extends ApiService { platformUtilsService, environmentService, appIdService, - vaultTimeoutSettingsService, + refreshAccessTokenErrorCallback, + logService, logoutCallback, + vaultTimeoutSettingsService, customUserAgent, ); } diff --git a/apps/cli/src/service-container.ts b/apps/cli/src/service-container.ts index 882791ef9c..53039e9147 100644 --- a/apps/cli/src/service-container.ts +++ b/apps/cli/src/service-container.ts @@ -255,6 +255,8 @@ export class ServiceContainer { p = path.join(process.env.HOME, ".config/Bitwarden CLI"); } + const logoutCallback = async () => await this.logout(); + this.platformUtilsService = new CliPlatformUtilsService(ClientType.Cli, packageJson); this.logService = new ConsoleLogService( this.platformUtilsService.isDev(), @@ -337,6 +339,7 @@ export class ServiceContainer { this.keyGenerationService, this.encryptService, this.logService, + logoutCallback, ); const migrationRunner = new MigrationRunner( @@ -421,13 +424,19 @@ export class ServiceContainer { VaultTimeoutStringType.Never, // default vault timeout ); + const refreshAccessTokenErrorCallback = () => { + throw new Error("Refresh Access token error"); + }; + this.apiService = new NodeApiService( this.tokenService, this.platformUtilsService, this.environmentService, this.appIdService, + refreshAccessTokenErrorCallback, + this.logService, + logoutCallback, this.vaultTimeoutSettingsService, - async (expired: boolean) => await this.logout(), customUserAgent, ); @@ -485,7 +494,7 @@ export class ServiceContainer { this.logService, this.organizationService, this.keyGenerationService, - async (expired: boolean) => await this.logout(), + logoutCallback, this.stateProvider, ); @@ -660,7 +669,7 @@ export class ServiceContainer { this.sendApiService, this.userDecryptionOptionsService, this.avatarService, - async (expired: boolean) => await this.logout(), + logoutCallback, this.billingAccountProfileStateService, this.tokenService, this.authService, diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 7feea649c3..561e9b2df9 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -1,3 +1,4 @@ +import { DialogRef } from "@angular/cdk/dialog"; import { Component, NgZone, @@ -13,6 +14,7 @@ import { filter, firstValueFrom, map, Subject, takeUntil, timeout } from "rxjs"; import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref"; import { ModalService } from "@bitwarden/angular/services/modal.service"; import { FingerprintDialogComponent } from "@bitwarden/auth/angular"; +import { LogoutReason } from "@bitwarden/auth/common"; import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service"; import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service"; import { SearchService } from "@bitwarden/common/abstractions/search.service"; @@ -48,7 +50,7 @@ import { CollectionService } from "@bitwarden/common/vault/abstractions/collecti import { InternalFolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { CipherType } from "@bitwarden/common/vault/enums"; -import { DialogService, ToastService } from "@bitwarden/components"; +import { DialogService, ToastOptions, ToastService } from "@bitwarden/components"; import { DeleteAccountComponent } from "../auth/delete-account.component"; import { LoginApprovalComponent } from "../auth/login/login-approval.component"; @@ -108,6 +110,7 @@ export class AppComponent implements OnInit, OnDestroy { private idleTimer: number = null; private isIdle = false; private activeUserId: UserId = null; + private activeSimpleDialog: DialogRef = null; private destroy$ = new Subject(); @@ -207,7 +210,7 @@ export class AppComponent implements OnInit, OnDestroy { break; case "logout": this.loading = message.userId == null || message.userId === this.activeUserId; - await this.logOut(!!message.expired, message.userId); + await this.logOut(message.logoutReason, message.userId); this.loading = false; break; case "lockVault": @@ -545,9 +548,73 @@ export class AppComponent implements OnInit, OnDestroy { this.messagingService.send("updateAppMenu", { updateRequest: updateRequest }); } + private async displayLogoutReason(logoutReason: LogoutReason) { + let toastOptions: ToastOptions; + + switch (logoutReason) { + case "invalidSecurityStamp": + case "sessionExpired": { + toastOptions = { + variant: "warning", + title: this.i18nService.t("loggedOut"), + message: this.i18nService.t("loginExpired"), + }; + break; + } + // We don't expect these scenarios to be common, but we want the user to + // understand why they are being logged out before a process reload. + case "accessTokenUnableToBeDecrypted": { + // Don't create multiple dialogs if this fires multiple times + if (this.activeSimpleDialog) { + // Let the caller of this function listen for the dialog to close + return firstValueFrom(this.activeSimpleDialog.closed); + } + + this.activeSimpleDialog = this.dialogService.openSimpleDialogRef({ + title: { key: "loggedOut" }, + content: { key: "accessTokenUnableToBeDecrypted" }, + acceptButtonText: { key: "ok" }, + cancelButtonText: null, + type: "danger", + }); + + await firstValueFrom(this.activeSimpleDialog.closed); + this.activeSimpleDialog = null; + + break; + } + case "refreshTokenSecureStorageRetrievalFailure": { + // Don't create multiple dialogs if this fires multiple times + if (this.activeSimpleDialog) { + // Let the caller of this function listen for the dialog to close + return firstValueFrom(this.activeSimpleDialog.closed); + } + + this.activeSimpleDialog = this.dialogService.openSimpleDialogRef({ + title: { key: "loggedOut" }, + content: { key: "refreshTokenSecureStorageRetrievalFailure" }, + acceptButtonText: { key: "ok" }, + cancelButtonText: null, + type: "danger", + }); + + await firstValueFrom(this.activeSimpleDialog.closed); + this.activeSimpleDialog = null; + + break; + } + } + + if (toastOptions) { + this.toastService.showToast(toastOptions); + } + } + // Even though the userId parameter is no longer optional doesn't mean a message couldn't be // passing null-ish values to us. - private async logOut(expired: boolean, userId: UserId) { + private async logOut(logoutReason: LogoutReason, userId: UserId) { + await this.displayLogoutReason(logoutReason); + const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(map((a) => a?.id)), ); @@ -620,15 +687,7 @@ export class AppComponent implements OnInit, OnDestroy { // This must come last otherwise the logout will prematurely trigger // a process reload before all the state service user data can be cleaned up if (userBeingLoggedOut === activeUserId) { - this.authService.logOut(async () => { - if (expired) { - this.platformUtilsService.showToast( - "warning", - this.i18nService.t("loggedOut"), - this.i18nService.t("loginExpired"), - ); - } - }); + this.authService.logOut(async () => {}); } } @@ -710,7 +769,7 @@ export class AppComponent implements OnInit, OnDestroy { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises options[1] === "logOut" - ? this.logOut(false, userId as UserId) + ? this.logOut("vaultTimeout", userId as UserId) : await this.vaultTimeoutService.lock(userId); } } diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index 8a91771da2..9a0a2b6a2c 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -743,6 +743,9 @@ "loggedOut": { "message": "Logged out" }, + "loggedOutDesc": { + "message": "You have been logged out of your account." + }, "loginExpired": { "message": "Your login session has expired." }, @@ -1212,6 +1215,12 @@ } } }, + "errorRefreshingAccessToken":{ + "message": "Access Token Refresh Error" + }, + "errorRefreshingAccessTokenDesc":{ + "message": "No refresh token or API keys found. Please try logging out and logging back in." + }, "help": { "message": "Help" }, @@ -2474,6 +2483,12 @@ "important": { "message": "Important:" }, + "accessTokenUnableToBeDecrypted": { + "message": "You have been logged out because your access token could not be decrypted. Please log in again to resolve this issue." + }, + "refreshTokenSecureStorageRetrievalFailure": { + "message": "You have been logged out because your refresh token could not be retrieved. Please log in again to resolve this issue." + }, "masterPasswordHint": { "message": "Your master password cannot be recovered if you forget it!" }, diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index d30d6ad821..59a306189a 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -3,6 +3,7 @@ import * as path from "path"; import { app } from "electron"; import { Subject, firstValueFrom } from "rxjs"; +import { LogoutReason } from "@bitwarden/auth/common"; import { TokenService as TokenServiceAbstraction } from "@bitwarden/common/auth/abstractions/token.service"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { TokenService } from "@bitwarden/common/auth/services/token.service"; @@ -31,6 +32,7 @@ import { DefaultSingleUserStateProvider } from "@bitwarden/common/platform/state import { DefaultStateProvider } from "@bitwarden/common/platform/state/implementations/default-state.provider"; import { StateEventRegistrarService } from "@bitwarden/common/platform/state/state-event-registrar.service"; import { MemoryStorageService as MemoryStorageServiceForStateProviders } from "@bitwarden/common/platform/state/storage/memory-storage.service"; +import { UserId } from "@bitwarden/common/types/guid"; /* eslint-enable import/no-restricted-paths */ import { DesktopAutofillSettingsService } from "./autofill/services/desktop-autofill-settings.service"; @@ -182,6 +184,7 @@ export class Main { this.keyGenerationService, this.encryptService, this.logService, + async (logoutReason: LogoutReason, userId?: UserId) => {}, ); this.migrationRunner = new MigrationRunner( @@ -207,11 +210,9 @@ export class Main { ); this.desktopSettingsService = new DesktopSettingsService(stateProvider); - const biometricStateService = new DefaultBiometricStateService(stateProvider); this.windowMain = new WindowMain( - this.stateService, biometricStateService, this.logService, this.storageService, diff --git a/apps/desktop/src/main/window.main.ts b/apps/desktop/src/main/window.main.ts index 64b4bc48d2..e82d16ee9f 100644 --- a/apps/desktop/src/main/window.main.ts +++ b/apps/desktop/src/main/window.main.ts @@ -6,7 +6,6 @@ import { app, BrowserWindow, ipcMain, nativeTheme, screen, session } from "elect import { firstValueFrom } from "rxjs"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; @@ -38,7 +37,6 @@ export class WindowMain { readonly defaultHeight = 600; constructor( - private stateService: StateService, private biometricStateService: BiometricStateService, private logService: LogService, private storageService: AbstractStorageService, diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 6c71309243..254f23eeb2 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -14,6 +14,7 @@ import { timer, } from "rxjs"; +import { LogoutReason } from "@bitwarden/auth/common"; import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service"; import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service"; import { SearchService } from "@bitwarden/common/abstractions/search.service"; @@ -40,7 +41,7 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service"; import { InternalFolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; -import { DialogService, ToastService } from "@bitwarden/components"; +import { DialogService, ToastOptions, ToastService } from "@bitwarden/components"; import { PolicyListService } from "./admin-console/core/policy-list.service"; import { @@ -148,7 +149,7 @@ export class AppComponent implements OnDestroy, OnInit { this.router.navigate(["/"]); break; case "logout": - await this.logOut(!!message.expired, message.redirect); + await this.logOut(message.logoutReason, message.redirect); break; case "lockVault": await this.vaultTimeoutService.lock(); @@ -278,7 +279,34 @@ export class AppComponent implements OnDestroy, OnInit { this.destroy$.complete(); } - private async logOut(expired: boolean, redirect = true) { + private async displayLogoutReason(logoutReason: LogoutReason) { + let toastOptions: ToastOptions; + switch (logoutReason) { + case "invalidSecurityStamp": + case "sessionExpired": { + toastOptions = { + variant: "warning", + title: this.i18nService.t("loggedOut"), + message: this.i18nService.t("loginExpired"), + }; + break; + } + default: { + toastOptions = { + variant: "info", + title: this.i18nService.t("loggedOut"), + message: this.i18nService.t("loggedOutDesc"), + }; + break; + } + } + + this.toastService.showToast(toastOptions); + } + + private async logOut(logoutReason: LogoutReason, redirect = true) { + await this.displayLogoutReason(logoutReason); + await this.eventUploadService.uploadEvents(); const userId = (await this.stateService.getUserId()) as UserId; @@ -308,14 +336,6 @@ export class AppComponent implements OnDestroy, OnInit { await this.searchService.clearIndex(); this.authService.logOut(async () => { - if (expired) { - this.platformUtilsService.showToast( - "warning", - this.i18nService.t("loggedOut"), - this.i18nService.t("loginExpired"), - ); - } - await this.stateService.clean({ userId: userId }); await this.accountService.clean(userId); diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 23d2dc7572..875de3980a 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -587,6 +587,9 @@ "loggedOut": { "message": "Logged out" }, + "loggedOutDesc": { + "message": "You have been logged out of your account." + }, "loginExpired": { "message": "Your login session has expired." }, @@ -1050,6 +1053,12 @@ "copyUuid": { "message": "Copy UUID" }, + "errorRefreshingAccessToken":{ + "message": "Access Token Refresh Error" + }, + "errorRefreshingAccessTokenDesc":{ + "message": "No refresh token or API keys found. Please try logging out and logging back in." + }, "warning": { "message": "Warning" }, diff --git a/libs/angular/src/services/injection-tokens.ts b/libs/angular/src/services/injection-tokens.ts index 17a98498d6..40405b062c 100644 --- a/libs/angular/src/services/injection-tokens.ts +++ b/libs/angular/src/services/injection-tokens.ts @@ -1,6 +1,7 @@ import { InjectionToken } from "@angular/core"; import { Observable, Subject } from "rxjs"; +import { LogoutReason } from "@bitwarden/auth/common"; import { ClientType } from "@bitwarden/common/enums"; import { AbstractStorageService, @@ -36,7 +37,7 @@ export const MEMORY_STORAGE = new SafeInjectionToken("ME export const SECURE_STORAGE = new SafeInjectionToken("SECURE_STORAGE"); export const STATE_FACTORY = new SafeInjectionToken("STATE_FACTORY"); export const LOGOUT_CALLBACK = new SafeInjectionToken< - (expired: boolean, userId?: string) => Promise + (logoutReason: LogoutReason, userId?: string) => Promise >("LOGOUT_CALLBACK"); export const LOCKED_CALLBACK = new SafeInjectionToken<(userId?: string) => Promise>( "LOCKED_CALLBACK", @@ -53,3 +54,7 @@ export const INTRAPROCESS_MESSAGING_SUBJECT = new SafeInjectionToken< Subject>> >("INTRAPROCESS_MESSAGING_SUBJECT"); export const CLIENT_TYPE = new SafeInjectionToken("CLIENT_TYPE"); + +export const REFRESH_ACCESS_TOKEN_ERROR_CALLBACK = new SafeInjectionToken<() => void>( + "REFRESH_ACCESS_TOKEN_ERROR_CALLBACK", +); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 5a5b83522d..048c182900 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -13,6 +13,7 @@ import { InternalUserDecryptionOptionsServiceAbstraction, UserDecryptionOptionsService, UserDecryptionOptionsServiceAbstraction, + LogoutReason, } from "@bitwarden/auth/common"; import { ApiService as ApiServiceAbstraction } from "@bitwarden/common/abstractions/api.service"; import { AuditService as AuditServiceAbstraction } from "@bitwarden/common/abstractions/audit.service"; @@ -238,6 +239,7 @@ import { SyncNotifierService } from "@bitwarden/common/vault/services/sync/sync- import { SyncService } from "@bitwarden/common/vault/services/sync/sync.service"; import { TotpService } from "@bitwarden/common/vault/services/totp.service"; import { VaultSettingsService } from "@bitwarden/common/vault/services/vault-settings/vault-settings.service"; +import { ToastService } from "@bitwarden/components"; import { ImportApiService, ImportApiServiceAbstraction, @@ -281,6 +283,7 @@ import { DEFAULT_VAULT_TIMEOUT, INTRAPROCESS_MESSAGING_SUBJECT, CLIENT_TYPE, + REFRESH_ACCESS_TOKEN_ERROR_CALLBACK, } from "./injection-tokens"; import { ModalService } from "./modal.service"; @@ -322,8 +325,12 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: LOGOUT_CALLBACK, useFactory: - (messagingService: MessagingServiceAbstraction) => (expired: boolean, userId?: string) => - Promise.resolve(messagingService.send("logout", { expired: expired, userId: userId })), + (messagingService: MessagingServiceAbstraction) => + async (logoutReason: LogoutReason, userId?: string) => { + return Promise.resolve( + messagingService.send("logout", { logoutReason: logoutReason, userId: userId }), + ); + }, deps: [MessagingServiceAbstraction], }), safeProvider({ @@ -532,6 +539,7 @@ const safeProviders: SafeProvider[] = [ KeyGenerationServiceAbstraction, EncryptService, LogService, + LOGOUT_CALLBACK, ], }), safeProvider({ @@ -585,6 +593,17 @@ const safeProviders: SafeProvider[] = [ StateProvider, ], }), + safeProvider({ + provide: REFRESH_ACCESS_TOKEN_ERROR_CALLBACK, + useFactory: (toastService: ToastService, i18nService: I18nServiceAbstraction) => () => { + toastService.showToast({ + variant: "error", + title: i18nService.t("errorRefreshingAccessToken"), + message: i18nService.t("errorRefreshingAccessTokenDesc"), + }); + }, + deps: [ToastService, I18nServiceAbstraction], + }), safeProvider({ provide: ApiServiceAbstraction, useClass: ApiService, @@ -593,8 +612,10 @@ const safeProviders: SafeProvider[] = [ PlatformUtilsServiceAbstraction, EnvironmentService, AppIdServiceAbstraction, - VaultTimeoutSettingsServiceAbstraction, + REFRESH_ACCESS_TOKEN_ERROR_CALLBACK, + LogService, LOGOUT_CALLBACK, + VaultTimeoutSettingsServiceAbstraction, ], }), safeProvider({ diff --git a/libs/auth/src/common/index.ts b/libs/auth/src/common/index.ts index 936666e1a8..43efd7c638 100644 --- a/libs/auth/src/common/index.ts +++ b/libs/auth/src/common/index.ts @@ -3,5 +3,6 @@ */ export * from "./abstractions"; export * from "./models"; +export * from "./types"; export * from "./services"; export * from "./utilities"; diff --git a/libs/auth/src/common/types/index.ts b/libs/auth/src/common/types/index.ts new file mode 100644 index 0000000000..37ec426fb6 --- /dev/null +++ b/libs/auth/src/common/types/index.ts @@ -0,0 +1 @@ +export * from "./logout-reason.type"; diff --git a/libs/auth/src/common/types/logout-reason.type.ts b/libs/auth/src/common/types/logout-reason.type.ts new file mode 100644 index 0000000000..71fff51064 --- /dev/null +++ b/libs/auth/src/common/types/logout-reason.type.ts @@ -0,0 +1,10 @@ +export type LogoutReason = + | "invalidGrantError" + | "vaultTimeout" + | "invalidSecurityStamp" + | "logoutNotification" + | "keyConnectorError" + | "sessionExpired" + | "accessTokenUnableToBeDecrypted" + | "refreshTokenSecureStorageRetrievalFailure" + | "accountDeleted"; diff --git a/libs/common/src/auth/abstractions/token.service.ts b/libs/common/src/auth/abstractions/token.service.ts index d078051f64..a88dfbb278 100644 --- a/libs/common/src/auth/abstractions/token.service.ts +++ b/libs/common/src/auth/abstractions/token.service.ts @@ -70,16 +70,16 @@ export abstract class TokenService { /** * Gets the access token * @param userId - The optional user id to get the access token for; if not provided, the active user is used. - * @returns A promise that resolves with the access token or undefined. + * @returns A promise that resolves with the access token or null. */ - getAccessToken: (userId?: UserId) => Promise; + getAccessToken: (userId?: UserId) => Promise; /** * Gets the refresh token. * @param userId - The optional user id to get the refresh token for; if not provided, the active user is used. - * @returns A promise that resolves with the refresh token or undefined. + * @returns A promise that resolves with the refresh token or null. */ - getRefreshToken: (userId?: UserId) => Promise; + getRefreshToken: (userId?: UserId) => Promise; /** * Sets the API Key Client ID for the active user id in memory or disk based on the given vaultTimeoutAction and vaultTimeout. diff --git a/libs/common/src/auth/services/key-connector.service.ts b/libs/common/src/auth/services/key-connector.service.ts index 65d1030bd3..6b81844afb 100644 --- a/libs/common/src/auth/services/key-connector.service.ts +++ b/libs/common/src/auth/services/key-connector.service.ts @@ -1,5 +1,7 @@ import { firstValueFrom } from "rxjs"; +import { LogoutReason } from "@bitwarden/auth/common"; + import { ApiService } from "../../abstractions/api.service"; import { OrganizationService } from "../../admin-console/abstractions/organization/organization.service.abstraction"; import { OrganizationUserType } from "../../admin-console/enums"; @@ -57,7 +59,7 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { private logService: LogService, private organizationService: OrganizationService, private keyGenerationService: KeyGenerationService, - private logoutCallback: (expired: boolean, userId?: string) => Promise, + private logoutCallback: (logoutReason: LogoutReason, userId?: string) => Promise, private stateProvider: StateProvider, ) { this.usesKeyConnectorState = this.stateProvider.getActive(USES_KEY_CONNECTOR); @@ -192,7 +194,7 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { if (this.logoutCallback != null) { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.logoutCallback(false); + this.logoutCallback("keyConnectorError"); } throw new Error("Key Connector error"); } diff --git a/libs/common/src/auth/services/token.service.spec.ts b/libs/common/src/auth/services/token.service.spec.ts index 9c5dd9fc91..d7a4c52716 100644 --- a/libs/common/src/auth/services/token.service.spec.ts +++ b/libs/common/src/auth/services/token.service.spec.ts @@ -1,6 +1,8 @@ import { MockProxy, mock } from "jest-mock-extended"; import { firstValueFrom } from "rxjs"; +import { LogoutReason } from "@bitwarden/auth/common"; + import { FakeSingleUserStateProvider, FakeGlobalStateProvider } from "../../../spec"; import { VaultTimeoutAction } from "../../enums/vault-timeout-action.enum"; import { EncryptService } from "../../platform/abstractions/encrypt.service"; @@ -9,11 +11,18 @@ import { LogService } from "../../platform/abstractions/log.service"; import { AbstractStorageService } from "../../platform/abstractions/storage.service"; import { StorageLocation } from "../../platform/enums"; import { StorageOptions } from "../../platform/models/domain/storage-options"; +import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypto-key"; +import { CsprngArray } from "../../types/csprng"; import { UserId } from "../../types/guid"; import { VaultTimeout, VaultTimeoutStringType } from "../../types/vault-timeout.type"; import { ACCOUNT_ACTIVE_ACCOUNT_ID } from "./account.service"; -import { DecodedAccessToken, TokenService, TokenStorageLocation } from "./token.service"; +import { + AccessTokenKey, + DecodedAccessToken, + TokenService, + TokenStorageLocation, +} from "./token.service"; import { ACCESS_TOKEN_DISK, ACCESS_TOKEN_MEMORY, @@ -36,6 +45,7 @@ describe("TokenService", () => { let keyGenerationService: MockProxy; let encryptService: MockProxy; let logService: MockProxy; + let logoutCallback: jest.Mock, [logoutReason: LogoutReason, userId?: string]>; const memoryVaultTimeoutAction = VaultTimeoutAction.LogOut; const memoryVaultTimeout: VaultTimeout = 30; @@ -46,6 +56,9 @@ describe("TokenService", () => { const accessTokenJwt = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJodHRwOi8vbG9jYWxob3N0IiwibmJmIjoxNzA5MzI0MTExLCJpYXQiOjE3MDkzMjQxMTEsImV4cCI6MTcwOTMyNzcxMSwic2NvcGUiOlsiYXBpIiwib2ZmbGluZV9hY2Nlc3MiXSwiYW1yIjpbIkFwcGxpY2F0aW9uIl0sImNsaWVudF9pZCI6IndlYiIsInN1YiI6ImVjZTcwYTEzLTcyMTYtNDNjNC05OTc3LWIxMDMwMTQ2ZTFlNyIsImF1dGhfdGltZSI6MTcwOTMyNDEwNCwiaWRwIjoiYml0d2FyZGVuIiwicHJlbWl1bSI6ZmFsc2UsImVtYWlsIjoiZXhhbXBsZUBiaXR3YXJkZW4uY29tIiwiZW1haWxfdmVyaWZpZWQiOmZhbHNlLCJzc3RhbXAiOiJHWTdKQU82NENLS1RLQkI2WkVBVVlMMldPUVU3QVNUMiIsIm5hbWUiOiJUZXN0IFVzZXIiLCJvcmdvd25lciI6WyI5MmI0OTkwOC1iNTE0LTQ1YTgtYmFkYi1iMTAzMDE0OGZlNTMiLCIzOGVkZTMyMi1iNGI0LTRiZDgtOWUwOS1iMTA3MDExMmRjMTEiLCJiMmQwNzAyOC1hNTgzLTRjM2UtOGQ2MC1iMTA3MDExOThjMjkiLCJiZjkzNGJhMi0wZmQ0LTQ5ZjItYTk1ZS1iMTA3MDExZmM5ZTYiLCJjMGI3Zjc1ZC0wMTVmLTQyYzktYjNhNi1iMTA4MDE3NjA3Y2EiXSwiZGV2aWNlIjoiNGI4NzIzNjctMGRhNi00MWEwLWFkY2ItNzdmMmZlZWZjNGY0IiwianRpIjoiNzUxNjFCRTQxMzFGRjVBMkRFNTExQjhDNEUyRkY4OUEifQ.n7roP8sSbfwcYdvRxZNZds27IK32TW6anorE6BORx_Q"; + const encryptedAccessToken = + "2.rFNYSTJoljn8h6GOSNVYdQ==|4dIp7ONJzC+Kx1ClA+1aIAb7EqCQ4OjnADCYdCPg7BKkdheG+yM62ZiONFk+S6at84M+RnGWWO04aIjinTdJhlhyUmszePNATxIfX60Y+bFKQhlMuCtZpYdEmQDzXVgT43YRbf/6NnN9WzhefLqeMiocwoIJTEpLptb+Zcm7T3MJpkX4dR9w5LUOxUTNFEGd5PlWaI8FBavOkNsrzY5skRK70pvFABET5IDeRlKhi8NwbzvTzkO3SisLRzih+djiz5nEZf0+ujeGAp6P+o7l0mB0sXVsNJzcuE4S9QtHLnx31N6z3mQm5pOgP4EmEOdRIcQGc1p7dL1vXcXtaTJLtfKXoJjJbYT3wplnY9Pf8+2FVxdbM3bRB2yVsnEzgLcf9UchKThQSdOy8+5TO/prDbUt5mDpO4GmRltom5ncda8yJaD3Hw1DO7fa0Xh+kfeByxb1AwBC+GTPfqmo5uqr0J4dZsf9cGlPMTElwR3GYmD60OcQ6iDX36CZZjqqJqBwKSpepDXV39p9G347e6YAAvJenLDKtdjgfWXCMXbkwETbMgYooFDRd60KYsGIXV16UwzJSvczgTY2d+hYb2Cl0lClequaiwcRxLVtW2xau6qoEPjTqJjJi9I0Cs2WNL4LRH96Ir14a3bEtnTvkO1NjN+bQNon+KksaP2BqTbuiAfZbBP/cL4S1Oew4G00PSLZUGV5S1BI0ooJy6e2NLQJlYqfCeKM6RgpvgfOiXlZddVgkkB6lohLjyVvcSZNuKPjs1wZMZ9C76bKb6o39NFK8G3/YScELFf9gkueWjmhcjrs22+xNDn5rxXeedwIkVW9UJVNLc//eGxLfp70y8fNDcyTPRN1UUpqT8+wSz+9ZHl4DLUK0DE2jIveEDke8vi4MK/XLMC/c50rr1NCEuVy6iA3nwiOzVo/GNfeKTpzMcR/D9A0gxkC9GyZ3riSsMQsGNXhZCZLdsFYp0gLiiJxVilMUfyTWaygsNm87GPY3ep3GEHcq/pCuxrpLQQYT3V1j95WJvFxb8dSLiPHb8STR0GOZhe7SquI5LIRmYCFTo+3VBnItYeuin9i2xCIqWz886xIyllHN2BIPILbA1lCOsCsz1BRRGNqtLvmTeVRO8iujsHWBJicVgSI7/dgSJwcdOv2t4TIVtnN1hJkQnz+HZcJ2FYK/VWlo4UQYYoML52sBd1sSz/n8/8hrO2N4X9frHHNCrcxeoyChTKo2cm4rAxHylLbCZYvGt/KIW9x3AFkPBMr7tAc3yq98J0Crna8ukXc3F3uGb5QXLnBi//3zBDN6RCv7ByaFW5G0I+pglBegzeFBqKH8xwfy76B2e2VLFF8rz/r/wQzlumGFypsRhAoGxrkZyzjec/k+RNR0arf7TTX7ymC1cueTnItRDx89veW6WLlF53NpAGqC8GJSp4T2FGIIk01y29j6Ji7GOlQ8BUbyLWYjMfHf3khRzAfr6UC2QgVvKWQTKET4Y/b1nZCnwxeW8wC80GHtYGuarsU+KlsEw4242cjyIN1GobrWaA2GTOedQDEMWUA64McAw5fAvMEEao5DM7i57tMzJHeKfruyMuXYQkBca094vmATjJ/T+kIrWGIcmxCT/Fp2SW1hcxr6Ciwuog84LVfbVlUl2MAj3eC/xqL/5HP6Q3ObD0ld444GV+HSrQUqfIvEIn9gFmalW6TGugyhfROACCogoXbeIr1AyMUNDnl4EWlPl6u7SQvPX+itKyq4qhaK2J0W6f7ElLVQ5GbC2uwARuhXOi7mqEZ5FP0V675C5NPZOl2ZEd6BhmuyhGkmQEtEvw0DCKnbKM7bKMk90Y599DSnuEna4BNFBVjJ7k+BuNhXUKO+iNcDZT0pCQhOKRVLWsaqVff3BsuQ4zMEOVnccJwwAVipwSRyxZi8bF+Wyun6BVI8pz1CBvRMy+6ifmIq2awEL8NnV65hF2jyZDEVwsnrvCyT7MlM8l5C3MhqH/MgMcKqOsUz+P6Jv5sBi4WvojsaHzqxQ6miBHpHhGDpYH5K53LVs36henB/tOUTcg5ZnO4ZM67jjB7Oz7to+QnJsldp5Bdwvi1XD/4jeh/Llezu5/KwwytSHnZG1z6dZA7B8rKwnI+yN2Qnfi70h68jzGZ1xCOFPz9KMorNKP3XLw8x2g9H6lEBXdV95uc/TNw+WTJbvKRawns/DZhM1u/g13lU6JG19cht3dh/DlKRcJpj1AdOAxPiUubTSkhBmdwRj2BTTHrVlF3/9ladTP4s4f6Zj9TtQvR9CREVe7CboGflxDYC+Jww3PU50XLmxQjkuV5MkDAmBVcyFCFOcHhDRoxet4FX9ec0wjNeDpYtkI8B/qUS1Rp+is1jOxr4/ni|pabwMkF/SdYKdDlow4uKxaObrAP0urmv7N7fA9bedec="; + const accessTokenDecoded: DecodedAccessToken = { iss: "http://localhost", nbf: 1709324111, @@ -93,6 +106,7 @@ describe("TokenService", () => { keyGenerationService = mock(); encryptService = mock(); logService = mock(); + logoutCallback = jest.fn(); const supportsSecureStorage = false; // default to false; tests will override as needed tokenService = createTokenService(supportsSecureStorage); @@ -152,7 +166,7 @@ describe("TokenService", () => { expect(result).toEqual(true); }); - it("should return false if no access token exists in memory, disk, or secure storage", async () => { + it("returns false when no access token exists in memory, disk, or secure storage", async () => { // Act const result = await firstValueFrom(tokenService.hasAccessToken$(userIdFromAccessToken)); @@ -162,7 +176,7 @@ describe("TokenService", () => { }); describe("setAccessToken", () => { - it("should throw an error if the access token is null", async () => { + it("throws an error when the access token is null", async () => { // Act const result = tokenService.setAccessToken( null, @@ -173,7 +187,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("Access token is required."); }); - it("should throw an error if an invalid token is passed in", async () => { + it("throws an error when an invalid token is passed in", async () => { // Act const result = tokenService.setAccessToken( "invalidToken", @@ -216,7 +230,7 @@ describe("TokenService", () => { }); describe("Memory storage tests", () => { - it("should set the access token in memory", async () => { + it("set the access token in memory", async () => { // Act await tokenService.setAccessToken( accessTokenJwt, @@ -246,6 +260,14 @@ describe("TokenService", () => { }); describe("Disk storage tests (secure storage supported on platform)", () => { + const accessTokenKey = new SymmetricCryptoKey( + new Uint8Array(64) as CsprngArray, + ) as AccessTokenKey; + + const accessTokenKeyB64 = { + keyB64: + "lI7lSoejJ1HsrTkRs2Ipm0x+YcZMKpgm7WQGCNjAWmFAyGOKossXwBJvvtbxcYDZ0G0XNY8Gp7DBXZV2tWAO5w==", + }; beforeEach(() => { const supportsSecureStorage = true; tokenService = createTokenService(supportsSecureStorage); @@ -259,7 +281,7 @@ describe("TokenService", () => { .getFake(userIdFromAccessToken, ACCESS_TOKEN_MEMORY) .stateSubject.next([userIdFromAccessToken, accessTokenJwt]); - keyGenerationService.createKey.mockResolvedValue("accessTokenKey" as any); + keyGenerationService.createKey.mockResolvedValue(accessTokenKey); const mockEncryptedAccessToken = "encryptedAccessToken"; @@ -267,6 +289,11 @@ describe("TokenService", () => { encryptedString: mockEncryptedAccessToken, } as any); + // First call resolves to null to simulate no key in secure storage + // then resolves to the key to simulate the key being set in secure storage + // and retrieved successfully to ensure it was set. + secureStorageService.get.mockResolvedValueOnce(null).mockResolvedValue(accessTokenKeyB64); + // Act await tokenService.setAccessToken( accessTokenJwt, @@ -278,7 +305,7 @@ describe("TokenService", () => { // assert that the AccessTokenKey was set in secure storage expect(secureStorageService.save).toHaveBeenCalledWith( accessTokenKeySecureStorageKey, - "accessTokenKey", + accessTokenKey, secureStorageOptions, ); @@ -292,18 +319,85 @@ describe("TokenService", () => { singleUserStateProvider.getFake(userIdFromAccessToken, ACCESS_TOKEN_MEMORY).nextMock, ).toHaveBeenCalledWith(null); }); + + it("should fallback to disk storage for the access token if the access token cannot be set in secure storage", async () => { + // This tests the scenario where the access token key silently fails to be set in secure storage + + // Arrange: + keyGenerationService.createKey.mockResolvedValue(accessTokenKey); + + // First call resolves to null to simulate no key in secure storage + // and then resolves to no key after it should have been set + secureStorageService.get.mockResolvedValueOnce(null).mockResolvedValue(null); + + // Act + await tokenService.setAccessToken( + accessTokenJwt, + diskVaultTimeoutAction, + diskVaultTimeout, + ); + // Assert + + // assert that we tried to store the AccessTokenKey in secure storage + expect(secureStorageService.save).toHaveBeenCalledWith( + accessTokenKeySecureStorageKey, + accessTokenKey, + secureStorageOptions, + ); + + // assert that we logged the error + expect(logService.error).toHaveBeenCalledWith( + "SetAccessToken: storing encrypted access token in secure storage failed. Falling back to disk storage.", + new Error("New Access token key unable to be retrieved from secure storage."), + ); + + // assert that the access token was put on disk unencrypted + expect( + singleUserStateProvider.getFake(userIdFromAccessToken, ACCESS_TOKEN_DISK).nextMock, + ).toHaveBeenCalledWith(accessTokenJwt); + }); + + it("should fallback to disk storage for the access token if secure storage errors on trying to get an existing access token key", async () => { + // This tests the scenario for linux users who don't have secure storage configured. + + // Arrange: + keyGenerationService.createKey.mockResolvedValue(accessTokenKey); + + // Mock linux secure storage error + const secureStorageError = "Secure storage error"; + secureStorageService.get.mockRejectedValue(new Error(secureStorageError)); + + // Act + await tokenService.setAccessToken( + accessTokenJwt, + diskVaultTimeoutAction, + diskVaultTimeout, + ); + // Assert + + // assert that we logged the error + expect(logService.error).toHaveBeenCalledWith( + "SetAccessToken: storing encrypted access token in secure storage failed. Falling back to disk storage.", + new Error(secureStorageError), + ); + + // assert that the access token was put on disk unencrypted + expect( + singleUserStateProvider.getFake(userIdFromAccessToken, ACCESS_TOKEN_DISK).nextMock, + ).toHaveBeenCalledWith(accessTokenJwt); + }); }); }); describe("getAccessToken", () => { - it("should return undefined if no user id is provided and there is no active user in global state", async () => { + it("returns null when no user id is provided and there is no active user in global state", async () => { // Act const result = await tokenService.getAccessToken(); // Assert - expect(result).toBeUndefined(); + expect(result).toBeNull(); }); - it("should return null if no access token is found in memory, disk, or secure storage", async () => { + it("returns null when no access token is found in memory, disk, or secure storage", async () => { // Arrange globalStateProvider .getFake(ACCOUNT_ACTIVE_ACCOUNT_ID) @@ -317,11 +411,8 @@ describe("TokenService", () => { describe("Memory storage tests", () => { test.each([ - [ - "should get the access token from memory for the provided user id", - userIdFromAccessToken, - ], - ["should get the access token from memory with no user id provided", undefined], + ["gets the access token from memory when a user id is provided ", userIdFromAccessToken], + ["gets the access token from memory when no user id is provided", undefined], ])("%s", async (_, userId) => { // Arrange singleUserStateProvider @@ -350,11 +441,8 @@ describe("TokenService", () => { describe("Disk storage tests (secure storage not supported on platform)", () => { test.each([ - [ - "should get the access token from disk for the specified user id", - userIdFromAccessToken, - ], - ["should get the access token from disk with no user id specified", undefined], + ["gets the access token from disk when the user id is specified", userIdFromAccessToken], + ["gets the access token from disk when no user id is specified", undefined], ])("%s", async (_, userId) => { // Arrange singleUserStateProvider @@ -387,11 +475,11 @@ describe("TokenService", () => { test.each([ [ - "should get the encrypted access token from disk, decrypt it, and return it when user id is provided", + "gets the encrypted access token from disk, decrypts it, and returns it when a user id is provided", userIdFromAccessToken, ], [ - "should get the encrypted access token from disk, decrypt it, and return it when no user id is provided", + "gets the encrypted access token from disk, decrypts it, and returns it when no user id is provided", undefined, ], ])("%s", async (_, userId) => { @@ -423,11 +511,11 @@ describe("TokenService", () => { test.each([ [ - "should fallback and get the unencrypted access token from disk when there isn't an access token key in secure storage and a user id is provided", + "falls back and gets the unencrypted access token from disk when there isn't an access token key in secure storage and a user id is provided", userIdFromAccessToken, ], [ - "should fallback and get the unencrypted access token from disk when there isn't an access token key in secure storage and no user id is provided", + "falls back and gets the unencrypted access token from disk when there isn't an access token key in secure storage and no user id is provided", undefined, ], ])("%s", async (_, userId) => { @@ -455,11 +543,80 @@ describe("TokenService", () => { // Assert expect(result).toEqual(accessTokenJwt); }); + + it("logs the error and logs the user out when the access token key cannot be retrieved from secure storage if the access token is encrypted", async () => { + // This tests the intermittent windows 10/11 scenario in which the access token key was stored successfully in secure storage and the + // access token was encrypted with it and stored on disk successfully. However, on retrieval the access token key isn't able to + // retrieved for whatever reason. + + // Arrange + singleUserStateProvider + .getFake(userIdFromAccessToken, ACCESS_TOKEN_MEMORY) + .stateSubject.next([userIdFromAccessToken, undefined]); + + singleUserStateProvider + .getFake(userIdFromAccessToken, ACCESS_TOKEN_DISK) + .stateSubject.next([userIdFromAccessToken, encryptedAccessToken]); + + // No access token key set + + // Act + const result = await tokenService.getAccessToken(userIdFromAccessToken); + + // Assert + expect(result).toBeNull(); + + // assert that we logged the error + expect(logService.error).toHaveBeenCalledWith( + "Access token key not found to decrypt encrypted access token. Logging user out.", + ); + + // assert that we logged the user out + expect(logoutCallback).toHaveBeenCalledWith( + "accessTokenUnableToBeDecrypted", + userIdFromAccessToken, + ); + }); + + it("logs the error and logs the user out when secure storage errors on trying to get an access token key", async () => { + // This tests the linux scenario where users might not have secure storage support configured. + + // Arrange + singleUserStateProvider + .getFake(userIdFromAccessToken, ACCESS_TOKEN_MEMORY) + .stateSubject.next([userIdFromAccessToken, undefined]); + + singleUserStateProvider + .getFake(userIdFromAccessToken, ACCESS_TOKEN_DISK) + .stateSubject.next([userIdFromAccessToken, encryptedAccessToken]); + + // Mock linux secure storage error + const secureStorageError = "Secure storage error"; + secureStorageService.get.mockRejectedValue(new Error(secureStorageError)); + + // Act + const result = await tokenService.getAccessToken(userIdFromAccessToken); + + // Assert + expect(result).toBeNull(); + + // assert that we logged the error + expect(logService.error).toHaveBeenCalledWith( + "Access token key retrieval failed. Unable to decrypt encrypted access token. Logging user out.", + new Error(secureStorageError), + ); + + // assert that we logged the user out + expect(logoutCallback).toHaveBeenCalledWith( + "accessTokenUnableToBeDecrypted", + userIdFromAccessToken, + ); + }); }); }); describe("clearAccessToken", () => { - it("should throw an error if no user id is provided and there is no active user in global state", async () => { + it("throws an error when no user id is provided and there is no active user in global state", async () => { // Act // note: don't await here because we want to test the error const result = tokenService.clearAccessToken(); @@ -475,11 +632,11 @@ describe("TokenService", () => { test.each([ [ - "should clear the access token from all storage locations for the provided user id", + "clears the access token from all storage locations when a user id is provided", userIdFromAccessToken, ], [ - "should clear the access token from all storage locations for the global active user", + "clears the access token from all storage locations when there is a global active user", undefined, ], ])("%s", async (_, userId) => { @@ -519,7 +676,7 @@ describe("TokenService", () => { }); describe("decodeAccessToken", () => { - it("should throw an error if no access token provided or retrieved from state", async () => { + it("throws an error when no access token is provided or retrievable from state", async () => { // Access tokenService.getAccessToken = jest.fn().mockResolvedValue(null); @@ -530,7 +687,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("Access token not found."); }); - it("should decode the access token", async () => { + it("decodes the access token when a valid one is stored", async () => { // Arrange tokenService.getAccessToken = jest.fn().mockResolvedValue(accessTokenJwt); @@ -544,7 +701,7 @@ describe("TokenService", () => { describe("Data methods", () => { describe("getTokenExpirationDate", () => { - it("should throw an error if the access token cannot be decoded", async () => { + it("throws an error when the access token cannot be decoded", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockRejectedValue(new Error("Mock error")); @@ -555,7 +712,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("Failed to decode access token: Mock error"); }); - it("should return null if the decoded access token is null", async () => { + it("returns null when the decoded access token is null", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(null); @@ -566,7 +723,7 @@ describe("TokenService", () => { expect(result).toBeNull(); }); - it("should return null if the decoded access token does not have an expiration date", async () => { + it("returns null when the decoded access token does not have an expiration date", async () => { // Arrange const accessTokenDecodedWithoutExp = { ...accessTokenDecoded }; delete accessTokenDecodedWithoutExp.exp; @@ -581,7 +738,7 @@ describe("TokenService", () => { expect(result).toBeNull(); }); - it("should return null if the decoded access token has an non numeric expiration date", async () => { + it("returns null when the decoded access token has a non numeric expiration date", async () => { // Arrange const accessTokenDecodedWithNonNumericExp = { ...accessTokenDecoded, exp: "non-numeric" }; tokenService.decodeAccessToken = jest @@ -595,7 +752,7 @@ describe("TokenService", () => { expect(result).toBeNull(); }); - it("should return the expiration date of the access token", async () => { + it("returns the expiration date of the access token when a valid access token is stored", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(accessTokenDecoded); @@ -608,7 +765,7 @@ describe("TokenService", () => { }); describe("tokenSecondsRemaining", () => { - it("should return 0 if the tokenExpirationDate is null", async () => { + it("returns 0 when the tokenExpirationDate is null", async () => { // Arrange tokenService.getTokenExpirationDate = jest.fn().mockResolvedValue(null); @@ -619,7 +776,7 @@ describe("TokenService", () => { expect(result).toEqual(0); }); - it("should return the number of seconds remaining until the token expires", async () => { + it("returns the number of seconds remaining until the token expires", async () => { // Arrange // Lock the time to ensure a consistent test environment // otherwise we have flaky issues with set system time date and the Date.now() call. @@ -644,7 +801,7 @@ describe("TokenService", () => { jest.useRealTimers(); }); - it("should return the number of seconds remaining until the token expires, considering an offset", async () => { + it("returns the number of seconds remaining until the token expires when given an offset", async () => { // Arrange // Lock the time to ensure a consistent test environment // otherwise we have flaky issues with set system time date and the Date.now() call. @@ -672,7 +829,7 @@ describe("TokenService", () => { }); describe("tokenNeedsRefresh", () => { - it("should return true if token is within the default refresh threshold (5 min)", async () => { + it("returns true when the token is within the default refresh threshold (5 min)", async () => { // Arrange const tokenSecondsRemaining = 60; tokenService.tokenSecondsRemaining = jest.fn().mockResolvedValue(tokenSecondsRemaining); @@ -684,7 +841,7 @@ describe("TokenService", () => { expect(result).toEqual(true); }); - it("should return false if token is outside the default refresh threshold (5 min)", async () => { + it("returns false when the token is outside the default refresh threshold (5 min)", async () => { // Arrange const tokenSecondsRemaining = 600; tokenService.tokenSecondsRemaining = jest.fn().mockResolvedValue(tokenSecondsRemaining); @@ -696,7 +853,7 @@ describe("TokenService", () => { expect(result).toEqual(false); }); - it("should return true if token is within the specified refresh threshold", async () => { + it("returns true when the token is within the specified refresh threshold", async () => { // Arrange const tokenSecondsRemaining = 60; tokenService.tokenSecondsRemaining = jest.fn().mockResolvedValue(tokenSecondsRemaining); @@ -708,7 +865,7 @@ describe("TokenService", () => { expect(result).toEqual(true); }); - it("should return false if token is outside the specified refresh threshold", async () => { + it("returns false when the token is outside the specified refresh threshold", async () => { // Arrange const tokenSecondsRemaining = 600; tokenService.tokenSecondsRemaining = jest.fn().mockResolvedValue(tokenSecondsRemaining); @@ -722,7 +879,7 @@ describe("TokenService", () => { }); describe("getUserId", () => { - it("should throw an error if the access token cannot be decoded", async () => { + it("throws an error when the access token cannot be decoded", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockRejectedValue(new Error("Mock error")); @@ -733,7 +890,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("Failed to decode access token: Mock error"); }); - it("should throw an error if the decoded access token is null", async () => { + it("throws an error when the decoded access token is null", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(null); @@ -744,7 +901,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("No user id found"); }); - it("should throw an error if the decoded access token has a non-string user id", async () => { + it("throws an error when the decoded access token has a non-string user id", async () => { // Arrange const accessTokenDecodedWithNonStringSub = { ...accessTokenDecoded, sub: 123 }; tokenService.decodeAccessToken = jest @@ -758,7 +915,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("No user id found"); }); - it("should return the user id from the decoded access token", async () => { + it("returns the user id from the decoded access token when a valid access token is stored", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(accessTokenDecoded); @@ -771,7 +928,7 @@ describe("TokenService", () => { }); describe("getUserIdFromAccessToken", () => { - it("should throw an error if the access token cannot be decoded", async () => { + it("throws an error when the access token cannot be decoded", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockRejectedValue(new Error("Mock error")); @@ -782,7 +939,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("Failed to decode access token: Mock error"); }); - it("should throw an error if the decoded access token is null", async () => { + it("throws an error when the decoded access token is null", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(null); @@ -793,7 +950,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("No user id found"); }); - it("should throw an error if the decoded access token has a non-string user id", async () => { + it("throws an error when the decoded access token has a non-string user id", async () => { // Arrange const accessTokenDecodedWithNonStringSub = { ...accessTokenDecoded, sub: 123 }; tokenService.decodeAccessToken = jest @@ -807,7 +964,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("No user id found"); }); - it("should return the user id from the decoded access token", async () => { + it("returns the user id from the decoded access token when a valid access token is stored", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(accessTokenDecoded); @@ -820,7 +977,7 @@ describe("TokenService", () => { }); describe("getEmail", () => { - it("should throw an error if the access token cannot be decoded", async () => { + it("throws an error when the access token cannot be decoded", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockRejectedValue(new Error("Mock error")); @@ -831,7 +988,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("Failed to decode access token: Mock error"); }); - it("should throw an error if the decoded access token is null", async () => { + it("throws an error when the decoded access token is null", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(null); @@ -842,7 +999,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("No email found"); }); - it("should throw an error if the decoded access token has a non-string email", async () => { + it("throws an error when the decoded access token has a non-string email", async () => { // Arrange const accessTokenDecodedWithNonStringEmail = { ...accessTokenDecoded, email: 123 }; tokenService.decodeAccessToken = jest @@ -856,7 +1013,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("No email found"); }); - it("should return the email from the decoded access token", async () => { + it("returns the email from the decoded access token when a valid access token is stored", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(accessTokenDecoded); @@ -869,7 +1026,7 @@ describe("TokenService", () => { }); describe("getEmailVerified", () => { - it("should throw an error if the access token cannot be decoded", async () => { + it("throws an error when the access token cannot be decoded", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockRejectedValue(new Error("Mock error")); @@ -880,7 +1037,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("Failed to decode access token: Mock error"); }); - it("should throw an error if the decoded access token is null", async () => { + it("throws an error when the decoded access token is null", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(null); @@ -891,7 +1048,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("No email verification found"); }); - it("should throw an error if the decoded access token has a non-boolean email_verified", async () => { + it("throws an error when the decoded access token has a non-boolean email_verified", async () => { // Arrange const accessTokenDecodedWithNonBooleanEmailVerified = { ...accessTokenDecoded, @@ -908,7 +1065,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("No email verification found"); }); - it("should return the email_verified from the decoded access token", async () => { + it("returns the email_verified from the decoded access token when a valid access token is stored", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(accessTokenDecoded); @@ -921,7 +1078,7 @@ describe("TokenService", () => { }); describe("getName", () => { - it("should throw an error if the access token cannot be decoded", async () => { + it("throws an error when the access token cannot be decoded", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockRejectedValue(new Error("Mock error")); @@ -932,7 +1089,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("Failed to decode access token: Mock error"); }); - it("should return null if the decoded access token is null", async () => { + it("returns null when the decoded access token is null", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(null); @@ -943,7 +1100,7 @@ describe("TokenService", () => { expect(result).toBeNull(); }); - it("should return null if the decoded access token has a non-string name", async () => { + it("returns null when the decoded access token has a non-string name", async () => { // Arrange const accessTokenDecodedWithNonStringName = { ...accessTokenDecoded, name: 123 }; tokenService.decodeAccessToken = jest @@ -957,7 +1114,7 @@ describe("TokenService", () => { expect(result).toBeNull(); }); - it("should return the name from the decoded access token", async () => { + it("returns the name from the decoded access token when a valid access token is stored", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(accessTokenDecoded); @@ -970,7 +1127,7 @@ describe("TokenService", () => { }); describe("getIssuer", () => { - it("should throw an error if the access token cannot be decoded", async () => { + it("throws an error when the access token cannot be decoded", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockRejectedValue(new Error("Mock error")); @@ -981,7 +1138,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("Failed to decode access token: Mock error"); }); - it("should throw an error if the decoded access token is null", async () => { + it("throws an error when the decoded access token is null", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(null); @@ -992,7 +1149,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("No issuer found"); }); - it("should throw an error if the decoded access token has a non-string iss", async () => { + it("throws an error when the decoded access token has a non-string iss", async () => { // Arrange const accessTokenDecodedWithNonStringIss = { ...accessTokenDecoded, iss: 123 }; tokenService.decodeAccessToken = jest @@ -1006,7 +1163,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("No issuer found"); }); - it("should return the issuer from the decoded access token", async () => { + it("returns the issuer from the decoded access token when a valid access token is stored", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockResolvedValue(accessTokenDecoded); @@ -1019,7 +1176,7 @@ describe("TokenService", () => { }); describe("getIsExternal", () => { - it("should throw an error if the access token cannot be decoded", async () => { + it("throws an error when the access token cannot be decoded", async () => { // Arrange tokenService.decodeAccessToken = jest.fn().mockRejectedValue(new Error("Mock error")); @@ -1030,7 +1187,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("Failed to decode access token: Mock error"); }); - it("should return false if the amr (Authentication Method Reference) claim does not contain 'external'", async () => { + it("returns false when the amr (Authentication Method Reference) claim does not contain 'external'", async () => { // Arrange const accessTokenDecodedWithoutExternalAmr = { ...accessTokenDecoded, @@ -1047,7 +1204,7 @@ describe("TokenService", () => { expect(result).toEqual(false); }); - it("should return true if the amr (Authentication Method Reference) claim contains 'external'", async () => { + it("returns true when the amr (Authentication Method Reference) claim contains 'external'", async () => { // Arrange const accessTokenDecodedWithExternalAmr = { ...accessTokenDecoded, @@ -1073,7 +1230,7 @@ describe("TokenService", () => { const refreshTokenSecureStorageKey = `${userIdFromAccessToken}${refreshTokenPartialSecureStorageKey}`; describe("setRefreshToken", () => { - it("should throw an error if no user id is provided", async () => { + it("throws an error when no user id is provided", async () => { // Act // note: don't await here because we want to test the error const result = (tokenService as any).setRefreshToken( @@ -1113,7 +1270,7 @@ describe("TokenService", () => { }); describe("Memory storage tests", () => { - it("should set the refresh token in memory for the specified user id", async () => { + it("sets the refresh token in memory when given a user id", async () => { // Act await (tokenService as any).setRefreshToken( refreshToken, @@ -1130,7 +1287,7 @@ describe("TokenService", () => { }); describe("Disk storage tests (secure storage not supported on platform)", () => { - it("should set the refresh token in disk for the specified user id", async () => { + it("sets the refresh token in disk when given a user id", async () => { // Act await (tokenService as any).setRefreshToken( refreshToken, @@ -1152,7 +1309,7 @@ describe("TokenService", () => { tokenService = createTokenService(supportsSecureStorage); }); - it("should set the refresh token in secure storage, null out data on disk or in memory, and set a flag to indicate the token has been migrated for the specified user id", async () => { + it("sets the refresh token in secure storage, removes data on disk or in memory, and sets a flag to indicate the token has been migrated when given a user id", async () => { // Arrange: // For testing purposes, let's assume that the token is already in disk and memory singleUserStateProvider @@ -1163,6 +1320,9 @@ describe("TokenService", () => { .getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY) .stateSubject.next([userIdFromAccessToken, refreshToken]); + // We immediately call to get the refresh token from secure storage after setting it to ensure it was set. + secureStorageService.get.mockResolvedValue(refreshToken); + // Act await (tokenService as any).setRefreshToken( refreshToken, @@ -1187,18 +1347,166 @@ describe("TokenService", () => { singleUserStateProvider.getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY).nextMock, ).toHaveBeenCalledWith(null); }); + + it("tries to set the refresh token in secure storage then falls back to disk storage when the refresh token cannot be read back out of secure storage", async () => { + // Arrange: + // We immediately call to get the refresh token from secure storage after setting it to ensure it was set. + // So, set it to return null to mock a failure to set the refresh token in secure storage. + // This mocks the windows 10/11 intermittent issue where the token is not set in secure storage successfully. + secureStorageService.get.mockResolvedValue(null); + + // Act + await (tokenService as any).setRefreshToken( + refreshToken, + diskVaultTimeoutAction, + diskVaultTimeout, + userIdFromAccessToken, + ); + // Assert + + // assert that the refresh token was set in secure storage + expect(secureStorageService.save).toHaveBeenCalledWith( + refreshTokenSecureStorageKey, + refreshToken, + secureStorageOptions, + ); + + // assert that we tried to set the refresh token in secure storage, but it failed, so we reverted to disk storage + expect( + singleUserStateProvider.getFake(userIdFromAccessToken, REFRESH_TOKEN_DISK).nextMock, + ).toHaveBeenCalledWith(refreshToken); + }); + + it("tries to set the refresh token in secure storage, throws an error, then falls back to disk storage when secure storage isn't supported", async () => { + // Arrange: + // Mock the secure storage service to throw an error when trying to save the refresh token + // to simulate linux scenarios where a secure storage provider isn't configured. + secureStorageService.save.mockRejectedValue(new Error("Secure storage not supported")); + + // Act + await (tokenService as any).setRefreshToken( + refreshToken, + diskVaultTimeoutAction, + diskVaultTimeout, + userIdFromAccessToken, + ); + // Assert + + // assert that the refresh token was set in secure storage + expect(secureStorageService.save).toHaveBeenCalledWith( + refreshTokenSecureStorageKey, + refreshToken, + secureStorageOptions, + ); + + // assert that we tried to set the refresh token in secure storage, but it failed, so we reverted to disk storage + expect( + singleUserStateProvider.getFake(userIdFromAccessToken, REFRESH_TOKEN_DISK).nextMock, + ).toHaveBeenCalledWith(refreshToken); + }); + + it("returns the unencrypted access token when secure storage retrieval fails but the access token is still pre-migration", async () => { + // This tests the linux scenario where users might not have secure storage support configured. + + // Arrange + singleUserStateProvider + .getFake(userIdFromAccessToken, ACCESS_TOKEN_MEMORY) + .stateSubject.next([userIdFromAccessToken, undefined]); + + singleUserStateProvider + .getFake(userIdFromAccessToken, ACCESS_TOKEN_DISK) + .stateSubject.next([userIdFromAccessToken, accessTokenJwt]); + + // Mock linux secure storage error + const secureStorageError = "Secure storage error"; + secureStorageService.get.mockRejectedValue(new Error(secureStorageError)); + + // Act + const result = await tokenService.getAccessToken(userIdFromAccessToken); + + // Assert + // assert that we returned the unencrypted, pre-migration access token + expect(result).toBe(accessTokenJwt); + + // assert that we did not log an error or log the user out + expect(logService.error).not.toHaveBeenCalled(); + + expect(logoutCallback).not.toHaveBeenCalled(); + }); + + it("does not error and fallback to disk storage when passed a null value for the refresh token", async () => { + // Arrange + secureStorageService.get.mockResolvedValue(null); + + // Act + await (tokenService as any).setRefreshToken( + null, + diskVaultTimeoutAction, + diskVaultTimeout, + userIdFromAccessToken, + ); + + // Assert + expect(secureStorageService.save).toHaveBeenCalledWith( + refreshTokenSecureStorageKey, + null, + secureStorageOptions, + ); + + expect(logService.error).not.toHaveBeenCalled(); + + expect( + singleUserStateProvider.getFake(userIdFromAccessToken, REFRESH_TOKEN_DISK).nextMock, + ).toHaveBeenCalledWith(null); + + expect( + singleUserStateProvider.getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY).nextMock, + ).toHaveBeenCalledWith(null); + }); + + it("logs the error and logs the user out when the access token cannot be decrypted", async () => { + // Arrange + singleUserStateProvider + .getFake(userIdFromAccessToken, ACCESS_TOKEN_MEMORY) + .stateSubject.next([userIdFromAccessToken, undefined]); + + singleUserStateProvider + .getFake(userIdFromAccessToken, ACCESS_TOKEN_DISK) + .stateSubject.next([userIdFromAccessToken, encryptedAccessToken]); + + secureStorageService.get.mockResolvedValue(accessTokenKeyB64); + encryptService.decryptToUtf8.mockRejectedValue(new Error("Decryption error")); + + // Act + const result = await tokenService.getAccessToken(userIdFromAccessToken); + + // Assert + expect(result).toBeNull(); + + // assert that we logged the error + expect(logService.error).toHaveBeenCalledWith( + "Failed to decrypt access token", + new Error("Decryption error"), + ); + + // assert that we logged the user out + expect(logoutCallback).toHaveBeenCalledWith( + "accessTokenUnableToBeDecrypted", + userIdFromAccessToken, + ); + }); }); }); describe("getRefreshToken", () => { - it("should return undefined if no user id is provided and there is no active user in global state", async () => { + it("returns null when no user id is provided and there is no active user in global state", async () => { // Act const result = await (tokenService as any).getRefreshToken(); // Assert - expect(result).toBeUndefined(); + expect(result).toBeNull(); }); - it("should return null if no refresh token is found in memory, disk, or secure storage", async () => { + it("returns null when no refresh token is found in memory, disk, or secure storage", async () => { // Arrange globalStateProvider .getFake(ACCOUNT_ACTIVE_ACCOUNT_ID) @@ -1211,7 +1519,7 @@ describe("TokenService", () => { }); describe("Memory storage tests", () => { - it("should get the refresh token from memory with no user id specified (uses global active user)", async () => { + it("gets the refresh token from memory when no user id is specified (uses global active user)", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY) @@ -1233,7 +1541,7 @@ describe("TokenService", () => { expect(result).toEqual(refreshToken); }); - it("should get the refresh token from memory for the specified user id", async () => { + it("gets the refresh token from memory when a user id is specified", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY) @@ -1251,7 +1559,7 @@ describe("TokenService", () => { }); describe("Disk storage tests (secure storage not supported on platform)", () => { - it("should get the refresh token from disk with no user id specified", async () => { + it("gets the refresh token from disk when no user id is specified", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY) @@ -1272,7 +1580,7 @@ describe("TokenService", () => { expect(result).toEqual(refreshToken); }); - it("should get the refresh token from disk for the specified user id", async () => { + it("gets the refresh token from disk when a user id is specified", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY) @@ -1295,7 +1603,7 @@ describe("TokenService", () => { tokenService = createTokenService(supportsSecureStorage); }); - it("should get the refresh token from secure storage when no user id is specified and the migration flag is set to true", async () => { + it("gets the refresh token from secure storage when no user id is specified", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY) @@ -1318,7 +1626,7 @@ describe("TokenService", () => { expect(result).toEqual(refreshToken); }); - it("should get the refresh token from secure storage when user id is specified and the migration flag set to true", async () => { + it("gets the refresh token from secure storage when a user id is specified", async () => { // Arrange singleUserStateProvider @@ -1337,7 +1645,7 @@ describe("TokenService", () => { expect(result).toEqual(refreshToken); }); - it("should fallback and get the refresh token from disk when user id is specified and the migration flag is set to false even if the platform supports secure storage", async () => { + it("falls back and gets the refresh token from disk when a user id is specified even if the platform supports secure storage", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY) @@ -1357,7 +1665,7 @@ describe("TokenService", () => { expect(secureStorageService.get).not.toHaveBeenCalled(); }); - it("should fallback and get the refresh token from disk when no user id is specified and the migration flag is set to false even if the platform supports secure storage", async () => { + it("falls back and gets the refresh token from disk when no user id is specified even if the platform supports secure storage", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY) @@ -1381,11 +1689,80 @@ describe("TokenService", () => { // assert that secure storage was not called expect(secureStorageService.get).not.toHaveBeenCalled(); }); + + it("returns null when the refresh token is not found in memory, on disk, or in secure storage", async () => { + // Arrange + secureStorageService.get.mockResolvedValue(null); + + // Act + const result = await tokenService.getRefreshToken(userIdFromAccessToken); + + // Assert + expect(result).toBeNull(); + }); + + it("returns null and logs when the refresh token is not found in secure storage when it should be", async () => { + // This scenario mocks the case where we have intermittent windows 10/11 issues w/ secure storage not + // returning the refresh token when it should be there. + // Arrange + singleUserStateProvider + .getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY) + .stateSubject.next([userIdFromAccessToken, undefined]); + + singleUserStateProvider + .getFake(userIdFromAccessToken, REFRESH_TOKEN_DISK) + .stateSubject.next([userIdFromAccessToken, undefined]); + + secureStorageService.get.mockResolvedValue(null); + + // Act + const result = await tokenService.getRefreshToken(userIdFromAccessToken); + + // Assert + expect(result).toBeNull(); + + expect(logService.error).toHaveBeenCalledWith( + "Refresh token not found in secure storage. Access token will fail to refresh upon expiration or manual refresh.", + ); + }); + + it("logs out when retrieving the refresh token out of secure storage errors", async () => { + // This scenario mocks the case where linux users don't have secure storage configured. + // Arrange + singleUserStateProvider + .getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY) + .stateSubject.next([userIdFromAccessToken, undefined]); + + singleUserStateProvider + .getFake(userIdFromAccessToken, REFRESH_TOKEN_DISK) + .stateSubject.next([userIdFromAccessToken, undefined]); + + const secureStorageSvcMockErrorMsg = "Secure storage retrieval error"; + + secureStorageService.get.mockRejectedValue(new Error(secureStorageSvcMockErrorMsg)); + + // Act + const result = await tokenService.getRefreshToken(userIdFromAccessToken); + + // Assert + expect(result).toBeNull(); + + // expect that we logged an error and logged the user out + expect(logService.error).toHaveBeenCalledWith( + `Failed to retrieve refresh token from secure storage`, + new Error(secureStorageSvcMockErrorMsg), + ); + + expect(logoutCallback).toHaveBeenCalledWith( + "refreshTokenSecureStorageRetrievalFailure", + userIdFromAccessToken, + ); + }); }); }); describe("clearRefreshToken", () => { - it("should throw an error if no user id is provided", async () => { + it("throws an error when no user id is provided", async () => { // Act // note: don't await here because we want to test the error const result = (tokenService as any).clearRefreshToken(); @@ -1399,7 +1776,7 @@ describe("TokenService", () => { tokenService = createTokenService(supportsSecureStorage); }); - it("should clear the refresh token from all storage locations for the specified user id", async () => { + it("clears the refresh token from all storage locations when given a user id", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, REFRESH_TOKEN_MEMORY) @@ -1433,7 +1810,7 @@ describe("TokenService", () => { const clientId = "clientId"; describe("setClientId", () => { - it("should throw an error if no user id is provided and there is no active user in global state", async () => { + it("throws an error when no user id is provided and there is no active user in global state", async () => { // Act // note: don't await here because we want to test the error const result = tokenService.setClientId(clientId, VaultTimeoutAction.Lock, null); @@ -1470,7 +1847,7 @@ describe("TokenService", () => { }); describe("Memory storage tests", () => { - it("should set the client id in memory when there is an active user in global state", async () => { + it("sets the client id in memory when there is an active user in global state", async () => { // Arrange globalStateProvider .getFake(ACCOUNT_ACTIVE_ACCOUNT_ID) @@ -1486,7 +1863,7 @@ describe("TokenService", () => { ).toHaveBeenCalledWith(clientId); }); - it("should set the client id in memory for the specified user id", async () => { + it("sets the client id in memory when given a user id", async () => { // Act await tokenService.setClientId( clientId, @@ -1504,7 +1881,7 @@ describe("TokenService", () => { }); describe("Disk storage tests", () => { - it("should set the client id in disk when there is an active user in global state", async () => { + it("sets the client id in disk when there is an active user in global state", async () => { // Arrange globalStateProvider .getFake(ACCOUNT_ACTIVE_ACCOUNT_ID) @@ -1519,7 +1896,7 @@ describe("TokenService", () => { ).toHaveBeenCalledWith(clientId); }); - it("should set the client id in disk for the specified user id", async () => { + it("sets the client id on disk when given a user id", async () => { // Act await tokenService.setClientId( clientId, @@ -1537,14 +1914,14 @@ describe("TokenService", () => { }); describe("getClientId", () => { - it("should return undefined if no user id is provided and there is no active user in global state", async () => { + it("returns undefined when no user id is provided and there is no active user in global state", async () => { // Act const result = await tokenService.getClientId(); // Assert expect(result).toBeUndefined(); }); - it("should return null if no client id is found in memory or disk", async () => { + it("returns null when no client id is found in memory or disk", async () => { // Arrange globalStateProvider .getFake(ACCOUNT_ACTIVE_ACCOUNT_ID) @@ -1557,7 +1934,7 @@ describe("TokenService", () => { }); describe("Memory storage tests", () => { - it("should get the client id from memory with no user id specified (uses global active user)", async () => { + it("gets the client id from memory when no user id is specified (uses global active user)", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, API_KEY_CLIENT_ID_MEMORY) @@ -1580,7 +1957,7 @@ describe("TokenService", () => { expect(result).toEqual(clientId); }); - it("should get the client id from memory for the specified user id", async () => { + it("gets the client id from memory when given a user id", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, API_KEY_CLIENT_ID_MEMORY) @@ -1599,7 +1976,7 @@ describe("TokenService", () => { }); describe("Disk storage tests", () => { - it("should get the client id from disk with no user id specified", async () => { + it("gets the client id from disk when no user id is specified", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, API_KEY_CLIENT_ID_MEMORY) @@ -1620,7 +1997,7 @@ describe("TokenService", () => { expect(result).toEqual(clientId); }); - it("should get the client id from disk for the specified user id", async () => { + it("gets the client id from disk when a user id is specified", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, API_KEY_CLIENT_ID_MEMORY) @@ -1639,7 +2016,7 @@ describe("TokenService", () => { }); describe("clearClientId", () => { - it("should throw an error if no user id is provided and there is no active user in global state", async () => { + it("throws an error when no user id is provided and there is no active user in global state", async () => { // Act // note: don't await here because we want to test the error const result = (tokenService as any).clearClientId(); @@ -1647,7 +2024,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("User id not found. Cannot clear client id."); }); - it("should clear the client id from memory and disk for the specified user id", async () => { + it("clears the client id from memory and disk when a user id is specified", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, API_KEY_CLIENT_ID_MEMORY) @@ -1669,7 +2046,7 @@ describe("TokenService", () => { ).toHaveBeenCalledWith(null); }); - it("should clear the client id from memory and disk for the global active user", async () => { + it("clears the client id from memory and disk when there is a global active user", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, API_KEY_CLIENT_ID_MEMORY) @@ -1702,7 +2079,7 @@ describe("TokenService", () => { const clientSecret = "clientSecret"; describe("setClientSecret", () => { - it("should throw an error if no user id is provided and there is no active user in global state", async () => { + it("throws an error when no user id is provided and there is no active user in global state", async () => { // Act // note: don't await here because we want to test the error const result = tokenService.setClientSecret( @@ -1747,7 +2124,7 @@ describe("TokenService", () => { }); describe("Memory storage tests", () => { - it("should set the client secret in memory when there is an active user in global state", async () => { + it("sets the client secret in memory when there is an active user in global state", async () => { // Arrange globalStateProvider .getFake(ACCOUNT_ACTIVE_ACCOUNT_ID) @@ -1767,7 +2144,7 @@ describe("TokenService", () => { ).toHaveBeenCalledWith(clientSecret); }); - it("should set the client secret in memory for the specified user id", async () => { + it("sets the client secret in memory when a user id is specified", async () => { // Act await tokenService.setClientSecret( clientSecret, @@ -1785,7 +2162,7 @@ describe("TokenService", () => { }); describe("Disk storage tests", () => { - it("should set the client secret in disk when there is an active user in global state", async () => { + it("sets the client secret on disk when there is an active user in global state", async () => { // Arrange globalStateProvider .getFake(ACCOUNT_ACTIVE_ACCOUNT_ID) @@ -1805,7 +2182,7 @@ describe("TokenService", () => { ).toHaveBeenCalledWith(clientSecret); }); - it("should set the client secret in disk for the specified user id", async () => { + it("sets the client secret on disk when a user id is specified", async () => { // Act await tokenService.setClientSecret( clientSecret, @@ -1824,14 +2201,14 @@ describe("TokenService", () => { }); describe("getClientSecret", () => { - it("should return undefined if no user id is provided and there is no active user in global state", async () => { + it("returns undefined when no user id is provided and there is no active user in global state", async () => { // Act const result = await tokenService.getClientSecret(); // Assert expect(result).toBeUndefined(); }); - it("should return null if no client secret is found in memory or disk", async () => { + it("returns null when no client secret is found in memory or disk", async () => { // Arrange globalStateProvider .getFake(ACCOUNT_ACTIVE_ACCOUNT_ID) @@ -1844,7 +2221,7 @@ describe("TokenService", () => { }); describe("Memory storage tests", () => { - it("should get the client secret from memory with no user id specified (uses global active user)", async () => { + it("gets the client secret from memory when no user id is specified (uses global active user)", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, API_KEY_CLIENT_SECRET_MEMORY) @@ -1867,7 +2244,7 @@ describe("TokenService", () => { expect(result).toEqual(clientSecret); }); - it("should get the client secret from memory for the specified user id", async () => { + it("gets the client secret from memory when a user id is specified", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, API_KEY_CLIENT_SECRET_MEMORY) @@ -1886,7 +2263,7 @@ describe("TokenService", () => { }); describe("Disk storage tests", () => { - it("should get the client secret from disk with no user id specified", async () => { + it("gets the client secret from disk when no user id specified", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, API_KEY_CLIENT_SECRET_MEMORY) @@ -1907,7 +2284,7 @@ describe("TokenService", () => { expect(result).toEqual(clientSecret); }); - it("should get the client secret from disk for the specified user id", async () => { + it("gets the client secret from disk when a user id is specified", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, API_KEY_CLIENT_SECRET_MEMORY) @@ -1926,7 +2303,7 @@ describe("TokenService", () => { }); describe("clearClientSecret", () => { - it("should throw an error if no user id is provided and there is no active user in global state", async () => { + it("throws an error when no user id is provided and there is no active user in global state", async () => { // Act // note: don't await here because we want to test the error const result = (tokenService as any).clearClientSecret(); @@ -1934,7 +2311,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("User id not found. Cannot clear client secret."); }); - it("should clear the client secret from memory and disk for the specified user id", async () => { + it("clears the client secret from memory and disk when a user id is specified", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, API_KEY_CLIENT_SECRET_MEMORY) @@ -1958,7 +2335,7 @@ describe("TokenService", () => { ).toHaveBeenCalledWith(null); }); - it("should clear the client secret from memory and disk for the global active user", async () => { + it("clears the client secret from memory and disk when there is a global active user", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, API_KEY_CLIENT_SECRET_MEMORY) @@ -1990,7 +2367,7 @@ describe("TokenService", () => { }); describe("setTokens", () => { - it("should call to set all passed in tokens after deriving user id from the access token", async () => { + it("calls to set all tokens after deriving user id from the access token when called with valid params", async () => { // Arrange const refreshToken = "refreshToken"; // specific vault timeout actions and vault timeouts don't change this test so values don't matter. @@ -2042,7 +2419,7 @@ describe("TokenService", () => { ); }); - it("should not try to set client id and client secret if they are not passed in", async () => { + it("does not try to set client id and client secret when they are not passed in", async () => { // Arrange const refreshToken = "refreshToken"; const vaultTimeoutAction = VaultTimeoutAction.Lock; @@ -2076,7 +2453,7 @@ describe("TokenService", () => { expect(tokenService.setClientSecret).not.toHaveBeenCalled(); }); - it("should throw an error if the access token is invalid", async () => { + it("throws an error when the access token is invalid", async () => { // Arrange const accessToken = "invalidToken"; const refreshToken = "refreshToken"; @@ -2095,7 +2472,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("JWT must have 3 parts"); }); - it("should throw an error if the access token is missing", async () => { + it("throws an error when the access token is missing", async () => { // Arrange const accessToken: string = null; const refreshToken = "refreshToken"; @@ -2150,7 +2527,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("Vault Timeout Action is required."); }); - it("should not throw an error if the refresh token is missing and it should just not set it", async () => { + it("does not throw an error or set the refresh token when the refresh token is missing", async () => { // Arrange const refreshToken: string = null; const vaultTimeoutAction = VaultTimeoutAction.Lock; @@ -2166,7 +2543,7 @@ describe("TokenService", () => { }); describe("clearTokens", () => { - it("should call to clear all tokens for the specified user id", async () => { + it("calls to clear all tokens when given a specified user id", async () => { // Arrange const userId = "userId" as UserId; @@ -2187,7 +2564,7 @@ describe("TokenService", () => { expect((tokenService as any).clearClientSecret).toHaveBeenCalledWith(userId); }); - it("should call to clear all tokens for the active user id", async () => { + it("calls to clear all tokens when there is an active user", async () => { // Arrange const userId = "userId" as UserId; @@ -2210,7 +2587,7 @@ describe("TokenService", () => { expect((tokenService as any).clearClientSecret).toHaveBeenCalledWith(userId); }); - it("should not call to clear all tokens if no user id is provided and there is no active user in global state", async () => { + it("does not call to clear all tokens when no user id is provided and there is no active user in global state", async () => { // Arrange tokenService.clearAccessToken = jest.fn(); (tokenService as any).clearRefreshToken = jest.fn(); @@ -2228,7 +2605,7 @@ describe("TokenService", () => { describe("Two Factor Token methods", () => { describe("setTwoFactorToken", () => { - it("should set the email and two factor token when there hasn't been a previous record (initializing the record)", async () => { + it("sets the email and two factor token when there hasn't been a previous record (initializing the record)", async () => { // Arrange const email = "testUser@email.com"; const twoFactorToken = "twoFactorTokenForTestUser"; @@ -2240,7 +2617,7 @@ describe("TokenService", () => { ).toHaveBeenCalledWith({ [email]: twoFactorToken }); }); - it("should set the email and two factor token when there is an initialized value already (updating the existing record)", async () => { + it("sets the email and two factor token when there is an initialized value already (updating the existing record)", async () => { // Arrange const email = "testUser@email.com"; const twoFactorToken = "twoFactorTokenForTestUser"; @@ -2263,7 +2640,7 @@ describe("TokenService", () => { }); describe("getTwoFactorToken", () => { - it("should return the two factor token for the given email", async () => { + it("returns the two factor token when given an email", async () => { // Arrange const email = "testUser"; const twoFactorToken = "twoFactorTokenForTestUser"; @@ -2282,7 +2659,7 @@ describe("TokenService", () => { expect(result).toEqual(twoFactorToken); }); - it("should not return the two factor token for an email that doesn't exist", async () => { + it("does not return the two factor token when given an email that doesn't exist", async () => { // Arrange const email = "testUser"; const initialTwoFactorTokenRecord: Record = { @@ -2300,7 +2677,7 @@ describe("TokenService", () => { expect(result).toEqual(undefined); }); - it("should return null if there is no two factor token record", async () => { + it("returns null when there is no two factor token record", async () => { // Arrange globalStateProvider .getFake(EMAIL_TWO_FACTOR_TOKEN_RECORD_DISK_LOCAL) @@ -2315,7 +2692,7 @@ describe("TokenService", () => { }); describe("clearTwoFactorToken", () => { - it("should clear the two factor token for the given email when a record exists", async () => { + it("clears the two factor token for the given email when a record exists", async () => { // Arrange const email = "testUser"; const twoFactorToken = "twoFactorTokenForTestUser"; @@ -2336,7 +2713,7 @@ describe("TokenService", () => { ).toHaveBeenCalledWith({}); }); - it("should initialize the record if it doesn't exist and delete the value", async () => { + it("initializes the record and deletes the value when the record doesn't exist", async () => { // Arrange const email = "testUser"; @@ -2355,7 +2732,7 @@ describe("TokenService", () => { const mockSecurityStamp = "securityStamp"; describe("setSecurityStamp", () => { - it("should throw an error if no user id is provided and there is no active user in global state", async () => { + it("throws an error deletes the value no user id is provided and there is no active user in global state", async () => { // Act // note: don't await here because we want to test the error const result = tokenService.setSecurityStamp(mockSecurityStamp); @@ -2363,7 +2740,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("User id not found. Cannot set security stamp."); }); - it("should set the security stamp in memory when there is an active user in global state", async () => { + it("sets the security stamp in memory when there is an active user in global state", async () => { // Arrange globalStateProvider .getFake(ACCOUNT_ACTIVE_ACCOUNT_ID) @@ -2378,7 +2755,7 @@ describe("TokenService", () => { ).toHaveBeenCalledWith(mockSecurityStamp); }); - it("should set the security stamp in memory for the specified user id", async () => { + it("sets the security stamp in memory when a user id is specified", async () => { // Act await tokenService.setSecurityStamp(mockSecurityStamp, userIdFromAccessToken); @@ -2390,7 +2767,7 @@ describe("TokenService", () => { }); describe("getSecurityStamp", () => { - it("should throw an error if no user id is provided and there is no active user in global state", async () => { + it("throws an error when no user id is provided and there is no active user in global state", async () => { // Act // note: don't await here because we want to test the error const result = tokenService.getSecurityStamp(); @@ -2398,7 +2775,7 @@ describe("TokenService", () => { await expect(result).rejects.toThrow("User id not found. Cannot get security stamp."); }); - it("should return the security stamp from memory with no user id specified (uses global active user)", async () => { + it("returns the security stamp from memory when no user id is specified (uses global active user)", async () => { // Arrange globalStateProvider .getFake(ACCOUNT_ACTIVE_ACCOUNT_ID) @@ -2415,7 +2792,7 @@ describe("TokenService", () => { expect(result).toEqual(mockSecurityStamp); }); - it("should return the security stamp from memory for the specified user id", async () => { + it("returns the security stamp from memory when a user id is specified", async () => { // Arrange singleUserStateProvider .getFake(userIdFromAccessToken, SECURITY_STAMP_MEMORY) @@ -2601,6 +2978,7 @@ describe("TokenService", () => { keyGenerationService, encryptService, logService, + logoutCallback, ); } }); diff --git a/libs/common/src/auth/services/token.service.ts b/libs/common/src/auth/services/token.service.ts index 203d95429e..38d0a77b52 100644 --- a/libs/common/src/auth/services/token.service.ts +++ b/libs/common/src/auth/services/token.service.ts @@ -1,7 +1,7 @@ import { Observable, combineLatest, firstValueFrom, map } from "rxjs"; import { Opaque } from "type-fest"; -import { decodeJwtTokenToJson } from "@bitwarden/auth/common"; +import { LogoutReason, decodeJwtTokenToJson } from "@bitwarden/auth/common"; import { VaultTimeoutAction } from "../../enums/vault-timeout-action.enum"; import { EncryptService } from "../../platform/abstractions/encrypt.service"; @@ -111,7 +111,7 @@ export type DecodedAccessToken = { * A symmetric key for encrypting the access token before the token is stored on disk. * This key should be stored in secure storage. * */ -type AccessTokenKey = Opaque; +export type AccessTokenKey = Opaque; export class TokenService implements TokenServiceAbstraction { private readonly accessTokenKeySecureStorageKey: string = "_accessTokenKey"; @@ -132,6 +132,7 @@ export class TokenService implements TokenServiceAbstraction { private keyGenerationService: KeyGenerationService, private encryptService: EncryptService, private logService: LogService, + private logoutCallback: (logoutReason: LogoutReason, userId?: string) => Promise, ) { this.initializeState(); } @@ -145,10 +146,6 @@ export class TokenService implements TokenServiceAbstraction { ]).pipe(map(([disk, memory]) => Boolean(disk || memory))); } - // pivoting to an approach where we create a symmetric key we store in secure storage - // which is used to protect the data before persisting to disk. - // We will also use the same symmetric key to decrypt the data when reading from disk. - private initializeState(): void { this.emailTwoFactorTokenRecordGlobalState = this.globalStateProvider.get( EMAIL_TWO_FACTOR_TOKEN_RECORD_DISK_LOCAL, @@ -218,6 +215,14 @@ export class TokenService implements TokenServiceAbstraction { this.getSecureStorageOptions(userId), ); + // We are having intermittent issues with access token keys not saving into secure storage on windows 10/11. + // So, let's add a check to ensure we can read the value after writing it. + const accessTokenKey = await this.getAccessTokenKey(userId); + + if (!accessTokenKey) { + throw new Error("New Access token key unable to be retrieved from secure storage."); + } + return newAccessTokenKey; } @@ -238,6 +243,8 @@ export class TokenService implements TokenServiceAbstraction { } // First see if we have an accessTokenKey in secure storage and return it if we do + // Note: retrieving/saving data from/to secure storage on linux will throw if the + // distro doesn't have a secure storage provider let accessTokenKey: AccessTokenKey = await this.getAccessTokenKey(userId); if (!accessTokenKey) { @@ -255,15 +262,13 @@ export class TokenService implements TokenServiceAbstraction { } private async decryptAccessToken( + accessTokenKey: AccessTokenKey, encryptedAccessToken: EncString, - userId: UserId, ): Promise { - const accessTokenKey = await this.getAccessTokenKey(userId); - if (!accessTokenKey) { - // If we don't have an accessTokenKey, then that means we don't have an access token as it hasn't been set yet - // and we have to return null here to properly indicate the user isn't logged in. - return null; + throw new Error( + "decryptAccessToken: Access token key required. Cannot decrypt access token.", + ); } const decryptedAccessToken = await this.encryptService.decryptToUtf8( @@ -297,17 +302,32 @@ export class TokenService implements TokenServiceAbstraction { // store the access token directly. Instead, we encrypt with accessTokenKey and store that // in secure storage. - const encryptedAccessToken: EncString = await this.encryptAccessToken(accessToken, userId); + try { + const encryptedAccessToken: EncString = await this.encryptAccessToken( + accessToken, + userId, + ); - // Save the encrypted access token to disk - await this.singleUserStateProvider - .get(userId, ACCESS_TOKEN_DISK) - .update((_) => encryptedAccessToken.encryptedString); + // Save the encrypted access token to disk + await this.singleUserStateProvider + .get(userId, ACCESS_TOKEN_DISK) + .update((_) => encryptedAccessToken.encryptedString); - // TODO: PM-6408 - https://bitwarden.atlassian.net/browse/PM-6408 - // 2024-02-20: Remove access token from memory so that we migrate to encrypt the access token over time. - // Remove this call to remove the access token from memory after 3 releases. - await this.singleUserStateProvider.get(userId, ACCESS_TOKEN_MEMORY).update((_) => null); + // TODO: PM-6408 + // 2024-02-20: Remove access token from memory so that we migrate to encrypt the access token over time. + // Remove this call to remove the access token from memory after 3 months. + await this.singleUserStateProvider.get(userId, ACCESS_TOKEN_MEMORY).update((_) => null); + } catch (error) { + this.logService.error( + `SetAccessToken: storing encrypted access token in secure storage failed. Falling back to disk storage.`, + error, + ); + + // Fall back to disk storage for unecrypted access token + await this.singleUserStateProvider + .get(userId, ACCESS_TOKEN_DISK) + .update((_) => accessToken); + } return; } @@ -376,11 +396,11 @@ export class TokenService implements TokenServiceAbstraction { await this.singleUserStateProvider.get(userId, ACCESS_TOKEN_MEMORY).update((_) => null); } - async getAccessToken(userId?: UserId): Promise { + async getAccessToken(userId?: UserId): Promise { userId ??= await firstValueFrom(this.activeUserIdGlobalState.state$); if (!userId) { - return undefined; + return null; } // Try to get the access token from memory @@ -399,10 +419,41 @@ export class TokenService implements TokenServiceAbstraction { } if (this.platformSupportsSecureStorage) { - const accessTokenKey = await this.getAccessTokenKey(userId); + let accessTokenKey: AccessTokenKey; + try { + accessTokenKey = await this.getAccessTokenKey(userId); + } catch (error) { + if (EncString.isSerializedEncString(accessTokenDisk)) { + this.logService.error( + "Access token key retrieval failed. Unable to decrypt encrypted access token. Logging user out.", + error, + ); + await this.logoutCallback("accessTokenUnableToBeDecrypted", userId); + return null; + } + + // If the access token key is not found, but the access token is unencrypted then + // this indicates that this is the pre-migration state where the access token + // was stored unencrypted on disk. We can return the access token as is. + // Note: this is likely to only be hit for linux users who don't + // have a secure storage provider configured. + return accessTokenDisk; + } if (!accessTokenKey) { - // We know this is an unencrypted access token because we don't have an access token key + if (EncString.isSerializedEncString(accessTokenDisk)) { + // The access token is encrypted but we don't have the key to decrypt it for + // whatever reason so we have to log the user out. + this.logService.error( + "Access token key not found to decrypt encrypted access token. Logging user out.", + ); + + await this.logoutCallback("accessTokenUnableToBeDecrypted", userId); + + return null; + } + + // We know this is an unencrypted access token return accessTokenDisk; } @@ -410,17 +461,18 @@ export class TokenService implements TokenServiceAbstraction { const encryptedAccessTokenEncString = new EncString(accessTokenDisk as EncryptedString); const decryptedAccessToken = await this.decryptAccessToken( + accessTokenKey, encryptedAccessTokenEncString, - userId, ); return decryptedAccessToken; } catch (error) { - // If an error occurs during decryption, return null for logout. + // If an error occurs during decryption, logout and then return null. // We don't try to recover here since we'd like to know // if access token and key are getting out of sync. - this.logService.error( - `Failed to decrypt access token: ${error?.message ?? "Unknown error."}`, - ); + this.logService.error(`Failed to decrypt access token`, error); + + await this.logoutCallback("accessTokenUnableToBeDecrypted", userId); + return null; } } @@ -456,21 +508,49 @@ export class TokenService implements TokenServiceAbstraction { ); switch (storageLocation) { - case TokenStorageLocation.SecureStorage: - await this.saveStringToSecureStorage( - userId, - this.refreshTokenSecureStorageKey, - refreshToken, - ); + case TokenStorageLocation.SecureStorage: { + try { + await this.saveStringToSecureStorage( + userId, + this.refreshTokenSecureStorageKey, + refreshToken, + ); - // TODO: PM-6408 - https://bitwarden.atlassian.net/browse/PM-6408 - // 2024-02-20: Remove refresh token from memory and disk so that we migrate to secure storage over time. - // Remove these 2 calls to remove the refresh token from memory and disk after 3 releases. - await this.singleUserStateProvider.get(userId, REFRESH_TOKEN_DISK).update((_) => null); - await this.singleUserStateProvider.get(userId, REFRESH_TOKEN_MEMORY).update((_) => null); + // Check if the refresh token was able to be saved to secure storage by reading it + // immediately after setting it. This is needed due to intermittent silent failures on Windows 10/11. + const refreshTokenSecureStorage = await this.getStringFromSecureStorage( + userId, + this.refreshTokenSecureStorageKey, + ); + + // Only throw if the refresh token was not saved to secure storage + // If we only check for a nullish value out of secure storage without considering the input value, + // then we would end up falling back to disk storage if the input value was null. + if (refreshToken !== null && !refreshTokenSecureStorage) { + throw new Error("Refresh token failed to save to secure storage."); + } + + // TODO: PM-6408 + // 2024-02-20: Remove refresh token from memory and disk so that we migrate to secure storage over time. + // Remove these 2 calls to remove the refresh token from memory and disk after 3 months. + await this.singleUserStateProvider.get(userId, REFRESH_TOKEN_DISK).update((_) => null); + await this.singleUserStateProvider.get(userId, REFRESH_TOKEN_MEMORY).update((_) => null); + } catch (error) { + // This case could be hit for both Linux users who don't have secure storage configured + // or for Windows users who have intermittent issues with secure storage. + this.logService.error( + `SetRefreshToken: storing refresh token in secure storage failed. Falling back to disk storage.`, + error, + ); + + // Fall back to disk storage for refresh token + await this.singleUserStateProvider + .get(userId, REFRESH_TOKEN_DISK) + .update((_) => refreshToken); + } return; - + } case TokenStorageLocation.Disk: await this.singleUserStateProvider .get(userId, REFRESH_TOKEN_DISK) @@ -485,11 +565,11 @@ export class TokenService implements TokenServiceAbstraction { } } - async getRefreshToken(userId?: UserId): Promise { + async getRefreshToken(userId?: UserId): Promise { userId ??= await firstValueFrom(this.activeUserIdGlobalState.state$); if (!userId) { - return undefined; + return null; } // pre-secure storage migration: @@ -507,17 +587,30 @@ export class TokenService implements TokenServiceAbstraction { const refreshTokenDisk = await this.getStateValueByUserIdAndKeyDef(userId, REFRESH_TOKEN_DISK); if (refreshTokenDisk != null) { + // This handles the scenario pre-secure storage migration where the refresh token was stored on disk. return refreshTokenDisk; } if (this.platformSupportsSecureStorage) { - const refreshTokenSecureStorage = await this.getStringFromSecureStorage( - userId, - this.refreshTokenSecureStorageKey, - ); + try { + const refreshTokenSecureStorage = await this.getStringFromSecureStorage( + userId, + this.refreshTokenSecureStorageKey, + ); - if (refreshTokenSecureStorage != null) { - return refreshTokenSecureStorage; + if (refreshTokenSecureStorage != null) { + return refreshTokenSecureStorage; + } + + this.logService.error( + "Refresh token not found in secure storage. Access token will fail to refresh upon expiration or manual refresh.", + ); + } catch (error) { + // This case will be hit for Linux users who don't have secure storage configured. + + this.logService.error(`Failed to retrieve refresh token from secure storage`, error); + + await this.logoutCallback("refreshTokenSecureStorageRetrievalFailure", userId); } } diff --git a/libs/common/src/platform/models/domain/enc-string.spec.ts b/libs/common/src/platform/models/domain/enc-string.spec.ts index 4275c62350..7583c37e1e 100644 --- a/libs/common/src/platform/models/domain/enc-string.spec.ts +++ b/libs/common/src/platform/models/domain/enc-string.spec.ts @@ -179,6 +179,43 @@ describe("EncString", () => { it("is false if invalid", () => { expect(EncString.isSerializedEncString("2.iv|data")).toBe(false); }); + + it("should return false if a null string is passed in", () => { + // Act + const result = EncString.isSerializedEncString(null); + // Assert + expect(result).toBe(false); + }); + + it("should return false if an error is thrown while parsing the string", () => { + // Arrange + const value = "invalid.value"; + // Act + const result = EncString.isSerializedEncString(value); + // Assert + expect(result).toBe(false); + }); + + describe("Access Token Parsing Tests", () => { + const encryptedAccessToken = + "2.rFNYSTJoljn8h6GOSNVYdQ==|4dIp7ONJzC+Kx1ClA+1aIAb7EqCQ4OjnADCYdCPg7BKkdheG+yM62ZiONFk+S6at84M+RnGWWO04aIjinTdJhlhyUmszePNATxIfX60Y+bFKQhlMuCtZpYdEmQDzXVgT43YRbf/6NnN9WzhefLqeMiocwoIJTEpLptb+Zcm7T3MJpkX4dR9w5LUOxUTNFEGd5PlWaI8FBavOkNsrzY5skRK70pvFABET5IDeRlKhi8NwbzvTzkO3SisLRzih+djiz5nEZf0+ujeGAp6P+o7l0mB0sXVsNJzcuE4S9QtHLnx31N6z3mQm5pOgP4EmEOdRIcQGc1p7dL1vXcXtaTJLtfKXoJjJbYT3wplnY9Pf8+2FVxdbM3bRB2yVsnEzgLcf9UchKThQSdOy8+5TO/prDbUt5mDpO4GmRltom5ncda8yJaD3Hw1DO7fa0Xh+kfeByxb1AwBC+GTPfqmo5uqr0J4dZsf9cGlPMTElwR3GYmD60OcQ6iDX36CZZjqqJqBwKSpepDXV39p9G347e6YAAvJenLDKtdjgfWXCMXbkwETbMgYooFDRd60KYsGIXV16UwzJSvczgTY2d+hYb2Cl0lClequaiwcRxLVtW2xau6qoEPjTqJjJi9I0Cs2WNL4LRH96Ir14a3bEtnTvkO1NjN+bQNon+KksaP2BqTbuiAfZbBP/cL4S1Oew4G00PSLZUGV5S1BI0ooJy6e2NLQJlYqfCeKM6RgpvgfOiXlZddVgkkB6lohLjyVvcSZNuKPjs1wZMZ9C76bKb6o39NFK8G3/YScELFf9gkueWjmhcjrs22+xNDn5rxXeedwIkVW9UJVNLc//eGxLfp70y8fNDcyTPRN1UUpqT8+wSz+9ZHl4DLUK0DE2jIveEDke8vi4MK/XLMC/c50rr1NCEuVy6iA3nwiOzVo/GNfeKTpzMcR/D9A0gxkC9GyZ3riSsMQsGNXhZCZLdsFYp0gLiiJxVilMUfyTWaygsNm87GPY3ep3GEHcq/pCuxrpLQQYT3V1j95WJvFxb8dSLiPHb8STR0GOZhe7SquI5LIRmYCFTo+3VBnItYeuin9i2xCIqWz886xIyllHN2BIPILbA1lCOsCsz1BRRGNqtLvmTeVRO8iujsHWBJicVgSI7/dgSJwcdOv2t4TIVtnN1hJkQnz+HZcJ2FYK/VWlo4UQYYoML52sBd1sSz/n8/8hrO2N4X9frHHNCrcxeoyChTKo2cm4rAxHylLbCZYvGt/KIW9x3AFkPBMr7tAc3yq98J0Crna8ukXc3F3uGb5QXLnBi//3zBDN6RCv7ByaFW5G0I+pglBegzeFBqKH8xwfy76B2e2VLFF8rz/r/wQzlumGFypsRhAoGxrkZyzjec/k+RNR0arf7TTX7ymC1cueTnItRDx89veW6WLlF53NpAGqC8GJSp4T2FGIIk01y29j6Ji7GOlQ8BUbyLWYjMfHf3khRzAfr6UC2QgVvKWQTKET4Y/b1nZCnwxeW8wC80GHtYGuarsU+KlsEw4242cjyIN1GobrWaA2GTOedQDEMWUA64McAw5fAvMEEao5DM7i57tMzJHeKfruyMuXYQkBca094vmATjJ/T+kIrWGIcmxCT/Fp2SW1hcxr6Ciwuog84LVfbVlUl2MAj3eC/xqL/5HP6Q3ObD0ld444GV+HSrQUqfIvEIn9gFmalW6TGugyhfROACCogoXbeIr1AyMUNDnl4EWlPl6u7SQvPX+itKyq4qhaK2J0W6f7ElLVQ5GbC2uwARuhXOi7mqEZ5FP0V675C5NPZOl2ZEd6BhmuyhGkmQEtEvw0DCKnbKM7bKMk90Y599DSnuEna4BNFBVjJ7k+BuNhXUKO+iNcDZT0pCQhOKRVLWsaqVff3BsuQ4zMEOVnccJwwAVipwSRyxZi8bF+Wyun6BVI8pz1CBvRMy+6ifmIq2awEL8NnV65hF2jyZDEVwsnrvCyT7MlM8l5C3MhqH/MgMcKqOsUz+P6Jv5sBi4WvojsaHzqxQ6miBHpHhGDpYH5K53LVs36henB/tOUTcg5ZnO4ZM67jjB7Oz7to+QnJsldp5Bdwvi1XD/4jeh/Llezu5/KwwytSHnZG1z6dZA7B8rKwnI+yN2Qnfi70h68jzGZ1xCOFPz9KMorNKP3XLw8x2g9H6lEBXdV95uc/TNw+WTJbvKRawns/DZhM1u/g13lU6JG19cht3dh/DlKRcJpj1AdOAxPiUubTSkhBmdwRj2BTTHrVlF3/9ladTP4s4f6Zj9TtQvR9CREVe7CboGflxDYC+Jww3PU50XLmxQjkuV5MkDAmBVcyFCFOcHhDRoxet4FX9ec0wjNeDpYtkI8B/qUS1Rp+is1jOxr4/ni|pabwMkF/SdYKdDlow4uKxaObrAP0urmv7N7fA9bedec="; + const standardAccessToken = + "eyJhbGciOiJSUzI1NiIsImtpZCI6IkY5NjBFQzY4RThEMTBDMUEzNEE0OUYwODkwQkExQkExMDk4QUIzMjFSUzI1NiIsIng1dCI6Ii1XRHNhT2pSREJvMHBKOElrTG9ib1FtS3N5RSIsInR5cCI6ImF0K2p3dCJ9.eyJpc3MiOiJodHRwOi8vbG9jYWxob3N0IiwibmJmIjoxNzE0NjAwNzI3LCJpYXQiOjE3MTQ2MDA3MjcsImV4cCI6MTcxNDYwNDMyNywic2NvcGUiOlsiYXBpIiwib2ZmbGluZV9hY2Nlc3MiXSwiYW1yIjpbIkFwcGxpY2F0aW9uIl0sImNsaWVudF9pZCI6ImRlc2t0b3AiLCJzdWIiOiJlY2U3MGExMy03MjE2LTQzYzQtOTk3Ny1iMTAzMDE0NmUxZTciLCJhdXRoX3RpbWUiOjE3MTQ2MDA3MjcsImlkcCI6ImJpdHdhcmRlbiIsInByZW1pdW0iOmZhbHNlLCJlbWFpbCI6ImpzbmlkZXJcdTAwMkJsb2NhbEBiaXR3YXJkZW4uY29tIiwiZW1haWxfdmVyaWZpZWQiOmZhbHNlLCJzc3RhbXAiOiJHWTdKQU82NENLS1RLQkI2WkVBVVlMMldPUVU3QVNUMiIsIm5hbWUiOiJKYXJlZCBTbmlkZXIgMSIsIm9yZ293bmVyIjpbIjkyYjQ5OTA4LWI1MTQtNDVhOC1iYWRiLWIxMDMwMTQ4ZmU1MyIsIjM4ZWRlMzIyLWI0YjQtNGJkOC05ZTA5LWIxMDcwMTEyZGMxMSIsImIyZDA3MDI4LWE1ODMtNGMzZS04ZDYwLWIxMDcwMTE5OGMyOSIsImJmOTM0YmEyLTBmZDQtNDlmMi1hOTVlLWIxMDcwMTFmYzllNiIsImMwYjdmNzVkLTAxNWYtNDJjOS1iM2E2LWIxMDgwMTc2MDdjYSJdLCJkZXZpY2UiOiIwYmQzZWFmZC0yYjE3LTRiNGItYmUzNS1kMjIxNTE5MTA1ZmUiLCJqdGkiOiI0MEVGNjlEQ0QyNkI4MERDMkJFQ0UwODZDOTIxNDg5OSJ9.pRaZphZ8pygx3gHMdsKnCHWSBFAvm6gJ5aCLKbXIfx6Zc-LtQ_CkjO17rQaXlE4MwDt_n_wMzA38SDG2WzwjJrF3rWziPJrOMEdMGXLvVHyqxh0gcIiAQXZMYq0PdCYPBSDmsRZUZqg5BXFb9ylZjC0-m-EqDgl-i6OfxaHTPBCosX4_fr4bcyZtAaoaSeY4ZWf_1T8HrEzTlEyYKepHFzWdG3e4pJKHfs4sNGfs0uiW1awMqtRIPYI1n1F--oF5Hkm6jUJOdtrCKU0mKntyF4v7YRxgXdxUDqKw08nkk11vdPFVG87kWhR6ARYBWDp4AASy66YewqGhX7BNaekSTqK7DKxzQ9Adiv4XvmNEz3JO8tQrEFfE_mz9-WZiS6PlUipCxW-UtFp093_gHZh9_xgbuTdaO1u5_8Y42v0V_69v9WgzCGQGEWZ3PPaJsARGDO7FVKdPxP2S2lWIu22gydNHhfDZrOpBGHD1FpByfd5DbhKk0JdhHEPObs8RwNEweK-jlKmQpc_8bnhXFRUeMFrDL2Q2pNrYcDOpF1crIePPcWBk2_YdcWTqYVnGewT0toJ8sGlaAuAe6uOUZkBG3sxkOttkLYQtkxJYqt54gjazJ0N0GxAc0UBUDt0JnuLqk-cuxXiQO2_vHomTf7dilPq8fvqffrtrISxDVZenceg"; + + it("should return false if a non-encrypted string is passed in", () => { + // Act + const result = EncString.isSerializedEncString(standardAccessToken); + // Assert + expect(result).toBe(false); + }); + + it("should return true if an encrypted string is passed in", () => { + // Act + const result = EncString.isSerializedEncString(encryptedAccessToken); + // Assert + expect(result).toBe(true); + }); + }); }); it("valid", () => { diff --git a/libs/common/src/platform/models/domain/enc-string.ts b/libs/common/src/platform/models/domain/enc-string.ts index 5b99c216ad..1f4c8caf39 100644 --- a/libs/common/src/platform/models/domain/enc-string.ts +++ b/libs/common/src/platform/models/domain/enc-string.ts @@ -76,6 +76,7 @@ export class EncString implements Encrypted { } const { encType, encPieces } = EncString.parseEncryptedString(this.encryptedString); + this.encryptionType = encType; if (encPieces.length !== EXPECTED_NUM_PARTS_BY_ENCRYPTION_TYPE[encType]) { @@ -120,7 +121,7 @@ export class EncString implements Encrypted { encType = parseInt(headerPieces[0], null); encPieces = headerPieces[1].split("|"); } catch (e) { - return; + return { encType: NaN, encPieces: [] }; } } else { encPieces = encryptedString.split("|"); @@ -137,8 +138,16 @@ export class EncString implements Encrypted { } static isSerializedEncString(s: string): boolean { + if (s == null) { + return false; + } + const { encType, encPieces } = this.parseEncryptedString(s); + if (isNaN(encType) || encPieces.length === 0) { + return false; + } + return EXPECTED_NUM_PARTS_BY_ENCRYPTION_TYPE[encType] === encPieces.length; } diff --git a/libs/common/src/services/api.service.ts b/libs/common/src/services/api.service.ts index 8d7a53ec0e..bae9a34c10 100644 --- a/libs/common/src/services/api.service.ts +++ b/libs/common/src/services/api.service.ts @@ -1,5 +1,7 @@ import { firstValueFrom } from "rxjs"; +import { LogoutReason } from "@bitwarden/auth/common"; + import { ApiService as ApiServiceAbstraction } from "../abstractions/api.service"; import { VaultTimeoutSettingsService } from "../abstractions/vault-timeout/vault-timeout-settings.service"; import { OrganizationConnectionType } from "../admin-console/enums"; @@ -116,6 +118,7 @@ import { ProfileResponse } from "../models/response/profile.response"; import { UserKeyResponse } from "../models/response/user-key.response"; import { AppIdService } from "../platform/abstractions/app-id.service"; import { EnvironmentService } from "../platform/abstractions/environment.service"; +import { LogService } from "../platform/abstractions/log.service"; import { PlatformUtilsService } from "../platform/abstractions/platform-utils.service"; import { Utils } from "../platform/misc/utils"; import { UserId } from "../types/guid"; @@ -157,8 +160,10 @@ export class ApiService implements ApiServiceAbstraction { private platformUtilsService: PlatformUtilsService, private environmentService: EnvironmentService, private appIdService: AppIdService, + private refreshAccessTokenErrorCallback: () => void, + private logService: LogService, + private logoutCallback: (logoutReason: LogoutReason) => Promise, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, - private logoutCallback: (expired: boolean) => Promise, private customUserAgent: string = null, ) { this.device = platformUtilsService.getDevice(); @@ -247,7 +252,8 @@ export class ApiService implements ApiServiceAbstraction { try { await this.doAuthRefresh(); } catch (e) { - return Promise.reject(null); + this.logService.error("Error refreshing access token: ", e); + throw e; } } @@ -1720,7 +1726,9 @@ export class ApiService implements ApiServiceAbstraction { return this.doApiTokenRefresh(); } - throw new Error("Cannot refresh token, no refresh token or api keys are stored"); + this.refreshAccessTokenErrorCallback(); + + throw new Error("Cannot refresh access token, no refresh token or api keys are stored."); } protected async doRefreshToken(): Promise { @@ -1905,7 +1913,7 @@ export class ApiService implements ApiServiceAbstraction { responseJson != null && responseJson.error === "invalid_grant") ) { - await this.logoutCallback(true); + await this.logoutCallback("invalidGrantError"); return null; } } diff --git a/libs/common/src/services/notifications.service.ts b/libs/common/src/services/notifications.service.ts index 7dc54b849f..cae6fedbb8 100644 --- a/libs/common/src/services/notifications.service.ts +++ b/libs/common/src/services/notifications.service.ts @@ -2,6 +2,8 @@ import * as signalR from "@microsoft/signalr"; import * as signalRMsgPack from "@microsoft/signalr-protocol-msgpack"; import { firstValueFrom } from "rxjs"; +import { LogoutReason } from "@bitwarden/auth/common"; + import { AuthRequestServiceAbstraction } from "../../../auth/src/common/abstractions"; import { ApiService } from "../abstractions/api.service"; import { NotificationsService as NotificationsServiceAbstraction } from "../abstractions/notifications.service"; @@ -36,7 +38,7 @@ export class NotificationsService implements NotificationsServiceAbstraction { private appIdService: AppIdService, private apiService: ApiService, private environmentService: EnvironmentService, - private logoutCallback: (expired: boolean) => Promise, + private logoutCallback: (logoutReason: LogoutReason) => Promise, private stateService: StateService, private authService: AuthService, private authRequestService: AuthRequestServiceAbstraction, @@ -188,7 +190,7 @@ export class NotificationsService implements NotificationsServiceAbstraction { if (isAuthenticated) { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.logoutCallback(true); + this.logoutCallback("logoutNotification"); } break; case NotificationType.SyncSendCreate: diff --git a/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts b/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts index 51fca6e666..6a8071af0c 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts @@ -1,6 +1,8 @@ import { MockProxy, any, mock } from "jest-mock-extended"; import { BehaviorSubject, from, of } from "rxjs"; +import { LogoutReason } from "@bitwarden/auth/common"; + import { FakeAccountService, mockAccountServiceWith } from "../../../spec/fake-account-service"; import { SearchService } from "../../abstractions/search.service"; import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; @@ -36,7 +38,7 @@ describe("VaultTimeoutService", () => { let vaultTimeoutSettingsService: MockProxy; let stateEventRunnerService: MockProxy; let lockedCallback: jest.Mock, [userId: string]>; - let loggedOutCallback: jest.Mock, [expired: boolean, userId?: string]>; + let loggedOutCallback: jest.Mock, [logoutReason: LogoutReason, userId?: string]>; let vaultTimeoutActionSubject: BehaviorSubject; let availableVaultTimeoutActionsSubject: BehaviorSubject; @@ -190,7 +192,7 @@ describe("VaultTimeoutService", () => { }; const expectUserToHaveLoggedOut = (userId: string) => { - expect(loggedOutCallback).toHaveBeenCalledWith(false, userId); + expect(loggedOutCallback).toHaveBeenCalledWith("vaultTimeout", userId); }; const expectNoAction = (userId: string) => { diff --git a/libs/common/src/services/vault-timeout/vault-timeout.service.ts b/libs/common/src/services/vault-timeout/vault-timeout.service.ts index 5d07c0ebd3..8d8ecd68a5 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout.service.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout.service.ts @@ -1,5 +1,7 @@ import { combineLatest, filter, firstValueFrom, map, switchMap, timeout } from "rxjs"; +import { LogoutReason } from "@bitwarden/auth/common"; + import { SearchService } from "../../abstractions/search.service"; import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; import { VaultTimeoutService as VaultTimeoutServiceAbstraction } from "../../abstractions/vault-timeout/vault-timeout.service"; @@ -34,7 +36,10 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { private vaultTimeoutSettingsService: VaultTimeoutSettingsService, private stateEventRunnerService: StateEventRunnerService, private lockedCallback: (userId?: string) => Promise = null, - private loggedOutCallback: (expired: boolean, userId?: string) => Promise = null, + private loggedOutCallback: ( + logoutReason: LogoutReason, + userId?: string, + ) => Promise = null, ) {} async init(checkOnInterval: boolean) { @@ -145,7 +150,7 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { async logOut(userId?: string): Promise { if (this.loggedOutCallback != null) { - await this.loggedOutCallback(false, userId); + await this.loggedOutCallback("vaultTimeout", userId); } } diff --git a/libs/common/src/vault/services/sync/sync.service.ts b/libs/common/src/vault/services/sync/sync.service.ts index 2c69f83b05..109ecea035 100644 --- a/libs/common/src/vault/services/sync/sync.service.ts +++ b/libs/common/src/vault/services/sync/sync.service.ts @@ -1,6 +1,6 @@ import { firstValueFrom } from "rxjs"; -import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; +import { LogoutReason, UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { ApiService } from "../../../abstractions/api.service"; import { InternalOrganizationServiceAbstraction } from "../../../admin-console/abstractions/organization/organization.service.abstraction"; @@ -32,6 +32,7 @@ import { SendData } from "../../../tools/send/models/data/send.data"; import { SendResponse } from "../../../tools/send/models/response/send.response"; import { SendApiService } from "../../../tools/send/services/send-api.service.abstraction"; import { InternalSendService } from "../../../tools/send/services/send.service.abstraction"; +import { UserId } from "../../../types/guid"; import { CipherService } from "../../../vault/abstractions/cipher.service"; import { FolderApiServiceAbstraction } from "../../../vault/abstractions/folder/folder-api.service.abstraction"; import { InternalFolderService } from "../../../vault/abstractions/folder/folder.service.abstraction"; @@ -65,7 +66,7 @@ export class SyncService extends CoreSyncService { sendApiService: SendApiService, private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private avatarService: AvatarService, - private logoutCallback: (expired: boolean) => Promise, + private logoutCallback: (logoutReason: LogoutReason, userId?: UserId) => Promise, private billingAccountProfileStateService: BillingAccountProfileStateService, private tokenService: TokenService, authService: AuthService, @@ -147,7 +148,7 @@ export class SyncService extends CoreSyncService { const response = await this.apiService.getAccountRevisionDate(); if (response < 0 && this.logoutCallback) { // Account was deleted, log out now - await this.logoutCallback(false); + await this.logoutCallback("accountDeleted"); } if (new Date(response) <= lastSync) { @@ -160,7 +161,7 @@ export class SyncService extends CoreSyncService { const stamp = await this.tokenService.getSecurityStamp(response.id); if (stamp != null && stamp !== response.securityStamp) { if (this.logoutCallback != null) { - await this.logoutCallback(true); + await this.logoutCallback("invalidSecurityStamp"); } throw new Error("Stamp has changed"); diff --git a/libs/components/src/toast/toast.service.ts b/libs/components/src/toast/toast.service.ts index 8bbff02c41..974e7546e1 100644 --- a/libs/components/src/toast/toast.service.ts +++ b/libs/components/src/toast/toast.service.ts @@ -18,7 +18,7 @@ export type ToastOptions = { export class ToastService { constructor(private toastrService: ToastrService) {} - showToast(options: ToastOptions) { + showToast(options: ToastOptions): void { const toastrConfig: Partial = { payload: { message: options.message,