-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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) - New privilege for Associate entities for entities tags and glossary terms #8644
base: master
Are you sure you want to change the base?
Changes from 13 commits
c923b8b
f60a413
0da4478
5067773
0ee58b0
e03e2af
67a991f
df12d4a
e5216d4
7f002d5
c35550a
2750e54
15d56b1
80efa94
c60d9ab
bf81623
193a093
74074fb
f37a807
fae9dd0
4c1aa72
f703c95
8b43d10
49977c1
b235f0d
e98fb1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,9 @@ public CompletableFuture<Boolean> get(DataFetchingEnvironment environment) throw | |
return CompletableFuture.supplyAsync(() -> { | ||
|
||
// First, validate the batch | ||
validateTags(tagUrns, context); | ||
|
||
// Next, validate the batch | ||
validateInputResources(resources, context); | ||
|
||
try { | ||
|
@@ -50,6 +53,14 @@ public CompletableFuture<Boolean> get(DataFetchingEnvironment environment) throw | |
}); | ||
} | ||
|
||
private void validateTags(List<Urn> tagUrns, QueryContext context) { | ||
for (Urn tagUrn : tagUrns) { | ||
if (!LabelUtils.isAuthorizedToAssociateTag(context, tagUrn)) { | ||
throw new AuthorizationException("Only users granted permission to this tag can assign or remove it"); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. now in all these resolvers we'd be checking they have the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. EDIT_ENTITY_TAGS_PRIVILEGE is a privilege for the entity to which a list of tags are associated. ASSOCIATE_TAGS_PRIVILEGE is for the list of tags that are being associated. That is the reason I have created 2 different methods. |
||
} | ||
} | ||
|
||
private void validateInputResources(List<ResourceRefInput> resources, QueryContext context) { | ||
for (ResourceRefInput resource : resources) { | ||
validateInputResource(resource, context); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -201,6 +201,21 @@ public static boolean isAuthorizedToUpdateTags(@Nonnull QueryContext context, Ur | |
orPrivilegeGroups); | ||
} | ||
|
||
public static boolean isAuthorizedToAssociateTag(@Nonnull QueryContext context, Urn targetUrn) { | ||
|
||
// Decide whether the current principal should be allowed to associate the tag | ||
final DisjunctivePrivilegeGroup orPrivilegeGroups = new DisjunctivePrivilegeGroup(ImmutableList.of( | ||
new ConjunctivePrivilegeGroup(ImmutableList.of(PoliciesConfig.ASSOCIATE_TAGS_PRIVILEGE.getType())) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm I'm a little confused here - so what's the benefit of this new privilege when we have the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Please check my response to your other comment on the need for this new privilege |
||
)); | ||
|
||
return AuthorizationUtils.isAuthorized( | ||
context.getAuthorizer(), | ||
context.getActorUrn(), | ||
targetUrn.getEntityType(), | ||
targetUrn.toString(), | ||
orPrivilegeGroups); | ||
} | ||
|
||
public static boolean isAuthorizedToUpdateTerms(@Nonnull QueryContext context, Urn targetUrn, String subResource) { | ||
|
||
Boolean isTargetingSchema = subResource != null && subResource.length() > 0; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -97,6 +97,12 @@ function getGraphqlErrorCode(e) { | |
|
||
export const handleBatchError = (urns, e, defaultMessage) => { | ||
if (urns.length > 1 && getGraphqlErrorCode(e) === 403) { | ||
if (e.message === 'Only users granted permission to this tag can assign or remove it') { | ||
return { | ||
content: `${e.message}. The bulk edit being performed will not be saved.`, | ||
duration: 3, | ||
}; | ||
} | ||
Comment on lines
+100
to
+105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need this new message when we have a message about being unauthorized below? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new error message is for clarity to pinpoint which authorization is missing. |
||
return { | ||
content: | ||
'Your bulk edit selection included entities that you are unauthorized to update. The bulk edit being performed will not be saved.', | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Auth calls should come first before sending validation operations to the entity service, ideally done as soon as possible before any business logic occurs to short circuit.