Example_author author never checks if user is set/not anonymous, so the grant is granted to any anonymous users, which is not quite the expected thing in most situations.

Should add checks for if ($node->uid) and if ($account->uid)

#2011058: If no author, node_access_example_author grant allows all anonyomous traffic to edit/delete node examples module issue

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because the example is a bit confusing/insecure
Unfrozen changes Unfrozen because it only changes documentation to be better
CommentFileSizeAuthor
#17 D7-node-access-api-grants-all-to-uid-zero-2011066-17.patch1.16 KBadci_contributor
#11 node-access-api-grants-all-to-uid-zero-2011066-11.patch1.28 KBadci_contributor
#9 DrupalCore-8.x-dev-node-access-api-grants-all-to-uid-zero-2011066-9.patch1.27 KBjoshi.rohit100
#8 DrupalCore-8.x-dev-node-access-api-grants-all-to-uid-zero-2011066-8.patch2.08 KBGrimreaper
#5 DrupalCore-8.x-dev-node-access-api-grants-all-to-uid-zero-2011066-5.patch2.07 KBGrimreaper
#3 DrupalCore-8.x-dev-node-access-api-grants-all-to-uid-zero-2011066-3.patch1.32 KBGrimreaper
#2 DrupalCore-8.x-dev-node-access-api-grants-all-to-uid-zero-2011066-2.patch1.18 KBesbandeira
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

esbandeira’s picture

esbandeira CreditAttribution: esbandeira commented
Assigned: Unassigned » esbandeira

I'll work on this.

esbandeira’s picture

esbandeira CreditAttribution: esbandeira commented
Status: Active » Needs review
FileSize
DrupalCore-8.x-dev-node-access-api-grants-all-to-uid-zero-2011066-2.patch1.18 KB

Please review, I've perform the maintenance needed in this code.
Regards
Eduardo.

esbandeira’s picture

esbandeira CreditAttribution: esbandeira commented
Issue summary: View changes

adding link to examples module issue

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper commented
Issue summary: View changes
FileSize
DrupalCore-8.x-dev-node-access-api-grants-all-to-uid-zero-2011066-3.patch1.32 KB

Hello,

I remade a patch because the previous one was no more appliyable.

Is there something to test in the interface ?

Is there something to change in the documentation blocks ?

Else I see no problem.

Status: Needs review » Needs work

The last submitted patch, 3: DrupalCore-8.x-dev-node-access-api-grants-all-to-uid-zero-2011066-3.patch, failed testing.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper commented
Status: Needs work » Needs review
FileSize
DrupalCore-8.x-dev-node-access-api-grants-all-to-uid-zero-2011066-5.patch2.07 KB

Should be good now.

hefox’s picture

hefox CreditAttribution: hefox commented 
+++ b/core/modules/node/node.api.php
@@ -173,21 +173,23 @@
+  if ($account->uid) {

should this be $account->id()?

not familiar with account->id(), just guessing.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper commented

Hello,

I just review and rebase the previous patch.

If you tell me that $account->id() exists and it is better, ok no problem I will remake a patch.

You guess right, I was not familiar with its :)

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper commented
FileSize
DrupalCore-8.x-dev-node-access-api-grants-all-to-uid-zero-2011066-8.patch2.08 KB

Using $account->id() and $node->getOwnerId().

I don't know what I had in mind when I said "If you tell me that $account->id() exists" in the previous comment.

It was already in the code... Shame on me.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
FileSize
DrupalCore-8.x-dev-node-access-api-grants-all-to-uid-zero-2011066-9.patch1.27 KB

Created new patch as last patch failed to apply.
Please review.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Status: Needs review » Needs work
Issue tags: +Needs reroll
adci_contributor’s picture

adci_contributor CreditAttribution: adci_contributor commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
node-access-api-grants-all-to-uid-zero-2011066-11.patch1.28 KB

Trying to reroll

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Issue tags: +Documentation
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

This looks good.

I've also added a beta phase evaluation to the issue summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 6a3b965 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation for to the issue summary.

  • alexpott committed 6a3b965 on 8.0.x 
    Issue #2011066 by Grimreaper, esbandeira, joshi.rohit100,...
David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein commented
Version: 8.0.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)
Issue tags: +Needs backport to D7

Looks relevant for Drupal 7, I think, especially since the Examples module issue this grew out of was also for Drupal 7.

adci_contributor’s picture

adci_contributor CreditAttribution: adci_contributor commented
Status: Patch (to be ported) » Needs review
FileSize
D7-node-access-api-grants-all-to-uid-zero-2011066-17.patch1.16 KB

Trying to backport

  • alexpott committed 6a3b965 on 8.1.x 
    Issue #2011066 by Grimreaper, esbandeira, joshi.rohit100,...

  • alexpott committed 6a3b965 on 8.3.x 
    Issue #2011066 by Grimreaper, esbandeira, joshi.rohit100,...

  • alexpott committed 6a3b965 on 8.3.x 
    Issue #2011066 by Grimreaper, esbandeira, joshi.rohit100,...

  • alexpott committed 6a3b965 on 8.4.x 
    Issue #2011066 by Grimreaper, esbandeira, joshi.rohit100,...

  • alexpott committed 6a3b965 on 8.4.x 
    Issue #2011066 by Grimreaper, esbandeira, joshi.rohit100,...