Skip to content

Commit

Permalink
rename isUserWithNoRestriction to allowAccessWithNoRestriction
Browse files Browse the repository at this point in the history
  • Loading branch information
cecemei committed Jan 9, 2025
1 parent 18c4828 commit 3ba7d35
Show file tree
Hide file tree
Showing 27 changed files with 111 additions and 96 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ private QueryResponse runNativeQuery(QueryRequest request, AuthenticationResult
try {
queryLifecycle.initialize(query);
AuthorizationResult authorizationResult = queryLifecycle.authorize(authResult);
if (!authorizationResult.isUserWithNoRestriction()) {
if (!authorizationResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(Access.DEFAULT_ERROR_MESSAGE);
}
queryResponse = queryLifecycle.execute();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public ContainerRequest filter(ContainerRequest request)
getAuthorizerMapper()
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new WebApplicationException(
Response.status(Response.Status.FORBIDDEN)
.type(MediaType.TEXT_PLAIN)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ private void authorizeTable(
private void authorize(String resource, String key, Action action, HttpServletRequest request)
{
final AuthorizationResult authResult = authorizeAccess(resource, key, action, request);
if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public GetQueriesResponse doGetRunningQueries(
queries.sort(Comparator.comparing(DartQueryInfo::getStartTime).thenComparing(DartQueryInfo::getDartQueryId));

final GetQueriesResponse response;
if (stateReadAccess.isUserWithNoRestriction()) {
if (stateReadAccess.allowAccessWithNoRestriction()) {
// User can READ STATE, so they can see all running queries, as well as authentication details.
response = new GetQueriesResponse(queries);
} else {
Expand Down Expand Up @@ -247,7 +247,7 @@ public Response cancelQuery(

final AuthorizationResult authResult = authorizeCancellation(req, cancelables);

if (authResult.isUserWithNoRestriction()) {
if (authResult.allowAccessWithNoRestriction()) {
sqlLifecycleManager.removeAll(sqlQueryId, cancelables);

// Don't call cancel() on the cancelables. That just cancels native queries, which is useless here. Instead,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public DartQueryMaker(
@Override
public QueryResponse<Object[]> runQuery(DruidQuery druidQuery)
{
if (!plannerContext.getAuthorizationResult().isUserWithNoRestriction()) {
if (!plannerContext.getAuthorizationResult().allowAccessWithNoRestriction()) {
throw new ForbiddenException(plannerContext.getAuthorizationResult().getErrorMessage());
}
final MSQSpec querySpec = MSQTaskQueryMaker.makeQuerySpec(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public static void authorizeAdminRequest(
authorizerMapper
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}
}
Expand All @@ -67,7 +67,7 @@ public static void authorizeQueryRequest(
authorizerMapper
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public class MSQTaskQueryMaker implements QueryMaker
@Override
public QueryResponse<Object[]> runQuery(final DruidQuery druidQuery)
{
if (!plannerContext.getAuthorizationResult().isUserWithNoRestriction()) {
if (!plannerContext.getAuthorizationResult().allowAccessWithNoRestriction()) {
throw new ForbiddenException(plannerContext.getAuthorizationResult().getErrorMessage());
}
Hook.QUERY_PLAN.run(druidQuery.getQuery());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ private MSQControllerTask getMSQControllerTaskAndCheckPermission(
authorizerMapper
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(StringUtils.format(
"The current user[%s] cannot view query id[%s] since the query is owned by another user",
currentUser,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public static AuthorizationResult datasourceAuthorizationCheck(
);

AuthorizationResult authResult = AuthorizationUtils.authorizeResourceAction(req, resourceAction, authorizerMapper);
if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}
return authResult;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public Response taskPost(
resourceActions,
authorizerMapper
);
if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}

Expand Down Expand Up @@ -614,7 +614,7 @@ public Response getTasks(
authorizerMapper
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {

Check failure

Code scanning / CodeQL

User-controlled bypass of sensitive method High

Sensitive method may not be executed depending on a
this condition
, which flows from
user-controlled value
.
throw new WebApplicationException(
Response.status(Response.Status.FORBIDDEN)
.type(MediaType.TEXT_PLAIN)
Expand Down Expand Up @@ -663,7 +663,7 @@ public Response killPendingSegments(
authorizerMapper
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public boolean apply(PathSegment input)
getAuthorizerMapper()
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public ContainerRequest filter(ContainerRequest request)
getAuthorizerMapper()
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public SamplerResponse post(final SamplerSpec sampler, @Context final HttpServle
authorizerMapper
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}
return sampler.sample();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public Response specPost(final SupervisorSpec spec, @Context final HttpServletRe
authorizerMapper
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static AuthorizationResult authorizationCheck(
);

AuthorizationResult authResult = AuthorizationUtils.authorizeResourceAction(req, resourceAction, authorizerMapper);
if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ private AuthorizationResult doAuthorize(
transition(State.AUTHORIZING, State.UNAUTHORIZED);
} else {
transition(State.AUTHORIZING, State.AUTHORIZED);
if (this.baseQuery instanceof SegmentMetadataQuery && authorizationResult.isUserWithNoRestriction()) {
if (this.baseQuery instanceof SegmentMetadataQuery && authorizationResult.allowAccessWithNoRestriction()) {
// skip restrictions mapping for SegmentMetadataQuery from user with no restriction
} else {
this.baseQuery = this.baseQuery.withDataSource(this.baseQuery.getDataSource()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public Response cancelQuery(@PathParam("id") String queryId, @Context final Http
authorizerMapper
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public ContainerRequest filter(ContainerRequest request)
getAuthorizerMapper()
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public ContainerRequest filter(ContainerRequest request)
getAuthorizerMapper()
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public boolean apply(PathSegment input)
getAuthorizerMapper()
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public ContainerRequest filter(ContainerRequest request)
getAuthorizerMapper()
);

if (!authResult.isUserWithNoRestriction()) {
if (!authResult.allowAccessWithNoRestriction()) {
throw new ForbiddenException(authResult.getErrorMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public boolean allowBasicAccess()
* <li> no policy found
* <li> the user has a no-restriction policy
*/
public boolean isUserWithNoRestriction()
public boolean allowAccessWithNoRestriction()
{
return PERMISSION.ALLOW_NO_RESTRICTION.equals(permission) || (PERMISSION.ALLOW_WITH_RESTRICTION.equals(permission)
&& policyRestrictions.values()
Expand All @@ -151,7 +151,7 @@ public String getErrorMessage()
case DENY:
return Objects.requireNonNull(failureMessage);
case ALLOW_WITH_RESTRICTION:
if (!isUserWithNoRestriction()) {
if (!allowAccessWithNoRestriction()) {
return Access.DEFAULT_ERROR_MESSAGE;
}
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,33 +77,33 @@ public void testNoAccess()
{
AuthorizationResult result = AuthorizationResult.deny("this data source is not permitted");
assertFalse(result.allowBasicAccess());
assertFalse(result.isUserWithNoRestriction());
assertFalse(result.allowAccessWithNoRestriction());
assertEquals("this data source is not permitted", result.getErrorMessage());
assertFalse(result.isUserWithNoRestriction());
assertFalse(result.allowAccessWithNoRestriction());
}

@Test
public void testFullAccess()
{
AuthorizationResult result = AuthorizationResult.allowWithRestriction(ImmutableMap.of());
assertTrue(result.allowBasicAccess());
assertTrue(result.isUserWithNoRestriction());
assertTrue(result.allowAccessWithNoRestriction());
assertThrows(DruidException.class, result::getErrorMessage);

AuthorizationResult resultWithEmptyPolicy = AuthorizationResult.allowWithRestriction(ImmutableMap.of(
"table1",
Optional.empty()
));
assertTrue(resultWithEmptyPolicy.allowBasicAccess());
assertTrue(resultWithEmptyPolicy.isUserWithNoRestriction());
assertTrue(resultWithEmptyPolicy.allowAccessWithNoRestriction());
assertThrows(DruidException.class, resultWithEmptyPolicy::getErrorMessage);

AuthorizationResult resultWithNoRestrictionPolicy = AuthorizationResult.allowWithRestriction(ImmutableMap.of(
"table1",
Optional.of(NoRestrictionPolicy.INSTANCE)
));
assertTrue(resultWithNoRestrictionPolicy.allowBasicAccess());
assertTrue(resultWithNoRestrictionPolicy.isUserWithNoRestriction());
assertTrue(resultWithNoRestrictionPolicy.allowAccessWithNoRestriction());
assertThrows(DruidException.class, resultWithNoRestrictionPolicy::getErrorMessage);
}

Expand All @@ -120,7 +120,7 @@ public void testRestrictedAccess()
)))
));
assertTrue(result.allowBasicAccess());
assertFalse(result.isUserWithNoRestriction());
assertFalse(result.allowAccessWithNoRestriction());
assertEquals("Unauthorized", result.getErrorMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -518,11 +518,11 @@ public void testAuthorizeQueryContext_authorized()
revisedContext
);

Assert.assertTrue(lifecycle.authorize(mockRequest()).isUserWithNoRestriction());
Assert.assertTrue(lifecycle.authorize(mockRequest()).allowAccessWithNoRestriction());

lifecycle = createLifecycle(authConfig);
lifecycle.initialize(query);
Assert.assertTrue(lifecycle.authorize(authenticationResult).isUserWithNoRestriction());
Assert.assertTrue(lifecycle.authorize(authenticationResult).allowAccessWithNoRestriction());
}

@Test
Expand Down Expand Up @@ -610,11 +610,11 @@ public void testAuthorizeQueryContext_unsecuredKeys()
revisedContext
);

Assert.assertTrue(lifecycle.authorize(mockRequest()).isUserWithNoRestriction());
Assert.assertTrue(lifecycle.authorize(mockRequest()).allowAccessWithNoRestriction());

lifecycle = createLifecycle(authConfig);
lifecycle.initialize(query);
Assert.assertTrue(lifecycle.authorize(authenticationResult).isUserWithNoRestriction());
Assert.assertTrue(lifecycle.authorize(authenticationResult).allowAccessWithNoRestriction());
}

@Test
Expand Down
Loading

0 comments on commit 3ba7d35

Please sign in to comment.