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

Fix Set Policy on Groups with spaces #3453

Merged
merged 3 commits into from
Oct 28, 2024

Conversation

cesnietor
Copy link
Collaborator

@cesnietor cesnietor commented Oct 14, 2024

It was not possible to set a policy on a group with spaces:

Screenshot 2024-10-14 at 1 48 04 PM

We had a regex to validate the group name in the request but current UI doesn't allow the user to define that manually, the groups should already exists, so technically is not necessary. Right now this regex is wrong cause it doesn't allow spaces and it should. We could fix it but it but it might not allow other stuff, here we can rely on the server to return that error if something was wrong.

Screenshot 2024-10-14 at 2 53 29 PM

Test Steps:

  1. First:
  • Create a group with spaces like e.g. some group
  • Click on new group then Policies
  • Click on Assign policies
  • Assign policies and click save
  • All should work
  1. Also: Creating two groups
  • In Listing Policies
  • Select the two new groups
  • Select ASsign policies
  • Select policies to attach
  • Save (should work fine)

Copy link
Contributor

@bayasdev bayasdev left a comment

Choose a reason for hiding this comment

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

LGTM

@ramondeklein
Copy link
Collaborator

ramondeklein commented Oct 15, 2024

AWS IAM names have the following restriction (source):

Names of users, groups, roles, policies, instance profiles, server certificates, and paths must be alphanumeric, including the following common characters: plus (+), equals (=), comma (,), period (.), at (@), underscore (_), and hyphen (-).

I am surprised that MinIO accepts policy names with spaces. Not sure if that should be considered a bug. Fixing this in MinIO may result in issues for existing customers if they rely on this. It looks like there is only a check if a name is specified and there are no other checks (source).

@harshavardhana ^^^ What do you think? MinIO policies ≠ IAM policies and adding restrictions may cause issues for existing customers. But accepting policy names with arbitrary characters seems undesirable.

@ramondeklein ramondeklein merged commit dc19984 into minio:master Oct 28, 2024
20 checks passed
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.

4 participants