From 575a306e4a7d72f111e9e826bad0726bfc6616da Mon Sep 17 00:00:00 2001 From: Julien Camus Date: Thu, 3 Oct 2024 10:00:01 +0200 Subject: [PATCH] SONAR-23082 Improve exception handling in API --- .../server/exceptions/NotFoundException.java | 16 +- .../sonar/server/v2/common/ErrorMessages.java | 52 +++ .../RestResponseEntityExceptionHandler.java | 179 +++++++--- ...estResponseEntityExceptionHandlerTest.java | 323 +++++++++++++++++- .../server/permission/ws/AddUserActionIT.java | 27 ++ 5 files changed, 527 insertions(+), 70 deletions(-) create mode 100644 server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/common/ErrorMessages.java diff --git a/server/sonar-webserver-api/src/main/java/org/sonar/server/exceptions/NotFoundException.java b/server/sonar-webserver-api/src/main/java/org/sonar/server/exceptions/NotFoundException.java index 8785a310f86..4aec7e74d08 100644 --- a/server/sonar-webserver-api/src/main/java/org/sonar/server/exceptions/NotFoundException.java +++ b/server/sonar-webserver-api/src/main/java/org/sonar/server/exceptions/NotFoundException.java @@ -19,6 +19,7 @@ */ package org.sonar.server.exceptions; +import java.util.Objects; import javax.annotation.Nullable; import static java.lang.String.format; @@ -31,15 +32,13 @@ public class NotFoundException extends ServerException { } /** - * @throws NotFoundException if the value if null + * @throws NotFoundException if the value is null * @return the value */ public static T checkFound(@Nullable T value, String message, Object... messageArguments) { - if (value == null) { + return Objects.requireNonNullElseGet(value, () -> { throw new NotFoundException(format(message, messageArguments)); - } - - return value; + }); } /** @@ -47,10 +46,7 @@ public class NotFoundException extends ServerException { * @return the value */ public static T checkFoundWithOptional(java.util.Optional value, String message, Object... messageArguments) { - if (!value.isPresent()) { - throw new NotFoundException(format(message, messageArguments)); - } - - return value.get(); + return value.orElseThrow(() -> new NotFoundException(format(message, messageArguments))); } + } diff --git a/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/common/ErrorMessages.java b/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/common/ErrorMessages.java new file mode 100644 index 00000000000..e87420dbcef --- /dev/null +++ b/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/common/ErrorMessages.java @@ -0,0 +1,52 @@ +/* + * SonarQube + * Copyright (C) 2009-2024 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.v2.common; + +enum ErrorMessages { + ACCESS_DENIED("Access denied."), + AUTHENTICATION_FAILED("Authentication failed."), + BAD_REQUEST("Bad request."), + BIND_ERROR("Bind error."), + CONVERSION_FAILED("Failed to convert value."), + INTERNAL_SERVER_ERROR("An internal server error occurred."), + INVALID_INPUT_PROVIDED("Invalid input provided."), + INVALID_PARAMETER_TYPE("Invalid parameter type."), + INVALID_REQUEST_FORMAT("Invalid request format."), + INVALID_STATE("Invalid state."), + METHOD_NOT_SUPPORTED("Method not supported."), + RESOURCE_NOT_FOUND("Resource not found."), + REQUEST_TIMEOUT("The request timed out."), + SIZE_EXCEEDED("The uploaded file exceeds the maximum allowed size."), + UNEXPECTED_ERROR("An unexpected error occurred. Please contact support if the problem persists."), + UNACCEPTABLE_MEDIA_TYPE("The requested media type is not acceptable."), + UNSUPPORTED_MEDIA_TYPE("Unsupported media type."), + VALIDATION_ERROR("Validation error. Please check your input."), + ; + + private final String message; + + ErrorMessages(String message) { + this.message = message; + } + + public String getMessage() { + return message; + } +} diff --git a/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandler.java b/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandler.java index 1f7191db257..5496c38d666 100644 --- a/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandler.java +++ b/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandler.java @@ -19,27 +19,32 @@ */ package org.sonar.server.v2.common; +import java.io.FileNotFoundException; +import java.nio.file.AccessDeniedException; +import java.util.NoSuchElementException; import java.util.Optional; import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.exceptions.ForbiddenException; -import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.authentication.event.AuthenticationException; import org.sonar.server.exceptions.ServerException; -import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.v2.api.model.RestError; +import org.springframework.core.convert.ConversionFailedException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.validation.BindException; import org.springframework.validation.FieldError; +import org.springframework.web.HttpMediaTypeNotAcceptableException; +import org.springframework.web.HttpMediaTypeNotSupportedException; import org.springframework.web.HttpRequestMethodNotSupportedException; +import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.ServletRequestBindingException; import org.springframework.web.bind.annotation.ExceptionHandler; -import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.bind.annotation.RestControllerAdvice; +import org.springframework.web.context.request.async.AsyncRequestTimeoutException; import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; +import org.springframework.web.multipart.MaxUploadSizeExceededException; import org.springframework.web.servlet.NoHandlerFoundException; @RestControllerAdvice @@ -47,64 +52,152 @@ public class RestResponseEntityExceptionHandler { private static final Logger LOGGER = LoggerFactory.getLogger(RestResponseEntityExceptionHandler.class); - @ExceptionHandler({IllegalStateException.class}) - @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) - protected ResponseEntity handleIllegalStateException(IllegalStateException illegalStateException) { - return new ResponseEntity<>(new RestError(illegalStateException.getMessage()), HttpStatus.INTERNAL_SERVER_ERROR); + // region client + + @ExceptionHandler(HttpMessageNotReadableException.class) + protected ResponseEntity handleHttpMessageNotReadableException(HttpMessageNotReadableException ex) { + LOGGER.error(ErrorMessages.INVALID_REQUEST_FORMAT.getMessage(), ex); + return buildResponse(HttpStatus.BAD_REQUEST, ErrorMessages.INVALID_REQUEST_FORMAT); + } + + @ExceptionHandler(MethodArgumentNotValidException.class) + protected ResponseEntity handleMethodArgumentNotValidException(MethodArgumentNotValidException ex) { + LOGGER.error(ErrorMessages.VALIDATION_ERROR.getMessage(), ex); + String validationErrors = ex.getFieldErrors().stream() + .map(RestResponseEntityExceptionHandler::handleFieldError) + .collect(Collectors.joining()); + return buildResponse(HttpStatus.BAD_REQUEST, validationErrors); + } + + @ExceptionHandler(MethodArgumentTypeMismatchException.class) + protected ResponseEntity handleMethodArgumentTypeMismatchException(MethodArgumentTypeMismatchException ex) { + LOGGER.error(ErrorMessages.INVALID_PARAMETER_TYPE.getMessage(), ex); + return buildResponse(HttpStatus.BAD_REQUEST, ErrorMessages.INVALID_PARAMETER_TYPE); + } + + @ExceptionHandler(ConversionFailedException.class) + protected ResponseEntity handleConversionFailedException(ConversionFailedException ex) { + LOGGER.error(ErrorMessages.CONVERSION_FAILED.getMessage(), ex); + return buildResponse(HttpStatus.BAD_REQUEST, ErrorMessages.CONVERSION_FAILED); + } + + @ExceptionHandler(HttpRequestMethodNotSupportedException.class) + protected ResponseEntity handleHttpRequestMethodNotSupportedException(HttpRequestMethodNotSupportedException ex) { + LOGGER.error(ErrorMessages.METHOD_NOT_SUPPORTED.getMessage(), ex); + return buildResponse(HttpStatus.METHOD_NOT_ALLOWED, ErrorMessages.METHOD_NOT_SUPPORTED); + } + + @ExceptionHandler(HttpMediaTypeNotSupportedException.class) + protected ResponseEntity handleHttpMediaTypeNotSupportedException(HttpMediaTypeNotSupportedException ex) { + LOGGER.error(ErrorMessages.UNSUPPORTED_MEDIA_TYPE.getMessage(), ex); + return buildResponse(HttpStatus.UNSUPPORTED_MEDIA_TYPE, ErrorMessages.UNSUPPORTED_MEDIA_TYPE); + } + + @ExceptionHandler(HttpMediaTypeNotAcceptableException.class) + protected ResponseEntity handleHttpMediaTypeNotAcceptableException(HttpMediaTypeNotAcceptableException ex) { + LOGGER.error(ErrorMessages.UNACCEPTABLE_MEDIA_TYPE.getMessage(), ex); + return buildResponse(HttpStatus.NOT_ACCEPTABLE, ErrorMessages.UNACCEPTABLE_MEDIA_TYPE); } - @ExceptionHandler({IllegalArgumentException.class}) - @ResponseStatus(HttpStatus.BAD_REQUEST) - protected ResponseEntity handleIllegalArgumentException(IllegalArgumentException illegalArgumentException) { - return new ResponseEntity<>(new RestError(illegalArgumentException.getMessage()), HttpStatus.BAD_REQUEST); + @ExceptionHandler(IllegalArgumentException.class) + protected ResponseEntity handleIllegalArgumentException(IllegalArgumentException ex) { + LOGGER.error(ErrorMessages.INVALID_INPUT_PROVIDED.getMessage(), ex); + return buildResponse(HttpStatus.BAD_REQUEST, ex.getMessage()); } @ExceptionHandler(BindException.class) - @ResponseStatus(HttpStatus.BAD_REQUEST) - protected ResponseEntity handleBindException(BindException bindException) { - String validationErrors = bindException.getFieldErrors().stream() + protected ResponseEntity handleBindException(BindException ex) { + LOGGER.error(ErrorMessages.BIND_ERROR.getMessage(), ex); + String validationErrors = ex.getFieldErrors().stream() .map(RestResponseEntityExceptionHandler::handleFieldError) .collect(Collectors.joining()); - return new ResponseEntity<>(new RestError(validationErrors), HttpStatus.BAD_REQUEST); + return buildResponse(HttpStatus.BAD_REQUEST, validationErrors); } - private static String handleFieldError(FieldError fieldError) { - String fieldName = fieldError.getField(); - String rejectedValueAsString = Optional.ofNullable(fieldError.getRejectedValue()).map(Object::toString).orElse("{}"); - String defaultMessage = fieldError.getDefaultMessage(); - return String.format("Value %s for field %s was rejected. Error: %s.", rejectedValueAsString, fieldName, defaultMessage); + @ExceptionHandler({ + NoSuchElementException.class, + ServletRequestBindingException.class, + }) + protected ResponseEntity handleBadRequests(Exception ex) { + LOGGER.error(ErrorMessages.BAD_REQUEST.getMessage(), ex); + return buildResponse(HttpStatus.BAD_REQUEST, ErrorMessages.BAD_REQUEST); } - @ExceptionHandler({ServerException.class, ForbiddenException.class, UnauthorizedException.class, BadRequestException.class}) - protected ResponseEntity handleServerException(ServerException serverException) { - return new ResponseEntity<>(new RestError(serverException.getMessage()), - Optional.ofNullable(HttpStatus.resolve(serverException.httpCode())).orElse(HttpStatus.INTERNAL_SERVER_ERROR)); + // endregion client + + // region security + + @ExceptionHandler(AccessDeniedException.class) + protected ResponseEntity handleAccessDeniedException(AccessDeniedException ex) { + LOGGER.error(ErrorMessages.ACCESS_DENIED.getMessage(), ex); + return buildResponse(HttpStatus.FORBIDDEN, ErrorMessages.ACCESS_DENIED); } - @ExceptionHandler({NotFoundException.class}) - @ResponseStatus(HttpStatus.NOT_FOUND) - protected ResponseEntity handleNotFoundException(NotFoundException notFoundException) { - return new ResponseEntity<>(new RestError(notFoundException.getMessage()), HttpStatus.NOT_FOUND); + // endregion security + + // region server + + @ExceptionHandler(IllegalStateException.class) + protected ResponseEntity handleIllegalStateException(IllegalStateException ex) { + LOGGER.error(ErrorMessages.INVALID_STATE.getMessage(), ex); + return buildResponse(HttpStatus.INTERNAL_SERVER_ERROR, ex.getMessage()); } @ExceptionHandler({ - HttpMessageNotReadableException.class, - MethodArgumentTypeMismatchException.class, - HttpRequestMethodNotSupportedException.class, - ServletRequestBindingException.class, - NoHandlerFoundException.class + FileNotFoundException.class, + NoHandlerFoundException.class, }) - @ResponseStatus(HttpStatus.BAD_REQUEST) - protected ResponseEntity handleClientSideException(Exception exception) { - LOGGER.error("Error processing request", exception); - return new ResponseEntity<>(new RestError("An error occurred while processing the request."), HttpStatus.BAD_REQUEST); + protected ResponseEntity handleResourceNotFoundException(Exception ex) { + LOGGER.error(ErrorMessages.RESOURCE_NOT_FOUND.getMessage(), ex); + return buildResponse(HttpStatus.NOT_FOUND, ErrorMessages.RESOURCE_NOT_FOUND); + } + + @ExceptionHandler(AsyncRequestTimeoutException.class) + protected ResponseEntity handleAsyncRequestTimeoutException(AsyncRequestTimeoutException ex) { + LOGGER.error(ErrorMessages.REQUEST_TIMEOUT.getMessage(), ex); + return buildResponse(HttpStatus.SERVICE_UNAVAILABLE, ErrorMessages.REQUEST_TIMEOUT); + } + + @ExceptionHandler(MaxUploadSizeExceededException.class) + protected ResponseEntity handleMaxUploadSizeExceededException(MaxUploadSizeExceededException ex) { + LOGGER.error(ErrorMessages.SIZE_EXCEEDED.getMessage(), ex); + return buildResponse(HttpStatus.PAYLOAD_TOO_LARGE, ErrorMessages.SIZE_EXCEEDED); + } + + @ExceptionHandler(AuthenticationException.class) + protected ResponseEntity handleAuthenticationException(AuthenticationException ex) { + LOGGER.error(ErrorMessages.AUTHENTICATION_FAILED.getMessage(), ex); + return buildResponse(HttpStatus.UNAUTHORIZED, ErrorMessages.AUTHENTICATION_FAILED); + } + + @ExceptionHandler(ServerException.class) + protected ResponseEntity handleServerException(ServerException ex) { + final HttpStatus httpStatus = Optional.ofNullable(HttpStatus.resolve(ex.httpCode())).orElse(HttpStatus.INTERNAL_SERVER_ERROR); + final String errorMessage = Optional.ofNullable(ex.getMessage()).orElse(ErrorMessages.INTERNAL_SERVER_ERROR.getMessage()); + return buildResponse(httpStatus, errorMessage); } + // endregion server + @ExceptionHandler({Exception.class}) - @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) - protected ResponseEntity handleUnhandledException(Exception exception) { - LOGGER.error("Unhandled exception", exception); - return new ResponseEntity<>(new RestError("An unexpected error occurred. Please contact support if the problem persists."), HttpStatus.INTERNAL_SERVER_ERROR); + protected ResponseEntity handleUnhandledException(Exception ex) { + LOGGER.error(ErrorMessages.UNEXPECTED_ERROR.getMessage(), ex); + return buildResponse(HttpStatus.INTERNAL_SERVER_ERROR, ErrorMessages.UNEXPECTED_ERROR); + } + + private static ResponseEntity buildResponse(HttpStatus httpStatus, ErrorMessages errorMessages) { + return buildResponse(httpStatus, errorMessages.getMessage()); + } + + private static ResponseEntity buildResponse(HttpStatus httpStatus, String errorMessage) { + return ResponseEntity.status(httpStatus).body(new RestError(errorMessage)); + } + + private static String handleFieldError(FieldError fieldError) { + String fieldName = fieldError.getField(); + String rejectedValueAsString = Optional.ofNullable(fieldError.getRejectedValue()).map(Object::toString).orElse("{}"); + String defaultMessage = fieldError.getDefaultMessage(); + return String.format("Value %s for field %s was rejected. Error: %s.", rejectedValueAsString, fieldName, defaultMessage); } } diff --git a/server/sonar-webserver-webapi-v2/src/test/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandlerTest.java b/server/sonar-webserver-webapi-v2/src/test/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandlerTest.java index ba469536e8e..14384ec9309 100644 --- a/server/sonar-webserver-webapi-v2/src/test/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandlerTest.java +++ b/server/sonar-webserver-webapi-v2/src/test/java/org/sonar/server/v2/common/RestResponseEntityExceptionHandlerTest.java @@ -19,55 +19,344 @@ */ package org.sonar.server.v2.common; +import java.io.FileNotFoundException; +import java.io.InputStream; +import java.nio.file.AccessDeniedException; +import java.util.List; +import java.util.NoSuchElementException; import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.slf4j.event.Level; +import org.sonar.api.testfixtures.log.LogTesterJUnit5; +import org.sonar.server.authentication.event.AuthenticationEvent; +import org.sonar.server.authentication.event.AuthenticationException; +import org.sonar.server.exceptions.BadConfigurationException; +import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.exceptions.ServerException; +import org.sonar.server.exceptions.TemplateMatchingKeyException; +import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.v2.api.model.RestError; -import org.springframework.http.HttpInputMessage; +import org.springframework.core.MethodParameter; +import org.springframework.core.convert.ConversionFailedException; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.http.converter.HttpMessageNotReadableException; +import org.springframework.mock.http.MockHttpInputMessage; +import org.springframework.validation.BindException; +import org.springframework.validation.FieldError; +import org.springframework.web.HttpMediaTypeNotAcceptableException; +import org.springframework.web.HttpMediaTypeNotSupportedException; import org.springframework.web.HttpRequestMethodNotSupportedException; +import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.ServletRequestBindingException; +import org.springframework.web.context.request.async.AsyncRequestTimeoutException; import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; +import org.springframework.web.multipart.MaxUploadSizeExceededException; import org.springframework.web.servlet.NoHandlerFoundException; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; class RestResponseEntityExceptionHandlerTest { private final RestResponseEntityExceptionHandler underTest = new RestResponseEntityExceptionHandler(); + @RegisterExtension + private final LogTesterJUnit5 logs = new LogTesterJUnit5(); + + // region client + + @Test + void handleHttpMessageNotReadableException_shouldReturnBadRequest() { + var ex = new HttpMessageNotReadableException("Invalid format", new MockHttpInputMessage(InputStream.nullInputStream())); + ResponseEntity response = underTest.handleHttpMessageNotReadableException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.INVALID_REQUEST_FORMAT.getMessage()); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.INVALID_REQUEST_FORMAT.getMessage()); + } + + @Test + void handleMethodArgumentNotValidException_shouldReturnBadRequest() { + var fieldError = mock(FieldError.class); + var ex = mock(MethodArgumentNotValidException.class); + + when(fieldError.getDefaultMessage()).thenReturn(""); + when(fieldError.getField()).thenReturn(""); + when(ex.getFieldErrors()).thenReturn(List.of(fieldError)); + + ResponseEntity response = underTest.handleMethodArgumentNotValidException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo("Value {} for field was rejected. Error: ." /* ErrorMessages.VALIDATION_ERROR.getMessage() */); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.VALIDATION_ERROR.getMessage()); + } + + @Test + void handleMethodArgumentTypeMismatchException_shouldReturnBadRequest() { + MethodParameter methodParameter = mock(MethodParameter.class); + Throwable throwable = mock(Throwable.class); + + var ex = new MethodArgumentTypeMismatchException(null, null, "name", methodParameter, throwable); + + ResponseEntity response = underTest.handleMethodArgumentTypeMismatchException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.INVALID_PARAMETER_TYPE.getMessage()); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.INVALID_PARAMETER_TYPE.getMessage()); + } + + @Test + void handleConversionFailedException_shouldReturnBadRequest() { + var ex = new ConversionFailedException(null, TypeDescriptor.valueOf(Byte.class), null, mock(Throwable.class)); + + ResponseEntity response = underTest.handleConversionFailedException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.CONVERSION_FAILED.getMessage()); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.CONVERSION_FAILED.getMessage()); + } + + @Test + void handleHttpRequestMethodNotSupportedException_shouldReturnMethodNotAllowed() { + var ex = new HttpRequestMethodNotSupportedException("GET"); + + ResponseEntity response = underTest.handleHttpRequestMethodNotSupportedException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.METHOD_NOT_ALLOWED); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.METHOD_NOT_SUPPORTED.getMessage()); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.METHOD_NOT_SUPPORTED.getMessage()); + } + + @Test + void handleHttpMediaTypeNotSupportedException_shouldReturnUnsupportedMediaType() { + var ex = new HttpMediaTypeNotSupportedException("Unsupported media type"); + + ResponseEntity response = underTest.handleHttpMediaTypeNotSupportedException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNSUPPORTED_MEDIA_TYPE); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.UNSUPPORTED_MEDIA_TYPE.getMessage()); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.UNSUPPORTED_MEDIA_TYPE.getMessage()); + } + + @Test + void handleHttpMediaTypeNotAcceptableException_shouldReturnNotAcceptable() { + var ex = new HttpMediaTypeNotAcceptableException("Not acceptable"); + + ResponseEntity response = underTest.handleHttpMediaTypeNotAcceptableException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_ACCEPTABLE); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.UNACCEPTABLE_MEDIA_TYPE.getMessage()); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.UNACCEPTABLE_MEDIA_TYPE.getMessage()); + } + + @Test + void handleIllegalArgumentException_shouldReturnBadRequest() { + var ex = new IllegalArgumentException("Invalid argument"); + + ResponseEntity response = underTest.handleIllegalArgumentException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ex.getMessage() /* ErrorMessages.INVALID_INPUT_PROVIDED.getMessage() */); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.INVALID_INPUT_PROVIDED.getMessage()); + } + + @Test + void handleBindException_shouldReturnBadRequest() { + var ex = new BindException(new Object(), "target"); + ex.addError(new FieldError("target", "field", "Field error")); + + ResponseEntity response = underTest.handleBindException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).contains("Value {} for field field was rejected. Error: Field error."); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.BIND_ERROR.getMessage()); + } + + @ParameterizedTest + @MethodSource("badRequestsProvider") + void handleBadRequests_shouldReturnBadRequest(Exception ex) { + ResponseEntity response = underTest.handleBadRequests(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.BAD_REQUEST.getMessage()); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.BAD_REQUEST.getMessage()); + } + + static Stream badRequestsProvider() { + return Stream.of( + Arguments.of(new NoSuchElementException("Element not found")), + Arguments.of(new ServletRequestBindingException("Binding error"))); + } + + // endregion client + + // region security + + @Test + void handleAccessDeniedException_shouldReturnForbidden() { + var ex = new AccessDeniedException("Access denied"); + ResponseEntity response = underTest.handleAccessDeniedException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.ACCESS_DENIED.getMessage()); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.ACCESS_DENIED.getMessage()); + } + + @Test + void handleAuthenticationException_shouldReturnUnauthorized() { + var ex = AuthenticationException.newBuilder() + .setSource(AuthenticationEvent.Source.sso()) + .setLogin("mockLogin") + .setPublicMessage("Authentication failed") + .build(); + ResponseEntity response = underTest.handleAuthenticationException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.AUTHENTICATION_FAILED.getMessage()); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.AUTHENTICATION_FAILED.getMessage()); + } + + // endregion security + + // region server + + @Test + void handleIllegalStateException_shouldReturnInternalServerError() { + var ex = new IllegalStateException(); + ResponseEntity response = underTest.handleIllegalStateException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ex.getMessage() /* ErrorMessages.INVALID_STATE.getMessage() */); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.INVALID_STATE.getMessage()); + } + @ParameterizedTest - @MethodSource("clientSideExceptionsProvider") - void handleClientSideException_shouldUseGenericMessage(Exception exception) { + @MethodSource("resourceNotFoundExceptionProvider") + void handleResourceNotFoundException_shouldReturnNotFound(Exception ex) { + ResponseEntity response = underTest.handleResourceNotFoundException(ex); - ResponseEntity responseEntity = underTest.handleClientSideException(exception); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.RESOURCE_NOT_FOUND.getMessage()); - assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); - assertThat(responseEntity.getBody().message()).isEqualTo("An error occurred while processing the request."); + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.RESOURCE_NOT_FOUND.getMessage()); } - private static Stream clientSideExceptionsProvider() { + static Stream resourceNotFoundExceptionProvider() { return Stream.of( - Arguments.of(new HttpMessageNotReadableException("", (HttpInputMessage) null)), - Arguments.of(new MethodArgumentTypeMismatchException(null, null, null, null, null)), - Arguments.of(new HttpRequestMethodNotSupportedException("")), - Arguments.of(new ServletRequestBindingException("")), - Arguments.of(new NoHandlerFoundException("", null, null))); + Arguments.of(new FileNotFoundException("File not found")), + Arguments.of(new NoHandlerFoundException("GET", "URL", HttpHeaders.EMPTY))); } @Test - void handleUnhandledException_shouldUseGenericMessage() { + void handleAsyncRequestTimeoutException_shouldReturnServiceUnavailable() { + var ex = new AsyncRequestTimeoutException(); + ResponseEntity response = underTest.handleAsyncRequestTimeoutException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.REQUEST_TIMEOUT.getMessage()); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.REQUEST_TIMEOUT.getMessage()); + } - Exception exception = new Exception(); + @Test + void handleMaxUploadSizeExceededException_shouldReturnPayloadTooLarge() { + var ex = new MaxUploadSizeExceededException(0); + ResponseEntity response = underTest.handleMaxUploadSizeExceededException(ex); - ResponseEntity responseEntity = underTest.handleUnhandledException(exception); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.PAYLOAD_TOO_LARGE); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.SIZE_EXCEEDED.getMessage()); - assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); - assertThat(responseEntity.getBody().message()).isEqualTo("An unexpected error occurred. Please contact support if the problem persists."); + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.SIZE_EXCEEDED.getMessage()); } + @ParameterizedTest + @MethodSource("serverExceptionsProvider") + void handleServerException_shouldReturnCorrectHttpStatus(ServerException ex, HttpStatus expectedStatus) { + ResponseEntity response = underTest.handleServerException(ex); + + assertThat(response.getStatusCode()).isEqualTo(expectedStatus); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ex.getMessage()); + } + + static Stream serverExceptionsProvider() { + return Stream.of( + Arguments.of(new BadConfigurationException("Scope", "Bad config"), HttpStatus.BAD_REQUEST), + Arguments.of(BadRequestException.create("Bad request"), HttpStatus.BAD_REQUEST), + Arguments.of(new ForbiddenException("Access forbidden"), HttpStatus.FORBIDDEN), + Arguments.of(new NotFoundException("Not found"), HttpStatus.NOT_FOUND), + Arguments.of(new TemplateMatchingKeyException("Template matching error"), HttpStatus.BAD_REQUEST), + Arguments.of(new UnauthorizedException("Unauthorized access"), HttpStatus.UNAUTHORIZED)); + } + + @Test + void handleUnhandledException_shouldReturnInternalServerError() { + var ex = new Exception("Some unexpected error"); + ResponseEntity response = underTest.handleUnhandledException(ex); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().message()).isEqualTo(ErrorMessages.UNEXPECTED_ERROR.getMessage()); + + // Verify logging + assertThat(logs.logs(Level.ERROR)).contains(ErrorMessages.UNEXPECTED_ERROR.getMessage()); + } + + // endregion server + } diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/AddUserActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/AddUserActionIT.java index de7f3c7a0bb..f6f1a93a493 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/AddUserActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/permission/ws/AddUserActionIT.java @@ -36,6 +36,7 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.ServerException; +import org.sonar.server.exceptions.UnauthorizedException; import org.sonar.server.permission.PermissionService; import org.sonar.server.permission.PermissionServiceImpl; import org.sonar.server.ws.TestRequest; @@ -417,4 +418,30 @@ public class AddUserActionIT extends BasePermissionWsIT { .isInstanceOf(NotFoundException.class) .hasMessage("Entity not found"); } + + @Test + public void adding_permission_to_user_fails_if_not_authenticated_and_existing_login() { + userSession.anonymous(); + + TestRequest request = newRequest() + .setParam(PARAM_USER_LOGIN, user.getLogin()) + .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey()); + + assertThatThrownBy(request::execute) + .isInstanceOf(UnauthorizedException.class) + .hasMessage("Authentication is required"); + } + + @Test + public void adding_permission_to_user_fails_if_not_authenticated_and_non_existing_login() { + userSession.anonymous(); + + TestRequest request = newRequest() + .setParam(PARAM_USER_LOGIN, "non-existing-login") + .setParam(PARAM_PERMISSION, GlobalPermission.ADMINISTER.getKey()); + + assertThatThrownBy(request::execute) + .isInstanceOf(UnauthorizedException.class) + .hasMessage("Authentication is required"); + } } -- 2.39.5