From 24841346dc04f915b72a9eec334ba7010f00c03d Mon Sep 17 00:00:00 2001 From: Phillip Thelen Date: Fri, 16 Sep 2022 01:22:52 +0200 Subject: [PATCH 01/12] Purge Facebook (#13696) * 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. * purge Facebook. Only keep it in some select places to allow for some compatablilty. * Fix error * fix error * Let settings handle it when you don't have a password set but an email * fix error * Fix boolean logic * fix json conversion * . * fix password reset for old social accounts * 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. * purge Facebook. Only keep it in some select places to allow for some compatablilty. * Fix error * fix error * Let settings handle it when you don't have a password set but an email * fix error * Fix boolean logic * fix json conversion * fix password reset for old social accounts * Revert "lint fixes" This reverts commit c244b1651c2c23050d058fe15381518231d8a317. # Conflicts: # website/client/src/components/auth/registerLoginReset.vue # website/client/src/components/static/contact.vue * Revert "fix password reset for old social accounts" This reverts commit 7e0069a80fd20bc54d13cec4f83e072ad20d6610. * fix duplicate code * chore(misc): remove irrelevant changes * chore(privacy): update policy page with note about FB Co-authored-by: SabreCat --- test/api/unit/libs/email.test.js | 25 ---- test/api/unit/libs/password.test.js | 2 +- .../v3/integration/user/DELETE-user.test.js | 39 ----- .../DELETE-user_auth_social_network.test.js | 64 -------- .../user/auth/POST-user_auth_social.test.js | 141 ------------------ .../auth/POST-user_reset_password.test.js | 13 ++ .../client/src/components/auth/authForm.vue | 20 --- .../components/auth/registerLoginReset.vue | 4 - .../client/src/components/settings/site.vue | 4 +- .../client/src/components/static/privacy.vue | 7 +- website/client/vue.config.js | 1 - website/common/script/constants.js | 1 - website/server/libs/auth/index.js | 7 +- website/server/libs/auth/social.js | 10 +- website/server/libs/setupPassport.js | 15 -- 15 files changed, 29 insertions(+), 324 deletions(-) diff --git a/test/api/unit/libs/email.test.js b/test/api/unit/libs/email.test.js index b3fe180086..8673e03b62 100644 --- a/test/api/unit/libs/email.test.js +++ b/test/api/unit/libs/email.test.js @@ -13,11 +13,6 @@ function getUser () { username: 'username', email: 'email@email', }, - facebook: { - emails: [{ - value: 'email@facebook', - }], - }, google: { emails: [{ value: 'email@google', @@ -62,30 +57,12 @@ describe('emails', () => { expect(data).to.have.property('canSend', true); }); - it('returns correct user data [facebook users]', () => { - const attachEmail = requireAgain(pathToEmailLib); - const { getUserInfo } = attachEmail; - const user = getUser(); - delete user.profile.name; - delete user.auth.local.email; - delete user.auth.google.emails; - delete user.auth.apple.emails; - - const data = getUserInfo(user, ['name', 'email', '_id', 'canSend']); - - expect(data).to.have.property('name', user.auth.local.username); - expect(data).to.have.property('email', user.auth.facebook.emails[0].value); - expect(data).to.have.property('_id', user._id); - expect(data).to.have.property('canSend', true); - }); - it('returns correct user data [google users]', () => { const attachEmail = requireAgain(pathToEmailLib); const { getUserInfo } = attachEmail; const user = getUser(); delete user.profile.name; delete user.auth.local.email; - delete user.auth.facebook.emails; delete user.auth.apple.emails; const data = getUserInfo(user, ['name', 'email', '_id', 'canSend']); @@ -103,7 +80,6 @@ describe('emails', () => { delete user.profile.name; delete user.auth.local.email; delete user.auth.google.emails; - delete user.auth.facebook.emails; const data = getUserInfo(user, ['name', 'email', '_id', 'canSend']); @@ -118,7 +94,6 @@ describe('emails', () => { const { getUserInfo } = attachEmail; const user = getUser(); delete user.auth.local.email; - delete user.auth.facebook; delete user.auth.google; delete user.auth.apple; diff --git a/test/api/unit/libs/password.test.js b/test/api/unit/libs/password.test.js index 3ca2643b6d..d9f1572ab5 100644 --- a/test/api/unit/libs/password.test.js +++ b/test/api/unit/libs/password.test.js @@ -246,7 +246,7 @@ describe('Password Utilities', () => { it('returns false if the user has no local auth', async () => { const user = await generateUser({ auth: { - facebook: {}, + google: {}, }, }); const res = await validatePasswordResetCodeAndFindUser(encrypt(JSON.stringify({ diff --git a/test/api/v3/integration/user/DELETE-user.test.js b/test/api/v3/integration/user/DELETE-user.test.js index 18be9792ce..5c176fe141 100644 --- a/test/api/v3/integration/user/DELETE-user.test.js +++ b/test/api/v3/integration/user/DELETE-user.test.js @@ -289,45 +289,6 @@ describe('DELETE /user', () => { }); }); - context('user with Facebook auth', async () => { - beforeEach(async () => { - user = await generateUser({ - auth: { - facebook: { - id: 'facebook-id', - }, - }, - }); - }); - - it('returns an error if confirmation phrase is wrong', async () => { - await expect(user.del('/user', { - password: 'just-do-it', - })).to.eventually.be.rejected.and.eql({ - code: 401, - error: 'NotAuthorized', - message: t('incorrectDeletePhrase', { magicWord: 'DELETE' }), - }); - }); - - it('returns an error if confirmation phrase is not supplied', async () => { - await expect(user.del('/user', { - password: '', - })).to.eventually.be.rejected.and.eql({ - code: 400, - error: 'BadRequest', - message: t('missingPassword'), - }); - }); - - it('deletes a Facebook user', async () => { - await user.del('/user', { - password: DELETE_CONFIRMATION, - }); - await expect(checkExistence('users', user._id)).to.eventually.eql(false); - }); - }); - context('user with Google auth', async () => { beforeEach(async () => { user = await generateUser({ diff --git a/test/api/v3/integration/user/auth/DELETE-user_auth_social_network.test.js b/test/api/v3/integration/user/auth/DELETE-user_auth_social_network.test.js index 12bef95459..522a49d369 100644 --- a/test/api/v3/integration/user/auth/DELETE-user_auth_social_network.test.js +++ b/test/api/v3/integration/user/auth/DELETE-user_auth_social_network.test.js @@ -20,44 +20,6 @@ describe('DELETE social registration', () => { }); }); - context('Facebook', () => { - it('fails if user does not have an alternative registration method', async () => { - await user.update({ - 'auth.facebook.id': 'some-fb-id', - 'auth.local': { ok: true }, - }); - await expect(user.del('/user/auth/social/facebook')).to.eventually.be.rejected.and.eql({ - code: 401, - error: 'NotAuthorized', - message: t('cantDetachSocial'), - }); - }); - - it('succeeds if user has a local registration', async () => { - await user.update({ - 'auth.facebook.id': 'some-fb-id', - }); - - const response = await user.del('/user/auth/social/facebook'); - expect(response).to.eql({}); - await user.sync(); - expect(user.auth.facebook).to.be.undefined; - }); - - it('succeeds if user has a google registration', async () => { - await user.update({ - 'auth.facebook.id': 'some-fb-id', - 'auth.google.id': 'some-google-id', - 'auth.local': { ok: true }, - }); - - const response = await user.del('/user/auth/social/facebook'); - expect(response).to.eql({}); - await user.sync(); - expect(user.auth.facebook).to.be.undefined; - }); - }); - context('Google', () => { it('fails if user does not have an alternative registration method', async () => { await user.update({ @@ -81,19 +43,6 @@ describe('DELETE social registration', () => { await user.sync(); expect(user.auth.google).to.be.undefined; }); - - it('succeeds if user has a facebook registration', async () => { - await user.update({ - 'auth.google.id': 'some-google-id', - 'auth.facebook.id': 'some-facebook-id', - 'auth.local': { ok: true }, - }); - - const response = await user.del('/user/auth/social/google'); - expect(response).to.eql({}); - await user.sync(); - expect(user.auth.goodl).to.be.undefined; - }); }); context('Apple', () => { @@ -119,18 +68,5 @@ describe('DELETE social registration', () => { await user.sync(); expect(user.auth.apple).to.be.undefined; }); - - it('succeeds if user has a facebook registration', async () => { - await user.update({ - 'auth.apple.id': 'some-apple-id', - 'auth.facebook.id': 'some-facebook-id', - 'auth.local': { ok: true }, - }); - - const response = await user.del('/user/auth/social/apple'); - expect(response).to.eql({}); - await user.sync(); - expect(user.auth.goodl).to.be.undefined; - }); }); }); 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 c4ffe57efb..df976d0987 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 @@ -12,7 +12,6 @@ describe('POST /user/auth/social', () => { let user; const endpoint = '/user/auth/social'; let randomAccessToken = '123456'; - let randomFacebookId = 'facebookId'; let randomGoogleId = 'googleId'; let network = 'NoNetwork'; @@ -33,146 +32,6 @@ describe('POST /user/auth/social', () => { }); }); - describe('facebook', () => { - 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 - network, - }); - - expect(response.apiToken).to.exist; - expect(response.id).to.exist; - expect(response.newUser).to.be.true; - expect(response.username).to.exist; - - 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.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 () => { - const registerResponse = await api.post(endpoint, { - authResponse: { access_token: randomAccessToken }, // eslint-disable-line camelcase - network, - }); - - 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.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 () => { - const response = await user.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('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 - network, - }); - - await expect(getProperty('users', user._id, '_ABtests')).to.eventually.be.a('object'); - }); - }); - describe('google', () => { beforeEach(async () => { randomGoogleId = generateUUID(); 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..da66e5fd3e 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`; + 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', diff --git a/website/client/src/components/auth/authForm.vue b/website/client/src/components/auth/authForm.vue index e24951f496..6354f6d160 100644 --- a/website/client/src/components/auth/authForm.vue +++ b/website/client/src/components/auth/authForm.vue @@ -1,21 +1,5 @@