Skip to content

Commit

Permalink
NN-4114: Caching of auth client credentials (again) (#2155)
Browse files Browse the repository at this point in the history
* Revert "Revert "NN-4114: Caching of auth client credentials (#2150)" (#2154)"

This reverts commit c934a21.

* NN-4114: Use system client for client creds cache

Co-authored-by: sp-watson <steve.watson@digital,justice.gov.uk>
  • Loading branch information
sp-watson and sp-watson authored May 19, 2022
1 parent 901fd66 commit 5afb927
Show file tree
Hide file tree
Showing 9 changed files with 250 additions and 17 deletions.
3 changes: 2 additions & 1 deletion backend/api/oauthApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ export const oauthApiFactory = (client, { clientId, clientSecret, url }) => {
})

// eslint-disable-next-line camelcase
const parseOauthTokens = ({ access_token, refresh_token }) => ({
const parseOauthTokens = ({ access_token, refresh_token, expires_in }) => ({
access_token,
refresh_token,
expires_in,
})

const translateAuthClientError = (error) => {
Expand Down
123 changes: 123 additions & 0 deletions backend/api/systemOauthClient.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import { doesNotMatch } from 'assert'
import redisMock from 'redis-mock'
import { promisify } from 'util'
import systemOauthClient, {
clientCredsSetup,
enableLogDebugStatements,
getClientCredentialsTokens,
} from './systemOauthClient'

describe('system oauth client tests', () => {
const oauthApi = { makeTokenRequest: {} }

describe('Without Redis cache', () => {
beforeEach(() => {
oauthApi.makeTokenRequest = jest.fn().mockResolvedValue({
access_token: '123',
refresh_token: '456',
expires_in: 600,
})
clientCredsSetup(null, oauthApi, true)
})

it('Gets and returns token', async () => {
const clientCreds = await getClientCredentialsTokens(null)
expect(clientCreds).toEqual({
access_token: '123',
refresh_token: '456',
expires_in: 600,
})
})

it('Makes oauth requests each time', async () => {
await getClientCredentialsTokens(null)
await getClientCredentialsTokens(null)
expect(oauthApi.makeTokenRequest).toHaveBeenCalledTimes(2)
})
})

describe('With Redis cache', () => {
let mockRedis

beforeEach(() => {
oauthApi.makeTokenRequest = jest.fn().mockResolvedValue({
access_token: '123',
refresh_token: '456',
expires_in: 600,
})
mockRedis = redisMock.createClient()
clientCredsSetup(mockRedis, oauthApi, true)
})

it('Gets and returns token on the first call with correct expiry', async () => {
const clientCreds = await getClientCredentialsTokens('USER1')
expect(clientCreds).toEqual({
access_token: '123',
refresh_token: '456',
expires_in: 600,
})
expect(oauthApi.makeTokenRequest).toHaveBeenCalledTimes(1)
})

it('Returns the stored token the second call', async () => {
await getClientCredentialsTokens('USER1')
const clientCreds = await getClientCredentialsTokens('USER1')
expect(clientCreds).toEqual({
access_token: '123',
refresh_token: null,
})
expect(oauthApi.makeTokenRequest).toHaveBeenCalledTimes(0)
})

it('Returns the stored token per user', async () => {
await getClientCredentialsTokens('USER1')
const secondUserClientCreds = await getClientCredentialsTokens('USER2')
expect(secondUserClientCreds).toEqual({
access_token: '123',
refresh_token: '456',
expires_in: 600,
})
expect(oauthApi.makeTokenRequest).toHaveBeenCalledWith(
'grant_type=client_credentials&username=USER2',
'PSH-client_credentials'
)
})

it('Works when no username provided', async () => {
const noUserClientCreds = await getClientCredentialsTokens(null)
expect(noUserClientCreds).toEqual({
access_token: '123',
refresh_token: '456',
expires_in: 600,
})
})

it('Expires cached value 1 minute before token expiry', async () => {
await getClientCredentialsTokens('USER1')

const getTtl = promisify(mockRedis.ttl).bind(mockRedis)
const expiryTime = await getTtl('CC_USER1')

const tokenExpiry = 600
expect(expiryTime).toBeLessThan(tokenExpiry - 59)
expect(expiryTime).toBeGreaterThan(tokenExpiry - 120) // Allow 1 minute
})
})

describe('Logging debug statements', () => {
it('Will happen in t3', async () => {
const logDebug = enableLogDebugStatements({ app: { production: true }, phaseName: 'DEV' })
expect(logDebug).toEqual(true)
})

it('Will not happen in production', async () => {
const logDebug = enableLogDebugStatements({ app: { production: true } })
expect(logDebug).toEqual(false)
})

it('Will happen if no config', async () => {
const logDebug = enableLogDebugStatements({})
expect(logDebug).toEqual(true)
})
})
})
108 changes: 94 additions & 14 deletions backend/api/systemOauthClient.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,111 @@
import querystring from 'querystring'
import clientFactory from './oauthEnabledClient'

import { oauthApiFactory } from './oauthApi'
import config from '../config'
import { createClient } from 'redis'
import { promisify } from 'util'
import clientFactory from './oauthEnabledClient'
import logger from '../log'
import { oauthApiFactory } from './oauthApi'

export const getClientCredentialsTokens = async (username) => {
const oauthRequest = username
? querystring.stringify({ grant_type: 'client_credentials', username })
: querystring.stringify({ grant_type: 'client_credentials' })
let getRedisAsync
let setRedisAsync
let oauthClient
let logDebug

const oauthResult = await oauthApiFactory(
export const getSystemOauthApiClient = (configData) => {
return oauthApiFactory(
clientFactory({
baseUrl: config.apis.oauth2.url,
timeout: config.apis.oauth2.timeoutSeconds * 1000,
baseUrl: configData.apis.oauth2.url,
timeout: configData.apis.oauth2.timeoutSeconds * 1000,
}),
{
clientId: config.apis.oauth2.systemClientId,
clientSecret: config.apis.oauth2.systemClientSecret,
url: config.apis.oauth2.url,
clientId: configData.apis.oauth2.systemClientId,
clientSecret: configData.apis.oauth2.systemClientSecret,
url: configData.apis.oauth2.url,
}
).makeTokenRequest(oauthRequest, 'PSH-client_credentials')
)
}

export const getTokenStore = (configData) => {
const { enabled, host, port, password } = configData.redis
if (!enabled || !host) return null

const client = createClient({
host,
port,
password,
tls: configData.app.production ? {} : false,
})

client.on('error', (e: Error) => logger.error(e, 'Redis client error'))

logger.info(`Oauth token store created`)
return client
}

export const enableLogDebugStatements = (configData) => {
return !configData.app?.production || configData.phaseName === 'DEV'
}

export const clientCredsSetup = (tokenStore, oauthApi, logDebugStatements) => {
const redisTokenStore = tokenStore
getRedisAsync = redisTokenStore ? promisify(redisTokenStore?.get).bind(redisTokenStore) : (key) => {}
setRedisAsync = redisTokenStore
? promisify(redisTokenStore?.set).bind(redisTokenStore)
: (key, value, command, expiry) => {}

oauthClient = oauthApi

logDebug = logDebugStatements
}

const requestClientCredentials = async (username) => {
const oauthRequest = username
? querystring.stringify({ grant_type: 'client_credentials', username })
: querystring.stringify({ grant_type: 'client_credentials' })

const oauthResult = await oauthClient.makeTokenRequest(oauthRequest, 'PSH-client_credentials')

logger.debug(`Oauth request for grant type 'client_credentials', result status: successful`)
return oauthResult
}

// Remove this when we are confident caching is working
const debug = (operation: string, username: string) => {
if (logDebug) logger.info(`OAUTH CLIENT CREDS ${operation} FOR ${username}`)
}

const getKey = (username: string): string => {
const baseKey = username || '%ANONYMOUS%'
return `CC_${baseKey}`
}

export const getClientCredentialsTokens = async (username) => {
const key = getKey(username)

debug('GET', key)
const token = await getRedisAsync(key)
if (token) {
debug('GOT', key)
// We need to preserve the oauth result to avoid changing all the code and esp. tests (we use axios).
// According to axios-config-decorators.ts we only use the auth token, custom request headers and pagination.
// For client creds, pagination and custom headers are not relevant when getting client creds.
return {
// Need only access token - refresh token and authSource (as per useApiClientCreds.ts) are actually not used
// for client creds (the access functions are in contextProperties.ts)
access_token: token,
refresh_token: null,
}
}

const oauthResult = await requestClientCredentials(username)

// set TTL slightly less than expiry of token. Async but no need to wait
await setRedisAsync(key, oauthResult.access_token, 'EX', oauthResult.expires_in - 60)
debug(`SET-${oauthResult.expires_in}`, key)

return oauthResult
}

export default {
getClientCredentialsTokens,
}
7 changes: 7 additions & 0 deletions backend/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ import setupHealthChecks from './setupHealthChecks'
import setupBodyParsers from './setupBodyParsers'
import setupWebSecurity from './setupWebSecurity'
import setupAuth from './setupAuth'
import {
clientCredsSetup,
getTokenStore,
getSystemOauthApiClient,
enableLogDebugStatements,
} from './api/systemOauthClient'
import setupStaticContent from './setupStaticContent'
import nunjucksSetup from './utils/nunjucksSetup'
import setupRedirects from './setupRedirects'
Expand Down Expand Up @@ -47,6 +53,7 @@ app.set('view engine', 'njk')

nunjucksSetup(app)
phaseNameSetup(app, config)
clientCredsSetup(getTokenStore(config), getSystemOauthApiClient(config), enableLogDebugStatements(config))

app.use(cookieParser())
app.use(setupBodyParsers())
Expand Down
2 changes: 1 addition & 1 deletion backend/middleware/requestLimiter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ const initRedisStoreIfEnabled = ({ host, port, password, production }) => {
password,
tls: production ? {} : false,
})
client.on('error', (e: Error) => logger.error('Redis client error', e))
client.on('error', (e: Error) => logger.error(e, 'Redis client error'))
return { store: new RedisStore({ client }) }
}
2 changes: 1 addition & 1 deletion backend/setupWebSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export default () => {
tls: config.app.production ? {} : false,
})

client.on('error', (e: Error) => logger.error('Redis client error', e))
client.on('error', (e: Error) => logger.error(e, 'Redis client error'))

return new RedisStore({ client })
}
Expand Down
5 changes: 5 additions & 0 deletions integration-tests/mockApis/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ const stubClientCredentialsRequest = () =>
},
response: {
status: 200,
jsonBody: {
access_token: 'EXAMPLE_ACCESS_TOKEN',
refresh_token: 'EXAMPLE_REFRESH_TOKEN',
expires_in: 43200,
},
},
})

Expand Down
16 changes: 16 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@
"process": "0.11.10",
"raf": "^3.4.0",
"react-test-renderer": "16.8.6",
"redis-mock": "^0.56.3",
"redux-mock-store": "^1.5.3",
"sass-loader": "^12.1.0",
"set-cookie-parser": "^2.4.8",
Expand Down

0 comments on commit 5afb927

Please sign in to comment.