singularityhub / sregistry

server for storage and management of singularity images
https://singularityhub.github.io/sregistry
Mozilla Public License 2.0
103 stars 42 forks source link

Default roles for new users #73

Closed victorsndvg closed 6 years ago

victorsndvg commented 6 years ago

Hi @vsoch ,

SRegistry is taking advantage of python-social-auth and comes with several SSO preconfigured providers like Twitter, Google, etc.

In the particular case of Fiware (and probably others) you can locally install the identity manager. This means that the SRegistry admin/deployer can control/manage the user accounts under a particular authentication strategy. This is something similar to LDAP, but providing SSO.

In the project we are using SRegistry, we need to manage users and we also need SSO capabilities. This why we are going to use only Fiware and LDAP for authentication. We are going to use these authentication strategies with different purposes, Fiware to manage the accounts of a new portal containing several services and requiring SSO, and LDAP for the company.

We want to assign different (or the same) default privileges (roles) depending on the authentication method used. I would like to assign the admin role as default for Fiware users and superuser (or admin) role for LDAP users.

In my mind I have two possible solutions, 1.- Assign a default role for all users. E.g. all users are admin as default 2.- Assign a default role per OAuth strategy. E.g. all Fiware users are admin and all LDAP users are superuser as default.

What do you think? It has sense from your point of view?

vsoch commented 6 years ago

hey @victorsndvg ! I think sregistry should have basic roles to determine admin vs not (the manager and admin as exists now) and then have the option for the user to create additional roles for users (whatever they may be, nothing hard coded) using standard django permissions. https://timonweb.com/posts/how-to-get-a-list-of-all-user-permissions-available-in-django-based-project/ So we should chat about a way to support that while still being flexible to

  1. not having any extra types at all, and
  2. having one or more custom types that are added in a config somewhere, and then editable both from manage.py and possibly interface

It might be best to look into packages that manage permissions, or to take the most basic implementation and turn it into a plugin. Let me know your ideas for that!

victorsndvg commented 6 years ago

Hi @vsoch ,

note that I don't need extra roles. What I desire is a way to assign a default role for first-time-loged-in users instead of using the manage.py CLI each time a new user wants to get the right access to manage collections

vsoch commented 6 years ago

hey @victorsndvg ! I would argue you do want some kind of customization of roles, because there is no "default" that is going to fit all possible use cases for admins like yourself. Can you imagine all the other admins that would want some version of a role to be default for their users? Right now, the "default" role is basically one with no permissions, and since pushing images / creating collections is not something you want plainly open to the public (even when they register with a social auth, because arguably anyone can do that) that is the most reasonable choice.

I really think this is important, but we must go about it in a "not hard coded" way. The admin needs to be able to 1. define some set of roles of interest, and 2) set custom defaults based on his or her preferences. And then the default is to give no permissions, of course.

victorsndvg commented 6 years ago

Ok, I misunderstood your previous response... but I really don't catch it perfectly. Don't worry, I will read a little to try to perfectly understood what you mean and to provide some useful thoughts.

I agree with the two "mandatory points" previously exposed

vsoch commented 6 years ago

okay great! And please remember that I'm here to help - if you can lay out a basic structure or ideas, then I can help when you have trouble. I'm really excited about this sregistry client, and I think it will be great when it's ready for your use. Happy Holidays!

victorsndvg commented 6 years ago

Thanks @vsoch ,

I'm also excited about it! I think it's a great tool and I really think you are doing and amazing work on it.

This "requested feature" could be helpful, but I think the current status of the project is working like a rocket. I will try to propose suggestions and to collaborate in its improvement (as much as i can). Any future improvement will be very appreciated.

The good news from my side is that this week I'm going to launch our own SRegistry in production for a pilot infrastructure. Thanks again for your work!

I will try to think and collect from our use experience some interesting roles for end-users and web-admins.

vsoch commented 6 years ago

woohoo! That is super awesome!! I'll keep charging forward so you can soon have the client ready too.

dtrudg commented 6 years ago

For one aspect of what @victorsndvg was requesting, when thinking about LDAP, the normal way of mapping users into admin/superuser type groups in Django apps is to use the standard django.contrib.auth User model fields ('is_admin', 'is_superuser') and map LDAP users into them on login by configuring django_auth_ldap something like this:

        AUTH_LDAP_USER_FLAGS_BY_GROUP = {
            "is_active": "cn=active,ou=django,ou=groups,dc=example,dc=com",
            "is_staff": "cn=staff,ou=django,ou=groups,dc=example,dc=com",
            "is_superuser": "cn=superuser,ou=django,ou=groups,dc=example,dc=com"
        },

Is there a particular reason you have a separate admin field in sregistry/shub/apps/users/models.py rather than using is_staff or is_superuser which would allow direct mapping from LDAP groups as above?

vsoch commented 6 years ago

Nope, I mostly didn't think much about it! I do know about the is_superuser, but didn't know about the other roles. This would be great to have!

dtrudg commented 6 years ago

Could do a PR for this.... could replace User.admin with User.is_staff and wrap it so that the command to set an admin via User.admin still works etc. - but now LDAP_USER_FLAGS_BY_GROUP works since we use is_staff underneath.

-or- completely strip out all reference to that User.admin field, migrating it into is_staff

vsoch commented 6 years ago

+1 get rid of the User.admin field entirely

vsoch commented 6 years ago

okay will have a PR for this shortly

victorsndvg commented 6 years ago

I think we can close this issue in favor of #89 Please open it again if you think this should be open