bugfix: authentication issues (#666)

* bugfix: authentication issues

* fix: saneUrl/index.spec.js

* bugfix: backend tests

* bugfix: backend tests

deploy/app.js

@@ -42,9 +42,18 @@ app.use((req, _, next) => {
 
 const { configureAuth, ready: authReady } = require('./auth')
 
-const store = new MemoryStore({
-  checkPeriod: 86400000
-})
+/**
+ * memorystore (or perhaps lru-cache itself) does not properly close when server shuts
+ * this causes problems during tests
+ * So when testing app.js, set USE_DEFAULT_MEMORY_STORE to true
+ * see app.spec.js
+ */
+const { USE_DEFAULT_MEMORY_STORE } = process.env
+const store = USE_DEFAULT_MEMORY_STORE
+  ? (console.warn(`USE_DEFAULT_MEMORY_STORE is set to true, memleak expected. Do NOT use in prod.`), null)
+  : new MemoryStore({
+      checkPeriod: 86400000
+    })
 
 const SESSIONSECRET = process.env.SESSIONSECRET || 'this is not really a random session secret'
 
@@ -67,14 +76,15 @@ if (process.env.DISABLE_CSP && process.env.DISABLE_CSP === 'true') {
   require('./csp')(app)
 }
 
-
 /**
  * configure Auth
  * async function, but can start server without
  */
-configureAuth(app)
-  .then(() => console.log('configure auth properly'))
-  .catch(e => console.error('configure auth failed', e))
+
+(async () => {
+  await configureAuth(app)
+  app.use('/user', require('./user'))
+})()
 
 const PUBLIC_PATH = process.env.NODE_ENV === 'production'
   ? path.join(__dirname, 'public')
@@ -128,10 +138,7 @@ app.use(require('./devBanner'))
 /**
  * populate nonce token
  */
-const indexTemplate = require('fs').readFileSync(
-  path.join(PUBLIC_PATH, 'index.html'),
-  'utf-8'
-)
+const { indexTemplate } = require('./constants')
 app.get('/', cookieParser(), (req, res) => {
   const iavError = req.cookies && req.cookies['iav-error']
   
@@ -154,11 +161,6 @@ app.get('/', cookieParser(), (req, res) => {
 
 app.use('/logo', require('./logo'))
 
-/**
- * User route, for user profile/management
- */
-app.use('/user', require('./user'))
-
 app.get('/ready', async (req, res) => {
   const authIsReady = await authReady()
   const allReady = [ 

deploy/app.spec.js

+const { expect } = require('chai')
+const fs = require('fs')
+const { assert } = require('console')
+const express = require('express')
+const got = require('got')
+const sinon = require('sinon')
+
+let server
+const PORT=12345
+
+describe('authentication', () => {
+  
+  /**
+   * memorystore (or perhaps lru-cache itself) does not properly close when server.close()
+   * use default memory store for tests
+   */
+  process.env['USE_DEFAULT_MEMORY_STORE'] = true
+
+  const fakeFunctionObj = {
+    fakeAuthConfigureAuth: (req, res, next) => next(),
+    fakeAuthReady: async () => true,
+    fakeUserRouterFn: (req, res, next) => res.status(200).send()
+  }
+
+  before(async () => {
+    const auth = require('./auth')
+    const authConfigureAuthStub = sinon.stub(auth, 'configureAuth')
+    const authIsReadyStub = sinon.stub(auth, 'ready')
+  
+    require.cache[require.resolve('./datasets')] = {
+      exports: (req, res, next) => next()
+    }
+  
+    require.cache[require.resolve('./saneUrl')] = {
+      exports: (req, res, next) => next()
+    }
+
+    require.cache[require.resolve('./user')] = {
+      exports: fakeFunctionObj.fakeUserRouterFn
+    }
+
+    require.cache[require.resolve('./constants')] = {
+      exports: {
+        indexTemplate: ``
+      }
+    }
+  
+    authConfigureAuthStub.callsFake(app => {
+      app.use(fakeFunctionObj.fakeAuthConfigureAuth)
+      return Promise.resolve()
+    })
+
+    const expressApp = express()
+    const app = require('./app')    
+    expressApp.use(app)
+    server = expressApp.listen(PORT)
+  })
+
+  after(() => {
+    server.close()
+  })
+  
+  it('fakeAuthConfigureAuth is called before user router', async () => {
+    const spyFakeAuthConfigureAuth = sinon.spy(fakeFunctionObj, 'fakeAuthConfigureAuth')
+    const spyFakeUserRouterFn = sinon.spy(fakeFunctionObj, 'fakeUserRouterFn')
+    await got(`http://localhost:${PORT}/user`)
+    assert(
+      spyFakeAuthConfigureAuth.calledBefore(spyFakeUserRouterFn),
+      'fakeAuthConfigureAuth is called before user router'
+    )
+  })
+})

deploy/auth/index.js

-const objStoreDb = new Map()
 const HOST_PATHNAME = process.env.HOST_PATHNAME || ''
 const { retry } = require('../../common/util')
 
@@ -14,7 +13,7 @@ const configureAuth = async (app) => {
   const hbpOidc2 = require('./hbp-oidc-v2')
   
   const obj = await require('./util')()
-  const { initPassportJs } = obj
+  const { initPassportJs, objStoreDb } = obj
   initPassportJs(app)
 
   await retry(() => hbpOidc(app), { timeout: 1000, retries: 3 })

deploy/auth/util.js

 const { configureAuth, jwtDecode } = require('./oidc')
+const objStoreDb = new Map()
 
 const HOSTNAME = process.env.HOSTNAME || 'http://localhost:3000'
 const HOST_PATHNAME = process.env.HOST_PATHNAME || ''
@@ -50,6 +51,7 @@ const getPublicAccessToken = async () => {
 }
 
 const initPassportJs = app => {
+  console.log('init passport js')
   const passport = require('passport')
   
   app.use(passport.initialize())
@@ -70,6 +72,13 @@ const initPassportJs = app => {
 
 module.exports = async () => {
 
+  /**
+   * this configuration is required to acquire valid
+   * access tokens using refresh token
+   * 
+   * This is so that datasets can be retrieved when user
+   * is not authenticated
+   */
   const { client } = await configureAuth({
     clientId,
     clientSecret,
@@ -85,6 +94,7 @@ module.exports = async () => {
 
   return {
     initPassportJs,
+    objStoreDb,
     getPublicAccessToken: async () => await getPublicAccessToken()
   }
 }
\ No newline at end of file

deploy/constants.js

+const fs = require('fs')
+
+const PUBLIC_PATH = process.env.NODE_ENV === 'production'
+  ? path.join(__dirname, 'public')
+  : path.join(__dirname, '..', 'dist', 'aot')
+
+const indexTemplate = fs.readFileSync(
+  path.join(PUBLIC_PATH, 'index.html'),
+  'utf-8'
+)
+
+module.exports = {
+  indexTemplate
+}