Hello,

The following unexpected behavior is reproducible on unstable:

127.0.0.1:6379> ACL setuser gilad on >1234 ~* -@all +acl +get|x
OK
127.0.0.1:6379> HELLO 3
1# "server" => "redis"
2# "version" => "999.999.999"
3# "proto" => (integer) 3
4# "id" => (integer) 4
5# "mode" => "standalone"
6# "role" => "master"
7# "modules" => (empty array)
127.0.0.1:6379> AUTH gilad 1234
OK
127.0.0.1:6379> GET y
(error) NOPERM this user has no permissions to run the 'get' command or its subcommand
127.0.0.1:6379> GET x
(nil)

Discovery credit: @gilad-redis

Comment From: soloestoy

Maybe we need add a new flag to check if command has subcommand.

Comment From: antirez

Hello @itamarhaber, I can't see how this is a bug. Subcommands ACLs are a pattern matching feature, so basically, if you tell Redis that "GET X" is a denied set of arguments, it will refuse it. This has several advantages:

  1. It is future proof, we don't have to touch any code to allow ACLs to work with future versions of Redis.
  2. It allows to work with modules without Redis to understand how a module implements a given command.
  3. When implementing a new subcommand, we don't have to touch any ACL related code nor the command table.

The true disadvantage of this approach has nothing to do with what was described in this issue, but is instead that: if you mistype a command, the ACL will be registered without the user noticing that there is a problem. However this is the tradeoff in order to get many other advantages.

Comment From: antirez

Pinging @gilad-redis as well in case he wants to add some background / thought here.

Comment From: madolson

@itamarhaber Do you still think this is worth doing something about? It seems too minor to try to break compatibility for.

Comment From: itamarhaber

Nah, thanks for the bump, closing,