Rework plugin to use auth_aclcheck()

[fschrempf]

This replaces the code for checking the ACL rules by calls to the existing auth_aclcheck() function. This avoids the duplication of ACL evaluation logic and allows to use this plugin with ACL extension plugins like aclregex or aclplusregex.

The new logic goes through all the users and groups that are contained in the ACL rules and checks the permissions for these subjects. If the permissions for the subject exceed those of the ALL group, they are added to the list.

[fschrempf]

@splitbrain @annda Could you spare a moment to look at this PR? Thanks!

[fschrempf]

I added another commit to this, that moves most of the code to helper functions. This allows integration in other plugins or templates. See cosmocode/dokuwiki-template-sprintdoc#75 for a use-case (integration in sprintdoc template).

[fschrempf]

@splitbrain @annda Any chance to get this reviewed and merged? I've been successfully using this on a production instance for a couple of weeks now. But my PHP skills are not great, so I can't really tell if there might be performance issues in cases where there are a lot of ACL rules.

[splitbrain]

Hmm let me see if I understand the code correctly. You loop over all ACL entries and extract the user or group from it. Then you do a call to auth_aclcheck with the current page and the found user and group? The latter will do a loop over the ACLs again.

Yeah I fear this will have really bad performance on large ACL setups.

[fschrempf]

Thanks for looking at the proposal. I admit the solution is not great and a quick check shows that performance is indeed worse than before. Instead of looping over all ACL rules, we could also loop over all existing users and probe the permissions for each one, but that leads to the same performance issues.

I think a proper solution would be rewriting auth_aclcheck_cb() and split the logic of the ACL rule matching and rule evaluation into separate functions/events. That should enable us to efficiently get all permissions for a page without needing to probe for every user/group. Of course this leaves plugins implementing the AUTH_ACL_CHECK behind and requires changes in the plugins to adapt to the new logic. But it should be possible to somehow solve this with fallbacks to the old behavior.

What do you think?