diff --git a/server/Server.js b/server/Server.js index ef0888927..c1657ff1e 100644 --- a/server/Server.js +++ b/server/Server.js @@ -14,6 +14,7 @@ const { version } = require('../package.json') const is = require('./libs/requestIp/isJs') const fileUtils = require('./utils/fileUtils') const { toNumber } = require('./utils/index') +const { getRequestOrigin } = require('./utils/requestUtils') const Logger = require('./Logger') const Auth = require('./Auth') @@ -288,10 +289,9 @@ class Server { // if RouterBasePath is set, modify all requests to include the base path app.use((req, res, next) => { const urlStartsWithRouterBasePath = req.url.startsWith(global.RouterBasePath) - const host = req.get('host') - const protocol = req.secure || req.get('x-forwarded-proto') === 'https' ? 'https' : 'http' + const { origin } = getRequestOrigin(req) const prefix = urlStartsWithRouterBasePath ? global.RouterBasePath : '' - req.originalHostPrefix = `${protocol}://${host}${prefix}` + req.originalHostPrefix = `${origin}${prefix}` if (!urlStartsWithRouterBasePath) { req.url = `${global.RouterBasePath}${req.url}` } diff --git a/server/auth/TokenManager.js b/server/auth/TokenManager.js index 5933209c7..0c59ae7a1 100644 --- a/server/auth/TokenManager.js +++ b/server/auth/TokenManager.js @@ -6,6 +6,7 @@ const Logger = require('../Logger') const requestIp = require('../libs/requestIp') const jwt = require('../libs/jsonwebtoken') +const { isRequestSecure } = require('../utils/requestUtils') class TokenManager { /** @type {string} JWT secret key */ @@ -59,7 +60,7 @@ class TokenManager { setRefreshTokenCookie(req, res, refreshToken) { res.cookie('refresh_token', refreshToken, { httpOnly: true, - secure: req.secure || req.get('x-forwarded-proto') === 'https', + secure: isRequestSecure(req), sameSite: 'lax', maxAge: this.RefreshTokenExpiry * 1000, path: '/' diff --git a/server/utils/requestUtils.js b/server/utils/requestUtils.js new file mode 100644 index 000000000..f2672d977 --- /dev/null +++ b/server/utils/requestUtils.js @@ -0,0 +1,34 @@ +/** + * Whether the request was made over HTTPS. + * Uses Express `req.secure` and a strict `x-forwarded-proto === 'https'` check. + * + * @param {import('express').Request} req + * @returns {boolean} + */ +function isRequestSecure(req) { + return req.secure || req.get('x-forwarded-proto') === 'https' +} + +/** + * @param {import('express').Request} req + * @returns {'https' | 'http'} + */ +function getRequestProtocol(req) { + return isRequestSecure(req) ? 'https' : 'http' +} + +/** + * @param {import('express').Request} req + * @returns {{ protocol: 'https' | 'http', host: string, origin: string }} + */ +function getRequestOrigin(req) { + const protocol = getRequestProtocol(req) + const host = req.get('host') + return { protocol, host, origin: `${protocol}://${host}` } +} + +module.exports = { + isRequestSecure, + getRequestProtocol, + getRequestOrigin +} diff --git a/test/server/utils/requestUtils.test.js b/test/server/utils/requestUtils.test.js new file mode 100644 index 000000000..ee01af5a0 --- /dev/null +++ b/test/server/utils/requestUtils.test.js @@ -0,0 +1,40 @@ +const { expect } = require('chai') + +const { isRequestSecure, getRequestProtocol, getRequestOrigin } = require('../../../server/utils/requestUtils') + +function mockReq({ secure = false, host = 'books.example.com', xForwardedProto = null } = {}) { + return { + secure, + get(header) { + if (header === 'host') return host + if (header === 'x-forwarded-proto') return xForwardedProto + return null + } + } +} + +describe('requestUtils', () => { + it('isRequestSecure uses req.secure', () => { + expect(isRequestSecure(mockReq({ secure: true }))).to.equal(true) + expect(isRequestSecure(mockReq({ secure: false }))).to.equal(false) + }) + + it('isRequestSecure uses strict x-forwarded-proto check', () => { + expect(isRequestSecure(mockReq({ xForwardedProto: 'https' }))).to.equal(true) + expect(isRequestSecure(mockReq({ xForwardedProto: 'http' }))).to.equal(false) + expect(isRequestSecure(mockReq({ xForwardedProto: 'http, https' }))).to.equal(false) + }) + + it('getRequestProtocol returns https or http', () => { + expect(getRequestProtocol(mockReq({ secure: true }))).to.equal('https') + expect(getRequestProtocol(mockReq())).to.equal('http') + }) + + it('getRequestOrigin builds origin from protocol and host', () => { + expect(getRequestOrigin(mockReq({ secure: true }))).to.deep.equal({ + protocol: 'https', + host: 'books.example.com', + origin: 'https://books.example.com' + }) + }) +})