From f6184fddb09a1787fed259b046a66c919269a12f Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 11 Nov 2016 12:53:51 -0700 Subject: [PATCH 1/3] Added facebook login support + redis cache --- README.md | 9 ++-- app.js | 1 + cache.js | 97 ++++++++++++++++++++++++++++++++++++ passport.js | 2 +- routes/api/auth/index.js | 89 ++++++++++++++++++++++++++------- routes/api/comments/index.js | 2 +- views/auth-callback.ejs | 9 ++++ 7 files changed, 185 insertions(+), 24 deletions(-) create mode 100644 cache.js create mode 100644 views/auth-callback.ejs diff --git a/README.md b/README.md index ffc1f3077..0f217d89c 100644 --- a/README.md +++ b/README.md @@ -19,9 +19,12 @@ Runs Talk. The Talk application requires specific configuration options to be available inside the environment in order to run, those variables are listed here: -- `TALK_SESSION_SECRET` (*required*) - -- `TALK_FACEBOOK_APP_ID` (*required*) - -- `TALK_FACEBOOK_APP_SECRET` (*required*) - +- `TALK_SESSION_SECRET` (*required*) - a random string which will be used to + secure cookies +- `TALK_FACEBOOK_APP_ID` (*required*) - the Facebook app id for your Facebook + Login enabled app. +- `TALK_FACEBOOK_APP_SECRET` (*required*) - the Facebook app secret for your + Facebook Login enabled app. - `TALK_ROOT_URL` (*required*) - Root url of the installed application externally available in the format: `://` without the path. ### Running with Docker diff --git a/app.js b/app.js index d945d6254..f5b39a074 100644 --- a/app.js +++ b/app.js @@ -38,6 +38,7 @@ const session_opts = { rolling: true, saveUninitialized: false, resave: false, + name: 'talk.sid', cookie: { secure: false, maxAge: 18000000, // 30 minutes for expiry. diff --git a/cache.js b/cache.js new file mode 100644 index 000000000..efe689f9c --- /dev/null +++ b/cache.js @@ -0,0 +1,97 @@ +const redis = require('./redis'); + +const cache = module.exports = {}; + +/** + * This collects a key that may either be an array or a string and creates a + * unified key out of it. + * @param {Mixed} key Either an array of items composing a key or a string + * @return {String} A string that represents a key + */ +const keyfunc = (key) => { + if (Array.isArray(key)) { + return `cache[${key.join(':')}]`; + } + + return `cache[${key}]`; +}; + +/** + * This wraps a complicated function with a cache, in the event that the item is + * not inside the cache, it will perform the work to get it and then set it + * followed by returning the value. + * @param {Mixed} key Either an array of items or string represening this + * work + * @param {Integer} expiry Time in seconds for the cache entry to live for + * @param {Function} work A function that returns a promise that can be + * resolved as the value to cache. + * @return {Promise} Resolves to the value either retrieved from cache + */ +cache.wrap = (key, expiry, work) => { + return cache + .get(key) + .then((value) => { + if (value !== null) { + return value; + } + + return work() + .then((value) => { + return cache + .set(key, value, expiry) + .then(() => value); + }); + }); +}; + +/** + * This returns a promise that returns a promise that resolves with the value + * from the cache or null if it does not exist in the cache. + * @param {Mixed} key Either an array of items composing a key or a string + * @return {Promise} + */ +cache.get = (key) => new Promise((resolve, reject) => { + redis.get(keyfunc(key), (err, reply) => { + if (err) { + return reject(err); + } + + if (reply !== null) { + let value; + + try { + + // Parse the stored cache value from JSON. + value = JSON.parse(reply); + } catch (e) { + return reject(e); + } + + return resolve(value); + } + + resolve(null); + }); +}); + +/** + * This sets a value on the key with the expiry and then resolves once it is + * done. + * @param {Mixed} key Either an array of items composing a key or a string + * @param {Mixed} value Object to be serialized and set to the cache + * @param {Integer} expiry Time in seconds for the cache entry to live for + * @return {Promise} + */ +cache.set = (key, value, expiry) => new Promise((resolve, reject) => { + + // Serialize the value as JSON. + let reply = JSON.stringify(value); + + redis.set(keyfunc(key), reply, 'EX', expiry, (err) => { + if (err) { + return reject(err); + } + + return resolve(); + }); +}); diff --git a/passport.js b/passport.js index ae4776894..38a92075a 100644 --- a/passport.js +++ b/passport.js @@ -65,7 +65,7 @@ if (process.env.TALK_FACEBOOK_APP_ID && process.env.TALK_FACEBOOK_APP_SECRET && passport.use(new FacebookStrategy({ clientID: process.env.TALK_FACEBOOK_APP_ID, clientSecret: process.env.TALK_FACEBOOK_APP_SECRET, - callbackURL: `${process.env.TALK_ROOT_URL}/connect/facebook/callback` + callbackURL: `${process.env.TALK_ROOT_URL}/api/v1/auth/facebook/callback` }, (accessToken, refreshToken, profile, done) => { User .findOrCreateExternalUser(profile) diff --git a/routes/api/auth/index.js b/routes/api/auth/index.js index 57d126449..2c6f91676 100644 --- a/routes/api/auth/index.js +++ b/routes/api/auth/index.js @@ -4,40 +4,91 @@ const authorization = require('../../../middleware/authorization'); const router = express.Router(); +/** + * This returns the user if they are logged in. + */ router.get('/', authorization.needed(), (req, res) => { res.json(req.user); }); -router.delete('/', (req, res) => { - req.logout(); - res.status(204).end(); +/** + * This destroys the session of a user, if they have one. + */ +router.delete('/', authorization.needed(), (req, res) => { + req.session.destroy(() => { + res.status(204).end(); + }); }); +/** + * This sends back the user data as JSON. + */ +const HandleAuthCallback = (req, res, next) => (err, user) => { + if (err) { + return next(err); + } + + if (!user) { + return next(authorization.ErrNotAuthorized); + } + + // Perform the login of the user! + req.logIn(user, (err) => { + if (err) { + return next(err); + } + + // We logged in the user! Let's send back the user data. + res.json({user}); + }); +}; + +/** + * Returns the response to the login attempt via a popup callback with some JS. + */ +const HandleAuthPopupCallback = (req, res, next) => (err, user) => { + if (err) { + return res.render('auth-callback', {err: JSON.stringify(err), data: null}); + } + + if (!user) { + return res.render('auth-callback', {err: JSON.stringify(authorization.ErrNotAuthorized), data: null}); + } + + // Perform the login of the user! + req.logIn(user, (err) => { + if (err) { + return res.render('auth-callback', {err: JSON.stringify(err), data: null}); + } + + // We logged in the user! Let's send back the user data. + res.render('auth-callback', {err: null, data: JSON.stringify(user)}); + }); +}; + /** * Local auth endpoint, will recieve a email and password */ router.post('/local', (req, res, next) => { // Perform the local authentication. - passport.authenticate('local', (err, user) => { - if (err) { - return next(err); - } + passport.authenticate('local', HandleAuthCallback(req, res, next))(req, res, next); +}); - if (!user) { - return next(authorization.ErrNotAuthorized); - } +/** + * Facebook auth endpoint, this will redirect the user immediatly to facebook + * for authorization. + */ +router.get('/facebook', passport.authenticate('facebook')); - // Perform the login of the user! - req.logIn(user, (err) => { - if (err) { - return next(err); - } +/** + * Facebook callback endpoint, this will send the user a html page designed to + * send back the user credentials upon sucesfull login. + */ +router.get('/facebook/callback', (req, res, next) => { - // We logged in the user! Let's send back the user data. - res.json({user}); - }); - })(req, res, next); + // Perform the facebook login flow and pass the data back through the opener. + passport.authenticate('facebook', HandleAuthPopupCallback(req, res, next))(req, res, next); }); module.exports = router; diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index a2e77b417..bafc7dbcc 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -94,7 +94,7 @@ router.post('/:comment_id', (req, res, next) => { }); router.post('/:comment_id/status', (req, res, next) => { - + Comment .changeStatus(req.params.comment_id, req.body.status) .then(comment => res.status(200).send(comment)) diff --git a/views/auth-callback.ejs b/views/auth-callback.ejs new file mode 100644 index 000000000..d38d759ee --- /dev/null +++ b/views/auth-callback.ejs @@ -0,0 +1,9 @@ + + + + + + From 68433ec973d349d6500169eb593b0093b43d412c Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 11 Nov 2016 14:27:48 -0700 Subject: [PATCH 2/3] Added "display: popup" to the facebook dialog --- routes/api/auth/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routes/api/auth/index.js b/routes/api/auth/index.js index 2c6f91676..37b41df6b 100644 --- a/routes/api/auth/index.js +++ b/routes/api/auth/index.js @@ -79,7 +79,7 @@ router.post('/local', (req, res, next) => { * Facebook auth endpoint, this will redirect the user immediatly to facebook * for authorization. */ -router.get('/facebook', passport.authenticate('facebook')); +router.get('/facebook', passport.authenticate('facebook', {display: 'popup'})); /** * Facebook callback endpoint, this will send the user a html page designed to From 80e12d2e303b14e7d4ed1e51e6688912fec69276 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 11 Nov 2016 14:59:27 -0700 Subject: [PATCH 3/3] Added photo support --- models/user.js | 2 ++ passport.js | 3 ++- routes/api/auth/index.js | 4 ++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/models/user.js b/models/user.js index 6800b59ef..2cfb03fac 100644 --- a/models/user.js +++ b/models/user.js @@ -12,6 +12,7 @@ const UserSchema = new mongoose.Schema({ required: true }, displayName: String, + photo: String, disabled: Boolean, password: String, profiles: [{ @@ -139,6 +140,7 @@ UserSchema.statics.findOrCreateExternalUser = function(profile) { user = new User({ displayName: profile.displayName, roles: [], + photo: Array.isArray(profile.photos) && profile.photos.length > 0 ? profile.photos[0].value : null, profiles: [ { id: profile.id, diff --git a/passport.js b/passport.js index 38a92075a..2e084eafd 100644 --- a/passport.js +++ b/passport.js @@ -65,7 +65,8 @@ if (process.env.TALK_FACEBOOK_APP_ID && process.env.TALK_FACEBOOK_APP_SECRET && passport.use(new FacebookStrategy({ clientID: process.env.TALK_FACEBOOK_APP_ID, clientSecret: process.env.TALK_FACEBOOK_APP_SECRET, - callbackURL: `${process.env.TALK_ROOT_URL}/api/v1/auth/facebook/callback` + callbackURL: `${process.env.TALK_ROOT_URL}/api/v1/auth/facebook/callback`, + profileFields: ['id', 'displayName', 'picture.type(large)'] }, (accessToken, refreshToken, profile, done) => { User .findOrCreateExternalUser(profile) diff --git a/routes/api/auth/index.js b/routes/api/auth/index.js index 37b41df6b..87ffb62c8 100644 --- a/routes/api/auth/index.js +++ b/routes/api/auth/index.js @@ -14,7 +14,7 @@ router.get('/', authorization.needed(), (req, res) => { /** * This destroys the session of a user, if they have one. */ -router.delete('/', authorization.needed(), (req, res) => { +router.delete('/', (req, res) => { req.session.destroy(() => { res.status(204).end(); }); @@ -79,7 +79,7 @@ router.post('/local', (req, res, next) => { * Facebook auth endpoint, this will redirect the user immediatly to facebook * for authorization. */ -router.get('/facebook', passport.authenticate('facebook', {display: 'popup'})); +router.get('/facebook', passport.authenticate('facebook', {display: 'popup', authType: 'rerequest', scope: ['public_profile']})); /** * Facebook callback endpoint, this will send the user a html page designed to