From 014dc82baadd500049e503a3af34a54da8d8c5c0 Mon Sep 17 00:00:00 2001 From: Sean Sica <23294618+seansica@users.noreply.github.com> Date: Mon, 30 Dec 2024 13:39:00 -0500 Subject: [PATCH 1/8] feat: implement support for CORS allow-list - replace ENABLE_CORS_ANY_ORIGIN with CORS_ALLOWED_ORIGINS - add custom `domains` convict format to handle validating CORS_ALLOWED_ORIGINS - refactor CORS middleware implementation - See https://github.com/center-for-threat-informed-defense/attack-workbench-rest-api/issues/356 for details --- app/config/config.js | 59 ++++++++++++++++++++++++++++++++++++---- app/index.js | 64 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 105 insertions(+), 18 deletions(-) diff --git a/app/config/config.js b/app/config/config.js index dd90c0d6..ffef61a4 100644 --- a/app/config/config.js +++ b/app/config/config.js @@ -65,6 +65,55 @@ function arrayFormat(name) { convict.addFormat(arrayFormat('oidc-client')); convict.addFormat(arrayFormat('service-account')); +/** + * Validates a comma-separated string of domains or FQDNs. + * Allows the wildcard character `*` to indicate all origins. + * Supports the value `disable` to explicitly disable CORS. + * + * A valid FQDN must: + * - Contain only alphanumeric characters, hyphens, and dots. + * - Have at least one dot separating the domain levels. + * - End with a valid top-level domain (e.g., `.com`, `.org`). + * + * @param {string} value - The input string to validate. Can be a single domain, a wildcard (`*`), `disable`, or a comma-separated list of domains. + * @throws {Error} If any domain in the list is invalid. + * + * @example + * // Valid examples: + * validateDomains('*'); // No error + * validateDomains('example.com'); // No error + * validateDomains('example.com,api.example.com,sub.example.co.uk'); // No error + * validateDomains('disable'); // No error + * + * // Invalid examples: + * validateDomains('http://example.com'); // Throws error (protocol is not allowed) + * validateDomains('example_com'); // Throws error (underscore is not allowed) + * validateDomains('example'); // Throws error (missing top-level domain) + * validateDomains(',example.com'); // Throws error (empty domain before the comma) + */ +function validateDomains(value) { + if (value === '*' || value === 'disable') { + return; // '*' allows all origins; 'disable' explicitly disables CORS. + } + + const origins = value.split(',').map(origin => origin.trim()); + + // Regex to validate FQDNs + const fqdnRegex = /^(?!:\/\/)([a-zA-Z0-9-_]+\.)+[a-zA-Z]{2,}$/; + + origins.forEach(origin => { + if (!fqdnRegex.test(origin)) { + throw new Error(`Invalid domain: ${origin}`); + } + }); +} + +convict.addFormat({ + name: 'domains', + validate: validateDomains, + coerce: value => value.split(',').map(origin => origin.trim()), // Normalize the input +}); + function loadConfig() { const config = convict({ server: { @@ -74,11 +123,11 @@ function loadConfig() { default: 3000, env: 'PORT' }, - enableCorsAnyOrigin: { - doc: 'Access-Control-Allow-Origin will be set to the wildcard (*), allowing requests from any domain to access the REST API endpoints', - format: Boolean, - default: true, - env: 'ENABLE_CORS_ANY_ORIGIN' + corsAllowedOrigins: { + doc: 'Comma-separated list of origins allowed to access the REST API endpoints. Use * to allow any origin.', + format: 'domains', + default: '*', + env: 'CORS_ALLOWED_ORIGINS' } }, app: { diff --git a/app/index.js b/app/index.js index 556c9ffa..0f5c8fc7 100644 --- a/app/index.js +++ b/app/index.js @@ -16,6 +16,56 @@ function disableUpgradeInsecureRequests(app, helmet) { })); } +/** + * Configures and applies the CORS middleware to the Express application. + * + * - If `corsAllowedOrigins` is set to `disable`, CORS middleware is not applied, effectively disabling CORS. + * - If `corsAllowedOrigins` is `*`, it allows all origins. + * - Otherwise, it parses the comma-separated list of origins and uses them as the allowed origins. + * + * @param {import('express').Application} app - The Express application instance. + * @param {Object} config - The application configuration object. + * @param {Object} config.server - The server-specific configuration. + * @param {string} config.server.corsAllowedOrigins - The CORS allowed origins setting. + * @param {import('winston').Logger} logger - The logger instance for logging messages. + * + * @throws {Error} Throws an error if the configuration is invalid or missing required fields. + * + * @example + * // CORS is disabled + * const config = { server: { corsAllowedOrigins: 'disable' } }; + * setupCors(app, config, logger); // No CORS middleware applied + * + * @example + * // CORS allows all origins + * const config = { server: { corsAllowedOrigins: '*' } }; + * setupCors(app, config, logger); // CORS middleware with `origin: true` + * + * @example + * // CORS with specific origins + * const config = { server: { corsAllowedOrigins: 'example.com,api.example.com' } }; + * setupCors(app, config, logger); // CORS middleware with specific origins + */ +function setupCors(app, config, logger) { + const corsAllowedOrigins = config.server.corsAllowedOrigins; + + if (corsAllowedOrigins === 'disable') { + logger.info('CORS is disabled'); + return; // Skip setting up the CORS middleware + } + + logger.info('CORS is enabled'); + const cors = require('cors'); + const corsOptions = { + credentials: true, + origin: corsAllowedOrigins === '*' + ? true + : corsAllowedOrigins.split(',').map(origin => origin.trim()), + }; + + app.use(cors(corsOptions)); +} + /** * Creates a new instance of the express app. * @return The new express app @@ -37,19 +87,7 @@ exports.initializeApp = async function() { const requestId = require('./lib/requestId'); app.use(requestId); - // Allow CORS - if (config.server.enableCorsAnyOrigin) { - logger.info('CORS is enabled'); - const cors = require('cors'); - const corsOptions = { - credentials: true, - origin: true - }; - app.use(cors(corsOptions)); - } - else { - logger.info('CORS is not enabled'); - } + setupCors(app, config, logger); // Compress response bodies const compression = require('compression'); From dd5770f3a7446d68a9047d8fcb8b755d6eed6704 Mon Sep 17 00:00:00 2001 From: Sean Sica <23294618+seansica@users.noreply.github.com> Date: Mon, 30 Dec 2024 13:46:27 -0500 Subject: [PATCH 2/8] fix: remove unused function - `getWellKnownConfiguration` was throwing a no-unused-vars linting error --- app/tests/shared/keycloak.js | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/app/tests/shared/keycloak.js b/app/tests/shared/keycloak.js index a3555bdc..22232cce 100644 --- a/app/tests/shared/keycloak.js +++ b/app/tests/shared/keycloak.js @@ -119,19 +119,6 @@ async function getClientSecret(basePath, realmName, idOfClient, token) { } } -async function getWellKnownConfiguration(basePath, realmName, token) { - try { - const res = await request - .get(`${ basePath }/realms/${ realmName }/.well-known/openid-configuration`) - .set('Authorization', `bearer ${ token }`); - console.log(res); - } - catch (err) { - logger.error('Unable to get well known configuration'); - throw err; - } -} - async function createUser(basePath, realmName, userOptions, token) { const userData = { email: userOptions.email, From 634b562e1014b703f694c7e1aa9cbb143545b9e8 Mon Sep 17 00:00:00 2001 From: Sean Sica <23294618+seansica@users.noreply.github.com> Date: Mon, 30 Dec 2024 13:47:00 -0500 Subject: [PATCH 3/8] fix: apply formatting --- app/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/index.js b/app/index.js index 0f5c8fc7..40cdc5a3 100644 --- a/app/index.js +++ b/app/index.js @@ -58,9 +58,9 @@ function setupCors(app, config, logger) { const cors = require('cors'); const corsOptions = { credentials: true, - origin: corsAllowedOrigins === '*' - ? true - : corsAllowedOrigins.split(',').map(origin => origin.trim()), + origin: corsAllowedOrigins === '*' ? + true : + corsAllowedOrigins.split(',').map(origin => origin.trim()), }; app.use(cors(corsOptions)); From f1edbaf2d9673bfc6403b75cdf4eb1aac4cec1f7 Mon Sep 17 00:00:00 2001 From: Sean Sica <23294618+seansica@users.noreply.github.com> Date: Mon, 30 Dec 2024 13:55:28 -0500 Subject: [PATCH 4/8] docs: update environment variables table --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index e0efe502..4e112f1d 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ Note that any values set in a configuration file take precedence over values set | name | required | default | description | |--------------------------------------|----------|---------------|-----------------------------------------------------------| | **PORT** | no | `3000` | Port the HTTP server should listen on | -| **ENABLE_CORS_ANY_ORIGIN** | no | `true` | Allows requests from any domain to access the REST API endpoints | +| **CORS_ALLOWED_ORIGINS** | no | `*` | Configures CORS policy. Accepts a comma-separated list of allowed domains. (`*` allows all domains; `disable` disables CORS entirely.) | | **NODE_ENV** | no | `development` | Environment that the app is running in | | **DATABASE_URL** | yes | none | URL of the MongoDB server | | **AUTHN_MECHANISM** | no | `anonymous` | Mechanism to use for authenticating users | @@ -89,7 +89,7 @@ If the `JSON_CONFIG_PATH` environment variable is set, the app will also read co | name | type | corresponding environment variable | |-------------------------------------|----------|------------------------------------| | **server.port** | int | PORT | -| **server.enableCorsAnyOrigin** | boolean | ENABLE_CORS_ANY_ORIGIN | +| **server.corsAllowedOrigins** | boolean | CORS_ALLOWED_ORIGINS | | **app.env** | string | NODE_ENV | | **database.url** | string | DATABASE_URL | | **collectionIndex.defaultInterval** | int | DEFAULT_INTERVAL | From 5806f6c0d945e05223e38ab21323e79e91ec3f26 Mon Sep 17 00:00:00 2001 From: Sean Sica <23294618+seansica@users.noreply.github.com> Date: Mon, 30 Dec 2024 15:43:37 -0500 Subject: [PATCH 5/8] fix: handle cases where corsAllowedOrigins is a string or an array --- app/index.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/app/index.js b/app/index.js index 40cdc5a3..3648efb9 100644 --- a/app/index.js +++ b/app/index.js @@ -56,11 +56,22 @@ function setupCors(app, config, logger) { logger.info('CORS is enabled'); const cors = require('cors'); + + // Normalize corsAllowedOrigins to an array of origins + let origins; + if (typeof corsAllowedOrigins === 'string') { + origins = corsAllowedOrigins === '*' ? true : corsAllowedOrigins.split(',').map(origin => origin.trim()); + } else if (Array.isArray(corsAllowedOrigins)) { + origins = corsAllowedOrigins; // Already an array + } else { + throw new Error( + `Invalid value for server.corsAllowedOrigins: expected a string or array, but got ${typeof corsAllowedOrigins}` + ); + } + const corsOptions = { credentials: true, - origin: corsAllowedOrigins === '*' ? - true : - corsAllowedOrigins.split(',').map(origin => origin.trim()), + origin: origins, }; app.use(cors(corsOptions)); From fea9a62367ab534f1b8297029fdb1274dc88066b Mon Sep 17 00:00:00 2001 From: Sean Sica <23294618+seansica@users.noreply.github.com> Date: Mon, 30 Dec 2024 15:50:39 -0500 Subject: [PATCH 6/8] fix: handle cases where corsAllowedOrigins is a string or an array --- app/config/config.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/config/config.js b/app/config/config.js index ffef61a4..fd4112e7 100644 --- a/app/config/config.js +++ b/app/config/config.js @@ -96,7 +96,10 @@ function validateDomains(value) { return; // '*' allows all origins; 'disable' explicitly disables CORS. } - const origins = value.split(',').map(origin => origin.trim()); + // Normalize value to an array of origins + const origins = Array.isArray(value) + ? value + : value.split(',').map(origin => origin.trim()); // Regex to validate FQDNs const fqdnRegex = /^(?!:\/\/)([a-zA-Z0-9-_]+\.)+[a-zA-Z]{2,}$/; @@ -111,7 +114,12 @@ function validateDomains(value) { convict.addFormat({ name: 'domains', validate: validateDomains, - coerce: value => value.split(',').map(origin => origin.trim()), // Normalize the input + coerce: value => { + if (Array.isArray(value)) { + return value.map(origin => origin.trim()); + } + return value.split(',').map(origin => origin.trim()); // Normalize strings to arrays + }, }); function loadConfig() { From 86222fad4e72a89ebd4894c73cf4c8dfca9da742 Mon Sep 17 00:00:00 2001 From: Sean Sica <23294618+seansica@users.noreply.github.com> Date: Mon, 30 Dec 2024 15:54:16 -0500 Subject: [PATCH 7/8] feat: add logging to display CORS allowed domains --- app/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/index.js b/app/index.js index 3648efb9..62fa3a6a 100644 --- a/app/index.js +++ b/app/index.js @@ -54,7 +54,6 @@ function setupCors(app, config, logger) { return; // Skip setting up the CORS middleware } - logger.info('CORS is enabled'); const cors = require('cors'); // Normalize corsAllowedOrigins to an array of origins @@ -75,6 +74,8 @@ function setupCors(app, config, logger) { }; app.use(cors(corsOptions)); + + logger.info(`CORS is enabled for domains: ${origins}`) } /** From e154a894e5ce00eaa00847399420e9895f75eb1a Mon Sep 17 00:00:00 2001 From: Sean Sica <23294618+seansica@users.noreply.github.com> Date: Mon, 30 Dec 2024 16:36:39 -0500 Subject: [PATCH 8/8] feat: add support for protocol (http or https) in CORS origins --- app/config/config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/config/config.js b/app/config/config.js index fd4112e7..46af4f1b 100644 --- a/app/config/config.js +++ b/app/config/config.js @@ -101,11 +101,11 @@ function validateDomains(value) { ? value : value.split(',').map(origin => origin.trim()); - // Regex to validate FQDNs - const fqdnRegex = /^(?!:\/\/)([a-zA-Z0-9-_]+\.)+[a-zA-Z]{2,}$/; + // Regex to validate FQDNs with or without protocols + const originRegex = /^(https?:\/\/)?([a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}$/; origins.forEach(origin => { - if (!fqdnRegex.test(origin)) { + if (!originRegex.test(origin)) { throw new Error(`Invalid domain: ${origin}`); } });