can_write_to_container() with a group ignores owner_guid

[brettp]

canEdit() is checked, but then if it's a group it checks membership. It shouldn't check membership if canEdit() is true.

[Brett Profitt]

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

Changeset: 413e15c1f180d46a177b95506c696404ee91163b

[cash]

Looks like this has major issues.

[cash]

Looks like [72a8ae00d14f608cae7e697e5845841510375da6] 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]

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 [72a8ae00d14f608cae7e697e5845841510375da6]. 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.

[Brett Profitt]

Fixes #3722. Checking for user when checking container.

Changeset: 66a4e75b5f2ab9e44835b72cf85057955485813a

[cash]

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

[cash]

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

[cash]

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

Changeset: b0379f1f7f73cd51609f3190269bab849cf5eaf0

[cash]

Fixes #4018 Refs #3722 merged up to 1.7.14 into master

Changeset: f19305b23bba276ef2601227d1e12f6beceef8ae

[cash]

Fixes #4018 Refs #3722 merged up to 1.7.14 into master

Changeset: f19305b23bba276ef2601227d1e12f6beceef8ae