diff --git a/app/routes/auth.routes.js b/app/routes/auth.routes.js index 148271d..21bc544 100644 --- a/app/routes/auth.routes.js +++ b/app/routes/auth.routes.js @@ -4,6 +4,7 @@ const debug = require('debug'); const authConfig = require('../../config/auth.config.js'); const featureConfig = require('../../config/feature.config.js'); const loginThrottle = require('../utils/login-throttle.js'); +const { trustedUiOrigin } = require('../utils/ui-origin.js'); const { isProviderReady, getReadyProvider, @@ -141,6 +142,16 @@ module.exports = (app, path) => { error: 'This provider uses a credential login; post to /login instead.', }); } + // Where to send the browser once the identity provider is done: the UI passes its + // own origin, because it is usually on a different port or host from the API. Only + // an origin on the API's own host is kept (see ui-origin.js), and only for this + // login; without one the redirect stays relative to the API, as before. + const returnTo = trustedUiOrigin(req.query.returnTo, req); + if (returnTo) { + req.session.ssoReturnTo = returnTo; + } else if (req.session) { + delete req.session.ssoReturnTo; + } return passport.authenticate(strategyName(req.params.providerId))(req, res, next); }, ); @@ -154,21 +165,28 @@ module.exports = (app, path) => { // error handler, which would surface a bare 500 with a stack trace to someone who just // failed to log in. Both are the same thing from the user's perspective, so both land // on the login page, and the reason is logged rather than returned. + // + // The UI origin recorded when the login started is read up front: logging in replaces + // the session (to prevent session fixation), which discards it. A SAML assertion is + // usually posted back cross-site, which a SameSite=Lax session cookie does not + // accompany, so a SAML login normally falls back to the API's own origin. const ssoCallback = (req, res, next) => { const { providerId } = req.params; + const uiOrigin = (req.session && req.session.ssoReturnTo) || ''; + if (req.session) delete req.session.ssoReturnTo; return passport.authenticate(strategyName(providerId), (err, user, info) => { if (err) { log('SSO callback for provider %s failed: %s', providerId, err.message); - return res.redirect('/login?error=true'); + return res.redirect(`${uiOrigin}/login?error=true`); } if (!user) { log('SSO callback for provider %s denied: %s', providerId, (info && info.message) || 'no user'); - return res.redirect('/login?error=true'); + return res.redirect(`${uiOrigin}/login?error=true`); } return req.logIn(user, (loginErr) => { if (loginErr) return next(loginErr); // Successful authentication, redirect home. - return res.redirect('/'); + return res.redirect(`${uiOrigin}/`); }); })(req, res, next); }; diff --git a/app/utils/ui-origin.js b/app/utils/ui-origin.js new file mode 100644 index 0000000..9966a6d --- /dev/null +++ b/app/utils/ui-origin.js @@ -0,0 +1,37 @@ +/* + * Which browser origins count as "the Angles UI". + * + * The UI and the API are usually served from the same host on different ports (the UI on + * 3001, the API on 3000) or behind one proxy. The rule is the one CORS already applies: + * an origin belongs to the UI when its host is the API's own host, with `localhost` and + * `127.0.0.1` treated as the same host. Ports and schemes may differ. + * + * The SSO login uses the same rule to decide where to send the browser back to after + * the identity provider (`returnTo`). Accepting any origin there would make the API an + * open redirect, so anything else is ignored. + */ +const isLocal = (host) => host === 'localhost' || host === '127.0.0.1'; + +const isSameHost = (originHostname, requestHostname) => originHostname === requestHostname + || (isLocal(originHostname) && isLocal(requestHostname)); + +/** + * The origin (`scheme://host[:port]`) of `value` when it is an http(s) URL on the same + * host as the request, otherwise undefined. Only the origin is kept: the UI decides its + * own landing page. + */ +const trustedUiOrigin = (value, req) => { + if (typeof value !== 'string' || value.length === 0 || value.length > 2048) return undefined; + let url; + try { + url = new URL(value); + } catch (e) { + return undefined; + } + if (url.protocol !== 'http:' && url.protocol !== 'https:') return undefined; + if (url.username || url.password) return undefined; + if (!isSameHost(url.hostname, req.hostname)) return undefined; + return url.origin; +}; + +module.exports = { isSameHost, trustedUiOrigin }; diff --git a/config/auth.config.js b/config/auth.config.js index e2cc60f..bd1e695 100644 --- a/config/auth.config.js +++ b/config/auth.config.js @@ -28,6 +28,23 @@ const resolveSessionSecret = () => { return crypto.randomBytes(32).toString('hex'); }; +// The public origin the API is reached on, without a trailing slash: where identity +// providers send users back to. ANGLES_BASE_URL sets it explicitly. Without it, the API's +// own public URL (ANGLES_API_BASE_URL, which deployments already set for the UI and the +// Swagger docs) is used, so a deployment that sets that one does not silently register +// http://localhost:3000 with its identity provider. Like Swagger, a value without a +// scheme takes the first of SWAGGER_SCHEMES. +const resolveBaseUrl = () => { + const explicit = (process.env.ANGLES_BASE_URL || '').trim(); + const apiUrl = (process.env.ANGLES_API_BASE_URL || '').trim(); + let baseUrl = explicit || apiUrl || 'http://localhost:3000'; + if (!/^https?:\/\//i.test(baseUrl)) { + const scheme = (process.env.SWAGGER_SCHEMES || 'http').split(',')[0].trim() || 'http'; + baseUrl = `${scheme}://${baseUrl}`; + } + return baseUrl.replace(/\/+$/, ''); +}; + // These are only the in-memory defaults used before the persisted settings load at // startup. All authentication configuration (which providers exist, whether they are // enabled, issuers, client ids, secrets, group mappings) is managed exclusively through @@ -38,12 +55,14 @@ const resolveSessionSecret = () => { // The two exceptions below are genuine deployment/infrastructure values, not user config: // - sessionSecret: the session signing key (see resolveSessionSecret above). // - baseUrl: the public origin, used to derive each provider's callback URL. It depends -// on where the app is hosted and must match what is registered with the IdP. +// on where the app is hosted and must match what is registered with the IdP (see +// resolveBaseUrl above). module.exports = { + resolveBaseUrl, localAuthEnabled: true, sessionSecret: resolveSessionSecret(), // Public origin of this Angles instance, without a trailing slash. - baseUrl: (process.env.ANGLES_BASE_URL || 'http://localhost:3000').replace(/\/+$/, ''), + baseUrl: resolveBaseUrl(), // Populated from the database at startup by auth-settings-service. providers: [], }; diff --git a/docs/authentication.md b/docs/authentication.md index 3db9da3..d23b47f 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -9,12 +9,20 @@ changing a provider needs no restart and no redeploy. | Variable | Required | Purpose | | --- | --- | --- | -| `ANGLES_BASE_URL` | For SSO | Public origin of this instance, e.g. `https://angles.example.com`. Every provider's callback URL is derived from it and must match what is registered with the identity provider. Defaults to `http://localhost:3000`. | +| `ANGLES_BASE_URL` | For SSO | Public URL of the API, e.g. `https://angles.example.com`. Every provider's callback URL is derived from it and must match what is registered with the identity provider. Defaults to `ANGLES_API_BASE_URL` (the API URL the UI and the Swagger docs already use), and to `http://localhost:3000` when neither is set. | | `SESSION_SECRET` | In production | Signs session cookies. Startup fails without it when `NODE_ENV=production`. Anyone who knows it can forge a session for any account, including an admin, so generate a random value per deployment. | | `ANGLES_ADMIN_PASSWORD` | First run | Seeds the initial local admin account. | | `SECURE_COOKIES` | Over HTTPS | Set to `true` to mark the session cookie `Secure`. | | `TRUST_PROXY` | Behind a proxy | Set to `true` so `SECURE_COOKIES` works behind a TLS-terminating reverse proxy. | +After the identity provider, users return to the UI they started from: the UI sends +its own origin with the login, and the API redirects back to it. The API only accepts an +origin on its own host (the same rule it applies to cross-origin requests, with +`localhost` and `127.0.0.1` treated as one host), so it cannot be used to redirect +elsewhere. A SAML assertion is usually posted back cross-site without the session +cookie, so a SAML login lands on the API's own origin instead; serve the UI and the API +from one origin (through a reverse proxy) if you use SAML. + Provider secrets - OIDC client secrets, SAML private keys, LDAP bind credentials - are stored in the database and are **write-only** through the API. They are never returned; the settings response reports only a `…Set` boolean. Saving a form with a blank secret diff --git a/server.js b/server.js index 7121717..c17accf 100644 --- a/server.js +++ b/server.js @@ -19,6 +19,7 @@ const authSettingsService = require('./app/utils/auth-settings-service.js'); const featureSettingsService = require('./app/utils/feature-settings-service.js'); const adminSeedService = require('./app/utils/admin-seed-service.js'); const { securityHeaders } = require('./app/utils/security-headers.js'); +const { isSameHost } = require('./app/utils/ui-origin.js'); // mongo db config const dbConfig = require('./config/database.config.js'); @@ -41,11 +42,7 @@ const corsOptionsDelegate = (req, callback) => { if (origin) { try { const originUrl = new URL(origin); - const isLocal = (host) => host === 'localhost' || host === '127.0.0.1'; - const isSameHost = originUrl.hostname === req.hostname - || (isLocal(originUrl.hostname) && isLocal(req.hostname)); - - if (isSameHost) { + if (isSameHost(originUrl.hostname, req.hostname)) { corsOptions.origin = true; } else { corsOptions.origin = false; diff --git a/setup/docker-compose.yml b/setup/docker-compose.yml index 6196996..68c44bd 100644 --- a/setup/docker-compose.yml +++ b/setup/docker-compose.yml @@ -26,7 +26,10 @@ services: ports: - 3000:3000 environment: - - ANGLES_API_BASE_URL=http://localhost:3000 + # Public URL of the API: used by the Swagger docs and, unless ANGLES_BASE_URL is + # set, for each SSO provider's redirect/callback URL. Set it (here and on the + # frontend) to the URL users actually reach the API on. + - ANGLES_API_BASE_URL=${ANGLES_API_BASE_URL:-http://localhost:3000} - ANGLES_API_BASE_PATH=/rest/api/v1.0 - SWAGGER_SCHEMES=http - PORT=3000 @@ -40,10 +43,10 @@ services: # videos, traces and HTML snapshots. Stored on the angles_attachments volume. - ANGLES_ATTACHMENT_MAX_SIZE_MB=${ANGLES_ATTACHMENT_MAX_SIZE_MB:-100} - MONGO_URL=mongodb://angleshq:%40nglesPassword@mongodb:27017/angles - # Public origin of this instance. Each SSO provider's redirect/callback URL is - # derived from it, and must match what is registered with the identity provider, - # so set this to the URL users actually reach Angles on. - - ANGLES_BASE_URL=${ANGLES_BASE_URL:-http://localhost:3000} + # Overrides the public URL each SSO provider's redirect/callback URL is derived + # from, which must match what is registered with the identity provider. Leave it + # empty to use ANGLES_API_BASE_URL. + - ANGLES_BASE_URL=${ANGLES_BASE_URL:-} # Signs session cookies. Required when NODE_ENV=production; anyone who knows it can # forge a session for any account, so generate a random value per deployment. - SESSION_SECRET=${SESSION_SECRET:-} @@ -64,7 +67,7 @@ services: ports: - 3001:3001 environment: - - ANGLES_API_BASE_URL=http://localhost:3000 + - ANGLES_API_BASE_URL=${ANGLES_API_BASE_URL:-http://localhost:3000} - ANGLES_API_BASE_PATH=/rest/api/v1.0 - PORT=3001 depends_on: diff --git a/setup/kubernetes/02-angles-config.yaml b/setup/kubernetes/02-angles-config.yaml index b673be1..31df8f3 100644 --- a/setup/kubernetes/02-angles-config.yaml +++ b/setup/kubernetes/02-angles-config.yaml @@ -22,13 +22,12 @@ data: # Initial admin account is seeded on first startup from this password. Change it before # deploying (ideally move it to a Secret referenced via envFrom). ANGLES_ADMIN_PASSWORD: "ChangeMe!1" - # Public origin users actually reach Angles on, without a trailing slash. + # Public URL users actually reach the API on, without a trailing slash. # - # Only needed for SSO, but required as soon as any provider is configured: each - # provider's redirect/callback URL is derived from it and must match what is registered - # with the identity provider. The port below is the backend's hostPort; behind an - # Ingress or LoadBalancer set this to that public URL instead - # (e.g. "https://angles.example.com"). + # Each SSO provider's redirect/callback URL is derived from it and must match what is + # registered with the identity provider. Without it, ANGLES_API_BASE_URL (set on the + # pod) is used. The port below is the backend's hostPort; behind an Ingress or + # LoadBalancer set this to that public URL instead (e.g. "https://angles.example.com"). ANGLES_BASE_URL: "http://127.0.0.1:3002" # Signs session cookies. Startup fails without it when NODE_ENV=production. # diff --git a/test/settings.tests.js b/test/settings.tests.js index 1e33c8a..afb35d9 100644 --- a/test/settings.tests.js +++ b/test/settings.tests.js @@ -367,13 +367,17 @@ describe('Auth Settings API Tests', () => { // redirect to the IdP, and follow its redirect back to the callback. The ID token is // genuinely signed by the test IdP, so signature, issuer, audience and nonce // validation all run for real. - const completeLogin = async (providerId, claims) => { + // + // `returnTo` is the UI origin the login is started with, as the UI passes it. + const completeLogin = async (providerId, claims, returnTo) => { idp.nextClaims = claims; // A fresh agent per login, so the PKCE verifier and state stored in the session are // the ones this flow created. const agent = request.agent(app); - const start = await agent.get(`${baseUrl}auth/sso/${providerId}`).expect(302); + const start = await agent.get(`${baseUrl}auth/sso/${providerId}`) + .query(returnTo === undefined ? {} : { returnTo }) + .expect(302); // Follow the redirect to the IdP itself - a different server, so it needs its own // request rather than the app agent. @@ -593,6 +597,79 @@ describe('Auth Settings API Tests', () => { }); }); + // The UI and the API are usually on different ports (or hosts behind one proxy), so + // after the identity provider the API sends the browser back to the UI's origin. The + // supertest requests reach the API as 127.0.0.1, so localhost counts as the same host. + describe('OIDC login flow: returning to the UI', () => { + beforeEach((done) => { + adminAgent + .put(`${baseUrl}settings/auth`) + // defaultRole is explicit: a PUT that omits it keeps the stored value, and an + // earlier test sets one, which would let the "denied" login in. + .send({ providers: [oidcProvider({ defaultRole: '' })] }) + .expect(200) + .end(done); + }); + + it('returns to the UI origin the login was started from', async () => { + const { agent, res } = await completeLogin('okta', { + sub: 'return-1', email: 'return.ui@example.com', groups: ['angles-users'], + }, 'http://localhost:3001'); + res.status.should.equal(302); + res.headers.location.should.equal('http://localhost:3001/'); + await agent.get(`${baseUrl}auth/me`).expect(200); + }); + + it('returns a denied login to the UI login page', async () => { + const { res } = await completeLogin('okta', { + sub: 'return-2', email: 'return.denied@example.com', groups: ['some-other-group'], + }, 'http://localhost:3001'); + res.headers.location.should.equal('http://localhost:3001/login?error=true'); + }); + + it('keeps only the origin of the URL it is given', async () => { + const { res } = await completeLogin('okta', { + sub: 'return-3', email: 'return.path@example.com', groups: ['angles-users'], + }, 'http://127.0.0.1:3001/some/page?x=1'); + res.headers.location.should.equal('http://127.0.0.1:3001/'); + }); + + it('ignores an origin on another host, so it cannot be used as an open redirect', async () => { + const { agent, res } = await completeLogin('okta', { + sub: 'return-4', email: 'return.foreign@example.com', groups: ['angles-users'], + }, 'https://evil.example.org'); + res.headers.location.should.equal('/'); + await agent.get(`${baseUrl}auth/me`).expect(200); + }); + + it('ignores values that are not http(s) origins', async () => { + // eslint-disable-next-line no-script-url + const values = ['javascript:alert(1)', '//evil.example.org', 'not a url', 'http://user:pw@localhost:3001']; + // eslint-disable-next-line no-restricted-syntax + for (const [index, value] of values.entries()) { + // eslint-disable-next-line no-await-in-loop + const { res } = await completeLogin('okta', { + sub: `return-5-${index}`, email: `return.bad${index}@example.com`, groups: ['angles-users'], + }, value); + res.headers.location.should.equal('/'); + } + }); + + it('forgets the UI origin once the login is over', async () => { + const agent = request.agent(app); + await agent.get(`${baseUrl}auth/sso/okta`).query({ returnTo: 'http://localhost:3001' }).expect(302); + // A second login from the same browser without one goes back to the API origin. + idp.nextClaims = { sub: 'return-6', email: 'return.forget@example.com', groups: ['angles-users'] }; + const start = await agent.get(`${baseUrl}auth/sso/okta`).expect(302); + const authorize = await request(idp.issuer) + .get(start.headers.location.replace(idp.issuer, '')) + .redirects(0) + .expect(302); + const res = await agent.get(authorize.headers.location.replace(/^https?:\/\/[^/]+/, '')).redirects(0); + res.headers.location.should.equal('/'); + }); + }); + describe('GET /auth/sso/:providerId (runtime enablement)', () => { it('returns 404 for an unknown provider', (done) => { request(app).get(`${baseUrl}auth/sso/does-not-exist`).expect(404, done); diff --git a/test/sso-urls.tests.js b/test/sso-urls.tests.js new file mode 100644 index 0000000..b6a6b7a --- /dev/null +++ b/test/sso-urls.tests.js @@ -0,0 +1,93 @@ +/** + * The URLs an SSO login depends on: the public URL identity providers send users back to + * (each provider's callback is derived from it), and which UI origins a login may return + * to afterwards. + */ +const should = require('should'); +const { resolveBaseUrl } = require('../config/auth.config.js'); +const { trustedUiOrigin, isSameHost } = require('../app/utils/ui-origin.js'); + +const ENV_KEYS = ['ANGLES_BASE_URL', 'ANGLES_API_BASE_URL', 'SWAGGER_SCHEMES']; + +describe('SSO URL Tests', () => { + describe('resolveBaseUrl', () => { + let saved; + + beforeEach(() => { + saved = Object.fromEntries(ENV_KEYS.map((key) => [key, process.env[key]])); + ENV_KEYS.forEach((key) => { delete process.env[key]; }); + }); + + afterEach(() => { + ENV_KEYS.forEach((key) => { + if (saved[key] === undefined) delete process.env[key]; + else process.env[key] = saved[key]; + }); + }); + + it('uses ANGLES_BASE_URL when it is set', () => { + process.env.ANGLES_BASE_URL = 'https://angles.example.com/'; + process.env.ANGLES_API_BASE_URL = 'https://api.example.com'; + resolveBaseUrl().should.equal('https://angles.example.com'); + }); + + it('falls back to the API URL rather than localhost', () => { + process.env.ANGLES_API_BASE_URL = 'https://angles-api.example.com'; + resolveBaseUrl().should.equal('https://angles-api.example.com'); + }); + + it('treats an empty ANGLES_BASE_URL as unset, as docker-compose passes it', () => { + process.env.ANGLES_BASE_URL = ''; + process.env.ANGLES_API_BASE_URL = 'https://angles-api.example.com'; + resolveBaseUrl().should.equal('https://angles-api.example.com'); + }); + + it('adds the Swagger scheme to an API URL without one', () => { + process.env.ANGLES_API_BASE_URL = 'angles.internal:3000'; + process.env.SWAGGER_SCHEMES = 'https, http'; + resolveBaseUrl().should.equal('https://angles.internal:3000'); + process.env.SWAGGER_SCHEMES = ''; + resolveBaseUrl().should.equal('http://angles.internal:3000'); + }); + + it('defaults to http://localhost:3000 when nothing is configured', () => { + resolveBaseUrl().should.equal('http://localhost:3000'); + }); + }); + + describe('trustedUiOrigin', () => { + const req = (hostname) => ({ hostname }); + + it('accepts an origin on the API host on any port and scheme', () => { + trustedUiOrigin('https://angles.example.com:8443', req('angles.example.com')) + .should.equal('https://angles.example.com:8443'); + trustedUiOrigin('http://angles.example.com', req('angles.example.com')) + .should.equal('http://angles.example.com'); + }); + + it('treats localhost and 127.0.0.1 as the same host', () => { + trustedUiOrigin('http://localhost:3001', req('127.0.0.1')).should.equal('http://localhost:3001'); + isSameHost('127.0.0.1', 'localhost').should.equal(true); + }); + + it('keeps only the origin', () => { + trustedUiOrigin('http://localhost:3001/a/b?c=d#e', req('localhost')).should.equal('http://localhost:3001'); + }); + + it('rejects other hosts, look-alikes and non-http schemes', () => { + should.not.exist(trustedUiOrigin('https://evil.example.org', req('angles.example.com'))); + should.not.exist(trustedUiOrigin('https://angles.example.com.evil.org', req('angles.example.com'))); + // eslint-disable-next-line no-script-url + should.not.exist(trustedUiOrigin('javascript:alert(1)', req('localhost'))); + should.not.exist(trustedUiOrigin('//angles.example.com', req('angles.example.com'))); + should.not.exist(trustedUiOrigin('http://user:pw@localhost:3001', req('localhost'))); + }); + + it('rejects anything that is not a single string', () => { + should.not.exist(trustedUiOrigin(undefined, req('localhost'))); + should.not.exist(trustedUiOrigin('', req('localhost'))); + should.not.exist(trustedUiOrigin(['http://localhost:3001'], req('localhost'))); + should.not.exist(trustedUiOrigin(`http://localhost/${'a'.repeat(3000)}`, req('localhost'))); + }); + }); +});