Skip to content

Commit

Permalink
Improve test coverage & fix exposed issues (#80)
Browse files Browse the repository at this point in the history
* #79 Inject config into the AuthenticationService
* #79 Add test coverage to AuthorisationService.requireUserRole
* #79 Add tests to memory cache and fix exposed issues
* #79 Add test coverage to UserService.loadUserDetailsAndUpdateIfNecessary
* #79 Add docs to loadUserDetailsAndUpdateIfNecessary
* #79 Use function overloading to properly type UserService.getUsers and fix issue where role was not included in filter
* #79 Add unit test for getQuizDetails
* #79 Add new user service function to load a single user
* #79 Refactor and unit test createQuiz
* #79 Ignore rest siblings eslint violation
* #79 Throw explicit UnauthorisedError when unauthorised
  • Loading branch information
danielemery authored Nov 25, 2023
1 parent 9b44d32 commit a6291c9
Show file tree
Hide file tree
Showing 18 changed files with 579 additions and 114 deletions.
10 changes: 4 additions & 6 deletions src/auth/authentication.service.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
import jwt, { JwtHeader, SigningKeyCallback, VerifyOptions } from 'jsonwebtoken';
import jwksClient, { RsaSigningKey } from 'jwks-rsa';

import config from '../config/config';

export class AuthenticationService {
#client: jwksClient.JwksClient;
#options: VerifyOptions;

constructor() {
constructor(auth0Domain: string, auth0Audience: string) {
this.#client = jwksClient({
jwksUri: `https://${config.AUTH0_DOMAIN}/.well-known/jwks.json`,
jwksUri: `https://${auth0Domain}/.well-known/jwks.json`,
});
this.#options = {
algorithms: ['RS256'],
audience: config.AUTH0_AUDIENCE,
issuer: `https://${config.AUTH0_DOMAIN}/`,
audience: auth0Audience,
issuer: `https://${auth0Domain}/`,
};
}

Expand Down
1 change: 1 addition & 0 deletions src/auth/authorisation.errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export class UnauthorisedError extends Error {}
23 changes: 23 additions & 0 deletions src/auth/authorisation.service.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { QuizlordContext } from '..';
import { UnauthorisedError } from './authorisation.errors';
import { AuthorisationService } from './authorisation.service';

describe('authorisation', () => {
describe('authorisation.service', () => {
const sut = new AuthorisationService();
describe('requireUserRole', () => {
it('must do nothing if the user has the required role', () => {
const context = {
roles: ['USER', 'ADMIN'],
} as QuizlordContext;
sut.requireUserRole(context, 'ADMIN');
});
it('must throw an error if the user does not have the required role', () => {
const context = {
roles: ['USER'],
} as QuizlordContext;
expect(() => sut.requireUserRole(context, 'ADMIN')).toThrow(UnauthorisedError);
});
});
});
});
3 changes: 2 additions & 1 deletion src/auth/authorisation.service.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { Role } from '@prisma/client';
import { QuizlordContext } from '..';
import { UnauthorisedError } from './authorisation.errors';

export class AuthorisationService {
requireUserRole(context: QuizlordContext, role: Role) {
if (!context.roles.includes(role)) {
throw new Error('You are not authorised to perform this action');
throw new UnauthorisedError('You are not authorised to perform this action');
}
}
}
1 change: 1 addition & 0 deletions src/quiz/quiz.errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export class MustProvideAtLeastOneFileError extends Error {}
5 changes: 1 addition & 4 deletions src/quiz/quiz.gql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,10 @@ async function createQuiz(
context: QuizlordContext,
): Promise<CreateQuizResult> {
authorisationService.requireUserRole(context, 'USER');
return quizService.createQuiz({
return quizService.createQuiz(context.userId, {
type,
date,
files,
userId: context.userId,
email: context.email,
userName: context.userName,
});
}

Expand Down
141 changes: 138 additions & 3 deletions src/quiz/quiz.service.test.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,44 @@
import { v4 as uuidv4 } from 'uuid';

import { QuizService } from './quiz.service';
import { QuizPersistence } from './quiz.persistence';
import { S3FileService } from '../file/s3.service';
import { Decimal } from '@prisma/client/runtime/library';
import { UserService } from '../user/user.service';
import { MustProvideAtLeastOneFileError } from './quiz.errors';

jest.mock('uuid');

const mockedUUIDv4 = jest.mocked(uuidv4);
const mockPersistence = {
getQuizzesWithUserResults: jest.fn(),
createQuizWithImages: jest.fn(),
getCompletionScoreWithQuizTypesForUser: jest.fn(),
getQuizzesWithUserResults: jest.fn(),
getQuizByIdWithResults: jest.fn(),
};
const mockFileService = {
createKey: jest.fn(),
generateSignedUploadUrl: jest.fn(),
};
const mockUserService = {
getUser: jest.fn(),
};
const mockFileService = {};

const sut = new QuizService(mockPersistence as unknown as QuizPersistence, mockFileService as S3FileService);
const sut = new QuizService(
mockPersistence as unknown as QuizPersistence,
mockFileService as unknown as S3FileService,
mockUserService as unknown as UserService,
);

describe('quiz', () => {
describe('quiz.service', () => {
beforeEach(() => {
jest.clearAllMocks();
jest.restoreAllMocks();
});
afterEach(() => {
jest.useRealTimers();
});
describe('getQuizzesWithUserResults', () => {
it('must call getQuizzesWithUserResults on persistence with correct arguments and transform the result', async () => {
const persistenceResult = [
Expand Down Expand Up @@ -180,5 +203,117 @@ describe('quiz', () => {
});
});
});
describe('getQuizDetails', () => {
it('must call getQuizByIdWithResults on persistence with correct arguments and transform the result', async () => {
mockPersistence.getQuizByIdWithResults.mockResolvedValueOnce({
id: 'fake-quiz-id',
type: 'SHARK',
date: new Date('2023-01-01'),
uploadedAt: new Date('2023-01-02'),
completions: [],
images: [],
uploadedByUser: {
id: 'fake-user-id',
email: '[email protected]',
name: 'Master',
},
});

const actual = await sut.getQuizDetails('fake-quiz-id');

expect(actual).toEqual({
id: 'fake-quiz-id',
type: 'SHARK',
date: new Date('2023-01-01'),
uploadedAt: new Date('2023-01-02'),
completions: [],
images: [],
uploadedBy: {
id: 'fake-user-id',
email: '[email protected]',
name: 'Master',
},
});
});
});
describe('createQuiz', () => {
it('must throw if no files are provided', async () => {
await expect(() =>
sut.createQuiz('fake-user', { type: 'SHARK', date: new Date('2022-01-01'), files: [] }),
).rejects.toThrow(MustProvideAtLeastOneFileError);
});
it('must create quiz with upload link for each image', async () => {
const fakeNow = new Date('2023-02-12');
jest.useFakeTimers();
jest.setSystemTime(fakeNow);

mockedUUIDv4.mockReturnValueOnce('fake-quiz-id');
mockUserService.getUser.mockResolvedValueOnce({
email: '[email protected]',
});
mockFileService.createKey.mockReturnValueOnce('key-one').mockReturnValueOnce('key-two');
mockFileService.generateSignedUploadUrl
.mockResolvedValueOnce('https://upload-one.net')
.mockResolvedValueOnce('https://upload-two.net');

const actual = await sut.createQuiz('fake-user-id', {
date: new Date('2022-01-01'),
type: 'SHARK',
files: [
{
fileName: 'questions.jpg',
type: 'QUESTION',
},
{
fileName: 'answers.jpg',
type: 'ANSWER',
},
],
});

expect(mockedUUIDv4).toHaveBeenCalledTimes(1);
expect(mockFileService.createKey).toHaveBeenCalledTimes(2);
expect(mockFileService.createKey).toHaveBeenNthCalledWith(1, 'fake-quiz-id', 'questions.jpg');
expect(mockFileService.createKey).toHaveBeenNthCalledWith(2, 'fake-quiz-id', 'answers.jpg');
expect(mockFileService.generateSignedUploadUrl).toHaveBeenCalledTimes(2);
expect(mockFileService.generateSignedUploadUrl).toHaveBeenNthCalledWith(1, 'key-one');
expect(mockFileService.generateSignedUploadUrl).toHaveBeenNthCalledWith(2, 'key-two');
expect(mockPersistence.createQuizWithImages).toHaveBeenCalledTimes(1);
expect(mockPersistence.createQuizWithImages).toHaveBeenCalledWith(
{
date: new Date('2022-01-01'),
id: 'fake-quiz-id',
type: 'SHARK',
uploadedAt: new Date(fakeNow),
uploadedByUserId: 'fake-user-id',
},
[
{ imageKey: 'key-one', state: 'PENDING_UPLOAD', type: 'QUESTION' },
{ imageKey: 'key-two', state: 'PENDING_UPLOAD', type: 'ANSWER' },
],
);

expect(actual).toEqual({
quiz: {
myCompletions: [],
uploadedBy: {
email: '[email protected]',
id: 'fake-user-id',
name: undefined,
},
},
uploadLinks: [
{
fileName: 'questions.jpg',
link: 'https://upload-one.net',
},
{
fileName: 'answers.jpg',
link: 'https://upload-two.net',
},
],
});
});
});
});
});
71 changes: 52 additions & 19 deletions src/quiz/quiz.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,20 @@ import {
import { Quiz, QuizCompletion, QuizFilters, QuizImage } from './quiz.dto';
import { S3FileService } from '../file/s3.service';
import { QuizPersistence } from './quiz.persistence';
import { UserService } from '../user/user.service';
import { MustProvideAtLeastOneFileError } from './quiz.errors';

const MAXIMUM_QUIZ_PAGE_SIZE = 100;

export class QuizService {
#persistence: QuizPersistence;
#fileService: S3FileService;
#userService: UserService;

constructor(persistence: QuizPersistence, fileService: S3FileService) {
constructor(persistence: QuizPersistence, fileService: S3FileService, userService: UserService) {
this.#persistence = persistence;
this.#fileService = fileService;
this.#userService = userService;
}

/**
Expand All @@ -47,11 +51,18 @@ export class QuizService {
return { data: data.map((entry) => this.#quizPersistenceWithMyCompletionsToQuiz(entry)), hasMoreRows };
}

/**
* Get a quiz along with all its completions and images.
* @param id Id of the quiz to get details for.
* @returns The quiz and its completions.
*/
async getQuizDetails(id: string) {
const quiz = await this.#persistence.getQuizByIdWithResults({
id,
});
const { images, completions, uploadedByUser, ...quizFieldsThatDoNotRequireTransform } = quiz;
// TODO look into modifying the upstream https://eslint.org/docs/latest/rules/no-unused-vars#ignorerestsiblings linting rule
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { images, completions, uploadedByUser, uploadedByUserId, ...quizFieldsThatDoNotRequireTransform } = quiz;
return {
...quizFieldsThatDoNotRequireTransform,
completions: completions.map((entry) => this.#quizCompletionPersistenceToQuizCompletion(entry)),
Expand All @@ -64,21 +75,43 @@ export class QuizService {
};
}

async createQuiz({
userId,
email,
userName,
type,
date,
files,
}: {
userId: string;
email: string;
userName?: string;
type: QuizType;
date: Date;
files: { fileName: string; type: QuizImageType }[];
}) {
/**
* Create a quiz.
* The images are not provided directly here, but rather signed links
* are returned that can be used to upload.
*
* The quiz will only be marked as ready once the images have been uploaded.
*
* @param userId The id of the user creating the quiz.
* @param quizDetails The type, date and files for the quiz.
* @returns The created quiz and links that should be used for uploading the files.
*/
async createQuiz(
userId: string,
{
type,
date,
files,
}: {
/** The type of quiz being created. */
type: QuizType;
/** The date the quiz was published. */
date: Date;
/** Images for the quiz. */
files: {
/** Name of the file. */
fileName: string;
/** What the image is of. */
type: QuizImageType;
}[];
},
) {
if (files.length === 0) {
throw new MustProvideAtLeastOneFileError();
}

const user = await this.#userService.getUser(userId);

const uuid = uuidv4();
const filesWithKeys = files.map((file) => ({
...file,
Expand Down Expand Up @@ -107,8 +140,8 @@ export class QuizService {
completions: [],
uploadedByUser: {
id: userId,
email: email,
name: userName ?? null,
email: user.email,
name: user.name ?? null,
},
}),
uploadLinks: uploadLinks.map((ul) => ({ fileName: ul.fileName, link: ul.uploadLink })),
Expand Down
10 changes: 5 additions & 5 deletions src/service.locator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import config from './config/config';
const memoryCache = new MemoryCache();

// auth
export const authenticationService = new AuthenticationService();
export const authenticationService = new AuthenticationService(config.AUTH0_DOMAIN, config.AUTH0_AUDIENCE);
export const authorisationService = new AuthorisationService();

// prisma
Expand All @@ -24,14 +24,14 @@ export const prismaService = new PrismaService();
// file
export const fileService = new S3FileService(config.AWS_REGION, config.AWS_BUCKET_NAME, config.FILE_ACCESS_BASE_URL);

// quiz
export const quizPersistence = new QuizPersistence(prismaService);
export const quizService = new QuizService(quizPersistence, fileService);

// user
export const userPersistence = new UserPersistence(prismaService);
export const userService = new UserService(userPersistence);

// quiz
export const quizPersistence = new QuizPersistence(prismaService);
export const quizService = new QuizService(quizPersistence, fileService, userService);

// queue
export const queueService = new SQSQueueService(quizService);

Expand Down
Loading

0 comments on commit a6291c9

Please sign in to comment.