From 26a2e23f2d27e4240163fd2af647d443d63e12c2 Mon Sep 17 00:00:00 2001 From: Suat Karabacak Date: Sun, 10 Dec 2023 18:55:09 +0300 Subject: [PATCH 1/6] deny requests for ip restricted master keys --- src/middlewares.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/middlewares.js b/src/middlewares.js index 9319130188..b02f5e4e60 100644 --- a/src/middlewares.js +++ b/src/middlewares.js @@ -246,6 +246,11 @@ export function handleParseHeaders(req, res, next) { `Request using master key rejected as the request IP address '${clientIp}' is not set in Parse Server option 'masterKeyIps'.` ); isMaster = false; + // Create a custom error with a 403 status and a specific message #BreakingChange + const error = new Error(); + error.status = 403; // Forbidden status code + error.message = `Access denied: IP address '${clientIp}' is not authorized to use the master key.`; + throw error; } if (isMaster) { From 19d305adefaed2d276d48be5c22db99d7a2a5438 Mon Sep 17 00:00:00 2001 From: EhsanParsania <75175231+EhsanParsania@users.noreply.github.com> Date: Sun, 25 Feb 2024 11:46:16 +0400 Subject: [PATCH 2/6] fix test cases according to new error while using master key with not allowed ip --- spec/Middlewares.spec.js | 40 +++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/spec/Middlewares.spec.js b/spec/Middlewares.spec.js index 7ec50bd434..2e5dc002e4 100644 --- a/spec/Middlewares.spec.js +++ b/spec/Middlewares.spec.js @@ -33,6 +33,9 @@ describe('middlewares', () => { }); it('should use _ContentType if provided', done => { + AppCachePut(fakeReq.body._ApplicationId, { + masterKeyIps: ['127.0.0.1'], + }); expect(fakeReq.headers['content-type']).toEqual(undefined); const contentType = 'image/jpeg'; fakeReq.body._ContentType = contentType; @@ -144,22 +147,6 @@ describe('middlewares', () => { }); }); - it('should not succeed and log if the ip does not belong to masterKeyIps list', async () => { - const logger = require('../lib/logger').logger; - spyOn(logger, 'error').and.callFake(() => {}); - AppCachePut(fakeReq.body._ApplicationId, { - masterKey: 'masterKey', - masterKeyIps: ['10.0.0.1'], - }); - fakeReq.ip = '127.0.0.1'; - fakeReq.headers['x-parse-master-key'] = 'masterKey'; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaster).toBe(false); - expect(logger.error).toHaveBeenCalledWith( - `Request using master key rejected as the request IP address '127.0.0.1' is not set in Parse Server option 'masterKeyIps'.` - ); - }); - it('should not succeed if the ip does not belong to masterKeyIps list', async () => { AppCachePut(fakeReq.body._ApplicationId, { masterKey: 'masterKey', @@ -167,8 +154,13 @@ describe('middlewares', () => { }); fakeReq.ip = '127.0.0.1'; fakeReq.headers['x-parse-master-key'] = 'masterKey'; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaster).toBe(false); + try { + await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); + } catch (error) { + expect(error.message).toEqual( + `Access denied: IP address '127.0.0.1' is not authorized to use the master key.` + ); + } }); it('should not succeed if the ip does not belong to maintenanceKeyIps list', async () => { @@ -180,11 +172,13 @@ describe('middlewares', () => { }); fakeReq.ip = '10.0.0.2'; fakeReq.headers['x-parse-maintenance-key'] = 'masterKey'; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaintenance).toBe(false); - expect(logger.error).toHaveBeenCalledWith( - `Request using maintenance key rejected as the request IP address '10.0.0.2' is not set in Parse Server option 'maintenanceKeyIps'.` - ); + try { + await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); + } catch (error) { + expect(error.message).toEqual( + `Access denied: IP address '10.0.0.2' is not authorized to use the master key.` + ); + } }); it('should succeed if the ip does belong to masterKeyIps list', async () => { From 23f8298a3456fe72308511f135da7bde6fd2582d Mon Sep 17 00:00:00 2001 From: EhsanParsania <75175231+EhsanParsania@users.noreply.github.com> Date: Tue, 27 Feb 2024 23:07:19 +0400 Subject: [PATCH 3/6] prevent returning more information than necessary in error message Co-authored-by: Manuel <5673677+mtrezza@users.noreply.github.com> Signed-off-by: EhsanParsania <75175231+EhsanParsania@users.noreply.github.com> --- src/middlewares.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/middlewares.js b/src/middlewares.js index ced2d51275..880315253f 100644 --- a/src/middlewares.js +++ b/src/middlewares.js @@ -246,10 +246,9 @@ export function handleParseHeaders(req, res, next) { `Request using master key rejected as the request IP address '${clientIp}' is not set in Parse Server option 'masterKeyIps'.` ); isMaster = false; - // Create a custom error with a 403 status and a specific message #BreakingChange const error = new Error(); - error.status = 403; // Forbidden status code - error.message = `Access denied: IP address '${clientIp}' is not authorized to use the master key.`; + error.status = 403; + error.message = `unauthorized`; throw error; } From c8fe31beaf957c78464a656a07a28ce93fb07c5e Mon Sep 17 00:00:00 2001 From: EhsanParsania <75175231+EhsanParsania@users.noreply.github.com> Date: Tue, 27 Feb 2024 23:12:38 +0400 Subject: [PATCH 4/6] restructure middlewares test cases --- spec/Middlewares.spec.js | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/spec/Middlewares.spec.js b/spec/Middlewares.spec.js index 2e5dc002e4..5a7a2cc51f 100644 --- a/spec/Middlewares.spec.js +++ b/spec/Middlewares.spec.js @@ -154,13 +154,19 @@ describe('middlewares', () => { }); fakeReq.ip = '127.0.0.1'; fakeReq.headers['x-parse-master-key'] = 'masterKey'; + + let error; + try { await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - } catch (error) { - expect(error.message).toEqual( - `Access denied: IP address '127.0.0.1' is not authorized to use the master key.` - ); + } catch (err) { + error = err; } + + expect(error).toBeDefined(); + expect(error.message).toEqual( + `Access denied: IP address '127.0.0.1' is not authorized to use the master key.` + ); }); it('should not succeed if the ip does not belong to maintenanceKeyIps list', async () => { @@ -172,13 +178,19 @@ describe('middlewares', () => { }); fakeReq.ip = '10.0.0.2'; fakeReq.headers['x-parse-maintenance-key'] = 'masterKey'; + + let error; + try { await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - } catch (error) { - expect(error.message).toEqual( - `Access denied: IP address '10.0.0.2' is not authorized to use the master key.` - ); + } catch (err) { + error = err; } + + expect(error).toBeDefined(); + expect(error.message).toEqual( + `Access denied: IP address '10.0.0.2' is not authorized to use the master key.` + ); }); it('should succeed if the ip does belong to masterKeyIps list', async () => { From fad4e74399e94ea8e1ac30fa9925ce68b2603b35 Mon Sep 17 00:00:00 2001 From: EhsanParsania <75175231+EhsanParsania@users.noreply.github.com> Date: Wed, 28 Feb 2024 12:46:35 +0400 Subject: [PATCH 5/6] fix expectation message --- spec/Middlewares.spec.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/spec/Middlewares.spec.js b/spec/Middlewares.spec.js index 5a7a2cc51f..5be3ecff4d 100644 --- a/spec/Middlewares.spec.js +++ b/spec/Middlewares.spec.js @@ -164,9 +164,7 @@ describe('middlewares', () => { } expect(error).toBeDefined(); - expect(error.message).toEqual( - `Access denied: IP address '127.0.0.1' is not authorized to use the master key.` - ); + expect(error.message).toEqual(`unauthorized`); }); it('should not succeed if the ip does not belong to maintenanceKeyIps list', async () => { @@ -188,9 +186,7 @@ describe('middlewares', () => { } expect(error).toBeDefined(); - expect(error.message).toEqual( - `Access denied: IP address '10.0.0.2' is not authorized to use the master key.` - ); + expect(error.message).toEqual(`unauthorized`); }); it('should succeed if the ip does belong to masterKeyIps list', async () => { From 41bcc3deb9ac7aaef61948808bfaa4a55dfe93ab Mon Sep 17 00:00:00 2001 From: EhsanParsania <75175231+EhsanParsania@users.noreply.github.com> Date: Fri, 1 Mar 2024 01:48:55 +0400 Subject: [PATCH 6/6] check log in two middlewares tests --- spec/Middlewares.spec.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/spec/Middlewares.spec.js b/spec/Middlewares.spec.js index 5be3ecff4d..af547ec77a 100644 --- a/spec/Middlewares.spec.js +++ b/spec/Middlewares.spec.js @@ -147,7 +147,9 @@ describe('middlewares', () => { }); }); - it('should not succeed if the ip does not belong to masterKeyIps list', async () => { + it('should not succeed and log if the ip does not belong to masterKeyIps list', async () => { + const logger = require('../lib/logger').logger; + spyOn(logger, 'error').and.callFake(() => {}); AppCachePut(fakeReq.body._ApplicationId, { masterKey: 'masterKey', masterKeyIps: ['10.0.0.1'], @@ -165,9 +167,12 @@ describe('middlewares', () => { expect(error).toBeDefined(); expect(error.message).toEqual(`unauthorized`); + expect(logger.error).toHaveBeenCalledWith( + `Request using master key rejected as the request IP address '127.0.0.1' is not set in Parse Server option 'masterKeyIps'.` + ); }); - it('should not succeed if the ip does not belong to maintenanceKeyIps list', async () => { + it('should not succeed and log if the ip does not belong to maintenanceKeyIps list', async () => { const logger = require('../lib/logger').logger; spyOn(logger, 'error').and.callFake(() => {}); AppCachePut(fakeReq.body._ApplicationId, { @@ -187,6 +192,9 @@ describe('middlewares', () => { expect(error).toBeDefined(); expect(error.message).toEqual(`unauthorized`); + expect(logger.error).toHaveBeenCalledWith( + `Request using maintenance key rejected as the request IP address '10.0.0.2' is not set in Parse Server option 'maintenanceKeyIps'.` + ); }); it('should succeed if the ip does belong to masterKeyIps list', async () => {