Fix and rename Authentication plugin (#464)

https://eaflood.atlassian.net/browse/WATER-4085

In [Create authentication plugin](#351) we added the ability to authenticate and authorise requests to **water-abstraction-system** using the same data the rest of the service relies on, and the cookie [water-abstraction-ui](https://github.com/DEFRA/water-abstraction-ui) will pass through.

One of the nifty things in it was the use of [server.dependency()](https://hapi.dev/api/?v=21.3.2#-serverdependencydependencies-after). Currently, we rely on plugins being registered in a particular order to prevent catastrophe. What this PR tried to demonstrate was that by using `server.dependency()` we could break the dependence on the order they are registered.

We're now ready to enable authentication by default but when we tried with `server.auth.default('session')` we kept getting an error. Hapi kept telling us it didn't recognise that strategy.

No matter where we made the call we got the error. When we removed the call to `server.dependency()` and re-ordered the plugins in `server.js` the error went away and the default auth on our routes started working.

Reading posts like [Handling plugin dependencies](https://hapipal.com/best-practices/handling-plugin-dependencies) highlights there is a lot to think about when it comes to removing the dependence on plugin registration order. If we were maintainers of a plugin, we would need to nail this.

But as we are just registering our own for use solely in our own project it looks like considerable overhead and complexity we don't really need. If we screw up the order it becomes obvious pretty quickly that something is broken. So, in this change, we're going back to being non-clever with our plugins! 😁

On the second point, a re-read of the plugin and the associated service highlighted that these enable both authentication and authorisation in Hapi for our routes. Because of this, we're going to rename everything to `auth` so if folks in the future are looking for authorisation-related code they don't overlook the plugin and service.

app/plugins/authentication.plugin.js → app/plugins/auth.plugin.js

@@ -1,13 +1,13 @@
 'use strict'
 
 /**
- * Plugin to authenticate users
- * @module AuthenticationPlugin
+ * Plugin to authenticate and authorise users
+ * @module AuthPlugin
  */
 
-const AuthenticationConfig = require('../../config/authentication.config.js')
+const AuthService = require('../services/plugins/auth.service.js')
 
-const AuthenticationService = require('../services/plugins/authentication.service.js')
+const AuthenticationConfig = require('../../config/authentication.config.js')
 
 const TWO_HOURS_IN_MS = 2 * 60 * 60 * 1000
 
@@ -33,7 +33,7 @@ const TWO_HOURS_IN_MS = 2 * 60 * 60 * 1000
  * More info on authorisation and scope can be found at https://hapi.dev/api/?v=21.3.2#-routeoptionsauthaccessscope
  */
 
-const AuthenticationPlugin = {
+const AuthPlugin = {
   name: 'authentication',
   register: async (server, _options) => {
     // We wait for @hapi/cookie to be registered before setting up the authentication strategy
@@ -49,11 +49,11 @@ const AuthenticationPlugin = {
         },
         redirectTo: '/signin',
         validate: async (_request, session) => {
-          return AuthenticationService.go(session.userId)
+          return AuthService.go(session.userId)
         }
       })
     })
   }
 }
 
-module.exports = AuthenticationPlugin
+module.exports = AuthPlugin

app/server.js

@@ -3,7 +3,7 @@
 const Hapi = require('@hapi/hapi')
 
 const AirbrakePlugin = require('./plugins/airbrake.plugin.js')
-const AuthenticationPlugin = require('./plugins/authentication.plugin.js')
+const AuthPlugin = require('./plugins/auth.plugin.js')
 const BlippPlugin = require('./plugins/blipp.plugin.js')
 const ChargingModuleTokenCachePlugin = require('./plugins/charging-module-token-cache.plugin.js')
 const ErrorPagesPlugin = require('./plugins/error-pages.plugin.js')
@@ -19,11 +19,12 @@ const ViewsPlugin = require('./plugins/views.plugin.js')
 const ServerConfig = require('../config/server.config.js')
 
 const registerPlugins = async (server) => {
-  // Register the remaining plugins
+  // NOTE: This order matters to some plugins we register. Inserting into the order should be fine. But if you reorder
+  // any existing plugin registration double-check you haven't broken anything!
   await server.register(StopPlugin)
   await server.register(require('@hapi/inert'))
-  await server.register(AuthenticationPlugin)
   await server.register(require('@hapi/cookie'))
+  await server.register(AuthPlugin)
   await server.register(RouterPlugin)
   await server.register(HapiPinoPlugin())
   await server.register(AirbrakePlugin)

app/services/plugins/authentication.service.js → app/services/plugins/auth.service.js

@@ -1,14 +1,14 @@
 'use strict'
 
 /**
- * Used by `AuthenticationPlugin` to authenticate and authorise users
- * @module AuthenticationService
+ * Used by `AuthPlugin` to authenticate and authorise users
+ * @module AuthService
  */
 
 const FetchUserRolesAndGroupsService = require('../idm/fetch-user-roles-and-groups.service.js')
 
 /**
- * This service is intended to be used by our `AuthenticationPlugin` to authenticate and authorise users.
+ * This service is intended to be used by our `AuthPlugin` to authenticate and authorise users.
  *
  * We take a user id and look it up in the `idm` schema using `FetchUserRolesAndGroupsService`. This gives us a user
  * object along with arrays of role objects and group objects that the user has been assigned to.
@@ -19,6 +19,7 @@ const FetchUserRolesAndGroupsService = require('../idm/fetch-user-roles-and-grou
  * array of strings then the user's scope array must contain at least one of the strings.
  *
  * @param {Number} userId The user id to be authenticated
+ *
  * @returns {Object} response
  * @returns {Boolean} response.isValid Indicates whether the user was found
  * @returns {Object} response.credentials User credentials found in the IDM

test/services/plugins/authentication.service.test.js → test/services/plugins/auth.service.test.js

@@ -12,9 +12,9 @@ const { expect } = Code
 const FetchUserRolesAndGroupsService = require('../../../app/services/idm/fetch-user-roles-and-groups.service.js')
 
 // Thing under test
-const AuthenticationService = require('../../../app/services/plugins/authentication.service.js')
+const AuthService = require('../../../app/services/plugins/auth.service.js')
 
-describe('Authentication service', () => {
+describe('Auth service', () => {
   afterEach(() => {
     Sinon.restore()
   })
@@ -30,31 +30,31 @@ describe('Authentication service', () => {
     })
 
     it('returns isValid as `true`', async () => {
-      const result = await AuthenticationService.go(12345)
+      const result = await AuthService.go(12345)
 
       expect(result.isValid).to.be.true()
     })
 
     it('returns the user in credentials.user', async () => {
-      const result = await AuthenticationService.go(12345)
+      const result = await AuthService.go(12345)
 
       expect(result.credentials.user).to.equal({ name: 'User' })
     })
 
     it('returns the roles in credentials.roles', async () => {
-      const result = await AuthenticationService.go(12345)
+      const result = await AuthService.go(12345)
 
       expect(result.credentials.roles).to.equal([{ role: 'Role' }])
     })
 
     it('returns the groups in credentials.groups', async () => {
-      const result = await AuthenticationService.go(12345)
+      const result = await AuthService.go(12345)
 
       expect(result.credentials.groups).to.equal([{ group: 'Group' }])
     })
 
     it('returns the role names in credentials.scope', async () => {
-      const result = await AuthenticationService.go(12345)
+      const result = await AuthService.go(12345)
 
       expect(result.credentials.scope).to.equal(['Role'])
     })
@@ -71,31 +71,31 @@ describe('Authentication service', () => {
     })
 
     it('returns isValid as `false`', async () => {
-      const result = await AuthenticationService.go(12345)
+      const result = await AuthService.go(12345)
 
       expect(result.isValid).to.be.false()
     })
 
     it('returns `null` in credentials.user', async () => {
-      const result = await AuthenticationService.go(12345)
+      const result = await AuthService.go(12345)
 
       expect(result.credentials.user).to.be.null()
     })
 
     it('returns an empty array in credentials.roles', async () => {
-      const result = await AuthenticationService.go(12345)
+      const result = await AuthService.go(12345)
 
       expect(result.credentials.roles).to.be.empty()
     })
 
     it('returns an empty array in credentials.groups', async () => {
-      const result = await AuthenticationService.go(12345)
+      const result = await AuthService.go(12345)
 
       expect(result.credentials.groups).to.be.empty()
     })
 
     it('returns an empty array in credentials.scope', async () => {
-      const result = await AuthenticationService.go(12345)
+      const result = await AuthService.go(12345)
 
       expect(result.credentials.scope).to.be.empty()
     })