Fix ACL

[rubenseyer]

Summary (see later comments):

• ACL groups incorrectly instantiated without userid -1 leading to many spurious SuperUser ACLs
• Regular user anti-lockout had incorrect logic (and also the above issue)
• Add calculation of effective permissions (so we can actually tell the client what it can do)
• Make lookup of users by name case-insensitive (most notably making it impossible to add users without lowercase names to groups)
• Actually walk context chain when looking up group names in ACL query

---

Played around with a few channels and some ACLs and went from "everything is SuperUser and broken" to "seems to be working" at least, but with these issues it seems like the whole of ACLs could use some testing.

[streaps]

The grumble server starts again and I can add an @ALL group. The ACL is stored correctly and survives restart of the server unharmed. :)

Unfortunately it doesn't seem to have any effect. I haven't tried all permissions, but for example "register self" does not work at all.

[rubenseyer]

Yeah, seems like this hasn't been working for quite a while and nobody noticed until now.
The problem is that grumble never actually tells the connecting user they have the ability to register.
There are a couple of fixme comments which refer to implementing this.

grumble/cmd/grumble/server.go

Line 692 in df98375

// fixme(mkrautz): previously we calculated the user's

grumble/cmd/grumble/server.go

Line 902 in df98375

// fixme(mkrautz): re-add when we have ACL caching

It's a bigger fix than just four lines so I don't have time to look at this right now, but even though it technically is a new feature I can't imagine it to be too big. One could probably be inspired by the corresponding implementation in Murmur.

[rubenseyer]

I thought about it some more and couldn't resist (and besides it wasn't too many lines). The old HasPermission already traversed everything so it was just a matter of refactoring out the intermediate step of calculating the permissions from testing them.

Now, this doesn't implement ACL caching, but it doesn't come with worse performance than before anyway (because we traversed the entire tree before, too.) It wouldn't be too hard, I just didn't want to impose any design decisions on how it should be structured (but I would probably make an ACLCache type and move HasPermission et al. to methods on that, with a field on the server). I suggest benchmarks before I would bother with caching, because it would be a change that touches many lines of code, but then again "large" use-cases are not too keen on grumble for other reasons (namely no RPC).

I managed to register myself, but that was all I tested. If you come up with any weird combinations to try out please do.

[streaps]

Registration works
Write ACL ✓

but:
Groups and it's members are not saved (or not displayed) in the Groups tab.
modified inherited groups appear multiple times, like:

@all <- default (server)
@all <- root
@custom <- root
@all <- sub-channel "one"
@all <- this sub-sub-channel "two"

root
+ one
  + two

[rubenseyer]

Non-merged inheritance seems to be the usual Murmur behaviour (it makes sense since you might want to see where some rule is coming from).

Groups were saved (you can e.g. set ACLs on them in the dialog) but there was a problem with lookup. Seems there was a bug where the context chain for groups was never built, so grumble always returned zero groups.

Group members were usually not saved (unless you had a lowercase name) because that lookup should be case insensitive according to the Murmur implementation, and it looks like the client just sends a lowercase name. I changed the server lookup table to just use lowercase everywhere adding strings.ToLower calls.

[streaps]

Non-merged inheritance seems to be the usual Murmur behaviour (it makes sense since you might want to see where some rule is coming from).

You are right, I didn't test it properly. "Applies to sub-channel" was not set in the group on my Murmur server. After enabling it I see the exact same behaviour on Murmur too.

I haven't tested it extensively, but groups seem to work now.

[ravicant]

I dont think it working it automatically switch to superuser - https://i.ravicant.in/9Lb6Fknjp.gif