-
Notifications
You must be signed in to change notification settings - Fork 17
Design doc for generalized permissions model #2590
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
base: master
Are you sure you want to change the base?
Conversation
| **Other specialized roles.** Roles such as "publisher", "asset manager", | ||
| "reporter", etc. are now possible simply by defining roles with a subset of the | ||
| full permission list. The possibilities here are bounded only by the power set | ||
| of the permissions and the needs of the archive administrators. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. I have one question. Currently, you can see the list of owners with the /users API endpoint. Would that change? Maybe that would return a json with user ids and roles? Similarly, how would this information be displayed on the DLP? Would you only see owners or would you see all users with roles associated with this dandiset? I think it might be useful to sketch this out in a bit more detail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The question your're asking is actually orthogonal to this internal permission re-design. You're asking "Once we add more roles, how should we display these roles on the DLP?". That kind of change to the API and front-end is outside the scope of this design doc, as all this design doc concerns itself with is internally refactoring the permission system to allow for more complex features, like what you're describing.
Adding more roles to the system (viewer, reviewer, etc.) is an undertaking in and of itself. This proposed design is just the groundwork that makes those changes feasible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another question: Would an admin have the power to create a custom role or would they be limited to the types listed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with Jake that we'll need to consider how to expose this new functionality in the web app. In the case of Dandiset owners, that is a specific enough role that I believe we'll continue to support it as a first-class concept in the system.
Other roles might just appear in a special view (available to those with the manage_roles permission) that lists all roles and all members thereof, and enables changing the memberships. (This is all subject to a design/discussion round.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would an admin have the power to create a custom role or would they be limited to the types listed here?
We've only thought about this a little bit. My gut feeling is that changing the set of roles would be a deployment level operation, with no runtime controls. That is, there would be something like a Django management command that could create/destroy/edit roles, but the intent would be to determine what roles are needed for a given deployment, and then let the system run with those.
So, the short answer to your question is "yes" but this is another thing we'll need to discuss further.
As for "limited to the types listed here", I'm not sure I'm following you: the proposal is that for n permissions, there are 2^n possible roles (even if many of them, like ones that lack view but grant editing capabilities, are sort of non-sensical). Given that clarification, do you still see limitations here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, let me be more clear. I was asking whether it will be possible for a dandiset owner to specify a custom role for a user for that dandiset, e.g. a role that allows for just view and unembargo. It sounds like the answer is no.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see. That's an interesting idea. The answer is currently (as you said) "no", but I think this is something we could discuss further once the basic system is in place.
| the need for the proposed permissions system, but is not essential to | ||
| understanding the proposal; accordingly, feel free to skip it or read it later.) | ||
|
|
||
| ## History and Current Status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For completeness and also potentially comparison of permission models, I think it is worth mentioning that we actually started with girder with its iirc extensive and modular (different storage backends iirc) permission model. We indeed didn't use most of its features and creating of custom backend solution resulted in the simplest model to get started to fulfill the core mission of public data redistribution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Girder's storage backend system is orthogonal to its permissions system (a storage backend stores files, but all files in the system appear as "items", which are the level at which permissions are enforced). I don't think Girder's permission system has any bearing on the design of DANDI's permission model, but if you recall any specific details that make you think otherwise, I can add them in here.
| A similar role could be useful in a blind review workflow: a *reviewer* who has | ||
| read-only access but whose identity is not visible to the owners. This would be | ||
| controlled by admins rather than Dandiset owners (drawing a specific distinction | ||
| between those roles). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious as to why the DANDI Archive admins would manage the reviewer access? I would prefer to hand these controls to the Dandiset owners so that they can give access as needed and to reduce the admins workload. Similar to how Google Docs works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but the reviewers are often anonymous
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was envisioning that the Dandiset owner could create a public URL to an embargoed Dandiset that contains a string of random alphanumeric characters (similar to Google Docs) and send this URL along with their manuscript to the journal editor. I am not sure how Google Docs does this but their URLs don't seem to get indexed by search engines. And I have never had an issue with a public editable Google Doc being commented or edited by a malicious actor, so presumably the Google Doc URLs are very difficult to find.
If the above assumptions are incorrect or the engineering effort here would be too large, then we (the admins) can manually manage this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well that seems like a pretty different model. In that case, the dandisets are technically viewable by anyone, and are restricted in practical terms simply by having an unindexed URL. In that case, you would need to set the document to "Anyone can View", making it technically public. In Google, add documents have a random string identifier, so this is not a problem. However here we use a very predictable form, so we would need to assign a reviewable dandiset a different url that contains a random string. Then I suppose when we want to share it for real we could have that random url redirect to the predictable url. That seems pretty different from the strategy proposed in this document.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kabilar What you're talking about implementing isn't really to do with our user permission model, it's more like an alternate authentication system. We would need to go through our entire auth flow, and if they don't have access, additionally check against this alphanumeric string, delegating any access through that. While that's an interesting concept and should be explored, it's sort of out of the scope outlined in this design doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These would be akin to S3's presigned URLs, and the idea makes a lot of sense for reviewer access. Conceptually, I don't think this is difficult to engineer, but the devil is always in the details, and of course this would require a design round.
However, if that's the mechanism we have in mind for reviewer access, that means we can actually simplify the design of the permission system (no need for this "non-visible" role idea).
(On the topic of search engine indexing: these wouldn't be public URLs in any way; you'd generate one, send it to the intended user, and then they'd access the Dandiset using it, but it wouldn't be a discoverable URL that could be indexed by a search engine. It would be closer to a DLP URL plus some secret attached, that the server would verify as being legit. So for a search engine to index one, it would have to randomly try octodecillions of URLs until it found one that didn't 404, which no sane search engine is going to even try.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So are we imagining a button that provides a randomly generated "read only" link for an embargoed dandiset? How would asset access work? If I'm not mistaken, downloading and using neurosift for an asset still require a token
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So are we imagining a button that provides a randomly generated "read only" link for an embargoed dandiset?
I'm picturing something slightly more complex: a view where you can see how many such URLs have been generated, and enables revoking them, etc. But yes, basically.
How would asset access work? If I'm not mistaken, downloading and using neurosift for an asset still require a token
This is where we'll need to dig into the tech more. Right now, we generate a very coarse-grained token for token based access, which essentially masquerades as the user it's associated with. What we'll need to do instead is find a way to generate and maintain multiple tokens with varying levels of permission. You could still generate a "user token" that behaves like our current tokens do, but you'd also be able to make tokens with limited permissions (for use with systems like neurosift, etc.).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while i know this is about design, the flow of these comments triggered the following thought.
in some ways all access can be controlled through tokens with different permissions/roles attached. the token metadata stored in the database can control all kinds of authorization.
token_A: (read_only, dandiset=225, asset=null)
token_B: (read_write, dandiset=225, asset=<id>, expiry=20251201)
token_C: (read_only, dandiset=null, asset=null, rate_limiter=5/s)
and any api endpoint basically has this token checker attached. connected to this is a user role model on what kinds of tokens a user (admin, owner) can generate or use (registered user, anonymous user).
this has both roles (users) and resource restrictions (tokens).
| In a "hard coded" permission model, this is actually not too bad; unfortunately, | ||
| there’s more. | ||
|
|
||
| ### The second quirk: viewers/reviewers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we create a Dandiset editor role to provide further granularity? Editors would be able to edit metadata, edit data, unembargo, and publish versions. Owners would be able to additionally delete a deletable Dandiset and manage user roles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is that anyone can create such a role.
If you're asking whether we should have a few predetermined roles that sort of ship with DANDI (and you're saying that editor should be one of them, alongside owner), that's a good idea worth discussing.
Besides owner and edtior, what other roles make sense? viewer perhaps?
satra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looks great. do take a look at my role + resource comment to see if there are some adjustments in the proposed plan that can accommodate a broader set of resource restriction functions.
No description provided.