Skip to content

Commit caf1c40

Browse files
nicki-nhsharrim91
andauthored
CCM-11870-middleware-checks-client-id (#655)
Co-authored-by: Michael Harrison <[email protected]>
1 parent b0e3162 commit caf1c40

File tree

9 files changed

+247
-65
lines changed

9 files changed

+247
-65
lines changed

frontend/src/__tests__/middleware.test.ts

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@
44
import { NextRequest } from 'next/server';
55
import { getSessionServer } from '@utils/amplify-utils';
66
import { middleware } from '../middleware';
7+
import { getClientIdFromToken } from '@utils/token-utils';
78

89
jest.mock('@utils/amplify-utils');
10+
jest.mock('@utils/token-utils');
911

1012
const getTokenMock = jest.mocked(getSessionServer);
13+
const getClientIdFromTokenMock = jest.mocked(getClientIdFromToken);
1114

1215
function getCsp(response: Response) {
1316
const csp = response.headers.get('Content-Security-Policy');
@@ -19,6 +22,13 @@ afterAll(() => {
1922
process.env = OLD_ENV;
2023
});
2124

25+
beforeEach(() => {
26+
jest.resetAllMocks();
27+
getClientIdFromTokenMock.mockImplementation((token) =>
28+
token ? 'client1' : undefined
29+
);
30+
});
31+
2232
describe('middleware function', () => {
2333
it('If route is not registered in middleware, respond with 404', async () => {
2434
const url = new URL('https://url.com/message-templates/does-not-exist');
@@ -28,7 +38,7 @@ describe('middleware function', () => {
2838
expect(response.status).toBe(404);
2939
});
3040

31-
it('if request path is protected, and no access token is obtained, redirect to auth page', async () => {
41+
it('if request path is protected, and no access/id token is obtained, redirect to auth page', async () => {
3242
const url = new URL('https://url.com/message-templates');
3343
const request = new NextRequest(url);
3444
request.cookies.set('csrf_token', 'some-csrf-value');
@@ -50,9 +60,9 @@ describe('middleware function', () => {
5060
expect(response.cookies.get('csrf_token')?.value).toEqual('');
5161
});
5262

53-
it('if request path is protected, and access token is obtained, respond with CSP', async () => {
63+
it('if request path is protected, tokens exist AND token has client-id, respond with CSP', async () => {
5464
getTokenMock.mockResolvedValueOnce({
55-
accessToken: 'token',
65+
accessToken: 'access-token',
5666
clientId: 'client1',
5767
});
5868

@@ -61,6 +71,9 @@ describe('middleware function', () => {
6171
const response = await middleware(request);
6272
const csp = getCsp(response);
6373

74+
expect(response.status).toBe(200);
75+
expect(getClientIdFromTokenMock).toHaveBeenCalledTimes(1);
76+
6477
expect(csp).toEqual([
6578
"base-uri 'self'",
6679
"default-src 'none'",
@@ -79,12 +92,58 @@ describe('middleware function', () => {
7992
]);
8093
});
8194

95+
it('if request path is protected, tokens exist BUT token missing client-id, redirect to request-to-be-added page', async () => {
96+
getTokenMock.mockResolvedValueOnce({
97+
accessToken: 'access-token',
98+
});
99+
100+
getClientIdFromTokenMock.mockReturnValueOnce(undefined);
101+
getClientIdFromTokenMock.mockReturnValueOnce(undefined);
102+
103+
const url = new URL('https://url.com/message-templates');
104+
const request = new NextRequest(url);
105+
const response = await middleware(request);
106+
107+
expect(response.status).toBe(307);
108+
expect(response.headers.get('location')).toBe(
109+
'https://url.com/auth/request-to-be-added-to-a-service?redirect=%2Ftemplates%2Fmessage-templates'
110+
);
111+
});
112+
82113
it('if request path is not protected, respond with CSP', async () => {
83114
const url = new URL('https://url.com/create-and-submit-templates');
84115
const request = new NextRequest(url);
85116
const response = await middleware(request);
86117
const csp = getCsp(response);
87118

119+
expect(response.status).toBe(200);
120+
expect(csp).toEqual([
121+
"base-uri 'self'",
122+
"default-src 'none'",
123+
"frame-ancestors 'none'",
124+
"font-src 'self' https://assets.nhs.uk",
125+
"form-action 'self'",
126+
"frame-src 'self'",
127+
"connect-src 'self' https://cognito-idp.eu-west-2.amazonaws.com",
128+
"img-src 'self'",
129+
"manifest-src 'self'",
130+
"object-src 'none'",
131+
expect.stringMatching(/^script-src 'self' 'nonce-[\dA-Za-z]+'$/),
132+
expect.stringMatching(/^style-src 'self' 'nonce-[\dA-Za-z]+'$/),
133+
'upgrade-insecure-requests',
134+
'',
135+
]);
136+
});
137+
138+
it('public path (/auth/request-to-be-added-to-a-service) responds with CSP', async () => {
139+
const url = new URL(
140+
'https://url.com/auth/request-to-be-added-to-a-service'
141+
);
142+
const request = new NextRequest(url);
143+
const response = await middleware(request);
144+
const csp = getCsp(response);
145+
146+
expect(response.status).toBe(200);
88147
expect(csp).toEqual([
89148
"base-uri 'self'",
90149
"default-src 'none'",

frontend/src/__tests__/utils/amplify-utils.test.ts

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,7 @@
33
*/
44
import { sign } from 'jsonwebtoken';
55
import { fetchAuthSession } from 'aws-amplify/auth/server';
6-
import {
7-
getSessionServer,
8-
getSessionId,
9-
getClientId,
10-
} from '../../utils/amplify-utils';
6+
import { getSessionServer, getSessionId } from '../../utils/amplify-utils';
117

128
jest.mock('aws-amplify/auth/server');
139
jest.mock('@aws-amplify/adapter-nextjs/api');
@@ -73,7 +69,10 @@ describe('amplify-utils', () => {
7369

7470
const result = await getSessionServer();
7571

76-
expect(result).toEqual({ accessToken: undefined });
72+
expect(result).toEqual({
73+
accessToken: undefined,
74+
clientId: undefined,
75+
});
7776
});
7877

7978
test('getSessionServer - should return undefined properties if an error occurs', async () => {
@@ -83,7 +82,10 @@ describe('amplify-utils', () => {
8382

8483
const result = await getSessionServer();
8584

86-
expect(result).toEqual({ accessToken: undefined });
85+
expect(result).toEqual({
86+
accessToken: undefined,
87+
clientId: undefined,
88+
});
8789
});
8890

8991
describe('getSessionId', () => {
@@ -127,25 +129,4 @@ describe('amplify-utils', () => {
127129
expect(sessionId).toEqual('jti');
128130
});
129131
});
130-
131-
describe('getClientId', () => {
132-
test('returns undefined when client ID not found', async () => {
133-
const mockAccessToken = sign({}, 'key');
134-
135-
const clientId = await getClientId(mockAccessToken);
136-
137-
expect(clientId).toBeUndefined();
138-
});
139-
140-
test('retrieves client id from access token param', async () => {
141-
const mockAccessToken = sign(
142-
{ ['nhs-notify:client-id']: 'client2' },
143-
'key'
144-
);
145-
146-
const clientId = await getClientId(mockAccessToken);
147-
148-
expect(clientId).toEqual('client2');
149-
});
150-
});
151132
});
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/**
2+
* @jest-environment node
3+
*/
4+
import { sign } from 'jsonwebtoken';
5+
import { decodeJwt, getClaim, getClientIdFromToken } from '@utils/token-utils';
6+
import { JWT } from 'aws-amplify/auth';
7+
8+
describe('token-utils', () => {
9+
describe('decodeJwt', () => {
10+
it('decodes a valid JWT payload', () => {
11+
const token = sign({ testKey: 'value', testNum: 1 }, 'secret');
12+
const claims = decodeJwt(token);
13+
14+
expect(claims.testKey).toBe('value');
15+
expect(claims.testNum).toBe(1);
16+
});
17+
});
18+
19+
describe('getClaim', () => {
20+
it('returns a string when the claim exists (string)', () => {
21+
const claims = { testKey: 'value' } as unknown as JWT['payload'];
22+
23+
expect(getClaim(claims, 'testKey')).toBe('value');
24+
});
25+
26+
it('returns stringified value for non-strings', () => {
27+
const claims = { num: 123, bool: true } as unknown as JWT['payload'];
28+
29+
expect(getClaim(claims, 'num')).toBe('123');
30+
expect(getClaim(claims, 'bool')).toBe('true');
31+
});
32+
33+
it('returns undefined when the claim is missing, null or undefined', () => {
34+
const claims = { a: null, b: undefined } as unknown as JWT['payload'];
35+
36+
expect(getClaim(claims, 'missing')).toBeUndefined();
37+
expect(getClaim(claims, 'a')).toBeUndefined();
38+
expect(getClaim(claims, 'b')).toBeUndefined();
39+
});
40+
});
41+
42+
describe('getClientIdFromToken', () => {
43+
test('returns undefined when client ID not found', async () => {
44+
const mockAccessToken = sign({}, 'key');
45+
46+
const clientId = await getClientIdFromToken(mockAccessToken);
47+
48+
expect(clientId).toBeUndefined();
49+
});
50+
51+
test('retrieves client id from access token param', async () => {
52+
const mockAccessToken = sign(
53+
{ ['nhs-notify:client-id']: 'client2' },
54+
'key'
55+
);
56+
57+
const clientId = await getClientIdFromToken(mockAccessToken);
58+
59+
expect(clientId).toEqual('client2');
60+
});
61+
});
62+
});

frontend/src/middleware.ts

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { NextResponse, type NextRequest } from 'next/server';
22
import { getSessionServer } from '@utils/amplify-utils';
33
import { getBasePath } from '@utils/get-base-path';
4+
import { getClientIdFromToken } from '@utils/token-utils';
45

56
const protectedPaths = [
67
/^\/choose-a-template-type$/,
@@ -42,6 +43,7 @@ const publicPaths = [
4243
/^\/auth\/signin$/,
4344
/^\/auth\/signout$/,
4445
/^\/auth\/idle$/,
46+
/^\/auth\/request-to-be-added-to-a-service$/,
4547
];
4648

4749
function getContentSecurityPolicy(nonce: string) {
@@ -96,16 +98,15 @@ export async function middleware(request: NextRequest) {
9698
return new NextResponse('Page not found', { status: 404 });
9799
}
98100

99-
const { accessToken } = await getSessionServer({ forceRefresh: true });
101+
const { accessToken } = await getSessionServer({
102+
forceRefresh: true,
103+
});
100104

101105
if (!accessToken) {
106+
const path = `${getBasePath()}${pathname}`;
107+
102108
const redirectResponse = NextResponse.redirect(
103-
new URL(
104-
`/auth?redirect=${encodeURIComponent(
105-
`${getBasePath()}${request.nextUrl.pathname}`
106-
)}`,
107-
request.url
108-
)
109+
new URL(`/auth?redirect=${encodeURIComponent(path)}`, request.url)
109110
);
110111

111112
redirectResponse.headers.set('Content-Type', 'text/html');
@@ -115,6 +116,20 @@ export async function middleware(request: NextRequest) {
115116
return redirectResponse;
116117
}
117118

119+
const hasClientId = getClientIdFromToken(accessToken);
120+
121+
if (!hasClientId) {
122+
const missingClientIdRedirect = new URL(
123+
`/auth/request-to-be-added-to-a-service?redirect=${encodeURIComponent(
124+
`${getBasePath()}${pathname}`
125+
)}`,
126+
request.url
127+
);
128+
const redirectResponse = NextResponse.redirect(missingClientIdRedirect);
129+
130+
return redirectResponse;
131+
}
132+
118133
const response = NextResponse.next({
119134
request: {
120135
headers: requestHeaders,

frontend/src/utils/amplify-utils.ts

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
import { cookies } from 'next/headers';
66
import { createServerRunner } from '@aws-amplify/adapter-nextjs';
77
import { fetchAuthSession } from 'aws-amplify/auth/server';
8-
import { FetchAuthSessionOptions, JWT } from 'aws-amplify/auth';
9-
import { jwtDecode } from 'jwt-decode';
8+
import { FetchAuthSessionOptions } from 'aws-amplify/auth';
9+
import { decodeJwt, getClaim, getClientIdFromToken } from './token-utils';
1010

1111
const config = require('@/amplify_outputs.json');
1212

@@ -17,8 +17,8 @@ export const { runWithAmplifyServerContext } = createServerRunner({
1717
export async function getSessionServer(
1818
options: FetchAuthSessionOptions = {}
1919
): Promise<{
20-
accessToken: string | undefined;
21-
clientId: string | undefined;
20+
accessToken?: string;
21+
clientId?: string;
2222
}> {
2323
const session = await runWithAmplifyServerContext({
2424
nextServerContext: { cookies },
@@ -28,39 +28,23 @@ export async function getSessionServer(
2828
});
2929

3030
const accessToken = session?.tokens?.accessToken?.toString();
31-
const clientId = accessToken && (await getClientId(accessToken));
31+
const clientId = accessToken && getClientIdFromToken(accessToken);
3232

3333
return {
3434
accessToken,
3535
clientId,
3636
};
3737
}
3838

39-
export const getSessionId = async () => {
40-
return getAccessTokenParam('origin_jti');
41-
};
42-
43-
export const getClientId = async (accessToken: string) => {
44-
return getJwtPayload('nhs-notify:client-id', accessToken);
45-
};
46-
4739
const getAccessTokenParam = async (key: string) => {
4840
const authSession = await getSessionServer();
4941
const accessToken = authSession.accessToken;
5042

5143
if (!accessToken) return;
5244

53-
return getJwtPayload(key, accessToken);
45+
return getClaim(decodeJwt(accessToken), key);
5446
};
5547

56-
const getJwtPayload = (key: string, accessToken: string) => {
57-
const jwt = jwtDecode<JWT['payload']>(accessToken);
58-
59-
const value = jwt[key];
60-
61-
if (!value) {
62-
return;
63-
}
64-
65-
return value.toString();
48+
export const getSessionId = async () => {
49+
return getAccessTokenParam('origin_jti');
6650
};

frontend/src/utils/token-utils.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { jwtDecode } from 'jwt-decode';
2+
import { JWT } from 'aws-amplify/auth';
3+
4+
export const decodeJwt = (token: string): JWT['payload'] =>
5+
jwtDecode<JWT['payload']>(token);
6+
7+
export const getClaim = (
8+
claims: JWT['payload'],
9+
key: string
10+
): string | undefined => {
11+
const value = claims[key];
12+
return value == null ? undefined : String(value);
13+
};
14+
15+
export const getClientIdFromToken = (token: string) => {
16+
return getClaim(decodeJwt(token), 'nhs-notify:client-id');
17+
};

0 commit comments

Comments
 (0)