feature/38299 implement pupil-login in tslib (#1427)

* implement pupil-login definition * implement function definition * organise files * add pupilEvent entry on login. only update db if in new state and not updated already * create dedicated redis controller * implement separate controllers via interface * toggle at route level * finish pupil api controller separation * implement practice property across function and API * testing and fixes complete * align service bus connection env var * remove obsolete settings * update readme
DFEAGILEDEVOPS · Nov 29, 2019 · b9faedf · b9faedf
1 parent f0a8854
commit b9faedf
Show file tree

Hide file tree

Showing 27 changed files with 910 additions and 130 deletions.
diff --git a/caching.md b/caching.md
@@ -6,20 +6,8 @@ example...
 ``` javascript
 {
   _meta: {
-    type: object | number | string | array,
-    ttl: number
+    type: object | number | string | array
   },
-  value: {
-  }
+  value: string
 }
 ```
-
-### owner
-
-who creates and maintains the cache entry. Possible values...
-- sql: the data is stored in sql server, but may be consumed by any application within the solution
-- func-consumption: the func-consumption app
-- admin
-- pupil-api
-- functions
-- functions-app
diff --git a/deploy/service-bus/create.js b/deploy/service-bus/create.js
@@ -2,7 +2,7 @@
 
 require('dotenv').config()
 const azure = require('azure')
-const sbService = azure.createServiceBusService()
+const sbService = azure.createServiceBusService(process.env.SERVICE_BUS_CONNECTION_STRING)
 const queues = require('./queues-topics.json').queues
 const fiveGigabytes = 5120
 const fourteenDays = 'P14D'

diff --git a/deploy/service-bus/delete.js b/deploy/service-bus/delete.js
@@ -2,7 +2,7 @@
 
 require('dotenv').config()
 const azure = require('azure')
-const sbService = azure.createServiceBusService()
+const sbService = azure.createServiceBusService(process.env.SERVICE_BUS_CONNECTION_STRING)
 const queues = require('./queues-topics.json').queues
 
 const deleteQueue = (queueName) => (new Promise((resolve, reject) => {

diff --git a/func-consumption/check-sync/function.json b/func-consumption/check-sync/function.json
@@ -1,5 +1,4 @@
 {
-  "disabled": false,
   "bindings": [
     {
       "name": "preparedCheckSyncMessage",

diff --git a/func-consumption/package.json b/func-consumption/package.json
@@ -14,7 +14,7 @@
     "delete-specs": "find . -name \"*.spec.js\" -type f -delete",
     "clean": "rm -rf ./dist",
     "prestart": "yarn build && func extensions install",
-    "start:host": "func start",
+    "start:host": "func start -port 7072",
     "start": "yarn start:host",
     "start:dev": "yarn build && yarn start"
   },

diff --git a/func-consumption/pupil-auth/function.json b/func-consumption/pupil-auth/function.json
@@ -1,5 +1,4 @@
 {
-  "disabled": false,
   "bindings": [
     {
       "authLevel": "function",

diff --git a/func-consumption/pupil-login/function.json b/func-consumption/pupil-login/function.json
@@ -0,0 +1,19 @@
+{
+  "bindings": [
+    {
+      "name": "pupilLoginMessage",
+      "type": "serviceBusTrigger",
+      "direction": "in",
+      "queueName": "pupil-login",
+      "connection": "ServiceBusConnection"
+    },
+    {
+      "direction": "out",
+      "type": "table",
+      "name": "pupilEventTable",
+      "tableName": "pupilEvent",
+      "connection": "AZURE_STORAGE_CONNECTION_STRING"
+    }
+  ],
+  "scriptFile": "../dist/functions/pupil-login/index.js"
+}
diff --git a/func-consumption/pupil-prefs/function.json b/func-consumption/pupil-prefs/function.json
@@ -1,5 +1,4 @@
 {
-  "disabled": false,
   "bindings": [
     {
       "name": "pupilPrefsMessage",

diff --git a/func-consumption/util-diag/function.json b/func-consumption/util-diag/function.json
@@ -1,5 +1,4 @@
 {
-  "disabled": false,
   "bindings": [
     {
       "authLevel": "function",

diff --git a/func-consumption/util-submit-check/function.json b/func-consumption/util-submit-check/function.json
@@ -1,5 +1,4 @@
 {
-  "disabled": false,
   "bindings": [
     {
       "authLevel": "function",

diff --git a/pupil-api/package.json b/pupil-api/package.json
@@ -16,6 +16,7 @@
     "node": ">= 8.4"
   },
   "dependencies": {
+    "@azure/service-bus": "^1.1.1",
     "applicationinsights": "^1.0.8",
     "azure-storage": "^2.10.1",
     "bluebird": "^3.5.1",

diff --git a/pupil-api/readme.md b/pupil-api/readme.md
@@ -21,7 +21,7 @@ See the [package.json](./package.json) file for the full list of npm dependencie
 
 ## Running the application
 
-`AZURE_STORAGE_CONNECTION_STRING=xxx yarn start`
+`AZURE_STORAGE_CONNECTION_STRING=xxx SERVICE_BUS_CONNECTION_STRING=xxx yarn start`
 
 Will launch the app in development mode on http://localhost:3003/
 
@@ -36,8 +36,9 @@ When running in development the api documentation can be found at http://localho
 dotenv is installed and will load environment variables from a `.env` file stored in the root of the admin application,
 if you have created one.  See [documentation](https://www.npmjs.com/package/dotenv) for more info.
 
-* AZURE_STORAGE_CONNECTION_STRING - (required) - Storage account for upload file storage and queues.  Upload is only enabled for
+* AZURE_STORAGE_CONNECTION_STRING - (required when redis prepared checks disable) - Storage account for upload file storage and queues.  Upload is only enabled for
     production environments, but the queues are used by all environments.
+* SERVICE_BUS_CONNECTION_STRING - (required only when redis prepared checks enabled) - service bus for sending pupil login messages.
 * ENVIRONMENT_NAME - string - defaults to `Local-Dev`
 * PORT - number - defaults to `3003`
 * LOG_LEVEL - string - defaults to `debug`

diff --git a/pupil-api/src/config.ts b/pupil-api/src/config.ts
@@ -45,5 +45,8 @@ export default {
   RedisPreparedCheckExpiryInSeconds: parseToInt(process.env.PREPARED_CHECK_EXPIRY_SECONDS, 10) || 1800,
   FeatureToggles: {
     preparedChecksInRedis: toBool(process.env.FEATURE_TOGGLE_PREPARED_CHECKS_IN_REDIS) || false
+  },
+  ServiceBus: {
+    connectionString: process.env.SERVICE_BUS_CONNECTION_STRING
   }
 }
diff --git a/pupil-api/src/controllers/auth.controller.spec.ts b/pupil-api/src/controllers/auth.controller.spec.ts
@@ -6,36 +6,24 @@ import { AuthController } from './auth.controller'
 import * as httpMocks from 'node-mocks-http'
 import logger from '../services/log.service'
 import { pupilAuthenticationService } from '../services/azure-pupil-auth.service'
-import { IFeatureService } from '../services/feature.service'
 import { IPupilAuthenticationService } from '../services/redis-pupil-auth.service'
 
-const RedisPupilAuthServiceMock = jest.fn<IPupilAuthenticationService, any>(() => ({
-  authenticate: jest.fn()
-}))
-
-const FeatureServiceMock = jest.fn<IFeatureService, any>(() => ({
-  redisAuthMode: jest.fn()
-}))
-
 let req
 let res
 let mockResponse = {}
 let mockErrorResponse = new Error('mock error')
 let authController: AuthController
-let featureService: IFeatureService
 let redisPupilAuthService: IPupilAuthenticationService
 
 describe('auth controller', () => {
   beforeEach(() => {
     req = createMockRequest('application/json')
     req.body = { schoolPin: 'pin1', pupilPin: 'pin2' }
     res = httpMocks.createResponse()
-    featureService = new FeatureServiceMock()
-    redisPupilAuthService = new RedisPupilAuthServiceMock()
-    authController = new AuthController(featureService, redisPupilAuthService)
+    authController = new AuthController()
   })
 
-  it('returns an 400 error if the request is not JSON', async () => {
+  test('returns an 400 error if the request is not JSON', async () => {
     req = createMockRequest('text/html')
     spyOn(logger, 'error')
     await authController.postAuth(req, res)
@@ -44,7 +32,7 @@ describe('auth controller', () => {
     expect(data.error).toBe('Bad request')
   })
 
-  it('allows a content-type of application/json', async () => {
+  test('allows a content-type of application/json', async () => {
     spyOn(pupilAuthenticationService, 'authenticate').and.returnValue(Promise.resolve(mockResponse))
     req = createMockRequest('application/json')
     req.body = {
@@ -55,7 +43,7 @@ describe('auth controller', () => {
     expect(res.statusCode).toBe(200)
   })
 
-  it('allows a content-type of application/json with a charset', async () => {
+  test('allows a content-type of application/json with a charset', async () => {
     spyOn(pupilAuthenticationService, 'authenticate').and.returnValue(Promise.resolve(mockResponse))
     req = createMockRequest('application/json; charset=utf-8')
     req.body = {
@@ -66,17 +54,7 @@ describe('auth controller', () => {
     expect(res.statusCode).toBe(200)
   })
 
-  it('makes a call to the authentication service if redis not enabled', async () => {
-    featureService.redisAuthMode = jest.fn(() => {
-      return false
-    })
-    spyOn(pupilAuthenticationService, 'authenticate').and.returnValue(Promise.resolve(mockResponse))
-    req.body = { schoolPin: 'pin1', pupilPin: 'pin2' }
-    await authController.postAuth(req, res)
-    expect(pupilAuthenticationService.authenticate).toHaveBeenCalledWith('pin2', 'pin1')
-  })
-
-  it('returns unauthorised if the login failed', async () => {
+  test('returns unauthorised if the login failed', async () => {
     spyOn(pupilAuthenticationService, 'authenticate').and.returnValue(Promise.reject(mockErrorResponse))
     spyOn(logger, 'error')
     await authController.postAuth(req, res)
@@ -85,63 +63,48 @@ describe('auth controller', () => {
     expect(data.error).toBe('Unauthorised')
   })
 
-  it('shortcuts to return unauthorised if no schoolPin provided', async () => {
+  test('shortcuts to return unauthorised if no schoolPin provided', async () => {
     spyOn(pupilAuthenticationService, 'authenticate')
     spyOn(logger, 'error')
     req.body = {
       pupilPin: '1234'
     }
     await authController.postAuth(req, res)
     expect(pupilAuthenticationService.authenticate).not.toHaveBeenCalled()
-    expect(redisPupilAuthService.authenticate).not.toHaveBeenCalled()
     const data = JSON.parse(res._getData())
     expect(res.statusCode).toBe(401)
     expect(data.error).toBe('Unauthorised')
   })
 
-  it('shortcuts to return unauthorised if no pupilPin provided', async () => {
+  test('shortcuts to return unauthorised if no pupilPin provided', async () => {
     spyOn(pupilAuthenticationService, 'authenticate')
     spyOn(logger, 'error')
     req.body = {
       schoolPin: '1234'
     }
     await authController.postAuth(req, res)
     expect(pupilAuthenticationService.authenticate).not.toHaveBeenCalled()
-    expect(redisPupilAuthService.authenticate).not.toHaveBeenCalled()
     const data = JSON.parse(res._getData())
     expect(res.statusCode).toBe(401)
     expect(data.error).toBe('Unauthorised')
   })
 
-  it('returns a data packet to the client if authorisation is successful', async () => {
+  test('returns a data packet to the client if authorisation is successful', async () => {
     spyOn(pupilAuthenticationService, 'authenticate').and.returnValue(Promise.resolve(mockResponse))
     await authController.postAuth(req, res)
     const data = JSON.parse(res._getData())
     expect(res.statusCode).toBe(200)
     expect(data).toBeTruthy()
   })
 
-  it('returns a 401 if no redis preparedCheck found', async () => {
-    redisPupilAuthService.authenticate = jest.fn((schoolPin: string, pupilPin: string) => {
-      return undefined
-    })
-    featureService.redisAuthMode = jest.fn(() => {
-      return true
+  function createMockRequest (contentType: String): any {
+    return httpMocks.createRequest({
+      method: 'POST',
+      url: '/auth',
+      headers: {
+        'Content-Type': `${contentType}`,
+        'Content-Length': '42' // needed to make `req.is` work
+      }
     })
-    await authController.postAuth(req, res)
-    expect(res.statusCode).toBe(401)
-    const data = JSON.parse(res._getData())
-    expect(data.error).toBe('Unauthorised')
-  })
+  }
 })
-
-function createMockRequest (contentType: String): any {
-  return httpMocks.createRequest({
-    method: 'POST',
-    url: '/auth',
-    headers: {
-      'Content-Type': `${contentType}`,
-      'Content-Length': '42' // needed to make `req.is` work
-    }
-  })
-}
diff --git a/pupil-api/src/controllers/auth.controller.ts b/pupil-api/src/controllers/auth.controller.ts
@@ -2,23 +2,9 @@ import { Request, Response } from 'express'
 import logger from '../services/log.service'
 import * as apiResponse from './api-response'
 import { pupilAuthenticationService } from '../services/azure-pupil-auth.service'
-import { RedisPupilAuthenticationService, IPupilAuthenticationService } from '../services/redis-pupil-auth.service'
-import { FeatureService, IFeatureService } from '../services/feature.service'
+import { IAuthController } from '../routes/auth'
 
-export class AuthController {
-  private redisAuthService: IPupilAuthenticationService
-  private featureService: IFeatureService
-
-  constructor (featureService?: IFeatureService, redisAuthService?: IPupilAuthenticationService) {
-    if (featureService === undefined) {
-      featureService = new FeatureService()
-    }
-    this.featureService = featureService
-    if (redisAuthService === undefined) {
-      redisAuthService = new RedisPupilAuthenticationService()
-    }
-    this.redisAuthService = redisAuthService
-  }
+export class AuthController implements IAuthController {
 
   async postAuth (req: Request, res: Response) {
     const contentType = req.get('Content-Type')
@@ -30,16 +16,8 @@ export class AuthController {
     const { pupilPin, schoolPin } = req.body
     if (!schoolPin || !pupilPin) return apiResponse.unauthorised(res)
 
-    let data
     try {
-      if (this.featureService.redisAuthMode()) {
-        data = await this.redisAuthService.authenticate(schoolPin, pupilPin)
-        if (data === undefined) {
-          return apiResponse.unauthorised(res)
-        }
-      } else {
-        data = await pupilAuthenticationService.authenticate(pupilPin, schoolPin)
-      }
+      const data = await pupilAuthenticationService.authenticate(pupilPin, schoolPin)
       apiResponse.sendJson(res, data)
     } catch (error) {
       logger.error('Failed to authenticate pupil: ', error)