diff --git a/server/Auth.js b/server/Auth.js index 9c0e2fdb..a01d3a3c 100644 --- a/server/Auth.js +++ b/server/Auth.js @@ -213,6 +213,7 @@ class Auth { * @param {Request} req * @param {Response} res * @param {string} authMethod - The authentication method, default is 'local'. + * @returns {Object|null} - Returns error object if validation fails, null if successful */ paramsToCookies(req, res, authMethod = 'local') { const TWO_MINUTES = 120000 // 2 minutes in milliseconds @@ -227,13 +228,23 @@ class Auth { // Validate and store the callback URL if (!callback) { - return res.status(400).send({ message: 'No callback parameter' }) + res.status(400).send({ message: 'No callback parameter' }) + return { error: 'No callback parameter' } } + + // Security: Validate callback URL is same-origin only + if (!this.oidcAuthStrategy.isValidWebCallbackUrl(callback, req)) { + Logger.warn(`[Auth] Rejected invalid callback URL: ${callback}`) + res.status(400).send({ message: 'Invalid callback URL - must be same-origin' }) + return { error: 'Invalid callback URL - must be same-origin' } + } + res.cookie('auth_cb', callback, { maxAge: TWO_MINUTES, httpOnly: true }) } // Store the authentication method for long res.cookie('auth_method', authMethod, { maxAge: 1000 * 60 * 60 * 24 * 365 * 10, httpOnly: true }) + return null } /** @@ -254,7 +265,6 @@ class Auth { res.json(userResponse) } else { // UI request -> check if we have a callback url - // TODO: do we want to somehow limit the values for auth_cb? if (req.cookies.auth_cb) { let stateQuery = req.cookies.auth_state ? `&state=${req.cookies.auth_state}` : '' // UI request -> redirect to auth_cb url and send the jwt token as parameter @@ -350,7 +360,11 @@ class Auth { return res.status(authorizationUrlResponse.status).send(authorizationUrlResponse.error) } - this.paramsToCookies(req, res, authorizationUrlResponse.isMobileFlow ? 'openid-mobile' : 'openid') + // Check if paramsToCookies sent a response (e.g., due to invalid callback URL) + const cookieResult = this.paramsToCookies(req, res, authorizationUrlResponse.isMobileFlow ? 'openid-mobile' : 'openid') + if (cookieResult && cookieResult.error) { + return // Response already sent by paramsToCookies + } res.redirect(authorizationUrlResponse.authorizationUrl) }) diff --git a/server/Server.js b/server/Server.js index a384b93c..457aa61a 100644 --- a/server/Server.js +++ b/server/Server.js @@ -229,6 +229,10 @@ class Server { res.setHeader('Content-Security-Policy', "frame-ancestors 'self'") } + // Security: Prevent referrer leakage to protect against token exposure + // Using 'no-referrer' to completely prevent token leakage in referer headers + res.setHeader('Referrer-Policy', 'no-referrer') + /** * @temporary * This is necessary for the ebook & cover API endpoint in the mobile apps diff --git a/server/auth/OidcAuthStrategy.js b/server/auth/OidcAuthStrategy.js index 9a515c1a..cc998d4b 100644 --- a/server/auth/OidcAuthStrategy.js +++ b/server/auth/OidcAuthStrategy.js @@ -483,6 +483,49 @@ class OidcAuthStrategy { res.status(500).send('Internal Server Error') } } + + /** + * Validates if a callback URL is safe for redirect (same-origin only) + * @param {string} callbackUrl - The callback URL to validate + * @param {Request} req - Express request object to get current host + * @returns {boolean} - True if the URL is safe (same-origin), false otherwise + */ + isValidWebCallbackUrl(callbackUrl, req) { + if (!callbackUrl) return false + + try { + // Handle relative URLs - these are always safe if they start with router base path + if (callbackUrl.startsWith('/')) { + // Only allow relative paths that start with the router base path + if (callbackUrl.startsWith(global.RouterBasePath + '/')) { + return true + } + Logger.warn(`[OidcAuth] Rejected callback URL outside router base path: ${callbackUrl}`) + return false + } + + // For absolute URLs, ensure they point to the same origin + const callbackUrlObj = new URL(callbackUrl) + const currentProtocol = req.secure || req.get('x-forwarded-proto') === 'https' ? 'https' : 'http' + const currentHost = req.get('host') + + // Check if protocol and host match exactly + if (callbackUrlObj.protocol === currentProtocol + ':' && callbackUrlObj.host === currentHost) { + // Additional check: ensure path starts with router base path + if (callbackUrlObj.pathname.startsWith(global.RouterBasePath + '/')) { + return true + } + Logger.warn(`[OidcAuth] Rejected same-origin callback URL outside router base path: ${callbackUrl}`) + return false + } + + Logger.warn(`[OidcAuth] Rejected callback URL to different origin: ${callbackUrl} (expected ${currentProtocol}://${currentHost})`) + return false + } catch (error) { + Logger.error(`[OidcAuth] Invalid callback URL format: ${callbackUrl}`, error) + return false + } + } } module.exports = OidcAuthStrategy