You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by mcgilman <gi...@git.apache.org> on 2018/03/06 15:02:59 UTC
[GitHub] nifi pull request #2515: NIFI-4885: Granular component restrictions
GitHub user mcgilman opened a pull request:
https://github.com/apache/nifi/pull/2515
NIFI-4885: Granular component restrictions
NIFI-4885:
- Introducing more granular restricted component access policies.
- Current restricted components have been updated to use new granular restrictions.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mcgilman/nifi NIFI-4885
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/2515.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2515
----
commit 0610789f4e75f3983d54f75cec79e0a2bd8b7991
Author: Matt Gilman <ma...@...>
Date: 2018-02-16T21:21:47Z
NIFI-4885:
- Introducing more granular restricted component access policies.
----
---
[GitHub] nifi issue #2515: NIFI-4885: Granular component restrictions
Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on the issue:
https://github.com/apache/nifi/pull/2515
@mcgilman this all looks good to me as well! Given positive review feedback from @andrewmlim and a +1 from @scottyaslan I am happy with the changes and think this is a great improvement on our security model. +1 merged to master. Thanks!
---
[GitHub] nifi issue #2515: NIFI-4885: Granular component restrictions
Posted by andrewmlim <gi...@git.apache.org>.
Github user andrewmlim commented on the issue:
https://github.com/apache/nifi/pull/2515
Also, maybe a separate issue, but wanted to note that the MoveHDFS processor is tagged as "restricted" in the Add Processor window, but it is not treated like other restricted components.
---
[GitHub] nifi pull request #2515: NIFI-4885: Granular component restrictions
Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2515#discussion_r173858341
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/coordination/http/endpoints/CurrentUserEndpointMerger.java ---
@@ -53,6 +54,23 @@ protected void mergeResponses(final CurrentUserEntity clientEntity, final Map<No
mergePermissions(clientEntity.getPoliciesPermissions(), entity.getPoliciesPermissions());
mergePermissions(clientEntity.getProvenancePermissions(), entity.getProvenancePermissions());
mergePermissions(clientEntity.getTenantsPermissions(), entity.getTenantsPermissions());
+ mergePermissions(clientEntity.getSystemPermissions(), entity.getSystemPermissions());
+ mergePermissions(clientEntity.getTenantsPermissions(), entity.getTenantsPermissions());
+
+ final Set<ComponentRestrictionPermissionDTO> clientEntityComponentRestrictionsPermissions = clientEntity.getComponentRestrictionPermissions();
+ final Set<ComponentRestrictionPermissionDTO> entityComponentRestrictionsPermissions = entity.getComponentRestrictionPermissions();
+
+ // only retain the component restriction permissions in common
+ clientEntityComponentRestrictionsPermissions.retainAll(entityComponentRestrictionsPermissions);
+
+ // merge the component restriction permissions
+ clientEntityComponentRestrictionsPermissions.forEach(clientEntityPermission -> {
+ final ComponentRestrictionPermissionDTO entityPermission = entityComponentRestrictionsPermissions.stream().filter(entityComponentRestrictionsPermission -> {
+ return entityComponentRestrictionsPermission.getRequiredPermission().getId().equals(clientEntityPermission.getRequiredPermission().getId());
+ }).findFirst().orElse(null);
--- End diff --
Are we guaranteed at this point that there will be at least one entry? If so, then we should probably just use findFirst().get() because it makes this more clear. If not, then we could end up with a null value here, and the next line would then throw a NPE.
---
[GitHub] nifi issue #2515: NIFI-4885: Granular component restrictions
Posted by andrewmlim <gi...@git.apache.org>.
Github user andrewmlim commented on the issue:
https://github.com/apache/nifi/pull/2515
@mcgilman Looks good! Thanks for including a fix for MoveHDFS.
---
[GitHub] nifi pull request #2515: NIFI-4885: Granular component restrictions
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2515#discussion_r173869295
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/coordination/http/endpoints/CurrentUserEndpointMerger.java ---
@@ -53,6 +54,23 @@ protected void mergeResponses(final CurrentUserEntity clientEntity, final Map<No
mergePermissions(clientEntity.getPoliciesPermissions(), entity.getPoliciesPermissions());
mergePermissions(clientEntity.getProvenancePermissions(), entity.getProvenancePermissions());
mergePermissions(clientEntity.getTenantsPermissions(), entity.getTenantsPermissions());
+ mergePermissions(clientEntity.getSystemPermissions(), entity.getSystemPermissions());
+ mergePermissions(clientEntity.getTenantsPermissions(), entity.getTenantsPermissions());
+
+ final Set<ComponentRestrictionPermissionDTO> clientEntityComponentRestrictionsPermissions = clientEntity.getComponentRestrictionPermissions();
+ final Set<ComponentRestrictionPermissionDTO> entityComponentRestrictionsPermissions = entity.getComponentRestrictionPermissions();
+
+ // only retain the component restriction permissions in common
+ clientEntityComponentRestrictionsPermissions.retainAll(entityComponentRestrictionsPermissions);
+
+ // merge the component restriction permissions
+ clientEntityComponentRestrictionsPermissions.forEach(clientEntityPermission -> {
+ final ComponentRestrictionPermissionDTO entityPermission = entityComponentRestrictionsPermissions.stream().filter(entityComponentRestrictionsPermission -> {
+ return entityComponentRestrictionsPermission.getRequiredPermission().getId().equals(clientEntityPermission.getRequiredPermission().getId());
+ }).findFirst().orElse(null);
--- End diff --
Because we're doing a retainAll right before this we know that both collections will each have an entry for the current clientEntityPermission. I will update to use get() instead.
---
[GitHub] nifi pull request #2515: NIFI-4885: Granular component restrictions
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/2515
---
[GitHub] nifi issue #2515: NIFI-4885: Granular component restrictions
Posted by andrewmlim <gi...@git.apache.org>.
Github user andrewmlim commented on the issue:
https://github.com/apache/nifi/pull/2515
After deleting a specific granular policy, the following message appears:
Showing effective policy inherited from Process Group restricted-components. Override this policy.
The text "restricted-components" in the message is hyperlinked. If clicked you get a dialog that says:
Unable to locate group with id 'restricted-components'.
Unable to enter the selected group.
---
[GitHub] nifi issue #2515: NIFI-4885: Granular component restrictions
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/2515
Thanks for the feedback @andrewmlim! I've pushed another commit addressing the issue.
---
[GitHub] nifi issue #2515: NIFI-4885: Granular component restrictions
Posted by andrewmlim <gi...@git.apache.org>.
Github user andrewmlim commented on the issue:
https://github.com/apache/nifi/pull/2515
@mcgilman
I tested your latest changes and they look good. A couple minor things to address:
- In the message "Only listing restriction specific users. Users with permission "regardless of restriction" not shown but are also allowed." should add an "s" to the second instance of "restriction". So the correct message reads:
Only listing restriction specific users. Users with permission "regardless of restrictions" not shown but are also allowed.
- The following text is used in a tooltip and doc:
"Allows users to create/modify restricted components assuming otherwise sufficient permissions”
I thought this might be more clear:
“Allows users to create/modify restricted components assuming other permissions are sufficient”
-In the Admin and User Guides, change the content to:
Allows users to create/modify restricted components assuming other permissions are sufficient. The restricted components may indicate which specific permissions are required. Permissions can be granted for specific restrictions or be granted regardless of restrictions. If permission is granted regardless of restrictions, the user can create/modify all restricted components.
---
[GitHub] nifi issue #2515: NIFI-4885: Granular component restrictions
Posted by scottyaslan <gi...@git.apache.org>.
Github user scottyaslan commented on the issue:
https://github.com/apache/nifi/pull/2515
Thanks @mcgilman! I am a +1 for the UI code.
---