canEdit() allows invalid $user_guid argument #8038

Closed
mrclay opened this Issue Mar 5, 2015 · 6 comments

Comments

Projects
None yet
3 participants
@mrclay
Member

mrclay commented Mar 5, 2015

You can pass in GUIDs of non-users (e.g. 1), and it will function just fine, calling the plugin hook sending user => (site object).

Also you can pass in a non-existent/deleted GUID and it will silently convert this to the session user.

In both cases I doubt we want to silently let this pass this through.

@jdalsem

This comment has been minimized.

Show comment
Hide comment
@jdalsem

jdalsem Mar 5, 2015

Member

agree

Member

jdalsem commented Mar 5, 2015

agree

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Mar 5, 2015

Member

As @jdalsem mentioned, we may want to ignore access while calling get_user() to allow the case of "hidden" users. I've heard some systems have these. Then again, it might make a nasty BC break to change that behavior.

Member

mrclay commented Mar 5, 2015

As @jdalsem mentioned, we may want to ignore access while calling get_user() to allow the case of "hidden" users. I've heard some systems have these. Then again, it might make a nasty BC break to change that behavior.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Mar 5, 2015

Member

Options: throw InvalidArgumentException? Just return false and log an error for nicer BC?

Member

mrclay commented Mar 5, 2015

Options: throw InvalidArgumentException? Just return false and log an error for nicer BC?

@ewinslow

This comment has been minimized.

Show comment
Hide comment
@ewinslow

ewinslow Mar 5, 2015

Member

+1 for return false + log

On Thu, Mar 5, 2015, 7:16 AM Steve Clay notifications@github.com wrote:

Options: throw InvalidArgumentException? Just return false and log an
error for nicer BC?


Reply to this email directly or view it on GitHub
#8038 (comment).

Member

ewinslow commented Mar 5, 2015

+1 for return false + log

On Thu, Mar 5, 2015, 7:16 AM Steve Clay notifications@github.com wrote:

Options: throw InvalidArgumentException? Just return false and log an
error for nicer BC?


Reply to this email directly or view it on GitHub
#8038 (comment).

@jdalsem

This comment has been minimized.

Show comment
Hide comment

@jdalsem jdalsem self-assigned this Jul 7, 2015

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Sep 16, 2015

fix(permissions): All permissions functions handle user fetches consi…
…stently

All permissions functions now find hidden/disabled users and, when a
given GUID isn't found, they return false and issue a warning. Previously,
only some functions issued warnings, and some functions would silently
substitute the logged in user if a given GUID couldn't load.

This also adds functionality to the internal entity getter allowing fetching
entities that may be hidden/disabled.

Fixes #8941
Fixes #8038
@mrclay

This comment has been minimized.

Show comment
Hide comment
Member

mrclay commented Sep 16, 2015

PR #8944

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Sep 16, 2015

fix(permissions): All permissions functions handle user fetches consi…
…stently

All permissions functions now find hidden/disabled users and, when a
given GUID isn't found, they return false and issue a warning. Previously,
only some functions issued warnings, and some functions would silently
substitute the logged in user if a given GUID couldn't load.

This also adds functionality to the internal entity getter allowing fetching
entities that may be hidden/disabled.

Fixes #8941
Fixes #8038

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Sep 16, 2015

fix(permissions): All permissions functions handle user fetches consi…
…stently

All permissions functions now find hidden/disabled users and, when a
given GUID isn't found, they return false and issue a warning. Previously,
only some functions issued warnings, and some functions would silently
substitute the logged in user if a given GUID couldn't load.

This also adds functionality to the internal entity getter allowing fetching
entities that may be hidden/disabled.

Fixes #8941
Fixes #8038

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Sep 16, 2015

fix(permissions): All permissions functions handle user fetches consi…
…stently

All permissions functions now find hidden/disabled users and, when a
given GUID isn't found, they return false and issue a warning. Previously,
only some functions issued warnings, and some functions would silently
substitute the logged in user if a given GUID couldn't load.

This also adds functionality to the internal entity getter allowing fetching
entities that may be hidden/disabled.

Fixes #8941
Fixes #8038

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Sep 16, 2015

fix(permissions): All permissions functions handle user fetches consi…
…stently

All permissions functions now find hidden/disabled users and, when a
given GUID isn't found, they return false and issue a warning. Previously,
only some functions issued warnings, and some functions would silently
substitute the logged in user if a given GUID couldn't load.

This also adds functionality to the internal entity getter allowing fetching
entities that may be hidden/disabled.

Fixes #8941
Fixes #8038

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Sep 16, 2015

fix(permissions): All permissions functions handle user fetches consi…
…stently

All permissions functions now find hidden/disabled users and, when a
given GUID isn't found, they return false and issue a warning. Previously,
only some functions issued warnings, and some functions would silently
substitute the logged in user if a given GUID couldn't load.

This also adds functionality to the internal entity getter allowing fetching
entities that may be hidden/disabled.

Fixes #8941
Fixes #8038
Fixes #8945

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Sep 17, 2015

fix(permissions): All permissions functions handle user fetches consi…
…stently

All permissions functions now find hidden/disabled users and, when a
given GUID isn't found, they return false and issue a warning. Previously,
only some functions issued warnings, and some functions would silently
substitute the logged in user if a given GUID couldn't load.

This also adds functionality to the internal entity getter allowing fetching
entities that may be hidden/disabled.

Fixes #8941
Fixes #8038
Fixes #8945

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Sep 20, 2015

fix(permissions): All permissions functions handle user fetches consi…
…stently

All permissions functions now find hidden/disabled users and, when a
given GUID isn't found, they return false and issue a warning. Previously,
only some functions issued warnings, and some functions would silently
substitute the logged in user if a given GUID couldn't load.

Fixes #8941
Fixes #8038
Fixes #8945

@mrclay mrclay closed this in #8944 Oct 26, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment