From 428cf8135e4d8bca60c004bf5ef489174820be86 Mon Sep 17 00:00:00 2001 From: Berkeley Martinez Date: Thu, 21 Apr 2016 11:06:45 -0700 Subject: [PATCH 1/4] user-identity --- common/models/User-Identity.js | 54 ++++++++++++++++------------------ 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/common/models/User-Identity.js b/common/models/User-Identity.js index e57036a966..b04b27fcf1 100644 --- a/common/models/User-Identity.js +++ b/common/models/User-Identity.js @@ -40,49 +40,45 @@ export default function(UserIdent) { cb = options; options = {}; } - var autoLogin = options.autoLogin || !options.autoLogin; - var userIdentityModel = UserIdent; + const autoLogin = options.autoLogin || !options.autoLogin; + const userIdentityModel = UserIdent; profile.id = profile.id || profile.openid; - userIdentityModel.findOne({ + return userIdentityModel.findOne({ where: { provider: getSocialProvider(provider), externalId: profile.id } - }, function(err, identity) { - if (err) { - return cb(err); - } + }) + .then(identity => { if (identity) { identity.credentials = credentials; return identity.updateAttributes({ profile: profile, credentials: credentials, modified: new Date() - }, function(err) { - if (err) { - return cb(err); - } - // Find the user for the given identity - return identity.user(function(err, user) { - // Create access token if the autoLogin flag is set to true - if (!err && user && autoLogin) { - return (options.createAccessToken || createAccessToken)( - user, - function(err, token) { - cb(err, user, identity, token); - } - ); - } - return cb(err, user, identity); - }); + }) + .then(function() { + // Find the user for the given identity + return identity.user(function(err, user) { + // Create access token if the autoLogin flag is set to true + if (!err && user && autoLogin) { + return (options.createAccessToken || createAccessToken)( + user, + function(err, token) { + cb(err, user, identity, token); + } + ); + } + return cb(err, user, identity); + }); }); } // Find the user model - var userModel = userIdentityModel.relations.user && + const userModel = userIdentityModel.relations.user && userIdentityModel.relations.user.modelTo || loopback.getModelByType(loopback.User); - var userObj = options.profileToUser(provider, profile, options); + const userObj = options.profileToUser(provider, profile, options); if (!userObj.email && !options.emailOptional) { process.nextTick(function() { @@ -90,7 +86,7 @@ export default function(UserIdent) { }); } - var query; + const query; if (userObj.email) { query = { or: [ { username: userObj.username }, @@ -103,7 +99,7 @@ export default function(UserIdent) { if (err) { return cb(err); } - var date = new Date(); + const date = new Date(); return userIdentityModel.create({ provider: getSocialProvider(provider), externalId: profile.id, @@ -129,7 +125,7 @@ export default function(UserIdent) { }; UserIdent.observe('before save', function(ctx, next) { - var userIdent = ctx.currentInstance || ctx.instance; + const userIdent = ctx.currentInstance || ctx.instance; if (!userIdent) { debug('no user identity instance found'); return next(); From 8166bfbcd815d53545ff8bf1e1770148f52990d5 Mon Sep 17 00:00:00 2001 From: Berkeley Martinez Date: Thu, 21 Apr 2016 20:35:19 -0700 Subject: [PATCH 2/4] Remove o-auth account creation Accounts can only be created with Github or email --- common/models/User-Identity.js | 161 +++++++++++++++------------------ common/models/user.js | 2 + server/component-passport.js | 12 +-- server/utils/auth.js | 7 +- 4 files changed, 86 insertions(+), 96 deletions(-) diff --git a/common/models/User-Identity.js b/common/models/User-Identity.js index b04b27fcf1..f6815a38bc 100644 --- a/common/models/User-Identity.js +++ b/common/models/User-Identity.js @@ -12,18 +12,6 @@ const { defaultProfileImage } = require('../utils/constantStrings.json'); const githubRegex = (/github/i); const debug = debugFactory('fcc:models:userIdent'); -function createAccessToken(user, ttl, cb) { - if (arguments.length === 2 && typeof ttl === 'function') { - cb = ttl; - ttl = 0; - } - user.accessTokens.create({ - created: new Date(), - ttl: Math.min(ttl || user.constructor.settings.ttl, - user.constructor.settings.maxTTL) - }, cb); -} - export default function(UserIdent) { // original source // github.com/strongloop/loopback-component-passport @@ -40,88 +28,89 @@ export default function(UserIdent) { cb = options; options = {}; } - const autoLogin = options.autoLogin || !options.autoLogin; const userIdentityModel = UserIdent; profile.id = profile.id || profile.openid; - return userIdentityModel.findOne({ + const filter = { where: { provider: getSocialProvider(provider), externalId: profile.id } - }) - .then(identity => { - if (identity) { - identity.credentials = credentials; - return identity.updateAttributes({ - profile: profile, - credentials: credentials, - modified: new Date() - }) - .then(function() { - // Find the user for the given identity - return identity.user(function(err, user) { - // Create access token if the autoLogin flag is set to true - if (!err && user && autoLogin) { - return (options.createAccessToken || createAccessToken)( - user, - function(err, token) { - cb(err, user, identity, token); - } - ); - } - return cb(err, user, identity); - }); - }); - } - // Find the user model - const userModel = userIdentityModel.relations.user && - userIdentityModel.relations.user.modelTo || - loopback.getModelByType(loopback.User); - - const userObj = options.profileToUser(provider, profile, options); - - if (!userObj.email && !options.emailOptional) { - process.nextTick(function() { - return cb('email is missing from the user profile'); - }); - } - - const query; - if (userObj.email) { - query = { or: [ - { username: userObj.username }, - { email: userObj.email } - ]}; - } else { - query = { username: userObj.username }; - } - return userModel.findOrCreate({ where: query }, userObj, (err, user) => { - if (err) { - return cb(err); + }; + return userIdentityModel.findOne(filter) + .then(identity => { + // identity already exists + // find user and log them in + if (identity) { + identity.credentials = credentials; + const options = { + profile: profile, + credentials: credentials, + modified: new Date() + }; + return identity.updateAttributes(options) + // grab user associated with identity + .then(() => identity.user()) + .then(user => { + // Create access token for user + const options = { + created: new Date(), + ttl: user.constructor.settings.ttl + }; + return user.accessTokens.create(options) + .then(token => ({ user, token })); + }) + .then(({ token, user })=> { + cb(null, user, identity, token); + }) + .catch(err => cb(err)); } - const date = new Date(); - return userIdentityModel.create({ - provider: getSocialProvider(provider), - externalId: profile.id, - authScheme: authScheme, - profile: profile, - credentials: credentials, - userId: user.id, - created: date, - modified: date - }, function(err, identity) { - if (!err && user && autoLogin) { - return (options.createAccessToken || createAccessToken)( - user, - function(err, token) { - cb(err, user, identity, token); - } - ); - } - return cb(err, user, identity); - }); + // Find the user model + const userModel = userIdentityModel.relations.user && + userIdentityModel.relations.user.modelTo || + loopback.getModelByType(loopback.User); + + const userObj = options.profileToUser(provider, profile, options); + if (getSocialProvider(provider) !== 'github') { + return process.nextTick(() => cb( + new Error( + 'accounts can only be created using Github or though email' + ) + )); + } + + let query; + if (userObj.email) { + query = { or: [ + { username: userObj.username }, + { email: userObj.email } + ]}; + } else { + query = { username: userObj.username }; + } + return userModel.findOrCreate({ where: query }, userObj) + .then(([ user ]) => { + const promises = [ + userIdentityModel.create({ + provider: getSocialProvider(provider), + externalId: profile.id, + authScheme: authScheme, + profile: profile, + credentials: credentials, + userId: user.id, + created: new Date(), + modified: new Date() + }), + user.accessTokens.create({ + created: new Date(), + ttl: user.constructor.settings.ttl + }) + ]; + return Promise.all(promises) + .then(([ identity, token ]) => ({ user, identity, token })); + }) + .then(({ user, token, identity }) => cb(null, user, identity, token)) + .catch(err => cb(err)); }); - }); }; UserIdent.observe('before save', function(ctx, next) { diff --git a/common/models/user.js b/common/models/user.js index e406eecf1c..0ced33a968 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -41,6 +41,8 @@ module.exports = function(User) { User.definition.properties.rand.default = function() { return Math.random(); }; + // increase user accessToken ttl to 900 days + User.settings.ttl = 900 * 24 * 60 * 60 * 1000; // username should not be in blacklist User.validatesExclusionOf('username', { diff --git a/server/component-passport.js b/server/component-passport.js index db96edcb86..88be17fd71 100644 --- a/server/component-passport.js +++ b/server/component-passport.js @@ -13,18 +13,18 @@ import { const passportOptions = { emailOptional: true, profileToUser(provider, profile) { - var emails = profile.emails; + const emails = profile.emails; // NOTE(berks): get email or set to null. // MongoDB indexs email but can be sparse(blank) - var email = emails && emails[0] && emails[0].value ? + const email = emails && emails[0] && emails[0].value ? emails[0].value : null; // create random username // username will be assigned when camper signups for Github - var username = 'fcc' + uuid.v4().slice(0, 8); - var password = generateKey('password'); - var userObj = { + const username = 'fcc' + uuid.v4().slice(0, 8); + const password = generateKey('password'); + let userObj = { username: username, password: password }; @@ -41,7 +41,7 @@ const passportOptions = { } if (/github/.test(provider)) { - setProfileFromGithub(userObj, profile, profile._json); + userObj = setProfileFromGithub(userObj, profile, profile._json); } return userObj; } diff --git a/server/utils/auth.js b/server/utils/auth.js index 52afd45861..da282b7022 100644 --- a/server/utils/auth.js +++ b/server/utils/auth.js @@ -1,5 +1,3 @@ -import assign from 'object.assign'; - const providerHash = { facebook: ({ id }) => id, twitter: ({ username }) => username, @@ -32,15 +30,16 @@ export function setProfileFromGithub( name } ) { - return assign( + return Object.assign( user, - { isGithubCool: true, isMigrationGrandfathered: false }, { name, + email: user.email || githubEmail, username: username.toLowerCase(), location, joinedGithubOn, website, + isGithubCool: true, picture, githubId, githubURL, From caa49aea20ea38fc2451a44773f1fd231f63f352 Mon Sep 17 00:00:00 2001 From: Mrugesh Mohapatra Date: Tue, 3 May 2016 10:54:56 +0530 Subject: [PATCH 3/4] Refactor Email Update URL Change --- client/less/main.less | 2 +- server/boot/user.js | 2 +- server/views/account/settings.jade | 4 ++-- server/views/account/{email-update.jade => update-email.jade} | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) rename server/views/account/{email-update.jade => update-email.jade} (97%) diff --git a/client/less/main.less b/client/less/main.less index 192581626a..60320c5d6a 100644 --- a/client/less/main.less +++ b/client/less/main.less @@ -506,7 +506,7 @@ a[href="/email-signup"], a[href="/email-signin"] { } } -form.email-update .btn{ +form.update-email .btn{ margin:0; width:40%; display:inline-block; diff --git a/server/boot/user.js b/server/boot/user.js index f168514ba7..989a37edd6 100644 --- a/server/boot/user.js +++ b/server/boot/user.js @@ -241,7 +241,7 @@ module.exports = function(app) { if (!req.user) { return res.redirect('/'); } - return res.render('account/email-update', { + return res.render('account/update-email', { title: 'Update your Email' }); } diff --git a/server/views/account/settings.jade b/server/views/account/settings.jade index fa5df4841d..c992ece48e 100644 --- a/server/views/account/settings.jade +++ b/server/views/account/settings.jade @@ -54,7 +54,7 @@ block content p.large-p.text-center em #{user.email} .col-xs-12 - a.btn.btn-lg.btn-block.btn-primary.btn-link-social(href='/email-update') + a.btn.btn-lg.btn-block.btn-primary.btn-link-social(href='/update-email') i.fa.fa-envelope | Update my Email .row @@ -100,7 +100,7 @@ block content p.large-p.text-center | You don't have an email id associated to this account. .col-xs-12 - a.btn.btn-lg.btn-block.btn-primary.btn-link-social(href='/email-update') + a.btn.btn-lg.btn-block.btn-primary.btn-link-social(href='/update-email') i.fa.fa-envelope | Update my Email diff --git a/server/views/account/email-update.jade b/server/views/account/update-email.jade similarity index 97% rename from server/views/account/email-update.jade rename to server/views/account/update-email.jade index 06475f2e1f..bef48afab9 100644 --- a/server/views/account/email-update.jade +++ b/server/views/account/update-email.jade @@ -8,7 +8,7 @@ block content span.ion-close-circled#flash-close #flash-content h2.text-center Update your email address here: - form.form-horizontal.email-update(method='POST', action='/api/users/#{user.id}/update-email', name="updateEmailForm") + form.form-horizontal.update-email(method='POST', action='/api/users/#{user.id}/update-email', name="updateEmailForm") .row .col-sm-6.col-sm-offset-3 input(type='hidden', name='_csrf', value=_csrf) From 7b8dc2e77eacd49bbd46d2714fd4d792c4f87b3e Mon Sep 17 00:00:00 2001 From: Berkeley Martinez Date: Mon, 2 May 2016 22:26:18 -0700 Subject: [PATCH 4/4] Add friendly error message on auth create attemp --- common/models/User-Identity.js | 12 +++++++----- server/middlewares/error-handlers.js | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/common/models/User-Identity.js b/common/models/User-Identity.js index f6815a38bc..5293dfbc48 100644 --- a/common/models/User-Identity.js +++ b/common/models/User-Identity.js @@ -15,6 +15,8 @@ const debug = debugFactory('fcc:models:userIdent'); export default function(UserIdent) { // original source // github.com/strongloop/loopback-component-passport + const createAccountMessage = + 'Accounts can only be created using GitHub or though email'; UserIdent.login = function( provider, authScheme, @@ -71,11 +73,11 @@ export default function(UserIdent) { const userObj = options.profileToUser(provider, profile, options); if (getSocialProvider(provider) !== 'github') { - return process.nextTick(() => cb( - new Error( - 'accounts can only be created using Github or though email' - ) - )); + const err = new Error(createAccountMessage); + err.userMessage = createAccountMessage; + err.messageType = 'info'; + err.redirectTo = '/signin'; + return process.nextTick(() => cb(err)); } let query; diff --git a/server/middlewares/error-handlers.js b/server/middlewares/error-handlers.js index ddabe06a38..c37cd46908 100644 --- a/server/middlewares/error-handlers.js +++ b/server/middlewares/error-handlers.js @@ -25,11 +25,11 @@ export default function prodErrorHandler() { var message = 'Oops! Something went wrong. Please try again later'; if (type === 'html') { if (typeof req.flash === 'function') { - req.flash('errors', { - msg: message + req.flash(err.messageType || 'errors', { + msg: err.userMessage || message }); } - return res.redirect('/map'); + return res.redirect(err.redirectTo || '/map'); // json } else if (type === 'json') { res.setHeader('Content-Type', 'application/json');