compucorp / uk.co.compucorp.membershipextras

Membership Extras for CiviCRM
Other
5 stars 8 forks source link

MAE-393: Allow non administers to manage payment plans #430

Closed omarabuhussein closed 2 years ago

omarabuhussein commented 2 years ago

Overview

Users without 'administer CiviCRM' permissions are unable to create payment plans (they cannot select payment plan schedule), and unable to manage payment plans installments using "View/Modify Future Instalments" action. This purpose of this PR is to provide a way for users to do these actions with permissions that give less control over the entire system.

Before

1- If a user with permission to create memberships and contributions ('edit memberships' and 'edit contributions') which are needed to access the membership creation form, they still won't be able to submit a payment plan membership, due to a missing "schedule" field, and the form will complain about insufficient permission.

2022-05-28 09_57_09-Settings

2- If the same user tried to use the "View/Modify Future Instalments" action on an existing payment plan, they they won't be able to add, delete or modify the line items, in both "Current Period" and "Next Period" tabs, here is an example of the error the users gets when trying to add new membership in the "Next period" tab:

2022-05-28 10_00_45-dsaadsa dsadasasd _ compuclient129 — Mozilla Firefox

After

In this PR I've fixed the permissions need to do the above operations, so that users with both "edit contributions" and "edit memberships" can do the operations mentioned in the before section, thus they no longer need to have "administer CiviCRM" permission do them.

I also now added permissions checks, to make sure that "View/Modify Future Instalments" action on the payment plan, only appears to users with both "edit contributions" and "edit memberships" permissions.

Technical Details

To understand the reason behind this issue, I should give a bit of explanation of how API permissions work in Civi. There are two kind of APIs in CiviCRM:

1- Core APIs with permissions to access them defined in CiviCRM core, specifically inside CRM_Core_Permission class , if a core entity does not have permissions defined in this file, then CiviCRM will default their permissions to administer CiviCRM.

2- Extension APIs, extension developers can define the permissions for such APIs using hook_civicrm_alterAPIPermissions hook, if you don't do that then Civi will by default assumes their permission administer CiviCRM same as above.

In both cases above, non of these permissions on the API will be checked unless a flag (or a parameter) called check_permissions is set to True, but whether you can set this flag in your API call or not, and what is its default value will be, depends on how you call the API, because Civi offers various ways in which you can query the API, which are:

$result = civicrm_api3('Case', 'get', [
]);

but the logged in user does not have access to view "Cases", then they will still get results, because Civi assumes that the extension developer knows that and fine with it. But if in the other hand, the call above was replaced with this:

$result = civicrm_api3('Case', 'get', [
  'check_permissions' => True,
]);

Then permissions check will be enforced, and the users without "Cases" permissions won't be able to view the result, in this example something like tis will appear:

API permission check failed for Case/get call; insufficient permission: require ( access my cases and activities or access all cases and activities or basic case information )

If in the other hand, the extension developer query an API that has no permissions defined in Civi Core, such as PriceFieldValue API (which is a core entity but without permissions defined in Civi Core), and the extension developer forces permission check with check_permissions flag, then similar error to the one above will appear but instead complaining that the user does not have 'administer CiviCRM' permission.

So what is happening here ?

There are two parts to this:

A- For the payment plan creation page, CiviCRM calls an API defined in Membershipextras called PaymentSchedule to determine the allowed payment plan "schedules" allowed for the selected membership, and because we have no permissions defined for them, and because they are Ajax requests, CiviCRM defaults this API endpoint permission to 'administer CiviCRM' as we explained above.

B- For the manage installment screen, we do a lot of Ajax API calls, some of them are to API endpoints with permissions defined in CiviCRM core, such as "LineItem" and "Contribution", while others are either: 1) Core CiviCRM APIs but without permissions defined on them such as 'PriceFieldValue', 2) APIs defined in Membershipextras, and we have only one in this case called 'ContributionRecurLineItem', thus CiviCRM will enforce 'adminsiter CiviCMR' permission on such calls.

Because of that, these screens will be unusable by users without 'administer CiviCRM' permission. Though naturally any user with both 'edit contributions' and 'edit memberships' should be able to use them, and thus in this PR, I implemented membershipextras_civicrm_alterAPIPermissions hook in which I enforced the following:

On top of that, I've updated the permissions on all the routes related to the Manage installments form, in which now it requires both 'edit contributions' and 'edit memberships' to access them, instead of 'edit contributions' alone, because the actions on this screen will result in modifications to the memberships, so both of these permissions should be required to do that.

olayiwola-compucorp commented 2 years ago

wow, thanks for the detailed explanation of what is happening @omarabuhussein.