diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java index 2eba7a4227..59e07a05aa 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java @@ -30,7 +30,7 @@ public enum AppsmithError { JSON_PROCESSING_ERROR(400, 4022, "Json processing error with error {0}"), INVALID_CREDENTIALS(200, 4023, "Invalid credentials provided. Did you input the credentials correctly?"), DUPLICATE_KEY(409, 4024, "Duplicate key error"), - UNAUTHORIZED_ACCESS(401, 4025, "Unauthorized access"), + UNAUTHORIZED_ACCESS(403, 4025, "Unauthorized access"), INVALID_DATASOURCE_NAME(400, 4026, "Datasource name is invalid"), NO_RESOURCE_FOUND(404, 4027, "Unable to find {0} with id {1}"), GENERIC_BAD_REQUEST(400, 4028, "Bad Request: {0}"), diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java index 5c484bcb66..9260b66513 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java @@ -7,6 +7,7 @@ import com.segment.analytics.Analytics; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; +import org.springframework.security.access.AccessDeniedException; import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.ResponseBody; @@ -14,6 +15,7 @@ import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.ServerWebInputException; import reactor.core.publisher.Mono; + /** * This class catches all the Exceptions and formats them into a proper ResponseDTO object before * sending it to the client. @@ -51,43 +53,57 @@ public class GlobalExceptionHandler { @ExceptionHandler @ResponseBody public Mono> catchDuplicateKeyException(org.springframework.dao.DuplicateKeyException e, ServerWebExchange exchange) { - exchange.getResponse().setStatusCode(HttpStatus.BAD_REQUEST); + AppsmithError appsmithError = AppsmithError.DUPLICATE_KEY; + exchange.getResponse().setStatusCode(HttpStatus.resolve(appsmithError.getHttpErrorCode())); log.error("", e); rollbar.log(e); - return Mono.just(new ResponseDTO<>(HttpStatus.BAD_REQUEST.value(), new ErrorDTO(AppsmithError.DUPLICATE_KEY.getAppErrorCode(), - AppsmithError.DUPLICATE_KEY.getMessage(e.getMessage())))); + return Mono.just(new ResponseDTO<>(appsmithError.getHttpErrorCode(), new ErrorDTO(appsmithError.getAppErrorCode(), + appsmithError.getMessage(e.getMessage())))); } @ExceptionHandler @ResponseBody public Mono> catchTimeoutException(java.util.concurrent.TimeoutException e, ServerWebExchange exchange) { - exchange.getResponse().setStatusCode(HttpStatus.GATEWAY_TIMEOUT); + AppsmithError appsmithError = AppsmithError.PLUGIN_EXECUTION_TIMEOUT; + exchange.getResponse().setStatusCode(HttpStatus.resolve(appsmithError.getHttpErrorCode())); log.error("", e); rollbar.log(e); - return Mono.just(new ResponseDTO<>(HttpStatus.GATEWAY_TIMEOUT.value(), new ErrorDTO(AppsmithError.PLUGIN_EXECUTION_TIMEOUT.getHttpErrorCode(), - AppsmithError.PLUGIN_EXECUTION_TIMEOUT.getMessage()))); + return Mono.just(new ResponseDTO<>(appsmithError.getHttpErrorCode(), new ErrorDTO(appsmithError.getAppErrorCode(), + appsmithError.getMessage()))); } @ExceptionHandler @ResponseBody public Mono> catchServerWebInputException(ServerWebInputException e, ServerWebExchange exchange) { - exchange.getResponse().setStatusCode(HttpStatus.BAD_REQUEST); + AppsmithError appsmithError = AppsmithError.GENERIC_BAD_REQUEST; + exchange.getResponse().setStatusCode(HttpStatus.resolve(appsmithError.getHttpErrorCode())); log.error("", e); rollbar.log(e); - return Mono.just(new ResponseDTO<>(HttpStatus.BAD_REQUEST.value(), new ErrorDTO(AppsmithError.GENERIC_BAD_REQUEST.getHttpErrorCode(), - AppsmithError.GENERIC_BAD_REQUEST.getMessage(e.getReason())))); + return Mono.just(new ResponseDTO<>(appsmithError.getHttpErrorCode(), new ErrorDTO(appsmithError.getAppErrorCode(), + appsmithError.getMessage(e.getReason())))); } @ExceptionHandler @ResponseBody public Mono> catchPluginException(AppsmithPluginException e, ServerWebExchange exchange) { - exchange.getResponse().setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR); + AppsmithError appsmithError = AppsmithError.INTERNAL_SERVER_ERROR; + exchange.getResponse().setStatusCode(HttpStatus.resolve(appsmithError.getHttpErrorCode())); log.error("", e); rollbar.log(e); - return Mono.just(new ResponseDTO<>(HttpStatus.INTERNAL_SERVER_ERROR.value(), new ErrorDTO(AppsmithError.INTERNAL_SERVER_ERROR.getHttpErrorCode(), + return Mono.just(new ResponseDTO<>(appsmithError.getHttpErrorCode(), new ErrorDTO(appsmithError.getAppErrorCode(), e.getLocalizedMessage()))); } + @ExceptionHandler + @ResponseBody + public Mono> catchAccessDeniedException(AccessDeniedException e, ServerWebExchange exchange) { + AppsmithError appsmithError = AppsmithError.UNAUTHORIZED_ACCESS; + exchange.getResponse().setStatusCode(HttpStatus.resolve(appsmithError.getHttpErrorCode())); + log.error("", e); + rollbar.log(e); + return Mono.just(new ResponseDTO<>(appsmithError.getHttpErrorCode(), new ErrorDTO(appsmithError.getAppErrorCode(), + appsmithError.getMessage()))); + } /** * This function catches the generic Exception class and is meant to be a catch all to ensure that we don't leak * any information to the client. Ideally, the function #catchAppsmithException should be used @@ -99,10 +115,11 @@ public class GlobalExceptionHandler { @ExceptionHandler @ResponseBody public Mono> catchException(Exception e, ServerWebExchange exchange) { - exchange.getResponse().setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR); + AppsmithError appsmithError = AppsmithError.INTERNAL_SERVER_ERROR; + exchange.getResponse().setStatusCode(HttpStatus.resolve(appsmithError.getHttpErrorCode())); log.error("", e); rollbar.log(e); - return Mono.just(new ResponseDTO<>(HttpStatus.INTERNAL_SERVER_ERROR.value(), new ErrorDTO(AppsmithError.INTERNAL_SERVER_ERROR.getHttpErrorCode(), - AppsmithError.INTERNAL_SERVER_ERROR.getMessage()))); + return Mono.just(new ResponseDTO<>(appsmithError.getHttpErrorCode(), new ErrorDTO(appsmithError.getAppErrorCode(), + appsmithError.getMessage()))); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AclFilter.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AclFilter.java index 6338358023..fd09f64a45 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AclFilter.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AclFilter.java @@ -2,11 +2,17 @@ package com.appsmith.server.filters; import com.appsmith.server.acl.AclService; import com.appsmith.server.acl.OpaResponse; +import com.appsmith.server.dtos.ResponseDTO; import com.appsmith.server.exceptions.AppsmithError; -import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.exceptions.ErrorDTO; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.core.io.buffer.DataBuffer; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.stereotype.Component; import org.springframework.web.server.ServerWebExchange; @@ -19,10 +25,12 @@ import reactor.core.publisher.Mono; public class AclFilter implements WebFilter { final AclService aclService; + final ObjectMapper objectMapper; @Autowired - public AclFilter(AclService aclService) { + public AclFilter(AclService aclService, ObjectMapper objectMapper) { this.aclService = aclService; + this.objectMapper = objectMapper; } /** @@ -62,8 +70,28 @@ public class AclFilter implements WebFilter { log.debug("Got ACL response: {}", acl); return acl; }) - .filter(acl -> acl.getResult()) - .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.UNAUTHORIZED_ACCESS))) - .flatMap(acl -> chain.filter(exchange)); + .flatMap(acl -> { + if (acl != null && acl.getResult()) { + // Acl returned true. Continue with the filter chain + return chain.filter(exchange); + } + + // The Acl response is false. Return unauthorized exception to the client + // We construct the error response JSON here because throwing an exception here doesn't get caught + // in the {@see GlobalExceptionHandler}. + AppsmithError error = AppsmithError.UNAUTHORIZED_ACCESS; + exchange.getResponse().setStatusCode(HttpStatus.resolve(error.getHttpErrorCode())); + exchange.getResponse().getHeaders().setContentType(MediaType.APPLICATION_JSON); + try { + ResponseDTO responseBody = new ResponseDTO<>(error.getHttpErrorCode(), new ErrorDTO(error.getAppErrorCode(), + error.getMessage())); + String responseStr = objectMapper.writeValueAsString(responseBody); + DataBuffer buffer = exchange.getResponse().bufferFactory().allocateBuffer().write(responseStr.getBytes()); + return exchange.getResponse().writeWith(Mono.just(buffer)); + } catch (JsonProcessingException e) { + log.error("Exception caught while serializing JSON in AclFilter. Cause: ", e); + return exchange.getResponse().writeWith(Mono.empty()); + } + }); } }