From 7c7367a5ab253246962741e0b0e564ae62dce6b0 Mon Sep 17 00:00:00 2001 From: Andrew Foote Date: Fri, 22 May 2026 12:15:58 -0400 Subject: [PATCH 1/2] Fix database inconsistency with account cleanup --- src/repositories/baseUserRepository.js | 226 ++++++++++++++----------- 1 file changed, 126 insertions(+), 100 deletions(-) diff --git a/src/repositories/baseUserRepository.js b/src/repositories/baseUserRepository.js index 720fae515..4fda674a8 100644 --- a/src/repositories/baseUserRepository.js +++ b/src/repositories/baseUserRepository.js @@ -412,103 +412,131 @@ class BaseUserRepository extends BaseRepository { * @param {boolean} [isRegistryObject=true] - If false, returns a legacy user object. * @returns {Promise} The updated user object. */ - async updateUser (username, orgShortname, incomingParameters, options = {}, isRegistryObject = true, requestingUserUUID = null) { +async updateUser (username, orgShortname, incomingParameters, options = {}, isRegistryObject = true, requestingUserUUID = null) { const { deepRemoveEmpty } = require('../utils/utils') const baseOrgRepository = new BaseOrgRepository() const legacyUserRepo = new UserRepository() const { createAuditLogEntry } = require('./baseOrgRepositoryHelpers') const registryOrg = await baseOrgRepository.getOrgObject(orgShortname, false, options) const originalRegistryOrg = registryOrg.toObject() + const legacyUser = await legacyUserRepo.findOneByUserNameAndOrgUUID(username, registryOrg.UUID, null, options) - const registryUser = await this.findOneByUsernameAndOrgShortname(username, orgShortname, options, true) // WE always want the registry user + const registryUser = await this.findOneByUsernameAndOrgShortname(username, orgShortname, options, true) - registryUser.username = incomingParameters?.new_username ?? registryUser.username - legacyUser.username = incomingParameters?.new_username ?? legacyUser.username + if (!registryUser && !legacyUser) { + throw new Error('User not found') + } + + // Safely assign usernames defensively + if (registryUser) { + registryUser.username = incomingParameters?.new_username ?? registryUser.username + } + if (legacyUser) { + legacyUser.username = incomingParameters?.new_username ?? legacyUser.username + } if (incomingParameters?.active != null) { const isConsideredActive = incomingParameters.active === true || String(incomingParameters.active).toLowerCase() === 'true' - registryUser.status = isConsideredActive ? 'active' : 'inactive' - legacyUser.active = incomingParameters.active ?? legacyUser.active + if (registryUser) { + registryUser.status = isConsideredActive ? 'active' : 'inactive' + } + if (legacyUser) { + legacyUser.active = incomingParameters.active ?? legacyUser.active + } } ['name.last', 'name.first', 'name.middle', 'name.suffix'].forEach(field => { - _.set(registryUser, field, _.get(incomingParameters, field, _.get(registryUser, field, ''))) - _.set(legacyUser, field, _.get(incomingParameters, field, _.get(legacyUser, field, ''))) + if (registryUser) _.set(registryUser, field, _.get(incomingParameters, field, _.get(registryUser, field, ''))) + if (legacyUser) _.set(legacyUser, field, _.get(incomingParameters, field, _.get(legacyUser, field, ''))) }) + // Get the UUID from whichever user object actually exists + const userUUID = registryUser?.UUID ?? legacyUser?.UUID + const rolesToAdd = _.flattenDeep(_.compact(_.get(incomingParameters, 'active_roles.add'))) const rolesToRemove = _.flattenDeep(_.compact(_.get(incomingParameters, 'active_roles.remove'))) - if (rolesToRemove.includes('ADMIN')) { + + if (rolesToRemove.includes('ADMIN') && userUUID) { if (Array.isArray(registryOrg.admins)) { - registryOrg.admins.pull(registryUser.UUID) + registryOrg.admins.pull(userUUID) } } - if (rolesToAdd.includes('ADMIN') && !incomingParameters?.org_short_name) { - // Use the already fetched registryOrg instead of querying again + if (rolesToAdd.includes('ADMIN') && !incomingParameters?.org_short_name && userUUID) { if (!Array.isArray(registryOrg.admins)) { registryOrg.admins = [] } - registryOrg.admins.addToSet(registryUser.UUID) + registryOrg.admins.addToSet(userUUID) } - const initialRoles = legacyUser.authority?.active_roles ?? [] + // Handle roles calculation fallback + const initialRoles = legacyUser?.authority?.active_roles ?? [] const finalRoles = [...new Set([...initialRoles, ...rolesToAdd])].filter(role => !rolesToRemove.includes(role)) - registryUser.role = finalRoles[0] ?? '' - _.set(legacyUser, 'authority.active_roles', finalRoles) + + if (registryUser) { + registryUser.role = finalRoles[0] ?? '' + } + if (legacyUser) { + _.set(legacyUser, 'authority.active_roles', finalRoles) + } - if (incomingParameters?.org_short_name) { - // Remove us from the old users Array + if (incomingParameters?.org_short_name && userUUID) { if (Array.isArray(registryOrg.users)) { - registryOrg.users.pull(registryUser.UUID) + registryOrg.users.pull(userUUID) } - if (registryOrg.admins && registryOrg.admins.includes(registryUser.UUID)) { - registryOrg.admins.pull(registryUser.UUID) + if (registryOrg.admins && registryOrg.admins.includes(userUUID)) { + registryOrg.admins.pull(userUUID) } - // Add us to the new org (this is a genuine cross-org migration, so we must fetch the new org) + const newOrg = await baseOrgRepository.getOrgObject(incomingParameters.org_short_name) const originalNewOrg = newOrg.toObject() if (!Array.isArray(newOrg.users)) { newOrg.users = [] } - newOrg.users.addToSet(registryUser.UUID) + newOrg.users.addToSet(userUUID) - if (registryUser.role.includes('ADMIN')) { + const isUserAdmin = registryUser?.role?.includes('ADMIN') || finalRoles.includes('ADMIN') + if (isUserAdmin) { if (!Array.isArray(newOrg.admins)) { newOrg.admins = [] } - newOrg.admins.addToSet(registryUser.UUID) + newOrg.admins.addToSet(userUUID) } - legacyUser.org_UUID = newOrg.UUID + if (legacyUser) { + legacyUser.org_UUID = newOrg.UUID + } await newOrg.save(options) if (requestingUserUUID) { await createAuditLogEntry(newOrg, originalNewOrg, requestingUserUUID, options) } } - delete registryUser.role - // Single unified save for the primary org at the end + if (registryUser) { + delete registryUser.role + } + await registryOrg.save(options) if (requestingUserUUID) { await createAuditLogEntry(registryOrg, originalRegistryOrg, requestingUserUUID, options) } - await legacyUser.save(options) - await registryUser.save(options) + // Save only records that exist + if (legacyUser) await legacyUser.save(options) + if (registryUser) await registryUser.save(options) if (!isRegistryObject) { + if (!legacyUser) throw new Error('Legacy record missing; cannot return legacy format.') const plainJavascriptLegacyUser = legacyUser.toObject() - legacyUser.role = finalRoles[0] ?? '' + plainJavascriptLegacyUser.role = finalRoles[0] ?? '' delete plainJavascriptLegacyUser.__v delete plainJavascriptLegacyUser._id delete plainJavascriptLegacyUser.secret - // return deepRemoveEmpty(plainJavascriptLegacyUser) return plainJavascriptLegacyUser } + if (!registryUser) throw new Error('Registry record missing; cannot return registry format.') const plainJavascriptRegistryUser = registryUser.toObject() - // Remove private things delete plainJavascriptRegistryUser.__v delete plainJavascriptRegistryUser._id delete plainJavascriptRegistryUser.__t @@ -526,79 +554,70 @@ class BaseUserRepository extends BaseRepository { * @param {boolean} [isRegistryObject=true] - If false, accepts/returns legacy format. * @returns {Promise} The updated user object. */ - async updateUserFull (identifier, incomingUser, options = {}, isRegistryObject = true, requestingUserUUID = null) { +async updateUserFull (identifier, incomingUser, options = {}, isRegistryObject = true, requestingUserUUID = null) { const legacyUserRepo = new UserRepository() - // Find registry user by UUID const registryUser = await this.findUserByUUID(identifier, options) - if (!registryUser) { - throw new Error('Registry user not found') - } - - // Find legacy user const legacyUser = await legacyUserRepo.findOneByUUID(identifier) - if (!legacyUser) { - throw new Error('Legacy user not found') + + // Fail only if completely missing everywhere + if (!registryUser && !legacyUser) { + throw new Error('User not found in any repository') } const { ...incomingUserBody } = incomingUser - let legacyObjectRaw - let registryObjectRaw - - if (!isRegistryObject) { - legacyObjectRaw = incomingUserBody - registryObjectRaw = this.convertLegacyToRegistry(incomingUserBody) - } else { - registryObjectRaw = incomingUserBody - legacyObjectRaw = this.convertRegistryToLegacy(incomingUserBody) - } + let legacyObjectRaw = isRegistryObject ? this.convertRegistryToLegacy(incomingUserBody) : incomingUserBody + let registryObjectRaw = isRegistryObject ? incomingUserBody : this.convertLegacyToRegistry(incomingUserBody) const protectedFieldsRegistry = ['_id', 'UUID', '__v', 'secret', 'created', 'last_updated'] const protectedFieldsLegacy = ['_id', 'UUID', '__v', 'secret', 'time', 'org_UUID'] - const updatedRegistryUser = registryUser.overwrite(_.mergeWith(_.pick(registryUser.toObject(), protectedFieldsRegistry), _.omit(registryObjectRaw, protectedFieldsRegistry), skipNulls)) - const updatedLegacyUser = legacyUser.overwrite(_.mergeWith(_.pick(legacyUser.toObject(), protectedFieldsLegacy), _.omit(legacyObjectRaw, protectedFieldsLegacy), skipNulls)) + let updatedRegistryUser = null + let updatedLegacyUser = null - if (updatedRegistryUser.status !== 'active') { - updatedRegistryUser.status = 'inactive' - updatedLegacyUser.active = false - } else { - updatedLegacyUser.active = true + if (registryUser) { + updatedRegistryUser = registryUser.overwrite(_.mergeWith(_.pick(registryUser.toObject(), protectedFieldsRegistry), _.omit(registryObjectRaw, protectedFieldsRegistry), skipNulls)) + if (updatedRegistryUser.status !== 'active') { + updatedRegistryUser.status = 'inactive' + } + } + + if (legacyUser) { + updatedLegacyUser = legacyUser.overwrite(_.mergeWith(_.pick(legacyUser.toObject(), protectedFieldsLegacy), _.omit(legacyObjectRaw, protectedFieldsLegacy), skipNulls)) + // Align status from incoming payload or resolved registry state + const targetStatus = registryUser ? updatedRegistryUser.status : (isRegistryObject ? registryObjectRaw.status : 'active') + updatedLegacyUser.active = (targetStatus === 'active') } try { if (incomingUser.org_short_name) { const baseOrgRepository = new BaseOrgRepository() const { createAuditLogEntry } = require('./baseOrgRepositoryHelpers') - const currentOrgUUID = legacyUser.org_UUID - const currentOrg = await baseOrgRepository.findOneByUUID(currentOrgUUID) - const originalCurrentOrg = currentOrg.toObject() + + // Grab current org UUID using whichever document is real + const currentOrgUUID = legacyUser ? legacyUser.org_UUID : registryUser?.org_UUID // Fallback if schema supports it + const currentOrg = currentOrgUUID ? await baseOrgRepository.findOneByUUID(currentOrgUUID) : null const newOrg = await baseOrgRepository.findOneByShortName(incomingUser.org_short_name) - const originalNewOrg = newOrg.toObject() if (!newOrg) { throw new Error(`Organization ${incomingUser.org_short_name} not found`) } - // 1. Remove user from old org's users list - if (Array.isArray(currentOrg.users)) { - currentOrg.users.pull(identifier) + // Clean up old org if found + if (currentOrg) { + const originalCurrentOrg = currentOrg.toObject() + if (Array.isArray(currentOrg.users)) currentOrg.users.pull(identifier) + if (currentOrg.admins && currentOrg.admins.includes(identifier)) currentOrg.admins.pull(identifier) + await currentOrg.save(options) + if (requestingUserUUID) await createAuditLogEntry(currentOrg, originalCurrentOrg, requestingUserUUID, options) } - // 2. Remove user from old org's admins list (if present) - if (currentOrg.admins && currentOrg.admins.includes(identifier)) { - currentOrg.admins.pull(identifier) - } - - // 3. Add user to new org's users list - if (!Array.isArray(newOrg.users)) { - newOrg.users = [] - } + // Setup new org + const originalNewOrg = newOrg.toObject() + if (!Array.isArray(newOrg.users)) newOrg.users = [] newOrg.users.addToSet(identifier) - // 4. Add user to new org's admins list (if they are an admin) - const isAdmin = updatedRegistryUser.role === 'ADMIN' || (updatedLegacyUser.authority && updatedLegacyUser.authority.active_roles && updatedLegacyUser.authority.active_roles.includes('ADMIN')) - + const isAdmin = updatedRegistryUser?.role === 'ADMIN' || (updatedLegacyUser?.authority?.active_roles?.includes('ADMIN')) if (isAdmin) { if (!Array.isArray(newOrg.admins)) { newOrg.admins = [] @@ -606,40 +625,34 @@ class BaseUserRepository extends BaseRepository { newOrg.admins.addToSet(identifier) } - // 5. Update user's org_UUID - updatedLegacyUser.org_UUID = newOrg.UUID + if (updatedLegacyUser) { + updatedLegacyUser.org_UUID = newOrg.UUID + } - // Save org changes - await currentOrg.save(options) await newOrg.save(options) - if (requestingUserUUID) { - await createAuditLogEntry(currentOrg, originalCurrentOrg, requestingUserUUID, options) await createAuditLogEntry(newOrg, originalNewOrg, requestingUserUUID, options) } } - await updatedLegacyUser.save(options) - await updatedRegistryUser.save(options) + // Conditionally save records + if (updatedLegacyUser) await updatedLegacyUser.save(options) + if (updatedRegistryUser) await updatedRegistryUser.save(options) + } catch (error) { throw new Error('Failed to update user: ' + error.message) } if (!isRegistryObject) { + if (!updatedLegacyUser) throw new Error('Legacy record missing; cannot output legacy format.') const plain = updatedLegacyUser.toObject() - delete plain._id - delete plain.__v - delete plain.secret + delete plain._id; delete plain.__v; delete plain.secret return plain } - // Retrieve updated registry user + if (!updatedRegistryUser) throw new Error('Registry record missing; cannot output registry format.') const plainJsRegistryUser = updatedRegistryUser.toObject() - delete plainJsRegistryUser._id - delete plainJsRegistryUser.__v - delete plainJsRegistryUser.secret - delete plainJsRegistryUser.authority - + delete plainJsRegistryUser._id; delete plainJsRegistryUser.__v; delete plainJsRegistryUser.secret; delete plainJsRegistryUser.authority return plainJsRegistryUser } @@ -653,7 +666,7 @@ class BaseUserRepository extends BaseRepository { * @param {boolean} [isRegistryObject=true] - Unused parameter. * @returns {Promise} The new random secret key. */ - async resetSecret (username, orgShortName, options = {}, isRegistryObject = true) { +async resetSecret (username, orgShortName, options = {}, isRegistryObject = true) { const legacyUserRepo = new UserRepository() const baseOrgRepository = new BaseOrgRepository() @@ -661,12 +674,25 @@ class BaseUserRepository extends BaseRepository { const legUser = await legacyUserRepo.findOneByUserNameAndOrgUUID(username, legOrgUUID, null, options) const regUser = await this.findOneByUsernameAndOrgShortname(username, orgShortName, options, true) + // Fail ONLY if the user is completely missing from both collections + if (!legUser && !regUser) { + throw new Error('User not found in registry or legacy system.') + } + const randomKey = cryptoRandomString({ length: getConstants().CRYPTO_RANDOM_STRING_LENGTH }) const secret = await argon2.hash(randomKey) - legUser.secret = secret - regUser.secret = secret - await legUser.save(options) - await regUser.save(options) + + // Defensively update legacy if present + if (legUser) { + legUser.secret = secret + await legUser.save(options) + } + + // Defensively update registry if present + if (regUser) { + regUser.secret = secret + await regUser.save(options) + } return randomKey } From d4df615972ae2e6543b941800f912ed8d61b1ca1 Mon Sep 17 00:00:00 2001 From: Andrew Foote Date: Fri, 22 May 2026 12:23:55 -0400 Subject: [PATCH 2/2] Fixing linting issues from last commit --- src/repositories/baseUserRepository.js | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/repositories/baseUserRepository.js b/src/repositories/baseUserRepository.js index 4fda674a8..e8e46029d 100644 --- a/src/repositories/baseUserRepository.js +++ b/src/repositories/baseUserRepository.js @@ -412,14 +412,14 @@ class BaseUserRepository extends BaseRepository { * @param {boolean} [isRegistryObject=true] - If false, returns a legacy user object. * @returns {Promise} The updated user object. */ -async updateUser (username, orgShortname, incomingParameters, options = {}, isRegistryObject = true, requestingUserUUID = null) { + async updateUser (username, orgShortname, incomingParameters, options = {}, isRegistryObject = true, requestingUserUUID = null) { const { deepRemoveEmpty } = require('../utils/utils') const baseOrgRepository = new BaseOrgRepository() const legacyUserRepo = new UserRepository() const { createAuditLogEntry } = require('./baseOrgRepositoryHelpers') const registryOrg = await baseOrgRepository.getOrgObject(orgShortname, false, options) const originalRegistryOrg = registryOrg.toObject() - + const legacyUser = await legacyUserRepo.findOneByUserNameAndOrgUUID(username, registryOrg.UUID, null, options) const registryUser = await this.findOneByUsernameAndOrgShortname(username, orgShortname, options, true) @@ -455,7 +455,7 @@ async updateUser (username, orgShortname, incomingParameters, options = {}, isRe const rolesToAdd = _.flattenDeep(_.compact(_.get(incomingParameters, 'active_roles.add'))) const rolesToRemove = _.flattenDeep(_.compact(_.get(incomingParameters, 'active_roles.remove'))) - + if (rolesToRemove.includes('ADMIN') && userUUID) { if (Array.isArray(registryOrg.admins)) { registryOrg.admins.pull(userUUID) @@ -472,7 +472,7 @@ async updateUser (username, orgShortname, incomingParameters, options = {}, isRe // Handle roles calculation fallback const initialRoles = legacyUser?.authority?.active_roles ?? [] const finalRoles = [...new Set([...initialRoles, ...rolesToAdd])].filter(role => !rolesToRemove.includes(role)) - + if (registryUser) { registryUser.role = finalRoles[0] ?? '' } @@ -487,7 +487,7 @@ async updateUser (username, orgShortname, incomingParameters, options = {}, isRe if (registryOrg.admins && registryOrg.admins.includes(userUUID)) { registryOrg.admins.pull(userUUID) } - + const newOrg = await baseOrgRepository.getOrgObject(incomingParameters.org_short_name) const originalNewOrg = newOrg.toObject() if (!Array.isArray(newOrg.users)) { @@ -515,7 +515,7 @@ async updateUser (username, orgShortname, incomingParameters, options = {}, isRe if (registryUser) { delete registryUser.role } - + await registryOrg.save(options) if (requestingUserUUID) { await createAuditLogEntry(registryOrg, originalRegistryOrg, requestingUserUUID, options) @@ -554,20 +554,20 @@ async updateUser (username, orgShortname, incomingParameters, options = {}, isRe * @param {boolean} [isRegistryObject=true] - If false, accepts/returns legacy format. * @returns {Promise} The updated user object. */ -async updateUserFull (identifier, incomingUser, options = {}, isRegistryObject = true, requestingUserUUID = null) { + async updateUserFull (identifier, incomingUser, options = {}, isRegistryObject = true, requestingUserUUID = null) { const legacyUserRepo = new UserRepository() const registryUser = await this.findUserByUUID(identifier, options) const legacyUser = await legacyUserRepo.findOneByUUID(identifier) - + // Fail only if completely missing everywhere if (!registryUser && !legacyUser) { throw new Error('User not found in any repository') } const { ...incomingUserBody } = incomingUser - let legacyObjectRaw = isRegistryObject ? this.convertRegistryToLegacy(incomingUserBody) : incomingUserBody - let registryObjectRaw = isRegistryObject ? incomingUserBody : this.convertLegacyToRegistry(incomingUserBody) + const legacyObjectRaw = isRegistryObject ? this.convertRegistryToLegacy(incomingUserBody) : incomingUserBody + const registryObjectRaw = isRegistryObject ? incomingUserBody : this.convertLegacyToRegistry(incomingUserBody) const protectedFieldsRegistry = ['_id', 'UUID', '__v', 'secret', 'created', 'last_updated'] const protectedFieldsLegacy = ['_id', 'UUID', '__v', 'secret', 'time', 'org_UUID'] @@ -593,7 +593,7 @@ async updateUserFull (identifier, incomingUser, options = {}, isRegistryObject = if (incomingUser.org_short_name) { const baseOrgRepository = new BaseOrgRepository() const { createAuditLogEntry } = require('./baseOrgRepositoryHelpers') - + // Grab current org UUID using whichever document is real const currentOrgUUID = legacyUser ? legacyUser.org_UUID : registryUser?.org_UUID // Fallback if schema supports it const currentOrg = currentOrgUUID ? await baseOrgRepository.findOneByUUID(currentOrgUUID) : null @@ -638,7 +638,6 @@ async updateUserFull (identifier, incomingUser, options = {}, isRegistryObject = // Conditionally save records if (updatedLegacyUser) await updatedLegacyUser.save(options) if (updatedRegistryUser) await updatedRegistryUser.save(options) - } catch (error) { throw new Error('Failed to update user: ' + error.message) } @@ -666,7 +665,7 @@ async updateUserFull (identifier, incomingUser, options = {}, isRegistryObject = * @param {boolean} [isRegistryObject=true] - Unused parameter. * @returns {Promise} The new random secret key. */ -async resetSecret (username, orgShortName, options = {}, isRegistryObject = true) { + async resetSecret (username, orgShortName, options = {}, isRegistryObject = true) { const legacyUserRepo = new UserRepository() const baseOrgRepository = new BaseOrgRepository()