diff --git a/datahub-graphql-core/src/main/resources/entity.graphql b/datahub-graphql-core/src/main/resources/entity.graphql index 92efd71255..2fbc5160b3 100644 --- a/datahub-graphql-core/src/main/resources/entity.graphql +++ b/datahub-graphql-core/src/main/resources/entity.graphql @@ -9323,19 +9323,22 @@ The resources that a DataHub Access Policy applies to """ type ResourceFilter { """ + Deprecated, use filter instead The type of the resource the policy should apply to Not required because in the future we want to support filtering by type OR by domain """ - type: String + type: String @deprecated """ + Deprecated, use filter instead A list of specific resource urns to apply the filter to """ - resources: [String!] + resources: [String!] @deprecated """ + Deprecated, use filter instead Whether of not to apply the filter to all resources of the type """ - allResources: Boolean + allResources: Boolean @deprecated """ Whether of not to apply the filter to all resources of the type @@ -9469,17 +9472,20 @@ Input required when creating or updating an Access Policies Determines which res """ input ResourceFilterInput { """ + Deprecated, use filter field instead The type of the resource the policy should apply to Not required because in the future we want to support filtering by type OR by domain """ - type: String + type: String @deprecated """ + Deprecated, use filter instead A list of specific resource urns to apply the filter to """ resources: [String!] """ + Deprecated, use empty filter instead Whether of not to apply the filter to all resources of the type """ allResources: Boolean diff --git a/metadata-service/auth-impl/src/main/java/com/datahub/authorization/PolicyEngine.java b/metadata-service/auth-impl/src/main/java/com/datahub/authorization/PolicyEngine.java index f01c0c3f20..b486106f74 100644 --- a/metadata-service/auth-impl/src/main/java/com/datahub/authorization/PolicyEngine.java +++ b/metadata-service/auth-impl/src/main/java/com/datahub/authorization/PolicyEngine.java @@ -232,9 +232,8 @@ public class PolicyEngine { .setValues( new StringArray(Collections.singletonList(policyResourceFilter.getType())))); } - if (policyResourceFilter.hasType() - && policyResourceFilter.hasResources() - && !policyResourceFilter.isAllResources()) { + + if (policyResourceFilter.hasResources() && !policyResourceFilter.isAllResources()) { criteria.add( new PolicyMatchCriterion() .setField(EntityFieldType.URN.name()) diff --git a/metadata-service/auth-impl/src/test/java/com/datahub/authorization/PolicyEngineTest.java b/metadata-service/auth-impl/src/test/java/com/datahub/authorization/PolicyEngineTest.java index 016a7ba34f..c94d1c76cf 100644 --- a/metadata-service/auth-impl/src/test/java/com/datahub/authorization/PolicyEngineTest.java +++ b/metadata-service/auth-impl/src/test/java/com/datahub/authorization/PolicyEngineTest.java @@ -955,6 +955,56 @@ public class PolicyEngineTest { verify(_entityClient, times(0)).batchGetV2(any(), any(), any(), any()); } + @Test + public void testEvaluatePolicyResourceFilterSpecificResourceMatchLegacyWithNoType() + throws Exception { + final DataHubPolicyInfo dataHubPolicyInfo = new DataHubPolicyInfo(); + dataHubPolicyInfo.setType(METADATA_POLICY_TYPE); + dataHubPolicyInfo.setState(ACTIVE_POLICY_STATE); + dataHubPolicyInfo.setPrivileges(new StringArray("EDIT_ENTITY_TAGS")); + dataHubPolicyInfo.setDisplayName("My Test Display"); + dataHubPolicyInfo.setDescription("My test display!"); + dataHubPolicyInfo.setEditable(true); + + final DataHubActorFilter actorFilter = new DataHubActorFilter(); + actorFilter.setResourceOwners(true); + actorFilter.setAllUsers(true); + actorFilter.setAllGroups(true); + dataHubPolicyInfo.setActors(actorFilter); + + final DataHubResourceFilter resourceFilter = new DataHubResourceFilter(); + resourceFilter.setAllResources(true); + + ResolvedEntitySpec resourceSpec = buildEntityResolvers("dataset", RESOURCE_URN); + PolicyEngine.PolicyEvaluationResult result = + _policyEngine.evaluatePolicy( + systemOperationContext, + dataHubPolicyInfo, + resolvedAuthorizedUserSpec, + "EDIT_ENTITY_TAGS", + Optional.of(resourceSpec)); + assertTrue(result.isGranted()); + + StringArray resourceUrns = new StringArray(); + resourceUrns.add(RESOURCE_URN); // Filter applies to specific resource. + resourceFilter.setResources(resourceUrns); + resourceFilter.setAllResources(false); // This time should match due to RESOURCE_URN + dataHubPolicyInfo.setResources(resourceFilter); + + resourceSpec = buildEntityResolvers("dataset", RESOURCE_URN); + result = + _policyEngine.evaluatePolicy( + systemOperationContext, + dataHubPolicyInfo, + resolvedAuthorizedUserSpec, + "EDIT_ENTITY_TAGS", + Optional.of(resourceSpec)); + assertTrue(result.isGranted()); + + // Verify no network calls + verify(_entityClient, times(0)).batchGetV2(any(), any(), any(), any()); + } + @Test public void testEvaluatePolicyResourceFilterSpecificResourceMatch() throws Exception { final DataHubPolicyInfo dataHubPolicyInfo = new DataHubPolicyInfo(); @@ -1037,6 +1087,60 @@ public class PolicyEngineTest { verify(_entityClient, times(0)).batchGetV2(any(), any(), any(), any()); } + @Test + public void testEvaluatePolicyResourceFilterSpecificResourceLegacyNoMatchWithNoType() + throws Exception { + final DataHubPolicyInfo dataHubPolicyInfo = new DataHubPolicyInfo(); + dataHubPolicyInfo.setType(METADATA_POLICY_TYPE); + dataHubPolicyInfo.setState(ACTIVE_POLICY_STATE); + dataHubPolicyInfo.setPrivileges(new StringArray("EDIT_ENTITY_TAGS")); + dataHubPolicyInfo.setDisplayName("My Test Display"); + dataHubPolicyInfo.setDescription("My test display!"); + dataHubPolicyInfo.setEditable(true); + + final DataHubActorFilter actorFilter = new DataHubActorFilter(); + actorFilter.setResourceOwners(true); + actorFilter.setAllUsers(true); + actorFilter.setAllGroups(true); + dataHubPolicyInfo.setActors(actorFilter); + + final DataHubResourceFilter resourceFilter = new DataHubResourceFilter(); + StringArray resourceUrns = new StringArray(); + resourceUrns.add(RESOURCE_URN); // Filter applies to specific resource. + resourceFilter.setResources(resourceUrns); + dataHubPolicyInfo.setResources(resourceFilter); + + ResolvedEntitySpec resourceSpec = + buildEntityResolvers( + "dataset", "urn:li:dataset:random"); // A resource not covered by the policy. + PolicyEngine.PolicyEvaluationResult result = + _policyEngine.evaluatePolicy( + systemOperationContext, + dataHubPolicyInfo, + resolvedAuthorizedUserSpec, + "EDIT_ENTITY_TAGS", + Optional.of(resourceSpec)); + assertFalse(result.isGranted()); + + resourceFilter.setAllResources(true); + dataHubPolicyInfo.setResources(resourceFilter); + + resourceSpec = + buildEntityResolvers( + "dataset", "urn:li:dataset:random"); // A resource not covered by the policy. + result = + _policyEngine.evaluatePolicy( + systemOperationContext, + dataHubPolicyInfo, + resolvedAuthorizedUserSpec, + "EDIT_ENTITY_TAGS", + Optional.of(resourceSpec)); + assertTrue(result.isGranted()); // Due to allResources set to true + + // Verify no network calls + verify(_entityClient, times(0)).batchGetV2(any(), any(), any(), any()); + } + @Test public void testEvaluatePolicyResourceFilterResourceUrnStartsWithMatch() throws Exception { final DataHubPolicyInfo dataHubPolicyInfo = new DataHubPolicyInfo();