Skip to content
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.

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

feat(auth): Fine grained ownership policies #7499

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
e0ad49f
Initial approach to fine-grained resource owners handling. Types are
skrydal Mar 1, 2023
ed670a0
Adjusting logging message
skrydal Mar 4, 2023
62dc5c3
Adjusting existing tests
skrydal Mar 4, 2023
65b3a87
Merge branch 'datahub-project:master' into fine_grained_ownership_pol…
skrydal Mar 4, 2023
9c2657e
Created tests for new use-cases
skrydal Mar 4, 2023
ebce9b6
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 6, 2023
7d02aae
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 7, 2023
0940909
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 8, 2023
ea10c6e
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 8, 2023
755ac7f
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 9, 2023
feaba4f
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 10, 2023
3918a2a
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 11, 2023
8bcd8e0
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 13, 2023
cc2be41
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 13, 2023
362578b
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 14, 2023
dc5d719
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 14, 2023
f9a4e9c
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 15, 2023
35f7c62
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 23, 2023
330d919
Merge branch 'master' into fine_grained_ownership_policies
skrydal Mar 31, 2023
4cb5d84
Merge branch 'master' into fine_grained_ownership_policies
skrydal Apr 4, 2023
a1a2f3a
Merge branch 'master' into fine_grained_ownership_policies
skrydal Apr 14, 2023
7442aae
Merge branch 'master' into fine_grained_ownership_policies
skrydal May 12, 2023
1606138
Merge branch 'datahub-project:master' into fine_grained_ownership_pol…
skrydal May 30, 2023
4f20bb1
Adjusting to new approach
skrydal Jun 1, 2023
002e774
Adjusting UI
skrydal Jun 1, 2023
9e01299
Better display of the resource owners
skrydal Jun 3, 2023
7641023
Merge branch 'datahub-project:master' into fine_grained_ownership_pol…
skrydal Jun 3, 2023
d57319a
Added automatic resolution of OwnershipType name to be used in UI
skrydal Jun 5, 2023
43e1abb
Fixing style
skrydal Jun 5, 2023
4088fb1
Fixing tests
skrydal Jun 5, 2023
ba949fe
Fixing style
skrydal Jun 5, 2023
74364d0
Fixing style
skrydal Jun 5, 2023
cc8517c
Merge branch 'master' into fine_grained_ownership_policies
skrydal Jun 5, 2023
1560bc0
Merge branch 'master' into fine_grained_ownership_policies
skrydal Jun 5, 2023
b40b885
Added capability of updating policies via UI
skrydal Jun 6, 2023
df952c1
Merge branch 'master' into fine_grained_ownership_policies
skrydal Jun 7, 2023
daa80f6
Reduced verbosity
skrydal Jun 7, 2023
319b17e
Merge branch 'master' into fine_grained_ownership_policies
skrydal Jun 7, 2023
30f1a44
Merge branch 'master' into fine_grained_ownership_policies
skrydal Jun 8, 2023
dcaa6c3
Merge branch 'master' into fine_grained_ownership_policies
skrydal Jun 13, 2023
85a97c1
Merge branch 'master' into fine_grained_ownership_policies
skrydal Jun 21, 2023
eed5d05
Applied review comments' suggestions
skrydal Jun 21, 2023
e221525
Fixing tests
skrydal Jun 21, 2023
3077809
Merge branch 'master' into fine_grained_ownership_policies
skrydal Jun 25, 2023
d77d110
Update PolicyDetailsModal.tsx
jjoyce0510 Jun 28, 2023
3d8cf6a
Merge branch 'master' into fine_grained_ownership_policies
skrydal Jun 28, 2023
bbe4457
Merge branch 'master' into fine_grained_ownership_policies
skrydal Jun 29, 2023
9b780fa
Merge branch 'master' into fine_grained_ownership_policies
jjoyce0510 Jun 30, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -1634,6 +1634,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.getResourceOwnersTypes();
})));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
package com.linkedin.datahub.graphql.resolvers.policy.mappers;

import com.linkedin.common.UrnArray;
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.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;
Expand Down Expand Up @@ -53,6 +54,10 @@ private ActorFilter mapActors(final DataHubActorFilter actorFilter) {
result.setAllGroups(actorFilter.isAllGroups());
result.setAllUsers(actorFilter.isAllUsers());
result.setResourceOwners(actorFilter.isResourceOwners());
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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ private DataHubActorFilter mapActors(final ActorFilterInput actorInput) {
result.setAllGroups(actorInput.getAllGroups());
result.setAllUsers(actorInput.getAllUsers());
result.setResourceOwners(actorInput.getResourceOwners());
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())));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.linkedin.datahub.graphql.types.policy;

import com.linkedin.common.UrnArray;
import com.linkedin.common.urn.Urn;
import com.linkedin.data.DataMap;
import com.linkedin.datahub.graphql.generated.ActorFilter;
Expand Down Expand Up @@ -67,6 +68,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
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()));
}
Expand Down
15 changes: 15 additions & 0 deletions datahub-graphql-core/src/main/resources/entity.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -8007,6 +8007,16 @@ type ActorFilter {
"""
resourceOwners: Boolean!

"""
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!]

"""
Whether the filter should apply to all users
"""
Expand Down Expand Up @@ -8135,6 +8145,11 @@ input ActorFilterInput {
"""
resourceOwners: Boolean!

"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for adding these!

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
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ const toPolicyInput = (policy: Omit<Policy, 'urn'>): PolicyUpdateInput => {
allUsers: policy.actors.allUsers,
allGroups: policy.actors.allGroups,
resourceOwners: policy.actors.resourceOwners,
resourceOwnersTypes: policy.actors.resourceOwnersTypes,
},
};
if (policy.resources !== null && policy.resources !== undefined) {
Expand Down
64 changes: 63 additions & 1 deletion datahub-web-react/src/app/permissions/policy/PolicyActorForm.tsx
Original file line number Diff line number Diff line change
@@ -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 = {
Expand Down Expand Up @@ -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.
Expand All @@ -46,12 +52,37 @@ 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,
resourceOwnersTypes: value ? actors.resourceOwnersTypes : null,
});
};

const onSelectOwnershipTypeActor = (newType: string) => {
const newResourceOwnersTypes: Maybe<string[]> = [...(actors.resourceOwnersTypes || []), newType];
setActors({
...actors,
resourceOwnersTypes: newResourceOwnersTypes,
});
};

const onDeselectOwnershipTypeActor = (type: string) => {
const newResourceOwnersTypes: Maybe<string[]> = actors.resourceOwnersTypes?.filter((u: string) => u !== type);
setActors({
...actors,
resourceOwnersTypes: newResourceOwnersTypes?.length ? newResourceOwnersTypes : null,
});
};

Expand Down Expand Up @@ -175,6 +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.resourceOwnersTypes || [];

const tagRender = (props) => {
// eslint-disable-next-line react/prop-types
Expand Down Expand Up @@ -215,6 +247,36 @@ export default function PolicyActorForm({ policyType, actors, setActors }: Props
selected privileges.
</Typography.Paragraph>
<Switch size="small" checked={actors.resourceOwners} onChange={onToggleAppliesToOwners} />
{actors.resourceOwners && (
<OwnershipWrapper>
<Typography.Paragraph>
List of types of ownership which will be used to match owners. If empty, the policies
will applied to any type of ownership.
</Typography.Paragraph>
<Select
value={ownershipTypesSelectValue}
mode="multiple"
placeholder="Ownership types"
onSelect={(asset: any) => onSelectOwnershipTypeActor(asset)}
onDeselect={(asset: any) => onDeselectOwnershipTypeActor(asset)}
tagRender={(tagProps) => {
return (
<Tag closable={tagProps.closable} onClose={tagProps.onClose}>
{ownershipTypesMap[tagProps.value.toString()]}
</Tag>
);
}}
>
{ownershipTypes.map((resOwnershipType) => {
return (
<Select.Option value={resOwnershipType.urn}>
{resOwnershipType?.info?.name}
</Select.Option>
);
})}
</Select>
</OwnershipWrapper>
)}
</Form.Item>
)}
<Form.Item label={<Typography.Text strong>Users</Typography.Text>}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,22 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege
);
};

const resourceOwnersField = (actors) => {
if (!actors?.resourceOwners) {
return <PoliciesTag>No</PoliciesTag>;
}
if ((actors?.resolvedOwnershipTypes?.length ?? 0) > 0) {
return (
<div>
{actors?.resolvedOwnershipTypes?.map((type) => (
<PoliciesTag>{type.info.name}</PoliciesTag>
))}
</div>
);
}
return <PoliciesTag>Yes - All owners</PoliciesTag>;
};

return (
<Modal title={policy?.name} visible={visible} onCancel={onClose} closable width={800} footer={actionButtons}>
<PolicyContainer>
Expand Down Expand Up @@ -180,7 +196,7 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege
<div>
<Typography.Title level={5}>Applies to Owners</Typography.Title>
<ThinDivider />
<PoliciesTag>{policy?.actors?.resourceOwners ? 'True' : 'False'}</PoliciesTag>
{resourceOwnersField(policy?.actors)}
</div>
<div>
<Typography.Title level={5}>Applies to Users</Typography.Title>
Expand Down
7 changes: 7 additions & 0 deletions datahub-web-react/src/graphql/policy.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ query listPolicies($input: ListPoliciesInput!) {
allUsers
allGroups
resourceOwners
resourceOwnersTypes
resolvedOwnershipTypes {
urn
info {
name
}
}
resolvedUsers {
username
urn
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
namespace com.linkedin.policy

import com.linkedin.common.Urn
import com.linkedin.ownership.OwnershipTypeInfo

/**
* Information used to filter DataHub actors.
Expand All @@ -23,6 +24,11 @@ record DataHubActorFilter {
*/
resourceOwners: boolean = false

/**
* Define type of ownership for the policy
*/
resourceOwnersTypes: optional array[Urn]

/**
* Whether the filter should apply to all users.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,19 @@

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.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;
Expand All @@ -28,6 +32,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -307,11 +312,34 @@ private boolean isOwnerMatch(
if (!actorFilter.isResourceOwners() || !requestResource.isPresent()) {
return false;
}
return isActorOwner(actor, requestResource.get(), context);
List<Urn> ownershipTypes = actorFilter.getResourceOwnersTypes();
return isActorOwner(actor, requestResource.get(), ownershipTypes, context);
}

private boolean isActorOwner(Urn actor, ResolvedResourceSpec resourceSpec, PolicyEvaluationContext context) {
Set<String> owners = resourceSpec.getOwners();
private Set<String> getOwnersForType(ResourceSpec resourceSpec, List<Urn> ownershipTypes) {
Urn entityUrn = UrnUtils.getUrn(resourceSpec.getResource());
EnvelopedAspect ownershipAspect;
try {
EntityResponse response = _entityClient.getV2(entityUrn.getEntityType(), entityUrn,
Copy link
Collaborator

Choose a reason for hiding this comment

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

The final comment from me - I'd really prefer to not see this logic in the Policy Engine. This can be abstracted away to the ResolvedResourceSpec and handled internally.

The policy engine should simply ask for the ownership types for the resouce and get it back without knowing how that happens

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 ownership aspect for urn {}", entityUrn, e);
return Collections.emptySet();
}
Ownership ownership = new Ownership(ownershipAspect.getValue().data());
Stream<Owner> ownersStream = ownership.getOwners().stream();
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<Urn> ownershipTypes, PolicyEvaluationContext context) {
Set<String> owners = this.getOwnersForType(resourceSpec.getSpec(), ownershipTypes);
if (isUserOwner(actor, owners)) {
return true;
}
Expand Down
Loading
Loading