yifan-c commented on code in PR #145: URL: https://github.com/apache/cassandra-sidecar/pull/145#discussion_r1833412360
########## server-common/src/main/java/org/apache/cassandra/sidecar/db/schema/CassandraSystemTableSchema.java: ########## @@ -0,0 +1,52 @@ +/* + * 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.schema; + +import com.datastax.driver.core.Metadata; +import com.datastax.driver.core.Session; +import org.jetbrains.annotations.NotNull; + +/** + * Schema for existing tables in Cassandra. + */ +public abstract class CassandraSystemTableSchema extends TableSchema +{ + @Override + protected boolean exists(@NotNull Metadata metadata) + { + return true; + } + + @Override + protected boolean initializeInternal(@NotNull Session session) + { + prepareStatements(session); + return true; + } + + /** + * Should not create new tables, since schema is for existing tables. + */ + @Override + Review Comment: nit: empty line ########## server/src/main/java/org/apache/cassandra/sidecar/db/SystemAuthDatabaseAccessor.java: ########## @@ -0,0 +1,89 @@ +/* + * 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.HashMap; +import java.util.Map; + +import com.datastax.driver.core.BoundStatement; +import com.datastax.driver.core.ResultSet; +import com.datastax.driver.core.Row; +import com.google.inject.Inject; +import com.google.inject.Singleton; +import org.apache.cassandra.sidecar.common.server.CQLSessionProvider; +import org.apache.cassandra.sidecar.db.schema.SystemAuthSchema; + +/** + * Database Accessor that queries cassandra to get information maintained under system_auth keyspace. + */ +@Singleton +public class SystemAuthDatabaseAccessor extends DatabaseAccessor<SystemAuthSchema> +{ + @Inject + public SystemAuthDatabaseAccessor(SystemAuthSchema systemAuthSchema, + CQLSessionProvider sessionProvider) + { + super(systemAuthSchema, sessionProvider); + } + + /** + * Queries Cassandra for the role associated with given identity. + * + * @param identity Identity of user extracted + * @return the role associated with the given identity in Cassandra + */ + public String findRoleFromIdentity(String identity) + { + if (!tableSchemaPrepared()) + { + throw new IllegalStateException("SystemAuthSchema was not prepared, values can not be retrieved from table"); + } + BoundStatement statement = tableSchema.selectRoleFromIdentity() + .bind(identity); + ResultSet result = execute(statement); + return result.one().getString("role"); + } + + /** + * Queries Cassandra for all rows in identity_to_role table + * + * @return - {@code List<Row>} containing each row in the identity to roles table + */ + public Map<String, String> findAllIdentityToRoles() + { + if (!tableSchemaPrepared()) + { + throw new IllegalStateException("SystemAuthSchema was not prepared, values can not be retrieved from table"); + } + BoundStatement statement = tableSchema.getAllRolesAndIdentities().bind(); + + ResultSet resultSet = execute(statement); + Map<String, String> results = new HashMap<>(); + for (Row row : resultSet) + { + results.put(row.getString("identity"), row.getString("role")); + } + return results; + } + + private boolean tableSchemaPrepared() + { + return tableSchema.selectRoleFromIdentity() != null && tableSchema.getAllRolesAndIdentities() != null; + } Review Comment: How about replacing with this method and update the callsites? ```suggestion private void ensureIdentityToRoleTableAccess() { if (tableSchema.selectRoleFromIdentity() == null || tableSchema.getAllRolesAndIdentities() == null) { throw new IllegalStateException("SystemAuthSchema was not prepared, values can not be retrieved from table"); } } ``` ########## server/src/main/java/org/apache/cassandra/sidecar/accesscontrol/authentication/MutualTlsAuthenticationHandler.java: ########## @@ -0,0 +1,58 @@ +/* + * 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.accesscontrol.authentication; + +import io.netty.handler.codec.http.HttpResponseStatus; +import io.vertx.core.AsyncResult; +import io.vertx.core.Future; +import io.vertx.core.Handler; +import io.vertx.ext.auth.User; +import io.vertx.ext.auth.authentication.CertificateCredentials; +import io.vertx.ext.auth.mtls.MutualTlsAuthentication; +import io.vertx.ext.web.RoutingContext; +import io.vertx.ext.web.handler.HttpException; +import io.vertx.ext.web.handler.impl.AuthenticationHandlerImpl; + +/** + * Handler for verifying user certificates for Mutual TLS authentication. {@link MutualTlsAuthenticationHandler} can be + * chained with other {@link io.vertx.ext.web.handler.AuthenticationHandler} implementations. + */ +public class MutualTlsAuthenticationHandler extends AuthenticationHandlerImpl<MutualTlsAuthentication> +{ + public MutualTlsAuthenticationHandler(MutualTlsAuthentication authProvider) + { + super(authProvider); + } + + @Override + public void authenticate(RoutingContext ctx, Handler<AsyncResult<User>> handler) + { + if (!ctx.request().isSSL()) + { + ctx.response().setStatusCode(HttpResponseStatus.BAD_REQUEST.code()).end(); + return; + } + + CertificateCredentials certificateCredentials = CertificateCredentials.fromHttpRequest(ctx.request()); + + authProvider.authenticate(certificateCredentials) + .onSuccess(user -> handler.handle(Future.succeededFuture(user))) + .onFailure(cause -> handler.handle(Future.failedFuture(new HttpException(HttpResponseStatus.UNAUTHORIZED.code(), cause)))); Review Comment: nit: alternative that is less wordy ```suggestion authProvider.authenticate(certificateCredentials) .recover(cause -> { // converts any exception to unauthorized http exception throw new HttpException(HttpResponseStatus.UNAUTHORIZED.code(), cause); }) .andThen(handler); ``` ########## server/src/main/java/org/apache/cassandra/sidecar/server/MainModule.java: ########## @@ -182,9 +194,63 @@ public Vertx vertx(SidecarConfiguration sidecarConfiguration, MetricRegistryFact return Vertx.vertx(vertxOptions); } + @Provides + @Singleton + public IdentityToRoleCache identityRoleCache(Vertx vertx, + SidecarConfiguration sidecarConfiguration, + SystemAuthDatabaseAccessor systemAuthDatabaseAccessor) + { + IdentityToRoleCache identityToRoleCache + = new IdentityToRoleCache(vertx, + sidecarConfiguration.accessControlConfiguration().permissionCacheConfiguration(), + systemAuthDatabaseAccessor); + return identityToRoleCache; + } + + @Provides + @Singleton + public ChainAuthHandler chainAuthHandler(Vertx vertx, + SidecarConfiguration sidecarConfiguration, + IdentityToRoleCache identityToRoleCache) + { + ChainAuthHandler chainAuthHandler = ChainAuthHandler.any(); + try + { + MutualTlsAuthenticatorConfiguration mTLSAuthenticatorConfig = sidecarConfiguration + .accessControlConfiguration() + .authenticatorsConfiguration() + .mTlsAuthenticatorConfiguration(); + if (mTLSAuthenticatorConfig.enabled()) + { + CertificateValidator certificateValidator = (CertificateValidator) Class.forName(mTLSAuthenticatorConfig.certificateValidator()).newInstance(); + CertificateIdentityExtractor certificateIdentityExtractor; + if (mTLSAuthenticatorConfig.certificateIdentityExtractor().equalsIgnoreCase(CassandraIdentityExtractor.class.getName())) + { + certificateIdentityExtractor + = new CassandraIdentityExtractor(identityToRoleCache, sidecarConfiguration.accessControlConfiguration().adminIdentities()); + } + else + { + certificateIdentityExtractor = new SpiffeIdentityExtractor(); + } Review Comment: nit: `CertificateIdentityExtractor` can be created the same way as `CertificateValidator` Not a big concern. Feel free to ignore. ########## server/src/main/java/org/apache/cassandra/sidecar/db/schema/SystemAuthSchema.java: ########## @@ -0,0 +1,86 @@ +/* + * 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.schema; + +import com.datastax.driver.core.KeyspaceMetadata; +import com.datastax.driver.core.PreparedStatement; +import com.datastax.driver.core.Session; +import org.jetbrains.annotations.NotNull; + +/** + * Schema for getting information stored in system_auth keyspace. + */ +public class SystemAuthSchema extends CassandraExistingSchema +{ + private static final String IDENTITY_TO_ROLE_TABLE = "identity_to_role"; + private PreparedStatement selectRoleFromIdentity; + private PreparedStatement getAllRolesAndIdentities; + + protected String keyspaceName() + { + return "system_auth"; + } + + @Override + protected void prepareStatements(@NotNull Session session) + { + KeyspaceMetadata keyspaceMetadata = session.getCluster().getMetadata().getKeyspace(keyspaceName()); + // identity_to_role table exists in Cassandra versions starting 5.x + if (keyspaceMetadata == null || keyspaceMetadata.getTable(IDENTITY_TO_ROLE_TABLE) == null) + { + return; + } + selectRoleFromIdentity = prepare(selectRoleFromIdentity, + session, + CqlLiterals.selectRoleFromIdentity()); + + getAllRolesAndIdentities = prepare(getAllRolesAndIdentities, + session, + CqlLiterals.getAllRolesAndIdentities()); + } + + protected String tableName() + { + throw new UnsupportedOperationException("SystemAuthSchema supports reading information from multiple " + + "tables in system_auth keyspace"); + } + + public PreparedStatement selectRoleFromIdentity() + { + return selectRoleFromIdentity; + } + + public PreparedStatement getAllRolesAndIdentities() + { + return getAllRolesAndIdentities; + } Review Comment: nit: add `@Nullable` for the return value ########## server/src/main/java/org/apache/cassandra/sidecar/accesscontrol/AuthCache.java: ########## @@ -0,0 +1,157 @@ +/* + * 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.accesscontrol; + +import java.util.Collections; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; +import java.util.function.Supplier; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.LoadingCache; +import io.vertx.core.Vertx; +import io.vertx.core.eventbus.EventBus; +import org.apache.cassandra.sidecar.config.CacheConfiguration; +import org.jetbrains.annotations.VisibleForTesting; + +import static com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly; +import static org.apache.cassandra.sidecar.server.SidecarServerEvents.ON_ALL_CASSANDRA_CQL_READY; + +/** + * Caches information needed for authenticating sidecar users. + * + * @param <K> Key type + * @param <V> Value type + */ +public abstract class AuthCache<K, V> +{ + protected static final Logger LOGGER = LoggerFactory.getLogger(AuthCache.class); + protected final String name; + protected final Vertx vertx; + protected final Function<K, V> loadFunction; + protected final Supplier<Map<K, V>> bulkLoadFunction; + protected final CacheConfiguration config; + + // cache is null when AuthCache is disabled + protected volatile LoadingCache<K, V> cache; + + protected AuthCache(String name, + Vertx vertx, + Function<K, V> loadFunction, + Supplier<Map<K, V>> bulkLoadFunction, + CacheConfiguration cacheConfiguration) + { + this.name = name; + this.vertx = vertx; + this.loadFunction = loadFunction; + this.bulkLoadFunction = bulkLoadFunction; + this.config = cacheConfiguration; + this.cache = initCache(); + + if (this.config.enabled()) + { + configureSidecarServerEventListener(); + } + } + + /** + * Retrieves a value from the cache. Will call {@link LoadingCache#get(Object)} which will + * "load" the value if it's not present, thus populating the key. When the cache is disabled, data is fetched + * with loadFunction. + * + * @param k key + * @return The current value of {@code K} if cached or loaded. + * + * See {@link LoadingCache#get(Object)} for possible exceptions. + */ + public V get(K k) + { + if (!config.enabled()) + { + return loadFunction.apply(k); + } + return cache.get(k); + } + + /** + * Retrieves all cached entries. Will call {@link LoadingCache#asMap()} which does not trigger "load". When cache + * is disabled, data is fetched with bulkLoadFunction. + * + * @return a map of cached key-value pairs + */ + public Map<K, V> getAll() + { + if (!config.enabled()) + { + return bulkLoadFunction.get(); + } + return Collections.unmodifiableMap(cache.asMap()); + } + + private LoadingCache<K, V> initCache() + { + if (!config.enabled()) Review Comment: nit: the condition can be combined with line#71 in the file. ########## server/src/test/integration/org/apache/cassandra/sidecar/testing/IntegrationTestModule.java: ########## @@ -47,13 +58,26 @@ */ public class IntegrationTestModule extends AbstractModule { + public static final String ADMIN_IDENTITY = "spiffe://cassandra/sidecar/admin"; Review Comment: nit: same string declared in multiple locations. maybe consolidate. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
