Skip to content

Commit da39ba4

Browse files
committed
CCM-11870 Check for client ID on protected routes
1 parent e3e6690 commit da39ba4

File tree

6 files changed

+121
-41
lines changed

6 files changed

+121
-41
lines changed

frontend/src/__tests__/middleware.test.ts

Lines changed: 65 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,14 +38,15 @@ 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');
3545

3646
getTokenMock.mockResolvedValueOnce({
3747
accessToken: undefined,
3848
clientId: undefined,
49+
idToken: undefined,
3950
});
4051

4152
const response = await middleware(request);
@@ -50,17 +61,21 @@ describe('middleware function', () => {
5061
expect(response.cookies.get('csrf_token')?.value).toEqual('');
5162
});
5263

53-
it('if request path is protected, and access token is obtained, respond with CSP', async () => {
64+
it('if request path is protected, tokens exist AND token has client-id, respond with CSP', async () => {
5465
getTokenMock.mockResolvedValueOnce({
55-
accessToken: 'token',
66+
accessToken: 'access-token',
5667
clientId: 'client1',
68+
idToken: 'id-token',
5769
});
5870

5971
const url = new URL('https://url.com/message-templates');
6072
const request = new NextRequest(url);
6173
const response = await middleware(request);
6274
const csp = getCsp(response);
6375

76+
expect(response.status).toBe(200);
77+
expect(getClientIdFromTokenMock).toHaveBeenCalledTimes(2);
78+
6479
expect(csp).toEqual([
6580
"base-uri 'self'",
6681
"default-src 'none'",
@@ -79,12 +94,59 @@ describe('middleware function', () => {
7994
]);
8095
});
8196

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

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

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

Lines changed: 1 addition & 26 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

@@ -160,25 +156,4 @@ describe('amplify-utils', () => {
160156
expect(sessionId).toEqual('jti');
161157
});
162158
});
163-
164-
describe('getClientId', () => {
165-
test('returns undefined when client ID not found', async () => {
166-
const mockAccessToken = sign({}, 'key');
167-
168-
const clientId = await getClientId(mockAccessToken);
169-
170-
expect(clientId).toBeUndefined();
171-
});
172-
173-
test('retrieves client id from access token param', async () => {
174-
const mockAccessToken = sign(
175-
{ ['nhs-notify:client-id']: 'client2' },
176-
'key'
177-
);
178-
179-
const clientId = await getClientId(mockAccessToken);
180-
181-
expect(clientId).toEqual('client2');
182-
});
183-
});
184159
});

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@
22
* @jest-environment node
33
*/
44
import { sign } from 'jsonwebtoken';
5-
import { decodeJwt, getClaim, getIdTokenClaims } from '@utils/token-utils';
5+
import {
6+
decodeJwt,
7+
getClaim,
8+
getClientIdFromToken,
9+
getIdTokenClaims,
10+
} from '@utils/token-utils';
611
import { JWT } from 'aws-amplify/auth';
712

813
describe('token-utils', () => {
@@ -134,4 +139,25 @@ describe('token-utils', () => {
134139
expect(claims.displayName).toBeUndefined();
135140
});
136141
});
142+
143+
describe('getClientIdFromToken', () => {
144+
test('returns undefined when client ID not found', async () => {
145+
const mockAccessToken = sign({}, 'key');
146+
147+
const clientId = await getClientIdFromToken(mockAccessToken);
148+
149+
expect(clientId).toBeUndefined();
150+
});
151+
152+
test('retrieves client id from access token param', async () => {
153+
const mockAccessToken = sign(
154+
{ ['nhs-notify:client-id']: 'client2' },
155+
'key'
156+
);
157+
158+
const clientId = await getClientIdFromToken(mockAccessToken);
159+
160+
expect(clientId).toEqual('client2');
161+
});
162+
});
137163
});

frontend/src/middleware.ts

Lines changed: 22 additions & 5 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,14 +98,14 @@ 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, idToken } = await getSessionServer({
102+
forceRefresh: true,
103+
});
100104

101-
if (!accessToken) {
105+
if (!accessToken || !idToken) {
102106
const redirectResponse = NextResponse.redirect(
103107
new URL(
104-
`/auth?redirect=${encodeURIComponent(
105-
`${getBasePath()}${request.nextUrl.pathname}`
106-
)}`,
108+
`/auth?redirect=${encodeURIComponent(`${getBasePath()}${pathname}`)}`,
107109
request.url
108110
)
109111
);
@@ -115,6 +117,21 @@ export async function middleware(request: NextRequest) {
115117
return redirectResponse;
116118
}
117119

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

frontend/src/utils/amplify-utils.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { cookies } from 'next/headers';
66
import { createServerRunner } from '@aws-amplify/adapter-nextjs';
77
import { fetchAuthSession } from 'aws-amplify/auth/server';
88
import { FetchAuthSessionOptions } from 'aws-amplify/auth';
9-
import { decodeJwt, getClaim } from './token-utils';
9+
import { decodeJwt, getClaim, getClientIdFromToken } from './token-utils';
1010

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

@@ -29,7 +29,7 @@ export async function getSessionServer(
2929
});
3030

3131
const accessToken = session?.tokens?.accessToken?.toString();
32-
const clientId = accessToken && getClientId(accessToken);
32+
const clientId = accessToken && getClientIdFromToken(accessToken);
3333

3434
const idToken = session?.tokens?.idToken?.toString();
3535

@@ -52,7 +52,3 @@ const getAccessTokenParam = async (key: string) => {
5252
export const getSessionId = async () => {
5353
return getAccessTokenParam('origin_jti');
5454
};
55-
56-
export const getClientId = (accessToken: string) => {
57-
return getClaim(decodeJwt(accessToken), 'nhs-notify:client-id');
58-
};

frontend/src/utils/token-utils.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ export const getClaim = (
1212
return value == null ? undefined : String(value);
1313
};
1414

15+
export const getClientIdFromToken = (token: string) => {
16+
return getClaim(decodeJwt(token), 'nhs-notify:client-id');
17+
};
18+
1519
export const getIdTokenClaims = (
1620
idToken: string
1721
): {

0 commit comments

Comments
 (0)