From 52014970a1901577c96e75e6613155b210670ba0 Mon Sep 17 00:00:00 2001 From: david-rocca Date: Fri, 11 Jul 2025 14:08:56 -0400 Subject: [PATCH 1/2] Actually close my sessions correctly --- .../org.controller/org.controller.js | 65 ++++++++++--------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js index 317cf9af7..5727397f3 100644 --- a/src/controller/org.controller/org.controller.js +++ b/src/controller/org.controller/org.controller.js @@ -363,6 +363,7 @@ async function createOrg (req, res, next) { if (legResult && regResult) { logger.info({ uuid: req.ctx.uuid, message: legResult.short_name + ' organization was not created because it already exists.' }) + await session.abortTransaction() return res.status(400).json(error.orgExists(legOrg.short_name)) } @@ -432,7 +433,7 @@ async function createOrg (req, res, next) { await session.abortTransaction() throw error } finally { - session.endSession() + await session.endSession() } logger.info(JSON.stringify(payload)) @@ -470,7 +471,6 @@ async function updateOrg (req, res, next) { if (!orgToUpdate) { logger.info({ uuid: req.ctx.uuid, message: `Organization ${shortNameParam} not found.` }) await session.abortTransaction() - session.endSession() return res.status(404).json(error.orgDnePathParam(shortNameParam)) } @@ -480,7 +480,6 @@ async function updateOrg (req, res, next) { // This indicates an inconsistent state, as an Org should have a corresponding RegistryOrg if created by the system logger.error({ uuid: req.ctx.uuid, message: `Registry org counterpart for ${orgToUpdate.short_name} (UUID: ${orgToUpdate.UUID}) not found. Data inconsistency.` }) await session.abortTransaction() - session.endSession() return res.status(500).json(error.serverError('Inconsistent organization data: Registry counterpart missing.')) } @@ -613,12 +612,12 @@ async function updateOrg (req, res, next) { if (newOrgUpdates.short_name && newOrgUpdates.short_name !== orgToUpdate.short_name) { const existingLegOrg = await orgRepo.findOneByShortName(newOrgUpdates.short_name, { session }) if (existingLegOrg && existingLegOrg.UUID !== orgToUpdate.UUID) { - await session.abortTransaction(); session.endSession() + await session.abortTransaction() return res.status(403).json(error.duplicateShortname(newOrgUpdates.short_name)) } const existingRegOrg = await regOrgRepo.findOneByShortName(newRegOrgUpdates.short_name, { session }) if (existingRegOrg && existingRegOrg.UUID !== regOrgToUpdate.UUID) { - await session.abortTransaction(); session.endSession() + await session.abortTransaction() return res.status(403).json(error.duplicateShortname(newRegOrgUpdates.short_name)) } } @@ -672,7 +671,7 @@ async function updateOrg (req, res, next) { } next(err) } finally { - session.endSession() + await session.endSession() } } @@ -712,12 +711,12 @@ async function createUser (req, res, next) { const regUsers = await userRegistryRepo.findUsersByOrgUUID(orgUUID, { session }) if (users && regUsers && users !== regUsers) { - await session.abortTransaction(); session.endSession() + await session.abortTransaction() return res.status(500).json({ message: 'Data inconsistency' }) } if (users >= 100) { - await session.abortTransaction(); session.endSession() + await session.abortTransaction() return res.status(400).json(error.userLimitReached()) } @@ -734,12 +733,12 @@ async function createUser (req, res, next) { const key = keyRaw.toLowerCase() if (key === 'uuid') { - await session.abortTransaction(); session.endSession() + await session.abortTransaction() return res.status(400).json(error.uuidProvided('user')) } if (key === 'org_uuid') { - await session.abortTransaction(); session.endSession() + await session.abortTransaction() return res.status(400).json(error.uuidProvided('org')) } @@ -793,7 +792,7 @@ async function createUser (req, res, next) { // check if user is only an Admin (not Secretatiat) and the user does not belong to the same organization as the new user if (!isSecretariat && isAdmin) { if (requesterOrgUUID !== orgUUID) { - await session.abortTransaction(); session.endSession() + await session.abortTransaction() return res.status(403).json(error.notOrgAdminOrSecretariat()) // The Admin user must belong to the new user's organization } } @@ -812,7 +811,7 @@ async function createUser (req, res, next) { const resultReg = await userRegistryRepo.findOneByUserNameAndOrgUUID(newRegistryUser.user_id, orgUUID, null, { session }) if (resultLeg || resultReg) { logger.info({ uuid: req.ctx.uuid, message: newUser.username + ' was not created because it already exists.' }) - await session.abortTransaction(); session.endSession() + await session.abortTransaction() return res.status(400).json(error.userExists(newUser.username)) } @@ -849,6 +848,8 @@ async function createUser (req, res, next) { return res.status(200).json(responseMessage) } catch (err) { next(err) + } finally { + await session.endSession() } } @@ -889,12 +890,12 @@ async function updateUser (req, res, next) { if (!targetOrgLegUUID || !targetOrgRegUUID) { logger.error({ uuid: req.ctx.uuid, message: `Target organization ${shortNameParams} not found in one or both collections.` }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(404).json(error.orgDnePathParam(shortNameParams)) } if (targetOrgLegUUID !== targetOrgRegUUID) { logger.error({ uuid: req.ctx.uuid, message: 'Registry and Legacy Org UUIDs do not match for target org. Data inconsistency.' }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(500).json(error.serverError('Inconsistent organization data.')) } @@ -905,18 +906,18 @@ async function updateUser (req, res, next) { if (targetUserUUID !== requesterUUID) { if (!targetUserUUID) { logger.info({ uuid: req.ctx.uuid, message: 'User DNE' }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(404).json(error.userDne(usernameParams)) } logger.info({ uuid: req.ctx.uuid, message: 'Not same user or secretariat' }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(403).json(error.notSameUserOrSecretariat()) } } if (shortNameParams !== requesterShortName && !isRequesterSecretariat) { logger.info({ uuid: req.ctx.uuid, message: `${shortNameParams} organization data can only be modified by users of the same organization or the Secretariat.` }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(403).json(error.notSameOrgOrSecretariat()) } @@ -925,7 +926,7 @@ async function updateUser (req, res, next) { if (!userLeg && !userReg) { // If user doesn't exist in EITHER system. logger.info({ uuid: req.ctx.uuid, message: `User ${usernameParams} does not exist for ${shortNameParams} organization.` }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(404).json(error.userDne(usernameParams)) } @@ -943,7 +944,7 @@ async function updateUser (req, res, next) { // Specific check for org_short_name (Secretariat only) if (queryParameters.org_short_name && !isRequesterSecretariat) { logger.info({ uuid: req.ctx.uuid, message: 'Only Secretariat can reassign user organization.' }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(403).json(error.notAllowedToChangeOrganization()) } @@ -951,7 +952,7 @@ async function updateUser (req, res, next) { if ((queryParameters.new_username || queryParameters['active_roles.remove'] || queryParameters['active_roles.add'])) { if (!isRequesterSecretariat && !isAdmin) { logger.info({ uuid: req.ctx.uuid, message: `User ${requesterUsername} (not Admin/Secretariat) trying to modify admin-only fields.` }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(403).json(error.notOrgAdminOrSecretariatUpdate()) } } @@ -961,7 +962,7 @@ async function updateUser (req, res, next) { const unameToCheck = await userLegRepo.findOneByUserNameAndOrgUUID(queryParameters.new_username, targetOrgRegUUID, null, { session }) if (unameToCheck) { logger.info({ uuid: req.ctx.uuid, message: queryParameters.new_username + ' was not created because it already exists.' }) - await session.abortTransaction(); session.endSession() + await session.abortTransaction() return res.status(403).json(error.duplicateUsername(queryParameters.new_username, shortNameParams)) } } @@ -1024,7 +1025,7 @@ async function updateUser (req, res, next) { handlers[key]() } catch (handlerError) { logger.info({ uuid: req.ctx.uuid, message: handlerError.message || `Auth error in handler for ${key}` }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(403).json(handlerError instanceof Error ? { name: handlerError.name, error: handlerError.message } : handlerError) } } @@ -1032,7 +1033,7 @@ async function updateUser (req, res, next) { if (queryParameters.active) { if (requesterUUID === targetUserUUID) { - await session.abortTransaction; await session.endSession() + await session.abortTransaction() return res.status(403).json(error.notOrgAdminOrSecretariatUpdate()) } } @@ -1040,7 +1041,7 @@ async function updateUser (req, res, next) { // Check to make sure we are NOT self demoting if (removeRolesCollector.includes('ADMIN')) { if (requesterUUID === targetUserUUID) { - await session.abortTransaction; await session.endSession() + await session.abortTransaction() return res.status(403).json(error.notAllowedToSelfDemote()) } } @@ -1166,7 +1167,7 @@ async function updateUser (req, res, next) { const legUpdateResult = await userLegRepo.updateByUUID(userLeg.UUID, legacyUserUpdatePayload, { session }) if (!legUpdateResult || legUpdateResult.modifiedCount === 0) { if (legUpdateResult && legUpdateResult.matchedCount === 0) { - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(404).json(error.userDne(userLeg.username)) } } else { @@ -1178,7 +1179,7 @@ async function updateUser (req, res, next) { const regUpdateResult = await userRegRepo.updateByUUID(userReg.UUID, registryUserUpdatePayload, { session }) if (!regUpdateResult || regUpdateResult.modifiedCount === 0) { if (regUpdateResult && regUpdateResult.matchedCount === 0) { - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(404).json(error.userDne(userReg.user_id)) } } else { @@ -1286,7 +1287,7 @@ async function resetSecret (req, res, next) { if (!targetOrgUUID) { logger.info({ uuid: req.ctx.uuid, message: 'User DNE' }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(404).json(error.orgDnePathParam(orgShortName)) } @@ -1295,16 +1296,19 @@ async function resetSecret (req, res, next) { // check if orgUUID and orgRegUUID are the same if (orgUUID.toString() !== orgRegUUID.toString()) { logger.info({ uuid: req.ctx.uuid, message: 'The organization UUID and the organization registry UUID are not the same.' }) + await session.abortTransaction() return res.status(500).json(error.internalServerError()) } if (!orgUUID && !orgRegUUID) { logger.info({ uuid: req.ctx.uuid, message: orgShortName + ' organization does not exist.' }) + await session.abortTransaction() return res.status(404).json(error.orgDnePathParam(orgShortName)) } if (orgShortName !== requesterShortName && !isSecretariat) { logger.info({ uuid: req.ctx.uuid, message: orgShortName + ' organization can only be viewed by the users of the same organization or the Secretariat.' }) + await session.abortTransaction() return res.status(403).json(error.notSameOrgOrSecretariat()) } @@ -1313,6 +1317,7 @@ async function resetSecret (req, res, next) { if (!oldUser && !oldUserRegistry) { logger.info({ uuid: req.ctx.uuid, message: username + ' user does not exist.' }) + await session.abortTransaction() return res.status(404).json(error.userDne(username)) } @@ -1324,7 +1329,7 @@ async function resetSecret (req, res, next) { if (targetUserUUID !== requesterUUID) { if (!targetUserUUID) { logger.info({ uuid: req.ctx.uuid, message: 'User DNE' }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(404).json(error.userDne(username)) } } @@ -1335,6 +1340,7 @@ async function resetSecret (req, res, next) { // check if the requester is not and admin; if admin, the requester must be from the same org as the user if (!isAdmin || (isAdmin && orgShortName !== requesterShortName)) { logger.info({ uuid: req.ctx.uuid, message: 'The api secret can only be reset by the Secretariat, an Org admin or if the requester is the user.' }) + await session.abortTransaction() return res.status(403).json(error.notSameUserOrSecretariat()) } } @@ -1348,6 +1354,7 @@ async function resetSecret (req, res, next) { if (user.matchedCount === 0 || userReg.matchedCount === 0) { logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + username + ' does not exist for ' + orgShortName + ' organization.' }) + await session.abortTransaction() return res.status(404).json(error.userDne(username)) } await session.commitTransaction() @@ -1355,7 +1362,7 @@ async function resetSecret (req, res, next) { await session.abortTransaction() throw error } finally { - session.endSession() + await session.endSession() } logger.info({ uuid: req.ctx.uuid, message: `The API secret was successfully reset and sent to ${username}` }) From 36dfe21cb6fd11a6433ccb34db6bed83d8192b08 Mon Sep 17 00:00:00 2001 From: david-rocca Date: Fri, 11 Jul 2025 14:11:35 -0400 Subject: [PATCH 2/2] missed a few --- src/controller/org.controller/org.controller.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js index 5727397f3..b5b95bb66 100644 --- a/src/controller/org.controller/org.controller.js +++ b/src/controller/org.controller/org.controller.js @@ -1055,7 +1055,7 @@ async function updateUser (req, res, next) { if (newOrgShortNameToMoveTo) { if (newOrgShortNameToMoveTo === shortNameParams) { logger.info({ uuid: req.ctx.uuid, message: `User ${usernameParams} is already in organization ${newOrgShortNameToMoveTo}.` }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(403).json(error.alreadyInOrg(newOrgShortNameToMoveTo, usernameParams)) } newTargetLegacyOrgUUID = await orgLegRepo.getOrgUUID(newOrgShortNameToMoveTo, { session }) @@ -1063,12 +1063,12 @@ async function updateUser (req, res, next) { if (!newTargetLegacyOrgUUID || !newTargetRegistryOrgUUID) { logger.info({ uuid: req.ctx.uuid, message: `New target organization ${newOrgShortNameToMoveTo} does not exist.` }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(404).json(error.orgDne(newOrgShortNameToMoveTo, 'org_short_name', 'query')) } if (newTargetLegacyOrgUUID !== newTargetRegistryOrgUUID) { logger.error({ uuid: req.ctx.uuid, message: `New target organization ${newOrgShortNameToMoveTo} has mismatched legacy/registry UUIDs.` }) - await session.abortTransaction(); await session.endSession() + await session.abortTransaction() return res.status(500).json(error.serverError('Inconsistent new target organization data.')) }