Skip to content
Open
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
40 changes: 40 additions & 0 deletions apps/backend/src/auth/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import {
AdminCreateUserCommand,
} from '@aws-sdk/client-cognito-identity-provider';

import {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: include these imports in the same one as above

AdminAddUserToGroupCommand,
AdminRemoveUserFromGroupCommand,
} from '@aws-sdk/client-cognito-identity-provider';

import CognitoAuthConfig from './aws-exports';
import { SignUpDto } from './dtos/sign-up.dto';
import { createHmac } from 'crypto';
Expand Down Expand Up @@ -70,4 +75,39 @@ export class AuthService {
}
}
}

async addUserToGroup(username: string, groupName: string): Promise<void> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Yurika-Kan now that we have this function, should we be calling it for when we create a new volunteer as well? what about pantry and fm?

const command = new AdminAddUserToGroupCommand({
UserPoolId: CognitoAuthConfig.userPoolId,
Username: username,
GroupName: groupName,
});

try {
await this.providerClient.send(command);
} catch (error) {
throw new InternalServerErrorException(
`Failed to add user to group ${groupName}`,
);
}
}

async removeUserFromGroup(
username: string,
groupName: string,
): Promise<void> {
const command = new AdminRemoveUserFromGroupCommand({
UserPoolId: CognitoAuthConfig.userPoolId,
Username: username,
GroupName: groupName,
});

try {
await this.providerClient.send(command);
} catch (error) {
throw new InternalServerErrorException(
`Failed to remove user from group ${groupName}`,
);
}
}
}
6 changes: 5 additions & 1 deletion apps/backend/src/pantries/pantries.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Test, TestingModule } from '@nestjs/testing';
import { PantriesService } from './pantries.service';
import { getRepositoryToken } from '@nestjs/typeorm';
import { In } from 'typeorm';
import { DataSource, In } from 'typeorm';
import { Pantry } from './pantries.entity';
import {
BadRequestException,
Expand Down Expand Up @@ -145,6 +145,10 @@ describe('PantriesService', () => {
provide: getRepositoryToken(FoodManufacturer),
useValue: testDataSource.getRepository(FoodManufacturer),
},
{
provide: DataSource,
useValue: testDataSource,
},
],
}).compile();

Expand Down
8 changes: 8 additions & 0 deletions apps/backend/src/users/dtos/update-user-role.dto.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { IsEnum, IsNotEmpty } from 'class-validator';
import { Role } from '../types';

export class UpdateUserRoleDto {
@IsEnum(Role)
@IsNotEmpty()
role!: Role;
}
71 changes: 70 additions & 1 deletion apps/backend/src/users/users.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { userSchemaDto } from './dtos/userSchema.dto';
import { Test, TestingModule } from '@nestjs/testing';
import { mock } from 'jest-mock-extended';
import { UpdateUserInfoDto } from './dtos/update-user-info.dto';
import { BadRequestException } from '@nestjs/common';
import { UpdateUserRoleDto } from './dtos/update-user-role.dto';
import { BadRequestException, NotFoundException } from '@nestjs/common';
import { AuthenticatedRequest } from '../auth/authenticated-request';

const mockUserService = mock<UsersService>();
Expand All @@ -31,6 +32,7 @@ describe('UsersController', () => {
mockUserService.create.mockReset();
mockUserService.getUserDashboardStats.mockReset();
mockUserService.getRecentPendingApplications.mockReset();
mockUserService.promoteVolunteerToAdmin.mockReset();

const module: TestingModule = await Test.createTestingModule({
controllers: [UsersController],
Expand Down Expand Up @@ -211,4 +213,71 @@ describe('UsersController', () => {
expect(result).toEqual([]);
});
});

describe('PATCH /:id/role', () => {
it('should promote volunteer to admin successfully', async () => {
const promotedUser: Partial<User> = {
...mockUser1,
role: Role.ADMIN,
};

const dto: UpdateUserRoleDto = { role: Role.ADMIN };

mockUserService.promoteVolunteerToAdmin.mockResolvedValueOnce(
promotedUser as User,
);

const result = await controller.promoteToAdmin(1, dto);

expect(result).toEqual(promotedUser);
expect(result.role).toBe(Role.ADMIN);
expect(mockUserService.promoteVolunteerToAdmin).toHaveBeenCalledWith(1);
});

it('should throw BadRequestException when role is not admin', async () => {
const dto: UpdateUserRoleDto = { role: Role.VOLUNTEER };

await expect(controller.promoteToAdmin(1, dto)).rejects.toThrow(
new BadRequestException('Only promotion to admin is supported'),
);

expect(mockUserService.promoteVolunteerToAdmin).not.toHaveBeenCalled();
});

it('should throw BadRequestException when role is pantry', async () => {
const dto: UpdateUserRoleDto = { role: Role.PANTRY };

await expect(controller.promoteToAdmin(1, dto)).rejects.toThrow(
new BadRequestException('Only promotion to admin is supported'),
);

expect(mockUserService.promoteVolunteerToAdmin).not.toHaveBeenCalled();
});

it('should throw NotFoundException from service when user not found', async () => {
const dto: UpdateUserRoleDto = { role: Role.ADMIN };

mockUserService.promoteVolunteerToAdmin.mockRejectedValueOnce(
new NotFoundException('User 999 not found'),
);

await expect(controller.promoteToAdmin(999, dto)).rejects.toThrow(
new NotFoundException('User 999 not found'),
);
});

it('should throw BadRequestException from service when user is not a volunteer', async () => {
const dto: UpdateUserRoleDto = { role: Role.ADMIN };

mockUserService.promoteVolunteerToAdmin.mockRejectedValueOnce(
new BadRequestException(
'User 1 is not a volunteer. Current role: admin',
),
);

await expect(controller.promoteToAdmin(1, dto)).rejects.toThrow(
BadRequestException,
);
});
});
});
14 changes: 14 additions & 0 deletions apps/backend/src/users/users.controller.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
BadRequestException,
Controller,
Delete,
Get,
Expand All @@ -14,6 +15,7 @@ import { UsersService } from './users.service';
import { User } from './users.entity';
import { userSchemaDto } from './dtos/userSchema.dto';
import { UpdateUserInfoDto } from './dtos/update-user-info.dto';
import { UpdateUserRoleDto } from './dtos/update-user-role.dto';
import { PendingApplication, Role } from './types';
import { AuthenticatedRequest } from '../auth/authenticated-request';
import { JwtAuthGuard } from '../auth/jwt-auth.guard';
Expand Down Expand Up @@ -53,6 +55,18 @@ export class UsersController {
return this.usersService.update(id, dto);
}

@Patch('/:id/role')
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can we rename this to something more descriptive, perhaps @Patch('/:id/promote-volunteer)

@Roles(Role.ADMIN)
async promoteToAdmin(
@Param('id', ParseIntPipe) id: number,
@Body() dto: UpdateUserRoleDto,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a very rare case where we do not need a body. We know we are already promoting the volunteer to admin, so we shouldn't need to verify any other data. Can we delete this and the dto, and the check in the controller?

): Promise<User> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of our patch api calls will return void you'll see, let's do the same here and update the service function and all tests.

if (dto.role !== Role.ADMIN) {
throw new BadRequestException('Only promotion to admin is supported');
}
return this.usersService.promoteVolunteerToAdmin(id);
}

// Keeping these two as functionality seems useful
@Post('/')
async createUser(@Body() createUserDto: userSchemaDto): Promise<User> {
Expand Down
119 changes: 119 additions & 0 deletions apps/backend/src/users/users.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ jest.setTimeout(60000);

const mockAuthService = {
adminCreateUser: jest.fn().mockResolvedValue('mock-sub'),
addUserToGroup: jest.fn().mockResolvedValue(undefined),
removeUserFromGroup: jest.fn().mockResolvedValue(undefined),
};
const mockEmailsService = mock<EmailsService>();

Expand Down Expand Up @@ -125,6 +127,8 @@ describe('UsersService', () => {

beforeEach(async () => {
mockAuthService.adminCreateUser.mockClear();
mockAuthService.addUserToGroup.mockClear();
mockAuthService.removeUserFromGroup.mockClear();
mockEmailsService.sendEmails.mockClear();
await testDataSource.runMigrations();
});
Expand Down Expand Up @@ -730,4 +734,119 @@ describe('UsersService', () => {
expect(types).toContain('food_manufacturer');
});
});

describe('promoteVolunteerToAdmin', () => {
it('should promote volunteer to admin successfully', async () => {
const volunteers = await testDataSource.getRepository(User).find({
where: { role: Role.VOLUNTEER },
});
expect(volunteers.length).toBeGreaterThan(0);
const volunteer = volunteers[0];

const result = await service.promoteVolunteerToAdmin(volunteer.id);

expect(result.role).toBe(Role.ADMIN);
expect(result.id).toBe(volunteer.id);
});

it('should clear volunteer pantry assignments after promotion', async () => {
const volunteer = await testDataSource.getRepository(User).findOne({
where: { role: Role.VOLUNTEER },
relations: ['pantries'],
});
expect(volunteer).toBeDefined();

await service.promoteVolunteerToAdmin(volunteer!.id);

const assignments = await testDataSource.query(
`SELECT * FROM volunteer_assignments WHERE volunteer_id = $1`,
[volunteer!.id],
);
expect(assignments).toHaveLength(0);
});

it('should call Cognito addUserToGroup and removeUserFromGroup', async () => {
const volunteer = await testDataSource.getRepository(User).findOne({
where: { role: Role.VOLUNTEER },
});
expect(volunteer).toBeDefined();

await service.promoteVolunteerToAdmin(volunteer!.id);

if (volunteer!.userCognitoSub) {
expect(mockAuthService.addUserToGroup).toHaveBeenCalledWith(
volunteer!.email,
'admin',
);
expect(mockAuthService.removeUserFromGroup).toHaveBeenCalledWith(
volunteer!.email,
'volunteer',
);
}
});

it('should throw NotFoundException when user does not exist', async () => {
await expect(service.promoteVolunteerToAdmin(99999)).rejects.toThrow(
NotFoundException,
);
});

it('should throw BadRequestException when user is already admin', async () => {
const admin = await testDataSource.getRepository(User).findOne({
where: { role: Role.ADMIN },
});
expect(admin).toBeDefined();

await expect(service.promoteVolunteerToAdmin(admin!.id)).rejects.toThrow(
BadRequestException,
);
});

it('should throw BadRequestException when user is pantry', async () => {
const pantryUser = await testDataSource.getRepository(User).findOne({
where: { role: Role.PANTRY },
});
expect(pantryUser).toBeDefined();

await expect(
service.promoteVolunteerToAdmin(pantryUser!.id),
).rejects.toThrow(BadRequestException);
});

it('should throw BadRequestException when user is food manufacturer', async () => {
const fmUser = await testDataSource.getRepository(User).findOne({
where: { role: Role.FOODMANUFACTURER },
});
expect(fmUser).toBeDefined();

await expect(service.promoteVolunteerToAdmin(fmUser!.id)).rejects.toThrow(
BadRequestException,
);
});

it('should rollback if Cognito fails', async () => {
const userRepo = testDataSource.getRepository(User);
const volunteer = await userRepo.findOne({
where: { role: Role.VOLUNTEER },
});
expect(volunteer).toBeDefined();

// Set userCognitoSub so the Cognito code path is triggered
volunteer!.userCognitoSub = 'test-cognito-sub';
await userRepo.save(volunteer!);

mockAuthService.addUserToGroup.mockRejectedValueOnce(
new Error('Cognito error'),
);

await expect(
service.promoteVolunteerToAdmin(volunteer!.id),
).rejects.toThrow(InternalServerErrorException);

const userAfter = await userRepo.findOne({
where: { id: volunteer!.id },
});
expect(userAfter!.role).toBe(Role.VOLUNTEER);
});
});
});
Loading
Loading