JWT auth fixes (#367)

ClickHouse · Dec 17, 2024 · 64f266d · 64f266d
1 parent 862b552
commit 64f266d
Show file tree

Hide file tree

Showing 10 changed files with 176 additions and 170 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,26 @@
+# 1.10.0 (Common, Node.js, Web)
+
+## New features
+
+- Added support for JWT authentication (ClickHouse Cloud feature) in both Node.js and Web API packages. JWT token can be set via `access_token` client configuration option.
+
+  ```ts
+  const client = createClient({
+    // ...
+    access_token: '<JWT access token>',
+  })
+  ```
+
+  Access token can also be configured via the URL params, e.g., `https://host:port?access_token=...`.
+
+  It is also possible to override the access token for a particular request (see `BaseQueryParams.auth` for more details).
+
+  NB: do not mix access token and username/password credentials in the configuration; the client will throw an error if both are set.
+
 # 1.9.1 (Node.js only)
 
+## Bug fixes
+
 - Fixed an uncaught exception that could happen in case of malformed ClickHouse response when response compression is enabled ([#363](https://github.com/ClickHouse/clickhouse-js/issues/363))
 
 # 1.9.0 (Common, Node.js, Web)

diff --git a/packages/client-common/__tests__/integration/auth.test.ts b/packages/client-common/__tests__/integration/auth.test.ts
@@ -7,10 +7,8 @@ describe('authentication', () => {
   let invalidAuthClient: ClickHouseClient
   beforeEach(() => {
     invalidAuthClient = createTestClient({
-      auth: {
-        username: 'gibberish',
-        password: 'gibberish',
-      },
+      username: 'gibberish',
+      password: 'gibberish',
     })
   })
   afterEach(async () => {

diff --git a/packages/client-common/__tests__/unit/config.test.ts b/packages/client-common/__tests__/unit/config.test.ts
@@ -96,7 +96,8 @@ describe('config', () => {
           pathname: '/my_proxy',
           request_timeout: 42_000,
           max_open_connections: 144,
-          auth: { username: 'bob', password: 'secret' },
+          username: 'bob',
+          password: 'secret',
           database: 'analytics',
           http_headers: {
             'X-CLICKHOUSE-AUTH': 'secret_header',
@@ -123,10 +124,8 @@ describe('config', () => {
         pathname: '/my_proxy',
         request_timeout: 42_000,
         max_open_connections: 144,
-        auth: {
-          username: 'bob',
-          password: 'secret',
-        },
+        username: 'bob',
+        password: 'secret',
         database: 'analytics',
         http_headers: {
           'X-CLICKHOUSE-AUTH': 'secret_header',
@@ -179,60 +178,6 @@ describe('config', () => {
       }) // should not be modified
     })
 
-    it('should be able to use the deprecated username parameter', async () => {
-      const deprecated: BaseClickHouseClientConfigOptions = {
-        username: 'bob',
-      }
-      const res = prepareConfigWithURL(deprecated, logger, null)
-      expect(res).toEqual({
-        ...defaultConfig,
-        auth: {
-          username: 'bob',
-          // to be "normalized" later by `getConnectionParams`
-          password: undefined,
-        },
-      })
-      expect(deprecated).toEqual({
-        username: 'bob',
-      }) // should not be modified
-    })
-
-    it('should be able to use the deprecated password parameter', async () => {
-      const deprecated: BaseClickHouseClientConfigOptions = {
-        password: 'secret',
-      }
-      const res = prepareConfigWithURL(deprecated, logger, null)
-      expect(res).toEqual({
-        ...defaultConfig,
-        auth: {
-          username: undefined,
-          password: 'secret',
-        },
-      })
-      expect(deprecated).toEqual({
-        password: 'secret',
-      }) // should not be modified
-    })
-
-    it('should be able to use both deprecated username and password parameters', async () => {
-      const deprecated: BaseClickHouseClientConfigOptions = {
-        username: 'bob',
-        password: 'secret',
-      }
-      const res = prepareConfigWithURL(deprecated, logger, null)
-      expect(res).toEqual({
-        ...defaultConfig,
-        auth: {
-          username: 'bob',
-          password: 'secret',
-        },
-      })
-      expect(deprecated).toEqual({
-        username: 'bob',
-        password: 'secret',
-      }) // should not be modified
-    })
-
     // tested more thoroughly in the loadConfigOptionsFromURL section;
     // this is just a validation that everything works together
     it('should use settings from the URL', async () => {
@@ -257,10 +202,8 @@ describe('config', () => {
       expect(res).toEqual({
         ...defaultConfig,
         url: new URL('https://my.host:8443/'),
-        auth: {
-          username: 'bob',
-          password: 'secret',
-        },
+        username: 'bob',
+        password: 'secret',
         database: 'analytics',
         application: 'my_app',
         impl_specific_setting: 42,
@@ -367,12 +310,11 @@ describe('config', () => {
           url: new URL('https://my.host:8443'),
           database: 'analytics',
           application: 'my_app',
-          auth: {
-            access_token: 'jwt_secret',
-          },
+          access_token: 'jwt_secret',
         } as unknown as BaseClickHouseClientConfigOptionsWithURL)
       })
 
+      // this will throw later during the config validation anyway
       it('should correctly override credentials auth with JWT if both are present', async () => {
         const url = new URL(
           'https://bob:[email protected]:8443/analytics?' +
@@ -384,9 +326,9 @@ describe('config', () => {
           url: new URL('https://my.host:8443'),
           database: 'analytics',
           application: 'my_app',
-          auth: {
-            access_token: 'jwt_secret',
-          },
+          username: 'bob',
+          password: 'secret',
+          access_token: 'jwt_secret',
         } as unknown as BaseClickHouseClientConfigOptionsWithURL)
       })
     })
@@ -402,6 +344,12 @@ describe('config', () => {
   })
 
   describe('getConnectionParams', () => {
+    const authErrorMatcher = jasmine.objectContaining({
+      message: jasmine.stringContaining(
+        'Please use only one authentication method',
+      ),
+    })
+
     it('should return the default connection params', async () => {
       const res = getConnectionParams(
         {
@@ -441,10 +389,8 @@ describe('config', () => {
             request: true,
             response: false,
           },
-          auth: {
-            username: 'bob',
-            password: 'secret',
-          },
+          username: 'bob',
+          password: 'secret',
           database: 'analytics',
           clickhouse_settings: {
             async_insert: 1,
@@ -482,6 +428,76 @@ describe('config', () => {
         application_id: 'my_app',
       })
     })
+
+    it('should throw if both JWT and username are set', async () => {
+      expect(() =>
+        getConnectionParams(
+          {
+            url: new URL('https://my.host:8443/'),
+            username: 'bob',
+            access_token: 'jwt',
+          },
+          logger,
+        ),
+      ).toThrow(authErrorMatcher)
+    })
+
+    it('should throw if both JWT and password are set', async () => {
+      expect(() =>
+        getConnectionParams(
+          {
+            url: new URL('https://my.host:8443/'),
+            password: 'secret',
+            access_token: 'jwt',
+          },
+          logger,
+        ),
+      ).toThrow(authErrorMatcher)
+    })
+
+    it('should throw if JWT, username, and password are all set', async () => {
+      expect(() =>
+        getConnectionParams(
+          {
+            url: new URL('https://my.host:8443/'),
+            username: 'bob',
+            password: 'secret',
+            access_token: 'jwt',
+          },
+          logger,
+        ),
+      ).toThrow(authErrorMatcher)
+    })
+
+    it('should not throw if only JWT auth is set', async () => {
+      expect(
+        getConnectionParams(
+          {
+            url: new URL('https://my.host:8443/'),
+            access_token: 'secret-token',
+          },
+          logger,
+        ),
+      ).toEqual({
+        url: new URL('https://my.host:8443/'),
+        request_timeout: 30_000,
+        max_open_connections: 10,
+        compression: {
+          decompress_response: false,
+          compress_request: false,
+        },
+        auth: {
+          access_token: 'secret-token',
+          type: 'JWT',
+        },
+        database: 'default',
+        clickhouse_settings: {},
+        log_writer: jasmine.any(LogWriter),
+        keep_alive: { enabled: true },
+        application_id: undefined,
+        http_headers: {},
+      })
+    })
   })
 
   describe('mergeConfigs', () => {
@@ -492,32 +508,26 @@ describe('config', () => {
     it('should leave the base config as-is when there is nothing from the URL', async () => {
       const base: BaseClickHouseClientConfigOptions = {
         url: 'http://localhost:8123',
-        auth: {
-          username: 'bob',
-          password: 'secret',
-        },
+        username: 'bob',
+        password: 'secret',
       }
       expect(mergeConfigs(base, {}, logger)).toEqual(base)
     })
 
     it('should take URL values first, then base config for the rest', async () => {
       const base: BaseClickHouseClientConfigOptions = {
         url: 'https://my.host:8124',
-        auth: {
-          username: 'bob',
-          password: 'secret',
-        },
+        username: 'bob',
+        password: 'secret',
       }
       const fromURL: BaseClickHouseClientConfigOptions = {
-        auth: { password: 'secret_from_url!' },
+        password: 'secret_from_url!',
       }
       const res = mergeConfigs(base, fromURL, logger)
       expect(res).toEqual({
         url: 'https://my.host:8124',
-        auth: {
-          username: 'bob',
-          password: 'secret_from_url!',
-        },
+        username: 'bob',
+        password: 'secret_from_url!',
       })
     })
 
@@ -526,25 +536,29 @@ describe('config', () => {
         url: 'https://my.host:8124',
       }
       const fromURL: BaseClickHouseClientConfigOptions = {
-        auth: { username: 'bob', password: 'secret' },
+        username: 'bob',
+        password: 'secret',
       }
       const res = mergeConfigs(base, fromURL, logger)
       expect(res).toEqual({
         url: 'https://my.host:8124',
-        auth: { username: 'bob', password: 'secret' },
+        username: 'bob',
+        password: 'secret',
       })
     })
 
     // realistically, we will always have at least URL in the base config
     it('should only take the URL values when there is nothing in the base config', async () => {
       const fromURL: BaseClickHouseClientConfigOptions = {
         url: 'https://my.host:8443',
-        auth: { username: 'bob', password: 'secret' },
+        username: 'bob',
+        password: 'secret',
       }
       const res = mergeConfigs({}, fromURL, logger)
       expect(res).toEqual({
         url: 'https://my.host:8443',
-        auth: { username: 'bob', password: 'secret' },
+        username: 'bob',
+        password: 'secret',
       })
     })
 
@@ -748,7 +762,8 @@ describe('config', () => {
       const res = loadConfigOptionsFromURL(url, null)
       expect(res[0].toString()).toEqual('https://my.host:8124/') // pathname will be attached later.
       expect(res[1]).toEqual({
-        auth: { username: 'bob', password: 'secret' },
+        username: 'bob',
+        password: 'secret',
         database: 'analytics',
         application: 'my_app',
         pathname: '/my_proxy',
@@ -778,7 +793,8 @@ describe('config', () => {
       const res = loadConfigOptionsFromURL(url, null)
       expect(res[0].toString()).toEqual('http://localhost:8124/')
       expect(res[1]).toEqual({
-        auth: { username: 'bob', password: 'secret' },
+        username: 'bob',
+        password: 'secret',
         database: 'analytics',
       })
     })
@@ -920,7 +936,8 @@ describe('config', () => {
       const res = loadConfigOptionsFromURL(url, handler)
       expect(res[0].toString()).toEqual('https://my.host:8124/')
       expect(res[1]).toEqual({
-        auth: { username: 'bob', password: 'secret' },
+        username: 'bob',
+        password: 'secret',
         database: 'analytics',
         application: 'my_app',
         session_id: 'sticky',
@@ -970,7 +987,8 @@ describe('config', () => {
       const res = loadConfigOptionsFromURL(url, handler)
       expect(res[0].toString()).toEqual('https://my.host:8124/')
       expect(res[1]).toEqual({
-        auth: { username: 'bob', password: 'secret' },
+        username: 'bob',
+        password: 'secret',
         database: 'analytics',
         application: 'my_app',
         session_id: 'sticky',

diff --git a/packages/client-common/__tests__/utils/client.ts b/packages/client-common/__tests__/utils/client.ts
@@ -56,7 +56,7 @@ export function createTestClient<Stream = unknown>(
   if (isCloudTestEnv()) {
     const cloudConfig: BaseClickHouseClientConfigOptions = {
       url: `https://${getFromEnv(EnvKeys.host)}:8443`,
-      auth: { password: getFromEnv(EnvKeys.password) },
+      password: getFromEnv(EnvKeys.password),
       database: databaseName,
       request_timeout: 60_000,
       ...logging,