From fa23ea75b8080f09f198e26d81236e7006537c75 Mon Sep 17 00:00:00 2001 From: hugh-obrien Date: Sun, 2 Sep 2018 13:47:16 +0100 Subject: [PATCH 01/10] Call university ip matcher api when checking notifications --- .../Notifications/NotificationsBuilder.coffee | 15 +++++++++++++++ .../Notifications/NotificationsController.coffee | 10 ++++++++++ 2 files changed, 25 insertions(+) diff --git a/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee b/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee index 941f4d4d4d..849995db6c 100644 --- a/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee +++ b/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee @@ -1,5 +1,7 @@ logger = require("logger-sharelatex") NotificationsHandler = require("./NotificationsHandler") +request = require "request" +settings = require "settings-sharelatex" module.exports = @@ -29,3 +31,16 @@ module.exports = NotificationsHandler.createNotification user._id, @key, "notification_project_invite", messageOpts, invite.expires, callback read: (callback=()->) -> NotificationsHandler.markAsReadByKeyOnly @key, callback + + ipMatcherAffiliation: (userId, ip) -> + return null unless settings?.apis?.v1?.url # service is not configured + request { + method: 'GET' + url: "#{settings.apis.v1.url}/api/v2/users/ip_matcher/#{userId}" + auth: { user: settings.apis.v1.user, pass: settings.apis.v1.pass } + body: { ip: ip } + json: true + timeout: 20 * 1000 + }, (error, response, body) -> + return error if error? + return null if response.statusCode == 204 diff --git a/services/web/app/coffee/Features/Notifications/NotificationsController.coffee b/services/web/app/coffee/Features/Notifications/NotificationsController.coffee index 5b83a60248..517f17d325 100644 --- a/services/web/app/coffee/Features/Notifications/NotificationsController.coffee +++ b/services/web/app/coffee/Features/Notifications/NotificationsController.coffee @@ -1,12 +1,22 @@ NotificationsHandler = require("./NotificationsHandler") +NotificationsBuilder = require("./NotificationsBuilder") AuthenticationController = require("../Authentication/AuthenticationController") +Settings = require 'settings-sharelatex' logger = require("logger-sharelatex") _ = require("underscore") module.exports = getAllUnreadNotifications: (req, res)-> + ip = req.headers['x-forwarded-for'] || + req.connection.remoteAddress || + req.socket.remoteAddress user_id = AuthenticationController.getLoggedInUserId(req) + + # in v2 add notifications for matching university IPs + if Settings.overleaf? + NotificationsBuilder.ipMatcherAffiliation(user_id, ip) + NotificationsHandler.getUserNotifications user_id, (err, unreadNotifications)-> unreadNotifications = _.map unreadNotifications, (notification)-> notification.html = req.i18n.translate(notification.templateKey, notification.messageOpts) From 38faa5c25ea2275215269d6a548f50fde089c607 Mon Sep 17 00:00:00 2001 From: hugh-obrien Date: Sun, 2 Sep 2018 15:22:45 +0100 Subject: [PATCH 02/10] correctly create and display ip matched affiliations --- .../Notifications/NotificationsBuilder.coffee | 35 +++++++++++++------ .../NotificationsController.coffee | 4 ++- .../app/views/project/list/notifications.pug | 10 ++++++ 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee b/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee index 849995db6c..229c97cb04 100644 --- a/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee +++ b/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee @@ -33,14 +33,27 @@ module.exports = NotificationsHandler.markAsReadByKeyOnly @key, callback ipMatcherAffiliation: (userId, ip) -> - return null unless settings?.apis?.v1?.url # service is not configured - request { - method: 'GET' - url: "#{settings.apis.v1.url}/api/v2/users/ip_matcher/#{userId}" - auth: { user: settings.apis.v1.user, pass: settings.apis.v1.pass } - body: { ip: ip } - json: true - timeout: 20 * 1000 - }, (error, response, body) -> - return error if error? - return null if response.statusCode == 204 + key: "ip-matched-affiliation-#{ip}" + create: (callback=()->) -> + return null unless settings?.apis?.v1?.url # service is not configured + _key = @key + request { + method: 'GET' + url: "#{settings.apis.v1.url}/api/v2/users/ip_matcher/#{userId}" + auth: { user: settings.apis.v1.user, pass: settings.apis.v1.pass } + body: { ip: ip } + json: true + timeout: 20 * 1000 + }, (error, response, body) -> + return error if error? + return null if response.statusCode == 204 + + messageOpts = + university_id: body.university_id + university_name: body.university_name + content: body.ad_copy + logger.log user_id:userId, key:_key, "creating notification key for user" + NotificationsHandler.createNotification userId, _key, "notification_ip_matched_affiliation", messageOpts, null, callback + + read: (callback = ->)-> + NotificationsHandler.markAsReadWithKey userId, @key, callback diff --git a/services/web/app/coffee/Features/Notifications/NotificationsController.coffee b/services/web/app/coffee/Features/Notifications/NotificationsController.coffee index 517f17d325..5fde9bc955 100644 --- a/services/web/app/coffee/Features/Notifications/NotificationsController.coffee +++ b/services/web/app/coffee/Features/Notifications/NotificationsController.coffee @@ -15,7 +15,9 @@ module.exports = # in v2 add notifications for matching university IPs if Settings.overleaf? - NotificationsBuilder.ipMatcherAffiliation(user_id, ip) + NotificationsBuilder.ipMatcherAffiliation(user_id, ip).create((err) -> + return err + ) NotificationsHandler.getUserNotifications user_id, (err, unreadNotifications)-> unreadNotifications = _.map unreadNotifications, (notification)-> diff --git a/services/web/app/views/project/list/notifications.pug b/services/web/app/views/project/list/notifications.pug index 55798d6a2b..2b4466ebab 100644 --- a/services/web/app/views/project/list/notifications.pug +++ b/services/web/app/views/project/list/notifications.pug @@ -60,6 +60,16 @@ span(ng-controller="NotificationsController").userNotifications button(ng-click="dismiss(notification)").close.pull-right span(aria-hidden="true") × span.sr-only #{translate("close")} + .alert.alert-info(ng-switch-when="notification_ip_matched_affiliation") + div.notification_inner + .notification_body + | Add an email for: {{ notification.messageOpts.university_name }} + a.pull-right.btn.btn-sm.btn-info(href="/user/settings") + | Add Affiliation + 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 From de83df2703e9078d4f504e7859e76b1a098864bb Mon Sep 17 00:00:00 2001 From: hugh-obrien Date: Sun, 2 Sep 2018 17:28:51 +0100 Subject: [PATCH 03/10] adding tests for ip matching notifications --- .../NotificationsBuilderTests.coffee | 40 +++++++++++++++++++ .../NotificationsControllerTests.coffee | 15 +++++++ 2 files changed, 55 insertions(+) create mode 100644 services/web/test/unit/coffee/Notifications/NotificationsBuilderTests.coffee diff --git a/services/web/test/unit/coffee/Notifications/NotificationsBuilderTests.coffee b/services/web/test/unit/coffee/Notifications/NotificationsBuilderTests.coffee new file mode 100644 index 0000000000..8b984a8cee --- /dev/null +++ b/services/web/test/unit/coffee/Notifications/NotificationsBuilderTests.coffee @@ -0,0 +1,40 @@ +SandboxedModule = require('sandboxed-module') +assert = require('chai').assert +require('chai').should() +sinon = require('sinon') +modulePath = require('path').join __dirname, '../../../../app/js/Features/Notifications/NotificationsBuilder.js' + +describe 'NotificationsBuilder', -> + user_id = "123nd3ijdks" + + beforeEach -> + @handler = + createNotification: sinon.stub().callsArgWith(5) + + @settings = apis: { v1: { url: 'v1.url', user: '', pass: '' } } + @body = {university_id: 1, university_name: 'stanford', ad_copy: 'v1 ad content'} + response = {statusCode: 200} + @request = sinon.stub().returns(@stubResponse).callsArgWith(1, null, response, @body) + @controller = SandboxedModule.require modulePath, requires: + "./NotificationsHandler":@handler + "settings-sharelatex":@settings + 'request': @request + "logger-sharelatex": + log:-> + err:-> + + it 'should call v1 and create affiliation notifications', (done)-> + ip = '192.168.0.1' + @controller.ipMatcherAffiliation(user_id, ip).create (callback)=> + @request.calledOnce.should.equal true + expectedOpts = + university_id: @body.university_id + university_name: @body.university_name + content: @body.ad_copy + @handler.createNotification.calledWith( + user_id, + "ip-matched-affiliation-#{ip}", + "notification_ip_matched_affiliation", + expectedOpts + ).should.equal true + done() diff --git a/services/web/test/unit/coffee/Notifications/NotificationsControllerTests.coffee b/services/web/test/unit/coffee/Notifications/NotificationsControllerTests.coffee index 126b223f04..9fab8a96f5 100644 --- a/services/web/test/unit/coffee/Notifications/NotificationsControllerTests.coffee +++ b/services/web/test/unit/coffee/Notifications/NotificationsControllerTests.coffee @@ -13,7 +13,14 @@ describe 'NotificationsController', -> @handler = getUserNotifications: sinon.stub().callsArgWith(1) markAsRead: sinon.stub().callsArgWith(2) + @builder = + ipMatcherAffiliation: sinon.stub().returns({create: sinon.stub()}) + @settings = + apis: { v1: { url: 'v1.url', user: '', pass: '' } } @req = + headers: {} + connection: + remoteAddress: "192.170.18.1" params: notification_id:notification_id session: @@ -25,6 +32,8 @@ describe 'NotificationsController', -> getLoggedInUserId: sinon.stub().returns(@req.session.user._id) @controller = SandboxedModule.require modulePath, requires: "./NotificationsHandler":@handler + "./NotificationsBuilder":@builder + "settings-sharelatex":@settings "underscore":@underscore = map:(arr)-> return arr 'logger-sharelatex': @@ -44,3 +53,9 @@ describe 'NotificationsController', -> @controller.markNotificationAsRead @req, send:=> @handler.markAsRead.calledWith(user_id, notification_id).should.equal true done() + + it 'should call the builder with the user ip if v2', (done)-> + @settings.overleaf = true + @controller.getAllUnreadNotifications @req, send:(body)=> + @builder.ipMatcherAffiliation.calledWith(user_id, @req.connection.remoteAddress).should.equal true + done() From bf2ea4e7b3a9688cad95696921d476ec654faebe Mon Sep 17 00:00:00 2001 From: hugh-obrien Date: Tue, 4 Sep 2018 17:26:15 +0100 Subject: [PATCH 04/10] test against ip matcher for notification on login if different from previous ip --- .../Authentication/AuthenticationController.coffee | 11 +++++++++++ services/web/app/coffee/models/User.coffee | 1 + 2 files changed, 12 insertions(+) diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index 8a2c33536a..926d72a257 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -11,6 +11,7 @@ UserHandler = require("../User/UserHandler") UserSessionsManager = require("../User/UserSessionsManager") Analytics = require "../Analytics/AnalyticsManager" passport = require 'passport' +NotificationsBuilder = require("../Notifications/NotificationsBuilder") module.exports = AuthenticationController = @@ -72,6 +73,7 @@ module.exports = AuthenticationController = finishLogin: (user, req, res, next) -> redir = AuthenticationController._getRedirectFromSession(req) || "/project" AuthenticationController._loginAsyncHandlers(req, user) + AuthenticationController.ipMatchCheck(req, user) AuthenticationController.afterLoginSessionSetup req, user, (err) -> if err? return next(err) @@ -119,6 +121,15 @@ module.exports = AuthenticationController = # capture the request ip for use when creating the session user._login_req_ip = req.ip + ipMatchCheck: (req, user) -> + if req.ip != user.lastLoginIp + NotificationsBuilder.ipMatcherAffiliation(user._id, req.ip).create((err) -> + return err + ) + UserUpdater.updateUser user._id.toString(), { + $set: { "lastLoginIp": req.ip } + } + setInSessionUser: (req, props) -> for key, value of props if req?.session?.passport?.user? diff --git a/services/web/app/coffee/models/User.coffee b/services/web/app/coffee/models/User.coffee index 95bedebbf4..23b59375f4 100644 --- a/services/web/app/coffee/models/User.coffee +++ b/services/web/app/coffee/models/User.coffee @@ -22,6 +22,7 @@ UserSchema = new Schema confirmed : {type : Boolean, default : false} signUpDate : {type : Date, default: () -> new Date() } lastLoggedIn : {type : Date} + lastLoginIp : {type : String, default : ''} loginCount : {type : Number, default: 0} holdingAccount : {type : Boolean, default: false} ace : { From d950e14b3fe5b5fb2ced2295381a894fd459e185 Mon Sep 17 00:00:00 2001 From: hugh-obrien Date: Wed, 5 Sep 2018 09:44:45 +0100 Subject: [PATCH 05/10] use new routes and params from v1 ip matcher endpoint --- .../Notifications/NotificationsBuilder.coffee | 8 ++++---- .../Notifications/NotificationsController.coffee | 14 +++++++++++--- .../web/app/views/project/list/notifications.pug | 3 ++- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee b/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee index 229c97cb04..419a2a21f1 100644 --- a/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee +++ b/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee @@ -39,7 +39,7 @@ module.exports = _key = @key request { method: 'GET' - url: "#{settings.apis.v1.url}/api/v2/users/ip_matcher/#{userId}" + url: "#{settings.apis.v1.url}/api/v2/users/#{userId}/ip_matcher" auth: { user: settings.apis.v1.user, pass: settings.apis.v1.pass } body: { ip: ip } json: true @@ -49,9 +49,9 @@ module.exports = return null if response.statusCode == 204 messageOpts = - university_id: body.university_id - university_name: body.university_name - content: body.ad_copy + university_id: body.id + university_name: body.name + content: body.enrolment_ad_html logger.log user_id:userId, key:_key, "creating notification key for user" NotificationsHandler.createNotification userId, _key, "notification_ip_matched_affiliation", messageOpts, null, callback diff --git a/services/web/app/coffee/Features/Notifications/NotificationsController.coffee b/services/web/app/coffee/Features/Notifications/NotificationsController.coffee index 5fde9bc955..53ae2af4ac 100644 --- a/services/web/app/coffee/Features/Notifications/NotificationsController.coffee +++ b/services/web/app/coffee/Features/Notifications/NotificationsController.coffee @@ -2,7 +2,9 @@ NotificationsHandler = require("./NotificationsHandler") NotificationsBuilder = require("./NotificationsBuilder") AuthenticationController = require("../Authentication/AuthenticationController") Settings = require 'settings-sharelatex' +UserGetter = require("../User/UserGetter") logger = require("logger-sharelatex") +async = require "async" _ = require("underscore") module.exports = @@ -15,9 +17,15 @@ module.exports = # in v2 add notifications for matching university IPs if Settings.overleaf? - NotificationsBuilder.ipMatcherAffiliation(user_id, ip).create((err) -> - return err - ) + UserGetter.getUser user_id, { 'lastLoginIp': 1 }, (error, user) -> + console.log(user.lastLoginIp) + if ip != user.lastLoginIp + async.series ([ + () -> + NotificationsBuilder.ipMatcherAffiliation(user_id, ip).create((err) -> + return err + ) + ]) NotificationsHandler.getUserNotifications user_id, (err, unreadNotifications)-> unreadNotifications = _.map unreadNotifications, (notification)-> diff --git a/services/web/app/views/project/list/notifications.pug b/services/web/app/views/project/list/notifications.pug index 2b4466ebab..ef2ac66440 100644 --- a/services/web/app/views/project/list/notifications.pug +++ b/services/web/app/views/project/list/notifications.pug @@ -63,7 +63,8 @@ span(ng-controller="NotificationsController").userNotifications .alert.alert-info(ng-switch-when="notification_ip_matched_affiliation") div.notification_inner .notification_body - | Add an email for: {{ notification.messageOpts.university_name }} + | Add an email for + strong {{ notification.messageOpts.university_name }} a.pull-right.btn.btn-sm.btn-info(href="/user/settings") | Add Affiliation span().notification_close From f20d27986b6165c0841b4954f6325fb6ade60413 Mon Sep 17 00:00:00 2001 From: hugh-obrien Date: Wed, 5 Sep 2018 10:20:41 +0100 Subject: [PATCH 06/10] create ip match notifications without forcing replacement --- .../Features/Notifications/NotificationsBuilder.coffee | 2 +- .../Features/Notifications/NotificationsHandler.coffee | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee b/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee index 419a2a21f1..1b9eb56cf9 100644 --- a/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee +++ b/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee @@ -53,7 +53,7 @@ module.exports = university_name: body.name content: body.enrolment_ad_html logger.log user_id:userId, key:_key, "creating notification key for user" - NotificationsHandler.createNotification userId, _key, "notification_ip_matched_affiliation", messageOpts, null, callback + NotificationsHandler.createNotification userId, _key, "notification_ip_matched_affiliation", messageOpts, null, false, callback read: (callback = ->)-> NotificationsHandler.markAsReadWithKey userId, @key, callback diff --git a/services/web/app/coffee/Features/Notifications/NotificationsHandler.coffee b/services/web/app/coffee/Features/Notifications/NotificationsHandler.coffee index 5a6ca47c2e..a0f6ae5c12 100644 --- a/services/web/app/coffee/Features/Notifications/NotificationsHandler.coffee +++ b/services/web/app/coffee/Features/Notifications/NotificationsHandler.coffee @@ -29,12 +29,15 @@ module.exports = unreadNotifications = [] callback(null, unreadNotifications) - createNotification: (user_id, key, templateKey, messageOpts, expiryDateTime, callback)-> + createNotification: (user_id, key, templateKey, messageOpts, expiryDateTime, forceCreate, callback)-> + if !callback + callback = forceCreate + forceCreate = true payload = { key:key messageOpts:messageOpts templateKey:templateKey - forceCreate: true + forceCreate:forceCreate } if expiryDateTime? payload.expires = expiryDateTime From 23e6292fd7c60dccde57407fef8b74024e12c31d Mon Sep 17 00:00:00 2001 From: hugh-obrien Date: Wed, 5 Sep 2018 10:44:34 +0100 Subject: [PATCH 07/10] updating tests for ip matcher logic --- .../Notifications/NotificationsController.coffee | 1 - .../AuthenticationControllerTests.coffee | 1 + .../Notifications/NotificationsBuilderTests.coffee | 10 +++++----- .../Notifications/NotificationsControllerTests.coffee | 5 ++++- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/services/web/app/coffee/Features/Notifications/NotificationsController.coffee b/services/web/app/coffee/Features/Notifications/NotificationsController.coffee index 53ae2af4ac..08ac42b6ac 100644 --- a/services/web/app/coffee/Features/Notifications/NotificationsController.coffee +++ b/services/web/app/coffee/Features/Notifications/NotificationsController.coffee @@ -18,7 +18,6 @@ module.exports = # in v2 add notifications for matching university IPs if Settings.overleaf? UserGetter.getUser user_id, { 'lastLoginIp': 1 }, (error, user) -> - console.log(user.lastLoginIp) if ip != user.lastLoginIp async.series ([ () -> diff --git a/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee index 24af9971d2..5f28a207b2 100644 --- a/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee +++ b/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee @@ -603,6 +603,7 @@ describe "AuthenticationController", -> @AuthenticationController._loginAsyncHandlers = sinon.stub() @AuthenticationController.afterLoginSessionSetup = sinon.stub().callsArgWith(2, null) @AuthenticationController._clearRedirectFromSession = sinon.stub() + @UserUpdater.updateUser = sinon.stub() @req.headers = {accept: 'application/json, whatever'} @res.json = sinon.stub() @res.redirect = sinon.stub() diff --git a/services/web/test/unit/coffee/Notifications/NotificationsBuilderTests.coffee b/services/web/test/unit/coffee/Notifications/NotificationsBuilderTests.coffee index 8b984a8cee..941e26df1e 100644 --- a/services/web/test/unit/coffee/Notifications/NotificationsBuilderTests.coffee +++ b/services/web/test/unit/coffee/Notifications/NotificationsBuilderTests.coffee @@ -9,10 +9,10 @@ describe 'NotificationsBuilder', -> beforeEach -> @handler = - createNotification: sinon.stub().callsArgWith(5) + createNotification: sinon.stub().callsArgWith(6) @settings = apis: { v1: { url: 'v1.url', user: '', pass: '' } } - @body = {university_id: 1, university_name: 'stanford', ad_copy: 'v1 ad content'} + @body = {id: 1, name: 'stanford', enrolment_ad_html: 'v1 ad content'} response = {statusCode: 200} @request = sinon.stub().returns(@stubResponse).callsArgWith(1, null, response, @body) @controller = SandboxedModule.require modulePath, requires: @@ -28,9 +28,9 @@ describe 'NotificationsBuilder', -> @controller.ipMatcherAffiliation(user_id, ip).create (callback)=> @request.calledOnce.should.equal true expectedOpts = - university_id: @body.university_id - university_name: @body.university_name - content: @body.ad_copy + university_id: @body.id + university_name: @body.name + content: @body.enrolment_ad_html @handler.createNotification.calledWith( user_id, "ip-matched-affiliation-#{ip}", diff --git a/services/web/test/unit/coffee/Notifications/NotificationsControllerTests.coffee b/services/web/test/unit/coffee/Notifications/NotificationsControllerTests.coffee index 9fab8a96f5..191b869fec 100644 --- a/services/web/test/unit/coffee/Notifications/NotificationsControllerTests.coffee +++ b/services/web/test/unit/coffee/Notifications/NotificationsControllerTests.coffee @@ -15,6 +15,8 @@ describe 'NotificationsController', -> markAsRead: sinon.stub().callsArgWith(2) @builder = ipMatcherAffiliation: sinon.stub().returns({create: sinon.stub()}) + @userGetter = + getUser: sinon.stub().callsArgWith 2, null, {lastLoginIp: '192.170.18.2'} @settings = apis: { v1: { url: 'v1.url', user: '', pass: '' } } @req = @@ -33,6 +35,7 @@ describe 'NotificationsController', -> @controller = SandboxedModule.require modulePath, requires: "./NotificationsHandler":@handler "./NotificationsBuilder":@builder + "../User/UserGetter": @userGetter "settings-sharelatex":@settings "underscore":@underscore = map:(arr)-> return arr @@ -49,7 +52,7 @@ describe 'NotificationsController', -> @handler.getUserNotifications.calledWith(user_id).should.equal true done() - it 'should send a delete request when a delete has been received to mark a notification', (done)-> + it 'should send a remove request when notification read', (done)-> @controller.markNotificationAsRead @req, send:=> @handler.markAsRead.calledWith(user_id, notification_id).should.equal true done() From 5605e1c5c3ddae9144002fc7249542f273aec957 Mon Sep 17 00:00:00 2001 From: hugh-obrien Date: Wed, 5 Sep 2018 11:02:13 +0100 Subject: [PATCH 08/10] update copy for ip match notifications --- services/web/app/views/project/list/notifications.pug | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/services/web/app/views/project/list/notifications.pug b/services/web/app/views/project/list/notifications.pug index ef2ac66440..04ba3827dc 100644 --- a/services/web/app/views/project/list/notifications.pug +++ b/services/web/app/views/project/list/notifications.pug @@ -63,8 +63,12 @@ span(ng-controller="NotificationsController").userNotifications .alert.alert-info(ng-switch-when="notification_ip_matched_affiliation") div.notification_inner .notification_body - | Add an email for - strong {{ notification.messageOpts.university_name }} + | It looks like you're at + strong {{ notification.messageOpts.university_name }}!
+ | Did you know that {{notification.messageOpts.university_name}} is providing + strong free Overleaf Professional accounts + | to everyone at {{notification.messageOpts.university_name}}?
+ | Add an institutional email address to claim your account. a.pull-right.btn.btn-sm.btn-info(href="/user/settings") | Add Affiliation span().notification_close From 8ef90a0dcbdfcd19ea9e8753e3bafe03aaf9075c Mon Sep 17 00:00:00 2001 From: hugh-obrien Date: Wed, 5 Sep 2018 15:28:26 +0100 Subject: [PATCH 09/10] move call for creating ip matched notifcation to project controller --- .../AuthenticationController.coffee | 2 +- .../Notifications/NotificationsBuilder.coffee | 2 +- .../NotificationsController.coffee | 19 ------------------ .../Features/Project/ProjectController.coffee | 12 +++++++++++ .../AuthenticationControllerTests.coffee | 3 +-- .../NotificationsControllerTests.coffee | 20 +------------------ .../Project/ProjectControllerTests.coffee | 16 +++++++++++++++ 7 files changed, 32 insertions(+), 42 deletions(-) diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index 926d72a257..49dcd25359 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -73,7 +73,6 @@ module.exports = AuthenticationController = finishLogin: (user, req, res, next) -> redir = AuthenticationController._getRedirectFromSession(req) || "/project" AuthenticationController._loginAsyncHandlers(req, user) - AuthenticationController.ipMatchCheck(req, user) AuthenticationController.afterLoginSessionSetup req, user, (err) -> if err? return next(err) @@ -114,6 +113,7 @@ module.exports = AuthenticationController = UserHandler.setupLoginData(user, ()->) LoginRateLimiter.recordSuccessfulLogin(user.email) AuthenticationController._recordSuccessfulLogin(user._id) + AuthenticationController.ipMatchCheck(req, user) Analytics.recordEvent(user._id, "user-logged-in", {ip:req.ip}) Analytics.identifyUser(user._id, req.sessionID) logger.log email: user.email, user_id: user._id.toString(), "successful log in" diff --git a/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee b/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee index 1b9eb56cf9..c0280450ab 100644 --- a/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee +++ b/services/web/app/coffee/Features/Notifications/NotificationsBuilder.coffee @@ -46,7 +46,7 @@ module.exports = timeout: 20 * 1000 }, (error, response, body) -> return error if error? - return null if response.statusCode == 204 + return null unless response.statusCode == 200 messageOpts = university_id: body.id diff --git a/services/web/app/coffee/Features/Notifications/NotificationsController.coffee b/services/web/app/coffee/Features/Notifications/NotificationsController.coffee index 08ac42b6ac..5b83a60248 100644 --- a/services/web/app/coffee/Features/Notifications/NotificationsController.coffee +++ b/services/web/app/coffee/Features/Notifications/NotificationsController.coffee @@ -1,31 +1,12 @@ NotificationsHandler = require("./NotificationsHandler") -NotificationsBuilder = require("./NotificationsBuilder") AuthenticationController = require("../Authentication/AuthenticationController") -Settings = require 'settings-sharelatex' -UserGetter = require("../User/UserGetter") logger = require("logger-sharelatex") -async = require "async" _ = require("underscore") module.exports = getAllUnreadNotifications: (req, res)-> - ip = req.headers['x-forwarded-for'] || - req.connection.remoteAddress || - req.socket.remoteAddress user_id = AuthenticationController.getLoggedInUserId(req) - - # in v2 add notifications for matching university IPs - if Settings.overleaf? - UserGetter.getUser user_id, { 'lastLoginIp': 1 }, (error, user) -> - if ip != user.lastLoginIp - async.series ([ - () -> - NotificationsBuilder.ipMatcherAffiliation(user_id, ip).create((err) -> - return err - ) - ]) - NotificationsHandler.getUserNotifications user_id, (err, unreadNotifications)-> unreadNotifications = _.map unreadNotifications, (notification)-> notification.html = req.i18n.translate(notification.templateKey, notification.messageOpts) diff --git a/services/web/app/coffee/Features/Project/ProjectController.coffee b/services/web/app/coffee/Features/Project/ProjectController.coffee index 24dca35e96..e5acd1dfb9 100644 --- a/services/web/app/coffee/Features/Project/ProjectController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectController.coffee @@ -26,6 +26,8 @@ TokenAccessHandler = require '../TokenAccess/TokenAccessHandler' CollaboratorsHandler = require '../Collaborators/CollaboratorsHandler' Modules = require '../../infrastructure/Modules' ProjectEntityHandler = require './ProjectEntityHandler' +UserGetter = require("../User/UserGetter") +NotificationsBuilder = require("../Notifications/NotificationsBuilder") crypto = require 'crypto' { V1ConnectionError } = require '../Errors/Errors' Features = require('../../infrastructure/Features') @@ -209,6 +211,16 @@ module.exports = ProjectController = user = results.user warnings = ProjectController._buildWarningsList results.v1Projects + # in v2 add notifications for matching university IPs + if Settings.overleaf? + ip = req.headers['x-forwarded-for'] || + req.connection.remoteAddress || + req.socket.remoteAddress + UserGetter.getUser user_id, { 'lastLoginIp': 1 }, (error, user) -> + if ip != user.lastLoginIp + NotificationsBuilder.ipMatcherAffiliation(user._id, ip).create((err) -> + return err + ) ProjectController._injectProjectOwners projects, (error, projects) -> return next(error) if error? diff --git a/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee index 5f28a207b2..300a4663e7 100644 --- a/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee +++ b/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee @@ -15,7 +15,7 @@ describe "AuthenticationController", -> tk.freeze(Date.now()) @AuthenticationController = SandboxedModule.require modulePath, requires: "./AuthenticationManager": @AuthenticationManager = {} - "../User/UserUpdater" : @UserUpdater = {} + "../User/UserUpdater" : @UserUpdater = {updateUser:sinon.stub()} "metrics-sharelatex": @Metrics = { inc: sinon.stub() } "../Security/LoginRateLimiter": @LoginRateLimiter = { processLoginRequest:sinon.stub(), recordSuccessfulLogin:sinon.stub() } "../User/UserHandler": @UserHandler = {setupLoginData:sinon.stub()} @@ -603,7 +603,6 @@ describe "AuthenticationController", -> @AuthenticationController._loginAsyncHandlers = sinon.stub() @AuthenticationController.afterLoginSessionSetup = sinon.stub().callsArgWith(2, null) @AuthenticationController._clearRedirectFromSession = sinon.stub() - @UserUpdater.updateUser = sinon.stub() @req.headers = {accept: 'application/json, whatever'} @res.json = sinon.stub() @res.redirect = sinon.stub() diff --git a/services/web/test/unit/coffee/Notifications/NotificationsControllerTests.coffee b/services/web/test/unit/coffee/Notifications/NotificationsControllerTests.coffee index 191b869fec..126b223f04 100644 --- a/services/web/test/unit/coffee/Notifications/NotificationsControllerTests.coffee +++ b/services/web/test/unit/coffee/Notifications/NotificationsControllerTests.coffee @@ -13,16 +13,7 @@ describe 'NotificationsController', -> @handler = getUserNotifications: sinon.stub().callsArgWith(1) markAsRead: sinon.stub().callsArgWith(2) - @builder = - ipMatcherAffiliation: sinon.stub().returns({create: sinon.stub()}) - @userGetter = - getUser: sinon.stub().callsArgWith 2, null, {lastLoginIp: '192.170.18.2'} - @settings = - apis: { v1: { url: 'v1.url', user: '', pass: '' } } @req = - headers: {} - connection: - remoteAddress: "192.170.18.1" params: notification_id:notification_id session: @@ -34,9 +25,6 @@ describe 'NotificationsController', -> getLoggedInUserId: sinon.stub().returns(@req.session.user._id) @controller = SandboxedModule.require modulePath, requires: "./NotificationsHandler":@handler - "./NotificationsBuilder":@builder - "../User/UserGetter": @userGetter - "settings-sharelatex":@settings "underscore":@underscore = map:(arr)-> return arr 'logger-sharelatex': @@ -52,13 +40,7 @@ describe 'NotificationsController', -> @handler.getUserNotifications.calledWith(user_id).should.equal true done() - it 'should send a remove request when notification read', (done)-> + it 'should send a delete request when a delete has been received to mark a notification', (done)-> @controller.markNotificationAsRead @req, send:=> @handler.markAsRead.calledWith(user_id, notification_id).should.equal true done() - - it 'should call the builder with the user ip if v2', (done)-> - @settings.overleaf = true - @controller.getAllUnreadNotifications @req, send:(body)=> - @builder.ipMatcherAffiliation.calledWith(user_id, @req.connection.remoteAddress).should.equal true - done() diff --git a/services/web/test/unit/coffee/Project/ProjectControllerTests.coffee b/services/web/test/unit/coffee/Project/ProjectControllerTests.coffee index b909376cca..f7edc94ad1 100644 --- a/services/web/test/unit/coffee/Project/ProjectControllerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectControllerTests.coffee @@ -69,6 +69,10 @@ describe "ProjectController", -> @CollaboratorsHandler = userIsTokenMember: sinon.stub().callsArgWith(2, null, false) @ProjectEntityHandler = {} + @NotificationBuilder = + ipMatcherAffiliation: sinon.stub().returns({create: sinon.stub()}) + @UserGetter = + getUser: sinon.stub().callsArgWith 2, null, {lastLoginIp: '192.170.18.2'} @Modules = hooks: fire: sinon.stub() @@ -105,11 +109,16 @@ describe "ProjectController", -> "./ProjectEntityHandler": @ProjectEntityHandler "../Errors/Errors": Errors "../../infrastructure/Features": @Features + "../Notifications/NotificationsBuilder":@NotificationBuilder + "../User/UserGetter": @UserGetter @projectName = "£12321jkj9ujkljds" @req = params: Project_id: @project_id + headers: {} + connection: + remoteAddress: "192.170.18.1" session: user: @user body: @@ -301,6 +310,13 @@ describe "ProjectController", -> done() @ProjectController.projectListPage @req, @res + it "should create trigger ip matcher notifications", (done)-> + @settings.overleaf = true + @res.render = (pageName, opts)=> + @NotificationBuilder.ipMatcherAffiliation.called.should.equal true + done() + @ProjectController.projectListPage @req, @res + it "should send the projects", (done)-> @res.render = (pageName, opts)=> opts.projects.length.should.equal (@projects.length + @collabertions.length + @readOnly.length + @tokenReadAndWrite.length + @tokenReadOnly.length) From 1e04a09ec6369fd4db94e7771d31b8d4fdcc86f0 Mon Sep 17 00:00:00 2001 From: hugh-obrien Date: Fri, 7 Sep 2018 18:15:32 +0100 Subject: [PATCH 10/10] remove unnecessary error returns and ip fetching --- .../Authentication/AuthenticationController.coffee | 4 +--- .../app/coffee/Features/Project/ProjectController.coffee | 9 ++------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index 49dcd25359..f8d90756b2 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -123,9 +123,7 @@ module.exports = AuthenticationController = ipMatchCheck: (req, user) -> if req.ip != user.lastLoginIp - NotificationsBuilder.ipMatcherAffiliation(user._id, req.ip).create((err) -> - return err - ) + NotificationsBuilder.ipMatcherAffiliation(user._id, req.ip).create() UserUpdater.updateUser user._id.toString(), { $set: { "lastLoginIp": req.ip } } diff --git a/services/web/app/coffee/Features/Project/ProjectController.coffee b/services/web/app/coffee/Features/Project/ProjectController.coffee index e5acd1dfb9..59c0647c19 100644 --- a/services/web/app/coffee/Features/Project/ProjectController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectController.coffee @@ -213,14 +213,9 @@ module.exports = ProjectController = # in v2 add notifications for matching university IPs if Settings.overleaf? - ip = req.headers['x-forwarded-for'] || - req.connection.remoteAddress || - req.socket.remoteAddress UserGetter.getUser user_id, { 'lastLoginIp': 1 }, (error, user) -> - if ip != user.lastLoginIp - NotificationsBuilder.ipMatcherAffiliation(user._id, ip).create((err) -> - return err - ) + if req.ip != user.lastLoginIp + NotificationsBuilder.ipMatcherAffiliation(user._id, req.ip).create() ProjectController._injectProjectOwners projects, (error, projects) -> return next(error) if error?