ES-2395: CordaRestClient to support non-static credentials (#6255)

libs/rest/generated-rest-client/build.gradle

@@ -36,6 +36,7 @@ dependencies {
     implementation libs.okHttp
 
     testImplementation libs.javalin
+    testRuntimeOnly libs.log4j.slf4j
 }
 
 // Pick up the generated code so we can use it in src/main & src/test

libs/rest/generated-rest-client/src/main/kotlin/net/corda/restclient/CordaRestClient.kt

@@ -19,7 +19,6 @@ import net.corda.restclient.generated.apis.RBACRoleApi
 import net.corda.restclient.generated.apis.RBACUserApi
 import net.corda.restclient.generated.apis.VirtualNodeApi
 import net.corda.restclient.generated.apis.VirtualNodeMaintenanceApi
-import net.corda.restclient.generated.infrastructure.ApiClient
 import okhttp3.OkHttpClient
 import java.net.URI
 import java.security.SecureRandom
@@ -31,8 +30,6 @@ import javax.net.ssl.X509TrustManager
 
 @Suppress("LongParameterList")
 class CordaRestClient(
-    private val baseUrl: String,
-    private val httpClient: OkHttpClient,
     val certificatesClient: CertificateApi,
     val configurationClient: ConfigurationApi,
     val cpiClient: CPIApi,
@@ -59,11 +56,6 @@ class CordaRestClient(
 
         /**
          * Create an instance of CordaRestClient with the given baseUrl, username and password.
-         * Please note, if you use this multiple times with different credentials, you will be overwriting the previous credentials.
-         * e.g.
-         * val adminClient = createHttpClient(baseUrl, "admin", adminPassword)
-         * val userClient = createHttpClient(baseUrl, "user", userPassword)
-         * The `adminClient` will have the credentials of the `userClient` after the second call.
          *
          * @param baseUrl The base URL of the Corda node.
          * @param username The username to authenticate with.
@@ -119,10 +111,16 @@ class CordaRestClient(
 
             val urlWithDefaultBasePath = baseUrl.toString() + defaultBasePath
 
-            val builder = ApiClient.apply {
-                this.username = username
-                this.password = password
-            }.builder
+            val builder = OkHttpClient.Builder()
+
+            // Always add the basic auth header with the supplied credentials
+            builder.addInterceptor { chain ->
+                val request = chain.request().newBuilder()
+                    .header("Authorization", okhttp3.Credentials.basic(username, password))
+                    .build()
+                val response = chain.proceed(request)
+                response
+            }
 
             // Disable SSL verification if insecure is true
             if (insecure) {
@@ -135,8 +133,6 @@ class CordaRestClient(
 
             val client = builder.build()
             return CordaRestClient(
-                baseUrl = urlWithDefaultBasePath,
-                httpClient = client,
                 certificatesClient = certificatesClient ?: CertificateApi(urlWithDefaultBasePath, client),
                 configurationClient = configurationClient ?: ConfigurationApi(urlWithDefaultBasePath, client),
                 cpiClient = cpiClient ?: CPIApi(urlWithDefaultBasePath, client),

libs/rest/generated-rest-client/src/test/kotlin/net/corda/restclient/generated/TestClientAuthHeader.kt

@@ -0,0 +1,97 @@
+package net.corda.restclient.generated
+
+import io.javalin.Javalin
+import io.javalin.http.ContentType
+import net.corda.restclient.CordaRestClient
+import net.corda.restclient.generated.infrastructure.ApiClient
+import org.assertj.core.api.AssertionsForClassTypes.assertThat
+import org.junit.jupiter.api.AfterAll
+import org.junit.jupiter.api.BeforeAll
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import java.net.URI
+
+class TestClientAuthHeader {
+
+    companion object {
+        private lateinit var app: Javalin
+
+        @BeforeAll
+        @JvmStatic
+        fun setup() {
+            app = Javalin.create().start(0)
+            app.get("api/v5_3/mgm/1234/info") { ctx ->
+
+                assertThat(ctx.basicAuthCredentialsExist()).isTrue()
+
+                // Respond with the credentials from the basic auth header
+                val (user, password) = ctx.basicAuthCredentials()
+                ctx.contentType(ContentType.APPLICATION_JSON)
+                    .result("$user:$password")
+            }
+        }
+
+        @AfterAll
+        @JvmStatic
+        fun teardown() { app.stop() }
+    }
+
+    private val localhost = URI.create("http:localhost:${app.port()}")
+
+    private fun CordaRestClient.echoCredentials(): String {
+        return this.mgmClient.getMgmHoldingidentityshorthashInfo("1234")
+    }
+
+    @BeforeEach
+    fun resetStaticCredentials() {
+        ApiClient.username = null
+        ApiClient.password = null
+    }
+
+    private fun assertThatStaticCredsAreNull() {
+        assertThat(ApiClient.username).isNull()
+        assertThat(ApiClient.password).isNull()
+    }
+
+    @Test
+    fun defaultCredentialsAreCorrect() {
+        val defaultClient = CordaRestClient.createHttpClient(localhost)
+        assertThat(defaultClient.echoCredentials()).isEqualTo("admin:admin")
+
+        assertThatStaticCredsAreNull()
+    }
+
+    @Test
+    fun staticCredentialsAreIgnored() {
+        val defaultClient = CordaRestClient.createHttpClient(localhost, "foo", "bar")
+        assertThat(defaultClient.echoCredentials()).isEqualTo("foo:bar")
+
+        assertThatStaticCredsAreNull()
+
+        ApiClient.apply {
+            username = "foo-static"
+            password = "bar-static"
+        }
+        assertThat(defaultClient.echoCredentials()).isEqualTo("foo:bar")
+
+        assertThat(ApiClient.username).isEqualTo("foo-static")
+        assertThat(ApiClient.password).isEqualTo("bar-static")
+    }
+
+    @Test
+    fun twoClientsUseUniqueCredentials() {
+        val client1 = CordaRestClient.createHttpClient(localhost, "user-one", "password-one")
+        val client2 = CordaRestClient.createHttpClient(localhost, "user-two", "password-two")
+        ApiClient.apply {
+            username = "user-static"
+            password = "password-static"
+        }
+        assertThat(client1.echoCredentials()).isEqualTo("user-one:password-one")
+        assertThat(client2.echoCredentials()).isEqualTo("user-two:password-two")
+
+        resetStaticCredentials()
+
+        assertThat(client1.echoCredentials()).isEqualTo("user-one:password-one")
+        assertThat(client2.echoCredentials()).isEqualTo("user-two:password-two")
+    }
+}

libs/rest/generated-rest-client/src/test/kotlin/net/corda/restclient/generated/TestKnownIssues.kt

@@ -6,6 +6,7 @@ import net.corda.restclient.generated.apis.CertificateApi
 import net.corda.restclient.generated.models.GenerateCsrWrapperRequest
 import org.assertj.core.api.Assertions.assertThat
 import org.assertj.core.api.Assertions.assertThatCode
+import org.assertj.core.api.Assertions.assertThatThrownBy
 import org.junit.jupiter.api.AfterAll
 import org.junit.jupiter.api.BeforeAll
 import org.junit.jupiter.api.Test
@@ -225,4 +226,38 @@ class TestKnownIssues {
             assertThat(response).isEqualTo(greeting)
         }
     }
+
+    /**
+     * Test workaround `applyWorkaroundForClientErrorMessage`
+     * By default generated client doesn't include the response body
+     * in the exception message of client errors (4xx).
+     * We want to include the error response body in the exception message for better logging.
+     */
+    @Test
+    fun clientAndServerErrorsIncludeBodyInMessage() {
+        val errorJson = """
+            {
+                "title": "Test error",
+                "reason": "Test reason"
+            }
+        """.trimIndent()
+        app
+            .get("api/v5_3/key/client/error") { ctx ->
+                ctx.status(400)
+                ctx.result(errorJson)
+            }
+            .get("api/v5_3/key/server/error") { ctx ->
+                ctx.status(500)
+                ctx.result(errorJson)
+            }
+        val client = CordaRestClient.createHttpClient(baseUrl = localhost)
+
+        // Default behaviour: Server errors include the response body in the exception message
+        assertThatThrownBy { client.keyManagementClient.getKeyTenantidKeyid("server", "error") }
+            .hasMessageContaining(errorJson)
+
+        // Behaviour added by the workaround: Client errors should include the response body in the exception message
+        assertThatThrownBy { client.keyManagementClient.getKeyTenantidKeyid("client", "error") }
+            .hasMessageContaining(errorJson)
+    }
 }

tools/plugins/network/src/pluginSmokeTest/kotlin/net/corda/cli/plugins/network/OnboardMemberTest.kt

@@ -37,8 +37,8 @@ class OnboardMemberTest {
         private lateinit var defaulGroupPolicyLocation: String
         private val restClient = CordaRestClient.createHttpClient(
             baseUrl = DEFAULT_CLUSTER.rest.uri,
-            username = user,
-            password = password,
+            username = DEFAULT_CLUSTER.rest.user,
+            password = DEFAULT_CLUSTER.rest.password,
             insecure = true,
         )