Merge pull request #17831 from overleaf/msm-filter-saml-error-log

[web] Filter saml error logs by path

GitOrigin-RevId: 4ca9e156657afc893f38fed7ec6b00cbb7a608ef

services/web/app/src/Features/SamlLog/SamlLogHandler.js

@@ -3,6 +3,9 @@ const SessionManager = require('../Authentication/SessionManager')
 const logger = require('@overleaf/logger')
 const { err: errSerializer } = require('@overleaf/logger/serializers')
 const { callbackify } = require('util')
+const Settings = require('@overleaf/settings')
+
+const ALLOWED_PATHS = Settings.saml?.logAllowList || ['/saml/']
 
 async function log(req, data, samlAssertion) {
   let providerId, sessionId
@@ -10,14 +13,18 @@ async function log(req, data, samlAssertion) {
   data = data || {}
 
   try {
-    const samlLog = new SamlLog()
     const { path, query } = req
+    if (!ALLOWED_PATHS.some(allowedPath => path.startsWith(allowedPath))) {
+      return
+    }
+
     const { saml } = req.session
     const userId = SessionManager.getLoggedInUserId(req.session)
 
     providerId = (req.session.saml?.universityId || '').toString()
     sessionId = (req.sessionID || '').toString().substr(0, 8)
 
+    const samlLog = new SamlLog()
     samlLog.providerId = providerId
     samlLog.sessionId = sessionId
     samlLog.path = path

services/web/test/unit/src/SamlLog/SamlLogHandlerTests.js

@@ -34,6 +34,7 @@ describe('SamlLogHandler', function () {
         {
           session: { saml: { universityId: providerId } },
           sessionID: sessionId,
+          path: '/saml/ukamf',
         },
         data
       )
@@ -62,6 +63,7 @@ describe('SamlLogHandler', function () {
         {
           session: { saml: { universityId: providerId } },
           sessionID: sessionId,
+          path: '/saml/ukamf',
         },
         circularRef
       )
@@ -91,6 +93,7 @@ describe('SamlLogHandler', function () {
         {
           session: { saml: { universityId: providerId } },
           sessionID: sessionId,
+          path: '/saml/ukamf',
         },
         data
       )
@@ -106,4 +109,54 @@ describe('SamlLogHandler', function () {
       )
     })
   })
+
+  describe('with /saml/group-sso path', function () {
+    let err
+
+    beforeEach(async function () {
+      err = new Error()
+      samlLog.save = sinon.stub().rejects(err)
+
+      await SamlLogHandler.promises.log(
+        {
+          session: { saml: { universityId: providerId } },
+          sessionID: sessionId,
+          path: '/saml/group-sso',
+        },
+        data
+      )
+    })
+
+    it('should log error', function () {
+      this.logger.error.should.have.been.calledOnce.and.calledWithMatch(
+        {
+          err,
+          sessionId: sessionId.substr(0, 8),
+        },
+        'SamlLog Error'
+      )
+    })
+  })
+
+  describe('with a path not in the allow list', function () {
+    let err
+
+    beforeEach(async function () {
+      err = new Error()
+      samlLog.save = sinon.stub().rejects(err)
+
+      await SamlLogHandler.promises.log(
+        {
+          session: { saml: { universityId: providerId } },
+          sessionID: sessionId,
+          path: '/unsupported',
+        },
+        data
+      )
+    })
+
+    it('should not log any error', function () {
+      this.logger.error.should.not.have.been.called
+    })
+  })
 })