Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Hello
Profile ProfilePermissionProvider
extends EntityPermissionProvider
adding some extra permissions "View any {profile_type}" & "View own {profile_type}".
However, those permissions are not being checked in ProfileAccessControlHandler
, given that EntityApiAccessControlHandler::checkEntityOwnerPermissions
is not being extended to support these granular fields.
As a result, I'm having issues with a user having "view own personal profile" permission but not the generic "view profile" one.
Comment | File | Size | Author |
---|---|---|---|
#18 | permissions-improvement-2926019.patch | 4.83 KB | anpel |
#7 | permissions-improvement-2926019-7.patch | 4.47 KB | plopesc |
#7 | permissions-improvement-2926019-7-empty.patch | 231 bytes | plopesc |
Comments
Comment #2
plopescHere is a first patch. Let's see how it behaves.
Comment #3
plopescComment #5
plopescSorry, wrong patch
Comment #6
plopescComment #7
plopescUploading a new version of the patch.
Investigating Entity module, discovered that
UncacheableEntityPermissionProvider
provides the extra permissions that Profile module is creating by himself, andUncacheableEntityAccessControlHandler
checks them. So no need to addextra logic in our classes.The use of this classes adds a new permission "View own profiles" and change the machine name of the "View profile entities" one, so added a hook_update_N implementation to make this change in existing sites.
The only drawback is that this requires beta1 version of entity module, that requires Drupal 8.3.x. Is that a big problem?
Attaching also an empty patch file to demonstrate that permissions related tests are broken with current code.
Thank you
Comment #9
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedreroll on 1.x-1.0-rc
drupal-8.4.3/html/modules/contrib/profile$ git apply permissions-improvement-2926019-7.patch
error: patch failed: profile.info.yml:4
error: profile.info.yml: patch does not apply
drupal-8.4.3/html/modules/contrib/profile$ patch -p1 < permissions-improvement-2926019-7.patch
patching file composer.json
patching file profile.info.yml
Hunk #1 FAILED at 4.
1 out of 1 hunk FAILED -- saving rejects to file profile.info.yml.rej
patching file profile.install
patching file src/Entity/Profile.php
patching file src/ProfileAccessControlHandler.php
patching file src/ProfilePermissionProvider.php
aegir@insitehost:~/platforms/drupal/8/servers/1-dev/sng-contrib/dev-social-8.x-1.8-drupal-8.4.3/html/modules/contrib/profile$ more profile.info.yml.rej
--- profile.info.yml
+++ profile.info.yml
@@ -4,8 +4,7 @@
core: 8.x
configure: entity.profile_type.collection
dependencies:
- - user
- - entity
- - field
- - views
- - system (>=8.1.0)
+ - drupal:field
+ - drupal:user
+ - drupal:views
+ - entity:entity (>=8.x-1.0-beta1)
Comment #10
plopescPatches have to apply against -dev version.
Drupal packaging system adds automatic changes to .info.yml files, so that's probably the reason why patch is not applying cleanly in that file.
Also, as #7 demonstrates, the patch apply properly because testbot worked as expected.
Could you try to apply it against -dev and confirm that works?
Thank you.
Comment #11
zrpnrPatch in #7 applies cleanly to 8.x-1.x-dev with git or composer.
This adds a lot to the changes to using entity for permissions started in https://www.drupal.org/project/profile/issues/2844908
and fixes the main issues I was having with "view own" and "edit own".
Code looks good! Thanks, marking RTBC.
Comment #12
mglamanI'm concerned the tests were not finding bugs. Really we need tests to prove a fix.
Comment #13
mglamanWe need test coverage. Also, do we want it to be "view any"? I feel like the default would be "own"
Comment #14
mglamanWe don't need tests. We have them. That's why this failed in the one issue.
Comment #16
mglamanCommitted, sorry for initial setback to needs work. Appreciated plopesc!
Comment #18
anpel CreditAttribution: anpel commentedReroll on 8.x-1.xThats what happens when you are in a rush. Can't delete this so please ignore.