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

getCorpora return value should contain permissions #77

Closed
francoisromain opened this issue Nov 14, 2017 · 10 comments
Closed

getCorpora return value should contain permissions #77

francoisromain opened this issue Nov 14, 2017 · 10 comments

Comments

@francoisromain
Copy link

francoisromain commented Nov 14, 2017

To update a corpus, a user needs ADMIN (3) permission on this specific corpus.

When we list all corpora on a page and want to display an Update corpus button for each corpus allowed to the current admin user, we need to:

  • list all corpora with getCorpora or getCorpus
  • loop over each corpora with getCorpusPermissions (and make as many subsequent http call) to get the permissions

It would be much lighter if getCorpora / getCorpus return value contained permissions (inside groups and users objects) for each corpus.

@hbredin
Copy link
Member

hbredin commented Nov 14, 2017

This would indeed make filtering corpora based on permissions much easier.

However, returning permissions for the whole set of users and groups could lead to security/privacy issues. I'd rather only return the current user's permission.

This would be enough to solve the issue, right?

@francoisromain
Copy link
Author

For the current user, we need to know the permissions on each corpus. Something like permission: 3 would be good.

@hbredin
Copy link
Member

hbredin commented Nov 14, 2017

Yes. That's what I had in mind.
I'll try to draft a first version but cannot really provide an ETA.

@francoisromain
Copy link
Author

Thinking about it, we also need to know where the permission comes from (in case it is changed afterward)… Let me think about it a bit more and come back to you. Thanks

@francoisromain
Copy link
Author

francoisromain commented Nov 14, 2017

Currently if the current user is not an admin, he can not know to which groups he belongs.

So it necessary to have:

  • on each readable corpus: a groups and a users properties with the permission level. users contains only one entry with the current user id and related permission level. groups contains all the groups the user belongs and the permission level.

(I guess we will have the same requirement for anything with permissions: media, layers, etc.)

And also it would help to have:

  • on the current user: a groups array with the id of all the groups he belongs (on the me function response for example).

@hbredin
Copy link
Member

hbredin commented Nov 15, 2017

I will try to patch GET /corpus to also return permissions.user and permissions.groups properties.

Concerning the second point, GET /me/group does return the groups of the current user.

@francoisromain
Copy link
Author

Thank you.
Oh I didn't see GET /me/group in the doc. It will work but still it would be more convenient to receive a groups property on the GET /me response (one less request).

@hbredin
Copy link
Member

hbredin commented Nov 15, 2017

Just pushed cde63fc to develop branch that adds a groups entry to the GET /me response.

@hbredin
Copy link
Member

hbredin commented Nov 15, 2017

Just pushed df051bf to develop branch that adds a 'permissions' entry to list of corpora and layers. Only permissions for current user and their groups are listed.

I still need to do more tests to check that it did not break other parts of the API but that would be great if you could try it...

@francoisromain
Copy link
Author

francoisromain commented Nov 15, 2017

Yes this works perfectly. Thank you. Note that the root user receives the permissions for every user on a corpus (which is good).

@hbredin hbredin closed this as completed Nov 25, 2017
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

2 participants