[Debt][Security] Refactor updateRoles user policy

[yonikid15]

🤖 Resolves #11204

👋 Introduction

• Refactors UserPolicy->updateRoles to have an "else false" style ending. This will ensure that the security logic has a safe fall through for unrecognized states.

🧪 Testing

1. Build app
2. Login as different user roles and ensure updating roles works correctly. It can get confusing remembering which roles can update others. I'd suggest viewing the UserPolicyTest.php and rolePermissions.php

For example: A community recruiter can only update roles of pools in their community.

[yonikid15]

Hmm, this logic doesn't look quite right to me. The original looped through each role and completed only if all were true. The new one loops through each role and completes early if any of them are true.

I'd suggest using something like every to evaluate each array then end with something like

if ($attachRolesAreValid && $detachRolesAreValid) {
return true;
}
//fallthrough
return false;

Wow, good catch @petertgiles :+1: Hopefully I got it right this time 8ca8d05

[yonikid15]

Sorry, I feel like I'm being really picky. But this is a security refactor...

No sorry needed, this is important and we should get it right :+1: