Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Use express builtin 'trust proxy' #399

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ indent_size = 2
indent_style = space
insert_final_newline = true
trim_trailing_whitespace = true
quote_type = single
9 changes: 2 additions & 7 deletions src/app-account.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import express from 'express';
import errorMiddleware from './util/error-middleware.js';
import validateUser, { validateAuthHeader } from './util/validate-user.js';
import validateUser from './util/validate-user.js';
import {
bootstrap,
login,
Expand Down Expand Up @@ -50,12 +50,7 @@ app.post('/login', (req, res) => {
res.send({ status: 'error', reason: 'invalid-header' });
return;
} else {
if (validateAuthHeader(req)) {
tokenRes = login(headerVal);
} else {
res.send({ status: 'error', reason: 'proxy-not-trusted' });
return;
}
tokenRes = login(headerVal);
}
break;
}
Expand Down
1 change: 1 addition & 0 deletions src/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ process.on('unhandledRejection', (reason) => {

app.disable('x-powered-by');
app.use(cors());
app.set('trust proxy', config.trustedProxies);
app.use(
rateLimit({
windowMs: 60 * 1000,
Expand Down
2 changes: 1 addition & 1 deletion src/config-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { ServerOptions } from 'https';
export interface Config {
mode: 'test' | 'development';
loginMethod: 'password' | 'header';
trustedProxies: string[];
trustedProxies: string[] | number | boolean;
dataDir: string;
projectRoot: string;
port: number;
Expand Down
49 changes: 38 additions & 11 deletions src/load-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,32 @@ function parseJSON(path, allowMissing = false) {
return JSON.parse(text);
}

/**
* @param {string | undefined} envVar
* @param {string[] | number | boolean} defaultValue
* @returns {string[] | number | boolean}
*/
function parseTrustedProxiesEnvVar(envVar, defaultValue) {
if (!envVar) {
return defaultValue;
}

if (envVar.toLowerCase() === 'true' || envVar.toLowerCase() === 'false') {
return envVar.toLowerCase() === 'true';
}

if (typeof envVar === 'string' && !Number.isNaN(Number(envVar))) {
return Number(envVar);
}

if (envVar.includes(',')) {
return envVar.split(',').map((item) => item.trim());
}

console.warn(`Failed to parse Trusted Proxies value: ${envVar}`);
return defaultValue;
}

let userConfig;
if (process.env.ACTUAL_CONFIG_PATH) {
debug(
Expand All @@ -50,13 +76,7 @@ if (process.env.ACTUAL_CONFIG_PATH) {
let defaultConfig = {
loginMethod: 'password',
// assume local networks are trusted for header authentication
trustedProxies: [
'10.0.0.0/8',
'172.16.0.0/12',
'192.168.0.0/16',
'fc00::/7',
'::1/128',
],
trustedProxies: ['uniquelocal', 'loopback'],
port: 5006,
hostname: '::',
webRoot: path.join(
Expand Down Expand Up @@ -100,9 +120,10 @@ const finalConfig = {
loginMethod: process.env.ACTUAL_LOGIN_METHOD
? process.env.ACTUAL_LOGIN_METHOD.toLowerCase()
: config.loginMethod,
trustedProxies: process.env.ACTUAL_TRUSTED_PROXIES
? process.env.ACTUAL_TRUSTED_PROXIES.split(',').map((q) => q.trim())
: config.trustedProxies,
trustedProxies: parseTrustedProxiesEnvVar(
process.env.ACTUAL_TRUSTED_PROXIES,
config.trustedProxies,
),
port: +process.env.ACTUAL_PORT || +process.env.PORT || config.port,
hostname: process.env.ACTUAL_HOSTNAME || config.hostname,
serverFiles: process.env.ACTUAL_SERVER_FILES || config.serverFiles,
Expand Down Expand Up @@ -143,7 +164,13 @@ debug(`using server files directory ${finalConfig.serverFiles}`);
debug(`using user files directory ${finalConfig.userFiles}`);
debug(`using web root directory ${finalConfig.webRoot}`);
debug(`using login method ${finalConfig.loginMethod}`);
debug(`using trusted proxies ${finalConfig.trustedProxies.join(', ')}`);
debug(
`using trusted proxies ${
Array.isArray(finalConfig.trustedProxies)
? finalConfig.trustedProxies.join(', ')
: finalConfig.trustedProxies
}`,
);

if (finalConfig.https) {
debug(`using https key: ${'*'.repeat(finalConfig.https.key.length)}`);
Expand Down
26 changes: 0 additions & 26 deletions src/util/validate-user.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import { getSession } from '../account-db.js';
import config from '../load-config.js';
import proxyaddr from 'proxy-addr';
import ipaddr from 'ipaddr.js';

/**
* @param {import('express').Request} req
Expand All @@ -28,26 +25,3 @@ export default function validateUser(req, res) {

return session;
}

export function validateAuthHeader(req) {
if (config.trustedProxies.length == 0) {
return true;
}

let sender = proxyaddr(req, 'uniquelocal');
let sender_ip = ipaddr.process(sender);
const rangeList = {
allowed_ips: config.trustedProxies.map((q) => ipaddr.parseCIDR(q)),
};
/* eslint-disable @typescript-eslint/ban-ts-comment */
// @ts-ignore : there is an error in the ts definition for the function, but this is valid
var matched = ipaddr.subnetMatch(sender_ip, rangeList, 'fail');
/* eslint-enable @typescript-eslint/ban-ts-comment */
if (matched == 'allowed_ips') {
console.info(`Header Auth Login permitted from ${sender}`);
return true;
} else {
console.warn(`Header Auth Login attempted from ${sender}`);
return false;
}
}
6 changes: 6 additions & 0 deletions upcoming-release-notes/399.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Maintenance
authors: [djm2k]
---

Adjust the `trustedProxies` config option to work with [expressjs](https://expressjs.com/en/guide/behind-proxies.html).