Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BNGP-5491: Implement OAuth authentication #896

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import Boom from '@hapi/boom'
import { submitGetRequest, submitPostRequest } from '../helpers/server.js'
import constants from '../../../utils/constants.js'
import wreck from '@hapi/wreck'
import getWithAuth from '../../../utils/get-with-auth.js'
import { BACKEND_API } from '../../../utils/config.js'
import { SessionMap } from '../../../utils/sessionMap.js'
const url = constants.routes.DEVELOPER_BNG_NUMBER

jest.mock('../../../utils/get-with-auth.js')

describe(url, () => {
describe('GET', () => {
it(`should render the ${url.substring(1)} view`, async () => {
Expand Down Expand Up @@ -33,59 +36,73 @@ describe(url, () => {

it('Should continue journey if valid BGS number provided', async () => {
postOptions.payload.bgsNumber = 'BGS-010124001'
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'registered' })
const res = await submitPostRequest(postOptions)
expect(res.headers.location).toEqual('/developer/upload-metric-file')
})

it('Should fail journey if BGS number blank', async () => {
const res = await submitPostRequest(postOptions, 500)
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'registered' })
expect(res.payload).toContain('Sorry, there is a problem with the service')
})

it('Should show appropriate error if only spaces provided', async () => {
postOptions.payload.bgsNumber = ''
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'registered' })
const res = await submitPostRequest(postOptions, 200)
expect(res.payload).toContain('Enter your biodiversity gain site number')
})

it('Should show appropriate error if BGS number is in active state', async () => {
postOptions.payload.bgsNumber = 'active '
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'active' })
const res = await submitPostRequest(postOptions, 200)
expect(res.payload).toContain('This gain site registration is not complete - wait until you have confirmation.')
})

it('Should show appropriate error if BGS number is in rejected state', async () => {
postOptions.payload.bgsNumber = ' rejected'
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'rejected' })
const res = await submitPostRequest(postOptions, 200)
expect(res.payload).toContain('This reference is for a rejected application - enter a reference for an approved gain site.')
})

it('Should show appropriate error if BGS number does not exist', async () => {
postOptions.payload.bgsNumber = ' doesNotExist '
getWithAuth.mockImplementationOnce(() => {
throw Boom.notFound()
})
const res = await submitPostRequest(postOptions, 200)
expect(res.payload).toContain('The gain site reference was not recognised - enter a reference for an approved gain site.')
})

it('Should show appropriate error if BGS number is in removed state', async () => {
postOptions.payload.bgsNumber = 'removed'
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'removed' })
const res = await submitPostRequest(postOptions, 200)
expect(res.payload).toContain('This reference is for a gain site which is no longer registered.')
})

it('Should show appropriate error if BGS number is in internally removed state', async () => {
postOptions.payload.bgsNumber = 'internally-removed'
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'internally-removed' })
const res = await submitPostRequest(postOptions, 200)
expect(res.payload).toContain('This reference is for a gain site which is no longer registered.')
})

it('Should show appropriate error if BGS number is in inactive state', async () => {
postOptions.payload.bgsNumber = 'inactive'
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'inactive' })
const res = await submitPostRequest(postOptions, 200)
expect(res.payload).toContain('This reference is for a gain site which has been withdrawn from registration.')
})

it('Should show appropriate error if BGS number return unknown error', async () => {
postOptions.payload.bgsNumber = 'default-error'
getWithAuth.mockImplementationOnce(() => {
throw Boom.badRequest()
})
const res = await submitPostRequest(postOptions, 200)
expect(res.payload).toContain('There was a problem checking your gain site reference - please try again later.')
})
Expand All @@ -105,6 +122,7 @@ describe(url, () => {
bgsNumber: 'BGS-010124001'
}
}
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'Registered' })
const developerBgsNumber = require('../../developer/biodiversity-gain-site-number')
await developerBgsNumber.default[1].handler(request, h)
expect(viewResult).toBe(constants.routes.DEVELOPER_CHECK_UPLOAD_METRIC)
Expand All @@ -127,6 +145,7 @@ describe(url, () => {
bgsNumber
}
}
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'Registered' })
const developerBgsNumber = require('../../developer/biodiversity-gain-site-number')
await developerBgsNumber.default[1].handler(request, h)
expect(viewResult).toBe(constants.routes.DEVELOPER_CHECK_AND_SUBMIT)
Expand All @@ -149,6 +168,7 @@ describe(url, () => {
bgsNumber
}
}
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'Registered' })
const developerBgsNumber = require('../../developer/biodiversity-gain-site-number')
await developerBgsNumber.default[1].handler(request, h)
expect(viewResult).toBe(constants.routes.DEVELOPER_CHECK_AND_SUBMIT)
Expand All @@ -170,49 +190,46 @@ describe(url, () => {
bgsNumber
}
}
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'Registered' })
const developerBgsNumber = require('../../developer/biodiversity-gain-site-number')
await developerBgsNumber.default[1].handler(request, h)
expect(viewResult).toBe(constants.routes.DEVELOPER_UPLOAD_METRIC)
})

it('Should call the API with a `code` query param if one is configured', async () => {
jest.mock('@hapi/wreck')
const spy = jest.spyOn(wreck, 'get')
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'registered' })
jest.replaceProperty(BACKEND_API, 'CODE_QUERY_PARAMETER', 'test123')

postOptions.payload.bgsNumber = 'BGS-010124001'
await submitPostRequest(postOptions)
expect(spy.mock.calls[0][0]).toContain('code=test123')
expect(getWithAuth.mock.calls[0][0]).toContain('code=test123')
})

it('Should not call the API with a `code` query param if one is not configured', async () => {
jest.mock('@hapi/wreck')
const spy = jest.spyOn(wreck, 'get')
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'registered' })
jest.replaceProperty(BACKEND_API, 'CODE_QUERY_PARAMETER', undefined)

postOptions.payload.bgsNumber = 'BGS-010124001'
await submitPostRequest(postOptions)
expect(spy.mock.calls[0][0]).not.toContain('code=')
expect(getWithAuth.mock.calls[0][0]).not.toContain('code=')
})

it('Should accept mock BGS value (used for acceptance test) if configured and matches bgsNumber, bypassing API call', async () => {
jest.mock('@hapi/wreck')
const spy = jest.spyOn(wreck, 'get')
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'registered' })
jest.replaceProperty(BACKEND_API, 'MOCK_BGS_FOR_ACCEPTANCE', 'test123')

postOptions.payload.bgsNumber = 'test123'
await submitPostRequest(postOptions)
expect(spy).toHaveBeenCalledTimes(0)
expect(getWithAuth).toHaveBeenCalledTimes(0)
})

it('Should ignore mock BGS value (used for acceptance test) if configured and doesn\'t matches bgsNumber, not bypassing API call', async () => {
jest.mock('@hapi/wreck')
const spy = jest.spyOn(wreck, 'get')
getWithAuth.mockResolvedValueOnce({ gainsiteStatus: 'registered' })
jest.replaceProperty(BACKEND_API, 'MOCK_BGS_FOR_ACCEPTANCE', 'test123')

postOptions.payload.bgsNumber = 'notTest123'
await submitPostRequest(postOptions)
expect(spy).toHaveBeenCalledTimes(1)
expect(getWithAuth).toHaveBeenCalledTimes(1)
})
})
})
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import constants from '../../utils/constants.js'
import { BACKEND_API } from '../../utils/config.js'
import getWithAuth from '../../utils/get-with-auth.js'
import { deleteBlobFromContainers } from '../../utils/azure-storage.js'
import wreck from '@hapi/wreck'

const getGainSiteApiUrl = bgsNumber => {
const getGainsiteApiUrl = bgsNumber => {
const url = new URL(`${BACKEND_API.BASE_URL}gainsite/${bgsNumber}`)
if (BACKEND_API.CODE_QUERY_PARAMETER) {
url.searchParams.set('code', BACKEND_API.CODE_QUERY_PARAMETER)
Expand All @@ -12,7 +12,11 @@ const getGainSiteApiUrl = bgsNumber => {
}

const getStatusErrorMessage = status => {
switch (status.toLowerCase()) {
if (!status) {
return null
}

switch (status) {
case 'active':
return 'This gain site registration is not complete - wait until you have confirmation.'
case 'rejected':
Expand All @@ -22,41 +26,54 @@ const getStatusErrorMessage = status => {
return 'This reference is for a gain site which is no longer registered.'
case 'inactive':
return 'This reference is for a gain site which has been withdrawn from registration.'
case 'not-found':
return 'The gain site reference was not recognised - enter a reference for an approved gain site.'
case 'empty':
return 'Enter your biodiversity gain site number'
default:
return 'There was a problem checking your gain site reference - please try again later.'
}
}

const checkBGSNumber = async (bgsNumber, hrefId) => {
let errorText
const determineGainsiteStatus = async (bgsNumber) => {
if (!bgsNumber.trim()) {
return 'empty'
}

try {
const gainsiteUrl = getGainsiteApiUrl(bgsNumber)
const payload = await getWithAuth(gainsiteUrl.href)

if (typeof payload.gainsiteStatus !== 'string') {
throw Error()
}

const lowerCaseStatus = payload.gainsiteStatus.toLowerCase()

if (lowerCaseStatus !== 'registered') {
return payload.gainsiteStatus.toLowerCase()
}
} catch (err) {
if (err.isBoom && err.output.statusCode === 404) {
return 'not-found'
} else {
return 'generic-error'
}
}

return null
}

const checkBGSNumber = async (bgsNumber, hrefId) => {
// Allow a specific mock value for acceptance tests so that we don't need to add test
// values to the production system. If mock value is set and matches what is entered,
// then don't call API and don't raise an error
if (BACKEND_API.MOCK_BGS_FOR_ACCEPTANCE && bgsNumber === BACKEND_API.MOCK_BGS_FOR_ACCEPTANCE) {
return null
}

if (!bgsNumber.trim()) {
errorText = 'Enter your biodiversity gain site number'
} else {
const gainsiteUrl = getGainSiteApiUrl(bgsNumber)

try {
const { payload } = await wreck.get(gainsiteUrl.href, {
json: true,
headers: {
'Ocp-Apim-Subscription-Key': BACKEND_API.SUBSCRIPTION_KEY
}
})

if (payload.gainsiteStatus !== 'Registered') {
errorText = getStatusErrorMessage(payload.gainsiteStatus)
}
} catch (err) {
errorText = 'The gain site reference was not recognised - enter a reference for an approved gain site.'
}
}
const gainsiteStatus = await determineGainsiteStatus(bgsNumber)
const errorText = getStatusErrorMessage(gainsiteStatus)

if (errorText) {
return [{
Expand Down
88 changes: 88 additions & 0 deletions packages/webapp/src/utils/__tests__/get-with-auth.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import wreck from '@hapi/wreck'
import getWithAuth, { resetTokenCache } from '../get-with-auth.js'
import { BACKEND_API } from '../config'

jest.mock('@hapi/wreck')

describe('getWithAuth', () => {
const mockUrl = 'https://mock-url.com/api/data'

beforeEach(() => {
jest.clearAllMocks()
resetTokenCache()
})

it("should reuse the cached token if it hasn't expired", async () => {
BACKEND_API.USE_OAUTH = true

resetTokenCache({
token: 'cached-token',
expiration: Math.floor(Date.now() / 1000) + 3600
})

wreck.get.mockImplementation(() => {
return Promise.resolve({
payload: { data: 'mock-data' }
})
})

const result = await getWithAuth(mockUrl)

expect(wreck.post).not.toHaveBeenCalled()
expect(wreck.get).toHaveBeenCalledWith(mockUrl, expect.objectContaining({
headers: expect.objectContaining({
Authorization: 'Bearer cached-token'
})
}))
expect(result).toEqual({ data: 'mock-data' })
})

it('should fetch a new token if none is cached', async () => {
BACKEND_API.USE_OAUTH = true

const mockToken = 'new-token'
wreck.post.mockImplementation(() => {
return Promise.resolve({
payload: {
access_token: mockToken,
expires_in: 3600
}
})
})

wreck.get.mockImplementation(() => {
return Promise.resolve({
payload: { data: 'mock-data' }
})
})

const result = await getWithAuth(mockUrl)

expect(wreck.post).toHaveBeenCalled()
expect(wreck.get).toHaveBeenCalledWith(mockUrl, expect.objectContaining({
headers: expect.objectContaining({
Authorization: `Bearer ${mockToken}`
})
}))
expect(result).toEqual({ data: 'mock-data' })
})

it('should use subscription key header when USE_OAUTH is false', async () => {
BACKEND_API.USE_OAUTH = false
BACKEND_API.SUBSCRIPTION_KEY = 'mock-subscription-key'

wreck.get.mockImplementation(() => {
return Promise.resolve({
payload: { data: 'mock-data' }
})
})

await getWithAuth(mockUrl)

expect(wreck.get).toHaveBeenCalledWith(mockUrl, expect.objectContaining({
headers: expect.objectContaining({
'Ocp-Apim-Subscription-Key': 'mock-subscription-key'
})
}))
})
})
7 changes: 6 additions & 1 deletion packages/webapp/src/utils/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,10 @@ export const BACKEND_API = {
BASE_URL: process.env.BACKEND_API_BASE_URL ?? 'http://localhost:3000/test/api/',
SUBSCRIPTION_KEY: process.env.BACKEND_API_SUBSCRIPTION_KEY ?? 'test123',
CODE_QUERY_PARAMETER: process.env.BACKEND_API_CODE_QUERY_PARAMETER,
MOCK_BGS_FOR_ACCEPTANCE: process.env.BACKEND_API_MOCK_BGS_FOR_ACCEPTANCE
MOCK_BGS_FOR_ACCEPTANCE: process.env.BACKEND_API_MOCK_BGS_FOR_ACCEPTANCE,
OAUTH_CLIENT_ID: process.env.BACKEND_API_OAUTH_CLIENT_ID,
OAUTH_SCOPE: process.env.BACKEND_API_OAUTH_SCOPE,
OAUTH_SECRET: process.env.BACKEND_API_OAUTH_SECRET,
OAUTH_TENANT_ID: process.env.BACKEND_API_OAUTH_TENANT_ID,
USE_OAUTH: process.env.BACKEND_API_USE_OAUTH ? JSON.parse(process.env.BACKEND_API_USE_OAUTH) : false
}
Loading
Loading