Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate possibility of more finely grained permisssions #108

Closed
rafrombrc opened this issue Jul 10, 2017 · 5 comments
Closed

Investigate possibility of more finely grained permisssions #108

rafrombrc opened this issue Jul 10, 2017 · 5 comments
Assignees
Milestone

Comments

@rafrombrc
Copy link
Member

Right now redash supports group access to specific data sources, but we may want to have more finely grained permissions where one group can access only a subset of a data source. And there may be workarounds where we use multiple data sources with different permissions.

@rafrombrc rafrombrc added this to the 7 milestone Jul 12, 2017
@alison985
Copy link

I took a brief look at how data source groups are currently implemented.

Options

I think these are the options for adding more granular permissions:

  1. Workaround: Multiple data sources for the same data resource that have different tables allowed. (i.e. add tables allowed to the data source definition)
  2. Another layer: Implement another layer of permissions (i.e. role > data source groups > data source tables). (i.e. add another object below data source groups in the hierarchy)
  3. More abstraction: Abstract permissions more broadly so things like "only view dashboards" or "only set alerts" are possible long term.
  4. Combo of 2&3.
  5. Add tables_allowed to data source groups.

Considerations

i. Long-term administrative maintenance. Both being able to keep track of table permission sets and being able to change permission sets without having to re-enter data source credentials.
ii. Programming time/complexity.
iii. Upstream integration. IMO, this should be contributed upstream and then pulled into the Mozilla fork. To do otherwise creates long-term code maintenance issues.
iv. Longer-term permission related issues/work.
v. Tables allowed versus tables disallowed. Do we keep track of tables allowed to be used or tables disallowed to be used? I believe given other context that I have that we want a tables allowed model. This will mean as tables are added that they will have to be pro-actively added to a permission set in order for anyone (besides Admins) to be able to see/use them. This avoids accidentally showing information to users who should not have access to it.
vi. Keeping or altering the view only v. full access distinction. I believe that we want to keep this distinction and add a third: full access to certain tables. I believe we do not want to have or deal with "view only to certain tables" or a combined "view these and full access to these others" permission level. The combined case could be dealt with through adding multiple data source groups to the same group.
vii. Overlapping and intersecting permissions. Since users can only belong to 1 group, this issue would come up at the data source groups level. To avoid difficult-to-identify issues, I believe data source groups should be additive. If the group the user belongs to has a data source group that allows access, the user has access. This will allow multiple data source groups for the same data source to be permissioned to a user group.

Evaluation of Options

  1. is probably the fastest/easiest to implement but it is messy because it could mean having a user with permission set A tables and permission set B tables. If they needed to be able to join across the sets you'd have to have permission set C tables, and pretty soon the administration becomes Not Fun At All.
  2. More complex to implement but easier for maintenance of permissions in an instance (i.e. less typing/selections).
  3. Seems like overkill at this time but is probably reasonable long term. There is probably something that can be learned from this model to help in the implementation of Add scrollbar to "Dashboards" dropdown accessible #2 to make Make query "draft" status explicit #3 easier long term.
  4. Too hacky/complex.
  5. RECOMMENDED Keeps most of data flow in tact, doesn't add another object/hierarchy level, provides more fine-grainedness. Works with considerations.

Recommended Plan of Action

I recommend proceeding with option 5 in the following manner:
A) Touch base with various folks to get feedback.
B) Programming steps include, based on upstream current code:

  • Add to the "View only"/"Full Access" options: "Full Access to certain tables".
  • Add tables_allowed field to data source groups object. Change unique index to require data source id, group id, and tables allowed. Add a name field to the object as well to make it clearer/easier to keep track of table groupings.
  • Add UI to data sources configuration page. (Take advantage of toggle functionality by table and the schema browser code used on query page, if possible.) This will also allow data sources to be assigned to groups at data source creation. Default create all tables group and give to the Admin group. Do not do any work to auto-add tables to the "all" group after data source is created. Do not allow for group creation on this page. Saving and deleting a data source group is a separate operation from the data source itself. (i.e. saving a DSG doesn't effect saving of DS.)
  • Make tables_allowed permission to be obeyed throughout the application.
  • Check data sources group page functionality to make sure you can still remove a data source group from the group on the DSG page. However, adding a data source group or changing permission level for the data source will be disallowed on this page. Add links to each data source detail page listed and the data sources list page.
  • Add tests for permission permutation cases.

Note, this plan of action doesn't account for the case of limiting which fields within a table are viewable/full access. Under this plan the Meta data source table data_sources would be not available at all to non-admin users, not even the 3 fields currently showing to users.

Thoughts?

@rafrombrc
Copy link
Member Author

This seems reasonable to me. I'd say we should get feedback from Arik re: if he thinks this is sane and whether or not he'd be up for merging it upstream.

@alison985 alison985 self-assigned this Jul 18, 2017
@arikfr
Copy link

arikfr commented Jul 23, 2017

Finally had time to review this and realized that I'm missing more details on the use case(s) you're trying to address. Because the eventual solution depends on this.

Regardless a few comments:

  • "Since users can only belong to 1 group": actually users can belong to multiple groups (see how admins belong both to the default group and admin).
  • "Add tables_allowed to data source groups": we had such feature in the past and I removed it. It's hard to enforce properly and introduces a security concern, and it's better to leave this to the database which already has a robust implementation for table level and sometimes even column level permissions. For such cases the data source <-> group type permissions makes sense IMO.

To extend the current permissions system's capabilities what I had in mind is:

  • Allow giving view-only permission on a per query and per dashboard basis. This will probably be used mainly to create read only user with limited access to the data (otherwise give them full read access to the data source).
  • Allow giving view-only permissions on a per query/dashboard basis + limit the user to specific parameter value. This is similar to the above, but allows sharing the same dashboard with different users who have different "row level" access (like different partners/clients).

To support both of these new use cases, there is a dependent task which is to allow secure execution of parameterized queries. Today parameters in queries are open to SQL injections (by design) -- to allow true "read only" users we will have to close this security concern. I did some research on this and have a few ideas on how to tackle this.

@alison985
Copy link

Thanks @arikfr. That background on removal of the feature before is great context. My understanding of the use case here is that a data source can house multiple datasets and some datasets should have restricted access.

A couple of questions:

  • Are you saying that to handle the restriction of different groupings of tables within a data source to different users the ideal method is to make differently permissioned database users and create multiple data sources (one per database user) in re:Dash instead of adding code functionality?
  • When you say "For such cases the data source <-> group type permissions makes sense IMO." does this mean purposely deciding not to support the case where a user wants to query across tables that are in different data source groups? The way I read and interpret this you could end up with Tables A-C in data source AA and Tables D-F in data source BB and not be able to query tables A and D together as an admin without a data source CC that is the union of Tables A-F, even though they reference the same database.
  • Why is "Allow giving view-only permission on a per query and per dashboard basis." dependent on parameterized queries?
  • Are you saying that in the UI for each query and each dashboard the owner (or admin) of the query or dashboard could pick which data source groups and/or users have view access to the query or dashboard? In this way they could opt someone or some group into viewing the query regardless of the users other permissions? If so, what would you think about an admin page so admin's could audit those extra permissions?
  • For the dependent task of "allow secure execution of parameterized queries" is that something you're looking to tackle on your own?

@alison985 alison985 modified the milestones: 8, 7 Jul 28, 2017
@arikfr
Copy link

arikfr commented Aug 3, 2017

Are you saying that to handle the restriction of different groupings of tables within a data source to different users the ideal method is to make differently permissioned database users and create multiple data sources (one per database user) in re:Dash instead of adding code functionality?

Yes.

When you say "For such cases the data source <-> group type permissions makes sense IMO." does this mean purposely deciding not to support the case where a user wants to query across tables that are in different data source groups? The way I read and interpret this you could end up with Tables A-C in data source AA and Tables D-F in data source BB and not be able to query tables A and D together as an admin without a data source CC that is the union of Tables A-F, even though they reference the same database.

Well, I would love to support this use case in a more elegant way than having to create data source CC. But I'm not sure how common this use case is. Usually what I've seen is that you have subset of the data that only some people need (PII data, finances, etc) and this is restricted. The rest is open.

Why is "Allow giving view-only permission on a per query and per dashboard basis." dependent on parameterized queries?

Because eventually people would like to give permission to a user on a specific query/dashboard that uses parameters, and current implementation requires full access to the data source.

Are you saying that in the UI for each query and each dashboard the owner (or admin) of the query or dashboard could pick which data source groups and/or users have view access to the query or dashboard? In this way they could opt someone or some group into viewing the query regardless of the users other permissions? If so, what would you think about an admin page so admin's could audit those extra permissions?

Are we talking about future implementation or the current one?

For the dependent task of "allow secure execution of parameterized queries" is that something you're looking to tackle on your own?

Well, I will have to eventually unless someone does it before me :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants