FIX(server): Don't ignore explicit PermissionQuery

7439bc4fe6b423ff0d36d3b1d529e474c14e9b01 introduced the ability for
clients to query for their permissions in any given channel. Since that
commit also added an automatic permissions broadcast for channels that a
client enters (and also its parent channel), it also introduced a
system to keep track of whether a given permission set has already been
broadcast to a given client. Supposedly this was meant as to not flood
the client with PermissionQuery messages if it just keeps switching
between the same channels (and thus already knows about the respective
permissions after having joined a channel for the first time).

Oddly enough though, when a client explicitly requests to be informed
about a channel's permissions, the cache would also be consolidated and
if the server was under the impression that the client already knows
about these permissions, its explicit query would simply be ignored.

This is of course nonsense, since ultimately it is the client who knows
whether it wants to be informed about those permissions (again) or not.

Therefore, this commit ensures that explicit queries are always
answered. No need to consolidate the cache and decide on whether the
server feels like answering or not.

Fixes #5699

2 files changed, 11 insertions, 4 deletions

diff --git a/src/murmur/Server.cpp b/src/murmur/Server.cpp
index cb9462e76..8bda808dc 100644
--- a/src/murmur/Server.cpp
+++ b/src/murmur/Server.cpp
@@ -2026,7 +2026,7 @@ QFlags< ChanACL::Perm > Server::effectivePermissions(ServerUser *p, Channel *c)
 	return ChanACL::effectivePermissions(p, c, &acCache);
 }
 
-void Server::sendClientPermission(ServerUser *u, Channel *c, bool forceupdate) {
+void Server::sendClientPermission(ServerUser *u, Channel *c, bool explicitlyRequested) {
 	unsigned int perm;
 
 	if (u->iId == 0)
@@ -2034,14 +2034,21 @@ void Server::sendClientPermission(ServerUser *u, Channel *c, bool forceupdate) {
 
 	{
 		QMutexLocker qml(&qmCache);
+		// Abuse that hasPermission will update acCache with the latest permissions (all of them,
+		// not only the requested one) so that we can pull this information out of it afterwards.
 		ChanACL::hasPermission(u, c, ChanACL::Enter, &acCache);
 		perm = acCache.value(u)->value(c);
 	}
 
-	if (forceupdate)
+	if (explicitlyRequested) {
+		// Store the last channel the client showed explicit interest in
 		u->iLastPermissionCheck = c->iId;
+	}
 
-	if (u->qmPermissionSent.value(c->iId) != perm) {
+	if (explicitlyRequested || u->qmPermissionSent.value(c->iId) != perm) {
+		// Send the permission info only if the client has explicitly asked for it
+		// or if the permissions have changed since the last time the client has
+		// been informed about permission for this channel.
 		u->qmPermissionSent.insert(c->iId, perm);
 
 		MumbleProto::PermissionQuery mppq;
diff --git a/src/murmur/Server.h b/src/murmur/Server.h
index eb90a35d3..7f02cf32d 100644
--- a/src/murmur/Server.h
+++ b/src/murmur/Server.h
@@ -322,7 +322,7 @@ public:
 
 	bool hasPermission(ServerUser *p, Channel *c, QFlags< ChanACL::Perm > perm);
 	QFlags< ChanACL::Perm > effectivePermissions(ServerUser *p, Channel *c);
-	void sendClientPermission(ServerUser *u, Channel *c, bool updatelast = false);
+	void sendClientPermission(ServerUser *u, Channel *c, bool explicitlyRequested = false);
 	void flushClientPermissionCache(ServerUser *u, MumbleProto::PermissionQuery &mpqq);
 	void clearACLCache(User *p = nullptr);
 	void clearWhisperTargetCache();