Merge pull request #1303 from alphagov/ldeb-fix-glitch

Fix authentication on Glitch

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Unreleased
 
+## Fixes
+
+- [#1303: Fix authentication on Glitch](https://github.com/alphagov/govuk-prototype-kit/pull/1303)
+
 # 12.0.3 (Fix release)
 
 ## Fixes

app/config.js

@@ -25,7 +25,7 @@ module.exports = {
   // Force HTTP to redirect to HTTPS on production
   useHttps: 'true',
 
-  // Enable or disable Browser Sync
+  // Enable or disable Browser Sync (local development only)
   useBrowserSync: 'true'
 
 }

lib/middleware/authentication/authentication.js

@@ -3,7 +3,7 @@ const url = require('url')
 
 // Local dependencies
 const config = require('../../../app/config')
-const encryptPassword = require('../../utils').encryptPassword
+const { encryptPassword, getNodeEnv } = require('../../utils')
 
 // Local variables
 const allowedPathsWhenUnauthenticated = [
@@ -45,20 +45,15 @@ module.exports = authentication
 
 function shouldUseAuth () {
   const useAuth = (process.env.USE_AUTH || config.useAuth || 'not set').toLowerCase()
+
   if (useAuth !== 'true') {
     return false
   }
 
-  const isRunningInGlitch = process.env.PROJECT_REMIX_CHAIN !== undefined
-  if (isRunningInGlitch) {
+  if (getNodeEnv() === 'production') {
     return true
   }
 
-  const safeNodeEnv = process.env.NODE_ENV || 'not set'
-  const isRunningInProduction = safeNodeEnv.toLowerCase() === 'production'
-  if (isRunningInProduction) {
-    return true
-  }
   return false
 }
 

lib/utils.js

@@ -13,6 +13,28 @@ const matter = require('gray-matter')
 // Local dependencies
 const config = require('../app/config.js')
 
+// Are we running on Glitch.com?
+const onGlitch = function () {
+  // there isn't an official way to check, but this was recommended
+  // https://support.glitch.com/t/detect-if-app-is-running-on-glitch/3120
+  return Boolean(process.env.PROJECT_REMIX_CHAIN)
+}
+
+exports.onGlitch = onGlitch
+
+// Get a normalised form of NODE_ENV
+//
+// Returns a lower-case string representing the environment the node.js app
+// is running in. Normally this will be one of `production`, `development`,
+// or `test`, although it can be any lower-case string. In most
+// circumstances the value is derived from the environment variable
+// NODE_ENV, defaulting to `development` if that is not set.
+exports.getNodeEnv = function () {
+  const glitchEnv = onGlitch() ? 'production' : false // Glitch doesn't set NODE_ENV, but we want to treat it as production
+  const env = (process.env.NODE_ENV || glitchEnv || 'development').toLowerCase()
+  return env
+}
+
 // Tweak the Markdown renderer
 const defaultMarkedRenderer = marked.defaults.renderer || new marked.Renderer()
 
@@ -125,14 +147,7 @@ exports.findAvailablePort = function (app, callback) {
 
 // Redirect HTTP requests to HTTPS
 exports.forceHttps = function (req, res, next) {
-  var protocol = req.headers['x-forwarded-proto']
-  // Glitch returns a comma separated list for x-forwarded-proto
-  // We need the first to determine if running on https
-  if (protocol) {
-    protocol = protocol.split(',').shift()
-  }
-
-  if (protocol !== 'https') {
+  if (req.protocol !== 'https') {
     console.log('Redirecting request to https')
     // 302 temporary - this is a feature that can be disabled
     return res.redirect(302, 'https://' + req.get('Host') + req.url)

lib/utils.test.js

@@ -4,6 +4,65 @@ const nunjucks = require('nunjucks')
 
 const utils = require('./utils.js')
 
+const originalEnvironmentVariables = process.env
+
+describe('onGlitch', () => {
+  beforeEach(() => {
+    process.env = {}
+  })
+
+  afterEach(() => {
+    process.env = originalEnvironmentVariables
+  })
+
+  it('returns false if envvar PROJECT_REMIX_CHAIN is not set', () => {
+    expect(utils.onGlitch()).toBe(false)
+  })
+
+  it('returns true if envvar PROJECT_REMIX_CHAIN is set', () => {
+    process.env.PROJECT_REMIX_CHAIN = '["dead-beef"]'
+    expect(utils.onGlitch()).toBe(true)
+  })
+})
+
+describe('getNodeEnd', () => {
+  beforeEach(() => {
+    process.env = {}
+  })
+
+  afterEach(() => {
+    process.env = originalEnvironmentVariables
+  })
+
+  it('returns the value of NODE_ENV', () => {
+    process.env.NODE_ENV = 'production'
+    expect(utils.getNodeEnv()).toBe('production')
+
+    process.env.NODE_ENV = 'test'
+    expect(utils.getNodeEnv()).toBe('test')
+  })
+
+  it('defaults to development if NODE_ENV is not set or empty', () => {
+    expect(utils.getNodeEnv()).toBe('development')
+
+    process.env.NODE_ENV = ''
+    expect(utils.getNodeEnv()).toBe('development')
+  })
+
+  it('always returns a lower-case string', () => {
+    process.env.NODE_ENV = 'FOOBAR'
+    expect(utils.getNodeEnv()).toBe('foobar')
+  })
+
+  it('returns production if running on Glitch and NODE_ENV not set or empty', () => {
+    process.env.PROJECT_REMIX_CHAIN = '["dead-beef"]'
+    expect(utils.getNodeEnv()).toBe('production')
+
+    process.env.NODE_ENV = ''
+    expect(utils.getNodeEnv()).toBe('production')
+  })
+})
+
 describe('checked', () => {
   var ctx, checked
 

listen-on-port.js

@@ -9,7 +9,7 @@ const utils = require('./lib/utils.js')
 
 // Set up configuration variables
 var useBrowserSync = config.useBrowserSync.toLowerCase()
-var env = (process.env.NODE_ENV || 'development').toLowerCase()
+var env = utils.getNodeEnv()
 
 utils.findAvailablePort(server, function (port) {
   console.log('Listening on port ' + port + '   url: http://localhost:' + port)

server.js

@@ -49,8 +49,7 @@ if (useV6) {
 
 // Set up configuration variables
 var releaseVersion = packageJson.version
-var glitchEnv = (process.env.PROJECT_REMIX_CHAIN) ? 'production' : false // glitch.com
-var env = (process.env.NODE_ENV || glitchEnv || 'development').toLowerCase()
+var env = utils.getNodeEnv()
 var useAutoStoreData = process.env.USE_AUTO_STORE_DATA || config.useAutoStoreData
 var useCookieSessionStore = process.env.USE_COOKIE_SESSION_STORE || config.useCookieSessionStore
 var useHttps = process.env.USE_HTTPS || config.useHttps