ASTAD-28445 Open redirect issue na auth

package.json

@@ -9,6 +9,7 @@
     "build": "npx rollup -c --exports default",
     "prepublish": "npm run build",
     "lint": "eslint . --ext .js",
+    "lint:fix": "eslint . --ext .js --fix",
     "test": "cross-env NODE_ENV=test nyc --reporter=lcov --reporter=text mocha"
   },
   "publishConfig": {

readme.md

@@ -120,6 +120,10 @@ The login router & the SSO middleware use the same configuration.
 - **url**: *String* 
   Url where the user will be retrieved with after login has succeeded  
   (e.g.: https://api-gw-a.antwerpen.be/acpaas/shared-identity-data/v1)
+- **allowedDomains**: *String[]* 
+  List of domains allowed to redirect to after successful login  
+  (e.g.: ['antwerpen.be'])
+  (e.g.: ['antwerpen.be', 'digipolis.be', 'gate15.be'])
 
 
 ### API Store configuration

src/controller.js

@@ -11,6 +11,7 @@ import { getHost, logoutEncrypt, runHooks } from './helpers';
 import createDeleteSessionsHook from './hooks/deleteSessions';
 import createAssuranceLevelAndAuthMethodHook from './hooks/assuranceLevelAndAuthMethod';
 import createDetermineLoginTypeHook from './hooks/determineLoginType';
+import { isValidCallbackUrl } from './util/isValidCallbackUrl';
 
 const EXPIRY_MARGIN = 5 * 60 * 1000;
 export default function createController(config) {
@@ -26,6 +27,7 @@ export default function createController(config) {
     errorRedirect = '/',
     key: objectKey = 'user',
     logLevel = 'error',
+    allowedDomains,
   } = config;
 
   const logger = pino({
@@ -162,7 +164,7 @@ export default function createController(config) {
     const stateKey = uuid.v4();
     const url = createLoginUrl(host, stateKey, req.query);
     req.session.loginKey = stateKey;
-    req.session.fromUrl = req.query.fromUrl || '/';
+    req.session.fromUrl = isValidCallbackUrl(req.query.fromUrl, allowedDomains) ? req.query.fromUrl : '/';
     runHooks(preLoginHooks, req, res, () => req.session.save(() => res.redirect(url)));
   }
 
@@ -215,8 +217,7 @@ export default function createController(config) {
 
         const username = getProp(user, 'dataSources.aprofiel.username');
         logger.debug(
-          `finished hooks, redirecting ${username} to ${
-            req.session.fromUrl || '/'
+          `finished hooks, redirecting ${username} to ${req.session.fromUrl || '/'
           }`,
         );
         return req.session.save(() => res.redirect(req.session.fromUrl || '/'));

src/middleware/sso.js

@@ -3,14 +3,21 @@ import pino from 'pino';
 
 import { getSessions } from '../sessionStore';
 import { getAccessToken } from '../accessToken';
+import { isValidCallbackUrl } from '../util/isValidCallbackUrl';
 
 function getFallbackFromUrl(req, port) {
   return `${req.protocol}://${req.hostname}${port ? `:${port}` : ''}${req.originalUrl}`;
 }
 
-function getFromUrl(req, port) {
-  const rawFromUrl = req.query.fromUrl || req.query.fromurl || getFallbackFromUrl(req, port);
-  return encodeURIComponent(rawFromUrl);
+function getFromUrl(req, port, allowedDomains) {
+  if (
+    (!req.query.fromUrl && !req.query.fromurl)
+    || !isValidCallbackUrl(req.query.fromUrl || req.query.fromurl, allowedDomains)
+  ) {
+    return encodeURIComponent(getFallbackFromUrl(req, port));
+  }
+
+  return encodeURIComponent(req.query.fromUrl || req.query.fromurl);
 }
 
 function getSessionWithAssuranceLevel(sessions, assuranceLevel) {
@@ -28,6 +35,7 @@ export default function sso(options) {
     port = false,
     ssoCookieName = 'dgp.auth.ssokey',
     shouldUpgradeAssuranceLevel = true,
+    allowedDomains,
   } = options;
 
   const loginPath = `${basePath}/login`;
@@ -67,7 +75,7 @@ export default function sso(options) {
         return next();
       }
 
-      const baseRedirectUrl = `${loginPath}?fromUrl=${getFromUrl(req, port)}`;
+      const baseRedirectUrl = `${loginPath}?fromUrl=${getFromUrl(req, port, allowedDomains)}`;
       const highSession = getSessionWithAssuranceLevel(sessions, 'high');
 
       if (highSession) {

src/util/isValidCallbackUrl.js

@@ -0,0 +1,14 @@
+export function isValidCallbackUrl(url, allowedDomains = ['antwerpen.be']) {
+  try {
+    const callbackUrl = new URL(url);
+
+    if (callbackUrl.protocol !== 'https:') {
+      return false;
+    }
+
+    const regex = new RegExp(`(${allowedDomains.map((allowedDomain) => `${allowedDomain.replace('.', '\\.')}$`).join('|')})`);
+    return regex.test(callbackUrl.host);
+  } catch (error) {
+    return false;
+  }
+}

test/login.js

@@ -9,7 +9,7 @@ describe('GET /login', () => {
   it('should redirect to login', (done) => {
     const router = createRouter(mockExpress, correctConfig);
     const host = 'http://www.app.com';
-    const fromUrl = 'test.com/d';
+    const fromUrl = 'https://test.com/d';
     let redirectUrl = false;
     const req = reqres.req({
       url: '/auth/login',
@@ -47,6 +47,47 @@ describe('GET /login', () => {
     router.handle(req, res);
   });
 
+  it('should not redirect if fromUrl is invalid', (done) => {
+    const router = createRouter(mockExpress, correctConfig);
+    const host = 'http://www.app.com';
+    const fromUrl = 'https://invalid.com/d';
+    let redirectUrl = false;
+    const req = reqres.req({
+      url: '/auth/login',
+      query: {
+        fromUrl,
+      },
+      get: () => host,
+      session: {
+        save: (cb) => {
+          cb();
+        },
+      },
+    });
+    const res = reqres.res({
+      header: () => { },
+      redirect(val) {
+        redirectUrl = val;
+        this.emit('end');
+      },
+    });
+
+    res.on('end', () => {
+      assert(redirectUrl);
+      assert(req.session.fromUrl === '/');
+      assert(redirectUrl.includes(encodeURIComponent(host)));
+      assert(redirectUrl.includes(encodeURIComponent(correctConfig.clientId)));
+      const scopes = correctConfig.defaultScopes.join(' ');
+      assert(
+        redirectUrl
+          .includes(encodeURIComponent(scopes)),
+      );
+      return done();
+    });
+
+    router.handle(req, res);
+  });
+
   it('should redirect to login with language if supplied', (done) => {
     const router = createRouter(mockExpress, correctConfig);
     const host = 'http://www.app.com';

test/mocks/correctConfig.js

@@ -15,6 +15,7 @@ const config = {
   url: 'https://api-gw-a.antwerpen.be/acpaas/shared-identity-data/v1',
   consentUrl: 'https://api-gw-a.antwerpen.be/acpaas/consent/v1',
   refresh: true,
+  allowedDomains: ['test.com']
 };
 
 export default config;