Problem/Motivation
Admins may mark a profile field as private. However, doing so has no effect.
Steps to reproduce:
- Create a new profile type,
test
. Add two fields,fullname
andbio
. For bio, select "This is a private field". - Create two users. For both, fill in both profile fields.
- Assign the User module permission "view user information" (
access user profiles
) and the Profile module permission "test: View any profile" (view any test profile
) to authenticated users. - Log in as one of the two users you created.
- Visit the profile page of the other user you created.
Expected result: you see the value of the fullname field but not that of the (private) bio field.
Actual result: you see the values of both fields.
The relevant test incorrectly passes. The issue is that the test user is assigned a permission that doesn't allow access to the test profile type. The "other" user doesn't see the private field only because they can't seen the profile in the first place.
The minimum needed to for the test fail, demonstrating the bug, is in #2. The current patch includes an expanded test to cover the case where a user has view access to a profile but can't view a private field.
Proposed resolution
- Implement
hook_entity_field_access()
. - Improve tests by correcting the permission assigned to the "other" test user and testing for both public and private field access.
Remaining tasks
The new hook implementation will require a cache clear to show up. This should be noted in release notes.
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#23 | private_fields_are-2854336-23.patch | 5.07 KB | mglaman |
#20 | private_fields_are-2854336-20.patch | 3.1 KB | mglaman |
#14 | profile-private-2854336-14.patch | 8.73 KB | nedjo |
#14 | profile-private-2854336-14-test-only.patch | 6.63 KB | nedjo |
#14 | interdiff.txt | 1.29 KB | nedjo |
Comments
Comment #2
nedjoThe tests in
ProfileFieldAccessTest
currently pass. Apparently, this test passes only because the user doesn't have access to the profile at all:The test setup assigns the permission "view own [profile type] profile". Instead, it should assign the permission "view any [profile type] profile", so that the other user can access the user profile page.
Attached patch fixes this.
A better approach to testing might be to have two fields, one private and one public, and test both for all users.
Anyway, posting this to see what the testbot makes of it.
[Edited to remove superfluous/incorrect details.]
Comment #4
nedjoHere's a patch including a fix to the bug. Tests still need work.
Comment #6
nedjoWork in progress.
Expanding the test to introduce a second field so we can distinguish between the case where a profile page can't be accessed and that where only a single field can't be accessed.
Not yet passing for me locally.
Comment #10
nedjoOkay, this is now passing for me locally.
Changes to the patch:
Also filling in steps to reproduce in issue summary.
Comment #13
nedjoThe new hook implementation will require a cache clear to show up. This should be noted in release notes.
Comment #14
nedjoFix some typos in code comments.
Fill in details on resolution in issue summary.
Comment #17
nedjoComment #18
mglamanWill help wrap this up after #2844908: Start using the Entity API permission code.
Comment #19
mglamanComment #20
mglamanHere's a reroll against changes with permissions. The bypass permission was removed since it provided same functionality as administer.
Comment #22
mglamanComment #23
mglamanRe-roll to add second field (like original patch) which isn't private.
Comment #25
mglamanThanks, nedjo!