Alee/decoupling auth client from redis (#11)

* Decoupling auth client from redis impl

package.json

@@ -121,6 +121,7 @@
     "@types/jest": "^26.0.19",
     "@types/node": "^14.14.13",
     "@types/passport": "^1.0.4",
+    "@types/redis": "^2.8.28",
     "@types/superagent": "^4.1.10",
     "@types/supertest": "^2.0.10",
     "@typescript-eslint/eslint-plugin": "^4.10.0",

server/config.ts

@@ -37,7 +37,7 @@ export default {
   staticResourceCacheDuration: 20,
   redis: {
     host: process.env.REDIS_HOST,
-    port: process.env.REDIS_PORT || 6379,
+    port: parseInt(process.env.REDIS_PORT, 10) || 6379,
     password: process.env.REDIS_AUTH_TOKEN,
     tls_enabled: get('REDIS_TLS_ENABLED', 'false'),
   },

server/data/hmppsAuthClient.test.ts

@@ -1,33 +1,27 @@
 import nock from 'nock'
-import redis from 'redis'
 
 import config from '../config'
 import HmppsAuthClient from './hmppsAuthClient'
+import TokenStore from './tokenStore'
 
-const username = 'Bob'
-const token = { access_token: 'token-1', expires_in: 300 }
+jest.mock('./tokenStore')
 
-jest.mock('redis', () => ({
-  createClient: jest.fn().mockReturnThis(),
-  on: jest.fn(),
-  get: jest.fn(),
-  set: jest.fn().mockImplementation((key, value, command, ttl, callback) => callback(null, null)),
-}))
+const tokenStore = new TokenStore(null) as jest.Mocked<TokenStore>
 
-function givenRedisResponse(storedToken: string) {
-  redis.get.mockImplementation((key, callback) => callback(null, storedToken))
-}
+const username = 'Bob'
+const token = { access_token: 'token-1', expires_in: 300 }
 
 describe('hmppsAuthClient', () => {
   let fakeHmppsAuthApi: nock.Scope
   let hmppsAuthClient: HmppsAuthClient
 
   beforeEach(() => {
     fakeHmppsAuthApi = nock(config.apis.hmppsAuth.url)
-    hmppsAuthClient = new HmppsAuthClient()
+    hmppsAuthClient = new HmppsAuthClient(tokenStore)
   })
 
   afterEach(() => {
+    jest.resetAllMocks()
     nock.cleanAll()
   })
 
@@ -59,19 +53,18 @@ describe('hmppsAuthClient', () => {
 
   describe('getSystemClientToken', () => {
     it('should instantiate the redis client', async () => {
-      givenRedisResponse(token.access_token)
+      tokenStore.getToken.mockResolvedValue(token.access_token)
       await hmppsAuthClient.getSystemClientToken(username)
-      expect(redis.createClient).toBeCalledTimes(1)
     })
 
     it('should return token from redis if one exists', async () => {
-      givenRedisResponse(token.access_token)
+      tokenStore.getToken.mockResolvedValue(token.access_token)
       const output = await hmppsAuthClient.getSystemClientToken(username)
       expect(output).toEqual(token.access_token)
     })
 
     it('should return token from HMPPS Auth with username', async () => {
-      givenRedisResponse(null)
+      tokenStore.getToken.mockResolvedValue(null)
 
       fakeHmppsAuthApi
         .post(`/oauth/token`, 'grant_type=client_credentials&username=Bob')
@@ -82,11 +75,11 @@ describe('hmppsAuthClient', () => {
       const output = await hmppsAuthClient.getSystemClientToken(username)
 
       expect(output).toEqual(token.access_token)
-      expect(redis.set).toBeCalledWith('Bob', token.access_token, 'EX', 240, expect.any(Function))
+      expect(tokenStore.setToken).toBeCalledWith('Bob', token.access_token, 240)
     })
 
     it('should return token from HMPPS Auth without username', async () => {
-      givenRedisResponse(null)
+      tokenStore.getToken.mockResolvedValue(null)
 
       fakeHmppsAuthApi
         .post(`/oauth/token`, 'grant_type=client_credentials')
@@ -97,7 +90,7 @@ describe('hmppsAuthClient', () => {
       const output = await hmppsAuthClient.getSystemClientToken()
 
       expect(output).toEqual(token.access_token)
-      expect(redis.set).toBeCalledWith('Bob', token.access_token, 'EX', 240, expect.any(Function))
+      expect(tokenStore.setToken).toBeCalledWith('%ANONYMOUS%', token.access_token, 240)
     })
   })
 })

server/data/hmppsAuthClient.ts

@@ -1,7 +1,6 @@
 import superagent from 'superagent'
 import querystring from 'querystring'
-import redis from 'redis'
-import { promisify } from 'util'
+import type TokenStore from './tokenStore'
 
 import logger from '../../log'
 import config from '../config'
@@ -10,20 +9,6 @@ import RestClient from './restClient'
 
 const timeoutSpec = config.apis.hmppsAuth.timeout
 const hmppsAuthUrl = config.apis.hmppsAuth.url
-const redisClient = redis.createClient({
-  port: config.redis.port,
-  password: config.redis.password,
-  host: config.redis.host,
-  tls: config.redis.tls_enabled === 'true' ? {} : false,
-  prefix: 'systemToken:',
-})
-
-redisClient.on('error', error => {
-  logger.error(error, `Redis error`)
-})
-
-const getRedisAsync = promisify(redisClient.get).bind(redisClient)
-const setRedisAsync = promisify(redisClient.set).bind(redisClient)
 
 function getSystemClientTokenFromHmppsAuth(username?: string): Promise<superagent.Response> {
   const clientToken = generateOauthClientToken(
@@ -47,16 +32,18 @@ function getSystemClientTokenFromHmppsAuth(username?: string): Promise<superagen
     .timeout(timeoutSpec)
 }
 
-interface User {
+export interface User {
   name: string
   activeCaseLoadId: string
 }
 
-interface UserRole {
+export interface UserRole {
   roleCode: string
 }
 
 export default class HmppsAuthClient {
+  constructor(private readonly tokenStore: TokenStore) {}
+
   private restClient(token: string): RestClient {
     return new RestClient('HMPPS Auth Client', config.apis.hmppsAuth, token)
   }
@@ -73,17 +60,17 @@ export default class HmppsAuthClient {
   }
 
   async getSystemClientToken(username?: string): Promise<string> {
-    const redisKey = username || '%ANONYMOUS%'
+    const key = username || '%ANONYMOUS%'
 
-    const tokenFromRedis = await getRedisAsync(redisKey)
-    if (tokenFromRedis) {
-      return tokenFromRedis
+    const token = await this.tokenStore.getToken(key)
+    if (token) {
+      return token
     }
 
     const newToken = await getSystemClientTokenFromHmppsAuth(username)
 
     // set TTL slightly less than expiry of token. Async but no need to wait
-    await setRedisAsync(redisKey, newToken.body.access_token, 'EX', newToken.body.expires_in - 60)
+    await this.tokenStore.setToken(key, newToken.body.access_token, newToken.body.expires_in - 60)
 
     return newToken.body.access_token
   }

server/data/tokenStore.test.ts

@@ -0,0 +1,34 @@
+import { RedisClient } from 'redis'
+import TokenStore from './tokenStore'
+
+const redisClient = {
+  on: jest.fn(),
+  get: jest.fn(),
+  set: jest.fn(),
+}
+
+describe('tokenStore', () => {
+  let tokenStore: TokenStore
+
+  beforeEach(() => {
+    tokenStore = new TokenStore((redisClient as unknown) as RedisClient)
+  })
+
+  afterEach(() => {
+    jest.resetAllMocks()
+  })
+
+  it('Can retrieve token', async () => {
+    redisClient.get.mockImplementation((key, callback) => callback(null, 'token-1'))
+
+    await expect(tokenStore.getToken('user-1')).resolves.toBe('token-1')
+
+    expect(redisClient.get).toHaveBeenCalledWith('user-1', expect.any(Function))
+  })
+
+  it('Can set token', async () => {
+    await tokenStore.setToken('user-1', 'token-1', 10)
+
+    expect(redisClient.set).toHaveBeenCalledWith('user-1', 'token-1', 'EX', 10, expect.any(Function))
+  })
+})

server/data/tokenStore.ts

@@ -0,0 +1,38 @@
+import redis from 'redis'
+import { promisify } from 'util'
+
+import logger from '../../log'
+import config from '../config'
+
+const createRedisClient = () => {
+  return redis.createClient({
+    port: config.redis.port,
+    password: config.redis.password,
+    host: config.redis.host,
+    tls: config.redis.tls_enabled === 'true' ? {} : false,
+    prefix: 'systemToken:',
+  })
+}
+
+export default class TokenStore {
+  private getRedisAsync: (key: string) => Promise<string>
+
+  private setRedisAsync: (key: string, value: string, mode: string, durationSeconds: number) => Promise<void>
+
+  constructor(redisClient: redis.RedisClient = createRedisClient()) {
+    redisClient.on('error', error => {
+      logger.error(error, `Redis error`)
+    })
+
+    this.getRedisAsync = promisify(redisClient.get).bind(redisClient)
+    this.setRedisAsync = promisify(redisClient.set).bind(redisClient)
+  }
+
+  public async setToken(key: string, token: string, durationSeconds: number): Promise<void> {
+    this.setRedisAsync(key, token, 'EX', durationSeconds)
+  }
+
+  public async getToken(key: string): Promise<string> {
+    return this.getRedisAsync(key)
+  }
+}

server/index.ts

@@ -1,8 +1,9 @@
 import createApp from './app'
 import HmppsAuthClient from './data/hmppsAuthClient'
+import TokenStore from './data/tokenStore'
 import UserService from './services/userService'
 
-const hmppsAuthClient = new HmppsAuthClient()
+const hmppsAuthClient = new HmppsAuthClient(new TokenStore())
 const userService = new UserService(hmppsAuthClient)
 
 const app = createApp(userService)

server/services/userService.test.ts

@@ -1,19 +1,21 @@
 import UserService from './userService'
-import MockedHmppsAuthClient from '../data/testutils/hmppsAuthClientSetup'
+import HmppsAuthClient, { User } from '../data/hmppsAuthClient'
+
+jest.mock('../data/hmppsAuthClient')
 
 const token = 'some token'
 
 describe('User service', () => {
-  let hmppsAuthClient
+  let hmppsAuthClient: jest.Mocked<HmppsAuthClient>
   let userService: UserService
 
   describe('getUser', () => {
     beforeEach(() => {
-      hmppsAuthClient = new MockedHmppsAuthClient()
+      hmppsAuthClient = new HmppsAuthClient(null) as jest.Mocked<HmppsAuthClient>
       userService = new UserService(hmppsAuthClient)
     })
     it('Retrieves and formats user name', async () => {
-      hmppsAuthClient.getUser.mockResolvedValue({ name: 'john smith' })
+      hmppsAuthClient.getUser.mockResolvedValue({ name: 'john smith' } as User)
 
       const result = await userService.getUser(token)