Make roles more intuitive

eric-burel commented 4 years ago

Following discussion with @juliensl

Current

Currently there is an ordering in roles like this: guests < members < admins.

But this imply confusing cases, a permission ["guests", "admins"] still allow members, because "members" > "guests". You would need a specific helper to handle this case. It can happen for "sign in" routes for example, that are only visible for non connected users.

Permissions mostly serves 2 use cases:

CRUD operations. In this case guest < members makes sense because you seldom have guest-only CRUD operation
Routing. In this case you may have a few "guest-only" routes, so guests != members

Proposal 1: without hierarchy

Instead we could propose following hierarchy:

guests are not connected people, actual guests that are NOT members (so no hierarchy)
members are connected people
admins is the only special case, you can add it for consistency but admin has access to all routes

So permissions would look like this:

[ ]: no one
['guests']: guests, but NOT members => eg for a sign in page
['guests', 'members']: guests AND members, so basically anyone (I think it's possible, but not worth it to add an anyone shorthand for this case) => home page for example
['member']: connected members only, no guests => every private page

Proposal 2: guests is renamed anyone

If we keep the current system, it may be good to rename guest to make it more explicit, eg call it anyone instead. So intuitively we read permission as "allowed to anyone" instead as "limited to guests".

https://github.com/VulcanJS/Vulcan/pull/2540

eric-burel commented 4 years ago

Another possibility is just renaming "guests" to be less prone to confusion, eg anyone. Because we often use the terme "guest" to describe specifically non-connected users, while in Vulcan it's a bit different.

juliensl commented 4 years ago

Hi Eric !

So permissions would look like this:

* [ ]: no one

* ['guests']: guests, but NOT members => eg for a sign in page

* ['guests', 'members']: guests AND members, so basically anyone (I think it's possible, but not worth it to add an `anyone` shorthand for this case) => home page for example

* ['member']: connected members only, no guests => every private page

It looks like the same role system than before, isn't it ?

eric-burel commented 4 years ago

No currently ['guests'] will allow 'members' too as far as I understand? Because getGroups add them automatically

juliensl commented 4 years ago

No currently ['guests'] will allow 'members' too as far as I understand? Because getGroups add them automatically

I wanted to say "before the current system", when getGroups did not add guests automatically.

eric-burel commented 4 years ago

Ok I understand. I should take a look at the commit history when I have some spare time.

juliensl commented 4 years ago

/**
 * @summary get a list of a user's groups
 * @param {Object} user
 */
Users.getGroups = (user, document) => {

  let userGroups = [];

  if (!user) { // guests user

    userGroups = ['guests'];

  } else {

    userGroups = ['members'];

    if (document && Users.owns(user, document)) {
      userGroups.push('owners');
    }

    if (user.groups) { // custom groups
      userGroups = userGroups.concat(user.groups);
    }

    if (Users.isAdmin(user)) { // admin
      userGroups.push('admins');
    }

  }

  return userGroups;

};

This is the previous getGroups function, you cannot be a guest and a member. So, it is making a permission system as you present in the first post :)

eric-burel commented 4 years ago

Yeah indeed it's Sacha's commit from 4 months ago ^^ Commit: a15874d3ad59a9d98d494045b4256d63224eb705

@SachaG this indeed ends up being a breaking change in existing app from 1.13 to 1.14

eric-burel commented 4 years ago

I've updated the proposals on the top post. I tend to like "Proposal 2", we keep the hierarchy (it really makes sense for CRUD operations, even if it makes guest-only routes more tedious to create), but rework the naming.

VulcanJS / Vulcan