From 1146c8f5bf839df3283dd4a812e7ddc2e0358c18 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Tue, 8 Feb 2022 13:38:46 +1000 Subject: [PATCH] [Tech debt] Refactor authService (#213) * Add OrganizationLogInStrategy * Use noop TwoFactorService --- jslib | 2 +- src/app/accounts/apiKey.component.ts | 6 +- src/app/services/services.module.ts | 18 ++-- src/bwdc.ts | 22 +++-- .../organizationLogIn.strategy.ts | 65 +++++++++++++ src/program.ts | 1 + src/services/api.service.ts | 3 +- src/services/auth.service.ts | 96 ++++--------------- src/services/noop/noopTwoFactor.service.ts | 42 ++++++++ 9 files changed, 158 insertions(+), 97 deletions(-) create mode 100644 src/misc/logInStrategies/organizationLogIn.strategy.ts create mode 100644 src/services/noop/noopTwoFactor.service.ts diff --git a/jslib b/jslib index e0cc754d..6b850857 160000 --- a/jslib +++ b/jslib @@ -1 +1 @@ -Subproject commit e0cc754d6fe962a5e7eae6d1dead8b44606d4853 +Subproject commit 6b8508579f89b4c54afa6aab2b7155aac70fb8a9 diff --git a/src/app/accounts/apiKey.component.ts b/src/app/accounts/apiKey.component.ts index 7532b3e8..de8f120c 100644 --- a/src/app/accounts/apiKey.component.ts +++ b/src/app/accounts/apiKey.component.ts @@ -12,8 +12,8 @@ import { StateService } from "../../abstractions/state.service"; import { ModalService } from "jslib-angular/services/modal.service"; -import { HtmlStorageLocation } from "jslib-common/enums/htmlStorageLocation"; import { Utils } from "jslib-common/misc/utils"; +import { ApiLogInCredentials } from "jslib-common/models/domain/logInCredentials"; @Component({ selector: "app-apiKey", @@ -76,7 +76,9 @@ export class ApiKeyComponent { } try { - this.formPromise = this.authService.logInApiKey(this.clientId, this.clientSecret); + this.formPromise = this.authService.logIn( + new ApiLogInCredentials(this.clientId, this.clientSecret) + ); await this.formPromise; const organizationId = await this.stateService.getEntityId(); await this.stateService.setOrganizationId(organizationId); diff --git a/src/app/services/services.module.ts b/src/app/services/services.module.ts index b067be2c..1f883c95 100644 --- a/src/app/services/services.module.ts +++ b/src/app/services/services.module.ts @@ -11,6 +11,7 @@ import { LaunchGuardService } from "./launch-guard.service"; import { I18nService } from "../../services/i18n.service"; import { SyncService } from "../../services/sync.service"; +import { NoopTwoFactorService } from "../../services/noop/noopTwoFactor.service"; import { JslibServicesModule } from "jslib-angular/services/jslib-services.module"; @@ -34,11 +35,11 @@ import { PlatformUtilsService as PlatformUtilsServiceAbstraction } from "jslib-c import { StateMigrationService as StateMigrationServiceAbstraction } from "jslib-common/abstractions/stateMigration.service"; import { StorageService as StorageServiceAbstraction } from "jslib-common/abstractions/storage.service"; import { TokenService as TokenServiceAbstraction } from "jslib-common/abstractions/token.service"; -import { VaultTimeoutService as VaultTimeoutServiceAbstraction } from "jslib-common/abstractions/vaultTimeout.service"; +import { TwoFactorService as TwoFactorServiceAbstraction } from "jslib-common/abstractions/twoFactor.service"; import { StateService as StateServiceAbstraction } from "../../abstractions/state.service"; -import { ApiService, refreshToken } from "../../services/api.service"; +import { refreshToken } from "../../services/api.service"; import { AuthService } from "../../services/auth.service"; import { StateService } from "../../services/state.service"; import { StateMigrationService } from "../../services/stateMigration.service"; @@ -60,7 +61,6 @@ function refreshTokenCallback(injector: Injector) { export function initFactory( environmentService: EnvironmentServiceAbstraction, i18nService: I18nService, - authService: AuthService, platformUtilsService: PlatformUtilsServiceAbstraction, stateService: StateServiceAbstraction, cryptoService: CryptoServiceAbstraction @@ -69,7 +69,6 @@ export function initFactory( await stateService.init(); await environmentService.setUrlsFromStorage(); await i18nService.init(); - authService.init(); const htmlEl = window.document.documentElement; htmlEl.classList.add("os_" + platformUtilsService.getDeviceString()); htmlEl.classList.add("locale_" + i18nService.translationLocale); @@ -103,7 +102,6 @@ export function initFactory( deps: [ EnvironmentServiceAbstraction, I18nServiceAbstraction, - AuthServiceAbstraction, PlatformUtilsServiceAbstraction, StateServiceAbstraction, CryptoServiceAbstraction, @@ -170,15 +168,13 @@ export function initFactory( ApiServiceAbstraction, TokenServiceAbstraction, AppIdServiceAbstraction, - I18nServiceAbstraction, PlatformUtilsServiceAbstraction, MessagingServiceAbstraction, - VaultTimeoutServiceAbstraction, LogServiceAbstraction, - CryptoFunctionServiceAbstraction, - EnvironmentServiceAbstraction, KeyConnectorServiceAbstraction, + EnvironmentServiceAbstraction, StateServiceAbstraction, + TwoFactorServiceAbstraction, ], }, { @@ -232,6 +228,10 @@ export function initFactory( StateMigrationServiceAbstraction, ], }, + { + provide: TwoFactorServiceAbstraction, + useClass: NoopTwoFactorService, + }, ], }) export class ServicesModule {} diff --git a/src/bwdc.ts b/src/bwdc.ts index 58d93bf8..f023128d 100644 --- a/src/bwdc.ts +++ b/src/bwdc.ts @@ -8,6 +8,7 @@ import { AuthService } from "./services/auth.service"; import { I18nService } from "./services/i18n.service"; import { KeytarSecureStorageService } from "./services/keytarSecureStorage.service"; import { LowdbStorageService } from "./services/lowdbStorage.service"; +import { NoopTwoFactorService } from "./services/noop/noopTwoFactor.service"; import { StateService } from "./services/state.service"; import { StateMigrationService } from "./services/stateMigration.service"; import { SyncService } from "./services/sync.service"; @@ -37,15 +38,16 @@ import { SettingsService } from "jslib-common/services/settings.service"; import { TokenService } from "jslib-common/services/token.service"; import { StorageService as StorageServiceAbstraction } from "jslib-common/abstractions/storage.service"; +import { TwoFactorService as TwoFactorServiceAbstraction } from "jslib-common/abstractions/twoFactor.service"; import { Program } from "./program"; import { Account } from "./models/account"; -import { GlobalStateFactory } from "jslib-common/factories/globalStateFactory"; import { StateFactory } from "jslib-common/factories/stateFactory"; import { GlobalState } from "jslib-common/models/domain/globalState"; +import { ApiLogInCredentials } from "jslib-common/models/domain/logInCredentials"; // tslint:disable-next-line const packageJson = require("./package.json"); @@ -83,6 +85,7 @@ export class Main { stateMigrationService: StateMigrationService; organizationService: OrganizationService; providerService: ProviderService; + twoFactorService: TwoFactorServiceAbstraction; constructor() { const applicationName = "Bitwarden Directory Connector"; @@ -160,7 +163,8 @@ export class Main { " (" + this.platformUtilsService.getDeviceString().toUpperCase() + ")", - (clientId, clientSecret) => this.authService.logInApiKey(clientId, clientSecret) + (clientId, clientSecret) => + this.authService.logIn(new ApiLogInCredentials(clientId, clientSecret)) ); this.containerService = new ContainerService(this.cryptoService); @@ -172,23 +176,24 @@ export class Main { this.apiService, this.tokenService, this.logService, - this.organizationService + this.organizationService, + this.cryptoFunctionService ); + this.twoFactorService = new NoopTwoFactorService(); + this.authService = new AuthService( this.cryptoService, this.apiService, this.tokenService, this.appIdService, - this.i18nService, this.platformUtilsService, this.messagingService, - null, this.logService, - this.cryptoFunctionService, - this.environmentService, this.keyConnectorService, - this.stateService + this.environmentService, + this.stateService, + this.twoFactorService ); this.syncService = new SyncService( @@ -281,7 +286,6 @@ export class Main { // }); const locale = await this.stateService.getLocale(); await this.i18nService.init(locale); - this.authService.init(); const installedVersion = await this.stateService.getInstalledVersion(); const currentVersion = await this.platformUtilsService.getApplicationVersion(); diff --git a/src/misc/logInStrategies/organizationLogIn.strategy.ts b/src/misc/logInStrategies/organizationLogIn.strategy.ts new file mode 100644 index 00000000..1a46b162 --- /dev/null +++ b/src/misc/logInStrategies/organizationLogIn.strategy.ts @@ -0,0 +1,65 @@ +import { LogInStrategy } from "jslib-common/misc/logInStrategies/logIn.strategy"; + +import { ApiTokenRequest } from "jslib-common/models/request/identityToken/apiTokenRequest"; + +import { IdentityTokenResponse } from "jslib-common/models/response/identityTokenResponse"; + +import { AccountKeys, AccountProfile, AccountTokens } from "jslib-common/models/domain/account"; +import { AuthResult } from "jslib-common/models/domain/authResult"; +import { ApiLogInCredentials } from "jslib-common/models/domain/logInCredentials"; + +import { Account, DirectoryConfigurations, DirectorySettings } from "src/models/account"; + +export class OrganizationLogInStrategy extends LogInStrategy { + tokenRequest: ApiTokenRequest; + + async logIn(credentials: ApiLogInCredentials) { + this.tokenRequest = new ApiTokenRequest( + credentials.clientId, + credentials.clientSecret, + await this.buildTwoFactor(), + await this.buildDeviceRequest() + ); + + return this.startLogIn(); + } + + protected async processTokenResponse(response: IdentityTokenResponse): Promise { + await this.saveAccountInformation(response); + return new AuthResult(); + } + + protected async saveAccountInformation(tokenResponse: IdentityTokenResponse) { + const clientId = this.tokenRequest.clientId; + const entityId = clientId.split("organization.")[1]; + const clientSecret = this.tokenRequest.clientSecret; + + await this.stateService.addAccount( + new Account({ + profile: { + ...new AccountProfile(), + ...{ + userId: entityId, + apiKeyClientId: clientId, + entityId: entityId, + }, + }, + tokens: { + ...new AccountTokens(), + ...{ + accessToken: tokenResponse.accessToken, + refreshToken: tokenResponse.refreshToken, + }, + }, + keys: { + ...new AccountKeys(), + ...{ + apiKeyClientSecret: clientSecret, + }, + }, + directorySettings: new DirectorySettings(), + directoryConfigurations: new DirectoryConfigurations(), + }) + ); + } +} diff --git a/src/program.ts b/src/program.ts index 90b6c44b..6c0cbd48 100644 --- a/src/program.ts +++ b/src/program.ts @@ -106,6 +106,7 @@ export class Program extends BaseProgram { this.main.stateService, this.main.cryptoService, this.main.policyService, + this.main.twoFactorService, "connector" ); diff --git a/src/services/api.service.ts b/src/services/api.service.ts index c9f8fd6d..19f225cc 100644 --- a/src/services/api.service.ts +++ b/src/services/api.service.ts @@ -4,6 +4,7 @@ import { PlatformUtilsService } from "jslib-common/abstractions/platformUtils.se import { TokenService } from "jslib-common/abstractions/token.service"; import { StateService } from "../abstractions/state.service"; +import { ApiLogInCredentials } from "jslib-common/models/domain/logInCredentials"; import { ApiService as ApiServiceBase } from "jslib-common/services/api.service"; export async function refreshToken(stateService: StateService, authService: AuthService) { @@ -11,7 +12,7 @@ export async function refreshToken(stateService: StateService, authService: Auth const clientId = await stateService.getApiKeyClientId(); const clientSecret = await stateService.getApiKeyClientSecret(); if (clientId != null && clientSecret != null) { - await authService.logInApiKey(clientId, clientSecret); + await authService.logIn(new ApiLogInCredentials(clientId, clientSecret)); } } catch (e) { return Promise.reject(e); diff --git a/src/services/auth.service.ts b/src/services/auth.service.ts index d5991286..ddbdea9e 100644 --- a/src/services/auth.service.ts +++ b/src/services/auth.service.ts @@ -1,28 +1,22 @@ +import { OrganizationLogInStrategy } from "../misc/logInStrategies/organizationLogIn.strategy"; + import { ApiService } from "jslib-common/abstractions/api.service"; import { AppIdService } from "jslib-common/abstractions/appId.service"; import { CryptoService } from "jslib-common/abstractions/crypto.service"; -import { CryptoFunctionService } from "jslib-common/abstractions/cryptoFunction.service"; import { EnvironmentService } from "jslib-common/abstractions/environment.service"; -import { I18nService } from "jslib-common/abstractions/i18n.service"; import { KeyConnectorService } from "jslib-common/abstractions/keyConnector.service"; import { LogService } from "jslib-common/abstractions/log.service"; import { MessagingService } from "jslib-common/abstractions/messaging.service"; import { PlatformUtilsService } from "jslib-common/abstractions/platformUtils.service"; import { TokenService } from "jslib-common/abstractions/token.service"; -import { VaultTimeoutService } from "jslib-common/abstractions/vaultTimeout.service"; -import { StateService } from "../abstractions/state.service"; +import { TwoFactorService } from "jslib-common/abstractions/twoFactor.service"; import { AuthService as AuthServiceBase } from "jslib-common/services/auth.service"; -import { Account, DirectoryConfigurations, DirectorySettings } from "src/models/account"; - -import { AccountKeys, AccountProfile, AccountTokens } from "jslib-common/models/domain/account"; import { AuthResult } from "jslib-common/models/domain/authResult"; +import { ApiLogInCredentials } from "jslib-common/models/domain/logInCredentials"; -import { DeviceRequest } from "jslib-common/models/request/deviceRequest"; -import { TokenRequest } from "jslib-common/models/request/tokenRequest"; - -import { IdentityTokenResponse } from "jslib-common/models/response/identityTokenResponse"; +import { StateService } from "../abstractions/state.service"; export class AuthService extends AuthServiceBase { constructor( @@ -30,90 +24,42 @@ export class AuthService extends AuthServiceBase { apiService: ApiService, tokenService: TokenService, appIdService: AppIdService, - i18nService: I18nService, platformUtilsService: PlatformUtilsService, messagingService: MessagingService, - vaultTimeoutService: VaultTimeoutService, logService: LogService, - cryptoFunctionService: CryptoFunctionService, - environmentService: EnvironmentService, keyConnectorService: KeyConnectorService, - stateService: StateService + environmentService: EnvironmentService, + stateService: StateService, + twoFactorService: TwoFactorService ) { super( cryptoService, apiService, tokenService, appIdService, - i18nService, platformUtilsService, messagingService, - vaultTimeoutService, logService, - cryptoFunctionService, keyConnectorService, environmentService, stateService, - false + twoFactorService ); } - async logInApiKey(clientId: string, clientSecret: string): Promise { - this.selectedTwoFactorProviderType = null; - if (clientId.startsWith("organization")) { - return await this.organizationLogInHelper(clientId, clientSecret); - } - return await super.logInApiKey(clientId, clientSecret); - } - - private async organizationLogInHelper(clientId: string, clientSecret: string) { - const appId = await this.appIdService.getAppId(); - const entityId = clientId.split("organization.")[1]; - const deviceRequest = new DeviceRequest(appId, this.platformUtilsService); - const request = new TokenRequest( - null, - null, - [clientId, clientSecret], - null, - null, - false, - null, - deviceRequest + async logIn(credentials: ApiLogInCredentials): Promise { + const strategy = new OrganizationLogInStrategy( + this.cryptoService, + this.apiService, + this.tokenService, + this.appIdService, + this.platformUtilsService, + this.messagingService, + this.logService, + this.stateService, + this.twoFactorService ); - const response = await this.apiService.postIdentityToken(request); - const result = new AuthResult(); - result.twoFactor = !(response as any).accessToken; - - const tokenResponse = response as IdentityTokenResponse; - result.resetMasterPassword = tokenResponse.resetMasterPassword; - await this.stateService.addAccount( - new Account({ - profile: { - ...new AccountProfile(), - ...{ - userId: entityId, - apiKeyClientId: clientId, - entityId: entityId, - }, - }, - tokens: { - ...new AccountTokens(), - ...{ - accessToken: tokenResponse.accessToken, - refreshToken: tokenResponse.refreshToken, - }, - }, - keys: { - ...new AccountKeys(), - ...{ - apiKeyClientSecret: clientSecret, - }, - }, - directorySettings: new DirectorySettings(), - directoryConfigurations: new DirectoryConfigurations(), - }) - ); - return result; + return strategy.logIn(credentials); } } diff --git a/src/services/noop/noopTwoFactor.service.ts b/src/services/noop/noopTwoFactor.service.ts new file mode 100644 index 00000000..d68086d7 --- /dev/null +++ b/src/services/noop/noopTwoFactor.service.ts @@ -0,0 +1,42 @@ +import { + TwoFactorProviderDetails, + TwoFactorService, +} from "jslib-common/abstractions/twoFactor.service"; + +import { TwoFactorProviderType } from "jslib-common/enums/twoFactorProviderType"; + +import { IdentityTwoFactorResponse } from "jslib-common/models/response/identityTwoFactorResponse"; + +export class NoopTwoFactorService implements TwoFactorService { + init() { + // Noop + } + + getSupportedProviders(win: Window): TwoFactorProviderDetails[] { + return null; + } + + getDefaultProvider(webAuthnSupported: boolean): TwoFactorProviderType { + return null; + } + + setSelectedProvider(type: TwoFactorProviderType) { + // Noop + } + + clearSelectedProvider() { + // Noop + } + + setProviders(response: IdentityTwoFactorResponse) { + // Noop + } + + clearProviders() { + // Noop + } + + getProviders(): Map { + return null; + } +}