Support authentication on the level of ElasticClient (#3124)

* Support authentication on the level of ElasticClient * Remove documentation citing to use http client-level authentication * Remove separate options * Remove tagless * Make authentication the last parameter, give it default value
Philippus · Nov 7, 2024 · f96e357 · f96e357
1 parent 9842ed7
commit f96e357
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 59 deletions.
diff --git a/docs/clients.md b/docs/clients.md
@@ -24,24 +24,21 @@ val client = ElasticClient(JavaClient(ElasticProperties(nodes)))
 
 ### Credentials
 
-The java client is itself just a simple wrapper around the Apache HTTP client library, so anything you can do with that client, can you do with the `JavaClient`
-
-The `JavaClient` accepts a callback of type `HttpClientConfigCallback` which is invoked when the client is being created. In this we can set credentials.
-
+Credentials can be passed by defining `CommonRequestOptions` implicit in the visibility scope when calling
+`client.execute()` method:
 
 ```scala
-val callback = new HttpClientConfigCallback {
-  override def customizeHttpClient(httpClientBuilder: HttpAsyncClientBuilder): HttpAsyncClientBuilder = {
-     val creds = new BasicCredentialsProvider()
-     creds.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials("sammy", "letmein"))
-     httpClientBuilder.setDefaultCredentialsProvider(creds)
-  }
-}
+implicit val options: CommonRequestOptions = CommonRequestOptions.defaults.copy(
+  credentials = Some(BasicHttpCredentials("user", "pass"))
+)
 
-val props = ElasticProperties("http://host1:9200")
-val client = ElasticClient(JavaClient(props, requestConfigCallback = NoOpRequestConfigCallback, httpClientConfigCallback = callback))
+client.execute(search("myindex"))
 ```
 
+Currently, two methods of authentication are supported:
+
+* `Authentication.UsernamePassword` to pass a username and password
+* `Authentication.ApiKey` to pass an API key
 
 
 

diff --git a/elastic4s-client-akka/src/test/scala/com/sksamuel/elastic4s/akka/AkkaHttpClientTest.scala b/elastic4s-client-akka/src/test/scala/com/sksamuel/elastic4s/akka/AkkaHttpClientTest.scala
@@ -1,16 +1,13 @@
 package com.sksamuel.elastic4s.akka
 
 import akka.actor.ActorSystem
-import com.sksamuel.elastic4s.{ElasticClient, ElasticRequest, Executor, HttpClient, HttpResponse}
 import com.sksamuel.elastic4s.requests.common.HealthStatus
 import com.sksamuel.elastic4s.testkit.DockerTests
+import com.sksamuel.elastic4s.{Authentication, CommonRequestOptions, ElasticClient}
 import org.scalatest.BeforeAndAfterAll
 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should.Matchers
 
-import java.nio.charset.StandardCharsets
-import java.util.Base64
-import scala.concurrent.Future
 import scala.util.Try
 
 class AkkaHttpClientTest extends AnyFlatSpec with Matchers with DockerTests with BeforeAndAfterAll {
@@ -106,12 +103,9 @@ class AkkaHttpClientTest extends AnyFlatSpec with Matchers with DockerTests with
   }
 
   it should "propagate headers if included" in {
-    implicit val executor: Executor[Future] = new Executor[Future] {
-      override def exec(client: HttpClient, request: ElasticRequest): Future[HttpResponse] = {
-        val cred = Base64.getEncoder.encodeToString("user123:pass123".getBytes(StandardCharsets.UTF_8))
-        Executor.FutureExecutor.exec(client, request.copy(headers = Map("Authorization" -> s"Basic $cred")))
-      }
-    }
+    implicit val requestOptions: CommonRequestOptions = CommonRequestOptions.defaults.copy(
+      authentication = Authentication.UsernamePassword("user123", "pass123")
+    )
 
     client.execute {
       catHealth()

diff --git a/elastic4s-client-pekko/src/test/scala/com/sksamuel/elastic4s/pekko/PekkoHttpClientTest.scala b/elastic4s-client-pekko/src/test/scala/com/sksamuel/elastic4s/pekko/PekkoHttpClientTest.scala
@@ -1,16 +1,13 @@
 package com.sksamuel.elastic4s.pekko
 
-import org.apache.pekko.actor.ActorSystem
-import com.sksamuel.elastic4s.{ElasticClient, ElasticRequest, Executor, HttpClient, HttpResponse}
 import com.sksamuel.elastic4s.requests.common.HealthStatus
 import com.sksamuel.elastic4s.testkit.DockerTests
+import com.sksamuel.elastic4s.{Authentication, CommonRequestOptions, ElasticClient}
+import org.apache.pekko.actor.ActorSystem
 import org.scalatest.BeforeAndAfterAll
 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should.Matchers
 
-import java.nio.charset.StandardCharsets
-import java.util.Base64
-import scala.concurrent.Future
 import scala.util.Try
 
 class PekkoHttpClientTest extends AnyFlatSpec with Matchers with DockerTests with BeforeAndAfterAll {
@@ -106,12 +103,9 @@ class PekkoHttpClientTest extends AnyFlatSpec with Matchers with DockerTests wit
   }
 
   it should "propagate headers if included" in {
-    implicit val executor: Executor[Future] = new Executor[Future] {
-      override def exec(client: HttpClient, request: ElasticRequest): Future[HttpResponse] = {
-        val cred = Base64.getEncoder.encodeToString("user123:pass123".getBytes(StandardCharsets.UTF_8))
-        Executor.FutureExecutor.exec(client, request.copy(headers = Map("Authorization" -> s"Basic $cred")))
-      }
-    }
+    implicit val requestOptions: CommonRequestOptions = CommonRequestOptions.defaults.copy(
+      authentication = Authentication.UsernamePassword("user123", "pass123")
+    )
 
     client.execute {
       catHealth()

diff --git a/...4s-client-sttp/src/test/scala/com/sksamuel/elastic4s/sttp/SttpRequestHttpClientTest.scala b/...4s-client-sttp/src/test/scala/com/sksamuel/elastic4s/sttp/SttpRequestHttpClientTest.scala
@@ -1,25 +1,18 @@
 package com.sksamuel.elastic4s.sttp
 
-import com.sksamuel.elastic4s.{ElasticClient, ElasticNodeEndpoint, ElasticRequest, Executor, HttpClient, HttpResponse}
 import com.sksamuel.elastic4s.testkit.DockerTests
+import com.sksamuel.elastic4s.{Authentication, CommonRequestOptions, ElasticClient, ElasticNodeEndpoint}
 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should.Matchers
 
-import java.nio.charset.StandardCharsets
-import java.util.Base64
-import scala.concurrent.Future
-
 class SttpRequestHttpClientTest extends AnyFlatSpec with Matchers with DockerTests {
   private lazy val sttpClient = SttpRequestHttpClient(ElasticNodeEndpoint("http", elasticHost, elasticPort.toInt, None))
   override val client = ElasticClient(sttpClient)
 
   "SttpRequestHttpClient" should "propagate headers if included" in {
-    implicit val executor: Executor[Future] = new Executor[Future] {
-      override def exec(client: HttpClient, request: ElasticRequest): Future[HttpResponse] = {
-        val cred = Base64.getEncoder.encodeToString("user123:pass123".getBytes(StandardCharsets.UTF_8))
-        Executor.FutureExecutor.exec(client, request.copy(headers = Map("Authorization" -> s"Basic $cred")))
-      }
-    }
+    implicit val options: CommonRequestOptions = CommonRequestOptions.defaults.copy(
+      authentication = Authentication.UsernamePassword("user123", "pass123")
+    )
 
     client.execute {
       catHealth()

diff --git a/elastic4s-core/src/main/scala/com/sksamuel/elastic4s/ElasticClient.scala b/elastic4s-core/src/main/scala/com/sksamuel/elastic4s/ElasticClient.scala
@@ -3,6 +3,7 @@ package com.sksamuel.elastic4s
 import com.fasterxml.jackson.module.scala.JavaTypeable
 import org.slf4j.{Logger, LoggerFactory}
 
+import java.util.Base64
 import scala.concurrent.duration.{Duration, _}
 import scala.language.higherKinds
 
@@ -49,9 +50,11 @@ case class ElasticClient(client: HttpClient) extends AutoCloseable {
       request2
     }
 
-    val request4 = options.headers.foldLeft(request3){ case (acc, (key, value)) => acc.addHeader(key, value) }
+    val request4 = request3.addHeaders(options.headers)
 
-    val f = executor.exec(client, request4)
+    val request5 = authenticate(request4, options.authentication)
+
+    val f = executor.exec(client, request5)
     functor.map(f) { resp =>
       handler.responseHandler.handle(resp) match {
         case Right(u) => RequestSuccess(resp.statusCode, resp.entity.map(_.content), resp.headers, u)
@@ -60,12 +63,49 @@ case class ElasticClient(client: HttpClient) extends AutoCloseable {
     }
   }
 
+  private def authenticate(request: ElasticRequest, authentication: Authentication): ElasticRequest = {
+    authentication match {
+      case Authentication.UsernamePassword(username, password) =>
+        request.addHeader(
+          "Authorization",
+          "Basic " + Base64.getEncoder.encodeToString(s"$username:$password".getBytes)
+        )
+      case Authentication.ApiKey(apiKey) =>
+        request.addHeader(
+          "Authorization",
+          "ApiKey " + Base64.getEncoder.encodeToString(apiKey.getBytes)
+        )
+      case Authentication.NoAuth =>
+        request
+    }
+  }
+
 
   def close(): Unit = client.close()
 }
 
-case class CommonRequestOptions(timeout: Duration, masterNodeTimeout: Duration, headers: Map[String, String] = Map.empty)
+sealed trait Authentication
+
+object Authentication {
+  case class UsernamePassword(username: String, password: String) extends Authentication
+
+  case class ApiKey(apiKey: String) extends Authentication
+
+  case object NoAuth extends Authentication
+}
+
+case class CommonRequestOptions(
+  timeout: Duration,
+  masterNodeTimeout: Duration,
+  headers: Map[String, String] = Map.empty,
+  authentication: Authentication = Authentication.NoAuth,
+)
 
 object CommonRequestOptions {
-  implicit val defaults: CommonRequestOptions = CommonRequestOptions(0.seconds, 0.seconds, Map.empty)
+  implicit val defaults: CommonRequestOptions = CommonRequestOptions(
+    timeout = 0.seconds,
+    masterNodeTimeout = 0.seconds,
+    headers = Map.empty,
+    authentication = Authentication.NoAuth,
+  )
 }
diff --git a/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/ElasticRequest.scala b/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/ElasticRequest.scala
@@ -13,6 +13,8 @@ case class ElasticRequest(method: String, endpoint: String, params: Map[String,
   def addParameter(name: String, value: String): ElasticRequest = copy(params = params + (name -> value))
 
   def addHeader(name: String, value: String): ElasticRequest = copy(headers = headers + (name -> value))
+
+  def addHeaders(headers: Map[String, String]): ElasticRequest = copy(headers = this.headers ++ headers)
 }
 
 object ElasticRequest {

diff --git a/elastic4s-tests/src/test/scala/com/sksamuel/elastic4s/http/ElasticClientTests.scala b/elastic4s-tests/src/test/scala/com/sksamuel/elastic4s/http/ElasticClientTests.scala
@@ -1,13 +1,10 @@
 package com.sksamuel.elastic4s.http
 
-import com.sksamuel.elastic4s.{ElasticRequest, Executor, HttpClient, HttpResponse}
 import com.sksamuel.elastic4s.testkit.DockerTests
+import com.sksamuel.elastic4s.{Authentication, CommonRequestOptions}
 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should.Matchers
 
-import java.nio.charset.StandardCharsets
-import java.util.Base64
-import scala.concurrent.Future
 import scala.util.Try
 
 class ElasticClientTests extends AnyFlatSpec with Matchers with DockerTests {
@@ -25,12 +22,9 @@ class ElasticClientTests extends AnyFlatSpec with Matchers with DockerTests {
   }
 
   it should "propagate headers if included" in {
-    implicit val executor: Executor[Future] = new Executor[Future] {
-      override def exec(client: HttpClient, request: ElasticRequest): Future[HttpResponse] = {
-        val cred = Base64.getEncoder.encodeToString("user123:pass123".getBytes(StandardCharsets.UTF_8))
-        Executor.FutureExecutor.exec(client, request.copy(headers = Map("Authorization" -> s"Basic $cred")))
-      }
-    }
+    implicit val requestOptions: CommonRequestOptions = CommonRequestOptions.defaults.copy(
+      authentication = Authentication.UsernamePassword("user123", "pass123")
+    )
 
     client.execute {
       catHealth()