Fix blank guest room authorization

examples/simple_room_server/MyMesh.cpp

@@ -1,4 +1,12 @@
 #include "MyMesh.h"
+#include "RoomAuth.h"
+
+static_assert(static_cast<uint8_t>(room_server::LoginPermission::Guest) == PERM_ACL_GUEST,
+              "room login guest permission must match the ACL role");
+static_assert(static_cast<uint8_t>(room_server::LoginPermission::ReadWrite) == PERM_ACL_READ_WRITE,
+              "room login read/write permission must match the ACL role");
+static_assert(static_cast<uint8_t>(room_server::LoginPermission::Admin) == PERM_ACL_ADMIN,
+              "room login admin permission must match the ACL role");
 
 #define REPLY_DELAY_MILLIS          1500
 #define PUSH_NOTIFY_DELAY_MILLIS    2000
@@ -325,19 +333,14 @@ void MyMesh::onAnonDataRecv(mesh::Packet *packet, const uint8_t *secret, const m
       }
     }
     if (client == NULL) {
-      uint8_t perm;
+      const auto login_permission = room_server::resolveLoginPermission((char *)&data[8], _prefs.password,
-      if (strcmp((char *)&data[8], _prefs.password) == 0) { // check for valid admin password
+                                                                        _prefs.guest_password,
-        perm = PERM_ACL_ADMIN;
+                                                                        _prefs.allow_read_only);
-      } else {
+      if (login_permission == room_server::LoginPermission::Rejected) {
-        if (strcmp((char *)&data[8], _prefs.guest_password) == 0) {   // check the room/public password
+        MESH_DEBUG_PRINTLN("Incorrect room password");
-          perm = PERM_ACL_READ_WRITE;
+        return; // no response. Client will timeout
-        } else if (_prefs.allow_read_only) {
-          perm = PERM_ACL_GUEST;
-        } else {
-          MESH_DEBUG_PRINTLN("Incorrect room password");
-          return; // no response. Client will timeout
-        }
       }
+      const uint8_t perm = static_cast<uint8_t>(login_permission);
 
       client = acl.putClient(sender, 0);  // add to known clients (if not already known)
       if (sender_timestamp <= client->last_timestamp) {

examples/simple_room_server/RoomAuth.h

@@ -0,0 +1,31 @@
+#pragma once
+
+#include <stdint.h>
+#include <string.h>
+
+namespace room_server {
+
+enum class LoginPermission : uint8_t {
+  Guest = 0,
+  ReadWrite = 2,
+  Admin = 3,
+  Rejected = 0xFF,
+};
+
+inline LoginPermission resolveLoginPermission(const char *supplied_password, const char *admin_password,
+                                              const char *guest_password, bool allow_read_only) {
+  if (strcmp(supplied_password, admin_password) == 0) {
+    return LoginPermission::Admin;
+  }
+  // An empty guest password disables authenticated read/write access. Without
+  // this guard, a blank login is promoted before open read-only access applies.
+  if (guest_password[0] != 0 && strcmp(supplied_password, guest_password) == 0) {
+    return LoginPermission::ReadWrite;
+  }
+  if (allow_read_only) {
+    return LoginPermission::Guest;
+  }
+  return LoginPermission::Rejected;
+}
+
+} // namespace room_server

test/test_room_auth/test_room_auth.cpp

@@ -0,0 +1,39 @@
+#include <gtest/gtest.h>
+
+#include "../../examples/simple_room_server/RoomAuth.h"
+
+using room_server::LoginPermission;
+using room_server::resolveLoginPermission;
+
+TEST(RoomAuth, MatchingAdminPasswordGrantsAdmin) {
+  EXPECT_EQ(LoginPermission::Admin, resolveLoginPermission("admin", "admin", "guest", true));
+}
+
+TEST(RoomAuth, AdminPasswordTakesPriorityWhenPasswordsMatch) {
+  EXPECT_EQ(LoginPermission::Admin, resolveLoginPermission("shared", "shared", "shared", true));
+}
+
+TEST(RoomAuth, MatchingNonEmptyGuestPasswordTakesPriorityOverOpenReadOnly) {
+  EXPECT_EQ(LoginPermission::ReadWrite, resolveLoginPermission("guest", "admin", "guest", true));
+}
+
+TEST(RoomAuth, BlankGuestPasswordDoesNotGrantReadWrite) {
+  EXPECT_EQ(LoginPermission::Guest, resolveLoginPermission("", "admin", "", true));
+}
+
+TEST(RoomAuth, OpenReadOnlyAccessAcceptsAnyNonAdminAsGuest) {
+  EXPECT_EQ(LoginPermission::Guest, resolveLoginPermission("wrong", "admin", "guest", true));
+}
+
+TEST(RoomAuth, ClosedServerRejectsBlankPasswordWithBlankGuestPassword) {
+  EXPECT_EQ(LoginPermission::Rejected, resolveLoginPermission("", "admin", "", false));
+}
+
+TEST(RoomAuth, ClosedServerRejectsIncorrectPassword) {
+  EXPECT_EQ(LoginPermission::Rejected, resolveLoginPermission("wrong", "admin", "guest", false));
+}
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}