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.
---
