From e595c0548ed1a481dbd024ec66975e3a453bd471 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Fri, 1 Apr 2022 11:28:23 +1000 Subject: [PATCH] EC-134 Fix api token refresh (#749) * Fix apikey token refresh * Refactor: use class for TokenRequestTwoFactor --- .../src/components/two-factor.component.ts | 7 +--- angular/src/services/jslib-services.module.ts | 5 ++- .../logInStrategies/logIn.strategy.spec.ts | 17 ++++---- common/src/abstractions/auth.service.ts | 2 +- .../misc/logInStrategies/logIn.strategy.ts | 14 ++----- .../logInStrategies/passwordLogin.strategy.ts | 2 +- common/src/models/domain/logInCredentials.ts | 2 +- .../request/identityToken/apiTokenRequest.ts | 3 +- .../identityToken/passwordTokenRequest.ts | 3 +- .../request/identityToken/ssoTokenRequest.ts | 3 +- .../request/identityToken/tokenRequest.ts | 7 +--- .../identityToken/tokenRequestTwoFactor.ts | 9 ++++ common/src/services/api.service.ts | 42 ++++++++++++------- common/src/services/auth.service.ts | 2 +- common/src/services/state.service.ts | 9 ++-- node/src/cli/commands/login.command.ts | 20 ++------- node/src/services/nodeApi.service.ts | 15 +++++-- 17 files changed, 82 insertions(+), 80 deletions(-) create mode 100644 common/src/models/request/identityToken/tokenRequestTwoFactor.ts diff --git a/angular/src/components/two-factor.component.ts b/angular/src/components/two-factor.component.ts index a40dc3c594..f83441f34c 100644 --- a/angular/src/components/two-factor.component.ts +++ b/angular/src/components/two-factor.component.ts @@ -14,6 +14,7 @@ import { TwoFactorService } from "jslib-common/abstractions/twoFactor.service"; import { TwoFactorProviderType } from "jslib-common/enums/twoFactorProviderType"; import { WebAuthnIFrame } from "jslib-common/misc/webauthn_iframe"; import { AuthResult } from "jslib-common/models/domain/authResult"; +import { TokenRequestTwoFactor } from "jslib-common/models/request/identityToken/tokenRequestTwoFactor"; import { TwoFactorEmailRequest } from "jslib-common/models/request/twoFactorEmailRequest"; import { TwoFactorProviders } from "jslib-common/services/twoFactor.service"; @@ -191,11 +192,7 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI async doSubmit() { this.formPromise = this.authService.logInTwoFactor( - { - provider: this.selectedProviderType, - token: this.token, - remember: this.remember, - }, + new TokenRequestTwoFactor(this.selectedProviderType, this.token, this.remember), this.captchaToken ); const response: AuthResult = await this.formPromise; diff --git a/angular/src/services/jslib-services.module.ts b/angular/src/services/jslib-services.module.ts index d35deeb1a4..8d8aa85bea 100644 --- a/angular/src/services/jslib-services.module.ts +++ b/angular/src/services/jslib-services.module.ts @@ -211,12 +211,14 @@ import { ValidationService } from "./validation.service"; tokenService: TokenServiceAbstraction, platformUtilsService: PlatformUtilsServiceAbstraction, environmentService: EnvironmentServiceAbstraction, - messagingService: MessagingServiceAbstraction + messagingService: MessagingServiceAbstraction, + appIdService: AppIdServiceAbstraction ) => new ApiService( tokenService, platformUtilsService, environmentService, + appIdService, async (expired: boolean) => messagingService.send("logout", { expired: expired }) ), deps: [ @@ -224,6 +226,7 @@ import { ValidationService } from "./validation.service"; PlatformUtilsServiceAbstraction, EnvironmentServiceAbstraction, MessagingServiceAbstraction, + AppIdServiceAbstraction, ], }, { diff --git a/common/spec/misc/logInStrategies/logIn.strategy.spec.ts b/common/spec/misc/logInStrategies/logIn.strategy.spec.ts index 49e36bb66d..5f68881bad 100644 --- a/common/spec/misc/logInStrategies/logIn.strategy.spec.ts +++ b/common/spec/misc/logInStrategies/logIn.strategy.spec.ts @@ -18,6 +18,7 @@ import { AuthResult } from "jslib-common/models/domain/authResult"; import { EncString } from "jslib-common/models/domain/encString"; import { PasswordLogInCredentials } from "jslib-common/models/domain/logInCredentials"; import { PasswordTokenRequest } from "jslib-common/models/request/identityToken/passwordTokenRequest"; +import { TokenRequestTwoFactor } from "jslib-common/models/request/identityToken/tokenRequestTwoFactor"; import { IdentityCaptchaResponse } from "jslib-common/models/response/identityCaptchaResponse"; import { IdentityTokenResponse } from "jslib-common/models/response/identityTokenResponse"; import { IdentityTwoFactorResponse } from "jslib-common/models/response/identityTwoFactorResponse"; @@ -236,11 +237,11 @@ describe("LogInStrategy", () => { it("sends 2FA token provided by user to server (single step)", async () => { // This occurs if the user enters the 2FA code as an argument in the CLI apiService.postIdentityToken(Arg.any()).resolves(identityTokenResponseFactory()); - credentials.twoFactor = { - provider: twoFactorProviderType, - token: twoFactorToken, - remember: twoFactorRemember, - }; + credentials.twoFactor = new TokenRequestTwoFactor( + twoFactorProviderType, + twoFactorToken, + twoFactorRemember + ); await passwordLogInStrategy.logIn(credentials); @@ -268,11 +269,7 @@ describe("LogInStrategy", () => { apiService.postIdentityToken(Arg.any()).resolves(identityTokenResponseFactory()); await passwordLogInStrategy.logInTwoFactor( - { - provider: twoFactorProviderType, - token: twoFactorToken, - remember: twoFactorRemember, - }, + new TokenRequestTwoFactor(twoFactorProviderType, twoFactorToken, twoFactorRemember), null ); diff --git a/common/src/abstractions/auth.service.ts b/common/src/abstractions/auth.service.ts index 447ade6c21..bebac7300b 100644 --- a/common/src/abstractions/auth.service.ts +++ b/common/src/abstractions/auth.service.ts @@ -5,7 +5,7 @@ import { SsoLogInCredentials, } from "../models/domain/logInCredentials"; import { SymmetricCryptoKey } from "../models/domain/symmetricCryptoKey"; -import { TokenRequestTwoFactor } from "../models/request/identityToken/tokenRequest"; +import { TokenRequestTwoFactor } from "../models/request/identityToken/tokenRequestTwoFactor"; export abstract class AuthService { masterPasswordHash: string; diff --git a/common/src/misc/logInStrategies/logIn.strategy.ts b/common/src/misc/logInStrategies/logIn.strategy.ts index 86bfa30bdb..a58a7bc898 100644 --- a/common/src/misc/logInStrategies/logIn.strategy.ts +++ b/common/src/misc/logInStrategies/logIn.strategy.ts @@ -19,7 +19,7 @@ import { DeviceRequest } from "../../models/request/deviceRequest"; import { ApiTokenRequest } from "../../models/request/identityToken/apiTokenRequest"; import { PasswordTokenRequest } from "../../models/request/identityToken/passwordTokenRequest"; import { SsoTokenRequest } from "../../models/request/identityToken/ssoTokenRequest"; -import { TokenRequestTwoFactor } from "../../models/request/identityToken/tokenRequest"; +import { TokenRequestTwoFactor } from "../../models/request/identityToken/tokenRequestTwoFactor"; import { KeysRequest } from "../../models/request/keysRequest"; import { IdentityCaptchaResponse } from "../../models/response/identityCaptchaResponse"; import { IdentityTokenResponse } from "../../models/response/identityTokenResponse"; @@ -86,18 +86,10 @@ export abstract class LogInStrategy { const storedTwoFactorToken = await this.tokenService.getTwoFactorToken(); if (storedTwoFactorToken != null) { - return { - token: storedTwoFactorToken, - provider: TwoFactorProviderType.Remember, - remember: false, - }; + return new TokenRequestTwoFactor(TwoFactorProviderType.Remember, storedTwoFactorToken, false); } - return { - token: null, - provider: null, - remember: false, - }; + return new TokenRequestTwoFactor(); } protected async saveAccountInformation(tokenResponse: IdentityTokenResponse) { diff --git a/common/src/misc/logInStrategies/passwordLogin.strategy.ts b/common/src/misc/logInStrategies/passwordLogin.strategy.ts index 6db930e693..81947e562d 100644 --- a/common/src/misc/logInStrategies/passwordLogin.strategy.ts +++ b/common/src/misc/logInStrategies/passwordLogin.strategy.ts @@ -13,7 +13,7 @@ import { AuthResult } from "../../models/domain/authResult"; import { PasswordLogInCredentials } from "../../models/domain/logInCredentials"; import { SymmetricCryptoKey } from "../../models/domain/symmetricCryptoKey"; import { PasswordTokenRequest } from "../../models/request/identityToken/passwordTokenRequest"; -import { TokenRequestTwoFactor } from "../../models/request/identityToken/tokenRequest"; +import { TokenRequestTwoFactor } from "../../models/request/identityToken/tokenRequestTwoFactor"; import { LogInStrategy } from "./logIn.strategy"; diff --git a/common/src/models/domain/logInCredentials.ts b/common/src/models/domain/logInCredentials.ts index b89d2bac61..c1e23610e4 100644 --- a/common/src/models/domain/logInCredentials.ts +++ b/common/src/models/domain/logInCredentials.ts @@ -1,5 +1,5 @@ import { AuthenticationType } from "../../enums/authenticationType"; -import { TokenRequestTwoFactor } from "../request/identityToken/tokenRequest"; +import { TokenRequestTwoFactor } from "../request/identityToken/tokenRequestTwoFactor"; export class PasswordLogInCredentials { readonly type = AuthenticationType.Password; diff --git a/common/src/models/request/identityToken/apiTokenRequest.ts b/common/src/models/request/identityToken/apiTokenRequest.ts index dabaead94a..247ba1746e 100644 --- a/common/src/models/request/identityToken/apiTokenRequest.ts +++ b/common/src/models/request/identityToken/apiTokenRequest.ts @@ -1,6 +1,7 @@ import { DeviceRequest } from "../deviceRequest"; -import { TokenRequest, TokenRequestTwoFactor } from "./tokenRequest"; +import { TokenRequest } from "./tokenRequest"; +import { TokenRequestTwoFactor } from "./tokenRequestTwoFactor"; export class ApiTokenRequest extends TokenRequest { constructor( diff --git a/common/src/models/request/identityToken/passwordTokenRequest.ts b/common/src/models/request/identityToken/passwordTokenRequest.ts index 0cafd1478e..eda100374d 100644 --- a/common/src/models/request/identityToken/passwordTokenRequest.ts +++ b/common/src/models/request/identityToken/passwordTokenRequest.ts @@ -3,7 +3,8 @@ import { Utils } from "../../../misc/utils"; import { CaptchaProtectedRequest } from "../captchaProtectedRequest"; import { DeviceRequest } from "../deviceRequest"; -import { TokenRequest, TokenRequestTwoFactor } from "./tokenRequest"; +import { TokenRequest } from "./tokenRequest"; +import { TokenRequestTwoFactor } from "./tokenRequestTwoFactor"; export class PasswordTokenRequest extends TokenRequest implements CaptchaProtectedRequest { constructor( diff --git a/common/src/models/request/identityToken/ssoTokenRequest.ts b/common/src/models/request/identityToken/ssoTokenRequest.ts index 267de2ed99..c7697c2f13 100644 --- a/common/src/models/request/identityToken/ssoTokenRequest.ts +++ b/common/src/models/request/identityToken/ssoTokenRequest.ts @@ -1,6 +1,7 @@ import { DeviceRequest } from "../deviceRequest"; -import { TokenRequest, TokenRequestTwoFactor } from "./tokenRequest"; +import { TokenRequest } from "./tokenRequest"; +import { TokenRequestTwoFactor } from "./tokenRequestTwoFactor"; export class SsoTokenRequest extends TokenRequest { constructor( diff --git a/common/src/models/request/identityToken/tokenRequest.ts b/common/src/models/request/identityToken/tokenRequest.ts index 36a61e1de0..82a4a394c5 100644 --- a/common/src/models/request/identityToken/tokenRequest.ts +++ b/common/src/models/request/identityToken/tokenRequest.ts @@ -1,11 +1,6 @@ -import { TwoFactorProviderType } from "../../../enums/twoFactorProviderType"; import { DeviceRequest } from "../deviceRequest"; -export interface TokenRequestTwoFactor { - provider: TwoFactorProviderType; - token: string; - remember: boolean; -} +import { TokenRequestTwoFactor } from "./tokenRequestTwoFactor"; export abstract class TokenRequest { protected device?: DeviceRequest; diff --git a/common/src/models/request/identityToken/tokenRequestTwoFactor.ts b/common/src/models/request/identityToken/tokenRequestTwoFactor.ts new file mode 100644 index 0000000000..567e8c66e6 --- /dev/null +++ b/common/src/models/request/identityToken/tokenRequestTwoFactor.ts @@ -0,0 +1,9 @@ +import { TwoFactorProviderType } from "jslib-common/enums/twoFactorProviderType"; + +export class TokenRequestTwoFactor { + constructor( + public provider: TwoFactorProviderType = null, + public token: string = null, + public remember: boolean = false + ) {} +} diff --git a/common/src/services/api.service.ts b/common/src/services/api.service.ts index 5e51b448c0..da2fcd5d64 100644 --- a/common/src/services/api.service.ts +++ b/common/src/services/api.service.ts @@ -1,3 +1,7 @@ +import { AppIdService } from "jslib-common/abstractions/appId.service"; +import { DeviceRequest } from "jslib-common/models/request/deviceRequest"; +import { TokenRequestTwoFactor } from "jslib-common/models/request/identityToken/tokenRequestTwoFactor"; + import { ApiService as ApiServiceAbstraction } from "../abstractions/api.service"; import { EnvironmentService } from "../abstractions/environment.service"; import { PlatformUtilsService } from "../abstractions/platformUtils.service"; @@ -174,7 +178,6 @@ import { UserKeyResponse } from "../models/response/userKeyResponse"; import { SendAccessView } from "../models/view/sendAccessView"; export class ApiService implements ApiServiceAbstraction { - protected apiKeyRefresh: (clientId: string, clientSecret: string) => Promise; private device: DeviceType; private deviceType: string; private isWebClient = false; @@ -184,6 +187,7 @@ export class ApiService implements ApiServiceAbstraction { private tokenService: TokenService, private platformUtilsService: PlatformUtilsService, private environmentService: EnvironmentService, + private appIdService: AppIdService, private logoutCallback: (expired: boolean) => Promise, private customUserAgent: string = null ) { @@ -2332,20 +2336,6 @@ export class ApiService implements ApiServiceAbstraction { throw new Error("Cannot refresh token, no refresh token or api keys are stored"); } - protected async doApiTokenRefresh(): Promise { - const clientId = await this.tokenService.getClientId(); - const clientSecret = await this.tokenService.getClientSecret(); - if ( - Utils.isNullOrWhitespace(clientId) || - Utils.isNullOrWhitespace(clientSecret) || - this.apiKeyRefresh == null - ) { - throw new Error(); - } - - await this.apiKeyRefresh(clientId, clientSecret); - } - protected async doRefreshToken(): Promise { const refreshToken = await this.tokenService.getRefreshToken(); if (refreshToken == null || refreshToken === "") { @@ -2389,6 +2379,28 @@ export class ApiService implements ApiServiceAbstraction { } } + protected async doApiTokenRefresh(): Promise { + const clientId = await this.tokenService.getClientId(); + const clientSecret = await this.tokenService.getClientSecret(); + + const appId = await this.appIdService.getAppId(); + const deviceRequest = new DeviceRequest(appId, this.platformUtilsService); + + const tokenRequest = new ApiTokenRequest( + clientId, + clientSecret, + new TokenRequestTwoFactor(), + deviceRequest + ); + + const response = await this.postIdentityToken(tokenRequest); + if (!(response instanceof IdentityTokenResponse)) { + throw new Error("Invalid response received when refreshing api token"); + } + + await this.tokenService.setToken(response.accessToken); + } + private async send( method: "GET" | "POST" | "PUT" | "DELETE", path: string, diff --git a/common/src/services/auth.service.ts b/common/src/services/auth.service.ts index 66f5340af1..fe4542b8e2 100644 --- a/common/src/services/auth.service.ts +++ b/common/src/services/auth.service.ts @@ -23,7 +23,7 @@ import { SsoLogInCredentials, } from "../models/domain/logInCredentials"; import { SymmetricCryptoKey } from "../models/domain/symmetricCryptoKey"; -import { TokenRequestTwoFactor } from "../models/request/identityToken/tokenRequest"; +import { TokenRequestTwoFactor } from "../models/request/identityToken/tokenRequestTwoFactor"; import { PreloginRequest } from "../models/request/preloginRequest"; import { ErrorResponse } from "../models/response/errorResponse"; diff --git a/common/src/services/state.service.ts b/common/src/services/state.service.ts index 9d035fc785..39a08f5018 100644 --- a/common/src/services/state.service.ts +++ b/common/src/services/state.service.ts @@ -2489,11 +2489,10 @@ export class StateService< protected async deAuthenticateAccount(userId: string) { await this.setAccessToken(null, { userId: userId }); await this.setLastActive(null, { userId: userId }); - const index = this.state.authenticatedAccounts.indexOf(userId); - if (index > -1) { - this.state.authenticatedAccounts.splice(index, 1); - await this.storageService.save(keys.authenticatedAccounts, this.state.authenticatedAccounts); - } + this.state.authenticatedAccounts = this.state.authenticatedAccounts.filter( + (activeUserId) => activeUserId !== userId + ); + await this.storageService.save(keys.authenticatedAccounts, this.state.authenticatedAccounts); } protected async removeAccountFromDisk(userId: string) { diff --git a/node/src/cli/commands/login.command.ts b/node/src/cli/commands/login.command.ts index f8c9a6b97d..ee6f822845 100644 --- a/node/src/cli/commands/login.command.ts +++ b/node/src/cli/commands/login.command.ts @@ -24,7 +24,7 @@ import { PasswordLogInCredentials, SsoLogInCredentials, } from "jslib-common/models/domain/logInCredentials"; -import { TokenRequestTwoFactor } from "jslib-common/models/request/identityToken/tokenRequest"; +import { TokenRequestTwoFactor } from "jslib-common/models/request/identityToken/tokenRequestTwoFactor"; import { TwoFactorEmailRequest } from "jslib-common/models/request/twoFactorEmailRequest"; import { UpdateTempPasswordRequest } from "jslib-common/models/request/updateTempPasswordRequest"; import { ErrorResponse } from "jslib-common/models/response/errorResponse"; @@ -150,11 +150,7 @@ export class LoginCommand { const twoFactor = twoFactorToken == null ? null - : { - provider: twoFactorMethod, - token: twoFactorToken, - remember: false, - }; + : new TokenRequestTwoFactor(twoFactorMethod, twoFactorToken, false); try { if (this.validatedParams != null) { @@ -258,21 +254,13 @@ export class LoginCommand { } response = await this.authService.logInTwoFactor( - { - provider: selectedProvider.type, - token: twoFactorToken, - remember: false, - }, + new TokenRequestTwoFactor(selectedProvider.type, twoFactorToken), null ); } if (response.captchaSiteKey) { - const twoFactorRequest: TokenRequestTwoFactor = { - provider: selectedProvider.type, - token: twoFactorToken, - remember: false, - }; + const twoFactorRequest = new TokenRequestTwoFactor(selectedProvider.type, twoFactorToken); const handledResponse = await this.handleCaptchaRequired(twoFactorRequest); // Error Response diff --git a/node/src/services/nodeApi.service.ts b/node/src/services/nodeApi.service.ts index 9ffafe3c49..7dd0341213 100644 --- a/node/src/services/nodeApi.service.ts +++ b/node/src/services/nodeApi.service.ts @@ -2,6 +2,7 @@ import * as FormData from "form-data"; import { HttpsProxyAgent } from "https-proxy-agent"; import * as fe from "node-fetch"; +import { AppIdService } from "jslib-common/abstractions/appId.service"; import { EnvironmentService } from "jslib-common/abstractions/environment.service"; import { PlatformUtilsService } from "jslib-common/abstractions/platformUtils.service"; import { TokenService } from "jslib-common/abstractions/token.service"; @@ -18,12 +19,18 @@ export class NodeApiService extends ApiService { tokenService: TokenService, platformUtilsService: PlatformUtilsService, environmentService: EnvironmentService, + appIdService: AppIdService, logoutCallback: (expired: boolean) => Promise, - customUserAgent: string = null, - apiKeyRefresh: (clientId: string, clientSecret: string) => Promise + customUserAgent: string = null ) { - super(tokenService, platformUtilsService, environmentService, logoutCallback, customUserAgent); - this.apiKeyRefresh = apiKeyRefresh; + super( + tokenService, + platformUtilsService, + environmentService, + appIdService, + logoutCallback, + customUserAgent + ); } nativeFetch(request: Request): Promise {