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 + }) + }) })