updateOrganization update

This commit is contained in:
xfarrow 2024-02-16 21:43:10 +01:00
parent 91eb4d4b56
commit 9e4e268212
1 changed files with 20 additions and 24 deletions

View File

@ -374,27 +374,24 @@ async function updateOrganization(req, res){
} }
} }
// TODO CHECK CORRECTNESS !! /**
// DELETE * DELETE Request
*
* Deletes the specified organization if the logged user is
* one of its administrator
*/
async function deleteOrganization(req, res){ async function deleteOrganization(req, res){
const organizationIdToDelete = req.params.id; const organizationIdToDelete = req.params.id;
try { try {
// Here we do not actually need a transaction. Two different queries,
// one who checks if the user is admin and one to delete the organization would've const isOrganizationAdmin = await knex('OrganizationAdministrator')
// been sufficient and non-exploitable, but still it'd have been a
// TOC/TOU weakness (https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use).
// Whether a good practice or not is matter of debate.
// There are other points in the code using the same technique to address the same
// problem
knex.transaction(async (trx) => {
// Check if the current user is a organization's administrator
const isOrganizationAdmin = await trx('OrganizationAdministrator')
.where('id_person', req.jwt.person_id) .where('id_person', req.jwt.person_id)
.where('id_organization', req.body.organization_id) .where('id_organization', organizationIdToDelete)
.select('*') .select('*')
.first(); .first();
// Potential TOCTOU weakeness not to be worried about
if(!isOrganizationAdmin){ if(!isOrganizationAdmin){
return res.status(403).json({error : "Forbidden"}); return res.status(403).json({error : "Forbidden"});
} }
@ -404,7 +401,6 @@ async function deleteOrganization(req, res){
.del(); .del();
return res.status(200).json({success: true}); return res.status(200).json({success: true});
});
} }
catch (error) { catch (error) {
console.error(error); console.error(error);