From f8aee7b35d8891308ebc7ea15cee65aff5d93eb6 Mon Sep 17 00:00:00 2001 From: David Humphrey Date: Thu, 8 Apr 2021 12:10:19 -0400 Subject: [PATCH] Allow Vercel preview PR URLs for auth redirects on staging --- config/env.staging | 2 +- src/api/auth/package.json | 1 + src/api/auth/src/middleware.js | 84 +++++++++++++++++++++++++++++++++ src/api/auth/src/routes.js | 85 +++------------------------------- src/api/auth/src/util.js | 14 ++++++ src/api/auth/test/util.test.js | 56 ++++++++++++++++++++++ 6 files changed, 162 insertions(+), 80 deletions(-) create mode 100644 src/api/auth/src/middleware.js create mode 100644 src/api/auth/src/util.js create mode 100644 src/api/auth/test/util.test.js diff --git a/config/env.staging b/config/env.staging index 053fc07faf..92deb1134f 100644 --- a/config/env.staging +++ b/config/env.staging @@ -82,7 +82,7 @@ ADMINISTRATORS=user1@example.com # 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 index 665cfdcc4f..cb9d38dfce 100644 --- 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 new file mode 100644 index 0000000000..68bc891648 --- /dev/null +++ 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 index f3d6b719de..be68ff45b8 100644 --- 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 new file mode 100644 index 0000000000..6a7672d874 --- /dev/null +++ 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 new file mode 100644 index 0000000000..5542ac95d7 --- /dev/null +++ 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); + }); +});