From 8d92ae4010e8a3ff15822c34d998b9107808688d Mon Sep 17 00:00:00 2001 From: "Robert St. John" Date: Thu, 21 Nov 2024 19:27:11 -0700 Subject: [PATCH] refactor(service): users/auth: refactor some app level logic to internal/domain services for reuse --- .../ingress/ingress.adapters.db.mongoose.ts | 6 +- service/src/ingress/ingress.app.api.ts | 2 +- service/src/ingress/ingress.app.impl.ts | 48 +++++--------- service/src/ingress/ingress.entities.ts | 35 +++++++--- service/src/ingress/ingress.services.api.ts | 37 ++++++++++- service/src/ingress/ingress.services.impl.ts | 66 +++++++++++++++++-- service/src/ingress/local-idp.app.impl.ts | 2 +- 7 files changed, 141 insertions(+), 55 deletions(-) diff --git a/service/src/ingress/ingress.adapters.db.mongoose.ts b/service/src/ingress/ingress.adapters.db.mongoose.ts index 5bbea9597..4cc7b9968 100644 --- a/service/src/ingress/ingress.adapters.db.mongoose.ts +++ b/service/src/ingress/ingress.adapters.db.mongoose.ts @@ -157,9 +157,9 @@ export class UserIngressBindingsMongooseRepository implements UserIngressBinding constructor(readonly model: UserIngressBindingsModel) {} - async readBindingsForUser(userId: UserId): Promise { + async readBindingsForUser(userId: UserId): Promise { const doc = await this.model.findById(userId, null, { lean: true }) - return doc ? { userId, bindingsByIdp: new Map(Object.entries(doc?.bindings || {})) } : null + return { userId, bindingsByIdpId: new Map(Object.entries(doc?.bindings || {})) } } async readAllBindingsForIdp(idpId: IdentityProviderId, paging?: PagingParameters | undefined): Promise> { @@ -170,7 +170,7 @@ export class UserIngressBindingsMongooseRepository implements UserIngressBinding const _id = new ObjectId(userId) const bindingsUpdate = { $set: { [`bindings.${binding.idpId}`]: binding } } const doc = await this.model.findOneAndUpdate({ _id }, bindingsUpdate, { upsert: true, new: true }) - return { userId, bindingsByIdp: new Map(Object.entries(doc.bindings)) } + return { userId, bindingsByIdpId: new Map(Object.entries(doc.bindings)) } } async deleteBinding(userId: UserId, idpId: IdentityProviderId): Promise { diff --git a/service/src/ingress/ingress.app.api.ts b/service/src/ingress/ingress.app.api.ts index c926c9e42..1889b833a 100644 --- a/service/src/ingress/ingress.app.api.ts +++ b/service/src/ingress/ingress.app.api.ts @@ -41,7 +41,7 @@ export interface AdmitFromIdentityProviderRequest { } export interface AdmitFromIdentityProviderResult { - mageAccount: User + mageAccount: UserExpanded admissionToken: string } diff --git a/service/src/ingress/ingress.app.impl.ts b/service/src/ingress/ingress.app.impl.ts index ae46cd297..6233cc26e 100644 --- a/service/src/ingress/ingress.app.impl.ts +++ b/service/src/ingress/ingress.app.impl.ts @@ -1,9 +1,8 @@ import { entityNotFound, infrastructureError } from '../app.api/app.api.errors' import { AppResponse } from '../app.api/app.api.global' -import { UserRepository } from '../entities/users/entities.users' import { AdmitFromIdentityProviderOperation, AdmitFromIdentityProviderRequest, authenticationFailedError, EnrollMyselfOperation, EnrollMyselfRequest } from './ingress.app.api' -import { IdentityProviderRepository, IdentityProviderUser, UserIngressBindingsRepository } from './ingress.entities' -import { EnrollNewUser } from './ingress.services.api' +import { IdentityProviderRepository, IdentityProviderUser } from './ingress.entities' +import { AdmissionDeniedReason, AdmitUserFromIdentityProviderAccount, EnrollNewUser } from './ingress.services.api' import { LocalIdpCreateAccountOperation } from './local-idp.app.api' import { JWTService, TokenAssertion } from './verification' @@ -32,12 +31,13 @@ export function CreateEnrollMyselfOperation(createLocalIdpAccount: LocalIdpCreat } const enrollmentResult = await enrollNewUser(candidateMageAccount, localIdp) + // TODO: auto-activate account after enrollment policy throw new Error('unimplemented') } } -export function CreateAdmitFromIdentityProviderOperation(idpRepo: IdentityProviderRepository, ingressBindingRepo: UserIngressBindingsRepository, userRepo: UserRepository, enrollNewUser: EnrollNewUser, tokenService: JWTService): AdmitFromIdentityProviderOperation { +export function CreateAdmitFromIdentityProviderOperation(idpRepo: IdentityProviderRepository, admitFromIdpAccount: AdmitUserFromIdentityProviderAccount, tokenService: JWTService): AdmitFromIdentityProviderOperation { return async function admitFromIdentityProvider(req: AdmitFromIdentityProviderRequest): ReturnType { const idp = await idpRepo.findIdpByName(req.identityProviderName) if (!idp) { @@ -45,42 +45,24 @@ export function CreateAdmitFromIdentityProviderOperation(idpRepo: IdentityProvid } const idpAccount = req.identityProviderUser console.info(`admitting user ${idpAccount.username} from identity provider ${idp.name}`) - const mageAccount = await userRepo.findByUsername(idpAccount.username) - .then(existingAccount => { - if (existingAccount) { - return ingressBindingRepo.readBindingsForUser(existingAccount.id).then(ingressBindings => { - return { mageAccount: existingAccount, ingressBindings } - }) + const admission = await admitFromIdpAccount(idpAccount, idp) + if (admission.action === 'denied') { + if (admission.mageAccount) { + if (admission.reason === AdmissionDeniedReason.PendingApproval) { + return AppResponse.error(authenticationFailedError(admission.mageAccount.username, idp.name, 'Your account requires approval from a Mage administrator.')) } - return enrollNewUser(idpAccount, idp) - }) - .then(enrolled => { - const { mageAccount, ingressBindings } = enrolled - if (ingressBindings.bindingsByIdp.has(idp.id)) { - return mageAccount + if (admission.reason === AdmissionDeniedReason.Disabled) { + return AppResponse.error(authenticationFailedError(admission.mageAccount.username, idp.name, 'Your account is disabled.')) } - console.error(`user ${mageAccount.username} has no ingress binding to identity provider ${idp.name}`) - return null - }) - .catch(err => { - console.error(`error creating user account ${idpAccount.username} from identity provider ${idp.name}`, err) - return null - }) - if (!mageAccount) { + } return AppResponse.error(authenticationFailedError(idpAccount.username, idp.name)) } - if (!mageAccount.active) { - return AppResponse.error(authenticationFailedError(mageAccount.username, idp.name, 'Your account requires approval from a Mage administrator.')) - } - if (!mageAccount.enabled) { - return AppResponse.error(authenticationFailedError(mageAccount.username, idp.name, 'Your account is disabled.')) - } try { - const admissionToken = await tokenService.generateToken(mageAccount.id, TokenAssertion.Authenticated, 5 * 60) - return AppResponse.success({ mageAccount, admissionToken }) + const admissionToken = await tokenService.generateToken(admission.mageAccount.id, TokenAssertion.Authenticated, 5 * 60) + return AppResponse.success({ mageAccount: admission.mageAccount, admissionToken }) } catch (err) { - console.error(`error generating admission token while authenticating user ${mageAccount.username}`, err) + console.error(`error generating admission token while authenticating user ${admission.mageAccount.username}`, err) return AppResponse.error(infrastructureError('An unexpected error occurred while generating an authentication token.')) } } diff --git a/service/src/ingress/ingress.entities.ts b/service/src/ingress/ingress.entities.ts index 6a4dc3d91..46c284646 100644 --- a/service/src/ingress/ingress.entities.ts +++ b/service/src/ingress/ingress.entities.ts @@ -105,15 +105,13 @@ export interface UserIngressBinding { * multiple ingress bindings for different identity providers with different account identifiers. */ idpAccountId?: string - /** - * Any attributes the identity provider or protocol needs to persist about the account mapping - */ - idpAccountAttrs?: Record + // TODO: unused for now + // idpAccountAttrs?: Record } export type UserIngressBindings = { userId: UserId - bindingsByIdp: Map + bindingsByIdpId: Map } export type IdentityProviderMutableAttrs = Omit @@ -133,7 +131,7 @@ export interface UserIngressBindingsRepository { /** * Return null if the user has no persisted bindings entry. */ - readBindingsForUser(userId: UserId): Promise + readBindingsForUser(userId: UserId): Promise readAllBindingsForIdp(idpId: IdentityProviderId, paging?: PagingParameters): Promise> /** * Save the given ingress binding to the bindings dictionary for the given user, creating or updating as necessary. @@ -154,10 +152,29 @@ export interface UserIngressBindingsRepository { deleteAllBindingsForIdp(idpId: IdentityProviderId): Promise } +export type AdmissionAction = + | { admitNew: UserIngressBinding, admitExisting: false, deny: false } + | { admitExisting: UserIngressBinding, admitNew: false, deny: false } + | { deny: true, admitNew: false, admitExisting: false } + +export function determinUserIngressBindingAdmission(idpAccount: IdentityProviderUser, idp: IdentityProvider, bindings: UserIngressBindings): AdmissionAction { + if (bindings.bindingsByIdpId.size === 0) { + // new user account + const now = new Date(Date.now()) + return { admitNew: { created: now, updated: now, idpId: idp.id, idpAccountId: idpAccount.idpAccountId }, admitExisting: false, deny: false } + } + const binding = bindings.bindingsByIdpId.get(idp.id) + if (binding) { + // existing account bound to idp + return { admitExisting: binding, admitNew: false, deny: false } + } + return { deny: true, admitNew: false, admitExisting: false } +} + /** - * Return a new user object from the given identity provider account information suitable to persist as newly enrolled - * user. The enrollment policy for the identity provider determines the `active` flag and assigned role for the new - * user. + * Return a new user object from the given identity provider account information suitable to persist as a newly + * enrolled user. The enrollment policy for the identity provider determines the `active` flag and assigned role for + * the new user. */ export function createEnrollmentCandidateUser(idpAccount: IdentityProviderUser, idp: IdentityProvider): Omit { const policy = idp.userEnrollmentPolicy diff --git a/service/src/ingress/ingress.services.api.ts b/service/src/ingress/ingress.services.api.ts index 63315f1c0..a3ad3c8a6 100644 --- a/service/src/ingress/ingress.services.api.ts +++ b/service/src/ingress/ingress.services.api.ts @@ -1,6 +1,39 @@ -import { User } from '../entities/users/entities.users' +import { UserExpanded } from '../entities/users/entities.users' import { IdentityProvider, IdentityProviderUser, UserIngressBindings } from './ingress.entities' +export type AdmissionResult = + | { + /** + * `'admitted'` if the user account is valid for admission and access to Mage, `'denied'` otherwise + */ + action: 'admitted', + /** + * The existing or newly enrolled Mage account + */ + mageAccount: UserExpanded, + /** + * Whether the admission resulted in a new Mage account enrollment + */ + enrolled: boolean, + } + | { + action: 'denied', + reason: AdmissionDeniedReason, + mageAccount: UserExpanded | null, + enrolled: boolean, + } + +export enum AdmissionDeniedReason { + PendingApproval = 'PendingApproval', + Disabled = 'Disabled', + NameConflict = 'NameConflict', + InternalError = 'InternalError', +} + +export interface AdmitUserFromIdentityProviderAccount { + (idpAccount: IdentityProviderUser, idp: IdentityProvider): Promise +} + export interface EnrollNewUser { - (idpAccount: IdentityProviderUser, idp: IdentityProvider): Promise<{ mageAccount: User, ingressBindings: UserIngressBindings }> + (idpAccount: IdentityProviderUser, idp: IdentityProvider): Promise<{ mageAccount: UserExpanded, ingressBindings: UserIngressBindings }> } \ No newline at end of file diff --git a/service/src/ingress/ingress.services.impl.ts b/service/src/ingress/ingress.services.impl.ts index 540aec846..0c0edadcb 100644 --- a/service/src/ingress/ingress.services.impl.ts +++ b/service/src/ingress/ingress.services.impl.ts @@ -1,8 +1,8 @@ import { MageEventId } from '../entities/events/entities.events' import { Team, TeamId } from '../entities/teams/entities.teams' -import { User, UserId, UserRepository, UserRepositoryError } from '../entities/users/entities.users' -import { createEnrollmentCandidateUser, IdentityProvider, IdentityProviderUser, UserIngressBindingsRepository, UserIngressBindings } from './ingress.entities' -import { EnrollNewUser } from './ingress.services.api' +import { UserExpanded, UserId, UserRepository, UserRepositoryError } from '../entities/users/entities.users' +import { createEnrollmentCandidateUser, IdentityProvider, IdentityProviderUser, UserIngressBindingsRepository, UserIngressBindings, determinUserIngressBindingAdmission } from './ingress.entities' +import { AdmissionDeniedReason, AdmissionResult, AdmitUserFromIdentityProviderAccount, EnrollNewUser } from './ingress.services.api' export interface AssignTeamMember { (member: UserId, team: TeamId): Promise @@ -12,20 +12,74 @@ export interface FindEventTeam { (mageEventId: MageEventId): Promise } -export function CreateProcessNewUserEnrollmentService(userRepo: UserRepository, ingressBindingRepo: UserIngressBindingsRepository, findEventTeam: FindEventTeam, assignTeamMember: AssignTeamMember): EnrollNewUser { - return async function processNewUserEnrollment(idpAccount: IdentityProviderUser, idp: IdentityProvider): Promise<{ mageAccount: User, ingressBindings: UserIngressBindings }> { +export function CreateUserAdmissionService(userRepo: UserRepository, ingressBindingRepo: UserIngressBindingsRepository, enrollNewUser: EnrollNewUser): AdmitUserFromIdentityProviderAccount { + return async function(idpAccount: IdentityProviderUser, idp: IdentityProvider): Promise { + return userRepo.findByUsername(idpAccount.username) + .then(existingAccount => { + if (existingAccount) { + return ingressBindingRepo.readBindingsForUser(existingAccount.id).then(ingressBindings => { + return { enrolled: false, mageAccount: existingAccount, ingressBindings } + }) + } + console.info(`enrolling new user account ${idpAccount.username} from identity provider ${idp.name}`) + return enrollNewUser(idpAccount, idp).then(enrollment => ({ enrolled: true, ...enrollment })) + }) + .then(userIngress => { + const { enrolled, mageAccount, ingressBindings } = userIngress + const idpAdmission = determinUserIngressBindingAdmission(idpAccount, idp, ingressBindings) + if (idpAdmission.deny) { + console.error(`user ${mageAccount.username} has no ingress binding to identity provider ${idp.name}`) + return { action: 'denied', reason: AdmissionDeniedReason.NameConflict, enrolled, mageAccount } + } + if (idpAdmission.admitNew) { + return ingressBindingRepo.saveUserIngressBinding(mageAccount.id, idpAdmission.admitNew) + .then(() => ({ action: 'admitted', mageAccount, enrolled })) + .catch(err => { + console.error(`error saving ingress binding for user ${mageAccount.username} to idp ${idp.name}`, err) + return { action: 'denied', reason: AdmissionDeniedReason.InternalError, mageAccount, enrolled } + }) + } + return { action: 'admitted', mageAccount, enrolled } + }) + .then(userIngress => { + const { action, mageAccount, enrolled } = userIngress + if (!mageAccount) { + return { action: 'denied', reason: AdmissionDeniedReason.InternalError, mageAccount, enrolled } + } + if (action === 'denied') { + return userIngress + } + if (!mageAccount.active) { + return { action: 'denied', reason: AdmissionDeniedReason.PendingApproval, mageAccount, enrolled } + } + if (!mageAccount.enabled) { + return { action: 'denied', reason: AdmissionDeniedReason.Disabled, mageAccount, enrolled } + } + return userIngress + }) + .catch(err => { + console.error(`error admitting user account ${idpAccount.username} from identity provider ${idp.name}`, err) + return { action: 'denied', reason: AdmissionDeniedReason.InternalError, enrolled: false, mageAccount: null } + }) + } +} + +export function CreateNewUserEnrollmentService(userRepo: UserRepository, ingressBindingRepo: UserIngressBindingsRepository, findEventTeam: FindEventTeam, assignTeamMember: AssignTeamMember): EnrollNewUser { + return async function processNewUserEnrollment(idpAccount: IdentityProviderUser, idp: IdentityProvider): Promise<{ mageAccount: UserExpanded, ingressBindings: UserIngressBindings }> { console.info(`enrolling new user account ${idpAccount.username} from identity provider ${idp.name}`) const candidate = createEnrollmentCandidateUser(idpAccount, idp) const mageAccount = await userRepo.create(candidate) if (mageAccount instanceof UserRepositoryError) { throw mageAccount } + const now = new Date() const ingressBindings = await ingressBindingRepo.saveUserIngressBinding( mageAccount.id, { idpId: idp.id, idpAccountId: idpAccount.username, - idpAccountAttrs: {}, + created: now, + updated: now, } ) if (ingressBindings instanceof Error) { diff --git a/service/src/ingress/local-idp.app.impl.ts b/service/src/ingress/local-idp.app.impl.ts index c35807235..5a99c4520 100644 --- a/service/src/ingress/local-idp.app.impl.ts +++ b/service/src/ingress/local-idp.app.impl.ts @@ -36,7 +36,7 @@ export function CreateLocalIdpCreateAccountOperation(repo: LocalIdpRepository): const createdAccount = await repo.createLocalAccount(candidateAccount) if (createdAccount instanceof LocalIdpError) { if (createdAccount instanceof LocalIdpDuplicateUsernameError) { - console.info(`attempted to create local account with duplicate username ${req.username}`) + console.error(`attempted to create local account with duplicate username ${req.username}`, createdAccount) } return AppResponse.error(invalidInput(`Failed to create account ${req.username}.`)) }