From aeb4b51f3341b2743942a819b2f29316094a1a99 Mon Sep 17 00:00:00 2001 From: Harry Dbrandy Date: Sun, 7 Apr 2019 10:52:24 +0100 Subject: [PATCH 1/5] feat(oauth): :sparkles: add middleware layer for handling oauth payload This adds an extra layer for creating a user from the payload received after making OAuth calls from the frontend. It solves the problem in consuming the social authentication endpoint from an external frontend domain. [Delivers #165167110], [#164198183] --- package.json | 1 + server/middlewares/socialCreate.js | 37 ++++++++++++++++++++ server/middlewares/validator.js | 28 +++++++++++++++ server/routes/auth.js | 12 ++++++- test/integrations/routes/auth.test.js | 50 +++++++++++++++++++++++++++ 5 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 server/middlewares/socialCreate.js diff --git a/package.json b/package.json index f0fd562..7269422 100644 --- a/package.json +++ b/package.json @@ -96,6 +96,7 @@ "mocha": "^6.0.2", "mocha-lcov-reporter": "^1.3.0", "mocha-prepare": "^0.1.0", + "node-mocks-http": "^1.7.3", "nodemon": "^1.18.10", "npm-run-all": "^4.1.5", "nyc": "^13.3.0", diff --git a/server/middlewares/socialCreate.js b/server/middlewares/socialCreate.js new file mode 100644 index 0000000..0d45bee --- /dev/null +++ b/server/middlewares/socialCreate.js @@ -0,0 +1,37 @@ +import models from '../models'; + +/** + * Register a new user with oAuthentication + * + * @export + * @param {Request} request Request object + * @param {Response} response Response object + * @param {Function} next call to next middleware + * @returns {(Error|Function)} Proceeds to the next middleware else throws http error + */ +async function socialCreate(request, response, next) { + const { firstname, lastname, email } = request.body; + let user; + try { + user = await models.User.findOne({ where: { email } }); + if (!user) { + user = await models.User.create({ email }); + const role = await models.Role.findOne({ where: { name: 'user' } }); + await user.setRole(role); + await models.Profile.create({ + firstname, + lastname, + bio: '', + gender: '', + userId: user.id, + username: email, + }); + } + request.user = user; + return next(); + } catch (error) { + next(error); + } +} + +export default socialCreate; diff --git a/server/middlewares/validator.js b/server/middlewares/validator.js index 400c5b8..97aa51e 100644 --- a/server/middlewares/validator.js +++ b/server/middlewares/validator.js @@ -342,4 +342,32 @@ export default class Validator { .withMessage('roleId parameter must be an integer'), ]; } + + /** + * Validates payload for social oauth + * + * @static + * @returns {array} - The array of express validator chains + * @memberof Validator + */ + static validateSocial() { + return [ + ...Validator.validateFirstname(), + ...Validator.validateLastname(), + ...Validator.validateEmail(), + body('socialId') + .isInt() + .withMessage('socialId must be a string'), + body('socialType') + .not().isEmpty() + .withMessage('A valid service provider is required') + .custom((provider, { req }) => { + const allowedProviders = ['facebook', 'google', 'twitter']; + if (allowedProviders.indexOf(provider) === -1) { + throw new Error('Invalid service provider'); + } + return provider; + }), + ]; + } } diff --git a/server/routes/auth.js b/server/routes/auth.js index 0f9eb07..73e6c6a 100644 --- a/server/routes/auth.js +++ b/server/routes/auth.js @@ -1,7 +1,9 @@ import express from 'express'; import passport from '../config/passport'; import authController from '../controllers/authController'; - +import socialCreate from '../middlewares/socialCreate'; +import ValidatorHandler from '../middlewares/handleValidation'; +import Validator from '../middlewares/validator'; const router = express.Router(); @@ -114,4 +116,12 @@ router.get( authController.socialAuth ); +router.post( + '/social', + Validator.validateSocial(), + ValidatorHandler.handleValidation, + socialCreate, + authController.socialAuth +); + export default router; diff --git a/test/integrations/routes/auth.test.js b/test/integrations/routes/auth.test.js index 40e3e09..826b635 100644 --- a/test/integrations/routes/auth.test.js +++ b/test/integrations/routes/auth.test.js @@ -1,6 +1,9 @@ +/* eslint-disable no-unused-expressions */ import chai, { expect } from 'chai'; import chaiHttp from 'chai-http'; +import faker from 'faker'; import app from '../../../server'; +import { STATUS } from '../../../server/helpers/constants'; chai.use(chaiHttp); @@ -66,4 +69,51 @@ describe('API endpoint: /api/auth', () => { expect(response.res.statusMessage).to.equal('Not Found'); }); }); + + describe('API endpoint: auth/social', () => { + it('should register user and respond with user payload', (done) => { + const payload = { + firstname: faker.name.firstName(), + lastname: faker.name.lastName(), + email: faker.internet.email(), + socialType: 'facebook', + socialId: 3212313213, + }; + + chai + .request(app) + .post('/api/v1/auth/social') + .send(payload) + .end((err, res) => { + expect(err).to.be.null; + expect(res).to.have.status(STATUS.OK); + expect(res.body) + .to.haveOwnProperty('user') + .to.have.keys(['id', 'email', 'token']); + done(); + }); + }); + it('should return an error if payload is invalid', (done) => { + const payload = { + firstname: faker.name.firstName(), + lastname: faker.name.lastName(), + email: 'invalid email', + socialType: 'invalid provider', + socialId: 'invalid', + }; + + chai + .request(app) + .post('/api/v1/auth/social') + .send(payload) + .end((err, res) => { + expect(res).to.have.status(STATUS.BAD_REQUEST); + expect(res.body) + .to.haveOwnProperty('data') + .to.be.an('array') + .to.be.lengthOf(3); + done(); + }); + }); + }); }); From f1f55a4dd5910cd789cc62a959f29920b9e09eef Mon Sep 17 00:00:00 2001 From: Harry Dbrandy Date: Sun, 7 Apr 2019 11:32:30 +0100 Subject: [PATCH 2/5] chore(npmPackage): :fire: remove unused package --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 7269422..f0fd562 100644 --- a/package.json +++ b/package.json @@ -96,7 +96,6 @@ "mocha": "^6.0.2", "mocha-lcov-reporter": "^1.3.0", "mocha-prepare": "^0.1.0", - "node-mocks-http": "^1.7.3", "nodemon": "^1.18.10", "npm-run-all": "^4.1.5", "nyc": "^13.3.0", From 4af37f55116dbe903659f629dd517b86bcfc9046 Mon Sep 17 00:00:00 2001 From: Harry Dbrandy Date: Tue, 9 Apr 2019 12:36:31 +0100 Subject: [PATCH 3/5] feat(socialAuth): resolve lint warnings with minimal update to validator --- server/middlewares/socialCreate.js | 2 +- server/middlewares/validator.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/middlewares/socialCreate.js b/server/middlewares/socialCreate.js index 0d45bee..8791653 100644 --- a/server/middlewares/socialCreate.js +++ b/server/middlewares/socialCreate.js @@ -7,7 +7,7 @@ import models from '../models'; * @param {Request} request Request object * @param {Response} response Response object * @param {Function} next call to next middleware - * @returns {(Error|Function)} Proceeds to the next middleware else throws http error + * @returns {Function} Proceeds to the next middleware else throws http error */ async function socialCreate(request, response, next) { const { firstname, lastname, email } = request.body; diff --git a/server/middlewares/validator.js b/server/middlewares/validator.js index 97aa51e..a311c80 100644 --- a/server/middlewares/validator.js +++ b/server/middlewares/validator.js @@ -357,7 +357,7 @@ export default class Validator { ...Validator.validateEmail(), body('socialId') .isInt() - .withMessage('socialId must be a string'), + .withMessage('socialId must be an integer'), body('socialType') .not().isEmpty() .withMessage('A valid service provider is required') From a1eb4a6b351883501af8d674eca4ca37ecff8c8d Mon Sep 17 00:00:00 2001 From: Harry Dbrandy Date: Sun, 14 Apr 2019 11:11:52 +0100 Subject: [PATCH 4/5] feat(oauth): implement a redirect back to the client app - Updated ENV config to include client redirect url - Remove additional middleware layer --- .env.example | 3 ++ server/controllers/authController.js | 16 ++------- server/middlewares/socialCreate.js | 37 -------------------- server/middlewares/validator.js | 28 --------------- server/routes/auth.js | 22 +++++------- test/integrations/routes/auth.test.js | 49 --------------------------- 6 files changed, 14 insertions(+), 141 deletions(-) delete mode 100644 server/middlewares/socialCreate.js diff --git a/.env.example b/.env.example index 347f36c..b4d7be1 100644 --- a/.env.example +++ b/.env.example @@ -26,6 +26,9 @@ GOOGLE_CLIENT_ID= GOOGLE_CLIENT_SECRET= GOOGLE_CALLBACK_URL= +# Redirect URL to client app (preferably the login page) +CLIENT_REDIRECT_URL= + # sessions details COOKIE_SECRET= diff --git a/server/controllers/authController.js b/server/controllers/authController.js index 117816f..3dde918 100644 --- a/server/controllers/authController.js +++ b/server/controllers/authController.js @@ -1,4 +1,4 @@ -import { generateToken } from '../helpers/utils'; +import { generateToken, env } from '../helpers/utils'; import logger from '../helpers/logger'; /** @@ -16,18 +16,8 @@ export default class authController { static async socialAuth(req, res) { const { dataValues } = req.user; try { - const userToken = await generateToken({ email: dataValues.email, id: dataValues.id }); - const { - id, - email, - } = dataValues; - return res.status(200).json({ - user: { - id, - email, - token: userToken, - } - }); + const token = await generateToken({ email: dataValues.email, id: dataValues.id }); + res.redirect(`${env('CLIENT_REDIRECT_URL')}?token=${token}`); } catch (err) { logger.log(err); } diff --git a/server/middlewares/socialCreate.js b/server/middlewares/socialCreate.js deleted file mode 100644 index 8791653..0000000 --- a/server/middlewares/socialCreate.js +++ /dev/null @@ -1,37 +0,0 @@ -import models from '../models'; - -/** - * Register a new user with oAuthentication - * - * @export - * @param {Request} request Request object - * @param {Response} response Response object - * @param {Function} next call to next middleware - * @returns {Function} Proceeds to the next middleware else throws http error - */ -async function socialCreate(request, response, next) { - const { firstname, lastname, email } = request.body; - let user; - try { - user = await models.User.findOne({ where: { email } }); - if (!user) { - user = await models.User.create({ email }); - const role = await models.Role.findOne({ where: { name: 'user' } }); - await user.setRole(role); - await models.Profile.create({ - firstname, - lastname, - bio: '', - gender: '', - userId: user.id, - username: email, - }); - } - request.user = user; - return next(); - } catch (error) { - next(error); - } -} - -export default socialCreate; diff --git a/server/middlewares/validator.js b/server/middlewares/validator.js index a311c80..400c5b8 100644 --- a/server/middlewares/validator.js +++ b/server/middlewares/validator.js @@ -342,32 +342,4 @@ export default class Validator { .withMessage('roleId parameter must be an integer'), ]; } - - /** - * Validates payload for social oauth - * - * @static - * @returns {array} - The array of express validator chains - * @memberof Validator - */ - static validateSocial() { - return [ - ...Validator.validateFirstname(), - ...Validator.validateLastname(), - ...Validator.validateEmail(), - body('socialId') - .isInt() - .withMessage('socialId must be an integer'), - body('socialType') - .not().isEmpty() - .withMessage('A valid service provider is required') - .custom((provider, { req }) => { - const allowedProviders = ['facebook', 'google', 'twitter']; - if (allowedProviders.indexOf(provider) === -1) { - throw new Error('Invalid service provider'); - } - return provider; - }), - ]; - } } diff --git a/server/routes/auth.js b/server/routes/auth.js index 73e6c6a..644acc6 100644 --- a/server/routes/auth.js +++ b/server/routes/auth.js @@ -1,12 +1,14 @@ import express from 'express'; import passport from '../config/passport'; import authController from '../controllers/authController'; -import socialCreate from '../middlewares/socialCreate'; -import ValidatorHandler from '../middlewares/handleValidation'; -import Validator from '../middlewares/validator'; +import { env } from '../helpers/utils'; const router = express.Router(); +const failureRedirect = () => ({ + failureRedirect: `${env('CLIENT_REDIRECT_URL')}?failure=true` +}); + /** * @swagger * definitions: @@ -55,7 +57,7 @@ router.get( router.get( '/facebook/redirect', - passport.authenticate('facebook', { failureRedirect: '/' }), + passport.authenticate('facebook', failureRedirect()), authController.socialAuth ); @@ -86,7 +88,7 @@ router.get( router.get( '/google/redirect', - passport.authenticate('google', { failureRedirect: '/' }), + passport.authenticate('google', failureRedirect()), authController.socialAuth ); @@ -112,15 +114,7 @@ router.get( router.get( '/twitter/redirect', - passport.authenticate('twitter', { failureRedirect: '/' }), - authController.socialAuth -); - -router.post( - '/social', - Validator.validateSocial(), - ValidatorHandler.handleValidation, - socialCreate, + passport.authenticate('twitter', failureRedirect()), authController.socialAuth ); diff --git a/test/integrations/routes/auth.test.js b/test/integrations/routes/auth.test.js index 826b635..5110020 100644 --- a/test/integrations/routes/auth.test.js +++ b/test/integrations/routes/auth.test.js @@ -1,9 +1,7 @@ /* eslint-disable no-unused-expressions */ import chai, { expect } from 'chai'; import chaiHttp from 'chai-http'; -import faker from 'faker'; import app from '../../../server'; -import { STATUS } from '../../../server/helpers/constants'; chai.use(chaiHttp); @@ -69,51 +67,4 @@ describe('API endpoint: /api/auth', () => { expect(response.res.statusMessage).to.equal('Not Found'); }); }); - - describe('API endpoint: auth/social', () => { - it('should register user and respond with user payload', (done) => { - const payload = { - firstname: faker.name.firstName(), - lastname: faker.name.lastName(), - email: faker.internet.email(), - socialType: 'facebook', - socialId: 3212313213, - }; - - chai - .request(app) - .post('/api/v1/auth/social') - .send(payload) - .end((err, res) => { - expect(err).to.be.null; - expect(res).to.have.status(STATUS.OK); - expect(res.body) - .to.haveOwnProperty('user') - .to.have.keys(['id', 'email', 'token']); - done(); - }); - }); - it('should return an error if payload is invalid', (done) => { - const payload = { - firstname: faker.name.firstName(), - lastname: faker.name.lastName(), - email: 'invalid email', - socialType: 'invalid provider', - socialId: 'invalid', - }; - - chai - .request(app) - .post('/api/v1/auth/social') - .send(payload) - .end((err, res) => { - expect(res).to.have.status(STATUS.BAD_REQUEST); - expect(res.body) - .to.haveOwnProperty('data') - .to.be.an('array') - .to.be.lengthOf(3); - done(); - }); - }); - }); }); From e2d2d22c4fa3aee617df9231dba283a5db4868d9 Mon Sep 17 00:00:00 2001 From: Harry Dbrandy Date: Tue, 16 Apr 2019 11:33:40 +0100 Subject: [PATCH 5/5] feat: adjust user generated token to include profile info Enhanced social authentication to include user profile upon saving the user information --- server/config/passport.js | 16 +++++++++++++--- server/controllers/usersController.js | 4 ++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/server/config/passport.js b/server/config/passport.js index c5038a6..09a38e4 100644 --- a/server/config/passport.js +++ b/server/config/passport.js @@ -6,7 +6,7 @@ import { env, validateConfigVariable } from '../helpers/utils'; import logger from '../helpers/logger'; import models from '../models'; -const { User } = models; +const { User, Profile } = models; // Ensure that ENV config variables is set validateConfigVariable([ @@ -34,13 +34,23 @@ validateConfigVariable([ export const generateOrFindUser = async (accessToken, refreshToken, profile, done) => { if (profile.emails[0]) { + const email = profile.emails[0].value; try { - await User.findOrCreate({ where: { email: profile.emails[0].value } }) + await User.findOrCreate({ where: { email } }) /* the "spread" divides the array that findOrCreate method returns into 2 parts and passes them as arguments to the callback function, which treats them as "user" and "created". */ - .spread((user, created) => { + .spread(async (user, created) => { + // create user profile + const [firstname, lastname] = profile.displayName.split(' '); + user.firstname = firstname; + user.lastname = lastname; + user.image = profile.photos[0].value; + user.userId = user.id; + user.bio = ''; + user.username = email; + await Profile.create(user); done(null, user); }); } catch (err) { diff --git a/server/controllers/usersController.js b/server/controllers/usersController.js index 1ec998b..f9d1985 100644 --- a/server/controllers/usersController.js +++ b/server/controllers/usersController.js @@ -31,10 +31,9 @@ class UsersController { const { body } = request; try { - const user = await User.create(body, { raw: true }); + const user = await User.create(body); const role = await models.Role.findOne({ where: { name: 'user' } }); await user.setRole(role); - const token = await generateToken({ user }); // generate confirm token const confirmationToken = await generateToken({ email: user.email }); // generate confirm link @@ -57,6 +56,7 @@ class UsersController { await models.Profile.create(user); // create a user default settings await models.Setting.create({ userId: user.userId }); + const token = await generateToken({ user }); Response.send(response, STATUS.CREATED, { token, id: user.id }); await Mail.sendMail(data); return;