You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "sarankk (via GitHub)" <gi...@apache.org> on 2025/07/02 18:11:48 UTC
[PR] CASSSIDECAR-270: Avoid ending response in authentication handler, doesn't allow chaining of auth handlers [cassandra-sidecar]
sarankk opened a new pull request, #236:
URL: https://github.com/apache/cassandra-sidecar/pull/236
(no comment)
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-270: Avoid ending response in authentication handler, doesn't allow chaining of auth handlers [cassandra-sidecar]
Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c commented on code in PR #236:
URL: https://github.com/apache/cassandra-sidecar/pull/236#discussion_r2180999582
##########
server/src/test/java/org/apache/cassandra/sidecar/acl/authentication/MutualTLSAuthenticationHandlerTest.java:
##########
@@ -218,6 +233,47 @@ void testCertificateWithoutIdentity(VertxTestContext testContext) throws Excepti
}));
}
+ @Test
+ void testAuthHandlerChaining() throws Exception
+ {
+ Map<String, String> params = new HashMap<>()
+ {
+ {
+ put("certificate_validator", "io.vertx.ext.auth.mtls.impl.CertificateValidatorImpl");
+ put("certificate_identity_extractor", "org.apache.cassandra.sidecar.acl.authentication.CassandraIdentityExtractor");
+ }
+ };
+ ChainAuthHandlerImpl chainAuthHandlerSucceeds = (ChainAuthHandlerImpl) ChainAuthHandler.any();
+ MutualTlsAuthenticationHandlerFactory mTLSAuthFactory = injector.getInstance(MutualTlsAuthenticationHandlerFactory.class);
+ SidecarConfiguration config = injector.getInstance(SidecarConfiguration.class);
+ chainAuthHandlerSucceeds.add(mTLSAuthFactory.create(vertx, config.accessControlConfiguration(), params));
+ chainAuthHandlerSucceeds.add(new TestAuthHandler(new NoOpAuthentication(), true));
+
+ RoutingContext mockContext = mock(RoutingContext.class);
+ HttpServerRequest mockServerRequest = mock(HttpServerRequest.class);
+ HttpConnection mockHttpConnection = mock(HttpConnection.class);
+ when(mockServerRequest.connection()).thenReturn(mockHttpConnection);
+ when(mockContext.request()).thenReturn(mockServerRequest);
+
+ ChainAuthHandlerImpl chainAuthHandlerFails = (ChainAuthHandlerImpl) ChainAuthHandler.any();
+ chainAuthHandlerFails.add(mTLSAuthFactory.create(vertx, config.accessControlConfiguration(), params));
+ chainAuthHandlerFails.add(new TestAuthHandler(new NoOpAuthentication(), false));
+
+ CountDownLatch waitForResponse = new CountDownLatch(2);
+ // mTLS handler will fail without SSL, results succeeds with TestAuthHandler
+ chainAuthHandlerSucceeds.authenticate(mockContext, res -> {
+ assertThat(res.succeeded()).isTrue();
+ waitForResponse.countDown();
+ });
+
+ // response fails when TestAuthHandler fails too
+ chainAuthHandlerFails.authenticate(mockContext, res -> {
+ assertThat(res.failed()).isTrue();
Review Comment:
ha. this is fixed already.
##########
server/src/test/java/org/apache/cassandra/sidecar/acl/authentication/MutualTLSAuthenticationHandlerTest.java:
##########
@@ -218,6 +233,47 @@ void testCertificateWithoutIdentity(VertxTestContext testContext) throws Excepti
}));
}
+ @Test
+ void testAuthHandlerChaining() throws Exception
+ {
+ Map<String, String> params = new HashMap<>()
+ {
+ {
+ put("certificate_validator", "io.vertx.ext.auth.mtls.impl.CertificateValidatorImpl");
+ put("certificate_identity_extractor", "org.apache.cassandra.sidecar.acl.authentication.CassandraIdentityExtractor");
+ }
+ };
+ ChainAuthHandlerImpl chainAuthHandlerSucceeds = (ChainAuthHandlerImpl) ChainAuthHandler.any();
+ MutualTlsAuthenticationHandlerFactory mTLSAuthFactory = injector.getInstance(MutualTlsAuthenticationHandlerFactory.class);
+ SidecarConfiguration config = injector.getInstance(SidecarConfiguration.class);
+ chainAuthHandlerSucceeds.add(mTLSAuthFactory.create(vertx, config.accessControlConfiguration(), params));
+ chainAuthHandlerSucceeds.add(new TestAuthHandler(new NoOpAuthentication(), true));
+
+ RoutingContext mockContext = mock(RoutingContext.class);
+ HttpServerRequest mockServerRequest = mock(HttpServerRequest.class);
+ HttpConnection mockHttpConnection = mock(HttpConnection.class);
+ when(mockServerRequest.connection()).thenReturn(mockHttpConnection);
+ when(mockContext.request()).thenReturn(mockServerRequest);
+
+ ChainAuthHandlerImpl chainAuthHandlerFails = (ChainAuthHandlerImpl) ChainAuthHandler.any();
+ chainAuthHandlerFails.add(mTLSAuthFactory.create(vertx, config.accessControlConfiguration(), params));
+ chainAuthHandlerFails.add(new TestAuthHandler(new NoOpAuthentication(), false));
+
+ CountDownLatch waitForResponse = new CountDownLatch(2);
+ // mTLS handler will fail without SSL, results succeeds with TestAuthHandler
+ chainAuthHandlerSucceeds.authenticate(mockContext, res -> {
+ assertThat(res.succeeded()).isTrue();
+ waitForResponse.countDown();
+ });
+
+ // response fails when TestAuthHandler fails too
+ chainAuthHandlerFails.authenticate(mockContext, res -> {
+ assertThat(res.failed()).isFalse();
+ waitForResponse.countDown();
+ });
+ waitForResponse.await(1, TimeUnit.MINUTES);
Review Comment:
```suggestion
assertThat(waitForResponse.await(1, TimeUnit.MINUTES)).isTrue();
```
Otherwise, it just holds for 1 minutes and exit happily.
##########
server/src/main/java/org/apache/cassandra/sidecar/acl/authentication/MutualTlsAuthenticationHandler.java:
##########
@@ -63,17 +63,16 @@ public void authenticate(RoutingContext ctx, Handler<AsyncResult<User>> handler)
{
if (!ctx.request().isSSL())
{
- ctx.response().setStatusCode(HttpResponseStatus.BAD_REQUEST.code()).end();
+ handler.handle(Future.failedFuture(wrapHttpException(HttpResponseStatus.BAD_REQUEST,
Review Comment:
nit: add a comment to explain the expected behavior.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-270: Avoid ending response in authentication handler, doesn't allow chaining of auth handlers [cassandra-sidecar]
Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c merged PR #236:
URL: https://github.com/apache/cassandra-sidecar/pull/236
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-270: Avoid ending response in authentication handler, doesn't allow chaining of auth handlers [cassandra-sidecar]
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #236:
URL: https://github.com/apache/cassandra-sidecar/pull/236#discussion_r2181019150
##########
server/src/test/java/org/apache/cassandra/sidecar/acl/authentication/MutualTLSAuthenticationHandlerTest.java:
##########
@@ -218,6 +233,47 @@ void testCertificateWithoutIdentity(VertxTestContext testContext) throws Excepti
}));
}
+ @Test
+ void testAuthHandlerChaining() throws Exception
+ {
+ Map<String, String> params = new HashMap<>()
+ {
+ {
+ put("certificate_validator", "io.vertx.ext.auth.mtls.impl.CertificateValidatorImpl");
+ put("certificate_identity_extractor", "org.apache.cassandra.sidecar.acl.authentication.CassandraIdentityExtractor");
+ }
+ };
Review Comment:
Minor NIT
```suggestion
Map<String, String> params = Map.of("certificate_validator", "io.vertx.ext.auth.mtls.impl.CertificateValidatorImpl",
"certificate_identity_extractor", "org.apache.cassandra.sidecar.acl.authentication.CassandraIdentityExtractor");
```
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org