Feature/54223 adminlogonevent school id col (#2331)

* add migration

* add local strategy

* Add index to fk col

* Update local strategy tests

* Mod dfe strategy

* Remove lint

* admin logon event tests

* Update sign_in_steps.rb

Co-authored-by: Guy Harwood <[email protected]>
Co-authored-by: Mohsen Qureshi <[email protected]>
Co-authored-by: Mohsen Qureshi <[email protected]>

admin/authentication/local-strategy.js

@@ -6,6 +6,7 @@ const logger = require('../services/log.service').getLogger()
 
 const userDataService = require('../services/data-access/user.data.service')
 const adminLogonEventDataService = require('../services/data-access/admin-logon-event.data.service')
+const { isNumber, isNotNil, isInteger } = require('ramda-adjunct')
 
 module.exports = async function (req, userIdentifier, password, done) {
   /**
@@ -16,7 +17,8 @@ module.exports = async function (req, userIdentifier, password, done) {
     body: JSON.stringify(R.omit(['password'], R.prop('body', req))),
     remoteIp: req.headers['x-forwarded-for'] || req.connection.remoteAddress,
     userAgent: req.headers['user-agent'],
-    loginMethod: 'local'
+    loginMethod: 'local',
+    school_id: null
   }
 
   try {
@@ -48,6 +50,10 @@ module.exports = async function (req, userIdentifier, password, done) {
       id: user.id
     }
 
+    if (isNotNil(user.schoolId) && isNumber(user.schoolId) && isInteger(user.schoolId)) {
+      logonEvent.school_id = user.schoolId
+    }
+
     // Success - valid login
     logonEvent.user_id = user.id
     await saveValidLogonEvent(logonEvent, sessionData)

admin/services/dfe-signin.service.js

@@ -92,6 +92,11 @@ const service = {
       isAuthenticated: true,
       authProviderSessionToken: dfeUser.id_token
     }
+
+    if (schoolRecord) {
+      logonEvent.school_id = schoolRecord.id
+    }
+
     await adminLogonEventDataService.sqlCreate(logonEvent)
 
     // set id to sql record id

admin/spec/back-end/authentication/local-strategy.spec.js

@@ -21,6 +21,7 @@ describe('localStrategy', () => {
       }
     }
     const req = httpMocks.createRequest(reqParams)
+
     test('validates and saves a valid helpdesk user', async () => {
       const user = {
         id: 1,
@@ -48,6 +49,7 @@ describe('localStrategy', () => {
         id: user.id
       })
     })
+
     test('validates and saves a valid teacher user', async () => {
       const user = {
         id: 1,
@@ -74,13 +76,14 @@ describe('localStrategy', () => {
         id: user.id
       })
     })
+
     test('registers an invalid logon event if user is not found', async () => {
       jest.spyOn(userDataService, 'sqlFindUserInfoByIdentifier').mockImplementation()
       jest.spyOn(adminLogonEventDataService, 'sqlCreate').mockImplementation()
       const doneFunc = (err, res) => err || res
       await validateAndSave(req, 'teacher1', 'password', doneFunc)
       expect(adminLogonEventDataService.sqlCreate).toHaveBeenCalledWith(
-        { sessionId: 1, body: '{}', remoteIp: 'remoteAddress', userAgent: undefined, loginMethod: 'local', errorMsg: 'Invalid user', isAuthenticated: false }
+        { sessionId: 1, body: '{}', remoteIp: 'remoteAddress', userAgent: undefined, loginMethod: 'local', errorMsg: 'Invalid user', isAuthenticated: false, school_id: null }
       )
     })
   })

db/migrations/schema-objects/20221027152055.do.add-school-id-to-adminlogonevent.sql

@@ -0,0 +1,29 @@
+-- Add new school_id column
+IF NOT EXISTS(
+                SELECT *
+                FROM sys.columns
+               WHERE object_ID = object_id('mtc_admin.adminLogonEvent')
+                 AND col_name(object_ID, column_Id) = 'school_id')
+    BEGIN
+        ALTER TABLE [mtc_admin].[adminLogonEvent]
+            ADD [school_id] INT;
+    END
+
+go
+
+-- make it a foreign key
+IF NOT EXISTS(SELECT *
+                FROM INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE
+               WHERE CONSTRAINT_COLUMN_USAGE.TABLE_SCHEMA = 'mtc_admin'
+                 AND CONSTRAINT_COLUMN_USAGE.TABLE_NAME = 'adminLogonEvent'
+                 AND CONSTRAINT_COLUMN_USAGE.COLUMN_NAME = 'school_id'
+                 AND CONSTRAINT_NAME = 'FK_adminLogonEvent_school_id')
+    BEGIN
+        ALTER TABLE [mtc_admin].[adminLogonEvent]
+            ADD CONSTRAINT [FK_adminLogonEvent_school_id]
+                FOREIGN KEY (school_id) REFERENCES [mtc_admin].[school] (id);
+    END
+
+-- add an index
+DROP INDEX IF EXISTS [mtc_admin].[adminLogonEvent].[IX_adminLogonEvent_school_id];
+CREATE INDEX IX_adminLogonEvent_school_id ON mtc_admin.adminLogonEvent([school_id]);

db/migrations/schema-objects/20221027152055.undo.add-school-id-to-adminlogonevent.sql

@@ -0,0 +1,19 @@
+-- drop index
+DROP INDEX IF EXISTS [mtc_admin].[adminLogonEvent].[IX_adminLogonEvent_school_id];
+
+-- drop foreign key
+IF EXISTS(SELECT *
+            FROM INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE
+           WHERE CONSTRAINT_COLUMN_USAGE.TABLE_SCHEMA = 'mtc_admin'
+             AND CONSTRAINT_COLUMN_USAGE.TABLE_NAME = 'adminLogonEvent'
+             AND CONSTRAINT_COLUMN_USAGE.COLUMN_NAME = 'school_id'
+             AND CONSTRAINT_NAME = 'FK_adminLogonEvent_school_id')
+    BEGIN
+        ALTER TABLE [mtc_admin].[adminLogonEvent]
+            DROP CONSTRAINT [FK_adminLogonEvent_school_id];
+    END
+
+
+
+-- drop column
+ALTER TABLE [mtc_admin].[adminLogonEvent] DROP COLUMN IF EXISTS  [school_id];

test/admin-hpa/features/sign_in.feature

@@ -21,4 +21,17 @@ Feature:
   Scenario: Contact page can be accessed via the footer
     Given I am on the sign in page
     When I decide to get in contact
-    Then I should be taken to the contact page for mtc
+    Then I should be taken to the contact page for mtc
+
+  Scenario Outline: School ID is recorded when a user logs in
+    Given I have logged in with <role>
+    Then school ID or null should be recorded depnding if the user is linked to a school or not
+
+    Examples:
+      | role            |
+      | teacher1        |
+      | test-developer  |
+      | service-manager |
+      | helpdesk        |
+      | tech-support    |
+      | sta-admin       |

test/admin-hpa/features/step_definitions/sign_in_steps.rb

@@ -62,6 +62,16 @@
 end
 
 Given(/^I have logged in with (.*)$/) do |teacher|
+  @teacher = teacher
   sign_in_page.load
-  sign_in_page.login(teacher, 'password')
+  sign_in_page.login(@teacher, 'password')
+end
+
+
+Then(/^school ID or null should be recorded depnding if the user is linked to a school or not$/) do
+  user = SqlDbHelper.find_teacher(@teacher)
+  logon_event_school_id = SqlDbHelper.login_event(user['id']).map {|a| a['school_id']}.uniq
+  expect(logon_event_school_id.size).to eql 1
+  user_school_id = user['school_id']
+  expect(user_school_id).to eql logon_event_school_id.first
 end

test/admin-hpa/features/support/sql_db_helper.rb

@@ -704,4 +704,10 @@ def self.pupil_audit_record(pupil_id)
     result.each {|row| row.map}
   end
 
+  def self.login_event(user_id)
+    sql = "SELECT * FROM [mtc_admin].[adminLogonEvent] WHERE user_id = #{user_id}"
+    result = SQL_CLIENT.execute(sql)
+    result.each {|row| row.map}
+  end
+
 end