Uploaded image for project: 'Nuxeo Platform'
  1. Nuxeo Platform
  2. NXP-12493

Automation batch execution returns HTTP 500 instead of 403 in case of a DocumentSecurityException

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 5.7.3, 5.7.3-SNAPSHOT
    • Fix Version/s: 7.2
    • Component/s: Automation
    • Tags:
    • Backlog priority:
      350

      Description

      For example when executing the NuxeoDrive.CreateFile operation to create a file in a folder on which the READ permission has been denied for the connected user during the file batch upload.
      This is the culprit code in BatchResource#exec(@Context HttpServletRequest request, ExecutionRequest xreq):

      try {
                  OperationContext ctx = xreq.createContext(request,
                          getCoreSession(request));
      
                  Object result;
                  if (StringUtils.isEmpty(fileIdx)) {
                      result = bm.execute(batchId, operationId,
                              getCoreSession(request), ctx, params);
                  } else {
                      result = bm.execute(batchId, fileIdx, operationId,
                              getCoreSession(request), ctx, params);
                  }
                  return ResponseHelper.getResponse(result, request);
              } catch (Exception e) {
                  log.error("Error while executing automation batch ", e);
                  if (ExceptionHandler.isSecurityError(e)) {
                      return Response.status(Status.FORBIDDEN).entity(
                              "{\"error\" : \"" + e.getMessage() + "\"}").build();
                  } else {
                      return Response.status(Status.INTERNAL_SERVER_ERROR).entity(
                              "{\"error\" : \"" + e.getMessage() + "\"}").build();
                  }
              }
      

      because:

      • bm.execute throws a TraceException caused by a DocumentSecurityException which is fine
      • but ExceptionHandler.isSecurityError(e) returns false because the status bound to DocumentSecurityException is 403 and not 401

      There are several things here:

      1. Does ExceptionHandler.isSecurityError(e) really make sense, if it checks for a 401 in which case a 403 is returned by the BatchResource?
      2. We should remove the try {...} catch {...} block to let the exception bubble.
      3. In this case, the WebEngineExceptionMapper will handle the exception, but for now it doesn't manage the (exception type) <--> (HTTP status code) mapping well because:
        • It doesn't recurse on the exception causes to find the root one, as ExceptionHandler does (see WebException.wrap()).
        • Mapping is not systematic, for instance WebResourceNotFoundException is instantiated with the right status code: 404, but WebSecurityException is not (no 403, so default 500 is used).
          => We should process recursion to use the root cause and review the WebException implementations to handle the most common cases, especially 403.
          => Then see how to factorize WebEngineExceptionMapper and ExceptionHandler.
          => Finally we should make this pluggable
      4. In OperationServiceImpl#run(OperationContext ctx, OperationType operationType, Map<String, Object> params), instead of throwing a TraceException, we should throw an OperationException using a setter to set the tracer.

        Attachments

          Issue Links

            Activity

              People

              • Votes:
                5 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  PagerDuty

                  Error rendering 'com.pagerduty.jira-server-plugin:PagerDuty'. Please contact your Jira administrators.