From 142f9cc2bf493508a3c096fe9a8f2e2cf31657cf Mon Sep 17 00:00:00 2001 From: Albert Tregnaghi Date: Thu, 21 Sep 2023 17:04:19 +0200 Subject: [PATCH 1/2] Changed behavior of java api client to avoid 401 problems #2553 - created unit test to reproduce problem (wire mock test) - changed login behavior, instead of using a authenticator object we use now a request interceptor and set the basic auth header directly - removed server url and trust all setters - introduced new constructor inside abstract SecHub client to have fields inside mocked client as well --- .../sechub/api/AbstractSecHubClient.java | 18 ++-- .../sechub/api/DefaultSecHubClient.java | 5 +- .../sechub/api/MockedSecHubClient.java | 2 + .../sechub/api/internal/ApiClientBuilder.java | 71 ++++++++---- .../internal/SecHubClientAuthenticator.java | 20 ---- .../api/DefaultSechubClientWireMockTest.java | 101 ++++++++++++++++++ 6 files changed, 165 insertions(+), 52 deletions(-) delete mode 100644 sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/internal/SecHubClientAuthenticator.java create mode 100644 sechub-api-java/src/test/java/com/mercedesbenz/sechub/api/DefaultSechubClientWireMockTest.java diff --git a/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/AbstractSecHubClient.java b/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/AbstractSecHubClient.java index 915b4df0a2..794a989078 100644 --- a/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/AbstractSecHubClient.java +++ b/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/AbstractSecHubClient.java @@ -18,8 +18,14 @@ public abstract class AbstractSecHubClient implements SecHubClient { private Set secHubClientListeners; - public AbstractSecHubClient() { - secHubClientListeners = new LinkedHashSet<>(); + public AbstractSecHubClient(URI serverUri, String username, String apiToken, boolean trustAll) { + this.serverUri = serverUri; + this.trustAll = trustAll; + + this.secHubClientListeners = new LinkedHashSet<>(); + + setUsername(username); + setApiToken(apiToken); } public void setUsername(String username) { @@ -30,14 +36,6 @@ public void setApiToken(String apiToken) { this.sealedApiToken = apiTokenAccess.seal(apiToken); } - public void setServerUri(URI serverUri) { - this.serverUri = serverUri; - } - - public void setTrustAll(boolean trustAll) { - this.trustAll = trustAll; - } - @Override public boolean isTrustAll() { return trustAll; diff --git a/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/DefaultSecHubClient.java b/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/DefaultSecHubClient.java index 4df61339bb..f3bea5a5eb 100644 --- a/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/DefaultSecHubClient.java +++ b/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/DefaultSecHubClient.java @@ -70,10 +70,7 @@ public DefaultSecHubClient(URI serverUri, String username, String apiToken) { } public DefaultSecHubClient(URI serverUri, String username, String apiToken, boolean trustAll) { - setUsername(username); - setApiToken(apiToken); - setServerUri(serverUri); - setTrustAll(trustAll); + super(serverUri, username, apiToken, trustAll); apiClient = new ApiClientBuilder().createApiClient(this, mapper); diff --git a/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/MockedSecHubClient.java b/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/MockedSecHubClient.java index 615fcb082b..80b5134e33 100644 --- a/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/MockedSecHubClient.java +++ b/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/MockedSecHubClient.java @@ -1,5 +1,6 @@ package com.mercedesbenz.sechub.api; +import java.net.URI; import java.nio.file.Path; import java.util.ArrayList; import java.util.HashMap; @@ -33,6 +34,7 @@ public class MockedSecHubClient extends AbstractSecHubClient { private Set userToProjectAssignments = new HashSet<>(); public MockedSecHubClient() { + super(URI.create("https://localhost/mocked-sechub"), "mockuser", "pseudo-token", true); mockDataAccess = new MockDataAccess(); } diff --git a/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/internal/ApiClientBuilder.java b/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/internal/ApiClientBuilder.java index ca1a2a9aff..22b9ef3e7e 100644 --- a/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/internal/ApiClientBuilder.java +++ b/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/internal/ApiClientBuilder.java @@ -1,15 +1,18 @@ // SPDX-License-Identifier: MIT package com.mercedesbenz.sechub.api.internal; +import java.net.Socket; import java.net.http.HttpClient; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; +import java.util.Base64; import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; import javax.net.ssl.TrustManager; -import javax.net.ssl.X509TrustManager; +import javax.net.ssl.X509ExtendedTrustManager; import com.fasterxml.jackson.databind.ObjectMapper; import com.mercedesbenz.sechub.api.SecHubClient; @@ -18,36 +21,30 @@ public class ApiClientBuilder { public ApiClient createApiClient(SecHubClient client, ObjectMapper mapper) { - HttpClient.Builder builder = HttpClient.newBuilder().authenticator(new SecHubClientAuthenticator(client)); + HttpClient.Builder builder = HttpClient.newBuilder(); if (client.isTrustAll()) { builder.sslContext(createTrustAllSSLContext()); } + ApiClient apiClient = new ApiClient(builder, mapper, client.getServerUri().toString()); + apiClient.setRequestInterceptor((request) -> { + request.setHeader("Authorization", createBasicAuthenticationHeader(client)); + }); return apiClient; } + private static final String createBasicAuthenticationHeader(SecHubClient client) { + String valueToEncode = client.getUsername() + ":" + client.getSealedApiToken(); + return "Basic " + Base64.getEncoder().encodeToString(valueToEncode.getBytes()); + } + private SSLContext createTrustAllSSLContext() { SSLContext sslContext = null; try { sslContext = SSLContext.getInstance("TLS"); - TrustManager trustManager = new X509TrustManager() { - - private X509Certificate[] emptyCertificatesArray = new X509Certificate[] {}; - - public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException { - /* we do not check the client - we trust all */ - } - - public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { - /* we do not check the server - we trust all */ - } - - public X509Certificate[] getAcceptedIssuers() { - return emptyCertificatesArray; - } - }; + TrustManager trustManager = new TrustAllManager(); sslContext.init(null, new TrustManager[] { trustManager }, null); @@ -57,4 +54,42 @@ public X509Certificate[] getAcceptedIssuers() { } } + + private class TrustAllManager extends X509ExtendedTrustManager { + + private X509Certificate[] emptyCertificatesArray = new X509Certificate[] {}; + + public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException { + /* we do not check - we trust all */ + } + + public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { + /* we do not check - we trust all */ + } + + public X509Certificate[] getAcceptedIssuers() { + return emptyCertificatesArray; + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException { + /* we do not check - we trust all */ + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException { + /* we do not check - we trust all */ + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException { + /* we do not check - we trust all */ + + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException { + /* we do not check - we trust all */ + } + }; } diff --git a/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/internal/SecHubClientAuthenticator.java b/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/internal/SecHubClientAuthenticator.java deleted file mode 100644 index 64ccc526cb..0000000000 --- a/sechub-api-java/src/main/java/com/mercedesbenz/sechub/api/internal/SecHubClientAuthenticator.java +++ /dev/null @@ -1,20 +0,0 @@ -// SPDX-License-Identifier: MIT -package com.mercedesbenz.sechub.api.internal; - -import java.net.Authenticator; -import java.net.PasswordAuthentication; - -import com.mercedesbenz.sechub.api.SecHubClient; - -public class SecHubClientAuthenticator extends Authenticator { - private PasswordAuthentication paswordAuthentication; - - public SecHubClientAuthenticator(SecHubClient client) { - this.paswordAuthentication = new PasswordAuthentication(client.getUsername(), client.getSealedApiToken().toCharArray()); - } - - @Override - protected PasswordAuthentication getPasswordAuthentication() { - return paswordAuthentication; - } -} \ No newline at end of file diff --git a/sechub-api-java/src/test/java/com/mercedesbenz/sechub/api/DefaultSechubClientWireMockTest.java b/sechub-api-java/src/test/java/com/mercedesbenz/sechub/api/DefaultSechubClientWireMockTest.java new file mode 100644 index 0000000000..1faac028ff --- /dev/null +++ b/sechub-api-java/src/test/java/com/mercedesbenz/sechub/api/DefaultSechubClientWireMockTest.java @@ -0,0 +1,101 @@ +package com.mercedesbenz.sechub.api; + +import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.*; +import static org.junit.Assert.*; + +import java.net.URI; + +import org.junit.Rule; +import org.junit.Test; + +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import com.mercedesbenz.sechub.test.TestPortProvider; + +import wiremock.org.apache.http.HttpStatus; + +/** + * Junit 4 test because of missing official WireMock Junit5 extension - so we + * use WireMock Rule and Junit4. + * + * @author Albert Tregnaghi + * + */ +public class DefaultSechubClientWireMockTest { + + private static final String EXAMPLE_TOKEN = "example-token"; + + private static final String EXAMPLE_USER = "example-user"; + private static final String APPLICATION_JSON = "application/json"; + + private static final int HTTPS_PORT = TestPortProvider.DEFAULT_INSTANCE.getWireMockTestHTTPSPort(); + + private static final int HTTP_PORT = TestPortProvider.DEFAULT_INSTANCE.getWireMockTestHTTPPort(); + + @Rule + public WireMockRule wireMockRule = new WireMockRule(wireMockConfig().port(HTTP_PORT).httpsPort(HTTPS_PORT)); + + @Test + public void fetch_sechub_status_with_basic_auth() throws Exception { + + /* prepare */ + String statusBody = """ + [ { + "key" : "status.scheduler.enabled", + "value" : "true" + }, { + "key" : "status.scheduler.jobs.all", + "value" : "2" + } ] + """; + stubFor(get(urlEqualTo("/api/admin/status")).withBasicAuth(EXAMPLE_USER, EXAMPLE_TOKEN) + .willReturn(aResponse().withStatus(HttpStatus.SC_OK).withHeader("Content-Type", APPLICATION_JSON).withBody(statusBody))); + + DefaultSecHubClient client = createTestClientWithExampleCredentials(); + + /* execute */ + SecHubStatus status = client.fetchSecHubStatus(); + + /* test */ + verify(getRequestedFor(urlEqualTo("/api/admin/status"))); + assertNotNull(status); + assertEquals("true", status.getStatusInformationMap().get("status.scheduler.enabled")); + assertEquals("2", status.getStatusInformationMap().get("status.scheduler.jobs.all")); + + } + + @Test + public void credential_changes_on_client_after_creation_are_possible() throws Exception { + + /* prepare */ + String statusBody = """ + [ { + "key" : "status.scheduler.enabled", + "value" : "false" + }, { + "key" : "status.scheduler.jobs.all", + "value" : "0" + } ] + """; + stubFor(get(urlEqualTo("/api/admin/status")).withBasicAuth("other-user", "other-token") + .willReturn(aResponse().withStatus(HttpStatus.SC_OK).withHeader("Content-Type", APPLICATION_JSON).withBody(statusBody))); + + DefaultSecHubClient client = createTestClientWithExampleCredentials(); + + /* execute */ + client.setApiToken("other-token"); + client.setUsername("other-user"); + + /* test */ + SecHubStatus status = client.fetchSecHubStatus(); + verify(getRequestedFor(urlEqualTo("/api/admin/status"))); + + assertEquals("false", status.getStatusInformationMap().get("status.scheduler.enabled")); + assertEquals("0", status.getStatusInformationMap().get("status.scheduler.jobs.all")); + } + + private DefaultSecHubClient createTestClientWithExampleCredentials() { + DefaultSecHubClient client = new DefaultSecHubClient(URI.create(wireMockRule.baseUrl()), EXAMPLE_USER, EXAMPLE_TOKEN, true); + return client; + } +} From 6f72a34fdcc0cad02ea6a34252aaf46a9a0cb3d6 Mon Sep 17 00:00:00 2001 From: Albert Tregnaghi Date: Wed, 27 Sep 2023 07:30:29 +0200 Subject: [PATCH 2/2] Added wiremock as test dependency to sechub-api-java #2553 --- sechub-api-java/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/sechub-api-java/build.gradle b/sechub-api-java/build.gradle index e524be45e1..976f58ab00 100644 --- a/sechub-api-java/build.gradle +++ b/sechub-api-java/build.gradle @@ -20,6 +20,7 @@ dependencies { testImplementation spring_boot_dependency.junit_jupiter testImplementation library.apache_commons_io testImplementation library.mockito_inline + testImplementation library.wiremock } /*