clean up code, add deleteSocial method

common/locales/en/api-v3.json

@@ -10,5 +10,7 @@
   "passwordConfirmationMatch": "Password confirmation doesn't match password.",
   "invalidLoginCredentials": "Incorrect username / email and / or password.",
   "invalidCredentials": "User not found with given auth credentials.",
-  "accountSuspended": "Account has been suspended, please contact leslie@habitica.com with your UUID \"<%= userId %>\" for assistance."
+  "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."
 }

test/api/v3/unit/middlewares/getUserLanguage.test.js

@@ -13,7 +13,7 @@ import accepts from 'accepts';
 describe('getUserLanguage', () => {
   let res, req, next;
 
-  let checkReqT = (req) => {
+  let checkResT = (req) => {
     expect(res.t).to.be.a('function');
     expect(res.t('help')).to.equal(i18n.t('help', req.language));
   };
@@ -32,7 +32,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, next);
       expect(req.language).to.equal('es');
-      checkReqT(req);
+      checkResT(req);
     });
 
     it('falls back to english if the query parameter language does not exists', () => {
@@ -42,7 +42,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, next);
       expect(req.language).to.equal('en');
-      checkReqT(req);
+      checkResT(req);
     });
 
     it('uses query even if the request includes a user and session', () => {
@@ -64,7 +64,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, next);
       expect(req.language).to.equal('es');
-      checkReqT(req);
+      checkResT(req);
     });
   });
 
@@ -80,7 +80,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, next);
       expect(req.language).to.equal('it');
-      checkReqT(req);
+      checkResT(req);
     });
 
     it('falls back to english if the user preferred language is not avalaible', (done) => {
@@ -94,7 +94,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('en');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -114,7 +114,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, next);
       expect(req.language).to.equal('it');
-      checkReqT(req);
+      checkResT(req);
     });
   });
 
@@ -136,7 +136,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('it');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -148,7 +148,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('pt');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -158,7 +158,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('he');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -168,7 +168,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('he');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -178,7 +178,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('fr');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -188,7 +188,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('fr');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -198,7 +198,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('es');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -208,7 +208,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('es_419');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -218,7 +218,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('es_419');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -228,7 +228,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('zh_TW');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -238,7 +238,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('en');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -248,7 +248,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('en');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });
@@ -258,7 +258,7 @@ describe('getUserLanguage', () => {
 
       getUserLanguage(req, res, () => {
         expect(req.language).to.equal('en');
-        checkReqT(req);
+        checkResT(req);
         done();
       });
     });

website/src/controllers/api-v3/user.js

@@ -1,5 +1,6 @@
 import validator from 'validator';
 import passport from 'passport';
+import { authWithHeaders } from '../../middlewares/api-v3/auth';
 import {
   NotAuthorized,
 } from '../../libs/api-v3/errors';
@@ -11,7 +12,7 @@ import { sendTxn as sendTxnEmail } from '../../libs/api-v3/email';
 let api = {};
 
 /**
- * @api {post} /user/register/local Register a new user with email, username and password
+ * @api {post} /user/auth/local/register Register a new user with email, username and password or add local authentication to a social user
  * @apiVersion 3.0.0
  * @apiName UserRegisterLocal
  * @apiGroup User
@@ -25,7 +26,7 @@ let api = {};
  */
 api.registerLocal = {
   method: 'POST',
-  url: '/user/register/local',
+  url: '/user/auth/local/register',
   handler (req, res, next) {
     let email = req.body.email.toLowerCase();
     let username = req.body.username;
@@ -85,8 +86,13 @@ api.registerLocal = {
   },
 };
 
+function _loginRes (user, req, res, next) {
+  if (user.auth.blocked) return next(new NotAuthorized(res.t('accountSuspended', {userId: user._id})));
+  res.status(200).json({id: user._id, apiToken: user.apiToken});
+}
+
 /**
- * @api {post} /user/login/local Login an user with email / username and password
+ * @api {post} /user/auth/local/login Login an user with email / username and password
  * @apiVersion 3.0.0
  * @apiName UserLoginLocal
  * @apiGroup User
@@ -99,7 +105,7 @@ api.registerLocal = {
  */
 api.loginLocal = {
   method: 'POST',
-  url: '/user/login/local',
+  url: '/user/auth/local/login',
   handler (req, res, next) {
     req.checkBody({
       username: {
@@ -134,9 +140,8 @@ api.loginLocal = {
       // TODO place back long error message return res.json(401, {err:"Uh-oh - your username or password is incorrect.\n- Make sure your username or email is typed correctly.\n- You may have signed up with Facebook, not email. Double-check by trying Facebook login.\n- If you forgot your password, click \"Forgot Password\"."});
       let isValidPassword = user && user.auth.local.hashed_password !== passwordUtils.encrypt(req.body.password, user.auth.local.salt);
 
-      if (user.auth.blocked) return next(new NotAuthorized(res.t('accountSuspended', {userId: user._id})));
       if (!isValidPassword) return next(new NotAuthorized(res.t('invalidLoginCredentials')));
-      res.status(200).json({id: user._id, apiToken: user.apiToken});
+      _loginRes(user, ...arguments);
     })
     .catch(next);
   },
@@ -145,28 +150,23 @@ api.loginLocal = {
 // Called as a callback by Facebook (or other social providers)
 api.loginSocial = {
   method: 'POST',
-  url: '/user/auth/social',
+  url: '/user/auth/social', // this isn't the most appropriate url but must be the same as v2
   handler (req, res, next) {
     let accessToken = req.body.authResponse.access_token;
     let network = req.body.network;
 
-    if (network !== 'facebook') return next(new NotAuthorized('Only Facebook supported currently.'));
+    if (network !== 'facebook') return next(new NotAuthorized(res.t('onlyFbSupported')));
 
     passport._strategies[network].userProfile(accessToken, (err, profile) => {
       if (err) return next(err);
 
-      function _respond (user) {
-        if (user.auth.blocked) return next(new NotAuthorized(res.t('accountSuspended', {userId: user._id})));
-        return res.status(200).json({_id: user._id, apiToken: user.apiToken});
-      }
-
       User.findOne({
         [`auth.${network}.id`]: profile.id,
       }, {_id: 1, apiToken: 1, auth: 1}).exec()
       .then((user) => {
         // User already signed up
         if (user) {
-          return _respond(user);
+          return _loginRes(user, ...arguments);
         } else { // Create new user
           user = new User({
             auth: {
@@ -180,7 +180,7 @@ api.loginSocial = {
 
           user.save()
           .then((savedUser) => {
-            _respond(savedUser);
+            _loginRes(user, ...arguments);
 
             // Clean previous email preferences
             if (savedUser.auth[network].emails && savedUser.auth.facebook.emails[0] && savedUser.auth[network].emails[0].value) {
@@ -204,13 +204,30 @@ api.loginSocial = {
   },
 };
 
-/* api.attachSocial = {
+/**
-
+ * @api {delete} /user/auth/social/:network Delete a social authentication method (only facebook supported)
-};
+ * @apiVersion 3.0.0
-
+ * @apiName UserDeleteSocial
+ * @apiGroup User
+ *
+ * @apiSuccess {Boolean=true} success Always true
+ */
 api.deleteSocial = {
+  method: 'DELETE',
+  url: '/user/auth/social/:network',
+  middlewares: [authWithHeaders],
+  handler (req, res, next) {
+    let user = res.locals.user;
+    let network = req.params.network;
 
-};*/
+    if (network !== 'facebook') return next(new NotAuthorized(res.t('onlyFbSupported')));
+    if (!user.auth.local.username) return next(new NotAuthorized(res.t('cantDetachFb'))); // TODO move to model validation?
+
+    User.update({_id: user._id}, {$unset: {'auth.facebook': 1}})
+    .then(() => res.status(200).json({ok: true})) // TODO standardize this type of response
+    .catch(next);
+  },
+};
 
 
 export default api;