This is an automated email from the ASF dual-hosted git repository.
xiangfu0 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 b5397bb0459 [controller] Require unrestricted table scope for
cluster-level BasicAuth access (#18975)
b5397bb0459 is described below
commit b5397bb0459d24917636d5e78f860e3fe5d61d96
Author: deepinsight coder <[email protected]>
AuthorDate: Sun Oct 11 23:09:49 2026 -0700
[controller] Require unrestricted table scope for cluster-level BasicAuth
access (#18975)
* [controller] Require unrestricted table scope for cluster-level BasicAuth
access
Fixes #14595
Controller endpoints that address the cluster rather than a table were
reachable by any authenticated BasicAuth principal holding the matching
CRUD permission, regardless of the principal's table scope. A user
confined to a single table could therefore drive cluster-level endpoints
— the reported case deleted another user through DELETE /users/{name}.
#19234 made these endpoints honor the requested permission, but left
table scope unchecked, so a table-scoped principal with the permission
still passed. Two mechanisms are needed, and neither works alone.
First, a request that names no table is cluster-wide, so it now requires
a principal with neither a table allow-list nor an exclude-list in
addition to the requested permission. Both the static and the
ZooKeeper-backed factories inherit this from BaseBasicAuthAccessControl,
and the table overload routes a null table name to the same check so the
rule cannot be sidestepped by omitting the table.
Second, that check keys off whether a table name was resolved, and
resolution also accepted query parameters the endpoint never declared.
JAX-RS surfaces every query parameter present on the URI, so appending
?tableName=<a table the caller owns> to a cluster endpoint made it
evaluate as a table-scoped request and pass, reproducing the original
exploit against the new check. Only parameters the endpoint declares as
@QueryParam may now identify the table. The rule lives in
FineGrainedAuthUtils so the coarse filter and the fine-grained path share
it, which also covers the broker's filter. Path parameters are template
variables of the endpoint's own @Path and stay authoritative. Only
method-level @QueryParam binding is recognized; @BeanParam and field
injection are treated as declaring nothing, which denies table scope
rather than granting it. The per-method lookup is memoized because
Method#getParameterAnnotations re-parses class-file bytes on every call
and this runs before authentication.
The fine-grained overload keeps its authenticated-only check: several
table-scoped endpoints declare a cluster action, and for those the filter
already resolved the table name and ran the table-scoped check.
hasAccess(HttpHeaders, TargetType) also stays an authentication check
because it backs auth/verify/v2, the controller UI's login gate, which
renders false as "invalid credentials".
Defects surfaced while establishing the invariant, fixed here:
- copyTable declared @Authorize(TargetType.TABLE) with no paramName, so
it defaulted to "" and POST /tables/{tableName}/copy failed
authorization with 500 for every caller, admins included. A reflection
guard now asserts every table-targeted endpoint binds its paramName.
- getTaskDebugInfo's table filter sat inside a null check on the Helix
task config, so subtasks whose table could not be determined were
reported regardless of the requested filter. It now fails closed.
- The only in-tree example of a custom AccessControl returned
`tableName == null || allowed.contains(tableName)` — the shape that
leaves this issue open in a downstream implementation.
- BasicAuthPrincipal copies its table sets, which now gate cluster access
and would otherwise widen with a caller's later mutation.
- AccessControl documents which overload carries the cluster policy; its
two defaults delegate to each other, so an implementation overriding
only one and routing null back would recurse.
The static factory logs a startup warning naming principals that are
table-scoped yet hold a write permission, since those are the service
identities whose segment-completion and task callbacks will be denied.
Backward-incompatible. Table-scoped principals lose every controller
endpoint that names no table:
- Cluster-wide listings such as GET /tables and GET /schemas enumerate
the whole cluster rather than the caller's scope, so they are denied;
the controller UI's listing pages are unavailable to such a principal.
- Realtime segment completion and minion task callbacks carry no table
name, so a table-scoped service token is denied and commit stalls.
- POST /segments documents tableName as optional, falling back to segment
metadata; table-scoped principals must now send it.
- The controller's multi-stage /sql path authorizes cluster-wide by
design, so table-scoped principals lose it even for their own tables.
- excludeTables-only principals are treated as scoped. Removing the
exclusion would re-grant the excluded table, so the migration is a
separate unrestricted principal rather than a widened scoped one.
Migration: give operators two principals, a table-scoped one for table
work and an unrestricted one for cluster work, and grant the unrestricted
one before rolling controllers — during the mixed-version window the same
request answers 200 or 403 depending on which controller serves it.
Custom AccessControl implementations are affected even though the
interface is unchanged: the table name they receive is now restricted to
path parameters and declared @QueryParams, so a request that previously
arrived on the table overload may now arrive on the cluster overload with
a null table name.
Scope: controller only. The broker's BasicAuth controls share
BasicAuthPrincipal and have a comparable gap, left for a follow-up.
* [controller] Make BasicAuth table narrowing explicit
Cluster endpoints retain cluster scope unless their authorization
annotation explicitly identifies a table parameter. Preserve table-specific
task, segment, upload, and ingestion operations while denying task-wide debug
metadata to scoped principals. Restore BasicAuthPrincipal subclass
compatibility and add focused regressions for explicit scope and task-debug
denial.
* [controller] Address BasicAuth scope review feedback
---------
Co-authored-by: Xiang Fu <[email protected]>
---
helm/pinot/values.yaml | 10 +
.../pinot/controller/api/access/AccessControl.java | 17 +-
.../api/access/AuthenticationFilter.java | 57 +++--
.../api/access/BaseBasicAuthAccessControl.java | 56 ++++-
.../api/access/BasicAuthAccessControlFactory.java | 4 +
...PinotControllerPeriodicTaskRestletResource.java | 4 +-
.../resources/PinotIngestionRestletResource.java | 6 +-
.../api/resources/PinotSegmentRestletResource.java | 9 +-
.../PinotSegmentUploadDownloadRestletResource.java | 4 +-
.../api/resources/PinotTableRestletResource.java | 2 +-
.../api/resources/PinotTaskRestletResource.java | 14 +-
.../core/minion/PinotHelixTaskResourceManager.java | 11 +-
.../api/PinotSegmentUploadAuthorizationTest.java | 7 +-
.../api/access/AuthenticationFilterTest.java | 277 +++++++++++++++++++--
.../access/BasicAuthAccessControlFactoryTest.java | 101 +++++++-
...ontrollerClusterBasicAuthAuthorizationTest.java | 108 ++++++--
.../minion/PinotHelixTaskResourceManagerTest.java | 16 ++
.../java/org/apache/pinot/core/auth/Authorize.java | 7 +
.../apache/pinot/core/auth/BasicAuthPrincipal.java | 31 ++-
.../pinot/core/auth/FineGrainedAuthUtils.java | 72 ++++--
.../org/apache/pinot/core/auth/BasicAuthTest.java | 85 ++++---
.../pinot/core/auth/FineGrainedAuthUtilsTest.java | 159 ++++++++----
22 files changed, 871 insertions(+), 186 deletions(-)
diff --git a/helm/pinot/values.yaml b/helm/pinot/values.yaml
index 735698522ee..64079c99b16 100644
--- a/helm/pinot/values.yaml
+++ b/helm/pinot/values.yaml
@@ -82,6 +82,16 @@ pinotAuth:
# - access.control.principals.user.password=secret
# - access.control.principals.user.tables=baseballStats,otherstuff
# - access.control.principals.user.permissions=READ
+ # Setting `tables` (or `excludeTables`) scopes a principal to those tables
and, on the controller, withholds every
+ # endpoint that names no table: cluster configs, instances, users, and
cluster-wide listings such as GET /tables.
+ # That also includes realtime segment-completion callbacks
(/segmentConsumed, /segmentCommitStart, and the rest of
+ # LLCSegmentCompletionHandlers) and minion task callbacks that carry no
tableName, so a table-scoped server or
+ # minion token will 403 and stall ingestion. The deprecated GET /auth/verify
probe, called without tableName, now
+ # answers false for a scoped principal (the UI uses /auth/verify/v2 and is
unaffected).
+ # `admin` above is unscoped and keeps that access. Give an operator both: a
scoped principal for table work and an
+ # unscoped one for cluster work. Grant the unscoped principal before rolling
controllers; during the mixed-version
+ # window the same request answers 200 or 403 depending on which controller
serves it. Widening `tables` to restore
+ # cluster access would also widen table access.
#
------------------------------------------------------------------------------
# Pinot Controller:
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControl.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControl.java
index b12d3e21835..d80f2655146 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControl.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AccessControl.java
@@ -33,7 +33,13 @@ public interface AccessControl extends
FineGrainedAccessControl {
/// Return whether the client has permission to the given table
///
- /// @param tableName name of the table to be accessed
+ /// A `null` table name means the endpoint named no table — either it
declares no table parameter, or the request
+ /// omitted an optional one — so the request addresses the cluster rather
than a table. Implementations must apply
+ /// their cluster policy in that case; returning `true` for a `null` table
name grants every cluster endpoint.
+ /// Note that a caller-supplied query parameter only names the table when
the endpoint declares it, so a table name
+ /// appended to a cluster endpoint arrives here as `null`.
+ ///
+ /// @param tableName name of the table to be accessed, or `null` when the
request names no table
/// @param accessType type of the access
/// @param httpHeaders HTTP headers containing requester identity
/// @param endpointUrl the request url for which this access control is
called
@@ -45,6 +51,15 @@ public interface AccessControl extends
FineGrainedAccessControl {
/// Return whether the client has permission to access the endpoints with
are not table level
///
+ /// This is the cluster-wide gate. The request named no table, so it may
reach state belonging to any table: grant it
+ /// only to a principal whose authority spans the whole cluster, never to
one scoped to a subset of tables. Granting
+ /// it on a narrower scope is what made table-scoped users able to drive
cluster endpoints in apache/pinot#14595.
+ ///
+ /// This overload is the one to implement the cluster policy in. Its default
delegates to
+ /// [#hasAccess(String, AccessType, HttpHeaders, String)] with a `null`
table name, so an implementation that also
+ /// routes a `null` table name back here must override both overloads —
overriding only one leaves the two defaults
+ /// delegating to each other.
+ ///
/// @param accessType type of the access
/// @param httpHeaders HTTP headers
/// @param endpointUrl the request url for which this access control is
called
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AuthenticationFilter.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AuthenticationFilter.java
index f2bca693357..c5f34fa70ef 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AuthenticationFilter.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/AuthenticationFilter.java
@@ -24,7 +24,9 @@ import java.io.IOException;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.HashSet;
+import java.util.List;
import java.util.Set;
+import javax.annotation.Nullable;
import javax.inject.Inject;
import javax.inject.Provider;
import javax.ws.rs.DELETE;
@@ -58,6 +60,9 @@ public class AuthenticationFilter implements
ContainerRequestFilter {
private static final String KEY_TABLE_NAME = "tableName";
private static final String KEY_TABLE_NAME_WITH_TYPE = "tableNameWithType";
private static final String KEY_SCHEMA_NAME = "schemaName";
+ /// Parameter names that identify the table an endpoint acts on, in the
order they are consulted.
+ private static final List<String> TABLE_NAME_KEYS =
+ List.of(KEY_TABLE_NAME, KEY_TABLE_NAME_WITH_TYPE, KEY_SCHEMA_NAME);
@Inject
Provider<Request> _requestProvider;
@@ -140,35 +145,55 @@ public class AuthenticationFilter implements
ContainerRequestFilter {
return AccessType.READ;
}
+ /// Resolves the table `endpointMethod` acts on, or `null` when the request
addresses the cluster rather than a
+ /// table. `AccessControlUtils.validatePermission` picks the table-scoped or
the cluster-wide check on that answer,
+ /// so it must not be steerable by the caller.
+ ///
+ /// Path parameters are template variables of the endpoint's own `@Path`,
hence part of its declaration. Query
+ /// parameters are caller-supplied and JAX-RS surfaces every one present on
the URI, so only those the endpoint
+ /// declares may name the table: otherwise a caller could append
`?tableName=<a table it is scoped to>` to a cluster
+ /// endpoint and have it authorized as a table-scoped request.
+ ///
+ @Nullable
@VisibleForTesting
static String extractTableName(Method endpointMethod, MultivaluedMap<String,
String> pathParameters,
MultivaluedMap<String, String> queryParameters) {
Authorize authorize = endpointMethod.getAnnotation(Authorize.class);
if (authorize != null && authorize.targetType() == TargetType.TABLE) {
- return FineGrainedAuthUtils.findRawTargetId(authorize, pathParameters,
queryParameters);
+ // Same primitive as FineGrainedAuthUtils.validateFineGrainedAuth: path
first, then a declared query param.
+ return FineGrainedAuthUtils.findRawTargetId(authorize, endpointMethod,
pathParameters, queryParameters);
+ }
+ if (authorize != null && authorize.targetType() == TargetType.CLUSTER) {
+ String parameter = authorize.tableScopeParam();
+ if (parameter.isEmpty()) {
+ return null;
+ }
+ String target = pathParameters.getFirst(parameter);
+ if (target != null) {
+ return target;
+ }
+ return
FineGrainedAuthUtils.getDeclaredQueryParams(endpointMethod).contains(parameter)
+ ? queryParameters.getFirst(parameter) : null;
}
- return extractTableName(pathParameters, queryParameters);
- }
-
- @VisibleForTesting
- static String extractTableName(MultivaluedMap<String, String> pathParameters,
- MultivaluedMap<String, String> queryParameters) {
String tableName = extractTableName(pathParameters);
if (tableName != null) {
return tableName;
}
- return extractTableName(queryParameters);
+ for (String key : TABLE_NAME_KEYS) {
+ if (queryParameters.containsKey(key)
+ &&
FineGrainedAuthUtils.getDeclaredQueryParams(endpointMethod).contains(key)) {
+ return queryParameters.getFirst(key);
+ }
+ }
+ return null;
}
+ @Nullable
private static String extractTableName(MultivaluedMap<String, String> mmap) {
- if (mmap.containsKey(KEY_TABLE_NAME)) {
- return mmap.getFirst(KEY_TABLE_NAME);
- }
- if (mmap.containsKey(KEY_TABLE_NAME_WITH_TYPE)) {
- return mmap.getFirst(KEY_TABLE_NAME_WITH_TYPE);
- }
- if (mmap.containsKey(KEY_SCHEMA_NAME)) {
- return mmap.getFirst(KEY_SCHEMA_NAME);
+ for (String key : TABLE_NAME_KEYS) {
+ if (mmap.containsKey(key)) {
+ return mmap.getFirst(key);
+ }
}
return null;
}
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/BaseBasicAuthAccessControl.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/BaseBasicAuthAccessControl.java
index e411ffd8a0e..ca902566109 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/BaseBasicAuthAccessControl.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/BaseBasicAuthAccessControl.java
@@ -20,24 +20,42 @@ package org.apache.pinot.controller.api.access;
import java.util.Objects;
import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import javax.annotation.Nullable;
import javax.ws.rs.NotAuthorizedException;
import javax.ws.rs.core.HttpHeaders;
import org.apache.pinot.core.auth.BasicAuthPrincipal;
import org.apache.pinot.core.auth.TargetType;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/// Shared controller BasicAuth policy, independent of how principals are
loaded and credentials are verified.
-/// This class is stateless and thread-safe; subclass principal resolution
must also be thread-safe.
+/// Thread-safe; its only state is the set of principals already reported in a
table-scope denial log. Subclass
+/// principal resolution must also be thread-safe.
abstract class BaseBasicAuthAccessControl<P extends BasicAuthPrincipal>
implements AccessControl {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(BaseBasicAuthAccessControl.class);
+
+ /// Principal names already reported by [#warnOnceAboutTableScope]; bounded
by the number of configured principals.
+ private final Set<String> _tableScopeDenialsReported =
ConcurrentHashMap.newKeySet();
+
@Override
public final boolean protectAnnotatedOnly() {
return false;
}
@Override
- public final boolean hasAccess(String tableName, AccessType accessType,
HttpHeaders httpHeaders,
+ public final boolean hasAccess(@Nullable String tableName, AccessType
accessType, HttpHeaders httpHeaders,
String endpointUrl) {
+ // A null table name means the request named no table, which makes it
cluster-wide however the caller reached this
+ // overload. Route it to the cluster check so the scope rule cannot be
sidestepped by omitting the table — the
+ // /auth/verify probe takes an optional table name and reaches this
overload directly. Safe because the cluster
+ // overload below is overridden here rather than left as the interface
default, which would delegate back.
+ if (tableName == null) {
+ return hasAccess(accessType, httpHeaders, endpointUrl);
+ }
Optional<P> principal = getPrincipal(httpHeaders);
if (principal.isEmpty()) {
throw new NotAuthorizedException("Basic");
@@ -48,22 +66,54 @@ abstract class BaseBasicAuthAccessControl<P extends
BasicAuthPrincipal> implemen
&& authenticatedPrincipal.hasPermission(Objects.toString(accessType));
}
+ /// Guards endpoints that name no table. Such a request is cluster-wide, so
beyond the requested permission it
+ /// requires a principal whose table scope is unrestricted: a principal
confined to a subset of tables must not reach
+ /// cluster state that lies outside that subset. That includes realtime
segment-completion callbacks
+ /// (`LLCSegmentCompletionHandlers`) and minion task callbacks that carry no
table name, so a table-scoped server
+ /// or minion token is denied and ingestion stalls until the token is moved
to an unrestricted principal.
@Override
public final boolean hasAccess(AccessType accessType, HttpHeaders
httpHeaders, String endpointUrl) {
Optional<P> principal = getPrincipal(httpHeaders);
if (principal.isEmpty()) {
throw new NotAuthorizedException("Basic");
}
- return principal.get().hasPermission(Objects.toString(accessType));
+ P authenticatedPrincipal = principal.get();
+ if (!authenticatedPrincipal.hasUnrestrictedTableAccess()) {
+ warnOnceAboutTableScope(authenticatedPrincipal, endpointUrl);
+ return false;
+ }
+ return authenticatedPrincipal.hasPermission(Objects.toString(accessType));
+ }
+
+ /// Reports the first cluster-level denial caused by table scope for each
principal. Without it the only trace is a
+ /// generic 403, which gives an operator no hint that the table list is the
cause — and the affected callers include
+ /// service identities driving realtime segment completion and minion task
callbacks, where the visible symptom is
+ /// stalled ingestion rather than a failed API call. Reported on denial
rather than at startup so that principals
+ /// created later, which is how the ZooKeeper-backed factory is used, are
covered too.
+ private void warnOnceAboutTableScope(P principal, String endpointUrl) {
+ if (_tableScopeDenialsReported.add(principal.getName())) {
+ LOGGER.warn("Denied BasicAuth principal '{}' access to '{}': the
endpoint names no table, so it requires a "
+ + "principal without a table allow-list or exclude-list. Grant
unrestricted table scope to any "
+ + "principal used as a service identity.", principal.getName(),
endpointUrl);
+ }
}
@Override
public final boolean hasAccess(HttpHeaders httpHeaders, TargetType
targetType, String targetId, String action) {
// Basic auth permissions are CRUD access types, not action names.
AuthenticationFilter enforces the resolved
// AccessType before invoking this fine-grained check, so this overload
must only prevent unauthenticated access.
+ // In particular it must not require unrestricted table scope for
TargetType.CLUSTER: several table-scoped
+ // endpoints declare a cluster action, and for those the filter already
resolved the table name and ran the
+ // table-scoped check above. Endpoints that name no table go through the
cluster-wide check above instead.
return getPrincipal(httpHeaders).isPresent();
}
+ /// Reports whether the principal may act on the given target type at all,
without a specific target or action.
+ /// This backs `auth/verify/v2`, which the controller UI calls as its login
gate and renders as "invalid credentials"
+ /// when it returns false, so it must stay an authentication check rather
than report cluster capability — otherwise
+ /// a table-scoped principal is told its correct credentials are invalid.
Authorization for a specific request is
+ /// decided by the overloads above, so such a principal signs in and reaches
its own tables through the table-scoped
+ /// endpoints, while cluster-wide views such as `GET /tables` remain denied.
@Override
public final boolean hasAccess(HttpHeaders httpHeaders, TargetType
targetType) {
return getPrincipal(httpHeaders).isPresent();
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/BasicAuthAccessControlFactory.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/BasicAuthAccessControlFactory.java
index 3bebb1e1641..9d8d3c615e5 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/BasicAuthAccessControlFactory.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/access/BasicAuthAccessControlFactory.java
@@ -42,6 +42,10 @@ import org.apache.pinot.spi.env.PinotConfiguration;
///
controller.admin.access.control.principals.user456.tables=stuff,lessImportantStuff
/// controller.admin.access.control.principals.user456.permissions=read,update
/// ```
+///
+/// Setting `tables` or `excludeTables` scopes a principal to those tables and
withholds every endpoint that names no
+/// table, because such a request is cluster-wide. `*` means unrestricted, as
does omitting the key. A principal used
+/// as a service identity — realtime segment completion, minion task callbacks
— must therefore be left unrestricted.
public class BasicAuthAccessControlFactory implements AccessControlFactory {
private static final String PREFIX =
"controller.admin.access.control.principals";
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotControllerPeriodicTaskRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotControllerPeriodicTaskRestletResource.java
index ed4964227bd..c875f088516 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotControllerPeriodicTaskRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotControllerPeriodicTaskRestletResource.java
@@ -77,7 +77,7 @@ public class PinotControllerPeriodicTaskRestletResource {
@Produces(MediaType.APPLICATION_JSON)
@Path("/run")
@Authenticate(AccessType.UPDATE)
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.EXECUTE_TASK)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.EXECUTE_TASK, tableScopeParam = "tableName")
@ApiOperation(value = "Run periodic task against table. If table name is
missing, task will run against all tables.")
public Response runPeriodicTask(
@ApiParam(value = "Periodic task name", required = true)
@QueryParam("taskname") String periodicTaskName,
@@ -112,7 +112,7 @@ public class PinotControllerPeriodicTaskRestletResource {
@Produces(MediaType.APPLICATION_JSON)
@Path("/run")
@Authenticate(AccessType.UPDATE)
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.EXECUTE_TASK)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.EXECUTE_TASK, tableScopeParam = "tableName")
@ApiOperation(value = "Run periodic task against table with custom
properties. If table name is missing, task will "
+ "run against all tables.")
public Response runPeriodicTaskWithProperties(
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotIngestionRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotIngestionRestletResource.java
index d4696e9e892..b32880d6b94 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotIngestionRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotIngestionRestletResource.java
@@ -131,7 +131,8 @@ public class PinotIngestionRestletResource {
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.MULTIPART_FORM_DATA)
@Path("/ingestFromFile")
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.INGEST_FILE)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.INGEST_FILE,
+ tableScopeParam = "tableNameWithType")
@Authenticate(AccessType.CREATE)
@ApiOperation(value = "Ingest a file", notes = "Creates a segment using
given file and pushes it to Pinot. "
+ "\n All steps happen on the controller. This API is NOT meant for
production environments/large input files. "
@@ -176,7 +177,8 @@ public class PinotIngestionRestletResource {
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.MULTIPART_FORM_DATA)
@Path("/ingestFromURI")
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.INGEST_FILE)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.INGEST_FILE,
+ tableScopeParam = "tableNameWithType")
@Authenticate(AccessType.CREATE)
@ApiOperation(value = "Ingest from the given URI", notes =
"Creates a segment using file at the given URI and pushes it to Pinot. "
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
index 54945be5228..1d0e1ec224a 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
@@ -358,7 +358,8 @@ public class PinotSegmentRestletResource {
/// thus effective in resetting segments or consumers in error states.
@POST
@Path("segments/{tableNameWithType}/{segmentName}/reset")
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.RESET_SEGMENT)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.RESET_SEGMENT,
+ tableScopeParam = "tableNameWithType")
@Authenticate(AccessType.UPDATE)
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation(
@@ -395,7 +396,8 @@ public class PinotSegmentRestletResource {
/// thus effective in resetting segments or consumers in error states.
@POST
@Path("segments/{tableNameWithType}/reset")
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.RESET_SEGMENT)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.RESET_SEGMENT,
+ tableScopeParam = "tableNameWithType")
@Produces(MediaType.APPLICATION_JSON)
@Authenticate(AccessType.UPDATE)
@ApiOperation(
@@ -925,7 +927,8 @@ public class PinotSegmentRestletResource {
@POST
@Path("/segments/{tableNameWithType}/updateZKTimeInterval")
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.UPDATE_TIME_INTERVAL)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.UPDATE_TIME_INTERVAL,
+ tableScopeParam = "tableNameWithType")
@Authenticate(AccessType.UPDATE)
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation(value = "Update the start and end time of the segments based
on latest schema",
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
index af7b6d930c1..d90164e46c7 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
@@ -910,7 +910,7 @@ public class PinotSegmentUploadDownloadRestletResource {
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
@Path("/segments")
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.UPLOAD_SEGMENT)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.UPLOAD_SEGMENT, tableScopeParam = "tableName")
@Authenticate(AccessType.CREATE)
@ApiOperation(value = "Upload a segment", notes = "Upload a segment as json")
@ApiResponses(value = {
@@ -953,7 +953,7 @@ public class PinotSegmentUploadDownloadRestletResource {
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.MULTIPART_FORM_DATA)
@Path("/segments")
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.UPLOAD_SEGMENT)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.UPLOAD_SEGMENT, tableScopeParam = "tableName")
@Authenticate(AccessType.CREATE)
@ApiOperation(value = "Upload a segment", notes = "Upload a segment as
binary")
@ApiResponses(value = {
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
index 345f9f19fbd..3a3ad2c7a87 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
@@ -285,7 +285,7 @@ public class PinotTableRestletResource {
@POST
@Path("/tables/{tableName}/copy")
- @Authorize(targetType = TargetType.TABLE, action =
Actions.Table.CREATE_TABLE)
+ @Authorize(targetType = TargetType.TABLE, paramName = "tableName", action =
Actions.Table.CREATE_TABLE)
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation(value = "Copy a table's schema and config from another
cluster", notes = "Non upsert table only")
public CopyTableResponse copyTable(
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java
index 49a0ad92495..7e42f50f8cc 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java
@@ -261,7 +261,7 @@ public class PinotTaskRestletResource {
@GET
@Path("/tasks/{taskType}/{tableNameWithType}/state")
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.GET_TASK)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.GET_TASK, tableScopeParam = "tableNameWithType")
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation("List all tasks for the given task type")
@ManagedAsync
@@ -281,7 +281,7 @@ public class PinotTaskRestletResource {
@GET
@Path("/tasks/{taskType}/{tableNameWithType}/metadata")
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.GET_TASK)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.GET_TASK, tableScopeParam = "tableNameWithType")
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation("Get task metadata for the given task type and table")
public String getTaskMetadataByTable(
@@ -301,7 +301,8 @@ public class PinotTaskRestletResource {
@DELETE
@Path("/tasks/{taskType}/{tableNameWithType}/metadata")
@Authenticate(AccessType.DELETE)
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.DELETE_TASK)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.DELETE_TASK,
+ tableScopeParam = "tableNameWithType")
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation("Delete task metadata for the given task type and table")
public SuccessResponse deleteTaskMetadataByTable(
@@ -369,7 +370,8 @@ public class PinotTaskRestletResource {
@GET
@Path("/tasks/{taskType}/{tableNameWithType}/debug")
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.DEBUG_TASK)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.DEBUG_TASK,
+ tableScopeParam = "tableNameWithType")
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation("Fetch information for all the tasks for the given task type
and table")
@ManagedAsync
@@ -395,7 +397,7 @@ public class PinotTaskRestletResource {
@GET
@Produces(MediaType.APPLICATION_JSON)
@Path("/tasks/generator/{tableNameWithType}/{taskType}/debug")
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.GET_TASK)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.GET_TASK, tableScopeParam = "tableNameWithType")
@ApiOperation("Fetch task generation information for the recent runs of the
given task for the given table")
@ManagedAsync
public void getTaskGenerationDebugInfo(
@@ -815,7 +817,7 @@ public class PinotTaskRestletResource {
@POST
@Path("/tasks/schedule")
- @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.CREATE_TASK)
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.CREATE_TASK, tableScopeParam = "tableName")
@Produces(MediaType.APPLICATION_JSON)
@Authenticate(AccessType.UPDATE)
@ApiOperation("Schedule tasks and return a map from task type to task name
scheduled. If task type is missing, "
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
index bf6d1c04517..5a3ab6fd40f 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
@@ -1294,11 +1294,12 @@ public class PinotHelixTaskResourceManager {
String taskIdForPartition =
jobContext.getTaskIdForPartition(partition);
TaskPartitionState partitionState =
jobContext.getPartitionState(partition);
TaskConfig helixTaskConfig =
jobConfig.getTaskConfig(taskIdForPartition);
- PinotTaskConfig pinotTaskConfig = null;
- if (helixTaskConfig != null) {
- pinotTaskConfig =
PinotTaskConfig.fromHelixTaskConfig(helixTaskConfig);
- if ((tableNameWithType != null) &&
(!tableNameWithType.equals(pinotTaskConfig.getTableName()))) {
- // Filter task configs that match this table name
+ PinotTaskConfig pinotTaskConfig =
+ helixTaskConfig != null ?
PinotTaskConfig.fromHelixTaskConfig(helixTaskConfig) : null;
+ if (tableNameWithType != null) {
+ // A table filter was requested, so keep only subtasks confirmably
belonging to that table. A missing Helix
+ // task config leaves the subtask's table unknown, so drop it rather
than report it under the filter.
+ if (pinotTaskConfig == null ||
!tableNameWithType.equals(pinotTaskConfig.getTableName())) {
continue;
}
}
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSegmentUploadAuthorizationTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSegmentUploadAuthorizationTest.java
index b078fe249bb..fa688c95468 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSegmentUploadAuthorizationTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSegmentUploadAuthorizationTest.java
@@ -405,7 +405,12 @@ public class PinotSegmentUploadAuthorizationTest extends
ControllerTest {
@Override
public boolean hasAccess(String tableName, AccessType accessType,
HttpHeaders httpHeaders,
String endpointUrl) {
- return tableName == null || _allowedTables.contains(tableName);
+ // A null table name means the request named no table, so it is
cluster-wide and must be decided by the
+ // cluster policy below rather than allowed by default.
+ if (tableName == null) {
+ return hasAccess(accessType, httpHeaders, endpointUrl);
+ }
+ return _allowedTables.contains(tableName);
}
@Override
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/AuthenticationFilterTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/AuthenticationFilterTest.java
index a17d2ef3e23..b777a692615 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/AuthenticationFilterTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/AuthenticationFilterTest.java
@@ -19,12 +19,22 @@
package org.apache.pinot.controller.api.access;
+import java.io.File;
+import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+import javax.ws.rs.BeanParam;
import javax.ws.rs.DELETE;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.PUT;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.QueryParam;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MultivaluedHashMap;
import javax.ws.rs.core.MultivaluedMap;
@@ -36,6 +46,7 @@ import
org.apache.pinot.controller.api.resources.PinotControllerPeriodicTaskRest
import org.apache.pinot.controller.api.resources.PinotInstanceRestletResource;
import org.apache.pinot.controller.api.resources.PinotQueryResource;
import org.apache.pinot.controller.api.resources.PinotTableRestletResource;
+import org.apache.pinot.controller.api.resources.PinotTaskRestletResource;
import org.apache.pinot.core.auth.Actions;
import org.apache.pinot.core.auth.Authorize;
import org.apache.pinot.core.auth.TargetType;
@@ -49,7 +60,7 @@ public class AuthenticationFilterTest {
private final AuthenticationFilter _authFilter = new AuthenticationFilter();
@Test
- public void testExtractTableNameWithTableNameInPathParams() {
+ public void testExtractTableNameWithTableNameInPathParams() throws Exception
{
MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
pathParams.putSingle("tableName", "A");
@@ -58,11 +69,11 @@ public class AuthenticationFilterTest {
queryParams.putSingle("tableName", "D");
queryParams.putSingle("tableNameWithType", "E");
queryParams.putSingle("schemaName", "F");
- assertEquals(AuthenticationFilter.extractTableName(pathParams,
queryParams), "A");
+
assertEquals(AuthenticationFilter.extractTableName(allTableQueryParamsMethod(),
pathParams, queryParams), "A");
}
@Test
- public void testExtractTableNameWithTableNameWithTypeInPathParams() {
+ public void testExtractTableNameWithTableNameWithTypeInPathParams() throws
Exception {
MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
pathParams.putSingle("tableNameWithType", "B");
@@ -70,52 +81,58 @@ public class AuthenticationFilterTest {
queryParams.putSingle("tableName", "D");
queryParams.putSingle("tableNameWithType", "E");
queryParams.putSingle("schemaName", "F");
- assertEquals(AuthenticationFilter.extractTableName(pathParams,
queryParams), "B");
+
assertEquals(AuthenticationFilter.extractTableName(allTableQueryParamsMethod(),
pathParams, queryParams), "B");
}
@Test
- public void testExtractTableNameWithSchemaNameInPathParams() {
+ public void testExtractTableNameWithSchemaNameInPathParams() throws
Exception {
MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
pathParams.putSingle("schemaName", "C");
queryParams.putSingle("tableName", "D");
queryParams.putSingle("tableNameWithType", "E");
queryParams.putSingle("schemaName", "F");
- assertEquals(AuthenticationFilter.extractTableName(pathParams,
queryParams), "C");
+
assertEquals(AuthenticationFilter.extractTableName(allTableQueryParamsMethod(),
pathParams, queryParams), "C");
}
@Test
- public void testExtractTableNameWithTableNameInQueryParams() {
+ public void testExtractTableNameWithTableNameInQueryParams() throws
Exception {
MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
queryParams.putSingle("tableName", "D");
queryParams.putSingle("tableNameWithType", "E");
queryParams.putSingle("schemaName", "F");
- assertEquals(AuthenticationFilter.extractTableName(pathParams,
queryParams), "D");
+
assertEquals(AuthenticationFilter.extractTableName(allTableQueryParamsMethod(),
pathParams, queryParams), "D");
}
@Test
- public void testExtractTableNameWithTableNameWithTypeInQueryParams() {
+ public void testExtractTableNameWithTableNameWithTypeInQueryParams() throws
Exception {
MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
queryParams.putSingle("tableNameWithType", "E");
queryParams.putSingle("schemaName", "F");
- assertEquals(AuthenticationFilter.extractTableName(pathParams,
queryParams), "E");
+
assertEquals(AuthenticationFilter.extractTableName(allTableQueryParamsMethod(),
pathParams, queryParams), "E");
}
@Test
- public void testExtractTableNameWithSchemaNameInQueryParams() {
+ public void testExtractTableNameWithSchemaNameInQueryParams() throws
Exception {
MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
queryParams.putSingle("schemaName", "F");
- assertEquals(AuthenticationFilter.extractTableName(pathParams,
queryParams), "F");
+
assertEquals(AuthenticationFilter.extractTableName(allTableQueryParamsMethod(),
pathParams, queryParams), "F");
}
@Test
- public void testExtractTableNameWithEmptyParams() {
+ public void testExtractTableNameWithEmptyParams() throws Exception {
MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
- assertNull(AuthenticationFilter.extractTableName(pathParams, queryParams));
+
assertNull(AuthenticationFilter.extractTableName(allTableQueryParamsMethod(),
pathParams, queryParams));
+ }
+
+ private static Method allTableQueryParamsMethod()
+ throws Exception {
+ return
AuthenticationFilterTest.class.getMethod("methodWithAllTableQueryParams",
String.class, String.class,
+ String.class);
}
@Test
@@ -124,17 +141,25 @@ public class AuthenticationFilterTest {
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
queryParams.putSingle("tableName", "A");
- Method clusterMethod =
AuthenticationFilterTest.class.getMethod("methodWithClusterAuthorization");
- assertEquals(AuthenticationFilter.extractTableName(clusterMethod,
pathParams, queryParams), "A");
+ // A cluster endpoint that declares a table query parameter is
table-scoped when the caller supplies it.
+ Method clusterMethodWithTableParam =
+
AuthenticationFilterTest.class.getMethod("methodWithClusterAuthorizationAndTableParam",
String.class);
+
assertEquals(AuthenticationFilter.extractTableName(clusterMethodWithTableParam,
pathParams, queryParams), "A");
queryParams.clear();
- assertNull(AuthenticationFilter.extractTableName(clusterMethod,
pathParams, queryParams));
+
assertNull(AuthenticationFilter.extractTableName(clusterMethodWithTableParam,
pathParams, queryParams));
queryParams.putSingle("tableName", "A");
- Method tableMethod =
AuthenticationFilterTest.class.getMethod("methodWithTableAuthorization");
+ Method tableMethod =
AuthenticationFilterTest.class.getMethod("methodWithTableAuthorization",
String.class);
assertEquals(AuthenticationFilter.extractTableName(tableMethod,
pathParams, queryParams), "A");
+ // The same rule applies to the annotated branch: an @Authorize paramName
the endpoint never binds is not the
+ // caller's to supply, so it resolves no table and the request stays
cluster-scoped.
+ Method unboundParamMethod =
+
AuthenticationFilterTest.class.getMethod("methodWithTableAuthorizationAndUnboundParam");
+ assertNull(AuthenticationFilter.extractTableName(unboundParamMethod,
pathParams, queryParams));
+
pathParams.putSingle("materializedViewTableName", "B");
Method customTableParamMethod =
AuthenticationFilterTest.class.getMethod("methodWithCustomTableParamAuthorization");
@@ -144,6 +169,95 @@ public class AuthenticationFilterTest {
assertNull(AuthenticationFilter.extractTableName(customTableParamMethod,
pathParams, queryParams));
}
+ /// `@BeanParam` and methods with no parameter annotations declare no query
params, so `?tableName=` must not
+ /// resolve a table. That pins the fail-closed direction if someone later
widens `declaredQueryParams`.
+ @Test
+ public void testBeanParamAndUnannotatedMethodsFailClosed()
+ throws Exception {
+ MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
+ MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
+ queryParams.putSingle("tableName", "A");
+
+ Method beanParamMethod =
AuthenticationFilterTest.class.getMethod("methodWithBeanParam", Object.class);
+ assertNull(AuthenticationFilter.extractTableName(beanParamMethod,
pathParams, queryParams));
+
+ Method tableBeanParamMethod =
+
AuthenticationFilterTest.class.getMethod("methodWithTableAuthorizationAndBeanParam",
Object.class);
+ assertNull(AuthenticationFilter.extractTableName(tableBeanParamMethod,
pathParams, queryParams));
+
+ Method unannotatedMethod =
AuthenticationFilterTest.class.getMethod("methodWithNoParameterAnnotations");
+ assertNull(AuthenticationFilter.extractTableName(unannotatedMethod,
pathParams, queryParams));
+ }
+
+ /// A caller must not be able to make a cluster endpoint look table-scoped
by appending a table query parameter the
+ /// endpoint never declared. `AccessControlUtils.validatePermission` picks
the table-scoped or the cluster-wide check
+ /// based on whether a table name was resolved, so an undeclared parameter
honored here would let a principal scoped
+ /// to one table pass the cluster check for every cluster endpoint.
+ @Test
+ public void testUndeclaredTableQueryParamDoesNotDowngradeClusterScope()
throws Exception {
+ MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
+ MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
+
+ Method clusterMethod =
AuthenticationFilterTest.class.getMethod("methodWithClusterAuthorization");
+ for (String key : List.of("tableName", "tableNameWithType", "schemaName"))
{
+ queryParams.clear();
+ queryParams.putSingle(key, "A");
+ assertNull(AuthenticationFilter.extractTableName(clusterMethod,
pathParams, queryParams),
+ "undeclared query parameter " + key + " must not resolve a table
name");
+ }
+
+ // A cluster declaration stays authoritative even when its path includes a
table filter.
+ queryParams.clear();
+ pathParams.putSingle("tableName", "A");
+ assertNull(AuthenticationFilter.extractTableName(clusterMethod,
pathParams, queryParams));
+ }
+
+ @Test
+ public void testExplicitClusterTableScopeUsesPathAndQueryParameters() throws
Exception {
+ Method method =
AuthenticationFilterTest.class.getMethod("methodWithClusterAuthorizationAndTableParam",
+ String.class);
+ MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
+ MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
+ pathParams.putSingle("tableName", "pathTable");
+ queryParams.putSingle("tableName", "queryTable");
+ assertEquals(AuthenticationFilter.extractTableName(method, pathParams,
queryParams), "pathTable");
+ pathParams.clear();
+ assertEquals(AuthenticationFilter.extractTableName(method, pathParams,
queryParams), "queryTable");
+ queryParams.clear();
+ assertNull(AuthenticationFilter.extractTableName(method, pathParams,
queryParams));
+ }
+
+ @Test
+ public void testTaskDebugFilterDoesNotGrantTableScope() {
+ Method method = Arrays.stream(PinotTaskRestletResource.class
+ .getDeclaredMethods()).filter(candidate ->
candidate.getName().equals("getTaskDebugInfo"))
+ .findFirst().orElseThrow();
+ MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
+ queryParams.putSingle("tableName", "ownedTable_OFFLINE");
+ assertNull(AuthenticationFilter.extractTableName(method, new
MultivaluedHashMap<>(), queryParams));
+ }
+
+ @Test
+ public void testDeclaredTableQueryParamScopesRequest() throws Exception {
+ MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
+ MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
+
+ Method typedTableParamMethod = AuthenticationFilterTest.class.getMethod(
+ "methodWithClusterAuthorizationAndTypedTableParam", String.class);
+ queryParams.putSingle("tableNameWithType", "A");
+ assertEquals(AuthenticationFilter.extractTableName(typedTableParamMethod,
pathParams, queryParams), "A");
+
+ // Only the declared key counts, even when an undeclared one of higher
precedence is also supplied.
+ queryParams.putSingle("tableName", "B");
+ assertEquals(AuthenticationFilter.extractTableName(typedTableParamMethod,
pathParams, queryParams), "A");
+
+ Method schemaParamMethod =
+
AuthenticationFilterTest.class.getMethod("methodWithClusterAuthorizationAndSchemaParam",
String.class);
+ queryParams.clear();
+ queryParams.putSingle("schemaName", "C");
+ assertEquals(AuthenticationFilter.extractTableName(schemaParamMethod,
pathParams, queryParams), "C");
+ }
+
@Test
public void testExtractAccessTypeWithAuthAnnotation() throws Exception {
Method method =
AuthenticationFilterTest.class.getMethod("methodWithAuthAnnotation");
@@ -179,15 +293,102 @@ public class AuthenticationFilterTest {
@Test
public void testSegmentCompletionGetEndpointsDeclareCreateAccess() {
+ MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
+ MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
+ queryParams.putSingle("tableName", "someTable");
for (String methodName : new String[]{"extendBuildTime",
"segmentConsumed", "segmentStoppedConsuming",
"segmentCommitStart", "reduceSegmentSize"}) {
Method method =
Arrays.stream(LLCSegmentCompletionHandlers.class.getDeclaredMethods())
.filter(candidate ->
candidate.getName().equals(methodName)).findFirst().orElseThrow();
assertEquals(_authFilter.extractAccessType(method), AccessType.CREATE);
assertEquals(method.getAnnotation(Authorize.class).action(),
Actions.Cluster.COMMIT_SEGMENT);
+ // These callbacks name no table, so they take the cluster-wide check.
That is why a table-scoped server token
+ // is denied and realtime commit stalls — the consequence documented on
BaseBasicAuthAccessControl.
+ assertNull(AuthenticationFilter.extractTableName(method, pathParams,
queryParams),
+ methodName + " must not resolve a table name");
}
}
+ /// Guards the assumption stated on
[org.apache.pinot.core.auth.FineGrainedAuthUtils#getDeclaredQueryParams]: a
+ /// table target's parameter must be bound as a `@PathParam` or a
method-level `@QueryParam`. An endpoint binding it
+ /// via `@BeanParam` or field injection would resolve no table and hard-fail
with 400 for every caller, admin
+ /// included. Cluster endpoints opting into table scope must also bind their
`tableScopeParam`, otherwise a typo
+ /// would silently retain cluster-wide scope and deny table-scoped callers.
+ @Test
+ public void testAuthorizeTableParamsAreBoundOnTheMethod()
+ throws Exception {
+ List<Class<?>> resourceClasses = controllerResourceClasses();
+ assertTrue(resourceClasses.size() > 20, "expected to discover the
controller resource classes, found "
+ + resourceClasses.size());
+ List<String> violations = new ArrayList<>();
+ for (Class<?> resourceClass : resourceClasses) {
+ for (Method method : resourceClass.getDeclaredMethods()) {
+ Authorize authorize = method.getAnnotation(Authorize.class);
+ if (authorize == null) {
+ continue;
+ }
+ String parameterName;
+ if (authorize.targetType() == TargetType.TABLE) {
+ parameterName = authorize.paramName();
+ } else if (authorize.targetType() == TargetType.CLUSTER &&
!authorize.tableScopeParam().isEmpty()) {
+ parameterName = authorize.tableScopeParam();
+ } else {
+ continue;
+ }
+ if (!bindsParameter(method, parameterName)) {
+ violations.add(resourceClass.getSimpleName() + "#" +
method.getName() + " declares table parameter '"
+ + parameterName + "'");
+ }
+ }
+ }
+ assertTrue(violations.isEmpty(), "@Authorize table parameters must be
bound as @PathParam or @QueryParam: "
+ + violations);
+ }
+
+ /// Loads every class in the controller's `api.resources` package from the
compiled output, so the guard covers
+ /// endpoints added later without anyone having to remember to list them
here.
+ private static List<Class<?>> controllerResourceClasses()
+ throws Exception {
+ String packageName = PinotTableRestletResource.class.getPackageName();
+ // Resolve through URI so a build path containing spaces or other escaped
characters still works.
+ Path classesRoot = Path.of(
+
PinotTableRestletResource.class.getProtectionDomain().getCodeSource().getLocation().toURI());
+ Path packageRoot = classesRoot.resolve(packageName.replace('.', '/'));
+ List<Class<?>> classes = new ArrayList<>();
+ // Walked recursively so sub-packages such as api.resources.ddl are
covered as they are added.
+ try (Stream<Path> paths = Files.walk(packageRoot)) {
+ for (Path classFile : paths.filter(Files::isRegularFile).toList()) {
+ String fileName = classFile.getFileName().toString();
+ if (!fileName.endsWith(".class") || fileName.contains("$")) {
+ continue;
+ }
+ String relative = classesRoot.relativize(classFile).toString();
+ String className = relative.substring(0, relative.length() -
".class".length())
+ .replace(File.separatorChar, '.');
+ try {
+ classes.add(Class.forName(className));
+ } catch (ClassNotFoundException | NoClassDefFoundError e) {
+ // Not loadable in the test classpath; it cannot serve requests
either.
+ }
+ }
+ }
+ return classes;
+ }
+
+ private static boolean bindsParameter(Method method, String parameterName) {
+ for (Annotation[] parameterAnnotations : method.getParameterAnnotations())
{
+ for (Annotation annotation : parameterAnnotations) {
+ if (annotation instanceof PathParam pathParam &&
pathParam.value().equals(parameterName)) {
+ return true;
+ }
+ if (annotation instanceof QueryParam queryParam &&
queryParam.value().equals(parameterName)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
@Test
public void testBrokerForTableEndpointDeclaresTableAuthorization() throws
Exception {
Method method =
PinotBrokerRestletResource.class.getMethod("getBrokersForTableV2",
String.class, String.class,
@@ -248,12 +449,50 @@ public class AuthenticationFilterTest {
public void methodWithClusterAuthorization() {
}
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.EXECUTE_TASK, tableScopeParam = "tableName")
+ public void
methodWithClusterAuthorizationAndTableParam(@QueryParam("tableName") String
tableName) {
+ }
+
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.INGEST_FILE,
+ tableScopeParam = "tableNameWithType")
+ public void methodWithClusterAuthorizationAndTypedTableParam(
+ @QueryParam("tableNameWithType") String tableNameWithType) {
+ }
+
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.GET_SCHEMA, tableScopeParam = "schemaName")
+ public void
methodWithClusterAuthorizationAndSchemaParam(@QueryParam("schemaName") String
schemaName) {
+ }
+
+ /// Stands in for an endpoint that binds every table-identifying query
parameter, so the precedence rules can be
+ /// exercised independently of the declaration filter.
+ public void methodWithAllTableQueryParams(@QueryParam("tableName") String
tableName,
+ @QueryParam("tableNameWithType") String tableNameWithType,
@QueryParam("schemaName") String schemaName) {
+ }
+
+ @Authorize(targetType = TargetType.TABLE, paramName = "tableName", action =
Actions.Table.GET_TABLE_CONFIG)
+ public void methodWithTableAuthorization(@QueryParam("tableName") String
tableName) {
+ }
+
+ /// Declares a table target whose parameter the method never binds — a
misannotation the resolution must fail closed
+ /// on rather than trust the caller for.
@Authorize(targetType = TargetType.TABLE, paramName = "tableName", action =
Actions.Table.GET_TABLE_CONFIG)
- public void methodWithTableAuthorization() {
+ public void methodWithTableAuthorizationAndUnboundParam() {
}
@Authorize(targetType = TargetType.TABLE, paramName =
"materializedViewTableName",
action = Actions.Table.GET_TABLE_CONFIG)
public void methodWithCustomTableParamAuthorization() {
}
+
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.GET_CLUSTER_CONFIG)
+ public void methodWithBeanParam(@BeanParam Object bean) {
+ }
+
+ @Authorize(targetType = TargetType.TABLE, paramName = "tableName", action =
Actions.Table.GET_TABLE_CONFIG)
+ public void methodWithTableAuthorizationAndBeanParam(@BeanParam Object bean)
{
+ }
+
+ @Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.GET_CLUSTER_CONFIG)
+ public void methodWithNoParameterAnnotations() {
+ }
}
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/BasicAuthAccessControlFactoryTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/BasicAuthAccessControlFactoryTest.java
index 3d9dcc9337d..a93dcd4732c 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/BasicAuthAccessControlFactoryTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/BasicAuthAccessControlFactoryTest.java
@@ -57,6 +57,12 @@ public class BasicAuthAccessControlFactoryTest {
private static final String FULL_PASSWORD = "fullPassword";
private static final String WILDCARD_USER = "wildcard";
private static final String WILDCARD_PASSWORD = "wildcardPassword";
+ private static final String SCOPED_ADMIN_USER = "scopedAdmin";
+ private static final String SCOPED_ADMIN_PASSWORD = "scopedAdminPassword";
+ private static final String EXCLUDED_USER = "excluded";
+ private static final String EXCLUDED_PASSWORD = "excludedPassword";
+ private static final String CLUSTER_READER_USER = "clusterReader";
+ private static final String CLUSTER_READER_PASSWORD =
"clusterReaderPassword";
private static final String CORRUPT_USER = "corrupt";
private static final String ENDPOINT_URL = "/cluster/configs";
@@ -68,11 +74,23 @@ public class BasicAuthAccessControlFactoryTest {
@Test(dataProvider = "accessControls")
public void testClusterPermissions(AccessControl accessControl) {
- HttpHeaders restrictedHeaders = headers(RESTRICTED_USER,
RESTRICTED_PASSWORD);
- assertTrue(accessControl.hasAccess(AccessType.READ, restrictedHeaders,
ENDPOINT_URL));
- assertFalse(accessControl.hasAccess(AccessType.CREATE, restrictedHeaders,
ENDPOINT_URL));
- assertFalse(accessControl.hasAccess(AccessType.UPDATE, restrictedHeaders,
ENDPOINT_URL));
- assertFalse(accessControl.hasAccess(AccessType.DELETE, restrictedHeaders,
ENDPOINT_URL));
+ // A principal confined to a subset of tables holds no cluster-wide
access, whichever permissions it was granted.
+ for (Map.Entry<String, String> tableScopedUser : Map.of(RESTRICTED_USER,
RESTRICTED_PASSWORD, SCOPED_ADMIN_USER,
+ SCOPED_ADMIN_PASSWORD, EXCLUDED_USER, EXCLUDED_PASSWORD).entrySet()) {
+ HttpHeaders tableScopedHeaders = headers(tableScopedUser.getKey(),
tableScopedUser.getValue());
+ for (AccessType accessType : AccessType.values()) {
+ assertFalse(accessControl.hasAccess(accessType, tableScopedHeaders,
ENDPOINT_URL),
+ tableScopedUser.getKey() + " must not hold cluster " + accessType);
+ }
+ }
+
+ // Table scope is necessary but not sufficient: an unrestricted principal
is still held to its permissions.
+ HttpHeaders clusterReaderHeaders = headers(CLUSTER_READER_USER,
CLUSTER_READER_PASSWORD);
+ assertTrue(accessControl.hasAccess(AccessType.READ, clusterReaderHeaders,
ENDPOINT_URL));
+ for (AccessType accessType : List.of(AccessType.CREATE, AccessType.UPDATE,
AccessType.DELETE)) {
+ assertFalse(accessControl.hasAccess(accessType, clusterReaderHeaders,
ENDPOINT_URL),
+ CLUSTER_READER_USER + " holds only READ, so it must not hold cluster
" + accessType);
+ }
HttpHeaders fullHeaders = headers(FULL_USER, FULL_PASSWORD);
for (AccessType accessType : AccessType.values()) {
@@ -117,7 +135,12 @@ public class BasicAuthAccessControlFactoryTest {
@Test(dataProvider = "accessControls")
public void testAuthenticationProbeRemainsNonThrowing(AccessControl
accessControl) {
assertFalse(accessControl.hasAccess(headers("unknown", "wrong"),
TargetType.CLUSTER));
+ assertTrue(accessControl.hasAccess(headers(FULL_USER, FULL_PASSWORD),
TargetType.CLUSTER));
+
+ // This probe backs the UI login gate, so it stays an authentication
check: a table-scoped principal must still be
+ // able to sign in and work with the tables it is scoped to. Cluster
requests are denied by the checks above.
assertTrue(accessControl.hasAccess(headers(RESTRICTED_USER,
RESTRICTED_PASSWORD), TargetType.CLUSTER));
+ assertTrue(accessControl.hasAccess(headers(SCOPED_ADMIN_USER,
SCOPED_ADMIN_PASSWORD), TargetType.CLUSTER));
}
@Test(dataProvider = "accessControls")
@@ -140,6 +163,11 @@ public class BasicAuthAccessControlFactoryTest {
for (AccessType accessType : AccessType.values()) {
assertTrue(accessControl.hasAccess(ALLOWED_TABLE, accessType,
fullHeaders, ENDPOINT_URL));
}
+
+ // An exclude-list principal loses cluster access but keeps every table it
was not excluded from.
+ HttpHeaders excludedHeaders = headers(EXCLUDED_USER, EXCLUDED_PASSWORD);
+ assertTrue(accessControl.hasAccess(ALLOWED_TABLE, AccessType.READ,
excludedHeaders, ENDPOINT_URL));
+ assertFalse(accessControl.hasAccess("otherTable", AccessType.READ,
excludedHeaders, ENDPOINT_URL));
}
@Test(dataProvider = "accessControls")
@@ -160,6 +188,45 @@ public class BasicAuthAccessControlFactoryTest {
BasicAuthTokenUtils.toBasicAuthToken(RESTRICTED_USER,
RESTRICTED_PASSWORD)));
}
+ /// Regression test for [issue
14595](https://github.com/apache/pinot/issues/14595): a principal scoped to a
single
+ /// table held every permission on cluster-level endpoints, letting it
delete resources such as other users. Table
+ /// scope must be honored on endpoints that name no table, while the
principal keeps access to its own table.
+ @Test(dataProvider = "accessControls")
+ public void
testTableScopedPrincipalCannotReachClusterEndpoints(AccessControl
accessControl) {
+ HttpHeaders scopedAdminHeaders = headers(SCOPED_ADMIN_USER,
SCOPED_ADMIN_PASSWORD);
+
+ ControllerApplicationException exception =
expectThrows(ControllerApplicationException.class,
+ () -> AccessControlUtils.validatePermission(null, AccessType.DELETE,
scopedAdminHeaders, "/users/admin",
+ accessControl));
+ assertEquals(exception.getResponse().getStatus(),
Response.Status.FORBIDDEN.getStatusCode());
+
+ // The same principal still holds every permission on the table it is
scoped to.
+ for (AccessType accessType : AccessType.values()) {
+ assertTrue(accessControl.hasAccess(ALLOWED_TABLE, accessType,
scopedAdminHeaders, ENDPOINT_URL));
+ }
+ assertFalse(accessControl.hasAccess("otherTable", AccessType.DELETE,
scopedAdminHeaders, ENDPOINT_URL));
+
+ // An unrestricted principal with the same permission is still authorized
cluster-wide.
+ AccessControlUtils.validatePermission(null, AccessType.DELETE,
headers(FULL_USER, FULL_PASSWORD), "/users/admin",
+ accessControl);
+
+ // Omitting the table name must not sidestep the scope rule via the table
overload. An exclude-list principal is
+ // the case that slips through a plain hasTable(null) check, since it has
no allow-list to fail against.
+ for (String tableScopedUser : List.of(SCOPED_ADMIN_USER, EXCLUDED_USER)) {
+ HttpHeaders tableScopedHeaders = headers(tableScopedUser,
tableScopedUser + "Password");
+ assertFalse(accessControl.hasAccess(null, AccessType.DELETE,
tableScopedHeaders, "/users/admin"),
+ tableScopedUser + " must not be authorized for a request that names
no table");
+ }
+
+ // The same branch must still authorize an unrestricted principal, and
still hold it to its permissions —
+ // /auth/verify reaches this overload with an absent table name, so an
over-denial here misreports admins.
+ assertTrue(accessControl.hasAccess(null, AccessType.DELETE,
headers(FULL_USER, FULL_PASSWORD), "/users/admin"));
+ assertFalse(accessControl.hasAccess(null, AccessType.DELETE,
+ headers(CLUSTER_READER_USER, CLUSTER_READER_PASSWORD),
"/users/admin"));
+ assertTrue(accessControl.hasAccess(null, AccessType.READ,
+ headers(CLUSTER_READER_USER, CLUSTER_READER_PASSWORD), "/users"));
+ }
+
@Test
public void testMalformedCachedZkPrincipalFailsClosed()
throws Exception {
@@ -175,13 +242,21 @@ public class BasicAuthAccessControlFactoryTest {
private static AccessControl createStaticAccessControl() {
Map<String, Object> properties = Map.ofEntries(
- Map.entry("controller.admin.access.control.principals",
"restricted,full,wildcard"),
+ Map.entry("controller.admin.access.control.principals",
+ "restricted,full,wildcard,scopedAdmin,excluded,clusterReader"),
Map.entry("controller.admin.access.control.principals.restricted.password",
RESTRICTED_PASSWORD),
Map.entry("controller.admin.access.control.principals.restricted.tables",
ALLOWED_TABLE),
Map.entry("controller.admin.access.control.principals.restricted.permissions",
"read"),
Map.entry("controller.admin.access.control.principals.full.password",
FULL_PASSWORD),
Map.entry("controller.admin.access.control.principals.full.permissions",
"create,read,update,delete"),
-
Map.entry("controller.admin.access.control.principals.wildcard.password",
WILDCARD_PASSWORD));
+
Map.entry("controller.admin.access.control.principals.wildcard.password",
WILDCARD_PASSWORD),
+
Map.entry("controller.admin.access.control.principals.scopedAdmin.password",
SCOPED_ADMIN_PASSWORD),
+
Map.entry("controller.admin.access.control.principals.scopedAdmin.tables",
ALLOWED_TABLE),
+
Map.entry("controller.admin.access.control.principals.scopedAdmin.permissions",
"create,read,update,delete"),
+
Map.entry("controller.admin.access.control.principals.excluded.password",
EXCLUDED_PASSWORD),
+
Map.entry("controller.admin.access.control.principals.excluded.excludeTables",
"otherTable"),
+
Map.entry("controller.admin.access.control.principals.clusterReader.password",
CLUSTER_READER_PASSWORD),
+
Map.entry("controller.admin.access.control.principals.clusterReader.permissions",
"read"));
BasicAuthAccessControlFactory factory = new
BasicAuthAccessControlFactory();
factory.init(new PinotConfiguration(properties));
return factory.create();
@@ -195,6 +270,11 @@ public class BasicAuthAccessControlFactoryTest {
user(FULL_USER, FULL_PASSWORD, null,
List.of(org.apache.pinot.spi.config.user.AccessType.values())),
user(WILDCARD_USER, WILDCARD_PASSWORD, null, null),
+ user(SCOPED_ADMIN_USER, SCOPED_ADMIN_PASSWORD, List.of(ALLOWED_TABLE),
+ List.of(org.apache.pinot.spi.config.user.AccessType.values())),
+ user(EXCLUDED_USER, EXCLUDED_PASSWORD, null, List.of("otherTable"),
null),
+ user(CLUSTER_READER_USER, CLUSTER_READER_PASSWORD, null,
+ List.of(org.apache.pinot.spi.config.user.AccessType.READ)),
new UserConfig(CORRUPT_USER, " ", ComponentType.CONTROLLER.name(),
RoleType.USER.name(), null, null, null));
List<String> userNames =
users.stream().map(UserConfig::getUsernameWithComponent).toList();
List<String> userPaths = userNames.stream().map(name -> "/CONFIGS/USER/" +
name).toList();
@@ -222,8 +302,13 @@ public class BasicAuthAccessControlFactoryTest {
private static UserConfig user(String username, String password,
List<String> tables,
List<org.apache.pinot.spi.config.user.AccessType> permissions) {
+ return user(username, password, tables, null, permissions);
+ }
+
+ private static UserConfig user(String username, String password,
List<String> tables, List<String> excludeTables,
+ List<org.apache.pinot.spi.config.user.AccessType> permissions) {
return new UserConfig(username, BcryptUtils.encrypt(password),
ComponentType.CONTROLLER.name(),
- RoleType.USER.name(), tables, null, permissions);
+ RoleType.USER.name(), tables, excludeTables, permissions);
}
private static HttpHeaders headers(String username, String password) {
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/ControllerClusterBasicAuthAuthorizationTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/ControllerClusterBasicAuthAuthorizationTest.java
index 1bca6300fbe..74b600a57d3 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/ControllerClusterBasicAuthAuthorizationTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/access/ControllerClusterBasicAuthAuthorizationTest.java
@@ -46,8 +46,8 @@ import static org.testng.Assert.fail;
public class ControllerClusterBasicAuthAuthorizationTest extends
ControllerTest {
private static final String ADMIN_USER = "clusterAdmin";
private static final String ADMIN_PASSWORD = "clusterAdminPassword";
- private static final String RESTRICTED_USER = "clusterReader";
- private static final String RESTRICTED_PASSWORD = "clusterReaderPassword";
+ private static final String TABLE_READ_USER = "tableReader";
+ private static final String TABLE_READ_PASSWORD = "tableReaderPassword";
private static final String CREATE_USER = "clusterCreator";
private static final String CREATE_PASSWORD = "clusterCreatorPassword";
private static final String UPDATE_USER = "clusterUpdater";
@@ -56,6 +56,8 @@ public class ControllerClusterBasicAuthAuthorizationTest
extends ControllerTest
private static final String DELETE_PASSWORD = "clusterDeleterPassword";
private static final String TABLE_UPDATE_USER = "tableUpdater";
private static final String TABLE_UPDATE_PASSWORD = "tableUpdaterPassword";
+ private static final String TABLE_ADMIN_USER = "tableAdmin";
+ private static final String TABLE_ADMIN_PASSWORD = "tableAdminPassword";
private static final String ALLOWED_TABLE = "allowedTable";
private static final String DISALLOWED_TABLE = "disallowedTable";
@@ -80,39 +82,43 @@ public class ControllerClusterBasicAuthAuthorizationTest
extends ControllerTest
controllerStarted = true;
Map<String, String> adminHeaders = authHeaders(ADMIN_USER,
ADMIN_PASSWORD);
- Map<String, String> restrictedHeaders = authHeaders(RESTRICTED_USER,
RESTRICTED_PASSWORD);
+ Map<String, String> tableReadHeaders = authHeaders(TABLE_READ_USER,
TABLE_READ_PASSWORD);
Map<String, String> createHeaders = authHeaders(CREATE_USER,
CREATE_PASSWORD);
Map<String, String> updateHeaders = authHeaders(UPDATE_USER,
UPDATE_PASSWORD);
Map<String, String> deleteHeaders = authHeaders(DELETE_USER,
DELETE_PASSWORD);
Map<String, String> tableUpdateHeaders = authHeaders(TABLE_UPDATE_USER,
TABLE_UPDATE_PASSWORD);
+ Map<String, String> tableAdminHeaders = authHeaders(TABLE_ADMIN_USER,
TABLE_ADMIN_PASSWORD);
String clusterConfigsUrl = _controllerBaseApiUrl + "/cluster/configs";
if (zkBacked) {
addZkUsers();
- TestUtils.waitForCondition(aVoid -> getStatus(clusterConfigsUrl,
restrictedHeaders) == 200
+ // Unloaded users answer 401; once loaded every non-admin principal
here is denied cluster access with 403.
+ TestUtils.waitForCondition(aVoid -> getStatus(clusterConfigsUrl,
tableReadHeaders) == 403
&& getStatus(clusterConfigsUrl, createHeaders) == 403
&& getStatus(clusterConfigsUrl, updateHeaders) == 403
&& getStatus(clusterConfigsUrl, deleteHeaders) == 403
- && getStatus(clusterConfigsUrl, tableUpdateHeaders) == 403,
TIMEOUT_MS,
+ && getStatus(clusterConfigsUrl, tableUpdateHeaders) == 403
+ && getStatus(clusterConfigsUrl, tableAdminHeaders) == 403,
TIMEOUT_MS,
"Controller users were not loaded from ZooKeeper");
}
assertGetStatus(clusterConfigsUrl, authHeaders("unknown", "incorrect"),
401);
- assertGetStatus(clusterConfigsUrl, restrictedHeaders, 200);
+ assertGetStatus(clusterConfigsUrl, adminHeaders, 200);
assertTrue(JsonUtils.stringToJsonNode(
- sendPostRequest(_controllerBaseApiUrl + "/query/tableNames", "[]",
restrictedHeaders)).isEmpty());
+ sendPostRequest(_controllerBaseApiUrl + "/query/tableNames", "[]",
adminHeaders)).isEmpty());
assertTrue(JsonUtils.stringToJsonNode(
- sendPostRequest(_controllerBaseApiUrl +
"/instances/updateTags/validate", "[]", restrictedHeaders))
+ sendPostRequest(_controllerBaseApiUrl +
"/instances/updateTags/validate", "[]", adminHeaders))
.isEmpty());
- assertTableScope(restrictedHeaders, tableUpdateHeaders,
authHeaders("unknown", "incorrect"));
+ assertClusterScope(tableReadHeaders, tableAdminHeaders);
+ assertTableScope(tableReadHeaders, tableUpdateHeaders,
authHeaders("unknown", "incorrect"));
String configKey = "pinot.controller.auth.test." + configKeySuffix;
String zkPath = "/controller-auth-test-" + configKeySuffix;
String zkCreateUrl = _controllerBaseApiUrl + "/zk/create?path=" + zkPath;
String zkDeleteUrl = _controllerBaseApiUrl + "/zk/delete?path=" + zkPath;
- assertHttpError(403, () -> sendPostRequest(zkCreateUrl, "{not-json",
restrictedHeaders));
- assertHttpError(403, () -> sendPostRequest(clusterConfigsUrl,
"{not-json", restrictedHeaders));
- assertHttpError(403, () -> sendDeleteRequest(clusterConfigsUrl + "/" +
configKey, restrictedHeaders));
+ assertHttpError(403, () -> sendPostRequest(zkCreateUrl, "{not-json",
tableReadHeaders));
+ assertHttpError(403, () -> sendPostRequest(clusterConfigsUrl,
"{not-json", tableReadHeaders));
+ assertHttpError(403, () -> sendDeleteRequest(clusterConfigsUrl + "/" +
configKey, tableReadHeaders));
assertHttpError(403, () -> sendPostRequest(zkCreateUrl, "{not-json",
updateHeaders));
assertHttpError(403, () -> sendPostRequest(clusterConfigsUrl,
"{not-json", createHeaders));
@@ -163,14 +169,15 @@ public class ControllerClusterBasicAuthAuthorizationTest
extends ControllerTest
}
controllerConfiguration.put("controller.admin.access.control.principals",
- String.join(",", ADMIN_USER, RESTRICTED_USER, CREATE_USER,
UPDATE_USER, DELETE_USER, TABLE_UPDATE_USER));
+ String.join(",", ADMIN_USER, TABLE_READ_USER, CREATE_USER,
UPDATE_USER, DELETE_USER, TABLE_UPDATE_USER,
+ TABLE_ADMIN_USER));
controllerConfiguration.put("controller.admin.access.control.principals."
+ ADMIN_USER + ".password",
ADMIN_PASSWORD);
- controllerConfiguration.put("controller.admin.access.control.principals."
+ RESTRICTED_USER + ".password",
- RESTRICTED_PASSWORD);
- controllerConfiguration.put("controller.admin.access.control.principals."
+ RESTRICTED_USER + ".tables",
+ controllerConfiguration.put("controller.admin.access.control.principals."
+ TABLE_READ_USER + ".password",
+ TABLE_READ_PASSWORD);
+ controllerConfiguration.put("controller.admin.access.control.principals."
+ TABLE_READ_USER + ".tables",
ALLOWED_TABLE);
- controllerConfiguration.put("controller.admin.access.control.principals."
+ RESTRICTED_USER + ".permissions",
+ controllerConfiguration.put("controller.admin.access.control.principals."
+ TABLE_READ_USER + ".permissions",
"read");
addStaticPrincipal(controllerConfiguration, CREATE_USER, CREATE_PASSWORD,
"create");
addStaticPrincipal(controllerConfiguration, UPDATE_USER, UPDATE_PASSWORD,
"update");
@@ -178,6 +185,9 @@ public class ControllerClusterBasicAuthAuthorizationTest
extends ControllerTest
addStaticPrincipal(controllerConfiguration, TABLE_UPDATE_USER,
TABLE_UPDATE_PASSWORD, "update");
controllerConfiguration.put("controller.admin.access.control.principals."
+ TABLE_UPDATE_USER + ".tables",
ALLOWED_TABLE);
+ addStaticPrincipal(controllerConfiguration, TABLE_ADMIN_USER,
TABLE_ADMIN_PASSWORD, "create,read,update,delete");
+ controllerConfiguration.put("controller.admin.access.control.principals."
+ TABLE_ADMIN_USER + ".tables",
+ ALLOWED_TABLE);
}
private static void addStaticPrincipal(Map<String, Object>
controllerConfiguration, String username,
@@ -189,13 +199,71 @@ public class ControllerClusterBasicAuthAuthorizationTest
extends ControllerTest
private void addZkUsers()
throws IOException {
- addZkUser(RESTRICTED_USER, RESTRICTED_PASSWORD, List.of(ALLOWED_TABLE),
+ addZkUser(TABLE_READ_USER, TABLE_READ_PASSWORD, List.of(ALLOWED_TABLE),
org.apache.pinot.spi.config.user.AccessType.READ);
addZkUser(CREATE_USER, CREATE_PASSWORD, null,
org.apache.pinot.spi.config.user.AccessType.CREATE);
addZkUser(UPDATE_USER, UPDATE_PASSWORD, null,
org.apache.pinot.spi.config.user.AccessType.UPDATE);
addZkUser(DELETE_USER, DELETE_PASSWORD, null,
org.apache.pinot.spi.config.user.AccessType.DELETE);
addZkUser(TABLE_UPDATE_USER, TABLE_UPDATE_PASSWORD, List.of(ALLOWED_TABLE),
org.apache.pinot.spi.config.user.AccessType.UPDATE);
+ addZkUser(TABLE_ADMIN_USER, TABLE_ADMIN_PASSWORD, List.of(ALLOWED_TABLE),
+ org.apache.pinot.spi.config.user.AccessType.values());
+ }
+
+ /// Asserts the fix for [issue
14595](https://github.com/apache/pinot/issues/14595): principals confined to a
subset
+ /// of tables are denied endpoints that address the cluster rather than a
table, whatever permissions they hold.
+ private void assertClusterScope(Map<String, String> tableReadHeaders,
Map<String, String> tableAdminHeaders)
+ throws IOException {
+ // The reported exploit: a table-scoped user deleting another user through
a cluster-level endpoint.
+ assertHttpError(403,
+ () -> sendDeleteRequest(_controllerBaseApiUrl + "/users/" + ADMIN_USER
+ "?component=CONTROLLER",
+ tableAdminHeaders));
+ assertGetStatus(_controllerBaseApiUrl + "/users", tableAdminHeaders, 403);
+
+ // Other cluster-level endpoints follow the same rule, including read-only
ones.
+ for (Map<String, String> tableScopedHeaders : List.of(tableReadHeaders,
tableAdminHeaders)) {
+ assertGetStatus(_controllerBaseApiUrl + "/cluster/configs",
tableScopedHeaders, 403);
+ assertGetStatus(_controllerBaseApiUrl + "/cluster/info",
tableScopedHeaders, 403);
+ assertGetStatus(_controllerBaseApiUrl + "/instances",
tableScopedHeaders, 403);
+ assertHttpError(403,
+ () -> sendPostRequest(_controllerBaseApiUrl + "/query/tableNames",
"[]", tableScopedHeaders));
+
+ // Cluster-wide listings are denied too: they enumerate every table in
the cluster rather than filtering to the
+ // principal's scope, so the controller UI's listing pages are
unavailable to a table-scoped principal.
+ assertGetStatus(_controllerBaseApiUrl + "/tables", tableScopedHeaders,
403);
+ assertGetStatus(_controllerBaseApiUrl + "/schemas", tableScopedHeaders,
403);
+
+ // Naming a table the principal owns must not make a cluster endpoint
pass as a table-scoped request. These
+ // endpoints declare no table parameter, so the parameter is not theirs
to be scoped by.
+ for (String tableParam : List.of("tableName", "tableNameWithType",
"schemaName")) {
+ String scopedSuffix = "?" + tableParam + "=" + ALLOWED_TABLE;
+ assertGetStatus(_controllerBaseApiUrl + "/cluster/configs" +
scopedSuffix, tableScopedHeaders, 403);
+ assertGetStatus(_controllerBaseApiUrl + "/users" + scopedSuffix,
tableScopedHeaders, 403);
+ }
+ }
+ assertHttpError(403, () -> sendDeleteRequest(
+ _controllerBaseApiUrl + "/users/" + ADMIN_USER +
"?component=CONTROLLER&tableName=" + ALLOWED_TABLE,
+ tableAdminHeaders));
+
+ // A table filter only limits subtasks; task-level debug metadata remains
cluster-wide.
+ assertGetStatus(_controllerBaseApiUrl +
"/tasks/task/missing/debug?tableName=" + ALLOWED_TABLE + "_OFFLINE",
+ tableAdminHeaders, 403);
+
+ // The principal keeps the endpoints that genuinely declare a table
parameter, cluster action or not.
+ assertGetStatus(_controllerBaseApiUrl +
"/periodictask/run?taskname=missing&tableName=" + ALLOWED_TABLE,
+ tableAdminHeaders, 404);
+ // Omitting that parameter means "every table", which is the escalation
this change closes for this endpoint.
+ assertGetStatus(_controllerBaseApiUrl +
"/periodictask/run?taskname=missing", tableAdminHeaders, 403);
+
+ // Segment upload documents tableName as optional, falling back to the
segment metadata. A table-scoped principal
+ // must now send it, because without it the request names no table and is
treated as cluster-wide.
+ assertHttpError(403, () -> sendPostRequest(_controllerBaseApiUrl +
"/segments", "", tableAdminHeaders));
+
+ // The controller's multi-stage query path authorizes cluster-wide by
design (it has no cross-table rule), so a
+ // table-scoped principal is denied there even for a table it owns. The
broker path is unaffected.
+ assertHttpError(403, () -> sendPostRequest(_controllerBaseApiUrl + "/sql",
+ "{\"sql\":\"SELECT 1 FROM " + ALLOWED_TABLE +
"\",\"queryOptions\":\"useMultistageEngine=true\"}",
+ tableAdminHeaders));
}
private void assertTableScope(Map<String, String> readHeaders, Map<String,
String> updateHeaders,
@@ -212,10 +280,10 @@ public class ControllerClusterBasicAuthAuthorizationTest
extends ControllerTest
}
private void addZkUser(String username, String password, List<String> tables,
- org.apache.pinot.spi.config.user.AccessType permission)
+ org.apache.pinot.spi.config.user.AccessType... permissions)
throws IOException {
_helixResourceManager.addUser(new UserConfig(username, password,
ComponentType.CONTROLLER.name(),
- RoleType.USER.name(), tables, null, List.of(permission)));
+ RoleType.USER.name(), tables, null, List.of(permissions)));
}
private static Map<String, String> authHeaders(String username, String
password) {
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManagerTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManagerTest.java
index 2e6aedbf6b3..fbecb58482e 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManagerTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManagerTest.java
@@ -637,6 +637,22 @@ public class PinotHelixTaskResourceManagerTest {
PinotHelixTaskResourceManager.TaskDebugInfo result2 =
mgr.getTaskDebugInfo(taskName, otherTableNameWithType, 1);
verifyResultFilteredByTable(result2, "task2",
TaskPartitionState.COMPLETED, otherTableNameWithType);
+
+ // A subtask whose Helix task config is missing has no determinable table,
so a requested table filter must drop
+ // it rather than report it. The filter is part of the authorization
boundary for table-scoped principals on
+ // GET /tasks/task/{taskName}/debug, so including it would disclose
another table's subtasks.
+ when(jobConfig.getTaskConfig("task2")).thenReturn(null);
+ PinotHelixTaskResourceManager.TaskDebugInfo filtered =
+ mgr.getTaskDebugInfo(taskName, tableNameWithType, 1);
+ verifyResultFilteredByTable(filtered, "task1", TaskPartitionState.RUNNING,
tableNameWithType);
+ assertEquals(filtered.getSubtaskCount().getTotal(), 1);
+
+ // ... and it must not leak through a filter naming the table whose config
went missing.
+ PinotHelixTaskResourceManager.TaskDebugInfo noneMatching =
+ mgr.getTaskDebugInfo(taskName, otherTableNameWithType, 1);
+ assertTrue(noneMatching == null || noneMatching.getSubtaskInfos() == null
+ || noneMatching.getSubtaskInfos().isEmpty(),
+ "a subtask with no resolvable table must not be reported under a table
filter");
}
private static void
verifyResultFilteredByTable(PinotHelixTaskResourceManager.TaskDebugInfo
taskDebugInfo,
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/auth/Authorize.java
b/pinot-core/src/main/java/org/apache/pinot/core/auth/Authorize.java
index 22139606c71..bbbf1796ef3 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/auth/Authorize.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/auth/Authorize.java
@@ -37,6 +37,13 @@ public @interface Authorize {
// If the target type is the Pinot cluster, then this field is not required
String paramName() default "";
+ /// Optional path or query parameter that narrows a cluster operation to one
table for coarse access control.
+ /// Use only when the entire operation is limited to that table, rather than
filtering part of the response.
+ /// A missing parameter, or a name absent from the endpoint's path
parameters and declared query parameters,
+ /// resolves no table and retains cluster-wide scope, which denies
table-scoped principals. Fine-grained
+ /// authorization still uses targetType.
+ String tableScopeParam() default "";
+
// action to validate on the specific resource
String action();
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/auth/BasicAuthPrincipal.java
b/pinot-core/src/main/java/org/apache/pinot/core/auth/BasicAuthPrincipal.java
index 80997af2d60..35910f1f05f 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/auth/BasicAuthPrincipal.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/auth/BasicAuthPrincipal.java
@@ -18,11 +18,14 @@
*/
package org.apache.pinot.core.auth;
+import java.util.Collections;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
+import javax.annotation.Nullable;
/// Container object for basic auth principal
@@ -44,12 +47,21 @@ public class BasicAuthPrincipal {
Set<String> permissions, Map<String, List<String>> rlsFilters) {
_name = name;
_token = token;
- _tables = tables;
- _excludeTables = excludeTables;
+ // Copied because hasUnrestrictedTableAccess() gates cluster-level
authorization off these sets: a caller that
+ // retained and later cleared its own set would silently widen this
principal to unrestricted scope. Copied
+ // permissively rather than with Set.copyOf, which rejects null elements —
a user record stored in ZooKeeper can
+ // carry a null table name, and the broker and server rebuild every
principal on each request, so one malformed
+ // record would otherwise fail authentication for every user.
+ _tables = copyOfNullable(tables);
+ _excludeTables = copyOfNullable(excludeTables);
_permissions = permissions.stream().map(s ->
s.toLowerCase()).collect(Collectors.toSet());
_rlsFilters = rlsFilters;
}
+ private static Set<String> copyOfNullable(@Nullable Set<String> values) {
+ return values == null ? Set.of() : Collections.unmodifiableSet(new
HashSet<>(values));
+ }
+
public String getName() {
return _name;
}
@@ -62,6 +74,21 @@ public class BasicAuthPrincipal {
return isTableIncluded(tableName) && isTableNotExcluded(tableName);
}
+ /// Returns whether this principal is scoped to every table, i.e. it carries
neither an allow-list nor an
+ /// exclude-list. Access control implementations use this to decide requests
that name no table, so the return value
+ /// must satisfy: `true` implies [#hasTable(String)] holds for every table
name. A subclass that narrows table scope
+ /// by any other means — in particular by overriding [#hasTable(String)] —
must override this method to match, or it
+ /// will report unrestricted scope while denying individual tables.
+ ///
+ /// Table lists are the signal rather than [ZkBasicAuthPrincipal]'s
`RoleType`. The static factory has no role, both
+ /// controller factories share this check, and an ADMIN that also carries an
allow-list is still scoped. Granting
+ /// cluster access by role would let that principal reach cluster state
outside the list. The bootstrapped admin
+ /// is bootstrapped with a null `UserConfig#getTables()`, which normalizes
to an empty set here, so
+ /// `RoleType.ADMIN` without a table list still passes.
+ public boolean hasUnrestrictedTableAccess() {
+ return _tables.isEmpty() && _excludeTables.isEmpty();
+ }
+
private boolean isTableIncluded(String tableName) {
return _tables.isEmpty() || _tables.contains(tableName);
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/auth/FineGrainedAuthUtils.java
b/pinot-core/src/main/java/org/apache/pinot/core/auth/FineGrainedAuthUtils.java
index 384bf439dca..590650da788 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/auth/FineGrainedAuthUtils.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/auth/FineGrainedAuthUtils.java
@@ -19,8 +19,14 @@
package org.apache.pinot.core.auth;
+import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nullable;
+import javax.ws.rs.QueryParam;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MultivaluedMap;
@@ -37,34 +43,62 @@ import org.slf4j.LoggerFactory;
public class FineGrainedAuthUtils {
private static final Logger LOGGER =
LoggerFactory.getLogger(FineGrainedAuthUtils.class);
+ /// Memoizes [#getDeclaredQueryParams(Method)]; bounded by the number of
endpoints.
+ private static final Map<Method, Set<String>> DECLARED_QUERY_PARAMS = new
ConcurrentHashMap<>();
private FineGrainedAuthUtils() {
}
- /// Returns the parameter from the path or query params.
- /// @param paramName to look for
- /// @param pathParams path params
- /// @param queryParams query params
- /// @return the value of the parameter
- private static String findParam(String paramName, MultivaluedMap<String,
String> pathParams,
- MultivaluedMap<String, String> queryParams) {
- String name = pathParams.getFirst(paramName);
- if (name == null) {
- name = queryParams.getFirst(paramName);
+ /// Returns the names `endpointMethod` binds as [QueryParam]s.
+ ///
+ /// Only method-level `@QueryParam` binding is recognized; an endpoint
binding parameters through `@BeanParam` or
+ /// resource-class field injection is treated as declaring none. That
direction denies table scope rather than
+ /// granting it, so it fails closed. No in-tree controller or broker
resource uses either form today.
+ ///
+ /// Results are memoized because [Method#getParameterAnnotations()]
re-parses the class-file annotation bytes on
+ /// every call, and this runs on every request before authentication. The
key set is bounded by the number of
+ /// endpoints.
+ public static Set<String> getDeclaredQueryParams(Method endpointMethod) {
+ Set<String> declared = DECLARED_QUERY_PARAMS.get(endpointMethod);
+ if (declared != null) {
+ return declared;
+ }
+ return DECLARED_QUERY_PARAMS.computeIfAbsent(endpointMethod,
FineGrainedAuthUtils::findDeclaredQueryParams);
+ }
+
+ private static Set<String> findDeclaredQueryParams(Method endpointMethod) {
+ Set<String> declared = new HashSet<>();
+ for (Annotation[] parameterAnnotations :
endpointMethod.getParameterAnnotations()) {
+ for (Annotation parameterAnnotation : parameterAnnotations) {
+ if (parameterAnnotation instanceof QueryParam queryParam) {
+ declared.add(queryParam.value());
+ }
+ }
}
- return name;
+ return Set.copyOf(declared);
}
/// Finds the raw target parameter identified by an [Authorize] annotation.
///
+ /// Path parameters are template variables of the endpoint's own `@Path` and
are always trusted. Query parameters
+ /// are caller-supplied, so only a name the method binds as `@QueryParam`
may identify the table.
+ ///
/// @param auth annotation identifying the authorization target
+ /// @param endpointMethod the resource method, used to decide which query
parameters are declared
/// @param pathParams request path parameters
/// @param queryParams request query parameters
/// @return the unnormalized table parameter value, or `null` for a cluster
target or missing table parameter
@Nullable
- public static String findRawTargetId(Authorize auth, MultivaluedMap<String,
String> pathParams,
- MultivaluedMap<String, String> queryParams) {
- return auth.targetType() == TargetType.TABLE ? findParam(auth.paramName(),
pathParams, queryParams) : null;
+ public static String findRawTargetId(Authorize auth, Method endpointMethod,
+ MultivaluedMap<String, String> pathParams, MultivaluedMap<String,
String> queryParams) {
+ if (auth.targetType() != TargetType.TABLE) {
+ return null;
+ }
+ String targetId = pathParams.getFirst(auth.paramName());
+ if (targetId == null &&
getDeclaredQueryParams(endpointMethod).contains(auth.paramName())) {
+ targetId = queryParams.getFirst(auth.paramName());
+ }
+ return targetId;
}
/// Validate fine-grained authorization for APIs.
@@ -74,8 +108,8 @@ public class FineGrainedAuthUtils {
/// a. The targetType is [TargetType#CLUSTER]. In this case, the
paramName field
/// ([Authorize#paramName()]) is not used, since the target is the
Pinot cluster.
/// b. The targetType is [TargetType#TABLE]. In this case, the paramName
field
- /// ([Authorize#paramName()]) is mandatory, and it must be found in
either the path parameters or the
- /// query parameters.
+ /// ([Authorize#paramName()]) is mandatory, and it must be found in
the path parameters or a query
+ /// parameter the method binds as `@QueryParam`.
/// 2. [Authorize] annotation is not present on the method. In this use the
default authorization.
///
/// @param endpointMethod of the API
@@ -97,8 +131,10 @@ public class FineGrainedAuthUtils {
Response.Status.INTERNAL_SERVER_ERROR);
}
- // find the paramName in the path or query params
- targetId = findRawTargetId(auth, uriInfo.getPathParameters(),
uriInfo.getQueryParameters());
+ // Path params are part of the endpoint declaration. Query params are
caller-supplied, so only a name
+ // the method binds as @QueryParam may identify the table. Otherwise a
caller could append
+ // ?tableName=<a table it is scoped to> and have a cluster-wide
request authorized as table-scoped.
+ targetId = findRawTargetId(auth, endpointMethod,
uriInfo.getPathParameters(), uriInfo.getQueryParameters());
if (StringUtils.isBlank(targetId)) {
throw new WebApplicationException(
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/auth/BasicAuthTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/auth/BasicAuthTest.java
index 0c2ede7c51e..e31c15c1985 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/auth/BasicAuthTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/auth/BasicAuthTest.java
@@ -20,59 +20,84 @@ package org.apache.pinot.core.auth;
import com.google.common.collect.ImmutableSet;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import org.apache.pinot.spi.env.PinotConfiguration;
-import org.testng.Assert;
import org.testng.annotations.Test;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
public class BasicAuthTest {
@Test
public void testBasicAuthPrincipal() {
- Assert.assertTrue(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
+ assertTrue(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
ImmutableSet.of("READ"))
.hasTable("myTable"));
- Assert.assertTrue(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable", "myTable1"),
+ assertTrue(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable", "myTable1"),
Set.of(), ImmutableSet.of("Read"))
.hasTable("myTable1"));
- Assert.assertFalse(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
+ assertFalse(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
ImmutableSet.of("read"))
.hasTable("myTable1"));
- Assert.assertFalse(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable", "myTable1"),
+ assertFalse(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable", "myTable1"),
Set.of(), ImmutableSet.of("read"))
.hasTable("myTable2"));
- Assert.assertFalse(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), ImmutableSet.of("myTable"),
+ assertFalse(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), ImmutableSet.of("myTable"),
ImmutableSet.of("read"))
.hasTable("myTable"));
- Assert.assertFalse(new BasicAuthPrincipal("name", "token", Set.of(),
ImmutableSet.of("myTable"),
+ assertFalse(new BasicAuthPrincipal("name", "token", Set.of(),
ImmutableSet.of("myTable"),
ImmutableSet.of("read"))
.hasTable("myTable"));
- Assert.assertTrue(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), ImmutableSet.of("myTable1"),
+ assertTrue(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), ImmutableSet.of("myTable1"),
ImmutableSet.of("read"))
.hasTable("myTable"));
-
- Assert.assertTrue(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
+ assertTrue(new BasicAuthPrincipal("name", "token", Set.of(), Set.of(),
Set.of("read"))
+ .hasUnrestrictedTableAccess());
+ assertFalse(new BasicAuthPrincipal("name", "token", Set.of("myTable"),
Set.of(), Set.of("read"))
+ .hasUnrestrictedTableAccess());
+ assertFalse(new BasicAuthPrincipal("name", "token", Set.of(),
Set.of("myTable"), Set.of("read"))
+ .hasUnrestrictedTableAccess());
+
+ assertTrue(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
ImmutableSet.of("READ"))
.hasPermission("read"));
- Assert.assertTrue(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
+ assertTrue(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
ImmutableSet.of("Read"))
.hasPermission("READ"));
- Assert.assertTrue(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
+ assertTrue(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
ImmutableSet.of("read"))
.hasPermission("Read"));
- Assert.assertFalse(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
+ assertFalse(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
ImmutableSet.of("read"))
.hasPermission("write"));
- Assert.assertEquals(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
+ assertEquals(new BasicAuthPrincipal("name", "token",
ImmutableSet.of("myTable"), Set.of(),
ImmutableSet.of("read"), Map.of("myTable", List.of("cityID > 100")))
.getRLSFilters("myTable"), Optional.of(List.of("cityID > 100")));
}
+ @Test
+ public void testBasicAuthPrincipalWithNullTableEntry() {
+ Set<String> tables = new HashSet<>(Set.of("myTable"));
+ tables.add(null);
+ BasicAuthPrincipal included = new BasicAuthPrincipal("name", "token",
tables, Set.of(), Set.of("read"));
+ assertTrue(included.hasTable("myTable"));
+ assertFalse(included.hasTable("otherTable"));
+ assertFalse(included.hasUnrestrictedTableAccess());
+
+ BasicAuthPrincipal excluded = new BasicAuthPrincipal("name", "token",
Set.of(), tables, Set.of("read"));
+ assertFalse(excluded.hasTable("myTable"));
+ assertTrue(excluded.hasTable("otherTable"));
+ assertFalse(excluded.hasUnrestrictedTableAccess());
+ }
+
@Test
public void testExtractBasicAuthPrincipals() {
// Test basic configuration with multiple principals
@@ -90,54 +115,54 @@ public class BasicAuthTest {
List<BasicAuthPrincipal> principals = BasicAuthPrincipalUtils
.extractBasicAuthPrincipals(configuration, "principals");
- Assert.assertEquals(principals.size(), 2);
+ assertEquals(principals.size(), 2);
// Verify admin principal (should have no table restrictions)
BasicAuthPrincipal adminPrincipal = principals.stream()
.filter(p -> p.getName().equals("admin"))
.findFirst()
.orElse(null);
- Assert.assertNotNull(adminPrincipal);
- Assert.assertEquals(adminPrincipal.getName(), "admin");
+ assertNotNull(adminPrincipal);
+ assertEquals(adminPrincipal.getName(), "admin");
// Verify user principal
BasicAuthPrincipal userPrincipal = principals.stream()
.filter(p -> p.getName().equals("user"))
.findFirst()
.orElse(null);
- Assert.assertNotNull(userPrincipal);
- Assert.assertEquals(userPrincipal.getName(), "user");
+ assertNotNull(userPrincipal);
+ assertEquals(userPrincipal.getName(), "user");
Set<String> expectedTables = ImmutableSet.of("lessImportantStuff",
"lesserImportantStuff", "leastImportantStuff");
expectedTables.forEach(tableName -> {
- Assert.assertTrue(userPrincipal.hasTable(tableName));
+ assertTrue(userPrincipal.hasTable(tableName));
});
Set<String> expectedExcludeTables = ImmutableSet.of("excludedTable");
expectedExcludeTables.forEach(tableName -> {
- Assert.assertFalse(userPrincipal.hasTable(tableName));
+ assertFalse(userPrincipal.hasTable(tableName));
});
Set<String> expectedPermissions = ImmutableSet.of("read", "write");
expectedPermissions.forEach(permission -> {
- Assert.assertTrue(userPrincipal.hasPermission(permission));
+ assertTrue(userPrincipal.hasPermission(permission));
});
// Verify RLS filters
List<String> lessImportantStuffFilters =
userPrincipal.getRLSFilters("lessImportantStuff").get();
- Assert.assertNotNull(lessImportantStuffFilters);
- Assert.assertEquals(lessImportantStuffFilters.size(), 2);
- Assert.assertTrue(lessImportantStuffFilters.contains("cityID > 100"));
- Assert.assertTrue(lessImportantStuffFilters.contains("status = 'active'"));
+ assertNotNull(lessImportantStuffFilters);
+ assertEquals(lessImportantStuffFilters.size(), 2);
+ assertTrue(lessImportantStuffFilters.contains("cityID > 100"));
+ assertTrue(lessImportantStuffFilters.contains("status = 'active'"));
List<String> lesserImportantStuffFilters =
userPrincipal.getRLSFilters("lesserImportantStuff").get();
- Assert.assertNotNull(lesserImportantStuffFilters);
- Assert.assertEquals(lesserImportantStuffFilters.size(), 1);
- Assert.assertTrue(lesserImportantStuffFilters.contains("region = 'US'"));
+ assertNotNull(lesserImportantStuffFilters);
+ assertEquals(lesserImportantStuffFilters.size(), 1);
+ assertTrue(lesserImportantStuffFilters.contains("region = 'US'"));
// Verify no RLS filters for leastImportantStuff (not configured)
-
Assert.assertTrue(userPrincipal.getRLSFilters("leastImportantStuff").isEmpty());
+ assertTrue(userPrincipal.getRLSFilters("leastImportantStuff").isEmpty());
}
@Test(expectedExceptions = IllegalArgumentException.class,
expectedExceptionsMessageRegExp = "must provide "
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/auth/FineGrainedAuthUtilsTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/auth/FineGrainedAuthUtilsTest.java
index 4d9ec48c527..bd19d42720e 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/auth/FineGrainedAuthUtilsTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/auth/FineGrainedAuthUtilsTest.java
@@ -20,6 +20,7 @@ package org.apache.pinot.core.auth;
import java.lang.reflect.Method;
import java.net.URI;
+import javax.ws.rs.QueryParam;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MultivaluedHashMap;
@@ -27,115 +28,175 @@ import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo;
import org.apache.pinot.spi.utils.CommonConstants;
-import org.mockito.Mockito;
-import org.testng.Assert;
import org.testng.annotations.Test;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;
-
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
+import static org.testng.Assert.fail;
public class FineGrainedAuthUtilsTest {
@Test
- public void testFindRawTargetId() throws Exception {
+ public void testFindRawTargetId()
+ throws Exception {
MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
- Authorize tableAuth =
TestResource.class.getDeclaredMethod("getTable").getAnnotation(Authorize.class);
+ Method tableMethod = TestResource.class.getDeclaredMethod("getTable");
+ Method tableQueryMethod =
TestResource.class.getDeclaredMethod("getTableByQuery", String.class);
+ Authorize tableAuth = tableMethod.getAnnotation(Authorize.class);
+ Authorize tableQueryAuth = tableQueryMethod.getAnnotation(Authorize.class);
Authorize clusterAuth =
getAnnotatedMethod().getAnnotation(Authorize.class);
pathParams.putSingle("tableName", "pathTable");
- assertEquals(FineGrainedAuthUtils.findRawTargetId(tableAuth, pathParams,
queryParams), "pathTable");
+ assertEquals(FineGrainedAuthUtils.findRawTargetId(tableAuth, tableMethod,
pathParams, queryParams), "pathTable");
pathParams.clear();
queryParams.putSingle("tableName", "queryTable");
- assertEquals(FineGrainedAuthUtils.findRawTargetId(tableAuth, pathParams,
queryParams), "queryTable");
- assertNull(FineGrainedAuthUtils.findRawTargetId(clusterAuth, pathParams,
queryParams));
+ // The annotation names tableName, but getTable never binds it, so the
query value is not trusted.
+ assertNull(FineGrainedAuthUtils.findRawTargetId(tableAuth, tableMethod,
pathParams, queryParams));
+ assertEquals(FineGrainedAuthUtils.findRawTargetId(tableQueryAuth,
tableQueryMethod, pathParams, queryParams),
+ "queryTable");
+ assertNull(FineGrainedAuthUtils.findRawTargetId(clusterAuth,
getAnnotatedMethod(), pathParams, queryParams));
queryParams.clear();
- assertNull(FineGrainedAuthUtils.findRawTargetId(tableAuth, pathParams,
queryParams));
+ assertNull(FineGrainedAuthUtils.findRawTargetId(tableAuth, tableMethod,
pathParams, queryParams));
+ }
+
+ @Test
+ public void testValidateFineGrainedAuthIgnoresUndeclaredTableQueryParam()
+ throws Exception {
+ FineGrainedAccessControl ac = mock(FineGrainedAccessControl.class);
+ when(ac.hasAccess(any(HttpHeaders.class), any(), any(), any()))
+ .thenReturn(true);
+
+ UriInfo mockUriInfo = mock(UriInfo.class);
+ MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
+ MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
+ queryParams.putSingle("tableName", "callerPicked");
+ when(mockUriInfo.getPathParameters()).thenReturn(pathParams);
+ when(mockUriInfo.getQueryParameters()).thenReturn(queryParams);
+
when(mockUriInfo.getRequestUri()).thenReturn(URI.create("http://localhost/tables";));
+ HttpHeaders mockHttpHeaders = mock(HttpHeaders.class);
+
+ Method unboundTableMethod =
TestResource.class.getDeclaredMethod("getTable");
+ try {
+ FineGrainedAuthUtils.validateFineGrainedAuth(unboundTableMethod,
mockUriInfo, mockHttpHeaders, ac);
+ fail("Expected WebApplicationException");
+ } catch (WebApplicationException e) {
+ assertTrue(e.getMessage().contains("Missing required table parameter"));
+ assertEquals(e.getResponse().getStatus(),
Response.Status.BAD_REQUEST.getStatusCode());
+ }
+ verify(ac, never())
+ .hasAccess(any(HttpHeaders.class), any(), any(), any());
+ }
+
+ @Test
+ public void testValidateFineGrainedAuthAllowsDeclaredTableQueryParam()
+ throws Exception {
+ FineGrainedAccessControl ac = mock(FineGrainedAccessControl.class);
+ when(ac.hasAccess(any(HttpHeaders.class), any(), any(), any()))
+ .thenReturn(true);
+
+ UriInfo mockUriInfo = mock(UriInfo.class);
+ MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
+ MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
+ queryParams.putSingle("tableName", "ownedTable");
+ when(mockUriInfo.getPathParameters()).thenReturn(pathParams);
+ when(mockUriInfo.getQueryParameters()).thenReturn(queryParams);
+ HttpHeaders mockHttpHeaders = mock(HttpHeaders.class);
+
+ Method declaredTableMethod =
TestResource.class.getDeclaredMethod("getTableByQuery", String.class);
+ FineGrainedAuthUtils.validateFineGrainedAuth(declaredTableMethod,
mockUriInfo, mockHttpHeaders, ac);
+ verify(ac).hasAccess(mockHttpHeaders, TargetType.TABLE, "ownedTable",
"getTable");
}
@Test
public void testValidateFineGrainedAuthAllowed() {
- FineGrainedAccessControl ac = Mockito.mock(FineGrainedAccessControl.class);
- Mockito.when(ac.hasAccess(Mockito.any(HttpHeaders.class), Mockito.any(),
Mockito.any(), Mockito.any()))
+ FineGrainedAccessControl ac = mock(FineGrainedAccessControl.class);
+ when(ac.hasAccess(any(HttpHeaders.class), any(), any(), any()))
.thenReturn(true);
- UriInfo mockUriInfo = Mockito.mock(UriInfo.class);
- HttpHeaders mockHttpHeaders = Mockito.mock(HttpHeaders.class);
+ UriInfo mockUriInfo = mock(UriInfo.class);
+ HttpHeaders mockHttpHeaders = mock(HttpHeaders.class);
FineGrainedAuthUtils.validateFineGrainedAuth(getAnnotatedMethod(),
mockUriInfo, mockHttpHeaders, ac);
}
@Test
public void testValidateFineGrainedAuthDenied() {
- FineGrainedAccessControl ac = Mockito.mock(FineGrainedAccessControl.class);
- Mockito.when(ac.hasAccess(Mockito.any(HttpHeaders.class), Mockito.any(),
Mockito.any(), Mockito.any()))
+ FineGrainedAccessControl ac = mock(FineGrainedAccessControl.class);
+ when(ac.hasAccess(any(HttpHeaders.class), any(), any(), any()))
.thenReturn(false);
- UriInfo mockUriInfo = Mockito.mock(UriInfo.class);
- HttpHeaders mockHttpHeaders = Mockito.mock(HttpHeaders.class);
+ UriInfo mockUriInfo = mock(UriInfo.class);
+ HttpHeaders mockHttpHeaders = mock(HttpHeaders.class);
try {
FineGrainedAuthUtils.validateFineGrainedAuth(getAnnotatedMethod(),
mockUriInfo, mockHttpHeaders, ac);
- Assert.fail("Expected WebApplicationException");
+ fail("Expected WebApplicationException");
} catch (WebApplicationException e) {
- Assert.assertTrue(e.getMessage().contains("Access denied to getCluster
in the cluster"));
- Assert.assertEquals(e.getResponse().getStatus(),
Response.Status.FORBIDDEN.getStatusCode());
+ assertTrue(e.getMessage().contains("Access denied to getCluster in the
cluster"));
+ assertEquals(e.getResponse().getStatus(),
Response.Status.FORBIDDEN.getStatusCode());
}
}
@Test
public void testValidateFineGrainedAuthWithNoSuchMethodError() {
- FineGrainedAccessControl ac = Mockito.mock(FineGrainedAccessControl.class);
- Mockito.when(ac.hasAccess(Mockito.any(HttpHeaders.class), Mockito.any(),
Mockito.any(), Mockito.any()))
+ FineGrainedAccessControl ac = mock(FineGrainedAccessControl.class);
+ when(ac.hasAccess(any(HttpHeaders.class), any(), any(), any()))
.thenThrow(new NoSuchMethodError("Method not found"));
- UriInfo mockUriInfo = Mockito.mock(UriInfo.class);
- HttpHeaders mockHttpHeaders = Mockito.mock(HttpHeaders.class);
+ UriInfo mockUriInfo = mock(UriInfo.class);
+ HttpHeaders mockHttpHeaders = mock(HttpHeaders.class);
try {
FineGrainedAuthUtils.validateFineGrainedAuth(getAnnotatedMethod(),
mockUriInfo, mockHttpHeaders, ac);
- Assert.fail("Expected WebApplicationException");
+ fail("Expected WebApplicationException");
} catch (WebApplicationException e) {
- Assert.assertTrue(e.getMessage().contains("Failed to check for access"));
- Assert.assertEquals(e.getResponse().getStatus(),
Response.Status.INTERNAL_SERVER_ERROR.getStatusCode());
+ assertTrue(e.getMessage().contains("Failed to check for access"));
+ assertEquals(e.getResponse().getStatus(),
Response.Status.INTERNAL_SERVER_ERROR.getStatusCode());
}
}
@Test
public void testMissingTableParameterIsBadRequest() {
- UriInfo mockUriInfo = Mockito.mock(UriInfo.class);
- Mockito.when(mockUriInfo.getPathParameters()).thenReturn(new
MultivaluedHashMap<>());
- Mockito.when(mockUriInfo.getQueryParameters()).thenReturn(new
MultivaluedHashMap<>());
-
Mockito.when(mockUriInfo.getRequestUri()).thenReturn(URI.create("http://localhost/v2/segments";));
+ UriInfo mockUriInfo = mock(UriInfo.class);
+ when(mockUriInfo.getPathParameters()).thenReturn(new
MultivaluedHashMap<>());
+ when(mockUriInfo.getQueryParameters()).thenReturn(new
MultivaluedHashMap<>());
+
when(mockUriInfo.getRequestUri()).thenReturn(URI.create("http://localhost/v2/segments";));
- WebApplicationException exception =
Assert.expectThrows(WebApplicationException.class,
+ WebApplicationException exception =
expectThrows(WebApplicationException.class,
() ->
FineGrainedAuthUtils.validateFineGrainedAuth(getTableAnnotatedMethod(),
mockUriInfo,
- Mockito.mock(HttpHeaders.class),
Mockito.mock(FineGrainedAccessControl.class)));
+ mock(HttpHeaders.class), mock(FineGrainedAccessControl.class)));
- Assert.assertEquals(exception.getResponse().getStatus(),
Response.Status.BAD_REQUEST.getStatusCode());
- Assert.assertTrue(exception.getMessage().contains("Missing required table
parameter 'tableName'"));
+ assertEquals(exception.getResponse().getStatus(),
Response.Status.BAD_REQUEST.getStatusCode());
+ assertTrue(exception.getMessage().contains("Missing required table
parameter 'tableName'"));
}
@Test
public void testInvalidTableParameterIsBadRequest() {
- UriInfo mockUriInfo = Mockito.mock(UriInfo.class);
+ UriInfo mockUriInfo = mock(UriInfo.class);
MultivaluedHashMap<String, String> queryParameters = new
MultivaluedHashMap<>();
queryParameters.putSingle("tableName", "databaseB.testTable");
- Mockito.when(mockUriInfo.getPathParameters()).thenReturn(new
MultivaluedHashMap<>());
- Mockito.when(mockUriInfo.getQueryParameters()).thenReturn(queryParameters);
-
Mockito.when(mockUriInfo.getRequestUri()).thenReturn(URI.create("http://localhost/v2/segments";));
- HttpHeaders headers = Mockito.mock(HttpHeaders.class);
-
Mockito.when(headers.getHeaderString(CommonConstants.DATABASE)).thenReturn("databaseA");
+ when(mockUriInfo.getPathParameters()).thenReturn(new
MultivaluedHashMap<>());
+ when(mockUriInfo.getQueryParameters()).thenReturn(queryParameters);
+
when(mockUriInfo.getRequestUri()).thenReturn(URI.create("http://localhost/v2/segments";));
+ HttpHeaders headers = mock(HttpHeaders.class);
+
when(headers.getHeaderString(CommonConstants.DATABASE)).thenReturn("databaseA");
- WebApplicationException exception =
Assert.expectThrows(WebApplicationException.class,
+ WebApplicationException exception =
expectThrows(WebApplicationException.class,
() ->
FineGrainedAuthUtils.validateFineGrainedAuth(getTableAnnotatedMethod(),
mockUriInfo, headers,
- Mockito.mock(FineGrainedAccessControl.class)));
+ mock(FineGrainedAccessControl.class)));
- Assert.assertEquals(exception.getResponse().getStatus(),
Response.Status.BAD_REQUEST.getStatusCode());
- Assert.assertTrue(exception.getMessage().contains("Invalid table parameter
'tableName'"));
+ assertEquals(exception.getResponse().getStatus(),
Response.Status.BAD_REQUEST.getStatusCode());
+ assertTrue(exception.getMessage().contains("Invalid table parameter
'tableName'"));
}
static class TestResource {
@@ -148,7 +209,11 @@ public class FineGrainedAuthUtilsTest {
}
@Authorize(targetType = TargetType.TABLE, paramName = "tableName", action
= "uploadSegment")
- void uploadSegment() {
+ void uploadSegment(@QueryParam("tableName") String tableName) {
+ }
+
+ @Authorize(targetType = TargetType.TABLE, paramName = "tableName", action
= "getTable")
+ void getTableByQuery(@QueryParam("tableName") String tableName) {
}
}
@@ -162,7 +227,7 @@ public class FineGrainedAuthUtilsTest {
private Method getTableAnnotatedMethod() {
try {
- return TestResource.class.getDeclaredMethod("uploadSegment");
+ return TestResource.class.getDeclaredMethod("uploadSegment",
String.class);
} catch (NoSuchMethodException e) {
throw new RuntimeException(e);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
