Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/108#discussion_r180112539 --- Diff: nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/FlowResource.java --- @@ -62,4 +85,214 @@ public Response getAvailableFlowFields() { return Response.status(Response.Status.OK).entity(fields).build(); } + @GET + @Path("{flowId}") + @Consumes(MediaType.WILDCARD) + @Produces(MediaType.APPLICATION_JSON) + @ApiOperation( + value = "Gets a flow", + response = VersionedFlow.class, + extensions = { + @Extension(name = "access-policy", properties = { + @ExtensionProperty(name = "action", value = "read"), + @ExtensionProperty(name = "resource", value = "/buckets/{bucketId}") }) + } + ) + @ApiResponses({ + @ApiResponse(code = 400, message = HttpStatusMessages.MESSAGE_400), + @ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401), + @ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403), + @ApiResponse(code = 404, message = HttpStatusMessages.MESSAGE_404), + @ApiResponse(code = 409, message = HttpStatusMessages.MESSAGE_409) }) + public Response getFlow( + @PathParam("flowId") + @ApiParam("The flow identifier") + final String flowId) { + + final VersionedFlow flow = registryService.getFlow(flowId); + + // this should never happen, but if somehow the back-end didn't populate the bucket id let's make sure the flow isn't returned + if (StringUtils.isBlank(flow.getBucketIdentifier())) { + throw new IllegalStateException("Unable to authorize access because bucket identifier is null or blank"); + } + + authorizeBucketAccess(RequestAction.READ, flow.getBucketIdentifier()); --- End diff -- When I try this endpoint with an unauthorized user, I get the following response back: ``` HTTP/1.1 403 Forbidden Connection: close Date: Mon, 09 Apr 2018 14:08:27 GMT Content-Type: text/plain Content-Length: 101 Server: Jetty(9.4.3.v20170317) Unable to view Bucket with ID 6eaeae9c-dbdb-4af3-a98e-4f3b880a0fb2. Contact the system administrator. ``` The 403 status code is good, but I'm not sure about the error message in the response. If someone is attempting to access the flow through /flows/{id}, I don't think the server should return the bucket id containing the flow, as that's leaking information the user would not otherwise have access to. It's a fairly harmless piece of information on it's own, but in a multi-tenant scenario it could reveal more than the owner of the bucket would like, especially if correlated with other information an attacker is able to obtain. It's probably not a huge issue, but if you agree, we could strip this by wrapping the call to authorizeBucketAccess() in a try catch that obscures the error message returned in the response body. We would have to do this for all the GET methods in the FlowResource. Something like: ``` try { authorizeBucketAccess(RequestAction.READ, flow.getBucketIdentifier()); } catch (AccessDeniedException e) { throw new AccessDeniedException("User not authorized to view the specified flow"); } By adding the root throwable to the cause of a custom exception, we could even keep the root cause with the bucket id in the logs for easier admin troubleshooting. Now that I think about it, that approach for customizing HTTP response error messages to differ from internal/logged error message probably be a better approach than the solution that we came up with in PR #99, which inadvertently introduced a "log & throw" pattern in order to maintain resource ids in the logs.
---