[wip] Use hook for checkAccess

[eileenmcnaughton]

Overview

@colemanw @seamuslee001 @totten - I decided to take a pass at the checkAccess function we discussed. This works in this limited context - with the trickiest part here being convergence on which pseudoconstant / string to use to denote the action. However, for v4 api we have the ability for update & delete with WHERE clauses - so mysql might make more sense - eg. 'WHERE id NOT IN ()';

Note the test class I touched is a pretty good starting place for finding 'already-tested-code-that-is-relevant'

Before

The way to prevent an unpermissioned update is the pre-hook - but this can't be tested before taking action

After

We have a starting point for how a hook might look to do a pre-check

Technical Details

Note this is an example of an existing pre-hook implementation

function financialacls_civicrm_pre($op, $objectName, $id, &$params) {
  if (!financialacls_is_acl_limiting_enabled()) {
    return;
  }
  if ($objectName === 'LineItem' && !empty($params['check_permissions'])) {
    $operationMap = ['delete' => CRM_Core_Action::DELETE, 'edit' => CRM_Core_Action::UPDATE, 'create' => CRM_Core_Action::ADD];
    CRM_Financial_BAO_FinancialType::getAvailableFinancialTypes($types, $operationMap[$op]);
    if (empty($params['financial_type_id'])) {
      $params['financial_type_id'] = CRM_Core_DAO::getFieldValue('CRM_Price_DAO_LineItem', $params['id'], 'financial_type_id');
    }
    if (!in_array($params['financial_type_id'], array_keys($types))) {
      throw new API_Exception('You do not have permission to ' . $op . ' this line item');
    }
  }

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[seamuslee001]

@eileenmcnaughton I'm guessing we may want to call core methods that already exist before we get to the hook invocation and would we want extensions to potentially override core functions e.g. result of CRM_Mailing_BAO_Mailing::acl or whatever is that function?

[eileenmcnaughton]

@seamuslee001 yeah - this is the function on the DAO class which would be overridden by various BAO classes

[seamuslee001]

@eileenmcnaughton I guess in my mind at least wouldn't that mean we end up with a lot of duplicated code around the place because you would want to copy n paste a fair chunk of this function on the other DAOs?

[eileenmcnaughton]

test this please

[eileenmcnaughton]

@colemanw closing as a discussion point rather than a PR for review

[colemanw]

@eileenmcnaughton I opened #20170 with a modified version of this function. Notable differences:

• Called statically on BAO classes
• Automatically dispatches to protected method BAO::_checkAccess()` if available.
• Only acts on one record at a time (this allows the hook to escalate as well as revoke permissions)