Remove ACL(s) from node - from all modules

When you say 'acl', do you mean node_access records?

I mean that we should change this function:
function acl_node_clear_acls($nid, $module) {

to

function acl_node_clear_acls($nid, $module, $all_modules = FALSE) {
if $all_modules is TRUE then clear all the {acl}, {acl_node}, {acl_user} for this node.

Why would you want to do that? You cannot possibly know what you're breaking if you're doing that.

You can only clear {acl_node} "for this node", at most! Clearing any other table destroys data that must be preserved!

ACL is a shared infrastructure module, and we don't know what other modules are using it. For example, Forum Access is using it, and as the maintainer of FA, I'd be outraged if I had to deal with support calls about selected nodes that suddenly become inaccessible for no good reason.

How can you even contemplate such an approach???

I think your concern is valid for many cases. But here's the scenario that made me think about it:
I control the publishing of a node through ACL, while it's in ACL it's being edited by different people/ roles:
* Using wf_ng action i've added a node to ACL.
* using CA (content access) I've added the node to a different ACL, through the CA form.
* Now I decide to to 'publish' the node by removing all ACL from all modules. Currently for this I have now to remove the wf_ng ACL and the CA ACL from different places.

If you disagree then I'll accept it, but please don't get angry of just suggesting - not trying to do any harm :D

Ok, sorry...

The records in ACL are grants, you can't "'publish' the node by removing all ACL from all modules". This will typically make the node inaccessible.

Forum Access uses ACL to implement forum moderators. forum_access/TID records in {acl} get moderator records in {acl_user}, and FA automatically adds an {acl_node} record for every node that is created in the given forum, so that the moderators have full access to that node. If you remove that node's {acl_node} records, then the moderators lose their ability to edit/delete that node, and you've broken FA. Other ACL clients may do similar or wildly different things — we don't know, but we must not disturb their records!

The key here is that you must never assume that you're the only ACL client, and you must restrict your activities to ACLs where module=='your_module' (or == a cooperating module, such as wf_ng and CA, if you want to tie the two of them together).

This is part of ACL's contract, and I thought this was clear. Apparently, there is a fundamental misunderstanding here, and it makes me uneasy about what you're trying to do with wf_ng. Please review wf_ng and the proposed ACL patch in #200579: Provide workflow-ng integration in this light.

This idea of "'publish' the node by removing all ACL from all modules" seems to be based on a severe bug in Content Access — see #226090: Content Access disables other node access modules: Content Access behaves as if it were the only node access module in the Drupal world. This bug needs to be fixed first, and then you'll probably have to redesign your functionality...

Ok, I see your point about preventing a total destruction of records.
* I've created a patch to remove CA optimization of the 'view' grants.
* I'm sorry if I made you 'scared' by this suggestion, but apart of this there suggestion there is no 'messing' with other ACL tables. The current wf_ng integration is following Fago's original proposal. IMO The wf_ng is still be valid after the CA bug is fixed as it doesn't rely on CA.

Are you in the IRC?

Thank you for the CA patch — let's hope fago commits it soon...

If wf_ng doesn't rely on CA illegitimately creating 'view' grants, then how do you "'publish' the node by removing all ACL from all modules"?

No, I'm not on IRC.

You are right of course - the example I gave was incorrect ("publish the node by removing all ACL from all modules"). Sorry for that.
Just to clarify - the wf_ng is adding ACL(s) under the ACL module. It has no interaction with non-ACL modules.

Thanks for you help on getting it right :) I'm marking this issue as won't fix.

Ok, just to clarify what you mean with "under the ACL module":

wf_ng is passing 'workflow_ng' as module name for all of its calls to ACL, right?

And the node_access entries created by wf_ng through ACL are showing as realm=='acl' and explain=='workflow_ng/: ', right?

This is how it's supposed to work — please verify!

No, the explain is ACL. It isn't the wf_ng module doing the action, it's the ACL module itself (As per fago's explanation in the proposal).

i.e:
realm=='acl' and explain=='ACL'

Yes, I see it:

set "acl" as module

This is where the confusion comes from: a server trying to get a life of its own and forgetting that its primary purpose is to serve clients. I had failed to see this perspective.

We're a bit on the edge here. The .inc file is called acl_workflow_ng.inc and it provides wf_ng-specific extensions — I think we should compromise and use module='acl-wfng' and try to use as much of the acl.module API as possible, because this helps to avoid overstepping the boundaries that we need to keep.

I've updated the patch. The wf_ng is using _only_ the acl.module API, and doesn't try to take any non-API action.

@salvis,
One more thing was bothering me was giving an example of publishing using ACL (which is of course not yet possible). However, this is what I have in mind:
1. Through the CA module I gave the ACL a higher priority (again, not yet possible).
2. Node can be seen by everyone.
3. I've added user and node to ACL - node can be seen only by ACL users.
4. Remove ACL (i.e. publish) - again all user can see.