correction of the MFA detection #618

Author: VinceFINET

build/src/api/core/orgcheck-api-secretsauce.js

@@ -693,7 +693,7 @@ const ALL_SCORE_RULES = [
     }, {
         id: 72,
         description: 'User is logging directly without MFA',
-        formula: (/** @type {SFDC_User} */ d) => d.nbDirectLoginWithoutMFA > 0 && d.hasMfaByPass !== true,
+        formula: (/** @type {SFDC_User} */ d) => d.nbDirectLoginsWithoutMFA > 0 && d.hasMfaByPass !== true,
         errorMessage: `This user is logging in directly to Salesforce without using MFA (Multi-Factor Authentication). And this user has not the MFA bypass enabled. Please work with your user to make them use MFA for better security.`,
         badField: 'nbDirectLoginWithoutMFA',
         applicable: [ SFDC_User ],

build/src/api/data/orgcheck-api-data-user.js

@@ -125,23 +125,30 @@ export class SFDC_User extends Data {
     permissionSetRefs;
 
     /**
-     * @description Number of direct login without using MFA
+     * @description Number of direct logins to salesforce
      * @type {number}
      * @public
      */
-    nbDirectLoginWithoutMFA;
+    nbDirectLogins;
 
     /**
-     * @description Number of direct login with using MFA
+     * @description Number of direct logins without using MFA
      * @type {number}
      * @public
      */
-    nbDirectLoginWithMFA;
+    nbDirectLoginsWithoutMFA;
 
     /**
-     * @description Number of indirect login via SSO
+     * @description Number of direct logins with using MFA
      * @type {number}
      * @public
      */
-    nbSSOLogin;
+    nbDirectLoginsWithMFA;
+
+    /**
+     * @description Number of indirect logins via SSO
+     * @type {number}
+     * @public
+     */
+    nbSSOLogins;
 }

build/src/api/dataset/orgcheck-api-dataset-internalactiveusers.js

@@ -38,12 +38,20 @@ export class DatasetInternalActiveUsers extends Dataset {
                     'AND Assignee.ContactId = NULL ' +
                     'AND Assignee.Profile.Id != NULL'
         }, {
-            string: 'SELECT UserId, LoginType, AuthMethodReference, Status, COUNT(Id) CntLogin ' +
+            string: 'SELECT UserId, Status, LoginType, COUNT(Id) CntLogins ' +
                     'FROM LoginHistory ' +
-                    `WHERE (LoginType = 'Application' ` + // Adding parenthesis because we have a OR here
-                    `OR LoginType LIKE '%SSO%') ` +       // and the option 'queryMoreField' will add a final AND
-                    `GROUP BY UserId, LoginType, AuthMethodReference, Status `,
+                    `WHERE (LoginType = 'Application' OR LoginType LIKE '%SSO%') ` + // the option 'queryMoreField' will add a final AND so that's why we have parenthesis here around the OR statement
+                    'GROUP BY UserId, Status, LoginType ' +
+                    'ORDER BY UserId, Status, LoginType ',
             queryMoreField: 'LoginTime' // aggregate does not support calling QueryMore, use the custom instead
+        }, {
+            string: 'SELECT UserId, Policy, COUNT(Id) CntVerifications ' +
+                    'FROM VerificationHistory ' +
+                    `WHERE Activity = 'Login' ` +
+                    `AND Status IN ('AutomatedSuccess', 'Succeeded') ` +
+                    `AND (LoginHistory.LoginType = 'Application')` +
+                    'GROUP BY UserId, Policy ',
+            queryMoreField: 'VerificationTime' // aggregate does not support calling QueryMore, use the custom instead
         }], logger);
 
         // Init the factory and records
@@ -53,6 +61,7 @@ export class DatasetInternalActiveUsers extends Dataset {
         const userRecords = results[0];
         const assignmentRecords = results[1];
         const loginRecords = results[2];
+        const verifRecords = results[3];
         logger?.log(`Parsing ${userRecords.length} users...`);
         const users = new Map(await Processor.map(userRecords, async (/** @type {any} */ record) => {
 
@@ -74,9 +83,10 @@ export class DatasetInternalActiveUsers extends Dataset {
                     isAdminLike: false,
                     hasMfaByPass: false,
                     hasDebugMode: record.UserPreferencesUserDebugModePref === true,
-                    nbDirectLoginWithMFA: 0,
-                    nbDirectLoginWithoutMFA: 0,
-                    nbSSOLogin: 0,
+                    nbDirectLogins: 0,
+                    nbDirectLoginsWithMFA: 0,
+                    nbDirectLoginsWithoutMFA: 0,
+                    nbSSOLogins: 0,
                     url: sfdcManager.setupUrl(id, SalesforceMetadataTypes.USER)
                 }
             });
@@ -123,21 +133,31 @@ export class DatasetInternalActiveUsers extends Dataset {
                     // depending on the login access (direct or sso)
                     if (record.LoginType === 'Application') {
                         // Direct login access via browser
-                        if (record.AuthMethodReference) {
-                            // MFA used from Salesforce
-                            user.nbDirectLoginWithMFA += record.CntLogin;
-                        } else {
-                            // No MFA used from Salesforce
-                            user.nbDirectLoginWithoutMFA += record.CntLogin;
-                        }
+                        user.nbDirectLogins += record.CntLogins;
                     } else {
                         // SSO login access via IDP
-                        user.nbSSOLogin += record.CntLogin;
+                        user.nbSSOLogins += record.CntLogins;
                     }
                 }
             }
         });
 
+        // Now process the user verifications aggregates
+        logger?.log(`Parsing ${verifRecords.length} user verifications aggregates...`);
+        await Processor.forEach(verifRecords, (/** @type {any} */ record) => {
+
+            const userId = sfdcManager.caseSafeId(record.UserId);
+            const user = users.get(userId);
+            if (user) {
+                // If MFA is used for internal logins
+                if (record.Policy === 'TwoFactorAuthentication') {
+                    user.nbDirectLoginsWithMFA += record.CntVerifications;
+                } else {
+                    user.nbDirectLoginsWithoutMFA += record.CntVerifications;
+                }
+            }
+        });
+
         // FINALLY!!!! Compute the score of all items
         logger?.log(`Computing scores for ${users.size} users...`);
         await Processor.forEach(users, (/** @type {SFDC_User} */ user) => {

build/test/api/unit/orgcheck-api-datasets.unit.test.js

@@ -609,4 +609,52 @@ describe('tests.api.unit.Datasets', () => {
     });
   });
 
+    describe('Specific test for DatasetInternalActiveUsers', () => {  
+    const dataset = new DatasetInternalActiveUsers();
+    it('checks if this dataset class runs correctly', async () => {
+      const sfdcManager = new SfdcManagerMock();
+      sfdcManager.addSoqlQueryResponse('FROM User ', [
+        { Id: '001', Name: 'User is OK', ProfileId: '001', LastLoginDate: new Date(), LastPasswordChangeDate: new Date(), NumberOfFailedLogins: 0, UserPreferencesLightningExperiencePreferred: true, UserPreferencesUserDebugModePref: false },
+        { Id: '002', Name: 'User never logged', ProfileId: '001', LastLoginDate: null, LastPasswordChangeDate: new Date(), NumberOfFailedLogins: 0, UserPreferencesLightningExperiencePreferred: true, UserPreferencesUserDebugModePref: false },
+        { Id: '003', Name: 'User never changed pwd', ProfileId: '001', LastLoginDate: new Date(), LastPasswordChangeDate: null, NumberOfFailedLogins: 0, UserPreferencesLightningExperiencePreferred: true, UserPreferencesUserDebugModePref: false },
+        { Id: '004', Name: 'User has failed logins', ProfileId: '001', LastLoginDate: new Date(), LastPasswordChangeDate: new Date(), NumberOfFailedLogins: 5, UserPreferencesLightningExperiencePreferred: true, UserPreferencesUserDebugModePref: false },
+        { Id: '005', Name: 'User is still under classic', ProfileId: '001', LastLoginDate: new Date(), LastPasswordChangeDate: new Date(), NumberOfFailedLogins: 0, UserPreferencesLightningExperiencePreferred: false, UserPreferencesUserDebugModePref: false },
+        { Id: '006', Name: 'User has debug mode on', ProfileId: '001', LastLoginDate: new Date(), LastPasswordChangeDate: new Date(), NumberOfFailedLogins: 0, UserPreferencesLightningExperiencePreferred: true, UserPreferencesUserDebugModePref: true },
+      ]);
+      sfdcManager.addSoqlQueryResponse('FROM LoginHistory ', [
+        { UserId: '001', LoginType: 'Application', Status: 'Success', CntLogins: 92 }, // nbDirectLoginWithoutMFA += 92 for user 001
+        { UserId: '001', LoginType: 'Application', Status: 'Failure', CntLogins: 12 },
+        { UserId: '001', LoginType: 'zz SSO 1', Status: 'Success', CntLogins: 4 }, // nbSSOLogins += 4 for user 001
+        { UserId: '001', LoginType: 'zz SSO 2', Status: 'Success', CntLogins: 9 }, // nbSSOLogins += 9 for user 001
+        { UserId: '001', LoginType: 'zz SSO 2', Status: 'Failure', CntLogins: 1 },
+        { UserId: '002', LoginType: 'Application', Status: 'Success', CntLogins: 1 }, // nbDirectLoginsWithoutMFA += 1 for user 002
+        { UserId: '002', LoginType: 'zz SSO 1', Status: 'Failure', CntLogins: 1 }, 
+        { UserId: '002', LoginType: 'zz SSO 2', Status: 'Success', CntLogins: 2 } // nbSSOLogins += 2 for user 002
+      ]);
+      sfdcManager.addSoqlQueryResponse('FROM VerificationHistory ', [ 
+        { UserId: '001', Policy: 'TwoFactorAuthentication', CntVerifications: 91 }, // nbDirectLoginsWithMFA += 91 for user 001
+        { UserId: '001', Policy: 'other', CntVerifications: 21 },
+        { UserId: '002', Policy: 'TwoFactorAuthentication', CntVerifications: 0 }, // nbDirectLoginsWithMFA += 0 for user 002
+        { UserId: '002', Policy: 'other', CntVerifications: 0 },
+      ]);
+      const dataFactory = new DataFactoryMock();
+      const logger = new SimpleLoggerMock();
+      const results = await dataset.run(sfdcManager, dataFactory, logger);
+      expect(results).toBeDefined();
+      expect(results instanceof Map).toBeTruthy();
+      expect(results.size).toBe(6);
+      expect(results.get('002').lastLogin).toBe(null);
+      expect(results.get('005').onLightningExperience).toBe(false);
+      expect(results.get('006').hasDebugMode).toBe(true);
+      expect(results.get('001').nbDirectLogins).toBe(92);
+      expect(results.get('001').nbDirectLoginsWithMFA).toBe(91);
+      expect(results.get('001').nbDirectLoginsWithoutMFA).toBe(21);
+      expect(results.get('001').nbSSOLogins).toBe(13);
+      expect(results.get('002').nbDirectLogins).toBe(1);
+      expect(results.get('002').nbDirectLoginsWithMFA).toBe(0);
+      expect(results.get('002').nbDirectLoginsWithoutMFA).toBe(0);
+      expect(results.get('002').nbSSOLogins).toBe(2);
+    });
+  });
+
 });

force-app/main/default/lwc/orgcheckApp/__tests__/orgcheckApp.tableDefinitions.test.js

@@ -262,7 +262,7 @@ describe('c-orgcheck-app', () => {
             // Properties used in the app in the corresponding TableDefintion
             const expectedProperties = [ 'url', 'score', 'id', 'name', 'onLightningExperience', 'lastLogin', 'numberFailedLogins', 'isAdminLike',
                 'lastPasswordChange', 'importantPermissions', 'importantPermissionsGrantedBy', 'profileRef', 'permissionSetRefs', 'hasMfaByPass', 
-                'nbDirectLoginWithMFA', 'nbDirectLoginWithoutMFA', 'nbSSOLogin', 'hasDebugMode' ];
+                'nbDirectLoginsWithMFA', 'nbDirectLoginsWithoutMFA', 'nbSSOLogins', 'nbDirectLogins', 'hasDebugMode' ];
             // Calculate the expected fields that are not in the object
             expect(expectedProperties.filter((p) => objectProperties.includes(p) === false)).toStrictEqual([]);
             // Calculate the fields that are in the object but not in the expected properties

force-app/main/default/lwc/orgcheckApp/subs/tableDefinitions.js

@@ -806,9 +806,9 @@ export class TableDefinitions {
             { label: 'Failed logins',          type: ColumnType.NUM,  data: { value: 'numberFailedLogins' }},
             { label: 'Has MFA by-pass?',       type: ColumnType.CHK,  data: { value: 'hasMfaByPass' }},
             { label: 'Has Debug mode?',        type: ColumnType.CHK,  data: { value: 'hasDebugMode' }},
-            { label: '#SF Logins w/o MFA',     type: ColumnType.NUM,  data: { value: 'nbDirectLoginWithoutMFA' }},
-            { label: '#SF Logins w/ MFA',      type: ColumnType.NUM,  data: { value: 'nbDirectLoginWithMFA' }},
-            { label: '#SSO Logins',            type: ColumnType.NUM,  data: { value: 'nbSSOLogin' }},
+            { label: '#SF Logins w/o MFA',     type: ColumnType.NUM,  data: { value: 'nbDirectLoginsWithoutMFA' }},
+            { label: '#SF Logins w/ MFA',      type: ColumnType.NUM,  data: { value: 'nbDirectLoginsWithMFA' }},
+            { label: '#SSO Logins',            type: ColumnType.NUM,  data: { value: 'nbSSOLogins' }},
             { label: 'Password change',        type: ColumnType.DTM,  data: { value: 'lastPasswordChange' }},
             { label: 'Is Admin-like?',         type: ColumnType.CHK,  data: { value: 'isAdminLike' }},
             { label: 'Api Enabled',            type: ColumnType.CHK,  data: { value: 'importantPermissions.apiEnabled' }},