From a9ff34a88b68a2f1df68bc1fe54ebf5b17f4e70a Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 27 May 2021 15:32:09 +0200 Subject: [PATCH] SONAR-14871 Basic project binding validation endpoint - Add 'scope' field to HTTP error messages --- .../exceptions/BadConfigurationException.java | 57 +++++++++++++++++++ .../exceptions/BadRequestException.java | 2 +- .../BadConfigurationExceptionTest.java | 36 ++++++++++++ .../almsettings/ws/AlmSettingsSupport.java | 9 ++- .../org/sonar/server/ws/WebServiceEngine.java | 18 +++++- .../sonar/server/ws/WebServiceEngineTest.java | 20 +++++-- 6 files changed, 132 insertions(+), 10 deletions(-) create mode 100644 server/sonar-webserver-api/src/main/java/org/sonar/server/exceptions/BadConfigurationException.java create mode 100644 server/sonar-webserver-api/src/test/java/org/sonar/server/exceptions/BadConfigurationExceptionTest.java diff --git a/server/sonar-webserver-api/src/main/java/org/sonar/server/exceptions/BadConfigurationException.java b/server/sonar-webserver-api/src/main/java/org/sonar/server/exceptions/BadConfigurationException.java new file mode 100644 index 00000000000..3ca9b46f0a3 --- /dev/null +++ b/server/sonar-webserver-api/src/main/java/org/sonar/server/exceptions/BadConfigurationException.java @@ -0,0 +1,57 @@ +/* + * SonarQube + * Copyright (C) 2009-2021 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.exceptions; + +import com.google.common.base.MoreObjects; +import java.util.List; + +import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; +import static java.util.Collections.singletonList; + +/** + * Provided request is not valid within given scope and can not be processed. + */ +public class BadConfigurationException extends ServerException { + private final String scope; + private final transient List errors; + + public BadConfigurationException(String scope, String errorMessage) { + super(HTTP_BAD_REQUEST, errorMessage); + this.scope = scope; + this.errors = singletonList(errorMessage); + } + + public String scope() { + return scope; + } + + public List errors() { + return this.errors; + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("scope", this.scope) + .add("errors", this.errors()) + .toString(); + } + +} diff --git a/server/sonar-webserver-api/src/main/java/org/sonar/server/exceptions/BadRequestException.java b/server/sonar-webserver-api/src/main/java/org/sonar/server/exceptions/BadRequestException.java index be0158662bf..69962095d09 100644 --- a/server/sonar-webserver-api/src/main/java/org/sonar/server/exceptions/BadRequestException.java +++ b/server/sonar-webserver-api/src/main/java/org/sonar/server/exceptions/BadRequestException.java @@ -34,7 +34,7 @@ public class BadRequestException extends ServerException { private final transient List errors; - private BadRequestException(List errors) { + BadRequestException(List errors) { super(HTTP_BAD_REQUEST, errors.get(0)); this.errors = errors; } diff --git a/server/sonar-webserver-api/src/test/java/org/sonar/server/exceptions/BadConfigurationExceptionTest.java b/server/sonar-webserver-api/src/test/java/org/sonar/server/exceptions/BadConfigurationExceptionTest.java new file mode 100644 index 00000000000..a28db60f67b --- /dev/null +++ b/server/sonar-webserver-api/src/test/java/org/sonar/server/exceptions/BadConfigurationExceptionTest.java @@ -0,0 +1,36 @@ +/* + * SonarQube + * Copyright (C) 2009-2021 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.exceptions; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class BadConfigurationExceptionTest { + + @Test + public void testMethods() { + BadConfigurationException ex = new BadConfigurationException("my-scope", "error"); + + assertThat(ex.scope()).isEqualTo("my-scope"); + assertThat(ex).hasToString("BadConfigurationException{scope=my-scope, errors=[error]}"); + } + +} diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/AlmSettingsSupport.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/AlmSettingsSupport.java index 1357e7aac2e..83b6adb9b6d 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/AlmSettingsSupport.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/almsettings/ws/AlmSettingsSupport.java @@ -67,13 +67,16 @@ public class AlmSettingsSupport { if (!multipleAlmFeatureProvider.enabled() && !dbClient.almSettingDao().selectByAlm(dbSession, alm).isEmpty()) { throw BadRequestException.create("A " + alm + " setting is already defined"); } - } } - public ProjectDto getProject(DbSession dbSession, String projectKey) { + public ProjectDto getProjectAsAdmin(DbSession dbSession, String projectKey) { + return getProject(dbSession, projectKey, ADMIN); + } + + public ProjectDto getProject(DbSession dbSession, String projectKey, String projectPermission) { ProjectDto project = componentFinder.getProjectByKey(dbSession, projectKey); - userSession.checkProjectPermission(ADMIN, project); + userSession.checkProjectPermission(projectPermission, project); return project; } diff --git a/server/sonar-webserver-ws/src/main/java/org/sonar/server/ws/WebServiceEngine.java b/server/sonar-webserver-ws/src/main/java/org/sonar/server/ws/WebServiceEngine.java index 715607cee1e..52082613569 100644 --- a/server/sonar-webserver-ws/src/main/java/org/sonar/server/ws/WebServiceEngine.java +++ b/server/sonar-webserver-ws/src/main/java/org/sonar/server/ws/WebServiceEngine.java @@ -28,16 +28,17 @@ import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.apache.catalina.connector.ClientAbortException; import org.picocontainer.Startable; +import org.sonar.api.impl.ws.ValidatingRequest; import org.sonar.api.server.ServerSide; import org.sonar.api.server.ws.LocalConnector; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; -import org.sonar.api.impl.ws.ValidatingRequest; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.api.utils.text.JsonWriter; import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.exceptions.BadConfigurationException; import org.sonar.server.exceptions.ServerException; import org.sonarqube.ws.MediaTypes; @@ -48,9 +49,9 @@ import static java.util.Objects.requireNonNull; import static org.apache.commons.lang.StringUtils.substring; import static org.apache.commons.lang.StringUtils.substringAfterLast; import static org.apache.commons.lang.StringUtils.substringBeforeLast; +import static org.sonar.server.exceptions.NotFoundException.checkFound; import static org.sonar.server.ws.RequestVerifier.verifyRequest; import static org.sonar.server.ws.ServletRequest.SUPPORTED_MEDIA_TYPES_BY_URL_SUFFIX; -import static org.sonar.server.exceptions.NotFoundException.checkFound; /** * @since 4.2 @@ -110,6 +111,8 @@ public class WebServiceEngine implements LocalConnector, Startable { action.handler().handle(request, response); } catch (IllegalArgumentException e) { sendErrors(request, response, e, 400, singletonList(e.getMessage())); + } catch (BadConfigurationException e) { + sendErrors(request, response, e, 400, e.errors(), e.scope()); } catch (BadRequestException e) { sendErrors(request, response, e, 400, e.errors()); } catch (ServerException e) { @@ -128,6 +131,10 @@ public class WebServiceEngine implements LocalConnector, Startable { } private static void sendErrors(Request request, Response response, Exception exception, int status, List errors) { + sendErrors(request, response, exception, status, errors, null); + } + + private static void sendErrors(Request request, Response response, Exception exception, int status, List errors, @Nullable String scope) { if (isRequestAbortedByClient(exception)) { // do not pollute logs. We can't do anything -> use DEBUG level // see org.sonar.server.ws.ServletResponse#output() @@ -160,6 +167,7 @@ public class WebServiceEngine implements LocalConnector, Startable { stream.setMediaType(MediaTypes.JSON); try (JsonWriter json = JsonWriter.of(new OutputStreamWriter(stream.output(), StandardCharsets.UTF_8))) { json.beginObject(); + writeScope(scope, json); writeErrors(json, errors); json.endObject(); } catch (Exception e) { @@ -168,6 +176,12 @@ public class WebServiceEngine implements LocalConnector, Startable { } } + private static void writeScope(@Nullable String scope, JsonWriter json) { + if (scope != null) { + json.prop("scope", scope); + } + } + private static boolean isRequestAbortedByClient(Exception exception) { return Throwables.getCausalChain(exception).stream().anyMatch(t -> t instanceof ClientAbortException); } diff --git a/server/sonar-webserver-ws/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java b/server/sonar-webserver-ws/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java index 7b86c69e5e7..47d301ff577 100644 --- a/server/sonar-webserver-ws/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java +++ b/server/sonar-webserver-ws/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java @@ -24,7 +24,6 @@ import javax.servlet.http.HttpServletResponse; import org.apache.catalina.connector.ClientAbortException; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.mockito.Mockito; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.RequestHandler; @@ -32,6 +31,7 @@ import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; +import org.sonar.server.exceptions.BadConfigurationException; import org.sonar.server.exceptions.BadRequestException; import org.sonarqube.ws.MediaTypes; @@ -50,9 +50,6 @@ public class WebServiceEngineTest { @Rule public LogTester logTester = new LogTester(); - @Rule - public ExpectedException expectedException = ExpectedException.none(); - @Test public void load_ws_definitions_at_startup() { WebServiceEngine underTest = new WebServiceEngine(new WebService[] { @@ -336,6 +333,21 @@ public class WebServiceEngineTest { assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); } + @Test + public void return_400_on_BadConfigurationException_with_single_message_and_scope() { + Request request = new TestRequest().setPath("api/foo"); + + DumbResponse response = run(request, newWs("api/foo", a -> a.setHandler((req, resp) -> { + throw new BadConfigurationException("PROJECT", "Bad request !"); + }))); + + assertThat(response.stream().outputAsString()).isEqualTo( + "{\"scope\":\"PROJECT\",\"errors\":[{\"msg\":\"Bad request !\"}]}"); + assertThat(response.stream().status()).isEqualTo(400); + assertThat(response.stream().mediaType()).isEqualTo(MediaTypes.JSON); + assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); + } + @Test public void return_error_message_containing_character_percent() { Request request = new TestRequest().setPath("api/foo"); -- 2.39.5