diff --git a/src/acl.c b/src/acl.c index b4fb52c50..30fdc8e07 100644 --- a/src/acl.c +++ b/src/acl.c @@ -478,17 +478,30 @@ sds ACLDescribeUserCommandRules(user *u) { /* Try to add or subtract each category one after the other. Often a * single category will not perfectly match the set of commands into * it, so at the end we do a final pass adding/removing the single commands - * needed to make the bitmap exactly match. */ + * needed to make the bitmap exactly match. A temp user is maintained to + * keep track of categories already applied. */ + user tu = {0}; + user *tempuser = &tu; + memcpy(tempuser->allowed_commands, + u->allowed_commands, + sizeof(u->allowed_commands)); + for (int j = 0; ACLCommandCategories[j].flag != 0; j++) { unsigned long on, off; - ACLCountCategoryBitsForUser(u,&on,&off,ACLCommandCategories[j].name); + ACLCountCategoryBitsForUser(tempuser,&on,&off,ACLCommandCategories[j].name); if ((additive && on > off) || (!additive && off > on)) { sds op = sdsnewlen(additive ? "+@" : "-@", 2); op = sdscat(op,ACLCommandCategories[j].name); ACLSetUser(fakeuser,op,-1); + + sds invop = sdsnewlen(additive ? "-@" : "+@", 2); + invop = sdscat(invop,ACLCommandCategories[j].name); + ACLSetUser(tempuser,invop,-1); + rules = sdscatsds(rules,op); rules = sdscatlen(rules," ",1); sdsfree(op); + sdsfree(invop); } } diff --git a/tests/unit/acl.tcl b/tests/unit/acl.tcl index f015f75a0..12f59e749 100644 --- a/tests/unit/acl.tcl +++ b/tests/unit/acl.tcl @@ -135,6 +135,20 @@ start_server {tags {"acl"}} { assert_match {*+acl*} $cmdstr } + # A regression test make sure that as long as there is a simple + # category defining the commands, that it will be used as is. + test {ACL GETUSER provides reasonable results} { + # Test for future commands where allowed + r ACL setuser additive reset +@all -@write + set cmdstr [dict get [r ACL getuser additive] commands] + assert_match {+@all -@write} $cmdstr + + # Test for future commands are disallowed + r ACL setuser subtractive reset -@all +@read + set cmdstr [dict get [r ACL getuser subtractive] commands] + assert_match {-@all +@read} $cmdstr + } + test {ACL #5998 regression: memory leaks adding / removing subcommands} { r AUTH default "" r ACL setuser newuser reset -debug +debug|a +debug|b +debug|c