sarankk commented on code in PR #247:
URL: https://github.com/apache/cassandra-sidecar/pull/247#discussion_r2350207278
##########
server/src/main/java/org/apache/cassandra/sidecar/acl/authentication/ReloadingJwtAuthenticationHandler.java:
##########
@@ -72,14 +80,24 @@ public ReloadingJwtAuthenticationHandler(Vertx vertx,
this.vertx = vertx;
this.jwtParameters = jwtParameters;
this.roleProcessor = roleProcessor;
-
- periodicTaskExecutor.schedule(new OAuth2AuthHandlerGenerateTask());
+ if
(jwtParameters.jwtAuthType().equals(JwtParameters.AuthType.STATELESS))
+ {
+ periodicTaskExecutor.schedule(new
PeriodicStatelessJwtRefreshTask());
+ }
+ else if
(jwtParameters.jwtAuthType().equals(JwtParameters.AuthType.OAUTH))
+ {
+ periodicTaskExecutor.schedule(new OAuth2AuthHandlerGenerateTask());
+ }
+ else
+ {
+ throw new IllegalStateException("Unsupported JWT Auth Type: " +
jwtParameters.jwtAuthType());
+ }
}
@Override
public void authenticate(RoutingContext context,
Handler<AsyncResult<User>> handler)
{
- OAuth2AuthHandlerImpl oAuth2AuthHandler = delegateHandler.get();
+ AuthenticationHandlerInternal oAuth2AuthHandler =
delegateHandler.get();
Review Comment:
Nit: Rename to more generic variable name
```suggestion
AuthenticationHandlerInternal authHandler = delegateHandler.get();
```
##########
server/src/main/java/org/apache/cassandra/sidecar/acl/authentication/ReloadingJwtAuthenticationHandler.java:
##########
@@ -205,4 +229,55 @@ public void execute(Promise<Void> promise)
});
}
}
+
+ private class PeriodicStatelessJwtRefreshTask implements PeriodicTask
+ {
+ private final String taskName =
String.format("PeriodicStatelessJwtRefreshTask_%s", jwtParameters.site());
+ @Override
+ public DurationSpec delay()
+ {
+ return jwtParameters.configDiscoverInterval();
+ }
+
+ @Override
+ public DurationSpec initialDelay()
+ {
+ return SecondBoundConfiguration.ZERO;
+ }
+
+
+ @Override
+ public void execute(Promise<Void> promise)
+ {
+ WebClient webClient = WebClient.create(vertx, new
WebClientOptions().setSsl(true));
+ if (!jwtParameters.enabled())
+ {
+ delegateHandler.set(null);
+ promise.complete();
+ return;
+ }
+ String jwtPemUri = jwtParameters.site();
+ webClient.getAbs(jwtPemUri).send()
+ .onSuccess(ar -> {
+ String pem = ar.bodyAsString();
+ JWTAuthOptions jwtAuthOptions = new JWTAuthOptions()
+ .addPubSecKey(new
PubSecKeyOptions()
+
.setAlgorithm("RS256")
+
.setBuffer(pem));
+ JWTAuth auth = JWTAuth.create(vertx, jwtAuthOptions);
+ AuthenticationHandlerInternal jwtAuthHandlerDelegate =
new JWTAuthHandlerImpl(auth, null);
+ delegateHandler.set(jwtAuthHandlerDelegate);
+ promise.complete();
+ }).onFailure(cause -> {
+ LOGGER.error("Error encountered when refreshing
stateless JWT PEM material.", cause);
Review Comment:
Will be good to add metrics around successful and failed refreshes
##########
server/src/main/java/org/apache/cassandra/sidecar/acl/authentication/ReloadingJwtAuthenticationHandler.java:
##########
@@ -205,4 +229,55 @@ public void execute(Promise<Void> promise)
});
}
}
+
+ private class PeriodicStatelessJwtRefreshTask implements PeriodicTask
+ {
+ private final String taskName =
String.format("PeriodicStatelessJwtRefreshTask_%s", jwtParameters.site());
+ @Override
+ public DurationSpec delay()
+ {
+ return jwtParameters.configDiscoverInterval();
+ }
+
+ @Override
+ public DurationSpec initialDelay()
+ {
+ return SecondBoundConfiguration.ZERO;
+ }
+
+
+ @Override
+ public void execute(Promise<Void> promise)
+ {
+ WebClient webClient = WebClient.create(vertx, new
WebClientOptions().setSsl(true));
Review Comment:
Shall we create client at Periodic task level? Also do we need certs for
`SSL` connection?
##########
server/src/test/java/org/apache/cassandra/sidecar/acl/authentication/ReloadingJwtAuthenticationHandlerTest.java:
##########
@@ -75,4 +95,102 @@ void testDelegateHandlerNotCreatedWhenJWTDisabled()
assertThat(result.cause()).hasMessage("Service Unavailable");
});
}
+
+ @Test
+ void testStatelessJwtAuthenticationWithValidToken() throws Exception
+ {
+ // Generate a test RSA key pair and PEM
+ KeyPairGenerator keyGen = KeyPairGenerator.getInstance("RSA");
+ keyGen.initialize(2048);
+ KeyPair keyPair = keyGen.generateKeyPair();
+
+ String publicKeyPem = "-----BEGIN PUBLIC KEY-----\n" +
+
Base64.getEncoder().encodeToString(keyPair.getPublic().getEncoded()) +
+ "\n-----END PUBLIC KEY-----";
+
+ Vertx vertx = Vertx.vertx();
+ try
+ {
+ // Mock HTTP server to serve the PEM
+ HttpServer mockServer = vertx.createHttpServer();
+ CountDownLatch serverLatch = new CountDownLatch(1);
+
+ mockServer.requestHandler(request -> {
+ request.response()
+ .putHeader("Content-Type", "text/plain")
+ .end(publicKeyPem);
+ }).listen(8080, result -> serverLatch.countDown());
Review Comment:
Can we get port dynamically or use port `0` here?
##########
server/src/test/java/org/apache/cassandra/sidecar/acl/authentication/ReloadingJwtAuthenticationHandlerTest.java:
##########
@@ -75,4 +95,102 @@ void testDelegateHandlerNotCreatedWhenJWTDisabled()
assertThat(result.cause()).hasMessage("Service Unavailable");
});
}
+
+ @Test
+ void testStatelessJwtAuthenticationWithValidToken() throws Exception
+ {
+ // Generate a test RSA key pair and PEM
+ KeyPairGenerator keyGen = KeyPairGenerator.getInstance("RSA");
+ keyGen.initialize(2048);
+ KeyPair keyPair = keyGen.generateKeyPair();
+
+ String publicKeyPem = "-----BEGIN PUBLIC KEY-----\n" +
+
Base64.getEncoder().encodeToString(keyPair.getPublic().getEncoded()) +
+ "\n-----END PUBLIC KEY-----";
+
+ Vertx vertx = Vertx.vertx();
+ try
+ {
+ // Mock HTTP server to serve the PEM
+ HttpServer mockServer = vertx.createHttpServer();
+ CountDownLatch serverLatch = new CountDownLatch(1);
+
+ mockServer.requestHandler(request -> {
+ request.response()
+ .putHeader("Content-Type", "text/plain")
+ .end(publicKeyPem);
+ }).listen(8080, result -> serverLatch.countDown());
+
+ serverLatch.await(5, TimeUnit.SECONDS);
+
+ // Configure for stateless authentication
+ JwtParameterExtractor parameterExtractor = new
JwtParameterExtractor(Map.of("enabled", "true",
+
"site", "http://localhost:8080/jwks";,
+
"jwt_auth_type",
JwtParameters.AuthType.STATELESS.toString().toLowerCase()));
+ JwtRoleProcessor mockRoleProcessor = mock(JwtRoleProcessor.class);
+
when(mockRoleProcessor.processRoles(any())).thenReturn(List.of("test_role"));
+ ReloadingJwtAuthenticationHandler handler =
getReloadingJwtAuthenticationHandler(vertx, parameterExtractor,
mockRoleProcessor);
+
+ // Wait a bit for the handler to be set
+ Thread.sleep(1000);
Review Comment:
Nit: To avoid wait, we could add a loopAssert here
```suggestion
loopAssert(1, () ->
assertNotNull(handler.delegateHandler.get()));
```
--
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]
