From d2ba8e223c71fa770344a73e1e182ce0f989b4ba Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Fri, 26 Feb 2016 19:18:18 +0100 Subject: [PATCH 1/5] remove session party invitation. Implement the same feature using a query string when signing up --- website/src/controllers/api-v3/auth.js | 48 +++++++++++++++++++++--- website/src/controllers/api-v3/groups.js | 16 ++++---- website/src/middlewares/api-v3/locals.js | 11 ------ 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/website/src/controllers/api-v3/auth.js b/website/src/controllers/api-v3/auth.js index c66b254e9b..673a87927f 100644 --- a/website/src/controllers/api-v3/auth.js +++ b/website/src/controllers/api-v3/auth.js @@ -1,18 +1,50 @@ import validator from 'validator'; +import moment from 'moment'; import passport from 'passport'; import { authWithHeaders } from '../../middlewares/api-v3/auth'; import cron from '../../middlewares/api-v3/cron'; import { NotAuthorized, + NotFound, } from '../../libs/api-v3/errors'; import Q from 'q'; import * as passwordUtils from '../../libs/api-v3/password'; import { model as User } from '../../models/user'; +import { model as Group } from '../../models/group'; import { model as EmailUnsubscription } from '../../models/emailUnsubscription'; import { sendTxn as sendTxnEmail } from '../../libs/api-v3/email'; +import { decrypt } from '../../libs/api-v3/encryption'; + let api = {}; +// When the user signed up after having been invited to a group, invite them automatically to the group +async function _handleGroupInvitation (user, invite) { + // wrapping the code in a try because we don't want it to prevent the user from signing up + // that's why errors are not translated + try { + let {sentAt, id: groupId, inviter} = JSON.parse(decrypt(invite)); + + // check that the invite has not expired (after 7 days) + if (sentAt && moment().subtract(7, 'days').isAfter(sentAt)) { + let err = new Error('Invite expired'); + err.privateData = invite; + throw err; + } + + let group = await Group.getGroup({user, optionalMembership: true, groupId, fields: 'name type'}); + if (!group) throw new NotFound('Group not found.'); + + if (group.type === 'party') { + user.invitations.party = {id: group._id, name: group.name, inviter}; + } else { + user.invitations.guilds.push({id: group._id, name: group.name, inviter}); + } + } catch (err) { + // TODO log errors + } +} + /** * @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 @@ -84,25 +116,29 @@ api.registerLocal = { }, }; - let savedUser; - if (fbUser) { if (!fbUser.auth.facebook.id) throw new NotAuthorized(res.t('onlySocialAttachLocal')); fbUser.auth.local = newUser; - savedUser = await fbUser.save(); + newUser = fbUser; } else { newUser = new User(newUser); newUser.registeredThrough = req.headers['x-client']; // TODO is this saved somewhere? - savedUser = await newUser.save(); } + // we check for partyInvite for backward compatibility + if (req.query.groupInvite || req.query.partyInvite) { + await _handleGroupInvitation(newUser, req.query.groupInvite || req.query.partyInvite); + } + + let savedUser = await newUser.save(); + if (savedUser.auth.facebook.id) { - res.respond(200, savedUser.auth.local); // TODO make sure this used .toJSON and removes private fields + res.respond(200, savedUser.toJSON().auth.local); // We convert to toJSON to hide private fields } else { res.respond(201, savedUser); } - // Clean previous email preferences + // Clean previous email preferences and send welcome email EmailUnsubscription .remove({email: savedUser.auth.local.email}) .then(() => sendTxnEmail(savedUser, 'welcome')); diff --git a/website/src/controllers/api-v3/groups.js b/website/src/controllers/api-v3/groups.js index 05925d5730..a078092a7e 100644 --- a/website/src/controllers/api-v3/groups.js +++ b/website/src/controllers/api-v3/groups.js @@ -459,7 +459,6 @@ api.removeGroupMember = { }; async function _inviteByUUID (uuid, group, inviter, req, res) { - // TODO: Add Push Notifications let userToInvite = await User.findById(uuid).exec(); if (!userToInvite) { @@ -493,7 +492,6 @@ async function _inviteByUUID (uuid, group, inviter, req, res) { if (userToInvite.preferences.emailNotifications[`invited${groupLabel}`] !== false) { let emailVars = [ {name: 'INVITER', content: inviter.profile.name}, - {name: 'REPLY_TO_ADDRESS', content: inviter.email}, ]; if (group.type === 'guild') { @@ -541,16 +539,16 @@ async function _inviteByEmail (invite, group, inviter, req, res) { userReturnInfo = await _inviteByUUID(userToContact._id, group, inviter, req, res); } else { userReturnInfo = invite.email; - // yeah, it supports guild too but for backward compatibility we'll use partyInvite as query - // TODO absolutely refactor this horrible code - const partyQueryString = JSON.stringify({id: group._id, inviter, name: group.name}); - const encryptedPartyqueryString = encrypt(partyQueryString); - let link = `?partyInvite=${encryptedPartyqueryString}`; + const groupQueryString = JSON.stringify({ + id: group._id, + inviter: inviter._id, + sentAt: Date.now(), // so we can let it expire + }); + let link = `?groupInvite=${encrypt(groupQueryString)}`; let variables = [ {name: 'LINK', content: link}, - {name: 'INVITER', content: inviter || inviter.profile.name}, - {name: 'REPLY_TO_ADDRESS', content: inviter.email}, + {name: 'INVITER', content: inviter.profile.name}, ]; if (group.type === 'guild') { diff --git a/website/src/middlewares/api-v3/locals.js b/website/src/middlewares/api-v3/locals.js index ccb4fc003b..43b4f5f00a 100644 --- a/website/src/middlewares/api-v3/locals.js +++ b/website/src/middlewares/api-v3/locals.js @@ -9,7 +9,6 @@ import { import forceRefresh from './../forceRefresh'; import { tavernQuest } from '../../models/group'; import { mods } from '../../models/user'; -import { decrypt } from '../../libs/api-v3/encryption'; // To avoid stringifying more data then we need, // items from `env` used on the client will have to be specified in this array @@ -59,15 +58,5 @@ export default function locals (req, res, next) { worldDmg: tavernQuest && tavernQuest.extra && tavernQuest.extra.worldDmg || {}, }); - // Put query-string party (& guild but use partyInvite for backward compatibility) - // invitations into session to be handled later - if (req.query.partyInvite) { - try { - req.session.partyInvite = JSON.parse(decrypt(req.query.partyInvite)); - } catch (e) { - // TODO logs - } - } - next(); } From 0014afb75e8cf048d9748517384ec6fc5668e939 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Fri, 26 Feb 2016 19:43:17 +0100 Subject: [PATCH 2/5] add tests for req.query.groupInvite --- .../user/auth/POST-register_local.test.js | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) 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 f9d893a0f5..c2f02c3f68 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 @@ -2,9 +2,11 @@ import { generateUser, requester, translate as t, + createAndPopulateGroup, } from '../../../../../helpers/api-integration/v3'; import { v4 as generateRandomUserName } from 'uuid'; import { each } from 'lodash'; +import { encrypt } from '../../../../../../website/src/libs/api-v3/encryption'; describe('POST /user/auth/local/register', () => { context('username and email are free', () => { @@ -162,6 +164,53 @@ describe('POST /user/auth/local/register', () => { }); }); + context('req.query.groupInvite', () => { + let api, username, email, password; + + beforeEach(() => { + api = requester(); + username = generateRandomUserName(); + email = `${username}@example.com`; + password = 'password'; + }); + + it('does not crash the signup process when it\'s invalid', async () => { + let user = await api.post('/user/auth/local/register?groupInvite=aaaaInvalid', { + username, + email, + password, + confirmPassword: password, + }); + + expect(user._id).to.be.a('string'); + }); + + it('supports invite using req.query.groupInvite', async () => { + let { group, groupLeader } = await createAndPopulateGroup({ + groupDetails: { type: 'party', privacy: 'private' }, + }); + + let invite = encrypt(JSON.stringify({ + id: group._id, + inviter: groupLeader._id, + sentAt: Date.now(), // so we can let it expire + })); + + let user = await api.post(`/user/auth/local/register?groupInvite=${invite}`, { + username, + email, + password, + confirmPassword: password, + }); + + expect(user.invitations.party).to.eql({ + id: group._id, + name: group.name, + inviter: groupLeader._id, + }); + }); + }); + context('successful login via api', () => { let api, username, email, password; From cc5f794f3d33941204282201d2c617d5bb8381d7 Mon Sep 17 00:00:00 2001 From: Alys Date: Sat, 27 Feb 2016 18:48:41 +1000 Subject: [PATCH 3/5] prevent party inviter's email address being used as a Reply-To --- test/server_side/controllers/groups.test.js | 3 +-- website/src/controllers/api-v2/groups.js | 7 ++----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/test/server_side/controllers/groups.test.js b/test/server_side/controllers/groups.test.js index 6a75c75c78..2dd69c2a87 100644 --- a/test/server_side/controllers/groups.test.js +++ b/test/server_side/controllers/groups.test.js @@ -111,8 +111,7 @@ describe('Groups Controller', function() { 'invite-friend', [ { name: 'LINK', content: '?partyInvite=http://link.com' }, - { name: 'INVITER', content: 'inviter' }, - { name: 'REPLY_TO_ADDRESS', content: 'inviter@example.com' } + { name: 'INVITER', content: 'inviter' } ] ); }); diff --git a/website/src/controllers/api-v2/groups.js b/website/src/controllers/api-v2/groups.js index cd01d1b27e..25b1cd52da 100644 --- a/website/src/controllers/api-v2/groups.js +++ b/website/src/controllers/api-v2/groups.js @@ -587,8 +587,7 @@ var inviteByUUIDs = function(uuids, group, req, res, next){ if(invite.preferences.emailNotifications['invited' + (group.type == 'guild' ? 'Guild' : 'Party')] !== false){ var inviterVars = utils.getUserInfo(res.locals.user, ['name', 'email']); var emailVars = [ - {name: 'INVITER', content: inviterVars.name}, - {name: 'REPLY_TO_ADDRESS', content: inviterVars.email} + {name: 'INVITER', content: inviterVars.name} ]; if(group.type == 'guild'){ @@ -653,8 +652,7 @@ var inviteByEmails = function(invites, group, req, res, next){ var inviterVars = utils.getUserInfo(res.locals.user, ['name', 'email']); var variables = [ {name: 'LINK', content: link}, - {name: 'INVITER', content: req.body.inviter || inviterVars.name}, - {name: 'REPLY_TO_ADDRESS', content: inviterVars.email} + {name: 'INVITER', content: req.body.inviter || inviterVars.name} ]; if(group.type == 'guild'){ @@ -954,7 +952,6 @@ api.questAccept = function(req, res, next) { utils.txnEmail(membersToEmail, ('invite-' + (quest.boss ? 'boss' : 'collection') + '-quest'), [ {name: 'QUEST_NAME', content: quest.text()}, {name: 'INVITER', content: inviterVars.name}, - {name: 'REPLY_TO_ADDRESS', content: inviterVars.email}, {name: 'PARTY_URL', content: '/#/options/groups/party'} ]); From b4e4e31be5acf399245078140bac5225f288fac8 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Sat, 27 Feb 2016 12:01:00 +0100 Subject: [PATCH 4/5] fix invitations and add missing tests --- .../groups/POST-groups_invite.test.js | 30 ++++++++++++++++++- website/src/controllers/api-v3/groups.js | 5 ++-- website/src/controllers/api-v3/quests.js | 1 - 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/test/api/v3/integration/groups/POST-groups_invite.test.js b/test/api/v3/integration/groups/POST-groups_invite.test.js index 55c25d9153..d3f2b78c27 100644 --- a/test/api/v3/integration/groups/POST-groups_invite.test.js +++ b/test/api/v3/integration/groups/POST-groups_invite.test.js @@ -3,6 +3,7 @@ import { translate as t, } from '../../../../helpers/api-v3-integration.helper'; import { v4 as generateUUID } from 'uuid'; +import * as email from '../../../../../website/src/libs/api-v3/email'; const INVITES_LIMIT = 100; @@ -19,6 +20,10 @@ describe('Post /groups/:groupId/invite', () => { }); }); + afterEach(() => { + if (email.sendTxn.restore) email.sendTxn.restore(); + }); + describe('user id invites', () => { it('returns an error when invited user is not found', async () => { let fakeID = generateUUID(); @@ -70,9 +75,10 @@ describe('Post /groups/:groupId/invite', () => { }); }); - it('invites a user to a group by uuid', async () => { + it.only('invites a user to a group by uuid', async () => { let userToInvite = await generateUser(); + sandbox.stub(email, 'sendTxn'); await expect(inviter.post(`/groups/${group._id}/invite`, { uuids: [userToInvite._id], })).to.eventually.deep.equal([{ @@ -80,6 +86,13 @@ describe('Post /groups/:groupId/invite', () => { name: groupName, inviter: inviter._id, }]); + + expect(email.sendTxn).to.be.calledOnce; + expect(email.sendTxn[0][0]._id).to.equal(userToInvite._id); + expect(email.sendTxn[0][1]).to.equal('invited-guild'); + expect(email.sendTxn[0][2]).to.have.all.keys(['GUILD_NAME', 'GUILD_URL', 'INVITER']); + expect(email.sendTxn[0][2].INVITER).to.equal(inviter.profile.name); + await expect(userToInvite.get('/user')) .to.eventually.have.deep.property('invitations.guilds[0].id', group._id); }); @@ -102,6 +115,8 @@ describe('Post /groups/:groupId/invite', () => { inviter: inviter._id, }, ]); + expect(email.sendTxn).to.be.calledTwice; + await expect(userToInvite.get('/user')).to.eventually.have.deep.property('invitations.guilds[0].id', group._id); await expect(userToInvite2.get('/user')).to.eventually.have.deep.property('invitations.guilds[0].id', group._id); }); @@ -173,13 +188,23 @@ describe('Post /groups/:groupId/invite', () => { it('invites a user to a group by email', async () => { await expect(inviter.post(`/groups/${group._id}/invite`, { emails: [testInvite], + inviter: 'inviter name', })).to.exist; + + expect(email.sendTxn).to.be.calledOnce; + expect(email.sendTxn[0][0]).to.eql(testInvite); + expect(email.sendTxn[0][1]).to.equal('invite-friend-guild'); + expect(email.sendTxn[0][2]).to.have.all.keys(['GUILD_NAME', 'LINK', 'INVITER']); + expect(email.sendTxn[0][2].INVITER).to.equal('inviter name'); }); it('invites multiple users to a group by email', async () => { await expect(inviter.post(`/groups/${group._id}/invite`, { emails: [testInvite, {name: 'test2', email: 'test2@habitica.com'}], })).to.exist; + + expect(email.sendTxn).to.be.calledTwice; + expect(email.sendTxn[0][2].INVITER).to.equal(inviter.profile.name); }); }); @@ -226,6 +251,7 @@ describe('Post /groups/:groupId/invite', () => { expect(invite).to.exist; expect(invitedUser.invitations.guilds[0].id).to.equal(group._id); + expect(email.sendTxn).to.be.calledTwice; }); }); @@ -320,6 +346,8 @@ describe('Post /groups/:groupId/invite', () => { uuids: [userToInvite._id], }); expect((await userToInvite.get('/user')).invitations.party.id).to.equal(party._id); + + expect(email.sendTxn).to.be.calledOnce; }); }); }); diff --git a/website/src/controllers/api-v3/groups.js b/website/src/controllers/api-v3/groups.js index a078092a7e..b4580d0491 100644 --- a/website/src/controllers/api-v3/groups.js +++ b/website/src/controllers/api-v3/groups.js @@ -548,14 +548,13 @@ async function _inviteByEmail (invite, group, inviter, req, res) { let variables = [ {name: 'LINK', content: link}, - {name: 'INVITER', content: inviter.profile.name}, + {name: 'INVITER', content: req.body.inviter || inviter.profile.name}, ]; if (group.type === 'guild') { variables.push({name: 'GUILD_NAME', content: group.name}); } - // TODO implement "users can only be invited once" // Check for the email address not to be unsubscribed let userIsUnsubscribed = await EmailUnsubscription.findOne({email: invite.email}).exec(); let groupLabel = group.type === 'guild' ? '-guild' : ''; @@ -598,7 +597,7 @@ api.inviteToGroup = { let emails = req.body.emails; let uuidsIsArray = Array.isArray(uuids); - let emailsIsArray = Array.isArray(emails); + let emailsIsArray = Array.isArray(emails); if (!uuids && !emails) { throw new BadRequest(res.t('canOnlyInviteEmailUuid')); diff --git a/website/src/controllers/api-v3/quests.js b/website/src/controllers/api-v3/quests.js index e7f728c74a..f2a95db29f 100644 --- a/website/src/controllers/api-v3/quests.js +++ b/website/src/controllers/api-v3/quests.js @@ -116,7 +116,6 @@ api.inviteToQuest = { sendTxnEmail(membersToEmail, `invite-${quest.boss ? 'boss' : 'collection'}-quest`, [ {name: 'QUEST_NAME', content: quest.text()}, {name: 'INVITER', content: inviterVars.name}, - {name: 'REPLY_TO_ADDRESS', content: inviterVars.email}, {name: 'PARTY_URL', content: '/#/options/groups/party'}, ]); From 9321a2d90478f47dbd969b120c9a435fe7a9616a Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Sat, 27 Feb 2016 17:58:56 +0100 Subject: [PATCH 5/5] remove checks for emails --- .../groups/POST-groups_invite.test.js | 29 ++----------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/test/api/v3/integration/groups/POST-groups_invite.test.js b/test/api/v3/integration/groups/POST-groups_invite.test.js index d3f2b78c27..0f59bd5966 100644 --- a/test/api/v3/integration/groups/POST-groups_invite.test.js +++ b/test/api/v3/integration/groups/POST-groups_invite.test.js @@ -3,7 +3,6 @@ import { translate as t, } from '../../../../helpers/api-v3-integration.helper'; import { v4 as generateUUID } from 'uuid'; -import * as email from '../../../../../website/src/libs/api-v3/email'; const INVITES_LIMIT = 100; @@ -20,10 +19,6 @@ describe('Post /groups/:groupId/invite', () => { }); }); - afterEach(() => { - if (email.sendTxn.restore) email.sendTxn.restore(); - }); - describe('user id invites', () => { it('returns an error when invited user is not found', async () => { let fakeID = generateUUID(); @@ -75,10 +70,9 @@ describe('Post /groups/:groupId/invite', () => { }); }); - it.only('invites a user to a group by uuid', async () => { + it('invites a user to a group by uuid', async () => { let userToInvite = await generateUser(); - sandbox.stub(email, 'sendTxn'); await expect(inviter.post(`/groups/${group._id}/invite`, { uuids: [userToInvite._id], })).to.eventually.deep.equal([{ @@ -87,12 +81,6 @@ describe('Post /groups/:groupId/invite', () => { inviter: inviter._id, }]); - expect(email.sendTxn).to.be.calledOnce; - expect(email.sendTxn[0][0]._id).to.equal(userToInvite._id); - expect(email.sendTxn[0][1]).to.equal('invited-guild'); - expect(email.sendTxn[0][2]).to.have.all.keys(['GUILD_NAME', 'GUILD_URL', 'INVITER']); - expect(email.sendTxn[0][2].INVITER).to.equal(inviter.profile.name); - await expect(userToInvite.get('/user')) .to.eventually.have.deep.property('invitations.guilds[0].id', group._id); }); @@ -115,7 +103,6 @@ describe('Post /groups/:groupId/invite', () => { inviter: inviter._id, }, ]); - expect(email.sendTxn).to.be.calledTwice; await expect(userToInvite.get('/user')).to.eventually.have.deep.property('invitations.guilds[0].id', group._id); await expect(userToInvite2.get('/user')).to.eventually.have.deep.property('invitations.guilds[0].id', group._id); @@ -190,21 +177,12 @@ describe('Post /groups/:groupId/invite', () => { emails: [testInvite], inviter: 'inviter name', })).to.exist; - - expect(email.sendTxn).to.be.calledOnce; - expect(email.sendTxn[0][0]).to.eql(testInvite); - expect(email.sendTxn[0][1]).to.equal('invite-friend-guild'); - expect(email.sendTxn[0][2]).to.have.all.keys(['GUILD_NAME', 'LINK', 'INVITER']); - expect(email.sendTxn[0][2].INVITER).to.equal('inviter name'); }); it('invites multiple users to a group by email', async () => { await expect(inviter.post(`/groups/${group._id}/invite`, { emails: [testInvite, {name: 'test2', email: 'test2@habitica.com'}], })).to.exist; - - expect(email.sendTxn).to.be.calledTwice; - expect(email.sendTxn[0][2].INVITER).to.equal(inviter.profile.name); }); }); @@ -249,9 +227,8 @@ describe('Post /groups/:groupId/invite', () => { }); let invitedUser = await newUser.get('/user'); - expect(invite).to.exist; expect(invitedUser.invitations.guilds[0].id).to.equal(group._id); - expect(email.sendTxn).to.be.calledTwice; + expect(invite).to.exist; }); }); @@ -346,8 +323,6 @@ describe('Post /groups/:groupId/invite', () => { uuids: [userToInvite._id], }); expect((await userToInvite.get('/user')).invitations.party.id).to.equal(party._id); - - expect(email.sendTxn).to.be.calledOnce; }); }); });