From e0ad49f0208bc462e1138cdb3abcefb7f43b82c8 Mon Sep 17 00:00:00 2001 From: skrydal Date: Wed, 1 Mar 2023 13:11:40 +0100 Subject: [PATCH 01/17] Initial approach to fine-grained resource owners handling. Types are visible in UI and enforced for users (no groups) --- .../mappers/PolicyInfoPolicyMapper.java | 16 ++++--- .../types/policy/DataHubPolicyMapper.java | 7 ++++ .../src/main/resources/entity.graphql | 2 + .../permissions/policy/PolicyDetailsModal.tsx | 9 ++++ datahub-web-react/src/graphql/policy.graphql | 1 + .../linkedin/policy/DataHubActorFilter.pdl | 6 +++ .../datahub/authorization/PolicyEngine.java | 42 ++++++++++++++----- 7 files changed, 64 insertions(+), 19 deletions(-) diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java index fdcbb7eda8338..74a44d4717f0c 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java @@ -1,21 +1,15 @@ package com.linkedin.datahub.graphql.resolvers.policy.mappers; +import com.linkedin.common.OwnershipTypeArray; import com.linkedin.common.urn.Urn; -import com.linkedin.datahub.graphql.generated.ActorFilter; -import com.linkedin.datahub.graphql.generated.Policy; -import com.linkedin.datahub.graphql.generated.PolicyMatchCondition; -import com.linkedin.datahub.graphql.generated.PolicyMatchCriterion; -import com.linkedin.datahub.graphql.generated.PolicyMatchCriterionValue; -import com.linkedin.datahub.graphql.generated.PolicyMatchFilter; -import com.linkedin.datahub.graphql.generated.PolicyState; -import com.linkedin.datahub.graphql.generated.PolicyType; -import com.linkedin.datahub.graphql.generated.ResourceFilter; +import com.linkedin.datahub.graphql.generated.*; import com.linkedin.datahub.graphql.types.common.mappers.UrnToEntityMapper; import com.linkedin.datahub.graphql.types.mappers.ModelMapper; import com.linkedin.policy.DataHubActorFilter; import com.linkedin.policy.DataHubPolicyInfo; import com.linkedin.policy.DataHubResourceFilter; import java.net.URISyntaxException; +import java.util.List; import java.util.stream.Collectors; import javax.annotation.Nonnull; @@ -53,6 +47,10 @@ private ActorFilter mapActors(final DataHubActorFilter actorFilter) { result.setAllGroups(actorFilter.isAllGroups()); result.setAllUsers(actorFilter.isAllUsers()); result.setResourceOwners(actorFilter.isResourceOwners()); + OwnershipTypeArray resourceOwnersTypes = actorFilter.getResourceOwnersType(); + if (resourceOwnersTypes != null) { + result.setResourceOwnersTypes(resourceOwnersTypes.stream().map(type -> OwnershipType.valueOf(type.toString())).collect(Collectors.toList())); + } if (actorFilter.hasGroups()) { result.setGroups(actorFilter.getGroups().stream().map(Urn::toString).collect(Collectors.toList())); } diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java index 3d28446872a22..c847ffeccdcd9 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java @@ -1,5 +1,6 @@ package com.linkedin.datahub.graphql.types.policy; +import com.linkedin.common.OwnershipTypeArray; import com.linkedin.common.urn.Urn; import com.linkedin.data.DataMap; import com.linkedin.datahub.graphql.generated.ActorFilter; @@ -12,6 +13,7 @@ import com.linkedin.datahub.graphql.generated.PolicyState; import com.linkedin.datahub.graphql.generated.PolicyType; import com.linkedin.datahub.graphql.generated.ResourceFilter; +import com.linkedin.datahub.graphql.generated.OwnershipType; import com.linkedin.datahub.graphql.types.common.mappers.UrnToEntityMapper; import com.linkedin.datahub.graphql.types.common.mappers.util.MappingHelper; import com.linkedin.datahub.graphql.types.mappers.ModelMapper; @@ -67,6 +69,11 @@ private ActorFilter mapActors(final DataHubActorFilter actorFilter) { result.setAllGroups(actorFilter.isAllGroups()); result.setAllUsers(actorFilter.isAllUsers()); result.setResourceOwners(actorFilter.isResourceOwners()); + // Change here is not executed at the moment - leaving it for the future + OwnershipTypeArray resourceOwnersTypes = actorFilter.getResourceOwnersType(); + if (resourceOwnersTypes != null) { + result.setResourceOwnersTypes(resourceOwnersTypes.stream().map(type -> OwnershipType.valueOf(type.toString())).collect(Collectors.toList())); + } if (actorFilter.hasGroups()) { result.setGroups(actorFilter.getGroups().stream().map(Urn::toString).collect(Collectors.toList())); } diff --git a/datahub-graphql-core/src/main/resources/entity.graphql b/datahub-graphql-core/src/main/resources/entity.graphql index f3bd93e3eb729..ace0df99ba509 100644 --- a/datahub-graphql-core/src/main/resources/entity.graphql +++ b/datahub-graphql-core/src/main/resources/entity.graphql @@ -7823,6 +7823,8 @@ type ActorFilter { """ resourceOwners: Boolean! + resourceOwnersTypes: [OwnershipType] + """ Whether the filter should apply to all users """ diff --git a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx index 3b5fc03f10a15..e1088931928da 100644 --- a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx +++ b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx @@ -65,6 +65,7 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege const isActive = policy?.state === PolicyState.Active; const isMetadataPolicy = policy?.type === PolicyType.Metadata; + const isResourceOwnersTypesDefined = (policy?.actors?.resourceOwnersTypes?.length ?? 0) > 0; const resources = convertLegacyResourceFilter(policy?.resources); const resourceTypes = getFieldValues(resources?.filter, 'RESOURCE_TYPE') || []; @@ -180,6 +181,14 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege Applies to Owners {policy?.actors?.resourceOwners ? 'True' : 'False'} + {isResourceOwnersTypesDefined && ( +
+ Only to owners of types: + {policy?.actors?.resourceOwnersTypes?.map((type) => ( + {type} + ))} +
+ )}
Applies to Users diff --git a/datahub-web-react/src/graphql/policy.graphql b/datahub-web-react/src/graphql/policy.graphql index cb2b66ffc61a5..38ceb0afe93ed 100644 --- a/datahub-web-react/src/graphql/policy.graphql +++ b/datahub-web-react/src/graphql/policy.graphql @@ -34,6 +34,7 @@ query listPolicies($input: ListPoliciesInput!) { allUsers allGroups resourceOwners + resourceOwnersTypes resolvedUsers { username urn diff --git a/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl b/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl index 307ba2ba3d09a..a40f46cf2b9e6 100644 --- a/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl +++ b/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl @@ -1,6 +1,7 @@ namespace com.linkedin.policy import com.linkedin.common.Urn +import com.linkedin.common.OwnershipType /** * Information used to filter DataHub actors. @@ -23,6 +24,11 @@ record DataHubActorFilter { */ resourceOwners: boolean = false + /** + * Define type of ownership for policy + */ + resourceOwnersType: optional array[OwnershipType] + /** * Whether the filter should apply to all users. */ 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 45e3e4b8b188b..8bc1ebb9d446a 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 @@ -2,15 +2,20 @@ import com.datahub.authentication.Authentication; import com.google.common.collect.ImmutableSet; +import com.linkedin.common.Owner; +import com.linkedin.common.Ownership; +import com.linkedin.common.OwnershipType; import com.linkedin.common.urn.Urn; import com.linkedin.common.urn.UrnUtils; import com.linkedin.data.template.StringArray; import com.linkedin.entity.EntityResponse; +import com.linkedin.entity.EnvelopedAspect; import com.linkedin.entity.EnvelopedAspectMap; import com.linkedin.entity.client.EntityClient; import com.linkedin.identity.GroupMembership; import com.linkedin.identity.NativeGroupMembership; import com.linkedin.identity.RoleMembership; +import com.linkedin.metadata.Constants; import com.linkedin.metadata.authorization.PoliciesConfig; import com.linkedin.policy.DataHubActorFilter; import com.linkedin.policy.DataHubPolicyInfo; @@ -20,14 +25,9 @@ import com.linkedin.policy.PolicyMatchCriterionArray; import com.linkedin.policy.PolicyMatchFilter; import java.net.URISyntaxException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.annotation.Nullable; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -303,11 +303,33 @@ private boolean isOwnerMatch( if (!actorFilter.isResourceOwners() || !requestResource.isPresent()) { return false; } - return isActorOwner(actor, requestResource.get(), context); + List ownershipTypes = actorFilter.getResourceOwnersType(); + return isActorOwner(actor, requestResource.get(), ownershipTypes, context); } - private boolean isActorOwner(Urn actor, ResolvedResourceSpec resourceSpec, PolicyEvaluationContext context) { - Set owners = resourceSpec.getOwners(); + private Set getOwnersForType(ResourceSpec resourceSpec, List ownershipTypes) { + Urn entityUrn = UrnUtils.getUrn(resourceSpec.getResource()); + EnvelopedAspect ownershipAspect; + try { + EntityResponse response = _entityClient.getV2(entityUrn.getEntityType(), entityUrn, + Collections.singleton(Constants.OWNERSHIP_ASPECT_NAME), _systemAuthentication); + if (response == null || !response.getAspects().containsKey(Constants.OWNERSHIP_ASPECT_NAME)) { + return Collections.emptySet(); + } + ownershipAspect = response.getAspects().get(Constants.OWNERSHIP_ASPECT_NAME); + } catch (Exception e) { + log.error("Error while retrieving domains aspect for urn {}", entityUrn, e); + return Collections.emptySet(); + } + Ownership ownership = new Ownership(ownershipAspect.getValue().data()); + Stream ownersStream = ownership.getOwners().stream(); + if (ownershipTypes != null) + ownersStream = ownersStream.filter(owner -> ownershipTypes.contains(owner.getType())); + return ownersStream.map(owner -> owner.getOwner().toString()).collect(Collectors.toSet()); + } + + private boolean isActorOwner(Urn actor, ResolvedResourceSpec resourceSpec, List ownershipTypes, PolicyEvaluationContext context) { + Set owners = this.getOwnersForType(resourceSpec.getSpec(), ownershipTypes); if (isUserOwner(actor, owners)) { return true; } From ed670a0563292a81cc25be2db3e86a2ec30db328 Mon Sep 17 00:00:00 2001 From: skrydal Date: Sat, 4 Mar 2023 21:04:53 +0100 Subject: [PATCH 02/17] Adjusting logging message --- .../src/main/java/com/datahub/authorization/PolicyEngine.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8bc1ebb9d446a..223fa7cf9d53f 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 @@ -318,7 +318,7 @@ private Set getOwnersForType(ResourceSpec resourceSpec, List Date: Sat, 4 Mar 2023 21:10:38 +0100 Subject: [PATCH 03/17] Adjusting existing tests --- .../authorization/PolicyEngineTest.java | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) 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 732b7633aad1a..1ea2d09340c37 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 @@ -4,12 +4,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.linkedin.common.AuditStamp; -import com.linkedin.common.Owner; -import com.linkedin.common.OwnerArray; -import com.linkedin.common.Ownership; -import com.linkedin.common.OwnershipType; -import com.linkedin.common.UrnArray; +import com.linkedin.common.*; import com.linkedin.common.urn.Urn; import com.linkedin.common.urn.UrnUtils; import com.linkedin.data.template.StringArray; @@ -21,6 +16,7 @@ import com.linkedin.identity.CorpUserInfo; import com.linkedin.identity.GroupMembership; import com.linkedin.identity.RoleMembership; +import com.linkedin.metadata.Constants; import com.linkedin.policy.DataHubActorFilter; import com.linkedin.policy.DataHubPolicyInfo; import com.linkedin.policy.DataHubResourceFilter; @@ -507,6 +503,13 @@ public void testEvaluatePolicyActorFilterUserResourceOwnersMatch() throws Except resourceFilter.setType("dataset"); dataHubPolicyInfo.setResources(resourceFilter); + final EntityResponse entityResponse = new EntityResponse(); + final EnvelopedAspectMap aspectMap = new EnvelopedAspectMap(); + aspectMap.put(OWNERSHIP_ASPECT_NAME, new EnvelopedAspect().setValue(new Aspect(createOwnershipAspect(true, false).data()))); + entityResponse.setAspects(aspectMap); + when(_entityClient.getV2(eq(resourceUrn.getEntityType()), eq(resourceUrn), eq(Collections.singleton(Constants.OWNERSHIP_ASPECT_NAME)), + any())).thenReturn(entityResponse); + ResolvedResourceSpec resourceSpec = buildResourceResolvers("dataset", RESOURCE_URN, ImmutableSet.of(AUTHORIZED_PRINCIPAL), Collections.emptySet()); // Assert authorized user can edit entity tags, because he is a user owner. @@ -542,6 +545,13 @@ public void testEvaluatePolicyActorFilterGroupResourceOwnersMatch() throws Excep resourceFilter.setType("dataset"); dataHubPolicyInfo.setResources(resourceFilter); + final EntityResponse entityResponse = new EntityResponse(); + final EnvelopedAspectMap aspectMap = new EnvelopedAspectMap(); + aspectMap.put(OWNERSHIP_ASPECT_NAME, new EnvelopedAspect().setValue(new Aspect(createOwnershipAspect(false, true).data()))); + entityResponse.setAspects(aspectMap); + when(_entityClient.getV2(eq(resourceUrn.getEntityType()), eq(resourceUrn), eq(Collections.singleton(Constants.OWNERSHIP_ASPECT_NAME)), + any())).thenReturn(entityResponse); + ResolvedResourceSpec resourceSpec = buildResourceResolvers("dataset", RESOURCE_URN, ImmutableSet.of(AUTHORIZED_GROUP), Collections.emptySet()); // Assert authorized user can edit entity tags, because he is a user owner. @@ -905,6 +915,12 @@ public void testGetGrantedPrivileges() throws Exception { _policyEngine.getGrantedPrivileges(policies, UrnUtils.getUrn(AUTHORIZED_PRINCIPAL), Optional.of(resourceSpec)), ImmutableList.of("PRIVILEGE_1")); + final EntityResponse entityResponse = new EntityResponse(); + final EnvelopedAspectMap aspectMap = new EnvelopedAspectMap(); + aspectMap.put(OWNERSHIP_ASPECT_NAME, new EnvelopedAspect().setValue(new Aspect(createOwnershipAspect(true, false).data()))); + entityResponse.setAspects(aspectMap); + when(_entityClient.getV2(eq(resourceUrn.getEntityType()), eq(resourceUrn), eq(Collections.singleton(Constants.OWNERSHIP_ASPECT_NAME)), + any())).thenReturn(entityResponse); resourceSpec = buildResourceResolvers("dataset", RESOURCE_URN, Collections.singleton(AUTHORIZED_PRINCIPAL), Collections.singleton(DOMAIN_URN)); // Is owner assertEquals( From 9c2657e74496cb07c6b52a300f8c1a89bea99fde Mon Sep 17 00:00:00 2001 From: skrydal Date: Sat, 4 Mar 2023 21:16:32 +0100 Subject: [PATCH 04/17] Created tests for new use-cases --- .../authorization/PolicyEngineTest.java | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) 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 1ea2d09340c37..2e618f8099154 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 @@ -523,6 +523,84 @@ public void testEvaluatePolicyActorFilterUserResourceOwnersMatch() throws Except eq(null), any()); } + @Test + public void testEvaluatePolicyActorFilterUserResourceOwnersTypeMatch() 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(false); + actorFilter.setAllGroups(false); + actorFilter.setResourceOwnersType(new OwnershipTypeArray(Collections.singletonList(OwnershipType.DATAOWNER))); + dataHubPolicyInfo.setActors(actorFilter); + + final DataHubResourceFilter resourceFilter = new DataHubResourceFilter(); + resourceFilter.setAllResources(true); + resourceFilter.setType("dataset"); + dataHubPolicyInfo.setResources(resourceFilter); + + final EntityResponse entityResponse = new EntityResponse(); + final EnvelopedAspectMap aspectMap = new EnvelopedAspectMap(); + aspectMap.put(OWNERSHIP_ASPECT_NAME, new EnvelopedAspect().setValue(new Aspect(createOwnershipAspect(true, false).data()))); + entityResponse.setAspects(aspectMap); + when(_entityClient.getV2(eq(resourceUrn.getEntityType()), eq(resourceUrn), eq(Collections.singleton(Constants.OWNERSHIP_ASPECT_NAME)), + any())).thenReturn(entityResponse); + + ResolvedResourceSpec resourceSpec = + buildResourceResolvers("dataset", RESOURCE_URN, ImmutableSet.of(AUTHORIZED_PRINCIPAL), Collections.emptySet()); + + PolicyEngine.PolicyEvaluationResult result1 = + _policyEngine.evaluatePolicy(dataHubPolicyInfo, AUTHORIZED_PRINCIPAL, "EDIT_ENTITY_TAGS", + Optional.of(resourceSpec)); + assertTrue(result1.isGranted()); + } + + @Test + public void testEvaluatePolicyActorFilterUserResourceOwnersTypeNoMatch() 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(false); + actorFilter.setAllGroups(false); + actorFilter.setResourceOwnersType(new OwnershipTypeArray(Collections.singletonList(OwnershipType.TECHNICAL_OWNER))); + dataHubPolicyInfo.setActors(actorFilter); + + final DataHubResourceFilter resourceFilter = new DataHubResourceFilter(); + resourceFilter.setAllResources(true); + resourceFilter.setType("dataset"); + dataHubPolicyInfo.setResources(resourceFilter); + + final EntityResponse entityResponse = new EntityResponse(); + final EnvelopedAspectMap aspectMap = new EnvelopedAspectMap(); + aspectMap.put(OWNERSHIP_ASPECT_NAME, new EnvelopedAspect().setValue(new Aspect(createOwnershipAspect(true, false).data()))); + entityResponse.setAspects(aspectMap); + when(_entityClient.getV2(eq(resourceUrn.getEntityType()), eq(resourceUrn), eq(Collections.singleton(Constants.OWNERSHIP_ASPECT_NAME)), + any())).thenReturn(entityResponse); + + ResolvedResourceSpec resourceSpec = + buildResourceResolvers("dataset", RESOURCE_URN, ImmutableSet.of(AUTHORIZED_PRINCIPAL), Collections.emptySet()); + + PolicyEngine.PolicyEvaluationResult result1 = + _policyEngine.evaluatePolicy(dataHubPolicyInfo, AUTHORIZED_PRINCIPAL, "EDIT_ENTITY_TAGS", + Optional.of(resourceSpec)); + assertFalse(result1.isGranted()); + } + @Test public void testEvaluatePolicyActorFilterGroupResourceOwnersMatch() throws Exception { From 4f20bb122c02b8f8a26956496709a1531dfcffac Mon Sep 17 00:00:00 2001 From: skrydal Date: Thu, 1 Jun 2023 14:31:33 +0200 Subject: [PATCH 05/17] Adjusting to new approach --- .../policy/mappers/PolicyInfoPolicyMapper.java | 7 +++---- .../graphql/types/policy/DataHubPolicyMapper.java | 8 ++++---- .../src/main/resources/entity.graphql | 2 +- .../com/linkedin/policy/DataHubActorFilter.pdl | 3 +-- .../com/datahub/authorization/PolicyEngine.java | 14 +++++++------- .../com.linkedin.entity.entities.snapshot.json | 8 ++++++++ .../com.linkedin.platform.platform.snapshot.json | 8 ++++++++ 7 files changed, 32 insertions(+), 18 deletions(-) diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java index 74a44d4717f0c..52dc80034dfc0 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java @@ -1,6 +1,6 @@ package com.linkedin.datahub.graphql.resolvers.policy.mappers; -import com.linkedin.common.OwnershipTypeArray; +import com.linkedin.common.UrnArray; import com.linkedin.common.urn.Urn; import com.linkedin.datahub.graphql.generated.*; import com.linkedin.datahub.graphql.types.common.mappers.UrnToEntityMapper; @@ -9,7 +9,6 @@ import com.linkedin.policy.DataHubPolicyInfo; import com.linkedin.policy.DataHubResourceFilter; import java.net.URISyntaxException; -import java.util.List; import java.util.stream.Collectors; import javax.annotation.Nonnull; @@ -47,9 +46,9 @@ private ActorFilter mapActors(final DataHubActorFilter actorFilter) { result.setAllGroups(actorFilter.isAllGroups()); result.setAllUsers(actorFilter.isAllUsers()); result.setResourceOwners(actorFilter.isResourceOwners()); - OwnershipTypeArray resourceOwnersTypes = actorFilter.getResourceOwnersType(); + UrnArray resourceOwnersTypes = actorFilter.getResourceOwnersTypesUrns(); if (resourceOwnersTypes != null) { - result.setResourceOwnersTypes(resourceOwnersTypes.stream().map(type -> OwnershipType.valueOf(type.toString())).collect(Collectors.toList())); + result.setResourceOwnersTypesUrns(resourceOwnersTypes.stream().map(Urn::toString).collect(Collectors.toList())); } if (actorFilter.hasGroups()) { result.setGroups(actorFilter.getGroups().stream().map(Urn::toString).collect(Collectors.toList())); diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java index c847ffeccdcd9..dbde819c677df 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java @@ -1,6 +1,6 @@ package com.linkedin.datahub.graphql.types.policy; -import com.linkedin.common.OwnershipTypeArray; +import com.linkedin.common.UrnArray; import com.linkedin.common.urn.Urn; import com.linkedin.data.DataMap; import com.linkedin.datahub.graphql.generated.ActorFilter; @@ -70,9 +70,9 @@ private ActorFilter mapActors(final DataHubActorFilter actorFilter) { result.setAllUsers(actorFilter.isAllUsers()); result.setResourceOwners(actorFilter.isResourceOwners()); // Change here is not executed at the moment - leaving it for the future - OwnershipTypeArray resourceOwnersTypes = actorFilter.getResourceOwnersType(); - if (resourceOwnersTypes != null) { - result.setResourceOwnersTypes(resourceOwnersTypes.stream().map(type -> OwnershipType.valueOf(type.toString())).collect(Collectors.toList())); + UrnArray resourceOwnersTypesUrns = actorFilter.getResourceOwnersTypesUrns(); + if (resourceOwnersTypesUrns != null) { + result.setResourceOwnersTypesUrns(resourceOwnersTypesUrns.stream().map(type -> type.toString()).collect(Collectors.toList())); } if (actorFilter.hasGroups()) { result.setGroups(actorFilter.getGroups().stream().map(Urn::toString).collect(Collectors.toList())); diff --git a/datahub-graphql-core/src/main/resources/entity.graphql b/datahub-graphql-core/src/main/resources/entity.graphql index 9ee6ff6558fdb..662920d0f56e5 100644 --- a/datahub-graphql-core/src/main/resources/entity.graphql +++ b/datahub-graphql-core/src/main/resources/entity.graphql @@ -7965,7 +7965,7 @@ type ActorFilter { """ resourceOwners: Boolean! - resourceOwnersTypes: [OwnershipType] + resourceOwnersTypesUrns: [String] """ Whether the filter should apply to all users diff --git a/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl b/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl index a40f46cf2b9e6..da8a443bd5609 100644 --- a/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl +++ b/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl @@ -1,7 +1,6 @@ namespace com.linkedin.policy import com.linkedin.common.Urn -import com.linkedin.common.OwnershipType /** * Information used to filter DataHub actors. @@ -27,7 +26,7 @@ record DataHubActorFilter { /** * Define type of ownership for policy */ - resourceOwnersType: optional array[OwnershipType] + resourceOwnersTypesUrns: optional array[Urn] /** * Whether the filter should apply to all users. 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 223fa7cf9d53f..2f648482db164 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 @@ -303,11 +303,11 @@ private boolean isOwnerMatch( if (!actorFilter.isResourceOwners() || !requestResource.isPresent()) { return false; } - List ownershipTypes = actorFilter.getResourceOwnersType(); - return isActorOwner(actor, requestResource.get(), ownershipTypes, context); + List ownershipTypesUrns = actorFilter.getResourceOwnersTypesUrns(); + return isActorOwner(actor, requestResource.get(), ownershipTypesUrns, context); } - private Set getOwnersForType(ResourceSpec resourceSpec, List ownershipTypes) { + private Set getOwnersForType(ResourceSpec resourceSpec, List ownershipTypesUrns) { Urn entityUrn = UrnUtils.getUrn(resourceSpec.getResource()); EnvelopedAspect ownershipAspect; try { @@ -323,13 +323,13 @@ private Set getOwnersForType(ResourceSpec resourceSpec, List ownersStream = ownership.getOwners().stream(); - if (ownershipTypes != null) - ownersStream = ownersStream.filter(owner -> ownershipTypes.contains(owner.getType())); + if (ownershipTypesUrns != null) + ownersStream = ownersStream.filter(owner -> ownershipTypesUrns.contains(owner.getTypeUrn())); return ownersStream.map(owner -> owner.getOwner().toString()).collect(Collectors.toSet()); } - private boolean isActorOwner(Urn actor, ResolvedResourceSpec resourceSpec, List ownershipTypes, PolicyEvaluationContext context) { - Set owners = this.getOwnersForType(resourceSpec.getSpec(), ownershipTypes); + private boolean isActorOwner(Urn actor, ResolvedResourceSpec resourceSpec, List ownershipTypesUrns, PolicyEvaluationContext context) { + Set owners = this.getOwnersForType(resourceSpec.getSpec(), ownershipTypesUrns); if (isUserOwner(actor, owners)) { return true; } diff --git a/metadata-service/restli-api/src/main/snapshot/com.linkedin.entity.entities.snapshot.json b/metadata-service/restli-api/src/main/snapshot/com.linkedin.entity.entities.snapshot.json index 244afb09f5799..cf0edba87f344 100644 --- a/metadata-service/restli-api/src/main/snapshot/com.linkedin.entity.entities.snapshot.json +++ b/metadata-service/restli-api/src/main/snapshot/com.linkedin.entity.entities.snapshot.json @@ -5250,6 +5250,14 @@ "type" : "boolean", "doc" : "Whether the filter should return true for owners of a particular resource.\nOnly applies to policies of type 'Metadata', which have a resource associated with them.", "default" : false + }, { + "name" : "resourceOwnersTypesUrns", + "type" : { + "type" : "array", + "items" : "com.linkedin.common.Urn" + }, + "doc" : "Define type of ownership for policy", + "optional" : true }, { "name" : "allUsers", "type" : "boolean", diff --git a/metadata-service/restli-api/src/main/snapshot/com.linkedin.platform.platform.snapshot.json b/metadata-service/restli-api/src/main/snapshot/com.linkedin.platform.platform.snapshot.json index 6a8c8f054779f..e5210069df551 100644 --- a/metadata-service/restli-api/src/main/snapshot/com.linkedin.platform.platform.snapshot.json +++ b/metadata-service/restli-api/src/main/snapshot/com.linkedin.platform.platform.snapshot.json @@ -5244,6 +5244,14 @@ "type" : "boolean", "doc" : "Whether the filter should return true for owners of a particular resource.\nOnly applies to policies of type 'Metadata', which have a resource associated with them.", "default" : false + }, { + "name" : "resourceOwnersTypesUrns", + "type" : { + "type" : "array", + "items" : "com.linkedin.common.Urn" + }, + "doc" : "Define type of ownership for policy", + "optional" : true }, { "name" : "allUsers", "type" : "boolean", From 002e774c982720d71b238e019c50f37da440a3b7 Mon Sep 17 00:00:00 2001 From: skrydal Date: Thu, 1 Jun 2023 16:26:36 +0200 Subject: [PATCH 06/17] Adjusting UI --- .../src/app/permissions/policy/PolicyDetailsModal.tsx | 6 +++--- datahub-web-react/src/graphql/policy.graphql | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx index b3d4405bd931b..9e261c98672d4 100644 --- a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx +++ b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx @@ -65,7 +65,7 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege const isActive = policy?.state === PolicyState.Active; const isMetadataPolicy = policy?.type === PolicyType.Metadata; - const isResourceOwnersTypesDefined = (policy?.actors?.resourceOwnersTypes?.length ?? 0) > 0; + const isResourceOwnersTypesUrnsDefined = (policy?.actors?.resourceOwnersTypesUrns?.length ?? 0) > 0; const resources = convertLegacyResourceFilter(policy?.resources); const resourceTypes = getFieldValues(resources?.filter, 'RESOURCE_TYPE') || []; @@ -182,10 +182,10 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege Applies to Owners {policy?.actors?.resourceOwners ? 'True' : 'False'} - {isResourceOwnersTypesDefined && ( + {isResourceOwnersTypesUrnsDefined && (
Only to owners of types: - {policy?.actors?.resourceOwnersTypes?.map((type) => ( + {policy?.actors?.resourceOwnersTypesUrns?.map((type) => ( {type} ))}
diff --git a/datahub-web-react/src/graphql/policy.graphql b/datahub-web-react/src/graphql/policy.graphql index 38ceb0afe93ed..ede83b117fdac 100644 --- a/datahub-web-react/src/graphql/policy.graphql +++ b/datahub-web-react/src/graphql/policy.graphql @@ -34,7 +34,7 @@ query listPolicies($input: ListPoliciesInput!) { allUsers allGroups resourceOwners - resourceOwnersTypes + resourceOwnersTypesUrns resolvedUsers { username urn From 9e01299a948e234232025db1f5be3c18b4888e57 Mon Sep 17 00:00:00 2001 From: skrydal Date: Sun, 4 Jun 2023 00:51:18 +0200 Subject: [PATCH 07/17] Better display of the resource owners --- .../permissions/policy/PolicyDetailsModal.tsx | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx index 9e261c98672d4..f5b58562d3682 100644 --- a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx +++ b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx @@ -65,7 +65,6 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege const isActive = policy?.state === PolicyState.Active; const isMetadataPolicy = policy?.type === PolicyType.Metadata; - const isResourceOwnersTypesUrnsDefined = (policy?.actors?.resourceOwnersTypesUrns?.length ?? 0) > 0; const resources = convertLegacyResourceFilter(policy?.resources); const resourceTypes = getFieldValues(resources?.filter, 'RESOURCE_TYPE') || []; @@ -104,6 +103,23 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege ); }; + const resourceOwnersField = (actors) => { + if (!actors?.resourceOwners) { + return Not applied; + } + if ((actors?.resourceOwnersTypesUrns?.length ?? 0) > 0) { + return ( +
+ Only to owners of types: + {actors?.resourceOwnersTypesUrns?.map((type) => ( + {type} + ))} +
+ ); + } + return Applies to all owners; + }; + return ( @@ -181,15 +197,7 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege
Applies to Owners - {policy?.actors?.resourceOwners ? 'True' : 'False'} - {isResourceOwnersTypesUrnsDefined && ( -
- Only to owners of types: - {policy?.actors?.resourceOwnersTypesUrns?.map((type) => ( - {type} - ))} -
- )} + {resourceOwnersField(policy?.actors)}
Applies to Users From d57319aa6e19d43ad66c06a0bb3ca5fdbcd36f04 Mon Sep 17 00:00:00 2001 From: skrydal Date: Mon, 5 Jun 2023 02:45:55 +0200 Subject: [PATCH 08/17] Added automatic resolution of OwnershipType name to be used in UI --- .../com/linkedin/datahub/graphql/GmsGraphQLEngine.java | 3 +++ datahub-graphql-core/src/main/resources/entity.graphql | 2 ++ .../src/app/permissions/policy/PolicyDetailsModal.tsx | 6 +++--- datahub-web-react/src/graphql/policy.graphql | 7 ++++++- .../pegasus/com/linkedin/policy/DataHubActorFilter.pdl | 1 + 5 files changed, 15 insertions(+), 4 deletions(-) diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/GmsGraphQLEngine.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/GmsGraphQLEngine.java index d3fbfafe5e5dd..27d7aa4ffc4f4 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/GmsGraphQLEngine.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/GmsGraphQLEngine.java @@ -1622,6 +1622,9 @@ private void configurePolicyResolvers(final RuntimeWiring.Builder builder) { })).dataFetcher("resolvedRoles", new LoadableTypeBatchResolver<>(dataHubRoleType, (env) -> { final ActorFilter filter = env.getSource(); return filter.getRoles(); + })).dataFetcher("resolvedOwnershipTypes", new LoadableTypeBatchResolver<>(ownershipType, (env) -> { + final ActorFilter filter = env.getSource(); + return filter.getResourceOwnersTypesUrns(); }))); } diff --git a/datahub-graphql-core/src/main/resources/entity.graphql b/datahub-graphql-core/src/main/resources/entity.graphql index 662920d0f56e5..bbd1c86201abf 100644 --- a/datahub-graphql-core/src/main/resources/entity.graphql +++ b/datahub-graphql-core/src/main/resources/entity.graphql @@ -7967,6 +7967,8 @@ type ActorFilter { resourceOwnersTypesUrns: [String] + resolvedOwnershipTypes: [OwnershipTypeEntity] + """ Whether the filter should apply to all users """ diff --git a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx index f5b58562d3682..886ff994fc092 100644 --- a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx +++ b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx @@ -107,12 +107,12 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege if (!actors?.resourceOwners) { return Not applied; } - if ((actors?.resourceOwnersTypesUrns?.length ?? 0) > 0) { + if ((actors?.resolvedOwnershipTypes?.length ?? 0) > 0) { return (
Only to owners of types: - {actors?.resourceOwnersTypesUrns?.map((type) => ( - {type} + {actors?.resolvedOwnershipTypes?.map((type) => ( + {type.info.name} ))}
); diff --git a/datahub-web-react/src/graphql/policy.graphql b/datahub-web-react/src/graphql/policy.graphql index ede83b117fdac..cad5464fc42f5 100644 --- a/datahub-web-react/src/graphql/policy.graphql +++ b/datahub-web-react/src/graphql/policy.graphql @@ -34,7 +34,12 @@ query listPolicies($input: ListPoliciesInput!) { allUsers allGroups resourceOwners - resourceOwnersTypesUrns + resolvedOwnershipTypes { + urn + info { + name + } + } resolvedUsers { username urn diff --git a/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl b/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl index da8a443bd5609..c90fe72ca8349 100644 --- a/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl +++ b/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl @@ -1,6 +1,7 @@ namespace com.linkedin.policy import com.linkedin.common.Urn +import com.linkedin.ownership.OwnershipTypeInfo /** * Information used to filter DataHub actors. From 43e1abbef90f3f608aa15e61d96b12443d70c8f8 Mon Sep 17 00:00:00 2001 From: skrydal Date: Mon, 5 Jun 2023 03:06:17 +0200 Subject: [PATCH 09/17] Fixing style --- .../policy/mappers/PolicyInfoPolicyMapper.java | 10 +++++++++- .../graphql/types/policy/DataHubPolicyMapper.java | 1 - 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java index 52dc80034dfc0..73d982a47563c 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java @@ -2,7 +2,15 @@ import com.linkedin.common.UrnArray; import com.linkedin.common.urn.Urn; -import com.linkedin.datahub.graphql.generated.*; +import com.linkedin.datahub.graphql.generated.Policy; +import com.linkedin.datahub.graphql.generated.PolicyMatchCondition; +import com.linkedin.datahub.graphql.generated.PolicyMatchCriterion; +import com.linkedin.datahub.graphql.generated.PolicyMatchCriterionValue; +import com.linkedin.datahub.graphql.generated.PolicyMatchFilter; +import com.linkedin.datahub.graphql.generated.PolicyState; +import com.linkedin.datahub.graphql.generated.PolicyType; +import com.linkedin.datahub.graphql.generated.ActorFilter; +import com.linkedin.datahub.graphql.generated.ResourceFilter; import com.linkedin.datahub.graphql.types.common.mappers.UrnToEntityMapper; import com.linkedin.datahub.graphql.types.mappers.ModelMapper; import com.linkedin.policy.DataHubActorFilter; diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java index dbde819c677df..34d91bfce474a 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java @@ -13,7 +13,6 @@ import com.linkedin.datahub.graphql.generated.PolicyState; import com.linkedin.datahub.graphql.generated.PolicyType; import com.linkedin.datahub.graphql.generated.ResourceFilter; -import com.linkedin.datahub.graphql.generated.OwnershipType; import com.linkedin.datahub.graphql.types.common.mappers.UrnToEntityMapper; import com.linkedin.datahub.graphql.types.common.mappers.util.MappingHelper; import com.linkedin.datahub.graphql.types.mappers.ModelMapper; From 4088fb1bc97ffc2a53bbf0bded20e570b12d417d Mon Sep 17 00:00:00 2001 From: skrydal Date: Mon, 5 Jun 2023 04:10:31 +0200 Subject: [PATCH 10/17] Fixing tests --- .../authorization/PolicyEngineTest.java | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) 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 2e618f8099154..2373bc6ffa3fb 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 @@ -47,6 +47,10 @@ public class PolicyEngineTest { private static final String DOMAIN_URN = "urn:li:domain:domain1"; + private static final String OWNERSHIP_TYPE_URN = "urn:li:ownershipType:__system__technical_owner"; + + private static final String OTHER_OWNERSHIP_TYPE_URN = "urn:li:ownershipType:__system__data_steward"; + private EntityClient _entityClient; private PolicyEngine _policyEngine; @@ -538,7 +542,7 @@ public void testEvaluatePolicyActorFilterUserResourceOwnersTypeMatch() throws Ex actorFilter.setResourceOwners(true); actorFilter.setAllUsers(false); actorFilter.setAllGroups(false); - actorFilter.setResourceOwnersType(new OwnershipTypeArray(Collections.singletonList(OwnershipType.DATAOWNER))); + actorFilter.setResourceOwnersTypesUrns(new UrnArray(ImmutableList.of(Urn.createFromString(OWNERSHIP_TYPE_URN)))); dataHubPolicyInfo.setActors(actorFilter); final DataHubResourceFilter resourceFilter = new DataHubResourceFilter(); @@ -548,7 +552,7 @@ public void testEvaluatePolicyActorFilterUserResourceOwnersTypeMatch() throws Ex final EntityResponse entityResponse = new EntityResponse(); final EnvelopedAspectMap aspectMap = new EnvelopedAspectMap(); - aspectMap.put(OWNERSHIP_ASPECT_NAME, new EnvelopedAspect().setValue(new Aspect(createOwnershipAspect(true, false).data()))); + aspectMap.put(OWNERSHIP_ASPECT_NAME, new EnvelopedAspect().setValue(new Aspect(createOwnershipAspectWithTypeUrn(OWNERSHIP_TYPE_URN).data()))); entityResponse.setAspects(aspectMap); when(_entityClient.getV2(eq(resourceUrn.getEntityType()), eq(resourceUrn), eq(Collections.singleton(Constants.OWNERSHIP_ASPECT_NAME)), any())).thenReturn(entityResponse); @@ -577,7 +581,7 @@ public void testEvaluatePolicyActorFilterUserResourceOwnersTypeNoMatch() throws actorFilter.setResourceOwners(true); actorFilter.setAllUsers(false); actorFilter.setAllGroups(false); - actorFilter.setResourceOwnersType(new OwnershipTypeArray(Collections.singletonList(OwnershipType.TECHNICAL_OWNER))); + actorFilter.setResourceOwnersTypesUrns(new UrnArray(ImmutableList.of(Urn.createFromString(OWNERSHIP_TYPE_URN)))); dataHubPolicyInfo.setActors(actorFilter); final DataHubResourceFilter resourceFilter = new DataHubResourceFilter(); @@ -587,7 +591,7 @@ public void testEvaluatePolicyActorFilterUserResourceOwnersTypeNoMatch() throws final EntityResponse entityResponse = new EntityResponse(); final EnvelopedAspectMap aspectMap = new EnvelopedAspectMap(); - aspectMap.put(OWNERSHIP_ASPECT_NAME, new EnvelopedAspect().setValue(new Aspect(createOwnershipAspect(true, false).data()))); + aspectMap.put(OWNERSHIP_ASPECT_NAME, new EnvelopedAspect().setValue(new Aspect(createOwnershipAspectWithTypeUrn(OTHER_OWNERSHIP_TYPE_URN).data()))); entityResponse.setAspects(aspectMap); when(_entityClient.getV2(eq(resourceUrn.getEntityType()), eq(resourceUrn), eq(Collections.singleton(Constants.OWNERSHIP_ASPECT_NAME)), any())).thenReturn(entityResponse); @@ -1128,6 +1132,20 @@ private Ownership createOwnershipAspect(final Boolean addUserOwner, final Boolea return ownershipAspect; } + private Ownership createOwnershipAspectWithTypeUrn(final String typeUrn) throws Exception { + final Ownership ownershipAspect = new Ownership(); + final OwnerArray owners = new OwnerArray(); + + final Owner userOwner = new Owner(); + userOwner.setOwner(Urn.createFromString(AUTHORIZED_PRINCIPAL)); + userOwner.setTypeUrn(Urn.createFromString(typeUrn)); + owners.add(userOwner); + + ownershipAspect.setOwners(owners); + ownershipAspect.setLastModified(new AuditStamp().setTime(0).setActor(Urn.createFromString("urn:li:corpuser:foo"))); + return ownershipAspect; + } + private EntityResponse createAuthorizedEntityResponse() throws URISyntaxException { final EntityResponse entityResponse = new EntityResponse(); final EnvelopedAspectMap aspectMap = new EnvelopedAspectMap(); From ba949fe1723c973f39f57ea0a97a47ce7bc034c7 Mon Sep 17 00:00:00 2001 From: skrydal Date: Mon, 5 Jun 2023 04:15:30 +0200 Subject: [PATCH 11/17] Fixing style --- .../java/com/datahub/authorization/PolicyEngine.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) 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 2f648482db164..cdf4d6c8ea407 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 @@ -4,7 +4,6 @@ import com.google.common.collect.ImmutableSet; import com.linkedin.common.Owner; import com.linkedin.common.Ownership; -import com.linkedin.common.OwnershipType; import com.linkedin.common.urn.Urn; import com.linkedin.common.urn.UrnUtils; import com.linkedin.data.template.StringArray; @@ -25,7 +24,13 @@ import com.linkedin.policy.PolicyMatchCriterionArray; import com.linkedin.policy.PolicyMatchFilter; import java.net.URISyntaxException; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nullable; @@ -323,8 +328,9 @@ private Set getOwnersForType(ResourceSpec resourceSpec, List owners } Ownership ownership = new Ownership(ownershipAspect.getValue().data()); Stream ownersStream = ownership.getOwners().stream(); - if (ownershipTypesUrns != null) + if (ownershipTypesUrns != null) { ownersStream = ownersStream.filter(owner -> ownershipTypesUrns.contains(owner.getTypeUrn())); + } return ownersStream.map(owner -> owner.getOwner().toString()).collect(Collectors.toSet()); } From 74364d073ecda1829f208e93d8e4ec48cab7b2c4 Mon Sep 17 00:00:00 2001 From: skrydal Date: Mon, 5 Jun 2023 04:32:35 +0200 Subject: [PATCH 12/17] Fixing style --- .../java/com/datahub/authorization/PolicyEngineTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 2373bc6ffa3fb..1b6500df44b96 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 @@ -4,7 +4,12 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.linkedin.common.*; +import com.linkedin.common.AuditStamp; +import com.linkedin.common.Owner; +import com.linkedin.common.OwnerArray; +import com.linkedin.common.Ownership; +import com.linkedin.common.OwnershipType; +import com.linkedin.common.UrnArray; import com.linkedin.common.urn.Urn; import com.linkedin.common.urn.UrnUtils; import com.linkedin.data.template.StringArray; From b40b8852d13e95e352a902c53e428abe6f43e427 Mon Sep 17 00:00:00 2001 From: skrydal Date: Tue, 6 Jun 2023 13:39:27 +0200 Subject: [PATCH 13/17] Added capability of updating policies via UI Co-Authored-By: davidlacedonia --- .../mappers/PolicyUpdateInputInfoMapper.java | 3 + .../src/main/resources/entity.graphql | 6 +- .../app/permissions/policy/ManagePolicies.tsx | 1 + .../permissions/policy/PolicyActorForm.tsx | 70 ++++++++++++++++++- .../permissions/policy/PolicyDetailsModal.tsx | 1 - datahub-web-react/src/graphql/policy.graphql | 1 + 6 files changed, 78 insertions(+), 4 deletions(-) diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyUpdateInputInfoMapper.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyUpdateInputInfoMapper.java index 1e0f41c68b32e..430ec1b031b94 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyUpdateInputInfoMapper.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyUpdateInputInfoMapper.java @@ -51,6 +51,9 @@ private DataHubActorFilter mapActors(final ActorFilterInput actorInput) { result.setAllGroups(actorInput.getAllGroups()); result.setAllUsers(actorInput.getAllUsers()); result.setResourceOwners(actorInput.getResourceOwners()); + if (actorInput.getResourceOwnersTypesUrns() != null) { + result.setResourceOwnersTypesUrns(new UrnArray(actorInput.getResourceOwnersTypesUrns().stream().map(this::createUrn).collect(Collectors.toList()))); + } if (actorInput.getGroups() != null) { result.setGroups(new UrnArray(actorInput.getGroups().stream().map(this::createUrn).collect(Collectors.toList()))); } diff --git a/datahub-graphql-core/src/main/resources/entity.graphql b/datahub-graphql-core/src/main/resources/entity.graphql index bbd1c86201abf..06ad19efab21d 100644 --- a/datahub-graphql-core/src/main/resources/entity.graphql +++ b/datahub-graphql-core/src/main/resources/entity.graphql @@ -7965,9 +7965,9 @@ type ActorFilter { """ resourceOwners: Boolean! - resourceOwnersTypesUrns: [String] + resourceOwnersTypesUrns: [String!] - resolvedOwnershipTypes: [OwnershipTypeEntity] + resolvedOwnershipTypes: [OwnershipTypeEntity!] """ Whether the filter should apply to all users @@ -8097,6 +8097,8 @@ input ActorFilterInput { """ resourceOwners: Boolean! + resourceOwnersTypesUrns: [String!] + """ Whether the filter should apply to all users """ diff --git a/datahub-web-react/src/app/permissions/policy/ManagePolicies.tsx b/datahub-web-react/src/app/permissions/policy/ManagePolicies.tsx index 369e6a76cf7dd..25d5401bb9d61 100644 --- a/datahub-web-react/src/app/permissions/policy/ManagePolicies.tsx +++ b/datahub-web-react/src/app/permissions/policy/ManagePolicies.tsx @@ -109,6 +109,7 @@ const toPolicyInput = (policy: Omit): PolicyUpdateInput => { allUsers: policy.actors.allUsers, allGroups: policy.actors.allGroups, resourceOwners: policy.actors.resourceOwners, + resourceOwnersTypesUrns: policy.actors.resourceOwnersTypesUrns, }, }; if (policy.resources !== null && policy.resources !== undefined) { diff --git a/datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx b/datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx index 785d0226dfe19..61b3082f2279b 100644 --- a/datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx +++ b/datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx @@ -1,10 +1,12 @@ import React from 'react'; import { Form, Select, Switch, Tag, Typography } from 'antd'; import styled from 'styled-components'; +import { Maybe } from 'graphql/jsutils/Maybe'; import { useEntityRegistry } from '../../useEntityRegistry'; import { ActorFilter, CorpUser, EntityType, PolicyType, SearchResult } from '../../../types.generated'; import { useGetSearchResultsLazyQuery } from '../../../graphql/search.generated'; +import { useListOwnershipTypesQuery } from '../../../graphql/ownership.generated'; import { CustomAvatar } from '../../shared/avatar'; type Props = { @@ -36,6 +38,10 @@ const SearchResultContent = styled.div` align-items: center; `; +const OwnershipWrapper = styled.div` + margin-top: 12px; +`; + /** * Component used to construct the "actors" portion of a DataHub * access Policy by populating an ActorFilter object. @@ -46,12 +52,41 @@ export default function PolicyActorForm({ policyType, actors, setActors }: Props // Search for actors while building policy. const [userSearch, { data: userSearchData }] = useGetSearchResultsLazyQuery(); const [groupSearch, { data: groupSearchData }] = useGetSearchResultsLazyQuery(); - + const { data: ownershipData } = useListOwnershipTypesQuery({ + variables: { + input: {}, + }, + }); + const ownershipTypes = + ownershipData?.listOwnershipTypes?.ownershipTypes.filter((type) => type.urn !== 'urn:li:ownershipType:none') || + []; + const ownershipTypesMap = Object.fromEntries(ownershipTypes.map((type) => [type.urn, type.info?.name])); // Toggle the "Owners" switch const onToggleAppliesToOwners = (value: boolean) => { setActors({ ...actors, resourceOwners: value, + resourceOwnersTypesUrns: value ? actors.resourceOwnersTypesUrns : null, + }); + }; + + const onSelectOwnershipTypeActor = (newType: string) => { + console.log(`Called onSelect with ${newType}`); + const newResourceOwnersTypesUrns: Maybe = [...(actors.resourceOwnersTypesUrns || []), newType]; + console.log({ newResourceOwnersTypesUrns }); + setActors({ + ...actors, + resourceOwnersTypesUrns: newResourceOwnersTypesUrns, + }); + }; + + const onDeselectOwnershipTypeActor = (type: string) => { + const newResourceOwnersTypesUrns: Maybe = actors.resourceOwnersTypesUrns?.filter( + (u: string) => u !== type, + ); + setActors({ + ...actors, + resourceOwnersTypesUrns: newResourceOwnersTypesUrns?.length ? newResourceOwnersTypesUrns : null, }); }; @@ -175,6 +210,7 @@ export default function PolicyActorForm({ policyType, actors, setActors }: Props // Select dropdown values. const usersSelectValue = actors.allUsers ? ['All'] : actors.users || []; const groupsSelectValue = actors.allGroups ? ['All'] : actors.groups || []; + const ownershipTypesSelectValue = actors.resourceOwnersTypesUrns || []; const tagRender = (props) => { // eslint-disable-next-line react/prop-types @@ -215,6 +251,38 @@ export default function PolicyActorForm({ policyType, actors, setActors }: Props selected privileges. + {actors.resourceOwners && ( + + + List of types of ownership which will be used to match owners. If empty, the policies + will applied to any type of ownership. + + + + )} )} Users}> diff --git a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx index 886ff994fc092..df0a25284de35 100644 --- a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx +++ b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx @@ -110,7 +110,6 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege if ((actors?.resolvedOwnershipTypes?.length ?? 0) > 0) { return (
- Only to owners of types: {actors?.resolvedOwnershipTypes?.map((type) => ( {type.info.name} ))} diff --git a/datahub-web-react/src/graphql/policy.graphql b/datahub-web-react/src/graphql/policy.graphql index cad5464fc42f5..9c857b78c7b4e 100644 --- a/datahub-web-react/src/graphql/policy.graphql +++ b/datahub-web-react/src/graphql/policy.graphql @@ -34,6 +34,7 @@ query listPolicies($input: ListPoliciesInput!) { allUsers allGroups resourceOwners + resourceOwnersTypesUrns resolvedOwnershipTypes { urn info { From daa80f6f6631865225ef6950a48d43b9b878fbc3 Mon Sep 17 00:00:00 2001 From: skrydal Date: Wed, 7 Jun 2023 14:47:11 +0200 Subject: [PATCH 14/17] Reduced verbosity --- .../src/app/permissions/policy/PolicyActorForm.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx b/datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx index 61b3082f2279b..e8a29b4d96f8f 100644 --- a/datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx +++ b/datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx @@ -71,9 +71,7 @@ export default function PolicyActorForm({ policyType, actors, setActors }: Props }; const onSelectOwnershipTypeActor = (newType: string) => { - console.log(`Called onSelect with ${newType}`); const newResourceOwnersTypesUrns: Maybe = [...(actors.resourceOwnersTypesUrns || []), newType]; - console.log({ newResourceOwnersTypesUrns }); setActors({ ...actors, resourceOwnersTypesUrns: newResourceOwnersTypesUrns, @@ -264,7 +262,6 @@ export default function PolicyActorForm({ policyType, actors, setActors }: Props onSelect={(asset: any) => onSelectOwnershipTypeActor(asset)} onDeselect={(asset: any) => onDeselectOwnershipTypeActor(asset)} tagRender={(tagProps) => { - console.log({ tagProps }); return ( {ownershipTypesMap[tagProps.value.toString()]} @@ -273,7 +270,6 @@ export default function PolicyActorForm({ policyType, actors, setActors }: Props }} > {ownershipTypes.map((resOwnershipType) => { - console.log({ resOwnershipType }); return ( {resOwnershipType?.info?.name} From eed5d0537c38c10cd28e52918481a266b1180a98 Mon Sep 17 00:00:00 2001 From: skrydal Date: Wed, 21 Jun 2023 13:33:51 +0200 Subject: [PATCH 15/17] Applied review comments' suggestions --- .../linkedin/datahub/graphql/GmsGraphQLEngine.java | 2 +- .../policy/mappers/PolicyInfoPolicyMapper.java | 4 ++-- .../mappers/PolicyUpdateInputInfoMapper.java | 4 ++-- .../graphql/types/policy/DataHubPolicyMapper.java | 6 +++--- .../src/main/resources/entity.graphql | 13 +++++++++++-- .../src/app/permissions/policy/ManagePolicies.tsx | 2 +- .../src/app/permissions/policy/PolicyActorForm.tsx | 14 ++++++-------- datahub-web-react/src/graphql/policy.graphql | 2 +- .../com/linkedin/policy/DataHubActorFilter.pdl | 4 ++-- .../com/datahub/authorization/PolicyEngine.java | 14 +++++++------- .../com.linkedin.entity.entities.snapshot.json | 4 ++-- .../com.linkedin.platform.platform.snapshot.json | 4 ++-- 12 files changed, 40 insertions(+), 33 deletions(-) diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/GmsGraphQLEngine.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/GmsGraphQLEngine.java index 27d7aa4ffc4f4..4b939a7c110d4 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/GmsGraphQLEngine.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/GmsGraphQLEngine.java @@ -1624,7 +1624,7 @@ private void configurePolicyResolvers(final RuntimeWiring.Builder builder) { return filter.getRoles(); })).dataFetcher("resolvedOwnershipTypes", new LoadableTypeBatchResolver<>(ownershipType, (env) -> { final ActorFilter filter = env.getSource(); - return filter.getResourceOwnersTypesUrns(); + return filter.getResourceOwnersTypes(); }))); } diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java index 73d982a47563c..b9a6bf07be8c8 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyInfoPolicyMapper.java @@ -54,9 +54,9 @@ private ActorFilter mapActors(final DataHubActorFilter actorFilter) { result.setAllGroups(actorFilter.isAllGroups()); result.setAllUsers(actorFilter.isAllUsers()); result.setResourceOwners(actorFilter.isResourceOwners()); - UrnArray resourceOwnersTypes = actorFilter.getResourceOwnersTypesUrns(); + UrnArray resourceOwnersTypes = actorFilter.getResourceOwnersTypes(); if (resourceOwnersTypes != null) { - result.setResourceOwnersTypesUrns(resourceOwnersTypes.stream().map(Urn::toString).collect(Collectors.toList())); + result.setResourceOwnersTypes(resourceOwnersTypes.stream().map(Urn::toString).collect(Collectors.toList())); } if (actorFilter.hasGroups()) { result.setGroups(actorFilter.getGroups().stream().map(Urn::toString).collect(Collectors.toList())); diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyUpdateInputInfoMapper.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyUpdateInputInfoMapper.java index 430ec1b031b94..cb323b60dd465 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyUpdateInputInfoMapper.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/policy/mappers/PolicyUpdateInputInfoMapper.java @@ -51,8 +51,8 @@ private DataHubActorFilter mapActors(final ActorFilterInput actorInput) { result.setAllGroups(actorInput.getAllGroups()); result.setAllUsers(actorInput.getAllUsers()); result.setResourceOwners(actorInput.getResourceOwners()); - if (actorInput.getResourceOwnersTypesUrns() != null) { - result.setResourceOwnersTypesUrns(new UrnArray(actorInput.getResourceOwnersTypesUrns().stream().map(this::createUrn).collect(Collectors.toList()))); + if (actorInput.getResourceOwnersTypes() != null) { + result.setResourceOwnersTypes(new UrnArray(actorInput.getResourceOwnersTypes().stream().map(this::createUrn).collect(Collectors.toList()))); } if (actorInput.getGroups() != null) { result.setGroups(new UrnArray(actorInput.getGroups().stream().map(this::createUrn).collect(Collectors.toList()))); diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java index 34d91bfce474a..167e1615fc4cc 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/policy/DataHubPolicyMapper.java @@ -69,9 +69,9 @@ private ActorFilter mapActors(final DataHubActorFilter actorFilter) { result.setAllUsers(actorFilter.isAllUsers()); result.setResourceOwners(actorFilter.isResourceOwners()); // Change here is not executed at the moment - leaving it for the future - UrnArray resourceOwnersTypesUrns = actorFilter.getResourceOwnersTypesUrns(); - if (resourceOwnersTypesUrns != null) { - result.setResourceOwnersTypesUrns(resourceOwnersTypesUrns.stream().map(type -> type.toString()).collect(Collectors.toList())); + UrnArray resourceOwnersTypes = actorFilter.getResourceOwnersTypes(); + if (resourceOwnersTypes != null) { + result.setResourceOwnersTypes(resourceOwnersTypes.stream().map(Urn::toString).collect(Collectors.toList())); } if (actorFilter.hasGroups()) { result.setGroups(actorFilter.getGroups().stream().map(Urn::toString).collect(Collectors.toList())); diff --git a/datahub-graphql-core/src/main/resources/entity.graphql b/datahub-graphql-core/src/main/resources/entity.graphql index 06ad19efab21d..427e300a35212 100644 --- a/datahub-graphql-core/src/main/resources/entity.graphql +++ b/datahub-graphql-core/src/main/resources/entity.graphql @@ -7965,8 +7965,14 @@ type ActorFilter { """ resourceOwners: Boolean! - resourceOwnersTypesUrns: [String!] + """ + Set of OwnershipTypes to apply the policy to (if resourceOwners field is set to True) + """ + resourceOwnersTypes: [String!] + """ + Set of OwnershipTypes to apply the policy to (if resourceOwners field is set to True), resolved. + """ resolvedOwnershipTypes: [OwnershipTypeEntity!] """ @@ -8097,7 +8103,10 @@ input ActorFilterInput { """ resourceOwners: Boolean! - resourceOwnersTypesUrns: [String!] + """ + Set of OwnershipTypes to apply the policy to (if resourceOwners field is set to True) + """ + resourceOwnersTypes: [String!] """ Whether the filter should apply to all users diff --git a/datahub-web-react/src/app/permissions/policy/ManagePolicies.tsx b/datahub-web-react/src/app/permissions/policy/ManagePolicies.tsx index 25d5401bb9d61..08327d40a7165 100644 --- a/datahub-web-react/src/app/permissions/policy/ManagePolicies.tsx +++ b/datahub-web-react/src/app/permissions/policy/ManagePolicies.tsx @@ -109,7 +109,7 @@ const toPolicyInput = (policy: Omit): PolicyUpdateInput => { allUsers: policy.actors.allUsers, allGroups: policy.actors.allGroups, resourceOwners: policy.actors.resourceOwners, - resourceOwnersTypesUrns: policy.actors.resourceOwnersTypesUrns, + resourceOwnersTypes: policy.actors.resourceOwnersTypes, }, }; if (policy.resources !== null && policy.resources !== undefined) { diff --git a/datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx b/datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx index e8a29b4d96f8f..31b9472a7e53b 100644 --- a/datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx +++ b/datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx @@ -66,25 +66,23 @@ export default function PolicyActorForm({ policyType, actors, setActors }: Props setActors({ ...actors, resourceOwners: value, - resourceOwnersTypesUrns: value ? actors.resourceOwnersTypesUrns : null, + resourceOwnersTypes: value ? actors.resourceOwnersTypes : null, }); }; const onSelectOwnershipTypeActor = (newType: string) => { - const newResourceOwnersTypesUrns: Maybe = [...(actors.resourceOwnersTypesUrns || []), newType]; + const newResourceOwnersTypes: Maybe = [...(actors.resourceOwnersTypes || []), newType]; setActors({ ...actors, - resourceOwnersTypesUrns: newResourceOwnersTypesUrns, + resourceOwnersTypes: newResourceOwnersTypes, }); }; const onDeselectOwnershipTypeActor = (type: string) => { - const newResourceOwnersTypesUrns: Maybe = actors.resourceOwnersTypesUrns?.filter( - (u: string) => u !== type, - ); + const newResourceOwnersTypes: Maybe = actors.resourceOwnersTypes?.filter((u: string) => u !== type); setActors({ ...actors, - resourceOwnersTypesUrns: newResourceOwnersTypesUrns?.length ? newResourceOwnersTypesUrns : null, + resourceOwnersTypes: newResourceOwnersTypes?.length ? newResourceOwnersTypes : null, }); }; @@ -208,7 +206,7 @@ export default function PolicyActorForm({ policyType, actors, setActors }: Props // Select dropdown values. const usersSelectValue = actors.allUsers ? ['All'] : actors.users || []; const groupsSelectValue = actors.allGroups ? ['All'] : actors.groups || []; - const ownershipTypesSelectValue = actors.resourceOwnersTypesUrns || []; + const ownershipTypesSelectValue = actors.resourceOwnersTypes || []; const tagRender = (props) => { // eslint-disable-next-line react/prop-types diff --git a/datahub-web-react/src/graphql/policy.graphql b/datahub-web-react/src/graphql/policy.graphql index 9c857b78c7b4e..4ff25d5aa946b 100644 --- a/datahub-web-react/src/graphql/policy.graphql +++ b/datahub-web-react/src/graphql/policy.graphql @@ -34,7 +34,7 @@ query listPolicies($input: ListPoliciesInput!) { allUsers allGroups resourceOwners - resourceOwnersTypesUrns + resourceOwnersTypes resolvedOwnershipTypes { urn info { diff --git a/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl b/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl index c90fe72ca8349..5fcb0819d9f69 100644 --- a/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl +++ b/metadata-models/src/main/pegasus/com/linkedin/policy/DataHubActorFilter.pdl @@ -25,9 +25,9 @@ record DataHubActorFilter { resourceOwners: boolean = false /** - * Define type of ownership for policy + * Define type of ownership for the policy */ - resourceOwnersTypesUrns: optional array[Urn] + resourceOwnersTypes: optional array[Urn] /** * Whether the filter should apply to all users. 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 50459b700b74a..6a36fac7de4e0 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 @@ -312,11 +312,11 @@ private boolean isOwnerMatch( if (!actorFilter.isResourceOwners() || !requestResource.isPresent()) { return false; } - List ownershipTypesUrns = actorFilter.getResourceOwnersTypesUrns(); - return isActorOwner(actor, requestResource.get(), ownershipTypesUrns, context); + List ownershipTypes = actorFilter.getResourceOwnersTypes(); + return isActorOwner(actor, requestResource.get(), ownershipTypes, context); } - private Set getOwnersForType(ResourceSpec resourceSpec, List ownershipTypesUrns) { + private Set getOwnersForType(ResourceSpec resourceSpec, List ownershipTypes) { Urn entityUrn = UrnUtils.getUrn(resourceSpec.getResource()); EnvelopedAspect ownershipAspect; try { @@ -332,14 +332,14 @@ private Set getOwnersForType(ResourceSpec resourceSpec, List owners } Ownership ownership = new Ownership(ownershipAspect.getValue().data()); Stream ownersStream = ownership.getOwners().stream(); - if (ownershipTypesUrns != null) { - ownersStream = ownersStream.filter(owner -> ownershipTypesUrns.contains(owner.getTypeUrn())); + if (ownershipTypes != null) { + ownersStream = ownersStream.filter(owner -> ownershipTypes.contains(owner.getTypeUrn())); } return ownersStream.map(owner -> owner.getOwner().toString()).collect(Collectors.toSet()); } - private boolean isActorOwner(Urn actor, ResolvedResourceSpec resourceSpec, List ownershipTypesUrns, PolicyEvaluationContext context) { - Set owners = this.getOwnersForType(resourceSpec.getSpec(), ownershipTypesUrns); + private boolean isActorOwner(Urn actor, ResolvedResourceSpec resourceSpec, List ownershipTypes, PolicyEvaluationContext context) { + Set owners = this.getOwnersForType(resourceSpec.getSpec(), ownershipTypes); if (isUserOwner(actor, owners)) { return true; } diff --git a/metadata-service/restli-api/src/main/snapshot/com.linkedin.entity.entities.snapshot.json b/metadata-service/restli-api/src/main/snapshot/com.linkedin.entity.entities.snapshot.json index cf0edba87f344..ebc9e870af3e3 100644 --- a/metadata-service/restli-api/src/main/snapshot/com.linkedin.entity.entities.snapshot.json +++ b/metadata-service/restli-api/src/main/snapshot/com.linkedin.entity.entities.snapshot.json @@ -5251,12 +5251,12 @@ "doc" : "Whether the filter should return true for owners of a particular resource.\nOnly applies to policies of type 'Metadata', which have a resource associated with them.", "default" : false }, { - "name" : "resourceOwnersTypesUrns", + "name" : "resourceOwnersTypes", "type" : { "type" : "array", "items" : "com.linkedin.common.Urn" }, - "doc" : "Define type of ownership for policy", + "doc" : "Define type of ownership for the policy", "optional" : true }, { "name" : "allUsers", diff --git a/metadata-service/restli-api/src/main/snapshot/com.linkedin.platform.platform.snapshot.json b/metadata-service/restli-api/src/main/snapshot/com.linkedin.platform.platform.snapshot.json index e5210069df551..c34407325e165 100644 --- a/metadata-service/restli-api/src/main/snapshot/com.linkedin.platform.platform.snapshot.json +++ b/metadata-service/restli-api/src/main/snapshot/com.linkedin.platform.platform.snapshot.json @@ -5245,12 +5245,12 @@ "doc" : "Whether the filter should return true for owners of a particular resource.\nOnly applies to policies of type 'Metadata', which have a resource associated with them.", "default" : false }, { - "name" : "resourceOwnersTypesUrns", + "name" : "resourceOwnersTypes", "type" : { "type" : "array", "items" : "com.linkedin.common.Urn" }, - "doc" : "Define type of ownership for policy", + "doc" : "Define type of ownership for the policy", "optional" : true }, { "name" : "allUsers", From e221525de3df3bf85d19dc6af12b0e567c5e1feb Mon Sep 17 00:00:00 2001 From: skrydal Date: Wed, 21 Jun 2023 13:40:16 +0200 Subject: [PATCH 16/17] Fixing tests --- .../test/java/com/datahub/authorization/PolicyEngineTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 1b6500df44b96..99d8fee309d91 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 @@ -547,7 +547,7 @@ public void testEvaluatePolicyActorFilterUserResourceOwnersTypeMatch() throws Ex actorFilter.setResourceOwners(true); actorFilter.setAllUsers(false); actorFilter.setAllGroups(false); - actorFilter.setResourceOwnersTypesUrns(new UrnArray(ImmutableList.of(Urn.createFromString(OWNERSHIP_TYPE_URN)))); + actorFilter.setResourceOwnersTypes(new UrnArray(ImmutableList.of(Urn.createFromString(OWNERSHIP_TYPE_URN)))); dataHubPolicyInfo.setActors(actorFilter); final DataHubResourceFilter resourceFilter = new DataHubResourceFilter(); @@ -586,7 +586,7 @@ public void testEvaluatePolicyActorFilterUserResourceOwnersTypeNoMatch() throws actorFilter.setResourceOwners(true); actorFilter.setAllUsers(false); actorFilter.setAllGroups(false); - actorFilter.setResourceOwnersTypesUrns(new UrnArray(ImmutableList.of(Urn.createFromString(OWNERSHIP_TYPE_URN)))); + actorFilter.setResourceOwnersTypes(new UrnArray(ImmutableList.of(Urn.createFromString(OWNERSHIP_TYPE_URN)))); dataHubPolicyInfo.setActors(actorFilter); final DataHubResourceFilter resourceFilter = new DataHubResourceFilter(); From d77d110ff48a6c6d7d96b6843f855958d64a23e9 Mon Sep 17 00:00:00 2001 From: John Joyce Date: Wed, 28 Jun 2023 08:59:55 -0700 Subject: [PATCH 17/17] Update PolicyDetailsModal.tsx Change copy in the details modal --- .../src/app/permissions/policy/PolicyDetailsModal.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx index df0a25284de35..68e91983babdb 100644 --- a/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx +++ b/datahub-web-react/src/app/permissions/policy/PolicyDetailsModal.tsx @@ -105,7 +105,7 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege const resourceOwnersField = (actors) => { if (!actors?.resourceOwners) { - return Not applied; + return No; } if ((actors?.resolvedOwnershipTypes?.length ?? 0) > 0) { return ( @@ -116,7 +116,7 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege
); } - return Applies to all owners; + return Yes - All owners; }; return (