fix(sso): stop sending OIDC logins to localhost; return users to the UI

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);
   };

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 };

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: [],
 };

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

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;

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:

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.
   #

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);