Repository: nifi-registry Updated Branches: refs/heads/master 580f77549 -> f2d712f32
NIFIREG-72 Fix API status codes for errors Fixes Tenants and Policies to return 404 for resource not found cases. This closes #60. Signed-off-by: Bryan Bende <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/nifi-registry/repo Commit: http://git-wip-us.apache.org/repos/asf/nifi-registry/commit/f2d712f3 Tree: http://git-wip-us.apache.org/repos/asf/nifi-registry/tree/f2d712f3 Diff: http://git-wip-us.apache.org/repos/asf/nifi-registry/diff/f2d712f3 Branch: refs/heads/master Commit: f2d712f32b8b6f3c2a60514b4d318de8af02fc29 Parents: 580f775 Author: Kevin Doran <[email protected]> Authored: Fri Dec 15 16:17:37 2017 -0500 Committer: Bryan Bende <[email protected]> Committed: Mon Dec 18 08:37:25 2017 -0500 ---------------------------------------------------------------------- .../registry/service/AuthorizationService.java | 24 +++++++-- .../security/authorization/RequestAction.java | 2 +- .../registry/web/api/AccessPolicyResource.java | 55 ++++++++++++-------- .../nifi/registry/web/api/TenantResource.java | 31 +++++++++-- 4 files changed, 80 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/nifi-registry/blob/f2d712f3/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/AuthorizationService.java ---------------------------------------------------------------------- diff --git a/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/AuthorizationService.java b/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/AuthorizationService.java index f3e9c40..5543c7e 100644 --- a/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/AuthorizationService.java +++ b/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/AuthorizationService.java @@ -214,6 +214,9 @@ public class AuthorizationService { try { final org.apache.nifi.registry.security.authorization.User updatedUser = ((ConfigurableUserGroupProvider) userGroupProvider).updateUser(userFromDTO(user)); + if (updatedUser == null) { + return null; + } return userToDTO(updatedUser); } finally { this.writeLock.unlock(); @@ -225,7 +228,9 @@ public class AuthorizationService { this.writeLock.lock(); try { User deletedUserDTO = getUser(identifier); - ((ConfigurableUserGroupProvider) userGroupProvider).deleteUser(identifier); + if (deletedUserDTO != null) { + ((ConfigurableUserGroupProvider) userGroupProvider).deleteUser(identifier); + } return deletedUserDTO; } finally { this.writeLock.unlock(); @@ -271,6 +276,9 @@ public class AuthorizationService { try { final org.apache.nifi.registry.security.authorization.Group updatedGroup = ((ConfigurableUserGroupProvider) userGroupProvider).updateGroup(userGroupFromDTO(userGroup)); + if (updatedGroup == null) { + return null; + } return userGroupToDTO(updatedGroup); } finally { writeLock.unlock(); @@ -282,7 +290,9 @@ public class AuthorizationService { writeLock.lock(); try { final UserGroup userGroupDTO = getUserGroup(identifier); - ((ConfigurableUserGroupProvider) userGroupProvider).deleteGroup(identifier); + if (userGroupDTO != null) { + ((ConfigurableUserGroupProvider) userGroupProvider).deleteGroup(identifier); + } return userGroupDTO; } finally { writeLock.unlock(); @@ -371,11 +381,17 @@ public class AuthorizationService { // Don't allow changing action or resource of existing policy (should only be adding/removing users/groups) org.apache.nifi.registry.security.authorization.AccessPolicy currentAccessPolicy = accessPolicyProvider.getAccessPolicy(accessPolicy.getIdentifier()); + if (currentAccessPolicy == null) { + return null; + } accessPolicy.setResource(currentAccessPolicy.getResource()); accessPolicy.setAction(currentAccessPolicy.getAction().toString()); org.apache.nifi.registry.security.authorization.AccessPolicy updatedAccessPolicy = ((ConfigurableAccessPolicyProvider) accessPolicyProvider).updateAccessPolicy(accessPolicyFromDTO(accessPolicy)); + if (updatedAccessPolicy == null) { + return null; + } return accessPolicyToDTO(updatedAccessPolicy); } finally { writeLock.unlock(); @@ -387,7 +403,9 @@ public class AuthorizationService { writeLock.lock(); try { AccessPolicy deletedAccessPolicyDTO = getAccessPolicy(identifier); - ((ConfigurableAccessPolicyProvider) accessPolicyProvider).deleteAccessPolicy(identifier); + if (deletedAccessPolicyDTO != null) { + ((ConfigurableAccessPolicyProvider) accessPolicyProvider).deleteAccessPolicy(identifier); + } return deletedAccessPolicyDTO; } finally { writeLock.unlock(); http://git-wip-us.apache.org/repos/asf/nifi-registry/blob/f2d712f3/nifi-registry-security-api/src/main/java/org/apache/nifi/registry/security/authorization/RequestAction.java ---------------------------------------------------------------------- diff --git a/nifi-registry-security-api/src/main/java/org/apache/nifi/registry/security/authorization/RequestAction.java b/nifi-registry-security-api/src/main/java/org/apache/nifi/registry/security/authorization/RequestAction.java index adf07f2..def3de4 100644 --- a/nifi-registry-security-api/src/main/java/org/apache/nifi/registry/security/authorization/RequestAction.java +++ b/nifi-registry-security-api/src/main/java/org/apache/nifi/registry/security/authorization/RequestAction.java @@ -50,7 +50,7 @@ public enum RequestAction { stringJoiner.add(ra.toString()); } String allowableValues = stringJoiner.toString(); - throw new IllegalArgumentException("Action must be one of [" + allowableValues + "]"); + throw new IllegalArgumentException("Action '" + action + "' is invalid. Must be one of [" + allowableValues + "]"); } } } http://git-wip-us.apache.org/repos/asf/nifi-registry/blob/f2d712f3/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/AccessPolicyResource.java ---------------------------------------------------------------------- diff --git a/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/AccessPolicyResource.java b/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/AccessPolicyResource.java index 50474d3..f918d4c 100644 --- a/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/AccessPolicyResource.java +++ b/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/AccessPolicyResource.java @@ -16,25 +16,13 @@ */ package org.apache.nifi.registry.web.api; -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.Consumes; -import javax.ws.rs.DELETE; -import javax.ws.rs.GET; -import javax.ws.rs.POST; -import javax.ws.rs.PUT; -import javax.ws.rs.Path; -import javax.ws.rs.PathParam; -import javax.ws.rs.Produces; -import javax.ws.rs.core.Context; -import javax.ws.rs.core.MediaType; -import javax.ws.rs.core.Response; - import io.swagger.annotations.Api; import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiParam; import io.swagger.annotations.ApiResponse; import io.swagger.annotations.ApiResponses; import org.apache.nifi.registry.authorization.Resource; +import org.apache.nifi.registry.exception.ResourceNotFoundException; import org.apache.nifi.registry.security.authorization.Authorizer; import org.apache.nifi.registry.security.authorization.AuthorizerCapabilityDetection; import org.apache.nifi.registry.security.authorization.RequestAction; @@ -48,7 +36,20 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.Consumes; +import javax.ws.rs.DELETE; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.PUT; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; +import javax.ws.rs.Produces; +import javax.ws.rs.core.Context; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; import java.net.URI; +import java.util.Collections; import java.util.List; /** @@ -96,6 +97,7 @@ public class AccessPolicyResource extends AuthorizableApplicationResource { final AccessPolicy requestAccessPolicy) { verifyAuthorizerSupportsConfigurablePolicies(); + authorizeAccess(RequestAction.WRITE); if (requestAccessPolicy == null) { throw new IllegalArgumentException("Access policy details must be specified when creating a new policy."); @@ -108,8 +110,6 @@ public class AccessPolicyResource extends AuthorizableApplicationResource { } RequestAction.valueOfValue(requestAccessPolicy.getAction()); - authorizeAccess(RequestAction.WRITE); - AccessPolicy createdPolicy = authorizationService.createAccessPolicy(requestAccessPolicy); String locationUri = generateAccessPolicyUri(createdPolicy); @@ -132,14 +132,16 @@ public class AccessPolicyResource extends AuthorizableApplicationResource { @ApiResponses({ @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401), @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403), - @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_409), - @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_409) }) + @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) }) public Response getAccessPolicies() { verifyAuthorizerIsManaged(); authorizeAccess(RequestAction.READ); - final List<AccessPolicy> accessPolicies = authorizationService.getAccessPolicies(); + List<AccessPolicy> accessPolicies = authorizationService.getAccessPolicies(); + if (accessPolicies == null) { + accessPolicies = Collections.emptyList(); + } return generateOkResponse(accessPolicies).build(); } @@ -168,9 +170,12 @@ public class AccessPolicyResource extends AuthorizableApplicationResource { @PathParam("id") final String identifier) { verifyAuthorizerIsManaged(); + authorizeAccess(RequestAction.READ); final AccessPolicy accessPolicy = authorizationService.getAccessPolicy(identifier); - authorizeAccess(RequestAction.READ); + if (accessPolicy == null) { + throw new ResourceNotFoundException("No access policy found with ID + " + identifier); + } return generateOkResponse(accessPolicy).build(); } @@ -211,14 +216,16 @@ public class AccessPolicyResource extends AuthorizableApplicationResource { final String rawResource) { verifyAuthorizerIsManaged(); + authorizeAccess(RequestAction.READ); // parse the action and resource type final RequestAction requestAction = RequestAction.valueOfValue(action); final String resource = "/" + rawResource; - authorizeAccess(RequestAction.READ); - AccessPolicy accessPolicy = authorizationService.getAccessPolicy(resource, requestAction); + if (accessPolicy == null) { + throw new ResourceNotFoundException("No policy found for action='" + action + "', resource='" + resource + "'"); + } return generateOkResponse(accessPolicy).build(); } @@ -255,6 +262,7 @@ public class AccessPolicyResource extends AuthorizableApplicationResource { final AccessPolicy requestAccessPolicy) { verifyAuthorizerSupportsConfigurablePolicies(); + authorizeAccess(RequestAction.WRITE); if (requestAccessPolicy == null) { throw new IllegalArgumentException("Access policy details must be specified when updating a policy."); @@ -264,8 +272,6 @@ public class AccessPolicyResource extends AuthorizableApplicationResource { + "policy id of the requested resource (%s).", requestAccessPolicy.getIdentifier(), identifier)); } - authorizeAccess(RequestAction.WRITE); - AccessPolicy createdPolicy = authorizationService.updateAccessPolicy(requestAccessPolicy); String locationUri = generateAccessPolicyUri(createdPolicy); @@ -302,6 +308,9 @@ public class AccessPolicyResource extends AuthorizableApplicationResource { verifyAuthorizerSupportsConfigurablePolicies(); authorizeAccess(RequestAction.DELETE); AccessPolicy deletedPolicy = authorizationService.deleteAccessPolicy(identifier); + if (deletedPolicy == null) { + throw new ResourceNotFoundException("No access policy found with ID + " + identifier); + } return generateOkResponse(deletedPolicy).build(); } http://git-wip-us.apache.org/repos/asf/nifi-registry/blob/f2d712f3/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/TenantResource.java ---------------------------------------------------------------------- diff --git a/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/TenantResource.java b/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/TenantResource.java index d9cb66a..07db72b 100644 --- a/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/TenantResource.java +++ b/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/TenantResource.java @@ -22,6 +22,7 @@ import io.swagger.annotations.ApiParam; import io.swagger.annotations.ApiResponse; import io.swagger.annotations.ApiResponses; import org.apache.commons.lang3.StringUtils; +import org.apache.nifi.registry.exception.ResourceNotFoundException; import org.apache.nifi.registry.security.authorization.Authorizer; import org.apache.nifi.registry.security.authorization.AuthorizerCapabilityDetection; import org.apache.nifi.registry.security.authorization.RequestAction; @@ -180,6 +181,9 @@ public class TenantResource extends AuthorizableApplicationResource { authorizeAccess(RequestAction.READ); final User user = authorizationService.getUser(identifier); + if (user == null) { + throw new ResourceNotFoundException("No user found with identifier " + identifier); + } return generateOkResponse(user).build(); } @@ -214,6 +218,7 @@ public class TenantResource extends AuthorizableApplicationResource { final User requestUser) { verifyAuthorizerSupportsConfigurableUserGroups(); + authorizeAccess(RequestAction.WRITE); if (requestUser == null) { throw new IllegalArgumentException("User details must be specified when updating a user."); @@ -223,9 +228,11 @@ public class TenantResource extends AuthorizableApplicationResource { + "user id of the requested resource (%s).", requestUser.getIdentifier(), identifier)); } - authorizeAccess(RequestAction.WRITE); - final User updatedUser = authorizationService.updateUser(requestUser); + if (updatedUser == null) { + throw new ResourceNotFoundException("No user found with identifier " + identifier); + } + return generateOkResponse(updatedUser).build(); } @@ -260,6 +267,9 @@ public class TenantResource extends AuthorizableApplicationResource { authorizeAccess(RequestAction.DELETE); final User user = authorizationService.deleteUser(identifier); + if (user == null) { + throw new ResourceNotFoundException("No user found with identifier " + identifier); + } return generateOkResponse(user).build(); } @@ -296,6 +306,7 @@ public class TenantResource extends AuthorizableApplicationResource { ) final UserGroup requestUserGroup) { verifyAuthorizerSupportsConfigurableUserGroups(); + authorizeAccess(RequestAction.WRITE); if (requestUserGroup == null) { throw new IllegalArgumentException("User group details must be specified when creating a new group."); @@ -307,11 +318,9 @@ public class TenantResource extends AuthorizableApplicationResource { throw new IllegalArgumentException("User group identity must be specified when creating a new group."); } - authorizeAccess(RequestAction.WRITE); - UserGroup createdGroup = authorizationService.createUserGroup(requestUserGroup); - String locationUri = generateUserGroupUri(createdGroup); + String locationUri = generateUserGroupUri(createdGroup); return generateCreatedResponse(URI.create(locationUri), createdGroup).build(); } @@ -372,6 +381,10 @@ public class TenantResource extends AuthorizableApplicationResource { authorizeAccess(RequestAction.READ); final UserGroup userGroup = authorizationService.getUserGroup(identifier); + if (userGroup == null) { + throw new ResourceNotFoundException("No group found with identifier " + identifier); + } + return generateOkResponse(userGroup).build(); } @@ -418,6 +431,10 @@ public class TenantResource extends AuthorizableApplicationResource { authorizeAccess(RequestAction.WRITE); UserGroup updatedUserGroup = authorizationService.updateUserGroup(requestUserGroup); + if (updatedUserGroup == null) { + throw new ResourceNotFoundException("No group found with identifier " + identifier); + } + return generateOkResponse(updatedUserGroup).build(); } @@ -452,6 +469,10 @@ public class TenantResource extends AuthorizableApplicationResource { authorizeAccess(RequestAction.DELETE); final UserGroup userGroup = authorizationService.deleteUserGroup(identifier); + if (userGroup == null) { + throw new ResourceNotFoundException("No group found with identifier " + identifier); + } + return generateOkResponse(userGroup).build(); }
