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).
- (Prerequisite -- enable "preserve hidden grants" -- admin/config/people/nodeaccess)
- On the nodeaccess settings page (admin/config/people/nodeaccess):
- ...check which roles are enabled in the "Allowed roles" fieldset;
- ...check which content types are enabled in the "Content type grant access" fieldset;
- ...pick a content type for which the "Grants" tab is enabled; and then,
- 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.
- Rebuild permissions;
- Log in with this role -- note that you are granted the permissions even though the role is unallowed or "hidden" in the grant form;
- 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);
- Rebuild permissions;
- Log in with this role again and you loose the grants given by your role that was not shown or hidden in the form.
Comment | File | Size | Author |
---|---|---|---|
#37 | nodeaccess-n2140819-testonly.patch | 4.32 KB | DamienMcKenna |
#25 | interdiff_10-18.txt | 1.14 KB | alison |
#18 | preserve-ignored-2140819-18.patch | 1.24 KB | LeDucDuBleuet |
|
Comments
Comment #1
joelpittetI think I just ran into this issue too. This likely needs tests to confirm the bug.
Comment #2
willzyx CreditAttribution: willzyx commentedReproducing 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!
Comment #3
vlad.pavlovic CreditAttribution: vlad.pavlovic commentedI 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.
Comment #4
justdave CreditAttribution: justdave commentedJust 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.
Comment #5
vlad.pavlovic CreditAttribution: vlad.pavlovic commentedOh, so #2 didn't accurately describe your situation, I believe. I will reopen this.
Comment #6
willzyx CreditAttribution: willzyx commentedmaybe 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?
Comment #7
justdave CreditAttribution: justdave commentedCurrently 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. :-)
Comment #8
davery CreditAttribution: davery commentedThis rhymes with what i've run into. See my post here: https://www.drupal.org/node/2555957.
Comment #9
karoop CreditAttribution: karoop commentedToday 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 ;)
Comment #10
LeDucDuBleuet CreditAttribution: LeDucDuBleuet as a volunteer commentedThe 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!
Comment #11
Krilo_89 CreditAttribution: Krilo_89 at Synetic commentedI 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.
Comment #12
pensrulz CreditAttribution: pensrulz commentedI 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
Comment #13
alisonComment #14
alison@todo: provide detailed steps to reproduce in issue summary
Comment #15
LeDucDuBleuet CreditAttribution: LeDucDuBleuet as a volunteer commentedSteps to reproduce :
And with the patch, you keep the hidden grants from "unallowed" roles when the option is checked as intended.
Comment #16
alisonOof, 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.
Comment #17
alisonComment #18
LeDucDuBleuet CreditAttribution: LeDucDuBleuet as a volunteer commentedPreviously, 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.
Comment #19
alisonThank 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.
Comment #20
alisonComment #21
DamienMcKennaComment #22
DamienMcKennaComment #23
Darren OhWith the patch, I can no longer reproduce the bug.
Comment #24
alisonThank 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.Comment #25
alisonTake 2 -- interdiff for #10 to #18, sorry about before!
Comment #26
alisonUpdate:
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).
After saving the aforementioned ^^ nodeaccess settings (admin/config/people/nodeaccess), I rebuilt permissions. Then:
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?)
Comment #27
DamienMcKennaYou might consider holding off on this until the underlying problem can be confirmed via test coverage?
Comment #28
alison@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.
Comment #29
alisonComment #30
alisonRelease blocker for 7.x-1.8.
Comment #31
LeDucDuBleuet CreditAttribution: LeDucDuBleuet as a volunteer commentedWell, 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.
Comment #32
LeDucDuBleuet CreditAttribution: LeDucDuBleuet as a volunteer commented@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.
Comment #33
LeDucDuBleuet CreditAttribution: LeDucDuBleuet as a volunteer commentedOk 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.
Comment #34
alisonThanks for retesting and reporting back, @LeDucDuBleuet!
(I'm really wishing I used an OL instead of a UL in #26...)
Re: #33:
The 6th bullet in my second bulleted list in #26:
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
Comment #35
LeDucDuBleuet CreditAttribution: LeDucDuBleuet as a volunteer commentedWell the patch is so simple and working, I don't why we would not publish it as is even without test coverage for now... :-)
Comment #36
DamienMcKennaGoing to work on test coverage for this during today's Contrib Half Hour meeting.
Comment #37
DamienMcKennaHere's what we came up with during the limited time of the meeting.
Comment #39
alisonComment #40
DamienMcKenna