Skip to content

Commit c5a128c

Browse files
kiro-agent[bot]kiro-agent[bot]
authored
fix: look up admin email from DynamoDB instead of unreliable event claims (#30)
1 parent ddeb7cc commit c5a128c

5 files changed

Lines changed: 260 additions & 12 deletions

File tree

‎backend/lib/api-stack.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,7 @@ export class APIStack extends cdk.Stack {
395395
NAMESPACE: namespace,
396396
TRAININGS_TABLE_NAME: trainingsTable.tableName,
397397
VOCABULARY_LISTS_TABLE_NAME: vocabularyListsTable.tableName,
398+
USERS_TABLE_NAME: usersTable.tableName,
398399
},
399400
};
400401

@@ -536,6 +537,7 @@ export class APIStack extends cdk.Stack {
536537
});
537538

538539
trainingsTable.grantReadWriteData(getTrainingStatisticsFunction);
540+
usersTable.grantReadData(getTrainingStatisticsFunction);
539541

540542
const getTrainingStatisticsDataSource = api.addLambdaDataSource(
541543
'GetTrainingStatisticsDataSource',
@@ -555,6 +557,7 @@ export class APIStack extends cdk.Stack {
555557
});
556558

557559
trainingsTable.grantReadWriteData(getTrainingDayStatisticsFunction);
560+
usersTable.grantReadData(getTrainingDayStatisticsFunction);
558561

559562
const getTrainingDayStatisticsDataSource = api.addLambdaDataSource(
560563
'GetTrainingDayStatisticsDataSource',
@@ -574,6 +577,7 @@ export class APIStack extends cdk.Stack {
574577
});
575578

576579
trainingsTable.grantReadWriteData(getTrainingOverviewStatisticsFunction);
580+
usersTable.grantReadData(getTrainingOverviewStatisticsFunction);
577581

578582
const getTrainingOverviewStatisticsDataSource = api.addLambdaDataSource(
579583
'GetTrainingOverviewStatisticsDataSource',

‎backend/src/gql-lambda-functions/Query.getTrainingDayStatistics.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { TrainingService } from '../services/training-service';
2+
import { UserRepository } from '../repositories/user-repository';
23

34
const ADMIN_EMAILS = ['johannes.koch@gmail.com', 'lockhead+joe1@lockhead.info'];
45

@@ -15,15 +16,11 @@ interface Event {
1516
};
1617
identity: {
1718
sub: string;
18-
claims?: {
19-
email?: string;
20-
};
2119
};
2220
}
2321

2422
export const handler = async (event: Event) => {
2523
const callerUserId = event.identity?.sub;
26-
const callerEmail = event.identity?.claims?.email;
2724
const { date, userId: targetUserId } = event.arguments;
2825

2926
if (!callerUserId) {
@@ -36,6 +33,9 @@ export const handler = async (event: Event) => {
3633

3734
let effectiveUserId = callerUserId;
3835
if (targetUserId) {
36+
const userRepo = UserRepository.getInstance();
37+
const callerUser = await userRepo.getById(callerUserId);
38+
const callerEmail = callerUser?.email;
3939
if (!callerEmail || !ADMIN_EMAILS.includes(callerEmail)) {
4040
return {
4141
success: false,

‎backend/src/gql-lambda-functions/Query.getTrainingOverviewStatistics.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { TrainingService } from '../services/training-service';
2+
import { UserRepository } from '../repositories/user-repository';
23

34
const ADMIN_EMAILS = ['johannes.koch@gmail.com', 'lockhead+joe1@lockhead.info'];
45

@@ -16,15 +17,11 @@ interface Event {
1617
};
1718
identity: {
1819
sub: string;
19-
claims?: {
20-
email?: string;
21-
};
2220
};
2321
}
2422

2523
export const handler = async (event: Event) => {
2624
const callerUserId = event.identity?.sub;
27-
const callerEmail = event.identity?.claims?.email;
2825
const { fromDate, toDate, userId: targetUserId } = event.arguments;
2926

3027
if (!callerUserId) {
@@ -37,6 +34,9 @@ export const handler = async (event: Event) => {
3734

3835
let effectiveUserId = callerUserId;
3936
if (targetUserId) {
37+
const userRepo = UserRepository.getInstance();
38+
const callerUser = await userRepo.getById(callerUserId);
39+
const callerEmail = callerUser?.email;
4040
if (!callerEmail || !ADMIN_EMAILS.includes(callerEmail)) {
4141
return {
4242
success: false,

‎backend/src/gql-lambda-functions/Query.getTrainingStatistics.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { TrainingService } from '../services/training-service';
2+
import { UserRepository } from '../repositories/user-repository';
23

34
const ADMIN_EMAILS = ['johannes.koch@gmail.com', 'lockhead+joe1@lockhead.info'];
45

@@ -15,15 +16,11 @@ interface Event {
1516
};
1617
identity: {
1718
sub: string;
18-
claims?: {
19-
email?: string;
20-
};
2119
};
2220
}
2321

2422
export const handler = async (event: Event) => {
2523
const callerUserId = event.identity?.sub;
26-
const callerEmail = event.identity?.claims?.email;
2724
const { trainingId, userId: targetUserId } = event.arguments;
2825

2926
if (!callerUserId) {
@@ -37,6 +34,9 @@ export const handler = async (event: Event) => {
3734
// If a targetUserId is provided, only admins may use it
3835
let effectiveUserId = callerUserId;
3936
if (targetUserId) {
37+
const userRepo = UserRepository.getInstance();
38+
const callerUser = await userRepo.getById(callerUserId);
39+
const callerEmail = callerUser?.email;
4040
if (!callerEmail || !ADMIN_EMAILS.includes(callerEmail)) {
4141
return {
4242
success: false,

‎backend/test/admin-authorization.test.ts‎

Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
import { describe, test, expect, beforeEach } from 'vitest';
2+
import { mockClient } from 'aws-sdk-client-mock';
3+
import { DynamoDBDocumentClient, GetCommand, QueryCommand } from '@aws-sdk/lib-dynamodb';
4+
import { handler as getTrainingStatisticsHandler } from '../src/gql-lambda-functions/Query.getTrainingStatistics';
5+
import { handler as getTrainingOverviewStatisticsHandler } from '../src/gql-lambda-functions/Query.getTrainingOverviewStatistics';
6+
import { handler as getTrainingDayStatisticsHandler } from '../src/gql-lambda-functions/Query.getTrainingDayStatistics';
7+
8+
const ddbMock = mockClient(DynamoDBDocumentClient);
9+
10+
const USERS_TABLE = 'train-with-joe-users-sandbox';
11+
const TRAININGS_TABLE = 'train-with-joe-trainings-sandbox';
12+
13+
const ADMIN_EMAIL = 'johannes.koch@gmail.com';
14+
const NON_ADMIN_EMAIL = 'regular-user@example.com';
15+
16+
const makeUser = (id: string, email: string) => ({
17+
id,
18+
email,
19+
createdAt: '2024-01-01T00:00:00.000Z',
20+
updatedAt: '2024-01-01T00:00:00.000Z',
21+
});
22+
23+
describe('Admin Authorization', () => {
24+
beforeEach(() => {
25+
ddbMock.reset();
26+
});
27+
28+
describe('Query.getTrainingStatistics', () => {
29+
test('should return authentication error when no identity.sub', async () => {
30+
const result = await getTrainingStatisticsHandler({
31+
arguments: { trainingId: 'training-1' },
32+
identity: { sub: '' },
33+
} as any);
34+
35+
expect(result.success).toBe(false);
36+
expect(result.error).toBe('Authentication required');
37+
});
38+
39+
test('should deny non-admin user from viewing other user statistics', async () => {
40+
const callerId = 'caller-123';
41+
42+
ddbMock.on(GetCommand).callsFake((input) => {
43+
if (input.TableName === USERS_TABLE) {
44+
return { Item: makeUser(callerId, NON_ADMIN_EMAIL) };
45+
}
46+
return {};
47+
});
48+
49+
const result = await getTrainingStatisticsHandler({
50+
arguments: { trainingId: 'training-1', userId: 'other-user-456' },
51+
identity: { sub: callerId },
52+
} as any);
53+
54+
expect(result.success).toBe(false);
55+
expect(result.error).toBe("Not authorized to view other users' statistics");
56+
});
57+
58+
test('should deny when caller user not found in DynamoDB', async () => {
59+
const callerId = 'non-existent-user';
60+
61+
ddbMock.on(GetCommand).callsFake((input) => {
62+
if (input.TableName === USERS_TABLE) {
63+
return {};
64+
}
65+
return {};
66+
});
67+
68+
const result = await getTrainingStatisticsHandler({
69+
arguments: { trainingId: 'training-1', userId: 'other-user-456' },
70+
identity: { sub: callerId },
71+
} as any);
72+
73+
expect(result.success).toBe(false);
74+
expect(result.error).toBe("Not authorized to view other users' statistics");
75+
});
76+
77+
test('should allow admin user to view other user statistics', async () => {
78+
const callerId = 'admin-123';
79+
const targetUserId = 'target-456';
80+
81+
ddbMock.on(GetCommand).callsFake((input) => {
82+
if (input.TableName === USERS_TABLE) {
83+
return { Item: makeUser(callerId, ADMIN_EMAIL) };
84+
}
85+
if (input.TableName === TRAININGS_TABLE) {
86+
return {};
87+
}
88+
return {};
89+
});
90+
91+
ddbMock.on(QueryCommand).resolves({ Items: [] });
92+
93+
const result = await getTrainingStatisticsHandler({
94+
arguments: { trainingId: 'training-1', userId: targetUserId },
95+
identity: { sub: callerId },
96+
} as any);
97+
98+
// Should not return the authorization error
99+
expect(result.error).not.toBe("Not authorized to view other users' statistics");
100+
});
101+
});
102+
103+
describe('Query.getTrainingOverviewStatistics', () => {
104+
test('should return authentication error when no identity.sub', async () => {
105+
const result = await getTrainingOverviewStatisticsHandler({
106+
arguments: { fromDate: '2024-01-01', toDate: '2024-01-31' },
107+
identity: { sub: '' },
108+
} as any);
109+
110+
expect(result.success).toBe(false);
111+
expect(result.error).toBe('Authentication required');
112+
});
113+
114+
test('should deny non-admin user from viewing other user statistics', async () => {
115+
const callerId = 'caller-123';
116+
117+
ddbMock.on(GetCommand).callsFake((input) => {
118+
if (input.TableName === USERS_TABLE) {
119+
return { Item: makeUser(callerId, NON_ADMIN_EMAIL) };
120+
}
121+
return {};
122+
});
123+
124+
const result = await getTrainingOverviewStatisticsHandler({
125+
arguments: { fromDate: '2024-01-01', toDate: '2024-01-31', userId: 'other-user-456' },
126+
identity: { sub: callerId },
127+
} as any);
128+
129+
expect(result.success).toBe(false);
130+
expect(result.error).toBe("Not authorized to view other users' statistics");
131+
});
132+
133+
test('should deny when caller user not found in DynamoDB', async () => {
134+
const callerId = 'non-existent-user';
135+
136+
ddbMock.on(GetCommand).callsFake((input) => {
137+
if (input.TableName === USERS_TABLE) {
138+
return {};
139+
}
140+
return {};
141+
});
142+
143+
const result = await getTrainingOverviewStatisticsHandler({
144+
arguments: { fromDate: '2024-01-01', toDate: '2024-01-31', userId: 'other-user-456' },
145+
identity: { sub: callerId },
146+
} as any);
147+
148+
expect(result.success).toBe(false);
149+
expect(result.error).toBe("Not authorized to view other users' statistics");
150+
});
151+
152+
test('should allow admin user to view other user statistics', async () => {
153+
const callerId = 'admin-123';
154+
const targetUserId = 'target-456';
155+
156+
ddbMock.on(GetCommand).callsFake((input) => {
157+
if (input.TableName === USERS_TABLE) {
158+
return { Item: makeUser(callerId, ADMIN_EMAIL) };
159+
}
160+
return {};
161+
});
162+
163+
ddbMock.on(QueryCommand).resolves({ Items: [] });
164+
165+
const result = await getTrainingOverviewStatisticsHandler({
166+
arguments: { fromDate: '2024-01-01', toDate: '2024-01-31', userId: targetUserId },
167+
identity: { sub: callerId },
168+
} as any);
169+
170+
expect(result.error).not.toBe("Not authorized to view other users' statistics");
171+
});
172+
});
173+
174+
describe('Query.getTrainingDayStatistics', () => {
175+
test('should return authentication error when no identity.sub', async () => {
176+
const result = await getTrainingDayStatisticsHandler({
177+
arguments: { date: '2024-01-15' },
178+
identity: { sub: '' },
179+
} as any);
180+
181+
expect(result.success).toBe(false);
182+
expect(result.error).toBe('Authentication required');
183+
});
184+
185+
test('should deny non-admin user from viewing other user statistics', async () => {
186+
const callerId = 'caller-123';
187+
188+
ddbMock.on(GetCommand).callsFake((input) => {
189+
if (input.TableName === USERS_TABLE) {
190+
return { Item: makeUser(callerId, NON_ADMIN_EMAIL) };
191+
}
192+
return {};
193+
});
194+
195+
const result = await getTrainingDayStatisticsHandler({
196+
arguments: { date: '2024-01-15', userId: 'other-user-456' },
197+
identity: { sub: callerId },
198+
} as any);
199+
200+
expect(result.success).toBe(false);
201+
expect(result.error).toBe("Not authorized to view other users' statistics");
202+
});
203+
204+
test('should deny when caller user not found in DynamoDB', async () => {
205+
const callerId = 'non-existent-user';
206+
207+
ddbMock.on(GetCommand).callsFake((input) => {
208+
if (input.TableName === USERS_TABLE) {
209+
return {};
210+
}
211+
return {};
212+
});
213+
214+
const result = await getTrainingDayStatisticsHandler({
215+
arguments: { date: '2024-01-15', userId: 'other-user-456' },
216+
identity: { sub: callerId },
217+
} as any);
218+
219+
expect(result.success).toBe(false);
220+
expect(result.error).toBe("Not authorized to view other users' statistics");
221+
});
222+
223+
test('should allow admin user to view other user statistics', async () => {
224+
const callerId = 'admin-123';
225+
const targetUserId = 'target-456';
226+
227+
ddbMock.on(GetCommand).callsFake((input) => {
228+
if (input.TableName === USERS_TABLE) {
229+
return { Item: makeUser(callerId, ADMIN_EMAIL) };
230+
}
231+
return {};
232+
});
233+
234+
ddbMock.on(QueryCommand).resolves({ Items: [] });
235+
236+
const result = await getTrainingDayStatisticsHandler({
237+
arguments: { date: '2024-01-15', userId: targetUserId },
238+
identity: { sub: callerId },
239+
} as any);
240+
241+
expect(result.error).not.toBe("Not authorized to view other users' statistics");
242+
});
243+
});
244+
});

0 commit comments

Comments
 (0)