Skip to content

Commit

Permalink
fix: adds tests and moves function out
Browse files Browse the repository at this point in the history
  • Loading branch information
as1729 committed Jan 5, 2024
1 parent 1f5f37d commit b932112
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 13 deletions.
31 changes: 30 additions & 1 deletion packages/server/__tests__/lib/access-helpers.test.js
Original file line number Diff line number Diff line change
@@ -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');

Expand Down Expand Up @@ -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);
});
});
});
13 changes: 12 additions & 1 deletion packages/server/src/lib/access-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
15 changes: 4 additions & 11 deletions packages/server/src/routes/sessions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;

Check warning

Code scanning / CodeQL

Sensitive data read from GET request Medium

Route handler
for GET requests uses query parameter as sensitive data.
if (passcode) {
res.sendFile(path.join(__dirname, '../static/login_redirect.html'));
Expand Down

0 comments on commit b932112

Please sign in to comment.