Allow Vercel preview PR URLs for auth redirects on staging

Seneca-CDOT · Apr 9, 2021 · f8aee7b · f8aee7b
1 parent a88e462
commit f8aee7b
Show file tree

Hide file tree

Showing 6 changed files with 162 additions and 80 deletions.
diff --git a/config/env.staging b/config/env.staging
@@ -82,7 +82,7 @@ [email protected]
 # Origins of web apps that we'll allow for redirects. See src/api/auth/test
 # In addition to the staging front-end, we also allow Vercel's various domains
 # to interact with the backend services.
-ALLOWED_APP_ORIGINS=https://dev.telescope.cdot.systems https://telescope-git-master-humphd.vercel.app https://telescope-humphd.vercel.app https://telescope-dusky.now.sh
+ALLOWED_APP_ORIGINS=https://dev.telescope.cdot.systems https://telescope-git-master-humphd.vercel.app https://telescope-humphd.vercel.app https://telescope-dusky.now.sh https://*-humphd.vercel.app
 
 # The URI of the auth server
 JWT_ISSUER=https://dev.api.telescope.cdot.systems/v1/auth

diff --git a/src/api/auth/package.json b/src/api/auth/package.json
@@ -20,6 +20,7 @@
     "express-session": "^1.17.1",
     "http-errors": "^1.8.0",
     "jsonwebtoken": "^8.5.1",
+    "minimatch": "^3.0.4",
     "node-fetch": "^2.6.1",
     "passport": "^0.4.1",
     "passport-saml": "^2.1.0"

diff --git a/src/api/auth/src/middleware.js b/src/api/auth/src/middleware.js
@@ -0,0 +1,84 @@
+const { logger, createError } = require('@senecacdot/satellite');
+const { celebrate, Segments, Joi } = require('celebrate');
+
+const { matchOrigin } = require('./util');
+
+// Space-separated list of App origins that we know about and will allow
+// to be used as part of login redirects. You only need to specify
+// the origin (scheme://domain:port), for each of these vs. the full URL.
+// ALLOWED_APP_DOMAINS="http://app.com http://localhost:3000"
+const { ALLOWED_APP_ORIGINS } = process.env;
+if (!(ALLOWED_APP_ORIGINS && ALLOWED_APP_ORIGINS.length)) {
+  throw new Error('Missing ALLOWED_APP_ORIGINS env variable');
+}
+let allowedOrigins;
+try {
+  allowedOrigins = ALLOWED_APP_ORIGINS.trim()
+    .split(/ +/)
+    .map((uri) => new URL(uri).origin);
+} catch (err) {
+  throw new Error(`Invalid URI in ALLOWED_APP_ORIGINS: ${err.message}`);
+}
+logger.info({ allowedOrigins }, 'Accepting Login/Logout for accepted origins');
+
+// Middleware to make sure the redirect_uri we get on the query string
+// is for an origin that was previously registered with us (e.g., it's allowed).
+// We want to avoid redirecting users to apps we don't know about. We support
+// including a wildcard character '*' to allow origins with variable segments,
+// for example: https://telescope-pzgueymdv-humphd.vercel.app/ -> https://*-humphd.vercel.app/
+module.exports.validateRedirectUriOrigin = function validateRedirectUriOrigin() {
+  return (req, res, next) => {
+    const redirectUri = req.query.redirect_uri;
+    try {
+      const redirectOrigin = new URL(redirectUri).origin;
+      if (!matchOrigin(redirectOrigin, allowedOrigins)) {
+        logger.warn(
+          `Invalid redirect_uri passed to /login: ${redirectUri}, [${allowedOrigins.join(', ')}]`
+        );
+        next(createError(401, `redirect_uri not allowed: ${redirectUri}`));
+      } else {
+        // Origin is allowed, let this request continue
+        next();
+      }
+    } catch (err) {
+      next(err);
+    }
+  };
+};
+
+// Middleware to validate the presence and format of the redirect_uri and
+// state values on the query string. The redirect_uri must be a valid
+// http:// or https:// URI, and state is optional. NOTE: we validate the
+// origin of the redirect_uri itself in another middleware.
+module.exports.validateRedirectAndStateParams = function validateRedirectAndStateParams() {
+  return celebrate({
+    [Segments.QUERY]: Joi.object().keys({
+      redirect_uri: Joi.string()
+        .uri({
+          scheme: [/https?/],
+        })
+        .required(),
+      // state is optional
+      state: Joi.string(),
+    }),
+  });
+};
+
+// Middleware to capture authorization details passed on the query string
+// to the session.  We use the object name passed to use (login or logout).
+module.exports.captureAuthDetailsOnSession = function captureAuthDetailsOnSession() {
+  return (req, res, next) => {
+    // We'll always have a redirect_uri
+    req.session.authDetails = {
+      redirectUri: req.query.redirect_uri,
+    };
+
+    // Add state if present (optional)
+    if (req.query.state) {
+      req.session.authDetails.state = req.query.state;
+    }
+
+    logger.debug({ authDetails: req.session.authDetails }, 'adding session details');
+    next();
+  };
+};
diff --git a/src/api/auth/src/routes.js b/src/api/auth/src/routes.js
@@ -1,91 +1,18 @@
 const { Router, logger } = require('@senecacdot/satellite');
 const passport = require('passport');
-const { celebrate, Segments, Joi, errors } = require('celebrate');
+const { errors } = require('celebrate');
 const createError = require('http-errors');
 
 const { createToken } = require('./token');
 const { samlMetadata } = require('./authentication');
-
-// Space-separated list of App origins that we know about and will allow
-// to be used as part of login redirects. You only need to specify
-// the origin (scheme://domain:port), for each of these vs. the full URL.
-// ALLOWED_APP_DOMAINS="http://app.com http://localhost:3000"
-const { ALLOWED_APP_ORIGINS } = process.env;
-if (!(ALLOWED_APP_ORIGINS && ALLOWED_APP_ORIGINS.length)) {
-  throw new Error('Missing ALLOWED_APP_ORIGINS env variable');
-}
-let allowedOrigins;
-try {
-  allowedOrigins = ALLOWED_APP_ORIGINS.trim()
-    .split(/ +/)
-    .map((uri) => new URL(uri).origin);
-} catch (err) {
-  throw new Error(`Invalid URI in ALLOWED_APP_ORIGINS: ${err.message}`);
-}
-logger.info({ allowedOrigins }, 'Accepting Login/Logout for accepted origins');
+const {
+  validateRedirectAndStateParams,
+  validateRedirectUriOrigin,
+  captureAuthDetailsOnSession,
+} = require('./middleware');
 
 const router = Router();
 
-// Middleware to validate the presence and format of the redirect_uri and
-// state values on the query string. The redirect_uri must be a valid
-// http:// or https:// URI, and state is optional. NOTE: we validate the
-// origin of the redirect_uri itself in another middleware.
-function validateRedirectAndStateParams() {
-  return celebrate({
-    [Segments.QUERY]: Joi.object().keys({
-      redirect_uri: Joi.string()
-        .uri({
-          scheme: [/https?/],
-        })
-        .required(),
-      // state is optional
-      state: Joi.string(),
-    }),
-  });
-}
-
-// Middleware to make sure the redirect_uri we get on the query string
-// is for an origin that was previously registered with us (e.g., it's allowed).
-// We want to avoid redirecting users to apps we don't know about.
-function validateRedirectUriOrigin() {
-  return (req, res, next) => {
-    const redirectUri = req.query.redirect_uri;
-    try {
-      const redirectOrigin = new URL(redirectUri).origin;
-      if (!allowedOrigins.includes(redirectOrigin)) {
-        logger.warn(
-          `Invalid redirect_uri passed to /login: ${redirectUri}, [${allowedOrigins.join(', ')}]`
-        );
-        next(createError(401, `redirect_uri not allowed`));
-      } else {
-        // Origin is allowed, let this request continue
-        next();
-      }
-    } catch (err) {
-      next(err);
-    }
-  };
-}
-
-// Middleware to capture authorization details passed on the query string
-// to the session.  We use the object name passed to use (login or logout).
-function captureAuthDetailsOnSession() {
-  return (req, res, next) => {
-    // We'll always have a redirect_uri
-    req.session.authDetails = {
-      redirectUri: req.query.redirect_uri,
-    };
-
-    // Add state if present (optional)
-    if (req.query.state) {
-      req.session.authDetails.state = req.query.state;
-    }
-
-    logger.debug({ authDetails: req.session.authDetails }, 'adding session details');
-    next();
-  };
-}
-
 /**
  * /login allows users to authenticate with the external SAML SSO provider.
  * The caller needs to provide a request_uri query param to indicate where

diff --git a/src/api/auth/src/util.js b/src/api/auth/src/util.js
@@ -0,0 +1,14 @@
+const minimatch = require('minimatch');
+
+// Check to see if an origin matches the list of allowed origins we have.
+// If any of the allowed origins includes a '*' wildcard, we do a fuzzy match,
+// otherwise we do a full match
+module.exports.matchOrigin = (origin, allowedOrigins) =>
+  allowedOrigins.some((allowedOrigin) => {
+    // If there's a '*' character, try a fuzzy match
+    if (allowedOrigin.includes('*')) {
+      return minimatch(origin, allowedOrigin);
+    }
+    // Otherwise, do a full comparison
+    return origin === allowedOrigin;
+  });
diff --git a/src/api/auth/test/util.test.js b/src/api/auth/test/util.test.js
@@ -0,0 +1,56 @@
+const { matchOrigin } = require('../src/util');
+
+describe('matchOrigin()', () => {
+  it('should match an exact origin', () => {
+    const allowedOrigins = ['http://localhost:8000'];
+    expect(matchOrigin('http://localhost:8000', allowedOrigins)).toBe(true);
+  });
+
+  it('should match an origin with a * wildcard', () => {
+    const allowedOrigins = ['http://*.localhost'];
+    expect(matchOrigin('http://dev.localhost', allowedOrigins)).toBe(true);
+  });
+
+  it('should reject an exact origin that is not allowed', () => {
+    const allowedOrigins = ['http://localhost:9000'];
+    expect(matchOrigin('http://localhost:1234', allowedOrigins)).toBe(false);
+  });
+
+  it('should reject a fuzzy matched origin that is not allowed', () => {
+    const allowedOrigins = ['http://*.localhost'];
+    expect(matchOrigin('https://google.com', allowedOrigins)).toBe(false);
+  });
+
+  it('should support the Vercel preview PR case', () => {
+    const allowedOrigins = ['https://*-humphd.vercel.app/', 'https://dev.telescope.cdot.systems'];
+    expect(matchOrigin('https://telescope-pzgueymdv-humphd.vercel.app/', allowedOrigins)).toBe(
+      true
+    );
+  });
+
+  it('should support the Telescope cases', () => {
+    const allowedOrigins = [
+      'https://*-humphd.vercel.app/',
+      'https://dev.telescope.cdot.systems',
+      'https://telescope.cdot.systems',
+    ];
+    expect(matchOrigin('https://dev.telescope.cdot.systems', allowedOrigins)).toBe(true);
+    expect(matchOrigin('https://telescope.cdot.systems', allowedOrigins)).toBe(true);
+  });
+
+  it('should fail similar cases to our Telescope cases', () => {
+    const allowedOrigins = [
+      'https://*-humphd.vercel.app/',
+      'https://dev.telescope.cdot.systems',
+      'https://telescope.cdot.systems',
+    ];
+    // No https
+    expect(matchOrigin('http://dev.telescope.cdot.systems', allowedOrigins)).toBe(false);
+    // Different sub-domain
+    expect(matchOrigin('https://api.telescope.cdot.systems', allowedOrigins)).toBe(false);
+    // Different Vercel domain
+    expect(
+      matchOrigin('https://telescope-pzgueymdv-someone-else.vercel.app/', allowedOrigins)
+    ).toBe(false);
+  });
+});