Skip to content

Add set policy to multiple user/groups #382

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

Merged
merged 4 commits into from
Nov 30, 2020

Conversation

cesnietor
Copy link
Collaborator

Added api that allow setting policy to multiple users and/or groups.
PUT api on /api/v1/set-policy-multi/<policy_name>

Request payload response e.g.:

{
    "users": ["user1"],
    "groups": ["newgroup1"]
}

users and groups can be empty.

  • Tests included

@cesnietor cesnietor self-assigned this Nov 10, 2020
@dvaldivia dvaldivia added the enhancement New feature or request label Nov 11, 2020
// setPolicyMultipleEntities sets a policy to multiple users/groups
func setPolicyMultipleEntities(ctx context.Context, client MinioAdmin, policyName string, users, groups []string) error {
for _, user := range users {
if err := client.setPolicy(ctx, policyName, user, false); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if there's an error because a single user is wrong we'll go into a bad state wouldn't we, we some users were applied the policy and the rest was not due to one incorrect user in the middle of the list, why dont we add a validation for users and groups before we apply the policies?

Copy link
Collaborator Author

@cesnietor cesnietor Nov 11, 2020

Choose a reason for hiding this comment

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

yes, there's currently no way to make this 100% atomic. Name validation won't suffice, since it can still fail due to other circumstances.

Copy link
Collaborator

Choose a reason for hiding this comment

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

it's ok if it's not atomic, but we should at least make sure all the users and groups are valid before we start applying the policy

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've updated to validate on request the user/group string based on a regex defined on aws restrictions.

User names can be a combination of up to 64 letters, digits, and these characters: plus (+), equal (=), comma (,), period (.), at sign (@), underscore (_), and hyphen (-). 

@cesnietor cesnietor merged commit 829833f into minio:master Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants