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

Write only management API for User and Group resources #119

Merged
merged 26 commits into from
Apr 21, 2021

Conversation

refs
Copy link
Member

@refs refs commented Apr 14, 2021

What?

Essentially a write-only management API for User and Group resources. The CS3 API already provides the means for retrieving information for resources such as users and groups but it does not dictate how such resources are created. These minimal changes would add a different write layer to an administration API that would deal with destructive actions. Ultimately admin actions MUST only be taken by users with the role of administrator.

Adding roles will not be yet accounted for in this patch, and implementations should instead rely on their own logic to deal with roles, but a PR will follow up that would add roles to the CS3 API.

Design Choices

  • Namespace (separate) users from groups write/delete operations instead of delegating all to a single Admin API. I considered this as an approach taken in places such as identity.
  • Write operations SHOULD always return the latest version of a resource, therefore Write methods are required to return a whole Group and User resource.
  • No special status codes needed to be added considered the minimal additions.

@refs refs marked this pull request as ready for review April 14, 2021 10:26
@refs refs requested a review from labkode as a code owner April 14, 2021 10:26
@ishank011
Copy link
Contributor

@refs can you run make and push? That'll generate the documentation

cs3/admin/group/v1beta1/group_api.proto Outdated Show resolved Hide resolved
cs3/admin/group/v1beta1/group_api.proto Outdated Show resolved Hide resolved
cs3/admin/user/v1beta1/user_api.proto Outdated Show resolved Hide resolved
@refs
Copy link
Member Author

refs commented Apr 21, 2021

thanks for the review! good catch with the offset attribute ^^

Copy link
Contributor

@ishank011 ishank011 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but would more descriptive method names be helpful? Eg, CreateGroup, AddUserToGroup?

@refs
Copy link
Member Author

refs commented Apr 21, 2021

Oh, I was thinking the same, then did a little read on Google's guidelines and left the namespace be more explicit. BUT apparently I did not have a look at the examples which explicitly append the resource name to the method haha, let me amend the changes

@refs
Copy link
Member Author

refs commented Apr 21, 2021

@ishank011 just renamed the methods to stick to these guidelines 👍

@ishank011 ishank011 merged commit bc19782 into cs3org:main Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants