From bfdfcc9901c2b38e242af9f6a39aec05f43c312d Mon Sep 17 00:00:00 2001 From: Phillip Thelen Date: Wed, 17 Nov 2021 18:43:06 +0100 Subject: [PATCH] Fix issues and tests --- config.json.example | 4 +- .../user/auth/GET-user_auth_apple.test.js | 8 +- .../user/auth/POST-user_auth_social.test.js | 164 ++++++++++++++++-- .../auth/POST-user_reset_password.test.js | 13 ++ website/server/libs/auth/social.js | 13 +- 5 files changed, 179 insertions(+), 23 deletions(-) diff --git a/config.json.example b/config.json.example index 04c1c535b4..fbf18f1b82 100644 --- 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", diff --git 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 index 975fccfa3f..e5beba1fc0 100644 --- 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 @@ -9,9 +9,11 @@ describe('GET /user/auth/apple', () => { let api; let user; const appleEndpoint = '/user/auth/apple'; + let randomAppleId = '123456'; - before(async () => { - const expectedResult = { id: 'appleId', name: 'an apple user' }; + beforeEach(async () => { + randomAppleId = generateUUID(); + const expectedResult = { id: randomAppleId, name: 'an apple user' }; sandbox.stub(appleAuth, 'appleProfile').returns(Promise.resolve(expectedResult)); }); @@ -26,7 +28,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'); }); diff --git 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 index eff5525603..f6cbc7910d 100644 --- 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 @@ -5,19 +5,21 @@ import { translate as t, getProperty, } from '../../../../../helpers/api-integration/v3'; +import { v4 as generateUUID } from 'uuid'; 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,19 @@ 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 +60,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 +78,49 @@ 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 +129,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 +162,19 @@ 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 +184,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 +203,49 @@ 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 +254,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 diff --git 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 index 8603fa7cb3..036ce689e2 100644 --- 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` + user.auth.google.emails = [email]; + await user.sync(); + const previousPassword = user.auth.local.hashed_password; + const response = await user.post(endpoint, { + email: email, + }); + expect(response).to.eql({ data: {}, message: t('passwordReset') }); + await user.sync(); + expect(user.auth.local.hashed_password).to.not.eql(previousPassword); + }); + it('same message on error as on success', async () => { const response = await user.post(endpoint, { email: 'nonExistent@email.com', diff --git a/website/server/libs/auth/social.js b/website/server/libs/auth/social.js index 33d8282a7e..61883a2f87 100644 --- a/website/server/libs/auth/social.js +++ b/website/server/libs/auth/social.js @@ -58,12 +58,15 @@ export async function loginSocial (req, res) { // eslint-disable-line import/pre email = profile.emails[0].value.toLowerCase(); } - if (!existingUser) { - existingUser = await User.findOne({ 'auth.local.email': email }, { 'auth.local': 1 }).exec(); + if (!existingUser && email) { + existingUser = await User.findOne({ 'auth.local.email': email }).exec(); } if (existingUser) { - existingUser.auth[network] = user.auth[network]; + existingUser.auth[network] = { + id: profile.id, + emails: profile.emails, + }; user = existingUser; } else { const generatedUsername = generateUsername(); @@ -97,10 +100,10 @@ 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 (email) {