fix: removed node-machine-id (#2269)

Lumieducation · Jun 4, 2022 · e4605cb · e4605cb
1 parent 433a26b
commit e4605cb
Show file tree

Hide file tree

Showing 10 changed files with 104 additions and 71 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -24,7 +24,6 @@
         "i18next-node-fs-backend": "2.1.3",
         "matomo-tracker": "2.2.4",
         "nanoid": "3.3.2",
-        "node-machine-id": "1.1.12",
         "object-hash": "3.0.0",
         "promisepipe": "3.0.0",
         "recursive-readdir": "2.2.2",

diff --git a/server/src/config/IH5PEditorSettings.ts b/server/src/config/IH5PEditorSettings.ts
@@ -8,6 +8,7 @@ export default interface IH5PEditorSettings {
     h5pUuid: string;
     language: string;
     lastVersion: string;
+    machineId: string;
     privacyPolicyConsent: boolean;
     token: string;
     usageStatistics: boolean;

diff --git a/server/src/config/SettingsCache.ts b/server/src/config/SettingsCache.ts
@@ -5,6 +5,7 @@ import { supportedLocales } from '../boot/i18n';
 import defaultSettings from './defaultSettings';
 import IH5PEditorSettings from './IH5PEditorSettings';
 import Logger from '../helpers/Logger';
+import { nanoid } from 'nanoid';
 
 const log = new Logger('SettingsStorage');
 
@@ -93,41 +94,56 @@ export default class SettingsStorage {
      * same
      */
     public validateAndFixSettings(settings: IH5PEditorSettings): boolean {
+        let changed = false;
+
         log.debug(`Validating settings. Language: ${settings.language}`);
         let lng = settings.language;
         if (!lng) {
             settings.language = 'en';
-            return true;
-        }
-        // allow regular language codes like de-DE or zh-HANS
-        if (
-            !/^[a-z]{2,3}(-[A-Z]{2,6})?$/i.test(settings.language) ||
-            !supportedLocales.find((l) => l.code === lng)
-        ) {
-            log.debug(
-                'The language in the settings is either malformed or not in the list of supported locales.'
-            );
-            // converts es-419 into es
-            const res = /^([a-z]{2,3})(-.{2,6})?$/i.exec(settings.language);
-            if (res.length > 0) {
-                log.debug('Removing variant code from language');
-                lng = res[1];
-            }
-        }
-        if (supportedLocales.find((l) => l.code === lng)) {
-            if (settings.language !== lng) {
+            changed = true;
+        } else {
+            let lngOk = false;
+            // allow regular language codes like de-DE or zh-HANS
+            if (
+                !/^[a-z]{2,3}(-[A-Z]{2,6})?$/i.test(settings.language) ||
+                !supportedLocales.find((l) => l.code === lng)
+            ) {
                 log.debug(
-                    `Changing language '${settings.language}' to '${lng}'`
+                    'The language in the settings is either malformed or not in the list of supported locales.'
                 );
-                settings.language = lng;
-                return true;
+                // converts es-419 into es
+                const res = /^([a-z]{2,3})(-.{2,6})?$/i.exec(settings.language);
+                if (res.length > 0) {
+                    log.debug('Removing variant code from language');
+                    lng = res[1];
+                }
+            }
+            if (supportedLocales.find((l) => l.code === lng)) {
+                if (settings.language !== lng) {
+                    log.debug(
+                        `Changing language '${settings.language}' to '${lng}'`
+                    );
+                    settings.language = lng;
+                    changed = true;
+                    lngOk = true;
+                }
+                log.debug('Language code is ok');
+                lngOk = true;
+            }
+            if (!lngOk) {
+                log.debug('Falling back to English language.');
+                settings.language = 'en';
+                changed = true;
             }
-            log.debug('Language code is ok');
-            return false;
         }
-        log.debug('Falling back to English language.');
-        settings.language = 'en';
-        return true;
+
+        if (!settings.machineId || settings.machineId === '') {
+            log.debug('Generating new machine id');
+            settings.machineId = nanoid();
+            changed = true;
+        }
+
+        return changed;
     }
 
     /**
@@ -241,7 +257,8 @@ export default class SettingsStorage {
                 lastVersion: loadedSettings.lastVersion,
                 privacyPolicyConsent: loadedSettings.privacyPolicyConsent,
                 token: loadedSettings.token,
-                usageStatistics: loadedSettings.usageStatistics
+                usageStatistics: loadedSettings.usageStatistics,
+                machineId: loadedSettings.machineId
             }
         };
     }

diff --git a/server/src/config/__test__/SettingsCache.test.ts b/server/src/config/__test__/SettingsCache.test.ts
@@ -8,20 +8,48 @@ describe('SettingsCache', () => {
     });
 
     it('fixed es-419 (Spanish, Latin America) language codes', () => {
-        const settings = { ...defaultSettings, language: 'es-419' };
+        const settings = {
+            ...defaultSettings,
+            machineId: '123',
+            language: 'es-419'
+        };
         expect(settingsCache.validateAndFixSettings(settings)).toBe(true);
         expect(settings.language).toEqual('es');
     });
 
     it('leaves language codes like es-MX', () => {
-        const settings = { ...defaultSettings, language: 'es-mx' };
+        const settings = {
+            ...defaultSettings,
+            machineId: '123',
+            language: 'es-mx'
+        };
         expect(settingsCache.validateAndFixSettings(settings)).toBe(false);
         expect(settings.language).toEqual('es-mx');
     });
 
     it('leaves language codes like es', () => {
-        const settings = { ...defaultSettings, language: 'es' };
+        const settings = {
+            ...defaultSettings,
+            machineId: '123',
+            language: 'es'
+        };
         expect(settingsCache.validateAndFixSettings(settings)).toBe(false);
         expect(settings.language).toEqual('es');
     });
+
+    it('generates machineId', () => {
+        const settings = { ...defaultSettings };
+        expect(settingsCache.validateAndFixSettings(settings)).toBe(true);
+        expect(settings.machineId).not.toEqual('');
+    });
+
+    it('leaves existing machineId', () => {
+        const settings = {
+            ...defaultSettings,
+            language: 'de',
+            machineId: 'asd'
+        };
+        expect(settingsCache.validateAndFixSettings(settings)).toBe(false);
+        expect(settings.machineId).toEqual('asd');
+    });
 });
diff --git a/server/src/config/defaultSettings.ts b/server/src/config/defaultSettings.ts
@@ -13,7 +13,8 @@ const defaultSettings: IH5PEditorSettings = {
     privacyPolicyConsent: false,
     token: '',
     usageStatistics: false,
-    h5pUuid: ''
+    h5pUuid: '',
+    machineId: ''
 };
 
 export default defaultSettings;
diff --git a/server/src/main.ts b/server/src/main.ts
@@ -16,7 +16,6 @@ import initUpdater from './services/updater';
 import createWebsocket from './boot/websocket';
 import serverConfigFactory from './config/defaultPaths';
 import matomo from './boot/matomo';
-import { machineId } from 'node-machine-id';
 import i18next from 'i18next';
 import fsExtra from 'fs-extra';
 
@@ -203,11 +202,12 @@ if (!gotSingleInstanceLock) {
 
     app.on('before-quit', async () => {
         try {
-            if ((await settingsCache.getSettings()).usageStatistics) {
+            const settings = await settingsCache.getSettings();
+            if (settings.usageStatistics) {
                 const data = {
                     url: '/Lumi',
-                    _id: await machineId(),
-                    uid: await machineId(),
+                    _id: settings.machineId,
+                    uid: settings.machineId,
                     e_c: 'App',
                     e_a: 'quit',
                     lang: electron.app.getLocale(),
@@ -320,11 +320,12 @@ if (!gotSingleInstanceLock) {
         }
 
         try {
-            if ((await settingsCache.getSettings()).usageStatistics) {
+            const settings = await settingsCache.getSettings();
+            if (settings.usageStatistics) {
                 const data = {
                     url: '/Lumi',
-                    _id: await machineId(),
-                    uid: await machineId(),
+                    _id: settings.machineId,
+                    uid: settings.machineId,
                     e_c: 'App',
                     e_a: 'start',
                     lang: electron.app.getLocale(),

diff --git a/server/src/routes/index.ts b/server/src/routes/index.ts
@@ -46,7 +46,7 @@ export default function (
     log.info('setting up routes');
 
     router.use('/api/v1/auth', authRoutes());
-    router.use('/api/v1/track', trackingRoutes(serverConfig, settingsCache));
+    router.use('/api/v1/track', trackingRoutes(settingsCache));
     router.use(
         '/api/v1/analytics',
         analyticRoutes(getBrowserWindow, electronState)

diff --git a/server/src/routes/trackingRoutes.ts b/server/src/routes/trackingRoutes.ts
@@ -1,20 +1,15 @@
 import express from 'express';
 import matomo from '../boot/matomo';
 import * as Sentry from '@sentry/node';
-import IServerConfig from '../config/IPaths';
 import electron from 'electron';
 import * as os from 'os';
-import { machineId } from 'node-machine-id';
 import { nanoid } from 'nanoid';
 
 import SettingsCache from '../config/SettingsCache';
 
 const id = nanoid(16);
 
-export default function (
-    serverConfig: IServerConfig,
-    settingsCache: SettingsCache
-): express.Router {
+export default function (settingsCache: SettingsCache): express.Router {
     const router = express.Router();
     router.post(
         `/`,
@@ -24,12 +19,13 @@ export default function (
             next: express.NextFunction
         ) => {
             try {
-                if ((await settingsCache.getSettings()).usageStatistics) {
+                const settings = await settingsCache.getSettings();
+                if (settings.usageStatistics) {
                     const { action, category, name, value } = req.body;
                     const data = {
                         url: '/Lumi',
                         _id: id,
-                        uid: await machineId(),
+                        uid: settings.machineId,
                         e_c: category,
                         e_a: action,
                         e_n: name,

diff --git a/test/data/settings.json b/test/data/settings.json
@@ -1 +1 @@
-{"bugTracking":false,"firstOpen":false,"lastVersion":"11.3.0","language":"en","privacyPolicyConsent":true,"usageStatistics":false,"autoUpdates":false,"test":"abc","enableLumiRun":false,"h5pUuid":"test"}
+{"autoUpdates":false,"bugTracking":false,"enableLumiRun":false,"firstOpen":false,"language":"en","lastVersion":"11.3.0","privacyPolicyConsent":true,"usageStatistics":false,"h5pUuid":"test","machineId":"XzUd-SetAHhqIMGPDNuFz","test":"abc"}