From abd57e03cfdcee95b3988fcd56f5d83cacde88fc Mon Sep 17 00:00:00 2001
From: ilkin-overleaf <100852799+ilkin-overleaf@users.noreply.github.com>
Date: Mon, 27 May 2024 16:09:23 +0300
Subject: [PATCH] Merge pull request #17831 from
 overleaf/msm-filter-saml-error-log

[web] Filter saml error logs by path

GitOrigin-RevId: 4ca9e156657afc893f38fed7ec6b00cbb7a608ef
---
 .../src/Features/SamlLog/SamlLogHandler.js    |  9 +++-
 .../unit/src/SamlLog/SamlLogHandlerTests.js   | 53 +++++++++++++++++++
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/services/web/app/src/Features/SamlLog/SamlLogHandler.js b/services/web/app/src/Features/SamlLog/SamlLogHandler.js
index 24f68aea14..e8ddd6897b 100644
--- a/services/web/app/src/Features/SamlLog/SamlLogHandler.js
+++ b/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
diff --git a/services/web/test/unit/src/SamlLog/SamlLogHandlerTests.js b/services/web/test/unit/src/SamlLog/SamlLogHandlerTests.js
index 5400a1b963..c3ac36dacf 100644
--- a/services/web/test/unit/src/SamlLog/SamlLogHandlerTests.js
+++ b/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
+    })
+  })
 })