This is an automated email from the ASF dual-hosted git repository. ycai pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/cassandra-sidecar.git
The following commit(s) were added to refs/heads/trunk by this push: new 33525e24 CASSSIDECAR-333: Fix a bug that reads member_of field as a Set instead of List (#246) 33525e24 is described below commit 33525e244909b80581ac613a0e7b5017e139c562 Author: Saranya Krishnakumar <sarany...@apple.com> AuthorDate: Thu Aug 7 09:44:05 2025 -0700 CASSSIDECAR-333: Fix a bug that reads member_of field as a Set instead of List (#246) Patch by Saranya Krishnakumar; Reviewed by Francisco Guerrero, Yifan Cai for CASSSIDECAR-333 --- CHANGES.txt | 1 + .../apache/cassandra/sidecar/acl/AuthCache.java | 13 +++--- .../sidecar/db/SystemAuthDatabaseAccessor.java | 4 +- .../MutualTLSAuthenticationIntegrationTest.java | 6 --- .../acl/RoleBasedAuthorizationIntegrationTest.java | 16 +++++++- .../db/SystemAuthDatabaseAccessorIntTest.java | 47 ++++++++++++++++++++++ .../sidecar/testing/IntegrationTestBase.java | 6 +++ 7 files changed, 79 insertions(+), 14 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index fe9f4393..aaa4ea00 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,6 @@ 0.2.0 ----- + * Fix type used for reading member_of column in SystemAuthDatabaseAccessor (CASSSIDECAR-333) * Remove Jolokia agent (CASSSIDECAR-330) * Update Google Guice to 7.0.0 (CASSSIDECAR-89) * Fix rpm broken symlink (CASSSIDECAR-327) diff --git a/server/src/main/java/org/apache/cassandra/sidecar/acl/AuthCache.java b/server/src/main/java/org/apache/cassandra/sidecar/acl/AuthCache.java index a2f6a4b0..21e867df 100644 --- a/server/src/main/java/org/apache/cassandra/sidecar/acl/AuthCache.java +++ b/server/src/main/java/org/apache/cassandra/sidecar/acl/AuthCache.java @@ -46,7 +46,7 @@ import static org.apache.cassandra.sidecar.server.SidecarServerEvents.ON_SIDECAR */ public abstract class AuthCache<K, V> { - private static final Logger LOGGER = LoggerFactory.getLogger(AuthCache.class); + private final Logger logger = LoggerFactory.getLogger(this.getClass()); private final String name; private final Function<K, V> loadFunction; private final Supplier<Map<K, V>> bulkLoadFunction; @@ -150,27 +150,30 @@ public abstract class AuthCache<K, V> { if (!config.enabled()) { - LOGGER.info("Cache={} not enabled, skipping pre-warming", name); + logger.info("Cache={} not enabled, skipping pre-warming", name); return; } if (availableRetries < 1) { - LOGGER.warn("Retries exhausted, unexpected error pre-warming cache={}", name); + logger.warn("Retries exhausted, unexpected error pre-warming cache={}", name); return; } try { + long startTimeNanos = System.nanoTime(); cache.putAll(bulkLoadFunction.get()); + logger.info("Cache={} warmup completed successfully in {} nanoseconds", + name, System.nanoTime() - startTimeNanos); } catch (SchemaUnavailableException sue) { - LOGGER.warn("system_auth schema is unavailable. Skip warming up cache", sue); + logger.warn("system_auth schema is unavailable. Skip warming up cache", sue); } catch (Exception e) { - LOGGER.warn("Unexpected error encountered during pre-warming of cache={} ", name, e); + logger.warn("Unexpected error encountered during pre-warming of cache={} ", name, e); vertx.setTimer(config.warmupRetryInterval().toMillis(), t -> warmUpAsync(availableRetries - 1)); } } diff --git a/server/src/main/java/org/apache/cassandra/sidecar/db/SystemAuthDatabaseAccessor.java b/server/src/main/java/org/apache/cassandra/sidecar/db/SystemAuthDatabaseAccessor.java index a212e1a1..b79da180 100644 --- a/server/src/main/java/org/apache/cassandra/sidecar/db/SystemAuthDatabaseAccessor.java +++ b/server/src/main/java/org/apache/cassandra/sidecar/db/SystemAuthDatabaseAccessor.java @@ -171,8 +171,8 @@ public class SystemAuthDatabaseAccessor extends DatabaseAccessor<SystemAuthSchem } // check if superuser status has been granted indirectly to this user - List<String> memberOf = row.getList("member_of", String.class); - if (memberOf.stream().anyMatch(roleToSuperUser::get)) + Set<String> memberOf = row.getSet("member_of", String.class); + if (memberOf != null && !memberOf.isEmpty() && memberOf.stream().anyMatch(roleToSuperUser::get)) { roleToSuperUser.put(row.getString("role"), true); } diff --git a/server/src/test/integration/org/apache/cassandra/sidecar/acl/MutualTLSAuthenticationIntegrationTest.java b/server/src/test/integration/org/apache/cassandra/sidecar/acl/MutualTLSAuthenticationIntegrationTest.java index f189071f..52fc0e8e 100644 --- a/server/src/test/integration/org/apache/cassandra/sidecar/acl/MutualTLSAuthenticationIntegrationTest.java +++ b/server/src/test/integration/org/apache/cassandra/sidecar/acl/MutualTLSAuthenticationIntegrationTest.java @@ -267,10 +267,4 @@ class MutualTLSAuthenticationIntegrationTest extends IntegrationTestBase session.execute(String.format("INSERT INTO sidecar_internal.role_permissions_v1 (role, resource, permissions) " + "VALUES ('%s', '%s', {'%s'})", role, resource, permission)); } - - private void grantRole(String role, String roleToAssign) - { - Session session = maybeGetSession(); - session.execute(String.format("GRANT %s TO %s", roleToAssign, role)); - } } diff --git a/server/src/test/integration/org/apache/cassandra/sidecar/acl/RoleBasedAuthorizationIntegrationTest.java b/server/src/test/integration/org/apache/cassandra/sidecar/acl/RoleBasedAuthorizationIntegrationTest.java index d2b2226f..eaab0782 100644 --- a/server/src/test/integration/org/apache/cassandra/sidecar/acl/RoleBasedAuthorizationIntegrationTest.java +++ b/server/src/test/integration/org/apache/cassandra/sidecar/acl/RoleBasedAuthorizationIntegrationTest.java @@ -69,13 +69,14 @@ class RoleBasedAuthorizationIntegrationTest extends IntegrationTestBase // wait for cache refreshes Thread.sleep(3000); - testCompleteLatch = new CountDownLatch(35); + testCompleteLatch = new CountDownLatch(36); // permissions for test cases below are granted during prepareForTest to save cache refresh time. Please // refer to grantRequiredPermissions to check permissions granted for a test to understand verifications done in // test testForAdmin(context); testForSuperUser(context); + testForTransitiveSuperUser(context); testForNonAdmin(context); testGrantingForTable(context); testGrantingForKeyspace(context); @@ -112,6 +113,15 @@ class RoleBasedAuthorizationIntegrationTest extends IntegrationTestBase verifyAccess(context, testCompleteLatch, HttpMethod.GET, keyspaceSchemaRoute, clientKeystorePath, false); } + void testForTransitiveSuperUser(VertxTestContext context) throws Exception + { + String keyspaceSchemaRoute = String.format("/api/v1/keyspaces/%s/schema", "test_keyspace"); + // uses client keystore with superuser identity + Path clientKeystorePath = clientKeystorePath("spiffe://cassandra/sidecar/non_super_user_with_transitive_super_user"); + + verifyAccess(context, testCompleteLatch, HttpMethod.GET, keyspaceSchemaRoute, clientKeystorePath, false); + } + void testForNonAdmin(VertxTestContext context) { String keyspaceSchemaRoute = String.format("/api/v1/keyspaces/%s/schema", "non_admin_test_keyspace"); @@ -447,6 +457,10 @@ class RoleBasedAuthorizationIntegrationTest extends IntegrationTestBase createRole("super_user_test_role", true); insertIdentityRole(cassandraContext, "spiffe://cassandra/sidecar/super_user_test_user", "super_user_test_role"); + createRole("non_super_user_with_transitive_super_user_role", false); + grantRole("non_super_user_with_transitive_super_user_role", "super_user_test_role"); + insertIdentityRole(cassandraContext, "spiffe://cassandra/sidecar/non_super_user_with_transitive_super_user", "non_super_user_with_transitive_super_user_role"); + createRole("non_admin_test_role", false); insertIdentityRole(cassandraContext, "spiffe://cassandra/sidecar/non_admin_test_user", "non_admin_test_role"); diff --git a/server/src/test/integration/org/apache/cassandra/sidecar/db/SystemAuthDatabaseAccessorIntTest.java b/server/src/test/integration/org/apache/cassandra/sidecar/db/SystemAuthDatabaseAccessorIntTest.java new file mode 100644 index 00000000..df22a8c9 --- /dev/null +++ b/server/src/test/integration/org/apache/cassandra/sidecar/db/SystemAuthDatabaseAccessorIntTest.java @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.sidecar.db; + +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import org.apache.cassandra.sidecar.testing.IntegrationTestBase; +import org.apache.cassandra.testing.AuthMode; +import org.apache.cassandra.testing.CassandraIntegrationTest; + +import static org.assertj.core.api.Assertions.assertThat; + +class SystemAuthDatabaseAccessorIntTest extends IntegrationTestBase +{ + @CassandraIntegrationTest(authMode = AuthMode.PASSWORD) + void testCrudOperations() + { + waitForSchemaReady(10, TimeUnit.SECONDS); + + createRole("super_user_role", true); + createRole("non_super_user_role", false); + grantRole("non_super_user_role", "super_user_role"); + + SystemAuthDatabaseAccessor accessor = injector.getInstance(SystemAuthDatabaseAccessor.class); + Map<String, Boolean> actualSuperUsers = accessor.findAllRolesToSuperuserStatus(); + assertThat(actualSuperUsers.size()).isEqualTo(3); + assertThat(actualSuperUsers.get("super_user_role")).isTrue(); + assertThat(actualSuperUsers.get("non_super_user_role")).isTrue(); + } +} diff --git a/server/src/test/integration/org/apache/cassandra/sidecar/testing/IntegrationTestBase.java b/server/src/test/integration/org/apache/cassandra/sidecar/testing/IntegrationTestBase.java index 1ed60f78..ae69ca2d 100644 --- a/server/src/test/integration/org/apache/cassandra/sidecar/testing/IntegrationTestBase.java +++ b/server/src/test/integration/org/apache/cassandra/sidecar/testing/IntegrationTestBase.java @@ -401,6 +401,12 @@ public abstract class IntegrationTestBase session.execute("CREATE ROLE \"" + role + "\" WITH PASSWORD ='" + password + "' AND SUPERUSER = " + superUser + " AND LOGIN = true;"); } + protected void grantRole(String role, String roleToAssign) + { + Session session = maybeGetSession(); + session.execute("GRANT " + roleToAssign + " TO " + role + ";"); + } + // similar to awaitLatchOrTimeout, it throws either test exceptions (due to startAsync failures) or timeout exception public void awaitLatchOrThrow(CountDownLatch latch, long duration, TimeUnit timeUnit, String latchName) { --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org