Commit 12634853 authored by Slava Koyfman's avatar Slava Koyfman Committed by Oran Agra
Browse files

Disconnect pub-sub subscribers when revoking `allchannels` permission (#11992)

The existing logic for killing pub-sub clients did not handle the `allchannels`
permission correctly. For example, if you:

    ACL SETUSER foo allchannels

Have a client authenticate as the user `foo` and subscribe to a channel, and then:

    ACL SETUSER foo resetchannels

The subscribed client would not be disconnected, though new clients under that user
would be blocked from subscribing to any channels.

This was caused by an incomplete optimization in `ACLKillPubsubClientsIfNeeded`
checking whether the new channel permissions were a strict superset of the old ones.

(cherry picked from commit f38aa6bf)
(cherry picked from commit 9caeadb8661f5fabd8386e3f07d445fe6ca46e7f)
parent e030e351
...@@ -1284,7 +1284,14 @@ void ACLKillPubsubClientsIfNeeded(user *u, list *upcoming) { ...@@ -1284,7 +1284,14 @@ void ACLKillPubsubClientsIfNeeded(user *u, list *upcoming) {
listNode *ln, *lpn; listNode *ln, *lpn;
robj *o; robj *o;
int kill = 0; int kill = 0;
/* If any of the original rule has the all-channels permission, but the new
* one doesn't (verifed by the caller), then the new list is not a strict
* superset of the original, and the next loop can be skipped. */
if (u->flags & USER_FLAG_ALLCHANNELS) {
kill = 1;
}
/* Nothing to kill when the upcoming are a literal super set of the original /* Nothing to kill when the upcoming are a literal super set of the original
* permissions. */ * permissions. */
listRewind(u->channels,&li); listRewind(u->channels,&li);
......
...@@ -207,6 +207,20 @@ start_server {tags {"acl"}} { ...@@ -207,6 +207,20 @@ start_server {tags {"acl"}} {
$rd close $rd close
} {0} } {0}
test {Subscribers are killed when revoked of allchannels permission} {
set rd [redis_deferring_client]
r ACL setuser psuser allchannels
$rd AUTH psuser pspass
$rd read
$rd CLIENT SETNAME deathrow
$rd read
$rd PSUBSCRIBE foo
$rd read
r ACL setuser psuser resetchannels
assert_no_match {*deathrow*} [r CLIENT LIST]
$rd close
} {0}
test {Subscribers are pardoned if literal permissions are retained and/or gaining allchannels} { test {Subscribers are pardoned if literal permissions are retained and/or gaining allchannels} {
set rd [redis_deferring_client] set rd [redis_deferring_client]
r ACL setuser psuser resetchannels &foo:1 &bar:* r ACL setuser psuser resetchannels &foo:1 &bar:*
......
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