Skip to content

Commit 11a087e

Browse files
committed
Partial Address review feedback
1 parent 423e83a commit 11a087e

File tree

4 files changed

+158
-114
lines changed

4 files changed

+158
-114
lines changed
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.polaris.core.policy;
21+
22+
import static org.apache.polaris.core.policy.PredefinedPolicyTypes.ACCESS_CONTROL;
23+
24+
import com.google.common.collect.Lists;
25+
import java.util.List;
26+
import org.apache.iceberg.expressions.Expression;
27+
import org.apache.iceberg.expressions.ExpressionVisitors;
28+
import org.apache.iceberg.expressions.Expressions;
29+
import org.apache.iceberg.expressions.UnboundPredicate;
30+
import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
31+
import org.apache.polaris.core.policy.content.AccessControlPolicyContent;
32+
33+
public class AccessControlPolicyUtil {
34+
private AccessControlPolicyUtil() {}
35+
36+
public static String replaceContextVariable(
37+
String content, PolicyType policyType, AuthenticatedPolarisPrincipal authenticatedPrincipal) {
38+
if (policyType == ACCESS_CONTROL) {
39+
try {
40+
AccessControlPolicyContent policyContent = AccessControlPolicyContent.fromString(content);
41+
List<Expression> evaluatedRowFilterExpressions = Lists.newArrayList();
42+
43+
for (Expression rowFilterExpression : policyContent.getRowFilters()) {
44+
Expression evaluatedExpression =
45+
ExpressionVisitors.visit(
46+
rowFilterExpression,
47+
new ContextVariableReplacementVisitor(authenticatedPrincipal));
48+
evaluatedRowFilterExpressions.add(evaluatedExpression);
49+
}
50+
51+
// also nullify the principal role.
52+
policyContent.setPrincipalRole(null);
53+
policyContent.setRowFilters(evaluatedRowFilterExpressions);
54+
return AccessControlPolicyContent.toString(policyContent);
55+
} catch (Exception e) {
56+
return content;
57+
}
58+
}
59+
return content;
60+
}
61+
62+
public static boolean filterApplicablePolicy(
63+
PolicyEntity policyEntity, AuthenticatedPolarisPrincipal authenticatedPrincipal) {
64+
if (policyEntity.getPolicyType().equals(ACCESS_CONTROL)) {
65+
AccessControlPolicyContent content =
66+
AccessControlPolicyContent.fromString(policyEntity.getContent());
67+
String applicablePrincipal = content.getPrincipalRole();
68+
return applicablePrincipal == null
69+
|| authenticatedPrincipal.getActivatedPrincipalRoleNames().isEmpty()
70+
|| authenticatedPrincipal
71+
.getActivatedPrincipalRoleNames()
72+
.contains(content.getPrincipalRole());
73+
}
74+
75+
return true;
76+
}
77+
78+
/** Expression visitor that replaces context variables with evaluated expressions */
79+
private static class ContextVariableReplacementVisitor
80+
extends ExpressionVisitors.ExpressionVisitor<Expression> {
81+
private final AuthenticatedPolarisPrincipal authenticatedPrincipal;
82+
83+
public ContextVariableReplacementVisitor(AuthenticatedPolarisPrincipal authenticatedPrincipal) {
84+
this.authenticatedPrincipal = authenticatedPrincipal;
85+
}
86+
87+
@Override
88+
public <T> Expression predicate(UnboundPredicate<T> pred) {
89+
String refName = pred.ref().name();
90+
91+
if ("$current_principal_role".equals(refName)) {
92+
return evaluateCurrentPrincipalRole(pred);
93+
} else if ("$current_principal".equals(refName)) {
94+
return evaluateCurrentPrincipal(pred);
95+
}
96+
97+
// Return the original predicate if it doesn't reference context variables
98+
return pred;
99+
}
100+
101+
@Override
102+
public Expression alwaysTrue() {
103+
return Expressions.alwaysTrue();
104+
}
105+
106+
@Override
107+
public Expression alwaysFalse() {
108+
return Expressions.alwaysFalse();
109+
}
110+
111+
@Override
112+
public Expression not(Expression result) {
113+
return Expressions.not(result);
114+
}
115+
116+
@Override
117+
public Expression and(Expression leftResult, Expression rightResult) {
118+
return Expressions.and(leftResult, rightResult);
119+
}
120+
121+
@Override
122+
public Expression or(Expression leftResult, Expression rightResult) {
123+
return Expressions.or(leftResult, rightResult);
124+
}
125+
126+
private Expression evaluateCurrentPrincipalRole(UnboundPredicate<?> pred) {
127+
String val = (String) pred.literal().value();
128+
boolean containsRole = authenticatedPrincipal.getActivatedPrincipalRoleNames().contains(val);
129+
130+
return getExpression(pred, containsRole);
131+
}
132+
133+
private Expression evaluateCurrentPrincipal(UnboundPredicate<?> pred) {
134+
String val = (String) pred.literal().value();
135+
boolean principalMatches = authenticatedPrincipal.getName().equals(val);
136+
137+
return getExpression(pred, principalMatches);
138+
}
139+
140+
private Expression getExpression(UnboundPredicate<?> pred, boolean principalMatches) {
141+
if (pred.op().equals(Expression.Operation.EQ)) {
142+
return principalMatches ? Expressions.alwaysTrue() : Expressions.alwaysFalse();
143+
} else if (pred.op().equals(Expression.Operation.NOT_EQ)) {
144+
return principalMatches ? Expressions.alwaysFalse() : Expressions.alwaysTrue();
145+
} else {
146+
// For other operations, return the original predicate
147+
return pred;
148+
}
149+
}
150+
}
151+
}

polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyUtil.java

Lines changed: 0 additions & 108 deletions
This file was deleted.

runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/AbstractPolicyCatalogTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -656,7 +656,6 @@ public void testAttachAccessControlPolicyToTableAndCheckApplicablePolicy() {
656656
// attach a different type of policy to namespace
657657
policyCatalog.attachPolicy(POLICY1, POLICY_ATTACH_TARGET_NS, null);
658658
var applicablePolicies = policyCatalog.getApplicablePolicies(NS, TABLE.name(), ACCESS_CONTROL);
659-
System.out.println(applicablePolicies);
660659
// only p2 is with the type fetched
661660
assertThat(applicablePolicies.contains(policyToApplicablePolicy(p2, false, NS))).isTrue();
662661
}
@@ -676,7 +675,7 @@ private static ApplicablePolicy policyToApplicablePolicy(
676675

677676
private static String replaceContextVariable(String content, String policyType) {
678677
if (policyType.equals(ACCESS_CONTROL.getName())) {
679-
return "{\"principalRole\":\"ANALYST\",\"columnProjections\":[\"name\",\"location\"],\"rowFilters\":[\"{\\\"type\\\":\\\"eq\\\",\\\"term\\\":\\\"country\\\",\\\"value\\\":\\\"USA\\\"}\",\"false\"]}";
678+
return "{\"principalRole\":null,\"columnProjections\":[\"name\",\"location\"],\"rowFilters\":[\"{\\\"type\\\":\\\"eq\\\",\\\"term\\\":\\\"country\\\",\\\"value\\\":\\\"USA\\\"}\",\"false\"]}";
680679
}
681680
return content;
682681
}

service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@
5454
import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult;
5555
import org.apache.polaris.core.persistence.pagination.PageToken;
5656
import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView;
57+
import org.apache.polaris.core.policy.AccessControlPolicyUtil;
5758
import org.apache.polaris.core.policy.PolicyEntity;
5859
import org.apache.polaris.core.policy.PolicyType;
59-
import org.apache.polaris.core.policy.PolicyUtil;
6060
import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException;
6161
import org.apache.polaris.core.policy.exceptions.PolicyAttachException;
6262
import org.apache.polaris.core.policy.exceptions.PolicyInUseException;
@@ -428,10 +428,12 @@ private List<ApplicablePolicy> getEffectivePolicies(
428428

429429
return Stream.concat(
430430
nonInheritablePolicies.stream()
431-
.filter(p -> PolicyUtil.filterApplicablePolicy(p, authenticatedPrincipal))
431+
.filter(
432+
p -> AccessControlPolicyUtil.filterApplicablePolicy(p, authenticatedPrincipal))
432433
.map(policy -> constructApplicablePolicy(policy, false)),
433434
inheritablePolicies.values().stream()
434-
.filter(p -> PolicyUtil.filterApplicablePolicy(p, authenticatedPrincipal))
435+
.filter(
436+
p -> AccessControlPolicyUtil.filterApplicablePolicy(p, authenticatedPrincipal))
435437
.map(
436438
policy ->
437439
constructApplicablePolicy(
@@ -544,7 +546,7 @@ private ApplicablePolicy constructApplicablePolicy(PolicyEntity policyEntity, bo
544546
.setName(policyEntity.getName())
545547
.setDescription(policyEntity.getDescription())
546548
.setContent(
547-
PolicyUtil.replaceContextVariable(
549+
AccessControlPolicyUtil.replaceContextVariable(
548550
policyEntity.getContent(), policyEntity.getPolicyType(), authenticatedPrincipal))
549551
.setVersion(policyEntity.getPolicyVersion())
550552
.setInherited(inherited)

0 commit comments

Comments
 (0)