Unverified Commit 20c33fe6 authored by Binbin's avatar Binbin Committed by GitHub
Browse files

Show subcommand full name in error log / ACL LOG (#10105)

Use `getFullCommandName` to get the full name of the command.
It can also get the full name of the subcommand, like "script|help".

Before:
```
> SCRIPT HELP
(error) NOPERM this user has no permissions to run the 'help' command or its subcommand

> ACL LOG
    7) "object"
    8) "help"
```

After:
```
> SCRIPT HELP
(error) NOPERM this user has no permissions to run the 'script|help' command

> ACL LOG
    7) "object"
    8) "script|help"
```

Fix #10094
parent e22146b0
...@@ -1904,7 +1904,7 @@ void addACLLogEntry(client *c, int reason, int context, int argpos, sds username ...@@ -1904,7 +1904,7 @@ void addACLLogEntry(client *c, int reason, int context, int argpos, sds username
le->object = object; le->object = object;
} else { } else {
switch(reason) { switch(reason) {
case ACL_DENIED_CMD: le->object = sdsnew(c->cmd->name); break; case ACL_DENIED_CMD: le->object = getFullCommandName(c->cmd); break;
case ACL_DENIED_KEY: le->object = sdsdup(c->argv[argpos]->ptr); break; case ACL_DENIED_KEY: le->object = sdsdup(c->argv[argpos]->ptr); break;
case ACL_DENIED_CHANNEL: le->object = sdsdup(c->argv[argpos]->ptr); break; case ACL_DENIED_CHANNEL: le->object = sdsdup(c->argv[argpos]->ptr); break;
case ACL_DENIED_AUTH: le->object = sdsdup(c->argv[0]->ptr); break; case ACL_DENIED_AUTH: le->object = sdsdup(c->argv[0]->ptr); break;
......
...@@ -3439,10 +3439,14 @@ int processCommand(client *c) { ...@@ -3439,10 +3439,14 @@ int processCommand(client *c) {
addACLLogEntry(c,acl_retval,(c->flags & CLIENT_MULTI) ? ACL_LOG_CTX_MULTI : ACL_LOG_CTX_TOPLEVEL,acl_errpos,NULL,NULL); addACLLogEntry(c,acl_retval,(c->flags & CLIENT_MULTI) ? ACL_LOG_CTX_MULTI : ACL_LOG_CTX_TOPLEVEL,acl_errpos,NULL,NULL);
switch (acl_retval) { switch (acl_retval) {
case ACL_DENIED_CMD: case ACL_DENIED_CMD:
{
sds cmdname = getFullCommandName(c->cmd);
rejectCommandFormat(c, rejectCommandFormat(c,
"-NOPERM this user has no permissions to run " "-NOPERM this user has no permissions to run "
"the '%s' command or its subcommand", c->cmd->name); "the '%s' command", cmdname);
sdsfree(cmdname);
break; break;
}
case ACL_DENIED_KEY: case ACL_DENIED_KEY:
rejectCommandFormat(c, rejectCommandFormat(c,
"-NOPERM this user has no permissions to access " "-NOPERM this user has no permissions to access "
......
...@@ -64,7 +64,7 @@ start_server {tags {"acl external:skip"}} { ...@@ -64,7 +64,7 @@ start_server {tags {"acl external:skip"}} {
test {By default users are not able to access any command} { test {By default users are not able to access any command} {
catch {r SET foo bar} e catch {r SET foo bar} e
set e set e
} {*NOPERM*} } {*NOPERM*set*}
test {By default users are not able to access any key} { test {By default users are not able to access any key} {
r ACL setuser newuser +set r ACL setuser newuser +set
...@@ -161,17 +161,14 @@ start_server {tags {"acl external:skip"}} { ...@@ -161,17 +161,14 @@ start_server {tags {"acl external:skip"}} {
test {In transaction queue publish/subscribe/psubscribe to unauthorized channel will fail} { test {In transaction queue publish/subscribe/psubscribe to unauthorized channel will fail} {
r ACL setuser psuser +multi +discard r ACL setuser psuser +multi +discard
r MULTI r MULTI
catch {r PUBLISH notexits helloworld} e assert_error {*NOPERM*channel*} {r PUBLISH notexits helloworld}
r DISCARD r DISCARD
assert_match {*NOPERM*} $e
r MULTI r MULTI
catch {r SUBSCRIBE notexits foo:1} e assert_error {*NOPERM*channel*} {r SUBSCRIBE notexits foo:1}
r DISCARD r DISCARD
assert_match {*NOPERM*} $e
r MULTI r MULTI
catch {r PSUBSCRIBE notexits:* bar:*} e assert_error {*NOPERM*channel*} {r PSUBSCRIBE notexits:* bar:*}
r DISCARD r DISCARD
assert_match {*NOPERM*} $e
} }
test {It's possible to allow subscribing to a subset of channels} { test {It's possible to allow subscribing to a subset of channels} {
...@@ -285,7 +282,7 @@ start_server {tags {"acl external:skip"}} { ...@@ -285,7 +282,7 @@ start_server {tags {"acl external:skip"}} {
r INCR mycounter ; # Should not raise an error r INCR mycounter ; # Should not raise an error
catch {r PING} e catch {r PING} e
set e set e
} {*NOPERM*} } {*NOPERM*ping*}
test {ACLs can include or exclude whole classes of commands} { test {ACLs can include or exclude whole classes of commands} {
r ACL setuser newuser -@all +@set +acl r ACL setuser newuser -@all +@set +acl
...@@ -296,7 +293,7 @@ start_server {tags {"acl external:skip"}} { ...@@ -296,7 +293,7 @@ start_server {tags {"acl external:skip"}} {
catch {r SET foo bar} e catch {r SET foo bar} e
r ACL setuser newuser allcommands; # Undo commands ACL r ACL setuser newuser allcommands; # Undo commands ACL
set e set e
} {*NOPERM*} } {*NOPERM*set*}
test {ACLs can include single subcommands} { test {ACLs can include single subcommands} {
r ACL setuser newuser +@all -client r ACL setuser newuser +@all -client
...@@ -308,7 +305,7 @@ start_server {tags {"acl external:skip"}} { ...@@ -308,7 +305,7 @@ start_server {tags {"acl external:skip"}} {
r CLIENT SETNAME foo ; # Should not fail r CLIENT SETNAME foo ; # Should not fail
catch {r CLIENT KILL type master} e catch {r CLIENT KILL type master} e
set e set e
} {*NOPERM*} } {*NOPERM*client|kill*}
test {ACLs can exclude single subcommands, case 1} { test {ACLs can exclude single subcommands, case 1} {
r ACL setuser newuser +@all -client|kill r ACL setuser newuser +@all -client|kill
...@@ -318,7 +315,7 @@ start_server {tags {"acl external:skip"}} { ...@@ -318,7 +315,7 @@ start_server {tags {"acl external:skip"}} {
r CLIENT SETNAME foo ; # Should not fail r CLIENT SETNAME foo ; # Should not fail
catch {r CLIENT KILL type master} e catch {r CLIENT KILL type master} e
set e set e
} {*NOPERM*} } {*NOPERM*client|kill*}
test {ACLs can exclude single subcommands, case 2} { test {ACLs can exclude single subcommands, case 2} {
r ACL setuser newuser -@all +acl +config -config|set r ACL setuser newuser -@all +acl +config -config|set
...@@ -328,7 +325,7 @@ start_server {tags {"acl external:skip"}} { ...@@ -328,7 +325,7 @@ start_server {tags {"acl external:skip"}} {
r CONFIG GET loglevel; # Should not fail r CONFIG GET loglevel; # Should not fail
catch {r CONFIG SET loglevel debug} e catch {r CONFIG SET loglevel debug} e
set e set e
} {*NOPERM*} } {*NOPERM*config|set*}
test {ACLs can include a subcommand with a specific arg} { test {ACLs can include a subcommand with a specific arg} {
r ACL setuser newuser +@all -config|get r ACL setuser newuser +@all -config|get
...@@ -339,7 +336,7 @@ start_server {tags {"acl external:skip"}} { ...@@ -339,7 +336,7 @@ start_server {tags {"acl external:skip"}} {
r CONFIG GET appendonly; # Should not fail r CONFIG GET appendonly; # Should not fail
catch {r CONFIG GET loglevel} e catch {r CONFIG GET loglevel} e
set e set e
} {*NOPERM*} } {*NOPERM*config|get*}
test {ACLs including of a type includes also subcommands} { test {ACLs including of a type includes also subcommands} {
r ACL setuser newuser -@all +acl +@stream r ACL setuser newuser -@all +acl +@stream
...@@ -354,7 +351,7 @@ start_server {tags {"acl external:skip"}} { ...@@ -354,7 +351,7 @@ start_server {tags {"acl external:skip"}} {
r SELECT 0 r SELECT 0
catch {r SELECT 1} e catch {r SELECT 1} e
set e set e
} {*NOPERM*} } {*NOPERM*select*}
test {ACLs can block all DEBUG subcommands except one} { test {ACLs can block all DEBUG subcommands except one} {
r ACL setuser newuser -@all +acl +incr +debug|object r ACL setuser newuser -@all +acl +incr +debug|object
...@@ -364,7 +361,7 @@ start_server {tags {"acl external:skip"}} { ...@@ -364,7 +361,7 @@ start_server {tags {"acl external:skip"}} {
r DEBUG OBJECT key r DEBUG OBJECT key
catch {r DEBUG SEGFAULT} e catch {r DEBUG SEGFAULT} e
set e set e
} {*NOPERM*} } {*NOPERM*debug*}
test {ACLs set can include subcommands, if already full command exists} { test {ACLs set can include subcommands, if already full command exists} {
r ACL setuser bob +memory|doctor r ACL setuser bob +memory|doctor
...@@ -396,8 +393,7 @@ start_server {tags {"acl external:skip"}} { ...@@ -396,8 +393,7 @@ start_server {tags {"acl external:skip"}} {
r ACL setuser alice >passwd1 on r ACL setuser alice >passwd1 on
r AUTH alice passwd1 r AUTH alice passwd1
catch {r MEMORY DOCTOR} e assert_error {*NOPERM*memory|doctor*} {r MEMORY DOCTOR}
assert_match {*NOPERM*} $e
r MEMORY STATS ;# should work r MEMORY STATS ;# should work
# Validate the commands have got engulfed to -memory. # Validate the commands have got engulfed to -memory.
...@@ -405,10 +401,8 @@ start_server {tags {"acl external:skip"}} { ...@@ -405,10 +401,8 @@ start_server {tags {"acl external:skip"}} {
set cmdstr [dict get [r ACL getuser alice] commands] set cmdstr [dict get [r ACL getuser alice] commands]
assert_equal {+@all -memory} $cmdstr assert_equal {+@all -memory} $cmdstr
catch {r MEMORY DOCTOR} e assert_error {*NOPERM*memory|doctor*} {r MEMORY DOCTOR}
assert_match {*NOPERM*} $e assert_error {*NOPERM*memory|stats*} {r MEMORY STATS}
catch {r MEMORY STATS} e
assert_match {*NOPERM*} $e
# Appending to the existing access string of alice. # Appending to the existing access string of alice.
r ACL setuser alice -@all r ACL setuser alice -@all
...@@ -422,10 +416,8 @@ start_server {tags {"acl external:skip"}} { ...@@ -422,10 +416,8 @@ start_server {tags {"acl external:skip"}} {
r AUTH alice passwd1 r AUTH alice passwd1
catch {r GET key} e assert_error {*NOPERM*get*} {r GET key}
assert_match {*NOPERM*} $e assert_error {*NOPERM*memory|stats*} {r MEMORY STATS}
catch {r MEMORY STATS} e
assert_match {*NOPERM*} $e
# Auth newuser before the next test # Auth newuser before the next test
r AUTH newuser passwd1 r AUTH newuser passwd1
...@@ -486,7 +478,7 @@ start_server {tags {"acl external:skip"}} { ...@@ -486,7 +478,7 @@ start_server {tags {"acl external:skip"}} {
r ACL setuser antirez +eval +multi +exec r ACL setuser antirez +eval +multi +exec
r ACL setuser antirez resetchannels +publish r ACL setuser antirez resetchannels +publish
r AUTH antirez foo r AUTH antirez foo
catch {r GET foo} assert_error "*NOPERM*get*" {r GET foo}
r AUTH default "" r AUTH default ""
set entry [lindex [r ACL LOG] 0] set entry [lindex [r ACL LOG] 0]
assert {[dict get $entry username] eq {antirez}} assert {[dict get $entry username] eq {antirez}}
...@@ -495,14 +487,29 @@ start_server {tags {"acl external:skip"}} { ...@@ -495,14 +487,29 @@ start_server {tags {"acl external:skip"}} {
assert {[dict get $entry object] eq {get}} assert {[dict get $entry object] eq {get}}
} }
test "ACL LOG shows failed subcommand executions at toplevel" {
r ACL LOG RESET
r ACL DELUSER demo
r ACL SETUSER demo on nopass
r AUTH demo ""
assert_error "*NOPERM*script|help*" {r SCRIPT HELP}
r AUTH default ""
set entry [lindex [r ACL LOG] 0]
assert_equal [dict get $entry username] {demo}
assert_equal [dict get $entry context] {toplevel}
assert_equal [dict get $entry reason] {command}
assert_equal [dict get $entry object] {script|help}
}
test {ACL LOG is able to test similar events} { test {ACL LOG is able to test similar events} {
r ACL LOG RESET
r AUTH antirez foo r AUTH antirez foo
catch {r GET foo} catch {r GET foo}
catch {r GET foo} catch {r GET foo}
catch {r GET foo} catch {r GET foo}
r AUTH default "" r AUTH default ""
set entry [lindex [r ACL LOG] 0] set entry [lindex [r ACL LOG] 0]
assert {[dict get $entry count] == 4} assert {[dict get $entry count] == 3}
} }
test {ACL LOG is able to log keys access violations and key name} { test {ACL LOG is able to log keys access violations and key name} {
...@@ -662,12 +669,9 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"] tags ...@@ -662,12 +669,9 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"] tags
test {default: with config acl-pubsub-default resetchannels after reset, can not access any channels} { test {default: with config acl-pubsub-default resetchannels after reset, can not access any channels} {
r CONFIG SET acl-pubsub-default resetchannels r CONFIG SET acl-pubsub-default resetchannels
r ACL setuser default reset on nopass ~* +@all r ACL setuser default reset on nopass ~* +@all
catch {r SUBSCRIBE foo} e assert_error {*NOPERM*channel*} {r SUBSCRIBE foo}
assert_match {*NOPERM*} $e assert_error {*NOPERM*channel*} {r PSUBSCRIBE bar*}
catch {r PSUBSCRIBE bar*} e assert_error {*NOPERM*channel*} {r PUBLISH hello world}
assert_match {*NOPERM*} $e
catch {r PUBLISH hello world} e
assert_match {*NOPERM*} $e
r CONFIG SET acl-pubsub-default resetchannels r CONFIG SET acl-pubsub-default resetchannels
} }
...@@ -683,7 +687,7 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"] tags ...@@ -683,7 +687,7 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"] tags
assert_equal "3" [r sadd set 1 2 3] assert_equal "3" [r sadd set 1 2 3]
catch {r SET key value} e catch {r SET key value} e
set e set e
} {*NOPERM*} } {*NOPERM*set*}
test {ACL load and save} { test {ACL load and save} {
r ACL setuser eve +get allkeys >eve on r ACL setuser eve +get allkeys >eve on
...@@ -701,7 +705,7 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"] tags ...@@ -701,7 +705,7 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"] tags
r GET key r GET key
catch {r SET key value} e catch {r SET key value} e
set e set e
} {*NOPERM*} } {*NOPERM*set*}
test {ACL load and save with restricted channels} { test {ACL load and save with restricted channels} {
r AUTH alice alice r AUTH alice alice
...@@ -719,7 +723,7 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"] tags ...@@ -719,7 +723,7 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"] tags
catch {r publish test1 bar} e catch {r publish test1 bar} e
r ACL deluser harry r ACL deluser harry
set e set e
} {*NOPERM*} } {*NOPERM*channel*}
} }
set server_path [tmpdir "resetchannels.acl"] set server_path [tmpdir "resetchannels.acl"]
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment