Merge pull request #1824 from CVEProject/af-1730

Resolves #1730, Fix database inconsistency with account cleanup

Author: david-rocca

src/repositories/baseUserRepository.js

@@ -419,96 +419,124 @@ class BaseUserRepository extends BaseRepository {
     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)
+
+    if (!registryUser && !legacyUser) {
+      throw new Error('User not found')
+    }
 
-    registryUser.username = incomingParameters?.new_username ?? registryUser.username
-    legacyUser.username = incomingParameters?.new_username ?? legacyUser.username
+    // 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 (incomingParameters?.org_short_name) {
-      // Remove us from the old users Array
+    if (registryUser) {
+      registryUser.role = finalRoles[0] ?? ''
+    }
+    if (legacyUser) {
+      _.set(legacyUser, 'authority.active_roles', finalRoles)
+    }
+
+    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
@@ -529,117 +557,101 @@ class BaseUserRepository extends BaseRepository {
   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)
-    }
+    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']
 
-    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)
-        }
-
-        // 2. Remove user from old org's admins list (if present)
-        if (currentOrg.admins && currentOrg.admins.includes(identifier)) {
-          currentOrg.admins.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)
         }
 
-        // 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 = []
           }
           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
   }
 
@@ -661,12 +673,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
   }