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 2016/07/21 01:43:42 UTC
[GitHub] nifi pull request #698: Updating the UI to reflect the access policies being...
GitHub user mcgilman opened a pull request:
https://github.com/apache/nifi/pull/698
Updating the UI to reflect the access policies being enforced
NIFI-2244:
- Separating the access control check and the supports modification check for the selected component.
- Using separated access control check to support editing when possible without write permission.
NIFI-2307:
- Adding support for opening Connection configuration and Connection details dialog when source/destination permissions are not allowed.
- Enforcing connection permissions based on the source and destination comonent.
- Removing connection specific access policies.
NIFI-2265:
- Filtering out sensitive details in component status and status history when appropriate.
NIFI-1800:
- Adding parent process group id to the Controller Services table.
NIFI-2077:
- Removing some old un-used icons following the UI refresh.
NIFI-2242:
- Requiring write permissions for all components in a selection.
NIFI-2080:
- Updating style of the name in the selection context to handle scroll bars and use available width.
NIFI-2331:
- Addressing issue when removing a user/group which was causing the tenant policy to be removed.
NIFI-2335:
- Ensuring the flow is saved after starting/stopping a process group.
NIFI-2235:
- Ensuring we use consistent conditions between the context menu and the operate palette.
- Allowing users with read only access to the tenants page.
- Fixing current user integration test.
- Ensuring schedule methods are locked appropriately.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mcgilman/nifi NIFI-2244
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/698.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 #698
----
commit f62f48efe5f0e68f6c6f439565a31c9ae4e708b7
Author: Matt Gilman <ma...@gmail.com>
Date: 2016-07-21T01:41:06Z
NIFI-2244:
- Separating the access control check and the supports modification check for the selected component.
- Using separated access control check to support editing when possible without write permission.
NIFI-2307:
- Adding support for opening Connection configuration and Connection details dialog when source/destination permissions are not allowed.
- Enforcing connection permissions based on the source and destination comonent.
- Removing connection specific access policies.
NIFI-2265:
- Filtering out sensitive details in component status and status history when appropriate.
NIFI-1800:
- Adding parent process group id to the Controller Services table.
NIFI-2077:
- Removing some old un-used icons following the UI refresh.
NIFI-2242:
- Requiring write permissions for all components in a selection.
NIFI-2080:
- Updating style of the name in the selection context to handle scroll bars and use available width.
NIFI-2331:
- Addressing issue when removing a user/group which was causing the tenant policy to be removed.
NIFI-2335:
- Ensuring the flow is saved after starting/stopping a process group.
NIFI-2235:
- Ensuring we use consistent conditions between the context menu and the operate palette.
- Allowing users with read only access to the tenants page.
- Fixing current user integration test.
- Ensuring schedule methods are locked appropriately.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/698
Yeah, I'm unable to replicate. The first message you mentioned sounds like the request was attempted like it was prior to the fix. Maybe your browser had some old JS cached. The "Must be authorized..." message is a client side check that I added to address NIFI-2242. Let me know if you still seeing any weirdness.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #698: Updating the UI to reflect the access policies being...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on a diff in the pull request:
https://github.com/apache/nifi/pull/698#discussion_r71795919
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/ConnectionAuthorizable.java ---
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.authorization;
+
+import org.apache.nifi.authorization.resource.Authorizable;
+import org.apache.nifi.connectable.Connectable;
+import org.apache.nifi.groups.ProcessGroup;
+
+/**
+ * Authorizable for policies of an Authorizable.
--- End diff --
I think this should be "Authorizable for policies of a Connection."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/698
It's not the same. Moving a connection would move it's bend points. Moving the source does not affect the connection. The start and end points are dynamic and based on the position of the source and destination.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
I don't think NIFI-2235 was addressed. I am still able to reproduce
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #698: Updating the UI to reflect the access policies being...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:
https://github.com/apache/nifi/pull/698#discussion_r71818711
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/FlowController.java ---
@@ -2147,9 +2147,15 @@ public ProcessGroupStatus getGroupStatus(final ProcessGroup group, final Reposit
return null;
}
+ // if the user is null, this is not a web request so we need to treat as authorized - this is the case when
+ // gather status for history in a background thread... we then will check the access of the user when pulling
+ // back history...
+ final NiFiUser user = NiFiUserUtils.getNiFiUser();
+ final boolean isAuthorized = user == null ? true : group.isAuthorized(authorizer, RequestAction.READ, user);
--- End diff --
When handling a web request, the user will always be available. When this method is running outside the scope of a web request, there is nothing to attempt to authorize (nor should we). This conditional authorization seems appropriate to me. However, I think we can clean up the code to make the distinction a little more clear and deliberate.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/698
Good catch. Will update PR shortly. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/698
NIFI-2242 was when there are multiple components selected with mixed permissions so I think this may be a slightly different case but I'd be happy to address as part of this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
Doesn't seem to address NIFI-2242. I have write permission to GenerateFF but not LogAttribute. This UI mess up fixed itself after refresh.
<img width="770" alt="screen shot 2016-07-21 at 12 38 31 pm" src="https://cloud.githubusercontent.com/assets/11302527/17031083/768b0cb6-4f40-11e6-951b-d4a368533593.png">
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
Created a new ticket for an issue I saw:
https://issues.apache.org/jira/browse/NIFI-2347
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
+1
Visually verified code and comments were addressed. Did a contrib check build and on a 3 node secure cluster ran through each ticket and did many different iterations of various paths of logic. Thanks Matt @mcgilman, I will merge it in.
I will be creating a ticket for the controller service/process group Id because there is another problem with that panel (without W permissions the process group details don't show up).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
Actually I think that was another bug relating to inconsistent cluster state. I just retried and got a pop-up saying "Must be authorized to modify every component selected."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
I understand why, just seems odd though because the action "moving GenerateFF" vs "having GenerateFF and connection selected, attempt to move them" has the same end result.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
Created NIFI-2358 for the issues I found with PG config panel.
https://issues.apache.org/jira/browse/NIFI-2358
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
One thing I did notice is when I have selected just GenerateFF I can move it (UI properly moves relationship too) but if I have GenerateFF and connection selected it fails (message "Must be authorized to modify every component selected."). I believe this is because it is checking if I have permission to move the connection which in turn checks if I have permission to modify the destination (which I don't).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
I don't see how this is different, maybe I mis-explained it the first time:
LogAttribute I don't have write access
GenerateFF I do
I select all three (including conneciton) and attempt to move together
Get pop-up saying i can't modify LogAttribute
GenerateFF moves and UI looks like that
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
Github is taking a long time to replicate but here is a link to the commit in apache (I pushed it, I swear, lol):
https://git-wip-us.apache.org/repos/asf?p=nifi.git;a=commit;h=4a4d60e6af5014342039f751c7b058079d643f7b
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
As part of NIFI-1800, when viewing the config for a process group it should have the UUID at the top. This way at the very least I can tell which controller services belong to this ProcessGroup without trying to remember the exact UUID (no tab in that panel shows that information).
Also it may be best to have the name in that column instead of UUID.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #698: Updating the UI to reflect the access policies being...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/698
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
Relating to NIFI-2265. I have a process group that I do not have view permissions on. I have processors in that group that inherit the policy from the group. When I view the flow on the canvas or by clicking "stats" I properly don't the name but when I view "summary" (from the top menu) I see the names.
Tracking it down in the developer tools, it looks like it comes from the ProcessGroupStatusSnapshots (I don't have view access to that group either).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #698: Updating the UI to reflect the access policies being...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on a diff in the pull request:
https://github.com/apache/nifi/pull/698#discussion_r71795555
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/FlowController.java ---
@@ -2147,9 +2147,15 @@ public ProcessGroupStatus getGroupStatus(final ProcessGroup group, final Reposit
return null;
}
+ // if the user is null, this is not a web request so we need to treat as authorized - this is the case when
+ // gather status for history in a background thread... we then will check the access of the user when pulling
+ // back history...
+ final NiFiUser user = NiFiUserUtils.getNiFiUser();
+ final boolean isAuthorized = user == null ? true : group.isAuthorized(authorizer, RequestAction.READ, user);
--- End diff --
This seems like a very insecure way to do things (if null then authorized).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
Similar logic is possible when going from the sub group to root group.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
Reviewing
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/698
That is correct. Permissions of the connection are driven by the permissions of the source and destination.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #698: Updating the UI to reflect the access policies being enforc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/698
NIFI-2235 offending logic path:
copy processor in root PG
go into PG I do not have modify access
Context menu does not have paste, palette menu allows it to be clicked
after i click paste (and get no access) it correctly refreshes to show no ability to paste
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---