fix: acl compatibility

[kostasrim]

resolves: #3046

• remove acl categories from context and all acl checks
• category assign,ent now assigns all the acl commands for that category to the user
• introduce modification order of acl's per user
• acl rules are now printed in the same order as in redis/valkey
• remove old user_registry_test which was part of the poc

[dranikpg]

Some minor questions. Also, what's the reason for removing full category flags? Isn't that more efficient?

[dranikpg]

I just had a cool thought that we know this stuff at compiletime, so we could make a alias to a bitset<sizeof(categories)> that's easier to manage than an int flag 😎

[kostasrim]

We just normalize here. Had we replaced the flat_hash_map with that we would loose the index we map to ( CategoryToIdxStore just maps the category, e.g. the mask with an index that can be used to find its name). On the other hand if we replace the key, uint32_t with std::bitset it won't really make a difference since we don't manipulate any of the individual bits.

Maybe I am missing something so let me know :)

[dranikpg]

that's just a index[cid.GetBidIndex()][cid.GetFamily()] |= bit_index;

[dranikpg]

could there be any reason there's not a 1-to-1 mapping? If not, we could elminiate this store allotogether

[kostasrim]

category has large values depending on which bit is set, so for example SCRIPTING is 1<<20 which makes the 1-1 mapping impossible. The flow is that the user passes let's a +SCRIPTING, it's parsed to 1<<20. Now if we want to find the name of the category, we normalize the 1<<n into the index and then use that index in the REVERSE_CATEGORY_TABLE. Open to ideas if it can be improved 🤷

[dranikpg]

fun fact alias CommandChange = pair<> is also hashable, assingnable and comparable 🙂 I doubt you'll ever "accidentally" assign a regular pair to it 🙃

I don't see it being used in a variant

[kostasrim]

Some minor questions. Also, what's the reason for removing full category flags? Isn't that more efficient?

@dranikpg The problem is the semantics. So in redis/valkey the following chore works:

Add +@write to category section of the ACL and add -set to commands section of the ACL
Issue a set command on an already permitted key
See that set command works (it should `not`)

The reason that it breaks is because we take the conjuction between categories and command. This works for all the cases but breaks on this one. The order of WRITE, SET dictates if a user is allowed to execute the command -- it's not a conjuction. What we should actually check instead is if the user is allowed to execute the command (that's the only simple way to solve this corner case -- there other solutions by keep a separate table for categories that explicitly were removed -set but that would still require the order -- that is -@write got executed before +set or after?).

The simplest solution for me was to treat groups as a gateway to a list of commands. That way, each time we assign a group to a user we effectively allow or disallow them to execute command. That give us that right semantics for the above case because +@write -set or -@write +set does the right thing

The hash_maps are there to track changes. Such that now we print -set +write etc in the right order just like redis/valkey. It's a compatibility thing and their sizes should not be big (a few key/value pairs per user)

Last but not least, I would not say is necessary slower, for two reasons:

1. This is not the critical path
2. When we updated a category we would update a uint and now we update 15 uints withinin a vector which is still a constant number of changes on contiguous memory

p.s. I wil ltake care of the rest of your comments soon +1