getGroups changes

[juliensl]

Hy everybody !

The Users.getGroups function changed with the new version 1.14.1. Now, it is returning all the groups from guests to admin like :

• I am a guest : return ["guests"]
• I am a member : return ["guests", "members"]
• I am an admin : return ["guests", "members", "admin"]

So, the name of the Users.isMemberOf function sounds confusing for me, because if I am checking if a member isMemberOf(["guests"]), it will return true, where I am expecting a false.

So, I changed the name to isAtLeastMemberOfOne which is sounding better for me. What are you thinking about this ? Do you have another name idea ?

Also, I a little bit disagree with the new behavior of the getGroups function because, for me, guests and members are 2 distinct groups, there is no relation of superiority between them. Maybe I am expecting two different worklow for guests and members, and I do not want to mix them. What do you think ?

Anyway, with the new Users.getGroups function, we have to change the Users.getActions, because the !userGroups.includes('guests') verification is now false all the time. I changed it in the PR.

I let you thinking about this ;)

Have a good day and thank you for your work :)

[eric-burel]

Currently guest actually means anyone. We should clarify the way to disallow a route for members but allow it for non connected number. Thanks for opening this discussion it's interesting.

[eric-burel]

Thinking about this, I think we are right that we should have thought groups a bit differently, with no ordering between "guests" and "members". We would have ["anyone", "guests", "members", "admin"]. But there is also a natural ordering between "members" and "admin", so it's not that unnatural to have an order between "guests" and "members".

Therefore I think a change in behaviour is not worth it for now, instead we can improve our helpers naming and add doc where it makes sense. We could also create small helpers for permission eg "notSignedIn" to allow only guests on a route.

I'll open a proposal issue for a better role system, we can let that sink in for a while and ask other people opinion on this.

[SachaG]

Well if I'm honest I don't quite understand when you would need to allow something to non-logged in users but disallow it to logged-in users. By nature this is kind of impossible since any logged-in user can always become a non-logged-in user by logging out.

So I'm open to improvements, but I still don't really see the issue with the current system…

[eric-burel]

The only case I can think about is for routing, you don't want connected people to access the sign in route, they should be redirected. We have a few routes like this in Live for Good, for success messages after a long sign in process for example, a member should be redirected instead of seeing the message. I've opened a proposal, we can let it open for a while to think about it. I agree that there is a compromise, current behaviour is good too.

[juliensl]

Well if I'm honest I don't quite understand when you would need to allow something to non-logged in users but disallow it to logged-in users. By nature this is kind of impossible since any logged-in user can always become a non-logged-in user by logging out.

So I'm open to improvements, but I still don't really see the issue with the current system…

Hi !

You can have two specific workflows for guests and members on your website. For example, one specific homepage for guests and another for members. After that, there will be a specific guideline for guests to do specific actions and another specific guideline for members to do other actions.

Of course, members can log-out and play the guests workflow, but the goal is not to make some actions prohibited for members, but only masked actions and screens to the members to, for example, improve the UX / UI.

We decided to create theses specific workflow by checking if someone is a guest or is a member, we can replicate this behavior with the new logic, by checking if someone is a member or is not a member.
But I'm questioning myself about the role of guests group, it looks like completely useless now because I will never check if someone is a guest or not, anybody is a one.

Change the name from guest to anyone could be more understandable, but why keep it if it's never used ? We could maybe just delete it, have members and admins and play the permissions check on "is a member or not ?".

Do I missed a useful functionality for keeping guests or anyone group now ?

[eric-burel]

What bothers me more is the breaking change in this case. We should add more tests on this part too.

[SachaG]

Maybe this is a case of using permissions and groups for something they were not designed for? In the use case you describe I would test whether currentUser exists or not and then show different components, I would not try to use groups.

Do I missed a useful functionality for keeping guests or anyone group now ?

It's used to make schema fields public by setting canRead: ['guests'] on them.

[eric-burel]

Ok I think I get the misunderstanding. It seems that Sacha thinks mostly about CRUD operations when defining permissions while Julien is specifically working on routing and redirections here.

I agree that for CRUD operations "guests < members < admins" applies, you will barely ever have a "guest" only operation.

But for route you have "guest-only" routes, so guests may have diverging permissions.

So 2 solutions:

• we go the "guests < members" routes and we expect user to explicitly check for "currentUser" to differentiate guests and members (current behaviour). We work on naming if necessary. In this case I'd really prefer "anyone" rather than "guests".
• we rollback to "guests" != "members" (previous version from a few months ago)

The first solution is probably the more relevant if we consider that permissions are first meant to secure CRUD operations, given that guest-only routes is a limited use case. But we can take more opinions on https://github.com/VulcanJS/Vulcan/issues/2541