Redis Allowing modules to register commands to existing ACL categories.

Related to issue #11083

Module commands were not allowed to register commands to the existing categories. The only way was to either add each module command explicitly, or add +@all. I was also not possible to allow the existing users to have access to the module commands added at runtime in ACL categories.

This can be achieved by including the ACL categories explicitly with '@' prefix in 'strflags' flags of the RM_CreateCommand() as space separated categories. RM_CreateCommand("NewSet", "String", "write @write"); // Explicit model * Type of the command and the categories can be specified separately if there are multiple categories that are needed to be assigned. * In future, when modules will be allowed to create new categories, it will be very useful to specify and assign categories separately.

These flags can be converted from string to category flags by implementing a helper function matchAclCategoriesFlags() in the commandFlagsFromString() function.

To allow the existing users to have access to the module commands added at runtime in ACL categories, we can re-compute the allowed command (bit-map) from the command_rules list of existing users.

We considered and discussed 4 options and option 1 is recommended for implementing to allow modules to be added into existing categories: Option 1: RM_CreateCommand("NewSet", "String", "write @write"); // Explicit model Option 2: RM_CreateCommand("NewSet", "String", "apply-acls write"); // Have a new flag that applies categories Option 3: RM_CreateCommand("NewSet", "String", "write"); // Automatically apply categories. Option 4: RM_CreateCommand("NewSet", "String", "@write fast"); // Automatically apply categories based on the prefix.

Option 1 gives more control to modules to decide the behavior and access to the command, compared to other 3 options. Other options may break the current behavior of module commands and would not be backward compatible(eg. ‘-@all +@write’).

Please share your opinions and concerns.

Comment From: madolson

@oranagra @yossigo Would appreciate your input here.

Comment From: oranagra

seems like a clear cut to me, option 1 is the right one. p.s. why create an issue and not discuss in the existing one?

Comment From: madolson

The original issue had a bunch of related features, but they didn't all need to be implemented together. I thought it was easier to have sub-threads for each individual feature.