From 4382ef1a41dfa90734719eb2cc163c355ec0d733 Mon Sep 17 00:00:00 2001
From: jeremyben <bensimon.jeremy@gmail.com>
Date: Wed, 21 Jul 2021 18:53:32 +0200
Subject: [PATCH] fix: put and patch on user collection

---
 src/__tests__/guards.ts | 31 +++++++++++++++++++++++++++----
 src/guards.ts           | 21 ++++++++++++++++++++-
 src/users.ts            |  4 +++-
 3 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/src/__tests__/guards.ts b/src/__tests__/guards.ts
index f0c6e3c..b801160 100644
--- a/src/__tests__/guards.ts
+++ b/src/__tests__/guards.ts
@@ -3,7 +3,7 @@ import { inMemoryJsonServer, USER } from './shared/tools'
 
 let rq: supertest.SuperTest<supertest.Test>
 let bearer: { Authorization: string }
-let db: { users: any[]; messages: any[] }
+let db: { users: any[]; messages: any[]; secrets: any[] }
 
 beforeEach(async () => {
 	db = {
@@ -12,17 +12,21 @@ beforeEach(async () => {
 			{ id: 1, text: 'other', userId: 1 },
 			{ id: 2, text: 'mine', userId: 2 },
 		],
+		secrets: [],
 	}
+
 	const guards = {
 		users: 600,
 		messages: 640,
+		secrets: 600,
 	}
+
 	const app = inMemoryJsonServer(db, guards)
 	rq = supertest(app)
 
 	// Create user (will have id:2) and keep access token
-	const registerRes = await rq.post('/register').send(USER)
-	bearer = { Authorization: `Bearer ${registerRes.body.accessToken}` }
+	const res = await rq.post('/register').send(USER)
+	bearer = { Authorization: `Bearer ${res.body.accessToken}` }
 })
 
 describe('600: owner can read/write', () => {
@@ -40,6 +44,12 @@ describe('600: owner can read/write', () => {
 		await rq.get('/users/2').expect(401)
 		await rq.get('/users/2').set(bearer).expect(200)
 	})
+
+	test('[HAPPY] can write and read from private collection', async () => {
+		await rq.post('/secrets').set(bearer).send({ size: 'big', userId: '2' }).expect(201)
+		const res = await rq.get('/secrets/1').set(bearer)
+		expect(res.body).toEqual({ id: 1, userId: '2', size: 'big' })
+	})
 })
 
 describe('640: owner can read/write, logged can read', () => {
@@ -73,7 +83,20 @@ describe('640: owner can read/write, logged can read', () => {
 })
 
 test('[HAPPY] create another user after setting guards', async () => {
-	await rq.post('/users').send({ email: 'arthur@email.com', password: '1234' }).expect(201)
+	const res = await rq
+		.post('/users')
+		.send({ email: 'arthur@email.com', password: '1234' })
+		.expect(201)
+
+	const otherBearer = { Authorization: `Bearer ${res.body.accessToken}` }
+
+	await rq.get('/users/3').set(otherBearer).expect(200)
+
+	await rq
+		.put('/users/3')
+		.set(otherBearer)
+		.send({ email: 'arthur@email.com', password: '1234' })
+		.expect(200)
 })
 
 test('[HAPPY] other methods pass through', async () => {
diff --git a/src/guards.ts b/src/guards.ts
index 48f00a5..9bb5950 100644
--- a/src/guards.ts
+++ b/src/guards.ts
@@ -71,9 +71,10 @@ const privateOnly: Handler = (req, res, next) => {
 
 		// Creation and replacement
 		// check userId on the request body
-		if (req.method === 'POST' || req.method === 'PUT') {
+		if (req.method === 'POST') {
 			// TODO: use foreignKeySuffix instead of assuming the default "Id"
 			const hasRightUserId = String(req.body.userId) === req.claims!.sub
+
 			// No userId reference when creating a new user (duh)
 			const isUserResource = resource === 'users'
 
@@ -84,6 +85,24 @@ const privateOnly: Handler = (req, res, next) => {
 					'Private resource creation: request body must have a reference to the owner id'
 				)
 			}
+
+			return
+		}
+
+		if (req.method === 'PUT') {
+			// TODO: use foreignKeySuffix instead of assuming the default "Id"
+			const hasRightUserId = String(req.body.userId) === req.claims!.sub
+
+			const isUserResourceAndIsRightId = resource === 'users' && id === req.claims!.sub
+
+			if (hasRightUserId || isUserResourceAndIsRightId) {
+				next()
+			} else {
+				res.status(403).jsonp(
+					'Private resource replacement: request body must have a reference to the owner id'
+				)
+			}
+
 			return
 		}
 
diff --git a/src/users.ts b/src/users.ts
index d888058..a6d8eb5 100644
--- a/src/users.ts
+++ b/src/users.ts
@@ -167,12 +167,14 @@ const update: Handler = (req, res, next) => {
 /**
  * Users router
  */
+// Must match routes with and without guards
 export default Router()
 	.use(bodyParsingHandler)
 	.post('/users|register|signup', validate({ required: true }), create)
-	// Bypass eventual users guards to still allow creation
 	.post('/[640]{3}/users', validate({ required: true }), create)
 	.post('/login|signin', validate({ required: true }), login)
 	.put('/users/:id', validate({ required: true }), update)
+	.put('/[640]{3}/users/:id', validate({ required: true }), update)
 	.patch('/users/:id', validate({ required: false }), update)
+	.patch('/[640]{3}/users/:id', validate({ required: false }), update)
 	.use(errorHandler)