trac user Brett Profitt wrote on 41696525-01-02

Fixes #3722. can_write_to_container() doesn't override canEdit() permissions for group entities. elgg_override_permissions_hook() checks permissions for passed user.
Changeset: 413e15c

cash wrote on 41758108-06-28

Looks like this has major issues.

Milestone changed to Elgg 1.7.13 by cash on 41758108-06-28

cash wrote on 41760435-05-12

Looks like [72a8ae0] also dealt with this. I'm going to take a look at the original problem and the current solution. Then I'll post some thoughts here.

cash wrote on 41760490-01-20

can_write_to_container() is called from

• ElggEntity->canWriteToContainer() as a wrapper
• create_entity() to check if an entity can be added for an owner/container
• blog, bookmarks, file, and pages plugin to determine if an add menu item should be registered
• pages plugin to check if a user can edit a page

For the plugin usage, a user may or may not be logged in, but in the cases of a logged out user, it is expected that false is returned.

For create_entity(), we allow logged out users to create entities so that they can register. Further, there may be additional cases for this from 3rd party plugins.

In addition to fixing the bug described in this ticket, the commit also fixed a bug where the user object passed to the plugin hook was ignored by elgg_override_permissions_hook(). This probably should have been its own ticket. This secondary fix introduced the bug that was fixed in [72a8ae0]. The primary fix introduced a bug of the same variety as that one into can_write_to_container().

The fix looks simple - just add a check that the $user variable is non-null in the check for the $container variable being non-null.

I recommend that we add something into our best practices about checking object variables before calling a method on the object.

trac user Brett Profitt wrote on 41763558-02-05

Fixes #3722. Checking for user when checking container.
Changeset: 66a4e75

cash wrote on 41777013-07-31

I don't believe these changes have been merged into master/1.8

cash wrote on 41801637-08-18

Reopening due to change in how access works. See #3979 for details.

Milestone changed to Elgg 1.7.14 by cash on 41801637-08-18

cash wrote on 41801694-09-13

Fixes #3722 reverted to previous can_write_to_container() plus fix for group check overriding previous return value. Passed unit tests.
Changeset: b0379f1

cash wrote on 41847727-11-08

Fixes #4018 Refs #3722 merged up to 1.7.14 into master
Changeset: f19305b

cash wrote on 41850786-03-02

Fixes #4018 Refs #3722 merged up to 1.7.14 into master
Changeset: f19305b