NIFI-1781: UI authorization updates #461
Conversation
| /** | ||
| * Predicate for filtering schedulable Ports | ||
| */ | ||
| Predicate<Port> SCHEDULABLE_PORTS = port -> port.getScheduledState() != ScheduledState.DISABLED; |
There was a problem hiding this comment.
It seems odd to me that we are checking "!node.isRunning()" for ProcessorsNode but not checking "!port.isRunning()" for ports.
There was a problem hiding this comment.
I did not modify any logic here. Simply relocated to not repeat. If that logic is incorrect, I can certainly update it, but it hasn't changed here.
|
|
||
| // ensure authorized for each processor we will attempt to schedule | ||
| group.findAllProcessors().stream().filter(ScheduledState.RUNNING.equals(state) ? ProcessGroup.SCHEDULABLE_PROCESSORS : ProcessGroup.UNSCHEDULABLE_PROCESSORS).forEach(processor -> { | ||
| if (processor.isAuthorized(authorizer, RequestAction.WRITE)) { |
There was a problem hiding this comment.
It seems cleaner to me to use a filter here to check if the processor is authorized, rather than mixing stream filters with a forEach and then performing additional if-checks. I.e., instead of "if (processor.isAuthorized...) {...}" we could just do ".filter(processor -> processor.isAuthorized(...).forEach(...)" for processors, input ports & output ports
| // ensure read permission to every component in the snippet | ||
| final Snippet snippet = lookup.getSnippet(snippetId); | ||
| final Set<Authorizable> authorizables = new HashSet<>(); | ||
| authorizables.addAll(snippet.getProcessGroups().keySet().stream().map(id -> lookup.getProcessGroup(id)).collect(Collectors.toSet())); |
There was a problem hiding this comment.
It probably makes sense here to change this to:
snippet.getProcessGroups().keySet().stream().map(id -> lookup.getProcessGroup(id)).forEach(auth -> authorizables.add(auth));
This way we don't have to keep creating all of these intermediate Set objects just so that we can add them to the 'authorizables' set.
There was a problem hiding this comment.
Or better yet, to get rid of the 'authorizables' Set all together and define the final authorization lambda as an object and just call it on each of these streams. E.g.,
final Consumer<Authorizable> auth = authorizable -> authorizable.authorize(authorizer, RequestAciton.READ);
snippet.getProcessGroups().keySet().stream().map(id -> lookup.getProcessGroup(id)).forEach(auth);
snippet.getProcessors().keySet().stream().map(id -> lookup.getProcessor(id)).forEach(auth);
|
@mcgilman looks good for the most part. I left some comments in-line (most of which you've already addressed before I finished the review even!) Excellent. Once the last few things are merged in I'm a +1. Thanks for knocking this out. |
- Including access policies in the breadcrumb's trail. - Updating toolbox according to group access policies. - Updating actions in palette based on selection access policies. NIFI-1554: - Introducing authorization during two phase commit. - Introducing snippet authorization according to the encapsulated components and the action performed.
|
Thanks @markap14 All changes have been applied. |
- Including access policies in the breadcrumb's trail. - Updating toolbox according to group access policies. - Updating actions in palette based on selection access policies. NIFI-1554: - Introducing authorization during two phase commit. - Introducing snippet authorization according to the encapsulated components and the action performed. - This closes apache#461
NIFI-1781:
NIFI-1554: