[Backport 6.x] ApiKey should take precedence over basic auth (#1116)

* ApiKey should take precedence over basic auth

* Updated docs

* Updated test

Co-authored-by: Tomas Della Vedova <delvedor@users.noreply.github.com>

docs/authentication.asciidoc

@@ -33,6 +33,8 @@ const client = new Client({
 
 You can provide your credentials by passing the `username` and `password` parameters via the `auth` option.
 
+NOTE: If you provide both basic authentication credentials and the Api Key configuration, the Api Key will take precedence.
+
 [source,js]
 ----
 const { Client } = require('@elastic/elasticsearch')
@@ -60,6 +62,8 @@ const client = new Client({
 You can use the https://www.elastic.co/guide/en/elasticsearch/reference/7.x/security-api-create-api-key.html[ApiKey] authentication by passing the `apiKey` parameter via the `auth` option. +
 The `apiKey` parameter can be either a base64 encoded string or an object with the values that you can obtain from the https://www.elastic.co/guide/en/elasticsearch/reference/7.x/security-api-create-api-key.html[create api key endpoint].
 
+NOTE: If you provide both basic authentication credentials and the Api Key configuration, the Api Key will take precedence.
+
 [source,js]
 ----
 const { Client } = require('@elastic/elasticsearch')

lib/Connection.js

@@ -294,16 +294,14 @@ function resolve (host, path) {
 
 function prepareHeaders (headers = {}, auth) {
   if (auth != null && headers.authorization == null) {
-    if (auth.username && auth.password) {
-      headers.authorization = 'Basic ' + Buffer.from(`${auth.username}:${auth.password}`).toString('base64')
-    }
-
     if (auth.apiKey) {
       if (typeof auth.apiKey === 'object') {
         headers.authorization = 'ApiKey ' + Buffer.from(`${auth.apiKey.id}:${auth.apiKey.api_key}`).toString('base64')
       } else {
         headers.authorization = `ApiKey ${auth.apiKey}`
       }
+    } else if (auth.username && auth.password) {
+      headers.authorization = 'Basic ' + Buffer.from(`${auth.username}:${auth.password}`).toString('base64')
     }
   }
   return headers

lib/pool/BaseConnectionPool.js

@@ -42,13 +42,13 @@ class BaseConnectionPool {
       opts = this.urlToHost(opts)
     }
 
-    if (opts.url.username !== '' && opts.url.password !== '') {
+    if (this.auth !== null) {
+      opts.auth = this.auth
+    } else if (opts.url.username !== '' && opts.url.password !== '') {
       opts.auth = {
         username: decodeURIComponent(opts.url.username),
         password: decodeURIComponent(opts.url.password)
       }
-    } else if (this.auth !== null) {
-      opts.auth = this.auth
     }
 
     if (opts.ssl == null) opts.ssl = this._ssl

test/unit/client.test.js

@@ -467,6 +467,62 @@ test('Authentication', t => {
       })
     })
 
+    t.test('ApiKey should take precedence over basic auth (in url)', t => {
+      t.plan(3)
+
+      function handler (req, res) {
+        t.match(req.headers, {
+          authorization: 'ApiKey Zm9vOmJhcg=='
+        })
+        res.setHeader('Content-Type', 'application/json;utf=8')
+        res.end(JSON.stringify({ hello: 'world' }))
+      }
+
+      buildServer(handler, ({ port }, server) => {
+        const client = new Client({
+          node: `http://user:pwd@localhost:${port}`,
+          auth: {
+            apiKey: 'Zm9vOmJhcg=='
+          }
+        })
+
+        client.info((err, { body }) => {
+          t.error(err)
+          t.deepEqual(body, { hello: 'world' })
+          server.stop()
+        })
+      })
+    })
+
+    t.test('ApiKey should take precedence over basic auth (in opts)', t => {
+      t.plan(3)
+
+      function handler (req, res) {
+        t.match(req.headers, {
+          authorization: 'ApiKey Zm9vOmJhcg=='
+        })
+        res.setHeader('Content-Type', 'application/json;utf=8')
+        res.end(JSON.stringify({ hello: 'world' }))
+      }
+
+      buildServer(handler, ({ port }, server) => {
+        const client = new Client({
+          node: `http://localhost:${port}`,
+          auth: {
+            apiKey: 'Zm9vOmJhcg==',
+            username: 'user',
+            password: 'pwd'
+          }
+        })
+
+        client.info((err, { body }) => {
+          t.error(err)
+          t.deepEqual(body, { hello: 'world' })
+          server.stop()
+        })
+      })
+    })
+
     t.end()
   })
 
@@ -827,6 +883,44 @@ test('Elastic cloud config', t => {
     t.deepEqual(pool._ssl, { secureProtocol: 'TLSv1_2_method' })
   })
 
+  t.test('ApiKey should take precedence over basic auth', t => {
+    t.plan(5)
+    const client = new Client({
+      cloud: {
+        // 'localhost$abcd$efgh'
+        id: 'name:bG9jYWxob3N0JGFiY2QkZWZnaA=='
+      },
+      auth: {
+        username: 'elastic',
+        password: 'changeme',
+        apiKey: 'Zm9vOmJhcg=='
+      }
+    })
+
+    const pool = client.connectionPool
+    t.ok(pool instanceof CloudConnectionPool)
+    t.match(pool.connections.find(c => c.id === 'https://abcd.localhost/'), {
+      url: new URL('https://elastic:changeme@abcd.localhost'),
+      id: 'https://abcd.localhost/',
+      headers: {
+        authorization: 'ApiKey Zm9vOmJhcg=='
+      },
+      ssl: { secureProtocol: 'TLSv1_2_method' },
+      deadCount: 0,
+      resurrectTimeout: 0,
+      roles: {
+        master: true,
+        data: true,
+        ingest: true,
+        ml: false
+      }
+    })
+
+    t.strictEqual(client.transport.compression, 'gzip')
+    t.strictEqual(client.transport.suggestCompression, true)
+    t.deepEqual(pool._ssl, { secureProtocol: 'TLSv1_2_method' })
+  })
+
   t.test('Override default options', t => {
     t.plan(4)
     const client = new Client({