Problem/Motivation
The edit link in the breadcrumb region of the secondary toolbar shows even when the user does not have permission to edit the current page.
Steps to reproduce
- Install gin and gin_toolbar
- Create a user with permission to access the admin toolbar but without the permission to edit nodes
- Create a node and view it with the aforementioned user
Proposed resolution
Don't show the link when the user doesn't have the permission to edit the current page.
Remaining tasks
Perform proper access check in gin_toolbar_preprocess_toolbar before adding the link.
User interface changes
API changes
Data model changes
Issue fork gin_toolbar-3319445
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
seutje commentedLooks like the fallback link "Back to Administration" also doesn't do a proper access check.
Here's a partial patch.
Comment #3
seutje commentedAdded patch also check if user has permission to the admin_content route.
Comment #4
seutje commentedForgot
use Drupal\Core\Url;.Comment #5
seutje commentedForgot to update status.
Comment #6
adstokoe commented#4 Works for me! Thanks.
Comment #7
adstokoe commentedComment #8
jurgenhaasUnfortunately, the approach with
PathValidator::isValiddoesn't really achieve what's needed. The permission check only looks for the permissionlink to any page, but that doesn't necessarily mean that the current user is allowed to edit the current entity. The rest of the patch looks ok, but the permission check needs a different approach.Comment #9
jurgenhaasI have started an MR which uses the simple entity access controller to see if the user has edit permissions.
Comment #15
elgandoz commented#11: the merge was blocked by a conflict and I (unsuccessfully) tried to rebase from upstream, sorry about the mess.
#14: this is a new branch with the cherry picked commit by @jurgenhaas, with the merge conflict resolved.
I had to change the access check since that I tested the patch with different roles and with one of them (implementer) I wasn't able to see the breadcrumb despite having the "[CT]: Edit its own content" permission.
See the differences between a moderator and a implementer here:

The issue was basically the same as #3204423: Entity edit-form routes check "update" access but not "view" access, caused by the confusion of the names between "edit" and "update" permissions for the entity access check.
Before:
$entity->access('edit')After:
$entity->access('update')Now it works nicely as expected, please review.
Comment #16
jurgenhaasThanks @elgandoz for updating the MR. Unfortunately, it only applies to 1.0.x-dev but not to 1.0.0-RC3 because of this commit: https://git.drupalcode.org/project/gin_toolbar/-/commit/4e66cd7c20b249be...
But there isn't much we can do about that.
Comment #17
elgandoz commentedHey @jurgenhaas, I removed the commits past your one and amended it, in order to have the 'update' access check, in the original
3319445-edit-link-inbranch you created, and force-pushed.So now the diff from MR!25 should apply on
1.0.0-RC3, while MR!30 goes on1.0.x-devin order to get merged.Please check since I didn't test it.
Comment #18
jurgenhaasThanks @elgandoz that's a nice solution as well. So MR !25 is for the latest RC3 and MR !30 is for the dev release. This is working great.
Comment #20
liam morlandRebased
Comment #21
jurgenhaasMR!30 didn't apply any longer and I've done a rebase. However, one part of the fix needs to be handled in Gin now, since the part of the template where this is required has moved over there.
Comment #26
hanoiiThe rebased one was missing some twig changes, I happened to worked on fixing this before having found this issue :facepalm:
I a very similar thing and went ahead and push it, it makes it cleaner in the the twig and uses the proper drupal services.
Comment #28
dpiOverall proposal looks fine, however I've modified the MR to be a little more correct.
Comment #30
jurgenhaasComment #31
jurgenhaasComment #32
anil.sharma commentedUpdated patch for version 2.0.0
Comment #33
anil.sharma commentedAdded entity access check to version 8x.
Comment #34
er.garg.karanComment #36
liam morlandThe merge requests should be updated to target branch 2.0.x.
Comment #41
jurgenhaasComment #44
dpiThanks..
Comment #45
damienmckennaFYI while this probably would have fallen under PSA-2023-07-12, this should have been reported privately as a security issue and then discussed with the Drupal Security Team. Please try to keep this in mind if similar issues show up again. Thank you.