Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle anonymous user vs group during permission creation #352

Open
fmigneault opened this issue Sep 10, 2020 · 6 comments
Open

Handle anonymous user vs group during permission creation #352

fmigneault opened this issue Sep 10, 2020 · 6 comments
Assignees
Labels
enhancement Improvements in term of performance or behaviour investigate Issue or new component that needs further exploration
Milestone

Comments

@fmigneault
Copy link
Collaborator

Should we at least open an issue to enforce that and avoid unexpected behaviors?

Originally posted by @dbyrns in #340 (comment)

As described in the docs and observed a few times, setting permissions to user anonymous is permitted, but often leads the wrong impression that the resource would become public. To do so, it is rather the group anonymous that must receive the permission.

Since this mistake is easily encountered and produces results not matching expectations, we should try to think of a method to prevent this situation.

The biggest problem is that, technically, anonymous user is not special in any way compared to any other user. It is only created at startup for historical reasons (i.e.: when each user had their corresponding group named the same as their username), but more recent Magpie only requires the anonymous group which is automatically applied to all users. Checking explicitly for MAGPIE_ANONYMOUS_USER to block permission creation could create problematic situation where this was the intended user to apply the operation to.

  • How can we handle this better?
  • Should anonymous user remain at all?
@fmigneault fmigneault added enhancement Improvements in term of performance or behaviour investigate Issue or new component that needs further exploration labels Sep 10, 2020
@dbyrns
Copy link
Contributor

dbyrns commented Sep 10, 2020

I don't see the point in having this user at all if not for technical reason.
After investigation, if it's found that the user has no reason to be, then I would remove it.
In fact, the only special user that should be required is the admin one and still only to create other admins. Giving permissions to the admin user should not however be as confusing since it's a very common concept.

@fmigneault
Copy link
Collaborator Author

Admin user vs group has the exact same situation, but the issue is hidden since user within admin group is granted full access regardless. So specific permissions added explicitly on the user does not change the result.

For anonymous, I agree that the user could be removed altogether.
This will require a few tests to validate behavior as that dummy user is employed here and there in the code.

@fmigneault
Copy link
Collaborator Author

Some use case that I identified.
Only requests under /users/{user_name} possess the modifier flags inherit, cascade, effective to retrieve combined sets of user/group permissions.

When we are NOT* logged in, we can do /users/current/[...] and obtain the desired results correctly as these "current" routes are effectively other duplicated routes with explicit "current".
When we are logged in, one thing that was useful was to request /users/anonymous/[...] to retrieve the "public" inherited, effective, etc. permissions., since "current" would not point to "anonymous" anymore in this case.

We could argue that, given we target "anonymous" user, there shouldn't be any difference between inherited group permission from the "anonymous" group (or even effective user permissions), since that user is supposed to ONLY have unique membership to that group, none other and not even any direct permission on the user. The biggest problem in this case really is that routes /groups/{group_name} are admin-only, and opening it publicly only for that one use case of "anonymous" group will need a bit of work. It also feel counter-intuitive to fetch all effective/inherited permissions from /users-scoped routes, but do differently only for the use case of unauthenticated user, where we call /groups-based routes...

@dbyrns
Copy link
Contributor

dbyrns commented Sep 11, 2020

When we are logged in, one thing that was useful was to request /users/anonymous/[...]

In how this is useful? I mean, why would I want more than my own permissions or my effective permissions? For me, having access to public permissions would be the same as wanting to know my inherited permissions from groupA. And as you pointed out, this require an admin level and I'm fine with it.

@fmigneault
Copy link
Collaborator Author

For the end-user itself, it is not useful to have this information. At least I can't think of one.

It is useful for an admin to quickly debug a deployed instance.
For example, when a user unexpectedly could/couldn't have access to a resource, it is easier to evaluate the cause (is it due to inherited group other than public, due to the direct user permissions or due to specifically inherited group "anonymous").
There is no way to request "inherited" permissions with only a subset of groups. It is either all groups (inherited) or none (direct).

Calling /users/anonymous[...]?inherited=true was a quick way to obtain the public-only permissions and check the diff (with same format) as /users/<user-to-test>[...]?inherited=true. The same public-permissions should be available with /groups/anonymous[...], only requires some extra steps for comparison with user permissions.

@dbyrns
Copy link
Contributor

dbyrns commented Sep 11, 2020

Ok I see. Given that the only use case relate to an admin. I think that when we'll be ready to adress this issue it should not be hard to provide an alternative to the admin.
Quickly, I could see, for example, a new flag with /users/<user-to-test>[...]?explain=true that would provide a breakdown of permissions for the current user and its groups.

@fmigneault fmigneault added this to the Backlog milestone May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements in term of performance or behaviour investigate Issue or new component that needs further exploration
Projects
None yet
Development

No branches or pull requests

2 participants