Skip to content

auth: Refactor AuthorizationRequest Into Explicit Shapes #4409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sungwy wants to merge 5 commits into
base: main
Choose a base branch
from
Open

auth: Refactor AuthorizationRequest Into Explicit Shapes #4409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4960773
propose new shapes for AuthorizationRequest
sungwy May 7, 2026
51dc649
additional tests
sungwy May 12, 2026
ff44952
minor changes
sungwy May 12, 2026
633cb88
test nit
sungwy May 13, 2026
96623d5
adopt feedback - thanks dmitri
sungwy May 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 42 additions & 19 deletions ...th/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import jakarta.annotation.Nonnull;
import jakarta.annotation.Nullable;
import java.io.IOException;
Expand Down Expand Up @@ -109,25 +110,55 @@ public OpaPolarisAuthorizer(
*/
@Override
public void resolveAuthorizationInputs(
@Nonnull AuthorizationState authzState, @Nonnull AuthorizationRequest request) {
@Nonnull AuthorizationState authzState,
@Nonnull PolarisPrincipal polarisPrincipal,
@Nonnull AuthorizationRequest request) {
authzState.getResolutionManifest().resolveAll();
}

@Override
public void resolveAuthorizationInputs(
@Nonnull AuthorizationState authzState,
@Nonnull PolarisPrincipal polarisPrincipal,
@Nonnull List<AuthorizationRequest> requests) {
Preconditions.checkArgument(
!requests.isEmpty(), "Authorization request batch must contain at least one request");
authzState.getResolutionManifest().resolveAll();
}

@Override
@Nonnull
public AuthorizationDecision authorize(
@Nonnull AuthorizationState authzState, @Nonnull AuthorizationRequest request) {
@Nonnull AuthorizationState authzState,
@Nonnull PolarisPrincipal polarisPrincipal,
@Nonnull AuthorizationRequest request) {
boolean allowed =
queryOpa(
buildOpaAuthorizationInput(
request.getPrincipal(),
polarisPrincipal,
request.getOperation(),
toResourceEntitiesFromSecurables(request.getTargets()),
toResourceEntitiesFromSecurables(request.getSecondaries())));
toResourceEntitiesFromSecurable(request.getTarget()),
toResourceEntitiesFromSecurable(request.getSecondary())));
return allowed
? AuthorizationDecision.allow()
: AuthorizationDecision.deny(
"OPA denied authorization for " + request.formatForAuthorizationMessage());
"OPA denied authorization for principal="
+ polarisPrincipal.getName()
+ " operation="
+ request.getOperation());
Comment on lines 142 to +148
Copy link
Copy Markdown
Contributor Author

@sungwy sungwy May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Copilot. This was intentional, as it was suggested in a previous discussion that hiding internal security semantics may be beneficial.

}

@Override
@Nonnull
public AuthorizationDecision authorize(
@Nonnull AuthorizationState authzState,
@Nonnull PolarisPrincipal polarisPrincipal,
@Nonnull List<AuthorizationRequest> requests) {
Preconditions.checkArgument(
!requests.isEmpty(), "Authorization request batch must contain at least one request");
// Batch OPA evaluation remains sequential for backward compatibility until OPA adopts a
// batch-native payload schema.
return PolarisAuthorizer.super.authorize(authzState, polarisPrincipal, requests);
}

/**
Expand Down Expand Up @@ -297,8 +328,8 @@ private ImmutableContext buildContext() {
private ImmutableResource buildResource(
List<ResourceEntity> targets, List<ResourceEntity> secondaries) {
// Backward compatibility: keep the existing OPA input shape with separate target and
// secondary lists. Future work can align this with AuthorizationTargetBinding semantics
// using binding tuples like [(target, secondary), ...].
// secondary lists. Future work can align this with richer request shapes if OPA starts
// consuming pairwise authorization intent directly.
return ImmutableResource.builder().targets(targets).secondaries(secondaries).build();
}

Expand Down Expand Up @@ -362,16 +393,8 @@ private List<ResourceEntity> toResourceEntitiesFromResolvedPaths(
}

@Nonnull
private List<ResourceEntity> toResourceEntitiesFromSecurables(
@Nullable List<PolarisSecurable> securables) {
if (securables == null || securables.isEmpty()) {
return List.of();
}

List<ResourceEntity> entities = new ArrayList<>();
for (PolarisSecurable securable : securables) {
entities.add(buildResourceEntity(securable));
}
return entities;
private List<ResourceEntity> toResourceEntitiesFromSecurable(
@Nullable PolarisSecurable securable) {
return securable == null ? List.of() : List.of(buildResourceEntity(securable));
}
}
185 changes: 151 additions & 34 deletions ...pa/impl/src/test/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.net.InetSocketAddress;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand All @@ -51,7 +52,6 @@
import org.apache.polaris.core.auth.AuthorizationDecision;
import org.apache.polaris.core.auth.AuthorizationRequest;
import org.apache.polaris.core.auth.AuthorizationState;
import org.apache.polaris.core.auth.AuthorizationTargetBinding;
import org.apache.polaris.core.auth.PathSegment;
import org.apache.polaris.core.auth.PolarisAuthorizableOperation;
import org.apache.polaris.core.auth.PolarisPrincipal;
Expand Down Expand Up @@ -579,8 +579,9 @@ void resolveAuthorizationInputsResolvesAll() {
PolarisResolutionManifest resolutionManifest = mock(PolarisResolutionManifest.class);
AuthorizationState authzState = new AuthorizationState();
authzState.setResolutionManifest(resolutionManifest);
PolarisPrincipal principal = PolarisPrincipal.of("alice", Map.of(), Set.of("role-1"));

authorizer.resolveAuthorizationInputs(authzState, requestWithCatalogTarget());
authorizer.resolveAuthorizationInputs(authzState, principal, requestWithCatalogTarget());

verify(resolutionManifest).resolveAll();
}
Expand All @@ -589,6 +590,7 @@ void resolveAuthorizationInputsResolvesAll() {
void authorizeUsesIntentInputsAndAllows() throws Exception {
final String[] capturedRequestBody = new String[1];
AuthorizationRequest request = requestWithCatalogTarget();
PolarisPrincipal principal = PolarisPrincipal.of("alice", Map.of(), Set.of("role-1"));
HttpEntity mockEntity = HttpEntities.create("{\"result\":{\"allow\":true}}");
@SuppressWarnings("resource")
ClassicHttpResponse mockResponse = new BasicClassicHttpResponse(200);
Expand All @@ -613,7 +615,7 @@ <T> T httpClientExecute(
}
};

AuthorizationDecision decision = authorizer.authorize(authzState, request);
AuthorizationDecision decision = authorizer.authorize(authzState, principal, request);
ObjectMapper mapper = JsonMapper.builder().build();
JsonNode root = mapper.readTree(capturedRequestBody[0]);

Expand All @@ -637,6 +639,7 @@ <T> T httpClientExecute(
@Test
void authorizeDeniesWhenOpaDenies() {
AuthorizationRequest request = requestWithCatalogTarget();
PolarisPrincipal principal = PolarisPrincipal.of("alice", Map.of(), Set.of("role-1"));
HttpEntity mockEntity = HttpEntities.create("{\"result\":{\"allow\":false}}");
@SuppressWarnings("resource")
ClassicHttpResponse mockResponse = new BasicClassicHttpResponse(200);
Expand All @@ -659,7 +662,7 @@ <T> T httpClientExecute(
}
};

AuthorizationDecision decision = authorizer.authorize(authzState, request);
AuthorizationDecision decision = authorizer.authorize(authzState, principal, request);

assertThat(decision.isAllowed()).isFalse();
assertThat(decision.getMessage())
Expand All @@ -668,26 +671,21 @@ <T> T httpClientExecute(
assertThat(message)
.contains("OPA denied authorization")
.contains("operation=GET_CATALOG")
.contains("principal=alice")
.contains("targets=[CATALOG:catalog-1]")
.contains("secondaries=[]"));
.contains("principal=alice"));
}

@Test
void authorizeIncludesStructuredParentsFromSecurable() throws Exception {
final String[] capturedRequestBody = new String[1];
AuthorizationRequest request =
AuthorizationRequest.of(
PolarisPrincipal.of("alice", Map.of(), Set.of("role-1")),
PolarisAuthorizableOperation.LOAD_TABLE,
List.of(
AuthorizationTargetBinding.of(
PolarisSecurable.of(
new PathSegment(PolarisEntityType.CATALOG, "catalog1"),
new PathSegment(PolarisEntityType.NAMESPACE, "ns1"),
new PathSegment(PolarisEntityType.NAMESPACE, "ns2"),
new PathSegment(PolarisEntityType.TABLE_LIKE, "table1")),
null)));
PolarisSecurable.of(
new PathSegment(PolarisEntityType.CATALOG, "catalog1"),
new PathSegment(PolarisEntityType.NAMESPACE, "ns1"),
new PathSegment(PolarisEntityType.NAMESPACE, "ns2"),
new PathSegment(PolarisEntityType.TABLE_LIKE, "table1")));
PolarisPrincipal principal = PolarisPrincipal.of("alice", Map.of(), Set.of("role-1"));
HttpEntity mockEntity = HttpEntities.create("{\"result\":{\"allow\":true}}");
@SuppressWarnings("resource")
ClassicHttpResponse mockResponse = new BasicClassicHttpResponse(200);
Expand All @@ -712,7 +710,7 @@ <T> T httpClientExecute(
}
};

AuthorizationDecision decision = authorizer.authorize(authzState, request);
AuthorizationDecision decision = authorizer.authorize(authzState, principal, request);
ObjectMapper mapper = JsonMapper.builder().build();
JsonNode root = mapper.readTree(capturedRequestBody[0]);

Expand Down Expand Up @@ -868,18 +866,16 @@ void authorizeRenameIncludesTargetAndSecondaryPaths() throws Exception {

AuthorizationRequest request =
AuthorizationRequest.of(
PolarisPrincipal.of("alice", Map.of(), Set.of("role-1")),
PolarisAuthorizableOperation.RENAME_TABLE,
List.of(
AuthorizationTargetBinding.of(
PolarisSecurable.of(
new PathSegment(PolarisEntityType.CATALOG, "catalog1"),
new PathSegment(PolarisEntityType.NAMESPACE, "src_ns"),
new PathSegment(PolarisEntityType.TABLE_LIKE, "src_tbl")),
PolarisSecurable.of(
new PathSegment(PolarisEntityType.CATALOG, "catalog1"),
new PathSegment(PolarisEntityType.NAMESPACE, "dst_ns"),
new PathSegment(PolarisEntityType.TABLE_LIKE, "dst_tbl")))));
PolarisSecurable.of(
new PathSegment(PolarisEntityType.CATALOG, "catalog1"),
new PathSegment(PolarisEntityType.NAMESPACE, "src_ns"),
new PathSegment(PolarisEntityType.TABLE_LIKE, "src_tbl")),
PolarisSecurable.of(
new PathSegment(PolarisEntityType.CATALOG, "catalog1"),
new PathSegment(PolarisEntityType.NAMESPACE, "dst_ns"),
new PathSegment(PolarisEntityType.TABLE_LIKE, "dst_tbl")));
PolarisPrincipal principal = PolarisPrincipal.of("alice", Map.of(), Set.of("role-1"));

OpaPolarisAuthorizer authorizer =
new OpaPolarisAuthorizer(
Expand All @@ -897,7 +893,7 @@ <T> T httpClientExecute(
}
};

AuthorizationDecision decision = authorizer.authorize(authzState, request);
AuthorizationDecision decision = authorizer.authorize(authzState, principal, request);
ObjectMapper mapper = JsonMapper.builder().build();
JsonNode root = mapper.readTree(capturedRequestBody[0]);

Expand Down Expand Up @@ -934,14 +930,135 @@ <T> T httpClientExecute(
.isEqualTo(expectedDestination);
}

@Test
void authorizeSingleOperationBatchEvaluatesSequentially() throws Exception {
final List<String> capturedRequestBodies = new ArrayList<>();
HttpEntity mockEntity = HttpEntities.create("{\"result\":{\"allow\":true}}");
@SuppressWarnings("resource")
ClassicHttpResponse mockResponse = new BasicClassicHttpResponse(200);
mockResponse.setEntity(mockEntity);

PolarisResolutionManifest resolutionManifest = mock(PolarisResolutionManifest.class);
AuthorizationState authzState = new AuthorizationState();
authzState.setResolutionManifest(resolutionManifest);
PolarisPrincipal principal = PolarisPrincipal.of("alice", Map.of(), Set.of("role-1"));

OpaPolarisAuthorizer authorizer =
new OpaPolarisAuthorizer(
URI.create("http://opa.example.com:8181/v1/data/polaris/allow"),
mock(CloseableHttpClient.class),
JsonMapper.builder().build(),
null) {
@Override
<T> T httpClientExecute(
ClassicHttpRequest request, HttpClientResponseHandler<? extends T> responseHandler)
throws HttpException, IOException {
capturedRequestBodies.add(
new String(
request.getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8));
return responseHandler.handleResponse(mockResponse);
}
};

AuthorizationDecision decision =
authorizer.authorize(
authzState,
principal,
List.of(
AuthorizationRequest.of(
PolarisAuthorizableOperation.GET_CATALOG,
PolarisSecurable.of(new PathSegment(PolarisEntityType.CATALOG, "catalog-1"))),
AuthorizationRequest.of(
PolarisAuthorizableOperation.GET_CATALOG,
PolarisSecurable.of(new PathSegment(PolarisEntityType.CATALOG, "catalog-2")))));

ObjectMapper mapper = JsonMapper.builder().build();
assertThat(decision.isAllowed()).isTrue();
assertThat(capturedRequestBodies).hasSize(2);
JsonNode firstRoot = mapper.readTree(capturedRequestBodies.get(0));
JsonNode secondRoot = mapper.readTree(capturedRequestBodies.get(1));
assertThat(firstRoot.path("input").path("action").asText()).isEqualTo("GET_CATALOG");
assertThat(secondRoot.path("input").path("action").asText()).isEqualTo("GET_CATALOG");
JsonNode expectedFirstTargets =
mapper.readTree(
"""
[
{
"type": "CATALOG",
"name": "catalog-1",
"parents": []
}
]
""");
JsonNode expectedSecondTargets =
mapper.readTree(
"""
[
{
"type": "CATALOG",
"name": "catalog-2",
"parents": []
}
]
""");
assertThat(firstRoot.path("input").path("resource").path("targets"))
.isEqualTo(expectedFirstTargets);
assertThat(secondRoot.path("input").path("resource").path("targets"))
.isEqualTo(expectedSecondTargets);
assertThat(firstRoot.path("input").path("resource").path("secondaries")).isEmpty();
assertThat(secondRoot.path("input").path("resource").path("secondaries")).isEmpty();
}

@Test
void authorizeMultiOperationBatchEvaluatesSequentially() throws Exception {
final int[] requestCount = new int[1];
HttpEntity mockEntity = HttpEntities.create("{\"result\":{\"allow\":true}}");
@SuppressWarnings("resource")
ClassicHttpResponse mockResponse = new BasicClassicHttpResponse(200);
mockResponse.setEntity(mockEntity);

PolarisResolutionManifest resolutionManifest = mock(PolarisResolutionManifest.class);
AuthorizationState authzState = new AuthorizationState();
authzState.setResolutionManifest(resolutionManifest);
PolarisPrincipal principal = PolarisPrincipal.of("alice", Map.of(), Set.of("role-1"));

OpaPolarisAuthorizer authorizer =
new OpaPolarisAuthorizer(
URI.create("http://opa.example.com:8181/v1/data/polaris/allow"),
mock(CloseableHttpClient.class),
JsonMapper.builder().build(),
null) {
@Override
<T> T httpClientExecute(
ClassicHttpRequest request, HttpClientResponseHandler<? extends T> responseHandler)
throws HttpException, IOException {
requestCount[0]++;
return responseHandler.handleResponse(mockResponse);
}
};

AuthorizationDecision decision =
authorizer.authorize(
authzState,
principal,
List.of(
AuthorizationRequest.of(
PolarisAuthorizableOperation.GET_CATALOG,
PolarisSecurable.of(new PathSegment(PolarisEntityType.CATALOG, "catalog-1"))),
AuthorizationRequest.of(
PolarisAuthorizableOperation.LIST_NAMESPACES,
PolarisSecurable.of(
new PathSegment(PolarisEntityType.CATALOG, "catalog-1"),
new PathSegment(PolarisEntityType.NAMESPACE, "ns")))));

assertThat(decision.isAllowed()).isTrue();
assertThat(requestCount[0]).isEqualTo(2);
}

private AuthorizationRequest requestWithCatalogTarget() {
return AuthorizationRequest.of(
PolarisPrincipal.of("alice", Map.of(), Set.of("role-1")),
PolarisAuthorizableOperation.GET_CATALOG,
List.of(
AuthorizationTargetBinding.of(
PolarisSecurable.of(new PathSegment(PolarisEntityType.CATALOG, "catalog-1")),
null)));
PolarisSecurable.of(new PathSegment(PolarisEntityType.CATALOG, "catalog-1")));
}

private ResolvedPolarisEntity createResolvedEntity(PolarisEntity entity) {
Expand Down
Loading