Skip to content

Commit 20867fc

Browse files
hussein-m-kandilhussein-m-kandil
committed
Respond with 403 on non-owner/admin mutation
1 parent d9a2388 commit 20867fc

6 files changed

Lines changed: 67 additions & 104 deletions

File tree

‎src/middlewares/validators.ts‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,48 +4,40 @@ import { PublicUser } from '../types';
44
import passport from '../lib/passport';
55

66
export const createOwnerValidator = (
7-
getOwnerId: (req: Request, res: Response) => unknown
7+
getOwnerId: (req: Request, res: Response) => unknown,
88
): RequestHandler => {
99
return async (req: Request, res: Response, next: NextFunction) => {
1010
const reqUser = req.user as PublicUser | undefined;
1111
const owner = reqUser?.id === (await getOwnerId(req, res));
1212
if (owner) next();
13-
else res.status(401).end();
13+
else res.status(403).end();
1414
};
1515
};
1616

1717
export const createAdminOrOwnerValidator = (
18-
getOwnerId: (req: Request, res: Response) => unknown
18+
getOwnerId: (req: Request, res: Response) => unknown,
1919
): RequestHandler => {
2020
return async (req: Request, res: Response, next: NextFunction) => {
2121
const reqUser = req.user as PublicUser | undefined;
2222
const admin = reqUser?.isAdmin;
2323
const owner = reqUser?.id === (await getOwnerId(req, res));
2424
if (admin || owner) next();
25-
else res.status(401).end();
25+
else res.status(403).end();
2626
};
2727
};
2828

29-
export const adminValidator = (
30-
req: Request,
31-
res: Response,
32-
next: NextFunction
33-
) => {
29+
export const adminValidator = (req: Request, res: Response, next: NextFunction) => {
3430
const reqUser = req.user as PublicUser | undefined;
3531
const admin = reqUser?.isAdmin;
3632
if (admin) next();
37-
else res.status(401).end();
33+
else res.status(403).end();
3834
};
3935

4036
export const authValidator = passport.authenticate('jwt', {
4137
session: false,
4238
}) as RequestHandler;
4339

44-
export const optionalAuthValidator = async (
45-
req: Request,
46-
res: Response,
47-
next: NextFunction
48-
) => {
40+
export const optionalAuthValidator = async (req: Request, res: Response, next: NextFunction) => {
4941
if (req.headers.authorization) {
5042
// The purpose of this middleware is to optionally retrieve user info
5143
// if applicable, thereby preventing a 401 error on an invalid token

‎src/tests/api/setup.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,7 @@ export const setup = async (signinUrl: string, expApp: App = app) => {
283283
assertErrorRes: TestUtils.assertErrorRes,
284284
assertNotFoundErrorRes: TestUtils.assertNotFoundErrorRes,
285285
assertInvalidIdErrorRes: TestUtils.assertInvalidIdErrorRes,
286+
assertForbiddenErrorRes: TestUtils.assertForbiddenErrorRes,
286287
assertUnauthorizedErrorRes: TestUtils.assertUnauthorizedErrorRes,
287288
assertResponseWithValidationError: TestUtils.assertResponseWithValidationError,
288289
};

‎src/tests/api/v1/images.int.test.ts‎

Lines changed: 18 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,4 @@
1-
import {
2-
vi,
3-
it,
4-
expect,
5-
describe,
6-
afterAll,
7-
afterEach,
8-
beforeEach,
9-
} from 'vitest';
1+
import { vi, it, expect, describe, afterAll, afterEach, beforeEach } from 'vitest';
102
import { AppErrorResponse } from '@/types';
113
import { IMAGES_URL, SIGNIN_URL } from './utils';
124
import { Image } from 'prisma/client';
@@ -34,6 +26,7 @@ describe('Image endpoints', async () => {
3426
prepForAuthorizedTest,
3527
assertNotFoundErrorRes,
3628
assertInvalidIdErrorRes,
29+
assertForbiddenErrorRes,
3730
assertUnauthorizedErrorRes,
3831
assertResponseWithValidationError,
3932
} = await setup(SIGNIN_URL);
@@ -42,9 +35,7 @@ describe('Image endpoints', async () => {
4235
storage: { upload, remove },
4336
} = storageData;
4437

45-
const { authorizedApi, signedInUserData } = await prepForAuthorizedTest(
46-
userOneData
47-
);
38+
const { authorizedApi, signedInUserData } = await prepForAuthorizedTest(userOneData);
4839

4940
let url: string;
5041
const prepImageUrl = async () => {
@@ -68,10 +59,10 @@ describe('Image endpoints', async () => {
6859
assertUnauthorizedErrorRes(res);
6960
});
7061

71-
it('should respond with 401 on a request with user token', async () => {
62+
it('should respond with 403 on a request with user token', async () => {
7263
const { authorizedApi } = await prepForAuthorizedTest(userOneData);
7364
const res = await authorizedApi.get(IMAGES_URL);
74-
assertUnauthorizedErrorRes(res);
65+
assertForbiddenErrorRes(res);
7566
});
7667

7768
it('should respond with an empty array on a request with admin token', async () => {
@@ -160,10 +151,7 @@ describe('Image endpoints', async () => {
160151

161152
it('should upload the image with data', async () => {
162153
stream = fs.createReadStream('src/tests/files/good.jpg');
163-
const res = await authorizedApi
164-
.post(IMAGES_URL)
165-
.field(imagedata)
166-
.attach('image', stream);
154+
const res = await authorizedApi.post(IMAGES_URL).field(imagedata).attach('image', stream);
167155
expect(res.statusCode).toBe(201);
168156
assertImageData(res, imgData);
169157
expect(upload).toHaveBeenCalledOnce();
@@ -186,10 +174,7 @@ describe('Image endpoints', async () => {
186174
it('should upload the image with extra info', async () => {
187175
const info = 'Extra info...';
188176
stream = fs.createReadStream('src/tests/files/good.jpg');
189-
const res = await authorizedApi
190-
.post(IMAGES_URL)
191-
.field('info', info)
192-
.attach('image', stream);
177+
const res = await authorizedApi.post(IMAGES_URL).field('info', info).attach('image', stream);
193178
expect(res.statusCode).toBe(201);
194179
assertImageData(res, { ...imgOne, info });
195180
expect(upload).toHaveBeenCalledOnce();
@@ -280,16 +265,14 @@ describe('Image endpoints', async () => {
280265
assertUnauthorizedErrorRes(res);
281266
});
282267

283-
it('should respond with 401 if the current user is not the image owner', async () => {
268+
it('should respond with 403 if the current user is not the image owner', async () => {
284269
const { authorizedApi } = await prepForAuthorizedTest(userTwoData);
285270
const res = await authorizedApi.put(url).send();
286-
assertUnauthorizedErrorRes(res);
271+
assertForbiddenErrorRes(res);
287272
});
288273

289274
it('should respond with 404', async () => {
290-
const res = await authorizedApi
291-
.put(`${IMAGES_URL}/${crypto.randomUUID()}`)
292-
.send();
275+
const res = await authorizedApi.put(`${IMAGES_URL}/${crypto.randomUUID()}`).send();
293276
assertNotFoundErrorRes(res);
294277
});
295278

@@ -316,10 +299,7 @@ describe('Image endpoints', async () => {
316299

317300
it('should update the image with data', async () => {
318301
stream = fs.createReadStream('src/tests/files/good.jpg');
319-
const res = await authorizedApi
320-
.put(url)
321-
.field(imagedata)
322-
.attach('image', stream);
302+
const res = await authorizedApi.put(url).field(imagedata).attach('image', stream);
323303
expect(res.statusCode).toBe(200);
324304
assertImageData(res, imgData);
325305
expect(upload).toHaveBeenCalledOnce();
@@ -365,41 +345,29 @@ describe('Image endpoints', async () => {
365345

366346
it('should not update the image with invalid `xPos` type', async () => {
367347
stream = fs.createReadStream('src/tests/files/good.jpg');
368-
const res = await authorizedApi
369-
.put(url)
370-
.field('xPos', '25px')
371-
.attach('image', stream);
348+
const res = await authorizedApi.put(url).field('xPos', '25px').attach('image', stream);
372349
assertResponseWithValidationError(res, 'xPos');
373350
expect(upload).not.toHaveBeenCalledOnce();
374351
});
375352

376353
it('should not update the image with invalid `yPos` type', async () => {
377354
stream = fs.createReadStream('src/tests/files/good.jpg');
378-
const res = await authorizedApi
379-
.put(url)
380-
.field('yPos', '25px')
381-
.attach('image', stream);
355+
const res = await authorizedApi.put(url).field('yPos', '25px').attach('image', stream);
382356
assertResponseWithValidationError(res, 'yPos');
383357
expect(upload).not.toHaveBeenCalledOnce();
384358
});
385359

386360
it('should not update the image with invalid `scale` type', async () => {
387361
stream = fs.createReadStream('src/tests/files/good.jpg');
388-
const res = await authorizedApi
389-
.put(url)
390-
.field('scale', '125%')
391-
.attach('image', stream);
362+
const res = await authorizedApi.put(url).field('scale', '125%').attach('image', stream);
392363
assertResponseWithValidationError(res, 'scale');
393364
expect(upload).not.toHaveBeenCalledOnce();
394365
});
395366

396367
it('should update the avatar image and connect it to the current user', async () => {
397368
const userId = signedInUserData.user.id;
398369
stream = fs.createReadStream('src/tests/files/good.jpg');
399-
const res = await authorizedApi
400-
.put(url)
401-
.field('isAvatar', true)
402-
.attach('image', stream);
370+
const res = await authorizedApi.put(url).field('isAvatar', true).attach('image', stream);
403371
const dbAvatar = (await db.avatar.findMany({})).at(-1)!;
404372
expect((res.body as Image).id).toBe(dbAvatar.imageId);
405373
expect(res.statusCode).toBe(200);
@@ -418,16 +386,14 @@ describe('Image endpoints', async () => {
418386
assertUnauthorizedErrorRes(res);
419387
});
420388

421-
it('should respond with 401 if the current user is not the image owner', async () => {
389+
it('should respond with 403 if the current user is not the image owner', async () => {
422390
const { authorizedApi } = await prepForAuthorizedTest(userTwoData);
423391
const res = await authorizedApi.delete(url).send();
424-
assertUnauthorizedErrorRes(res);
392+
assertForbiddenErrorRes(res);
425393
});
426394

427395
it('should respond with 404', async () => {
428-
const res = await authorizedApi
429-
.delete(`${IMAGES_URL}/${crypto.randomUUID()}`)
430-
.send();
396+
const res = await authorizedApi.delete(`${IMAGES_URL}/${crypto.randomUUID()}`).send();
431397
assertNotFoundErrorRes(res);
432398
});
433399

‎src/tests/api/v1/posts.int.test.ts‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ describe('Post endpoints', async () => {
3737
prepForAuthorizedTest,
3838
assertNotFoundErrorRes,
3939
assertInvalidIdErrorRes,
40+
assertForbiddenErrorRes,
4041
assertResponseWithValidationError,
4142
} = await setup(SIGNIN_URL);
4243

@@ -352,14 +353,13 @@ describe('Post endpoints', async () => {
352353
assertInvalidIdErrorRes(res);
353354
});
354355

355-
it('should respond with 401 with non-owner credentials', async () => {
356+
it('should respond with 403 with non-owner credentials', async () => {
356357
const dbPost = await createPost({
357358
...postDataToUpdate,
358359
authorId: dbUserTwo.id,
359360
});
360361
const res = await authorizedApi.put(`${POSTS_URL}/${dbPost.id}`).send(postDataInput);
361-
expect(res.statusCode).toBe(401);
362-
expect(res.body).toStrictEqual({});
362+
assertForbiddenErrorRes(res);
363363
});
364364

365365
it('should update the tags', async () => {
@@ -588,7 +588,7 @@ describe('Post endpoints', async () => {
588588
expect(res.body).toStrictEqual({});
589589
});
590590

591-
it(`should respond with 401 on request with non-post${
591+
it(`should respond with 403 on request with non-post${
592592
forComment ? '/comment' : ''
593593
}-owner JWT`, async () => {
594594
const xUserSigninData = await signin(xUserData.username, xUserData.password);
@@ -600,8 +600,7 @@ describe('Post endpoints', async () => {
600600
: `${POSTS_URL}/${dbPost.id}`,
601601
)
602602
.set('Authorization', xUserSigninData.token);
603-
expect(res.statusCode).toBe(401);
604-
expect(res.body).toStrictEqual({});
603+
assertForbiddenErrorRes(res);
605604
});
606605

607606
it(`should admin be able delete a normal user ${
@@ -1227,7 +1226,7 @@ describe('Post endpoints', async () => {
12271226
assertNotFoundErrorRes(res);
12281227
});
12291228

1230-
it('should respond with 401 on a non-comment-owner JWT', async () => {
1229+
it('should respond with 403 on a non-comment-owner JWT', async () => {
12311230
const { signedInUserData, authorizedApi } = await prepForAuthorizedTest(xUserData);
12321231
const dbPost = await createPost(postDataToComment);
12331232
const res = await authorizedApi
@@ -1238,8 +1237,7 @@ describe('Post endpoints', async () => {
12381237
)
12391238
.set('authorization', signedInUserData.token)
12401239
.send(commentData);
1241-
expect(res.statusCode).toBe(401);
1242-
expect(res.body).toStrictEqual({});
1240+
assertForbiddenErrorRes(res);
12431241
});
12441242

12451243
it('should respond with 400 on a comment without content field', async () => {

0 commit comments

Comments
 (0)