From 5aeb54e9509b515fc603ba052598a2c34bb46501 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Wed, 7 Feb 2018 12:19:25 +0000 Subject: [PATCH 1/2] oauth: check status code before parsing body Fixes #285 --- backend/plugins/authorization.js | 36 +++++++++++++++++++------------- backend/routes.js | 28 +++++++++++++++---------- 2 files changed, 38 insertions(+), 26 deletions(-) diff --git a/backend/plugins/authorization.js b/backend/plugins/authorization.js index 9e40c4f7d..8e8b3fa0a 100644 --- a/backend/plugins/authorization.js +++ b/backend/plugins/authorization.js @@ -57,26 +57,32 @@ export function PlotlyOAuth(electron) { fetch(`${getSetting('PLOTLY_API_URL')}/v2/users/current`, { headers: {'Authorization': `Bearer ${plotlyAuthToken}`} }) - .then(userRes => userRes.json().then(userMeta => { + .then(userRes => { if (userRes.status !== 200) { - res.json(401, {error: {message: 'Please login to access this page.'}}); - return next(false); + return userRes.text().then(body => { + const errorMessage = `Error fetching user. Status: ${userRes.status}. Body: ${body}.`; + Logger.log(errorMessage, 0); + res.json(500, {error: {message: errorMessage}}); + return next(); + }); } - if (!contains(userMeta.username, getSetting('ALLOWED_USERS'))) { - // Remove any existing credentials and return error - res.clearCookie('db-connector-auth-token'); - res.clearCookie('plotly-auth-token'); - res.clearCookie('db-connector-user'); - res.json(403, {error: {message: `User ${userMeta.username} is not allowed to view this app`}}); - return next(false); - } + return userRes.json().then(userMeta => { + if (!contains(userMeta.username, getSetting('ALLOWED_USERS'))) { + // Remove any existing credentials and return error + res.clearCookie('db-connector-auth-token'); + res.clearCookie('plotly-auth-token'); + res.clearCookie('db-connector-user'); + res.json(403, {error: {message: `User ${userMeta.username} is not allowed to view this app`}}); + return next(false); + } - const dbConnectorAccessToken = generateAndSaveAccessToken(); - res.setCookie('db-connector-auth-token', dbConnectorAccessToken, getAccessTokenCookieOptions()); + const dbConnectorAccessToken = generateAndSaveAccessToken(); + res.setCookie('db-connector-auth-token', dbConnectorAccessToken, getAccessTokenCookieOptions()); - return next(); - })) + return next(); + }); + }) .catch(err => { Logger.log(err, 0); res.json(500, {error: {message: err.message}}); diff --git a/backend/routes.js b/backend/routes.js index 6cedab659..cd0b2d408 100644 --- a/backend/routes.js +++ b/backend/routes.js @@ -263,8 +263,17 @@ export default class Servers { fetch(`${getSetting('PLOTLY_API_URL')}/v2/users/current`, { headers: {'Authorization': `Bearer ${access_token}`} }) - .then(userRes => userRes.json().then(userMeta => { - if (userRes.status === 200) { + .then(userRes => { + if (userRes.status !== 200) { + return userRes.text().then(body => { + const errorMessage = `Error fetching user. Status: ${userRes.status}. Body: ${body}.`; + Logger.log(errorMessage, 0); + res.json(500, {error: {message: errorMessage}}); + return next(); + }); + } + + return userRes.json().then(userMeta => { const {username} = userMeta; if (!username) { res.json(500, {error: {message: `User was not found at ${getSetting('PLOTLY_API_URL')}`}}); @@ -327,15 +336,12 @@ export default class Servers { } res.json(403, {error: {message: `User ${username} is not allowed to view this app`}}); return next(); - } - Logger.log(userMeta, 0); - res.json(500, {error: {message: `Error ${userRes.status} fetching user`}}); - return next(); - })) - .catch(err => { - Logger.log(err, 0); - res.json(500, {error: {message: err.message}}); - return next(); + }) + .catch(err => { + Logger.log(err, 0); + res.json(500, {error: {message: err.message}}); + return next(); + }); }); }); From bc901daf225297be8bd4198346b4124a0be87e41 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Wed, 7 Feb 2018 20:57:03 +0000 Subject: [PATCH 2/2] oauth2: check username is set * Requesting https://api.plot.ly/v2/users/current with an invalid access token yields a response with status code 200 and username set to an empty string. --- backend/plugins/authorization.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/plugins/authorization.js b/backend/plugins/authorization.js index 8e8b3fa0a..1d6fcc82c 100644 --- a/backend/plugins/authorization.js +++ b/backend/plugins/authorization.js @@ -68,7 +68,7 @@ export function PlotlyOAuth(electron) { } return userRes.json().then(userMeta => { - if (!contains(userMeta.username, getSetting('ALLOWED_USERS'))) { + if (!userMeta.username || !contains(userMeta.username, getSetting('ALLOWED_USERS'))) { // Remove any existing credentials and return error res.clearCookie('db-connector-auth-token'); res.clearCookie('plotly-auth-token');