frankgh commented on code in PR #223:
URL: https://github.com/apache/cassandra-sidecar/pull/223#discussion_r2136682699
##########
server/src/main/java/org/apache/cassandra/sidecar/acl/authentication/MutualTlsAuthenticationHandler.java:
##########
@@ -88,10 +90,18 @@ public void authenticate(RoutingContext ctx,
Handler<AsyncResult<User>> handler)
List<String> identities =
extractIdentities(authN.result());
List<String> roles = extractCassandraRoles(identities);
- if (!roles.isEmpty())
+ String roleIntended =
ctx.request().getHeader(AUTH_ROLE);
+
+ if (roleIntended != null && !roleIntended.isEmpty() &&
!roles.contains(roleIntended))
{
-
authN.result().attributes().put(CASSANDRA_ROLES_ATTRIBUTE_NAME, roles);
+ String errMsg = String.format("User not authorized
for role %s", roleIntended);
+
handler.handle(Future.failedFuture(wrapHttpException(UNAUTHORIZED, errMsg)));
+ return;
}
+
+ List<String> rolesToAdd = roleIntended != null &&
!roleIntended.isEmpty()
+ ?
Collections.singletonList(roleIntended) : roles;
Review Comment:
NIT: now that we have java 11
```suggestion
? List.of(roleIntended) :
roles;
```
##########
server/src/main/java/org/apache/cassandra/sidecar/acl/authentication/MutualTlsAuthenticationHandler.java:
##########
@@ -88,10 +90,18 @@ public void authenticate(RoutingContext ctx,
Handler<AsyncResult<User>> handler)
List<String> identities =
extractIdentities(authN.result());
List<String> roles = extractCassandraRoles(identities);
- if (!roles.isEmpty())
+ String roleIntended =
ctx.request().getHeader(AUTH_ROLE);
+
+ if (roleIntended != null && !roleIntended.isEmpty() &&
!roles.contains(roleIntended))
{
-
authN.result().attributes().put(CASSANDRA_ROLES_ATTRIBUTE_NAME, roles);
+ String errMsg = String.format("User not authorized
for role %s", roleIntended);
Review Comment:
I think we should give more information here. Both pieces of information are
provided by the user, identities + role
```suggestion
String errMsg = String.format("None of the
identities %s are authorized for role %s", identities, roleIntended);
```
##########
vertx-client/src/main/java/org/apache/cassandra/sidecar/client/VertxHttpClient.java:
##########
@@ -272,6 +276,17 @@ protected HttpRequest<Buffer>
applyHeaders(HttpRequest<Buffer> vertxRequest, Map
return vertxRequest;
}
+ protected HttpRequest<Buffer> applyAuthHeader(HttpRequest<Buffer>
vertxRequest)
+ {
+ String cassandraRole = config().cassandraRole();
+ if (cassandraRole == null || cassandraRole.isEmpty())
Review Comment:
NIT, use
`org.apache.cassandra.sidecar.common.utils.StringUtils#isNullOrEmpty` instead
--
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-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
