Merge pull request #3608 from overleaf/jel-sso-log

Improve SSO error log

GitOrigin-RevId: a7d3899c662283e49b7505d24b3191213de3968e

services/web/app/src/Features/User/ThirdPartyIdentityManager.js

@@ -5,6 +5,7 @@ const EmailOptionsHelper = require(`${APP_ROOT}/Features/Email/EmailOptionsHelpe
 const Errors = require('../Errors/Errors')
 const _ = require('lodash')
 const logger = require('logger-sharelatex')
+const OError = require('@overleaf/o-error')
 const settings = require('settings-sharelatex')
 const { User } = require(`${APP_ROOT}/models/User`)
 const { promisifyAll } = require(`${APP_ROOT}/util/promises`)
@@ -13,7 +14,12 @@ const oauthProviders = settings.oauthProviders || {}
 
 function getUser(providerId, externalUserId, callback) {
   if (providerId == null || externalUserId == null) {
-    return callback(new Error('invalid arguments'))
+    return callback(
+      new OError('invalid SSO arguments', {
+        externalUserId,
+        providerId
+      })
+    )
   }
   const query = _getUserQuery(providerId, externalUserId)
   User.findOne(query, function(err, user) {

services/web/test/unit/src/User/ThirdPartyIdentityManagerTests.js

@@ -32,7 +32,7 @@ describe('ThirdPartyIdentityManager', function() {
         '../../../../app/src/models/User': {
           User: (this.User = {
             findOneAndUpdate: sinon.stub().yields(undefined, this.user),
-            findOne: sinon.stub()
+            findOne: sinon.stub().yields(undefined, undefined)
           })
         },
         'settings-sharelatex': {
@@ -48,6 +48,48 @@ describe('ThirdPartyIdentityManager', function() {
       }
     })
   })
+  describe('getUser', function() {
+    it('should an error when missing providerId or externalUserId', function(done) {
+      this.ThirdPartyIdentityManager.getUser(
+        undefined,
+        undefined,
+        (error, user) => {
+          expect(error).to.exist
+          expect(error.message).to.equal('invalid SSO arguments')
+          expect(error.info).to.deep.equal({
+            externalUserId: undefined,
+            providerId: undefined
+          })
+          done()
+        }
+      )
+    })
+    describe('when user linked', function() {
+      beforeEach(function() {
+        this.User.findOne.yields(undefined, this.user)
+      })
+
+      it('should return the user', async function() {
+        this.User.findOne.returns(undefined, this.user)
+        const user = await this.ThirdPartyIdentityManager.promises.getUser(
+          'google',
+          'an-id-linked'
+        )
+        expect(user).to.deep.equal(this.user)
+      })
+    })
+    it('should return ThirdPartyUserNotFoundError when no user linked', function(done) {
+      this.ThirdPartyIdentityManager.getUser(
+        'google',
+        'an-id-not-linked',
+        (error, user) => {
+          expect(error).to.exist
+          expect(error.name).to.equal('ThirdPartyUserNotFoundError')
+          done()
+        }
+      )
+    })
+  })
   describe('link', function() {
     it('should send email alert', async function() {
       await this.ThirdPartyIdentityManager.promises.link(