diff --git a/src/acl.c b/src/acl.c index 8257b59b1..782254eb9 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1766,7 +1766,11 @@ int ACLUserCheckChannelPerm(user *u, sds channel, int is_pattern) { return ACL_DENIED_CHANNEL; } -/* Lower level API that checks if a specified user is able to execute a given command. */ +/* Lower level API that checks if a specified user is able to execute a given command. + * + * If the command fails an ACL check, idxptr will be to set to the first argv entry that + * causes the failure, either 0 if the command itself fails or the idx of the key/channel + * that causes the failure */ int ACLCheckAllUserCommandPerm(user *u, struct redisCommand *cmd, robj **argv, int argc, int *idxptr) { listIter li; listNode *ln; @@ -2581,18 +2585,25 @@ void addACLLogEntry(client *c, int reason, int context, int argpos, sds username } } -const char* getAclErrorMessage(int acl_res) { - /* Notice that a variant of this code also exists on aclCommand so - * it also need to be updated on changed. */ +sds getAclErrorMessage(int acl_res, user *user, struct redisCommand *cmd, sds errored_val, int verbose) { switch (acl_res) { case ACL_DENIED_CMD: - return "can't run this command or subcommand"; + return sdscatfmt(sdsempty(), "User %S has no permissions to run " + "the '%S' command", user->name, cmd->fullname); case ACL_DENIED_KEY: - return "can't access at least one of the keys mentioned in the command arguments"; + if (verbose) { + return sdscatfmt(sdsempty(), "User %S has no permissions to access " + "the '%S' key", user->name, errored_val); + } else { + return sdsnew("No permissions to access a key"); + } case ACL_DENIED_CHANNEL: - return "can't publish to the channel mentioned in the command"; - default: - return "lacking the permissions for the command"; + if (verbose) { + return sdscatfmt(sdsempty(), "User %S has no permissions to access " + "the '%S' channel", user->name, errored_val); + } else { + return sdsnew("No permissions to access a channel"); + } } serverPanic("Reached deadcode on getAclErrorMessage"); } @@ -2956,22 +2967,8 @@ void aclCommand(client *c) { int idx; int result = ACLCheckAllUserCommandPerm(u, cmd, c->argv + 3, c->argc - 3, &idx); - /* Notice that a variant of this code also exists on getAclErrorMessage so - * it also need to be updated on changed. */ if (result != ACL_OK) { - sds err = sdsempty(); - if (result == ACL_DENIED_CMD) { - err = sdscatfmt(err, "This user has no permissions to run " - "the '%s' command", cmd->fullname); - } else if (result == ACL_DENIED_KEY) { - err = sdscatfmt(err, "This user has no permissions to access " - "the '%s' key", c->argv[idx + 3]->ptr); - } else if (result == ACL_DENIED_CHANNEL) { - err = sdscatfmt(err, "This user has no permissions to access " - "the '%s' channel", c->argv[idx + 3]->ptr); - } else { - serverPanic("Invalid permission result"); - } + sds err = getAclErrorMessage(result, u, cmd, c->argv[idx+3]->ptr, 1); addReplyBulkSds(c, err); return; } diff --git a/src/module.c b/src/module.c index 6d9425baf..11b47d99f 100644 --- a/src/module.c +++ b/src/module.c @@ -6003,7 +6003,10 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch sds object = (acl_retval == ACL_DENIED_CMD) ? sdsdup(c->cmd->fullname) : sdsdup(c->argv[acl_errpos]->ptr); addACLLogEntry(ctx->client, acl_retval, ACL_LOG_CTX_MODULE, -1, ctx->client->user->name, object); if (error_as_call_replies) { - sds msg = sdscatfmt(sdsempty(), "acl verification failed, %s.", getAclErrorMessage(acl_retval)); + /* verbosity should be same as processCommand() in server.c */ + sds acl_msg = getAclErrorMessage(acl_retval, ctx->client->user, c->cmd, c->argv[acl_errpos]->ptr, 0); + sds msg = sdscatfmt(sdsempty(), "-NOPERM %S\r\n", acl_msg); + sdsfree(acl_msg); reply = callReplyCreateError(msg, ctx); } errno = EACCES; diff --git a/src/script.c b/src/script.c index 12d87322d..3d9785312 100644 --- a/src/script.c +++ b/src/script.c @@ -335,7 +335,9 @@ static int scriptVerifyACL(client *c, sds *err) { int acl_retval = ACLCheckAllPerm(c, &acl_errpos); if (acl_retval != ACL_OK) { addACLLogEntry(c,acl_retval,ACL_LOG_CTX_LUA,acl_errpos,NULL,NULL); - *err = sdscatfmt(sdsempty(), "The user executing the script %s", getAclErrorMessage(acl_retval)); + sds msg = getAclErrorMessage(acl_retval, c->user, c->cmd, c->argv[acl_errpos]->ptr, 0); + *err = sdscatsds(sdsnew("ACL failure in script: "), msg); + sdsfree(msg); return C_ERR; } return C_OK; diff --git a/src/server.c b/src/server.c index 2c65aac66..4962b4fba 100644 --- a/src/server.c +++ b/src/server.c @@ -3752,28 +3752,9 @@ int processCommand(client *c) { int acl_retval = ACLCheckAllPerm(c,&acl_errpos); if (acl_retval != ACL_OK) { addACLLogEntry(c,acl_retval,(c->flags & CLIENT_MULTI) ? ACL_LOG_CTX_MULTI : ACL_LOG_CTX_TOPLEVEL,acl_errpos,NULL,NULL); - switch (acl_retval) { - case ACL_DENIED_CMD: - { - rejectCommandFormat(c, - "-NOPERM this user has no permissions to run " - "the '%s' command", c->cmd->fullname); - break; - } - case ACL_DENIED_KEY: - rejectCommandFormat(c, - "-NOPERM this user has no permissions to access " - "one of the keys used as arguments"); - break; - case ACL_DENIED_CHANNEL: - rejectCommandFormat(c, - "-NOPERM this user has no permissions to access " - "one of the channels used as arguments"); - break; - default: - rejectCommandFormat(c, "no permission"); - break; - } + sds msg = getAclErrorMessage(acl_retval, c->user, c->cmd, c->argv[acl_errpos]->ptr, 0); + rejectCommandFormat(c, "-NOPERM %s", msg); + sdsfree(msg); return C_OK; } diff --git a/src/server.h b/src/server.h index 2ec7dc7ea..4d6eb47ba 100644 --- a/src/server.h +++ b/src/server.h @@ -2805,7 +2805,7 @@ void addReplyCommandCategories(client *c, struct redisCommand *cmd); user *ACLCreateUnlinkedUser(); void ACLFreeUserAndKillClients(user *u); void addACLLogEntry(client *c, int reason, int context, int argpos, sds username, sds object); -const char* getAclErrorMessage(int acl_res); +sds getAclErrorMessage(int acl_res, user *user, struct redisCommand *cmd, sds errored_val, int verbose); void ACLUpdateDefaultUserPassword(sds password); sds genRedisInfoStringACLStats(sds info); diff --git a/tests/unit/acl-v2.tcl b/tests/unit/acl-v2.tcl index d836f9cb8..af23d745e 100644 --- a/tests/unit/acl-v2.tcl +++ b/tests/unit/acl-v2.tcl @@ -15,9 +15,9 @@ start_server {tags {"acl external:skip"}} { assert_equal "OK" [$r2 set write::foo bar] assert_equal "" [$r2 get read::foo] catch {$r2 get write::foo} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err catch {$r2 set read::foo bar} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err } test {Test ACL selectors by default have no permissions} { @@ -80,9 +80,9 @@ start_server {tags {"acl external:skip"}} { r set readstr bar assert_equal bar [$r2 get readstr] catch {$r2 set readstr bar} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err catch {$r2 get notread} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err } test {Test separate write permission} { @@ -92,9 +92,9 @@ start_server {tags {"acl external:skip"}} { # Note, SET is a RW command, so it's not used for testing $r2 LPUSH writelist 10 catch {$r2 GET writestr} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err catch {$r2 LPUSH notwrite 10} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err } test {Test separate read and write permissions} { @@ -104,7 +104,7 @@ start_server {tags {"acl external:skip"}} { r set read bar $r2 copy read write catch {$r2 copy write read} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err } test {Test separate read and write permissions on different selectors are not additive} { @@ -115,23 +115,23 @@ start_server {tags {"acl external:skip"}} { # Verify write selector $r2 LPUSH writelist 10 catch {$r2 GET writestr} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err catch {$r2 LPUSH notwrite 10} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err # Verify read selector r set readstr bar assert_equal bar [$r2 get readstr] catch {$r2 set readstr bar} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err catch {$r2 get notread} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err # Verify they don't combine catch {$r2 copy read write} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err catch {$r2 copy write read} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err } test {Test SET with separate read permission} { @@ -142,10 +142,10 @@ start_server {tags {"acl external:skip"}} { assert_equal {} [$r2 get readstr] # We don't have the permission to WRITE key. - assert_error {*NOPERM*keys*} {$r2 set readstr bar} - assert_error {*NOPERM*keys*} {$r2 set readstr bar get} - assert_error {*NOPERM*keys*} {$r2 set readstr bar ex 100} - assert_error {*NOPERM*keys*} {$r2 set readstr bar keepttl nx} + assert_error {*NOPERM*key*} {$r2 set readstr bar} + assert_error {*NOPERM*key*} {$r2 set readstr bar get} + assert_error {*NOPERM*key*} {$r2 set readstr bar ex 100} + assert_error {*NOPERM*key*} {$r2 set readstr bar keepttl nx} } test {Test SET with separate write permission} { @@ -157,13 +157,13 @@ start_server {tags {"acl external:skip"}} { assert_equal {OK} [$r2 set writestr get] # We don't have the permission to READ key. - assert_error {*NOPERM*keys*} {$r2 set get writestr} - assert_error {*NOPERM*keys*} {$r2 set writestr bar get} - assert_error {*NOPERM*keys*} {$r2 set writestr bar get ex 100} - assert_error {*NOPERM*keys*} {$r2 set writestr bar get keepttl nx} + assert_error {*NOPERM*key*} {$r2 set get writestr} + assert_error {*NOPERM*key*} {$r2 set writestr bar get} + assert_error {*NOPERM*key*} {$r2 set writestr bar get ex 100} + assert_error {*NOPERM*key*} {$r2 set writestr bar get keepttl nx} # this probably should be `ERR value is not an integer or out of range` - assert_error {*NOPERM*keys*} {$r2 set writestr bar ex get} + assert_error {*NOPERM*key*} {$r2 set writestr bar ex get} } test {Test SET with read and write permissions} { @@ -194,9 +194,9 @@ start_server {tags {"acl external:skip"}} { assert_equal {0} [$r2 bitfield readstr get u4 0] # We don't have the permission to WRITE key. - assert_error {*NOPERM*keys*} {$r2 bitfield readstr set u4 0 1} - assert_error {*NOPERM*keys*} {$r2 bitfield readstr get u4 0 set u4 0 1} - assert_error {*NOPERM*keys*} {$r2 bitfield readstr incrby u4 0 1} + assert_error {*NOPERM*key*} {$r2 bitfield readstr set u4 0 1} + assert_error {*NOPERM*key*} {$r2 bitfield readstr get u4 0 set u4 0 1} + assert_error {*NOPERM*key*} {$r2 bitfield readstr incrby u4 0 1} } test {Test BITFIELD with separate write permission} { @@ -206,9 +206,9 @@ start_server {tags {"acl external:skip"}} { assert_equal PONG [$r2 PING] # We don't have the permission to READ key. - assert_error {*NOPERM*keys*} {$r2 bitfield writestr get u4 0} - assert_error {*NOPERM*keys*} {$r2 bitfield writestr set u4 0 1} - assert_error {*NOPERM*keys*} {$r2 bitfield writestr incrby u4 0 1} + assert_error {*NOPERM*key*} {$r2 bitfield writestr get u4 0} + assert_error {*NOPERM*key*} {$r2 bitfield writestr set u4 0 1} + assert_error {*NOPERM*key*} {$r2 bitfield writestr incrby u4 0 1} } test {Test BITFIELD with read and write permissions} { @@ -242,7 +242,7 @@ start_server {tags {"acl external:skip"}} { # second selector. We should still show the logically first unmatched key. r ACL LOG RESET catch {$r2 MGET otherkey someotherkey} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err set entry [lindex [r ACL LOG] 0] assert_equal [dict get $entry username] "acl-log-test-selector" assert_equal [dict get $entry context] "toplevel" @@ -251,7 +251,7 @@ start_server {tags {"acl external:skip"}} { r ACL LOG RESET catch {$r2 MGET key otherkey someotherkey} err - assert_match "*NOPERM*keys*" $err + assert_match "*NOPERM*key*" $err set entry [lindex [r ACL LOG] 0] assert_equal [dict get $entry username] "acl-log-test-selector" assert_equal [dict get $entry context] "toplevel" @@ -308,8 +308,8 @@ start_server {tags {"acl external:skip"}} { test {Test various commands for command permissions} { r ACL setuser command-test -@all - assert_equal "This user has no permissions to run the 'set' command" [r ACL DRYRUN command-test set somekey somevalue] - assert_equal "This user has no permissions to run the 'get' command" [r ACL DRYRUN command-test get somekey] + assert_match {*has no permissions to run the 'set' command*} [r ACL DRYRUN command-test set somekey somevalue] + assert_match {*has no permissions to run the 'get' command*} [r ACL DRYRUN command-test get somekey] } test {Test various odd commands for key permissions} { @@ -317,29 +317,29 @@ start_server {tags {"acl external:skip"}} { # Test migrate, which is marked with incomplete keys assert_equal "OK" [r ACL DRYRUN command-test MIGRATE whatever whatever rw 0 500] - assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN command-test MIGRATE whatever whatever read 0 500] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN command-test MIGRATE whatever whatever write 0 500] + assert_match {*has no permissions to access the 'read' key*} [r ACL DRYRUN command-test MIGRATE whatever whatever read 0 500] + assert_match {*has no permissions to access the 'write' key*} [r ACL DRYRUN command-test MIGRATE whatever whatever write 0 500] assert_equal "OK" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 KEYS rw] - assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 KEYS read] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 KEYS write] + assert_match "*has no permissions to access the 'read' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 KEYS read] + assert_match "*has no permissions to access the 'write' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 KEYS write] assert_equal "OK" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH KEYS KEYS rw] - assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH KEYS KEYS read] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH KEYS KEYS write] + assert_match "*has no permissions to access the 'read' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH KEYS KEYS read] + assert_match "*has no permissions to access the 'write' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH KEYS KEYS write] assert_equal "OK" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH2 KEYS 123 KEYS rw] - assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH2 KEYS 123 KEYS read] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH2 KEYS 123 KEYS write] + assert_match "*has no permissions to access the 'read' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH2 KEYS 123 KEYS read] + assert_match "*has no permissions to access the 'write' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH2 KEYS 123 KEYS write] assert_equal "OK" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH2 USER KEYS KEYS rw] - assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH2 USER KEYS KEYS read] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH2 USER KEYS KEYS write] + assert_match "*has no permissions to access the 'read' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH2 USER KEYS KEYS read] + assert_match "*has no permissions to access the 'write' key" [r ACL DRYRUN command-test MIGRATE whatever whatever "" 0 5000 AUTH2 USER KEYS KEYS write] # Test SORT, which is marked with incomplete keys assert_equal "OK" [r ACL DRYRUN command-test SORT read STORE write] - assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN command-test SORT read STORE read] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN command-test SORT write STORE write] + assert_match {*has no permissions to access the 'read' key*} [r ACL DRYRUN command-test SORT read STORE read] + assert_match {*has no permissions to access the 'write' key*} [r ACL DRYRUN command-test SORT write STORE write] # Test EVAL, which uses the numkey keyspec (Also test EVAL_RO) assert_equal "OK" [r ACL DRYRUN command-test EVAL "" 1 rw1] - assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN command-test EVAL "" 1 read] + assert_match {*has no permissions to access the 'read' key*} [r ACL DRYRUN command-test EVAL "" 1 read] assert_equal "OK" [r ACL DRYRUN command-test EVAL_RO "" 1 rw1] assert_equal "OK" [r ACL DRYRUN command-test EVAL_RO "" 1 read] @@ -354,12 +354,12 @@ start_server {tags {"acl external:skip"}} { # Test GEORADIUS which uses the last type of keyspec, keyword assert_equal "OK" [r ACL DRYRUN command-test GEORADIUS read longitude latitude radius M STOREDIST write] assert_equal "OK" [r ACL DRYRUN command-test GEORADIUS read longitude latitude radius M] - assert_equal "This user has no permissions to access the 'read2' key" [r ACL DRYRUN command-test GEORADIUS read1 longitude latitude radius M STOREDIST read2] - assert_equal "This user has no permissions to access the 'write1' key" [r ACL DRYRUN command-test GEORADIUS write1 longitude latitude radius M STOREDIST write2] + assert_match {*has no permissions to access the 'read2' key*} [r ACL DRYRUN command-test GEORADIUS read1 longitude latitude radius M STOREDIST read2] + assert_match {*has no permissions to access the 'write1' key*} [r ACL DRYRUN command-test GEORADIUS write1 longitude latitude radius M STOREDIST write2] assert_equal "OK" [r ACL DRYRUN command-test GEORADIUS read longitude latitude radius M STORE write] assert_equal "OK" [r ACL DRYRUN command-test GEORADIUS read longitude latitude radius M] - assert_equal "This user has no permissions to access the 'read2' key" [r ACL DRYRUN command-test GEORADIUS read1 longitude latitude radius M STORE read2] - assert_equal "This user has no permissions to access the 'write1' key" [r ACL DRYRUN command-test GEORADIUS write1 longitude latitude radius M STORE write2] + assert_match {*has no permissions to access the 'read2' key*} [r ACL DRYRUN command-test GEORADIUS read1 longitude latitude radius M STORE read2] + assert_match {*has no permissions to access the 'write1' key*} [r ACL DRYRUN command-test GEORADIUS write1 longitude latitude radius M STORE write2] } # Existence test commands are not marked as access since they are the result @@ -368,15 +368,15 @@ start_server {tags {"acl external:skip"}} { test {Existence test commands are not marked as access} { assert_equal "OK" [r ACL DRYRUN command-test HEXISTS read foo] assert_equal "OK" [r ACL DRYRUN command-test HEXISTS write foo] - assert_equal "This user has no permissions to access the 'nothing' key" [r ACL DRYRUN command-test HEXISTS nothing foo] + assert_match {*has no permissions to access the 'nothing' key*} [r ACL DRYRUN command-test HEXISTS nothing foo] assert_equal "OK" [r ACL DRYRUN command-test HSTRLEN read foo] assert_equal "OK" [r ACL DRYRUN command-test HSTRLEN write foo] - assert_equal "This user has no permissions to access the 'nothing' key" [r ACL DRYRUN command-test HSTRLEN nothing foo] + assert_match {*has no permissions to access the 'nothing' key*} [r ACL DRYRUN command-test HSTRLEN nothing foo] assert_equal "OK" [r ACL DRYRUN command-test SISMEMBER read foo] assert_equal "OK" [r ACL DRYRUN command-test SISMEMBER write foo] - assert_equal "This user has no permissions to access the 'nothing' key" [r ACL DRYRUN command-test SISMEMBER nothing foo] + assert_match {*has no permissions to access the 'nothing' key*} [r ACL DRYRUN command-test SISMEMBER nothing foo] } # Unlike existence test commands, intersection cardinality commands process the data @@ -384,42 +384,42 @@ start_server {tags {"acl external:skip"}} { # requirement. test {Intersection cardinaltiy commands are access commands} { assert_equal "OK" [r ACL DRYRUN command-test SINTERCARD 2 read read] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN command-test SINTERCARD 2 write read] - assert_equal "This user has no permissions to access the 'nothing' key" [r ACL DRYRUN command-test SINTERCARD 2 nothing read] + assert_match {*has no permissions to access the 'write' key*} [r ACL DRYRUN command-test SINTERCARD 2 write read] + assert_match {*has no permissions to access the 'nothing' key*} [r ACL DRYRUN command-test SINTERCARD 2 nothing read] assert_equal "OK" [r ACL DRYRUN command-test ZCOUNT read 0 1] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN command-test ZCOUNT write 0 1] - assert_equal "This user has no permissions to access the 'nothing' key" [r ACL DRYRUN command-test ZCOUNT nothing 0 1] + assert_match {*has no permissions to access the 'write' key*} [r ACL DRYRUN command-test ZCOUNT write 0 1] + assert_match {*has no permissions to access the 'nothing' key*} [r ACL DRYRUN command-test ZCOUNT nothing 0 1] assert_equal "OK" [r ACL DRYRUN command-test PFCOUNT read read] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN command-test PFCOUNT write read] - assert_equal "This user has no permissions to access the 'nothing' key" [r ACL DRYRUN command-test PFCOUNT nothing read] + assert_match {*has no permissions to access the 'write' key*} [r ACL DRYRUN command-test PFCOUNT write read] + assert_match {*has no permissions to access the 'nothing' key*} [r ACL DRYRUN command-test PFCOUNT nothing read] assert_equal "OK" [r ACL DRYRUN command-test ZINTERCARD 2 read read] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN command-test ZINTERCARD 2 write read] - assert_equal "This user has no permissions to access the 'nothing' key" [r ACL DRYRUN command-test ZINTERCARD 2 nothing read] + assert_match {*has no permissions to access the 'write' key*} [r ACL DRYRUN command-test ZINTERCARD 2 write read] + assert_match {*has no permissions to access the 'nothing' key*} [r ACL DRYRUN command-test ZINTERCARD 2 nothing read] } test {Test general keyspace commands require some type of permission to execute} { assert_equal "OK" [r ACL DRYRUN command-test touch read] assert_equal "OK" [r ACL DRYRUN command-test touch write] assert_equal "OK" [r ACL DRYRUN command-test touch rw] - assert_equal "This user has no permissions to access the 'nothing' key" [r ACL DRYRUN command-test touch nothing] + assert_match {*has no permissions to access the 'nothing' key*} [r ACL DRYRUN command-test touch nothing] assert_equal "OK" [r ACL DRYRUN command-test exists read] assert_equal "OK" [r ACL DRYRUN command-test exists write] assert_equal "OK" [r ACL DRYRUN command-test exists rw] - assert_equal "This user has no permissions to access the 'nothing' key" [r ACL DRYRUN command-test exists nothing] + assert_match {*has no permissions to access the 'nothing' key*} [r ACL DRYRUN command-test exists nothing] assert_equal "OK" [r ACL DRYRUN command-test MEMORY USAGE read] assert_equal "OK" [r ACL DRYRUN command-test MEMORY USAGE write] assert_equal "OK" [r ACL DRYRUN command-test MEMORY USAGE rw] - assert_equal "This user has no permissions to access the 'nothing' key" [r ACL DRYRUN command-test MEMORY USAGE nothing] + assert_match {*has no permissions to access the 'nothing' key*} [r ACL DRYRUN command-test MEMORY USAGE nothing] assert_equal "OK" [r ACL DRYRUN command-test TYPE read] assert_equal "OK" [r ACL DRYRUN command-test TYPE write] assert_equal "OK" [r ACL DRYRUN command-test TYPE rw] - assert_equal "This user has no permissions to access the 'nothing' key" [r ACL DRYRUN command-test TYPE nothing] + assert_match {*has no permissions to access the 'nothing' key*} [r ACL DRYRUN command-test TYPE nothing] } test {Cardinality commands require some type of permission to execute} { @@ -428,7 +428,7 @@ start_server {tags {"acl external:skip"}} { assert_equal "OK" [r ACL DRYRUN command-test $command read] assert_equal "OK" [r ACL DRYRUN command-test $command write] assert_equal "OK" [r ACL DRYRUN command-test $command rw] - assert_equal "This user has no permissions to access the 'nothing' key" [r ACL DRYRUN command-test $command nothing] + assert_match {*has no permissions to access the 'nothing' key*} [r ACL DRYRUN command-test $command nothing] } } @@ -440,8 +440,8 @@ start_server {tags {"acl external:skip"}} { assert_equal "OK" [r ACL DRYRUN test-channels sunsubscribe channel] assert_equal "OK" [r ACL DRYRUN test-channels sunsubscribe otherchannel] - assert_equal "This user has no permissions to access the 'otherchannel' channel" [r ACL DRYRUN test-channels spublish otherchannel foo] - assert_equal "This user has no permissions to access the 'otherchannel' channel" [r ACL DRYRUN test-channels ssubscribe otherchannel foo] + assert_match {*has no permissions to access the 'otherchannel' channel*} [r ACL DRYRUN test-channels spublish otherchannel foo] + assert_match {*has no permissions to access the 'otherchannel' channel*} [r ACL DRYRUN test-channels ssubscribe otherchannel foo] } test {Test sort with ACL permissions} { diff --git a/tests/unit/acl.tcl b/tests/unit/acl.tcl index 9160b641a..c23f36890 100644 --- a/tests/unit/acl.tcl +++ b/tests/unit/acl.tcl @@ -93,7 +93,7 @@ start_server {tags {"acl external:skip"}} { r AUTH psuser pspass catch {r PUBLISH foo bar} e set e - } {*NOPERM*channels*} + } {*NOPERM*channel*} test {By default, only default user is not able to publish to any shard channel} { r AUTH default pwd @@ -101,7 +101,7 @@ start_server {tags {"acl external:skip"}} { r AUTH psuser pspass catch {r SPUBLISH foo bar} e set e - } {*NOPERM*channels*} + } {*NOPERM*channel*} test {By default, only default user is able to subscribe to any channel} { set rd [redis_deferring_client] @@ -117,7 +117,7 @@ start_server {tags {"acl external:skip"}} { catch {$rd read} e $rd close set e - } {*NOPERM*channels*} + } {*NOPERM*channel*} test {By default, only default user is able to subscribe to any shard channel} { set rd [redis_deferring_client] @@ -133,7 +133,7 @@ start_server {tags {"acl external:skip"}} { catch {$rd read} e $rd close set e - } {*NOPERM*channels*} + } {*NOPERM*channel*} test {By default, only default user is able to subscribe to any pattern} { set rd [redis_deferring_client] @@ -149,7 +149,7 @@ start_server {tags {"acl external:skip"}} { catch {$rd read} e $rd close set e - } {*NOPERM*channels*} + } {*NOPERM*channel*} test {It's possible to allow publishing to a subset of channels} { r ACL setuser psuser resetchannels &foo:1 &bar:* @@ -554,9 +554,9 @@ start_server {tags {"acl external:skip"}} { test "ACL requires explicit permission for scripting for EVAL_RO, EVALSHA_RO and FCALL_RO" { r ACL SETUSER scripter on nopass +readonly - assert_equal "This user has no permissions to run the 'eval_ro' command" [r ACL DRYRUN scripter EVAL_RO "" 0] - assert_equal "This user has no permissions to run the 'evalsha_ro' command" [r ACL DRYRUN scripter EVALSHA_RO "" 0] - assert_equal "This user has no permissions to run the 'fcall_ro' command" [r ACL DRYRUN scripter FCALL_RO "" 0] + assert_match {*has no permissions to run the 'eval_ro' command*} [r ACL DRYRUN scripter EVAL_RO "" 0] + assert_match {*has no permissions to run the 'evalsha_ro' command*} [r ACL DRYRUN scripter EVALSHA_RO "" 0] + assert_match {*has no permissions to run the 'fcall_ro' command*} [r ACL DRYRUN scripter FCALL_RO "" 0] } test {ACL #5998 regression: memory leaks adding / removing subcommands} { @@ -793,7 +793,7 @@ start_server {tags {"acl external:skip"}} { set current_invalid_channel_accesses [s acl_access_denied_channel] r ACL setuser invalidkeyuser on >passwd resetkeys allcommands r AUTH invalidkeyuser passwd - assert_error "*no permissions to access one of the keys*" {r get x} + assert_error "*NOPERM*key*" {r get x} r AUTH default "" assert {[s acl_access_denied_auth] eq $current_auth_failures} assert {[s acl_access_denied_cmd] eq $current_invalid_cmd_accesses} @@ -809,7 +809,7 @@ start_server {tags {"acl external:skip"}} { set current_invalid_channel_accesses [s acl_access_denied_channel] r ACL setuser invalidchanneluser on >passwd resetchannels allcommands r AUTH invalidkeyuser passwd - assert_error "*no permissions to access one of the channels*" {r subscribe x} + assert_error "*NOPERM*channel*" {r subscribe x} r AUTH default "" assert {[s acl_access_denied_auth] eq $current_auth_failures} assert {[s acl_access_denied_cmd] eq $current_invalid_cmd_accesses} diff --git a/tests/unit/moduleapi/aclcheck.tcl b/tests/unit/moduleapi/aclcheck.tcl index d96ea89cf..5dae6e1ad 100644 --- a/tests/unit/moduleapi/aclcheck.tcl +++ b/tests/unit/moduleapi/aclcheck.tcl @@ -63,13 +63,14 @@ start_server {tags {"modules acl"}} { # rm call check for key permission (y: only WRITE) assert_equal [r aclcheck.rm_call set y 5] OK assert_error {*NOPERM*} {r aclcheck.rm_call set y 5 get} - assert_error {ERR acl verification failed, can't access at least one of the keys mentioned in the command arguments.} {r aclcheck.rm_call_with_errors set y 5 get} + assert_error {*NOPERM*No permissions to access a key*} {r aclcheck.rm_call_with_errors set y 5 get} # rm call check for key permission (z: only READ) assert_error {*NOPERM*} {r aclcheck.rm_call set z 5} - assert_error {ERR acl verification failed, can't access at least one of the keys mentioned in the command arguments.} {r aclcheck.rm_call_with_errors set z 5} + catch {r aclcheck.rm_call_with_errors set z 5} e + assert_match {*NOPERM*No permissions to access a key*} $e assert_error {*NOPERM*} {r aclcheck.rm_call set z 6 get} - assert_error {ERR acl verification failed, can't access at least one of the keys mentioned in the command arguments.} {r aclcheck.rm_call_with_errors set z 6 get} + assert_error {*NOPERM*No permissions to access a key*} {r aclcheck.rm_call_with_errors set z 6 get} # verify that new log entry added set entry [lindex [r ACL LOG] 0] @@ -80,10 +81,8 @@ start_server {tags {"modules acl"}} { # rm call check for command permission r acl setuser default -set - catch {r aclcheck.rm_call set x 5} e - assert_match {*NOPERM*} $e - catch {r aclcheck.rm_call_with_errors set x 5} e - assert_match {ERR acl verification failed, can't run this command or subcommand.} $e + assert_error {*NOPERM*} {r aclcheck.rm_call set x 5} + assert_error {*NOPERM*has no permissions to run the 'set' command*} {r aclcheck.rm_call_with_errors set x 5} # verify that new log entry added set entry [lindex [r ACL LOG] 0] diff --git a/tests/unit/moduleapi/getkeys.tcl b/tests/unit/moduleapi/getkeys.tcl index 734c55fa2..b84bb0f73 100644 --- a/tests/unit/moduleapi/getkeys.tcl +++ b/tests/unit/moduleapi/getkeys.tcl @@ -64,14 +64,14 @@ start_server {tags {"modules"}} { test "module getkeys-api - ACL" { # legacy triple didn't provide flags, so they require both read and write assert_equal "OK" [r ACL DRYRUN testuser getkeys.command key rw] - assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN testuser getkeys.command key read] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN testuser getkeys.command key write] + assert_match {*has no permissions to access the 'read' key*} [r ACL DRYRUN testuser getkeys.command key read] + assert_match {*has no permissions to access the 'write' key*} [r ACL DRYRUN testuser getkeys.command key write] } test "module getkeys-api with flags - ACL" { assert_equal "OK" [r ACL DRYRUN testuser getkeys.command_with_flags key rw] assert_equal "OK" [r ACL DRYRUN testuser getkeys.command_with_flags key read] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN testuser getkeys.command_with_flags key write] + assert_match {*has no permissions to access the 'write' key*} [r ACL DRYRUN testuser getkeys.command_with_flags key write] } test "Unload the module - getkeys" { diff --git a/tests/unit/moduleapi/keyspecs.tcl b/tests/unit/moduleapi/keyspecs.tcl index 8491bc19e..9e68e9732 100644 --- a/tests/unit/moduleapi/keyspecs.tcl +++ b/tests/unit/moduleapi/keyspecs.tcl @@ -129,15 +129,15 @@ start_server {tags {"modules"}} { test "Module key specs: No spec, only legacy triple - ACL" { # legacy triple didn't provide flags, so they require both read and write assert_equal "OK" [r ACL DRYRUN testuser kspec.none rw val1] - assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN testuser kspec.none read val1] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN testuser kspec.none write val1] + assert_match {*has no permissions to access the 'read' key*} [r ACL DRYRUN testuser kspec.none read val1] + assert_match {*has no permissions to access the 'write' key*} [r ACL DRYRUN testuser kspec.none write val1] } test "Module key specs: tworanges - ACL" { assert_equal "OK" [r ACL DRYRUN testuser kspec.tworanges read write] assert_equal "OK" [r ACL DRYRUN testuser kspec.tworanges rw rw] - assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN testuser kspec.tworanges rw read] - assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN testuser kspec.tworanges write rw] + assert_match {*has no permissions to access the 'read' key*} [r ACL DRYRUN testuser kspec.tworanges rw read] + assert_match {*has no permissions to access the 'write' key*} [r ACL DRYRUN testuser kspec.tworanges write rw] } foreach cmd {kspec.none kspec.tworanges} { diff --git a/tests/unit/moduleapi/misc.tcl b/tests/unit/moduleapi/misc.tcl index 8aa5b9198..15e4f4c6c 100644 --- a/tests/unit/moduleapi/misc.tcl +++ b/tests/unit/moduleapi/misc.tcl @@ -444,7 +444,7 @@ start_server {tags {"modules"}} { r acl setuser default resetkeys catch {r test.rm_call_flags DC set x 10} e - assert_match {*ERR acl verification failed, can't access at least one of the keys*} $e + assert_match {*NOPERM No permissions to access a key*} $e r acl setuser default +@all ~* assert_equal [r get x] $x } @@ -452,4 +452,4 @@ start_server {tags {"modules"}} { test "Unload the module - misc" { assert_equal {OK} [r module unload misc] } -} \ No newline at end of file +} diff --git a/tests/unit/moduleapi/usercall.tcl b/tests/unit/moduleapi/usercall.tcl index 04f730005..c3d8b0312 100644 --- a/tests/unit/moduleapi/usercall.tcl +++ b/tests/unit/moduleapi/usercall.tcl @@ -45,8 +45,7 @@ start_server {tags {"modules usercall"}} { assert_equal [r usercall.get_acl] "off sanitize-payload ~* &* +@all -set" # fails here as testing acl in rm call - catch {r usercall.call_with_user_flag C set x 10} e - assert_match {*ERR acl verification failed*} $e + assert_error {*NOPERM User default has no permissions*} {r usercall.call_with_user_flag C set x 10} assert_equal [r usercall.call_with_user_flag C get x] 5 @@ -88,7 +87,7 @@ start_server {tags {"modules usercall"}} { # fails here in script, as rm_call will permit the eval call catch {r usercall.call_with_user_flag C evalsha $sha_set 0} e - assert_match {*ERR The user executing the script can't run this command or subcommand script*} $e + assert_match {*ERR ACL failure in script*} $e assert_equal [r usercall.call_with_user_flag C evalsha $sha_get 0] 1 }