Problem/Motivation

I have my grants set up such that people who are allowed to edit my Event nodes can access the Grant tab, which only has "view" access for the "anonymous" user listed on it. Recently, when someone makes a node visible to anonymous, the view access for "authenticated" is getting removed, even though "Preserve hidden grants" is set in the system preferences at admin/config/people/nodeaccess. This has the effect of removing the events from the calendar after the user logs in.

From original summary, but no longer applicable:

Note that in many of these cases, the person who made the change is known to have full admin privs, however the Grant tab still only lists Anonymous on it. One theory is that something in checking that priv assumed that the admin user would have had all the other checkboxes when he really doesn't.

Steps to reproduce

See also: even more detailed steps -- plus, steps for testing that disabling "preserve hidden grants" also works as designed.
@todo: copy the steps from that ^^ issue (#3073621: Add test coverage for "Preserve hidden grants" option) into our skeleton/work-in-progress test file (#37).

  1. (Prerequisite -- enable "preserve hidden grants" -- admin/config/people/nodeaccess)
  2. On the nodeaccess settings page (admin/config/people/nodeaccess):
    1. ...check which roles are enabled in the "Allowed roles" fieldset;
    2. ...check which content types are enabled in the "Content type grant access" fieldset;
    3. ...pick a content type for which the "Grants" tab is enabled; and then,
    4. Expand the settings for that ^^ content type and grant some permissions to add/edit/delete to a role that is NOT enabled in the "Allowed roles" fieldset.
  3. Rebuild permissions;
  4. Log in with this role -- note that you are granted the permissions even though the role is unallowed or "hidden" in the grant form;
  5. Now go on the grant form node/%/grant of a node for this content type and add some permissions to other roles or users (Notice only the roles in the allowed roles are listed there);
  6. Rebuild permissions;
  7. Log in with this role again and you loose the grants given by your role that was not shown or hidden in the form.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joelpittet’s picture

Title: " Preserve hidden grants" setting ignored » "Preserve hidden grants" setting ignored
Version: 7.x-1.0 » 7.x-1.x-dev
Issue tags: +Needs tests

I think I just ran into this issue too. This likely needs tests to confirm the bug.

willzyx’s picture

Reproducing this bug is simple.

Steps to reproduce:
1. Download and install a vanilla version of Drupal.
2. Install and enable the Nodeaccess module (7.x-1.4 or 7.x-1.x-dev).
3. Configure Nodeaccass (admin/config/people/nodeaccess) settings:
Allowed roles : "anonymous user"
Default roles for article content type : "authenticated user"
4. Create article node. By default it is published and the grants working correctly
5. Unpublish node.
6. Go to grant tab and grant view access for "anonymous user".
6. Publish node.
7. Done. You lose default configuration.. now only "anonymous user" can access the node.

the bug was introduced by this commit.
For now modify grant on unpublished node with default settings is very risky!

vlad.pavlovic’s picture

Status: Active » Fixed

I have reverted the commit in question and removed the else as per #2228713: unpublished nodes are visible. Only published nodes will get grants written from now on.

justdave’s picture

Just to make sure the situation was understood, in the original report, the nodes in question *were* published already. The content type where the event items get posted defaults to authenticated users only. The editors have access to grant anonymous access to an event, but originally could not control whether it was visible to authenticated users or not (the checkbox to remove the grant for authenticated users was not even shown to them). When this bug started happening, when they would grant anonymous access, it would remove authenticated, even though they couldn't see it to uncheck the box. I had to give them access to check/uncheck authenticated users in order for the authenticated user checkbox to stay checked. Since this had nothing to do with whether a node was published or not, I just wanted to make sure that's what you actually fixed, since your "fix" comment is about unpublished nodes.

vlad.pavlovic’s picture

Status: Fixed » Needs work

Oh, so #2 didn't accurately describe your situation, I believe. I will reopen this.

willzyx’s picture

maybe my fault.. I was misled by 7.x-1.0 » 7.x-1.x-dev tag
The issue was open for old version 7.x-1.0 of nodeaccess and my test was on 7.x-1.4 and 7.x-1.x-dev.
With steps described in #2 after this commit the results are very similar.

@justdave
what version of nodeaccess are you using? the issue occurs again with the latest version of the module ? can you describe the steps for replicate the issue?

justdave’s picture

Currently using 7.x-1.4. I will test and see if I can still reproduce when I get some time later tonight. Always possible this has been fixed at some point since I filed it. :-)

davery’s picture

This rhymes with what i've run into. See my post here: https://www.drupal.org/node/2555957.

karoop’s picture

Status: Needs work » Needs review
FileSize
1.29 KB

Today I discovered that I have a problem that fits this issue. In my case, setting grants on admin/config/people/nodeaccess for roles that were not included in Allowed Roles, with the 'Preserve hidden grants' ticked, would not do anything.

More specifically, I have a site using a tight-ish permissions system. Most roles are grantable on per-node basis, but there is one ('Content Editor') that I would like to set up using node types. All works fine for nodes that didn't have any custom grants set up, but once the node Grant tab has been used to modify the node permissions, the grants for the 'hidden' role are completely ignored.

I've dealt with the problem by adding the default grants from hidden roles in nodeaccess_node_access_records(). The patch passed the tests on my local machine and seems to be working.

I'm not using nodeaccess to deal with any other permissions than view, so I realised that a similar solution should probably be applied for hidden permissions as well as for hidden roles. This however is out of scope of the bug I required to fix on our system, so I'll just leave this patch here as an inspiration ;)

LeDucDuBleuet’s picture

Version: 7.x-1.x-dev » 7.x-1.6
Priority: Normal » Major
FileSize
1.76 KB

The preserve hidden grants feature is still broken in version 1.6.

The previous patch perserve-ignored-2140819-2.patch fixes the problem in the nodeaccess_node_access_records function but with current 1.6, we also need to fix the nodeaccess_get_grants function. This function should return grants for roles even without names in the array in order for the preserve feature to work properly.

After applying this new patch, you need to rebuild the permissions to get back the preserved hidden grants for un-allowed roles.

Thank you!

Krilo_89’s picture

Status: Needs review » Reviewed & tested by the community

I used preserve-ignored-2140819-10.patch and this one works. Before a lot of pages showed me a no-access page, but after this patch and rebuilding my permissions, the site was working again.

I think it's a shame that a security update comes out with a release that breaks the website. Therefore this patch must be added as soon as possible.

pensrulz’s picture

I had a role that was granted node access on a certain page using the grant feature. However, after logging out, any node setup this way would not be viewable by anonymous or non-authenticated users. The patch in #10 fixed it. Below is how I patched a ubuntu system.

on linux

cd to nodesaccess module
wget patch file
patch p1 < patch-file-name

go to /admin/reports/status and down the page, in small print you "rebuild permissions".

cheers

alison’s picture

Version: 7.x-1.6 » 7.x-1.x-dev
Issue tags: -Needs tests +Release blocker
alison’s picture

@todo: provide detailed steps to reproduce in issue summary

LeDucDuBleuet’s picture

Steps to reproduce :

  1. Grant some permissions to add/edit/delete to a role for a content type without checking it in the allowed roles on admin/config/people/nodeaccess;
  2. Rebuild permissions;
  3. Log in with this role and you are granted the permissions even though the role is unallowed or "hidden" in the grant form;
  4. Now go on the grant form node/%/grant of a node for this content type and add some permissions to other roles or users (Notice only the roles in the allowed roles are listed there);
  5. Rebuild permissions;
  6. Log in with this role again and you loose the grants given by your role that was not shown or hidden in the form.

And with the patch, you keep the hidden grants from "unallowed" roles when the option is checked as intended.

alison’s picture

Issue summary: View changes

Oof, sorry for the delayed response.

(1) Thanks for the steps to reproduce!
(2) I'll do a final run-through the steps without and then with the patch, and then get this thing shipped.

alison’s picture

Issue summary: View changes
LeDucDuBleuet’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.24 KB

Previously, I had re-rolled the patch without asking myself more questions but I did some more testing recently with this patch and it is working well... the first time.

If you go back and change the default access for unallowed or hidden roles, the nodes will keep the previous hidden grants for hidden roles. They will not get the new default grants. The only way to get them, is to uninstall the module and start over. :-(

So here is a new patch which always use the default grants for unallowed or hidden roles. No need to fix the nodeaccess_get_grants function with current dev so the second part of the patch is not needed anymore.

Thanks.

alison’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Release blocker

Thank you, @LeDucDuBleuet! Really appreciate the consideration and work.

I'm going remove this as a "release blocker," but note in release notes and on the project page as a known, ongoing bug.

alison’s picture

Status: Reviewed & tested by the community » Needs review
DamienMcKenna’s picture

DamienMcKenna’s picture

Darren Oh’s picture

Status: Needs review » Reviewed & tested by the community

With the patch, I can no longer reproduce the bug.

alison’s picture

FileSize
5.44 KB

Thank you, @Darren Oh and @DamienMcKenna!

**EDIT** please ignore the interdiff, so sorry, moving/clicking too quickly, wrong interdiff...

Attached an interdiff for #14-18, for the record.

alison’s picture

FileSize
1.14 KB

Take 2 -- interdiff for #10 to #18, sorry about before!

alison’s picture

Update:

I can't reproduce the original problem anymore :-/ I couldn't reproduce it on one of my own projects, but that project also has workbench_access, so who knows what's affecting what.

I created a simplystest.me sandbox with 7.x-1.6 plus the three committed patches (otherwise, the "allowed roles" settings wouldn't be available to me at all, among other things).

  • I have the default user roles plus two extra, for testing purposes (a role called "other," a role called "misc").
  • "Allowed roles" is just anonymous and authenticated.
  • "Content tab grant access" has Basic page enabled.
  • Basic page ctype settings are, "View" is enabled for anonymous, authenticated, and a user role called "other."

After saving the aforementioned ^^ nodeaccess settings (admin/config/people/nodeaccess), I rebuilt permissions. Then:

  • As "admin," I created a basic page called "nothing" (node/1).
  • I viewed this page as anonymous -- success.
  • I logged-in as "otheruser" (password = "pass") and viewed node #1 -- success.
  • (I closed the private browsing window I was in, to end that session/clear cookies/etc.)
  • Back as "admin," I went to node #1 and into the "grant" tab.
  • I unchecked "view" for "anonymous" and "authenticated" roles -- the role called "other" was not shown in the "grant" form.
  • I rebuilt permissions (fwiw there was no prompt, but I did it from "status report").
  • I viewed node #1 as anonymous -- "access denied" -- success.
  • I logged-in as "otheruser" (password = "pass") and viewed node #1 -- and I was able to view the node.
  • (Obviously, that's what we "want" -- users with the role "other" should get to see the basic page because "view" is enabled for that role back on the nodeaccess default settings for the basic page type. But, that's where I'm not able to reproduce the problem anymore.)

Feel free to poke around the simplytest.me sandbox -- user #1 is "admin"/"admin" (the default login for simplytest.me sandboxes).

.................
@LeDucDuBleuet -- What do you think? (Or, do I have the steps wrong, or?)

DamienMcKenna’s picture

You might consider holding off on this until the underlying problem can be confirmed via test coverage?

alison’s picture

@LeDucDuBleuet Maybe something in those three already-committed patches took care of it...?

@DamienMcKenna That's a good idea! It feels like this one might not make it into 1.7 after all, and if it doesn't, that could be a good issue to work on in parallel.

alison’s picture

Status: Reviewed & tested by the community » Needs review
alison’s picture

Issue tags: +Release blocker

Release blocker for 7.x-1.8.

LeDucDuBleuet’s picture

Well, I am skeptical, I don't see how it could work now but I will test again with 1.7 to see if it is really working for me as well... Thanks.

LeDucDuBleuet’s picture

@alisonjo2786 When changing something in the Grant tab on nodes, you do not needed to rebuild the permissions for the whole site that is why there is no prompt.

LeDucDuBleuet’s picture

Ok I have tested 1.7 for this feature and it is working well now indeed but not for nodes with custom permissions on their Grant tab.

Without the patch, as soon as you grant anything on a node Grant tab, this node will loose the hidden role permissions.

When applying the patch and rebuilding the permissions, the hidden role will keep its grant even on nodes with custom permissions on their Grant tab.

Since I cannot review this issue myself, we need someone else to confirm the result of the tests I just did.

Thank you.

alison’s picture

Thanks for retesting and reporting back, @LeDucDuBleuet!

(I'm really wishing I used an OL instead of a UL in #26...)

Re: #33:

Without the patch, as soon as you grant anything on a node Grant tab, this node will loose the hidden role permissions.

The 6th bullet in my second bulleted list in #26:

I unchecked "view" for "anonymous" and "authenticated" roles -- the role called "other" was not shown in the "grant" form.

I thought I was testing "if you set custom permissions on the Grant tab" in this step, but I may have misunderstood.

@LeDucDuBleuet Like you said, I do think we need additional review -- and, maybe adding test coverage #3073621: Add test coverage for "Preserve hidden grants" option would help?
>> If you have time, we're going to talk about test coverage in about a half-hour! https://www.drupal.org/project/contrib_half_hour

LeDucDuBleuet’s picture

Well the patch is so simple and working, I don't why we would not publish it as is even without test coverage for now... :-)

DamienMcKenna’s picture

Assigned: Unassigned » DamienMcKenna

Going to work on test coverage for this during today's Contrib Half Hour meeting.

DamienMcKenna’s picture

Here's what we came up with during the limited time of the meeting.

Status: Needs review » Needs work

The last submitted patch, 37: nodeaccess-n2140819-testonly.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

alison’s picture

Issue summary: View changes
DamienMcKenna’s picture

Assigned: DamienMcKenna » Unassigned