docassemble icon indicating copy to clipboard operation
docassemble copied to clipboard

Feature Request: Permissions overhaul

Open Gauntlet173 opened this issue 7 years ago • 9 comments
trafficstars

Right now it is not possible to create a privilege and give that privilege some, but not all, of the permissions that an admin has. Because we have users, and groups, and we can change what users are in each group, but we can't change what each group can do, except inside interviews.

Permissions should be checked, not membership in a privilege group, and Admins should have the ability to change the permissions granted to different privilege groups.

It should be possible for an Admin to use this system to delegate to a non-Admin user the ability to set or remove permission to access individual interviews.

This would make it possible, on a server-level, to have some users who are authorized to determine which interviews will be available to who, without those users having the ability to otherwise mess up the configuration of the server. This makes docassemble more SaaS-able.

Gauntlet173 avatar Jul 22 '18 19:07 Gauntlet173

If you want a non-admin user to have access to the contents of interviews, you can give that user the privilege of advocate. Then the user can use interview_list() and the related APIs.

There is currently no system for making privilege assignments such as "users A, B, and D can access interviews X and Y, and user C can access interview Z."

The docassemble privilege system is from Flask_User version 0.6. It supports users and roles, but doesn't have a system for associating roles with privileges to perform particular actions. If such a system were to be built, it would make sense for it to be built as part of the Flask-User project, because then any developers that use Flask could benefit from it.

jhpyle avatar Jul 22 '18 19:07 jhpyle

I started looking into this. Flask_User version 1.0, which is still in Alpha, has the concept of roles built in. It's incompatible with 0.6 though, so we should probably wait until the API is finalized before transitioning over.

epompeii avatar Aug 30 '18 14:08 epompeii

It seems that the user-role concept described on that page of the Flask-User v1.0 documentation is the same one from 0.6, which is already implemented in docassemble. See docassemble_webapp/docassemble/webapp/users/models.py.

jhpyle avatar Sep 02 '18 12:09 jhpyle

Thanks! I was conflating having roles with roles+permissions. Is having the concept of permissions in the data model something that you think would be worth having at this point?

epompeii avatar Sep 02 '18 15:09 epompeii

I think it's a worthy feature for Flask-User. If the Flask-User developers don't want it, I suppose I could implement a system in docassemble for it. I would implement it in a way where from the "Edit privileges" screen you could edit which tasks a given role can perform. There would be a SQL table of tasks, and a SQL table for the mapping between roles and tasks.

In memory, when a user is loaded, a list of tasks they can perform would also be loaded. This would all need to be in memory. I wouldn't want the calling of a random function in an interview to trigger a SQL call just to check if the user has permission to run the function.

I'm not sure how much of a priority this is. People should use emoji reactions on the GitHub issues to give a thumbs-up to issues they would benefit from.

jhpyle avatar Sep 03 '18 21:09 jhpyle

I agree that it would definitely be great feature to have in Flask-User, and it would be preferable to have it added upstream. I'll drop a line on their GitHub page to try and gauge interest or see if it is being worked on.

And that's what I was thinking as well, as far as the model and the permission management goes. When you're loading a user into memory, is it the Flask application's memory or is it on the Redis server?

Also agreed that a community based prioritization of tasks would be useful. I was considering asking whether to start a contributing channel on slack, but I don't know if that would create a disincentive for users opening up tickets here on GitHub. I would be interested in hearing your thoughts! As an aside, I don't see this as a pressing need either. I do think as docassemble gets used on larger projects though it is likely to be a more requested feature from sysadmins.

epompeii avatar Sep 03 '18 23:09 epompeii

By in-memory I mean in the RAM of the Flask application. The information could be in Redis, but I don't want redis overhead either. Redis is over the network if you have more than one application server.

I definitely think people should use GitHub rather than Slack for contribution-related things. If they don't know GitHub, they should learn!

jhpyle avatar Sep 03 '18 23:09 jhpyle

That makes sense from a performance perspective. I guess this is more of a general question, but when there is more than one application server, the load balancer picks an arbitrary application server. Does this mean that each time an HTTP/HTTPS (non-WebSocket) request is sent, that the application server needs to query the database to get information about the user or in a multi-server set up is this state cached on the redis server?

Haha, duly noted! And I created a ticket over on Flask-User, so we'll see what they say!

epompeii avatar Sep 04 '18 00:09 epompeii

I'm not sure how Flask handles user information or whether it caches the information it gets from SQL.

Docassemble is configured to use a Redis-based version of a key/value store for the Flask session information. But I think that session information is entirely separate from the user/role information stored in the current_user variable. Flask does not otherwise talk to Redis.

The state of an interview (interview answers) are all stored in SQL, so each page load involves a SQL query for the most recent step of the interview history.

jhpyle avatar Sep 04 '18 01:09 jhpyle

I am going through old issues to close them out. I overhauled docassemble's permissions system this year, so I am closing this issue.

jhpyle avatar Aug 13 '22 15:08 jhpyle