From b9321121b8ae2539f063a6810eb9e25fa058afc4 Mon Sep 17 00:00:00 2001 From: Aditya Sridhar Date: Thu, 4 Jan 2024 23:02:17 -0500 Subject: [PATCH] fix: adds tests and moves function out --- .../__tests__/lib/access-helpers.test.js | 31 ++++++++++++++++++- packages/server/src/lib/access-helpers.js | 13 +++++++- packages/server/src/routes/sessions.js | 15 +++------ 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/packages/server/__tests__/lib/access-helpers.test.js b/packages/server/__tests__/lib/access-helpers.test.js index 5d7802f19..da260f9d2 100644 --- a/packages/server/__tests__/lib/access-helpers.test.js +++ b/packages/server/__tests__/lib/access-helpers.test.js @@ -1,5 +1,6 @@ const { expect } = require('chai'); -const { getAdminAuthInfo } = require('../../src/lib/access-helpers'); +const sinon = require('sinon'); +const { getAdminAuthInfo, isMicrosoftSafeLinksRequest } = require('../../src/lib/access-helpers'); const db = require('../../src/db'); const fixtures = require('../db/seeds/fixtures'); @@ -88,4 +89,32 @@ describe('Acces Helper Module', () => { expect(result.selectedAgency).to.equal(fixtures.agencies.subAccountancy.id); }); }); + context('isMicrosoftSafeLinksRequest', () => { + it('early-returns if request is from Microsoft Safe Links', async () => { + const resFake = sinon.fake.returns(true); + const nextFake = sinon.fake.returns(true); + const requestFake = { + headers: { + 'user-agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/117.0.0.0 Safari/537.36 Edg/117.0.2045.47 OneOutlook/1.2023.927.100', + 'x-native-host': 'OneOutlook/1.2023.927.100', + }, + }; + await isMicrosoftSafeLinksRequest(requestFake, resFake, nextFake); + expect(resFake.json.calledOnceWith({ message: 'Success' })).to.equal(true); + expect(nextFake.notCalled).to.equal(true); + }); + it('proceeds normally if request is not from Microsoft Safe Links', async () => { + const resFake = sinon.fake.returns(true); + const nextFake = sinon.fake.returns(true); + const requestFake = { + headers: { + 'user-agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/117.0.0.0 Safari/537.36 Edg/117.0.2045.47 NotOutlook/1.2023.927.100', + 'x-native-host': 'NotOutlook/1.2023.927.100', + }, + }; + await isMicrosoftSafeLinksRequest(requestFake, resFake, nextFake); + expect(resFake.json.notCalled).to.equal(true); + expect(nextFake.calledOnce).to.equal(true); + }); + }); }); diff --git a/packages/server/src/lib/access-helpers.js b/packages/server/src/lib/access-helpers.js index 0006acfa1..94d802e3c 100755 --- a/packages/server/src/lib/access-helpers.js +++ b/packages/server/src/lib/access-helpers.js @@ -124,6 +124,17 @@ async function requireUSDRSuperAdminUser(req, res, next) { }); } +async function isMicrosoftSafeLinksRequest(req, res, next) { + const userAgent = req.headers['user-agent'] || ''; + const nativeHost = req.headers['x-native-host'] || ''; + if (userAgent.toLowerCase().includes('oneoutlook') || nativeHost.toLowerCase().includes('oneoutlook')) { + res.json({ message: 'Success' }); + return; + } + + next(); +} + module.exports = { - requireAdminUser, requireUser, isAuthorizedForAgency, isUserAuthorized, isUSDRSuperAdmin, requireUSDRSuperAdminUser, getAdminAuthInfo, + requireAdminUser, requireUser, isAuthorizedForAgency, isUserAuthorized, isUSDRSuperAdmin, requireUSDRSuperAdminUser, getAdminAuthInfo, isMicrosoftSafeLinksRequest, }; diff --git a/packages/server/src/routes/sessions.js b/packages/server/src/routes/sessions.js index 7217e1ce8..882331c14 100755 --- a/packages/server/src/routes/sessions.js +++ b/packages/server/src/routes/sessions.js @@ -3,6 +3,7 @@ const _ = require('lodash-checkit'); const path = require('path'); const { sendPassCode } = require('../lib/email'); const { validatePostLoginRedirectPath } = require('../lib/redirect_validation'); +const { isMicrosoftSafeLinksRequest } = require('../lib/access-helpers'); const router = express.Router({ mergeParams: true }); const { @@ -14,22 +15,14 @@ const { } = require('../db'); const { isUSDRSuperAdmin } = require('../lib/access-helpers'); -// NOTE(mbroussard): previously we allowed 2 uses to accommodate automated email systems that prefetch -// links. Now, we send login links through a clientside redirect instead so this should not be necessary. -// This has been updated to accommodate Microsoft Safe Links which will prefetch the link twice. +// Increasing the max-uses to ensure users are able to log-in even if their email client/security provider has clicked on the link already. +// Specifically the issue was identified with Microsoft Safe Links, which clicks on the link to check if it is safe. const MAX_ACCESS_TOKEN_USES = 4; // the validation URL is sent in the authentication email: // http://localhost:8080/api/sessions/?passcode=97fa7091-77ae-4905-b62e-97a7b4699abd // -router.get('/', async (req, res) => { - const userAgent = req.headers['user-agent'] || ''; - const nativeHost = req.headers['x-native-host'] || ''; - if (userAgent.toLowerCase().includes('oneoutlook') || nativeHost.toLowerCase().includes('oneoutlook')) { - res.json({ message: 'Success' }); - return; - } - +router.get('/', isMicrosoftSafeLinksRequest, async (req, res) => { const { passcode } = req.query; if (passcode) { res.sendFile(path.join(__dirname, '../static/login_redirect.html'));