diff --git a/API.md b/API.md index 6814c41..f6cc00f 100644 --- a/API.md +++ b/API.md @@ -905,7 +905,9 @@ Returns the OIDC discovery document with endpoint URLs, supported scopes, and si **Query Parameters:** - `response_type` — Must be `code` - `client_id` — Registered OAuth client ID -- `redirect_uri` — Must exactly match a URI registered for the client +- `redirect_uri` — Must match a URI registered for the client, either exactly + or against a registered wildcard pattern (`*` = one hostname label, `**` = + any number of labels) - `scope` — Space-separated: `openid`, `profile`, `email` - `state` — Opaque value returned unchanged in the redirect - `code_challenge` — PKCE challenge (SHA-256 of code_verifier, base64url-encoded) diff --git a/docs/oauth.md b/docs/oauth.md index d57795a..88e6944 100644 --- a/docs/oauth.md +++ b/docs/oauth.md @@ -36,7 +36,11 @@ An OAuth client represents an app that authenticates against the SSO. Each has: - `client_id` (UUID) + `client_secret` (bcrypt-hashed; the **raw secret is shown once** when the client is created or rotated — save it immediately). - `name`, `description`, `created_by` (the admin uid that created it). -- `redirect_uris` — allowed callback URLs (must match exactly). +- `redirect_uris` — allowed callback URLs. Each entry matches exactly, or may + use `*` (one hostname label) / `**` (any number of labels) as a wildcard — + e.g. `https://*.example.com/__proxy_auth/callback` covers every host + theta42/proxy fronts under `example.com`, so you don't have to register + each proxied host's callback individually. - `scopes` — requested scopes (default `openid profile email groups`). - `allowed_groups` — restrict the client to members of specific SSO groups (empty = any valid user). diff --git a/nodejs/app.js b/nodejs/app.js index 317b661..a5ab98a 100755 --- a/nodejs/app.js +++ b/nodejs/app.js @@ -3,6 +3,7 @@ const path = require('path'); const ejs = require('ejs') const express = require('express'); +const compression = require('compression'); // Set up the express app. const app = express(); @@ -40,8 +41,15 @@ app.onListen.push(function(){ }); }); +// Gzip text responses (HTML/JS/CSS/JSON). The admin UI loads ~13 separate, +// uncompressed vendor JS/CSS files on every full page navigation (a +// traditional multi-page app, not an SPA) — this alone meaningfully cuts +// bytes-over-the-wire and perceived load time on a real network, where it +// matters far more than on localhost. +app.use(compression()); + // load the JSON parser middleware. Express will parse JSON into native objects -// for any request that has JSON in its content type. +// for any request that has JSON in its content type. app.use(express.json()); app.set('trust proxy', 1); @@ -50,8 +58,9 @@ app.set('views', path.join(__dirname, 'views')); app.set('view engine', 'ejs'); // Have express server static content( images, CSS, browser JS) from the public -// local folder. -app.use('/static', express.static(path.join(__dirname, 'public'))) +// local folder. maxAge is short since this is the app's own JS/CSS, which +// changes on every deploy and isn't cache-busted/fingerprinted. +app.use('/static', express.static(path.join(__dirname, 'public'), {maxAge: '1h'})) // Routes for front end content. app.use('/', require('./routes/index')); diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index 26dc23c..f0e61bc 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -14,6 +14,7 @@ "@simpleworkjs/conf": "^1.1.0", "bcrypt": "^6.0.0", "bootstrap": "^5.3.8", + "compression": "^1.8.1", "ejs": "^3.1.10", "express": "^5.2.1", "express-rate-limit": "^8.5.2", @@ -2340,6 +2341,60 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/compressible": { + "version": "2.0.18", + "resolved": "https://registry.npmjs.org/compressible/-/compressible-2.0.18.tgz", + "integrity": "sha512-AF3r7P5dWxL8MxyITRMlORQNaOA2IkAFaTr4k7BUumjPtRpGDTZpl0Pb1XCO6JeDCBdp126Cgs9sMxqSjgYyRg==", + "license": "MIT", + "dependencies": { + "mime-db": ">= 1.43.0 < 2" + }, + "engines": { + "node": ">= 0.6" + } + }, + "node_modules/compression": { + "version": "1.8.1", + "resolved": "https://registry.npmjs.org/compression/-/compression-1.8.1.tgz", + "integrity": "sha512-9mAqGPHLakhCLeNyxPkK4xVo746zQ/czLH1Ky+vkitMnWfWZps8r0qXuwhwizagCRttsL4lfG4pIOvaWLpAP0w==", + "license": "MIT", + "dependencies": { + "bytes": "3.1.2", + "compressible": "~2.0.18", + "debug": "2.6.9", + "negotiator": "~0.6.4", + "on-headers": "~1.1.0", + "safe-buffer": "5.2.1", + "vary": "~1.1.2" + }, + "engines": { + "node": ">= 0.8.0" + } + }, + "node_modules/compression/node_modules/debug": { + "version": "2.6.9", + "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", + "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", + "license": "MIT", + "dependencies": { + "ms": "2.0.0" + } + }, + "node_modules/compression/node_modules/ms": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", + "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==", + "license": "MIT" + }, + "node_modules/compression/node_modules/negotiator": { + "version": "0.6.4", + "resolved": "https://registry.npmjs.org/negotiator/-/negotiator-0.6.4.tgz", + "integrity": "sha512-myRT3DiWPHqho5PrJaIRyaMv2kgYf0mUVgBNOYMuCH5Ki1yEiQaf/ZJuQ62nvpc44wL5WDbTX7yGJi1Neevw8w==", + "license": "MIT", + "engines": { + "node": ">= 0.6" + } + }, "node_modules/concat-map": { "version": "0.0.1", "resolved": "https://registry.npmjs.org/concat-map/-/concat-map-0.0.1.tgz", @@ -4976,6 +5031,15 @@ "node": ">= 0.8" } }, + "node_modules/on-headers": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/on-headers/-/on-headers-1.1.0.tgz", + "integrity": "sha512-737ZY3yNnXy37FHkQxPzt4UZ2UWPWiCZWLvFZ4fu5cueciegX0zGPnrlY6bwRg4FdQOe9YU8MkmJwGhoMybl8A==", + "license": "MIT", + "engines": { + "node": ">= 0.8" + } + }, "node_modules/once": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", diff --git a/nodejs/package.json b/nodejs/package.json index 8c3fcd5..4fb287c 100755 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -26,6 +26,7 @@ "@simpleworkjs/conf": "^1.1.0", "bcrypt": "^6.0.0", "bootstrap": "^5.3.8", + "compression": "^1.8.1", "ejs": "^3.1.10", "express": "^5.2.1", "express-rate-limit": "^8.5.2", diff --git a/nodejs/routes/index.js b/nodejs/routes/index.js index e22dc0c..7ad0453 100755 --- a/nodejs/routes/index.js +++ b/nodejs/routes/index.js @@ -26,13 +26,17 @@ const frontEndModules = ['bootstrap', 'mustache', 'jquery', '@fortawesome', // Server front end modules // https://stackoverflow.com/a/55700773/3140931 +// Vendor libraries only change when package versions are bumped (a rebuild), +// so they're safe to cache aggressively; ETag/Last-Modified (on by default) +// still cover that rare case with a cheap 304 instead of a stale asset. frontEndModules.forEach(dep => { - router.use(`/static-modules/${dep}`, express.static(path.join(__dirname, `../node_modules/${dep}`))) + router.use(`/static-modules/${dep}`, express.static(path.join(__dirname, `../node_modules/${dep}`), {maxAge: '7d'})) }); // Have express server static content( images, CSS, browser JS) from the public -// local folder. -router.use('/static', express.static(path.join(__dirname, '../public'))) +// local folder. Shorter maxAge than /static-modules since this is the app's +// own JS/CSS, which changes on every deploy and isn't cache-busted/fingerprinted. +router.use('/static', express.static(path.join(__dirname, '../public'), {maxAge: '1h'})) // Public health endpoint for container/orchestration healthchecks. // Mounted at / (no auth) in app.js, so this is intentionally unauthenticated. diff --git a/nodejs/routes/oauth.js b/nodejs/routes/oauth.js index ec75cb3..2a0b8a1 100644 Binary files a/nodejs/routes/oauth.js and b/nodejs/routes/oauth.js differ diff --git a/nodejs/tests/redirect_uri.test.js b/nodejs/tests/redirect_uri.test.js new file mode 100644 index 0000000..43cfd68 --- /dev/null +++ b/nodejs/tests/redirect_uri.test.js @@ -0,0 +1,66 @@ +'use strict'; + +const { redirectUriAllowed } = require('../routes/oauth'); + +// Pure logic, no LDAP/Redis needed -- regression coverage for the bug where +// theta42/proxy's per-host SSO callback (a different URL per proxied host, +// e.g. https://site.example.com/__proxy_auth/callback) could never match a +// single OAuth client's redirect_uris list without registering every host's +// callback individually. `*`/`**` wildcard support lets one registered +// pattern (e.g. https://*.example.com/__proxy_auth/callback) cover a whole +// domain's worth of proxied hosts. +describe('redirectUriAllowed', () => { + test('exact match still works with no wildcard present', () => { + expect(redirectUriAllowed( + ['https://app.example.com/cb'], + 'https://app.example.com/cb' + )).toBe(true); + }); + + test('rejects a uri that is not registered', () => { + expect(redirectUriAllowed( + ['https://app.example.com/cb'], + 'https://app.example.com/cb2' + )).toBe(false); + }); + + test('* matches exactly one hostname label', () => { + expect(redirectUriAllowed( + ['https://*.example.com/__proxy_auth/callback'], + 'https://site.example.com/__proxy_auth/callback' + )).toBe(true); + }); + + test('* does not span multiple labels', () => { + expect(redirectUriAllowed( + ['https://*.example.com/__proxy_auth/callback'], + 'https://site.nl.example.com/__proxy_auth/callback' + )).toBe(false); + }); + + test('** spans multiple labels', () => { + expect(redirectUriAllowed( + ['https://**.example.com/__proxy_auth/callback'], + 'https://site.nl.example.com/__proxy_auth/callback' + )).toBe(true); + }); + + test('scheme mismatch is not allowed even with a wildcard', () => { + expect(redirectUriAllowed( + ['https://*.example.com/__proxy_auth/callback'], + 'http://site.example.com/__proxy_auth/callback' + )).toBe(false); + }); + + test('a wildcard pattern does not match an unrelated domain', () => { + expect(redirectUriAllowed( + ['https://**.example.com/__proxy_auth/callback'], + 'https://evil.com/__proxy_auth/callback' + )).toBe(false); + }); + + test('empty/missing patterns list rejects everything', () => { + expect(redirectUriAllowed([], 'https://app.example.com/cb')).toBe(false); + expect(redirectUriAllowed(undefined, 'https://app.example.com/cb')).toBe(false); + }); +}); diff --git a/nodejs/views/oauth_clients.ejs b/nodejs/views/oauth_clients.ejs index 78374b4..eeaefe6 100644 --- a/nodejs/views/oauth_clients.ejs +++ b/nodejs/views/oauth_clients.ejs @@ -22,6 +22,9 @@
+ + * matches one hostname label, ** matches any number of labels. +
@@ -271,6 +274,12 @@ + + * matches one hostname label and ** matches any + number of labels, e.g. https://*.example.com/__proxy_auth/callback + covers every host theta42/proxy fronts under example.com without registering + each one individually. +