Skip to content

Resolves issue #1408 and #1407 - Fixes Session Closures #1437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 14, 2025
Merged

Resolves issue #1408 and #1407 - Fixes Session Closures #1437

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5201497
Actually close my sessions correctly
david-rocca Jul 11, 2025
36dfe21
missed a few
david-rocca Jul 11, 2025
82e4789
Merge branch 'dev' into dr-1408-endsession-fixes
david-rocca Jul 14, 2025
b9c6634
Merge branch 'dev' into dr-1408-endsession-fixes
david-rocca Jul 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 39 additions & 32 deletions src/controller/org.controller/org.controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
}

Expand All @@ -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.'))
}

Expand Down Expand Up @@ -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))
}
}
Expand Down Expand Up @@ -672,7 +671,7 @@ async function updateOrg (req, res, next) {
}
next(err)
} finally {
session.endSession()
await session.endSession()
}
}

Expand Down Expand Up @@ -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())
}

Expand All @@ -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'))
}

Expand Down Expand Up @@ -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
}
}
Expand All @@ -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))
}

Expand Down Expand Up @@ -849,6 +848,8 @@ async function createUser (req, res, next) {
return res.status(200).json(responseMessage)
} catch (err) {
next(err)
} finally {
await session.endSession()
}
}

Expand Down Expand Up @@ -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.'))
}

Expand All @@ -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())
}

Expand All @@ -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))
}

Expand All @@ -943,15 +944,15 @@ 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())
}

// General permission check for fields requiring admin/secretariat
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())
}
}
Expand All @@ -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))
}
}
Expand Down Expand Up @@ -1024,23 +1025,23 @@ 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)
}
}
}

if (queryParameters.active) {
if (requesterUUID === targetUserUUID) {
await session.abortTransaction; await session.endSession()
await session.abortTransaction()
return res.status(403).json(error.notOrgAdminOrSecretariatUpdate())
}
}

// 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())
}
}
Expand All @@ -1054,20 +1055,20 @@ 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 })
newTargetRegistryOrgUUID = await orgRegRepo.getOrgUUID(newOrgShortNameToMoveTo, { session })

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.'))
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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))
}

Expand All @@ -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())
}

Expand All @@ -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))
}

Expand All @@ -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))
}
}
Expand All @@ -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())
}
}
Expand All @@ -1348,14 +1354,15 @@ 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()
} catch (error) {
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}` })
Expand Down
Loading