3sd / civicrm-recalculate-recipients

Moved to https://lab.civicrm.org/extensions/civicrm-recalculate-recipients
Other
6 stars 5 forks source link

Take CiviMail author's ACLs into account when recalculating recipients #1

Closed michaelmcandrew closed 2 months ago

seamuslee001 commented 5 years ago

Just to be a bit more descriptive on this one

The problem is the following situation:

User A is an acled user and creates a mailing and only has access to contacts 1, 2,3 However contacts 4,5 would also be included in the list but are prevented based on the ACL

User B is the system user that does the actual processing of the email and in the re-calculation process it might include contacts 4 and 5 as well when sending the email when they shouldn't have been

michaelmcandrew commented 5 years ago

@seamuslee001 - thanks for the description - wondering if you have any ideas of how easily solvable this might be?

seamuslee001 commented 5 years ago

we do have scheduled_id column on civicrm_mailing so that would tell us who to become. the tricky issue would be to somehow get the ACL system to behave as if it was being used by that contact I haven't explored this but yeh plus we have to make sure any acl hooks were correctly fired as well

michaelmcandrew commented 5 years ago

yeah, sounds tricky :thinking:

michaelmcandrew commented 5 years ago

looks like we can use https://github.com/civicrm/civicrm-core/pull/15151 to help with this.

colemanw commented 5 years ago

So once https://github.com/civicrm/org.civicrm.api4/pull/191 is merged you can simply do

civicrm_api4('Foo', 'bar', ['actingUser' => $cid, ...
jmcclelland commented 2 years ago

I'm having a hard time figuring out the status of this issue - it looks like the required PR against core was not merged, and instead an alternate way to solve the immediate problem was resolved via authx.

So, I think we are back to square one without a method to get this to run with ACL permissions. Is that correct? Or are there any other bright ideas I might be missing?

michaelmcandrew commented 2 years ago

Hey there,

Would like to help but not sure of the details here (haven't used this extension since I wrote it).

FWIW it does feel to me like an issue that could be better tackled in core.

Michael

totten commented 2 years ago

I don't have a good answer, but a couple related notes.

You may find a path to implementation along the lines of this pseudocode:

function createGuzzle() {
  $handler = HandlerStack::create();
  $handler->unshift(\CRM_Utils_GuzzleMiddleware::authx(), 'civi_authx');
  $handler->unshift(\CRM_Utils_GuzzleMiddleware::url(), 'civi_url');
  return new \GuzzleHttp\Client(['handler' => $handler]);
}

function onBeforeStartMailing() {
  $client = createGuzzle();
  $client->post('route://civicrm/ajax/api4/Mailing/refreshRecipients', [
    'authx_contact_id' => $theMailingSubmitterContactId,
    'form_params' => ['where' => [['jobId', '=', $theMailingJobId]]]
  ]);
}

class Civi\Api4\Actions\Mailing\RefreshRecipients extends BasicBatchAction {
  public function _execute() { ... }
}

The use of GuzzleMiddleware::url() lets you request routes in prettier way. The use of GuzzleMiddleware::authx() allows you to send a "run-as" HTTP request (authx_contact_id => 123). (See also: https://github.com/civicrm/civicrm-core/pull/21261)

Relatedly, we're starting work on general improvements to queuing (originally https://lab.civicrm.org/dev/core/-/issues/1304). One of the ambitions (currently in exploration) is to allow queuing tasks which run-as a user, eg you could imagine that Mailing.submit adds a task like this:

$task = new CRM_Queue_Task();
$task->run_as = ['contactId' => 123, 'domainId' => 456];
$task->release_time = $mailingJob->scheduled_date;
$task->callback = 'CRM_Mailing_MyTask::sendMessages';
$task->arguments = [$mailingJob->id];

However, that's probably a longer road to solving this particular problem.

MegaphoneJon commented 2 years ago

With respect to solving this, I'd request making it optional. I'm currently investigating a use case to create a role for mailing creators that allows them to send but not see contact info. Recalculating at send time will be necessary, and this current bug/feature will be a help.

jmcclelland commented 2 years ago

I just took a stab at a very limited slice of this issue: with the PR #19, if ACLs are configured, this extension simply won't run.

I don't think it's much of a solution for anyone except our use case (we make extensions available to all our groups and they have the ability to turn them on - so we have to be sure nobody turns on this extension while using ACLs).

But in case it is useful for anyone... it's available.

michaelmcandrew commented 2 years ago

Thanks @jmcclelland - the readme says it is not acl friendly so puts the risk in the hands of the user. This one is safer. Not sure whether to merge or not.

It isn't in the extension directory so I am not sure what the usage is.

Happy to share maintainership if it is something you are making use of (we are still not using it ourselves!)

Michael

michaelmcandrew commented 2 months ago

Closing this issue as I am archiving this repo - please open an issue on https://lab.civicrm.org/extensions/civicrm-recalculate-recipients and reference this issue if you'd like to continue the discussion.