User permission management via Users view

[iostat]

Describe the feature you'd like
The ability to assign permissions to users from within Alcali itself. Currently, the only way to assign permissions to users that were created within Alcali is to go into the salt-master's configuration file, add an entry for the user under external_auth:rest with the desired permissions, and restart the salt-master (potentially, having to rinse-and-repeat for all masters in your fleet). However, the REST eauth API allows /api/token/verify to return a list of permissions to be granted to the user, which would then get merged with any defined in the external_auth:rest:<user> conf. If Alcali were to return a list of permissions to that request, we would no longer have to restart salt-master(s) on permission changes.

Furthermore, if Alcali were aware of user's permissions, it could start being selective as to which views to actually display to a user. For example, if a user without key management permission currently logs into Alcali, they just see an empty table under the Keys view. A more intuitive UX would be to simply remove that link from the menu bar outright.

If the mechanics for this are put into place, it would likely be relatively straightforward to introduce a notion of role-based access control (i.e., user groups) as well, removing the need to maintain repetitive sets of privileges (currently somewhat assuaged by YAML's ability to back-reference, but nonetheless not the most user-friendly) in either master config files, or in whatever Alcali widget would enable this (assuming there's no RBAC in the first iteration)

Additional context
Currently, /api/token/verify returns {'username':null} on a successful token verification, though returning {'username': ['perm1', 'perm2', 'etc']} is in theory all this necessary to enable this from the backend side (+ supporting SQL queries/tables/etc.).

[mattLLVW]

It's a tough one.

Alcali used to behave like this, and we are already storing salt permissions in users settings.

The thing is:

If Alcali were to return a list of permissions to that request, we would no longer have to restart salt-master(s) on permission changes.

I don't think that's true(correct me if i'm wrong!). Salt will always check external_auth ACL before executing states and modules.

if Alcali were aware of user's permissions, it could start being selective as to which views to actually display to a user.

That's how it worked in Alcali V1(before frontend refactoring) but it's pretty limited.We were checking for example if user any '@wheel' perms we would allow displaying the keys view.
But parsing permissions is hard and would have to be done in the frontend now.(plus it's only a frontend limitation, salt won't let you run state that are not allowed in the external_auth ACL anyway).

However, i agree that some sort of RBAC would be very good (for example, anyone can see all minions pillars, which is problematic if you store secrets there).

We just have to find a way to update perms without changing salt master config.

[shaidar]

I'm less concerned about using salt's external_auth to manage permissions and having to restart the service and more concerned about a limited user being able to view all minions and their pillar data. It would be nice to limit the view to what the permissions are set to in external_auth.

[ggiesen]

FWIW I hacked together a new auth module for salt combining the rest module for authentication and the pam module for groups (which in turn auths against LDAP at the system level) so at least salt permissions on the master based on groups rather than usernames. Obviously groups change far less often so you're not constant restarting the master every time you add a user.

[mattLLVW]

I'll soon be working on a RBAC that will look a lot like #323 limiting what users can and cannot see based on roles.

[ggiesen]

@mattLLVW,

FYI here's the branch I'm working on that adds LDAP group support:

https://github.com/latenighttales/alcali/compare/develop...ggiesen:alcali-group-support?expand=1

1. It expands the alcali eauth provider to provide group information back to salt (depends on setting AUTH_LDAP_MIRROR_GROUPS=True or a list of groups or AUTH_LDAP_MIRROR_GROUPS_EXCEPT to a list of groups)
2. It permits setting login and staff access based on group (AUTH_LDAP_USER_FLAGS_BY_GROUP)
3. It permits more complex group searching logic (which is required for our AD domain at least)
4. It permits more complex logic for group matching (multiple groups, excluding groups, etc)

It's really ugly right now as it depends on eval() to parse the group logic from a plain string, ie.:

AUTH_LDAP_REQUIRE_GROUP="( LDAPGroupQuery(\"cn=enabled,ou=groups,dc=example,dc=com\") | LDAPGroupQuery(\"cn=also_enabled,ou=groups,dc=example,dc=com\") ) & ~LDAPGroupQuery(\"cn=disabled,ou=groups,dc=example,dc=com\")"

and I'm contemplating changing the env strings for those into lists, ie.:

AUTH_LDAP_REQUIRE_GROUP='["(", "cn=enabled,ou=groups,dc=example,dc=com", "|", "cn=also_enabled,ou=groups,dc=example,dc=com", ")", "&", "~", "cn=disabled,ou=groups,dc=example,dc=com"]'

and attempting some form of at least basic validation (ie. check that each element is either in a list of allowed chars ['(', ')', '&', '|', '~'] or that it's a valid DN (ie. ldap.dn.is_dn('cn=John Doe,dc=example,dc=com') ) before piecing it back together and passing it to eval() to help keep the user from doing silly things. To preserve compatibility it'd check if the passed environment variable is a list of a string and fall back to the default behaviour if it's a string. Of course I'm open to better approaches.

Would also like to hear if such a PR would be useful and welcome.

We tend to use LDAP groups heavily as a form of access control/privilege assignment as manually assigning groups to each user within each individual app is very cumbersome. No issues at all with defining permissions to the each group at the app level, as long as assigning users to the actual groups can be done via LDAP.

[mattLLVW]

@ggiesen Nice work!
I'm definitely interested if it could be generic enough to be used by others.
Maybe replace eval by something more pythonic like getattr.
I'll play with it and get back to you.