From 8f1d9f051e306314d4c77c067b1879cd86c9dc12 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 18 Jan 2018 13:58:45 -0700 Subject: [PATCH 1/4] added encoding for the user data as json with proper encoding/escaping --- app.js | 1 + models/user.js | 4 ---- public/javascripts/auth-callback.js | 4 ++++ services/passport.js | 26 +++++++++++++++++++++++--- services/response.js | 27 +++++++++++++++++++++++++++ views/auth-callback.ejs | 9 ++------- 6 files changed, 57 insertions(+), 14 deletions(-) create mode 100644 public/javascripts/auth-callback.js create mode 100644 services/response.js diff --git a/app.js b/app.js index 225b5b480..43d4fb014 100644 --- a/app.js +++ b/app.js @@ -61,6 +61,7 @@ if (ENABLE_TRACING && APOLLO_ENGINE_KEY) { // Trust the first proxy in front of us, this will enable us to trust the fact // that SSL was terminated correctly. app.set('trust proxy', 1); +app.set('json escape', true); // Enable a suite of security good practices through helmet. We disable // frameguard to allow crossdomain injection of the embed. diff --git a/models/user.js b/models/user.js index 2aae9c4d3..717e43a88 100644 --- a/models/user.js +++ b/models/user.js @@ -209,10 +209,6 @@ const UserSchema = new Schema( delete ret.__v; delete ret._id; delete ret.password; - delete ret.status.username.history; - delete ret.status.banned.history; - delete ret.status.suspension.history; - delete ret.tokens; }, }, } diff --git a/public/javascripts/auth-callback.js b/public/javascripts/auth-callback.js new file mode 100644 index 000000000..f91f4d743 --- /dev/null +++ b/public/javascripts/auth-callback.js @@ -0,0 +1,4 @@ +document.addEventListener('DOMContentLoaded', function(event) { + localStorage.setItem('auth', document.getElementById('auth').innerText); + setTimeout(function() { window.close(); }, 50); +}); \ No newline at end of file diff --git a/services/passport.js b/services/passport.js index 369dc5e5c..59215dd54 100644 --- a/services/passport.js +++ b/services/passport.js @@ -12,6 +12,8 @@ const debug = require('debug')('talk:services:passport'); const bowser = require('bowser'); const ms = require('ms'); const _ = require('lodash'); +const { attachStaticLocals } = require('../middleware/staticTemplate'); +const { encodeJSONForHTML } = require('./response'); // Create a redis client to use for authentication. const { createClientFactory } = require('./redis'); @@ -81,6 +83,11 @@ const HandleGenerateCredentials = (req, res, next) => (err, user) => { SetTokenForSafari(req, res, token); + // Set the cache control headers. + res.header('Cache-Control', 'private, no-cache, no-store, must-revalidate'); + res.header('Expires', '-1'); + res.header('Pragma', 'no-cache'); + // Send back the details! res.json({ user, token }); }; @@ -89,15 +96,28 @@ const HandleGenerateCredentials = (req, res, next) => (err, user) => { * Returns the response to the login attempt via a popup callback with some JS. */ const HandleAuthPopupCallback = (req, res, next) => (err, user) => { + res.header('Cache-Control', 'private, no-cache, no-store, must-revalidate'); + res.header('Expires', '-1'); + res.header('Pragma', 'no-cache'); + + // Ensure the only scripts that can run here are those on the Talk domain. + res.header('Content-Security-Policy', "default-src 'self';"); + + // Attach static locals to the response locals object. + attachStaticLocals(res.locals); + + // Attach the encoder on the response locals object. + res.locals.encodeJSONForHTML = encodeJSONForHTML; + if (err) { return res.render('auth-callback', { - auth: JSON.stringify({ err, data: null }), + auth: { err, data: null }, }); } if (!user) { return res.render('auth-callback', { - auth: JSON.stringify({ err: errors.ErrNotAuthorized, data: null }), + auth: { err: errors.ErrNotAuthorized, data: null }, }); } @@ -108,7 +128,7 @@ const HandleAuthPopupCallback = (req, res, next) => (err, user) => { // We logged in the user! Let's send back the user data. res.render('auth-callback', { - auth: JSON.stringify({ err: null, data: { user, token } }), + auth: { err: null, data: { user, token } }, }); }; diff --git a/services/response.js b/services/response.js new file mode 100644 index 000000000..6e17cdb74 --- /dev/null +++ b/services/response.js @@ -0,0 +1,27 @@ +/** + * escapeHTMLEntities will escape HTML entities with their unicode counterparts. + * + * @param {String} string the string containing potentially unsafe characters + */ +const escapeHTMLEntities = string => + string.replace(/[<>&]/g, function(c) { + switch (c.charCodeAt(0)) { + case 0x3c: + return '\\u003c'; + case 0x3e: + return '\\u003e'; + case 0x26: + return '\\u0026'; + default: + return c; + } + }); + +/** + * encodeJSONForHTML will encode an object to be loaded on an HTML page. + * + * @param {Object} obj javascript object to encode + */ +const encodeJSONForHTML = obj => escapeHTMLEntities(JSON.stringify(obj)); + +module.exports = { escapeHTMLEntities, encodeJSONForHTML }; diff --git a/views/auth-callback.ejs b/views/auth-callback.ejs index 22103226d..307e84928 100644 --- a/views/auth-callback.ejs +++ b/views/auth-callback.ejs @@ -1,12 +1,7 @@ - + + From 844ce5dbfe1f71ce2d706cb65fd671a2e9b27341 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 18 Jan 2018 14:15:33 -0700 Subject: [PATCH 2/4] fixed json parse error --- app.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app.js b/app.js index 43d4fb014..225b5b480 100644 --- a/app.js +++ b/app.js @@ -61,7 +61,6 @@ if (ENABLE_TRACING && APOLLO_ENGINE_KEY) { // Trust the first proxy in front of us, this will enable us to trust the fact // that SSL was terminated correctly. app.set('trust proxy', 1); -app.set('json escape', true); // Enable a suite of security good practices through helmet. We disable // frameguard to allow crossdomain injection of the embed. From 49d20db26f453cc2022dbf97839de8cfba7cba43 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 18 Jan 2018 14:22:19 -0700 Subject: [PATCH 3/4] replaced homegrown with package --- package.json | 1 + services/response.js | 19 +------------------ yarn.lock | 2 +- 3 files changed, 3 insertions(+), 19 deletions(-) diff --git a/package.json b/package.json index 9234e4ef7..e8a7c9d01 100644 --- a/package.json +++ b/package.json @@ -92,6 +92,7 @@ "dotenv": "^4.0.0", "ejs": "^2.5.7", "env-rewrite": "^1.0.2", + "escape-html": "^1.0.3", "eventemitter2": "^4.1.2", "exports-loader": "^0.6.4", "express": "4.16.0", diff --git a/services/response.js b/services/response.js index 6e17cdb74..bfd3a7c97 100644 --- a/services/response.js +++ b/services/response.js @@ -1,21 +1,4 @@ -/** - * escapeHTMLEntities will escape HTML entities with their unicode counterparts. - * - * @param {String} string the string containing potentially unsafe characters - */ -const escapeHTMLEntities = string => - string.replace(/[<>&]/g, function(c) { - switch (c.charCodeAt(0)) { - case 0x3c: - return '\\u003c'; - case 0x3e: - return '\\u003e'; - case 0x26: - return '\\u0026'; - default: - return c; - } - }); +const escapeHTMLEntities = require('escape-html'); /** * encodeJSONForHTML will encode an object to be loaded on an HTML page. diff --git a/yarn.lock b/yarn.lock index 53de65471..310dfe8cc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2862,7 +2862,7 @@ es6-promise@^4.0.5: version "4.1.1" resolved "https://registry.yarnpkg.com/es6-promise/-/es6-promise-4.1.1.tgz#8811e90915d9a0dba36274f0b242dbda78f9c92a" -escape-html@~1.0.3: +escape-html@^1.0.3, escape-html@~1.0.3: version "1.0.3" resolved "https://registry.yarnpkg.com/escape-html/-/escape-html-1.0.3.tgz#0258eae4d3d0c0974de1c169188ef0051d1d1988" From 4bbd561a1b625758177107c19e289d4a58bf589a Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 18 Jan 2018 16:54:27 -0700 Subject: [PATCH 4/4] fix to cli --- bin/cli-users | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bin/cli-users b/bin/cli-users index 64d51e3b4..6e901f529 100755 --- a/bin/cli-users +++ b/bin/cli-users @@ -161,7 +161,10 @@ async function searchUsers() { } return data.users.nodes.map(user => { - const emails = user.emails.join(', '); + const emails = user.profiles + .filter(({ provider }) => provider === 'local') + .map(({ id }) => id) + .join(', '); return { name: `${user.username} (${emails}) ${user.id.gray} - ${ user.role.gray