Hey salvis and everyone,

Using ACL in combination with Content Access (see http://drupal.org/node/798106 for more background), I've tried to grant a user view access, update access, or delete access to a node (using Content Access' "Access Control" tab). But when I click the "Add user" button, the page reloads, but nothing happens (the user is not added).

After trying this for a few times, I eventually get the following error message:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '2' for key 1: INSERT INTO {content_access} (nid, settings) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1); Array ( [:db_insert_placeholder_0] => 2 [:db_insert_placeholder_1] => a:6:{s:4:"view";a:3:{i:0;i:1;i:1;i:2;i:2;i:3;}s:8:"view_own";a:3:{i:0;i:1;i:1;i:2;i:2;i:3;}s:6:"update";a:1:{i:0;i:3;}s:10:"update_own";a:1:{i:0;i:3;}s:6:"delete";a:1:{i:0;i:3;}s:10:"delete_own";a:1:{i:0;i:3;}} ) in content_access_save_per_node_settings() (line 398 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/content_access/content_access.module).

Any ideas? What I'm not clear about is whether this is a bug with the ACL module or with Content Access itself.

Thanks,
Ben

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

salvis’s picture

Status: Active » Needs work

Hmm, this is a tough one.

Apparently, the form is submitted to CA, not to ACL, because CA does something to its database tables. I must admit that I never fully understood the magic that merlinofchaos used with the embedded form, and apparently we need a different kind of magic for D7...

BenK’s picture

Thanks for the note, salvis. Any way we can check with Earl (merlinofchaos) about this?

--Ben

salvis’s picture

Well, the D6 code works just fine — it's the D7 code that causes trouble.

We can certainly talk to Earl or chx, but we need to do our homework first, like stepping through the code and checking what is happening, so we can ask intelligent questions.

atchijov’s picture

This SQL error has nothing to do with ACL per say. It is just a result of incorrect use of db_update() by CA module (incorrect in terms of D7) and I have fix for it.

Problem with "not adding users" does indeed lay in the way this form gets submitted (or rather fail to get submitted). As far as I can see, when you hit "Add user" button, none of callbacks from CA module gets invoked.

I will keep digging into this issue and will keep your posted.

(I am sort of made myself in charge of porting CA module to D7, this is why I am posting to this issue. BenK knows who I am)

atchijov’s picture

FileSize
19.01 KB

Ok, I have figured it out and it works now (see attached version). Problem with "submit" was due to the fact that

if (isset($form['#post'][$button_name]) && $form['#post'][$button_name] == $form['delete_button']['#value']) {

does not work anymore. The way to test it now (D7)

 if( isset(  $form_state[ 'triggering_element' ] ) &&  $form_state[ 'triggering_element' ]['#value'] == $form['delete_button']['#value'] ) {

Also, I have to fix acl_node_access_records(). db_query returns array of objects (not array of arrays), but $grants[] has to be array of arrays.

salvis’s picture

@atchijov: Thanks for jumping in!

My initial comment was a bit short. The problem is not the SQL error (which is in CA, I agree), but the fact that CA code is called at all when [Add user] or [Remove checked] are clicked. These two buttons should submit to ACL only (that's the magic I was talking about), and the result should be saved only when the user clicks [Submit].

As far as I can see, when you hit "Add user" button, none of callbacks from CA module gets invoked.

This is exactly how it's supposed to work. But then we should not get any SQL errors from CA at that point!

Please don't zip the files you post. If you need to patch more than one file then create a multi-file patch by diffing the ori/acl and patched/acl directories.

atchijov’s picture

FileSize
756 bytes
1.88 KB
BenK’s picture

Hey Andrei (atchijov),

I tried out your patches in #7 and things are working better. I'm able to successfully add users to an ACL using the "Add user" button and remove users from an ACL using the "Remove Checked" button.

The only problem comes up when the user who was granted access tries to view, edit, or delete the node.

Everything works fine if the node's title and teaser is displayed in a list of nodes (such as on the default front page). In this situation, the node title and teaser do appear to the user granted view access (and disappear if the user's view access is revoked). But if the user then tries to click through to view the node itself, he receives an "access denied" message.

So why would the access control seem to work in node lists, but not work on the actual node page? Is the role access for a node somehow overriding the ACL for the node (but only on the node page)?

@salvis: Is atchijov's patch in #7 consistent with how ACL's "magic" is supposed to work? I just wanted to check because it would be great to get some of these fixes committed as soon as we figure out the above-mentioned bug.

Thanks,
Ben

salvis’s picture

Status: Needs work » Needs review
FileSize
2.16 KB

@atchijov: Please follow the http://drupal.org/coding-standards (no tabs, no trailing spaces, space between if and ( but no spaces inside if (), line wrapping, etc.).

Here's a fixed patch. Please test this.

Install Devel Node Access (from http://drupal.org/project/devel) to verify that the proper node access records are written.

salvis’s picture

I still have doubts about the #9 patch, because I suspect that the buttons of the embedded ACL form cause CA's submit function to be called. This must NOT happen!

Also, I notice that CA embeds the ACL form three times. Do the three instances really work independently?

atchijov’s picture

@salvis, actually Ben was not quite correct in his original description. In my experience, that PDO Exception NEVER happen if you click "add" button, its only happen (used to happen) when you click "submit" button at the bottom of the page.

bottom line: there were nothing wrong with the way Drupal handling different buttons. Proper callbacks were invoked all the time.

BenK’s picture

I can confirm that atchijov's comment in #11 is accurate... my original bug report was a little misleading. The PDOException error occurred when I clicked the "submit" button at the bottom of the page (not when clicking the "Add User" buttons).

At the time, I wasn't clear on the behind-the-scenes distinction between adding users and submitting the changes.

Now I'll test salvis' updated patch in #9 (including using Devel Node Access)...

Excited to be moving forward!

--Ben

BenK’s picture

FileSize
44.29 KB
80.59 KB

Hey salvis and atchijov,

I tried out salvis' updated patch in #9. (I assumed that I only needed to apply patch #9, and not the patches in #7).

On the node access control page (node/[nid]/access), I was able to add and remove users using the "Add User" and "Removed Checked" buttons. But when I tried to submit this page, I received the following error message:

    *  Notice: Trying to get property of non-object in acl_node_access_records()  (line 182 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/acl/acl.module).
    * Notice: Trying to get property of non-object in acl_node_access_records() (line 182 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/acl/acl.module).
    * Notice: Trying to get property of non-object in acl_node_access_records() (line 182 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/acl/acl.module).

I believe it may be showing three of the same error messages because I tried to give the user named "editor" all three permissions (view, edit, delete). It does appear, however, that the user settings are being saved because upon refreshing the page, the "editor" user is being listed under "Grant view access," "Grant update access," and "Grant delete access."

Despite the ACL settings appearing as intended, however, node access is not functioning properly ("editor" is not able to view, edit, or delete the node). As suggested, I used the Devel Node Access module to verify this and it shows that "editor" does not have any of the access that I tried to grant.

In case any of this is unclear, I've attached screenshots of both my Devel Node Access table and my ACL settings for the node.

Any ideas what's causing the problem?

Thanks for both of you for your hard work on this!

Best,
Ben

P.S. Hey Andrei, salvis asked about why Content Access embeds the ACL form three times. Do you know if the three ACL form instances are intended to work independently like this?

salvis’s picture

FileSize
2.35 KB

Thanks for the feedback — here's an improved patch.

BenK’s picture

Hey salvis,

I tried out the patch in #14, but unfortunately Devel Node Access still shows the node access grants not actually being added. Additionally, I'm getting the following error message after editing a node's access control:

Notice: Trying to get property of non-object in acl_node_access_records() (line 194 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/acl/acl.module).
Notice: Trying to get property of non-object in acl_node_access_records() (line 194 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/acl/acl.module).
Notice: Trying to get property of non-object in acl_node_access_records() (line 194 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/acl/acl.module).

Also, when viewing a list of nodes (such as on the site front page), I'm getting the following errors:

Notice: Trying to get property of non-object in acl_node_access_records() (line 194 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/acl/acl.module).
Notice: Trying to get property of non-object in acl_node_access_records() (line 194 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/acl/acl.module).
Notice: Trying to get property of non-object in acl_node_access_records() (line 194 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/acl/acl.module).
Devel Node Access has detected duplicate records returned from hook_node_access_records():
11/acl/0/000 by acl
11/acl/0/000 by acl
11/acl/0/000 by acl

Any ideas for a revised patch?

--Ben

BenK’s picture

Status: Needs review » Needs work
FileSize
41.51 KB
32.6 KB

Hey salvis and Andrei,

It occurred to me that some of the error messages I posted in my last comment (#15) might be caused by the access control of nodes that were created before our most recent patches. So good news: I deleted all of my old nodes, created a couple of new ones, and all of the error messages went away! :-)

Node access still isn't totally working, but it does look like we're making progress and getting closer.

To test further, here's what I did (screenshots attached): For node 12, I turned off view, edit, and delete access for all roles (except for the administrator role). Then I used ACL to grant view, edit, and delete access just to the "editor" user (who does not have the administrator role) for node 12. The result was that the "editor" user could view node 12 in a list of nodes (such as on the front page), but received an access denied error if he actually visited the /node/12 page. Likewise, the "editor" user received an access denied message when visiting /node/12/edit.

But on the plus side, I am now seeing nice-looking ACL entries in the node_access entries table reported by Devel Node Access. (I don't think I saw these in prior patches.) However, in the "Access permissions by user" table reported by Devel Node Access, it still shows the "editor" user as being denied on all permissions.

So you can see for yourself, I've attached screenshots of both Devel Node Access tables to this comment. What do you think?

Thanks again,
Ben

salvis’s picture

FileSize
2.57 KB

Argh, missed yet another one! Here's the next iteration. I hope it'll be the last one...

"No: by content_access" — that's weird. atchijov will have to look into that once we have this sorted out here.

salvis’s picture

Status: Needs work » Needs review

Oh, and please try rebuilding the permissions and creating some nodes without any per-user access.

BenK’s picture

Hey salvis,

Thanks. I'm looking forward to testing the new patch in #17 and will do so shortly.

Quick question: If "No: by content_access" is weird, what is it supposed to say?

I'm new to Devel Node Access...

Cheers,
Ben

salvis’s picture

This is new in D7. In prior versions only the module that created the content type was able to authoritatively say NO. In D7 any module can do it, but it should rarely do so, because this overrides the node access completely.

I don't remember what the third value (besides YES and NO) is, but CA should return that third value, so that the decision is passed on to the node access system, where ACL's (and CA's!) grants are evaluated. DNA should say something like "YES: node access".

BenK’s picture

Hey salvis,

I just tried our you patch in #17. Unfortunately, I'm getting the exact same problems I that described in #16 (works properly on list pages, but doesn't work on node pages). The Devel Node Access tables look pretty much the same. As you suggested, I made sure I rebuilt permissions. Also, I tried some nodes without any per-user access control and those seemed to work fine.

So why is view access granted properly with lists of nodes but not on node pages themselves? Is Content Access somehow overruling node access set by ACL just on node pages? Could changing the "content node grants priority" of Content Access (currently set to "0" under advanced settings) help with this?

Looking forward to hearing your thoughts... and those of Andrei (atchijov), too.

--Ben

salvis’s picture

Status: Needs review » Fixed

Ok, thanks for your testing! I committed the ACL patch to the -dev version. Give it up to 12h to be repackaged.

Is Content Access somehow overruling node access set by ACL just on node pages?

Exactly.

Could changing the "content node grants priority" of Content Access (currently set to "0" under advanced settings) help with this?

No. CA needs to return the third value, as explained in #20.

BenK’s picture

Status: Fixed » Active

Hey salvis,

I talked to atchijov about replacing NODE_ACCESS_DENY with NODE_ACCESS_IGNORE and here's his response:

I am having problems with this. Either I do not understand how it is supposed to work or there is an issue wit the whole scheme of things.

Let assume that we want to configure access in such a way that anonymous users can access most of published articles, but not some. In current form I can do it, by enabling "View published content" for role "anonymous" (via "standard" user interface), then going to "Content/Access Control" and disabling "view any content" for anonymous user on particular nodes. In order to be able disable access, I have to return "deny".

If we do not want to use "deny", then the only way to do it is to disable "View published content" at "role" level, then enable "view any content" at content type level and then disable "view any content" for particular nodes.

I have 2 problems with this:
1) disabling "View published content" at "role" level - looks too drastic
2) "Access Control" in current form, will disable particular checkbox if role lucking given permission (so if we deny "View published content" for "anonymous", then in Access Control, "View any content" checkbox for role anonymous will be disabled).

Bottom line: it seems to me that at some point someone should be able to say "deny". If you have multiple modules which participate in node_access - then I do not understand how it is supposed to work.

(The full text of his comment is here: http://drupal.org/node/690610#comment-3207166)

Do you have any thoughts on how to solve this? Basically if we return NODE_ACCESS_IGNORE from Content Access, then how can we deny to everyone in the in the role except those added to the ACL?

Thanks,
Ben

salvis’s picture

The 'deny' is the default result in the node access system. If no module allows access (through the {node_access} table and hook_node_grants()), then access is denied.

New in D7: hook_node_access(). A module should only return NODE_ACCESS_DENY if it wants to preempt the node access system and not give any other module any chance to allow access.

The D6 version of CA was fully functional without that hook; you don't need to implement it. Just return NODE_ACCESS_IGNORE and see what happens...

1) disabling "View published content" at "role" level - looks too drastic

This is how it used to be done under D6. Try to implement your use case under D6 and study how it was done there.

AFAIK no one has implemented any non-trivial NA module under D7 yet, and we don't really know how to use the new hooks yet. We haven't proven that they really work yet. I'm working on porting FA, but I don't have a firm grip on this yet either. At this point I tend to think of the new hooks as convenience features, that may or may not open new possibilities or performance optimizations, but the basic functionality probably needs to be implemented using the old APIs, for multiple NA modules to get along with each other.

2) "Access Control" in current form, will disable particular checkbox if role lucking given permission (so if we deny "View published content" for "anonymous", then in Access Control, "View any content" checkbox for role anonymous will be disabled).

I'm not sure what atchijov is saying here. CA should not try to override a missing 'access content' permission. Read the node_access() function in node.module to understand how core intends this to work. There are many different checks that are performed in a well-defined sequence. Missing 'access content' results in a 'deny'' very early in the process. Returning NODE_ACCESS_DENY results in a 'deny' somewhere in the middle. The remaining checks can produce an 'allow', but if they all fail, then node_access() finally hits the return FALSE;, resulting in a 'deny'.

BenK’s picture

In researching this further, I'm wondering if ACL should actually be using hook_node_access_records_alter or hook_node_grants_alter, so that Content Access can return NODE_ACCESS_DENY and ACL can override it. Both of those hooks are new for Drupal 7.

What do you think, salvis?

I'm still new to all of this but trying to find a solution....

Cheers,
Ben

atchijov’s picture

RE: 2) "Access Control" in current form, will disable particular checkbox if role lucking given permission (so if we deny "View published content" for "anonymous", then in Access Control, "View any content" checkbox for role anonymous will be disabled).

Basically if you deny anonymous user role access to published content, then Access Control module will disable corresponding checkbox - making it impossible to "overwrite" it.

atchijov’s picture

I am still trying to wrap my brain about it, but it seems to me that if you have situation with multiple "access control" contributors, it is not enough to let them say "yes/no/dont-care". You have to specify order in which these contributors asked (and it is very possible that site admin should be able to specify such order).

atchijov’s picture

could it be that the problem is due to the fact that "core" role based permission system does not implement "dont-care" choice? All you can do it "yes/no".

Andrei

BenK’s picture

Hey salvis and Andrei,

Since I've got Content Access and ACL working successfully together in Drupal 6 (on a live site with tens of thousands of users) I went back and looked at my settings and used Devel Node Access to analyze the node access grants. Both of you have much deeper Drupal knowledge than me, but I'm trying to get to the root of the problem.

First, let me summarize the very common use case that atchijov and I are trying to solve. Basically, all users are granted the D6 "access content" permission so that they can view all nodes by default. (I agree with atchijov that it would be weird to uncheck this.) But on the "Premium" content type, I use Content Access to say that neither anonymous nor authenticated users can access this "Premium" content type. Then, I use Content Access with ACL to grant view access to "Premium" nodes to individual users (not roles) who I want to grant access to.

In D6, everything works perfectly. You can see in this D6 Devel Node Access block tables that I have attached. In the attached tables, the user named "The Fred-Man" is being granted access to the "Premium" node even though the role he is in (authenticated) does not have access. In D6, Content Access is returning a "NO" to both the anonymous and authenticated roles (equivalent to DENY in D7) and ACL is able to seemingly override this and grant individual per-user access to "The Fred-Man." All of this works perfectly without having to uncheck the core "access content" permission for any roles.

So as far as I can tell, this is the exact configuration I've been testing with in the D7 version... except that the configuration doesn't work in D7. So it appears that something has changed in D7 that is causing this to no longer work (specifically, the new treatment of NODE_ACCESS_DENY).

So in implementing Content Access, I think that atchijov needs to be able to DENY certain roles (because our use case is to deny by role and allow only per-user). I don't see how he can return an IGNORE because then those roles would still have access (because the roles are granted the "View Any Access" permission by D7 core). And I don't think the solution should rely on unchecking "View Any Access" for those roles because this step wasn't necessary in D6.

So where does this leave us? It seems like Content Access should return a DENY. This is important because in this use case, we really do want to preempt other modules from granting access by role. For instance, we don't want other node access modules (like Taxonomy Access Control or Organic Groups) to grant access to those roles (which they could do if we returned an IGNORE).

But then, ACL needs a way to override that DENY for an individual user. And this seems like the intended purpose of hook_node_access_records_alter or hook_node_grants_alter in D7. It seems to me that the reason both of these new hooks were added was because of how NODE_ACCESS_DENY now works.

A helpful video about these two new hooks can be found at: http://sf2010.drupal.org/conference/sessions/node-access-drupal-7. Go to 38:35 in the video for the discussion of these two new hooks and what they do. Ken Rickard (agentrickard) is doing the presentation... maybe we should ask him?

So what do you both think? Am I wrong here? (You both know much more Drupal than me, so I might be wrong!) But I'm trying to do what I can to help unravel this mystery...

Thanks,
Ben

atchijov’s picture

If I am reading this correctly, once ANY node returned DENY - game over. It looks like, solution lies in direction of completely ignoring hook_node_access() and instead use hook_node_access_records(). If we can agree that ACL will provide access records with HIGHER priority then Access Control, then it will always win.

Bottom line:
1) Content Access will not implement hook_node_access at all.
2) Instead it will employ hook_node_access_records with priority 0
3) ACL will use priority HIGHER then 0

I am not 100% sure that this is correct... your comments are more then welcom

Andrei

salvis’s picture

Title: Nothing Happens When Trying to Add a User to an ACL (ACL with Content Access) » Node Access Workshop
Category: bug » support

Have you read and understood the node_access() function?

Basically if you deny anonymous user role access to published content, then Access Control module will disable corresponding checkbox - making it impossible to "overwrite" it.

Yes, this is the correct behavior. In node_access(), if the user doesn't have 'access content' then core returns FALSE early. It never gets to calling any hooks. Consequently, there's no point in allowing the user to manipulate any checkboxes in CA, because CA never gets the chance to act.

it seems to me that if you have situation with multiple "access control" contributors, it is not enough to let them say "yes/no/dont-care".

But that's all you get from core. That's what you have to work with. If it was enough under D6, it's enough under D7, without even considering any of the new hooks.

Unless you positively know that a given node must be hidden (a function that hasn't been in CA so far, after all it's called Content Access, not Content Denial) or shown, you say "don't care" and rely on the fact that the node will be hidden, unless someone else (ACL or some other NA module) knows for sure that the node should be accessible.

You have to specify order in which these contributors asked (and it is very possible that site admin should be able to specify such order).

The order is determined by the weight of the module, which can theoretically be changed, even by the user, but because one weight is used for all hooks, you're not free to change it at will. If you really want to be the master and take full control, you could do this through the hook_node_access_records_alter() and hook_node_grants_alter() hooks, but this is way beyond the charter of CA (or FA).

Why do you feel you need more control? Why does the order matter to you? If ACL ran before CA and returned NODE_ACCESS_DENY, then CA would be out of luck. In our setting no one must return NODE_ACCESS_DENY! It's that simple.

could it be that the problem is due to the fact that "core" role based permission system does not implement "dont-care" choice? All you can do it "yes/no".

Not true. If you read node_access() you'll see that for 'access content' "no" mean DENY, but "yes" passes the buck on to whoever wants to say 'yes' or 'no'. If no NA module is installed, {node_access} has a single 0/all/0/100 record in there that will ultimately allow viewing. If any NA module is installed, core replaces this with nid/all/0/100 records for those nids where no NA module contributes any grant records, again resulting in allowing viewing. Otherwise core steps aside and it's up to hook_node_access_records() and hook_node_grants()...

Forget about hook_node_access() until you understand the basic mechanism. Oh, BTW, I do hope you've installed DNA from Devel and enabled Debug mode and the second block.

In researching this further, I'm wondering if ACL should actually be using hook_node_access_records_alter or hook_node_grants_alter, so that Content Access can return NODE_ACCESS_DENY and ACL can override it. Both of those hooks are new for Drupal 7.

NO!!! Read node_access()! If any module returns NODE_ACCESS_DENY from hook_node_access(), it doesn't matter what's in the {node_access} table (which is what you could influence with hook_node_access_records_alter()) nor is hook_node_grants_alter() ever fired because node_access() returns FALSE before it gets to firing hook_node_grants() and hook_node_grants_alter(). These two hooks are very, very advanced — forget about them until you have a very firm understanding of how node access works.

Please stop shooting in the dark. Node access is too complex for this. Don't bother to make any more suggestions until you've understood the purpose and working of each and every line in the node_access() function, as well as the implications of the order of those checks.

BenK’s picture

salvis,

Thank you for the very helpful information. I really appreciate it. Apparently, I have a lot to learn about node access and I'll read up more. ;-)

If you read node_access() you'll see that for 'access content' "no" mean DENY, but "yes" passes the buck on to whoever wants to say 'yes' or 'no'.

If I understand you correctly, I think one of my faulty assumptions was that I thought that if all roles have D7's "View Published Content" permission (equivalent to "access content" in D6) and Content Access returns IGNORE, then the node would be viewable by all users.

But what I think you're saying instead is that because core "passes the buck" and knows that a node access module is installed (and is exerting influence over that specific node), then access to the node will be denied (unless some other module explicitly grants access).

... you say "don't care" and rely on the fact that the node will be hidden, unless someone else (ACL or some other NA module) knows for sure that the node should be accessible.

It seems like the sentence you wrote above sums it up: If Content Access returns IGNORE, access to the node will be denied, even if all users and roles have "View Published Content" permission.

So, I think, here's the bottom line: If Content Access returns IGNORE instead of DENY, then all of these conflicts with ACL will go away and everything will work properly.

Does this sound about right?

Thanks again,
Ben

salvis’s picture

Yes, that sounds right.

(and is exerting influence over that specific node)

Let me expand this a little more: If no NA module supplies any record in hook_node_access_records(), then core will supply a nid/all/0/100 record (as I said before), which grants read access to the node. This means that CA (and any NA module) must supply at least one record for every node that it wants to control.

In the case of CA that's a nid/content_access_rid/0/000 record (if all checkboxes are off), as you can discover with DNA. Core will not store this record (000 doesn't grant any access), but it holds back its default nid/all/0/100 record, and the resulting absence of any {node_access} record for the given nid will ultimately cause access to be denied. If no one grants access, then it's denied.

BTW: CA for D6 (erroneously) supplies nid/content_access_rid/rid/100 records for all its nodes and all roles that have the 'administer nodes' permission. If you have any such role, you won't see the nid/content_access_rid/0/000 record, because CA optimizes what it returns.

if all roles have D7's "View Published Content" permission (equivalent to "access content" in D6) and Content Access returns IGNORE, then the node would be viewable by all users.

The permission is still 'access content'. 'View published content' is its translation to user speak, but we're talking at the developer level here. The node would be viewable by all users if either
— CA didn't return any access record (then core's nid/all/0/100 record would grant access) or
— CA has anonymous and authorized checked (its default state), in which case CA returns a nid/all/0/100 record (yes, it impersonates as core but DNA unveils it!)

However, if you remove all checkboxes and CA's hook_node_access() returns NODE_ACCESS_IGNORE, then access is denied, because no one grants it.

Let me emphasize this one last time: having 'access content' is a PREREQUISITE for all of these considerations. If a user has no role with 'access content', then the user will not be able to access any content — unless your module bypasses core's node access mechanism, which is generally considered to be a security leak.

BenK’s picture

salvis,

Wow. Your explanation was really, really helpful. Thanks so much. I just have one questions about something you said:

CA has anonymous and authorized checked (its default state), in which case CA returns a nid/all/0/100 record (yes, it impersonates as core but DNA unveils it!)

In our current D7 port of CA, CA by default has all boxes unchecked for a content type. But from what you're saying, it sounds like "View any" and "View own" should be checked by default for both anonymous and authenticated (to impersonate core).

I guess the only exception to this would be if a roll didn't have the "access content" permission (which would be unlikely in most use cases, but still could happen). In this case, all of the checkboxes for that roll should be disabled by default for the content type.

Am I understanding this correctly?

Thanks again,
Ben

salvis’s picture

Yes, this is correct: "View any" should be checked by default for both anonymous and authenticated (to impersonate core).

"View own" shouldn't matter if "View any" is checked, just set it to whichever state is more convenient (needs nothing to be stored). I'm not intimately familiar with CA's inner workings, but that's the reasonable behavior.

If you have "View any" unchecked upon installation, then all content will vanish and many admins will be shocked and cry for help. So, to avoid unnecessary support issues, the initial state should be checked.

This is correct, too: all of the checkboxes for that role should be disabled (and unchecked) for all content types. 'access content' is a global permission, not content-type-specific.

BenK’s picture

Great, thanks. atchijov uploaded a new version of Content Access and changed the module to return IGNORE instead of DENY. Everything now seems to work great. Content Access and ACL are playing nice together! :-)

I've just got one more new error message that surfaced. When visiting a node with per-node access control settings enabled--but no individual users granted view, update, or delete access--I'm getting the following error message reported by Devel Node Access:

Devel Node Access has detected duplicate records returned from hook_node_access_records():
4/acl/0/000 by acl
4/acl/0/000 by acl
4/acl/0/000 by acl

Interestingly, if I grant a user one permission (either view, update, or delete), one of the "4/acl/0/000 by acl" will go away. If I grant a user two or more permissions, then the error will go away completely.

Any ideas what might be causing this? Is it a Content Access issue rather than an ACL issue? (I can open a new issue for this if you like, but I thought it sort of flowed from our prior discussion so I kept it on this thread.)

I've attached my tables from Devel Node Access so that you can see what permission are set on the page.

Best,
Ben

salvis’s picture

Hmm, isn't it funny, when your watchdog uncovers your own shortcomings! :-)

The nid/acl/0/000 records should be nid/acl/acl_id/000 records. I've updated the -dev versions — give them up to 12h to be repackaged.

Thanks for the detailed report!

BenK’s picture

salvis,

Great! I've tested the latest -dev version and can confirm that your changes in #37 fixed the issue I reported in #36. Very cool!

There's is, however, one new issue that surfaced... Again, I'm not sure if it's a Content Access or ACL issue. Basically, I'm now getting the following error message when trying to save the site permission page:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '1-use text format 2' for key 'PRIMARY': INSERT INTO {role_permission} (rid, permission, module) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array ( [:db_insert_placeholder_0] => 1 [:db_insert_placeholder_1] => use text format 2 [:db_insert_placeholder_2] => filter ) in user_role_grant_permissions() (line 2865 of /Users/benkaplan/git/drupal/modules/user/user.module).

So I'm no longer able to save any changes to site permissions.

I'm working on a clean install of the latest -dev version of D7 core... and I didn't have this error before installing the latest ACL and Content Access. The error seemed to surface after I granted an "edit" permission to my "editor" role for the Article content type (on the content type access control defaults page). This seemed to cause a corresponding change on the D7 core permission page, which then caused the "Edit Any" and "Edit Own" permission to be disabled (and checked) on an individual Article node's access control page).

Any thoughts on why this error might be occurring? It's obviously a pretty major issue if it's causing the core permission page to no longer work...

Thanks,
Ben

salvis’s picture

ACL doesn't do anything with permissions. Disable ACL and verify that the error persists.

I'm puzzled about why 'use text format 2' is mentioned here though.

Does CA try to re-set all permissions, including those that are already set? The permissions tables have changed and there may be subtle changes in the API, too.

salvis’s picture

I think #38 is unrelated to node access but rather an inconsistency in the database content, probably caused by some stale data. I saw a similar error without any node access module installed.

See #607238-49: Permissions are assumed to be unique among modules, but uniqueness is not enforced.

BenK’s picture

Thanks, salvis! Your core patch at http://drupal.org/node/607238#comment-3218818 fixed the problem for me. I gave a +1 to your patch on that issue thread so that hopefully it will be committed soon.

Thanks again,
Ben

salvis’s picture

My pleasure, thanks to you!

salvis’s picture

Status: Active » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.