multinet-app / multinet-rfcs

A collection of proposals for Multinet development
0 stars 0 forks source link

RFC: Multinet permissions model #6

Closed waxlamp closed 4 years ago

waxlamp commented 4 years ago

This RFC lays out the permissions model used by Multinet. It is a simpler and coarser grained model than many systems use, designed to offer a measure of data protection and privacy while still allowing for quick implementation so the team is able to focus more on research issues than a more complex permissions model that may not bring major benefits as the project develops.

The model uses Role Based Access Control (RBAC) and Discretionary Access Control (DAC) to define who can perform what operations on the data in the system. The approach is coarse-grained because the permissions apply directly to whole workspaces, rather than individual tables or networks.

rendered

jjnesbitt commented 4 years ago

Should the proposal from https://github.com/multinet-app/multinet-server/issues/419 be included in the Implementation section?

alexsb commented 4 years ago

Thanks for this; it looks great. Here a few questions:

Is there a technical reason for the relatively fine-grained permission model? For example, remove and delete seem to be fairly nuanced.

I'm also not sure whether it's very helpful to separate write and remove permissions. If I can write, I can de-facto remove any content, by writing empty content anyways. So a simplification would be to allow remove and delte on write.

I'd probably also allow ppl with write access to grant roles; that's usually fairly useful.

I'd probably not have the Maintainer role; but allow co-owners? That's the github model, which seems to work well. I'd definitely also give the Maintainer grant permission, if we want to have that role.

waxlamp commented 4 years ago

Thanks for this; it looks great. Here a few questions:

Is there a technical reason for the relatively fine-grained permission model? For example, remove and delete seem to be fairly nuanced.

I think the reason for the nuance is a quirk of the Multinet data model (which probably we should write down for clarity's sake)--see below for more details, but "remove" refers to the ability to delete a table or network from a workspace, while "delete" refers to being able to delete an entire workspace altogether. They're definitely separate levels of power, and I don't think users who can delete tables ought to necessarily also be able to delete the entire workspace.

I'm also not sure whether it's very helpful to separate write and remove permissions. If I can write, I can de-facto remove any content, by writing empty content anyways. So a simplification would be to allow remove and delte on write.

This is not quite true: the way Multinet currently works, once you upload a table to a workspace, there's no way to further modify that table short of "removing" it--in particular, you can't simply upload an empty table to replace a non-empty one.

Because removal seems to be a "heavier" operation than writing a table, we elevated the remove permission into a separate tier from "write". On the other hand, if someone is allowed to write a table, then perhaps they should also be able to remove it (e.g., if they made a mistake in uploading the wrong CSV file, etc.).

All that being said, if you think it's better to collapse these two permissions (write and remove) into the "writer" role, that is fine with me. I just wanted to explain the technical background for why these are currently separate things.

I'd probably also allow ppl with write access to grant roles; that's usually fairly useful.

I'd probably not have the Maintainer role; but allow co-owners? That's the github model, which seems to work well. I'd definitely also give the Maintainer grant permission, if we want to have that role.

So we'll need to decide exactly how it will work, keeping in mind that each role in the system includes all permissions for the roles below it. That is, if writers have the grant permission, then there is no need to assign that to maintainer as well; the question will be, do we want the minimum granting role to be writer or maintainer?

If Maintainers receive the grant permission, then it seems like we may not need co-owners anymore. Maintainers would be able to do everything the Owner can do, aside from change the (main) ownership of a workspace (via the "transfer" permission). Can you confirm this reading of things? If I am correct here, I am also ok with renaming "Maintainer" to "Co-owner" (though in code, maintainer will read better than coowner).

So it looks like we have the following roles (with each entry listing just the new permissions introduced at that level):

Does this look right?

Thanks for your feedback @alexsb!

alexsb commented 4 years ago

Re write/remove: if I understand you correctly, that would mean that a writer can't delete a table they just created. I think that would be a UI/UX problem.

Your final summary looks great; I think it's good if we have the power to grant these permission more granularly on the back end, but summarize them for UI/UX. That way we can easily change stuff if we need to.

jjnesbitt commented 4 years ago

So it looks like we have the following roles (with each entry listing just the new permissions introduced at that level):

  • reader: read, query
  • writer: write, remove
  • maintainer (or coowner): grant, delete
  • owner: transfer

This seems a bit odd to me, as in this scenario, being the owner doesn't actually have any meaning, and as such transferring ownership would be useless. I'd think the delete role should be reserved for the owner. However, if this is the approach we want to take, I can table my concerns.

waxlamp commented 4 years ago

So it looks like we have the following roles (with each entry listing just the new permissions introduced at that level):

  • reader: read, query
  • writer: write, remove
  • maintainer (or coowner): grant, delete
  • owner: transfer

This seems a bit odd to me, as in this scenario, being the owner doesn't actually have any meaning, and as such transferring ownership would be useless. I'd think the delete role should be reserved for the owner. However, if this is the approach we want to take, I can table my concerns.

So it looks like we have the following roles (with each entry listing just the new permissions introduced at that level):

  • reader: read, query
  • writer: write, remove
  • maintainer (or coowner): grant, delete
  • owner: transfer

This seems a bit odd to me, as in this scenario, being the owner doesn't actually have any meaning, and as such transferring ownership would be useless. I'd think the delete role should be reserved for the owner. However, if this is the approach we want to take, I can table my concerns.

owner still has meaning, as the UI has to present a single actual owner for a workspace. I agree that delete should be reserved for the owner of a workspace, however. @alexsb, I will proceed with just that one change to the proposal above (i.e., moving the delete permission--that is, the permission to delete a workspace wholesale--to the owner role). If you have objections, let us know (and also we can always revisit this later).