diff --git a/services/web/app/coffee/Features/Analytics/AnalyticsController.coffee b/services/web/app/coffee/Features/Analytics/AnalyticsController.coffee index a57118723e..24173f2aed 100644 --- a/services/web/app/coffee/Features/Analytics/AnalyticsController.coffee +++ b/services/web/app/coffee/Features/Analytics/AnalyticsController.coffee @@ -6,9 +6,10 @@ module.exports = AnalyticsController = updateEditingSession: (req, res, next) -> userId = AuthenticationController.getLoggedInUserId(req) projectId = req.params.projectId + countryCode = req.session.countryCode || null if userId? - AnalyticsManager.updateEditingSession userId, projectId, {}, (error) -> + AnalyticsManager.updateEditingSession userId, projectId, countryCode, {}, (error) -> respondWith(error, res, next) else res.send 204 diff --git a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee index 3a3467b59e..be6b821183 100644 --- a/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee +++ b/services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee @@ -43,7 +43,7 @@ module.exports = opts.qs = {fromV2: 1} makeRequest opts, callback - updateEditingSession: (userId, projectId, segmentation = {}, callback = (error) ->) -> + updateEditingSession: (userId, projectId, countryCode, segmentation = {}, callback = (error) ->) -> if userId+"" == settings.smokeTest?.userId+"" return callback() opts = @@ -56,6 +56,7 @@ module.exports = qs: userId: userId projectId: projectId + countryCode: countryCode || undefined maxAttempts: 20 retryDelay: 5000 if settings.overleaf? diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index 3fbad520c9..98495cfcec 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -12,6 +12,7 @@ UserHandler = require("../User/UserHandler") UserSessionsManager = require("../User/UserSessionsManager") Analytics = require "../Analytics/AnalyticsManager" passport = require 'passport' +GeoIpLookup = require '../../infrastructure/GeoIpLookup' module.exports = AuthenticationController = @@ -47,12 +48,17 @@ module.exports = AuthenticationController = req.session[key] = value # copy to the old `session.user` location, for backward-comptability req.session.user = req.session.passport.user - req.session.save (err) -> + # try to geolocate the user to a country-code + GeoIpLookup.getDetails req.ip, (err, geoDetails) -> if err? - logger.err {user_id: user._id}, "error saving regenerated session after login" - return callback(err) - UserSessionsManager.trackSession(user, req.sessionID, () ->) - callback(null) + logger.err {err, ip: req.ip}, "error geolocating session, continuing" + req.session.countryCode = geoDetails?.country_code || null + req.session.save (err) -> + if err? + logger.err {user_id: user._id}, "error saving regenerated session after login" + return callback(err) + UserSessionsManager.trackSession(user, req.sessionID, () ->) + callback(err) passportLogin: (req, res, next) -> # This function is middleware which wraps the passport.authenticate middleware, diff --git a/services/web/app/coffee/infrastructure/GeoIpLookup.coffee b/services/web/app/coffee/infrastructure/GeoIpLookup.coffee index 88f3ed4bfa..1fb9e29127 100644 --- a/services/web/app/coffee/infrastructure/GeoIpLookup.coffee +++ b/services/web/app/coffee/infrastructure/GeoIpLookup.coffee @@ -31,6 +31,7 @@ module.exports = GeoIpLookup = e = new Error("no ip passed") return callback(e) ip = ip.trim().split(" ")[0] + console.log ">> ", URL.resolve(settings.apis.geoIpLookup.url,ip) opts = url: URL.resolve(settings.apis.geoIpLookup.url,ip) timeout: 1000 @@ -49,4 +50,4 @@ module.exports = GeoIpLookup = countryCode = ipDetails?.country_code?.toUpperCase() currencyCode = currencyMappings[countryCode] || "USD" logger.log ip:ip, currencyCode:currencyCode, ipDetails:ipDetails, "got currencyCode for ip" - callback(err, currencyCode, countryCode) \ No newline at end of file + callback(err, currencyCode, countryCode) diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee index 0c9b71c3ae..2d1dc5209d 100644 --- a/services/web/config/settings.defaults.coffee +++ b/services/web/config/settings.defaults.coffee @@ -139,7 +139,7 @@ module.exports = settings = apiKey: "" subdomain: "" geoIpLookup: - url: "http://#{process.env['GEOIP_HOST'] or 'localhost'}:8080/json" + url: "http://#{process.env['GEOIP_HOST'] or 'localhost'}:8080/json/" realTime: url: "http://#{process.env['REALTIME_HOST'] or 'localhost'}:3026" contacts: diff --git a/services/web/test/unit/coffee/Analytics/AnalyticsControllerTests.coffee b/services/web/test/unit/coffee/Analytics/AnalyticsControllerTests.coffee index 8ac73c5af1..edfc14c8ed 100644 --- a/services/web/test/unit/coffee/Analytics/AnalyticsControllerTests.coffee +++ b/services/web/test/unit/coffee/Analytics/AnalyticsControllerTests.coffee @@ -14,7 +14,7 @@ describe 'AnalyticsController', -> getLoggedInUserId: sinon.stub() @AnalyticsManager = - updateEditingSession: sinon.stub().callsArgWith(3) + updateEditingSession: sinon.stub().callsArgWith(4) recordEvent: sinon.stub().callsArgWith(3) @controller = SandboxedModule.require modulePath, requires: @@ -31,12 +31,18 @@ describe 'AnalyticsController', -> @req = params: projectId: "a project id" + session: {countryCode: 'US'} it "delegates to the AnalyticsManager", (done) -> @AuthenticationController.getLoggedInUserId.returns("1234") @controller.updateEditingSession @req, @res - @AnalyticsManager.updateEditingSession.calledWith("1234", "a project id", {}).should.equal true + @AnalyticsManager.updateEditingSession.calledWith( + "1234", + "a project id", + 'US', + {} + ).should.equal true done() describe "recordEvent", -> @@ -46,6 +52,7 @@ describe 'AnalyticsController', -> event:"i_did_something" body:"stuff" sessionID: "sessionIDHere" + session: {} it "should use the user_id", (done)-> @AuthenticationController.getLoggedInUserId.returns("1234") diff --git a/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee index 92d2a7dbdb..707b362eb9 100644 --- a/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee +++ b/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee @@ -28,6 +28,8 @@ describe "AuthenticationController", -> trackSession: sinon.stub() untrackSession: sinon.stub() revokeAllUserSessions: sinon.stub().callsArgWith(1, null) + "../../infrastructure/GeoIpLookup": @GeoIpLookup = + getDetails: sinon.stub().callsArgWith(1, null, {}) @user = _id: ObjectId() email: @email = "USER@example.com" @@ -172,6 +174,8 @@ describe "AuthenticationController", -> @req.session.save = sinon.stub().callsArgWith(0, null) @req.sessionStore = {generate: sinon.stub()} @UserSessionsManager.trackSession = sinon.stub() + @GeoIpLookup.getDetails = sinon.stub() + .callsArgWith(1, null, @geoDetails = {country_code: 'US'}) @call = (callback) => @AuthenticationController.afterLoginSessionSetup @req, @user, callback @@ -185,16 +189,23 @@ describe "AuthenticationController", -> @req.login.callCount.should.equal 1 done() - it 'should call req.session.save', (done) -> - @call (err) => - @req.session.save.callCount.should.equal 1 - done() - it 'should call UserSessionsManager.trackSession', (done) -> @call (err) => @UserSessionsManager.trackSession.callCount.should.equal 1 done() + it 'should call GeoIpLookup.getDetails', (done) -> + @call (err) => + @GeoIpLookup.getDetails.callCount.should.equal 1 + @req.session.countryCode.should.exist + @req.session.countryCode.should.equal @geoDetails.country_code + done() + + it 'should call req.session.save', (done) -> + @call (err) => + @req.session.save.callCount.should.equal 1 + done() + describe 'when req.session.save produces an error', -> beforeEach -> @@ -211,6 +222,39 @@ describe "AuthenticationController", -> @UserSessionsManager.trackSession.callCount.should.equal 0 done() + describe 'when GeoIpLookup produces an error', -> + beforeEach -> + @GeoIpLookup.getDetails = sinon.stub() + .callsArgWith(1, new Error('woops')) + + it 'should not produce an error', (done) -> + @call (err) => + expect(err).to.not.exist + done() + + it 'should still call req.session.save', (done) -> + @call (err) => + @req.session.save.callCount.should.equal 1 + expect(@req.session.countryCode).to.equal null + done() + + describe 'when GeoIpLookup produces no data', -> + beforeEach -> + @GeoIpLookup.getDetails = sinon.stub() + .callsArgWith(1, null, undefined) + + it 'should not produce an error', (done) -> + @call (err) => + expect(err).to.not.exist + done() + + it 'should still call req.session.save', (done) -> + @call (err) => + @req.session.save.callCount.should.equal 1 + expect(@req.session.countryCode).to.equal null + done() + + describe 'getSessionUser', -> it 'should get the user object from session', ->