Prerequisites to removing Facebook authentication (#13683)

* Don't sign in user when trying to connect a social account that was already created * Log social users into matching local auth accounts If the social account has an email that already exists as a local user, instead of creating a new account log them into their account and add the social auth to the account * If possible set local authentication email for social users * Allow password reset emails to be sent to social login users * lint fixes * Fix issues and tests * fix tests * Fix lint error.
2025-12-14 05:07:22 +01:00 · 2022-01-21 22:15:58 +01:00
parent d11810677c
commit 1177ad8b8c
12 changed files with 288 additions and 72 deletions
--- a/config.json.example
+++ b/config.json.example
@@ -32,8 +32,8 @@
  "LOGGLY_SUBDOMAIN": "example-subdomain",
  "LOGGLY_TOKEN": "example-token",
  "MAINTENANCE_MODE": "false",
-  "NODE_DB_URI": "mongodb://localhost:27017/habitica-dev?replicaSet=rs",
-  "TEST_DB_URI": "mongodb://localhost:27017/habitica-test?replicaSet=rs",
+  "NODE_DB_URI": "mongodb://localhost:27017/habitica-dev",
+  "TEST_DB_URI": "mongodb://localhost:27017/habitica-test",
  "MONGODB_POOL_SIZE": "10",
  "NODE_ENV": "development",
  "PATH": "bin:node_modules/.bin:/usr/local/bin:/usr/bin:/bin",
--- a/test/api/v3/integration/user/auth/GET-user_auth_apple.test.js
+++ b/test/api/v3/integration/user/auth/GET-user_auth_apple.test.js
@@ -1,3 +1,4 @@
+import { v4 as generateUUID } from 'uuid';
 import {
  generateUser,
  requester,
@@ -9,15 +10,18 @@ describe('GET /user/auth/apple', () => {
  let api;
  let user;
  const appleEndpoint = '/user/auth/apple';
-
-  before(async () => {
-    const expectedResult = { id: 'appleId', name: 'an apple user' };
-    sandbox.stub(appleAuth, 'appleProfile').returns(Promise.resolve(expectedResult));
-  });
+  let randomAppleId = '123456';

  beforeEach(async () => {
    api = requester();
    user = await generateUser();
+    randomAppleId = generateUUID();
+    const expectedResult = { id: randomAppleId, name: 'an apple user' };
+    sandbox.stub(appleAuth, 'appleProfile').returns(Promise.resolve(expectedResult));
+  });
+
+  afterEach(async () => {
+    appleAuth.appleProfile.restore();
  });

  it('registers a new user', async () => {
@@ -26,7 +30,7 @@ describe('GET /user/auth/apple', () => {
    expect(response.apiToken).to.exist;
    expect(response.id).to.exist;
    expect(response.newUser).to.be.true;
-    await expect(getProperty('users', response.id, 'auth.apple.id')).to.eventually.equal('appleId');
+    await expect(getProperty('users', response.id, 'auth.apple.id')).to.eventually.equal(randomAppleId);
    await expect(getProperty('users', response.id, 'profile.name')).to.eventually.equal('an apple user');
  });

--- a/test/api/v3/integration/user/auth/POST-user_auth_social.test.js
+++ b/test/api/v3/integration/user/auth/POST-user_auth_social.test.js
@@ -1,4 +1,5 @@
 import passport from 'passport';
+import { v4 as generateUUID } from 'uuid';
 import {
  generateUser,
  requester,
@@ -10,14 +11,15 @@ describe('POST /user/auth/social', () => {
  let api;
  let user;
  const endpoint = '/user/auth/social';
-  const randomAccessToken = '123456';
-  const facebookId = 'facebookId';
-  const googleId = 'googleId';
+  let randomAccessToken = '123456';
+  let randomFacebookId = 'facebookId';
+  let randomGoogleId = 'googleId';
  let network = 'NoNetwork';

  beforeEach(async () => {
    api = requester();
    user = await generateUser();
+    randomAccessToken = generateUUID();
  });

  it('fails if network is not supported', async () => {
@@ -32,12 +34,23 @@ describe('POST /user/auth/social', () => {
  });

  describe('facebook', () => {
-    before(async () => {
-      const expectedResult = { id: facebookId, displayName: 'a facebook user' };
+    beforeEach(async () => {
+      randomFacebookId = generateUUID();
+      const expectedResult = {
+        id: randomFacebookId,
+        displayName: 'a facebook user',
+        emails: [
+          { value: `${user.auth.local.username}+facebook@example.com` },
+        ],
+      };
      sandbox.stub(passport._strategies.facebook, 'userProfile').yields(null, expectedResult);
      network = 'facebook';
    });

+    afterEach(async () => {
+      passport._strategies.facebook.userProfile.restore();
+    });
+
    it('registers a new user', async () => {
      const response = await api.post(endpoint, {
        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
@@ -51,7 +64,8 @@ describe('POST /user/auth/social', () => {

      await expect(getProperty('users', response.id, 'profile.name')).to.eventually.equal('a facebook user');
      await expect(getProperty('users', response.id, 'auth.local.lowerCaseUsername')).to.exist;
-      await expect(getProperty('users', response.id, 'auth.facebook.id')).to.eventually.equal(facebookId);
+      await expect(getProperty('users', response.id, 'auth.local.email')).to.eventually.equal(`${user.auth.local.username}+facebook@example.com`);
+      await expect(getProperty('users', response.id, 'auth.facebook.id')).to.eventually.equal(randomFacebookId);
    });

    it('logs an existing user in', async () => {
@@ -68,6 +82,57 @@ describe('POST /user/auth/social', () => {
      expect(response.apiToken).to.eql(registerResponse.apiToken);
      expect(response.id).to.eql(registerResponse.id);
      expect(response.newUser).to.be.false;
+      expect(registerResponse.newUser).to.be.true;
+    });
+
+    it('logs an existing user in if they have local auth with matching email', async () => {
+      passport._strategies.facebook.userProfile.restore();
+      const expectedResult = {
+        id: randomFacebookId,
+        displayName: 'a facebook user',
+        emails: [
+          { value: user.auth.local.email },
+        ],
+      };
+      sandbox.stub(passport._strategies.facebook, 'userProfile').yields(null, expectedResult);
+
+      const response = await api.post(endpoint, {
+        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
+        network,
+      });
+
+      expect(response.apiToken).to.eql(user.apiToken);
+      expect(response.id).to.eql(user._id);
+      expect(response.newUser).to.be.false;
+    });
+
+    it('logs an existing user into their social account if they have local auth with matching email', async () => {
+      const registerResponse = await api.post(endpoint, {
+        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
+        network,
+      });
+      expect(registerResponse.newUser).to.be.true;
+      // This is important for existing accounts before the new social handling
+      passport._strategies.facebook.userProfile.restore();
+      const expectedResult = {
+        id: randomFacebookId,
+        displayName: 'a facebook user',
+        emails: [
+          { value: user.auth.local.email },
+        ],
+      };
+      sandbox.stub(passport._strategies.facebook, 'userProfile').yields(null, expectedResult);
+
+      const response = await api.post(endpoint, {
+        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
+        network,
+      });
+
+      expect(response.apiToken).to.eql(registerResponse.apiToken);
+      expect(response.id).to.eql(registerResponse.id);
+      expect(response.apiToken).not.to.eql(user.apiToken);
+      expect(response.id).not.to.eql(user._id);
+      expect(response.newUser).to.be.false;
    });

    it('add social auth to an existing user', async () => {
@@ -76,11 +141,28 @@ describe('POST /user/auth/social', () => {
        network,
      });

-      expect(response.apiToken).to.exist;
-      expect(response.id).to.exist;
+      expect(response.apiToken).to.eql(user.apiToken);
+      expect(response.id).to.eql(user._id);
      expect(response.newUser).to.be.false;
    });

+    it('does not log into other account if social auth already exists', async () => {
+      const registerResponse = await api.post(endpoint, {
+        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
+        network,
+      });
+      expect(registerResponse.newUser).to.be.true;
+
+      await expect(user.post(endpoint, {
+        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
+        network,
+      })).to.eventually.be.rejected.and.eql({
+        code: 401,
+        error: 'NotAuthorized',
+        message: t('socialAlreadyExists'),
+      });
+    });
+
    xit('enrolls a new user in an A/B test', async () => {
      await api.post(endpoint, {
        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
@@ -92,12 +174,23 @@ describe('POST /user/auth/social', () => {
  });

  describe('google', () => {
-    before(async () => {
-      const expectedResult = { id: googleId, displayName: 'a google user' };
+    beforeEach(async () => {
+      randomGoogleId = generateUUID();
+      const expectedResult = {
+        id: randomGoogleId,
+        displayName: 'a google user',
+        emails: [
+          { value: `${user.auth.local.username}+google@example.com` },
+        ],
+      };
      sandbox.stub(passport._strategies.google, 'userProfile').yields(null, expectedResult);
      network = 'google';
    });

+    afterEach(async () => {
+      passport._strategies.google.userProfile.restore();
+    });
+
    it('registers a new user', async () => {
      const response = await api.post(endpoint, {
        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
@@ -107,7 +200,8 @@ describe('POST /user/auth/social', () => {
      expect(response.apiToken).to.exist;
      expect(response.id).to.exist;
      expect(response.newUser).to.be.true;
-      await expect(getProperty('users', response.id, 'auth.google.id')).to.eventually.equal(googleId);
+      await expect(getProperty('users', response.id, 'auth.google.id')).to.eventually.equal(randomGoogleId);
+      await expect(getProperty('users', response.id, 'auth.local.email')).to.eventually.equal(`${user.auth.local.username}+google@example.com`);
      await expect(getProperty('users', response.id, 'profile.name')).to.eventually.equal('a google user');
    });

@@ -125,6 +219,57 @@ describe('POST /user/auth/social', () => {
      expect(response.apiToken).to.eql(registerResponse.apiToken);
      expect(response.id).to.eql(registerResponse.id);
      expect(response.newUser).to.be.false;
+      expect(registerResponse.newUser).to.be.true;
+    });
+
+    it('logs an existing user in if they have local auth with matching email', async () => {
+      passport._strategies.google.userProfile.restore();
+      const expectedResult = {
+        id: randomGoogleId,
+        displayName: 'a google user',
+        emails: [
+          { value: user.auth.local.email },
+        ],
+      };
+      sandbox.stub(passport._strategies.google, 'userProfile').yields(null, expectedResult);
+
+      const response = await api.post(endpoint, {
+        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
+        network,
+      });
+
+      expect(response.apiToken).to.eql(user.apiToken);
+      expect(response.id).to.eql(user._id);
+      expect(response.newUser).to.be.false;
+    });
+
+    it('logs an existing user into their social account if they have local auth with matching email', async () => {
+      const registerResponse = await api.post(endpoint, {
+        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
+        network,
+      });
+      expect(registerResponse.newUser).to.be.true;
+      // This is important for existing accounts before the new social handling
+      passport._strategies.google.userProfile.restore();
+      const expectedResult = {
+        id: randomGoogleId,
+        displayName: 'a google user',
+        emails: [
+          { value: user.auth.local.email },
+        ],
+      };
+      sandbox.stub(passport._strategies.google, 'userProfile').yields(null, expectedResult);
+
+      const response = await api.post(endpoint, {
+        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
+        network,
+      });
+
+      expect(response.apiToken).to.eql(registerResponse.apiToken);
+      expect(response.id).to.eql(registerResponse.id);
+      expect(response.apiToken).not.to.eql(user.apiToken);
+      expect(response.id).not.to.eql(user._id);
+      expect(response.newUser).to.be.false;
    });

    it('add social auth to an existing user', async () => {
@@ -133,11 +278,28 @@ describe('POST /user/auth/social', () => {
        network,
      });

-      expect(response.apiToken).to.exist;
-      expect(response.id).to.exist;
+      expect(response.apiToken).to.eql(user.apiToken);
+      expect(response.id).to.eql(user._id);
      expect(response.newUser).to.be.false;
    });

+    it('does not log into other account if social auth already exists', async () => {
+      const registerResponse = await api.post(endpoint, {
+        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
+        network,
+      });
+      expect(registerResponse.newUser).to.be.true;
+
+      await expect(user.post(endpoint, {
+        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
+        network,
+      })).to.eventually.be.rejected.and.eql({
+        code: 401,
+        error: 'NotAuthorized',
+        message: t('socialAlreadyExists'),
+      });
+    });
+
    xit('enrolls a new user in an A/B test', async () => {
      await api.post(endpoint, {
        authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase
--- a/test/api/v3/integration/user/auth/POST-user_reset_password.test.js
+++ b/test/api/v3/integration/user/auth/POST-user_reset_password.test.js
@@ -25,6 +25,19 @@ describe('POST /user/reset-password', async () => {
    expect(user.auth.local.hashed_password).to.not.eql(previousPassword);
  });

+  it('resets password for social users', async () => {
+    const email = `${user.auth.local.username}+google@example.com`;
+    await user.update({ 'auth.google.emails': [{ value: email }] });
+    await user.sync();
+    const previousPassword = user.auth.local.passwordResetCode;
+    const response = await user.post(endpoint, {
+      email,
+    });
+    expect(response).to.eql({ data: {}, message: t('passwordReset') });
+    await user.sync();
+    expect(user.auth.local.passwordResetCode).to.not.eql(previousPassword);
+  });
+
  it('same message on error as on success', async () => {
    const response = await user.post(endpoint, {
      email: 'nonExistent@email.com',
--- a/website/client/src/components/auth/registerLoginReset.vue
+++ b/website/client/src/components/auth/registerLoginReset.vue
@@ -20,8 +20,8 @@
        </div>
      </div>
      <div
-        class="form-group row text-center"
        v-if="!registering"
+        class="form-group row text-center"
      >
        <div class="col-12 col-md-12">
          <div
--- a/website/client/src/components/header/notifications/worldBoss.vue
+++ b/website/client/src/components/header/notifications/worldBoss.vue
@@ -178,12 +178,12 @@ import sword from '@/assets/svg/sword.svg';
 import { worldStateMixin } from '@/mixins/worldState';

 export default {
-  mixins: [
-    worldStateMixin,
-  ],
  components: {
    BaseNotification,
  },
+  mixins: [
+    worldStateMixin,
+  ],
  data () {
    const questData = quests.quests.dysheartener;

--- a/website/client/src/components/inventory/items/index.vue
+++ b/website/client/src/components/inventory/items/index.vue
@@ -194,7 +194,10 @@
                  <h4 class="popover-content-title">
                    {{ context.item.text }}
                  </h4>
-                  <questInfo :quest="context.item" :purchased="true" />
+                  <questInfo
+                    :quest="context.item"
+                    :purchased="true"
+                  />
                </div>
                <div v-else>
                  <h4 class="popover-content-title">
--- a/website/client/src/components/snackbars/notification.vue
+++ b/website/client/src/components/snackbars/notification.vue
@@ -1,10 +1,17 @@
 <template>
  <div class="notification-animation-holder">
-    <div class="notification-holder"
-         @click="handleOnClick()">
-      <div v-if="notification.type === 'drop'"
-           class="icon-item">
-        <div :class="notification.icon" class="icon-negative-margin"></div>
+    <div
+      class="notification-holder"
+      @click="handleOnClick()"
+    >
+      <div
+        v-if="notification.type === 'drop'"
+        class="icon-item"
+      >
+        <div
+          :class="notification.icon"
+          class="icon-negative-margin"
+        ></div>
      </div>

      <div
@@ -32,7 +39,10 @@
              class="svg-icon"
              v-html="icons.gold"
            ></div>
-            <div class="icon-text" v-html="notification.text"></div>
+            <div
+              class="icon-text"
+              v-html="notification.text"
+            ></div>
          </div>
        </div>
        <div
@@ -63,7 +73,10 @@
              class="svg-icon"
              v-html="icons.mana"
            ></div>
-            <div class="icon-text" v-html="notification.text"></div>
+            <div
+              class="icon-text"
+              v-html="notification.text"
+            ></div>
          </div>
        </div>
        <div
@@ -78,7 +91,10 @@
              class="svg-icon"
              v-html="icons.sword"
            ></div>
-            <div class="icon-text" v-html="notification.text"></div>
+            <div
+              class="icon-text"
+              v-html="notification.text"
+            ></div>
          </div>
        </div>
        <div
@@ -98,7 +114,6 @@
          </div>
        </div>
      </div>
-
    </div>
  </div>
 </template>
--- a/website/client/src/components/snackbars/notifications.vue
+++ b/website/client/src/components/snackbars/notifications.vue
@@ -81,12 +81,12 @@ const DELAY_DELETE_AND_NEW = 60;
 const DELAY_FILLING_ENTRIES = 240;

 export default {
-  mixins: [
-    worldStateMixin,
-  ],
  components: {
    notification,
  },
+  mixins: [
+    worldStateMixin,
+  ],
  props: {
    preventQueue: {
      type: Boolean,
--- a/website/common/locales/en/front.json
+++ b/website/common/locales/en/front.json
@@ -133,6 +133,7 @@
  "unsupportedNetwork": "This network is not currently supported.",
  "cantDetachSocial": "Account lacks another authentication method; can't detach this authentication method.",
  "onlySocialAttachLocal": "Local authentication can be added to only a social account.",
+  "socialAlreadyExists": "This social login is already linked to an existing Habitica account.",
  "invalidReqParams": "Invalid request parameters.",
  "memberIdRequired": "\"member\" must be a valid UUID.",
  "heroIdRequired": "\"heroId\" must be a valid UUID.",
--- a/website/server/controllers/api-v3/auth.js
+++ b/website/server/controllers/api-v3/auth.js
@@ -341,7 +341,14 @@ api.resetPassword = {
    if (validationErrors) throw validationErrors;

    const email = req.body.email.toLowerCase();
-    const user = await User.findOne({ 'auth.local.email': email }).exec();
+    const user = await User.findOne({
+      $or: [
+        { 'auth.local.email': email },
+        { 'auth.apple.emails.value': email },
+        { 'auth.google.emails.value': email },
+        { 'auth.facebook.emails.value': email },
+      ],
+    }).exec();

    if (user) {
      // create an encrypted link to be used to reset the password
--- a/website/server/libs/auth/social.js
+++ b/website/server/libs/auth/social.js
@@ -1,6 +1,6 @@
 import passport from 'passport';
 import common from '../../../common';
-import { BadRequest } from '../errors';
+import { BadRequest, NotAuthorized } from '../errors';
 import logger from '../logger';
 import {
  generateUsername,
@@ -24,7 +24,7 @@ function _passportProfile (network, accessToken) {
 }

 export async function loginSocial (req, res) { // eslint-disable-line import/prefer-default-export
-  const existingUser = res.locals.user;
+  let existingUser = res.locals.user;
  const { network } = req.body;

  const isSupportedNetwork = common.constants.SUPPORTED_SOCIAL_NETWORKS
@@ -47,9 +47,28 @@ export async function loginSocial (req, res) { // eslint-disable-line import/pre

  // User already signed up
  if (user) {
+    if (existingUser) {
+      throw new NotAuthorized(res.t('socialAlreadyExists'));
+    }
    return loginRes(user, req, res);
  }

+  let email;
+  if (profile.emails && profile.emails[0] && profile.emails[0].value) {
+    email = profile.emails[0].value.toLowerCase();
+  }
+
+  if (!existingUser && email) {
+    existingUser = await User.findOne({ 'auth.local.email': email }).exec();
+  }
+
+  if (existingUser) {
+    existingUser.auth[network] = {
+      id: profile.id,
+      emails: profile.emails,
+    };
+    user = existingUser;
+  } else {
    const generatedUsername = generateUsername();

    user = {
@@ -61,6 +80,7 @@ export async function loginSocial (req, res) { // eslint-disable-line import/pre
        local: {
          username: generatedUsername,
          lowerCaseUsername: generatedUsername,
+          email,
        },
      },
      profile: {
@@ -73,11 +93,6 @@ export async function loginSocial (req, res) { // eslint-disable-line import/pre
        verifiedUsername: true,
      },
    };
-
-  if (existingUser) {
-    existingUser.auth[network] = user.auth[network];
-    user = existingUser;
-  } else {
    user = new User(user);
    user.registeredThrough = req.headers['x-client']; // Not saved, used to create the correct tasks based on the device used
  }
@@ -85,19 +100,15 @@ export async function loginSocial (req, res) { // eslint-disable-line import/pre
  const savedUser = await user.save();

  if (!existingUser) {
-    user.newUser = true;
+    savedUser.newUser = true;
  }

-  const response = loginRes(user, req, res);
+  const response = loginRes(savedUser, req, res);

  // Clean previous email preferences
-  if (
-    savedUser.auth[network].emails
-    && savedUser.auth[network].emails[0]
-    && savedUser.auth[network].emails[0].value
-  ) {
+  if (email) {
    EmailUnsubscription
-      .remove({ email: savedUser.auth[network].emails[0].value.toLowerCase() })
+      .remove({ email })
      .exec()
      .then(() => {
        if (!existingUser) {