laurentgo commented on code in PR #2048: URL: https://github.com/apache/polaris/pull/2048#discussion_r2207710664
########## polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyUtil.java: ########## @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.policy; + +import static org.apache.polaris.core.policy.PredefinedPolicyTypes.ACCESS_CONTROL; + +import com.google.common.collect.Lists; +import java.util.List; +import org.apache.iceberg.expressions.Expression; +import org.apache.iceberg.expressions.Expressions; +import org.apache.iceberg.expressions.UnboundPredicate; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; +import org.apache.polaris.core.policy.content.AccessControlPolicyContent; + +public class PolicyUtil { Review Comment: (design) I would recommend to mark the class final as well Adding javadoc to the class and the public methods would be good as well. ########## polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyUtil.java: ########## @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.policy; + +import static org.apache.polaris.core.policy.PredefinedPolicyTypes.ACCESS_CONTROL; + +import com.google.common.collect.Lists; +import java.util.List; +import org.apache.iceberg.expressions.Expression; +import org.apache.iceberg.expressions.Expressions; +import org.apache.iceberg.expressions.UnboundPredicate; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; +import org.apache.polaris.core.policy.content.AccessControlPolicyContent; + +public class PolicyUtil { + private PolicyUtil() {} + + public static String replaceContextVariable( + String content, PolicyType policyType, AuthenticatedPolarisPrincipal authenticatedPrincipal) { + if (policyType == ACCESS_CONTROL) { + try { + AccessControlPolicyContent policyContent = AccessControlPolicyContent.fromString(content); + List<Expression> evaluatedRowFilterExpressions = Lists.newArrayList(); + for (Expression rowFilterExpression : policyContent.getRowFilters()) { + // check if the expression refers to context variable current_principal_role + if (rowFilterExpression instanceof UnboundPredicate<?>) { + UnboundPredicate<?> boundPredicate = (UnboundPredicate<?>) rowFilterExpression; + // check if this references to current_principal + if (boundPredicate.ref().name().equals("$current_principal_role")) { + // compare the literal and replace + String val = (String) boundPredicate.literal().value(); + if (authenticatedPrincipal.getActivatedPrincipalRoleNames().contains(val)) { + // TODO: see if we can utilize the expression evaluation of iceberg SDK + Expression result = + boundPredicate.op().equals(Expression.Operation.EQ) + ? Expressions.alwaysTrue() + : Expressions.alwaysFalse(); + evaluatedRowFilterExpressions.add(result); + } else { + Expression result = + boundPredicate.op().equals(Expression.Operation.NOT_EQ) + ? Expressions.alwaysTrue() + : Expressions.alwaysFalse(); + evaluatedRowFilterExpressions.add(result); + } + } else if (boundPredicate.ref().name().equals("$current_principal")) { + String val = (String) boundPredicate.literal().value(); + if (authenticatedPrincipal.getName().equals(val)) { + Expression result = + boundPredicate.op().equals(Expression.Operation.EQ) + ? Expressions.alwaysTrue() + : Expressions.alwaysFalse(); + evaluatedRowFilterExpressions.add(result); + } else { + Expression result = + boundPredicate.op().equals(Expression.Operation.NOT_EQ) + ? Expressions.alwaysTrue() + : Expressions.alwaysFalse(); + evaluatedRowFilterExpressions.add(result); + } + } else { + evaluatedRowFilterExpressions.add(rowFilterExpression); + } + } + } + + policyContent.setRowFilters(evaluatedRowFilterExpressions); + return AccessControlPolicyContent.toString(policyContent); + } catch (Exception e) { Review Comment: (design) is it the right thing to return the original content if an exception occurs? ########## polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyUtil.java: ########## @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.policy; + +import static org.apache.polaris.core.policy.PredefinedPolicyTypes.ACCESS_CONTROL; + +import com.google.common.collect.Lists; +import java.util.List; +import org.apache.iceberg.expressions.Expression; +import org.apache.iceberg.expressions.Expressions; +import org.apache.iceberg.expressions.UnboundPredicate; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; +import org.apache.polaris.core.policy.content.AccessControlPolicyContent; + +public class PolicyUtil { Review Comment: (best practices) can you please add some unit tests? ########## polaris-core/src/main/java/org/apache/polaris/core/policy/content/AccessControlPolicyContent.java: ########## @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.policy.content; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import com.google.common.base.Strings; +import java.util.List; +import java.util.Set; +import org.apache.iceberg.expressions.Expression; +import org.apache.polaris.core.policy.validator.InvalidPolicyException; + +public class AccessControlPolicyContent implements PolicyContent { Review Comment: (design) it may be seen as a preference, but it seems the overall language community is moving towards immutable objects as data carriers (like java records) and wonder if this is something we should adopt here as well ########## polaris-core/src/main/java/org/apache/polaris/core/policy/content/AccessControlPolicyContent.java: ########## @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.policy.content; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import com.google.common.base.Strings; +import java.util.List; +import java.util.Set; +import org.apache.iceberg.expressions.Expression; +import org.apache.polaris.core.policy.validator.InvalidPolicyException; + +public class AccessControlPolicyContent implements PolicyContent { + + // Optional, if there means policies is applicable to the role + private String principalRole; + + // TODO: model them as iceberg transforms + private List<String> columnProjections; + + // Iceberg expressions without context functions for now. + // Use a custom deserializer for the list of Iceberg Expressions + @JsonDeserialize(using = IcebergExpressionListDeserializer.class) + @JsonSerialize(using = IcebergExpressionListSerializer.class) + private List<Expression> rowFilters; + + private static final String DEFAULT_POLICY_SCHEMA_VERSION = "2025-02-03"; + private static final Set<String> POLICY_SCHEMA_VERSIONS = Set.of(DEFAULT_POLICY_SCHEMA_VERSION); + + public static AccessControlPolicyContent fromString(String content) { + if (Strings.isNullOrEmpty(content)) { + throw new InvalidPolicyException("Policy is empty"); + } + + AccessControlPolicyContent policy; + try { + policy = PolicyContentUtil.MAPPER.readValue(content, AccessControlPolicyContent.class); + } catch (Exception e) { + throw new InvalidPolicyException(e); + } + + boolean isProjectionsEmpty = + policy.getColumnProjections() == null || policy.getColumnProjections().isEmpty(); Review Comment: (design) Maybe we can instruct Jackson to treat null values as empty collection and avoid null checks? ########## polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyUtil.java: ########## @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.policy; + +import static org.apache.polaris.core.policy.PredefinedPolicyTypes.ACCESS_CONTROL; + +import com.google.common.collect.Lists; +import java.util.List; +import org.apache.iceberg.expressions.Expression; +import org.apache.iceberg.expressions.Expressions; +import org.apache.iceberg.expressions.UnboundPredicate; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; +import org.apache.polaris.core.policy.content.AccessControlPolicyContent; + +public class PolicyUtil { + private PolicyUtil() {} + + public static String replaceContextVariable( + String content, PolicyType policyType, AuthenticatedPolarisPrincipal authenticatedPrincipal) { + if (policyType == ACCESS_CONTROL) { + try { + AccessControlPolicyContent policyContent = AccessControlPolicyContent.fromString(content); + List<Expression> evaluatedRowFilterExpressions = Lists.newArrayList(); + for (Expression rowFilterExpression : policyContent.getRowFilters()) { + // check if the expression refers to context variable current_principal_role + if (rowFilterExpression instanceof UnboundPredicate<?>) { + UnboundPredicate<?> boundPredicate = (UnboundPredicate<?>) rowFilterExpression; + // check if this references to current_principal + if (boundPredicate.ref().name().equals("$current_principal_role")) { + // compare the literal and replace + String val = (String) boundPredicate.literal().value(); + if (authenticatedPrincipal.getActivatedPrincipalRoleNames().contains(val)) { + // TODO: see if we can utilize the expression evaluation of iceberg SDK + Expression result = + boundPredicate.op().equals(Expression.Operation.EQ) + ? Expressions.alwaysTrue() + : Expressions.alwaysFalse(); + evaluatedRowFilterExpressions.add(result); + } else { + Expression result = + boundPredicate.op().equals(Expression.Operation.NOT_EQ) + ? Expressions.alwaysTrue() + : Expressions.alwaysFalse(); + evaluatedRowFilterExpressions.add(result); + } + } else if (boundPredicate.ref().name().equals("$current_principal")) { + String val = (String) boundPredicate.literal().value(); + if (authenticatedPrincipal.getName().equals(val)) { + Expression result = + boundPredicate.op().equals(Expression.Operation.EQ) + ? Expressions.alwaysTrue() + : Expressions.alwaysFalse(); + evaluatedRowFilterExpressions.add(result); + } else { + Expression result = + boundPredicate.op().equals(Expression.Operation.NOT_EQ) + ? Expressions.alwaysTrue() + : Expressions.alwaysFalse(); + evaluatedRowFilterExpressions.add(result); + } + } else { + evaluatedRowFilterExpressions.add(rowFilterExpression); + } + } + } + + policyContent.setRowFilters(evaluatedRowFilterExpressions); + return AccessControlPolicyContent.toString(policyContent); + } catch (Exception e) { + return content; + } + } + return content; + } + + public static boolean filterApplicablePolicy( Review Comment: (design) this method doesn't have any javadoc, but shouldn't this belong to some dedicated class related to FGAC instead? ########## polaris-core/src/main/java/org/apache/polaris/core/policy/content/IcebergExpressionListDeserializer.java: ########## @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.policy.content; + +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import org.apache.iceberg.expressions.Expression; +import org.apache.iceberg.expressions.ExpressionParser; + +public class IcebergExpressionListDeserializer extends JsonDeserializer<List<Expression>> { Review Comment: (design) I wonder why a serializer/deserializer pair for the list is needed vs having it for the elementy type (`Expression`)? ########## service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java: ########## @@ -525,16 +535,17 @@ private static Policy constructPolicy(PolicyEntity policyEntity) { .build(); } - private static ApplicablePolicy constructApplicablePolicy( - PolicyEntity policyEntity, boolean inherited) { + private ApplicablePolicy constructApplicablePolicy(PolicyEntity policyEntity, boolean inherited) { Namespace parentNamespace = policyEntity.getParentNamespace(); return ApplicablePolicy.builder() .setPolicyType(policyEntity.getPolicyType().getName()) .setInheritable(policyEntity.getPolicyType().isInheritable()) .setName(policyEntity.getName()) .setDescription(policyEntity.getDescription()) - .setContent(policyEntity.getContent()) + .setContent( Review Comment: I'm not familiar with `PolicyCatalog` exact role but it seems it is used for CRUD operations on the policies. As such, replacing the content of some policies on the fly may not be the right thing to do because people would not be able to access the source of truth? (design) IMHO this seems like a leakage of an access control mechanism outside of the realm of PolarisAuthorizer ########## polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyUtil.java: ########## @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.policy; + +import static org.apache.polaris.core.policy.PredefinedPolicyTypes.ACCESS_CONTROL; + +import com.google.common.collect.Lists; +import java.util.List; +import org.apache.iceberg.expressions.Expression; +import org.apache.iceberg.expressions.Expressions; +import org.apache.iceberg.expressions.UnboundPredicate; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; +import org.apache.polaris.core.policy.content.AccessControlPolicyContent; + +public class PolicyUtil { + private PolicyUtil() {} + + public static String replaceContextVariable( + String content, PolicyType policyType, AuthenticatedPolarisPrincipal authenticatedPrincipal) { + if (policyType == ACCESS_CONTROL) { + try { + AccessControlPolicyContent policyContent = AccessControlPolicyContent.fromString(content); + List<Expression> evaluatedRowFilterExpressions = Lists.newArrayList(); Review Comment: `Lists.newArrayList()` is actually a deprecated pattern since Java 7 was introduced and can simply be replaced with `new ArrayList<>()`. The method is not marked as deprecated, but the Guava javadoc [states it](https://javadoc.io/static/com.google.guava/guava/33.4.8-jre/com/google/common/collect/Lists.html#newArrayList()) though ########## polaris-core/src/main/java/org/apache/polaris/core/policy/content/AccessControlPolicyContent.java: ########## @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.policy.content; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import com.google.common.base.Strings; +import java.util.List; +import java.util.Set; +import org.apache.iceberg.expressions.Expression; +import org.apache.polaris.core.policy.validator.InvalidPolicyException; + +public class AccessControlPolicyContent implements PolicyContent { + + // Optional, if there means policies is applicable to the role + private String principalRole; + + // TODO: model them as iceberg transforms + private List<String> columnProjections; + + // Iceberg expressions without context functions for now. + // Use a custom deserializer for the list of Iceberg Expressions + @JsonDeserialize(using = IcebergExpressionListDeserializer.class) + @JsonSerialize(using = IcebergExpressionListSerializer.class) + private List<Expression> rowFilters; + + private static final String DEFAULT_POLICY_SCHEMA_VERSION = "2025-02-03"; + private static final Set<String> POLICY_SCHEMA_VERSIONS = Set.of(DEFAULT_POLICY_SCHEMA_VERSION); + + public static AccessControlPolicyContent fromString(String content) { + if (Strings.isNullOrEmpty(content)) { + throw new InvalidPolicyException("Policy is empty"); + } + + AccessControlPolicyContent policy; + try { + policy = PolicyContentUtil.MAPPER.readValue(content, AccessControlPolicyContent.class); + } catch (Exception e) { + throw new InvalidPolicyException(e); + } + + boolean isProjectionsEmpty = + policy.getColumnProjections() == null || policy.getColumnProjections().isEmpty(); + boolean isRowFilterEmpty = policy.getRowFilters() == null || policy.getRowFilters().isEmpty(); + if (isProjectionsEmpty && isRowFilterEmpty) { + throw new InvalidPolicyException("Policy must contain 'columnProjections' or 'rowFilters'."); + } + + return policy; + } + + public static String toString(AccessControlPolicyContent content) { Review Comment: (design) Why a static method vs an instance method like `toJsonString()`? (you would not need to check if `content` is `null` for example) ########## polaris-core/src/main/java/org/apache/polaris/core/policy/content/AccessControlPolicyContent.java: ########## @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.policy.content; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import com.google.common.base.Strings; +import java.util.List; +import java.util.Set; +import org.apache.iceberg.expressions.Expression; +import org.apache.polaris.core.policy.validator.InvalidPolicyException; + +public class AccessControlPolicyContent implements PolicyContent { + + // Optional, if there means policies is applicable to the role + private String principalRole; + + // TODO: model them as iceberg transforms + private List<String> columnProjections; + + // Iceberg expressions without context functions for now. + // Use a custom deserializer for the list of Iceberg Expressions + @JsonDeserialize(using = IcebergExpressionListDeserializer.class) + @JsonSerialize(using = IcebergExpressionListSerializer.class) + private List<Expression> rowFilters; + + private static final String DEFAULT_POLICY_SCHEMA_VERSION = "2025-02-03"; Review Comment: What does this date represent? ########## polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyUtil.java: ########## @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.policy; + +import static org.apache.polaris.core.policy.PredefinedPolicyTypes.ACCESS_CONTROL; + +import com.google.common.collect.Lists; +import java.util.List; +import org.apache.iceberg.expressions.Expression; +import org.apache.iceberg.expressions.Expressions; +import org.apache.iceberg.expressions.UnboundPredicate; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; +import org.apache.polaris.core.policy.content.AccessControlPolicyContent; + +public class PolicyUtil { + private PolicyUtil() {} + + public static String replaceContextVariable( + String content, PolicyType policyType, AuthenticatedPolarisPrincipal authenticatedPrincipal) { + if (policyType == ACCESS_CONTROL) { + try { + AccessControlPolicyContent policyContent = AccessControlPolicyContent.fromString(content); + List<Expression> evaluatedRowFilterExpressions = Lists.newArrayList(); + for (Expression rowFilterExpression : policyContent.getRowFilters()) { + // check if the expression refers to context variable current_principal_role + if (rowFilterExpression instanceof UnboundPredicate<?>) { Review Comment: (design) I would recommend to use a visitor pattern (like `ExpressionVisitor`) which is better suited for expression analysis and substitution ########## polaris-core/src/main/java/org/apache/polaris/core/policy/content/PolicyContentUtil.java: ########## @@ -30,6 +31,8 @@ private static ObjectMapper configureMapper() { mapper.configure(DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES, true); // Fails if a required field is present but explicitly null, e.g., {"enable": null} mapper.configure(DeserializationFeature.FAIL_ON_NULL_CREATOR_PROPERTIES, true); + // This will make sure all the Iceberg parsers are loaded. + RESTSerializers.registerAll(mapper); Review Comment: Could that cause conflicts? ########## polaris-core/src/main/java/org/apache/polaris/core/policy/content/AccessControlPolicyContent.java: ########## @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.policy.content; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import com.google.common.base.Strings; +import java.util.List; +import java.util.Set; +import org.apache.iceberg.expressions.Expression; +import org.apache.polaris.core.policy.validator.InvalidPolicyException; + +public class AccessControlPolicyContent implements PolicyContent { + + // Optional, if there means policies is applicable to the role + private String principalRole; + + // TODO: model them as iceberg transforms + private List<String> columnProjections; + + // Iceberg expressions without context functions for now. + // Use a custom deserializer for the list of Iceberg Expressions + @JsonDeserialize(using = IcebergExpressionListDeserializer.class) + @JsonSerialize(using = IcebergExpressionListSerializer.class) + private List<Expression> rowFilters; + + private static final String DEFAULT_POLICY_SCHEMA_VERSION = "2025-02-03"; + private static final Set<String> POLICY_SCHEMA_VERSIONS = Set.of(DEFAULT_POLICY_SCHEMA_VERSION); + + public static AccessControlPolicyContent fromString(String content) { + if (Strings.isNullOrEmpty(content)) { + throw new InvalidPolicyException("Policy is empty"); + } + + AccessControlPolicyContent policy; + try { + policy = PolicyContentUtil.MAPPER.readValue(content, AccessControlPolicyContent.class); + } catch (Exception e) { Review Comment: (design) Not all exceptions are worth catching? Should a `RuntimeException` like `OutOfMemoryException` for example be transformed into an `InvalidPolicyException` instance? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
