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(kafka acl): disallow all alias for principal #1333

Merged
merged 1 commit into from
Nov 24, 2021
Merged

Conversation

rkpattnaik780
Copy link
Contributor

@rkpattnaik780 rkpattnaik780 commented Nov 23, 2021

Kafka acl commands should restrict user from setting principal value to all along with *.

Closes #1330

Verification Steps

  1. Try to run grant-access with user set to all

rhoas kafka acl grant-access --producer --user all --topic x
It should throw the error: to set a wilcard on the ACL principal, pass "--all-accounts".

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation change
  • Other (please specify)

Checklist

  • Documentation added for the feature
  • CI and all relevant tests are passing
  • Code Review completed
  • Verified independently by reviewer

@@ -83,7 +83,7 @@ func NewAdminACLCommand(f *factory.Factory) *cobra.Command {
}

// user and service account should not allow wildcard
if userID == aclutil.Wildcard || serviceAccount == aclutil.Wildcard {
if userID == aclutil.Wildcard || serviceAccount == aclutil.Wildcard || userID == aclutil.AllAlias || serviceAccount == aclutil.AllAlias {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about if aclutil.GetResourceName(userID) == aclutil.Wildcard

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it will be a good idea to use GetResourceName at places that don't support alias.

@rkpattnaik780 rkpattnaik780 merged commit 48ef32a into main Nov 24, 2021
@rkpattnaik780 rkpattnaik780 deleted the restrict_all branch November 24, 2021 06:33
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.

Handle "all" alias for user and service account flags in kafka acl commands
2 participants