Tighten up permissions in dc authorizers

patch by Stefan Miklosovic; reviewed by Francisco Guerrero for CASSANDRA-20225

Author: smiklosovic

CHANGES.txt

@@ -1,4 +1,5 @@
 4.0.16
+ * Tighten up permissions in dc authorizers (CASSANDRA-20225)
  * CBUtil serialization of UTF8 does not handle all UTF8 properly (CASSANDRA-20234)
  * Make hint expiry use request start time rather than timeout time for TTL (CASSANDRA-20014)
  * Do not attach rows and partitions to CheckForAbort when already attached (CASSANDRA-20135)

src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java

@@ -78,6 +78,9 @@ public void authorize(ClientState state) throws UnauthorizedException
         if (opts.getSuperuser().isPresent() && !isSuper)
             throw new UnauthorizedException("Only superusers are allowed to alter superuser status");
 
+        if (dcPermissions != null && !isSuper)
+            throw new UnauthorizedException("Only superusers are allowed to alter access to datacenters.");
+
         // superusers can do whatever else they like
         if (isSuper)
             return;

test/unit/org/apache/cassandra/auth/CassandraNetworkAuthorizerTest.java

@@ -40,7 +40,9 @@
 import org.apache.cassandra.db.Keyspace;
 import org.apache.cassandra.db.marshal.UTF8Type;
 import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.exceptions.UnauthorizedException;
 import org.apache.cassandra.service.ClientState;
+import org.assertj.core.api.Assertions;
 
 import static org.apache.cassandra.auth.AuthKeyspace.NETWORK_PERMISSIONS;
 import static org.apache.cassandra.auth.RoleTestUtils.LocalCassandraRoleManager;
@@ -113,7 +115,19 @@ private static ClientState getClientState()
         return state;
     }
 
+    private static ClientState getClientState(String role)
+    {
+        ClientState state = ClientState.forInternalCalls();
+        state.login(new AuthenticatedUser(role));
+        return state;
+    }
+
     private static void auth(String query, Object... args)
+    {
+        auth(query, getClientState(), args);
+    }
+
+    private static void auth(String query, ClientState clientState, Object... args)
     {
         CQLStatement statement = QueryProcessor.parseStatement(String.format(query, args)).prepare(ClientState.forInternalCalls());
         assert statement instanceof CreateRoleStatement
@@ -123,7 +137,8 @@ private static void auth(String query, Object... args)
 
         // invalidate roles cache so that any changes to the underlying roles are picked up
         Roles.clearCache();
-        authStmt.execute(getClientState());
+        authStmt.authorize(clientState);
+        authStmt.execute(clientState);
     }
 
     private static DCPermissions dcPerms(String username)
@@ -170,6 +185,27 @@ public void alter()
         assertDcPermRow(username);
     }
 
+    @Test
+    public void alterAsUser()
+    {
+        String username = createName();
+
+        assertNoDcPermRow(username);
+        auth("CREATE ROLE %s WITH PASSWORD = 'password' AND LOGIN = true AND ACCESS TO DATACENTERS {'dc1'}", username);
+        Assert.assertEquals(DCPermissions.subset("dc1"), dcPerms(username));
+        assertDcPermRow(username, "dc1");
+
+        // try to alter as a user
+        ClientState userState = getClientState(username);
+        Assertions.assertThatThrownBy(() -> auth("ALTER ROLE %s WITH ACCESS TO DATACENTERS {'dc1', 'dc2'}", userState, username))
+        .hasMessage("Only superusers are allowed to alter access to datacenters.")
+        .isInstanceOf(UnauthorizedException.class);
+
+        // nothing changed
+        Assert.assertEquals(DCPermissions.subset("dc1"), dcPerms(username));
+        assertDcPermRow(username, "dc1");
+    }
+
     @Test
     public void drop()
     {