From b1c9f0e76428d69e7ee39448ef182ad0a23327ec Mon Sep 17 00:00:00 2001 From: Matt Gilman Date: Thu, 8 Dec 2016 11:29:32 -0500 Subject: [PATCH] NIFI-2695: - Providing more granular and meaningful authorization error messages. This closes #1309. Signed-off-by: Bryan Bende --- .../AbstractPolicyBasedAuthorizer.java | 3 +- .../authorization/AuthorizationRequest.java | 31 ++++ .../authorization/AuthorizationResult.java | 8 +- .../apache/nifi/authorization/Resource.java | 7 + .../authorization/resource/Authorizable.java | 103 ++++++++++-- .../TestAbstractPolicyBasedAuthorizer.java | 6 +- .../nifi/authorization/FileAuthorizer.java | 8 +- .../authorization/FileAuthorizerTest.java | 12 +- .../resource/AccessPolicyAuthorizable.java | 4 +- .../resource/DataAuthorizable.java | 4 +- .../resource/ResourceFactory.java | 153 ++++++++++++++---- .../nifi/connectable/StandardConnection.java | 9 +- .../nifi/web/StandardNiFiServiceFacade.java | 36 ++++- .../StandardNiFiWebConfigurationContext.java | 4 +- .../apache/nifi/web/api/AccessResource.java | 4 +- .../nifi/web/api/ControllerResource.java | 15 +- .../apache/nifi/web/api/CountersResource.java | 15 +- .../org/apache/nifi/web/api/FlowResource.java | 4 +- .../nifi/web/api/ProvenanceResource.java | 4 +- .../apache/nifi/web/api/ResourceResource.java | 4 +- .../nifi/web/api/SiteToSiteResource.java | 4 +- .../web/api/SystemDiagnosticsResource.java | 4 +- .../config/AccessDeniedExceptionMapper.java | 7 +- .../nifi/web/controller/ControllerFacade.java | 4 +- .../src/main/webapp/js/nf/nf-common.js | 11 +- .../PersistentProvenanceRepository.java | 8 +- .../VolatileProvenanceRepository.java | 8 +- .../authorization/RangerNiFiAuthorizer.java | 10 +- .../TestRangerNiFiAuthorizer.java | 10 ++ 29 files changed, 400 insertions(+), 100 deletions(-) diff --git a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java index 98b8102cc0..0d047f1881 100644 --- a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java +++ b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java @@ -158,8 +158,7 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { return AuthorizationResult.approved(); } - - return AuthorizationResult.denied(); + return AuthorizationResult.denied(request.getExplanationSupplier().get()); } /** diff --git a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AuthorizationRequest.java b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AuthorizationRequest.java index da0a276b1e..4f5a8f8dd4 100644 --- a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AuthorizationRequest.java +++ b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AuthorizationRequest.java @@ -20,12 +20,15 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.function.Supplier; /** * Represents an authorization request for a given user/entity performing an action against a resource within some userContext. */ public class AuthorizationRequest { + public static final String DEFAULT_EXPLANATION = "Unable to perform the desired action."; + private final Resource resource; private final String identity; private final RequestAction action; @@ -33,6 +36,7 @@ public class AuthorizationRequest { private final boolean isAnonymous; private final Map userContext; private final Map resourceContext; + private final Supplier explanationSupplier; private AuthorizationRequest(final Builder builder) { Objects.requireNonNull(builder.resource, "The resource is required when creating an authorization request"); @@ -47,6 +51,16 @@ public class AuthorizationRequest { this.isAnonymous = builder.isAnonymous; this.userContext = builder.userContext == null ? null : Collections.unmodifiableMap(builder.userContext); this.resourceContext = builder.resourceContext == null ? null : Collections.unmodifiableMap(builder.resourceContext); + this.explanationSupplier = () -> { + final String explanation = builder.explanationSupplier.get(); + + // ensure the specified supplier returns non null + if (explanation == null) { + return DEFAULT_EXPLANATION; + } else { + return explanation; + } + }; } /** @@ -112,6 +126,15 @@ public class AuthorizationRequest { return resourceContext; } + /** + * A supplier for the explanation if access is denied. Non null. + * + * @return The explanation supplier if access is denied + */ + public Supplier getExplanationSupplier() { + return explanationSupplier; + } + /** * AuthorizationRequest builder. */ @@ -124,6 +147,7 @@ public class AuthorizationRequest { private RequestAction action; private Map userContext; private Map resourceContext; + private Supplier explanationSupplier = () -> DEFAULT_EXPLANATION; public Builder resource(final Resource resource) { this.resource = resource; @@ -164,6 +188,13 @@ public class AuthorizationRequest { return this; } + public Builder explanationSupplier(final Supplier explanationSupplier) { + if (explanationSupplier != null) { + this.explanationSupplier = explanationSupplier; + } + return this; + } + public AuthorizationRequest build() { return new AuthorizationRequest(this); } diff --git a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AuthorizationResult.java b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AuthorizationResult.java index a3f520c118..acfdc02ce5 100644 --- a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AuthorizationResult.java +++ b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AuthorizationResult.java @@ -28,7 +28,7 @@ public class AuthorizationResult { } private static final AuthorizationResult APPROVED = new AuthorizationResult(Result.Approved, null); - private static final AuthorizationResult RESOURCE_NOT_FOUND = new AuthorizationResult(Result.ResourceNotFound, null); + private static final AuthorizationResult RESOURCE_NOT_FOUND = new AuthorizationResult(Result.ResourceNotFound, "Not authorized for the requested resource."); private final Result result; private final String explanation; @@ -44,6 +44,10 @@ public class AuthorizationResult { throw new IllegalArgumentException("An explanation is required when the authorization request is denied."); } + if (Result.ResourceNotFound.equals(result) && explanation == null) { + throw new IllegalArgumentException("An explanation is required when the authorization request is resource not found."); + } + this.result = result; this.explanation = explanation; } @@ -83,7 +87,7 @@ public class AuthorizationResult { * @return a new denied AuthorizationResult */ public static AuthorizationResult denied() { - return denied("Access is denied"); + return denied(AuthorizationRequest.DEFAULT_EXPLANATION); } /** diff --git a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/Resource.java b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/Resource.java index 661b3269e6..2aae7b2b7f 100644 --- a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/Resource.java +++ b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/Resource.java @@ -34,4 +34,11 @@ public interface Resource { * @return name of this resource */ String getName(); + + /** + * The description of this resource that may be safely used in messages to the client. + * + * @return safe description + */ + String getSafeDescription(); } diff --git a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/resource/Authorizable.java b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/resource/Authorizable.java index 39e96422dc..3219ac257b 100644 --- a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/resource/Authorizable.java +++ b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/resource/Authorizable.java @@ -26,8 +26,8 @@ import org.apache.nifi.authorization.Resource; import org.apache.nifi.authorization.UserContextKeys; import org.apache.nifi.authorization.user.NiFiUser; -import java.util.Map; import java.util.HashMap; +import java.util.Map; public interface Authorizable { @@ -70,7 +70,7 @@ public interface Authorizable { */ default AuthorizationResult checkAuthorization(Authorizer authorizer, RequestAction action, NiFiUser user, Map resourceContext) { if (user == null) { - return AuthorizationResult.denied("Unknown user"); + return AuthorizationResult.denied("Unknown user."); } final Map userContext; @@ -81,15 +81,28 @@ public interface Authorizable { userContext = null; } - // build the request + final Resource resource = getResource(); final AuthorizationRequest request = new AuthorizationRequest.Builder() .identity(user.getIdentity()) .anonymous(user.isAnonymous()) .accessAttempt(false) .action(action) - .resource(getResource()) + .resource(resource) .resourceContext(resourceContext) .userContext(userContext) + .explanationSupplier(() -> { + // build the safe explanation + final StringBuilder safeDescription = new StringBuilder("Unable to "); + + if (RequestAction.READ.equals(action)) { + safeDescription.append("view "); + } else { + safeDescription.append("modify "); + } + safeDescription.append(resource.getSafeDescription()).append("."); + + return safeDescription.toString(); + }) .build(); // perform the authorization @@ -99,9 +112,37 @@ public interface Authorizable { if (Result.ResourceNotFound.equals(result.getResult())) { final Authorizable parent = getParentAuthorizable(); if (parent == null) { - return AuthorizationResult.denied(); + return AuthorizationResult.denied("No applicable policies could be found."); } else { - return parent.checkAuthorization(authorizer, action, user, resourceContext); + // create a custom authorizable to override the safe description but still defer to the parent authorizable + final Authorizable parentProxy = new Authorizable() { + @Override + public Authorizable getParentAuthorizable() { + return parent.getParentAuthorizable(); + } + + @Override + public Resource getResource() { + final Resource parentResource = parent.getResource(); + return new Resource() { + @Override + public String getIdentifier() { + return parentResource.getIdentifier(); + } + + @Override + public String getName() { + return parentResource.getName(); + } + + @Override + public String getSafeDescription() { + return resource.getSafeDescription(); + } + }; + } + }; + return parentProxy.checkAuthorization(authorizer, action, user, resourceContext); } } else { return result; @@ -133,7 +174,7 @@ public interface Authorizable { */ default void authorize(Authorizer authorizer, RequestAction action, NiFiUser user, Map resourceContext) throws AccessDeniedException { if (user == null) { - throw new AccessDeniedException("Unknown user"); + throw new AccessDeniedException("Unknown user."); } final Map userContext; @@ -144,23 +185,65 @@ public interface Authorizable { userContext = null; } + final Resource resource = getResource(); final AuthorizationRequest request = new AuthorizationRequest.Builder() .identity(user.getIdentity()) .anonymous(user.isAnonymous()) .accessAttempt(true) .action(action) - .resource(getResource()) + .resource(resource) .resourceContext(resourceContext) .userContext(userContext) + .explanationSupplier(() -> { + // build the safe explanation + final StringBuilder safeDescription = new StringBuilder("Unable to "); + + if (RequestAction.READ.equals(action)) { + safeDescription.append("view "); + } else { + safeDescription.append("modify "); + } + safeDescription.append(resource.getSafeDescription()).append("."); + + return safeDescription.toString(); + }) .build(); final AuthorizationResult result = authorizer.authorize(request); if (Result.ResourceNotFound.equals(result.getResult())) { final Authorizable parent = getParentAuthorizable(); if (parent == null) { - throw new AccessDeniedException("Access is denied"); + throw new AccessDeniedException("No applicable policies could be found."); } else { - parent.authorize(authorizer, action, user, resourceContext); + // create a custom authorizable to override the safe description but still defer to the parent authorizable + final Authorizable parentProxy = new Authorizable() { + @Override + public Authorizable getParentAuthorizable() { + return parent.getParentAuthorizable(); + } + + @Override + public Resource getResource() { + final Resource parentResource = parent.getResource(); + return new Resource() { + @Override + public String getIdentifier() { + return parentResource.getIdentifier(); + } + + @Override + public String getName() { + return parentResource.getName(); + } + + @Override + public String getSafeDescription() { + return resource.getSafeDescription(); + } + }; + } + }; + parentProxy.authorize(authorizer, action, user, resourceContext); } } else if (Result.Denied.equals(result.getResult())) { throw new AccessDeniedException(result.getExplanation()); diff --git a/nifi-framework-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java b/nifi-framework-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java index a5cf35121f..cda2ac5ffd 100644 --- a/nifi-framework-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java +++ b/nifi-framework-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java @@ -16,7 +16,6 @@ */ package org.apache.nifi.authorization; -import org.apache.nifi.authorization.MockPolicyBasedAuthorizer; import org.apache.nifi.authorization.exception.AuthorizerCreationException; import org.junit.Assert; import org.junit.Test; @@ -42,6 +41,11 @@ public class TestAbstractPolicyBasedAuthorizer { public String getName() { return "resource1"; } + + @Override + public String getSafeDescription() { + return "description1"; + } }; @Test diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java index 7f89dddc6a..be20b602be 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java @@ -447,8 +447,12 @@ public class FileAuthorizer extends AbstractPolicyBasedAuthorizer { // convert any access controls on ports to the appropriate policies for (PortDTO portDTO : ports) { - final boolean isInputPort = portDTO.getType() != null && portDTO.getType().equals("inputPort"); - final Resource resource = ResourceFactory.getDataTransferResource(isInputPort, portDTO.getId(), portDTO.getName()); + final Resource resource; + if (portDTO.getType() != null && portDTO.getType().equals("inputPort")) { + resource = ResourceFactory.getDataTransferResource(ResourceFactory.getComponentResource(ResourceType.InputPort, portDTO.getId(), portDTO.getName())); + } else { + resource = ResourceFactory.getDataTransferResource(ResourceFactory.getComponentResource(ResourceType.OutputPort, portDTO.getId(), portDTO.getName())); + } if (portDTO.getUserAccessControl() != null) { for (String userAccessControl : portDTO.getUserAccessControl()) { diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java index c335e19cd5..55a183988e 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java @@ -358,7 +358,8 @@ public class FileAuthorizerTest { assertEquals(2, user6Policies.get(ResourceType.SiteToSite.getValue()).size()); assertTrue(user6Policies.get(ResourceType.SiteToSite.getValue()).contains(RequestAction.WRITE)); - final Resource inputPortResource = ResourceFactory.getDataTransferResource(true, "2f7d1606-b090-4be7-a592-a5b70fb55531", "TCP Input"); + final Resource inputPortResource = ResourceFactory.getDataTransferResource( + ResourceFactory.getComponentResource(ResourceType.InputPort, "2f7d1606-b090-4be7-a592-a5b70fb55531", "TCP Input")); final AccessPolicy inputPortPolicy = authorizer.getUsersAndAccessPolicies().getAccessPolicy(inputPortResource.getIdentifier(), RequestAction.WRITE); assertNotNull(inputPortPolicy); assertEquals(1, inputPortPolicy.getUsers().size()); @@ -366,7 +367,8 @@ public class FileAuthorizerTest { assertEquals(1, inputPortPolicy.getGroups().size()); assertTrue(inputPortPolicy.getGroups().contains(group1.getIdentifier())); - final Resource outputPortResource = ResourceFactory.getDataTransferResource(false, "2f7d1606-b090-4be7-a592-a5b70fb55532", "TCP Output"); + final Resource outputPortResource = ResourceFactory.getDataTransferResource( + ResourceFactory.getComponentResource(ResourceType.OutputPort, "2f7d1606-b090-4be7-a592-a5b70fb55532", "TCP Output")); final AccessPolicy outputPortPolicy = authorizer.getUsersAndAccessPolicies().getAccessPolicy(outputPortResource.getIdentifier(), RequestAction.WRITE); assertNotNull(outputPortPolicy); assertEquals(1, outputPortPolicy.getUsers().size()); @@ -432,7 +434,8 @@ public class FileAuthorizerTest { final Group group1 = groups.iterator().next(); assertEquals("group1", group1.getName()); - final Resource inputPortResource = ResourceFactory.getDataTransferResource(true, "2f7d1606-b090-4be7-a592-a5b70fb55531", "TCP Input"); + final Resource inputPortResource = ResourceFactory.getDataTransferResource( + ResourceFactory.getComponentResource(ResourceType.InputPort, "2f7d1606-b090-4be7-a592-a5b70fb55531", "TCP Input")); final AccessPolicy inputPortPolicy = authorizer.getUsersAndAccessPolicies().getAccessPolicy(inputPortResource.getIdentifier(), RequestAction.WRITE); assertNotNull(inputPortPolicy); assertEquals(1, inputPortPolicy.getUsers().size()); @@ -440,7 +443,8 @@ public class FileAuthorizerTest { assertEquals(1, inputPortPolicy.getGroups().size()); assertTrue(inputPortPolicy.getGroups().contains(group1.getIdentifier())); - final Resource outputPortResource = ResourceFactory.getDataTransferResource(false, "2f7d1606-b090-4be7-a592-a5b70fb55532", "TCP Output"); + final Resource outputPortResource = ResourceFactory.getDataTransferResource( + ResourceFactory.getComponentResource(ResourceType.OutputPort, "2f7d1606-b090-4be7-a592-a5b70fb55532", "TCP Output")); final AccessPolicy outputPortPolicy = authorizer.getUsersAndAccessPolicies().getAccessPolicy(outputPortResource.getIdentifier(), RequestAction.WRITE); assertNotNull(outputPortPolicy); assertEquals(1, outputPortPolicy.getUsers().size()); diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/AccessPolicyAuthorizable.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/AccessPolicyAuthorizable.java index 041b982e6c..81f8a130b0 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/AccessPolicyAuthorizable.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/AccessPolicyAuthorizable.java @@ -89,7 +89,7 @@ public class AccessPolicyAuthorizable implements Authorizable, EnforcePolicyPerm @Override public AuthorizationResult checkAuthorization(Authorizer authorizer, RequestAction action, NiFiUser user, Map resourceContext) { if (user == null) { - throw new AccessDeniedException("Unknown user"); + throw new AccessDeniedException("Unknown user."); } final AuthorizationResult resourceResult = Authorizable.super.checkAuthorization(authorizer, action, user, resourceContext); @@ -105,7 +105,7 @@ public class AccessPolicyAuthorizable implements Authorizable, EnforcePolicyPerm @Override public void authorize(Authorizer authorizer, RequestAction action, NiFiUser user, Map resourceContext) throws AccessDeniedException { if (user == null) { - throw new AccessDeniedException("Unknown user"); + throw new AccessDeniedException("Unknown user."); } try { diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/DataAuthorizable.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/DataAuthorizable.java index cb0d0f1dd4..7269560d8d 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/DataAuthorizable.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/DataAuthorizable.java @@ -62,7 +62,7 @@ public class DataAuthorizable implements Authorizable, EnforcePolicyPermissionsT @Override public AuthorizationResult checkAuthorization(Authorizer authorizer, RequestAction action, NiFiUser user, Map resourceContext) { if (user == null) { - return AuthorizationResult.denied("Unknown user"); + return AuthorizationResult.denied("Unknown user."); } AuthorizationResult result = null; @@ -100,7 +100,7 @@ public class DataAuthorizable implements Authorizable, EnforcePolicyPermissionsT @Override public void authorize(Authorizer authorizer, RequestAction action, NiFiUser user, Map resourceContext) throws AccessDeniedException { if (user == null) { - throw new AccessDeniedException("Unknown user"); + throw new AccessDeniedException("Unknown user."); } // calculate the dn chain diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/ResourceFactory.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/ResourceFactory.java index 83c5bb9983..79392b7394 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/ResourceFactory.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/ResourceFactory.java @@ -32,6 +32,11 @@ public final class ResourceFactory { public String getName() { return "Controller"; } + + @Override + public String getSafeDescription() { + return "the controller"; + } }; private final static Resource FLOW_RESOURCE = new Resource() { @@ -44,6 +49,11 @@ public final class ResourceFactory { public String getName() { return "NiFi Flow"; } + + @Override + public String getSafeDescription() { + return "the user interface"; + } }; private final static Resource POLICY_RESOURCE = new Resource() { @@ -54,7 +64,12 @@ public final class ResourceFactory { @Override public String getName() { - return "Policy"; + return "Policies for "; + } + + @Override + public String getSafeDescription() { + return "the policies for "; } }; @@ -68,6 +83,11 @@ public final class ResourceFactory { public String getName() { return "Counters"; } + + @Override + public String getSafeDescription() { + return "counters"; + } }; private final static Resource PROVENANCE_RESOURCE = new Resource() { @@ -80,6 +100,11 @@ public final class ResourceFactory { public String getName() { return "Provenance"; } + + @Override + public String getSafeDescription() { + return "provenance"; + } }; private final static Resource DATA_RESOURCE = new Resource() { @@ -90,7 +115,12 @@ public final class ResourceFactory { @Override public String getName() { - return "Data"; + return "Data for "; + } + + @Override + public String getSafeDescription() { + return "the data for "; } }; @@ -104,6 +134,11 @@ public final class ResourceFactory { public String getName() { return "Proxy User Requests"; } + + @Override + public String getSafeDescription() { + return "proxy requests on behalf of users"; + } }; private final static Resource RESOURCE_RESOURCE = new Resource() { @@ -116,6 +151,11 @@ public final class ResourceFactory { public String getName() { return "NiFi Resources"; } + + @Override + public String getSafeDescription() { + return "resources"; + } }; private final static Resource SITE_TO_SITE_RESOURCE = new Resource() { @@ -128,6 +168,11 @@ public final class ResourceFactory { public String getName() { return "Site to Site"; } + + @Override + public String getSafeDescription() { + return "site-to-site details"; + } }; private final static Resource SYSTEM_RESOURCE = new Resource() { @@ -140,6 +185,11 @@ public final class ResourceFactory { public String getName() { return "System"; } + + @Override + public String getSafeDescription() { + return "system diagnostics"; + } }; private final static Resource RESTRICTED_COMPONENTS_RESOURCE = new Resource() { @@ -152,6 +202,11 @@ public final class ResourceFactory { public String getName() { return "Restricted Components"; } + + @Override + public String getSafeDescription() { + return "restricted components"; + } }; private final static Resource TENANT_RESOURCE = new Resource() { @@ -164,6 +219,11 @@ public final class ResourceFactory { public String getName() { return "Tenant"; } + + @Override + public String getSafeDescription() { + return "users/user groups"; + } }; private final static Resource POLICIES_RESOURCE = new Resource() { @@ -177,6 +237,11 @@ public final class ResourceFactory { public String getName() { return "Access Policies"; } + + @Override + public String getSafeDescription() { + return "policies"; + } }; /** @@ -271,30 +336,6 @@ public final class ResourceFactory { return TENANT_RESOURCE; } - /** - * Gets a Resource for performing transferring data to a port. - * - * @param isInputPort Whether this port is an input port or an output port - * @param identifier The identifier of the component being accessed - * @param name The name of the component being accessed - * @return The resource - */ - public static Resource getDataTransferResource(final boolean isInputPort, final String identifier, final String name) { - Objects.requireNonNull(identifier, "The component identifier must be specified."); - - return new Resource() { - @Override - public String getIdentifier() { - return String.format("%s/%s/%s", ResourceType.DataTransfer.getValue(), isInputPort ? "input-ports" : "output-ports", identifier); - } - - @Override - public String getName() { - return name; - } - }; - } - /** * Gets a Resource for performing transferring data to a port. * @@ -314,6 +355,11 @@ public final class ResourceFactory { public String getName() { return "Transfer data to " + resource.getName(); } + + @Override + public String getSafeDescription() { + return "data transfers to " + resource.getSafeDescription(); + } }; } @@ -342,7 +388,12 @@ public final class ResourceFactory { @Override public String getName() { - return "Policies for " + resource.getName(); + return POLICY_RESOURCE.getName() + resource.getName(); + } + + @Override + public String getSafeDescription() { + return POLICY_RESOURCE.getSafeDescription() + resource.getSafeDescription(); } }; } @@ -369,6 +420,47 @@ public final class ResourceFactory { public String getName() { return name; } + + @Override + public String getSafeDescription() { + final String componentType; + switch (resourceType) { + case ControllerService: + componentType = "Controller Service"; + break; + case ProcessGroup: + componentType = "Process Group"; + break; + case Template: + componentType = "Template"; + break; + case Funnel: + componentType = "Funnel"; + break; + case InputPort: + componentType = "Input Port"; + break; + case OutputPort: + componentType = "Output Port"; + break; + case Processor: + componentType = "Processor"; + break; + case RemoteProcessGroup: + componentType = "Remote Process Group"; + break; + case ReportingTask: + componentType = "Reporting Task"; + break; + case Label: + componentType = "Label"; + break; + default: + componentType = "Component"; + break; + } + return componentType + " with ID " + identifier; + } }; } @@ -387,7 +479,12 @@ public final class ResourceFactory { @Override public String getName() { - return "Data for " + resource.getName(); + return DATA_RESOURCE.getName() + resource.getName(); + } + + @Override + public String getSafeDescription() { + return DATA_RESOURCE.getSafeDescription() + resource.getSafeDescription(); } }; } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/connectable/StandardConnection.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/connectable/StandardConnection.java index 3d5efedb15..ddb4523151 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/connectable/StandardConnection.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/connectable/StandardConnection.java @@ -136,6 +136,11 @@ public final class StandardConnection implements Connection { return name; } + + @Override + public String getSafeDescription() { + return "Connection " + StandardConnection.this.getIdentifier(); + } }; } @@ -172,7 +177,7 @@ public final class StandardConnection implements Connection { @Override public AuthorizationResult checkAuthorization(Authorizer authorizer, RequestAction action, NiFiUser user, Map resourceContext) { if (user == null) { - return AuthorizationResult.denied("Unknown user"); + return AuthorizationResult.denied("Unknown user."); } // check the source @@ -188,7 +193,7 @@ public final class StandardConnection implements Connection { @Override public void authorize(Authorizer authorizer, RequestAction action, NiFiUser user, Map resourceContext) throws AccessDeniedException { if (user == null) { - throw new AccessDeniedException("Unknown user"); + throw new AccessDeniedException("Unknown user."); } getSourceAuthorizable().authorize(authorizer, action, user, resourceContext); diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java index 67f768a3f2..c8aad318a1 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java @@ -963,6 +963,11 @@ public class StandardNiFiServiceFacade implements NiFiServiceFacade { public String getName() { return resourceIdentifier; } + + @Override + public String getSafeDescription() { + return "User " + userId; + } }, () -> userDAO.deleteUser(userId), false, // no user specific policies to remove @@ -993,6 +998,11 @@ public class StandardNiFiServiceFacade implements NiFiServiceFacade { public String getName() { return resourceIdentifier; } + + @Override + public String getSafeDescription() { + return "User Group " + userGroupId; + } }, () -> userGroupDAO.deleteUserGroup(userGroupId), false, // no user group specific policies to remove @@ -1020,6 +1030,11 @@ public class StandardNiFiServiceFacade implements NiFiServiceFacade { public String getName() { return accessPolicy.getResource(); } + + @Override + public String getSafeDescription() { + return "Policy " + accessPolicyId; + } }, () -> accessPolicyDAO.deleteAccessPolicy(accessPolicyId), false, // no need to clean up any policies as it's already been removed above @@ -2514,6 +2529,7 @@ public class StandardNiFiServiceFacade implements NiFiServiceFacade { .accessAttempt(false) .action(RequestAction.WRITE) .userContext(userContext) + .explanationSupplier(() -> "Unable to retrieve port details.") .build(); final AuthorizationResult result = authorizer.authorize(request); @@ -2680,6 +2696,11 @@ public class StandardNiFiServiceFacade implements NiFiServiceFacade { public String getName() { return resource; } + + @Override + public String getSafeDescription() { + return "Policy " + resource; + } }; } }; @@ -3100,7 +3121,7 @@ public class StandardNiFiServiceFacade implements NiFiServiceFacade { return entityFactory.createStatusHistoryEntity(dto, permissions); } - private boolean authorizeAction(final Action action) { + private AuthorizationResult authorizeAction(final Action action) { final String sourceId = action.getSourceId(); final Component type = action.getSourceType(); @@ -3149,12 +3170,11 @@ public class StandardNiFiServiceFacade implements NiFiServiceFacade { } } catch (final ResourceNotFoundException e) { // if the underlying component is gone, disallow - return false; + return AuthorizationResult.denied("The component of this action is no longer in the data flow."); } // perform the authorization - final AuthorizationResult result = authorizable.checkAuthorization(authorizer, RequestAction.READ, NiFiUserUtils.getNiFiUser()); - return Result.Approved.equals(result.getResult()); + return authorizable.checkAuthorization(authorizer, RequestAction.READ, NiFiUserUtils.getNiFiUser()); } @Override @@ -3178,7 +3198,8 @@ public class StandardNiFiServiceFacade implements NiFiServiceFacade { if (history.getActions() != null) { final List actionEntities = new ArrayList<>(); for (final Action action : history.getActions()) { - actionEntities.add(entityFactory.createActionEntity(dtoFactory.createActionDto(action), authorizeAction(action))); + final AuthorizationResult result = authorizeAction(action); + actionEntities.add(entityFactory.createActionEntity(dtoFactory.createActionDto(action), Result.Approved.equals(result.getResult()))); } historyDto.setActions(actionEntities); } @@ -3197,9 +3218,10 @@ public class StandardNiFiServiceFacade implements NiFiServiceFacade { throw new ResourceNotFoundException(String.format("Unable to find action with id '%s'.", actionId)); } - final boolean authorized = authorizeAction(action); + final AuthorizationResult result = authorizeAction(action); + final boolean authorized = Result.Approved.equals(result.getResult()); if (!authorized) { - throw new AccessDeniedException("Access is denied."); + throw new AccessDeniedException(result.getExplanation()); } // return the action diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiWebConfigurationContext.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiWebConfigurationContext.java index 177e5576c2..8146a3933b 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiWebConfigurationContext.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiWebConfigurationContext.java @@ -115,12 +115,12 @@ public class StandardNiFiWebConfigurationContext implements NiFiWebConfiguration .accessAttempt(true) .action(RequestAction.READ) .userContext(userContext) + .explanationSupplier(() -> "Unable to view the user interface.") .build(); final AuthorizationResult result = authorizer.authorize(request); if (!Result.Approved.equals(result.getResult())) { - final String message = StringUtils.isNotBlank(result.getExplanation()) ? result.getExplanation() : "Access is denied"; - throw new AccessDeniedException(message); + throw new AccessDeniedException(result.getExplanation()); } }); } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java index 5c108a49ac..529f0498e2 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java @@ -252,7 +252,7 @@ public class AccessResource extends ApplicationResource { final NiFiUser user = NiFiUserUtils.getNiFiUser(); if (user == null) { - throw new AccessDeniedException("Unable to determine user details."); + throw new AccessDeniedException("No user authenticated in the request."); } final OtpAuthenticationToken authenticationToken = new OtpAuthenticationToken(user.getIdentity()); @@ -297,7 +297,7 @@ public class AccessResource extends ApplicationResource { final NiFiUser user = NiFiUserUtils.getNiFiUser(); if (user == null) { - throw new AccessDeniedException("Unable to determine user details."); + throw new AccessDeniedException("No user authenticated in the request."); } final OtpAuthenticationToken authenticationToken = new OtpAuthenticationToken(user.getIdentity()); diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ControllerResource.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ControllerResource.java index 20b9f87028..86ca792fcc 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ControllerResource.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ControllerResource.java @@ -112,12 +112,23 @@ public class ControllerResource extends ApplicationResource { .accessAttempt(true) .action(action) .userContext(userContext) + .explanationSupplier(() -> { + final StringBuilder explanation = new StringBuilder("Unable to "); + + if (RequestAction.READ.equals(action)) { + explanation.append("view "); + } else { + explanation.append("modify "); + } + explanation.append("the controller."); + + return explanation.toString(); + }) .build(); final AuthorizationResult result = authorizer.authorize(request); if (!Result.Approved.equals(result.getResult())) { - final String message = StringUtils.isNotBlank(result.getExplanation()) ? result.getExplanation() : "Access is denied"; - throw new AccessDeniedException(message); + throw new AccessDeniedException(result.getExplanation()); } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/CountersResource.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/CountersResource.java index e3a2b9eeab..3d6c3be7ad 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/CountersResource.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/CountersResource.java @@ -98,12 +98,23 @@ public class CountersResource extends ApplicationResource { .accessAttempt(true) .action(action) .userContext(userContext) + .explanationSupplier(() -> { + final StringBuilder explanation = new StringBuilder("Unable to "); + + if (RequestAction.READ.equals(action)) { + explanation.append("view "); + } else { + explanation.append("modify "); + } + explanation.append("counters."); + + return explanation.toString(); + }) .build(); final AuthorizationResult result = authorizer.authorize(request); if (!Result.Approved.equals(result.getResult())) { - final String message = StringUtils.isNotBlank(result.getExplanation()) ? result.getExplanation() : "Access is denied"; - throw new AccessDeniedException(message); + throw new AccessDeniedException(result.getExplanation()); } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/FlowResource.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/FlowResource.java index 1b9bc99c3d..a0317251fb 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/FlowResource.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/FlowResource.java @@ -218,12 +218,12 @@ public class FlowResource extends ApplicationResource { .accessAttempt(true) .action(RequestAction.READ) .userContext(userContext) + .explanationSupplier(() -> "Unable to view the user interface.") .build(); final AuthorizationResult result = authorizer.authorize(request); if (!Result.Approved.equals(result.getResult())) { - final String message = StringUtils.isNotBlank(result.getExplanation()) ? result.getExplanation() : "Access is denied"; - throw new AccessDeniedException(message); + throw new AccessDeniedException(result.getExplanation()); } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProvenanceResource.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProvenanceResource.java index 4e7a171a6a..9044bbe8b9 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProvenanceResource.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProvenanceResource.java @@ -108,12 +108,12 @@ public class ProvenanceResource extends ApplicationResource { .accessAttempt(true) .action(RequestAction.READ) .userContext(userContext) + .explanationSupplier(() -> "Unable to query provenance.") .build(); final AuthorizationResult result = authorizer.authorize(request); if (!Result.Approved.equals(result.getResult())) { - final String message = StringUtils.isNotBlank(result.getExplanation()) ? result.getExplanation() : "Access is denied"; - throw new AccessDeniedException(message); + throw new AccessDeniedException(result.getExplanation()); } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ResourceResource.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ResourceResource.java index 67c1b22399..cd41ed9f27 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ResourceResource.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ResourceResource.java @@ -78,12 +78,12 @@ public class ResourceResource extends ApplicationResource { .accessAttempt(true) .action(RequestAction.READ) .userContext(userContext) + .explanationSupplier(() -> "Unable to retrieve resources.") .build(); final AuthorizationResult result = authorizer.authorize(request); if (!Result.Approved.equals(result.getResult())) { - final String message = StringUtils.isNotBlank(result.getExplanation()) ? result.getExplanation() : "Access is denied"; - throw new AccessDeniedException(message); + throw new AccessDeniedException(result.getExplanation()); } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/SiteToSiteResource.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/SiteToSiteResource.java index efb1c26cb1..744a9f44db 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/SiteToSiteResource.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/SiteToSiteResource.java @@ -116,12 +116,12 @@ public class SiteToSiteResource extends ApplicationResource { .accessAttempt(true) .action(RequestAction.READ) .userContext(userContext) + .explanationSupplier(() -> "Unable to retrieve site to site details.") .build(); final AuthorizationResult result = authorizer.authorize(request); if (!Result.Approved.equals(result.getResult())) { - final String message = StringUtils.isNotBlank(result.getExplanation()) ? result.getExplanation() : "Access is denied"; - throw new AccessDeniedException(message); + throw new AccessDeniedException(result.getExplanation()); } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/SystemDiagnosticsResource.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/SystemDiagnosticsResource.java index 0be9c49aa6..04e86834c2 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/SystemDiagnosticsResource.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/SystemDiagnosticsResource.java @@ -81,12 +81,12 @@ public class SystemDiagnosticsResource extends ApplicationResource { .accessAttempt(true) .action(RequestAction.READ) .userContext(userContext) + .explanationSupplier(() -> "Unable to view system diagnostics.") .build(); final AuthorizationResult result = authorizer.authorize(request); if (!Result.Approved.equals(result.getResult())) { - final String message = StringUtils.isNotBlank(result.getExplanation()) ? result.getExplanation() : "Access is denied"; - throw new AccessDeniedException(message); + throw new AccessDeniedException(result.getExplanation()); } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/config/AccessDeniedExceptionMapper.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/config/AccessDeniedExceptionMapper.java index d55a38673e..9c5ba4dea7 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/config/AccessDeniedExceptionMapper.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/config/AccessDeniedExceptionMapper.java @@ -58,13 +58,16 @@ public class AccessDeniedExceptionMapper implements ExceptionMapper