The "view protected content" permission is used to determine whether a visitor should be allowed to view a node without having to enter a password. Isn't that what "bypass password protection" is for? i.e. isn't "view protected content" just a duplicate of "bypass password protection"? If not, the help text needs to be improved to explain why there are two permissions for what suggests are the same thing?

CommentFileSizeAuthor
#10 protected_node-remove_view_protected_content_permission-2424571-10.patch5.11 KBGrimreaper
#8 protected_node-remove_view_protected_content_permission-2424571-8.patch5.38 KBGrimreaper
#7 protected_node-remove_view_protected_content_permission-2424571-7.patch4.1 KBGrimreaper
#5 protected_node-n2424571-5.patch3.02 KBDamienMcKenna
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Grimreaper’s picture

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

Hello,

"view protected content" allows you to access the password form to enter the password and then access the protected node.

Yes, that's not very intuitive.

Documentation: https://www.drupal.org/node/2176635

I could not update the link on the project page.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented

Isn't that what the "access protected content" field is for?

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented

I noticed that granting the "view protected node" permission does indeed allow someone to view the node without having to enter the password. That does indeed seem to indicate that it is a duplicate of "bypass password protection" and should be dumped.

Grimreaper’s picture

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

Oups, sorry, I misread the permission and I responded too fast.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Status: Active » Needs review
FileSize
protected_node-n2424571-5.patch3.02 KB

This patch replaces use of the 'view protected content' permission with the 'bypass password protection' permission.

izus’s picture

izus CreditAttribution: izus commented
Status: Needs review » Needs work

Hi,

Thanks for the patch, i agree with the suggestion.

but in order to make the update not breacking the already assigned permissions, we need a hook_update_N to update roles that have the 'view protected content' and assign them the 'bypass password protection one'
also we need to do/trigger/check for some database cleanup (to be sure the deleted permission does no more exist in role_permission table)

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper commented
FileSize
protected_node-remove_view_protected_content_permission-2424571-7.patch4.1 KB

Rebasing the patch.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper commented
Status: Needs work » Needs review
FileSize
protected_node-remove_view_protected_content_permission-2424571-8.patch5.38 KB

Hello,

Here is a patch with a hook_update_n to manage the deletion.

izus’s picture

izus CreditAttribution: izus commented
Status: Needs review » Needs work

Hi,
thanks for the patch

+++ b/protected_node.install
@@ -200,3 +200,36 @@ function protected_node_update_7101() {
+    db_merge('role_permission')

here we should use user_role_grant_permissions as it takes also care of deleting some cache after the db request

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper commented
Status: Needs work » Needs review
FileSize
protected_node-remove_view_protected_content_permission-2424571-10.patch5.11 KB

Thanks for the review.

Yes, I reinvented the wheel with my db operations. I also use user_role_revoke_permissions.

I wonder If we should not add a node_access_needs_rebuild() ?

  • izus committed 412cb2a on 7.x-1.x authored by DamienMcKenna 
    Issue #2424571 by Grimreaper, DamienMcKenna, izus: Is "view protected...
izus’s picture

izus CreditAttribution: izus commented
Status: Needs review » Fixed

No need of rebuild access as the two API functions delete the accss cache.
Thank you al for contribution
this is now merged.

Status: Fixed » Closed (fixed)

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