From a877c85875017486b312bf5c8dbfc274feb7c432 Mon Sep 17 00:00:00 2001 From: bryandel Date: Fri, 10 Nov 2017 01:11:18 +0800 Subject: [PATCH 1/6] Fix for unhandled undefined config When an invalid application id is passed either for reset/change password or email verification, config.get returns undefined. This causes internal server. --- src/Routers/PublicAPIRouter.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Routers/PublicAPIRouter.js b/src/Routers/PublicAPIRouter.js index 889c13a937..a9fc5a6719 100644 --- a/src/Routers/PublicAPIRouter.js +++ b/src/Routers/PublicAPIRouter.js @@ -15,7 +15,7 @@ export class PublicAPIRouter extends PromiseRouter { const appId = req.params.appId; const config = Config.get(appId); - if (!config.publicServerURL) { + if (!config || !config.publicServerURL) { return this.missingPublicServerURL(); } @@ -40,7 +40,7 @@ export class PublicAPIRouter extends PromiseRouter { const appId = req.params.appId; const config = Config.get(appId); - if (!config.publicServerURL) { + if (!config || !config.publicServerURL) { return this.missingPublicServerURL(); } @@ -66,7 +66,7 @@ export class PublicAPIRouter extends PromiseRouter { changePassword(req) { return new Promise((resolve, reject) => { const config = Config.get(req.query.id); - if (!config.publicServerURL) { + if (!config || !config.publicServerURL) { return resolve({ status: 404, text: 'Not found.' @@ -89,7 +89,7 @@ export class PublicAPIRouter extends PromiseRouter { const config = req.config; - if (!config.publicServerURL) { + if (!config || !config.publicServerURL) { return this.missingPublicServerURL(); } @@ -114,7 +114,7 @@ export class PublicAPIRouter extends PromiseRouter { const config = req.config; - if (!config.publicServerURL) { + if (!config || !config.publicServerURL) { return this.missingPublicServerURL(); } From 94eb920e206ae9bdfc7ca18b5c39f8dd222f7d4a Mon Sep 17 00:00:00 2001 From: bryandel Date: Sat, 11 Nov 2017 13:27:45 +0800 Subject: [PATCH 2/6] Throwing a 403 exception instead of returning a 404 for an invalid app id Also, added a missing semicolon --- src/Routers/PublicAPIRouter.js | 40 +++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/src/Routers/PublicAPIRouter.js b/src/Routers/PublicAPIRouter.js index a9fc5a6719..6005c34895 100644 --- a/src/Routers/PublicAPIRouter.js +++ b/src/Routers/PublicAPIRouter.js @@ -15,7 +15,11 @@ export class PublicAPIRouter extends PromiseRouter { const appId = req.params.appId; const config = Config.get(appId); - if (!config || !config.publicServerURL) { + if(!config) { + this.invalidRequest(); + } + + if (!config.publicServerURL) { return this.missingPublicServerURL(); } @@ -40,7 +44,11 @@ export class PublicAPIRouter extends PromiseRouter { const appId = req.params.appId; const config = Config.get(appId); - if (!config || !config.publicServerURL) { + if(!config) { + this.invalidRequest(); + } + + if (!config.publicServerURL) { return this.missingPublicServerURL(); } @@ -66,7 +74,12 @@ export class PublicAPIRouter extends PromiseRouter { changePassword(req) { return new Promise((resolve, reject) => { const config = Config.get(req.query.id); - if (!config || !config.publicServerURL) { + + if(!config) { + this.invalidRequest(); + } + + if (!config.publicServerURL) { return resolve({ status: 404, text: 'Not found.' @@ -89,7 +102,11 @@ export class PublicAPIRouter extends PromiseRouter { const config = req.config; - if (!config || !config.publicServerURL) { + if(!config) { + this.invalidRequest(); + } + + if (!config.publicServerURL) { return this.missingPublicServerURL(); } @@ -114,7 +131,11 @@ export class PublicAPIRouter extends PromiseRouter { const config = req.config; - if (!config || !config.publicServerURL) { + if(!config) { + this.invalidRequest(); + } + + if (!config.publicServerURL) { return this.missingPublicServerURL(); } @@ -135,7 +156,7 @@ export class PublicAPIRouter extends PromiseRouter { location: `${config.passwordResetSuccessURL}?${params}` }); }, (err) => { - const params = qs.stringify({username: username, token: token, id: config.applicationId, error:err, app:config.appName}) + const params = qs.stringify({username: username, token: token, id: config.applicationId, error:err, app:config.appName}); return Promise.resolve({ status: 302, location: `${config.choosePasswordURL}?${params}` @@ -171,6 +192,13 @@ export class PublicAPIRouter extends PromiseRouter { }); } + invalidRequest() { + const error = new Error(); + error.status = 403; + error.message = "unauthorized"; + throw error; + } + setConfig(req) { req.config = Config.get(req.params.appId); return Promise.resolve(); From 6db01b7121b7d491d6bdc8d46bae647fead18181 Mon Sep 17 00:00:00 2001 From: Bryan de Leon Date: Sat, 11 Nov 2017 13:36:32 +0800 Subject: [PATCH 3/6] Fix indent issues --- src/Routers/PublicAPIRouter.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Routers/PublicAPIRouter.js b/src/Routers/PublicAPIRouter.js index 6005c34895..2c820a48d1 100644 --- a/src/Routers/PublicAPIRouter.js +++ b/src/Routers/PublicAPIRouter.js @@ -14,9 +14,9 @@ export class PublicAPIRouter extends PromiseRouter { const { token, username } = req.query; const appId = req.params.appId; const config = Config.get(appId); - - if(!config) { - this.invalidRequest(); + + if(!config){ + this.invalidRequest(): } if (!config.publicServerURL) { @@ -44,8 +44,8 @@ export class PublicAPIRouter extends PromiseRouter { const appId = req.params.appId; const config = Config.get(appId); - if(!config) { - this.invalidRequest(); + if(!config){ + this.invalidRequest(): } if (!config.publicServerURL) { @@ -74,9 +74,9 @@ export class PublicAPIRouter extends PromiseRouter { changePassword(req) { return new Promise((resolve, reject) => { const config = Config.get(req.query.id); - - if(!config) { - this.invalidRequest(); + + if(!config){ + this.invalidRequest(); } if (!config.publicServerURL) { @@ -102,8 +102,8 @@ export class PublicAPIRouter extends PromiseRouter { const config = req.config; - if(!config) { - this.invalidRequest(); + if(!config){ + this.invalidRequest(): } if (!config.publicServerURL) { @@ -131,8 +131,8 @@ export class PublicAPIRouter extends PromiseRouter { const config = req.config; - if(!config) { - this.invalidRequest(); + if(!config){ + this.invalidRequest(): } if (!config.publicServerURL) { From e62b88a0e5f5fd8fa134ce9c614e7b557bd52b68 Mon Sep 17 00:00:00 2001 From: Bryan de Leon Date: Sat, 11 Nov 2017 13:49:15 +0800 Subject: [PATCH 4/6] Fix invalid colon to semicolon --- src/Routers/PublicAPIRouter.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Routers/PublicAPIRouter.js b/src/Routers/PublicAPIRouter.js index 2c820a48d1..7ea1c3f342 100644 --- a/src/Routers/PublicAPIRouter.js +++ b/src/Routers/PublicAPIRouter.js @@ -16,7 +16,7 @@ export class PublicAPIRouter extends PromiseRouter { const config = Config.get(appId); if(!config){ - this.invalidRequest(): + this.invalidRequest(); } if (!config.publicServerURL) { @@ -45,7 +45,7 @@ export class PublicAPIRouter extends PromiseRouter { const config = Config.get(appId); if(!config){ - this.invalidRequest(): + this.invalidRequest(); } if (!config.publicServerURL) { @@ -103,7 +103,7 @@ export class PublicAPIRouter extends PromiseRouter { const config = req.config; if(!config){ - this.invalidRequest(): + this.invalidRequest(); } if (!config.publicServerURL) { @@ -132,7 +132,7 @@ export class PublicAPIRouter extends PromiseRouter { const config = req.config; if(!config){ - this.invalidRequest(): + this.invalidRequest(); } if (!config.publicServerURL) { From d245d0f13d5886540ad03dc6171dae4b3df70aec Mon Sep 17 00:00:00 2001 From: Bryan de Leon Date: Sat, 11 Nov 2017 14:01:54 +0800 Subject: [PATCH 5/6] Fix space and indent issues --- src/Routers/PublicAPIRouter.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Routers/PublicAPIRouter.js b/src/Routers/PublicAPIRouter.js index 7ea1c3f342..a126423cb0 100644 --- a/src/Routers/PublicAPIRouter.js +++ b/src/Routers/PublicAPIRouter.js @@ -14,7 +14,7 @@ export class PublicAPIRouter extends PromiseRouter { const { token, username } = req.query; const appId = req.params.appId; const config = Config.get(appId); - + if(!config){ this.invalidRequest(); } @@ -74,7 +74,7 @@ export class PublicAPIRouter extends PromiseRouter { changePassword(req) { return new Promise((resolve, reject) => { const config = Config.get(req.query.id); - + if(!config){ this.invalidRequest(); } @@ -193,10 +193,10 @@ export class PublicAPIRouter extends PromiseRouter { } invalidRequest() { - const error = new Error(); - error.status = 403; - error.message = "unauthorized"; - throw error; + const error = new Error(); + error.status = 403; + error.message = "unauthorized"; + throw error; } setConfig(req) { From 9ce201cbb5cca83d818ac93c01b247d8dfd069df Mon Sep 17 00:00:00 2001 From: bryandel Date: Sat, 11 Nov 2017 19:28:21 +0800 Subject: [PATCH 6/6] Tests for the fix for unhandled undefined config --- spec/PublicAPI.spec.js | 44 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/spec/PublicAPI.spec.js b/spec/PublicAPI.spec.js index f34ea0af55..c28914e348 100644 --- a/spec/PublicAPI.spec.js +++ b/spec/PublicAPI.spec.js @@ -63,3 +63,47 @@ describe("public API without publicServerURL", () => { }); }); }); + + +describe("public API supplied with invalid application id", () => { + beforeEach(done => { + reconfigureServer({appName: "unused"}) + .then(done, fail); + }); + + it("should get 403 on verify_email", (done) => { + request('http://localhost:8378/1/apps/invalid/verify_email', (err, httpResponse) => { + expect(httpResponse.statusCode).toBe(403); + done(); + }); + }); + + it("should get 403 choose_password", (done) => { + request('http://localhost:8378/1/apps/choose_password?id=invalid', (err, httpResponse) => { + expect(httpResponse.statusCode).toBe(403); + done(); + }); + }); + + it("should get 403 on get of request_password_reset", (done) => { + request('http://localhost:8378/1/apps/invalid/request_password_reset', (err, httpResponse) => { + expect(httpResponse.statusCode).toBe(403); + done(); + }); + }); + + + it("should get 403 on post of request_password_reset", (done) => { + request.post('http://localhost:8378/1/apps/invalid/request_password_reset', (err, httpResponse) => { + expect(httpResponse.statusCode).toBe(403); + done(); + }); + }); + + it("should get 403 on resendVerificationEmail", (done) => { + request('http://localhost:8378/1/apps/invalid/resend_verification_email', (err, httpResponse) => { + expect(httpResponse.statusCode).toBe(403); + done(); + }); + }); +});