This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 2719c5c9bc Returning tables names failing authorization in Exception
of Multi State Engine Queries (#13195)
2719c5c9bc is described below
commit 2719c5c9bc695d5257a1b5309c951e4a8b7dac9a
Author: Eaugene Thomas <[email protected]>
AuthorDate: Tue Jun 4 05:31:12 2024 +0530
Returning tables names failing authorization in Exception of Multi State
Engine Queries (#13195)
---
.../org/apache/pinot/broker/api/AccessControl.java | 68 ++++++++++++++-
.../broker/AllowAllAccessControlFactory.java | 11 ++-
.../pinot/broker/broker/AuthenticationFilter.java | 14 +++-
.../broker/BasicAuthAccessControlFactory.java | 35 +++++---
.../broker/ZkBasicAuthAccessControlFactory.java | 26 +++---
.../requesthandler/BaseBrokerRequestHandler.java | 12 ++-
.../BaseSingleStageBrokerRequestHandler.java | 20 +++--
.../MultiStageBrokerRequestHandler.java | 48 +++++++++--
.../api/AccessControlBackwardCompatibleTest.java | 97 ++++++++++++++++++++++
.../broker/broker/BasicAuthAccessControlTest.java | 50 +++++++----
.../pinot/core/auth/FineGrainedAccessControl.java | 18 ++++
.../apache/pinot/spi/auth/AuthorizationResult.java | 41 +++++++++
.../spi/auth/BasicAuthorizationResultImpl.java | 83 ++++++++++++++++++
.../pinot/spi/auth/TableAuthorizationResult.java | 74 +++++++++++++++++
.../spi/auth/BasicAuthorizationResultImplTest.java | 61 ++++++++++++++
.../spi/auth/TableAuthorizationResultTest.java | 68 +++++++++++++++
16 files changed, 660 insertions(+), 66 deletions(-)
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/api/AccessControl.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/api/AccessControl.java
index 85c1bf74ae..f18fcc7f41 100644
--- a/pinot-broker/src/main/java/org/apache/pinot/broker/api/AccessControl.java
+++ b/pinot-broker/src/main/java/org/apache/pinot/broker/api/AccessControl.java
@@ -23,6 +23,9 @@ import org.apache.pinot.common.request.BrokerRequest;
import org.apache.pinot.core.auth.FineGrainedAccessControl;
import org.apache.pinot.spi.annotations.InterfaceAudience;
import org.apache.pinot.spi.annotations.InterfaceStability;
+import org.apache.pinot.spi.auth.AuthorizationResult;
+import org.apache.pinot.spi.auth.BasicAuthorizationResultImpl;
+import org.apache.pinot.spi.auth.TableAuthorizationResult;
@InterfaceAudience.Public
@@ -31,32 +34,89 @@ public interface AccessControl extends
FineGrainedAccessControl {
/**
* First-step access control when processing broker requests. Decides
whether request is allowed to acquire resources
* for further processing. Request may still be rejected at table-level
later on.
- *
+ * The default implementation is kept to have backward compatibility with
the existing implementations
* @param requesterIdentity requester identity
*
* @return {@code true} if authorized, {@code false} otherwise
*/
+ @Deprecated
default boolean hasAccess(RequesterIdentity requesterIdentity) {
return true;
}
/**
- * Fine-grained access control on parsed broker request. May check table,
column, permissions, etc.
+ * First-step access control when processing broker requests. Decides
whether request is allowed to acquire resources
+ * for further processing. Request may still be rejected at table-level
later on.
+ * The default implementation returns a {@link BasicAuthorizationResultImpl}
with the result of the hasAccess() of
+ * the implementation
*
* @param requesterIdentity requester identity
+ *
+ * @return {@code AuthorizationResult} with the result of the access control
check
+ */
+ default AuthorizationResult authorize(RequesterIdentity requesterIdentity) {
+ return new BasicAuthorizationResultImpl(hasAccess(requesterIdentity));
+ }
+
+ /**
+ * Fine-grained access control on parsed broker request. May check table,
column, permissions, etc.
+ * The default implementation is kept to have backward compatibility with
the existing implementations
+ * @param requesterIdentity requester identity
* @param brokerRequest broker request (incl query)
*
* @return {@code true} if authorized, {@code false} otherwise
*/
- boolean hasAccess(RequesterIdentity requesterIdentity, BrokerRequest
brokerRequest);
+ @Deprecated
+ default boolean hasAccess(RequesterIdentity requesterIdentity, BrokerRequest
brokerRequest) {
+ throw new UnsupportedOperationException(
+ "Both hasAccess() and authorize() are not implemented . Do implement
authorize() method for new "
+ + "implementations.");
+ }
+
+ /**
+ * Verify access control on parsed broker request. May check table, column,
permissions, etc.
+ * The default implementation returns a {@link BasicAuthorizationResultImpl}
with the result of the hasAccess() of
+ * the implementation
+ *
+ * @param requesterIdentity requester identity
+ * @param brokerRequest broker request (incl query)
+ *
+ * @return {@code AuthorizationResult} with the result of the access control
check
+ */
+ default AuthorizationResult authorize(RequesterIdentity requesterIdentity,
BrokerRequest brokerRequest) {
+ return new BasicAuthorizationResultImpl(hasAccess(requesterIdentity,
brokerRequest));
+ }
/**
* Fine-grained access control on pinot tables.
+ * The default implementation is kept to have backward compatibility with
the existing implementations
*
* @param requesterIdentity requester identity
* @param tables Set of pinot tables used in the query. Table name can be
with or without tableType.
*
* @return {@code true} if authorized, {@code false} otherwise
*/
- boolean hasAccess(RequesterIdentity requesterIdentity, Set<String> tables);
+ @Deprecated
+ default boolean hasAccess(RequesterIdentity requesterIdentity, Set<String>
tables) {
+ throw new UnsupportedOperationException(
+ "Both hasAccess() and authorize() are not implemented . Do implement
authorize() method for new "
+ + "implementations.");
+ }
+
+ /**
+ * Verify access control on pinot tables.
+ * The default implementation returns a {@link TableAuthorizationResult}
with the result of the hasAccess() of the
+ * implementation
+ *
+ * @param requesterIdentity requester identity
+ * @param tables Set of pinot tables used in the query. Table name can be
with or without tableType.
+ *
+ * @return {@code TableAuthorizationResult} with the result of the access
control check
+ */
+ default TableAuthorizationResult authorize(RequesterIdentity
requesterIdentity, Set<String> tables) {
+ // Taking all tables when hasAccess Failed , to not break existing
implementations
+ // It will say all tables names failed AuthZ even only some failed AuthZ -
which is same as just boolean output
+ return hasAccess(requesterIdentity, tables) ?
TableAuthorizationResult.success()
+ : new TableAuthorizationResult(tables);
+ }
}
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/AllowAllAccessControlFactory.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/AllowAllAccessControlFactory.java
index 1e5a888a66..d0eb2ed65c 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/AllowAllAccessControlFactory.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/AllowAllAccessControlFactory.java
@@ -22,6 +22,9 @@ import java.util.Set;
import org.apache.pinot.broker.api.AccessControl;
import org.apache.pinot.broker.api.RequesterIdentity;
import org.apache.pinot.common.request.BrokerRequest;
+import org.apache.pinot.spi.auth.AuthorizationResult;
+import org.apache.pinot.spi.auth.BasicAuthorizationResultImpl;
+import org.apache.pinot.spi.auth.TableAuthorizationResult;
import org.apache.pinot.spi.env.PinotConfiguration;
@@ -41,13 +44,13 @@ public class AllowAllAccessControlFactory extends
AccessControlFactory {
private static class AllowAllAccessControl implements AccessControl {
@Override
- public boolean hasAccess(RequesterIdentity requesterIdentity,
BrokerRequest brokerRequest) {
- return true;
+ public AuthorizationResult authorize(RequesterIdentity requesterIdentity,
BrokerRequest brokerRequest) {
+ return BasicAuthorizationResultImpl.success();
}
@Override
- public boolean hasAccess(RequesterIdentity requesterIdentity, Set<String>
tables) {
- return true;
+ public TableAuthorizationResult authorize(RequesterIdentity
requesterIdentity, Set<String> tables) {
+ return TableAuthorizationResult.success();
}
}
}
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/AuthenticationFilter.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/AuthenticationFilter.java
index 787a9da685..a61a860356 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/AuthenticationFilter.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/AuthenticationFilter.java
@@ -34,10 +34,12 @@ import javax.ws.rs.core.Context;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo;
+import org.apache.commons.lang.StringUtils;
import org.apache.pinot.broker.api.AccessControl;
import org.apache.pinot.broker.api.HttpRequesterIdentity;
import org.apache.pinot.core.auth.FineGrainedAuthUtils;
import org.apache.pinot.core.auth.ManualAuthorization;
+import org.apache.pinot.spi.auth.AuthorizationResult;
import org.glassfish.grizzly.http.server.Request;
/**
@@ -82,9 +84,15 @@ public class AuthenticationFilter implements
ContainerRequestFilter {
HttpRequesterIdentity httpRequestIdentity =
HttpRequesterIdentity.fromRequest(request);
- // default authorization handling
- if (!accessControl.hasAccess(httpRequestIdentity)) {
- throw new WebApplicationException("Failed access check for " +
httpRequestIdentity.getEndpointUrl(),
+ AuthorizationResult authorizationResult =
accessControl.authorize(httpRequestIdentity);
+
+ if (!authorizationResult.hasAccess()) {
+ String failureMessage = authorizationResult.getFailureMessage();
+ if (StringUtils.isNotBlank(failureMessage)) {
+ failureMessage = "Reason: " + failureMessage;
+ }
+ throw new WebApplicationException(
+ "Failed access check for " + httpRequestIdentity.getEndpointUrl() +
"." + failureMessage,
Response.Status.FORBIDDEN);
}
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/BasicAuthAccessControlFactory.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/BasicAuthAccessControlFactory.java
index 2de2541d82..18c1932a52 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/BasicAuthAccessControlFactory.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/BasicAuthAccessControlFactory.java
@@ -20,6 +20,7 @@ package org.apache.pinot.broker.broker;
import com.google.common.base.Preconditions;
import java.util.Collection;
+import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
@@ -32,6 +33,8 @@ import org.apache.pinot.broker.api.RequesterIdentity;
import org.apache.pinot.common.request.BrokerRequest;
import org.apache.pinot.core.auth.BasicAuthPrincipal;
import org.apache.pinot.core.auth.BasicAuthUtils;
+import org.apache.pinot.spi.auth.AuthorizationResult;
+import org.apache.pinot.spi.auth.TableAuthorizationResult;
import org.apache.pinot.spi.env.PinotConfiguration;
@@ -78,12 +81,12 @@ public class BasicAuthAccessControlFactory extends
AccessControlFactory {
}
@Override
- public boolean hasAccess(RequesterIdentity requesterIdentity) {
- return hasAccess(requesterIdentity, (BrokerRequest) null);
+ public AuthorizationResult authorize(RequesterIdentity requesterIdentity) {
+ return authorize(requesterIdentity, (BrokerRequest) null);
}
@Override
- public boolean hasAccess(RequesterIdentity requesterIdentity,
BrokerRequest brokerRequest) {
+ public AuthorizationResult authorize(RequesterIdentity requesterIdentity,
BrokerRequest brokerRequest) {
Optional<BasicAuthPrincipal> principalOpt =
getPrincipalOpt(requesterIdentity);
if (!principalOpt.isPresent()) {
@@ -94,14 +97,22 @@ public class BasicAuthAccessControlFactory extends
AccessControlFactory {
if (brokerRequest == null || !brokerRequest.isSetQuerySource() ||
!brokerRequest.getQuerySource()
.isSetTableName()) {
// no table restrictions? accept
- return true;
+ return TableAuthorizationResult.success();
}
- return principal.hasTable(brokerRequest.getQuerySource().getTableName());
+ Set<String> failedTables = new HashSet<>();
+
+ if (!principal.hasTable(brokerRequest.getQuerySource().getTableName())) {
+ failedTables.add(brokerRequest.getQuerySource().getTableName());
+ }
+ if (failedTables.isEmpty()) {
+ return TableAuthorizationResult.success();
+ }
+ return new TableAuthorizationResult(failedTables);
}
@Override
- public boolean hasAccess(RequesterIdentity requesterIdentity, Set<String>
tables) {
+ public TableAuthorizationResult authorize(RequesterIdentity
requesterIdentity, Set<String> tables) {
Optional<BasicAuthPrincipal> principalOpt =
getPrincipalOpt(requesterIdentity);
if (!principalOpt.isPresent()) {
@@ -109,17 +120,19 @@ public class BasicAuthAccessControlFactory extends
AccessControlFactory {
}
if (tables == null || tables.isEmpty()) {
- return true;
+ return TableAuthorizationResult.success();
}
-
BasicAuthPrincipal principal = principalOpt.get();
+ Set<String> failedTables = new HashSet<>();
for (String table : tables) {
if (!principal.hasTable(table)) {
- return false;
+ failedTables.add(table);
}
}
-
- return true;
+ if (failedTables.isEmpty()) {
+ return TableAuthorizationResult.success();
+ }
+ return new TableAuthorizationResult(failedTables);
}
private Optional<BasicAuthPrincipal> getPrincipalOpt(RequesterIdentity
requesterIdentity) {
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/ZkBasicAuthAccessControlFactory.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/ZkBasicAuthAccessControlFactory.java
index 557484cc65..d8434a23a4 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/ZkBasicAuthAccessControlFactory.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/ZkBasicAuthAccessControlFactory.java
@@ -21,6 +21,7 @@ package org.apache.pinot.broker.broker;
import com.google.common.base.Preconditions;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
@@ -38,6 +39,8 @@ import org.apache.pinot.common.utils.BcryptUtils;
import org.apache.pinot.core.auth.BasicAuthPrincipal;
import org.apache.pinot.core.auth.BasicAuthUtils;
import org.apache.pinot.core.auth.ZkBasicAuthPrincipal;
+import org.apache.pinot.spi.auth.AuthorizationResult;
+import org.apache.pinot.spi.auth.TableAuthorizationResult;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
@@ -82,39 +85,42 @@ public class ZkBasicAuthAccessControlFactory extends
AccessControlFactory {
}
@Override
- public boolean hasAccess(RequesterIdentity requesterIdentity) {
- return hasAccess(requesterIdentity, (BrokerRequest) null);
+ public AuthorizationResult authorize(RequesterIdentity requesterIdentity) {
+ return authorize(requesterIdentity, (BrokerRequest) null);
}
@Override
- public boolean hasAccess(RequesterIdentity requesterIdentity,
BrokerRequest brokerRequest) {
+ public AuthorizationResult authorize(RequesterIdentity requesterIdentity,
BrokerRequest brokerRequest) {
if (brokerRequest == null || !brokerRequest.isSetQuerySource() ||
!brokerRequest.getQuerySource()
.isSetTableName()) {
// no table restrictions? accept
- return true;
+ return TableAuthorizationResult.success();
}
- return hasAccess(requesterIdentity,
Collections.singleton(brokerRequest.getQuerySource().getTableName()));
+ return authorize(requesterIdentity,
Collections.singleton(brokerRequest.getQuerySource().getTableName()));
}
@Override
- public boolean hasAccess(RequesterIdentity requesterIdentity, Set<String>
tables) {
+ public TableAuthorizationResult authorize(RequesterIdentity
requesterIdentity, Set<String> tables) {
Optional<ZkBasicAuthPrincipal> principalOpt =
getPrincipalAuth(requesterIdentity);
if (!principalOpt.isPresent()) {
throw new NotAuthorizedException("Basic");
}
if (tables == null || tables.isEmpty()) {
- return true;
+ return TableAuthorizationResult.success();
}
ZkBasicAuthPrincipal principal = principalOpt.get();
+ Set<String> failedTables = new HashSet<>();
for (String table : tables) {
if (!principal.hasTable(TableNameBuilder.extractRawTableName(table))) {
- return false;
+ failedTables.add(table);
}
}
-
- return true;
+ if (failedTables.isEmpty()) {
+ return TableAuthorizationResult.success();
+ }
+ return new TableAuthorizationResult(failedTables);
}
private Optional<ZkBasicAuthPrincipal> getPrincipalAuth(RequesterIdentity
requesterIdentity) {
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
index 309c9de11c..b43dcd9763 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
@@ -30,6 +30,7 @@ import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
+import org.apache.commons.lang3.StringUtils;
import org.apache.pinot.broker.api.AccessControl;
import org.apache.pinot.broker.api.RequesterIdentity;
import org.apache.pinot.broker.broker.AccessControlFactory;
@@ -42,6 +43,7 @@ import org.apache.pinot.common.metrics.BrokerMeter;
import org.apache.pinot.common.metrics.BrokerMetrics;
import org.apache.pinot.common.response.BrokerResponse;
import org.apache.pinot.common.response.broker.QueryProcessingException;
+import org.apache.pinot.spi.auth.AuthorizationResult;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.eventlistener.query.BrokerQueryEventListener;
import
org.apache.pinot.spi.eventlistener.query.BrokerQueryEventListenerFactory;
@@ -105,11 +107,17 @@ public abstract class BaseBrokerRequestHandler implements
BrokerRequestHandler {
// First-stage access control to prevent unauthenticated requests from
using up resources. Secondary table-level
// check comes later.
AccessControl accessControl = _accessControlFactory.create();
- if (!accessControl.hasAccess(requesterIdentity)) {
+ AuthorizationResult authorizationResult =
accessControl.authorize(requesterIdentity);
+ if (!authorizationResult.hasAccess()) {
_brokerMetrics.addMeteredGlobalValue(BrokerMeter.REQUEST_DROPPED_DUE_TO_ACCESS_ERROR,
1);
requestContext.setErrorCode(QueryException.ACCESS_DENIED_ERROR_CODE);
_brokerQueryEventListener.onQueryCompletion(requestContext);
- throw new WebApplicationException("Permission denied",
Response.Status.FORBIDDEN);
+ String failureMessage = authorizationResult.getFailureMessage();
+ if (StringUtils.isNotBlank(failureMessage)) {
+ failureMessage = "Reason: " + failureMessage;
+ }
+ throw new WebApplicationException("Permission denied." + failureMessage,
+ Response.Status.FORBIDDEN);
}
JsonNode sql = request.get(Broker.Request.SQL);
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java
index 9072f79932..5d6ee5bcc3 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java
@@ -83,6 +83,7 @@ import org.apache.pinot.core.routing.RoutingTable;
import org.apache.pinot.core.routing.TimeBoundaryInfo;
import org.apache.pinot.core.transport.ServerInstance;
import org.apache.pinot.core.util.GapfillUtils;
+import org.apache.pinot.spi.auth.AuthorizationResult;
import org.apache.pinot.spi.config.table.FieldConfig;
import org.apache.pinot.spi.config.table.QueryConfig;
import org.apache.pinot.spi.config.table.TableConfig;
@@ -366,18 +367,25 @@ public abstract class BaseSingleStageBrokerRequestHandler
extends BaseBrokerRequ
BrokerRequest brokerRequest =
CalciteSqlCompiler.convertToBrokerRequest(pinotQuery);
BrokerRequest serverBrokerRequest =
serverPinotQuery == pinotQuery ? brokerRequest :
CalciteSqlCompiler.convertToBrokerRequest(serverPinotQuery);
- boolean hasTableAccess =
- accessControl.hasAccess(requesterIdentity, serverBrokerRequest) &&
accessControl.hasAccess(httpHeaders,
- TargetType.TABLE, tableName, Actions.Table.QUERY);
+ AuthorizationResult authorizationResult =
accessControl.authorize(requesterIdentity, serverBrokerRequest);
+ if (authorizationResult.hasAccess()) {
+ authorizationResult = accessControl.authorize(httpHeaders,
TargetType.TABLE, tableName, Actions.Table.QUERY);
+ }
_brokerMetrics.addPhaseTiming(rawTableName,
BrokerQueryPhase.AUTHORIZATION,
System.nanoTime() - compilationEndTimeNs);
- if (!hasTableAccess) {
+ if (!authorizationResult.hasAccess()) {
_brokerMetrics.addMeteredTableValue(tableName,
BrokerMeter.REQUEST_DROPPED_DUE_TO_ACCESS_ERROR, 1);
- LOGGER.info("Access denied for request {}: {}, table: {}", requestId,
query, tableName);
+ LOGGER.info("Access denied for request {}: {}, table: {}, reason :{}",
requestId, query, tableName,
+ authorizationResult.getFailureMessage());
requestContext.setErrorCode(QueryException.ACCESS_DENIED_ERROR_CODE);
- throw new WebApplicationException("Permission denied",
Response.Status.FORBIDDEN);
+ String failureMessage = authorizationResult.getFailureMessage();
+ if (StringUtils.isNotBlank(failureMessage)) {
+ failureMessage = "Reason: " + failureMessage;
+ }
+ throw new WebApplicationException("Permission denied." +
failureMessage,
+ Response.Status.FORBIDDEN);
}
// Get the tables hit by the request
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
index 987ac2b901..8e60847308 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
@@ -25,10 +25,12 @@ import java.util.Map;
import java.util.Set;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeoutException;
+import java.util.stream.Collectors;
import javax.annotation.Nullable;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
+import org.apache.commons.lang.StringUtils;
import org.apache.http.conn.HttpClientConnectionManager;
import org.apache.pinot.broker.api.AccessControl;
import org.apache.pinot.broker.api.RequesterIdentity;
@@ -64,6 +66,7 @@ import
org.apache.pinot.query.runtime.plan.MultiStageQueryStats;
import org.apache.pinot.query.service.dispatch.QueryDispatcher;
import org.apache.pinot.query.type.TypeFactory;
import org.apache.pinot.query.type.TypeSystem;
+import org.apache.pinot.spi.auth.TableAuthorizationResult;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.exception.DatabaseConflictException;
import org.apache.pinot.spi.trace.RequestContext;
@@ -136,8 +139,15 @@ public class MultiStageBrokerRequestHandler extends
BaseBrokerRequestHandler {
queryPlanResult = queryEnvironment.explainQuery(query,
sqlNodeAndOptions, requestId);
String plan = queryPlanResult.getExplainPlan();
Set<String> tableNames = queryPlanResult.getTableNames();
- if (!hasTableAccess(requesterIdentity, tableNames, requestContext,
httpHeaders)) {
- throw new WebApplicationException("Permission denied",
Response.Status.FORBIDDEN);
+ TableAuthorizationResult tableAuthorizationResult =
+ hasTableAccess(requesterIdentity, tableNames, requestContext,
httpHeaders);
+ if (!tableAuthorizationResult.hasAccess()) {
+ String failureMessage =
tableAuthorizationResult.getFailureMessage();
+ if (StringUtils.isNotBlank(failureMessage)) {
+ failureMessage = "Reason: " + failureMessage;
+ }
+ throw new WebApplicationException("Permission denied. " +
failureMessage,
+ Response.Status.FORBIDDEN);
}
return constructMultistageExplainPlan(query, plan);
case SELECT:
@@ -182,8 +192,15 @@ public class MultiStageBrokerRequestHandler extends
BaseBrokerRequestHandler {
updatePhaseTimingForTables(tableNames,
BrokerQueryPhase.REQUEST_COMPILATION, compilationTimeNs);
// Validate table access.
- if (!hasTableAccess(requesterIdentity, tableNames, requestContext,
httpHeaders)) {
- throw new WebApplicationException("Permission denied",
Response.Status.FORBIDDEN);
+ TableAuthorizationResult tableAuthorizationResult =
+ hasTableAccess(requesterIdentity, tableNames, requestContext,
httpHeaders);
+ if (!tableAuthorizationResult.hasAccess()) {
+ String failureMessage = tableAuthorizationResult.getFailureMessage();
+ if (StringUtils.isNotBlank(failureMessage)) {
+ failureMessage = "Reason: " + failureMessage;
+ }
+ throw new WebApplicationException("Permission denied." + failureMessage,
+ Response.Status.FORBIDDEN);
}
// Validate QPS quota
@@ -267,13 +284,26 @@ public class MultiStageBrokerRequestHandler extends
BaseBrokerRequestHandler {
/**
* Validates whether the requester has access to all the tables.
*/
- private boolean hasTableAccess(RequesterIdentity requesterIdentity,
Set<String> tableNames,
+ private TableAuthorizationResult hasTableAccess(RequesterIdentity
requesterIdentity, Set<String> tableNames,
RequestContext requestContext, HttpHeaders httpHeaders) {
final long startTimeNs = System.nanoTime();
AccessControl accessControl = _accessControlFactory.create();
- boolean hasAccess = accessControl.hasAccess(requesterIdentity, tableNames)
&& tableNames.stream()
- .allMatch(table -> accessControl.hasAccess(httpHeaders,
TargetType.TABLE, table, Actions.Table.QUERY));
- if (!hasAccess) {
+
+ TableAuthorizationResult tableAuthorizationResult =
accessControl.authorize(requesterIdentity, tableNames);
+
+ Set<String> failedTables = tableNames.stream()
+ .filter(table -> !accessControl.hasAccess(httpHeaders,
TargetType.TABLE, table, Actions.Table.QUERY))
+ .collect(Collectors.toSet());
+
+ failedTables.addAll(tableAuthorizationResult.getFailedTables());
+
+ if (!failedTables.isEmpty()) {
+ tableAuthorizationResult = new TableAuthorizationResult(failedTables);
+ } else {
+ tableAuthorizationResult = TableAuthorizationResult.success();
+ }
+
+ if (!tableAuthorizationResult.hasAccess()) {
_brokerMetrics.addMeteredGlobalValue(BrokerMeter.REQUEST_DROPPED_DUE_TO_ACCESS_ERROR,
1);
LOGGER.warn("Access denied for requestId {}",
requestContext.getRequestId());
requestContext.setErrorCode(QueryException.ACCESS_DENIED_ERROR_CODE);
@@ -281,7 +311,7 @@ public class MultiStageBrokerRequestHandler extends
BaseBrokerRequestHandler {
updatePhaseTimingForTables(tableNames, BrokerQueryPhase.AUTHORIZATION,
System.nanoTime() - startTimeNs);
- return hasAccess;
+ return tableAuthorizationResult;
}
/**
diff --git
a/pinot-broker/src/test/java/org/apache/pinot/broker/api/AccessControlBackwardCompatibleTest.java
b/pinot-broker/src/test/java/org/apache/pinot/broker/api/AccessControlBackwardCompatibleTest.java
new file mode 100644
index 0000000000..ad781c9f88
--- /dev/null
+++
b/pinot-broker/src/test/java/org/apache/pinot/broker/api/AccessControlBackwardCompatibleTest.java
@@ -0,0 +1,97 @@
+/**
+ * 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.pinot.broker.api;
+
+import java.util.Set;
+import org.apache.pinot.common.request.BrokerRequest;
+import org.apache.pinot.spi.auth.AuthorizationResult;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+
+
+public class AccessControlBackwardCompatibleTest {
+
+ @Test
+ public void testBackwardCompatibleHasAccessBrokerRequest() {
+ AccessControl accessControl = new AllFalseAccessControlImpl();
+ HttpRequesterIdentity identity = new HttpRequesterIdentity();
+ BrokerRequest request = new BrokerRequest();
+ assertFalse(accessControl.authorize(identity, request).hasAccess());
+ }
+
+ @Test
+ public void testBackwardCompatibleHasAccessMutliTable() {
+ AccessControl accessControl = new AllFalseAccessControlImpl();
+ HttpRequesterIdentity identity = new HttpRequesterIdentity();
+ Set<String> tables = Set.of("table1", "table2");
+ AuthorizationResult result = accessControl.authorize(identity, tables);
+ assertFalse(result.hasAccess());
+ assertEquals(result.getFailureMessage(), "Authorization Failed for tables:
[table1, table2]");
+ }
+
+ @Test(expectedExceptions = UnsupportedOperationException.class)
+ public void testExceptionForNoImplAccessControlMultiTable() {
+ AccessControl accessControl = new NoImplAccessControl();
+ HttpRequesterIdentity identity = new HttpRequesterIdentity();
+ Set<String> tables = Set.of("table1", "table2");
+ accessControl.hasAccess(identity, tables);
+ }
+
+ @Test(expectedExceptions = UnsupportedOperationException.class)
+ public void testExceptionForNoImplAccessControlBrokerRequest() {
+ AccessControl accessControl = new NoImplAccessControl();
+ HttpRequesterIdentity identity = new HttpRequesterIdentity();
+ BrokerRequest request = new BrokerRequest();
+ accessControl.hasAccess(identity, request);
+ }
+
+ @Test(expectedExceptions = UnsupportedOperationException.class)
+ public void testExceptionForNoImplAccessControlAuthorizeMultiTable() {
+ AccessControl accessControl = new NoImplAccessControl();
+ HttpRequesterIdentity identity = new HttpRequesterIdentity();
+ Set<String> tables = Set.of("table1", "table2");
+ accessControl.authorize(identity, tables);
+ }
+
+ @Test(expectedExceptions = UnsupportedOperationException.class)
+ public void testExceptionForNoImplAccessControlAuthorizeBrokerRequest() {
+ AccessControl accessControl = new NoImplAccessControl();
+ HttpRequesterIdentity identity = new HttpRequesterIdentity();
+ BrokerRequest request = new BrokerRequest();
+ accessControl.authorize(identity, request);
+ }
+
+ class AllFalseAccessControlImpl implements AccessControl {
+
+ @Override
+ public boolean hasAccess(RequesterIdentity requesterIdentity,
BrokerRequest brokerRequest) {
+ return false;
+ }
+
+ @Override
+ public boolean hasAccess(RequesterIdentity requesterIdentity, Set<String>
tables) {
+ return false;
+ }
+ }
+
+ class NoImplAccessControl implements AccessControl {
+ }
+}
diff --git
a/pinot-broker/src/test/java/org/apache/pinot/broker/broker/BasicAuthAccessControlTest.java
b/pinot-broker/src/test/java/org/apache/pinot/broker/broker/BasicAuthAccessControlTest.java
index 42f121ecef..5eb444431a 100644
---
a/pinot-broker/src/test/java/org/apache/pinot/broker/broker/BasicAuthAccessControlTest.java
+++
b/pinot-broker/src/test/java/org/apache/pinot/broker/broker/BasicAuthAccessControlTest.java
@@ -29,6 +29,7 @@ import org.apache.pinot.broker.api.AccessControl;
import org.apache.pinot.broker.api.HttpRequesterIdentity;
import org.apache.pinot.common.request.BrokerRequest;
import org.apache.pinot.common.request.QuerySource;
+import org.apache.pinot.spi.auth.AuthorizationResult;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.testng.Assert;
import org.testng.annotations.BeforeClass;
@@ -66,7 +67,7 @@ public class BasicAuthAccessControlTest {
@Test(expectedExceptions = IllegalArgumentException.class)
public void testNullEntity() {
- _accessControl.hasAccess(null, (BrokerRequest) null);
+ _accessControl.authorize(null, (BrokerRequest) null);
}
@Test
@@ -77,7 +78,7 @@ public class BasicAuthAccessControlTest {
identity.setHttpHeaders(headers);
try {
- _accessControl.hasAccess(identity, (BrokerRequest) null);
+ _accessControl.authorize(identity, (BrokerRequest) null);
} catch (WebApplicationException e) {
Assert.assertEquals(e.getResponse().getStatus(), 401, "must return 401");
}
@@ -97,8 +98,8 @@ public class BasicAuthAccessControlTest {
BrokerRequest request = new BrokerRequest();
request.setQuerySource(source);
- Assert.assertTrue(_accessControl.hasAccess(identity, request));
- Assert.assertTrue(_accessControl.hasAccess(identity, _tableNames));
+ Assert.assertTrue(_accessControl.authorize(identity, request).hasAccess());
+ Assert.assertTrue(_accessControl.authorize(identity,
_tableNames).hasAccess());
}
@Test
@@ -114,16 +115,27 @@ public class BasicAuthAccessControlTest {
BrokerRequest request = new BrokerRequest();
request.setQuerySource(source);
-
- Assert.assertFalse(_accessControl.hasAccess(identity, request));
+ AuthorizationResult authorizationResult =
_accessControl.authorize(identity, request);
+ Assert.assertFalse(authorizationResult.hasAccess());
+ Assert.assertEquals(authorizationResult.getFailureMessage(),
+ "Authorization Failed for tables: [veryImportantStuff]");
Set<String> tableNames = new HashSet<>();
tableNames.add("veryImportantStuff");
- Assert.assertFalse(_accessControl.hasAccess(identity, tableNames));
+ authorizationResult = _accessControl.authorize(identity, tableNames);
+ Assert.assertFalse(authorizationResult.hasAccess());
+ Assert.assertEquals(authorizationResult.getFailureMessage(),
+ "Authorization Failed for tables: [veryImportantStuff]");
tableNames.add("lessImportantStuff");
- Assert.assertFalse(_accessControl.hasAccess(identity, tableNames));
+ authorizationResult = _accessControl.authorize(identity, tableNames);
+ Assert.assertFalse(authorizationResult.hasAccess());
+ Assert.assertEquals(authorizationResult.getFailureMessage(),
+ "Authorization Failed for tables: [veryImportantStuff]");
tableNames.add("lesserImportantStuff");
- Assert.assertFalse(_accessControl.hasAccess(identity, tableNames));
+ authorizationResult = _accessControl.authorize(identity, tableNames);
+ Assert.assertFalse(authorizationResult.hasAccess());
+ Assert.assertEquals(authorizationResult.getFailureMessage(),
+ "Authorization Failed for tables: [veryImportantStuff]");
}
@Test
@@ -139,15 +151,18 @@ public class BasicAuthAccessControlTest {
BrokerRequest request = new BrokerRequest();
request.setQuerySource(source);
-
- Assert.assertTrue(_accessControl.hasAccess(identity, request));
+ AuthorizationResult authorizationResult =
_accessControl.authorize(identity, request);
+ Assert.assertTrue(authorizationResult.hasAccess());
+ Assert.assertEquals(authorizationResult.getFailureMessage(), "");
Set<String> tableNames = new HashSet<>();
tableNames.add("lessImportantStuff");
tableNames.add("veryImportantStuff");
tableNames.add("lesserImportantStuff");
- Assert.assertTrue(_accessControl.hasAccess(identity, tableNames));
+ authorizationResult = _accessControl.authorize(identity, tableNames);
+ Assert.assertTrue(authorizationResult.hasAccess());
+ Assert.assertEquals(authorizationResult.getFailureMessage(), "");
}
@Test
@@ -159,11 +174,12 @@ public class BasicAuthAccessControlTest {
identity.setHttpHeaders(headers);
BrokerRequest request = new BrokerRequest();
-
- Assert.assertTrue(_accessControl.hasAccess(identity, request));
+ AuthorizationResult authorizationResult =
_accessControl.authorize(identity, request);
+ Assert.assertTrue(authorizationResult.hasAccess());
Set<String> tableNames = new HashSet<>();
- Assert.assertTrue(_accessControl.hasAccess(identity, tableNames));
+ authorizationResult = _accessControl.authorize(identity, tableNames);
+ Assert.assertTrue(authorizationResult.hasAccess());
}
@Test
@@ -180,7 +196,7 @@ public class BasicAuthAccessControlTest {
BrokerRequest request = new BrokerRequest();
request.setQuerySource(source);
- Assert.assertTrue(_accessControl.hasAccess(identity, request));
- Assert.assertTrue(_accessControl.hasAccess(identity, _tableNames));
+ Assert.assertTrue(_accessControl.authorize(identity, request).hasAccess());
+ Assert.assertTrue(_accessControl.authorize(identity,
_tableNames).hasAccess());
}
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/auth/FineGrainedAccessControl.java
b/pinot-core/src/main/java/org/apache/pinot/core/auth/FineGrainedAccessControl.java
index 4d65af327e..df6b51c66f 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/auth/FineGrainedAccessControl.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/auth/FineGrainedAccessControl.java
@@ -19,6 +19,8 @@
package org.apache.pinot.core.auth;
import javax.ws.rs.core.HttpHeaders;
+import org.apache.pinot.spi.auth.AuthorizationResult;
+import org.apache.pinot.spi.auth.BasicAuthorizationResultImpl;
/**
@@ -38,6 +40,22 @@ public interface FineGrainedAccessControl {
return true;
}
+ /**
+ * Verifies if the user has access to perform a specific action on a
particular resource.
+ * The default implementation returns a {@link BasicAuthorizationResultImpl}
with the result of the hasAccess() of
+ * the implementation
+ *
+ * @param httpHeaders HTTP headers
+ * @param targetType type of resource being accessed
+ * @param targetId id of the resource
+ * @param action type to validate
+ * @return An AuthorizationResult object, encapsulating whether the access
is granted or not.
+ */
+ default AuthorizationResult authorize(HttpHeaders httpHeaders, TargetType
targetType, String targetId,
+ String action) {
+ return new BasicAuthorizationResultImpl(hasAccess(httpHeaders, targetType,
targetId, action));
+ }
+
/**
* If an API is neither annotated with Authorize nor ManualAuthorization,
* this method will be called to check the default authorization.
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/auth/AuthorizationResult.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/auth/AuthorizationResult.java
new file mode 100644
index 0000000000..f915537e80
--- /dev/null
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/auth/AuthorizationResult.java
@@ -0,0 +1,41 @@
+/**
+ * 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.pinot.spi.auth;
+
+/**
+ * The AuthorizationResult interface defines the contract for authorization
results in the Pinot system.
+ * Implementations of this interface provide the access status and an optional
failure message indicating
+ * the reason for denied access.
+ */
+public interface AuthorizationResult {
+
+ /**
+ * Indicates whether the access is granted.
+ *
+ * @return true if access is granted, false otherwise.
+ */
+ boolean hasAccess();
+
+ /**
+ * Provides the failure message if access is denied.
+ *
+ * @return A string containing the failure message if access is denied,
otherwise an empty string or null.
+ */
+ String getFailureMessage();
+}
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/auth/BasicAuthorizationResultImpl.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/auth/BasicAuthorizationResultImpl.java
new file mode 100644
index 0000000000..95ba2c5124
--- /dev/null
+++
b/pinot-spi/src/main/java/org/apache/pinot/spi/auth/BasicAuthorizationResultImpl.java
@@ -0,0 +1,83 @@
+/**
+ * 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.pinot.spi.auth;
+
+import org.apache.commons.lang3.StringUtils;
+
+
+/**
+ * Implementation of the AuthorizationResult interface that provides basic
+ * authorization results including access status and failure messages.
+ */
+public class BasicAuthorizationResultImpl implements AuthorizationResult {
+
+ private static final BasicAuthorizationResultImpl SUCCESS = new
BasicAuthorizationResultImpl(true);
+ private final boolean _hasAccess;
+ private final String _failureMessage;
+
+ /**
+ * Constructs a BasicAuthorizationResultImpl with the specified access
status and failure message.
+ *
+ * @param hasAccess true if access is granted, false otherwise.
+ * @param failureMessage the failure message if access is denied.
+ */
+ public BasicAuthorizationResultImpl(boolean hasAccess, String
failureMessage) {
+ _hasAccess = hasAccess;
+ _failureMessage = failureMessage;
+ }
+
+ /**
+ * Constructs a BasicAuthorizationResultImpl with the specified access
status and an empty failure message.
+ *
+ * @param hasAccess true if access is granted, false otherwise.
+ */
+ public BasicAuthorizationResultImpl(boolean hasAccess) {
+ _hasAccess = hasAccess;
+ _failureMessage = StringUtils.EMPTY;
+ }
+
+ /**
+ * Creates a BasicAuthorizationResultImpl with access granted and no failure
message.
+ *
+ * @return a BasicAuthorizationResultImpl with access granted and an empty
failure message.
+ */
+ public static BasicAuthorizationResultImpl success() {
+ return SUCCESS;
+ }
+
+ /**
+ * Indicates whether access is granted.
+ *
+ * @return true if access is granted, false otherwise.
+ */
+ @Override
+ public boolean hasAccess() {
+ return _hasAccess;
+ }
+
+ /**
+ * Provides the failure message if access is denied.
+ *
+ * @return the failure message if access is denied, otherwise an empty
string.
+ */
+ @Override
+ public String getFailureMessage() {
+ return _failureMessage;
+ }
+}
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/auth/TableAuthorizationResult.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/auth/TableAuthorizationResult.java
new file mode 100644
index 0000000000..3ef77fc087
--- /dev/null
+++
b/pinot-spi/src/main/java/org/apache/pinot/spi/auth/TableAuthorizationResult.java
@@ -0,0 +1,74 @@
+/**
+ * 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.pinot.spi.auth;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+import org.apache.commons.lang.StringUtils;
+
+
+/**
+ * Implementation of the AuthorizationResult interface that provides
authorization results
+ * at the table level, including which tables failed authorization.
+ */
+public class TableAuthorizationResult implements AuthorizationResult {
+
+ private static final TableAuthorizationResult SUCCESS = new
TableAuthorizationResult(Set.of());
+ private final Set<String> _failedTables;
+
+ public TableAuthorizationResult(Set<String> failedTables) {
+ _failedTables = failedTables;
+ }
+
+ /**
+ * Creates a TableAuthorizationResult with no failed tables.
+ *
+ * @return a TableAuthorizationResult with no failed tables.
+ */
+ public static TableAuthorizationResult success() {
+ return SUCCESS;
+ }
+
+ @Override
+ public boolean hasAccess() {
+ return _failedTables.isEmpty();
+ }
+
+ public Set<String> getFailedTables() {
+ return _failedTables;
+ }
+
+ /**
+ * Provides the failure message indicating which tables failed authorization.
+ *
+ * @return a string containing the failure message if there are failed
tables, otherwise an empty string.
+ */
+ @Override
+ public String getFailureMessage() {
+ if (hasAccess()) {
+ return StringUtils.EMPTY;
+ }
+
+ List<String> failedTablesList = new ArrayList<>(_failedTables);
+ Collections.sort(failedTablesList); // Sort to make output deterministic
+ return "Authorization Failed for tables: " + failedTablesList;
+ }
+}
diff --git
a/pinot-spi/src/test/java/org/apache/pinot/spi/auth/BasicAuthorizationResultImplTest.java
b/pinot-spi/src/test/java/org/apache/pinot/spi/auth/BasicAuthorizationResultImplTest.java
new file mode 100644
index 0000000000..2f15938593
--- /dev/null
+++
b/pinot-spi/src/test/java/org/apache/pinot/spi/auth/BasicAuthorizationResultImplTest.java
@@ -0,0 +1,61 @@
+/**
+ * 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.pinot.spi.auth;
+
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+
+public class BasicAuthorizationResultImplTest {
+ @Test
+ public void testConstructorWithAccessAndMessage() {
+ BasicAuthorizationResultImpl result = new
BasicAuthorizationResultImpl(true, "Failure Message");
+ assertTrue(result.hasAccess());
+ assertEquals("Failure Message", result.getFailureMessage());
+ }
+
+ @Test
+ public void testConstructorWithAccessOnly() {
+ BasicAuthorizationResultImpl result = new
BasicAuthorizationResultImpl(true);
+ assertTrue(result.hasAccess());
+ assertEquals("", result.getFailureMessage());
+ }
+
+ @Test
+ public void testNoFailureResult() {
+ BasicAuthorizationResultImpl result =
BasicAuthorizationResultImpl.success();
+ assertTrue(result.hasAccess());
+ assertEquals("", result.getFailureMessage());
+ }
+
+ @Test
+ public void testSetHasAccess() {
+ BasicAuthorizationResultImpl result = new
BasicAuthorizationResultImpl(false);
+ assertFalse(result.hasAccess());
+ }
+
+ @Test
+ public void testSetFailureMessage() {
+ BasicAuthorizationResultImpl result = new
BasicAuthorizationResultImpl(true, "New Failure Message");
+ assertEquals("New Failure Message", result.getFailureMessage());
+ }
+}
diff --git
a/pinot-spi/src/test/java/org/apache/pinot/spi/auth/TableAuthorizationResultTest.java
b/pinot-spi/src/test/java/org/apache/pinot/spi/auth/TableAuthorizationResultTest.java
new file mode 100644
index 0000000000..306b20602f
--- /dev/null
+++
b/pinot-spi/src/test/java/org/apache/pinot/spi/auth/TableAuthorizationResultTest.java
@@ -0,0 +1,68 @@
+/**
+ * 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.pinot.spi.auth;
+
+import java.util.HashSet;
+import java.util.Set;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+
+public class TableAuthorizationResultTest {
+
+ @Test
+ public void testParameterizedConstructor() {
+ Set<String> failedTables = new HashSet<>();
+ failedTables.add("table1");
+ TableAuthorizationResult result = new
TableAuthorizationResult(failedTables);
+ Assert.assertFalse(result.hasAccess());
+ Assert.assertTrue(result.getFailureMessage().contains("table1"));
+ }
+
+ @Test
+ public void testAddFailedTable() {
+ TableAuthorizationResult result = new
TableAuthorizationResult(Set.of("table1"));
+ Assert.assertFalse(result.hasAccess());
+ Assert.assertEquals(result.getFailureMessage(), "Authorization Failed for
tables: [table1]");
+ }
+
+ @Test
+ public void testSetGetFailedTables() {
+ Set<String> failedTables = new HashSet<>();
+ failedTables.add("table1");
+ failedTables.add("table2");
+ TableAuthorizationResult result = new
TableAuthorizationResult(failedTables);
+ Assert.assertFalse(result.hasAccess());
+ Assert.assertEquals(result.getFailedTables(), failedTables);
+ }
+
+ @Test
+ public void testGetFailureMessage() {
+ TableAuthorizationResult result = new
TableAuthorizationResult(Set.of("table1", "table2"));
+ Assert.assertEquals(result.getFailureMessage(), "Authorization Failed for
tables: [table1, table2]");
+ }
+
+ @Test
+ public void testNoFailureResult() {
+ TableAuthorizationResult result = TableAuthorizationResult.success();
+ Assert.assertTrue(result.hasAccess());
+ Assert.assertEquals("", result.getFailureMessage());
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
