bharos commented on code in PR #14364: URL: https://github.com/apache/gravitino/pull/14364#discussion_r4461722325
########## core/src/test/java/org/apache/gravitino/authorization/TagPolicyTestUtils.java: ########## @@ -0,0 +1,131 @@ +/* + * 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.gravitino.authorization; + +import java.time.Instant; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.meta.AuditInfo; +import org.apache.gravitino.meta.PolicyEntity; +import org.apache.gravitino.meta.TagEntity; +import org.apache.gravitino.policy.Policy; +import org.apache.gravitino.policy.PolicyAssociationSelector; +import org.apache.gravitino.policy.PolicyContents; +import org.apache.gravitino.tag.TagAssignment; +import org.apache.gravitino.utils.NamespaceUtil; + +/** Shared tag-policy entities and a controllable reader for authorization tests. */ +final class TagPolicyTestUtils { + static final String METALAKE = "metalake"; + + private TagPolicyTestUtils() {} + + static TagEntity tag(long id, String name, TagAssignment assignment) { + return TagEntity.builder() + .withId(id) + .withName(name) + .withNamespace(NamespaceUtil.ofTag(METALAKE)) + .withProperties(Collections.emptyMap()) + .withAuditInfo(audit()) + .withAssignment(assignment) + .build(); + } + + static PolicyEntity policy( + long id, boolean enabled, List<Privilege.Name> privileges, List<String> roles) { + return PolicyEntity.builder() + .withId(id) + .withName("policy_" + id) + .withNamespace(NamespaceUtil.ofPolicy(METALAKE)) + .withPolicyType(Policy.BuiltInType.ACCESS_CONTROL) + .withEnabled(enabled) + .withContent(PolicyContents.accessControl(privileges, roles)) + .withAuditInfo(audit()) + .build(); + } + + static AuditInfo audit() { + return AuditInfo.builder().withCreator("tester").withCreateTime(Instant.EPOCH).build(); + } + + static PolicyEntity customPolicy() { + return PolicyEntity.builder() + .withId(99L) + .withName("custom") + .withNamespace(NamespaceUtil.ofPolicy(METALAKE)) + .withPolicyType(Policy.BuiltInType.CUSTOM) + .withEnabled(true) + .withContent( + PolicyContents.custom( + Collections.singletonMap("rule", "value"), + Collections.singleton(MetadataObject.Type.TABLE), + Collections.emptyMap())) + .withAuditInfo(audit()) + .build(); + } + + /** Counts reads and allows tests to inject assignment or policy-read failures. */ + static final class InMemoryReader implements TagPolicyReader { + final Map<MetadataObject, List<TagEntity>> assignments = new HashMap<>(); + final Map<String, List<ResolvedTagPolicy>> bindings = new HashMap<>(); + final AtomicInteger tagReads = new AtomicInteger(); + final AtomicInteger policyReads = new AtomicInteger(); + RuntimeException tagFailure; + RuntimeException policyFailure; + + void bind(TagEntity tag, PolicyEntity policy, PolicyAssociationSelector selector) { + bindings + .computeIfAbsent(tag.name(), ignored -> new ArrayList<>()) + .add(new ResolvedTagPolicy(tag, policy, selector)); + } + + /** {@inheritDoc} */ + @Override + public List<TagEntity> loadTags(String metalake, MetadataObject object) { + tagReads.incrementAndGet(); + if (tagFailure != null) { + throw tagFailure; + } + return assignments.getOrDefault(object, Collections.emptyList()); + } + + /** {@inheritDoc} */ + @Override + public List<ResolvedTagPolicy> loadPolicyBindings(String metalake, List<TagEntity> tags) { + policyReads.incrementAndGet(); + if (policyFailure != null) { + throw policyFailure; + } + List<ResolvedTagPolicy> result = new ArrayList<>(); + for (TagEntity tag : tags) { Review Comment: nit: duplicate tag names are silently collapsed. A precondition would catch it. ########## core/src/main/java/org/apache/gravitino/authorization/EntityStoreTagPolicyReader.java: ########## @@ -0,0 +1,110 @@ +/* + * 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.gravitino.authorization; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import org.apache.gravitino.Entity; +import org.apache.gravitino.EntityStore; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.RelationalEntity; +import org.apache.gravitino.SupportsRelationOperations; +import org.apache.gravitino.json.PolicyAssociationSelectorSerde; +import org.apache.gravitino.meta.PolicyEntity; +import org.apache.gravitino.meta.TagEntity; +import org.apache.gravitino.policy.AllValuesSelector; +import org.apache.gravitino.policy.PolicyAssociationSelector; +import org.apache.gravitino.utils.MetadataObjectUtil; +import org.apache.gravitino.utils.NameIdentifierUtil; + +/** Reads direct tag assignments and policy bindings through the entity store's relation API. */ +public final class EntityStoreTagPolicyReader implements TagPolicyReader { + private final EntityStore entityStore; + + /** + * Creates a reader backed by the relation store. + * + * @param entityStore the store to read + */ + public EntityStoreTagPolicyReader(EntityStore entityStore) { + this.entityStore = Objects.requireNonNull(entityStore, "entityStore must not be null"); + } + + /** {@inheritDoc} */ + @Override + public List<TagEntity> loadTags(String metalake, MetadataObject object) { Review Comment: If the target or an ancestor isn't in the entity store yet, this throws `NoSuchEntityException`, and that gets cached as a failure for the request. Should a missing object just mean no tags? Please add a test either way. ########## core/src/main/java/org/apache/gravitino/authorization/EntityStoreTagPolicyReader.java: ########## @@ -0,0 +1,110 @@ +/* + * 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.gravitino.authorization; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import org.apache.gravitino.Entity; +import org.apache.gravitino.EntityStore; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.RelationalEntity; +import org.apache.gravitino.SupportsRelationOperations; +import org.apache.gravitino.json.PolicyAssociationSelectorSerde; +import org.apache.gravitino.meta.PolicyEntity; +import org.apache.gravitino.meta.TagEntity; +import org.apache.gravitino.policy.AllValuesSelector; +import org.apache.gravitino.policy.PolicyAssociationSelector; +import org.apache.gravitino.utils.MetadataObjectUtil; +import org.apache.gravitino.utils.NameIdentifierUtil; + +/** Reads direct tag assignments and policy bindings through the entity store's relation API. */ +public final class EntityStoreTagPolicyReader implements TagPolicyReader { + private final EntityStore entityStore; + + /** + * Creates a reader backed by the relation store. + * + * @param entityStore the store to read + */ + public EntityStoreTagPolicyReader(EntityStore entityStore) { + this.entityStore = Objects.requireNonNull(entityStore, "entityStore must not be null"); + } + + /** {@inheritDoc} */ + @Override + public List<TagEntity> loadTags(String metalake, MetadataObject object) { + try { + return entityStore + .relationOperations() + .listEntitiesByRelation( + SupportsRelationOperations.Type.TAG_METADATA_OBJECT_REL, + MetadataObjectUtil.toEntityIdent(metalake, object), + MetadataObjectUtil.toEntityType(object)); + } catch (IOException e) { + throw new UncheckedIOException("Failed to read tag assignments for " + object, e); + } + } + + /** {@inheritDoc} */ + @Override + public List<ResolvedTagPolicy> loadPolicyBindings( + String metalake, List<TagEntity> effectiveTags) { + if (effectiveTags.isEmpty()) { + return Collections.emptyList(); + } + Map<NameIdentifier, TagEntity> tagsByIdentifier = new LinkedHashMap<>(); + for (TagEntity tag : effectiveTags) { + tagsByIdentifier.put(NameIdentifierUtil.ofTag(metalake, tag.name()), tag); Review Comment: nit: duplicate tag names are silently collapsed. A precondition would catch it. ########## core/src/main/java/org/apache/gravitino/authorization/TagPolicyResolver.java: ########## @@ -0,0 +1,77 @@ +/* + * 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.gravitino.authorization; + +import java.util.Arrays; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.meta.TagEntity; +import org.apache.gravitino.policy.Policy; +import org.apache.gravitino.tag.EffectiveTagResolver; + +/** Resolves tag assignments and policy bindings once per target object and request. */ +public final class TagPolicyResolver { + private final TagPolicyReader reader; + + /** + * Creates a resolver with a storage-backed or preloaded reader. + * + * @param reader the source of direct assignments and policy bindings + */ + public TagPolicyResolver(TagPolicyReader reader) { + this.reader = Objects.requireNonNull(reader, "reader must not be null"); + } + + /** + * Resolves enabled access policies and their effective tag assignments at the target. + * + * @param metalake the containing metalake + * @param target the original object whose access is being checked + * @param context the context shared by checks and list workers of this request + * @return immutable bindings in tag-name and policy-ID order; selectors have not been evaluated + * @throws RuntimeException if any tag or policy read fails + */ + public List<ResolvedTagPolicy> resolve( Review Comment: We hit storage before knowing whether the privilege can be conferred at all. Should callers check `PERMITTED_PRIVILEGES` (and the target type) first, so `USE_SCHEMA`-style checks don't pay for tag reads? -- 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]
