fix: incorrect accounting of sysAdmins when reassigning system role […

…NAY-6]
nayms · Apr 18, 2023 · fada081 · fada081
1 parent 0649519
commit fada081
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 1 deletion.
diff --git a/src/diamonds/nayms/libs/LibACL.sol b/src/diamonds/nayms/libs/LibACL.sol
@@ -41,8 +41,12 @@ library LibACL {
         require(_contextId != "", "invalid context ID");
         require(_roleId != "", "invalid role ID");
 
-        s.roles[_objectId][_contextId] = _roleId;
+        bytes32 oldRole = s.roles[_objectId][_contextId];
+        if(oldRole == LibHelpers._stringToBytes32(LibConstants.ROLE_SYSTEM_ADMIN) && oldRole != _roleId) {
+            require(s.sysAdmins > 1, "must have at least one system admin");
+        }
 
+        s.roles[_objectId][_contextId] = _roleId;
         if (_contextId == LibAdmin._getSystemId() && _roleId == LibHelpers._stringToBytes32(LibConstants.ROLE_SYSTEM_ADMIN)) {
             unchecked {
                 s.sysAdmins += 1;

diff --git a/test/T02ACL.t.sol b/test/T02ACL.t.sol
@@ -25,6 +25,11 @@ contract T02ACLTest is D03ProtocolDefaults, MockAccounts {
         nayms.unassignRole(account0Id, systemContext);
     }
 
+    function testReassignLastSystemAdminFails() public {
+        vm.expectRevert("must have at least one system admin");
+        nayms.assignRole(account0Id, systemContext, LibConstants.ROLE_BROKER);
+    }
+
     function testUnassignSystemAdmin() public {
         nayms.assignRole(signer1Id, systemContext, LibConstants.ROLE_SYSTEM_ADMIN);