Merge pull request #233 from simoleo89/fix/permissions-inputs

fix(permissions): fail closed on stale ranks

Emulator/src/main/java/com/eu/habbo/habbohotel/permissions/PermissionSetting.java

@@ -11,6 +11,10 @@ public enum PermissionSetting {
     ROOM_OWNER;
 
     public static PermissionSetting fromString(String value) {
+        if (value == null) {
+            return DISALLOWED;
+        }
+
         switch (value) {
             case "1":
                 return ALLOWED;

Emulator/src/main/java/com/eu/habbo/habbohotel/permissions/PermissionsManager.java

@@ -11,6 +11,7 @@ import org.slf4j.LoggerFactory;
 
 import java.sql.*;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -65,35 +66,45 @@ public class PermissionsManager {
     }
 
     private void loadPermissionsLegacy(Connection connection) throws SQLException {
+        Set<Integer> loadedRankIds = new HashSet<>();
+
         try (Statement statement = connection.createStatement(); ResultSet set = statement.executeQuery("SELECT * FROM permissions ORDER BY id ASC")) {
             while (set.next()) {
+                int rankId = set.getInt("id");
+                loadedRankIds.add(rankId);
+
                 Rank rank = null;
-                if (!this.ranks.containsKey(set.getInt("id"))) {
+                if (!this.ranks.containsKey(rankId)) {
                     rank = new Rank(set);
-                    this.ranks.put(set.getInt("id"), rank);
+                    this.ranks.put(rankId, rank);
                 } else {
-                    rank = this.ranks.get(set.getInt("id"));
+                    rank = this.ranks.get(rankId);
                     rank.load(set);
                 }
 
                 this.addBadgeMapping(rank);
             }
         }
+
+        this.pruneMissingRanks(loadedRankIds);
     }
 
     private boolean loadPermissionsNormalized(Connection connection) throws SQLException {
         boolean hasRanks = false;
         List<Rank> loadedRanks = new ArrayList<>();
+        Set<Integer> loadedRankIds = new HashSet<>();
 
         try (Statement statement = connection.createStatement(); ResultSet set = statement.executeQuery("SELECT * FROM permission_ranks ORDER BY id ASC")) {
             while (set.next()) {
                 hasRanks = true;
+                int rankId = set.getInt("id");
+                loadedRankIds.add(rankId);
 
-                Rank rank = this.ranks.get(set.getInt("id"));
+                Rank rank = this.ranks.get(rankId);
 
                 if (rank == null) {
-                    rank = new Rank(set.getInt("id"));
+                    rank = new Rank(rankId);
-                    this.ranks.put(set.getInt("id"), rank);
+                    this.ranks.put(rankId, rank);
                 }
 
                 rank.loadNormalizedMetadata(set);
@@ -141,9 +152,18 @@ public class PermissionsManager {
             }
         }
 
+        this.pruneMissingRanks(loadedRankIds);
         return hasDefinitions;
     }
 
+    private void pruneMissingRanks(Set<Integer> loadedRankIds) {
+        for (int rankId : this.ranks.keys()) {
+            if (!loadedRankIds.contains(rankId)) {
+                this.ranks.remove(rankId);
+            }
+        }
+    }
+
     private void ensureNormalizedRankColumns(Connection connection, List<Rank> loadedRanks) throws SQLException {
         Set<String> availableColumns = new HashSet<>();
 
@@ -254,6 +274,10 @@ public class PermissionsManager {
 
 
     public boolean hasPermission(Habbo habbo, String permission, boolean withRoomRights) {
+        if (habbo == null || habbo.getHabboInfo() == null || permission == null || permission.isBlank()) {
+            return false;
+        }
+
         if (!this.hasPermission(habbo.getHabboInfo().getRank(), permission, withRoomRights)) {
             for (HabboPlugin plugin : Emulator.getPluginManager().getPlugins()) {
                 if (plugin.hasPermission(habbo, permission)) {
@@ -269,15 +293,16 @@ public class PermissionsManager {
 
 
     public boolean hasPermission(Rank rank, String permission, boolean withRoomRights) {
-        return rank.hasPermission(permission, withRoomRights);
+        return rank != null && permission != null && !permission.isBlank() && rank.hasPermission(permission, withRoomRights);
     }
 
     public Set<String> getStaffBadges() {
-        return this.badges.keySet();
+        return Collections.unmodifiableSet(new HashSet<>(this.badges.keySet()));
     }
 
     public List<Rank> getRanksByBadgeCode(String code) {
-        return this.badges.get(code);
+        List<Rank> ranks = this.badges.get(code);
+        return ranks == null ? Collections.emptyList() : Collections.unmodifiableList(new ArrayList<>(ranks));
     }
 
     public List<Rank> getAllRanks() {

Emulator/src/main/java/com/eu/habbo/habbohotel/permissions/Rank.java

@@ -114,6 +114,10 @@ public class Rank {
     }
 
     public boolean hasPermission(String key, boolean isRoomOwner) {
+        if (key == null || key.isBlank()) {
+            return false;
+        }
+
         if (this.permissions.containsKey(key)) {
             Permission permission = this.permissions.get(key);
 

Emulator/src/test/java/com/eu/habbo/habbohotel/permissions/PermissionSettingContractTest.java

@@ -0,0 +1,21 @@
+package com.eu.habbo.habbohotel.permissions;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class PermissionSettingContractTest {
+
+    @Test
+    void unknownPermissionValuesFailClosed() {
+        assertEquals(PermissionSetting.DISALLOWED, PermissionSetting.fromString(null));
+        assertEquals(PermissionSetting.DISALLOWED, PermissionSetting.fromString(""));
+        assertEquals(PermissionSetting.DISALLOWED, PermissionSetting.fromString("999"));
+    }
+
+    @Test
+    void knownPermissionValuesMapToExplicitSettings() {
+        assertEquals(PermissionSetting.ALLOWED, PermissionSetting.fromString("1"));
+        assertEquals(PermissionSetting.ROOM_OWNER, PermissionSetting.fromString("2"));
+    }
+}

Emulator/src/test/java/com/eu/habbo/habbohotel/permissions/RankPermissionContractTest.java

@@ -0,0 +1,35 @@
+package com.eu.habbo.habbohotel.permissions;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class RankPermissionContractTest {
+
+    @Test
+    void missingPermissionsFailClosed() {
+        Rank rank = new Rank(1);
+
+        assertFalse(rank.hasPermission(null, false));
+        assertFalse(rank.hasPermission("", false));
+        assertFalse(rank.hasPermission("acc_supporttool", false));
+    }
+
+    @Test
+    void roomOwnerPermissionOnlyPassesWithRoomRights() {
+        Rank rank = new Rank(1);
+        rank.setPermission("acc_placefurni", PermissionSetting.ROOM_OWNER);
+
+        assertFalse(rank.hasPermission("acc_placefurni", false));
+        assertTrue(rank.hasPermission("acc_placefurni", true));
+    }
+
+    @Test
+    void allowedPermissionPassesWithoutRoomRights() {
+        Rank rank = new Rank(1);
+        rank.setPermission("acc_supporttool", PermissionSetting.ALLOWED);
+
+        assertTrue(rank.hasPermission("acc_supporttool", false));
+    }
+}