Skip to content

ACL LOAD fails on files generated by ACL SAVE when specific channels are allowed #8811

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.

Sign up for GitHub

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

Closed
oranagra wants to merge 4 commits into from
Closed

ACL LOAD fails on files generated by ACL SAVE when specific channels are allowed #8811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b839e21
ACL channels permission handling for save/load scenario.
hpatro Apr 15, 2021
c175de2
Default user state to be not affected by acl-pubsub-default.
hpatro Apr 16, 2021
a30770e
revert interface changes
oranagra Apr 18, 2021
bdcce0a
typo^2
oranagra Apr 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion redis.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ acllog-max-len 128
# order to provide better out-of-the-box Pub/Sub security. Therefore, it is
# recommended that you explicitly define Pub/Sub permissions for all users
# rather then rely on implicit default values. Once you've set explicit
# Pub/Sub for all exisitn users, you should uncomment the following line.
# Pub/Sub for all existing users, you should uncomment the following line.
#
# acl-pubsub-default resetchannels

Expand Down
1 change: 1 addition & 0 deletions src/acl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ sds ACLDescribeUser(user *u) {
if (u->flags & USER_FLAG_ALLCHANNELS) {
res = sdscatlen(res,"&* ",3);
} else {
res = sdscatlen(res,"resetchannels ",14);
listRewind(u->channels,&li);
while((ln = listNext(&li))) {
sds thispat = listNodeValue(ln);
Expand Down
2 changes: 2 additions & 0 deletions tests/assets/nodefaultuser.acl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
user alice on nopass ~* +@all
user bob on nopass ~* &* +@all
95 changes: 95 additions & 0 deletions tests/unit/acl.tcl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,30 @@ start_server {tags {"acl"}} {
set e
} {*NOPERM*channel*}

test {Validate subset of channels is prefixed with resetchannels flag} {
r ACL setuser hpuser on nopass resetchannels &foo +@all

# Verify resetchannels flag is prefixed before the channel name(s)
set users [r ACL LIST]
set curruser "hpuser"
foreach user [lshuffle $users] {
if {[string first $curruser $user] != -1} {
assert_equal {user hpuser on nopass resetchannels &foo +@all} $user
}
}

# authenticate as hpuser
r AUTH hpuser pass

assert_equal {0} [r PUBLISH foo bar]
catch {r PUBLISH bar game} e

# Falling back to psuser for the below tests
r AUTH psuser pspass
r ACL deluser hpuser
set e
} {*NOPERM*channel*}

test {In transaction queue publish/subscribe/psubscribe to unauthorized channel will fail} {
r ACL setuser psuser +multi +discard
r MULTI
Expand Down Expand Up @@ -523,8 +547,79 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"]] {
catch {r SET key value} e
set e
} {*NOPERM*}

test {ACL load and save with restricted channels} {
r AUTH alice alice
r ACL setuser harry on nopass resetchannels &test +@all ~*
r ACL save

# ACL load will free user and kill clients
r ACL load
catch {r ACL LIST} e
assert_match {*I/O error*} $e

reconnect
r AUTH harry anything
r publish test bar
catch {r publish test1 bar} e
r ACL deluser harry
set e
} {*NOPERM*}
}

set server_path [tmpdir "resetchannels.acl"]
exec cp -f tests/assets/nodefaultuser.acl $server_path
exec cp -f tests/assets/default.conf $server_path
start_server [list overrides [list "dir" $server_path "acl-pubsub-default" "resetchannels" "aclfile" "nodefaultuser.acl"]] {

test {Default user has access to all channels irrespective of flag} {
set channelinfo [dict get [r ACL getuser default] channels]
assert_equal "*" $channelinfo
set channelinfo [dict get [r ACL getuser alice] channels]
assert_equal "" $channelinfo
}

test {Update acl-pubsub-default, existing users shouldn't get affected} {
set channelinfo [dict get [r ACL getuser default] channels]
assert_equal "*" $channelinfo
r CONFIG set acl-pubsub-default allchannels
r ACL setuser mydefault
set channelinfo [dict get [r ACL getuser mydefault] channels]
assert_equal "*" $channelinfo
r CONFIG set acl-pubsub-default resetchannels
set channelinfo [dict get [r ACL getuser mydefault] channels]
assert_equal "*" $channelinfo
}

test {Single channel is valid} {
r ACL setuser onechannel &test
set channelinfo [dict get [r ACL getuser onechannel] channels]
assert_equal test $channelinfo
r ACL deluser onechannel
}

test {Single channel is not valid with allchannels} {
r CONFIG set acl-pubsub-default allchannels
catch {r ACL setuser onechannel &test} err
r CONFIG set acl-pubsub-default resetchannels
set err
} {*start with an empty list of channels*}
}

set server_path [tmpdir "resetchannels.acl"]
exec cp -f tests/assets/nodefaultuser.acl $server_path
exec cp -f tests/assets/default.conf $server_path
start_server [list overrides [list "dir" $server_path "acl-pubsub-default" "resetchannels" "aclfile" "nodefaultuser.acl"]] {

test {Only default user has access to all channels irrespective of flag} {
set channelinfo [dict get [r ACL getuser default] channels]
assert_equal "*" $channelinfo
set channelinfo [dict get [r ACL getuser alice] channels]
assert_equal "" $channelinfo
}
}


start_server {overrides {user "default on nopass ~* +@all"}} {
test {default: load from config file, can access any channels} {
r SUBSCRIBE foo
Expand Down