Skip to content

Implement PolicyCatalogHandler Stage 3: GetApplicablePolicies #1421

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

Merged
merged 4 commits into from
Apr 22, 2025
Merged

Implement PolicyCatalogHandler Stage 3: GetApplicablePolicies #1421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1051294
auth for getApplicablePolicies
HonahX Apr 21, 2025
978e25a
Fix style
HonahX Apr 22, 2025
19bfb40
Fix merge conflict
HonahX Apr 22, 2025
0da7b1e
nit
HonahX Apr 22, 2025
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
4 changes: 1 addition & 3 deletions polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,7 @@ public enum PolarisAuthorizableOperation {
DETACH_POLICY_FROM_TABLE(POLICY_DETACH, TABLE_DETACH_POLICY),
GET_APPLICABLE_POLICIES_ON_CATALOG(CATALOG_READ_PROPERTIES),
GET_APPLICABLE_POLICIES_ON_NAMESPACE(NAMESPACE_READ_PROPERTIES),
GET_APPLICABLE_POLICIES_ON_TABLE(TABLE_READ_PROPERTIES),
GET_APPLICABLE_POLICIES_ON_VIEW(VIEW_READ_PROPERTIES),
;
GET_APPLICABLE_POLICIES_ON_TABLE(TABLE_READ_PROPERTIES);

private final EnumSet<PolarisPrivilege> privilegesOnTarget;
private final EnumSet<PolarisPrivilege> privilegesOnSecondary;
Expand Down
64 changes: 64 additions & 0 deletions ...c/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogHandlerAuthzTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,4 +722,68 @@ public void testDetachFromPolicyInsufficientPrivileges() {

newWrapper(Set.of(PRINCIPAL_ROLE2)).detachPolicy(POLICY_NS1_1, detachPolicyRequest);
}

@Test
public void testGetApplicablePoliciesOnCatalogSufficientPrivileges() {
doTestSufficientPrivileges(
List.of(
PolarisPrivilege.CATALOG_READ_PROPERTIES,
PolarisPrivilege.CATALOG_WRITE_PROPERTIES,
PolarisPrivilege.CATALOG_MANAGE_METADATA),
() -> newWrapper().getApplicablePolicies(null, null, null),
null /* cleanupAction */);
}

@Test
public void testGetApplicablePoliciesOnCatalogInsufficientPrivileges() {
doTestInsufficientPrivileges(
List.of(
PolarisPrivilege.NAMESPACE_READ_PROPERTIES,
PolarisPrivilege.POLICY_READ,
PolarisPrivilege.TABLE_READ_PROPERTIES),
() -> newWrapper().getApplicablePolicies(null, null, null));
}

@Test
public void testGetApplicablePoliciesOnNamespaceSufficientPrivileges() {
doTestSufficientPrivileges(
List.of(
PolarisPrivilege.NAMESPACE_READ_PROPERTIES,
PolarisPrivilege.NAMESPACE_WRITE_PROPERTIES,
PolarisPrivilege.CATALOG_MANAGE_METADATA),
() -> newWrapper().getApplicablePolicies(NS1, null, null),
null /* cleanupAction */);
}

@Test
public void testGetApplicablePoliciesOnNamespaceInSufficientPrivileges() {
doTestInsufficientPrivileges(
List.of(
PolarisPrivilege.CATALOG_READ_PROPERTIES,
Copy link
Contributor

@flyrain flyrain Apr 22, 2025

Choose a reason for hiding this comment

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

Q: Does CATALOG_READ_PROPERTIES imply NAMESPACE_READ_PROPERTIES?

Copy link
Contributor Author

@HonahX HonahX Apr 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

No, CATALOG_READ_PROPERTIES is not a super privilege of NAMESPACE_READ_PROPERTIES.

polaris/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java

Lines 209 to 216 in 5f2a375

SUPER_PRIVILEGES.putAll(
NAMESPACE_READ_PROPERTIES,
List.of(
CATALOG_MANAGE_CONTENT,
CATALOG_MANAGE_METADATA,
NAMESPACE_FULL_METADATA,
NAMESPACE_READ_PROPERTIES,
NAMESPACE_WRITE_PROPERTIES));

Just to confirm: This should be the expected that user have right to getApplicablePolicies on catalog does not necessarily have the right to getApplicablePolicies on namespace?

PolarisPrivilege.POLICY_READ,
PolarisPrivilege.TABLE_READ_PROPERTIES),
() -> newWrapper().getApplicablePolicies(NS1, null, null));
}

@Test
public void testGetApplicablePoliciesOnTableSufficientPrivileges() {
doTestSufficientPrivileges(
List.of(
PolarisPrivilege.TABLE_READ_PROPERTIES,
Copy link
Contributor

@flyrain flyrain Apr 22, 2025

Choose a reason for hiding this comment

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

Q: is there a namespace level read privilege can cascade to privilege TABLE_READ_PROPERTIES?

Copy link
Contributor Author

@HonahX HonahX Apr 22, 2025

Choose a reason for hiding this comment

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

No, only CATALOG_MANAGE_METADATA which is a super privilege implies TABLE_READ_PROPERTIES

polaris/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java

Lines 217 to 226 in 5f2a375

SUPER_PRIVILEGES.putAll(
TABLE_READ_PROPERTIES,
List.of(
CATALOG_MANAGE_CONTENT,
CATALOG_MANAGE_METADATA,
TABLE_FULL_METADATA,
TABLE_READ_DATA,
TABLE_READ_PROPERTIES,
TABLE_WRITE_DATA,
TABLE_WRITE_PROPERTIES));

PolarisPrivilege.TABLE_WRITE_PROPERTIES,
PolarisPrivilege.CATALOG_MANAGE_METADATA),
() -> newWrapper().getApplicablePolicies(TABLE_NS1_1.namespace(), TABLE_NS1_1.name(), null),
null /* cleanupAction */);
}

@Test
public void testGetApplicablePoliciesOnTableInsufficientPrivileges() {
doTestInsufficientPrivileges(
List.of(
PolarisPrivilege.CATALOG_READ_PROPERTIES,
PolarisPrivilege.POLICY_READ,
PolarisPrivilege.NAMESPACE_READ_PROPERTIES),
() ->
newWrapper().getApplicablePolicies(TABLE_NS1_1.namespace(), TABLE_NS1_1.name(), null));
}
}
57 changes: 57 additions & 0 deletions .../common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/
package org.apache.polaris.service.catalog.policy;

import com.google.common.base.Strings;
import jakarta.annotation.Nullable;
import jakarta.ws.rs.core.SecurityContext;
import java.util.Arrays;
import java.util.HashSet;
Expand All @@ -26,6 +28,7 @@
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.NoSuchNamespaceException;
import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.exceptions.NotFoundException;
import org.apache.polaris.core.auth.PolarisAuthorizableOperation;
import org.apache.polaris.core.auth.PolarisAuthorizer;
import org.apache.polaris.core.catalog.PolarisCatalogHelpers;
Expand All @@ -43,6 +46,7 @@
import org.apache.polaris.service.types.AttachPolicyRequest;
import org.apache.polaris.service.types.CreatePolicyRequest;
import org.apache.polaris.service.types.DetachPolicyRequest;
import org.apache.polaris.service.types.GetApplicablePoliciesResponse;
import org.apache.polaris.service.types.ListPoliciesResponse;
import org.apache.polaris.service.types.LoadPolicyResponse;
import org.apache.polaris.service.types.PolicyAttachmentTarget;
Expand Down Expand Up @@ -134,6 +138,16 @@ public boolean detachPolicy(PolicyIdentifier identifier, DetachPolicyRequest req
return policyCatalog.detachPolicy(identifier, request.getTarget());
}

public GetApplicablePoliciesResponse getApplicablePolicies(
@Nullable Namespace namespace, @Nullable String targetName, @Nullable PolicyType policyType) {
authorizeGetApplicablePoliciesOperationOrThrow(namespace, targetName);

return GetApplicablePoliciesResponse.builder()
.setApplicablePolicies(
new HashSet<>(policyCatalog.getApplicablePolicies(namespace, targetName, policyType)))
.build();
}

private void authorizeBasicPolicyOperationOrThrow(
PolarisAuthorizableOperation op, PolicyIdentifier identifier) {
resolutionManifest =
Expand Down Expand Up @@ -161,6 +175,49 @@ private void authorizeBasicPolicyOperationOrThrow(
initializeCatalog();
}

private void authorizeGetApplicablePoliciesOperationOrThrow(
@Nullable Namespace namespace, @Nullable String targetName) {
if (namespace == null || namespace.isEmpty()) {
// catalog
PolarisAuthorizableOperation op =
PolarisAuthorizableOperation.GET_APPLICABLE_POLICIES_ON_CATALOG;
authorizeBasicCatalogOperationOrThrow(op);
} else if (Strings.isNullOrEmpty(targetName)) {
// namespace
PolarisAuthorizableOperation op =
PolarisAuthorizableOperation.GET_APPLICABLE_POLICIES_ON_NAMESPACE;
authorizeBasicNamespaceOperationOrThrow(op, namespace);
} else {
// table
TableIdentifier tableIdentifier = TableIdentifier.of(namespace, targetName);
PolarisAuthorizableOperation op =
PolarisAuthorizableOperation.GET_APPLICABLE_POLICIES_ON_TABLE;
// only Iceberg tables are supported
authorizeBasicTableLikeOperationOrThrow(
op, PolarisEntitySubType.ICEBERG_TABLE, tableIdentifier);
}
}

private void authorizeBasicCatalogOperationOrThrow(PolarisAuthorizableOperation op) {
resolutionManifest =
entityManager.prepareResolutionManifest(callContext, securityContext, catalogName);
resolutionManifest.resolveAll();

PolarisResolvedPathWrapper targetCatalog =
resolutionManifest.getResolvedReferenceCatalogEntity();
if (targetCatalog == null) {
throw new NotFoundException("Catalog not found");
}
authorizer.authorizeOrThrow(
authenticatedPrincipal,
resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(),
op,
targetCatalog,
null);

initializeCatalog();
Copy link
Contributor

@flyrain flyrain Apr 22, 2025

Choose a reason for hiding this comment

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

Just for my information: Is this mainly for refreshing the object resolutionManifest in the polarisCatalog? I guess we could give a better name to something like reinitiatCatalog(). Not a blocker though.

Copy link

@sfc-gh-jojiang sfc-gh-jojiang Apr 22, 2025

Choose a reason for hiding this comment

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

This is the time when the PolicyCatalog is initialized. The high-level workflow is that for each rest request, we first initialize a PoicyCatalogHandler and put entities into resolutionManifest, resolve and do authorization, if the authorization passed, a PolicyCatalog is initialized to handle the request. So PolicyCatalog object is never re-used after it serves one request

}

private void authorizePolicyMappingOperationOrThrow(
PolicyIdentifier identifier, PolicyAttachmentTarget target, boolean isAttach) {
resolutionManifest =
Expand Down