Skip to content

Commit

Permalink
fix(jobs): allow super admin to update any job & improve test coverage
Browse files Browse the repository at this point in the history
- Add super admin authorization to job updates
- Add unit tests for super admin and regular user scenarios
- Fix authorization logic in JobAccessGuard
- Maintain backward compatibility for job owners
- Update test coverage for edge cases

Related: #1257
  • Loading branch information
Daggahh committed Mar 2, 2025
1 parent 411c69b commit f906059
Show file tree
Hide file tree
Showing 12 changed files with 117 additions and 10 deletions.
37 changes: 37 additions & 0 deletions src/guards/job-access.guard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { Injectable, CanActivate, ExecutionContext } from '@nestjs/common';
import { InjectRepository } from '@nestjs/typeorm';
import { Repository } from 'typeorm';
import { Job } from '../modules/jobs/entities/job.entity';
import { User } from '@modules/user/entities/user.entity';

@Injectable()
export class JobAccessGuard implements CanActivate {
constructor(
@InjectRepository(Job)
private readonly jobRepository: Repository<Job>,
@InjectRepository(User)
private readonly userRepository: Repository<User>
) {}

async canActivate(context: ExecutionContext): Promise<boolean> {
const request = context.switchToHttp().getRequest();
const jobId = request.params.id;
const userId = request.user.sub;

const user = await this.userRepository.findOne({
where: { id: userId },
});

if (user?.is_superadmin) {
return true;
}

const job = await this.jobRepository.findOne({
where: { id: jobId },
relations: ['user'],
});

if (!job) return false;
return job.user.id === userId;
}
}
1 change: 1 addition & 0 deletions src/modules/invite/mocks/mockUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export const mockUser: User = {
first_name: 'John',
last_name: 'Doe',
is_active: true,
is_superadmin: false,
phone: '+1234567890',
status: 'Hello from the children of planet Earth',
id: 'some-uuid-value-here',
Expand Down
4 changes: 2 additions & 2 deletions src/modules/jobs/jobs.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { JobSearchDto } from './dto/jobSearch.dto';
import { UpdateJobDto } from './dto/update-job.dto';
import { JobOwnerGuard } from '../../guards/job-owner.guard';
import { AuthGuard } from '../../guards/auth.guard';
import { JobAccessGuard } from '@guards/job-access.guard';

@ApiTags('Jobs')
@Controller('jobs')
Expand Down Expand Up @@ -110,8 +111,7 @@ export class JobsController {
}

@Patch('/:id')
@UseGuards(AuthGuard)
@UseGuards(SuperAdminGuard, JobOwnerGuard)
@UseGuards(AuthGuard, JobAccessGuard)
@ApiBearerAuth()
@ApiOperation({ summary: 'Update a job posting' })
@ApiResponse({ status: 200, description: 'Job updated successfully' })
Expand Down
5 changes: 2 additions & 3 deletions src/modules/jobs/jobs.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,15 @@ import { OrganisationUserRole } from '@modules/role/entities/organisation-user-r
import { Profile } from '@modules/profile/entities/profile.entity';
import { Role } from '@modules/role/entities/role.entity';
import { UserModule } from '@modules/user/user.module';
import { JobOwnerGuard } from '@guards/job-owner.guard';
import { AuthGuard } from '@guards/auth.guard';
import { SuperAdminGuard } from '@guards/super-admin.guard';
import { JobAccessGuard } from '@guards/job-access.guard';

@Module({
imports: [
TypeOrmModule.forFeature([Job, User, JobApplication, Organisation, OrganisationUserRole, Profile, Role]),
UserModule,
],
providers: [JobsService, JobOwnerGuard, AuthGuard, SuperAdminGuard],
providers: [JobsService, JobAccessGuard, AuthGuard],
controllers: [JobsController],
})
export class JobsModule {}
15 changes: 10 additions & 5 deletions src/modules/jobs/jobs.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,16 +169,21 @@ export class JobsService {
this.validateUserId(userId);
this.validateUpdateData(updateJobDto);

const job = await this.jobRepository.findOne({
where: { id },
relations: ['user'],
});
const [job, user] = await Promise.all([
this.jobRepository.findOne({
where: { id },
relations: ['user'],
}),
this.userRepository.findOne({
where: { id: userId },
}),
]);

if (!job) {
throw new CustomHttpException('Job not found', HttpStatus.NOT_FOUND);
}

if (job.user.id !== userId) {
if (job.user.id !== userId && !user?.is_superadmin) {
throw new CustomHttpException('Unauthorized to update this job', HttpStatus.FORBIDDEN);
}

Expand Down
57 changes: 57 additions & 0 deletions src/modules/jobs/tests/jobs.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,12 @@ describe('JobsService', () => {
user: { ...mockUser, id: 'different_user_id' } as User,
} as Job;

// Mock both repository calls
jest.spyOn(jobRepository, 'findOne').mockResolvedValue(mockJob);
jest.spyOn(userRepository, 'findOne').mockResolvedValue({
...mockUser,
is_superadmin: false,
} as User);

await expect(service.update('job-id', updateDto, 'user_id')).rejects.toThrow(
new CustomHttpException('Unauthorized to update this job', HttpStatus.FORBIDDEN)
Expand Down Expand Up @@ -392,7 +397,12 @@ describe('JobsService', () => {
user: { ...mockUser, id: 'different_user_id' } as User,
} as Job;

// Mock both repository calls
jest.spyOn(jobRepository, 'findOne').mockResolvedValue(mockJob);
jest.spyOn(userRepository, 'findOne').mockResolvedValue({
...mockUser,
is_superadmin: false,
} as User);

await expect(service.update('job-id', updateDto, 'user_id')).rejects.toThrow(
new CustomHttpException('Unauthorized to update this job', HttpStatus.FORBIDDEN)
Expand Down Expand Up @@ -455,5 +465,52 @@ describe('JobsService', () => {
})
);
});

it('should allow super admin to update any job', async () => {
const superAdminUser = {
...mockUser,
is_superadmin: true,
id: 'super_admin_id',
};

const jobOwnedByOtherUser = {
...jobsMock[0],
user: { ...mockUser, id: 'other_user_id' } as User,
} as Job;

jest.spyOn(jobRepository, 'findOne').mockResolvedValue(jobOwnedByOtherUser);
jest.spyOn(userRepository, 'findOne').mockResolvedValue(superAdminUser as User);
jest.spyOn(jobRepository, 'save').mockResolvedValue({
...jobOwnedByOtherUser,
...updateDto,
} as Job);

const result = await service.update('job-id', updateDto, 'super_admin_id');

expect(result.status).toBe('success');
expect(result.status_code).toBe(200);
expect((result.data as Job).title).toBe(updateDto.title);
});

it('should not allow non-owner non-admin user to update job', async () => {
const regularUser = {
...mockUser,
is_superadmin: false,
id: 'regular_user_id',
};

const jobOwnedByOtherUser = {
...jobsMock[0],
user: { ...mockUser, id: 'other_user_id' } as User,
} as Job;

// Mock both repository calls
jest.spyOn(jobRepository, 'findOne').mockResolvedValue(jobOwnedByOtherUser);
jest.spyOn(userRepository, 'findOne').mockResolvedValue(regularUser as User);

await expect(service.update('job-id', updateDto, 'regular_user_id')).rejects.toThrow(
new CustomHttpException('Unauthorized to update this job', HttpStatus.FORBIDDEN)
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export const mockUser: User = {
status: 'Hello from the children of planet Earth',
hashPassword: async () => {},
is_active: true,
is_superadmin: false,
attempts_left: 3,
time_left: 3600,
owned_organisations: [],
Expand Down
1 change: 1 addition & 0 deletions src/modules/organisations/tests/mocks/organisation.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export const createMockOrganisation = (): Organisation => {
phone: '+1234567890',
hashPassword: async () => {},
is_active: true,
is_superadmin: false,
attempts_left: 3,
time_left: 3600,
owned_organisations: [],
Expand Down
1 change: 1 addition & 0 deletions src/modules/organisations/tests/mocks/user.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,5 @@ export const mockUser = {
comments: null,
cart: [],
organisations: null,
is_superadmin: false,
};
1 change: 1 addition & 0 deletions src/modules/profile/mocks/mockUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const mockUserWithProfile: User = {
first_name: 'John',
last_name: 'Doe',
is_active: true,
is_superadmin: false,
phone: '+1234567891',
id: 'some-uuid-value-here',
attempts_left: 2,
Expand Down
3 changes: 3 additions & 0 deletions src/modules/user/entities/user.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ export class User extends AbstractBaseEntity {
@Column({ default: false })
is_2fa_enabled: boolean;

@Column({ default: false })
is_superadmin: boolean;

@DeleteDateColumn({ nullable: true })
deletedAt?: Date;

Expand Down
1 change: 1 addition & 0 deletions src/modules/user/tests/mocks/user.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ export const mockUser: User = {
hashPassword: () => null,
cart: [],
organisations: null,
is_superadmin: false,
};

0 comments on commit f906059

Please sign in to comment.