From 04bd2e4ed292bf314ec3885dd74940ce472e9665 Mon Sep 17 00:00:00 2001 From: "Robert St. John" Date: Mon, 25 Nov 2024 12:59:45 -0700 Subject: [PATCH] refactor(service): users/auth: move local idp app layer operations to internal services to allow dependency injection and reuse --- service/src/ingress/ingress.app.impl.ts | 27 +++++---- service/src/ingress/ingress.protocol.local.ts | 19 ++++--- service/src/ingress/local-idp.app.api.ts | 12 ---- service/src/ingress/local-idp.app.impl.ts | 45 --------------- service/src/ingress/local-idp.entities.ts | 4 ++ service/src/ingress/local-idp.services.api.ts | 15 +++++ .../src/ingress/local-idp.services.impl.ts | 55 +++++++++++++++++++ 7 files changed, 99 insertions(+), 78 deletions(-) delete mode 100644 service/src/ingress/local-idp.app.api.ts delete mode 100644 service/src/ingress/local-idp.app.impl.ts create mode 100644 service/src/ingress/local-idp.services.api.ts create mode 100644 service/src/ingress/local-idp.services.impl.ts diff --git a/service/src/ingress/ingress.app.impl.ts b/service/src/ingress/ingress.app.impl.ts index 6233cc26e..7cf9af697 100644 --- a/service/src/ingress/ingress.app.impl.ts +++ b/service/src/ingress/ingress.app.impl.ts @@ -1,21 +1,25 @@ -import { entityNotFound, infrastructureError } from '../app.api/app.api.errors' +import { entityNotFound, infrastructureError, invalidInput } from '../app.api/app.api.errors' import { AppResponse } from '../app.api/app.api.global' import { AdmitFromIdentityProviderOperation, AdmitFromIdentityProviderRequest, authenticationFailedError, EnrollMyselfOperation, EnrollMyselfRequest } from './ingress.app.api' import { IdentityProviderRepository, IdentityProviderUser } from './ingress.entities' import { AdmissionDeniedReason, AdmitUserFromIdentityProviderAccount, EnrollNewUser } from './ingress.services.api' -import { LocalIdpCreateAccountOperation } from './local-idp.app.api' +import { LocalIdpError, LocalIdpInvalidPasswordError } from './local-idp.entities' +import { MageLocalIdentityProviderService } from './local-idp.services.api' import { JWTService, TokenAssertion } from './verification' -export function CreateEnrollMyselfOperation(createLocalIdpAccount: LocalIdpCreateAccountOperation, idpRepo: IdentityProviderRepository, enrollNewUser: EnrollNewUser): EnrollMyselfOperation { +export function CreateEnrollMyselfOperation(localIdp: MageLocalIdentityProviderService, idpRepo: IdentityProviderRepository, enrollNewUser: EnrollNewUser): EnrollMyselfOperation { return async function enrollMyself(req: EnrollMyselfRequest): ReturnType { - const localAccountCreate = await createLocalIdpAccount(req) - if (localAccountCreate.error) { - return AppResponse.error(localAccountCreate.error) + const localIdpAccount = await localIdp.createAccount(req) + if (localIdpAccount instanceof LocalIdpError) { + if (localIdpAccount instanceof LocalIdpInvalidPasswordError) { + return AppResponse.error(invalidInput(localIdpAccount.message)) + } + console.error('error creating local idp account for self-enrollment', localIdpAccount) + return AppResponse.error(invalidInput('Error creating local Mage account')) } - const localAccount = localAccountCreate.success! const candidateMageAccount: IdentityProviderUser = { - username: localAccount.username, + username: localIdpAccount.username, displayName: req.displayName, phones: [], } @@ -25,12 +29,11 @@ export function CreateEnrollMyselfOperation(createLocalIdpAccount: LocalIdpCreat if (req.phone) { candidateMageAccount.phones = [ { number: req.phone, type: 'Main' } ] } - const localIdp = await idpRepo.findIdpByName('local') - if (!localIdp) { + const idp = await idpRepo.findIdpByName('local') + if (!idp) { throw new Error('local idp not found') } - const enrollmentResult = await enrollNewUser(candidateMageAccount, localIdp) - + const enrollmentResult = await enrollNewUser(candidateMageAccount, idp) // TODO: auto-activate account after enrollment policy throw new Error('unimplemented') diff --git a/service/src/ingress/ingress.protocol.local.ts b/service/src/ingress/ingress.protocol.local.ts index 05f6ab5b5..d30ad7a05 100644 --- a/service/src/ingress/ingress.protocol.local.ts +++ b/service/src/ingress/ingress.protocol.local.ts @@ -3,8 +3,9 @@ import express from 'express' import { Strategy as LocalStrategy, VerifyFunction as LocalStrategyVerifyFunction } from 'passport-local' import { LocalIdpAccount } from './local-idp.entities' import { IdentityProviderUser } from './ingress.entities' -import { LocalIdpAuthenticateOperation } from './local-idp.app.api' import { IngressProtocolWebBinding, IngressResponseType } from './ingress.protocol.bindings' +import { MageLocalIdentityProviderService } from './local-idp.services.api' +import { permissionDenied } from '../app.api/app.api.errors' function userForLocalIdpAccount(account: LocalIdpAccount): IdentityProviderUser { @@ -15,15 +16,15 @@ function userForLocalIdpAccount(account: LocalIdpAccount): IdentityProviderUser } } -function createLocalStrategy(localIdpAuthenticate: LocalIdpAuthenticateOperation, flowState: string | undefined): passport.Strategy { +function createLocalStrategy(localIdp: MageLocalIdentityProviderService, flowState: string | undefined): passport.Strategy { const verify: LocalStrategyVerifyFunction = async function LocalIngressProtocolVerify(username, password, done) { - const authResult = await localIdpAuthenticate({ username, password }) - if (authResult.success) { - const localAccount = authResult.success - const localIdpUser = userForLocalIdpAccount(localAccount) - return done(null, { admittingFromIdentityProvider: { idpName: 'local', account: localIdpUser, flowState } }) + const authResult = await localIdp.authenticate({ username, password }) + if (!authResult || authResult.failed) { + return done(permissionDenied('local authentication failed', username)) } - return done(authResult.error) + const localAccount = authResult.authenticated + const localIdpUser = userForLocalIdpAccount(localAccount) + return done(null, { admittingFromIdentityProvider: { idpName: 'local', account: localIdpUser, flowState } }) } return new LocalStrategy(verify) } @@ -40,7 +41,7 @@ const validateSigninRequest: express.RequestHandler = function LocalProtocolIngr next() } -export function createWebBinding(passport: passport.Authenticator, localIdpAuthenticate: LocalIdpAuthenticateOperation): IngressProtocolWebBinding { +export function createLocalProtocolWebBinding(passport: passport.Authenticator, localIdpAuthenticate: MageLocalIdentityProviderService): IngressProtocolWebBinding { return { ingressResponseType: IngressResponseType.Direct, beginIngressFlow: (req, res, next, flowState): any => { diff --git a/service/src/ingress/local-idp.app.api.ts b/service/src/ingress/local-idp.app.api.ts deleted file mode 100644 index 70ad88862..000000000 --- a/service/src/ingress/local-idp.app.api.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { EntityNotFoundError, InvalidInputError } from '../app.api/app.api.errors' -import { AppResponse } from '../app.api/app.api.global' -import { LocalIdpAccount, LocalIdpCredentials } from './local-idp.entities' - - -export interface LocalIdpAuthenticateOperation { - (req: LocalIdpCredentials): Promise> -} - -export interface LocalIdpCreateAccountOperation { - (req: LocalIdpCredentials): Promise> -} diff --git a/service/src/ingress/local-idp.app.impl.ts b/service/src/ingress/local-idp.app.impl.ts deleted file mode 100644 index 5a99c4520..000000000 --- a/service/src/ingress/local-idp.app.impl.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { invalidInput } from '../app.api/app.api.errors' -import { AppResponse } from '../app.api/app.api.global' -import { LocalIdpAuthenticateOperation, LocalIdpCreateAccountOperation } from './local-idp.app.api' -import { attemptAuthentication, LocalIdpDuplicateUsernameError, LocalIdpError, LocalIdpInvalidPasswordError, LocalIdpRepository, prepareNewAccount } from './local-idp.entities' - - -export function CreateLocalIdpAuthenticateOperation(repo: LocalIdpRepository): LocalIdpAuthenticateOperation { - return async function localIdpAuthenticate(req): ReturnType { - const account = await repo.readLocalAccount(req.username) - if (!account) { - console.info('local account does not exist:', req.username) - return AppResponse.error(invalidInput(`Failed to authenticate user ${req.username}`)) - } - const securityPolicy = await repo.readSecurityPolicy() - const attempt = await attemptAuthentication(account, req.password, securityPolicy.accountLock) - if (attempt.failed) { - console.info('local authentication failed', attempt.failed) - return AppResponse.error(invalidInput(`Failed to authenticate user ${req.username}`)) - } - const accountSaved = await repo.updateLocalAccount(attempt.authenticated) - if (accountSaved) { - return AppResponse.success(accountSaved) - } - console.error(`account for username ${req.username} did not exist for update after authentication`) - return AppResponse.error(invalidInput(`Failed to authenticate user ${req.username}`)) - } -} - -export function CreateLocalIdpCreateAccountOperation(repo: LocalIdpRepository): LocalIdpCreateAccountOperation { - return async function localIdpCreateAccount(req) { - const securityPolicy = await repo.readSecurityPolicy() - const candidateAccount = await prepareNewAccount(req.username, req.password, securityPolicy) - if (candidateAccount instanceof LocalIdpInvalidPasswordError) { - return AppResponse.error(invalidInput(`Failed to create account ${req.username}.`, [ candidateAccount.message, 'password' ])) - } - const createdAccount = await repo.createLocalAccount(candidateAccount) - if (createdAccount instanceof LocalIdpError) { - if (createdAccount instanceof LocalIdpDuplicateUsernameError) { - console.error(`attempted to create local account with duplicate username ${req.username}`, createdAccount) - } - return AppResponse.error(invalidInput(`Failed to create account ${req.username}.`)) - } - return AppResponse.success(createdAccount) - } -} \ No newline at end of file diff --git a/service/src/ingress/local-idp.entities.ts b/service/src/ingress/local-idp.entities.ts index d7a75217b..ba9a59790 100644 --- a/service/src/ingress/local-idp.entities.ts +++ b/service/src/ingress/local-idp.entities.ts @@ -158,6 +158,10 @@ export class LocalIdpFailedAuthenticationError extends LocalIdpError { } } +export class LocalIdpAccountNotFoundError extends LocalIdpError { + +} + function invalidPasswordError(reason: string): LocalIdpError { return new LocalIdpError(reason) } diff --git a/service/src/ingress/local-idp.services.api.ts b/service/src/ingress/local-idp.services.api.ts new file mode 100644 index 000000000..0a342d4df --- /dev/null +++ b/service/src/ingress/local-idp.services.api.ts @@ -0,0 +1,15 @@ +import { LocalIdpAccount, LocalIdpAuthenticationResult, LocalIdpCredentials, LocalIdpError } from './local-idp.entities' + + +export interface MageLocalIdentityProviderService { + createAccount(credentials: LocalIdpCredentials): Promise + /** + * Return `null` if no account for the given username exists. + */ + deleteAccount(username: string): Promise + /** + * Return `null` if no account for the given username exists. If authentication fails, update the corresponding + * account according to the service's account lock policy. + */ + authenticate(credentials: LocalIdpCredentials): Promise +} \ No newline at end of file diff --git a/service/src/ingress/local-idp.services.impl.ts b/service/src/ingress/local-idp.services.impl.ts new file mode 100644 index 000000000..8a94e5ff9 --- /dev/null +++ b/service/src/ingress/local-idp.services.impl.ts @@ -0,0 +1,55 @@ +import { attemptAuthentication, LocalIdpAccount, LocalIdpAuthenticationResult, LocalIdpCredentials, LocalIdpDuplicateUsernameError, LocalIdpError, LocalIdpInvalidPasswordError, LocalIdpRepository, prepareNewAccount } from './local-idp.entities' +import { MageLocalIdentityProviderService } from './local-idp.services.api' + + +export function createLocalIdentityProviderService(repo: LocalIdpRepository): MageLocalIdentityProviderService { + + async function createAccount(credentials: LocalIdpCredentials): Promise { + const securityPolicy = await repo.readSecurityPolicy() + const { username, password } = credentials + const candidateAccount = await prepareNewAccount(username, password, securityPolicy) + if (candidateAccount instanceof LocalIdpInvalidPasswordError) { + return candidateAccount + } + const createdAccount = await repo.createLocalAccount(candidateAccount) + if (createdAccount instanceof LocalIdpError) { + if (createdAccount instanceof LocalIdpDuplicateUsernameError) { + console.error(`attempted to create local account with duplicate username ${username}`, createdAccount) + } + return createdAccount + } + return createdAccount + } + + function deleteAccount(username: string): Promise { + return repo.deleteLocalAccount(username) + } + + async function authenticate(credentials: LocalIdpCredentials): Promise { + const { username, password } = credentials + const account = await repo.readLocalAccount(username) + if (!account) { + console.info('local account does not exist:', username) + return null + } + const securityPolicy = await repo.readSecurityPolicy() + const attempt = await attemptAuthentication(account, password, securityPolicy.accountLock) + if (attempt.failed) { + console.info('local authentication failed', attempt.failed) + return attempt + } + const accountSaved = await repo.updateLocalAccount(attempt.authenticated) + if (accountSaved) { + attempt.authenticated = accountSaved + return attempt + } + console.error(`account for username ${username} did not exist for update after authentication attempt`) + return null + } + + return { + createAccount, + deleteAccount, + authenticate, + } +}