Skip to content

Commit d1d314f

Browse files
committed
CCM-11870 Check for client ID in authorizer
1 parent 01c40fa commit d1d314f

File tree

2 files changed

+66
-5
lines changed

2 files changed

+66
-5
lines changed

lambdas/authorizer/src/__tests__/index.test.ts

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ const allowPolicy = {
4343
},
4444
context: {
4545
user: 'sub',
46+
clientId: 'client-123',
4647
},
4748
};
4849

@@ -72,10 +73,11 @@ afterEach(() => {
7273
process.env = originalEnv;
7374
});
7475

75-
test('returns Allow policy on valid token', async () => {
76+
test('returns Allow policy on valid token with clientId', async () => {
7677
lambdaCognitoAuthorizer.authorize.mockResolvedValue({
7778
success: true,
7879
subject: 'sub',
80+
clientId: 'client-123',
7981
});
8082

8183
const res = await handler(
@@ -99,6 +101,58 @@ test('returns Allow policy on valid token', async () => {
99101
);
100102
});
101103

104+
test('returns Deny policy when authorisation succeeds but clientId is missing', async () => {
105+
lambdaCognitoAuthorizer.authorize.mockResolvedValue({
106+
success: true,
107+
subject: 'sub',
108+
clientId: undefined,
109+
});
110+
111+
const res = await handler(
112+
mock<APIGatewayRequestAuthorizerEvent>({
113+
requestContext,
114+
headers: { Authorization: 'jwt' },
115+
type: 'REQUEST',
116+
}),
117+
mock<Context>(),
118+
jest.fn()
119+
);
120+
121+
expect(res).toEqual({
122+
...denyPolicy,
123+
context: { user: 'sub' },
124+
});
125+
expect(mockLogger.warn).toHaveBeenCalledWith(
126+
'Authorisation denied: missing clientId'
127+
);
128+
});
129+
130+
test('returns Deny policy when authorize succeeds but clientId is empty/whitespace', async () => {
131+
lambdaCognitoAuthorizer.authorize.mockResolvedValue({
132+
success: true,
133+
subject: 'sub',
134+
clientId: ' ',
135+
});
136+
137+
const res = await handler(
138+
mock<APIGatewayRequestAuthorizerEvent>({
139+
requestContext,
140+
headers: { Authorization: 'jwt' },
141+
type: 'REQUEST',
142+
}),
143+
mock<Context>(),
144+
jest.fn()
145+
);
146+
147+
expect(res).toEqual({
148+
...denyPolicy,
149+
context: { user: 'sub' },
150+
});
151+
expect(mockLogger.warn).toHaveBeenCalledWith(
152+
'Authorisation denied: missing clientId'
153+
);
154+
});
155+
102156
test('returns Deny policy on lambda misconfiguration', async () => {
103157
process.env.USER_POOL_ID = '';
104158

lambdas/authorizer/src/index.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,17 @@ export const handler: APIGatewayRequestAuthorizerHandler = async (event) => {
6565
);
6666

6767
if (authResult.success) {
68-
return generatePolicy(methodArn, 'Allow', {
69-
user: authResult.subject,
70-
clientId: authResult.clientId,
71-
});
68+
const clientId = authResult.clientId;
69+
70+
if (!clientId || clientId.trim() === '') {
71+
logger.warn('Authorisation denied: missing clientId');
72+
return generatePolicy(methodArn, 'Deny', { user: authResult.subject });
73+
} else {
74+
return generatePolicy(methodArn, 'Allow', {
75+
user: authResult.subject,
76+
clientId,
77+
});
78+
}
7279
}
7380

7481
return generatePolicy(methodArn, 'Deny');

0 commit comments

Comments
 (0)