diff --git a/apps/browser/src/auth/background/service-factories/auth-request-service.factory.ts b/apps/browser/src/auth/background/service-factories/auth-request-service.factory.ts index 295fedbadd..c18fd1a112 100644 --- a/apps/browser/src/auth/background/service-factories/auth-request-service.factory.ts +++ b/apps/browser/src/auth/background/service-factories/auth-request-service.factory.ts @@ -17,6 +17,10 @@ import { FactoryOptions, factory, } from "../../../platform/background/service-factories/factory-options"; +import { + stateProviderFactory, + StateProviderInitOptions, +} from "../../../platform/background/service-factories/state-provider.factory"; import { accountServiceFactory, AccountServiceInitOptions } from "./account-service.factory"; import { @@ -31,7 +35,8 @@ export type AuthRequestServiceInitOptions = AuthRequestServiceFactoryOptions & AccountServiceInitOptions & MasterPasswordServiceInitOptions & CryptoServiceInitOptions & - ApiServiceInitOptions; + ApiServiceInitOptions & + StateProviderInitOptions; export function authRequestServiceFactory( cache: { authRequestService?: AuthRequestServiceAbstraction } & CachedServices, @@ -48,6 +53,7 @@ export function authRequestServiceFactory( await internalMasterPasswordServiceFactory(cache, opts), await cryptoServiceFactory(cache, opts), await apiServiceFactory(cache, opts), + await stateProviderFactory(cache, opts), ), ); } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 4aecf8f585..105e7e2a38 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -596,6 +596,7 @@ export default class MainBackground { this.masterPasswordService, this.cryptoService, this.apiService, + this.stateProvider, ); this.authService = new AuthService( @@ -844,6 +845,7 @@ export default class MainBackground { logoutCallback, this.stateService, this.authService, + this.authRequestService, this.messagingService, ); diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index ebae308a81..4228eba965 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -483,6 +483,7 @@ export class Main { this.masterPasswordService, this.cryptoService, this.apiService, + this.stateProvider, ); this.billingAccountProfileStateService = new DefaultBillingAccountProfileStateService( diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index 07fcc5d3b8..5f59530d8c 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -3,6 +3,7 @@ import { FormBuilder } from "@angular/forms"; import { BehaviorSubject, firstValueFrom, Observable, Subject } from "rxjs"; import { concatMap, debounceTime, filter, map, switchMap, takeUntil, tap } from "rxjs/operators"; +import { AuthRequestServiceAbstraction } from "@bitwarden/auth/common"; import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; @@ -20,6 +21,7 @@ import { BiometricStateService } from "@bitwarden/common/platform/biometrics/bio import { ThemeType, KeySuffixOptions } from "@bitwarden/common/platform/enums"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { ThemeStateService } from "@bitwarden/common/platform/theming/theme-state.service"; +import { UserId } from "@bitwarden/common/types/guid"; import { DialogService } from "@bitwarden/components"; import { SetPinComponent } from "../../auth/components/set-pin.component"; @@ -60,6 +62,7 @@ export class SettingsComponent implements OnInit { showAppPreferences = true; currentUserEmail: string; + currentUserId: UserId; availableVaultTimeoutActions$: Observable; vaultTimeoutPolicyCallout: Observable<{ @@ -122,6 +125,7 @@ export class SettingsComponent implements OnInit { private desktopSettingsService: DesktopSettingsService, private biometricStateService: BiometricStateService, private desktopAutofillSettingsService: DesktopAutofillSettingsService, + private authRequestService: AuthRequestServiceAbstraction, ) { const isMac = this.platformUtilsService.getDevice() === DeviceType.MacOsDesktop; @@ -207,6 +211,7 @@ export class SettingsComponent implements OnInit { return; } this.currentUserEmail = await this.stateService.getEmail(); + this.currentUserId = (await this.stateService.getUserId()) as UserId; this.availableVaultTimeoutActions$ = this.refreshTimeoutSettings$.pipe( switchMap(() => this.vaultTimeoutSettingsService.availableVaultTimeoutActions$()), @@ -249,7 +254,8 @@ export class SettingsComponent implements OnInit { requirePasswordOnStart: await firstValueFrom( this.biometricStateService.requirePasswordOnStart$, ), - approveLoginRequests: (await this.stateService.getApproveLoginRequests()) ?? false, + approveLoginRequests: + (await this.authRequestService.getAcceptAuthRequests(this.currentUserId)) ?? false, clearClipboard: await firstValueFrom(this.autofillSettingsService.clearClipboardDelay$), minimizeOnCopyToClipboard: await this.stateService.getMinimizeOnCopyToClipboard(), enableFavicons: await firstValueFrom(this.domainSettingsService.showFavicons$), @@ -665,7 +671,10 @@ export class SettingsComponent implements OnInit { } async updateApproveLoginRequests() { - await this.stateService.setApproveLoginRequests(this.form.value.approveLoginRequests); + await this.authRequestService.setAcceptAuthRequests( + this.form.value.approveLoginRequests, + this.currentUserId, + ); } ngOnDestroy() { diff --git a/apps/desktop/src/vault/app/vault/vault.component.ts b/apps/desktop/src/vault/app/vault/vault.component.ts index e8aabbb20f..208bbc70f0 100644 --- a/apps/desktop/src/vault/app/vault/vault.component.ts +++ b/apps/desktop/src/vault/app/vault/vault.component.ts @@ -8,7 +8,7 @@ import { ViewContainerRef, } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; -import { Subject, takeUntil } from "rxjs"; +import { firstValueFrom, Subject, takeUntil } from "rxjs"; import { first } from "rxjs/operators"; import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref"; @@ -16,13 +16,13 @@ import { ModalService } from "@bitwarden/angular/services/modal.service"; import { VaultFilter } from "@bitwarden/angular/vault/vault-filter/models/vault-filter.model"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EventType } from "@bitwarden/common/enums"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service"; import { CipherType } from "@bitwarden/common/vault/enums"; @@ -32,6 +32,7 @@ import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { DialogService } from "@bitwarden/components"; import { PasswordRepromptService } from "@bitwarden/vault"; +import { AuthRequestServiceAbstraction } from "../../../../../../libs/auth/src/common/abstractions"; import { SearchBarService } from "../../../app/layout/search/search-bar.service"; import { GeneratorComponent } from "../../../app/tools/generator.component"; import { invokeMenu, RendererMenuItem } from "../../../utils"; @@ -102,11 +103,12 @@ export class VaultComponent implements OnInit, OnDestroy { private eventCollectionService: EventCollectionService, private totpService: TotpService, private passwordRepromptService: PasswordRepromptService, - private stateService: StateService, private searchBarService: SearchBarService, private apiService: ApiService, private dialogService: DialogService, private billingAccountProfileStateService: BillingAccountProfileStateService, + private authRequestService: AuthRequestServiceAbstraction, + private accountService: AccountService, ) {} async ngOnInit() { @@ -224,7 +226,8 @@ export class VaultComponent implements OnInit, OnDestroy { this.searchBarService.setEnabled(true); this.searchBarService.setPlaceholderText(this.i18nService.t("searchVault")); - const approveLoginRequests = await this.stateService.getApproveLoginRequests(); + const userId = (await firstValueFrom(this.accountService.activeAccount$)).id; + const approveLoginRequests = await this.authRequestService.getAcceptAuthRequests(userId); if (approveLoginRequests) { const authRequest = await this.apiService.getLastAuthRequest(); if (authRequest != null) { diff --git a/libs/angular/src/auth/components/login-via-auth-request.component.ts b/libs/angular/src/auth/components/login-via-auth-request.component.ts index 6ba94d3001..5a1180cd38 100644 --- a/libs/angular/src/auth/components/login-via-auth-request.component.ts +++ b/libs/angular/src/auth/components/login-via-auth-request.component.ts @@ -33,6 +33,7 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.serv import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; +import { UserId } from "@bitwarden/common/types/guid"; import { CaptchaProtectedComponent } from "./captcha-protected.component"; @@ -131,6 +132,7 @@ export class LoginViaAuthRequestComponent // This also prevents it from being lost on refresh as the // login service email does not persist. this.email = await this.stateService.getEmail(); + const userId = (await firstValueFrom(this.accountService.activeAccount$)).id; if (!this.email) { this.platformUtilsService.showToast("error", null, this.i18nService.t("userEmailMissing")); @@ -142,10 +144,10 @@ export class LoginViaAuthRequestComponent // We only allow a single admin approval request to be active at a time // so must check state to see if we have an existing one or not - const adminAuthReqStorable = await this.stateService.getAdminAuthRequest(); + const adminAuthReqStorable = await this.authRequestService.getAdminAuthRequest(userId); if (adminAuthReqStorable) { - await this.handleExistingAdminAuthRequest(adminAuthReqStorable); + await this.handleExistingAdminAuthRequest(adminAuthReqStorable, userId); } else { // No existing admin auth request; so we need to create one await this.startAuthRequestLogin(); @@ -173,7 +175,10 @@ export class LoginViaAuthRequestComponent this.destroy$.complete(); } - private async handleExistingAdminAuthRequest(adminAuthReqStorable: AdminAuthRequestStorable) { + private async handleExistingAdminAuthRequest( + adminAuthReqStorable: AdminAuthRequestStorable, + userId: UserId, + ) { // Note: on login, the SSOLoginStrategy will also call to see an existing admin auth req // has been approved and handle it if so. @@ -183,13 +188,13 @@ export class LoginViaAuthRequestComponent adminAuthReqResponse = await this.apiService.getAuthRequest(adminAuthReqStorable.id); } catch (error) { if (error instanceof ErrorResponse && error.statusCode === HttpStatusCode.NotFound) { - return await this.handleExistingAdminAuthReqDeletedOrDenied(); + return await this.handleExistingAdminAuthReqDeletedOrDenied(userId); } } // Request doesn't exist anymore if (!adminAuthReqResponse) { - return await this.handleExistingAdminAuthReqDeletedOrDenied(); + return await this.handleExistingAdminAuthReqDeletedOrDenied(userId); } // Re-derive the user's fingerprint phrase @@ -203,7 +208,7 @@ export class LoginViaAuthRequestComponent // Request denied if (adminAuthReqResponse.isAnswered && !adminAuthReqResponse.requestApproved) { - return await this.handleExistingAdminAuthReqDeletedOrDenied(); + return await this.handleExistingAdminAuthReqDeletedOrDenied(userId); } // Request approved @@ -211,6 +216,7 @@ export class LoginViaAuthRequestComponent return await this.handleApprovedAdminAuthRequest( adminAuthReqResponse, adminAuthReqStorable.privateKey, + userId, ); } @@ -219,9 +225,9 @@ export class LoginViaAuthRequestComponent await this.anonymousHubService.createHubConnection(adminAuthReqStorable.id); } - private async handleExistingAdminAuthReqDeletedOrDenied() { + private async handleExistingAdminAuthReqDeletedOrDenied(userId: UserId) { // clear the admin auth request from state - await this.stateService.setAdminAuthRequest(null); + await this.authRequestService.clearAdminAuthRequest(userId); // start new auth request // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. @@ -269,7 +275,8 @@ export class LoginViaAuthRequestComponent privateKey: this.authRequestKeyPair.privateKey, }); - await this.stateService.setAdminAuthRequest(adminAuthReqStorable); + const userId = (await firstValueFrom(this.accountService.activeAccount$)).id; + await this.authRequestService.setAdminAuthRequest(adminAuthReqStorable, userId); } else { await this.buildAuthRequest(AuthRequestType.AuthenticateAndUnlock); reqResponse = await this.apiService.postAuthRequest(this.authRequest); @@ -333,9 +340,11 @@ export class LoginViaAuthRequestComponent // if user has authenticated via SSO if (this.userAuthNStatus === AuthenticationStatus.Locked) { + const userId = (await firstValueFrom(this.accountService.activeAccount$)).id; return await this.handleApprovedAdminAuthRequest( authReqResponse, this.authRequestKeyPair.privateKey, + userId, ); } @@ -363,6 +372,7 @@ export class LoginViaAuthRequestComponent async handleApprovedAdminAuthRequest( adminAuthReqResponse: AuthRequestResponse, privateKey: ArrayBuffer, + userId: UserId, ) { // See verifyAndHandleApprovedAuthReq(...) for flow details // it's flow 2 or 3 based on presence of masterPasswordHash @@ -384,7 +394,7 @@ export class LoginViaAuthRequestComponent // clear the admin auth request from state so it cannot be used again (it's a one time use) // TODO: this should eventually be enforced via deleting this on the server once it is used - await this.stateService.setAdminAuthRequest(null); + await this.authRequestService.clearAdminAuthRequest(userId); this.platformUtilsService.showToast("success", null, this.i18nService.t("loginApproved")); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 3de36020da..b311000fb8 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -740,6 +740,7 @@ const safeProviders: SafeProvider[] = [ LOGOUT_CALLBACK, StateServiceAbstraction, AuthServiceAbstraction, + AuthRequestServiceAbstraction, MessagingServiceAbstraction, ], }), @@ -963,6 +964,7 @@ const safeProviders: SafeProvider[] = [ InternalMasterPasswordServiceAbstraction, CryptoServiceAbstraction, ApiServiceAbstraction, + StateProvider, ], }), safeProvider({ diff --git a/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts b/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts index 7af92fc8f8..b7ae903eac 100644 --- a/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts +++ b/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts @@ -1,12 +1,52 @@ import { Observable } from "rxjs"; +import { AdminAuthRequestStorable } from "@bitwarden/common/auth/models/domain/admin-auth-req-storable"; import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; import { AuthRequestPushNotification } from "@bitwarden/common/models/response/notification.response"; +import { UserId } from "@bitwarden/common/types/guid"; import { UserKey, MasterKey } from "@bitwarden/common/types/key"; export abstract class AuthRequestServiceAbstraction { /** Emits an auth request id when an auth request has been approved. */ authRequestPushNotification$: Observable; + + /** + * Returns true if the user has chosen to allow auth requests to show on this client. + * Intended to prevent spamming the user with auth requests. + * @param userId The user id. + * @throws If `userId` is not provided. + */ + abstract getAcceptAuthRequests: (userId: UserId) => Promise; + /** + * Sets whether to allow auth requests to show on this client for this user. + * @param accept Whether to allow auth requests to show on this client. + * @param userId The user id. + * @throws If `userId` is not provided. + */ + abstract setAcceptAuthRequests: (accept: boolean, userId: UserId) => Promise; + /** + * Returns an admin auth request for the given user if it exists. + * @param userId The user id. + * @throws If `userId` is not provided. + */ + abstract getAdminAuthRequest: (userId: UserId) => Promise; + /** + * Sets an admin auth request for the given user. + * Note: use {@link clearAdminAuthRequest} to clear the request. + * @param authRequest The admin auth request. + * @param userId The user id. + * @throws If `authRequest` or `userId` is not provided. + */ + abstract setAdminAuthRequest: ( + authRequest: AdminAuthRequestStorable, + userId: UserId, + ) => Promise; + /** + * Clears an admin auth request for the given user. + * @param userId The user id. + * @throws If `userId` is not provided. + */ + abstract clearAdminAuthRequest: (userId: UserId) => Promise; /** * Approve or deny an auth request. * @param approve True to approve, false to deny. diff --git a/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts b/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts index e47f0f88ee..4035a7be58 100644 --- a/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts @@ -132,7 +132,10 @@ export class AuthRequestLoginStrategy extends LoginStrategy { } } - protected override async setUserKey(response: IdentityTokenResponse): Promise { + protected override async setUserKey( + response: IdentityTokenResponse, + userId: UserId, + ): Promise { const authRequestCredentials = this.cache.value.authRequestCredentials; // User now may or may not have a master password // but set the master key encrypted user key if it exists regardless @@ -143,7 +146,6 @@ export class AuthRequestLoginStrategy extends LoginStrategy { } else { await this.trySetUserKeyWithMasterKey(); - const userId = (await this.stateService.getUserId()) as UserId; // Establish trust if required after setting user key await this.deviceTrustCryptoService.trustDeviceIfRequired(userId); } diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index df6aa171db..94f96d40d0 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -32,6 +32,7 @@ import { AccountProfile, AccountTokens, } from "@bitwarden/common/platform/models/domain/account"; +import { UserId } from "@bitwarden/common/types/guid"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; import { @@ -160,14 +161,11 @@ export abstract class LoginStrategy { * @param {IdentityTokenResponse} tokenResponse - The response from the server containing the identity token. * @returns {Promise} - A promise that resolves when the account information has been successfully saved. */ - protected async saveAccountInformation(tokenResponse: IdentityTokenResponse): Promise { + protected async saveAccountInformation(tokenResponse: IdentityTokenResponse): Promise { const accountInformation = await this.tokenService.decodeAccessToken(tokenResponse.accessToken); const userId = accountInformation.sub; - // If you don't persist existing admin auth requests on login, they will get deleted. - const adminAuthRequest = await this.stateService.getAdminAuthRequest({ userId }); - const vaultTimeoutAction = await this.stateService.getVaultTimeoutAction(); const vaultTimeout = await this.stateService.getVaultTimeout(); @@ -197,7 +195,6 @@ export abstract class LoginStrategy { tokens: { ...new AccountTokens(), }, - adminAuthRequest: adminAuthRequest?.toJSON(), }), ); @@ -206,6 +203,7 @@ export abstract class LoginStrategy { ); await this.billingAccountProfileStateService.setHasPremium(accountInformation.premium, false); + return userId as UserId; } protected async processTokenResponse(response: IdentityTokenResponse): Promise { @@ -228,7 +226,7 @@ export abstract class LoginStrategy { } // Must come before setting keys, user key needs email to update additional keys - await this.saveAccountInformation(response); + const userId = await this.saveAccountInformation(response); if (response.twoFactorToken != null) { // note: we can read email from access token b/c it was saved in saveAccountInformation @@ -238,7 +236,7 @@ export abstract class LoginStrategy { } await this.setMasterKey(response); - await this.setUserKey(response); + await this.setUserKey(response, userId); await this.setPrivateKey(response); this.messagingService.send("loggedIn"); @@ -248,7 +246,7 @@ export abstract class LoginStrategy { // The keys comes from different sources depending on the login strategy protected abstract setMasterKey(response: IdentityTokenResponse): Promise; - protected abstract setUserKey(response: IdentityTokenResponse): Promise; + protected abstract setUserKey(response: IdentityTokenResponse, userId: UserId): Promise; protected abstract setPrivateKey(response: IdentityTokenResponse): Promise; // Old accounts used master key for encryption. We are forcing migrations but only need to diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.ts b/libs/auth/src/common/login-strategies/password-login.strategy.ts index 52c97d5d85..2490c35a00 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -25,6 +25,7 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.serv import { HashPurpose } from "@bitwarden/common/platform/enums"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; +import { UserId } from "@bitwarden/common/types/guid"; import { MasterKey } from "@bitwarden/common/types/key"; import { LoginStrategyServiceAbstraction } from "../abstractions"; @@ -207,14 +208,16 @@ export class PasswordLoginStrategy extends LoginStrategy { await this.masterPasswordService.setMasterKeyHash(localMasterKeyHash, userId); } - protected override async setUserKey(response: IdentityTokenResponse): Promise { + protected override async setUserKey( + response: IdentityTokenResponse, + userId: UserId, + ): Promise { // If migration is required, we won't have a user key to set yet. if (this.encryptionKeyMigrationRequired(response)) { return; } await this.cryptoService.setMasterKeyEncryptedUserKey(response.key); - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); if (masterKey) { const userKey = await this.cryptoService.decryptUserKeyWithMasterKey(masterKey); diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts index bce62681d0..b78ad6dea6 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts @@ -301,7 +301,7 @@ describe("SsoLoginStrategy", () => { id: "1", privateKey: "PRIVATE" as any, } as AdminAuthRequestStorable; - stateService.getAdminAuthRequest.mockResolvedValue( + authRequestService.getAdminAuthRequest.mockResolvedValue( new AdminAuthRequestStorable(adminAuthRequest), ); }); @@ -364,7 +364,7 @@ describe("SsoLoginStrategy", () => { await ssoLoginStrategy.logIn(credentials); - expect(stateService.setAdminAuthRequest).toHaveBeenCalledWith(null); + expect(authRequestService.clearAdminAuthRequest).toHaveBeenCalled(); expect( authRequestService.setKeysAfterDecryptingSharedMasterKeyAndHash, ).not.toHaveBeenCalled(); diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index db0228a338..d8efd78984 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -216,7 +216,10 @@ export class SsoLoginStrategy extends LoginStrategy { // TODO: future passkey login strategy will need to support setting user key (decrypting via TDE or admin approval request) // so might be worth moving this logic to a common place (base login strategy or a separate service?) - protected override async setUserKey(tokenResponse: IdentityTokenResponse): Promise { + protected override async setUserKey( + tokenResponse: IdentityTokenResponse, + userId: UserId, + ): Promise { const masterKeyEncryptedUserKey = tokenResponse.key; // Note: masterKeyEncryptedUserKey is undefined for SSO JIT provisioned users @@ -232,7 +235,7 @@ export class SsoLoginStrategy extends LoginStrategy { // Note: TDE and key connector are mutually exclusive if (userDecryptionOptions?.trustedDeviceOption) { - await this.trySetUserKeyWithApprovedAdminRequestIfExists(); + await this.trySetUserKeyWithApprovedAdminRequestIfExists(userId); const hasUserKey = await this.cryptoService.hasUserKey(); @@ -252,9 +255,9 @@ export class SsoLoginStrategy extends LoginStrategy { // is responsible for deriving master key from MP entry and then decrypting the user key } - private async trySetUserKeyWithApprovedAdminRequestIfExists(): Promise { + private async trySetUserKeyWithApprovedAdminRequestIfExists(userId: UserId): Promise { // At this point a user could have an admin auth request that has been approved - const adminAuthReqStorable = await this.stateService.getAdminAuthRequest(); + const adminAuthReqStorable = await this.authRequestService.getAdminAuthRequest(userId); if (!adminAuthReqStorable) { return; @@ -268,7 +271,7 @@ export class SsoLoginStrategy extends LoginStrategy { } catch (error) { if (error instanceof ErrorResponse && error.statusCode === HttpStatusCode.NotFound) { // if we get a 404, it means the auth request has been deleted so clear it from storage - await this.stateService.setAdminAuthRequest(null); + await this.authRequestService.clearAdminAuthRequest(userId); } // Always return on an error here as we don't want to block the user from logging in @@ -295,12 +298,11 @@ export class SsoLoginStrategy extends LoginStrategy { if (await this.cryptoService.hasUserKey()) { // Now that we have a decrypted user key in memory, we can check if we // need to establish trust on the current device - const userId = (await this.stateService.getUserId()) as UserId; await this.deviceTrustCryptoService.trustDeviceIfRequired(userId); // if we successfully decrypted the user key, we can delete the admin auth request out of state // TODO: eventually we post and clean up DB as well once consumed on client - await this.stateService.setAdminAuthRequest(null); + await this.authRequestService.clearAdminAuthRequest(userId); this.platformUtilsService.showToast("success", null, this.i18nService.t("loginApproved")); } diff --git a/libs/auth/src/common/login-strategies/user-api-login.strategy.ts b/libs/auth/src/common/login-strategies/user-api-login.strategy.ts index 421746b49c..4a0d005b1c 100644 --- a/libs/auth/src/common/login-strategies/user-api-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/user-api-login.strategy.ts @@ -18,6 +18,7 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { UserId } from "@bitwarden/common/types/guid"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; import { UserApiLoginCredentials } from "../models/domain/login-credentials"; @@ -97,7 +98,10 @@ export class UserApiLoginStrategy extends LoginStrategy { } } - protected override async setUserKey(response: IdentityTokenResponse): Promise { + protected override async setUserKey( + response: IdentityTokenResponse, + userId: UserId, + ): Promise { await this.cryptoService.setMasterKeyEncryptedUserKey(response.key); if (response.apiUseKeyConnector) { @@ -116,8 +120,8 @@ export class UserApiLoginStrategy extends LoginStrategy { ); } - protected async saveAccountInformation(tokenResponse: IdentityTokenResponse) { - await super.saveAccountInformation(tokenResponse); + protected async saveAccountInformation(tokenResponse: IdentityTokenResponse): Promise { + const userId = await super.saveAccountInformation(tokenResponse); const vaultTimeout = await this.stateService.getVaultTimeout(); const vaultTimeoutAction = await this.stateService.getVaultTimeoutAction(); @@ -134,6 +138,7 @@ export class UserApiLoginStrategy extends LoginStrategy { vaultTimeoutAction as VaultTimeoutAction, vaultTimeout, ); + return userId; } exportCache(): CacheData { diff --git a/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts b/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts index 843978e2a2..8a62a8fb3c 100644 --- a/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/webauthn-login.strategy.ts @@ -17,6 +17,7 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { UserId } from "@bitwarden/common/types/guid"; import { UserKey } from "@bitwarden/common/types/key"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions"; @@ -98,7 +99,7 @@ export class WebAuthnLoginStrategy extends LoginStrategy { return Promise.resolve(); } - protected override async setUserKey(idTokenResponse: IdentityTokenResponse) { + protected override async setUserKey(idTokenResponse: IdentityTokenResponse, userId: UserId) { const masterKeyEncryptedUserKey = idTokenResponse.key; if (masterKeyEncryptedUserKey) { diff --git a/libs/auth/src/common/services/auth-request/auth-request.service.spec.ts b/libs/auth/src/common/services/auth-request/auth-request.service.spec.ts index f04628ffd9..5907048684 100644 --- a/libs/auth/src/common/services/auth-request/auth-request.service.spec.ts +++ b/libs/auth/src/common/services/auth-request/auth-request.service.spec.ts @@ -9,6 +9,7 @@ import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.se import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { StateProvider } from "@bitwarden/common/platform/state"; import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; import { MasterKey, UserKey } from "@bitwarden/common/types/key"; @@ -18,6 +19,7 @@ import { AuthRequestService } from "./auth-request.service"; describe("AuthRequestService", () => { let sut: AuthRequestService; + const stateProvider = mock(); let accountService: FakeAccountService; let masterPasswordService: FakeMasterPasswordService; const appIdService = mock(); @@ -38,6 +40,7 @@ describe("AuthRequestService", () => { masterPasswordService, cryptoService, apiService, + stateProvider, ); mockPrivateKey = new Uint8Array(64); @@ -59,6 +62,31 @@ describe("AuthRequestService", () => { }); }); + describe("AcceptAuthRequests", () => { + it("returns an error when userId isn't provided", async () => { + await expect(sut.getAcceptAuthRequests(undefined)).rejects.toThrow("User ID is required"); + await expect(sut.setAcceptAuthRequests(true, undefined)).rejects.toThrow( + "User ID is required", + ); + }); + }); + + describe("AdminAuthRequest", () => { + it("returns an error when userId isn't provided", async () => { + await expect(sut.getAdminAuthRequest(undefined)).rejects.toThrow("User ID is required"); + await expect(sut.setAdminAuthRequest(undefined, undefined)).rejects.toThrow( + "User ID is required", + ); + await expect(sut.clearAdminAuthRequest(undefined)).rejects.toThrow("User ID is required"); + }); + + it("does not allow clearing from setAdminAuthRequest", async () => { + await expect(sut.setAdminAuthRequest(null, "USER_ID" as UserId)).rejects.toThrow( + "Auth request is required", + ); + }); + }); + describe("approveOrDenyAuthRequest", () => { beforeEach(() => { cryptoService.rsaEncrypt.mockResolvedValue({ diff --git a/libs/auth/src/common/services/auth-request/auth-request.service.ts b/libs/auth/src/common/services/auth-request/auth-request.service.ts index 5f8dcfd729..062a10af14 100644 --- a/libs/auth/src/common/services/auth-request/auth-request.service.ts +++ b/libs/auth/src/common/services/auth-request/auth-request.service.ts @@ -1,8 +1,10 @@ -import { firstValueFrom, Observable, Subject } from "rxjs"; +import { Observable, Subject, firstValueFrom } from "rxjs"; +import { Jsonify } from "type-fest"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; +import { AdminAuthRequestStorable } from "@bitwarden/common/auth/models/domain/admin-auth-req-storable"; import { PasswordlessAuthRequest } from "@bitwarden/common/auth/models/request/passwordless-auth.request"; import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; import { AuthRequestPushNotification } from "@bitwarden/common/models/response/notification.response"; @@ -10,10 +12,43 @@ import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.ser import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { + AUTH_REQUEST_DISK_LOCAL, + StateProvider, + UserKeyDefinition, +} from "@bitwarden/common/platform/state"; +import { UserId } from "@bitwarden/common/types/guid"; import { MasterKey, UserKey } from "@bitwarden/common/types/key"; import { AuthRequestServiceAbstraction } from "../../abstractions/auth-request.service.abstraction"; +/** + * Disk-local to maintain consistency between tabs (even though + * approvals are currently only available on desktop). We don't + * want to clear this on logout as it's a user preference. + */ +export const ACCEPT_AUTH_REQUESTS_KEY = new UserKeyDefinition( + AUTH_REQUEST_DISK_LOCAL, + "acceptAuthRequests", + { + deserializer: (value) => value ?? false, + clearOn: [], + }, +); + +/** + * Disk-local to maintain consistency between tabs. We don't want to + * clear this on logout since admin auth requests are long-lived. + */ +export const ADMIN_AUTH_REQUEST_KEY = new UserKeyDefinition>( + AUTH_REQUEST_DISK_LOCAL, + "adminAuthRequest", + { + deserializer: (value) => value, + clearOn: [], + }, +); + export class AuthRequestService implements AuthRequestServiceAbstraction { private authRequestPushNotificationSubject = new Subject(); authRequestPushNotification$: Observable; @@ -24,10 +59,61 @@ export class AuthRequestService implements AuthRequestServiceAbstraction { private masterPasswordService: InternalMasterPasswordServiceAbstraction, private cryptoService: CryptoService, private apiService: ApiService, + private stateProvider: StateProvider, ) { this.authRequestPushNotification$ = this.authRequestPushNotificationSubject.asObservable(); } + async getAcceptAuthRequests(userId: UserId): Promise { + if (userId == null) { + throw new Error("User ID is required"); + } + + const value = await firstValueFrom( + this.stateProvider.getUser(userId, ACCEPT_AUTH_REQUESTS_KEY).state$, + ); + return value; + } + + async setAcceptAuthRequests(accept: boolean, userId: UserId): Promise { + if (userId == null) { + throw new Error("User ID is required"); + } + + await this.stateProvider.setUserState(ACCEPT_AUTH_REQUESTS_KEY, accept, userId); + } + + async getAdminAuthRequest(userId: UserId): Promise { + if (userId == null) { + throw new Error("User ID is required"); + } + + const authRequestSerialized = await firstValueFrom( + this.stateProvider.getUser(userId, ADMIN_AUTH_REQUEST_KEY).state$, + ); + const adminAuthRequestStorable = AdminAuthRequestStorable.fromJSON(authRequestSerialized); + return adminAuthRequestStorable; + } + + async setAdminAuthRequest(authRequest: AdminAuthRequestStorable, userId: UserId): Promise { + if (userId == null) { + throw new Error("User ID is required"); + } + if (authRequest == null) { + throw new Error("Auth request is required"); + } + + await this.stateProvider.setUserState(ADMIN_AUTH_REQUEST_KEY, authRequest.toJSON(), userId); + } + + async clearAdminAuthRequest(userId: UserId): Promise { + if (userId == null) { + throw new Error("User ID is required"); + } + + await this.stateProvider.setUserState(ADMIN_AUTH_REQUEST_KEY, null, userId); + } + async approveOrDenyAuthRequest( approve: boolean, authRequest: AuthRequestResponse, diff --git a/libs/common/src/auth/models/domain/admin-auth-req-storable.ts b/libs/common/src/auth/models/domain/admin-auth-req-storable.ts index 1eae7eeab1..df0341ac16 100644 --- a/libs/common/src/auth/models/domain/admin-auth-req-storable.ts +++ b/libs/common/src/auth/models/domain/admin-auth-req-storable.ts @@ -1,11 +1,7 @@ +import { Jsonify } from "type-fest"; + import { Utils } from "../../../platform/misc/utils"; -// TODO: Tech Debt: potentially create a type Storage shape vs using a class here in the future -// type StorageShape { -// id: string; -// privateKey: string; -// } -// so we can get rid of the any type passed into fromJSON and coming out of ToJSON export class AdminAuthRequestStorable { id: string; privateKey: Uint8Array; @@ -23,7 +19,7 @@ export class AdminAuthRequestStorable { }; } - static fromJSON(obj: any): AdminAuthRequestStorable { + static fromJSON(obj: Jsonify): AdminAuthRequestStorable { if (obj == null) { return null; } diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 3017ae7195..fd76cad6d1 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -1,6 +1,5 @@ import { Observable } from "rxjs"; -import { AdminAuthRequestStorable } from "../../auth/models/domain/admin-auth-req-storable"; import { KdfConfig } from "../../auth/models/domain/kdf-config"; import { BiometricKey } from "../../auth/types/biometric-key"; import { GeneratorOptions } from "../../tools/generator/generator-options"; @@ -124,11 +123,6 @@ export abstract class StateService { setDecryptedPinProtected: (value: EncString, options?: StorageOptions) => Promise; getDuckDuckGoSharedKey: (options?: StorageOptions) => Promise; setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise; - getAdminAuthRequest: (options?: StorageOptions) => Promise; - setAdminAuthRequest: ( - adminAuthRequest: AdminAuthRequestStorable, - options?: StorageOptions, - ) => Promise; getEmail: (options?: StorageOptions) => Promise; setEmail: (value: string, options?: StorageOptions) => Promise; getEmailVerified: (options?: StorageOptions) => Promise; @@ -207,7 +201,5 @@ export abstract class StateService { setVaultTimeout: (value: number, options?: StorageOptions) => Promise; getVaultTimeoutAction: (options?: StorageOptions) => Promise; setVaultTimeoutAction: (value: string, options?: StorageOptions) => Promise; - getApproveLoginRequests: (options?: StorageOptions) => Promise; - setApproveLoginRequests: (value: boolean, options?: StorageOptions) => Promise; nextUpActiveUser: () => Promise; } diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 753b15c09b..759a903514 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -1,6 +1,5 @@ import { Jsonify } from "type-fest"; -import { AdminAuthRequestStorable } from "../../../auth/models/domain/admin-auth-req-storable"; import { UriMatchStrategySetting } from "../../../models/domain/domain-service"; import { GeneratorOptions } from "../../../tools/generator/generator-options"; import { @@ -169,7 +168,6 @@ export class AccountSettings { protectedPin?: string; vaultTimeout?: number; vaultTimeoutAction?: string = "lock"; - approveLoginRequests?: boolean; /** @deprecated July 2023, left for migration purposes*/ pinProtected?: EncryptionPair = new EncryptionPair(); @@ -206,7 +204,6 @@ export class Account { profile?: AccountProfile = new AccountProfile(); settings?: AccountSettings = new AccountSettings(); tokens?: AccountTokens = new AccountTokens(); - adminAuthRequest?: Jsonify = null; constructor(init: Partial) { Object.assign(this, { @@ -230,7 +227,6 @@ export class Account { ...new AccountTokens(), ...init?.tokens, }, - adminAuthRequest: init?.adminAuthRequest, }); } @@ -245,7 +241,6 @@ export class Account { profile: AccountProfile.fromJSON(json?.profile), settings: AccountSettings.fromJSON(json?.settings), tokens: AccountTokens.fromJSON(json?.tokens), - adminAuthRequest: AdminAuthRequestStorable.fromJSON(json?.adminAuthRequest), }); } } diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 7dbac2b02a..3d512175a8 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -3,7 +3,6 @@ import { Jsonify, JsonValue } from "type-fest"; import { AccountService } from "../../auth/abstractions/account.service"; import { TokenService } from "../../auth/abstractions/token.service"; -import { AdminAuthRequestStorable } from "../../auth/models/domain/admin-auth-req-storable"; import { KdfConfig } from "../../auth/models/domain/kdf-config"; import { BiometricKey } from "../../auth/types/biometric-key"; import { GeneratorOptions } from "../../tools/generator/generator-options"; @@ -548,37 +547,6 @@ export class StateService< : await this.secureStorageService.save(DDG_SHARED_KEY, value, options); } - async getAdminAuthRequest(options?: StorageOptions): Promise { - options = this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()); - - if (options?.userId == null) { - return null; - } - - const account = await this.getAccount(options); - - return account?.adminAuthRequest - ? AdminAuthRequestStorable.fromJSON(account.adminAuthRequest) - : null; - } - - async setAdminAuthRequest( - adminAuthRequest: AdminAuthRequestStorable, - options?: StorageOptions, - ): Promise { - options = this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()); - - if (options?.userId == null) { - return; - } - - const account = await this.getAccount(options); - - account.adminAuthRequest = adminAuthRequest?.toJSON(); - - await this.saveAccount(account, options); - } - async getEmail(options?: StorageOptions): Promise { return ( await this.getAccount(this.reconcileOptions(options, await this.defaultInMemoryOptions())) @@ -1032,24 +1000,6 @@ export class StateService< ); } - async getApproveLoginRequests(options?: StorageOptions): Promise { - const approveLoginRequests = ( - await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskLocalOptions())) - )?.settings?.approveLoginRequests; - return approveLoginRequests; - } - - async setApproveLoginRequests(value: boolean, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()), - ); - account.settings.approveLoginRequests = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()), - ); - } - protected async getGlobals(options: StorageOptions): Promise { let globals: TGlobalState; if (this.useMemory(options.storageLocation)) { @@ -1392,7 +1342,6 @@ export class StateService< protected resetAccount(account: TAccount) { const persistentAccountInformation = { settings: account.settings, - adminAuthRequest: account.adminAuthRequest, }; return Object.assign(this.createAccount(), persistentAccountInformation); } diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index ed6ef1590d..518e5c51d6 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -45,6 +45,9 @@ export const LOGIN_EMAIL_DISK = new StateDefinition("loginEmail", "disk", { web: "disk-local", }); export const LOGIN_STRATEGY_MEMORY = new StateDefinition("loginStrategy", "memory"); +export const AUTH_REQUEST_DISK_LOCAL = new StateDefinition("authRequestLocal", "disk", { + web: "disk-local", +}); export const SSO_DISK = new StateDefinition("ssoLogin", "disk"); export const TOKEN_DISK = new StateDefinition("token", "disk"); export const TOKEN_DISK_LOCAL = new StateDefinition("tokenDiskLocal", "disk", { diff --git a/libs/common/src/services/notifications.service.ts b/libs/common/src/services/notifications.service.ts index 4dc8772d00..7dc54b849f 100644 --- a/libs/common/src/services/notifications.service.ts +++ b/libs/common/src/services/notifications.service.ts @@ -2,6 +2,7 @@ import * as signalR from "@microsoft/signalr"; import * as signalRMsgPack from "@microsoft/signalr-protocol-msgpack"; import { firstValueFrom } from "rxjs"; +import { AuthRequestServiceAbstraction } from "../../../auth/src/common/abstractions"; import { ApiService } from "../abstractions/api.service"; import { NotificationsService as NotificationsServiceAbstraction } from "../abstractions/notifications.service"; import { AuthService } from "../auth/abstractions/auth.service"; @@ -18,6 +19,7 @@ import { EnvironmentService } from "../platform/abstractions/environment.service import { LogService } from "../platform/abstractions/log.service"; import { MessagingService } from "../platform/abstractions/messaging.service"; import { StateService } from "../platform/abstractions/state.service"; +import { UserId } from "../types/guid"; import { SyncService } from "../vault/abstractions/sync/sync.service.abstraction"; export class NotificationsService implements NotificationsServiceAbstraction { @@ -37,6 +39,7 @@ export class NotificationsService implements NotificationsServiceAbstraction { private logoutCallback: (expired: boolean) => Promise, private stateService: StateService, private authService: AuthService, + private authRequestService: AuthRequestServiceAbstraction, private messagingService: MessagingService, ) { this.environmentService.environment$.subscribe(() => { @@ -199,10 +202,13 @@ export class NotificationsService implements NotificationsServiceAbstraction { await this.syncService.syncDeleteSend(notification.payload as SyncSendNotification); break; case NotificationType.AuthRequest: - if (await this.stateService.getApproveLoginRequests()) { - this.messagingService.send("openLoginApproval", { - notificationId: notification.payload.id, - }); + { + const userId = await this.stateService.getUserId(); + if (await this.authRequestService.getAcceptAuthRequests(userId as UserId)) { + this.messagingService.send("openLoginApproval", { + notificationId: notification.payload.id, + }); + } } break; default: diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 76f0d7fd46..77b949126f 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -52,6 +52,7 @@ import { DeleteInstalledVersion } from "./migrations/52-delete-installed-version import { DeviceTrustCryptoServiceStateProviderMigrator } from "./migrations/53-migrate-device-trust-crypto-svc-to-state-providers"; import { SendMigrator } from "./migrations/54-move-encrypted-sends"; import { MoveMasterKeyStateToProviderMigrator } from "./migrations/55-move-master-key-state-to-provider"; +import { AuthRequestMigrator } from "./migrations/56-move-auth-requests"; import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key"; import { MoveBiometricAutoPromptToAccount } from "./migrations/7-move-biometric-auto-prompt-to-account"; import { MoveStateVersionMigrator } from "./migrations/8-move-state-version"; @@ -59,7 +60,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 3; -export const CURRENT_VERSION = 55; +export const CURRENT_VERSION = 56; export type MinVersion = typeof MIN_VERSION; @@ -117,7 +118,8 @@ export function createMigrationBuilder() { .with(DeleteInstalledVersion, 51, 52) .with(DeviceTrustCryptoServiceStateProviderMigrator, 52, 53) .with(SendMigrator, 53, 54) - .with(MoveMasterKeyStateToProviderMigrator, 54, CURRENT_VERSION); + .with(MoveMasterKeyStateToProviderMigrator, 54, 55) + .with(AuthRequestMigrator, 55, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migrations/56-move-auth-requests.spec.ts b/libs/common/src/state-migrations/migrations/56-move-auth-requests.spec.ts new file mode 100644 index 0000000000..f6bddbce7d --- /dev/null +++ b/libs/common/src/state-migrations/migrations/56-move-auth-requests.spec.ts @@ -0,0 +1,138 @@ +import { MockProxy } from "jest-mock-extended"; + +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { mockMigrationHelper } from "../migration-helper.spec"; + +import { AuthRequestMigrator } from "./56-move-auth-requests"; + +function exampleJSON() { + return { + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["FirstAccount", "SecondAccount"], + FirstAccount: { + settings: { + otherStuff: "otherStuff2", + approveLoginRequests: true, + }, + otherStuff: "otherStuff3", + adminAuthRequest: { + id: "id1", + privateKey: "privateKey1", + }, + }, + SecondAccount: { + settings: { + otherStuff: "otherStuff4", + }, + otherStuff: "otherStuff5", + }, + }; +} + +function rollbackJSON() { + return { + user_FirstAccount_authRequestLocal_adminAuthRequest: { + id: "id1", + privateKey: "privateKey1", + }, + user_FirstAccount_authRequestLocal_acceptAuthRequests: true, + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["FirstAccount", "SecondAccount"], + FirstAccount: { + settings: { + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }, + SecondAccount: { + settings: { + otherStuff: "otherStuff4", + }, + otherStuff: "otherStuff5", + }, + }; +} + +const ADMIN_AUTH_REQUEST_KEY: KeyDefinitionLike = { + stateDefinition: { + name: "authRequestLocal", + }, + key: "adminAuthRequest", +}; + +const ACCEPT_AUTH_REQUESTS_KEY: KeyDefinitionLike = { + stateDefinition: { + name: "authRequestLocal", + }, + key: "acceptAuthRequests", +}; + +describe("AuthRequestMigrator", () => { + let helper: MockProxy; + let sut: AuthRequestMigrator; + + describe("migrate", () => { + beforeEach(() => { + helper = mockMigrationHelper(exampleJSON(), 55); + sut = new AuthRequestMigrator(55, 56); + }); + + it("removes the existing adminAuthRequest and approveLoginRequests", async () => { + await sut.migrate(helper); + + expect(helper.set).toHaveBeenCalledWith("FirstAccount", { + settings: { + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }); + expect(helper.set).not.toHaveBeenCalledWith("SecondAccount"); + }); + + it("sets the adminAuthRequest and approveLoginRequests under the new key definitions", async () => { + await sut.migrate(helper); + + expect(helper.setToUser).toHaveBeenCalledWith("FirstAccount", ADMIN_AUTH_REQUEST_KEY, { + id: "id1", + privateKey: "privateKey1", + }); + + expect(helper.setToUser).toHaveBeenCalledWith("FirstAccount", ACCEPT_AUTH_REQUESTS_KEY, true); + expect(helper.setToUser).not.toHaveBeenCalledWith("SecondAccount"); + }); + }); + + describe("rollback", () => { + beforeEach(() => { + helper = mockMigrationHelper(rollbackJSON(), 56); + sut = new AuthRequestMigrator(55, 56); + }); + + it("nulls the new adminAuthRequest and acceptAuthRequests values", async () => { + await sut.rollback(helper); + + expect(helper.setToUser).toHaveBeenCalledWith("FirstAccount", ADMIN_AUTH_REQUEST_KEY, null); + expect(helper.setToUser).toHaveBeenCalledWith("FirstAccount", ACCEPT_AUTH_REQUESTS_KEY, null); + }); + + it("sets back the adminAuthRequest and approveLoginRequests under old account object", async () => { + await sut.rollback(helper); + + expect(helper.set).toHaveBeenCalledWith("FirstAccount", { + adminAuthRequest: { + id: "id1", + privateKey: "privateKey1", + }, + settings: { + otherStuff: "otherStuff2", + approveLoginRequests: true, + }, + otherStuff: "otherStuff3", + }); + }); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/56-move-auth-requests.ts b/libs/common/src/state-migrations/migrations/56-move-auth-requests.ts new file mode 100644 index 0000000000..4fec3b2de0 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/56-move-auth-requests.ts @@ -0,0 +1,104 @@ +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { Migrator } from "../migrator"; + +type AdminAuthRequestStorable = { + id: string; + privateKey: string; +}; + +type ExpectedAccountType = { + adminAuthRequest?: AdminAuthRequestStorable; + settings?: { + approveLoginRequests?: boolean; + }; +}; + +const ADMIN_AUTH_REQUEST_KEY: KeyDefinitionLike = { + stateDefinition: { + name: "authRequestLocal", + }, + key: "adminAuthRequest", +}; + +const ACCEPT_AUTH_REQUESTS_KEY: KeyDefinitionLike = { + stateDefinition: { + name: "authRequestLocal", + }, + key: "acceptAuthRequests", +}; + +export class AuthRequestMigrator extends Migrator<55, 56> { + async migrate(helper: MigrationHelper): Promise { + const accounts = await helper.getAccounts(); + + async function migrateAccount(userId: string, account: ExpectedAccountType): Promise { + let updatedAccount = false; + + // Migrate admin auth request + const existingAdminAuthRequest = account?.adminAuthRequest; + + if (existingAdminAuthRequest != null) { + await helper.setToUser(userId, ADMIN_AUTH_REQUEST_KEY, existingAdminAuthRequest); + delete account.adminAuthRequest; + updatedAccount = true; + } + + // Migrate approve login requests + const existingApproveLoginRequests = account?.settings?.approveLoginRequests; + + if (existingApproveLoginRequests != null) { + await helper.setToUser(userId, ACCEPT_AUTH_REQUESTS_KEY, existingApproveLoginRequests); + delete account.settings.approveLoginRequests; + updatedAccount = true; + } + + if (updatedAccount) { + // Save the migrated account + await helper.set(userId, account); + } + } + + await Promise.all([...accounts.map(({ userId, account }) => migrateAccount(userId, account))]); + } + + async rollback(helper: MigrationHelper): Promise { + const accounts = await helper.getAccounts(); + + async function rollbackAccount(userId: string, account: ExpectedAccountType): Promise { + let updatedAccount = false; + // Rollback admin auth request + const migratedAdminAuthRequest: AdminAuthRequestStorable = await helper.getFromUser( + userId, + ADMIN_AUTH_REQUEST_KEY, + ); + + if (migratedAdminAuthRequest != null) { + account.adminAuthRequest = migratedAdminAuthRequest; + updatedAccount = true; + } + + await helper.setToUser(userId, ADMIN_AUTH_REQUEST_KEY, null); + + // Rollback approve login requests + const migratedAcceptAuthRequest: boolean = await helper.getFromUser( + userId, + ACCEPT_AUTH_REQUESTS_KEY, + ); + + if (migratedAcceptAuthRequest != null) { + account.settings = Object.assign(account.settings ?? {}, { + approveLoginRequests: migratedAcceptAuthRequest, + }); + updatedAccount = true; + } + + await helper.setToUser(userId, ACCEPT_AUTH_REQUESTS_KEY, null); + + if (updatedAccount) { + await helper.set(userId, account); + } + } + + await Promise.all([...accounts.map(({ userId, account }) => rollbackAccount(userId, account))]); + } +}