Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Merge branch 'feature/acl-unauthorized-response' into 'release'

Browse Source 
Correcting the error response returned by the AclFilter to match the ErrorDTO...

Correcting the error response returned by the AclFilter to match the ErrorDTO returned from controller functions

This ensures consistent responses from different parts of our application. The client can then rely on the server to provide a constistent response structure.

See merge request theappsmith/internal-tools-server!194
This commit is contained in:
Arpit Mohan 2020-02-07 10:15:48 +00:00
commit b28073cd4b
3 changed files with 65 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java
View File

@ -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}"),

45
app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java
View File

@ -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<ErrorDTO> object before
* sending it to the client.
@ -51,43 +53,57 @@ public class GlobalExceptionHandler {
@ExceptionHandler
@ResponseBody
public Mono<ResponseDTO<ErrorDTO>> 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<ResponseDTO<ErrorDTO>> 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<ResponseDTO<ErrorDTO>> 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<ResponseDTO<ErrorDTO>> 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<ResponseDTO<ErrorDTO>> 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<ResponseDTO<ErrorDTO>> 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())));
}
}

38
app/server/appsmith-server/src/main/java/com/appsmith/server/filters/AclFilter.java
View File

@ -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<ErrorDTO> 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());
}
});
}
}