diff --git a/common/locales/en/api-v3.json b/common/locales/en/api-v3.json index edd383f9b9..526cf273bf 100644 --- a/common/locales/en/api-v3.json +++ b/common/locales/en/api-v3.json @@ -4,7 +4,7 @@ "missingEmail": "Missing email.", "missingUsername": "Missing username.", "missingPassword": "Missing password.", - "invalidEmail": "Invalid email address.", + "notAnEmail": "Invalid email address.", "emailTaken": "Email already taken.", "usernameTaken": "Username already taken.", "passwordConfirmationMatch": "Password confirmation doesn't match password.", @@ -12,5 +12,7 @@ "invalidCredentials": "User not found with given auth credentials.", "accountSuspended": "Account has been suspended, please contact leslie@habitica.com with your UUID \"<%= userId %>\" for assistance.", "onlyFbSupported": "Only Facebook supported currently.", - "cantDetachFb": "Account lacks another authentication method, can't detach Facebook." + "cantDetachFb": "Account lacks another authentication method, can't detach Facebook.", + "onlySocialAttachLocal": "Local auth can only be added to a social account.", + "invalidReqParams": "Invalid request parameters." } diff --git a/test/api/v3/integration/user/auth/POST-register_local.test.js b/test/api/v3/integration/user/auth/POST-register_local.test.js index 8eca0ed07d..4ad2127cd4 100644 --- a/test/api/v3/integration/user/auth/POST-register_local.test.js +++ b/test/api/v3/integration/user/auth/POST-register_local.test.js @@ -39,9 +39,9 @@ describe('POST /user/auth/local/register', () => { password, confirmPassword: confirmPassword, })).to.eventually.be.rejected.and.eql({ - code: 401, - error: 'NotAuthorized', - message: t('passwordConfirmationMatch'), + code: 400, + error: 'BadRequest', + message: t('invalidReqParams'), }); }); @@ -56,9 +56,9 @@ describe('POST /user/auth/local/register', () => { password, confirmPassword, })).to.eventually.be.rejected.and.eql({ - code: 401, - error: 'NotAuthorized', - message: t('missingUsername'), + code: 400, + error: 'BadRequest', + message: t('invalidReqParams'), }); }); @@ -72,9 +72,9 @@ describe('POST /user/auth/local/register', () => { password, confirmPassword: password, })).to.eventually.be.rejected.and.eql({ - code: 401, - error: 'NotAuthorized', - message: t('missingEmail'), + code: 400, + error: 'BadRequest', + message: t('invalidReqParams'), }); }); @@ -90,9 +90,9 @@ describe('POST /user/auth/local/register', () => { password, confirmPassword: password, })).to.eventually.be.rejected.and.eql({ - code: 401, - error: 'NotAuthorized', - message: t('invalidEmail'), + code: 400, + error: 'BadRequest', + message: t('invalidReqParams'), }); }); @@ -107,9 +107,9 @@ describe('POST /user/auth/local/register', () => { email: email, confirmPassword: confirmPassword, })).to.eventually.be.rejected.and.eql({ - code: 401, - error: 'NotAuthorized', - message: t('missingPassword'), + code: 400, + error: 'BadRequest', + message: t('invalidReqParams'), }); }); }); diff --git a/website/src/controllers/api-v3/user.js b/website/src/controllers/api-v3/user.js index 643108886f..9dd471868f 100644 --- a/website/src/controllers/api-v3/user.js +++ b/website/src/controllers/api-v3/user.js @@ -12,7 +12,7 @@ import { sendTxn as sendTxnEmail } from '../../libs/api-v3/email'; let api = {}; /** - * @api {post} /user/auth/local/register Register a new user with email, username and password or add local authentication to a social user + * @api {post} /user/auth/local/register Register a new user with email, username and password or attach local auth to a social user * @apiVersion 3.0.0 * @apiName UserRegisterLocal * @apiGroup User @@ -22,21 +22,32 @@ let api = {}; * @apiParam {String} password Password for the new user account * @apiParam {String} confirmPassword Password confirmation * - * @apiSuccess {Object} user The user object + * @apiSuccess {Object} user The user object, if we just attached local auth to a social user then only user.auth.local */ api.registerLocal = { method: 'POST', + middlewares: [authWithHeaders(true)], url: '/user/auth/local/register', handler (req, res, next) { - let { email, username, password, confirmPassword } = req.body; + let fbUser = res.locals.user; // If adding local auth to social user - // Validate required params - if (!username) return next(new NotAuthorized(res.t('missingUsername'))); - if (!email) return next(new NotAuthorized(res.t('missingEmail'))); - if (!validator.isEmail(email)) return next(new NotAuthorized(res.t('invalidEmail'))); - if (!password) return next(new NotAuthorized(res.t('missingPassword'))); - if (password !== confirmPassword) return next(new NotAuthorized(res.t('passwordConfirmationMatch'))); + req.checkBody({ + email: { + notEmpty: {errorMessage: res.t('missingEmail')}, + isEmail: {errorMessage: res.t('notAnEmail')}, + }, + username: {notEmpty: {errorMessage: res.t('missingUsername')}}, + password: { + notEmpty: {errorMessage: res.t('missingPassword')}, + equals: {options: [req.body.confirmPassword], errorMessage: res.t('passwordConfirmationMatch')}, + }, + }); + let validationErrors = req.validationErrors(); + + if (validationErrors) return next(validationErrors); + + let { email, username, password } = req.body; // Get the lowercase version of username to check that we do not have duplicates // So we can search for it in the database and then reject the choosen username if 1 or more results are found email = email.toLowerCase(); @@ -57,7 +68,7 @@ api.registerLocal = { let salt = passwordUtils.makeSalt(); let hashed_password = passwordUtils.encrypt(password, salt); // eslint-disable-line camelcase - let newUser = new User({ + let newUser = { auth: { local: { username, @@ -70,11 +81,17 @@ api.registerLocal = { preferences: { language: req.language, }, - }); + }; - newUser.registeredThrough = req.headers['x-client']; // TODO is this saved somewhere? - - return newUser.save(); + if (fbUser) { + if (!fbUser.auth.facebook.id) throw new NotAuthorized(res.t('onlySocialAttachLocal')); + fbUser.auth.local = newUser; + return fbUser.save(); + } else { + newUser = new User(newUser); + newUser.registeredThrough = req.headers['x-client']; // TODO is this saved somewhere? + return newUser.save(); + } }) .then((savedUser) => { res.status(201).json(savedUser); @@ -84,12 +101,14 @@ api.registerLocal = { .remove({email: savedUser.auth.local.email}) .then(() => sendTxnEmail(savedUser, 'welcome')); - res.analytics.track('register', { - category: 'acquisition', - type: 'local', - gaLabel: 'local', - uuid: savedUser._id, - }); + if (!savedUser.auth.facebook.id) { + res.analytics.track('register', { + category: 'acquisition', + type: 'local', + gaLabel: 'local', + uuid: savedUser._id, + }); + } }) .catch(next); }, @@ -225,7 +244,7 @@ api.loginSocial = { api.deleteSocial = { method: 'DELETE', url: '/user/auth/social/:network', - middlewares: [authWithHeaders], + middlewares: [authWithHeaders()], handler (req, res, next) { let user = res.locals.user; let network = req.params.network; diff --git a/website/src/middlewares/api-v3/auth.js b/website/src/middlewares/api-v3/auth.js index 562af57130..27604bc445 100644 --- a/website/src/middlewares/api-v3/auth.js +++ b/website/src/middlewares/api-v3/auth.js @@ -8,29 +8,33 @@ import { } from '../../models/user'; // Authenticate a request through the x-api-user and x-api key header -export function authWithHeaders (req, res, next) { - let userId = req.header['x-api-user']; - let apiToken = req.header['x-api-key']; +// If optional is true, don't error on missing authentication +export function authWithHeaders (optional = false) { + return function authWithHeadersHandler (req, res, next) { + let userId = req.header['x-api-user']; + let apiToken = req.header['x-api-key']; - if (!userId || !apiToken) { - return next(new BadRequest(res.t('missingAuthHeaders'))); + if (!userId || !apiToken) { + if (optional) return next(); + return next(new BadRequest(res.t('missingAuthHeaders'))); + } + + User.findOne({ + _id: userId, + apiToken, + }) + .exec() + .then((user) => { + if (!user) throw new NotAuthorized(i18n.t('invalidCredentials')); + if (user.auth.blocked) throw new NotAuthorized(i18n.t('accountSuspended', {userId: user._id})); + + res.locals.user = user; + // TODO use either session/cookie or headers, not both + req.session.userId = user._id; + next(); + }) + .catch(next); } - - User.findOne({ - _id: userId, - apiToken, - }) - .exec() - .then((user) => { - if (!user) throw new NotAuthorized(i18n.t('invalidCredentials')); - if (user.auth.blocked) throw new NotAuthorized(i18n.t('accountSuspended', {userId: user._id})); - - res.locals.user = user; - // TODO use either session/cookie or headers, not both - req.session.userId = user._id; - next(); - }) - .catch(next); } // Authenticate a request through a valid session diff --git a/website/src/middlewares/api-v3/errorHandler.js b/website/src/middlewares/api-v3/errorHandler.js index 616af12ffb..57280db12a 100644 --- a/website/src/middlewares/api-v3/errorHandler.js +++ b/website/src/middlewares/api-v3/errorHandler.js @@ -36,7 +36,7 @@ export default function errorHandler (err, req, res, next) { // Handle errors by express-validator if (Array.isArray(err) && err[0].param && err[0].msg) { - responseErr = new BadRequest('Invalid request parameters.'); + responseErr = new BadRequest(res.t('invalidReqParams')); responseErr.errors = err; // TODO format }