fix: decouple axios version from core SDK

SmartThingsCommunity · Jan 10, 2024 · d60610c · d60610c
1 parent bdd6bcc
commit d60610c
Show file tree

Hide file tree

Showing 5 changed files with 191 additions and 129 deletions.
diff --git a/.changeset/friendly-turtles-kneel.md b/.changeset/friendly-turtles-kneel.md
@@ -0,0 +1,6 @@
+---
+"@smartthings/core-sdk": patch
+---
+
+Decouple from axios. Dependents of the core SDK no longer need to use the same version of
+axios, or even use axios at all.
diff --git a/src/authenticator.ts b/src/authenticator.ts
@@ -1,7 +1,7 @@
 import axios, { AxiosResponse, AxiosRequestConfig } from 'axios'
 import { MutexInterface } from 'async-mutex'
 
-import { EndpointClientConfig } from './endpoint-client'
+import { EndpointClientConfig, HttpClientHeaders } from './endpoint-client'
 
 
 /**
@@ -13,37 +13,26 @@ import { EndpointClientConfig } from './endpoint-client'
 export interface Authenticator {
 	login?(): Promise<void>
 	logout?(): Promise<void>
-	refresh?(requestConfig: AxiosRequestConfig, clientConfig: EndpointClientConfig): Promise<void>
+	refresh?(clientConfig: EndpointClientConfig): Promise<HttpClientHeaders>
 	acquireRefreshMutex?(): Promise<MutexInterface.Releaser>
 
 	/**
-	 * Performs required authentication steps to add credentials to the axios config, typically via Bearer Auth headers.
-	 * Expected to call other functions such as @see refresh as needed to return valid credentials.
-	 *
-	 * @param requestConfig AxiosRequestConfig to add credentials to and return otherwise unmodified
-	 */
-	authenticate(requestConfig: AxiosRequestConfig): Promise<AxiosRequestConfig>
-
-	/**
-	 * Performs required authentication steps and returns credentials as a string value
+	 * Performs required authentication steps and returns credentials as a set of HTTP headers which
+	 * must be included in authenticated requests.
 	 * Expected to perform any required steps (such as token refresh) needed to return valid credentials.
 	 *
 	 * @returns {string} valid auth token
 	 */
-	authenticateGeneric?(): Promise<string>
+	authenticate(): Promise<HttpClientHeaders>
 }
 
 
 /**
  * For use in tests or on endpoints that don't need any authentication.
  */
 export class NoOpAuthenticator implements Authenticator {
-	authenticate(requestConfig: AxiosRequestConfig): Promise<AxiosRequestConfig> {
-		return Promise.resolve(requestConfig)
-	}
-
-	authenticateGeneric(): Promise<string> {
-		return Promise.resolve('')
+	authenticate(): Promise<HttpClientHeaders> {
+		return Promise.resolve({})
 	}
 }
 
@@ -57,18 +46,17 @@ export class BearerTokenAuthenticator implements Authenticator {
 		// simple
 	}
 
-	authenticate(requestConfig: AxiosRequestConfig): Promise<AxiosRequestConfig> {
+	authenticate(): Promise<HttpClientHeaders> {
+		/*
 		return Promise.resolve({
 			...requestConfig,
 			headers: {
 				...requestConfig.headers,
 				Authorization: `Bearer ${this.token}`,
 			},
 		})
-	}
-
-	authenticateGeneric(): Promise<string> {
-		return Promise.resolve(this.token)
+		*/
+		return Promise.resolve({ Authorization: `Bearer ${this.token}` })
 	}
 }
 
@@ -101,17 +89,11 @@ export class RefreshTokenAuthenticator implements Authenticator {
 		// simple
 	}
 
-	authenticate(requestConfig: AxiosRequestConfig): Promise<AxiosRequestConfig> {
-		return Promise.resolve({
-			...requestConfig,
-			headers: {
-				...requestConfig.headers,
-				Authorization: `Bearer ${this.token}`,
-			},
-		})
+	authenticate(): Promise<HttpClientHeaders> {
+		return Promise.resolve({ Authorization: `Bearer ${this.token}` })
 	}
 
-	async refresh(requestConfig: AxiosRequestConfig, clientConfig: EndpointClientConfig): Promise<void> {
+	async refresh(clientConfig: EndpointClientConfig): Promise<HttpClientHeaders> {
 		const refreshData: RefreshData = await this.tokenStore.getRefreshData()
 		const headers = {
 			'Content-Type': 'application/x-www-form-urlencoded',
@@ -133,8 +115,8 @@ export class RefreshTokenAuthenticator implements Authenticator {
 				refreshToken: response.data.refresh_token,
 			}
 			this.token = authData.authToken
-			requestConfig.headers = { ...(requestConfig.headers ?? {}), Authorization: `Bearer ${this.token}` }
-			return this.tokenStore.putAuthData(authData)
+			await this.tokenStore.putAuthData(authData)
+			return { Authorization: `Bearer ${this.token}` }
 		}
 
 		throw Error(`error ${response.status} refreshing token, with message ${response.data}`)

diff --git a/src/endpoint-client.ts b/src/endpoint-client.ts
@@ -174,7 +174,7 @@ export class EndpointClient {
 			}
 		}
 
-		let axiosConfig: AxiosRequestConfig = {
+		const axiosConfig: AxiosRequestConfig = {
 			url: this.url(path),
 			method,
 			headers: options?.headerOverrides ? { ...headers, ...options.headerOverrides } : headers,
@@ -183,7 +183,8 @@ export class EndpointClient {
 			paramsSerializer: params => qs.stringify(params, { indices: false }),
 		}
 
-		axiosConfig = await this.config.authenticator.authenticate(axiosConfig)
+		const authHeaders = await this.config.authenticator.authenticate()
+		axiosConfig.headers = { ...axiosConfig.headers, ...authHeaders }
 
 		if (this.logger.isDebugEnabled()) {
 			this.logger.debug(`making axios request: ${scrubConfig(axiosConfig)}`)
@@ -208,6 +209,7 @@ export class EndpointClient {
 			return response.data
 		} catch (error: any) {
 			if (this.logger.isTraceEnabled()) {
+				// https://www.npmjs.com/package/axios#handling-errors
 				if (error.response) {
 					// server responded with non-200 response code
 					this.logger.trace(`axios response ${error.response.status}: data=${JSON.stringify(error.response.data)}`)
@@ -222,14 +224,16 @@ export class EndpointClient {
 				if (this.config.authenticator.acquireRefreshMutex) {
 					const release = await this.config.authenticator.acquireRefreshMutex()
 					try {
-						await this.config.authenticator.refresh(axiosConfig, this.config)
+						const newAuthHeaders = await this.config.authenticator.refresh(this.config)
+						axiosConfig.headers = { ...axiosConfig.headers, ...newAuthHeaders }
 						const response = await axios.request(axiosConfig)
 						return response.data
 					} finally {
 						release()
 					}
 				} else {
-					await this.config.authenticator.refresh(axiosConfig, this.config)
+					const newAuthHeaders = await this.config.authenticator.refresh(this.config)
+					axiosConfig.headers = { ...axiosConfig.headers, ...newAuthHeaders }
 					const response = await axios.request(axiosConfig)
 					return response.data
 				}

diff --git a/test/unit/authenticator.test.ts b/test/unit/authenticator.test.ts
@@ -28,50 +28,24 @@ describe('authenticators', () => {
 		jest.clearAllMocks()
 	})
 
-	const config = { url: 'https://api.smartthings.com', headers: { Test: 'test' } }
+	test('NoOpAuthenticator returns no headers', async () => {
+		const authenticator = new NoOpAuthenticator()
 
-	describe('NoOpAuthenticator', () => {
-		test('authenticate returns config unchanged', async () => {
-			const authenticator = new NoOpAuthenticator()
-			const data = await authenticator.authenticate(config)
-
-			expect(data).toBe(config)
-		})
-
-		test('authenticateGeneric returns empty string for token', async () => {
-			const authenticator = new NoOpAuthenticator()
-			const token = await authenticator.authenticateGeneric()
-
-			expect(token).toBe('')
-		})
+		expect(await authenticator.authenticate()).toStrictEqual({})
 	})
 
-	describe('BearerTokenAuthenticator', () => {
-		test('authenticate adds header with specified token', async () => {
-			const authenticator = new BearerTokenAuthenticator('a-bearer-token')
-			const data = await authenticator.authenticate(config)
+	test('BearerTokenAuthenticator returns header with specified token', async () => {
+		const authenticator = new BearerTokenAuthenticator('a-bearer-token')
 
-			expect(data.url).toBe(config.url)
-			expect(data.headers?.Authorization).toBe('Bearer a-bearer-token')
-			expect(data.headers?.Test).toBe('test')
-		})
-
-		test('authenticateGeneric returns specified token', async () => {
-			const authenticator = new BearerTokenAuthenticator('a-bearer-token')
-			const token = await authenticator.authenticateGeneric()
-
-			expect(token).toBe('a-bearer-token')
-		})
+		expect(await authenticator.authenticate()).toStrictEqual({ Authorization: 'Bearer a-bearer-token' })
 	})
 
 	describe('RefreshTokenAuthenticator', () => {
-		test('authenticate adds header with specified token', async () => {
+		test('authenticate returns header with specified token', async () => {
 			const tokenStore = new TokenStore()
 			const authenticator = new RefreshTokenAuthenticator('a-refreshable-bearer-token', tokenStore)
-			const data = await authenticator.authenticate(config)
 
-			expect(data.url).toBe(config.url)
-			expect(data.headers?.Authorization).toBe('Bearer a-refreshable-bearer-token')
+			expect(await authenticator.authenticate()).toStrictEqual({ Authorization: 'Bearer a-refreshable-bearer-token' })
 		})
 
 		test('refresh updates token', async () => {
@@ -86,7 +60,8 @@ describe('authenticators', () => {
 			const tokenStore = new TokenStore()
 			const authenticator = new RefreshTokenAuthenticator('a-refreshable-bearer-token', tokenStore)
 			const endpointConfig = { urlProvider: defaultSmartThingsURLProvider, authenticator }
-			await authenticator.refresh(config, endpointConfig)
+
+			expect(await authenticator.refresh(endpointConfig)).toStrictEqual({ Authorization: 'Bearer the-access-token' })
 
 			expect(axios.request).toHaveBeenCalledTimes(1)
 			expect(axios.request).toHaveBeenCalledWith({
@@ -114,7 +89,7 @@ describe('authenticators', () => {
 			const endpointConfig = { urlProvider: defaultSmartThingsURLProvider, authenticator }
 			let message
 			try {
-				await authenticator.refresh(config, endpointConfig)
+				await authenticator.refresh(endpointConfig)
 			// eslint-disable-next-line @typescript-eslint/no-explicit-any
 			} catch (error: any) {
 				message = error.message
@@ -133,10 +108,7 @@ describe('authenticators', () => {
 		const authenticator = new SequentialRefreshTokenAuthenticator('a-bearer-token', tokenStore, mutex)
 
 		test('authenticate adds header with specified token', async () => {
-			const data = await authenticator.authenticate(config)
-
-			expect(data.url).toBe(config.url)
-			expect(data.headers?.Authorization).toBe('Bearer a-bearer-token')
+			expect(await authenticator.authenticate()).toStrictEqual({ Authorization: 'Bearer a-bearer-token' })
 		})
 
 		describe('refresh', () => {
@@ -151,7 +123,7 @@ describe('authenticators', () => {
 			const endpointConfig = { urlProvider: defaultSmartThingsURLProvider, authenticator }
 
 			it('updates token', async () => {
-				await authenticator.refresh(config, endpointConfig)
+				await authenticator.refresh(endpointConfig)
 
 				expect(axios.request).toHaveBeenCalledTimes(1)
 				expect(axios.request).toHaveBeenCalledWith({
@@ -169,8 +141,7 @@ describe('authenticators', () => {
 			})
 
 			it('works on request with no existing headers', async () => {
-				const configWithoutHeaders = { url: 'https://api.smartthings.com' }
-				await authenticator.refresh(configWithoutHeaders, endpointConfig)
+				expect(await authenticator.refresh(endpointConfig)).toStrictEqual({ Authorization: 'Bearer the-access-token' })
 
 				expect(axios.request).toHaveBeenCalledTimes(1)
 				expect(axios.request).toHaveBeenCalledWith({