From 7e09c0e0b13370b94ace530862936de264001619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Wed, 30 May 2018 11:29:21 +0100 Subject: [PATCH 01/44] First stab at email token invites (WIP) --- .../coffee/Features/Email/EmailBuilder.coffee | 36 ++++++++++-- .../SubscriptionGroupHandler.coffee | 18 ++++-- .../Subscription/SubscriptionRouter.coffee | 14 ++++- .../Subscription/TeamInvitesController.coffee | 30 ++++++++++ .../Subscription/TeamInvitesHandler.coffee | 44 +++++++++++++++ .../web/app/coffee/models/TeamInvite.coffee | 16 ++++++ .../views/subscriptions/group/team_invite.pug | 55 +++++++++++++++++++ .../public/coffee/main/group-members.coffee | 4 +- .../team-invite-controller.coffee | 35 ++++++++++++ 9 files changed, 238 insertions(+), 14 deletions(-) create mode 100644 services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee create mode 100644 services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee create mode 100644 services/web/app/coffee/models/TeamInvite.coffee create mode 100644 services/web/app/views/subscriptions/group/team_invite.pug create mode 100644 services/web/public/coffee/main/subscription/team-invite-controller.coffee diff --git a/services/web/app/coffee/Features/Email/EmailBuilder.coffee b/services/web/app/coffee/Features/Email/EmailBuilder.coffee index 8b672bc4c5..9b975140f9 100644 --- a/services/web/app/coffee/Features/Email/EmailBuilder.coffee +++ b/services/web/app/coffee/Features/Email/EmailBuilder.coffee @@ -79,7 +79,7 @@ Thank you #{settings.appName} - <%= siteUrl %> """ - compiledTemplate: (opts) -> + compiledTemplate: (opts) -> SingleCTAEmailBody({ title: "Password Reset" greeting: "Hi," @@ -104,7 +104,7 @@ Thank you #{settings.appName} - <%= siteUrl %> """ - compiledTemplate: (opts) -> + compiledTemplate: (opts) -> SingleCTAEmailBody({ title: "#{ opts.project.name } – shared by #{ opts.owner.email }" greeting: "Hi," @@ -112,12 +112,38 @@ Thank you secondaryMessage: null ctaText: "View project" ctaURL: opts.inviteUrl - gmailGoToAction: + gmailGoToAction: target: opts.inviteUrl name: "View project" description: "Join #{ opts.project.name } at ShareLaTeX" }) + +templates.verifyEmailToJoinTeam = + subject: _.template "<%= inviterName %> has invited you to join a #{settings.appName} team" + layout: BaseWithHeaderEmailLayout + type:"notification" + plainTextTemplate: _.template """ + +Hi, please verify your email to join the team and get your free premium account. + +Click this link to verify now: <%= acceptInviteUrl %> + +Thank You + +#{settings.appName} - <%= siteUrl %> +""" + compiledTemplate: (opts) -> + SingleCTAEmailBody({ + title: "#{opts.inviterName} has invited you to join a #{settings.appName} team" + greeting: "Hi," + message: "please verify your email to join the team and get your free premium account" + secondaryMessage: null + ctaText: "Verify now" + ctaURL: opts.acceptInviteUrl + gmailGoToAction: null + }) + templates.completeJoinGroupAccount = subject: _.template "Verify Email to join <%= group_name %> group" layout: BaseWithHeaderEmailLayout @@ -131,7 +157,7 @@ Thank You #{settings.appName} - <%= siteUrl %> """ - compiledTemplate: (opts) -> + compiledTemplate: (opts) -> SingleCTAEmailBody({ title: "Verify Email to join #{ opts.group_name } group" greeting: "Hi," @@ -154,7 +180,7 @@ This is a test email sent from ShareLaTeX. #{settings.appName} - <%= siteUrl %> """ - compiledTemplate: (opts) -> + compiledTemplate: (opts) -> SingleCTAEmailBody({ title: "A Test Email from ShareLaTeX" greeting: "Hi," diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index 5b7c2e0740..3ac9ae0181 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -6,6 +6,7 @@ UserLocator = require("../User/UserLocator") LimitationsManager = require("./LimitationsManager") logger = require("logger-sharelatex") OneTimeTokenHandler = require("../Security/OneTimeTokenHandler") +TeamInvitesHandler = require("./TeamInvitesHandler") EmailHandler = require("../Email/EmailHandler") settings = require("settings-sharelatex") NotificationsBuilder = require("../Notifications/NotificationsBuilder") @@ -39,15 +40,24 @@ module.exports = SubscriptionGroupHandler = removeUserFromGroup: (adminUser_id, userToRemove_id, callback)-> SubscriptionUpdater.removeUserFromGroup adminUser_id, userToRemove_id, callback - + removeEmailInviteFromGroup: (adminUser_id, email, callback) -> SubscriptionUpdater.removeEmailInviteFromGroup adminUser_id, email, callback getPopulatedListOfMembers: (adminUser_id, callback)-> - SubscriptionLocator.getUsersSubscription adminUser_id, (err, subscription)-> + SubscriptionLocator.getUsersSubscription adminUser_id, (err, subscription)-> + return callback(err) if err? + users = [] for email in subscription.invited_emails or [] users.push buildEmailInviteViewModel(email) + + TeamInvitesHandler.getInvites subscription.id, (err, invites) -> + return callback(err) if err? + + for invite in invites or [] + users.push buildEmailInviteViewModel(invite.email) + jobs = _.map subscription.member_ids, (user_id)-> return (cb)-> UserLocator.findById user_id, (err, user)-> @@ -111,7 +121,7 @@ module.exports = SubscriptionGroupHandler = async.series jobs, callback buildUserViewModel = (user)-> - u = + u = email: user.email first_name: user.first_name last_name: user.last_name @@ -123,4 +133,4 @@ buildEmailInviteViewModel = (email) -> return { email: email holdingAccount: true - } \ No newline at end of file + } diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee index 8c5f631e12..b0b13f0184 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee @@ -1,6 +1,7 @@ AuthenticationController = require('../Authentication/AuthenticationController') SubscriptionController = require('./SubscriptionController') SubscriptionGroupController = require './SubscriptionGroupController' +TeamInvitesController = require './TeamInvitesController' Settings = require "settings-sharelatex" module.exports = @@ -13,8 +14,7 @@ module.exports = webRouter.get '/user/subscription/custom_account', AuthenticationController.requireLogin(), SubscriptionController.userCustomSubscriptionPage - - webRouter.get '/user/subscription/new', AuthenticationController.requireLogin(), SubscriptionController.paymentPage + webRouter.get '/user/subscription/new', AuthenticationController.requireLogin(), SubscriptionController.paymentPage webRouter.get '/user/subscription/thank-you', AuthenticationController.requireLogin(), SubscriptionController.successful_subscription @@ -26,6 +26,15 @@ module.exports = webRouter.delete '/subscription/group/email/:email', AuthenticationController.requireLogin(), SubscriptionGroupController.removeEmailInviteFromGroup webRouter.delete '/subscription/group/user', AuthenticationController.requireLogin(), SubscriptionGroupController.removeSelfFromGroup + # Team invites + webRouter.post '/subscription/invites', AuthenticationController.requireLogin(), + TeamInvitesController.createInvite + webRouter.get '/subscription/invites/:token/', AuthenticationController.requireLogin(), + TeamInvitesController.viewInvite + webRouter.put '/subscription/invites/:token/', AuthenticationController.requireLogin(), + TeamInvitesController.acceptInvite + webRouter.delete '/subscription/invites/:token/', AuthenticationController.requireLogin(), + TeamInvitesController.revokeInvite webRouter.get '/user/subscription/:subscription_id/group/invited', AuthenticationController.requireLogin(), SubscriptionGroupController.renderGroupInvitePage webRouter.post '/user/subscription/:subscription_id/group/begin-join', AuthenticationController.requireLogin(), SubscriptionGroupController.beginJoinGroup @@ -48,4 +57,3 @@ module.exports = # Currently used in acceptance tests only, as a way to trigger the syncing logic publicApiRouter.post "/user/:user_id/features/sync", AuthenticationController.httpAuth, SubscriptionController.refreshUserFeatures - diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee new file mode 100644 index 0000000000..a073c448e2 --- /dev/null +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee @@ -0,0 +1,30 @@ +logger = require("logger-sharelatex") +TeamInvitesHandler = require('./TeamInvitesHandler') +AuthenticationController = require("../Authentication/AuthenticationController") + +module.exports = + createInvite: (req, res, next) -> + adminUserId = AuthenticationController.getLoggedInUserId(req) + email = req.body.email + + TeamInvitesHandler.createInvite adminUserId, email, (err, invite) -> + next(err) if err? + inviteView = { user: + { email: invite.email, sentAt: invite.sentAt, holdingAccount: true } + } + res.json inviteView + + viewInvite: (req, res) -> + token = request.params.token + + TeamInvitesHandler.getInvite token, (err, invite) -> + next(err) if err? + + res.render "referal/bonus", + title: "bonus_please_recommend_us" + refered_users: refered_users + refered_user_count: (refered_users or []).length + + acceptInvite: (req, res) -> + + revokeInvite: (req, res) -> diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee new file mode 100644 index 0000000000..2d170aae93 --- /dev/null +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -0,0 +1,44 @@ +logger = require("logger-sharelatex") +crypto = require("crypto") + +settings = require("settings-sharelatex") + +UserLocator = require("../User/UserLocator") +SubscriptionLocator = require("./SubscriptionLocator") +TeamInvite = require("../../models/TeamInvite").TeamInvite +EmailHandler = require("../Email/EmailHandler") + +module.exports = TeamInvitesHandler = + + getInvites: (subscriptionId, callback) -> + TeamInvite.find(subscriptionId: subscriptionId, callback) + + createInvite: (adminUserId, email, callback) -> + + UserLocator.findById adminUserId, (error, groupAdmin) -> + SubscriptionLocator.getUsersSubscription adminUserId, (error, subscription) -> + return callback(error) if error? + + inviterName = "#{groupAdmin.first_name} #{groupAdmin.last_name}" + token = crypto.randomBytes(32).toString("hex") + + TeamInvite.create { + subscriptionId: subscription.id, + email: email, + token: token, + sentAt: new Date(), + }, (error, invite) -> + return callback(error) if error? + opts = + to : email + inviterName: inviterName + acceptInviteUrl: "#{settings.siteUrl}/subscription/invites/#{token}/" + EmailHandler.sendEmail "verifyEmailToJoinTeam", opts, (error) -> + return callback(error, invite) + + getInvite: (token, callback) -> + + + acceptInvite: (userId, token, callback) -> + + revokeInvite: (token, callback) -> diff --git a/services/web/app/coffee/models/TeamInvite.coffee b/services/web/app/coffee/models/TeamInvite.coffee new file mode 100644 index 0000000000..ba0d0dc288 --- /dev/null +++ b/services/web/app/coffee/models/TeamInvite.coffee @@ -0,0 +1,16 @@ +mongoose = require 'mongoose' +Settings = require 'settings-sharelatex' + +Schema = mongoose.Schema +ObjectId = Schema.ObjectId + +TeamInviteSchema = new Schema + subscriptionId : { type: ObjectId, ref: 'Subscription', required: true } + email : { type: String, required: true } + token : { type: String, required: true } + sentAt : { type: Date, required: true } + + +mongoose.model 'TeamInvite', TeamInviteSchema +exports.TeamInvite = mongoose.model 'TeamInvite' +exports.TeamInviteSchema = TeamInviteSchema diff --git a/services/web/app/views/subscriptions/group/team_invite.pug b/services/web/app/views/subscriptions/group/team_invite.pug new file mode 100644 index 0000000000..6d7e962ccc --- /dev/null +++ b/services/web/app/views/subscriptions/group/team_invite.pug @@ -0,0 +1,55 @@ +extends ../../layout + +block scripts + script(type='text/javascript'). + window.teamId = '#{teamId}' + window.hasPersonalSubscription = #{hasPersonalSubscription} + window.inviteToken = #{inviteToken} + +block content + .content.content-alt + .container + .row + .col-md-8.col-md-offset-2 + -if (query.expired) + .alert.alert-warning #{translate("email_link_expired")} + + .row + div   + .row + .col-md-8.col-md-offset-2(ng-cloak) + .card(ng-controller="TeamInviteController") + .page-header + h1.text-centered #{translate("you_are_invited_to_group", {teamName: teamName})} + + div(ng-show="view =='personalSubscription'").row.text-centered + div #{translate("cancel_personal_subscription_first")} + .row + .col-md-12   + .row + .col-md-12 + a.btn.btn.btn-default(ng-click="keepPersonalSubscription()", ng-disabled="inflight") #{translate("not_now")} + span   + a.btn.btn.btn-primary(ng-click="cancelPersonalSubscription()", ng-disabled="inflight") #{translate("cancel_your_subscription")} + + div(ng-show="view =='teamInvite'").row.text-centered + .row + .col-md-12 #{translate("group_provides_you_with_premium_account", {teamName: teamName})} + .row + .col-md-12   + .row + .col-md-12 + .text-center + a.btn.btn-default(href="/project") #{translate("not_now")} + span   + a.btn.btn.btn-primary(ng-click="joinTeam()", ng-disabled="inflight") #{translate("verify_email_address")} + + + span(ng-show="view =='requestSent'").row.text-centered.text-center + .row + .col-md-12 #{translate("check_email_to_complete_the_upgrade")} + .row + .col-md-12   + .row + .col-md-12 + a.btn.btn.btn-primary(href="/project") #{translate("done")} diff --git a/services/web/public/coffee/main/group-members.coffee b/services/web/public/coffee/main/group-members.coffee index 813ad4c768..94300f8ce5 100644 --- a/services/web/public/coffee/main/group-members.coffee +++ b/services/web/public/coffee/main/group-members.coffee @@ -22,7 +22,7 @@ define [ emails = parseEmails($scope.inputs.emails) for email in emails queuedHttp - .post("/subscription/group/user", { + .post("/subscription/invites", { email: email, _csrf: window.csrfToken }) @@ -56,4 +56,4 @@ define [ App.controller "SubscriptionGroupMemberListItemController", ($scope) -> $scope.$watch "user.selected", (value) -> if value? - $scope.updateSelectedUsers() \ No newline at end of file + $scope.updateSelectedUsers() diff --git a/services/web/public/coffee/main/subscription/team-invite-controller.coffee b/services/web/public/coffee/main/subscription/team-invite-controller.coffee new file mode 100644 index 0000000000..ab6b42c658 --- /dev/null +++ b/services/web/public/coffee/main/subscription/team-invite-controller.coffee @@ -0,0 +1,35 @@ +define [ + "base" +], (App) -> + App.controller "TeamInviteController", ($scope, $http) -> + + $scope.inflight = false + + if hasPersonalSubscription + $scope.view = "personalSubscription" + else + $scope.view = "teamInvite" + + $scope.keepPersonalSubscription = -> + $scope.view = "teamInvite" + + $scope.cancelPersonalSubscription = -> + $scope.inflight = true + request = $http.post "/user/subscription/cancel", {_csrf:window.csrfToken} + request.then ()-> + $scope.inflight = false + $scope.view = "teamInvite" + request.catch ()-> + console.log "the request failed" + + $scope.joinTeam = -> + $scope.view = "requestSent" + $scope.inflight = true + request = $http.put "/subscription/invites/:token/", {_csrf:window.csrfToken} + request.then (response)-> + { status } = response + $scope.inflight = false + if status != 200 # assume request worked + $scope.requestSent = false + request.catch ()-> + console.log "the request failed" From 9aa95cb0d5a28f0c0a6b3722a0a52c0c66824243 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Wed, 30 May 2018 13:06:27 +0100 Subject: [PATCH 02/44] Show team invites --- .../Subscription/TeamInvitesController.coffee | 22 ++++++++----- .../Subscription/TeamInvitesHandler.coffee | 31 +++++++++++++++++-- .../views/subscriptions/group/team_invite.pug | 31 +++++-------------- services/web/public/coffee/main.coffee | 1 + .../public/stylesheets/app/subscription.less | 10 ++++-- 5 files changed, 60 insertions(+), 35 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee index a073c448e2..0a48aa4838 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee @@ -1,6 +1,7 @@ logger = require("logger-sharelatex") TeamInvitesHandler = require('./TeamInvitesHandler') AuthenticationController = require("../Authentication/AuthenticationController") +ErrorController = require("../Errors/ErrorController") module.exports = createInvite: (req, res, next) -> @@ -14,16 +15,23 @@ module.exports = } res.json inviteView - viewInvite: (req, res) -> - token = request.params.token + viewInvite: (req, res, next) -> + token = req.params.token + userId = AuthenticationController.getLoggedInUserId(req) - TeamInvitesHandler.getInvite token, (err, invite) -> + TeamInvitesHandler.getInviteDetails token, userId, (err, results) -> next(err) if err? - res.render "referal/bonus", - title: "bonus_please_recommend_us" - refered_users: refered_users - refered_user_count: (refered_users or []).length + { invite, personalSubscription, inviterName } = results + + unless invite? + return ErrorController.notFound(req, res, next) + + res.render "subscriptions/group/team_invite", + inviterName: inviterName + inviteToken: invite.token + hasPersonalSubscription: personalSubscription? + acceptInvite: (req, res) -> diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index 2d170aae93..c106598d56 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -19,7 +19,7 @@ module.exports = TeamInvitesHandler = SubscriptionLocator.getUsersSubscription adminUserId, (error, subscription) -> return callback(error) if error? - inviterName = "#{groupAdmin.first_name} #{groupAdmin.last_name}" + inviterName = TeamInvitesHandler.inviterName(groupAdmin) token = crypto.randomBytes(32).toString("hex") TeamInvite.create { @@ -37,8 +37,35 @@ module.exports = TeamInvitesHandler = return callback(error, invite) getInvite: (token, callback) -> - + TeamInvite.findOne(token: token, callback) acceptInvite: (userId, token, callback) -> revokeInvite: (token, callback) -> + + getInviteDetails: (token, userId, callback) -> + TeamInvitesHandler.getInvite token, (err, invite) -> + callback(err) if err? + + SubscriptionLocator.getUsersSubscription userId, (err, personalSubscription) -> + callback(err) if err? + + SubscriptionLocator.getSubscription invite.subscriptionId, (err, teamSubscription) -> + callback(err) if err? + + UserLocator.findById teamSubscription.admin_id, (err, teamAdmin) -> + callback(err) if err? + + callback(null , { + invite: invite, + personalSubscription: personalSubscription, + team: teamSubscription, + inviterName: TeamInvitesHandler.inviterName(teamAdmin), + teamAdmin: teamAdmin + }) + + inviterName: (groupAdmin) -> + if groupAdmin.first_name and groupAdmin.last_name + "#{groupAdmin.first_name} #{groupAdmin.last_name} (#{groupAdmin.email})" + else + groupAdmin.email diff --git a/services/web/app/views/subscriptions/group/team_invite.pug b/services/web/app/views/subscriptions/group/team_invite.pug index 6d7e962ccc..ae065ea64b 100644 --- a/services/web/app/views/subscriptions/group/team_invite.pug +++ b/services/web/app/views/subscriptions/group/team_invite.pug @@ -4,10 +4,10 @@ block scripts script(type='text/javascript'). window.teamId = '#{teamId}' window.hasPersonalSubscription = #{hasPersonalSubscription} - window.inviteToken = #{inviteToken} + window.inviteToken = '#{inviteToken}' block content - .content.content-alt + .content.content-alt.team-invite .container .row .col-md-8.col-md-offset-2 @@ -20,36 +20,21 @@ block content .col-md-8.col-md-offset-2(ng-cloak) .card(ng-controller="TeamInviteController") .page-header - h1.text-centered #{translate("you_are_invited_to_group", {teamName: teamName})} + h1.text-centered #{translate("invited_to_group", {inviterName: inviterName})} - div(ng-show="view =='personalSubscription'").row.text-centered - div #{translate("cancel_personal_subscription_first")} - .row - .col-md-12   + div(ng-show="view =='personalSubscription'").row.text-centered.message + p #{translate("cancel_personal_subscription_first")} .row .col-md-12 a.btn.btn.btn-default(ng-click="keepPersonalSubscription()", ng-disabled="inflight") #{translate("not_now")} span   a.btn.btn.btn-primary(ng-click="cancelPersonalSubscription()", ng-disabled="inflight") #{translate("cancel_your_subscription")} - div(ng-show="view =='teamInvite'").row.text-centered - .row - .col-md-12 #{translate("group_provides_you_with_premium_account", {teamName: teamName})} - .row - .col-md-12   + div(ng-show="view =='teamInvite'").row.text-centered.message + p #{translate("accept_invite_to_join_team", {inviterName: inviterName})} .row .col-md-12 .text-center a.btn.btn-default(href="/project") #{translate("not_now")} span   - a.btn.btn.btn-primary(ng-click="joinTeam()", ng-disabled="inflight") #{translate("verify_email_address")} - - - span(ng-show="view =='requestSent'").row.text-centered.text-center - .row - .col-md-12 #{translate("check_email_to_complete_the_upgrade")} - .row - .col-md-12   - .row - .col-md-12 - a.btn.btn.btn-primary(href="/project") #{translate("done")} + a.btn.btn.btn-primary(ng-click="joinTeam()", ng-disabled="inflight") #{translate("join_team")} diff --git a/services/web/public/coffee/main.coffee b/services/web/public/coffee/main.coffee index 951923dc35..6034ed1bdd 100644 --- a/services/web/public/coffee/main.coffee +++ b/services/web/public/coffee/main.coffee @@ -17,6 +17,7 @@ define [ "main/announcements" "main/register-users" "main/subscription/group-subscription-invite-controller" + "main/subscription/team-invite-controller" "main/contact-us" "main/learn" "analytics/AbTestingManager" diff --git a/services/web/public/stylesheets/app/subscription.less b/services/web/public/stylesheets/app/subscription.less index 5cabafbec2..f2ff53250b 100644 --- a/services/web/public/stylesheets/app/subscription.less +++ b/services/web/public/stylesheets/app/subscription.less @@ -24,7 +24,7 @@ .input-feedback-message { display: none; font-size: 0.8em; - + .has-error & { display: inline-block; } @@ -58,7 +58,7 @@ & + & { border-left-width: 0; - border-radius: 0 @border-radius-large @border-radius-large 0; + border-radius: 0 @border-radius-large @border-radius-large 0; } &-selected { @@ -73,6 +73,10 @@ } } +.team-invite .message { + margin: 3em 0; +} + .capitalised { text-transform:capitalize; -} \ No newline at end of file +} From 11edfde153d444b5cc62e7efd06b8e7277975c20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 31 May 2018 11:54:50 +0100 Subject: [PATCH 03/44] Accept & revoke team invites --- .../Subscription/LimitationsManager.coffee | 7 +- .../SubscriptionGroupHandler.coffee | 7 +- .../Subscription/SubscriptionRouter.coffee | 2 +- .../Subscription/TeamInvitesController.coffee | 22 +++- .../Subscription/TeamInvitesHandler.coffee | 107 ++++++++++++------ .../web/app/coffee/models/Subscription.coffee | 2 + .../web/app/coffee/models/TeamInvite.coffee | 6 +- .../views/subscriptions/group/team_invite.pug | 15 ++- .../public/coffee/main/group-members.coffee | 2 +- .../team-invite-controller.coffee | 4 +- 10 files changed, 120 insertions(+), 54 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee b/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee index c6a13b3069..f09dde2453 100644 --- a/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee +++ b/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee @@ -19,7 +19,6 @@ module.exports = callback null, user.features.collaborators else callback null, Settings.defaultPlanCode.collaborators - canAddXCollaborators: (project_id, x_collaborators, callback = (error, allowed)->) -> @allowedNumberOfCollaboratorsInProject project_id, (error, allowed_number) => @@ -56,6 +55,10 @@ module.exports = return callback(err) if err? callback err, subscriptions.length > 0, subscriptions + teamHasReachedMemberLimit: (subscription) -> + currentTotal = (subscription.member_ids or []).length + (subscription.team_invites or []).length + return currentTotal >= subscription.membersLimit + hasGroupMembersLimitReached: (user_id, callback = (err, limitReached, subscription)->)-> SubscriptionLocator.getUsersSubscription user_id, (err, subscription)-> if err? @@ -68,5 +71,3 @@ module.exports = limitReached = currentTotal >= subscription.membersLimit logger.log user_id:user_id, limitReached:limitReached, currentTotal: currentTotal, membersLimit: subscription.membersLimit, "checking if subscription members limit has been reached" callback(err, limitReached, subscription) - -getOwnerIdOfProject = (project_id, callback)-> diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index 3ac9ae0181..d893bff7a1 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -52,11 +52,8 @@ module.exports = SubscriptionGroupHandler = for email in subscription.invited_emails or [] users.push buildEmailInviteViewModel(email) - TeamInvitesHandler.getInvites subscription.id, (err, invites) -> - return callback(err) if err? - - for invite in invites or [] - users.push buildEmailInviteViewModel(invite.email) + for teamInvite in subscription.teamInvites or [] + users.push buildEmailInviteViewModel(teamInvite.email) jobs = _.map subscription.member_ids, (user_id)-> return (cb)-> diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee index b0b13f0184..5aabd191de 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee @@ -33,7 +33,7 @@ module.exports = TeamInvitesController.viewInvite webRouter.put '/subscription/invites/:token/', AuthenticationController.requireLogin(), TeamInvitesController.acceptInvite - webRouter.delete '/subscription/invites/:token/', AuthenticationController.requireLogin(), + webRouter.delete '/subscription/invites/:email/', AuthenticationController.requireLogin(), TeamInvitesController.revokeInvite webRouter.get '/user/subscription/:subscription_id/group/invited', AuthenticationController.requireLogin(), SubscriptionGroupController.renderGroupInvitePage diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee index 0a48aa4838..17094adaa6 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee @@ -1,3 +1,4 @@ +settings = require "settings-sharelatex" logger = require("logger-sharelatex") TeamInvitesHandler = require('./TeamInvitesHandler') AuthenticationController = require("../Authentication/AuthenticationController") @@ -5,10 +6,10 @@ ErrorController = require("../Errors/ErrorController") module.exports = createInvite: (req, res, next) -> - adminUserId = AuthenticationController.getLoggedInUserId(req) + teamManagerId = AuthenticationController.getLoggedInUserId(req) email = req.body.email - TeamInvitesHandler.createInvite adminUserId, email, (err, invite) -> + TeamInvitesHandler.createInvite teamManagerId, email, (err, invite) -> next(err) if err? inviteView = { user: { email: invite.email, sentAt: invite.sentAt, holdingAccount: true } @@ -31,8 +32,23 @@ module.exports = inviterName: inviterName inviteToken: invite.token hasPersonalSubscription: personalSubscription? + appName: settings.appName - acceptInvite: (req, res) -> + acceptInvite: (req, res, next) -> + token = req.params.token + userId = AuthenticationController.getLoggedInUserId(req) + + TeamInvitesHandler.acceptInvite token, userId, (err, results) -> + next(err) if err? + + res.sendStatus 204 revokeInvite: (req, res) -> + email = req.params.email + teamManagerId = AuthenticationController.getLoggedInUserId(req) + + TeamInvitesHandler.revokeInvite teamManagerId, email, (err, results) -> + next(err) if err? + + res.sendStatus 204 diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index c106598d56..66ccd24a5c 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -2,10 +2,16 @@ logger = require("logger-sharelatex") crypto = require("crypto") settings = require("settings-sharelatex") +ObjectId = require("mongojs").ObjectId + +TeamInvite = require("../../models/TeamInvite").TeamInvite +Subscription = require("../../models/Subscription").Subscription UserLocator = require("../User/UserLocator") SubscriptionLocator = require("./SubscriptionLocator") -TeamInvite = require("../../models/TeamInvite").TeamInvite +SubscriptionUpdater = require("./SubscriptionUpdater") +LimitationsManager = require("./LimitationsManager") + EmailHandler = require("../Email/EmailHandler") module.exports = TeamInvitesHandler = @@ -13,59 +19,96 @@ module.exports = TeamInvitesHandler = getInvites: (subscriptionId, callback) -> TeamInvite.find(subscriptionId: subscriptionId, callback) - createInvite: (adminUserId, email, callback) -> - - UserLocator.findById adminUserId, (error, groupAdmin) -> - SubscriptionLocator.getUsersSubscription adminUserId, (error, subscription) -> + createInvite: (teamManagerId, email, callback) -> + UserLocator.findById teamManagerId, (error, teamManager) -> + SubscriptionLocator.getUsersSubscription teamManagerId, (error, subscription) -> return callback(error) if error? - inviterName = TeamInvitesHandler.inviterName(groupAdmin) + if LimitationsManager.teamHasReachedMemberLimit(subscription) + return callback(limitReached: true) + + existingInvite = subscription.teamInvites.find (invite) -> invite.email == email + + if existingInvite + return callback(alreadyInvited: true) + + inviterName = TeamInvitesHandler.inviterName(teamManager) token = crypto.randomBytes(32).toString("hex") - TeamInvite.create { - subscriptionId: subscription.id, + invite = { email: email, token: token, sentAt: new Date(), - }, (error, invite) -> + } + + subscription.teamInvites.push(invite) + + subscription.save (error) -> return callback(error) if error? + + # TODO: use standard way to canonalise email addresses opts = - to : email + to: email.trim().toLowerCase() inviterName: inviterName acceptInviteUrl: "#{settings.siteUrl}/subscription/invites/#{token}/" EmailHandler.sendEmail "verifyEmailToJoinTeam", opts, (error) -> return callback(error, invite) - getInvite: (token, callback) -> - TeamInvite.findOne(token: token, callback) + acceptInvite: (token, userId, callback) -> + TeamInvitesHandler.getInviteAndManager token, (err, invite, subscription, teamManager) -> + return callback(err) if err? + return callback(inviteNoLongerValid: true) unless invite? and teamManager? - acceptInvite: (userId, token, callback) -> + SubscriptionUpdater.addUserToGroup teamManager, userId, (err) -> + return callback(err) if err? - revokeInvite: (token, callback) -> + TeamInvitesHandler.removeInviteFromTeam(subscription.id, invite.email, callback) + + revokeInvite: (teamManagerId, email, callback) -> + SubscriptionLocator.getUsersSubscription teamManagerId, (err, teamSubscription) -> + return callback(err) if err? + + TeamInvitesHandler.removeInviteFromTeam(teamSubscription.id, email, callback) getInviteDetails: (token, userId, callback) -> - TeamInvitesHandler.getInvite token, (err, invite) -> - callback(err) if err? + TeamInvitesHandler.getInviteAndManager token, (err, invite, teamSubscription, teamManager) -> + return callback(err) if err? SubscriptionLocator.getUsersSubscription userId, (err, personalSubscription) -> - callback(err) if err? + return callback(err) if err? - SubscriptionLocator.getSubscription invite.subscriptionId, (err, teamSubscription) -> - callback(err) if err? + return callback(null , { + invite: invite, + personalSubscription: personalSubscription, + team: teamSubscription, + inviterName: TeamInvitesHandler.inviterName(teamManager), + teamManager: teamManager + }) - UserLocator.findById teamSubscription.admin_id, (err, teamAdmin) -> - callback(err) if err? + getInviteAndManager: (token, callback) -> + TeamInvitesHandler.getInvite token, (err, invite, teamSubscription) -> + return callback(err) if err? - callback(null , { - invite: invite, - personalSubscription: personalSubscription, - team: teamSubscription, - inviterName: TeamInvitesHandler.inviterName(teamAdmin), - teamAdmin: teamAdmin - }) + UserLocator.findById teamSubscription.admin_id, (err, teamManager) -> + return callback(err, invite, teamSubscription, teamManager) - inviterName: (groupAdmin) -> - if groupAdmin.first_name and groupAdmin.last_name - "#{groupAdmin.first_name} #{groupAdmin.last_name} (#{groupAdmin.email})" + getInvite: (token, callback) -> + Subscription.findOne 'teamInvites.token': token, (err, subscription) -> + return callback(err, subscription) if err? + return callback(teamNotFound: true) unless subscription? + + invite = subscription.teamInvites.find (i) -> i.token == token + return callback(null, invite, subscription) + + + removeInviteFromTeam: (subscriptionId, email, callback) -> + searchConditions = { _id: new ObjectId(subscriptionId.toString()) } + updateOp = { $pull: { teamInvites: { email: email.trim().toLowerCase() } } } + + Subscription.update(searchConditions, updateOp, callback) + + inviterName: (teamManager) -> + if teamManager.first_name and teamManager.last_name + "#{teamManager.first_name} #{teamManager.last_name} (#{teamManager.email})" else - groupAdmin.email + teamManager.email diff --git a/services/web/app/coffee/models/Subscription.coffee b/services/web/app/coffee/models/Subscription.coffee index aaf78a4ab2..f7bfb91224 100644 --- a/services/web/app/coffee/models/Subscription.coffee +++ b/services/web/app/coffee/models/Subscription.coffee @@ -1,5 +1,6 @@ mongoose = require 'mongoose' Settings = require 'settings-sharelatex' +TeamInviteSchema = require('./TeamInvite').TeamInviteSchema Schema = mongoose.Schema ObjectId = Schema.ObjectId @@ -8,6 +9,7 @@ SubscriptionSchema = new Schema admin_id : {type:ObjectId, ref:'User', index: {unique: true, dropDups: true}} member_ids : [ type:ObjectId, ref:'User' ] invited_emails: [ String ] + teamInvites : [ TeamInviteSchema ] recurlySubscription_id : String planCode : {type: String} groupPlan : {type: Boolean, default: false} diff --git a/services/web/app/coffee/models/TeamInvite.coffee b/services/web/app/coffee/models/TeamInvite.coffee index ba0d0dc288..ff4665cce5 100644 --- a/services/web/app/coffee/models/TeamInvite.coffee +++ b/services/web/app/coffee/models/TeamInvite.coffee @@ -5,11 +5,9 @@ Schema = mongoose.Schema ObjectId = Schema.ObjectId TeamInviteSchema = new Schema - subscriptionId : { type: ObjectId, ref: 'Subscription', required: true } email : { type: String, required: true } - token : { type: String, required: true } - sentAt : { type: Date, required: true } - + token : { type: String } + sentAt : { type: Date } mongoose.model 'TeamInvite', TeamInviteSchema exports.TeamInvite = mongoose.model 'TeamInvite' diff --git a/services/web/app/views/subscriptions/group/team_invite.pug b/services/web/app/views/subscriptions/group/team_invite.pug index ae065ea64b..d396bb4155 100644 --- a/services/web/app/views/subscriptions/group/team_invite.pug +++ b/services/web/app/views/subscriptions/group/team_invite.pug @@ -20,7 +20,7 @@ block content .col-md-8.col-md-offset-2(ng-cloak) .card(ng-controller="TeamInviteController") .page-header - h1.text-centered #{translate("invited_to_group", {inviterName: inviterName})} + h1.text-centered #{translate("invited_to_group", {inviterName: inviterName, appName: appName})} div(ng-show="view =='personalSubscription'").row.text-centered.message p #{translate("cancel_personal_subscription_first")} @@ -31,10 +31,19 @@ block content a.btn.btn.btn-primary(ng-click="cancelPersonalSubscription()", ng-disabled="inflight") #{translate("cancel_your_subscription")} div(ng-show="view =='teamInvite'").row.text-centered.message - p #{translate("accept_invite_to_join_team", {inviterName: inviterName})} + p #{translate("accept_invitation_gives_premium_account")} .row .col-md-12 .text-center a.btn.btn-default(href="/project") #{translate("not_now")} span   - a.btn.btn.btn-primary(ng-click="joinTeam()", ng-disabled="inflight") #{translate("join_team")} + a.btn.btn.btn-primary(ng-click="joinTeam()", ng-disabled="inflight") #{translate("accept_invitation")} + + div(ng-show="view =='inviteAccepted'").row.text-centered.text-center + .row + .col-md-12 You have joined the team managed by John Smith + .row + .col-md-12   + .row + .col-md-12 + a.btn.btn.btn-primary(href="/project") #{translate("done")} diff --git a/services/web/public/coffee/main/group-members.coffee b/services/web/public/coffee/main/group-members.coffee index 94300f8ce5..1d7e7ff104 100644 --- a/services/web/public/coffee/main/group-members.coffee +++ b/services/web/public/coffee/main/group-members.coffee @@ -35,7 +35,7 @@ define [ for user in $scope.selectedUsers do (user) -> if user.holdingAccount and !user._id? - url = "/subscription/group/email/#{encodeURIComponent(user.email)}" + url = "/subscription/invites/#{encodeURIComponent(user.email)}" else url = "/subscription/group/user/#{user._id}" queuedHttp({ diff --git a/services/web/public/coffee/main/subscription/team-invite-controller.coffee b/services/web/public/coffee/main/subscription/team-invite-controller.coffee index ab6b42c658..477078430b 100644 --- a/services/web/public/coffee/main/subscription/team-invite-controller.coffee +++ b/services/web/public/coffee/main/subscription/team-invite-controller.coffee @@ -23,12 +23,12 @@ define [ console.log "the request failed" $scope.joinTeam = -> - $scope.view = "requestSent" $scope.inflight = true - request = $http.put "/subscription/invites/:token/", {_csrf:window.csrfToken} + request = $http.put "/subscription/invites/#{window.inviteToken}/", {_csrf:window.csrfToken} request.then (response)-> { status } = response $scope.inflight = false + $scope.view = "inviteAccepted" if status != 200 # assume request worked $scope.requestSent = false request.catch ()-> From 39c8595c279fc649ed9e6c571f4339cbbd447afe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 31 May 2018 14:25:47 +0100 Subject: [PATCH 04/44] Split SubscriptionGroupInvites and rename to DomainSubscriptionJoin To not cause confusion with team invites. They are not really an invite because they are user initiated, but more of a user choosing to join a team. --- .../DomainSubscriptionController.coffee | 45 ++++++++++++++ .../SubscriptionGroupController.coffee | 62 ------------------- .../Subscription/SubscriptionRouter.coffee | 8 +-- .../group/{invite.pug => join.pug} | 4 +- .../subscriptions/group/successful_join.pug | 25 -------- services/web/public/coffee/main.coffee | 2 +- ...omain-subscription-join-controller.coffee} | 16 ++--- 7 files changed, 60 insertions(+), 102 deletions(-) create mode 100644 services/web/app/coffee/Features/Subscription/DomainSubscriptionController.coffee rename services/web/app/views/subscriptions/group/{invite.pug => join.pug} (92%) delete mode 100644 services/web/app/views/subscriptions/group/successful_join.pug rename services/web/public/coffee/main/subscription/{group-subscription-invite-controller.coffee => domain-subscription-join-controller.coffee} (67%) diff --git a/services/web/app/coffee/Features/Subscription/DomainSubscriptionController.coffee b/services/web/app/coffee/Features/Subscription/DomainSubscriptionController.coffee new file mode 100644 index 0000000000..aa78c103b7 --- /dev/null +++ b/services/web/app/coffee/Features/Subscription/DomainSubscriptionController.coffee @@ -0,0 +1,45 @@ +SubscriptionGroupHandler = require("./SubscriptionGroupHandler") +logger = require("logger-sharelatex") +SubscriptionLocator = require("./SubscriptionLocator") +ErrorsController = require("../Errors/ErrorController") +SubscriptionDomainHandler = require("./SubscriptionDomainHandler") +AuthenticationController = require('../Authentication/AuthenticationController') +async = require("async") + +module.exports = + newInvite: (req, res)-> + group_subscription_id = req.params.subscription_id + user_id = AuthenticationController.getLoggedInUserId(req) + licence = SubscriptionDomainHandler.findDomainLicenceBySubscriptionId(group_subscription_id) + if !licence? + return ErrorsController.notFound(req, res) + jobs = + partOfGroup: (cb)-> + SubscriptionGroupHandler.isUserPartOfGroup user_id, licence.group_subscription_id, cb + subscription: (cb)-> + SubscriptionLocator.getUsersSubscription user_id, cb + async.series jobs, (err, results)-> + {partOfGroup, subscription} = results + if partOfGroup + return res.redirect("/user/subscription/custom_account") + else + res.render "subscriptions/group/join", + title: "Group Invitation" + group_subscription_id:group_subscription_id + licenceName:licence.name + has_personal_subscription: subscription? + + createInvite: (req, res)-> + subscription_id = req.params.subscription_id + currentUser = AuthenticationController.getSessionUser(req) + if !currentUser? + logger.err {subscription_id}, "error getting current user" + return res.sendStatus 500 + licence = SubscriptionDomainHandler.findDomainLicenceBySubscriptionId(subscription_id) + if !licence? + return ErrorsController.notFound(req, res) + SubscriptionGroupHandler.sendVerificationEmail subscription_id, licence.name, currentUser.email, (err)-> + if err? + res.sendStatus 500 + else + res.sendStatus 200 diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee index 6b8bd99c35..ffb5c6e832 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee @@ -62,68 +62,6 @@ module.exports = users: users subscription: subscription - renderGroupInvitePage: (req, res)-> - group_subscription_id = req.params.subscription_id - user_id = AuthenticationController.getLoggedInUserId(req) - licence = SubscriptionDomainHandler.findDomainLicenceBySubscriptionId(group_subscription_id) - if !licence? - return ErrorsController.notFound(req, res) - jobs = - partOfGroup: (cb)-> - SubscriptionGroupHandler.isUserPartOfGroup user_id, licence.group_subscription_id, cb - subscription: (cb)-> - SubscriptionLocator.getUsersSubscription user_id, cb - async.series jobs, (err, results)-> - {partOfGroup, subscription} = results - if partOfGroup - return res.redirect("/user/subscription/custom_account") - else - res.render "subscriptions/group/invite", - title: "Group Invitation" - group_subscription_id:group_subscription_id - licenceName:licence.name - has_personal_subscription: subscription? - - beginJoinGroup: (req, res)-> - subscription_id = req.params.subscription_id - currentUser = AuthenticationController.getSessionUser(req) - if !currentUser? - logger.err {subscription_id}, "error getting current user" - return res.sendStatus 500 - licence = SubscriptionDomainHandler.findDomainLicenceBySubscriptionId(subscription_id) - if !licence? - return ErrorsController.notFound(req, res) - SubscriptionGroupHandler.sendVerificationEmail subscription_id, licence.name, currentUser.email, (err)-> - if err? - res.sendStatus 500 - else - res.sendStatus 200 - - completeJoin: (req, res)-> - currentUser = AuthenticationController.getSessionUser(req) - subscription_id = req.params.subscription_id - if !SubscriptionDomainHandler.findDomainLicenceBySubscriptionId(subscription_id)? - return ErrorsController.notFound(req, res) - email = currentUser?.email - logger.log subscription_id:subscription_id, user_id:currentUser?._id, email:email, "starting the completion of joining group" - SubscriptionGroupHandler.processGroupVerification email, subscription_id, req.query?.token, (err)-> - if err? and err == "token_not_found" - return res.redirect "/user/subscription/#{subscription_id}/group/invited?expired=true" - else if err? - return res.sendStatus 500 - else - logger.log subscription_id:subscription_id, email:email, "user successful completed join of group subscription" - return res.redirect "/user/subscription/#{subscription_id}/group/successful-join" - - renderSuccessfulJoinPage: (req, res)-> - subscription_id = req.params.subscription_id - licence = SubscriptionDomainHandler.findDomainLicenceBySubscriptionId(subscription_id) - if !SubscriptionDomainHandler.findDomainLicenceBySubscriptionId(subscription_id)? - return ErrorsController.notFound(req, res) - res.render "subscriptions/group/successful_join", - title: "Sucessfully joined group" - licenceName:licence.name - exportGroupCsv: (req, res)-> user_id = AuthenticationController.getLoggedInUserId(req) logger.log user_id: user_id, "exporting group csv" diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee index 5aabd191de..b853a5c2bf 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee @@ -1,6 +1,7 @@ AuthenticationController = require('../Authentication/AuthenticationController') SubscriptionController = require('./SubscriptionController') SubscriptionGroupController = require './SubscriptionGroupController' +DomainSubscriptionController = require './DomainSubscriptionController' TeamInvitesController = require './TeamInvitesController' Settings = require "settings-sharelatex" @@ -36,10 +37,9 @@ module.exports = webRouter.delete '/subscription/invites/:email/', AuthenticationController.requireLogin(), TeamInvitesController.revokeInvite - webRouter.get '/user/subscription/:subscription_id/group/invited', AuthenticationController.requireLogin(), SubscriptionGroupController.renderGroupInvitePage - webRouter.post '/user/subscription/:subscription_id/group/begin-join', AuthenticationController.requireLogin(), SubscriptionGroupController.beginJoinGroup - webRouter.get '/user/subscription/:subscription_id/group/complete-join', AuthenticationController.requireLogin(), SubscriptionGroupController.completeJoin - webRouter.get '/user/subscription/:subscription_id/group/successful-join', AuthenticationController.requireLogin(), SubscriptionGroupController.renderSuccessfulJoinPage + # Routes to join a domain licence team + webRouter.get '/user/subscription/:subscription_id/group/join', AuthenticationController.requireLogin(), DomainSubscriptionController.newInvite + webRouter.post '/user/subscription/:subscription_id/group/join', AuthenticationController.requireLogin(), DomainSubscriptionController.createInvite #recurly callback publicApiRouter.post '/user/subscription/callback', SubscriptionController.recurlyNotificationParser, SubscriptionController.recurlyCallback diff --git a/services/web/app/views/subscriptions/group/invite.pug b/services/web/app/views/subscriptions/group/join.pug similarity index 92% rename from services/web/app/views/subscriptions/group/invite.pug rename to services/web/app/views/subscriptions/group/join.pug index 3c67a79f81..7c6c214b98 100644 --- a/services/web/app/views/subscriptions/group/invite.pug +++ b/services/web/app/views/subscriptions/group/join.pug @@ -17,7 +17,7 @@ block content div   .row .col-md-8.col-md-offset-2(ng-cloak) - .card(ng-controller="GroupSubscriptionInviteController") + .card(ng-controller="DomainSubscriptionJoinController") .page-header h1.text-centered #{translate("you_are_invited_to_group", {groupName:licenceName})} @@ -31,7 +31,7 @@ block content span   a.btn.btn.btn-primary(ng-click="cancelSubscription()", ng-disabled="inflight") #{translate("cancel_your_subscription")} - div(ng-show="view =='groupSubscriptionInvite'").row.text-centered + div(ng-show="view =='domainSubscriptionJoin'").row.text-centered .row .col-md-12 #{translate("group_provides_you_with_premium_account", {groupName:licenceName})} .row diff --git a/services/web/app/views/subscriptions/group/successful_join.pug b/services/web/app/views/subscriptions/group/successful_join.pug deleted file mode 100644 index 9e748f4c38..0000000000 --- a/services/web/app/views/subscriptions/group/successful_join.pug +++ /dev/null @@ -1,25 +0,0 @@ -extends ../../layout - -block scripts - script(type='text/javascript'). - window.subscription_id = '#{subscription_id}' - -block content - .content.content-alt - .container - .row - .col-md-8.col-md-offset-2(ng-cloak) - .card - .page-header.row.text-centered - h1 #{translate("you_have_joined", {groupName:licenceName})} - div(ng-show="!requestSent").row.text-centered - .row - .span-md-12 #{translate("claim_premium_account", {groupName:licenceName})} - div - .row - .col-md-12   - .row - .span-md-12 - a.btn.btn-success(href="/project") #{translate("done")} - - diff --git a/services/web/public/coffee/main.coffee b/services/web/public/coffee/main.coffee index 6034ed1bdd..786bf1a94b 100644 --- a/services/web/public/coffee/main.coffee +++ b/services/web/public/coffee/main.coffee @@ -16,7 +16,7 @@ define [ "main/annual-upgrade" "main/announcements" "main/register-users" - "main/subscription/group-subscription-invite-controller" + "main/subscription/domain-subscription-join-controller" "main/subscription/team-invite-controller" "main/contact-us" "main/learn" diff --git a/services/web/public/coffee/main/subscription/group-subscription-invite-controller.coffee b/services/web/public/coffee/main/subscription/domain-subscription-join-controller.coffee similarity index 67% rename from services/web/public/coffee/main/subscription/group-subscription-invite-controller.coffee rename to services/web/public/coffee/main/subscription/domain-subscription-join-controller.coffee index eae5d68cc0..99150942e1 100644 --- a/services/web/public/coffee/main/subscription/group-subscription-invite-controller.coffee +++ b/services/web/public/coffee/main/subscription/domain-subscription-join-controller.coffee @@ -1,35 +1,35 @@ define [ "base" ], (App) -> - App.controller "GroupSubscriptionInviteController", ($scope, $http) -> + App.controller "DomainSubscriptionJoinController", ($scope, $http) -> $scope.inflight = false if has_personal_subscription $scope.view = "personalSubscription" - else - $scope.view = "groupSubscriptionInvite" + else + $scope.view = "domainSubscriptionJoin" $scope.keepPersonalSubscription = -> - $scope.view = "groupSubscriptionInvite" + $scope.view = "domainSubscriptionJoin" $scope.cancelSubscription = -> $scope.inflight = true request = $http.post "/user/subscription/cancel", {_csrf:window.csrfToken} request.then ()-> $scope.inflight = false - $scope.view = "groupSubscriptionInvite" + $scope.view = "domainSubscriptionJoin" request.catch ()-> - console.log "the request failed" + console.log "the request failed" $scope.joinGroup = -> $scope.view = "requestSent" $scope.inflight = true - request = $http.post "/user/subscription/#{group_subscription_id}/group/begin-join", {_csrf:window.csrfToken} + request = $http.post "/user/subscription/#{group_subscription_id}/group/join", {_csrf:window.csrfToken} request.then (response)-> { status } = response $scope.inflight = false if status != 200 # assume request worked $scope.requestSent = false request.catch ()-> - console.log "the request failed" \ No newline at end of file + console.log "the request failed" From d262de14d6f3d88983d45a15072e9f0b194ad930 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 31 May 2018 16:15:47 +0100 Subject: [PATCH 05/44] Use team invites to join domain licensed teams --- .../DomainLicenceController.coffee | 47 ++++++++ .../DomainSubscriptionController.coffee | 45 -------- .../SubscriptionDomainHandler.coffee | 2 +- .../Subscription/SubscriptionRouter.coffee | 6 +- .../Subscription/TeamInvitesController.coffee | 18 +-- .../Subscription/TeamInvitesHandler.coffee | 106 ++++++++---------- .../web/app/coffee/models/TeamInvite.coffee | 1 + .../subscriptions/{group => domain}/join.pug | 0 .../team_invite.pug => team/invite.pug} | 0 ...domain-subscription-join-controller.coffee | 2 +- 10 files changed, 109 insertions(+), 118 deletions(-) create mode 100644 services/web/app/coffee/Features/Subscription/DomainLicenceController.coffee delete mode 100644 services/web/app/coffee/Features/Subscription/DomainSubscriptionController.coffee rename services/web/app/views/subscriptions/{group => domain}/join.pug (100%) rename services/web/app/views/subscriptions/{group/team_invite.pug => team/invite.pug} (100%) diff --git a/services/web/app/coffee/Features/Subscription/DomainLicenceController.coffee b/services/web/app/coffee/Features/Subscription/DomainLicenceController.coffee new file mode 100644 index 0000000000..b2d3a1d2c6 --- /dev/null +++ b/services/web/app/coffee/Features/Subscription/DomainLicenceController.coffee @@ -0,0 +1,47 @@ +SubscriptionGroupHandler = require("./SubscriptionGroupHandler") +logger = require("logger-sharelatex") +SubscriptionLocator = require("./SubscriptionLocator") +ErrorsController = require("../Errors/ErrorController") +SubscriptionDomainHandler = require("./SubscriptionDomainHandler") +AuthenticationController = require('../Authentication/AuthenticationController') +TeamInvitesHandler = require('./TeamInvitesHandler') + +async = require("async") + +module.exports = + join: (req, res)-> + user = AuthenticationController.getSessionUser(req) + licence = SubscriptionDomainHandler.getLicenceUserCanJoin(user) + + if !licence? + return ErrorsController.notFound(req, res) + + jobs = + partOfGroup: (cb)-> + SubscriptionGroupHandler.isUserPartOfGroup user.id, licence.group_subscription_id, cb + subscription: (cb)-> + SubscriptionLocator.getUsersSubscription user.id, cb + + async.series jobs, (err, results)-> + { partOfGroup, subscription } = results + if partOfGroup + return res.redirect("/user/subscription/custom_account") + else + res.render "subscriptions/domain/join", + title: "Group Invitation" + group_subscription_id: licence.group_subscription_id + licenceName: licence.name + has_personal_subscription: subscription? + + createInvite: (req, res)-> + user = AuthenticationController.getSessionUser(req) + licence = SubscriptionDomainHandler.getLicenceUserCanJoin(user) + + if !licence? + return ErrorsController.notFound(req, res) + + TeamInvitesHandler.createDomainInvite user, licence, (err) -> + if err? + res.sendStatus 500 + else + res.sendStatus 200 diff --git a/services/web/app/coffee/Features/Subscription/DomainSubscriptionController.coffee b/services/web/app/coffee/Features/Subscription/DomainSubscriptionController.coffee deleted file mode 100644 index aa78c103b7..0000000000 --- a/services/web/app/coffee/Features/Subscription/DomainSubscriptionController.coffee +++ /dev/null @@ -1,45 +0,0 @@ -SubscriptionGroupHandler = require("./SubscriptionGroupHandler") -logger = require("logger-sharelatex") -SubscriptionLocator = require("./SubscriptionLocator") -ErrorsController = require("../Errors/ErrorController") -SubscriptionDomainHandler = require("./SubscriptionDomainHandler") -AuthenticationController = require('../Authentication/AuthenticationController') -async = require("async") - -module.exports = - newInvite: (req, res)-> - group_subscription_id = req.params.subscription_id - user_id = AuthenticationController.getLoggedInUserId(req) - licence = SubscriptionDomainHandler.findDomainLicenceBySubscriptionId(group_subscription_id) - if !licence? - return ErrorsController.notFound(req, res) - jobs = - partOfGroup: (cb)-> - SubscriptionGroupHandler.isUserPartOfGroup user_id, licence.group_subscription_id, cb - subscription: (cb)-> - SubscriptionLocator.getUsersSubscription user_id, cb - async.series jobs, (err, results)-> - {partOfGroup, subscription} = results - if partOfGroup - return res.redirect("/user/subscription/custom_account") - else - res.render "subscriptions/group/join", - title: "Group Invitation" - group_subscription_id:group_subscription_id - licenceName:licence.name - has_personal_subscription: subscription? - - createInvite: (req, res)-> - subscription_id = req.params.subscription_id - currentUser = AuthenticationController.getSessionUser(req) - if !currentUser? - logger.err {subscription_id}, "error getting current user" - return res.sendStatus 500 - licence = SubscriptionDomainHandler.findDomainLicenceBySubscriptionId(subscription_id) - if !licence? - return ErrorsController.notFound(req, res) - SubscriptionGroupHandler.sendVerificationEmail subscription_id, licence.name, currentUser.email, (err)-> - if err? - res.sendStatus 500 - else - res.sendStatus 200 diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionDomainHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionDomainHandler.coffee index a71d4496e7..ac07350338 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionDomainHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionDomainHandler.coffee @@ -15,7 +15,7 @@ module.exports = SubscriptionDomainHandler = getDomainLicencePage: (user)-> licence = SubscriptionDomainHandler._findDomainLicence(user.email) if licence?.verifyEmail - return "/user/subscription/#{licence.subscription_id}/group/invited" + return "/user/subscription/domain/join" else return undefined diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee index b853a5c2bf..cf4bd73be9 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee @@ -1,7 +1,7 @@ AuthenticationController = require('../Authentication/AuthenticationController') SubscriptionController = require('./SubscriptionController') SubscriptionGroupController = require './SubscriptionGroupController' -DomainSubscriptionController = require './DomainSubscriptionController' +DomainLicenceController = require './DomainLicenceController' TeamInvitesController = require './TeamInvitesController' Settings = require "settings-sharelatex" @@ -38,8 +38,8 @@ module.exports = TeamInvitesController.revokeInvite # Routes to join a domain licence team - webRouter.get '/user/subscription/:subscription_id/group/join', AuthenticationController.requireLogin(), DomainSubscriptionController.newInvite - webRouter.post '/user/subscription/:subscription_id/group/join', AuthenticationController.requireLogin(), DomainSubscriptionController.createInvite + webRouter.get '/user/subscription/domain/join', AuthenticationController.requireLogin(), DomainLicenceController.join + webRouter.post '/user/subscription/domain/join', AuthenticationController.requireLogin(), DomainLicenceController.createInvite #recurly callback publicApiRouter.post '/user/subscription/callback', SubscriptionController.recurlyNotificationParser, SubscriptionController.recurlyCallback diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee index 17094adaa6..7e44e319db 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee @@ -2,6 +2,7 @@ settings = require "settings-sharelatex" logger = require("logger-sharelatex") TeamInvitesHandler = require('./TeamInvitesHandler') AuthenticationController = require("../Authentication/AuthenticationController") +SubscriptionLocator = require("./SubscriptionLocator") ErrorController = require("../Errors/ErrorController") module.exports = @@ -20,19 +21,20 @@ module.exports = token = req.params.token userId = AuthenticationController.getLoggedInUserId(req) - TeamInvitesHandler.getInviteDetails token, userId, (err, results) -> + TeamInvitesHandler.getInvite token, (err, invite, teamSubscription) -> next(err) if err? - { invite, personalSubscription, inviterName } = results - unless invite? return ErrorController.notFound(req, res, next) - res.render "subscriptions/group/team_invite", - inviterName: inviterName - inviteToken: invite.token - hasPersonalSubscription: personalSubscription? - appName: settings.appName + SubscriptionLocator.getUsersSubscription userId, (err, personalSubscription) -> + return callback(err) if err? + + res.render "subscriptions/team/invite", + inviterName: invite.inviterName + inviteToken: invite.token + hasPersonalSubscription: personalSubscription? + appName: settings.appName acceptInvite: (req, res, next) -> diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index 66ccd24a5c..4a4cf37357 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -19,47 +19,39 @@ module.exports = TeamInvitesHandler = getInvites: (subscriptionId, callback) -> TeamInvite.find(subscriptionId: subscriptionId, callback) - createInvite: (teamManagerId, email, callback) -> + getInvite: (token, callback) -> + Subscription.findOne 'teamInvites.token': token, (err, subscription) -> + return callback(err, subscription) if err? + return callback(teamNotFound: true) unless subscription? + + invite = subscription.teamInvites.find (i) -> i.token == token + return callback(null, invite, subscription) + + createManagerInvite: (teamManagerId, email, callback) -> UserLocator.findById teamManagerId, (error, teamManager) -> + return callback(error) if error? + SubscriptionLocator.getUsersSubscription teamManagerId, (error, subscription) -> return callback(error) if error? - if LimitationsManager.teamHasReachedMemberLimit(subscription) - return callback(limitReached: true) + if teamManager.first_name and teamManager.last_name + inviterName = "#{teamManager.first_name} #{teamManager.last_name} (#{teamManager.email})" + else + inviterName = teamManager.email - existingInvite = subscription.teamInvites.find (invite) -> invite.email == email + TeamInvitesHandler.createInvite(subscription, email, inviterName, callback) - if existingInvite - return callback(alreadyInvited: true) - - inviterName = TeamInvitesHandler.inviterName(teamManager) - token = crypto.randomBytes(32).toString("hex") - - invite = { - email: email, - token: token, - sentAt: new Date(), - } - - subscription.teamInvites.push(invite) - - subscription.save (error) -> - return callback(error) if error? - - # TODO: use standard way to canonalise email addresses - opts = - to: email.trim().toLowerCase() - inviterName: inviterName - acceptInviteUrl: "#{settings.siteUrl}/subscription/invites/#{token}/" - EmailHandler.sendEmail "verifyEmailToJoinTeam", opts, (error) -> - return callback(error, invite) + createDomainInvite: (user, licence, callback) -> + SubscriptionLocator.getSubscription licence.subscription_id, (error, subscription) -> + return callback(error) if error? + TeamInvitesHandler.createInvite(subscription, user.email, licence.name, callback) acceptInvite: (token, userId, callback) -> - TeamInvitesHandler.getInviteAndManager token, (err, invite, subscription, teamManager) -> + TeamInvitesHandler.getInvite token, (err, invite, subscription) -> return callback(err) if err? - return callback(inviteNoLongerValid: true) unless invite? and teamManager? + return callback(inviteNoLongerValid: true) unless invite? - SubscriptionUpdater.addUserToGroup teamManager, userId, (err) -> + SubscriptionUpdater.addUserToGroup subscription.admin_id, userId, (err) -> return callback(err) if err? TeamInvitesHandler.removeInviteFromTeam(subscription.id, invite.email, callback) @@ -70,45 +62,39 @@ module.exports = TeamInvitesHandler = TeamInvitesHandler.removeInviteFromTeam(teamSubscription.id, email, callback) - getInviteDetails: (token, userId, callback) -> - TeamInvitesHandler.getInviteAndManager token, (err, invite, teamSubscription, teamManager) -> - return callback(err) if err? + createInvite: (subscription, email, inviterName, callback) -> + if LimitationsManager.teamHasReachedMemberLimit(subscription) + return callback(limitReached: true) - SubscriptionLocator.getUsersSubscription userId, (err, personalSubscription) -> - return callback(err) if err? + existingInvite = subscription.teamInvites.find (invite) -> invite.email == email - return callback(null , { - invite: invite, - personalSubscription: personalSubscription, - team: teamSubscription, - inviterName: TeamInvitesHandler.inviterName(teamManager), - teamManager: teamManager - }) + if existingInvite + return callback(alreadyInvited: true) - getInviteAndManager: (token, callback) -> - TeamInvitesHandler.getInvite token, (err, invite, teamSubscription) -> - return callback(err) if err? + token = crypto.randomBytes(32).toString("hex") - UserLocator.findById teamSubscription.admin_id, (err, teamManager) -> - return callback(err, invite, teamSubscription, teamManager) + invite = { + email: email, + token: token, + inviterName: inviterName, + sentAt: new Date(), + } - getInvite: (token, callback) -> - Subscription.findOne 'teamInvites.token': token, (err, subscription) -> - return callback(err, subscription) if err? - return callback(teamNotFound: true) unless subscription? + subscription.teamInvites.push(invite) - invite = subscription.teamInvites.find (i) -> i.token == token - return callback(null, invite, subscription) + subscription.save (error) -> + return callback(error) if error? + # TODO: use standard way to canonalise email addresses + opts = + to: email.trim().toLowerCase() + inviterName: inviterName + acceptInviteUrl: "#{settings.siteUrl}/subscription/invites/#{token}/" + EmailHandler.sendEmail "verifyEmailToJoinTeam", opts, (error) -> + return callback(error, invite) removeInviteFromTeam: (subscriptionId, email, callback) -> searchConditions = { _id: new ObjectId(subscriptionId.toString()) } updateOp = { $pull: { teamInvites: { email: email.trim().toLowerCase() } } } Subscription.update(searchConditions, updateOp, callback) - - inviterName: (teamManager) -> - if teamManager.first_name and teamManager.last_name - "#{teamManager.first_name} #{teamManager.last_name} (#{teamManager.email})" - else - teamManager.email diff --git a/services/web/app/coffee/models/TeamInvite.coffee b/services/web/app/coffee/models/TeamInvite.coffee index ff4665cce5..e488724745 100644 --- a/services/web/app/coffee/models/TeamInvite.coffee +++ b/services/web/app/coffee/models/TeamInvite.coffee @@ -7,6 +7,7 @@ ObjectId = Schema.ObjectId TeamInviteSchema = new Schema email : { type: String, required: true } token : { type: String } + inviterName : { type: String } sentAt : { type: Date } mongoose.model 'TeamInvite', TeamInviteSchema diff --git a/services/web/app/views/subscriptions/group/join.pug b/services/web/app/views/subscriptions/domain/join.pug similarity index 100% rename from services/web/app/views/subscriptions/group/join.pug rename to services/web/app/views/subscriptions/domain/join.pug diff --git a/services/web/app/views/subscriptions/group/team_invite.pug b/services/web/app/views/subscriptions/team/invite.pug similarity index 100% rename from services/web/app/views/subscriptions/group/team_invite.pug rename to services/web/app/views/subscriptions/team/invite.pug diff --git a/services/web/public/coffee/main/subscription/domain-subscription-join-controller.coffee b/services/web/public/coffee/main/subscription/domain-subscription-join-controller.coffee index 99150942e1..b2af052ca1 100644 --- a/services/web/public/coffee/main/subscription/domain-subscription-join-controller.coffee +++ b/services/web/public/coffee/main/subscription/domain-subscription-join-controller.coffee @@ -25,7 +25,7 @@ define [ $scope.joinGroup = -> $scope.view = "requestSent" $scope.inflight = true - request = $http.post "/user/subscription/#{group_subscription_id}/group/join", {_csrf:window.csrfToken} + request = $http.post "/user/subscription/domain/join", {_csrf:window.csrfToken} request.then (response)-> { status } = response $scope.inflight = false From 1c485c188416e80172f75a6f5b31c9cbb7ee3c58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 31 May 2018 16:42:09 +0100 Subject: [PATCH 06/44] Improve error handling --- .../Subscription/TeamInvitesController.coffee | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee index 7e44e319db..ef1d90aa6a 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee @@ -11,7 +11,7 @@ module.exports = email = req.body.email TeamInvitesHandler.createInvite teamManagerId, email, (err, invite) -> - next(err) if err? + return handleError(err, req, res, next) if err? inviteView = { user: { email: invite.email, sentAt: invite.sentAt, holdingAccount: true } } @@ -22,13 +22,13 @@ module.exports = userId = AuthenticationController.getLoggedInUserId(req) TeamInvitesHandler.getInvite token, (err, invite, teamSubscription) -> - next(err) if err? + return handleError(err, req, res, next) if err? - unless invite? + if !invite return ErrorController.notFound(req, res, next) SubscriptionLocator.getUsersSubscription userId, (err, personalSubscription) -> - return callback(err) if err? + return handleError(err, req, res, next) if err? res.render "subscriptions/team/invite", inviterName: invite.inviterName @@ -36,13 +36,12 @@ module.exports = hasPersonalSubscription: personalSubscription? appName: settings.appName - acceptInvite: (req, res, next) -> token = req.params.token userId = AuthenticationController.getLoggedInUserId(req) TeamInvitesHandler.acceptInvite token, userId, (err, results) -> - next(err) if err? + return handleError(err, req, res, next) if err? res.sendStatus 204 @@ -51,6 +50,12 @@ module.exports = teamManagerId = AuthenticationController.getLoggedInUserId(req) TeamInvitesHandler.revokeInvite teamManagerId, email, (err, results) -> - next(err) if err? + return handleError(err, req, res, next) if err? res.sendStatus 204 + +handleError = (err, req, res, next) -> + if err.teamNotFound or err.inviteNoLongerValid + ErrorController.notFound(req, res, next) + else + next(err) From 385fec191462de379f5662ac78b9ce56a73873d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Fri, 1 Jun 2018 11:23:25 +0100 Subject: [PATCH 07/44] Clean up code that uses invited_emails --- .../Subscription/LimitationsManager.coffee | 9 +- .../SubscriptionGroupController.coffee | 8 -- .../SubscriptionGroupHandler.coffee | 38 +------ .../Subscription/SubscriptionLocator.coffee | 2 +- .../Subscription/SubscriptionRouter.coffee | 1 - .../Subscription/SubscriptionUpdater.coffee | 23 +--- .../Subscription/TeamInvitesController.coffee | 2 +- .../Subscription/TeamInvitesHandler.coffee | 61 +++++++---- .../LimitationsManagerTests.coffee | 6 +- .../SubscriptionGroupControllerTests.coffee | 103 ------------------ .../SubscriptionGroupHandlerTests.coffee | 77 ++++--------- .../TeamInvitesHandlerTests.coffee | 22 ++++ 12 files changed, 96 insertions(+), 256 deletions(-) create mode 100644 services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee diff --git a/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee b/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee index f09dde2453..9b5137392f 100644 --- a/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee +++ b/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee @@ -6,7 +6,7 @@ Settings = require("settings-sharelatex") CollaboratorsHandler = require("../Collaborators/CollaboratorsHandler") CollaboratorsInvitesHandler = require("../Collaborators/CollaboratorsInviteHandler") -module.exports = +module.exports = LimitationsManager = allowedNumberOfCollaboratorsInProject: (project_id, callback) -> ProjectGetter.getProject project_id, owner_ref: true, (error, project) => return callback(error) if error? @@ -56,7 +56,7 @@ module.exports = callback err, subscriptions.length > 0, subscriptions teamHasReachedMemberLimit: (subscription) -> - currentTotal = (subscription.member_ids or []).length + (subscription.team_invites or []).length + currentTotal = (subscription.member_ids or []).length + (subscription.teamInvites or []).length return currentTotal >= subscription.membersLimit hasGroupMembersLimitReached: (user_id, callback = (err, limitReached, subscription)->)-> @@ -67,7 +67,6 @@ module.exports = if !subscription? logger.err user_id:user_id, "no subscription found for user" return callback("no subscription found") - currentTotal = (subscription.member_ids or []).length + (subscription.invited_emails or []).length - limitReached = currentTotal >= subscription.membersLimit - logger.log user_id:user_id, limitReached:limitReached, currentTotal: currentTotal, membersLimit: subscription.membersLimit, "checking if subscription members limit has been reached" + + limitReached = LimitationsManager.teamHasReachedMemberLimit(subscription) callback(err, limitReached, subscription) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee index ffb5c6e832..8e84f1fb0b 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee @@ -33,14 +33,6 @@ module.exports = return res.sendStatus 500 res.send() - removeEmailInviteFromGroup: (req, res)-> - adminUserId = AuthenticationController.getLoggedInUserId(req) - email = req.params.email - logger.log {adminUserId, email}, "removing email invite from group subscription" - SubscriptionGroupHandler.removeEmailInviteFromGroup adminUserId, email, (err)-> - return next(error) if error? - res.send() - removeSelfFromGroup: (req, res)-> adminUserId = req.query.admin_user_id userToRemove_id = AuthenticationController.getLoggedInUserId(req) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index d893bff7a1..b34dacc3e6 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -33,7 +33,7 @@ module.exports = SubscriptionGroupHandler = userViewModel = buildUserViewModel(user) callback(err, userViewModel) else - SubscriptionUpdater.addEmailInviteToGroup adminUserId, newEmail, (err) -> + TeamInvitesHandler.createManagerInvite adminUserId, newEmail, (err) -> return callback(err) if err? userViewModel = buildEmailInviteViewModel(newEmail) callback(err, userViewModel) @@ -41,16 +41,11 @@ module.exports = SubscriptionGroupHandler = removeUserFromGroup: (adminUser_id, userToRemove_id, callback)-> SubscriptionUpdater.removeUserFromGroup adminUser_id, userToRemove_id, callback - removeEmailInviteFromGroup: (adminUser_id, email, callback) -> - SubscriptionUpdater.removeEmailInviteFromGroup adminUser_id, email, callback - getPopulatedListOfMembers: (adminUser_id, callback)-> SubscriptionLocator.getUsersSubscription adminUser_id, (err, subscription)-> return callback(err) if err? users = [] - for email in subscription.invited_emails or [] - users.push buildEmailInviteViewModel(email) for teamInvite in subscription.teamInvites or [] users.push buildEmailInviteViewModel(teamInvite.email) @@ -86,37 +81,6 @@ module.exports = SubscriptionGroupHandler = completeJoinUrl: "#{settings.siteUrl}/user/subscription/#{subscription_id}/group/complete-join?token=#{token}" EmailHandler.sendEmail "completeJoinGroupAccount", opts, callback - processGroupVerification: (userEmail, subscription_id, token, callback)-> - logger.log userEmail:userEmail, subscription_id:subscription_id, "processing group verification for user" - OneTimeTokenHandler.getValueFromTokenAndExpire token, (err, token_subscription_id)-> - if err? or subscription_id != token_subscription_id - logger.err userEmail:userEmail, token:token, "token value not found for processing group verification" - return callback("token_not_found") - SubscriptionLocator.getSubscription subscription_id, (err, subscription)-> - if err? - logger.err err:err, subscription:subscription, userEmail:userEmail, subscription_id:subscription_id, "error getting subscription" - return callback(err) - if !subscription? - logger.warn subscription_id:subscription_id, userEmail:userEmail, "no subscription found" - return callback() - SubscriptionGroupHandler.addUserToGroup subscription?.admin_id, userEmail, callback - - convertEmailInvitesToMemberships: (email, user_id, callback = (err) ->) -> - SubscriptionLocator.getGroupsWithEmailInvite email, (err, groups = []) -> - return callback(err) if err? - logger.log {email, user_id, groups}, "found groups to convert from email invite to member" - jobs = [] - for group in groups - do (group) -> - jobs.push (cb) -> - SubscriptionUpdater.removeEmailInviteFromGroup group.admin_id, email, (err) -> - return cb(err) if err? - SubscriptionUpdater.addUserToGroup group.admin_id, user_id, (err) -> - return cb(err) if err? - logger.log {group_id: group._id, user_id, email}, "converted email invite to group membership" - return cb() - async.series jobs, callback - buildUserViewModel = (user)-> u = email: user.email diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee index 33376f504b..b0a6a5b021 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee @@ -32,4 +32,4 @@ module.exports = Subscription.find {member_ids: user_id}, {_id:1, planCode:1}, callback getGroupsWithEmailInvite: (email, callback) -> - Subscription.find { invited_emails: email }, callback \ No newline at end of file + Subscription.find { teamInvites: { email: email } }, callback diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee index cf4bd73be9..e512a1c3dd 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee @@ -24,7 +24,6 @@ module.exports = webRouter.post '/subscription/group/user', AuthenticationController.requireLogin(), SubscriptionGroupController.addUserToGroup webRouter.get '/subscription/group/export', AuthenticationController.requireLogin(), SubscriptionGroupController.exportGroupCsv webRouter.delete '/subscription/group/user/:user_id', AuthenticationController.requireLogin(), SubscriptionGroupController.removeUserFromGroup - webRouter.delete '/subscription/group/email/:email', AuthenticationController.requireLogin(), SubscriptionGroupController.removeEmailInviteFromGroup webRouter.delete '/subscription/group/user', AuthenticationController.requireLogin(), SubscriptionGroupController.removeSelfFromGroup # Team invites diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee index cb5c39d122..16dc5adf25 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee @@ -26,28 +26,20 @@ module.exports = SubscriptionUpdater = addUserToGroup: (adminUser_id, user_id, callback)-> logger.log adminUser_id:adminUser_id, user_id:user_id, "adding user into mongo subscription" - searchOps = + searchOps = admin_id: adminUser_id - insertOperation = + insertOperation = "$addToSet": {member_ids:user_id} Subscription.findAndModify searchOps, insertOperation, (err, subscription)-> if err? logger.err err:err, searchOps:searchOps, insertOperation:insertOperation, "error findy and modify add user to group" return callback(err) FeaturesUpdater.refreshFeatures user_id, callback - - addEmailInviteToGroup: (adminUser_id, email, callback) -> - logger.log {adminUser_id, email}, "adding email into mongo subscription" - searchOps = - admin_id: adminUser_id - insertOperation = - "$addToSet": {invited_emails: email} - Subscription.findAndModify searchOps, insertOperation, callback removeUserFromGroup: (adminUser_id, user_id, callback)-> - searchOps = + searchOps = admin_id: adminUser_id - removeOperation = + removeOperation = "$pull": {member_ids:user_id} Subscription.update searchOps, removeOperation, (err)-> if err? @@ -55,13 +47,6 @@ module.exports = SubscriptionUpdater = return callback(err) FeaturesUpdater.refreshFeatures user_id, callback - removeEmailInviteFromGroup: (adminUser_id, email, callback)-> - Subscription.update { - admin_id: adminUser_id - }, "$pull": { - invited_emails: email - }, callback - deleteSubscription: (subscription_id, callback = (error) ->) -> SubscriptionLocator.getSubscription subscription_id, (err, subscription) -> return callback(err) if err? diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee index ef1d90aa6a..565cdffcf1 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee @@ -10,7 +10,7 @@ module.exports = teamManagerId = AuthenticationController.getLoggedInUserId(req) email = req.body.email - TeamInvitesHandler.createInvite teamManagerId, email, (err, invite) -> + TeamInvitesHandler.createManagerInvite teamManagerId, email, (err, invite) -> return handleError(err, req, res, next) if err? inviteView = { user: { email: invite.email, sentAt: invite.sentAt, holdingAccount: true } diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index 4a4cf37357..37c99bafbe 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -1,5 +1,6 @@ logger = require("logger-sharelatex") crypto = require("crypto") +async = require("async") settings = require("settings-sharelatex") ObjectId = require("mongojs").ObjectId @@ -15,19 +16,16 @@ LimitationsManager = require("./LimitationsManager") EmailHandler = require("../Email/EmailHandler") module.exports = TeamInvitesHandler = - - getInvites: (subscriptionId, callback) -> - TeamInvite.find(subscriptionId: subscriptionId, callback) - getInvite: (token, callback) -> Subscription.findOne 'teamInvites.token': token, (err, subscription) -> - return callback(err, subscription) if err? + return callback(err) if err? return callback(teamNotFound: true) unless subscription? invite = subscription.teamInvites.find (i) -> i.token == token return callback(null, invite, subscription) createManagerInvite: (teamManagerId, email, callback) -> + logger.log {teamManagerId, email}, "Creating manager team invite" UserLocator.findById teamManagerId, (error, teamManager) -> return callback(error) if error? @@ -39,14 +37,16 @@ module.exports = TeamInvitesHandler = else inviterName = teamManager.email - TeamInvitesHandler.createInvite(subscription, email, inviterName, callback) + createInvite(subscription, email, inviterName, callback) createDomainInvite: (user, licence, callback) -> + logger.log {licence, email: user.email}, "Creating domain team invite" SubscriptionLocator.getSubscription licence.subscription_id, (error, subscription) -> return callback(error) if error? - TeamInvitesHandler.createInvite(subscription, user.email, licence.name, callback) + createInvite(subscription, user.email, licence.name, callback) acceptInvite: (token, userId, callback) -> + logger.log {userId}, "Accepting invite" TeamInvitesHandler.getInvite token, (err, invite, subscription) -> return callback(err) if err? return callback(inviteNoLongerValid: true) unless invite? @@ -54,22 +54,19 @@ module.exports = TeamInvitesHandler = SubscriptionUpdater.addUserToGroup subscription.admin_id, userId, (err) -> return callback(err) if err? - TeamInvitesHandler.removeInviteFromTeam(subscription.id, invite.email, callback) + removeInviteFromTeam(subscription.id, invite.email, callback) revokeInvite: (teamManagerId, email, callback) -> + logger.log {teamManagerId, email}, "Revoking invite" SubscriptionLocator.getUsersSubscription teamManagerId, (err, teamSubscription) -> return callback(err) if err? - TeamInvitesHandler.removeInviteFromTeam(teamSubscription.id, email, callback) + removeInviteFromTeam(teamSubscription.id, email, callback) - createInvite: (subscription, email, inviterName, callback) -> - if LimitationsManager.teamHasReachedMemberLimit(subscription) - return callback(limitReached: true) - - existingInvite = subscription.teamInvites.find (invite) -> invite.email == email - - if existingInvite - return callback(alreadyInvited: true) +createInvite = (subscription, email, inviterName, callback) -> + logger.log {subscriptionId: subscription.id, email, inviterName}, "Creating invite" + checkIfInviteIsPossible subscription, email, (error, possible, reason) -> + return callback(reason) unless possible? token = crypto.randomBytes(32).toString("hex") @@ -93,8 +90,30 @@ module.exports = TeamInvitesHandler = EmailHandler.sendEmail "verifyEmailToJoinTeam", opts, (error) -> return callback(error, invite) - removeInviteFromTeam: (subscriptionId, email, callback) -> - searchConditions = { _id: new ObjectId(subscriptionId.toString()) } - updateOp = { $pull: { teamInvites: { email: email.trim().toLowerCase() } } } +removeInviteFromTeam = (subscriptionId, email, callback) -> + searchConditions = { _id: new ObjectId(subscriptionId.toString()) } + updateOp = { $pull: { teamInvites: { email: email.trim().toLowerCase() } } } - Subscription.update(searchConditions, updateOp, callback) + Subscription.update(searchConditions, updateOp, callback) + +checkIfInviteIsPossible = (subscription, email, callback = (error, possible, reason) -> ) -> + if LimitationsManager.teamHasReachedMemberLimit(subscription) + logger.log {subscriptionId: subscription.id}, "team has reached member limit" + return callback(null, false, limitReached: true) + + existingInvite = subscription.teamInvites.find (invite) -> invite.email == email + + if existingInvite + logger.log {subscriptionId: subscription.id, email}, "user already invited" + return callback(null, false, alreadyInvited: true) + + async.map subscription.member_ids, UserLocator.findById, (error, members) -> + return callback(error) if error? + + existingMember = members.find (member) -> member.email == email + + if existingMember + logger.log {subscriptionId: subscription.id, email}, "user already in team" + return callback(null, false, alreadyInTeam: true) + else + return callback(null, true) diff --git a/services/web/test/unit/coffee/Subscription/LimitationsManagerTests.coffee b/services/web/test/unit/coffee/Subscription/LimitationsManagerTests.coffee index 3e0cf4c499..63af90f62d 100644 --- a/services/web/test/unit/coffee/Subscription/LimitationsManagerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/LimitationsManagerTests.coffee @@ -55,7 +55,7 @@ describe "LimitationsManager", -> it "should return the number of collaborators the user is allowed", -> @callback.calledWith(null, @user.features.collaborators).should.equal true - + describe "allowedNumberOfCollaboratorsForUser", -> describe "when the user has no features", -> beforeEach -> @@ -286,7 +286,9 @@ describe "LimitationsManager", -> @subscription = membersLimit: 3 member_ids: ["", ""] - invited_emails: ["bob@example.com"] + teamInvites: [ + { email: "bob@example.com", sentAt: new Date(), token: "hey" } + ] it "should return true if the limit is hit (including members and invites)", (done)-> @SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription) diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee index 09eab53063..086babd3e7 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee @@ -24,10 +24,8 @@ describe "SubscriptionGroupController", -> @GroupHandler = addUserToGroup: sinon.stub().callsArgWith(2, null, @user) removeUserFromGroup: sinon.stub().callsArgWith(2) - removeEmailInviteFromGroup: sinon.stub().callsArgWith(2) isUserPartOfGroup: sinon.stub() sendVerificationEmail:sinon.stub() - processGroupVerification:sinon.stub() getPopulatedListOfMembers: sinon.stub().callsArgWith(1, null, [@user]) @SubscriptionLocator = getUsersSubscription: sinon.stub().callsArgWith(1, null, @subscription) @AuthenticationController = @@ -80,16 +78,6 @@ describe "SubscriptionGroupController", -> done() @Controller.removeUserFromGroup @req, res - describe "removeEmailInviteFromGroup", -> - it "should use the admin id for the logged in user and take the email from the params", (done)-> - email = "jo@example.com" - @req.params = email: email - res = - send : => - @GroupHandler.removeEmailInviteFromGroup.calledWith(@adminUserId, email).should.equal true - done() - @Controller.removeEmailInviteFromGroup @req, res - describe "renderSubscriptionGroupAdminPage", -> it "should redirect you if you don't have a group account", (done)-> @subscription.groupPlan = false @@ -109,97 +97,6 @@ describe "SubscriptionGroupController", -> done() @Controller.renderSubscriptionGroupAdminPage @req, res - describe "renderGroupInvitePage", -> - describe "with a valid licence", -> - beforeEach -> - @SubscriptionDomainHandler.findDomainLicenceBySubscriptionId.returns({subscription_id:@subscription_id, adminUser_id:@adminUserId}) - - it "should render subscriptions/group/invite if not part of group", (done)-> - @GroupHandler.isUserPartOfGroup.callsArgWith(2, null, false) - res = - render : (pageName)=> - pageName.should.equal "subscriptions/group/invite" - done() - @Controller.renderGroupInvitePage @req, res - - it "should redirect to custom page if is already part of group", (done)-> - @GroupHandler.isUserPartOfGroup.callsArgWith(2, null, true) - res = - redirect : (location)=> - location.should.equal "/user/subscription/custom_account" - done() - @Controller.renderGroupInvitePage @req, res - - describe "without a valid licence", -> - beforeEach -> - @SubscriptionDomainHandler.findDomainLicenceBySubscriptionId.returns(undefined) - - it "should send a 500", (done)-> - @Controller.renderGroupInvitePage @req, {} - @ErrorsController.notFound.called.should.equal true - done() - - - - describe "beginJoinGroup", -> - describe "with a valid licence", -> - beforeEach -> - @licenceName = "get amazing licence" - @SubscriptionDomainHandler.findDomainLicenceBySubscriptionId.returns({name:@licenceName}) - @GroupHandler.sendVerificationEmail.callsArgWith(3) - - it "should ask the SubscriptionGroupHandler to send the verification email", (done)-> - res = - sendStatus : (statusCode)=> - statusCode.should.equal 200 - @GroupHandler.sendVerificationEmail.calledWith(@subscription_id, @licenceName, @user_email).should.equal true - done() - @Controller.beginJoinGroup @req, res - - describe "without a valid licence", -> - beforeEach -> - @SubscriptionDomainHandler.findDomainLicenceBySubscriptionId.returns(undefined) - - it "should send a 500", (done)-> - @Controller.beginJoinGroup @req, {} - @ErrorsController.notFound.called.should.equal true - done() - - - describe "completeJoin", -> - describe "with a valid licence", -> - beforeEach -> - @GroupHandler.processGroupVerification.callsArgWith(3) - @SubscriptionDomainHandler.findDomainLicenceBySubscriptionId.returns({name:@licenceName}) - - it "should redirect to the success page upon processGroupVerification", (done)-> - @req.query.token = @token - res = - redirect : (location)=> - @GroupHandler.processGroupVerification.calledWith(@user_email, @subscription_id, @token).should.equal true - location.should.equal "/user/subscription/#{@subscription_id}/group/successful-join" - done() - @Controller.completeJoin @req, res - - describe "without a valid licence", -> - - it "should send a 500", (done)-> - @SubscriptionDomainHandler.findDomainLicenceBySubscriptionId.returns(undefined) - @Controller.completeJoin @req, {} - @ErrorsController.notFound.called.should.equal true - done() - - it "should redirect to the invited page with querystring if token was not found", (done)-> - @SubscriptionDomainHandler.findDomainLicenceBySubscriptionId.returns({name:@licenceName}) - @req.query.token = @token - @GroupHandler.processGroupVerification.callsArgWith(3, "token_not_found") - res = - redirect : (location)=> - location.should.equal "/user/subscription/#{@subscription_id}/group/invited?expired=true" - done() - @Controller.completeJoin @req, res - - describe "exportGroupCsv", -> beforeEach -> diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee index 1daf43bbd7..59471c68f8 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee @@ -19,19 +19,20 @@ describe "SubscriptionGroupHandler", -> admin_id:@adminUser_id _id:@subscription_id - @SubscriptionLocator = + @SubscriptionLocator = getUsersSubscription: sinon.stub() getSubscriptionByMemberIdAndId: sinon.stub() getSubscription: sinon.stub() - @UserCreator = + @UserCreator = getUserOrCreateHoldingAccount: sinon.stub().callsArgWith(1, null, @user) @SubscriptionUpdater = addUserToGroup: sinon.stub().callsArgWith(2) removeUserFromGroup: sinon.stub().callsArgWith(2) - addEmailInviteToGroup: sinon.stub().callsArgWith(2) - removeEmailInviteFromGroup: sinon.stub().callsArgWith(2) + + @TeamInvitesHandler = + createManagerInvite: sinon.stub().callsArgWith(2) @UserLocator = findById: sinon.stub() @@ -47,7 +48,7 @@ describe "SubscriptionGroupHandler", -> @EmailHandler = sendEmail:sinon.stub() - @settings = + @settings = siteUrl:"http://www.sharelatex.com" @readStub = sinon.stub() @@ -58,6 +59,7 @@ describe "SubscriptionGroupHandler", -> "logger-sharelatex": log:-> "../User/UserCreator": @UserCreator "./SubscriptionUpdater": @SubscriptionUpdater + "./TeamInvitesHandler": @TeamInvitesHandler "./SubscriptionLocator": @SubscriptionLocator "../User/UserLocator": @UserLocator "./LimitationsManager": @LimitationsManager @@ -65,7 +67,7 @@ describe "SubscriptionGroupHandler", -> "../Email/EmailHandler":@EmailHandler "settings-sharelatex":@settings "../Notifications/NotificationsBuilder": @NotificationsBuilder - "logger-sharelatex": + "logger-sharelatex": err:-> log:-> warn:-> @@ -75,7 +77,7 @@ describe "SubscriptionGroupHandler", -> beforeEach -> @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription) @UserLocator.findByEmail.callsArgWith(1, null, @user) - + it "should find the user", (done)-> @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> @UserLocator.findByEmail.calledWith(@newEmail).should.equal true @@ -85,7 +87,7 @@ describe "SubscriptionGroupHandler", -> @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> @SubscriptionUpdater.addUserToGroup.calledWith(@adminUser_id, @user._id).should.equal true done() - + it "should not add the user to the group if the limit has been reached", (done)-> @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, true, @subscription) @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> @@ -103,11 +105,11 @@ describe "SubscriptionGroupHandler", -> @NotificationsBuilder.groupPlan.calledWith(@user, {subscription_id:@subscription._id}).should.equal true @readStub.called.should.equal true done() - - it "should add an email invite if no user is found", (done) -> + + it "should add a team invite if no user is found", (done) -> @UserLocator.findByEmail.callsArgWith(1, null, null) @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> - @SubscriptionUpdater.addEmailInviteToGroup.calledWith(@adminUser_id, @newEmail).should.equal true + @TeamInvitesHandler.createManagerInvite.calledWith(@adminUser_id, @newEmail).should.equal true done() describe "removeUserFromGroup", -> @@ -148,15 +150,18 @@ describe "SubscriptionGroupHandler", -> assert.deepEqual users[1], {_id:@subscription.member_ids[1]} assert.deepEqual users[2], {_id:@subscription.member_ids[2]} done() - + it "should return any invited users", (done) -> - @subscription.invited_emails = ["jo@example.com", "charlie@example.com"] + @subscription.teamInvites = [ + { email: "jo@example.com" }, + { email: "charlie@example.com" } + ] @Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=> users[0].email.should.equal "jo@example.com" users[0].holdingAccount.should.equal true users[1].email.should.equal "charlie@example.com" users[1].holdingAccount.should.equal true - users.length.should.equal @subscription.invited_emails.length + users.length.should.equal @subscription.teamInvites.length done() describe "isUserPartOfGroup", -> @@ -192,47 +197,3 @@ describe "SubscriptionGroupHandler", -> emailOpts.to.should.equal @email emailOpts.group_name.should.equal @licenceName done() - - describe "processGroupVerification", -> - beforeEach -> - @token = "31dDAd2Da" - @SubscriptionLocator.getSubscription.callsArgWith(1, null, @subscription) - @Handler.addUserToGroup = sinon.stub().callsArgWith(2) - - it "should addUserToGroup", (done)-> - @OneTimeTokenHandler.getValueFromTokenAndExpire.callsArgWith(1, null, @subscription_id) - @Handler.processGroupVerification @email, @subscription_id, @token, (err)=> - @Handler.addUserToGroup.calledWith(@adminUser_id, @email).should.equal true - done() - - it "should return token_not_found error if it couldn't get the token", (done)-> - @OneTimeTokenHandler.getValueFromTokenAndExpire.callsArgWith(1) - @Handler.processGroupVerification @email, @subscription_id, @token, (err)=> - err.should.equal "token_not_found" - done() - - describe "convertEmailInvitesToMemberships", -> - beforeEach -> - @SubscriptionLocator.getGroupsWithEmailInvite = sinon.stub().yields(null, @groups = [{ admin_id: "group-1" }, { admin_id: "group-2" }]) - - it "should get groups with the email address invited to", (done) -> - @Handler.convertEmailInvitesToMemberships @email, @user_id, (err) => - @SubscriptionLocator.getGroupsWithEmailInvite.calledWith(@email).should.equal true - done() - - it "should remove the email from each group", (done) -> - @Handler.convertEmailInvitesToMemberships @email, @user_id, (err) => - for group in @groups - @SubscriptionUpdater.removeEmailInviteFromGroup - .calledWith(group.admin_id, @email) - .should.equal true - done() - - it "should add the user to each group", (done) -> - @Handler.convertEmailInvitesToMemberships @email, @user_id, (err) => - for group in @groups - @SubscriptionUpdater.addUserToGroup - .calledWith(group.admin_id, @user_id) - .should.equal true - done() - diff --git a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee new file mode 100644 index 0000000000..f640887e88 --- /dev/null +++ b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee @@ -0,0 +1,22 @@ +SandboxedModule = require('sandboxed-module') +should = require('chai').should() +sinon = require 'sinon' +querystring = require 'querystring' +modulePath = "../../../../app/js/Features/Subscription/TeamInvitesHandler" + +describe "TeamInvitesHandler", -> + + describe "getInvite", -> + # TODO + + describe "createManagerInvite", -> + # TODO + + describe "createDomainInvite", -> + # TODO + + describe "acceptInvite", -> + # TODO + + describe "revokeInvite", -> + # TODO From 9b18e58b6840cc5a902f654706879a8e9e69349e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Fri, 1 Jun 2018 11:28:06 +0100 Subject: [PATCH 08/44] Choose a better method name This method is now only notifying users about a potential domain licence --- .../Features/User/UserController.coffee | 4 +-- .../coffee/Features/User/UserHandler.coffee | 30 ++++++++----------- .../coffee/User/UserControllerTests.coffee | 6 ++-- .../unit/coffee/User/UserHandlerTests.coffee | 30 +++++-------------- 4 files changed, 25 insertions(+), 45 deletions(-) diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index ccbd0a86f1..be1874f0cd 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/services/web/app/coffee/Features/User/UserController.coffee @@ -110,9 +110,9 @@ module.exports = UserController = logger.err err:err, user_id:user_id, "error getting user for email update" return res.send 500 AuthenticationController.setInSessionUser(req, {email: user.email, first_name: user.first_name, last_name: user.last_name}) - UserHandler.populateGroupLicenceInvite user, (err)-> #need to refresh this in the background + UserHandler.notifyDomainLicence user, (err)-> #need to refresh this in the background if err? - logger.err err:err, "error populateGroupLicenceInvite" + logger.err err:err, "error notifyDomainLicence" res.sendStatus(200) logout : (req, res)-> diff --git a/services/web/app/coffee/Features/User/UserHandler.coffee b/services/web/app/coffee/Features/User/UserHandler.coffee index 53b2fd9c74..1d8fda7db9 100644 --- a/services/web/app/coffee/Features/User/UserHandler.coffee +++ b/services/web/app/coffee/Features/User/UserHandler.coffee @@ -6,24 +6,20 @@ logger = require("logger-sharelatex") module.exports = UserHandler = - populateGroupLicenceInvite: (user, callback = ->)-> - SubscriptionGroupHandler.convertEmailInvitesToMemberships user.email, user._id, (err) -> - return callback(err) if err? + notifyDomainLicence: (user, callback = ->)-> + logger.log user_id:user._id, "notiying user about a potential domain licence" + licence = SubscriptionDomainHandler.getLicenceUserCanJoin user + if !licence? + return callback() - logger.log user_id:user._id, "populating any potential group licence invites" - licence = SubscriptionDomainHandler.getLicenceUserCanJoin user - if !licence? + SubscriptionGroupHandler.isUserPartOfGroup user._id, licence.subscription_id, (err, alreadyPartOfGroup)-> + if err? + return callback(err) + else if alreadyPartOfGroup + logger.log user_id:user._id, "user already part of team, not creating notifcation for them" return callback() - - SubscriptionGroupHandler.isUserPartOfGroup user._id, licence.subscription_id, (err, alreadyPartOfGroup)-> - if err? - return callback(err) - else if alreadyPartOfGroup - logger.log user_id:user._id, "user already part of group, not creating notifcation for them" - return callback() - else - NotificationsBuilder.groupPlan(user, licence).create(callback) + else + NotificationsBuilder.groupPlan(user, licence).create(callback) setupLoginData: (user, callback = ->)-> - @populateGroupLicenceInvite user, callback - + @notifyDomainLicence user, callback diff --git a/services/web/test/unit/coffee/User/UserControllerTests.coffee b/services/web/test/unit/coffee/User/UserControllerTests.coffee index c358f35b22..a9ab47e6cd 100644 --- a/services/web/test/unit/coffee/User/UserControllerTests.coffee +++ b/services/web/test/unit/coffee/User/UserControllerTests.coffee @@ -55,7 +55,7 @@ describe "UserController", -> @settings = siteUrl: "sharelatex.example.com" @UserHandler = - populateGroupLicenceInvite:sinon.stub().callsArgWith(1) + notifyDomainLicence: sinon.stub().callsArgWith(1) @UserSessionsManager = trackSession: sinon.stub() untrackSession: sinon.stub() @@ -267,12 +267,12 @@ describe "UserController", -> done() @UserController.updateUserSettings @req, @res - it "should call populateGroupLicenceInvite", (done)-> + it "should call notifyDomainLicence", (done)-> @req.body.email = @newEmail.toUpperCase() @UserUpdater.changeEmailAddress.callsArgWith(2) @res.sendStatus = (code)=> code.should.equal 200 - @UserHandler.populateGroupLicenceInvite.calledWith(@user).should.equal true + @UserHandler.notifyDomainLicence.calledWith(@user).should.equal true done() @UserController.updateUserSettings @req, @res diff --git a/services/web/test/unit/coffee/User/UserHandlerTests.coffee b/services/web/test/unit/coffee/User/UserHandlerTests.coffee index d6e3cdf07e..51c9a75dcd 100644 --- a/services/web/test/unit/coffee/User/UserHandlerTests.coffee +++ b/services/web/test/unit/coffee/User/UserHandlerTests.coffee @@ -7,21 +7,20 @@ SandboxedModule = require('sandboxed-module') describe "UserHandler", -> beforeEach -> - @user = + @user = _id:"12390i" email: "bob@bob.com" remove: sinon.stub().callsArgWith(0) - @licence = + @licence = subscription_id: 12323434 @SubscriptionDomainHandler = getLicenceUserCanJoin: sinon.stub() @SubscriptionGroupHandler = isUserPartOfGroup:sinon.stub() - convertEmailInvitesToMemberships: sinon.stub().callsArgWith(2) @createStub = sinon.stub().callsArgWith(0) - @NotificationsBuilder = + @NotificationsBuilder = groupPlan:sinon.stub().returns({create:@createStub}) @UserHandler = SandboxedModule.require modulePath, requires: @@ -30,16 +29,11 @@ describe "UserHandler", -> "../Subscription/SubscriptionDomainHandler":@SubscriptionDomainHandler "../Subscription/SubscriptionGroupHandler":@SubscriptionGroupHandler - describe "populateGroupLicenceInvite", -> + describe "notifyDomainLicence", -> describe "no licence", -> beforeEach (done)-> @SubscriptionDomainHandler.getLicenceUserCanJoin.returns() - @UserHandler.populateGroupLicenceInvite @user, done - - it "should call convertEmailInvitesToMemberships", -> - @SubscriptionGroupHandler.convertEmailInvitesToMemberships - .calledWith(@user.email, @user._id) - .should.equal true + @UserHandler.notifyDomainLicence @user, done it "should not call NotificationsBuilder", (done)-> @NotificationsBuilder.groupPlan.called.should.equal false @@ -53,28 +47,18 @@ describe "UserHandler", -> beforeEach (done)-> @SubscriptionDomainHandler.getLicenceUserCanJoin.returns(@licence) @SubscriptionGroupHandler.isUserPartOfGroup.callsArgWith(2, null, false) - @UserHandler.populateGroupLicenceInvite @user, done + @UserHandler.notifyDomainLicence @user, done it "should create notifcation", (done)-> @NotificationsBuilder.groupPlan.calledWith(@user, @licence).should.equal true done() - - it "should call convertEmailInvitesToMemberships", -> - @SubscriptionGroupHandler.convertEmailInvitesToMemberships - .calledWith(@user.email, @user._id) - .should.equal true describe "with matching licence user is already in", -> beforeEach (done)-> @SubscriptionDomainHandler.getLicenceUserCanJoin.returns(@licence) @SubscriptionGroupHandler.isUserPartOfGroup.callsArgWith(2, null, true) - @UserHandler.populateGroupLicenceInvite @user, done + @UserHandler.notifyDomainLicence @user, done it "should create notifcation", (done)-> @NotificationsBuilder.groupPlan.called.should.equal false done() - - it "should call convertEmailInvitesToMemberships", -> - @SubscriptionGroupHandler.convertEmailInvitesToMemberships - .calledWith(@user.email, @user._id) - .should.equal true \ No newline at end of file From 89735f4b8e74560eb42acf8194073f6255df0888 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Fri, 1 Jun 2018 15:36:00 +0100 Subject: [PATCH 09/44] Keep method name for compatibility --- services/web/app/coffee/Features/User/UserHandler.coffee | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/web/app/coffee/Features/User/UserHandler.coffee b/services/web/app/coffee/Features/User/UserHandler.coffee index 1d8fda7db9..df614ca1e0 100644 --- a/services/web/app/coffee/Features/User/UserHandler.coffee +++ b/services/web/app/coffee/Features/User/UserHandler.coffee @@ -23,3 +23,7 @@ module.exports = UserHandler = setupLoginData: (user, callback = ->)-> @notifyDomainLicence user, callback + + # Backwards compatibility: this is called from the public-registration module + populateGroupLicenceInvite: (user, callback = ->)-> + @notifyDomainLicence user, callback From 3bd18715dbc71aac863e443da95403bbde1b3dd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Fri, 1 Jun 2018 15:37:09 +0100 Subject: [PATCH 10/44] Add some tests --- .../Subscription/TeamInvitesHandler.coffee | 7 +- .../TeamInvitesHandlerTests.coffee | 211 +++++++++++++++++- 2 files changed, 205 insertions(+), 13 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index 37c99bafbe..ccab393bc0 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -66,12 +66,14 @@ module.exports = TeamInvitesHandler = createInvite = (subscription, email, inviterName, callback) -> logger.log {subscriptionId: subscription.id, email, inviterName}, "Creating invite" checkIfInviteIsPossible subscription, email, (error, possible, reason) -> - return callback(reason) unless possible? + return callback(error) if error? + return callback(reason) unless possible token = crypto.randomBytes(32).toString("hex") + # TODO: use standard way to canonalise email addresses invite = { - email: email, + email: email.trim().toLowerCase(), token: token, inviterName: inviterName, sentAt: new Date(), @@ -82,7 +84,6 @@ createInvite = (subscription, email, inviterName, callback) -> subscription.save (error) -> return callback(error) if error? - # TODO: use standard way to canonalise email addresses opts = to: email.trim().toLowerCase() inviterName: inviterName diff --git a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee index f640887e88..d538e18a16 100644 --- a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee @@ -1,22 +1,213 @@ SandboxedModule = require('sandboxed-module') should = require('chai').should() sinon = require 'sinon' +expect = require("chai").expect querystring = require 'querystring' modulePath = "../../../../app/js/Features/Subscription/TeamInvitesHandler" +ObjectId = require("mongojs").ObjectId + describe "TeamInvitesHandler", -> + beforeEach -> + @manager = { + id: "666666", + first_name: "Daenerys" + last_name: "Targaryen" + email: "daenerys@motherofdragons.com" + } - describe "getInvite", -> - # TODO + @token = "aaaaaaaaaaaaaaaaaaaaaa" - describe "createManagerInvite", -> - # TODO + @teamInvite = { + email: "jorah@mormont.org", + token: @token, + } - describe "createDomainInvite", -> - # TODO + @subscription = { + id: "55153a8014829a865bbf700d", + admin_id: @manager.id, + member_ids: [] + teamInvites: [ @teamInvite ] + save: sinon.stub().yields(null) + } - describe "acceptInvite", -> - # TODO + @SubscriptionLocator = { + getUsersSubscription: sinon.stub(), + getSubscription: sinon.stub().yields(null, @subscription) + } - describe "revokeInvite", -> - # TODO + @UserLocator = { + findById: sinon.stub() + } + + @SubscriptionUpdater = { + addUserToGroup: sinon.stub().yields() + } + + @LimitationsManager = { + teamHasReachedMemberLimit: sinon.stub().returns(false) + } + + @Subscription = { + findOne: sinon.stub().yields() + update: sinon.stub().yields() + } + + @EmailHandler = { + sendEmail: sinon.stub().yields(null) + } + + @newToken = "bbbbbbbbb" + + @crypto = { + randomBytes: => + toString: sinon.stub().returns(@newToken) + } + + @UserLocator.findById.withArgs(@manager.id).yields(null, @manager) + @SubscriptionLocator.getUsersSubscription.yields(null, @subscription) + @Subscription.findOne.yields(null, @subscription) + + @TeamInvitesHandler = SandboxedModule.require modulePath, requires: + "logger-sharelatex": { log: -> } + "crypto": @crypto + "settings-sharelatex": { siteUrl: "http://example.com" } + "../../models/TeamInvite": { TeamInvite: @TeamInvite = {} } + "../../models/Subscription": { Subscription: @Subscription } + "../User/UserLocator": @UserLocator + "./SubscriptionLocator": @SubscriptionLocator + "./SubscriptionUpdater": @SubscriptionUpdater + "./LimitationsManager": @LimitationsManager + "../Email/EmailHandler": @EmailHandler + + describe "getInvite", -> + it "returns the invite if there's one", (done) -> + @TeamInvitesHandler.getInvite @token, (err, invite, subscription) => + expect(err).to.eq(null) + expect(invite).to.deep.eq(@teamInvite) + expect(subscription).to.deep.eq(@subscription) + done() + + it "returns teamNotFound if there's none", (done) -> + @Subscription.findOne = sinon.stub().yields(null, null) + + @TeamInvitesHandler.getInvite @token, (err, invite, subscription) -> + expect(err).to.deep.eq(teamNotFound: true) + done() + + describe "createManagerInvite", -> + it "adds the team invite to the subscription", (done) -> + @TeamInvitesHandler.createManagerInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => + expect(err).to.eq(null) + expect(invite.token).to.eq(@newToken) + expect(invite.email).to.eq("john.snow@nightwatch.com") + expect(invite.inviterName).to.eq("Daenerys Targaryen (daenerys@motherofdragons.com)") + expect(@subscription.teamInvites).to.deep.include(invite) + done() + + it "sends an email", (done) -> + @TeamInvitesHandler.createManagerInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => + @EmailHandler.sendEmail.calledWith("verifyEmailToJoinTeam", + sinon.match({ + to: "john.snow@nightwatch.com", + inviterName: "Daenerys Targaryen (daenerys@motherofdragons.com)", + acceptInviteUrl: "http://example.com/subscription/invites/#{@newToken}/" + }) + ).should.equal true + done() + + describe "createDomainInvite", -> + beforeEach -> + @licence = + subscription_id: @subscription.id + name: "Team Daenerys" + + @user = + email: "John.Snow@nightwatch.com" + + it "adds the team invite to the subscription", (done) -> + @TeamInvitesHandler.createDomainInvite @user, @licence, (err, invite) => + expect(err).to.eq(null) + expect(invite.token).to.eq(@newToken) + expect(invite.email).to.eq("john.snow@nightwatch.com") + expect(invite.inviterName).to.eq("Team Daenerys") + expect(@subscription.teamInvites).to.deep.include(invite) + done() + + it "sends an email", (done) -> + @TeamInvitesHandler.createDomainInvite @user, @licence, (err, invite) => + @EmailHandler.sendEmail.calledWith("verifyEmailToJoinTeam", + sinon.match({ + to: "john.snow@nightwatch.com" + inviterName: "Team Daenerys" + acceptInviteUrl: "http://example.com/subscription/invites/#{@newToken}/" + }) + ).should.equal true + done() + + describe "acceptInvite", -> + beforeEach -> + @user = { + id: "123456789", + first_name: "Tyrion", + last_name: "Lannister", + email: "tyrion@lannister.com" + } + + @UserLocator.findById.withArgs(@user.id).yields(null, @user) + + @subscription.teamInvites.push({ + email: "john.snow@nightwatch.com", + token: "dddddddd", + inviterName: "Daenerys Targaryen (daenerys@motherofdragons.com)" + }) + + it "adds the user to the team", (done) -> + @TeamInvitesHandler.acceptInvite "dddddddd", @user.id, => + @SubscriptionUpdater.addUserToGroup.calledWith(@manager.id, @user.id).should.eq true + done() + + it "removes the invite from the subscription", (done) -> + @TeamInvitesHandler.acceptInvite "dddddddd", @user.id, => + @Subscription.update.calledWith( + { _id: new ObjectId("55153a8014829a865bbf700d") }, + { '$pull': { teamInvites: { email: 'john.snow@nightwatch.com' } } } + ).should.eq true + done() + + describe "revokeInvite", -> + it "removes the team invite from the subscription", (done) -> + @TeamInvitesHandler.revokeInvite @manager.id, "jorah@mormont.org", => + @Subscription.update.calledWith( + { _id: new ObjectId("55153a8014829a865bbf700d") }, + { '$pull': { teamInvites: { email: "jorah@mormont.org" } } } + ).should.eq true + done() + + describe "validation", -> + it "doesn't create an invite if the team limit has been reached", (done) -> + @LimitationsManager.teamHasReachedMemberLimit = sinon.stub().returns(true) + @TeamInvitesHandler.createManagerInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => + console.log('err, invite', err, invite) + expect(err).to.deep.equal(limitReached: true) + done() + + it "doen't create an invite if the email has already been invited",(done) -> + @TeamInvitesHandler.createManagerInvite @manager.id, "jorah@mormont.org", (err, invite) => + expect(err).to.deep.equal(alreadyInvited: true) + expect(invite).not.to.exist + done() + + it "doen't create an invite if the user is already part of the team", (done) -> + member = { + id: "1a2b", + email: "tyrion@lannister.com" + } + + @subscription.member_ids = [member.id] + @UserLocator.findById.withArgs(member.id).yields(null, member) + + @TeamInvitesHandler.createManagerInvite @manager.id, "tyrion@lannister.com", (err, invite) => + expect(err).to.deep.equal(alreadyInTeam: true) + expect(invite).not.to.exist + done() From 7fcdf6829626b557bee6be1dd35815728fb8efe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Fri, 1 Jun 2018 16:05:44 +0100 Subject: [PATCH 11/44] Remove dead code --- .../coffee/Features/Email/EmailBuilder.coffee | 25 ------------------- .../SubscriptionGroupHandler.coffee | 10 -------- .../SubscriptionGroupControllerTests.coffee | 1 - .../SubscriptionGroupHandlerTests.coffee | 18 ------------- 4 files changed, 54 deletions(-) diff --git a/services/web/app/coffee/Features/Email/EmailBuilder.coffee b/services/web/app/coffee/Features/Email/EmailBuilder.coffee index 9b975140f9..28a748431d 100644 --- a/services/web/app/coffee/Features/Email/EmailBuilder.coffee +++ b/services/web/app/coffee/Features/Email/EmailBuilder.coffee @@ -144,31 +144,6 @@ Thank You gmailGoToAction: null }) -templates.completeJoinGroupAccount = - subject: _.template "Verify Email to join <%= group_name %> group" - layout: BaseWithHeaderEmailLayout - type:"notification" - plainTextTemplate: _.template """ -Hi, please verify your email to join the <%= group_name %> and get your free premium account - -Click this link to verify now: <%= completeJoinUrl %> - -Thank You - -#{settings.appName} - <%= siteUrl %> -""" - compiledTemplate: (opts) -> - SingleCTAEmailBody({ - title: "Verify Email to join #{ opts.group_name } group" - greeting: "Hi," - message: "please verify your email to join the #{ opts.group_name } group and get your free premium account." - secondaryMessage: null - ctaText: "Verify now" - ctaURL: opts.completeJoinUrl - gmailGoToAction: null - }) - - templates.testEmail = subject: _.template "A Test Email from ShareLaTeX" layout: BaseWithHeaderEmailLayout diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index b34dacc3e6..b8d1b32aa0 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -71,16 +71,6 @@ module.exports = SubscriptionGroupHandler = logger.log user_id:user_id, subscription_id:subscription_id, partOfGroup:partOfGroup, "checking if user is part of a group" callback(err, partOfGroup) - - sendVerificationEmail: (subscription_id, licenceName, email, callback)-> - ONE_DAY_IN_S = 1000 * 60 * 60 * 24 - OneTimeTokenHandler.getNewToken subscription_id, {expiresIn:ONE_DAY_IN_S}, (err, token)-> - opts = - to : email - group_name: licenceName - completeJoinUrl: "#{settings.siteUrl}/user/subscription/#{subscription_id}/group/complete-join?token=#{token}" - EmailHandler.sendEmail "completeJoinGroupAccount", opts, callback - buildUserViewModel = (user)-> u = email: user.email diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee index 086babd3e7..d9ad55080f 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee @@ -25,7 +25,6 @@ describe "SubscriptionGroupController", -> addUserToGroup: sinon.stub().callsArgWith(2, null, @user) removeUserFromGroup: sinon.stub().callsArgWith(2) isUserPartOfGroup: sinon.stub() - sendVerificationEmail:sinon.stub() getPopulatedListOfMembers: sinon.stub().callsArgWith(1, null, [@user]) @SubscriptionLocator = getUsersSubscription: sinon.stub().callsArgWith(1, null, @subscription) @AuthenticationController = diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee index 59471c68f8..878751df2e 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee @@ -179,21 +179,3 @@ describe "SubscriptionGroupHandler", -> @Handler.isUserPartOfGroup @user_id, @subscription_id, (err, partOfGroup)-> partOfGroup.should.equal false done() - - - describe "sendVerificationEmail", -> - beforeEach -> - @token = "secret token" - @subscription_id = "123ed13123" - @licenceName = "great licnece" - @email = "bob@smith.com" - @OneTimeTokenHandler.getNewToken.callsArgWith(2, null, @token) - @EmailHandler.sendEmail.callsArgWith(2) - - it "should put a one time token into the email", (done)-> - @Handler.sendVerificationEmail @subscription_id, @licenceName, @email, (err)=> - emailOpts = @EmailHandler.sendEmail.args[0][1] - emailOpts.completeJoinUrl.should.equal "#{@settings.siteUrl}/user/subscription/#{@subscription_id}/group/complete-join?token=#{@token}" - emailOpts.to.should.equal @email - emailOpts.group_name.should.equal @licenceName - done() From a73a869d0388c950f98740328f4cd7a94c17d63f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Fri, 1 Jun 2018 16:17:11 +0100 Subject: [PATCH 12/44] I can haz some grammar --- services/web/app/coffee/Features/Email/EmailBuilder.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/Email/EmailBuilder.coffee b/services/web/app/coffee/Features/Email/EmailBuilder.coffee index 28a748431d..76fa5ebbea 100644 --- a/services/web/app/coffee/Features/Email/EmailBuilder.coffee +++ b/services/web/app/coffee/Features/Email/EmailBuilder.coffee @@ -120,7 +120,7 @@ Thank you templates.verifyEmailToJoinTeam = - subject: _.template "<%= inviterName %> has invited you to join a #{settings.appName} team" + subject: _.template "<%= inviterName %> has invited you to join a team on #{settings.appName}" layout: BaseWithHeaderEmailLayout type:"notification" plainTextTemplate: _.template """ @@ -135,7 +135,7 @@ Thank You """ compiledTemplate: (opts) -> SingleCTAEmailBody({ - title: "#{opts.inviterName} has invited you to join a #{settings.appName} team" + title: "#{opts.inviterName} has invited you to join a team on #{settings.appName}" greeting: "Hi," message: "please verify your email to join the team and get your free premium account" secondaryMessage: null From b0b2546e431fb3fbba29e460beb8762568599eec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Fri, 1 Jun 2018 16:45:36 +0100 Subject: [PATCH 13/44] Remove dead code --- .../coffee/Features/Subscription/SubscriptionLocator.coffee | 3 --- 1 file changed, 3 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee index b0a6a5b021..a32447c7e0 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee @@ -30,6 +30,3 @@ module.exports = getGroupSubscriptionsMemberOf: (user_id, callback)-> Subscription.find {member_ids: user_id}, {_id:1, planCode:1}, callback - - getGroupsWithEmailInvite: (email, callback) -> - Subscription.find { teamInvites: { email: email } }, callback From 58c4e72b2ad31332a80c2f5d12027454d5fd36f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Fri, 1 Jun 2018 16:50:00 +0100 Subject: [PATCH 14/44] Update references to UserLocator UserLocator was refactored in another branch: UserLocator -> UserGetter --- .../Features/Subscription/TeamInvitesHandler.coffee | 6 +++--- .../Subscription/TeamInvitesHandlerTests.coffee | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index ccab393bc0..9f4f10a810 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -8,7 +8,7 @@ ObjectId = require("mongojs").ObjectId TeamInvite = require("../../models/TeamInvite").TeamInvite Subscription = require("../../models/Subscription").Subscription -UserLocator = require("../User/UserLocator") +UserGetter = require("../User/UserGetter") SubscriptionLocator = require("./SubscriptionLocator") SubscriptionUpdater = require("./SubscriptionUpdater") LimitationsManager = require("./LimitationsManager") @@ -26,7 +26,7 @@ module.exports = TeamInvitesHandler = createManagerInvite: (teamManagerId, email, callback) -> logger.log {teamManagerId, email}, "Creating manager team invite" - UserLocator.findById teamManagerId, (error, teamManager) -> + UserGetter.getUser teamManagerId, (error, teamManager) -> return callback(error) if error? SubscriptionLocator.getUsersSubscription teamManagerId, (error, subscription) -> @@ -108,7 +108,7 @@ checkIfInviteIsPossible = (subscription, email, callback = (error, possible, rea logger.log {subscriptionId: subscription.id, email}, "user already invited" return callback(null, false, alreadyInvited: true) - async.map subscription.member_ids, UserLocator.findById, (error, members) -> + async.map subscription.member_ids, UserGetter.getUser, (error, members) -> return callback(error) if error? existingMember = members.find (member) -> member.email == email diff --git a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee index d538e18a16..2d0bd08f9e 100644 --- a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee @@ -36,8 +36,8 @@ describe "TeamInvitesHandler", -> getSubscription: sinon.stub().yields(null, @subscription) } - @UserLocator = { - findById: sinon.stub() + @UserGetter = { + getUser: sinon.stub() } @SubscriptionUpdater = { @@ -64,7 +64,7 @@ describe "TeamInvitesHandler", -> toString: sinon.stub().returns(@newToken) } - @UserLocator.findById.withArgs(@manager.id).yields(null, @manager) + @UserGetter.getUser.withArgs(@manager.id).yields(null, @manager) @SubscriptionLocator.getUsersSubscription.yields(null, @subscription) @Subscription.findOne.yields(null, @subscription) @@ -74,7 +74,7 @@ describe "TeamInvitesHandler", -> "settings-sharelatex": { siteUrl: "http://example.com" } "../../models/TeamInvite": { TeamInvite: @TeamInvite = {} } "../../models/Subscription": { Subscription: @Subscription } - "../User/UserLocator": @UserLocator + "../User/UserGetter": @UserGetter "./SubscriptionLocator": @SubscriptionLocator "./SubscriptionUpdater": @SubscriptionUpdater "./LimitationsManager": @LimitationsManager @@ -154,7 +154,7 @@ describe "TeamInvitesHandler", -> email: "tyrion@lannister.com" } - @UserLocator.findById.withArgs(@user.id).yields(null, @user) + @UserGetter.getUser.withArgs(@user.id).yields(null, @user) @subscription.teamInvites.push({ email: "john.snow@nightwatch.com", @@ -205,7 +205,7 @@ describe "TeamInvitesHandler", -> } @subscription.member_ids = [member.id] - @UserLocator.findById.withArgs(member.id).yields(null, member) + @UserGetter.getUser.withArgs(member.id).yields(null, member) @TeamInvitesHandler.createManagerInvite @manager.id, "tyrion@lannister.com", (err, invite) => expect(err).to.deep.equal(alreadyInTeam: true) From 863128a03035130fb797d1a99ed78f5da9552bb2 Mon Sep 17 00:00:00 2001 From: Nate Stemen Date: Tue, 5 Jun 2018 10:08:27 -0400 Subject: [PATCH 15/44] only suggest one document environment in project --- .../auto-complete/AutoCompleteManager.coffee | 5 ++-- .../auto-complete/EnvironmentManager.coffee | 29 +++++++++++++++++-- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/auto-complete/AutoCompleteManager.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/auto-complete/AutoCompleteManager.coffee index 78d12eebc3..a10dcf618b 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/auto-complete/AutoCompleteManager.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/auto-complete/AutoCompleteManager.coffee @@ -34,8 +34,7 @@ define [ enableLiveAutocompletion: false }) - commandCompleter = new CommandManager(@metadataManager) - + CommandCompleter = new CommandManager(@metadataManager) SnippetCompleter = new EnvironmentManager() PackageCompleter = new PackageManager(@metadataManager, Helpers) @@ -153,7 +152,7 @@ define [ callback null, result @editor.completers = [ - commandCompleter + CommandCompleter SnippetCompleter PackageCompleter ReferencesCompleter diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/auto-complete/EnvironmentManager.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/auto-complete/EnvironmentManager.coffee index 8208ec4a30..ca79f27bf4 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/auto-complete/EnvironmentManager.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/auto-complete/EnvironmentManager.coffee @@ -23,7 +23,7 @@ define () -> "thebibliography" ] - environmentNames = snippetNames.concat environments + environmentNames = snippetNames.concat(environments) staticSnippets = for env in environments { @@ -122,6 +122,18 @@ define () -> meta: "env" }] + documentSnippet = { + caption: "\\begin{document}..." + snippet: """ + \\begin{document} + $1 + \\end{document} + """ + meta: "env" + } + + staticSnippets.push(documentSnippet) + parseCustomEnvironments = (text) -> re = /^\\newenvironment{(\w+)}.*$/gm result = [] @@ -138,18 +150,31 @@ define () -> result = [] iterations = 0 while match = re.exec(text) - if match[1] not in environmentNames + if match[1] not in environmentNames and match[1] != "document" result.push {name: match[1], whitespace: match[2]} iterations += 1 if iterations >= 1000 return result return result + hasDocumentEnvironment = (text) -> + re = /^\\begin{document}/m + envs = [] + iterations = 0 + return re.exec(text) != null + class EnvironmentManager getCompletions: (editor, session, pos, prefix, callback) -> docText = session.getValue() customEnvironments = parseCustomEnvironments(docText) beginCommands = parseBeginCommands(docText) + if hasDocumentEnvironment(docText) + ind = staticSnippets.indexOf(documentSnippet) + if ind != -1 + staticSnippets.splice(ind, 1) + else + staticSnippets.push documentSnippet + parsedItemsMap = {} for environment in customEnvironments parsedItemsMap[environment.name] = environment From 26385718e6df65fbd8aa07d14b34a62227651d22 Mon Sep 17 00:00:00 2001 From: Nate Stemen Date: Tue, 5 Jun 2018 11:02:03 -0400 Subject: [PATCH 16/44] only suggest thebibliography once --- .../auto-complete/EnvironmentManager.coffee | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/auto-complete/EnvironmentManager.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/auto-complete/EnvironmentManager.coffee index ca79f27bf4..383bf1bd2e 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/auto-complete/EnvironmentManager.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/auto-complete/EnvironmentManager.coffee @@ -111,15 +111,6 @@ define () -> \\end{frame} """ meta: "env" - }, { - caption: "\\begin{thebibliography}..." - snippet: """ - \\begin{thebibliography}{$1} - \\bibitem{$2} - $3 - \\end{thebibliography} - """ - meta: "env" }] documentSnippet = { @@ -132,6 +123,16 @@ define () -> meta: "env" } + bibliographySnippet = { + caption: "\\begin{thebibliography}..." + snippet: """ + \\begin{thebibliography}{$1} + \\bibitem{$2} + $3 + \\end{thebibliography} + """ + meta: "env" + } staticSnippets.push(documentSnippet) parseCustomEnvironments = (text) -> @@ -163,6 +164,12 @@ define () -> iterations = 0 return re.exec(text) != null + hasBibliographyEnvironment = (text) -> + re = /^\\begin{thebibliography}/m + envs = [] + iterations = 0 + return re.exec(text) != null + class EnvironmentManager getCompletions: (editor, session, pos, prefix, callback) -> docText = session.getValue() @@ -175,6 +182,13 @@ define () -> else staticSnippets.push documentSnippet + if hasBibliographyEnvironment(docText) + ind = staticSnippets.indexOf(bibliographySnippet) + if ind != -1 + staticSnippets.splice(ind, 1) + else + staticSnippets.push bibliographySnippet + parsedItemsMap = {} for environment in customEnvironments parsedItemsMap[environment.name] = environment From e753ef3af52c8e67180c08f55ba19cc3f24c346d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Wed, 6 Jun 2018 12:35:13 +0100 Subject: [PATCH 17/44] Invite users in the invited_emails array We'll remove that attribute soon, but for the time being we want users to still be able to join the team. --- .../Subscription/SubscriptionLocator.coffee | 3 +++ .../Subscription/TeamInvitesHandler.coffee | 11 ++++++++++ .../Features/User/UserController.coffee | 4 ++-- .../coffee/Features/User/UserHandler.coffee | 12 +++++----- .../TeamInvitesHandlerTests.coffee | 19 +++++++++++++++- .../coffee/User/UserControllerTests.coffee | 6 ++--- .../unit/coffee/User/UserHandlerTests.coffee | 22 ++++++++++++++++--- 7 files changed, 63 insertions(+), 14 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee index a32447c7e0..33376f504b 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee @@ -30,3 +30,6 @@ module.exports = getGroupSubscriptionsMemberOf: (user_id, callback)-> Subscription.find {member_ids: user_id}, {_id:1, planCode:1}, callback + + getGroupsWithEmailInvite: (email, callback) -> + Subscription.find { invited_emails: email }, callback \ No newline at end of file diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index 9f4f10a810..da00e7b237 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -63,6 +63,17 @@ module.exports = TeamInvitesHandler = removeInviteFromTeam(teamSubscription.id, email, callback) + # Legacy method to allow a user to receive a confirmation email if their + # email is in Subscription.invited_emails when they join. We'll remove this + # after a short while. + createTeamInvitesForLegacyInvitedEmail: (email, callback) -> + SubscriptionLocator.getGroupsWithEmailInvite email, (err, teams) -> + return callback(err) if err? + + async.map teams, + (team, cb) -> TeamInvitesHandler.createManagerInvite(team.admin_id, email, cb) + , callback + createInvite = (subscription, email, inviterName, callback) -> logger.log {subscriptionId: subscription.id, email, inviterName}, "Creating invite" checkIfInviteIsPossible subscription, email, (error, possible, reason) -> diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index 744b77b4de..c25b20a3a6 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/services/web/app/coffee/Features/User/UserController.coffee @@ -110,9 +110,9 @@ module.exports = UserController = logger.err err:err, user_id:user_id, "error getting user for email update" return res.send 500 AuthenticationController.setInSessionUser(req, {email: user.email, first_name: user.first_name, last_name: user.last_name}) - UserHandler.notifyDomainLicence user, (err)-> #need to refresh this in the background + UserHandler.populateTeamInvites user, (err)-> #need to refresh this in the background if err? - logger.err err:err, "error notifyDomainLicence" + logger.err err:err, "error populateTeamInvites" res.sendStatus(200) logout : (req, res)-> diff --git a/services/web/app/coffee/Features/User/UserHandler.coffee b/services/web/app/coffee/Features/User/UserHandler.coffee index df614ca1e0..96aee5a246 100644 --- a/services/web/app/coffee/Features/User/UserHandler.coffee +++ b/services/web/app/coffee/Features/User/UserHandler.coffee @@ -1,11 +1,17 @@ SubscriptionDomainHandler = require("../Subscription/SubscriptionDomainHandler") NotificationsBuilder = require("../Notifications/NotificationsBuilder") SubscriptionGroupHandler = require("../Subscription/SubscriptionGroupHandler") +TeamInvitesHandler = require("../Subscription/TeamInvitesHandler") logger = require("logger-sharelatex") module.exports = UserHandler = + populateTeamInvites: (user, callback) -> + UserHandler.notifyDomainLicence user, (err) -> + return callback(err) if err? + TeamInvitesHandler.createTeamInvitesForLegacyInvitedEmail(user.email, callback) + notifyDomainLicence: (user, callback = ->)-> logger.log user_id:user._id, "notiying user about a potential domain licence" licence = SubscriptionDomainHandler.getLicenceUserCanJoin user @@ -22,8 +28,4 @@ module.exports = UserHandler = NotificationsBuilder.groupPlan(user, licence).create(callback) setupLoginData: (user, callback = ->)-> - @notifyDomainLicence user, callback - - # Backwards compatibility: this is called from the public-registration module - populateGroupLicenceInvite: (user, callback = ->)-> - @notifyDomainLicence user, callback + @populateTeamInvites user, callback diff --git a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee index 2d0bd08f9e..030282b6e6 100644 --- a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee @@ -184,11 +184,28 @@ describe "TeamInvitesHandler", -> ).should.eq true done() + describe "createTeamInvitesForLegacyInvitedEmail", (done) -> + beforeEach -> + @subscription.invited_emails = ["eddard@stark.com", "robert@stark.com"] + @TeamInvitesHandler.createManagerInvite = sinon.stub().yields(null) + @SubscriptionLocator.getGroupsWithEmailInvite = sinon.stub().yields(null, [@subscription]) + + it "sends an invitation email to addresses in the legacy invited_emails field", (done) -> + @TeamInvitesHandler.createTeamInvitesForLegacyInvitedEmail "eddard@stark.com", (err, invite) => + expect(err).not.to.exist + + @TeamInvitesHandler.createManagerInvite.calledWith( + @subscription.admin_id, + "eddard@stark.com" + ).should.eq true + + @TeamInvitesHandler.createManagerInvite.callCount.should.eq 1 + + done() describe "validation", -> it "doesn't create an invite if the team limit has been reached", (done) -> @LimitationsManager.teamHasReachedMemberLimit = sinon.stub().returns(true) @TeamInvitesHandler.createManagerInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => - console.log('err, invite', err, invite) expect(err).to.deep.equal(limitReached: true) done() diff --git a/services/web/test/unit/coffee/User/UserControllerTests.coffee b/services/web/test/unit/coffee/User/UserControllerTests.coffee index 17ac009789..26f345db39 100644 --- a/services/web/test/unit/coffee/User/UserControllerTests.coffee +++ b/services/web/test/unit/coffee/User/UserControllerTests.coffee @@ -55,7 +55,7 @@ describe "UserController", -> @settings = siteUrl: "sharelatex.example.com" @UserHandler = - notifyDomainLicence: sinon.stub().callsArgWith(1) + populateTeamInvites: sinon.stub().callsArgWith(1) @UserSessionsManager = trackSession: sinon.stub() untrackSession: sinon.stub() @@ -267,12 +267,12 @@ describe "UserController", -> done() @UserController.updateUserSettings @req, @res - it "should call notifyDomainLicence", (done)-> + it "should call populateTeamInvites", (done)-> @req.body.email = @newEmail.toUpperCase() @UserUpdater.changeEmailAddress.callsArgWith(2) @res.sendStatus = (code)=> code.should.equal 200 - @UserHandler.notifyDomainLicence.calledWith(@user).should.equal true + @UserHandler.populateTeamInvites.calledWith(@user).should.equal true done() @UserController.updateUserSettings @req, @res diff --git a/services/web/test/unit/coffee/User/UserHandlerTests.coffee b/services/web/test/unit/coffee/User/UserHandlerTests.coffee index 51c9a75dcd..5c82fd7bd3 100644 --- a/services/web/test/unit/coffee/User/UserHandlerTests.coffee +++ b/services/web/test/unit/coffee/User/UserHandlerTests.coffee @@ -23,17 +23,33 @@ describe "UserHandler", -> @NotificationsBuilder = groupPlan:sinon.stub().returns({create:@createStub}) + @TeamInvitesHandler = + createTeamInvitesForLegacyInvitedEmail: sinon.stub().yields() + @UserHandler = SandboxedModule.require modulePath, requires: "logger-sharelatex": @logger = { log: sinon.stub() } "../Notifications/NotificationsBuilder":@NotificationsBuilder "../Subscription/SubscriptionDomainHandler":@SubscriptionDomainHandler "../Subscription/SubscriptionGroupHandler":@SubscriptionGroupHandler + "../Subscription/TeamInvitesHandler": @TeamInvitesHandler + + describe "populateTeamInvites", -> + beforeEach (done)-> + @UserHandler.notifyDomainLicence = sinon.stub().yields() + @UserHandler.populateTeamInvites @user, done + + it "notifies the user about domain licences zzzzz", -> + @UserHandler.notifyDomainLicence.calledWith(@user).should.eq true + + it "notifies the user about legacy team invites", -> + @TeamInvitesHandler.createTeamInvitesForLegacyInvitedEmail + .calledWith(@user.email).should.eq true describe "notifyDomainLicence", -> describe "no licence", -> beforeEach (done)-> @SubscriptionDomainHandler.getLicenceUserCanJoin.returns() - @UserHandler.notifyDomainLicence @user, done + @UserHandler.populateTeamInvites @user, done it "should not call NotificationsBuilder", (done)-> @NotificationsBuilder.groupPlan.called.should.equal false @@ -47,7 +63,7 @@ describe "UserHandler", -> beforeEach (done)-> @SubscriptionDomainHandler.getLicenceUserCanJoin.returns(@licence) @SubscriptionGroupHandler.isUserPartOfGroup.callsArgWith(2, null, false) - @UserHandler.notifyDomainLicence @user, done + @UserHandler.populateTeamInvites @user, done it "should create notifcation", (done)-> @NotificationsBuilder.groupPlan.calledWith(@user, @licence).should.equal true @@ -57,7 +73,7 @@ describe "UserHandler", -> beforeEach (done)-> @SubscriptionDomainHandler.getLicenceUserCanJoin.returns(@licence) @SubscriptionGroupHandler.isUserPartOfGroup.callsArgWith(2, null, true) - @UserHandler.notifyDomainLicence @user, done + @UserHandler.populateTeamInvites @user, done it "should create notifcation", (done)-> @NotificationsBuilder.groupPlan.called.should.equal false From 8a317b5bfe6b24616af2377109018d96ccad7798 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Wed, 6 Jun 2018 12:47:59 +0100 Subject: [PATCH 18/44] Better wording --- services/web/app/coffee/Features/Email/EmailBuilder.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Email/EmailBuilder.coffee b/services/web/app/coffee/Features/Email/EmailBuilder.coffee index 76fa5ebbea..4f3db645a9 100644 --- a/services/web/app/coffee/Features/Email/EmailBuilder.coffee +++ b/services/web/app/coffee/Features/Email/EmailBuilder.coffee @@ -125,9 +125,9 @@ templates.verifyEmailToJoinTeam = type:"notification" plainTextTemplate: _.template """ -Hi, please verify your email to join the team and get your free premium account. +Please click the button below to join the team and enjoy the benefits of an upgraded <%= settings.appName %> account. -Click this link to verify now: <%= acceptInviteUrl %> +<%= acceptInviteUrl %> Thank You @@ -137,7 +137,7 @@ Thank You SingleCTAEmailBody({ title: "#{opts.inviterName} has invited you to join a team on #{settings.appName}" greeting: "Hi," - message: "please verify your email to join the team and get your free premium account" + message: "Join the Team" secondaryMessage: null ctaText: "Verify now" ctaURL: opts.acceptInviteUrl From 16cb5e0d352b2a50cf747237e3cb8db8ca4caf37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Wed, 6 Jun 2018 12:49:02 +0100 Subject: [PATCH 19/44] Propagate the error further up the stack Useful for error reporting and metrics. --- .../Features/Subscription/DomainLicenceController.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/DomainLicenceController.coffee b/services/web/app/coffee/Features/Subscription/DomainLicenceController.coffee index b2d3a1d2c6..82fd4671f1 100644 --- a/services/web/app/coffee/Features/Subscription/DomainLicenceController.coffee +++ b/services/web/app/coffee/Features/Subscription/DomainLicenceController.coffee @@ -33,7 +33,7 @@ module.exports = licenceName: licence.name has_personal_subscription: subscription? - createInvite: (req, res)-> + createInvite: (req, res, next)-> user = AuthenticationController.getSessionUser(req) licence = SubscriptionDomainHandler.getLicenceUserCanJoin(user) @@ -42,6 +42,6 @@ module.exports = TeamInvitesHandler.createDomainInvite user, licence, (err) -> if err? - res.sendStatus 500 + next(err) else res.sendStatus 200 From 553878064be76d45d0e0954a4924586f636cb266 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Wed, 6 Jun 2018 15:13:49 +0100 Subject: [PATCH 20/44] Resend email if the user has already been invited --- .../Subscription/TeamInvitesHandler.coffee | 29 ++++++++----------- .../TeamInvitesHandlerTests.coffee | 24 +++++++++++---- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index da00e7b237..e913e9a16c 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -80,25 +80,26 @@ createInvite = (subscription, email, inviterName, callback) -> return callback(error) if error? return callback(reason) unless possible - token = crypto.randomBytes(32).toString("hex") - # TODO: use standard way to canonalise email addresses - invite = { - email: email.trim().toLowerCase(), - token: token, - inviterName: inviterName, - sentAt: new Date(), - } + email = email.trim().toLowerCase() - subscription.teamInvites.push(invite) + invite = subscription.teamInvites.find (invite) -> invite.email == email + + if !invite? + invite ||= { email: email } + subscription.teamInvites.push(invite) + + invite.inviterName = inviterName + invite.token = crypto.randomBytes(32).toString("hex") + invite.sentAt = new Date() subscription.save (error) -> return callback(error) if error? opts = - to: email.trim().toLowerCase() + to: email inviterName: inviterName - acceptInviteUrl: "#{settings.siteUrl}/subscription/invites/#{token}/" + acceptInviteUrl: "#{settings.siteUrl}/subscription/invites/#{invite.token}/" EmailHandler.sendEmail "verifyEmailToJoinTeam", opts, (error) -> return callback(error, invite) @@ -113,12 +114,6 @@ checkIfInviteIsPossible = (subscription, email, callback = (error, possible, rea logger.log {subscriptionId: subscription.id}, "team has reached member limit" return callback(null, false, limitReached: true) - existingInvite = subscription.teamInvites.find (invite) -> invite.email == email - - if existingInvite - logger.log {subscriptionId: subscription.id, email}, "user already invited" - return callback(null, false, alreadyInvited: true) - async.map subscription.member_ids, UserGetter.getUser, (error, members) -> return callback(error) if error? diff --git a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee index 030282b6e6..3612ebcacd 100644 --- a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee @@ -116,6 +116,23 @@ describe "TeamInvitesHandler", -> ).should.equal true done() + it "refreshes the existing invite if the email has already been invited", (done) -> + originalInvite = Object.assign({}, @teamInvite) + + @TeamInvitesHandler.createManagerInvite @manager.id, originalInvite.email, (err, invite) => + expect(err).to.eq(null) + expect(invite).to.exist + + expect(@subscription.teamInvites.length).to.eq 1 + expect(@subscription.teamInvites).to.deep.include invite + + expect(invite.email).to.eq originalInvite.email + expect(invite.token).not.to.eq originalInvite.token + + @subscription.save.calledOnce.should.eq true + + done() + describe "createDomainInvite", -> beforeEach -> @licence = @@ -202,6 +219,7 @@ describe "TeamInvitesHandler", -> @TeamInvitesHandler.createManagerInvite.callCount.should.eq 1 done() + describe "validation", -> it "doesn't create an invite if the team limit has been reached", (done) -> @LimitationsManager.teamHasReachedMemberLimit = sinon.stub().returns(true) @@ -209,12 +227,6 @@ describe "TeamInvitesHandler", -> expect(err).to.deep.equal(limitReached: true) done() - it "doen't create an invite if the email has already been invited",(done) -> - @TeamInvitesHandler.createManagerInvite @manager.id, "jorah@mormont.org", (err, invite) => - expect(err).to.deep.equal(alreadyInvited: true) - expect(invite).not.to.exist - done() - it "doen't create an invite if the user is already part of the team", (done) -> member = { id: "1a2b", From b308dcef6327536dd749e1c02610f99fb4ded409 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Wed, 6 Jun 2018 15:24:24 +0100 Subject: [PATCH 21/44] Clean up markup - Avoid nested rows without cols in them - Use .row-spaced instead of empty rows to space content --- .../app/views/subscriptions/domain/join.pug | 22 ++++++------------- .../app/views/subscriptions/team/invite.pug | 6 ++--- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/services/web/app/views/subscriptions/domain/join.pug b/services/web/app/views/subscriptions/domain/join.pug index 7c6c214b98..0277710605 100644 --- a/services/web/app/views/subscriptions/domain/join.pug +++ b/services/web/app/views/subscriptions/domain/join.pug @@ -13,30 +13,24 @@ block content -if (query.expired) .alert.alert-warning #{translate("email_link_expired")} - .row - div   - .row + .row.row-spaced .col-md-8.col-md-offset-2(ng-cloak) .card(ng-controller="DomainSubscriptionJoinController") .page-header h1.text-centered #{translate("you_are_invited_to_group", {groupName:licenceName})} - div(ng-show="view =='personalSubscription'").row.text-centered + div(ng-show="view =='personalSubscription'").text-centered div #{translate("cancel_personal_subscription_first")} - .row - .col-md-12   - .row + .row.row-spaced .col-md-12 a.btn.btn.btn-default(ng-click="keepPersonalSubscription()", ng-disabled="inflight") #{translate("not_now")} span   a.btn.btn.btn-primary(ng-click="cancelSubscription()", ng-disabled="inflight") #{translate("cancel_your_subscription")} - div(ng-show="view =='domainSubscriptionJoin'").row.text-centered + div(ng-show="view =='domainSubscriptionJoin'").text-centered .row .col-md-12 #{translate("group_provides_you_with_premium_account", {groupName:licenceName})} - .row - .col-md-12   - .row + .row.row-spaced .col-md-12 .text-center a.btn.btn-default(href="/project") #{translate("not_now")} @@ -44,11 +38,9 @@ block content a.btn.btn.btn-primary(ng-click="joinGroup()", ng-disabled="inflight") #{translate("verify_email_address")} - span(ng-show="view =='requestSent'").row.text-centered.text-center + span(ng-show="view =='requestSent'").text-centered.text-center .row .col-md-12 #{translate("check_email_to_complete_the_upgrade")} - .row - .col-md-12   - .row + .row.row-spaced .col-md-12 a.btn.btn.btn-primary(href="/project") #{translate("done")} diff --git a/services/web/app/views/subscriptions/team/invite.pug b/services/web/app/views/subscriptions/team/invite.pug index d396bb4155..10a06da3dc 100644 --- a/services/web/app/views/subscriptions/team/invite.pug +++ b/services/web/app/views/subscriptions/team/invite.pug @@ -14,15 +14,13 @@ block content -if (query.expired) .alert.alert-warning #{translate("email_link_expired")} - .row - div   - .row + .row.row-spaced .col-md-8.col-md-offset-2(ng-cloak) .card(ng-controller="TeamInviteController") .page-header h1.text-centered #{translate("invited_to_group", {inviterName: inviterName, appName: appName})} - div(ng-show="view =='personalSubscription'").row.text-centered.message + div(ng-show="view =='personalSubscription'").text-centered.message p #{translate("cancel_personal_subscription_first")} .row .col-md-12 From 06efe1910b01e1378237f7f6516112ec948d0d42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Wed, 6 Jun 2018 15:30:40 +0100 Subject: [PATCH 22/44] Replace hardcoded string --- services/web/app/views/subscriptions/team/invite.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/views/subscriptions/team/invite.pug b/services/web/app/views/subscriptions/team/invite.pug index 10a06da3dc..f56fc8f879 100644 --- a/services/web/app/views/subscriptions/team/invite.pug +++ b/services/web/app/views/subscriptions/team/invite.pug @@ -39,7 +39,7 @@ block content div(ng-show="view =='inviteAccepted'").row.text-centered.text-center .row - .col-md-12 You have joined the team managed by John Smith + .col-md-12 #{translate("joined_team", {inviterName: inviterName})} .row .col-md-12   .row From 4885b700165d5fccc14fe2f2e7accba8c39d89ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Wed, 6 Jun 2018 16:33:01 +0100 Subject: [PATCH 23/44] Better translation key --- services/web/app/views/subscriptions/team/invite.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/views/subscriptions/team/invite.pug b/services/web/app/views/subscriptions/team/invite.pug index f56fc8f879..327d3bb2aa 100644 --- a/services/web/app/views/subscriptions/team/invite.pug +++ b/services/web/app/views/subscriptions/team/invite.pug @@ -29,7 +29,7 @@ block content a.btn.btn.btn-primary(ng-click="cancelPersonalSubscription()", ng-disabled="inflight") #{translate("cancel_your_subscription")} div(ng-show="view =='teamInvite'").row.text-centered.message - p #{translate("accept_invitation_gives_premium_account")} + p #{translate("join_team_explanation", {appName: appName})} .row .col-md-12 .text-center From 1fc047d08e60f8f5f357b3fbc623cf155e8c930e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Wed, 6 Jun 2018 17:11:25 +0100 Subject: [PATCH 24/44] Fix invitation resend --- .../app/coffee/Features/Email/EmailBuilder.coffee | 2 +- .../Subscription/TeamInvitesHandler.coffee | 14 +++++++++----- .../Subscription/TeamInvitesHandlerTests.coffee | 1 - 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/services/web/app/coffee/Features/Email/EmailBuilder.coffee b/services/web/app/coffee/Features/Email/EmailBuilder.coffee index 4f3db645a9..2ff07c3864 100644 --- a/services/web/app/coffee/Features/Email/EmailBuilder.coffee +++ b/services/web/app/coffee/Features/Email/EmailBuilder.coffee @@ -125,7 +125,7 @@ templates.verifyEmailToJoinTeam = type:"notification" plainTextTemplate: _.template """ -Please click the button below to join the team and enjoy the benefits of an upgraded <%= settings.appName %> account. +Please click the button below to join the team and enjoy the benefits of an upgraded <%= appName %> account. <%= acceptInviteUrl %> diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index e913e9a16c..c28546ffb3 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -86,12 +86,15 @@ createInvite = (subscription, email, inviterName, callback) -> invite = subscription.teamInvites.find (invite) -> invite.email == email if !invite? - invite ||= { email: email } + invite = { + email: email + inviterName: inviterName + token: crypto.randomBytes(32).toString("hex") + sentAt: new Date() + } subscription.teamInvites.push(invite) - - invite.inviterName = inviterName - invite.token = crypto.randomBytes(32).toString("hex") - invite.sentAt = new Date() + else + invite.sentAt = new Date() subscription.save (error) -> return callback(error) if error? @@ -100,6 +103,7 @@ createInvite = (subscription, email, inviterName, callback) -> to: email inviterName: inviterName acceptInviteUrl: "#{settings.siteUrl}/subscription/invites/#{invite.token}/" + appName: settings.appName EmailHandler.sendEmail "verifyEmailToJoinTeam", opts, (error) -> return callback(error, invite) diff --git a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee index 3612ebcacd..84069edcaf 100644 --- a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee @@ -127,7 +127,6 @@ describe "TeamInvitesHandler", -> expect(@subscription.teamInvites).to.deep.include invite expect(invite.email).to.eq originalInvite.email - expect(invite.token).not.to.eq originalInvite.token @subscription.save.calledOnce.should.eq true From 959db8017790a8029a6fc2a054ab178e19f1a125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 7 Jun 2018 12:49:46 +0100 Subject: [PATCH 25/44] Check all emails to prevent invite duplicates --- .../Features/Subscription/TeamInvitesHandler.coffee | 6 ++++-- .../Subscription/TeamInvitesHandlerTests.coffee | 12 ++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index c28546ffb3..9f6797bcb9 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -118,10 +118,12 @@ checkIfInviteIsPossible = (subscription, email, callback = (error, possible, rea logger.log {subscriptionId: subscription.id}, "team has reached member limit" return callback(null, false, limitReached: true) - async.map subscription.member_ids, UserGetter.getUser, (error, members) -> + UserGetter.getUserByAnyEmail email, (error, existingUser) -> return callback(error) if error? + return callback(null, true) unless existingUser? - existingMember = members.find (member) -> member.email == email + existingMember = subscription.member_ids.find (memberId) -> + memberId.toString() == existingUser._id.toString() if existingMember logger.log {subscriptionId: subscription.id, email}, "user already in team" diff --git a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee index 84069edcaf..eacab19d04 100644 --- a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee @@ -37,7 +37,8 @@ describe "TeamInvitesHandler", -> } @UserGetter = { - getUser: sinon.stub() + getUser: sinon.stub().yields(), + getUserByAnyEmail: sinon.stub().yields() } @SubscriptionUpdater = { @@ -65,6 +66,8 @@ describe "TeamInvitesHandler", -> } @UserGetter.getUser.withArgs(@manager.id).yields(null, @manager) + @UserGetter.getUserByAnyEmail.withArgs(@manager.email).yields(null, @manager) + @SubscriptionLocator.getUsersSubscription.yields(null, @subscription) @Subscription.findOne.yields(null, @subscription) @@ -170,7 +173,7 @@ describe "TeamInvitesHandler", -> email: "tyrion@lannister.com" } - @UserGetter.getUser.withArgs(@user.id).yields(null, @user) + @UserGetter.getUserByAnyEmail.withArgs(@user.email).yields(null, @user) @subscription.teamInvites.push({ email: "john.snow@nightwatch.com", @@ -226,14 +229,15 @@ describe "TeamInvitesHandler", -> expect(err).to.deep.equal(limitReached: true) done() - it "doen't create an invite if the user is already part of the team", (done) -> + it "doesn't create an invite if the user is already part of the team", (done) -> member = { id: "1a2b", + _id: "1a2b", email: "tyrion@lannister.com" } @subscription.member_ids = [member.id] - @UserGetter.getUser.withArgs(member.id).yields(null, member) + @UserGetter.getUserByAnyEmail.withArgs(member.email).yields(null, member) @TeamInvitesHandler.createManagerInvite @manager.id, "tyrion@lannister.com", (err, invite) => expect(err).to.deep.equal(alreadyInTeam: true) From 6639f61a431455ffaa48544be4e1f6efd6d6d252 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 7 Jun 2018 14:05:10 +0100 Subject: [PATCH 26/44] Remove extra space between rows --- services/web/app/views/subscriptions/domain/join.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/views/subscriptions/domain/join.pug b/services/web/app/views/subscriptions/domain/join.pug index 0277710605..1d528102b8 100644 --- a/services/web/app/views/subscriptions/domain/join.pug +++ b/services/web/app/views/subscriptions/domain/join.pug @@ -30,7 +30,7 @@ block content div(ng-show="view =='domainSubscriptionJoin'").text-centered .row .col-md-12 #{translate("group_provides_you_with_premium_account", {groupName:licenceName})} - .row.row-spaced + .row .col-md-12 .text-center a.btn.btn-default(href="/project") #{translate("not_now")} From 5e70825c9426783132dedff071d24c3ec42298d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 7 Jun 2018 14:10:01 +0100 Subject: [PATCH 27/44] Rename view variable to invite Makes the meaning more explicit --- .../Features/Subscription/SubscriptionGroupHandler.coffee | 4 ++-- .../coffee/Features/Subscription/TeamInvitesController.coffee | 2 +- services/web/app/views/subscriptions/group_admin.pug | 4 ++-- services/web/public/coffee/main/group-members.coffee | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index 619ee49351..2744a7786a 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -76,12 +76,12 @@ buildUserViewModel = (user)-> email: user.email first_name: user.first_name last_name: user.last_name - holdingAccount: user.holdingAccount + invite: user.holdingAccount _id: user._id return u buildEmailInviteViewModel = (email) -> return { email: email - holdingAccount: true + invite: true } diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee index 565cdffcf1..4d858fa6f6 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee @@ -13,7 +13,7 @@ module.exports = TeamInvitesHandler.createManagerInvite teamManagerId, email, (err, invite) -> return handleError(err, req, res, next) if err? inviteView = { user: - { email: invite.email, sentAt: invite.sentAt, holdingAccount: true } + { email: invite.email, sentAt: invite.sentAt, invite: true } } res.json inviteView diff --git a/services/web/app/views/subscriptions/group_admin.pug b/services/web/app/views/subscriptions/group_admin.pug index 3fe93f3b94..194cd090fa 100644 --- a/services/web/app/views/subscriptions/group_admin.pug +++ b/services/web/app/views/subscriptions/group_admin.pug @@ -49,8 +49,8 @@ block content span.name {{ user.first_name }} {{ user.last_name }} .col-md-2 span.registered - i.fa.fa-check.text-success(ng-show="!user.holdingAccount") - i.fa.fa-times(ng-show="user.holdingAccount") + i.fa.fa-check.text-success(ng-show="!user.invite") + i.fa.fa-times(ng-show="user.invite") li( ng-if="users.length == 0", ng-cloak diff --git a/services/web/public/coffee/main/group-members.coffee b/services/web/public/coffee/main/group-members.coffee index 1d7e7ff104..028639c741 100644 --- a/services/web/public/coffee/main/group-members.coffee +++ b/services/web/public/coffee/main/group-members.coffee @@ -34,7 +34,7 @@ define [ $scope.removeMembers = () -> for user in $scope.selectedUsers do (user) -> - if user.holdingAccount and !user._id? + if user.invite and !user._id? url = "/subscription/invites/#{encodeURIComponent(user.email)}" else url = "/subscription/group/user/#{user._id}" From 88d12b43ac46ab0facdc03c7fcd4499602ec9b63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 7 Jun 2018 14:21:53 +0100 Subject: [PATCH 28/44] Make CTA message match regular message --- services/web/app/coffee/Features/Email/EmailBuilder.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Email/EmailBuilder.coffee b/services/web/app/coffee/Features/Email/EmailBuilder.coffee index 2ff07c3864..8952e7b864 100644 --- a/services/web/app/coffee/Features/Email/EmailBuilder.coffee +++ b/services/web/app/coffee/Features/Email/EmailBuilder.coffee @@ -137,7 +137,7 @@ Thank You SingleCTAEmailBody({ title: "#{opts.inviterName} has invited you to join a team on #{settings.appName}" greeting: "Hi," - message: "Join the Team" + message: "Please click the button below to join the team and enjoy the benefits of an upgraded #{ opts.appName } account." secondaryMessage: null ctaText: "Verify now" ctaURL: opts.acceptInviteUrl From d1b1e6c29971964d9d6ef0f80425b117c037df9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 7 Jun 2018 15:01:48 +0100 Subject: [PATCH 29/44] Do not include the Licence ending in domain invites So the message reads like "University of Notre Dame has invited you to join a team on Overleaf" instead of ""University of Notre Dame licence..." --- .../coffee/Features/Subscription/TeamInvitesHandler.coffee | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index 9f6797bcb9..3164585f7a 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -41,9 +41,11 @@ module.exports = TeamInvitesHandler = createDomainInvite: (user, licence, callback) -> logger.log {licence, email: user.email}, "Creating domain team invite" + inviterName = licence.name.replace(/\s+licence$/i, licence.name) + SubscriptionLocator.getSubscription licence.subscription_id, (error, subscription) -> return callback(error) if error? - createInvite(subscription, user.email, licence.name, callback) + createInvite(subscription, user.email, inviterName, callback) acceptInvite: (token, userId, callback) -> logger.log {userId}, "Accepting invite" From 30b935befd30b73aa46b14c758916e89f5bef951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 7 Jun 2018 15:05:04 +0100 Subject: [PATCH 30/44] Prevent double rendering error --- .../coffee/Features/Subscription/TeamInvitesController.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee index 4d858fa6f6..ddc2a27888 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee @@ -43,7 +43,7 @@ module.exports = TeamInvitesHandler.acceptInvite token, userId, (err, results) -> return handleError(err, req, res, next) if err? - res.sendStatus 204 + res.sendStatus 204 revokeInvite: (req, res) -> email = req.params.email From 670f24ef6f1b56d6e7e8d6fca8b0c7f4ed885751 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 7 Jun 2018 15:35:18 +0100 Subject: [PATCH 31/44] Simplify method name --- .../SubscriptionGroupHandler.coffee | 2 +- .../Subscription/TeamInvitesController.coffee | 2 +- .../Subscription/TeamInvitesHandler.coffee | 4 ++-- .../SubscriptionGroupHandlerTests.coffee | 4 ++-- .../TeamInvitesHandlerTests.coffee | 18 +++++++++--------- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index 2744a7786a..af5b586451 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -33,7 +33,7 @@ module.exports = SubscriptionGroupHandler = userViewModel = buildUserViewModel(user) callback(err, userViewModel) else - TeamInvitesHandler.createManagerInvite adminUserId, newEmail, (err) -> + TeamInvitesHandler.createInvite adminUserId, newEmail, (err) -> return callback(err) if err? userViewModel = buildEmailInviteViewModel(newEmail) callback(err, userViewModel) diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee index ddc2a27888..f4f0933fdd 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee @@ -10,7 +10,7 @@ module.exports = teamManagerId = AuthenticationController.getLoggedInUserId(req) email = req.body.email - TeamInvitesHandler.createManagerInvite teamManagerId, email, (err, invite) -> + TeamInvitesHandler.createInvite teamManagerId, email, (err, invite) -> return handleError(err, req, res, next) if err? inviteView = { user: { email: invite.email, sentAt: invite.sentAt, invite: true } diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index 3164585f7a..69aff6cf81 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -24,7 +24,7 @@ module.exports = TeamInvitesHandler = invite = subscription.teamInvites.find (i) -> i.token == token return callback(null, invite, subscription) - createManagerInvite: (teamManagerId, email, callback) -> + createInvite: (teamManagerId, email, callback) -> logger.log {teamManagerId, email}, "Creating manager team invite" UserGetter.getUser teamManagerId, (error, teamManager) -> return callback(error) if error? @@ -73,7 +73,7 @@ module.exports = TeamInvitesHandler = return callback(err) if err? async.map teams, - (team, cb) -> TeamInvitesHandler.createManagerInvite(team.admin_id, email, cb) + (team, cb) -> TeamInvitesHandler.createInvite(team.admin_id, email, cb) , callback createInvite = (subscription, email, inviterName, callback) -> diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee index 4a2e01fb05..8b35a36fe7 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee @@ -29,7 +29,7 @@ describe "SubscriptionGroupHandler", -> removeUserFromGroup: sinon.stub().callsArgWith(2) @TeamInvitesHandler = - createManagerInvite: sinon.stub().callsArgWith(2) + createInvite: sinon.stub().callsArgWith(2) @UserGetter = getUser: sinon.stub() @@ -106,7 +106,7 @@ describe "SubscriptionGroupHandler", -> it "should add an email invite if no user is found", (done) -> @UserGetter.getUserByMainEmail.callsArgWith(1, null, null) @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> - @TeamInvitesHandler.createManagerInvite.calledWith(@adminUser_id, @newEmail).should.equal true + @TeamInvitesHandler.createInvite.calledWith(@adminUser_id, @newEmail).should.equal true done() describe "removeUserFromGroup", -> diff --git a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee index eacab19d04..8a977a8706 100644 --- a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee @@ -98,9 +98,9 @@ describe "TeamInvitesHandler", -> expect(err).to.deep.eq(teamNotFound: true) done() - describe "createManagerInvite", -> + describe "createInvite", -> it "adds the team invite to the subscription", (done) -> - @TeamInvitesHandler.createManagerInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => expect(err).to.eq(null) expect(invite.token).to.eq(@newToken) expect(invite.email).to.eq("john.snow@nightwatch.com") @@ -109,7 +109,7 @@ describe "TeamInvitesHandler", -> done() it "sends an email", (done) -> - @TeamInvitesHandler.createManagerInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => @EmailHandler.sendEmail.calledWith("verifyEmailToJoinTeam", sinon.match({ to: "john.snow@nightwatch.com", @@ -122,7 +122,7 @@ describe "TeamInvitesHandler", -> it "refreshes the existing invite if the email has already been invited", (done) -> originalInvite = Object.assign({}, @teamInvite) - @TeamInvitesHandler.createManagerInvite @manager.id, originalInvite.email, (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, originalInvite.email, (err, invite) => expect(err).to.eq(null) expect(invite).to.exist @@ -206,26 +206,26 @@ describe "TeamInvitesHandler", -> describe "createTeamInvitesForLegacyInvitedEmail", (done) -> beforeEach -> @subscription.invited_emails = ["eddard@stark.com", "robert@stark.com"] - @TeamInvitesHandler.createManagerInvite = sinon.stub().yields(null) + @TeamInvitesHandler.createInvite = sinon.stub().yields(null) @SubscriptionLocator.getGroupsWithEmailInvite = sinon.stub().yields(null, [@subscription]) it "sends an invitation email to addresses in the legacy invited_emails field", (done) -> @TeamInvitesHandler.createTeamInvitesForLegacyInvitedEmail "eddard@stark.com", (err, invite) => expect(err).not.to.exist - @TeamInvitesHandler.createManagerInvite.calledWith( + @TeamInvitesHandler.createInvite.calledWith( @subscription.admin_id, "eddard@stark.com" ).should.eq true - @TeamInvitesHandler.createManagerInvite.callCount.should.eq 1 + @TeamInvitesHandler.createInvite.callCount.should.eq 1 done() describe "validation", -> it "doesn't create an invite if the team limit has been reached", (done) -> @LimitationsManager.teamHasReachedMemberLimit = sinon.stub().returns(true) - @TeamInvitesHandler.createManagerInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => expect(err).to.deep.equal(limitReached: true) done() @@ -239,7 +239,7 @@ describe "TeamInvitesHandler", -> @subscription.member_ids = [member.id] @UserGetter.getUserByAnyEmail.withArgs(member.email).yields(null, member) - @TeamInvitesHandler.createManagerInvite @manager.id, "tyrion@lannister.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, "tyrion@lannister.com", (err, invite) => expect(err).to.deep.equal(alreadyInTeam: true) expect(invite).not.to.exist done() From a3bb99d7558527ade58efb473d0c8c02c26ede54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 7 Jun 2018 16:22:38 +0100 Subject: [PATCH 32/44] Honour invited_emails for the team being We'll remove them soon, but we want to keep them for a while so recent invites can join their teams. --- .../Subscription/LimitationsManager.coffee | 5 ++++- .../Subscription/SubscriptionGroupHandler.coffee | 3 +++ .../Subscription/SubscriptionUpdater.coffee | 7 +++++++ .../Subscription/TeamInvitesHandler.coffee | 14 +++++++++++--- .../Subscription/TeamInvitesHandlerTests.coffee | 5 +++++ 5 files changed, 30 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee b/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee index 9b5137392f..92011a5f22 100644 --- a/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee +++ b/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee @@ -56,7 +56,10 @@ module.exports = LimitationsManager = callback err, subscriptions.length > 0, subscriptions teamHasReachedMemberLimit: (subscription) -> - currentTotal = (subscription.member_ids or []).length + (subscription.teamInvites or []).length + currentTotal = (subscription.member_ids or []).length + + (subscription.teamInvites or []).length + + (subscription.invited_emails or []).length + return currentTotal >= subscription.membersLimit hasGroupMembersLimitReached: (user_id, callback = (err, limitReached, subscription)->)-> diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index af5b586451..37f535f36c 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -47,6 +47,9 @@ module.exports = SubscriptionGroupHandler = users = [] + for email in subscription.invited_emails or [] + users.push buildEmailInviteViewModel(email) + for teamInvite in subscription.teamInvites or [] users.push buildEmailInviteViewModel(teamInvite.email) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee index 16dc5adf25..58fea8c23d 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee @@ -47,6 +47,13 @@ module.exports = SubscriptionUpdater = return callback(err) FeaturesUpdater.refreshFeatures user_id, callback + removeEmailInviteFromGroup: (adminUser_id, email, callback)-> + Subscription.update { + admin_id: adminUser_id + }, "$pull": { + invited_emails: email + }, callback + deleteSubscription: (subscription_id, callback = (error) ->) -> SubscriptionLocator.getSubscription subscription_id, (err, subscription) -> return callback(err) if err? diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index 69aff6cf81..885e729394 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -110,10 +110,18 @@ createInvite = (subscription, email, inviterName, callback) -> return callback(error, invite) removeInviteFromTeam = (subscriptionId, email, callback) -> - searchConditions = { _id: new ObjectId(subscriptionId.toString()) } - updateOp = { $pull: { teamInvites: { email: email.trim().toLowerCase() } } } + email = email.trim().toLowerCase() + + searchConditions = { _id: new ObjectId(subscriptionId.toString()) } + + removeInvite = { $pull: { teamInvites: { email: email } } } + removeLegacyInvite = { $pull: { invited_emails: email } } + + async.series [ + (cb) -> Subscription.update(searchConditions, removeInvite, cb), + (cb) -> Subscription.update(searchConditions, removeLegacyInvite, cb), + ], callback - Subscription.update(searchConditions, updateOp, callback) checkIfInviteIsPossible = (subscription, email, callback = (error, possible, reason) -> ) -> if LimitationsManager.teamHasReachedMemberLimit(subscription) diff --git a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee index 8a977a8706..81a1d2718f 100644 --- a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee @@ -201,6 +201,11 @@ describe "TeamInvitesHandler", -> { _id: new ObjectId("55153a8014829a865bbf700d") }, { '$pull': { teamInvites: { email: "jorah@mormont.org" } } } ).should.eq true + + @Subscription.update.calledWith( + { _id: new ObjectId("55153a8014829a865bbf700d") }, + { '$pull': { invited_emails: "jorah@mormont.org" } } + ).should.eq true done() describe "createTeamInvitesForLegacyInvitedEmail", (done) -> From 85f4a31585732f850d412122d807402651476ba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 7 Jun 2018 16:49:17 +0100 Subject: [PATCH 33/44] Proper coffee syntax --- .../coffee/Features/Subscription/LimitationsManager.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee b/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee index 92011a5f22..d7144b0ba0 100644 --- a/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee +++ b/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee @@ -56,9 +56,9 @@ module.exports = LimitationsManager = callback err, subscriptions.length > 0, subscriptions teamHasReachedMemberLimit: (subscription) -> - currentTotal = (subscription.member_ids or []).length - + (subscription.teamInvites or []).length - + (subscription.invited_emails or []).length + currentTotal = (subscription.member_ids or []).length + + (subscription.teamInvites or []).length + + (subscription.invited_emails or []).length return currentTotal >= subscription.membersLimit From c4250e601ecfa6bb8fdd494d3f5d7c4758c41884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 7 Jun 2018 16:53:19 +0100 Subject: [PATCH 34/44] Minor copy change Registered -> Accepted invite --- services/web/app/views/subscriptions/group_admin.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/views/subscriptions/group_admin.pug b/services/web/app/views/subscriptions/group_admin.pug index 194cd090fa..cdb18a0320 100644 --- a/services/web/app/views/subscriptions/group_admin.pug +++ b/services/web/app/views/subscriptions/group_admin.pug @@ -32,7 +32,7 @@ block content .col-md-5 span.header #{translate("name")} .col-md-2 - span.header #{translate("registered")} + span.header #{translate("accepted_invite")} li.container-fluid( ng-repeat="user in users | orderBy:'email':true", ng-controller="SubscriptionGroupMemberListItemController" From 5159f6f33da494492c14385703f1a760dc41f91e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Fri, 8 Jun 2018 09:58:51 +0100 Subject: [PATCH 35/44] Check the subscription is in a group plan before adding new members --- .../Features/Subscription/TeamInvitesHandler.coffee | 5 +++++ .../Subscription/TeamInvitesHandlerTests.coffee | 13 ++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index 885e729394..b84cda82f0 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -124,6 +124,11 @@ removeInviteFromTeam = (subscriptionId, email, callback) -> checkIfInviteIsPossible = (subscription, email, callback = (error, possible, reason) -> ) -> + unless subscription.groupPlan + logger.log {subscriptionId: subscription.id}, + "can not add members to a subscription that is not in a group plan" + return callback(null, false, wrongPlan: true) + if LimitationsManager.teamHasReachedMemberLimit(subscription) logger.log {subscriptionId: subscription.id}, "team has reached member limit" return callback(null, false, limitReached: true) diff --git a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee index 81a1d2718f..720c8fadf7 100644 --- a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee @@ -26,9 +26,10 @@ describe "TeamInvitesHandler", -> @subscription = { id: "55153a8014829a865bbf700d", admin_id: @manager.id, - member_ids: [] - teamInvites: [ @teamInvite ] - save: sinon.stub().yields(null) + groupPlan: true, + member_ids: [], + teamInvites: [ @teamInvite ], + save: sinon.stub().yields(null), } @SubscriptionLocator = { @@ -234,6 +235,12 @@ describe "TeamInvitesHandler", -> expect(err).to.deep.equal(limitReached: true) done() + it "doesn't create an invite if the subscription is not in a group plan", (done) -> + @subscription.groupPlan = false + @TeamInvitesHandler.createInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => + expect(err).to.deep.equal(wrongPlan: true) + done() + it "doesn't create an invite if the user is already part of the team", (done) -> member = { id: "1a2b", From c45b4463bbe144ff9a9cf2d50a53820b432d9cc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Fri, 8 Jun 2018 10:05:26 +0100 Subject: [PATCH 36/44] Fix unit test --- .../SubscriptionGroupHandlerTests.coffee | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee index 8b35a36fe7..1bb2b70a32 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee @@ -74,7 +74,7 @@ describe "SubscriptionGroupHandler", -> beforeEach -> @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription) @UserGetter.getUserByMainEmail.callsArgWith(1, null, @user) - + it "should find the user", (done)-> @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> @UserGetter.getUserByMainEmail.calledWith(@newEmail).should.equal true @@ -102,7 +102,7 @@ describe "SubscriptionGroupHandler", -> @NotificationsBuilder.groupPlan.calledWith(@user, {subscription_id:@subscription._id}).should.equal true @readStub.called.should.equal true done() - + it "should add an email invite if no user is found", (done) -> @UserGetter.getUserByMainEmail.callsArgWith(1, null, null) @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> @@ -149,16 +149,18 @@ describe "SubscriptionGroupHandler", -> done() it "should return any invited users", (done) -> + @subscription.invited_emails = [ "jo@example.com" ] + @subscription.teamInvites = [ - { email: "jo@example.com" }, { email: "charlie@example.com" } ] + @Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=> users[0].email.should.equal "jo@example.com" - users[0].holdingAccount.should.equal true + users[0].invite.should.equal true users[1].email.should.equal "charlie@example.com" - users[1].holdingAccount.should.equal true - users.length.should.equal @subscription.teamInvites.length + users[1].invite.should.equal true + users.length.should.equal @subscription.teamInvites.length + @subscription.invited_emails.length done() describe "isUserPartOfGroup", -> From 43da34098eb8cf45021ce7ea8f5443ec25ba2a0e Mon Sep 17 00:00:00 2001 From: Jessica Lawshe Date: Fri, 8 Jun 2018 10:07:44 -0500 Subject: [PATCH 37/44] Existence check --- services/web/app/views/layout.pug | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/app/views/layout.pug b/services/web/app/views/layout.pug index 082e774d26..0fc6ef1b2b 100644 --- a/services/web/app/views/layout.pug +++ b/services/web/app/views/layout.pug @@ -54,7 +54,7 @@ html(itemscope, itemtype='http://schema.org/Product') meta(itemprop="image", name="image", content=metaImage.fields.file.url) //- Meta Tags: Twitter - -if (typeof(settings.social.twitter.handle) != "undefined") + -if (settings.social && settings.social.twitter && settings.social.twitter.handle) meta(name="twitter:site", content=settings.social.twitter.handle) meta(name="twitter:card", content=metaTwitterCard ? metaTwitterCard : 'summary') -if (typeof(metaTwitterDescription) != "undefined") @@ -65,7 +65,7 @@ html(itemscope, itemtype='http://schema.org/Product') meta(itemprop="image", name="image", content=metaImage.fields.file.url) //- Meta Tags: Open Graph - -if (typeof(settings.social.facebook.appId) != "undefined") + -if (settings.social && settings.social.facebook && settings.social.facebook.appId) meta(name="fb:app_id", content=settings.social.facebook.appId) -if (typeof(metaOpenGraphDescription) != "undefined") meta(itemprop="og:description", content=metaOpenGraphDescription) From 31827ae6b57c07111367e4b2c3edd3c9e966b2b7 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Mon, 28 May 2018 16:08:37 +0200 Subject: [PATCH 38/44] add emails attribute on user creation --- .../coffee/Features/User/UserCreator.coffee | 4 +++ .../coffee/Features/User/UserGetter.coffee | 9 ++++++- .../User/UserRegistrationHandler.coffee | 2 +- .../coffee/Features/User/UserUpdater.coffee | 11 +------- services/web/app/coffee/models/User.coffee | 4 +++ .../coffee/RegistrationTests.coffee | 14 ++++++++++ .../acceptance/coffee/helpers/User.coffee | 27 +++++++++++++++---- .../unit/coffee/User/UserCreatorTests.coffee | 8 ++++++ .../unit/coffee/User/UserGetterTests.coffee | 17 ++++++++++++ .../User/UserRegistrationHandlerTests.coffee | 8 +++--- .../unit/coffee/User/UserUpdaterTests.coffee | 17 +++--------- 11 files changed, 86 insertions(+), 35 deletions(-) diff --git a/services/web/app/coffee/Features/User/UserCreator.coffee b/services/web/app/coffee/Features/User/UserCreator.coffee index 0a0cc8641e..4b4dd30ae2 100644 --- a/services/web/app/coffee/Features/User/UserCreator.coffee +++ b/services/web/app/coffee/Features/User/UserCreator.coffee @@ -18,6 +18,10 @@ module.exports = UserCreator = user.ace.syntaxValidation = true user.featureSwitches?.pdfng = true + user.emails = [ + email: user.email + createdAt: new Date() + ] user.save (err)-> callback(err, user) diff --git a/services/web/app/coffee/Features/User/UserGetter.coffee b/services/web/app/coffee/Features/User/UserGetter.coffee index 2e6d4eada6..4ba750be17 100644 --- a/services/web/app/coffee/Features/User/UserGetter.coffee +++ b/services/web/app/coffee/Features/User/UserGetter.coffee @@ -64,12 +64,19 @@ module.exports = UserGetter = return callback(null, user) if user? db.userstubs.findOne query, projection, callback + # check for duplicate email address. This is also enforced at the DB level + ensureUniqueEmailAddress: (newEmail, callback) -> + @getUserByAnyEmail newEmail, (error, user) -> + return callback(message: 'alread_exists') if user? + callback(error) + [ 'getUser', 'getUserEmail', 'getUserByMainEmail', 'getUserByAnyEmail', 'getUsers', - 'getUserOrUserStubById' + 'getUserOrUserStubById', + 'ensureUniqueEmailAddress', ].map (method) -> metrics.timeAsyncMethod UserGetter, method, 'mongo.UserGetter', logger diff --git a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee index fab438ffa6..4a42d7f5fa 100644 --- a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee +++ b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee @@ -48,7 +48,7 @@ module.exports = UserRegistrationHandler = if !requestIsValid return callback(new Error("request is not valid")) userDetails.email = userDetails.email?.trim()?.toLowerCase() - UserGetter.getUserByMainEmail userDetails.email, (err, user) => + UserGetter.getUserByAnyEmail userDetails.email, (err, user) => if err? return callback err if user?.holdingAccount == false diff --git a/services/web/app/coffee/Features/User/UserUpdater.coffee b/services/web/app/coffee/Features/User/UserUpdater.coffee index fa9ee24450..22b31239bd 100644 --- a/services/web/app/coffee/Features/User/UserUpdater.coffee +++ b/services/web/app/coffee/Features/User/UserUpdater.coffee @@ -43,7 +43,7 @@ module.exports = UserUpdater = # Add a new email address for the user. Email cannot be already used by this # or any other user addEmailAddress: (userId, newEmail, callback) -> - @_ensureUniqueEmailAddress newEmail, (error) => + UserGetter.ensureUniqueEmailAddress newEmail, (error) => return callback(error) if error? update = $push: emails: email: newEmail, createdAt: new Date() @@ -81,20 +81,11 @@ module.exports = UserUpdater = return callback(new Error('Default email does not belong to user')) callback() - - # check for duplicate email address. This is also enforced at the DB level - _ensureUniqueEmailAddress: (newEmail, callback) -> - UserGetter.getUserByAnyEmail newEmail, (error, user) -> - return callback(message: 'alread_exists') if user? - callback() - - [ 'updateUser' 'changeEmailAddress' 'setDefaultEmailAddress' 'addEmailAddress' 'removeEmailAddress' - '_ensureUniqueEmailAddress' ].map (method) -> metrics.timeAsyncMethod(UserUpdater, method, 'mongo.UserUpdater', logger) diff --git a/services/web/app/coffee/models/User.coffee b/services/web/app/coffee/models/User.coffee index 009f582b2a..71982ba40b 100644 --- a/services/web/app/coffee/models/User.coffee +++ b/services/web/app/coffee/models/User.coffee @@ -8,6 +8,10 @@ ObjectId = Schema.ObjectId UserSchema = new Schema email : {type : String, default : ''} + emails: [{ + email: { type : String, default : '' }, + createdAt: { type : Date, default: () -> new Date() } + }], first_name : {type : String, default : ''} last_name : {type : String, default : ''} role : {type : String, default : ''} diff --git a/services/web/test/acceptance/coffee/RegistrationTests.coffee b/services/web/test/acceptance/coffee/RegistrationTests.coffee index 89d1bf3299..8bcf8c3685 100644 --- a/services/web/test/acceptance/coffee/RegistrationTests.coffee +++ b/services/web/test/acceptance/coffee/RegistrationTests.coffee @@ -128,6 +128,20 @@ describe "CSRF protection", -> expect(response.statusCode).to.equal 403 done() +describe "Register", -> + before -> + @user = new User() + + it 'Set emails attribute', (done) -> + @user.register (error, user) => + expect(error).to.not.exist + user.email.should.equal @user.email + user.emails.should.exist + user.emails.should.be.a 'array' + user.emails.length.should.equal 1 + user.emails[0].email.should.equal @user.email + done() + describe "LoginViaRegistration", -> before (done) -> diff --git a/services/web/test/acceptance/coffee/helpers/User.coffee b/services/web/test/acceptance/coffee/helpers/User.coffee index f83780b535..793ac6cd9f 100644 --- a/services/web/test/acceptance/coffee/helpers/User.coffee +++ b/services/web/test/acceptance/coffee/helpers/User.coffee @@ -22,9 +22,30 @@ class User jar: @jar }) + setExtraAttributes: (user) -> + throw new Error("User does not exist") unless user?._id? + @id = user._id.toString() + @_id = user._id.toString() + @first_name = user.first_name + @referal_id = user.referal_id + get: (callback = (error, user)->) -> db.users.findOne { _id: ObjectId(@_id) }, callback + register: (callback = (error, user) ->) -> + return callback(new Error('User already registered')) if @_id? + @getCsrfToken (error) => + return callback(error) if error? + @request.post { + url: '/register' + json: { @email, @password } + }, (error, response, body) => + return callback(error) if error? + db.users.findOne { email: @email }, (error, user) => + return callback(error) if error? + @setExtraAttributes user + callback(null, user) + login: (callback = (error) ->) -> @loginWith(@email, callback) @@ -47,11 +68,7 @@ class User return callback(error) if error? UserUpdater.updateUser user._id, $set: emails: @emails, (error) => return callback(error) if error? - @id = user?._id?.toString() - @_id = user?._id?.toString() - @first_name = user?.first_name - @referal_id = user?.referal_id - + @setExtraAttributes user callback(null, @password) setFeatures: (features, callback = (error) ->) -> diff --git a/services/web/test/unit/coffee/User/UserCreatorTests.coffee b/services/web/test/unit/coffee/User/UserCreatorTests.coffee index cc2b1ec150..3764c8a765 100644 --- a/services/web/test/unit/coffee/User/UserCreatorTests.coffee +++ b/services/web/test/unit/coffee/User/UserCreatorTests.coffee @@ -70,3 +70,11 @@ describe "UserCreator", -> assert.equal user.holdingAccount, true assert.equal user.last_name, "lastNammmmeee" done() + + it "should set emails attribute", (done)-> + @UserCreator.createNewUser email: @email, (err, user)=> + user.email.should.equal @email + user.emails.length.should.equal 1 + user.emails[0].email.should.equal @email + user.emails[0].createdAt.should.be.a 'date' + done() diff --git a/services/web/test/unit/coffee/User/UserGetterTests.coffee b/services/web/test/unit/coffee/User/UserGetterTests.coffee index 7fb14a7f7d..fbea0e8b53 100644 --- a/services/web/test/unit/coffee/User/UserGetterTests.coffee +++ b/services/web/test/unit/coffee/User/UserGetterTests.coffee @@ -85,3 +85,20 @@ describe "UserGetter", -> @findOne.calledTwice.should.equal true @findOne.calledWith(email: email, projection).should.equal true done() + + describe 'ensureUniqueEmailAddress', -> + beforeEach -> + @UserGetter.getUserByAnyEmail = sinon.stub() + + it 'should return error if existing user is found', (done)-> + @UserGetter.getUserByAnyEmail.callsArgWith(1, null, @fakeUser) + @UserGetter.ensureUniqueEmailAddress @newEmail, (err)=> + should.exist(err) + err.message.should.equal 'alread_exists' + done() + + it 'should return null if no user is found', (done)-> + @UserGetter.getUserByAnyEmail.callsArgWith(1) + @UserGetter.ensureUniqueEmailAddress @newEmail, (err)=> + should.not.exist(err) + done() diff --git a/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee b/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee index 9411059022..e738947171 100644 --- a/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee +++ b/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee @@ -14,7 +14,7 @@ describe "UserRegistrationHandler", -> @User = update: sinon.stub().callsArgWith(2) @UserGetter = - getUserByMainEmail: sinon.stub() + getUserByAnyEmail: sinon.stub() @UserCreator = createNewUser:sinon.stub().callsArgWith(1, null, @user) @AuthenticationManager = @@ -72,7 +72,7 @@ describe "UserRegistrationHandler", -> beforeEach -> @user.holdingAccount = true @handler._registrationRequestIsValid = sinon.stub().returns true - @UserGetter.getUserByMainEmail.callsArgWith(1, null, @user) + @UserGetter.getUserByAnyEmail.callsArgWith(1, null, @user) it "should not create a new user if there is a holding account there", (done)-> @handler.registerNewUser @passingRequest, (err)=> @@ -96,7 +96,7 @@ describe "UserRegistrationHandler", -> done() it "should return email registered in the error if there is a non holdingAccount there", (done)-> - @UserGetter.getUserByMainEmail.callsArgWith(1, null, @user = {holdingAccount:false}) + @UserGetter.getUserByAnyEmail.callsArgWith(1, null, @user = {holdingAccount:false}) @handler.registerNewUser @passingRequest, (err, user)=> err.should.deep.equal new Error("EmailAlreadyRegistered") user.should.deep.equal @user @@ -105,7 +105,7 @@ describe "UserRegistrationHandler", -> describe "validRequest", -> beforeEach -> @handler._registrationRequestIsValid = sinon.stub().returns true - @UserGetter.getUserByMainEmail.callsArgWith 1 + @UserGetter.getUserByAnyEmail.callsArgWith 1 it "should create a new user", (done)-> @handler.registerNewUser @passingRequest, (err)=> diff --git a/services/web/test/unit/coffee/User/UserUpdaterTests.coffee b/services/web/test/unit/coffee/User/UserUpdaterTests.coffee index b952a688ae..7b3be3df2b 100644 --- a/services/web/test/unit/coffee/User/UserUpdaterTests.coffee +++ b/services/web/test/unit/coffee/User/UserUpdaterTests.coffee @@ -17,6 +17,7 @@ describe "UserUpdater", -> @UserGetter = getUserEmail: sinon.stub() getUserByAnyEmail: sinon.stub() + ensureUniqueEmailAddress: sinon.stub() @logger = err: sinon.stub(), log: -> @UserUpdater = SandboxedModule.require modulePath, requires: "settings-sharelatex":@settings @@ -60,13 +61,13 @@ describe "UserUpdater", -> describe 'addEmailAddress', -> beforeEach -> - @UserUpdater._ensureUniqueEmailAddress = sinon.stub().callsArgWith(1) + @UserGetter.ensureUniqueEmailAddress = sinon.stub().callsArgWith(1) it 'add email', (done)-> @UserUpdater.updateUser = sinon.stub().callsArgWith(2, null) @UserUpdater.addEmailAddress @stubbedUser._id, @newEmail, (err)=> - @UserUpdater._ensureUniqueEmailAddress.called.should.equal true + @UserGetter.ensureUniqueEmailAddress.called.should.equal true should.not.exist(err) @UserUpdater.updateUser.calledWith( @stubbedUser._id, @@ -135,15 +136,3 @@ describe "UserUpdater", -> done() - describe '_ensureUniqueEmailAddress', -> - it 'should return error if existing user is found', (done)-> - @UserGetter.getUserByAnyEmail.callsArgWith(1, null, @stubbedUser) - @UserUpdater._ensureUniqueEmailAddress @newEmail, (err)=> - should.exist(err) - done() - - it 'should return null if no user is found', (done)-> - @UserGetter.getUserByAnyEmail.callsArgWith(1) - @UserUpdater._ensureUniqueEmailAddress @newEmail, (err)=> - should.not.exist(err) - done() From 5438a565b9622e208e876bd2d35144fc27307481 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Mon, 28 May 2018 16:09:22 +0200 Subject: [PATCH 39/44] use multiple emails when relevant --- .../CollaboratorsInviteController.coffee | 2 +- .../CollaboratorsInviteHandler.coffee | 2 +- .../SubscriptionGroupHandler.coffee | 2 +- .../CollaboratorsInviteControllerTests.coffee | 12 ++++++------ .../CollaboratorsInviteHandlerTests.coffee | 18 +++++++++--------- .../SubscriptionGroupHandlerTests.coffee | 10 +++++----- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee index f74a144bac..8b1b481994 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee @@ -27,7 +27,7 @@ module.exports = CollaboratorsInviteController = _checkShouldInviteEmail: (email, callback=(err, shouldAllowInvite)->) -> if Settings.restrictInvitesToExistingAccounts == true logger.log {email}, "checking if user exists with this email" - UserGetter.getUserByMainEmail email, {_id: 1}, (err, user) -> + UserGetter.getUserByAnyEmail email, {_id: 1}, (err, user) -> return callback(err) if err? userExists = user? and user?._id? callback(null, userExists) diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee index b511f56e53..58121dae2c 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee @@ -32,7 +32,7 @@ module.exports = CollaboratorsInviteHandler = _trySendInviteNotification: (projectId, sendingUser, invite, callback=(err)->) -> email = invite.email - UserGetter.getUserByMainEmail email, {_id: 1}, (err, existingUser) -> + UserGetter.getUserByAnyEmail email, {_id: 1}, (err, existingUser) -> if err? logger.err {projectId, email}, "error checking if user exists" return callback(err) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index 7e88a6c56a..81ca5d20bd 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -22,7 +22,7 @@ module.exports = SubscriptionGroupHandler = if limitReached logger.err adminUserId:adminUserId, newEmail:newEmail, "group subscription limit reached not adding user to group" return callback(limitReached:limitReached) - UserGetter.getUserByMainEmail newEmail, (err, user)-> + UserGetter.getUserByAnyEmail newEmail, (err, user)-> return callback(err) if err? if user? SubscriptionUpdater.addUserToGroup adminUserId, user._id, (err)-> diff --git a/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee b/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee index 4b57ff3697..affef2dc31 100644 --- a/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee +++ b/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee @@ -25,7 +25,7 @@ describe "CollaboratorsInviteController", -> @LimitationsManager = {} @UserGetter = - getUserByMainEmail: sinon.stub() + getUserByAnyEmail: sinon.stub() getUser: sinon.stub() @CollaboratorsInviteController = SandboxedModule.require modulePath, requires: @@ -716,7 +716,7 @@ describe "CollaboratorsInviteController", -> beforeEach -> @user = {_id: ObjectId().toString()} - @UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, null, @user) + @UserGetter.getUserByAnyEmail = sinon.stub().callsArgWith(2, null, @user) it 'should callback with `true`', (done) -> @call (err, shouldAllow) => @@ -728,7 +728,7 @@ describe "CollaboratorsInviteController", -> beforeEach -> @user = null - @UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, null, @user) + @UserGetter.getUserByAnyEmail = sinon.stub().callsArgWith(2, null, @user) it 'should callback with `false`', (done) -> @call (err, shouldAllow) => @@ -738,15 +738,15 @@ describe "CollaboratorsInviteController", -> it 'should have called getUser', (done) -> @call (err, shouldAllow) => - @UserGetter.getUserByMainEmail.callCount.should.equal 1 - @UserGetter.getUserByMainEmail.calledWith(@email, {_id: 1}).should.equal true + @UserGetter.getUserByAnyEmail.callCount.should.equal 1 + @UserGetter.getUserByAnyEmail.calledWith(@email, {_id: 1}).should.equal true done() describe 'when getUser produces an error', -> beforeEach -> @user = null - @UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, new Error('woops')) + @UserGetter.getUserByAnyEmail = sinon.stub().callsArgWith(2, new Error('woops')) it 'should callback with an error', (done) -> @call (err, shouldAllow) => diff --git a/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee b/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee index 58b373f61f..edad39a637 100644 --- a/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee +++ b/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee @@ -605,7 +605,7 @@ describe "CollaboratorsInviteHandler", -> _id: ObjectId() first_name: "jim" @existingUser = {_id: ObjectId()} - @UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, null, @existingUser) + @UserGetter.getUserByAnyEmail = sinon.stub().callsArgWith(2, null, @existingUser) @fakeProject = _id: @project_id name: "some project" @@ -626,8 +626,8 @@ describe "CollaboratorsInviteHandler", -> it 'should call getUser', (done) -> @call (err) => - @UserGetter.getUserByMainEmail.callCount.should.equal 1 - @UserGetter.getUserByMainEmail.calledWith(@invite.email).should.equal true + @UserGetter.getUserByAnyEmail.callCount.should.equal 1 + @UserGetter.getUserByAnyEmail.calledWith(@invite.email).should.equal true done() it 'should call getProject', (done) -> @@ -671,7 +671,7 @@ describe "CollaboratorsInviteHandler", -> describe 'when the user does not exist', -> beforeEach -> - @UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, null, null) + @UserGetter.getUserByAnyEmail = sinon.stub().callsArgWith(2, null, null) it 'should not produce an error', (done) -> @call (err) => @@ -680,8 +680,8 @@ describe "CollaboratorsInviteHandler", -> it 'should call getUser', (done) -> @call (err) => - @UserGetter.getUserByMainEmail.callCount.should.equal 1 - @UserGetter.getUserByMainEmail.calledWith(@invite.email).should.equal true + @UserGetter.getUserByAnyEmail.callCount.should.equal 1 + @UserGetter.getUserByAnyEmail.calledWith(@invite.email).should.equal true done() it 'should not call getProject', (done) -> @@ -698,7 +698,7 @@ describe "CollaboratorsInviteHandler", -> describe 'when the getUser produces an error', -> beforeEach -> - @UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, new Error('woops')) + @UserGetter.getUserByAnyEmail = sinon.stub().callsArgWith(2, new Error('woops')) it 'should produce an error', (done) -> @call (err) => @@ -707,8 +707,8 @@ describe "CollaboratorsInviteHandler", -> it 'should call getUser', (done) -> @call (err) => - @UserGetter.getUserByMainEmail.callCount.should.equal 1 - @UserGetter.getUserByMainEmail.calledWith(@invite.email).should.equal true + @UserGetter.getUserByAnyEmail.callCount.should.equal 1 + @UserGetter.getUserByAnyEmail.calledWith(@invite.email).should.equal true done() it 'should not call getProject', (done) -> diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee index 12bf842ca2..48e881d607 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee @@ -35,7 +35,7 @@ describe "SubscriptionGroupHandler", -> @UserGetter = getUser: sinon.stub() - getUserByMainEmail: sinon.stub() + getUserByAnyEmail: sinon.stub() @LimitationsManager = hasGroupMembersLimitReached: sinon.stub() @@ -78,11 +78,11 @@ describe "SubscriptionGroupHandler", -> describe "addUserToGroup", -> beforeEach -> @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription) - @UserGetter.getUserByMainEmail.callsArgWith(1, null, @user) - + @UserGetter.getUserByAnyEmail.callsArgWith(1, null, @user) + it "should find the user", (done)-> @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> - @UserGetter.getUserByMainEmail.calledWith(@newEmail).should.equal true + @UserGetter.getUserByAnyEmail.calledWith(@newEmail).should.equal true done() it "should add the user to the group", (done)-> @@ -109,7 +109,7 @@ describe "SubscriptionGroupHandler", -> done() it "should add an email invite if no user is found", (done) -> - @UserGetter.getUserByMainEmail.callsArgWith(1, null, null) + @UserGetter.getUserByAnyEmail.callsArgWith(1, null, null) @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> @SubscriptionUpdater.addEmailInviteToGroup.calledWith(@adminUser_id, @newEmail).should.equal true done() From b716f59442110653affe69c0656c16cdef9ea9e0 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 11 Jun 2018 14:19:47 +0100 Subject: [PATCH 40/44] Remove deprecated removeEmailInviteFromGroup --- .../Features/Subscription/SubscriptionGroupHandler.coffee | 4 ---- .../Features/Subscription/SubscriptionUpdater.coffee | 7 ------- 2 files changed, 11 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index acd783047a..776cec3cf8 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -42,10 +42,6 @@ module.exports = SubscriptionGroupHandler = removeUserFromGroup: (adminUser_id, userToRemove_id, callback)-> SubscriptionUpdater.removeUserFromGroup adminUser_id, userToRemove_id, callback - removeEmailInviteFromGroup: (adminUser_id, email, callback) -> - SubscriptionUpdater.removeEmailInviteFromGroup adminUser_id, email, callback - - replaceUserReferencesInGroups: (oldId, newId, callback) -> Subscription.update {admin_id: oldId}, {admin_id: newId}, (error) -> callback(error) if error? diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee index 58fea8c23d..16dc5adf25 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee @@ -47,13 +47,6 @@ module.exports = SubscriptionUpdater = return callback(err) FeaturesUpdater.refreshFeatures user_id, callback - removeEmailInviteFromGroup: (adminUser_id, email, callback)-> - Subscription.update { - admin_id: adminUser_id - }, "$pull": { - invited_emails: email - }, callback - deleteSubscription: (subscription_id, callback = (error) ->) -> SubscriptionLocator.getSubscription subscription_id, (err, subscription) -> return callback(err) if err? From 155102df640266f8802490171688afa2bebaf7bb Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 11 Jun 2018 14:20:35 +0100 Subject: [PATCH 41/44] Use Error classes, and ensure invited_emails is cleared on new invite --- .../app/coffee/Features/Errors/Errors.coffee | 2 +- .../Subscription/TeamInvitesController.coffee | 18 ++---- .../Subscription/TeamInvitesHandler.coffee | 30 ++++++--- .../TeamInvitesHandlerTests.coffee | 62 +++++++++++-------- 4 files changed, 62 insertions(+), 50 deletions(-) diff --git a/services/web/app/coffee/Features/Errors/Errors.coffee b/services/web/app/coffee/Features/Errors/Errors.coffee index b2ed088d15..af1e437d4b 100644 --- a/services/web/app/coffee/Features/Errors/Errors.coffee +++ b/services/web/app/coffee/Features/Errors/Errors.coffee @@ -56,7 +56,7 @@ V1HistoryNotSyncedError.prototype.__proto___ = Error.prototype ProjectHistoryDisabledError = (message) -> error = new Error(message) - error.name = "ProjectHistoryDisabledError " + error.name = "ProjectHistoryDisabledError" error.__proto__ = ProjectHistoryDisabledError.prototype return error ProjectHistoryDisabledError.prototype.__proto___ = Error.prototype diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee index f4f0933fdd..475c4684b0 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee @@ -11,7 +11,7 @@ module.exports = email = req.body.email TeamInvitesHandler.createInvite teamManagerId, email, (err, invite) -> - return handleError(err, req, res, next) if err? + return next(err) if err? inviteView = { user: { email: invite.email, sentAt: invite.sentAt, invite: true } } @@ -22,13 +22,13 @@ module.exports = userId = AuthenticationController.getLoggedInUserId(req) TeamInvitesHandler.getInvite token, (err, invite, teamSubscription) -> - return handleError(err, req, res, next) if err? + return next(err) if err? if !invite return ErrorController.notFound(req, res, next) SubscriptionLocator.getUsersSubscription userId, (err, personalSubscription) -> - return handleError(err, req, res, next) if err? + return next(err) if err? res.render "subscriptions/team/invite", inviterName: invite.inviterName @@ -41,8 +41,7 @@ module.exports = userId = AuthenticationController.getLoggedInUserId(req) TeamInvitesHandler.acceptInvite token, userId, (err, results) -> - return handleError(err, req, res, next) if err? - + return next(err) if err? res.sendStatus 204 revokeInvite: (req, res) -> @@ -50,12 +49,5 @@ module.exports = teamManagerId = AuthenticationController.getLoggedInUserId(req) TeamInvitesHandler.revokeInvite teamManagerId, email, (err, results) -> - return handleError(err, req, res, next) if err? - + return next(err) if err? res.sendStatus 204 - -handleError = (err, req, res, next) -> - if err.teamNotFound or err.inviteNoLongerValid - ErrorController.notFound(req, res, next) - else - next(err) diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index b84cda82f0..b27d0da8a1 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -14,12 +14,15 @@ SubscriptionUpdater = require("./SubscriptionUpdater") LimitationsManager = require("./LimitationsManager") EmailHandler = require("../Email/EmailHandler") +EmailHelper = require("../Helpers/EmailHelper") + +Errors = require "../Errors/Errors" module.exports = TeamInvitesHandler = getInvite: (token, callback) -> Subscription.findOne 'teamInvites.token': token, (err, subscription) -> return callback(err) if err? - return callback(teamNotFound: true) unless subscription? + return callback(new Errors.NotFoundError('team not found')) unless subscription? invite = subscription.teamInvites.find (i) -> i.token == token return callback(null, invite, subscription) @@ -37,7 +40,9 @@ module.exports = TeamInvitesHandler = else inviterName = teamManager.email - createInvite(subscription, email, inviterName, callback) + removeLegacyInvite subscription.id, email, (error) -> + return callback(error) if error? + createInvite(subscription, email, inviterName, callback) createDomainInvite: (user, licence, callback) -> logger.log {licence, email: user.email}, "Creating domain team invite" @@ -51,7 +56,7 @@ module.exports = TeamInvitesHandler = logger.log {userId}, "Accepting invite" TeamInvitesHandler.getInvite token, (err, invite, subscription) -> return callback(err) if err? - return callback(inviteNoLongerValid: true) unless invite? + return callback(new Errors.NotFoundError('invite not found')) unless invite? SubscriptionUpdater.addUserToGroup subscription.admin_id, userId, (err) -> return callback(err) if err? @@ -82,8 +87,7 @@ createInvite = (subscription, email, inviterName, callback) -> return callback(error) if error? return callback(reason) unless possible - # TODO: use standard way to canonalise email addresses - email = email.trim().toLowerCase() + email = EmailHelper.parseEmail(email) invite = subscription.teamInvites.find (invite) -> invite.email == email @@ -110,18 +114,24 @@ createInvite = (subscription, email, inviterName, callback) -> return callback(error, invite) removeInviteFromTeam = (subscriptionId, email, callback) -> - email = email.trim().toLowerCase() - + email = EmailHelper.parseEmail(email) searchConditions = { _id: new ObjectId(subscriptionId.toString()) } - removeInvite = { $pull: { teamInvites: { email: email } } } - removeLegacyInvite = { $pull: { invited_emails: email } } + logger.log {subscriptionId, email, searchConditions, removeInvite}, 'removeInviteFromTeam' async.series [ (cb) -> Subscription.update(searchConditions, removeInvite, cb), - (cb) -> Subscription.update(searchConditions, removeLegacyInvite, cb), + (cb) -> removeLegacyInvite(subscriptionId, email, cb), ], callback +removeLegacyInvite = (subscriptionId, email, callback) -> + Subscription.update({ + _id: new ObjectId(subscriptionId.toString()) + }, { + $pull: { + invited_emails: EmailHelper.parseEmail(email) + } + }, callback) checkIfInviteIsPossible = (subscription, email, callback = (error, possible, reason) -> ) -> unless subscription.groupPlan diff --git a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee index 720c8fadf7..527ba8a143 100644 --- a/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee @@ -6,6 +6,7 @@ querystring = require 'querystring' modulePath = "../../../../app/js/Features/Subscription/TeamInvitesHandler" ObjectId = require("mongojs").ObjectId +Errors = require("../../../../app/js/Features/Errors/Errors") describe "TeamInvitesHandler", -> beforeEach -> @@ -13,13 +14,13 @@ describe "TeamInvitesHandler", -> id: "666666", first_name: "Daenerys" last_name: "Targaryen" - email: "daenerys@motherofdragons.com" + email: "daenerys@example.com" } @token = "aaaaaaaaaaaaaaaaaaaaaa" @teamInvite = { - email: "jorah@mormont.org", + email: "jorah@example.com", token: @token, } @@ -83,6 +84,7 @@ describe "TeamInvitesHandler", -> "./SubscriptionUpdater": @SubscriptionUpdater "./LimitationsManager": @LimitationsManager "../Email/EmailHandler": @EmailHandler + "../Errors/Errors": Errors describe "getInvite", -> it "returns the invite if there's one", (done) -> @@ -96,25 +98,25 @@ describe "TeamInvitesHandler", -> @Subscription.findOne = sinon.stub().yields(null, null) @TeamInvitesHandler.getInvite @token, (err, invite, subscription) -> - expect(err).to.deep.eq(teamNotFound: true) + expect(err).to.be.instanceof(Errors.NotFoundError) done() describe "createInvite", -> it "adds the team invite to the subscription", (done) -> - @TeamInvitesHandler.createInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) => expect(err).to.eq(null) expect(invite.token).to.eq(@newToken) - expect(invite.email).to.eq("john.snow@nightwatch.com") - expect(invite.inviterName).to.eq("Daenerys Targaryen (daenerys@motherofdragons.com)") + expect(invite.email).to.eq("john.snow@example.com") + expect(invite.inviterName).to.eq("Daenerys Targaryen (daenerys@example.com)") expect(@subscription.teamInvites).to.deep.include(invite) done() it "sends an email", (done) -> - @TeamInvitesHandler.createInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) => @EmailHandler.sendEmail.calledWith("verifyEmailToJoinTeam", sinon.match({ - to: "john.snow@nightwatch.com", - inviterName: "Daenerys Targaryen (daenerys@motherofdragons.com)", + to: "john.snow@example.com", + inviterName: "Daenerys Targaryen (daenerys@example.com)", acceptInviteUrl: "http://example.com/subscription/invites/#{@newToken}/" }) ).should.equal true @@ -136,6 +138,14 @@ describe "TeamInvitesHandler", -> done() + it "removes any legacy invite from the subscription", (done) -> + @TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) => + @Subscription.update.calledWith( + { _id: new ObjectId("55153a8014829a865bbf700d") }, + { '$pull': { invited_emails: "john.snow@example.com" } } + ).should.eq true + done() + describe "createDomainInvite", -> beforeEach -> @licence = @@ -143,13 +153,13 @@ describe "TeamInvitesHandler", -> name: "Team Daenerys" @user = - email: "John.Snow@nightwatch.com" + email: "John.Snow@example.com" it "adds the team invite to the subscription", (done) -> @TeamInvitesHandler.createDomainInvite @user, @licence, (err, invite) => expect(err).to.eq(null) expect(invite.token).to.eq(@newToken) - expect(invite.email).to.eq("john.snow@nightwatch.com") + expect(invite.email).to.eq("john.snow@example.com") expect(invite.inviterName).to.eq("Team Daenerys") expect(@subscription.teamInvites).to.deep.include(invite) done() @@ -158,7 +168,7 @@ describe "TeamInvitesHandler", -> @TeamInvitesHandler.createDomainInvite @user, @licence, (err, invite) => @EmailHandler.sendEmail.calledWith("verifyEmailToJoinTeam", sinon.match({ - to: "john.snow@nightwatch.com" + to: "john.snow@example.com" inviterName: "Team Daenerys" acceptInviteUrl: "http://example.com/subscription/invites/#{@newToken}/" }) @@ -171,15 +181,15 @@ describe "TeamInvitesHandler", -> id: "123456789", first_name: "Tyrion", last_name: "Lannister", - email: "tyrion@lannister.com" + email: "tyrion@example.com" } @UserGetter.getUserByAnyEmail.withArgs(@user.email).yields(null, @user) @subscription.teamInvites.push({ - email: "john.snow@nightwatch.com", + email: "john.snow@example.com", token: "dddddddd", - inviterName: "Daenerys Targaryen (daenerys@motherofdragons.com)" + inviterName: "Daenerys Targaryen (daenerys@example.com)" }) it "adds the user to the team", (done) -> @@ -191,37 +201,37 @@ describe "TeamInvitesHandler", -> @TeamInvitesHandler.acceptInvite "dddddddd", @user.id, => @Subscription.update.calledWith( { _id: new ObjectId("55153a8014829a865bbf700d") }, - { '$pull': { teamInvites: { email: 'john.snow@nightwatch.com' } } } + { '$pull': { teamInvites: { email: 'john.snow@example.com' } } } ).should.eq true done() describe "revokeInvite", -> it "removes the team invite from the subscription", (done) -> - @TeamInvitesHandler.revokeInvite @manager.id, "jorah@mormont.org", => + @TeamInvitesHandler.revokeInvite @manager.id, "jorah@example.com", => @Subscription.update.calledWith( { _id: new ObjectId("55153a8014829a865bbf700d") }, - { '$pull': { teamInvites: { email: "jorah@mormont.org" } } } + { '$pull': { teamInvites: { email: "jorah@example.com" } } } ).should.eq true @Subscription.update.calledWith( { _id: new ObjectId("55153a8014829a865bbf700d") }, - { '$pull': { invited_emails: "jorah@mormont.org" } } + { '$pull': { invited_emails: "jorah@example.com" } } ).should.eq true done() describe "createTeamInvitesForLegacyInvitedEmail", (done) -> beforeEach -> - @subscription.invited_emails = ["eddard@stark.com", "robert@stark.com"] + @subscription.invited_emails = ["eddard@example.com", "robert@example.com"] @TeamInvitesHandler.createInvite = sinon.stub().yields(null) @SubscriptionLocator.getGroupsWithEmailInvite = sinon.stub().yields(null, [@subscription]) it "sends an invitation email to addresses in the legacy invited_emails field", (done) -> - @TeamInvitesHandler.createTeamInvitesForLegacyInvitedEmail "eddard@stark.com", (err, invite) => + @TeamInvitesHandler.createTeamInvitesForLegacyInvitedEmail "eddard@example.com", (err, invite) => expect(err).not.to.exist @TeamInvitesHandler.createInvite.calledWith( @subscription.admin_id, - "eddard@stark.com" + "eddard@example.com" ).should.eq true @TeamInvitesHandler.createInvite.callCount.should.eq 1 @@ -231,13 +241,13 @@ describe "TeamInvitesHandler", -> describe "validation", -> it "doesn't create an invite if the team limit has been reached", (done) -> @LimitationsManager.teamHasReachedMemberLimit = sinon.stub().returns(true) - @TeamInvitesHandler.createInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) => expect(err).to.deep.equal(limitReached: true) done() it "doesn't create an invite if the subscription is not in a group plan", (done) -> @subscription.groupPlan = false - @TeamInvitesHandler.createInvite @manager.id, "John.Snow@nightwatch.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) => expect(err).to.deep.equal(wrongPlan: true) done() @@ -245,13 +255,13 @@ describe "TeamInvitesHandler", -> member = { id: "1a2b", _id: "1a2b", - email: "tyrion@lannister.com" + email: "tyrion@example.com" } @subscription.member_ids = [member.id] @UserGetter.getUserByAnyEmail.withArgs(member.email).yields(null, member) - @TeamInvitesHandler.createInvite @manager.id, "tyrion@lannister.com", (err, invite) => + @TeamInvitesHandler.createInvite @manager.id, "tyrion@example.com", (err, invite) => expect(err).to.deep.equal(alreadyInTeam: true) expect(invite).not.to.exist done() From ed5bc70350636f1e82d018bcb5f84ac1f0f1ef86 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 11 Jun 2018 14:20:46 +0100 Subject: [PATCH 42/44] Tweak front-end layout --- .../app/views/subscriptions/domain/join.pug | 41 ++++++++----------- .../app/views/subscriptions/team/invite.pug | 37 +++++++---------- 2 files changed, 32 insertions(+), 46 deletions(-) diff --git a/services/web/app/views/subscriptions/domain/join.pug b/services/web/app/views/subscriptions/domain/join.pug index 1d528102b8..8c7c54234e 100644 --- a/services/web/app/views/subscriptions/domain/join.pug +++ b/services/web/app/views/subscriptions/domain/join.pug @@ -14,33 +14,26 @@ block content .alert.alert-warning #{translate("email_link_expired")} .row.row-spaced - .col-md-8.col-md-offset-2(ng-cloak) + .col-md-8.col-md-offset-2.text-center(ng-cloak) .card(ng-controller="DomainSubscriptionJoinController") .page-header h1.text-centered #{translate("you_are_invited_to_group", {groupName:licenceName})} - div(ng-show="view =='personalSubscription'").text-centered - div #{translate("cancel_personal_subscription_first")} - .row.row-spaced - .col-md-12 - a.btn.btn.btn-default(ng-click="keepPersonalSubscription()", ng-disabled="inflight") #{translate("not_now")} - span   - a.btn.btn.btn-primary(ng-click="cancelSubscription()", ng-disabled="inflight") #{translate("cancel_your_subscription")} + div(ng-show="view =='personalSubscription'") + p #{translate("cancel_personal_subscription_first")} + p + a.btn.btn.btn-default(ng-click="keepPersonalSubscription()", ng-disabled="inflight") #{translate("not_now")} + |   + a.btn.btn.btn-primary(ng-click="cancelSubscription()", ng-disabled="inflight") #{translate("cancel_your_subscription")} - div(ng-show="view =='domainSubscriptionJoin'").text-centered - .row - .col-md-12 #{translate("group_provides_you_with_premium_account", {groupName:licenceName})} - .row - .col-md-12 - .text-center - a.btn.btn-default(href="/project") #{translate("not_now")} - span   - a.btn.btn.btn-primary(ng-click="joinGroup()", ng-disabled="inflight") #{translate("verify_email_address")} + div(ng-show="view =='domainSubscriptionJoin'") + p #{translate("group_provides_you_with_premium_account", {groupName:licenceName})} + p + a.btn.btn-default(href="/project") #{translate("not_now")} + |   + a.btn.btn.btn-primary(ng-click="joinGroup()", ng-disabled="inflight") #{translate("verify_email_address")} - - span(ng-show="view =='requestSent'").text-centered.text-center - .row - .col-md-12 #{translate("check_email_to_complete_the_upgrade")} - .row.row-spaced - .col-md-12 - a.btn.btn.btn-primary(href="/project") #{translate("done")} + div(ng-show="view =='requestSent'") + p #{translate("check_email_to_complete_the_upgrade")} + p + a.btn.btn.btn-primary(href="/project") #{translate("done")} diff --git a/services/web/app/views/subscriptions/team/invite.pug b/services/web/app/views/subscriptions/team/invite.pug index 327d3bb2aa..46a0e8e306 100644 --- a/services/web/app/views/subscriptions/team/invite.pug +++ b/services/web/app/views/subscriptions/team/invite.pug @@ -15,33 +15,26 @@ block content .alert.alert-warning #{translate("email_link_expired")} .row.row-spaced - .col-md-8.col-md-offset-2(ng-cloak) + .col-md-8.col-md-offset-2.text-center(ng-cloak) .card(ng-controller="TeamInviteController") .page-header h1.text-centered #{translate("invited_to_group", {inviterName: inviterName, appName: appName})} - div(ng-show="view =='personalSubscription'").text-centered.message + div(ng-show="view =='personalSubscription'") p #{translate("cancel_personal_subscription_first")} - .row - .col-md-12 - a.btn.btn.btn-default(ng-click="keepPersonalSubscription()", ng-disabled="inflight") #{translate("not_now")} - span   - a.btn.btn.btn-primary(ng-click="cancelPersonalSubscription()", ng-disabled="inflight") #{translate("cancel_your_subscription")} + p + a.btn.btn.btn-default(ng-click="keepPersonalSubscription()", ng-disabled="inflight") #{translate("not_now")} + |   + a.btn.btn.btn-primary(ng-click="cancelPersonalSubscription()", ng-disabled="inflight") #{translate("cancel_your_subscription")} - div(ng-show="view =='teamInvite'").row.text-centered.message + div(ng-show="view =='teamInvite'") p #{translate("join_team_explanation", {appName: appName})} - .row - .col-md-12 - .text-center - a.btn.btn-default(href="/project") #{translate("not_now")} - span   - a.btn.btn.btn-primary(ng-click="joinTeam()", ng-disabled="inflight") #{translate("accept_invitation")} + p + a.btn.btn-default(href="/project") #{translate("not_now")} + |   + a.btn.btn.btn-primary(ng-click="joinTeam()", ng-disabled="inflight") #{translate("accept_invitation")} - div(ng-show="view =='inviteAccepted'").row.text-centered.text-center - .row - .col-md-12 #{translate("joined_team", {inviterName: inviterName})} - .row - .col-md-12   - .row - .col-md-12 - a.btn.btn.btn-primary(href="/project") #{translate("done")} + div(ng-show="view =='inviteAccepted'") + p #{translate("joined_team", {inviterName: inviterName})} + p + a.btn.btn.btn-primary(href="/project") #{translate("done")} From dbd6ea30e96eadf6caa21bf230ed8e27cf05c8d2 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 11 Jun 2018 15:22:42 +0100 Subject: [PATCH 43/44] Improve robustness of email validation --- .../Subscription/TeamInvitesController.coffee | 9 +++++++-- .../Subscription/TeamInvitesHandler.coffee | 14 +++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee index 475c4684b0..3fdd7e3e56 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee @@ -4,11 +4,14 @@ TeamInvitesHandler = require('./TeamInvitesHandler') AuthenticationController = require("../Authentication/AuthenticationController") SubscriptionLocator = require("./SubscriptionLocator") ErrorController = require("../Errors/ErrorController") +EmailHelper = require("../Helpers/EmailHelper") module.exports = createInvite: (req, res, next) -> teamManagerId = AuthenticationController.getLoggedInUserId(req) - email = req.body.email + email = EmailHelper.parseEmail(req.body.email) + if !email? + return res.sendStatus(400) TeamInvitesHandler.createInvite teamManagerId, email, (err, invite) -> return next(err) if err? @@ -45,8 +48,10 @@ module.exports = res.sendStatus 204 revokeInvite: (req, res) -> - email = req.params.email + email = EmailHelper.parseEmail(req.params.email) teamManagerId = AuthenticationController.getLoggedInUserId(req) + if !email? + return res.sendStatus(400) TeamInvitesHandler.revokeInvite teamManagerId, email, (err, results) -> return next(err) if err? diff --git a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee index b27d0da8a1..e09e99c94a 100644 --- a/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee @@ -28,6 +28,8 @@ module.exports = TeamInvitesHandler = return callback(null, invite, subscription) createInvite: (teamManagerId, email, callback) -> + email = EmailHelper.parseEmail(email) + return callback(new Error('invalid email')) if !email? logger.log {teamManagerId, email}, "Creating manager team invite" UserGetter.getUser teamManagerId, (error, teamManager) -> return callback(error) if error? @@ -45,12 +47,14 @@ module.exports = TeamInvitesHandler = createInvite(subscription, email, inviterName, callback) createDomainInvite: (user, licence, callback) -> - logger.log {licence, email: user.email}, "Creating domain team invite" + email = EmailHelper.parseEmail(user.email) + return callback(new Error('invalid email')) if !email? + logger.log {licence, email: email}, "Creating domain team invite" inviterName = licence.name.replace(/\s+licence$/i, licence.name) SubscriptionLocator.getSubscription licence.subscription_id, (error, subscription) -> return callback(error) if error? - createInvite(subscription, user.email, inviterName, callback) + createInvite(subscription, email, inviterName, callback) acceptInvite: (token, userId, callback) -> logger.log {userId}, "Accepting invite" @@ -64,6 +68,8 @@ module.exports = TeamInvitesHandler = removeInviteFromTeam(subscription.id, invite.email, callback) revokeInvite: (teamManagerId, email, callback) -> + email = EmailHelper.parseEmail(email) + return callback(new Error('invalid email')) if !email? logger.log {teamManagerId, email}, "Revoking invite" SubscriptionLocator.getUsersSubscription teamManagerId, (err, teamSubscription) -> return callback(err) if err? @@ -87,7 +93,6 @@ createInvite = (subscription, email, inviterName, callback) -> return callback(error) if error? return callback(reason) unless possible - email = EmailHelper.parseEmail(email) invite = subscription.teamInvites.find (invite) -> invite.email == email @@ -114,7 +119,6 @@ createInvite = (subscription, email, inviterName, callback) -> return callback(error, invite) removeInviteFromTeam = (subscriptionId, email, callback) -> - email = EmailHelper.parseEmail(email) searchConditions = { _id: new ObjectId(subscriptionId.toString()) } removeInvite = { $pull: { teamInvites: { email: email } } } logger.log {subscriptionId, email, searchConditions, removeInvite}, 'removeInviteFromTeam' @@ -129,7 +133,7 @@ removeLegacyInvite = (subscriptionId, email, callback) -> _id: new ObjectId(subscriptionId.toString()) }, { $pull: { - invited_emails: EmailHelper.parseEmail(email) + invited_emails: email } }, callback) From 1d7accabdd854f181cb0ec0847960f61557ef754 Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 12 Jun 2018 09:54:54 +0100 Subject: [PATCH 44/44] Update group notification for new group invite URL --- .../app/views/project/list/notifications.pug | 59 +++++++++++-------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/services/web/app/views/project/list/notifications.pug b/services/web/app/views/project/list/notifications.pug index ee004039fc..6dc0b02a4f 100644 --- a/services/web/app/views/project/list/notifications.pug +++ b/services/web/app/views/project/list/notifications.pug @@ -26,27 +26,38 @@ span(ng-controller="NotificationsController").userNotifications ) .row(ng-hide="notification.hide") .col-xs-12 - .alert.alert-info(ng-if="notification.templateKey == 'notification_project_invite'", ng-controller="ProjectInviteNotificationController") - div.notification_inner - .notification_body(ng-show="!notification.accepted") - | !{translate("notification_project_invite_message")} - a.pull-right.btn.btn-sm.btn-info(href, ng-click="accept()", ng-disabled="notification.inflight") - span(ng-show="!notification.inflight") #{translate("join_project")} - span(ng-show="notification.inflight") - i.fa.fa-fw.fa-spinner.fa-spin - |   - | #{translate("joining")}... - .notification_body(ng-show="notification.accepted") - | !{translate("notification_project_invite_accepted_message")} - a.pull-right.btn.btn-sm.btn-info(href="/project/{{ notification.messageOpts.projectId }}") #{translate("open_project")} - span().notification_close - button(ng-click="dismiss(notification)").close.pull-right - span(aria-hidden="true") × - span.sr-only #{translate("close")} - .alert.alert-info(ng-if="notification.templateKey != 'notification_project_invite'") - div.notification_inner - span(ng-bind-html="notification.html").notification_body - span().notification_close - button(ng-click="dismiss(notification)").close.pull-right - span(aria-hidden="true") × - span.sr-only #{translate("close")} \ No newline at end of file + div(ng-switch="notification.templateKey") + .alert.alert-info(ng-switch-when="notification_project_invite", ng-controller="ProjectInviteNotificationController") + div.notification_inner + .notification_body(ng-show="!notification.accepted") + | !{translate("notification_project_invite_message")} + a.pull-right.btn.btn-sm.btn-info(href, ng-click="accept()", ng-disabled="notification.inflight") + span(ng-show="!notification.inflight") #{translate("join_project")} + span(ng-show="notification.inflight") + i.fa.fa-fw.fa-spinner.fa-spin + |   + | #{translate("joining")}... + .notification_body(ng-show="notification.accepted") + | !{translate("notification_project_invite_accepted_message")} + a.pull-right.btn.btn-sm.btn-info(href="/project/{{ notification.messageOpts.projectId }}") #{translate("open_project")} + span().notification_close + button(ng-click="dismiss(notification)").close.pull-right + span(aria-hidden="true") × + span.sr-only #{translate("close")} + .alert.alert-info(ng-switch-when="notification_group_invite") + div.notification_inner + .notification_body + | #{translate("invited_to_join_team")}: {{ notification.messageOpts.groupName }} + a.pull-right.btn.btn-sm.btn-info(href="/user/subscription/domain/join") + | #{translate("join_team")} + span().notification_close + button(ng-click="dismiss(notification)").close.pull-right + span(aria-hidden="true") × + span.sr-only #{translate("close")} + .alert.alert-info(ng-switch-default) + div.notification_inner + span(ng-bind-html="notification.html").notification_body + span().notification_close + button(ng-click="dismiss(notification)").close.pull-right + span(aria-hidden="true") × + span.sr-only #{translate("close")} \ No newline at end of file