Prior to this PR, we've had a single set of policies for each resource. Guests can view users but only those with member_edit or member_edit_any permission can modify them, etc. The /manage/ pages have been locked down with an additional check laid out in User#active_admin_editor? that basically checks whether the user has permission to create any resource.
This has worked fine until now, but once we tell enlistment liaisons to start using the /manage/enlistments page, most of them won't have access to it, because they won't have access to create an enlistment (or any other resource) . Likewise, if we ever allowed all members to create their own Extended LOA (which would be reasonable to do), they'd also get access to /manage/ which we don't necessarily want.
So this PR creates a separate set of policies under the Manage:: namespace. These policies are specifically for access to the /manage/ pages. So you can only view /manage/enlistments if you have access to edit them, etc. In fact most policies are defined that way (view access requires edit access). So now, User#active_admin_editor? checks if the user has view access to any of the resources under /manage/ before determining whether they can access that directory.
But the tricky two were /manage/users and /manage/units. While it's clear who should be able to edit those resources, who should be able to view them? The answer is: basically anyone who has access to edit any of the other/manage/ resources.
This made it a little tricky, and means we still rely on having logic in User#active_admin_editor?. The solution was to have the Manage::UserPolicy and Manage::UnitPolicy say that any member can view a user or a unit. But then that would mean that any member can access /manage/. So User#active_admin_editor?'s logic explicitly omits User and Unit from its checks. The logic is thus: "does the user have view access to any resource in /manage/ other than User or Unit?"
I don't love this solution, and something tells me we could have stuck with one set of policy classes and just had wonkier exception logic in User#active_admin_editor?, but it's probably more future-proof to have two sets of policy classes anyway. I'll sleep on it and see if it still feels like the least-worst answer tomorrow.
Prior to this PR, we've had a single set of policies for each resource. Guests can view users but only those with
member_editormember_edit_anypermission can modify them, etc. The/manage/pages have been locked down with an additional check laid out inUser#active_admin_editor?that basically checks whether the user has permission to create any resource.This has worked fine until now, but once we tell enlistment liaisons to start using the
/manage/enlistmentspage, most of them won't have access to it, because they won't have access to create an enlistment (or any other resource) . Likewise, if we ever allowed all members to create their own Extended LOA (which would be reasonable to do), they'd also get access to/manage/which we don't necessarily want.So this PR creates a separate set of policies under the
Manage::namespace. These policies are specifically for access to the/manage/pages. So you can only view/manage/enlistmentsif you have access to edit them, etc. In fact most policies are defined that way (view access requires edit access). So now,User#active_admin_editor?checks if the user has view access to any of the resources under/manage/before determining whether they can access that directory.But the tricky two were
/manage/usersand/manage/units. While it's clear who should be able to edit those resources, who should be able to view them? The answer is: basically anyone who has access to edit any of the other/manage/resources.This made it a little tricky, and means we still rely on having logic in
User#active_admin_editor?. The solution was to have theManage::UserPolicyandManage::UnitPolicysay that any member can view a user or a unit. But then that would mean that any member can access/manage/. SoUser#active_admin_editor?'s logic explicitly omitsUserandUnitfrom its checks. The logic is thus: "does the user have view access to any resource in/manage/other thanUserorUnit?"I don't love this solution, and something tells me we could have stuck with one set of policy classes and just had wonkier exception logic in
User#active_admin_editor?, but it's probably more future-proof to have two sets of policy classes anyway. I'll sleep on it and see if it still feels like the least-worst answer tomorrow.Fixes #222
Remaining todo: