Permit ACL rules that negate (deny) access

[seamuslee001]

Overview

Permit Administrative users to create rules to deny access to custom groups / groups of contacts

See https://lab.civicrm.org/dev/core/-/issues/3753

Before

Only permit deny actions via hooks

After

Can use UI to create ACL rules to deny access to events, contacts, custom groups as needed

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

@seamuslee001 this is looking good. Please add a unit test.

[TobiasVoigt]

Hi there, it's so awesome to see that there's progress regarding this topic! Thank you very much for all of your efforts so far!

Today I went over to make some tests on the test site but soon ran into some problems:

I set up the use case I initially described by...

• creating a (priviliged) set of custom fields (called "With permission only")
• creating a new role in drupal ("testrole1")
• creating a new CiviCRM group ("Test Group 1")
• creating a test user for that role ("Test1 Test1") that is a member of the group
• turning off access to all custom fields in drupal access control
• creating a CiviCRM ACL Role ("Test Role 1") and assigning it to the group ("Test Group 1")
• creating three ACLs that should allow access to my new set of custom fields ONLY for the new ACL role

1. Allow access to all custom fields for all authenticated users
2. Deny access for the privileged set custom fields for all authenticated users
3. Allow access to the privileged set of custom fields for the privileged role

Yet this setup / combination of rules doesn't work. My test user can't see the privileged set of custom fields as expected.

What am I doing wrong? I realize that without some sort of priorisation or "weighting" of the ACLs, rule 2 may trump rule 3. Yet this sort of weighting would be important to be able to reduce the number of rules that have to be created.

The alternative would be to define rules for all sets of custom fields individually. But that would almost be as tedious a task as creating individual rules without the ability to negate / deny access.

So, is there any sort of weighting in place? If yes, how can I influence this weighting? Maybe I'm missing something important here - or maybe in my initial post I didn't make clear how important the "weighting" functionality of ACLs would be?

I'd be happy to go on testing anytime.

[seamuslee001]

@TobiasVoigt sorry for the delay in getting back to you, I think the problem is that the Deny rule (2) is overriding the Allow rule (3) hmm this is bit tricky

[TobiasVoigt]

@seamuslee001 Thanks for your feedback! I just noticed that the test site doesn't exist anymore. Could you set up a new test site for me to go on testing?

[TobiasVoigt]

@seamuslee001 Could you please set up a new test site? I'd like to keep testing to be able to give you more detailed feedback on this. Thanks!

[seamuslee001]

Jenkins test this please

[colemanw]

@TobiasVoigt that comment from Seamus should have triggered a demo site rebuild.

[TobiasVoigt]

Jenkins test this please

[TobiasVoigt]

@seamuslee001 / @colemanw - It seems that I was too late? The test site seems to be deleted already. Using the "Jenkins" comment didn't work - guess I don't have the necessary rights? Please be so kind to setup a new testsite - and if possible ensure that it isn't deleted for a few weeks at least? Thanks an best regards!

[colemanw]

test this please

[TobiasVoigt]

@colemanw / @seamuslee001

Thanks! I was able to test the new functions again and am pleased to let you know that we managed to realize our original use case - although in a slightly different way than I initially imagined...

When writing the original issue I thought that I could manage everything with ACLs by combining more than two rules. To give a priviliged role access to one restricted set of custom fields, I thought I could combine ACL rules in this way:

• Disable the right to "access all custom data" on CMS level
• ACL rule 1: Allow access to all custom field groups for all authenticated users
• ACL rule 2: Disallow access to the privileged set of custom fields for alle authenticated users
• ACL rule 3: Allow access to the privileged set of custom fields for a privileged role

I now understand that this isn't possible since in the current version negative rules ALWAYS override positive rules. In my case rule 2 will always override rule 3.

The only way to make this work with ACLs only would be by adding some sort of "weighting" option to each rule to manually define which rules have more weight than others. But I guess this additional feature would probably be a lot of extra work?

However: I then realized that I can still make it work for my original use case by creating an additional CMS role and grant the right to "access all custom data" to this additional role only. Thereby I don't need rule 3. I would have one privileged CMS role that has access to all custom data. But most important of all (that was the intention behind my initial post): I wouldn't have to write new ACL rules whenever I add a new "normal" user or a new "normal" set of custom fields.

So - thanks again to everybody involved. I think this will be very useful for many people!!

[colemanw]

Thanks for the review @TobiasVoigt that's very helpful. We'll get this finished up and merged shortly.

[seamuslee001]

@colemanw this should now have a unit test, I think if we wanted to do weighted rules that should be a follow up PR and I have updated the form / page as per your feedback

[colemanw]

Looks good @seamuslee001 I made one tweak to fix capitalization and now I agree this is ready to merge.

[colemanw]

@TobiasVoigt thanks for your feedback. This is now merged and we have followed up with a PR which allows you to prioritize rules over others; #26592