From 7e0c7c76b64bada4648f535c9b1d036c5c1c195f Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Wed, 6 Nov 2024 10:30:07 +0000 Subject: [PATCH 01/16] test: add initial ldap service unit tests --- .../ldap.ee/__tests__/ldap.service.test.ts | 180 ++++++++++++++++++ 1 file changed, 180 insertions(+) create mode 100644 packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts new file mode 100644 index 0000000000..7780723daa --- /dev/null +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -0,0 +1,180 @@ +import { mock } from 'jest-mock-extended'; +import { LdapService } from '@/ldap/ldap.service.ee'; +import { SettingsRepository } from '@/databases/repositories/settings.repository'; +import { Settings } from '@/databases/entities/settings'; +import { Logger } from '@/logging/logger.service'; +import type { LdapConfig } from '@/ldap/types'; +import { mockInstance } from '@test/mocking'; +import config from '@/config'; +import { LDAP_LOGIN_ENABLED, LDAP_LOGIN_LABEL } from '@/ldap/constants'; +import { sync } from 'fast-glob'; + +// Need fake timers to avoid setInterval +// problems with the scheduled sync +jest.useFakeTimers(); + +describe('LdapService', () => { + const OLD_ENV = process.env; + + const fakeConfig: LdapConfig = { + loginEnabled: true, + loginLabel: 'fakeLoginLabel', + connectionUrl: 'https://connection.url', + allowUnauthorizedCerts: true, + connectionSecurity: 'none', + connectionPort: 1234, + baseDn: 'dc=example,dc=com', + bindingAdminDn: 'uid=jdoe,ou=users,dc=example,dc=com', + bindingAdminPassword: 'fakePassword', + firstNameAttribute: 'givenName', + lastNameAttribute: 'sn', + emailAttribute: 'mail', + loginIdAttribute: 'uid', + ldapIdAttribute: 'uid', + userFilter: '', + synchronizationEnabled: true, + synchronizationInterval: 60, + searchPageSize: 1, + searchTimeout: 6, + }; + + beforeEach(() => { + process.env = { ...OLD_ENV }; + }); + + afterEach(() => { + jest.restoreAllMocks(); + + process.env = OLD_ENV; + }); + + describe('init()', () => { + it('should load the ldap configuration', async () => { + const settingsRepo = mockInstance(SettingsRepository); + + const ldapService = new LdapService(mock(), settingsRepo, mock(), mock()); + + settingsRepo.findOneByOrFail.mockResolvedValue({ + value: JSON.stringify(fakeConfig), + } as Settings); + const loadConfigSpy = jest.spyOn(ldapService, 'loadConfig'); + + await ldapService.init(); + + expect(loadConfigSpy).toHaveBeenCalledTimes(1); + }); + + it('should set expected configuration variables from LDAP config if ldap is enabled', async () => { + const settingsRepo = mockInstance(SettingsRepository); + const mockLogger = mock(); + + const ldapService = new LdapService(mockLogger, settingsRepo, mock(), mock()); + + const configSetSpy = jest.spyOn(config, 'set'); + + settingsRepo.findOneByOrFail.mockResolvedValue({ + value: JSON.stringify(fakeConfig), + } as Settings); + + await ldapService.init(); + + expect(configSetSpy).toHaveBeenNthCalledWith(1, LDAP_LOGIN_ENABLED, fakeConfig.loginEnabled); + expect(configSetSpy).toHaveBeenNthCalledWith( + 2, + 'userManagement.authenticationMethod', + 'ldap', + ); + expect(configSetSpy).toHaveBeenNthCalledWith(3, LDAP_LOGIN_LABEL, fakeConfig.loginLabel); + }); + + it('should set expected configuration variables from LDAP config if ldap is not enabled', async () => { + const givenConfig = { ...fakeConfig, loginEnabled: false }; + + const settingsRepo = mockInstance(SettingsRepository); + const mockLogger = mock(); + + const ldapService = new LdapService(mockLogger, settingsRepo, mock(), mock()); + + const configSetSpy = jest.spyOn(config, 'set'); + + settingsRepo.findOneByOrFail.mockResolvedValue({ + value: JSON.stringify(givenConfig), + } as Settings); + + await ldapService.init(); + + expect(configSetSpy).toHaveBeenNthCalledWith(1, LDAP_LOGIN_ENABLED, givenConfig.loginEnabled); + expect(configSetSpy).toHaveBeenNthCalledWith( + 2, + 'userManagement.authenticationMethod', + 'email', + ); + expect(configSetSpy).toHaveBeenNthCalledWith(3, LDAP_LOGIN_LABEL, givenConfig.loginLabel); + }); + + it('should show logger warning if authentication method is not ldap or email', async () => { + const settingsRepo = mockInstance(SettingsRepository); + const mockLogger = mock(); + + const ldapService = new LdapService(mockLogger, settingsRepo, mock(), mock()); + + settingsRepo.findOneByOrFail.mockResolvedValue({ + value: JSON.stringify(fakeConfig), + } as Settings); + config.set('userManagement.authenticationMethod', 'invalid'); + + await ldapService.init(); + + expect(mockLogger.warn).toHaveBeenCalledTimes(1); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Cannot set LDAP login enabled state when an authentication method other than email or ldap is active (current: invalid)', + expect.any(Error), + ); + }); + + it('should schedule syncing if config has enabled synchronization', async () => { + const givenConfig = { + ...fakeConfig, + synchronizationEnabled: true, + synchronizationInterval: 10, + }; + const settingsRepo = mockInstance(SettingsRepository); + const mockLogger = mock(); + + const ldapService = new LdapService(mockLogger, settingsRepo, mock(), mock()); + + settingsRepo.findOneByOrFail.mockResolvedValue({ + value: JSON.stringify(givenConfig), + } as Settings); + const setIntervalSpy = jest.spyOn(global, 'setInterval'); + + await ldapService.init(); + + expect(setIntervalSpy).toHaveBeenCalledTimes(1); + expect(setIntervalSpy).toHaveBeenCalledWith( + expect.any(Function), + givenConfig.synchronizationInterval * 60000, + ); + }); + + it('should throw an error if config has enabled synchronization but no synchronizationInterval is set', async () => { + const givenConfig = { + ...fakeConfig, + synchronizationEnabled: true, + synchronizationInterval: 0, + }; + const settingsRepo = mockInstance(SettingsRepository); + const mockLogger = mock(); + + const ldapService = new LdapService(mockLogger, settingsRepo, mock(), mock()); + + settingsRepo.findOneByOrFail.mockResolvedValue({ + value: JSON.stringify(givenConfig), + } as Settings); + const setIntervalSpy = jest.spyOn(global, 'setInterval'); + + await expect(ldapService.init()).rejects.toThrowError('Interval variable has to be defined'); + expect(setIntervalSpy).toHaveBeenCalledTimes(0); + }); + }); +}); From 23d543fc64a1b783cf744d054d16ff69c1b8899d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 6 Nov 2024 12:35:56 +0100 Subject: [PATCH 02/16] test(core): Suggestions for LDAP tests (#11586) --- .../ldap.ee/__tests__/ldap.service.test.ts | 127 ++++++++---------- 1 file changed, 55 insertions(+), 72 deletions(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index 7780723daa..6034b01049 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -1,22 +1,14 @@ import { mock } from 'jest-mock-extended'; -import { LdapService } from '@/ldap/ldap.service.ee'; -import { SettingsRepository } from '@/databases/repositories/settings.repository'; -import { Settings } from '@/databases/entities/settings'; -import { Logger } from '@/logging/logger.service'; -import type { LdapConfig } from '@/ldap/types'; -import { mockInstance } from '@test/mocking'; -import config from '@/config'; -import { LDAP_LOGIN_ENABLED, LDAP_LOGIN_LABEL } from '@/ldap/constants'; -import { sync } from 'fast-glob'; -// Need fake timers to avoid setInterval -// problems with the scheduled sync -jest.useFakeTimers(); +import config from '@/config'; +import { SettingsRepository } from '@/databases/repositories/settings.repository'; +import { LDAP_LOGIN_ENABLED, LDAP_LOGIN_LABEL } from '@/ldap/constants'; +import { LdapService } from '@/ldap/ldap.service.ee'; +import type { LdapConfig } from '@/ldap/types'; +import { mockInstance, mockLogger } from '@test/mocking'; describe('LdapService', () => { - const OLD_ENV = process.env; - - const fakeConfig: LdapConfig = { + const ldapConfig: LdapConfig = { loginEnabled: true, loginLabel: 'fakeLoginLabel', connectionUrl: 'https://connection.url', @@ -38,25 +30,24 @@ describe('LdapService', () => { searchTimeout: 6, }; - beforeEach(() => { - process.env = { ...OLD_ENV }; + beforeAll(() => { + // Need fake timers to avoid setInterval + // problems with the scheduled sync + jest.useFakeTimers(); }); - afterEach(() => { + beforeEach(() => { jest.restoreAllMocks(); - - process.env = OLD_ENV; }); describe('init()', () => { - it('should load the ldap configuration', async () => { - const settingsRepo = mockInstance(SettingsRepository); + it('should load the LDAP configuration', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), + }); - const ldapService = new LdapService(mock(), settingsRepo, mock(), mock()); + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); - settingsRepo.findOneByOrFail.mockResolvedValue({ - value: JSON.stringify(fakeConfig), - } as Settings); const loadConfigSpy = jest.spyOn(ldapService, 'loadConfig'); await ldapService.init(); @@ -64,43 +55,37 @@ describe('LdapService', () => { expect(loadConfigSpy).toHaveBeenCalledTimes(1); }); - it('should set expected configuration variables from LDAP config if ldap is enabled', async () => { - const settingsRepo = mockInstance(SettingsRepository); - const mockLogger = mock(); + it('should set expected configuration variables from LDAP config if LDAP is enabled', async () => { + const settingsRepository = mockInstance(SettingsRepository, { + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), + }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI - const ldapService = new LdapService(mockLogger, settingsRepo, mock(), mock()); + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); const configSetSpy = jest.spyOn(config, 'set'); - settingsRepo.findOneByOrFail.mockResolvedValue({ - value: JSON.stringify(fakeConfig), - } as Settings); - await ldapService.init(); - expect(configSetSpy).toHaveBeenNthCalledWith(1, LDAP_LOGIN_ENABLED, fakeConfig.loginEnabled); + expect(configSetSpy).toHaveBeenNthCalledWith(1, LDAP_LOGIN_ENABLED, ldapConfig.loginEnabled); expect(configSetSpy).toHaveBeenNthCalledWith( 2, 'userManagement.authenticationMethod', 'ldap', ); - expect(configSetSpy).toHaveBeenNthCalledWith(3, LDAP_LOGIN_LABEL, fakeConfig.loginLabel); + expect(configSetSpy).toHaveBeenNthCalledWith(3, LDAP_LOGIN_LABEL, ldapConfig.loginLabel); }); - it('should set expected configuration variables from LDAP config if ldap is not enabled', async () => { - const givenConfig = { ...fakeConfig, loginEnabled: false }; + it('should set expected configuration variables from LDAP config if LDAP is disabled', async () => { + const givenConfig = { ...ldapConfig, loginEnabled: false }; - const settingsRepo = mockInstance(SettingsRepository); - const mockLogger = mock(); + const settingsRepository = mockInstance(SettingsRepository, { + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(givenConfig) }), + }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI - const ldapService = new LdapService(mockLogger, settingsRepo, mock(), mock()); + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); const configSetSpy = jest.spyOn(config, 'set'); - settingsRepo.findOneByOrFail.mockResolvedValue({ - value: JSON.stringify(givenConfig), - } as Settings); - await ldapService.init(); expect(configSetSpy).toHaveBeenNthCalledWith(1, LDAP_LOGIN_ENABLED, givenConfig.loginEnabled); @@ -113,20 +98,20 @@ describe('LdapService', () => { }); it('should show logger warning if authentication method is not ldap or email', async () => { - const settingsRepo = mockInstance(SettingsRepository); - const mockLogger = mock(); + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), + }); - const ldapService = new LdapService(mockLogger, settingsRepo, mock(), mock()); + const logger = mockLogger(); + + const ldapService = new LdapService(logger, settingsRepository, mock(), mock()); - settingsRepo.findOneByOrFail.mockResolvedValue({ - value: JSON.stringify(fakeConfig), - } as Settings); config.set('userManagement.authenticationMethod', 'invalid'); await ldapService.init(); - expect(mockLogger.warn).toHaveBeenCalledTimes(1); - expect(mockLogger.warn).toHaveBeenCalledWith( + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith( 'Cannot set LDAP login enabled state when an authentication method other than email or ldap is active (current: invalid)', expect.any(Error), ); @@ -134,18 +119,17 @@ describe('LdapService', () => { it('should schedule syncing if config has enabled synchronization', async () => { const givenConfig = { - ...fakeConfig, + ...ldapConfig, synchronizationEnabled: true, synchronizationInterval: 10, }; - const settingsRepo = mockInstance(SettingsRepository); - const mockLogger = mock(); - const ldapService = new LdapService(mockLogger, settingsRepo, mock(), mock()); + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(givenConfig) }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); - settingsRepo.findOneByOrFail.mockResolvedValue({ - value: JSON.stringify(givenConfig), - } as Settings); const setIntervalSpy = jest.spyOn(global, 'setInterval'); await ldapService.init(); @@ -153,24 +137,23 @@ describe('LdapService', () => { expect(setIntervalSpy).toHaveBeenCalledTimes(1); expect(setIntervalSpy).toHaveBeenCalledWith( expect.any(Function), - givenConfig.synchronizationInterval * 60000, + givenConfig.synchronizationInterval * 60_000, ); }); it('should throw an error if config has enabled synchronization but no synchronizationInterval is set', async () => { - const givenConfig = { - ...fakeConfig, - synchronizationEnabled: true, - synchronizationInterval: 0, - }; - const settingsRepo = mockInstance(SettingsRepository); - const mockLogger = mock(); + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify({ + ...ldapConfig, + synchronizationEnabled: true, + synchronizationInterval: 0, + }), + }), + }); - const ldapService = new LdapService(mockLogger, settingsRepo, mock(), mock()); + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); - settingsRepo.findOneByOrFail.mockResolvedValue({ - value: JSON.stringify(givenConfig), - } as Settings); const setIntervalSpy = jest.spyOn(global, 'setInterval'); await expect(ldapService.init()).rejects.toThrowError('Interval variable has to be defined'); From 819961426a5ff8ed2df10e990af02d1cf9ecbec2 Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Thu, 7 Nov 2024 13:45:15 +0000 Subject: [PATCH 03/16] test: add more tests + mock ldapts client --- .../ldap.ee/__tests__/ldap.service.test.ts | 168 +++++++++++++++++- 1 file changed, 167 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index 6034b01049..98b1e2f448 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -1,5 +1,6 @@ import { mock } from 'jest-mock-extended'; +import { Client } from 'ldapts'; import config from '@/config'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; import { LDAP_LOGIN_ENABLED, LDAP_LOGIN_LABEL } from '@/ldap/constants'; @@ -7,11 +8,22 @@ import { LdapService } from '@/ldap/ldap.service.ee'; import type { LdapConfig } from '@/ldap/types'; import { mockInstance, mockLogger } from '@test/mocking'; +// Mock ldapts client +jest.mock('ldapts', () => { + const ClientMock = jest.fn(); + + ClientMock.prototype.bind = jest.fn(); + ClientMock.prototype.unbind = jest.fn(); + ClientMock.prototype.startTLS = jest.fn(); + + return { Client: ClientMock }; +}); + describe('LdapService', () => { const ldapConfig: LdapConfig = { loginEnabled: true, loginLabel: 'fakeLoginLabel', - connectionUrl: 'https://connection.url', + connectionUrl: 'connection.url', allowUnauthorizedCerts: true, connectionSecurity: 'none', connectionPort: 1234, @@ -160,4 +172,158 @@ describe('LdapService', () => { expect(setIntervalSpy).toHaveBeenCalledTimes(0); }); }); + + describe.skip('loadConfig()', () => {}); + describe.skip('updateConfig()', () => {}); + describe.skip('setConfig()', () => {}); + describe.skip('searchWithAdminBinding()', () => {}); + describe.skip('validUser()', () => {}); + describe.skip('findAndAuthenticateLdapUser()', () => {}); + describe('testConnection()', () => { + it('should throw expected error if init() is not called first', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await expect(ldapService.testConnection()).rejects.toThrowError( + 'Service cannot be used without setting the property config', + ); + }); + + it('should create a new client without TLS if connectionSecurity is set to "none"', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify({ ...ldapConfig, connectionSecurity: 'none' }), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await ldapService.init(); + await ldapService.testConnection(); + + expect(Client).toHaveBeenCalledTimes(1); + expect(Client).toHaveBeenCalledWith({ + url: `ldap://${ldapConfig.connectionUrl}:${ldapConfig.connectionPort}`, + }); + }); + + it('should create a new client with TLS enabled if connectionSecurity is set to "tls" and allowing unauthorized certificates', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify({ + ...ldapConfig, + connectionSecurity: 'tls', + allowUnauthorizedCerts: true, + }), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await ldapService.init(); + await ldapService.testConnection(); + + expect(Client).toHaveBeenCalledTimes(1); + expect(Client).toHaveBeenCalledWith({ + url: `ldaps://${ldapConfig.connectionUrl}:${ldapConfig.connectionPort}`, + tlsOptions: { + rejectUnauthorized: false, + }, + }); + }); + + it('should create a new client with TLS enabled if connectionSecurity is set to "tls" and not allowing unauthorized certificates', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify({ + ...ldapConfig, + connectionSecurity: 'tls', + allowUnauthorizedCerts: false, + }), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await ldapService.init(); + await ldapService.testConnection(); + + expect(Client).toHaveBeenCalledTimes(1); + expect(Client).toHaveBeenCalledWith({ + url: `ldaps://${ldapConfig.connectionUrl}:${ldapConfig.connectionPort}`, + tlsOptions: { + rejectUnauthorized: true, + }, + }); + }); + + it('should create a new client and start TLS if connectionSecurity is set to "startTls"', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify({ + ...ldapConfig, + connectionSecurity: 'startTls', + allowUnauthorizedCerts: true, + }), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await ldapService.init(); + await ldapService.testConnection(); + + expect(Client).toHaveBeenCalledTimes(1); + expect(Client.prototype.startTLS).toHaveBeenCalledTimes(1); + expect(Client.prototype.startTLS).toHaveBeenCalledWith({ + rejectUnauthorized: false, + }); + }); + it('should not create a new client if one has already been created', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await ldapService.init(); + await ldapService.testConnection(); + + expect(Client).toHaveBeenCalledTimes(1); + + await ldapService.testConnection(); + expect(Client).toHaveBeenCalledTimes(1); + }); + }); + + describe.skip('runSync()', () => {}); + describe('stopSync()', () => { + it('should clear the scheduled timer', async () => { + const givenConfig = { + ...ldapConfig, + synchronizationEnabled: true, + synchronizationInterval: 10, + }; + + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(givenConfig) }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + const clearIntervalSpy = jest.spyOn(global, 'clearInterval'); + + await ldapService.init(); + await ldapService.stopSync(); + + expect(clearIntervalSpy).toHaveBeenCalledTimes(1); + }); + }); }); From 00eed37d6ce7078e4e1b164b6cbf1cf26dfc7dd7 Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Thu, 7 Nov 2024 17:31:37 +0000 Subject: [PATCH 04/16] test: add setConfig tests --- .../ldap.ee/__tests__/ldap.service.test.ts | 108 +++++++++++++++++- 1 file changed, 106 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index 98b1e2f448..c54999c743 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -169,13 +169,116 @@ describe('LdapService', () => { const setIntervalSpy = jest.spyOn(global, 'setInterval'); await expect(ldapService.init()).rejects.toThrowError('Interval variable has to be defined'); - expect(setIntervalSpy).toHaveBeenCalledTimes(0); + expect(setIntervalSpy).not.toHaveBeenCalled(); }); }); describe.skip('loadConfig()', () => {}); describe.skip('updateConfig()', () => {}); - describe.skip('setConfig()', () => {}); + describe('setConfig()', () => { + it('should stop synchronization if the timer is running and the config is disabled', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const updatedLdapConfig = { ...ldapConfig, synchronizationEnabled: false }; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const clearIntervalSpy = jest.spyOn(global, 'clearInterval'); + + await ldapService.init(); + ldapService.setConfig(updatedLdapConfig); + + expect(clearIntervalSpy).toHaveBeenCalledTimes(1); + }); + + it('should schedule synchronization if the timer is not running and the config is enabled', () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const updatedLdapConfig = { + ...ldapConfig, + synchronizationEnabled: true, + synchronizationInterval: 999, + }; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const clearIntervalSpy = jest.spyOn(global, 'clearInterval'); + const setIntervalSpy = jest.spyOn(global, 'setInterval'); + + ldapService.setConfig(updatedLdapConfig); + + expect(clearIntervalSpy).not.toHaveBeenCalled(); + expect(setIntervalSpy).toHaveBeenCalledTimes(1); + expect(setIntervalSpy).toHaveBeenCalledWith( + expect.any(Function), + updatedLdapConfig.synchronizationInterval * 60_000, + ); + }); + + it('should throw an error if the timer is not running and the config is enabled but the synchronizationInterval is not set', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const updatedLdapConfig = { + ...ldapConfig, + synchronizationEnabled: true, + synchronizationInterval: 0, + }; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const clearIntervalSpy = jest.spyOn(global, 'clearInterval'); + const setIntervalSpy = jest.spyOn(global, 'setInterval'); + + const thrownSetConfig = () => ldapService.setConfig(updatedLdapConfig); + + expect(thrownSetConfig).toThrowError('Interval variable has to be defined'); + expect(setIntervalSpy).not.toHaveBeenCalled(); + expect(clearIntervalSpy).not.toHaveBeenCalled(); + }); + + it('should restart synchronization if the timer is running and the config is enabled', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const updatedLdapConfig = { + ...ldapConfig, + synchronizationEnabled: true, + synchronizationInterval: 1234, + }; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const clearIntervalSpy = jest.spyOn(global, 'clearInterval'); + const setIntervalSpy = jest.spyOn(global, 'setInterval'); + + await ldapService.init(); + ldapService.setConfig(updatedLdapConfig); + + expect(clearIntervalSpy).toHaveBeenCalledTimes(1); + expect(setIntervalSpy).toHaveBeenCalledTimes(2); + expect(setIntervalSpy).toHaveBeenNthCalledWith( + 1, + expect.any(Function), + ldapConfig.synchronizationInterval * 60_000, + ); + expect(setIntervalSpy).toHaveBeenNthCalledWith( + 2, + expect.any(Function), + updatedLdapConfig.synchronizationInterval * 60_000, + ); + }); + }); describe.skip('searchWithAdminBinding()', () => {}); describe.skip('validUser()', () => {}); describe.skip('findAndAuthenticateLdapUser()', () => {}); @@ -284,6 +387,7 @@ describe('LdapService', () => { rejectUnauthorized: false, }); }); + it('should not create a new client if one has already been created', async () => { const settingsRepository = mock({ findOneByOrFail: jest.fn().mockResolvedValue({ From 800b53aaf465978eb7e6bee4126de2e8d961d3ce Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Mon, 11 Nov 2024 16:51:27 +0000 Subject: [PATCH 05/16] test: add tests for loadConfig --- .../ldap.ee/__tests__/ldap.service.test.ts | 68 ++++++++++++++++++- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index c54999c743..633ea36cb8 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -1,6 +1,7 @@ import { mock } from 'jest-mock-extended'; - import { Client } from 'ldapts'; +import type { Cipher } from 'n8n-core'; + import config from '@/config'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; import { LDAP_LOGIN_ENABLED, LDAP_LOGIN_LABEL } from '@/ldap/constants'; @@ -173,8 +174,71 @@ describe('LdapService', () => { }); }); - describe.skip('loadConfig()', () => {}); + describe('loadConfig()', () => { + it('should retrieve the LDAP configuration from the settings repository', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await ldapService.loadConfig(); + + expect(settingsRepository.findOneByOrFail).toHaveBeenCalledTimes(1); + }); + + it('should throw an expected error if the LDAP configuration is not found', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockRejectedValue(new Error('LDAP configuration not found')), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await expect(ldapService.loadConfig()).rejects.toThrowError('LDAP configuration not found'); + }); + + it('should decipher the LDAP configuration admin password', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const cipherMock = mock({ + decrypt: jest.fn(), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, cipherMock, mock()); + + await ldapService.loadConfig(); + + expect(cipherMock.decrypt).toHaveBeenCalledTimes(1); + expect(cipherMock.decrypt).toHaveBeenCalledWith(ldapConfig.bindingAdminPassword); + }); + + it('should return the expected LDAP configuration', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const cipherMock = mock({ + decrypt: jest.fn().mockReturnValue('decryptedPassword'), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, cipherMock, mock()); + + const config = await ldapService.loadConfig(); + + expect(config).toEqual({ ...ldapConfig, bindingAdminPassword: 'decryptedPassword' }); + }); + }); + describe.skip('updateConfig()', () => {}); + describe('setConfig()', () => { it('should stop synchronization if the timer is running and the config is disabled', async () => { const settingsRepository = mock({ From 14c71201f74bc498e991611f7581426ec0c781de Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Mon, 11 Nov 2024 17:46:33 +0000 Subject: [PATCH 06/16] test: add tests for updateConfig() --- .../ldap.ee/__tests__/ldap.service.test.ts | 160 +++++++++++++++++- 1 file changed, 158 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index 633ea36cb8..b36b05d8a8 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -3,8 +3,9 @@ import { Client } from 'ldapts'; import type { Cipher } from 'n8n-core'; import config from '@/config'; +import { AuthIdentityRepository } from '@/databases/repositories/auth-identity.repository'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; -import { LDAP_LOGIN_ENABLED, LDAP_LOGIN_LABEL } from '@/ldap/constants'; +import { LDAP_LOGIN_ENABLED, LDAP_LOGIN_LABEL, LDAP_FEATURE_NAME } from '@/ldap/constants'; import { LdapService } from '@/ldap/ldap.service.ee'; import type { LdapConfig } from '@/ldap/types'; import { mockInstance, mockLogger } from '@test/mocking'; @@ -237,7 +238,162 @@ describe('LdapService', () => { }); }); - describe.skip('updateConfig()', () => {}); + describe('updateConfig()', () => { + it('should throw expected error if the LDAP configuration is invalid', async () => { + const settingsRepository = mockInstance(SettingsRepository, { + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), + }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + + const invalidLdapConfig = { ...ldapConfig, loginEnabled: 'notABoolean' }; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await expect( + ldapService.updateConfig(invalidLdapConfig as unknown as LdapConfig), + ).rejects.toThrowError('request.body.loginEnabled is not of a type(s) boolean'); + }); + + it('should throw expected error if login is enabled and the current authentication method is "saml"', async () => { + const settingsRepository = mockInstance(SettingsRepository, { + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), + }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + + config.set('userManagement.authenticationMethod', 'saml'); + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await expect(ldapService.updateConfig(ldapConfig)).rejects.toThrowError( + 'LDAP cannot be enabled if SSO in enabled', + ); + }); + + it('should encrypt the binding admin password', async () => { + const settingsRepository = mockInstance(SettingsRepository, { + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), + }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + + const cipherMock = mock({ + encrypt: jest.fn().mockReturnValue('encryptedPassword'), + }); + + config.set('userManagement.authenticationMethod', 'email'); + const ldapService = new LdapService(mockLogger(), settingsRepository, cipherMock, mock()); + + const newConfig = { ...ldapConfig }; + await ldapService.updateConfig(newConfig); + + expect(cipherMock.encrypt).toHaveBeenCalledTimes(1); + expect(cipherMock.encrypt).toHaveBeenCalledWith(ldapConfig.bindingAdminPassword); + expect(newConfig.bindingAdminPassword).toEqual('encryptedPassword'); + }); + + it('should delete all ldap identities if login is disabled and ldap users exist', async () => { + const settingsRepository = mockInstance(SettingsRepository, { + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), + }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + + const authIdentityRepository = mockInstance(AuthIdentityRepository, { + find: jest.fn().mockResolvedValue([{ user: { id: 'userId' } }]), + delete: jest.fn(), + }); + + const cipherMock = mock({ + encrypt: jest.fn().mockReturnValue('encryptedPassword'), + }); + + config.set('userManagement.authenticationMethod', 'email'); + const ldapService = new LdapService(mockLogger(), settingsRepository, cipherMock, mock()); + + const newConfig = { ...ldapConfig, loginEnabled: false, synchronizationEnabled: true }; + await ldapService.updateConfig(newConfig); + + expect(newConfig.synchronizationEnabled).toBeFalsy(); + expect(authIdentityRepository.delete).toHaveBeenCalledTimes(1); + expect(authIdentityRepository.delete).toHaveBeenCalledWith({ providerType: 'ldap' }); + }); + + it('should not delete ldap identities if login is disabled and there are no ldap identities', async () => { + const settingsRepository = mockInstance(SettingsRepository, { + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), + }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + + const authIdentityRepository = mockInstance(AuthIdentityRepository, { + find: jest.fn().mockResolvedValue([]), + delete: jest.fn(), + }); + + const cipherMock = mock({ + encrypt: jest.fn().mockReturnValue('encryptedPassword'), + }); + + config.set('userManagement.authenticationMethod', 'email'); + const ldapService = new LdapService(mockLogger(), settingsRepository, cipherMock, mock()); + + const newConfig = { ...ldapConfig, loginEnabled: false, synchronizationEnabled: true }; + await ldapService.updateConfig(newConfig); + + expect(newConfig.synchronizationEnabled).toBeFalsy(); + expect(authIdentityRepository.delete).not.toHaveBeenCalled(); + expect(authIdentityRepository.delete).not.toHaveBeenCalled(); + }); + + it('should update the LDAP configuration in the settings repository', async () => { + const settingsRepository = mockInstance(SettingsRepository, { + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), + update: jest.fn(), + }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + + mockInstance(AuthIdentityRepository, { + find: jest.fn().mockResolvedValue([{ user: { id: 'userId' } }]), + delete: jest.fn(), + }); + + const cipherMock = mock({ + encrypt: jest.fn().mockReturnValue('encryptedPassword'), + }); + + config.set('userManagement.authenticationMethod', 'email'); + const ldapService = new LdapService(mockLogger(), settingsRepository, cipherMock, mock()); + + const newConfig = { ...ldapConfig, loginEnabled: false, synchronizationEnabled: true }; + await ldapService.updateConfig(newConfig); + + expect(settingsRepository.update).toHaveBeenCalledTimes(1); + expect(settingsRepository.update).toHaveBeenCalledWith( + { key: LDAP_FEATURE_NAME }, + { value: JSON.stringify(newConfig), loadOnStartup: true }, + ); + }); + + it('should update the LDAP login label in the config', async () => { + const settingsRepository = mockInstance(SettingsRepository, { + findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), + update: jest.fn(), + }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + + mockInstance(AuthIdentityRepository, { + find: jest.fn().mockResolvedValue([{ user: { id: 'userId' } }]), + delete: jest.fn(), + }); + + const cipherMock = mock({ + encrypt: jest.fn().mockReturnValue('encryptedPassword'), + }); + const configSetSpy = jest.spyOn(config, 'set'); + + config.set('userManagement.authenticationMethod', 'email'); + const ldapService = new LdapService(mockLogger(), settingsRepository, cipherMock, mock()); + + const newConfig = { + ...ldapConfig, + loginEnabled: false, + synchronizationEnabled: true, + loginLabel: 'newLoginLabel', + }; + await ldapService.updateConfig(newConfig); + + expect(configSetSpy).toHaveBeenNthCalledWith(4, LDAP_LOGIN_LABEL, newConfig.loginLabel); + }); + }); describe('setConfig()', () => { it('should stop synchronization if the timer is running and the config is disabled', async () => { From 04cd02a9712936da20e156fc5b893ddefce7cd4c Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Tue, 12 Nov 2024 14:58:23 +0000 Subject: [PATCH 07/16] test: tests for validUser() --- .../ldap.ee/__tests__/ldap.service.test.ts | 80 ++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index b36b05d8a8..14e104108e 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -499,9 +499,87 @@ describe('LdapService', () => { ); }); }); + describe.skip('searchWithAdminBinding()', () => {}); - describe.skip('validUser()', () => {}); + + describe('validUser()', () => { + it('should throw expected error if no configuration has been set', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await expect(ldapService.validUser('dn', 'password')).rejects.toThrowError( + 'Service cannot be used without setting the property config', + ); + }); + + it('should bind the ldap client with the expected distinguished name and password', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const distinguishedName = 'uid=jdoe,ou=users,dc=example,dc=com'; + const password = 'password'; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await ldapService.init(); + await ldapService.validUser(distinguishedName, password); + + expect(Client.prototype.bind).toHaveBeenCalledTimes(1); + expect(Client.prototype.bind).toHaveBeenCalledWith(distinguishedName, password); + }); + + it('should throw expected error if binding fails', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const distinguishedName = 'uid=jdoe,ou=users,dc=example,dc=com'; + const password = 'password'; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + Client.prototype.bind = jest + .fn() + .mockRejectedValue(new Error('Error validating user against LDAP server')); + + await ldapService.init(); + + await expect(ldapService.validUser(distinguishedName, password)).rejects.toThrowError( + 'Error validating user against LDAP server', + ); + }); + + it('should unbind the client binding', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const distinguishedName = 'uid=jdoe,ou=users,dc=example,dc=com'; + const password = 'password'; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + await ldapService.init(); + await ldapService.validUser(distinguishedName, password); + + expect(Client.prototype.unbind).toHaveBeenCalledTimes(1); + }); + }); + describe.skip('findAndAuthenticateLdapUser()', () => {}); + describe('testConnection()', () => { it('should throw expected error if init() is not called first', async () => { const settingsRepository = mock({ From f6952b41f1d1e4bf5c425179839736a21ce6f204 Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Tue, 12 Nov 2024 17:14:13 +0000 Subject: [PATCH 08/16] test: tests for searchWithAdminBinding() --- .../ldap.ee/__tests__/ldap.service.test.ts | 173 +++++++++++++++++- 1 file changed, 171 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index 14e104108e..88eb9ab023 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -5,7 +5,12 @@ import type { Cipher } from 'n8n-core'; import config from '@/config'; import { AuthIdentityRepository } from '@/databases/repositories/auth-identity.repository'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; -import { LDAP_LOGIN_ENABLED, LDAP_LOGIN_LABEL, LDAP_FEATURE_NAME } from '@/ldap/constants'; +import { + BINARY_AD_ATTRIBUTES, + LDAP_LOGIN_ENABLED, + LDAP_LOGIN_LABEL, + LDAP_FEATURE_NAME, +} from '@/ldap/constants'; import { LdapService } from '@/ldap/ldap.service.ee'; import type { LdapConfig } from '@/ldap/types'; import { mockInstance, mockLogger } from '@test/mocking'; @@ -17,6 +22,7 @@ jest.mock('ldapts', () => { ClientMock.prototype.bind = jest.fn(); ClientMock.prototype.unbind = jest.fn(); ClientMock.prototype.startTLS = jest.fn(); + ClientMock.prototype.search = jest.fn(); return { Client: ClientMock }; }); @@ -500,7 +506,170 @@ describe('LdapService', () => { }); }); - describe.skip('searchWithAdminBinding()', () => {}); + describe('searchWithAdminBinding()', () => { + it('should bind admin client', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const cipherMock = mock({ + decrypt: jest.fn().mockReturnValue('decryptedPassword'), + }); + + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); + + const filter = ''; + + const ldapService = new LdapService(mockLogger(), settingsRepository, cipherMock, mock()); + + await ldapService.init(); + await ldapService.searchWithAdminBinding(filter); + + expect(Client.prototype.bind).toHaveBeenCalledTimes(1); + expect(Client.prototype.bind).toHaveBeenCalledWith( + ldapConfig.bindingAdminDn, + 'decryptedPassword', + ); + }); + + it('should call client search with expected parameters', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const cipherMock = mock({ + decrypt: jest.fn().mockReturnValue('decryptedPassword'), + }); + + const filter = ''; + + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, cipherMock, mock()); + + await ldapService.init(); + await ldapService.searchWithAdminBinding(filter); + + expect(Client.prototype.search).toHaveBeenCalledTimes(1); + expect(Client.prototype.search).toHaveBeenLastCalledWith(ldapConfig.baseDn, { + attributes: [ + ldapConfig.emailAttribute, + ldapConfig.ldapIdAttribute, + ldapConfig.firstNameAttribute, + ldapConfig.lastNameAttribute, + ldapConfig.emailAttribute, + ], + explicitBufferAttributes: BINARY_AD_ATTRIBUTES, + filter, + timeLimit: ldapConfig.searchTimeout, + paged: { pageSize: ldapConfig.searchPageSize }, + }); + }); + + it('should call client search with expected parameters when searchPageSize is 0', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify({ ...ldapConfig, searchPageSize: 0 }), + }), + }); + + const cipherMock = mock({ + decrypt: jest.fn().mockReturnValue('decryptedPassword'), + }); + + const filter = ''; + + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, cipherMock, mock()); + + await ldapService.init(); + await ldapService.searchWithAdminBinding(filter); + + expect(Client.prototype.search).toHaveBeenCalledTimes(1); + expect(Client.prototype.search).toHaveBeenLastCalledWith(ldapConfig.baseDn, { + attributes: [ + ldapConfig.emailAttribute, + ldapConfig.ldapIdAttribute, + ldapConfig.firstNameAttribute, + ldapConfig.lastNameAttribute, + ldapConfig.emailAttribute, + ], + explicitBufferAttributes: BINARY_AD_ATTRIBUTES, + filter, + timeLimit: ldapConfig.searchTimeout, + paged: true, + }); + }); + + it('should unbind client after search', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const cipherMock = mock({ + decrypt: jest.fn().mockReturnValue('decryptedPassword'), + }); + + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); + + const filter = ''; + + const ldapService = new LdapService(mockLogger(), settingsRepository, cipherMock, mock()); + + await ldapService.init(); + await ldapService.searchWithAdminBinding(filter); + + expect(Client.prototype.unbind).toHaveBeenCalledTimes(1); + }); + + it('should return expected search entries', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const cipherMock = mock({ + decrypt: jest.fn().mockReturnValue('decryptedPassword'), + }); + + const userList = [ + { + dn: 'uid=jdoe,ou=users,dc=example,dc=com', + cn: 'Jo Doe', + sn: 'Doe', + mail: 'jdoe@example.com', + memberOf: 'cn=admins,ou=groups,dc=example,dc=com', + }, + { + dn: 'uid=ghopper,ou=users,dc=example,dc=com', + cn: 'Grace Hopper', + sn: 'Hopper', + mail: 'ghopper@nasa.com', + memberOf: 'cn=admins,ou=groups,dc=example,dc=com', + }, + ]; + Client.prototype.search = jest.fn().mockResolvedValue({ + searchEntries: userList, + }); + + const filter = ''; + + const ldapService = new LdapService(mockLogger(), settingsRepository, cipherMock, mock()); + + await ldapService.init(); + const results = await ldapService.searchWithAdminBinding(filter); + + expect(results).toEqual(userList); + }); + }); describe('validUser()', () => { it('should throw expected error if no configuration has been set', async () => { From 136e0583f8043b76cd1eefe3c618beadf5e6ebe5 Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Wed, 20 Nov 2024 09:00:10 +0000 Subject: [PATCH 09/16] wip --- .../cli/src/ldap.ee/__tests__/ldap.service.test.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index 88eb9ab023..b1fd0d341b 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -874,7 +874,19 @@ describe('LdapService', () => { }); }); - describe.skip('runSync()', () => {}); + describe('runSync()', () => { + it.todo('should search for users with expected parameters'); + it.todo('should resolve binary attributes'); + it.todo('should throw expected error if search fails'); + it.todo('should process users if mode is "live"'); + it.todo('should write expected data to the database'); + it.todo( + 'should write expected data to the database with an error message if processing users fails', + ); + it.todo('should emit expected event if synchronization is enabled'); + it.todo('should emit expected event if synchronization is disabled'); + }); + describe('stopSync()', () => { it('should clear the scheduled timer', async () => { const givenConfig = { From 3ae6edc194cda4a3178e0e1e52535042ccd77bbe Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Fri, 6 Dec 2024 17:12:01 +0000 Subject: [PATCH 10/16] test: runSync initial tests --- .../ldap.ee/__tests__/ldap.service.test.ts | 245 +++++++++++++++++- 1 file changed, 237 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index b1fd0d341b..9ae1179559 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -1,3 +1,4 @@ +import { QueryFailedError } from '@n8n/typeorm'; import { mock } from 'jest-mock-extended'; import { Client } from 'ldapts'; import type { Cipher } from 'n8n-core'; @@ -5,6 +6,7 @@ import type { Cipher } from 'n8n-core'; import config from '@/config'; import { AuthIdentityRepository } from '@/databases/repositories/auth-identity.repository'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; +import type { EventService } from '@/events/event.service'; import { BINARY_AD_ATTRIBUTES, LDAP_LOGIN_ENABLED, @@ -15,6 +17,14 @@ import { LdapService } from '@/ldap/ldap.service.ee'; import type { LdapConfig } from '@/ldap/types'; import { mockInstance, mockLogger } from '@test/mocking'; +import { + getLdapIds, + createFilter, + resolveBinaryAttributes, + processUsers, + mapLdapUserToDbUser, +} from '../helpers.ee'; + // Mock ldapts client jest.mock('ldapts', () => { const ClientMock = jest.fn(); @@ -27,6 +37,14 @@ jest.mock('ldapts', () => { return { Client: ClientMock }; }); +jest.mock('../helpers.ee', () => ({ + ...jest.requireActual('../helpers.ee'), + getLdapIds: jest.fn(), + saveLdapSynchronization: jest.fn(), + resolveBinaryAttributes: jest.fn(), + processUsers: jest.fn(), +})); + describe('LdapService', () => { const ldapConfig: LdapConfig = { loginEnabled: true, @@ -43,7 +61,7 @@ describe('LdapService', () => { emailAttribute: 'mail', loginIdAttribute: 'uid', ldapIdAttribute: 'uid', - userFilter: '', + userFilter: '(uid=jdoe)', synchronizationEnabled: true, synchronizationInterval: 60, searchPageSize: 1, @@ -874,17 +892,228 @@ describe('LdapService', () => { }); }); - describe('runSync()', () => { - it.todo('should search for users with expected parameters'); - it.todo('should resolve binary attributes'); - it.todo('should throw expected error if search fails'); - it.todo('should process users if mode is "live"'); + describe.only('runSync()', () => { + it('should search for users with expected parameters', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const searchWithAdminBindingSpy = jest.spyOn(ldapService, 'searchWithAdminBinding'); + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + const expectedParameter = createFilter( + `(${ldapConfig.loginIdAttribute}=*)`, + ldapConfig.userFilter, + ); + + await ldapService.init(); + await ldapService.runSync('dry'); + + expect(searchWithAdminBindingSpy).toHaveBeenCalledTimes(1); + expect(searchWithAdminBindingSpy).toHaveBeenCalledWith(expectedParameter); + }); + + it('should resolve binary attributes for users', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const foundUsers = [ + { + dn: 'uid=jdoe,ou=users,dc=example,dc=com', + cn: ['John Doe'], + mail: ['jdoe@example.com'], + uid: ['jdoe'], + jpegPhoto: [Buffer.from('89504E470D0A1A0A', 'hex')], + }, + ]; + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: foundUsers }); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + await ldapService.init(); + await ldapService.runSync('dry'); + + expect(resolveBinaryAttributes).toHaveBeenCalledTimes(1); + expect(resolveBinaryAttributes).toHaveBeenCalledWith(foundUsers); + }); + + it('should throw expected error if search fails', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + Client.prototype.search = jest.fn().mockRejectedValue(new Error('Error finding users')); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + await ldapService.init(); + await expect(ldapService.runSync('dry')).rejects.toThrowError('Error finding users'); + }); + + it.skip('should process users if mode is "live"', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify({ ldapConfig }), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const foundUsers = [ + // New user + { + dn: 'uid=jdoe,ou=users,dc=example,dc=com', + cn: ['John Doe'], + givenName: 'John', + sn: 'Doe', + mail: ['jdoe@example.com'], + uid: ['jdoe'], + }, + // Existing user + // User to delete + ]; + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: foundUsers }); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + const createDatabaseUser = mapLdapUserToDbUser(foundUsers[0], ldapConfig, true); + + await ldapService.init(); + await ldapService.runSync('live'); + + expect(processUsers).toHaveBeenCalledTimes(1); + expect(processUsers).toHaveBeenCalledWith([createDatabaseUser], [], []); + }); + it.todo('should write expected data to the database'); it.todo( 'should write expected data to the database with an error message if processing users fails', ); - it.todo('should emit expected event if synchronization is enabled'); - it.todo('should emit expected event if synchronization is disabled'); + + it('should emit expected event if synchronization is enabled', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const eventServiceMock = mock({ + emit: jest.fn(), + }); + + const ldapService = new LdapService( + mockLogger(), + settingsRepository, + mock(), + eventServiceMock, + ); + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + await ldapService.init(); + await ldapService.runSync('dry'); + + expect(eventServiceMock.emit).toHaveBeenCalledTimes(1); + expect(eventServiceMock.emit).toHaveBeenCalledWith('ldap-general-sync-finished', { + error: '', + succeeded: true, + type: 'manual_dry', + usersSynced: 0, + }); + }); + + it('should emit expected event if synchronization is disabled', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const eventServiceMock = mock({ + emit: jest.fn(), + }); + + const ldapService = new LdapService( + mockLogger(), + settingsRepository, + mock(), + eventServiceMock, + ); + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + await ldapService.init(); + ldapService.stopSync(); + await ldapService.runSync('dry'); + + expect(eventServiceMock.emit).toHaveBeenCalledTimes(1); + expect(eventServiceMock.emit).toHaveBeenCalledWith('ldap-general-sync-finished', { + error: '', + succeeded: true, + type: 'scheduled', + usersSynced: 0, + }); + }); + + it('should emit expected event if processUsers fails', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const eventServiceMock = mock({ + emit: jest.fn(), + }); + + const ldapService = new LdapService( + mockLogger(), + settingsRepository, + mock(), + eventServiceMock, + ); + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + const mockedProcessUsers = processUsers as jest.Mock; + mockedProcessUsers.mockRejectedValue( + new QueryFailedError('Query', [], new Error('Error processing users')), + ); + + await ldapService.init(); + ldapService.stopSync(); + await ldapService.runSync('live'); + + expect(mockedProcessUsers).toHaveBeenCalledTimes(1); + expect(eventServiceMock.emit).toHaveBeenCalledTimes(1); + expect(eventServiceMock.emit).toHaveBeenCalledWith('ldap-general-sync-finished', { + error: 'Error processing users', + succeeded: true, + type: 'scheduled', + usersSynced: 0, + }); + }); }); describe('stopSync()', () => { From 4e1f8ae30f2f2dff05ddea97b876af9733a08b73 Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Mon, 9 Dec 2024 11:15:12 +0000 Subject: [PATCH 11/16] tests: final ldap tests --- .../ldap.ee/__tests__/ldap.service.test.ts | 231 ++++++++++++++++-- 1 file changed, 210 insertions(+), 21 deletions(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index 9ae1179559..20d179b425 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -2,6 +2,7 @@ import { QueryFailedError } from '@n8n/typeorm'; import { mock } from 'jest-mock-extended'; import { Client } from 'ldapts'; import type { Cipher } from 'n8n-core'; +import { randomString } from 'n8n-workflow'; import config from '@/config'; import { AuthIdentityRepository } from '@/databases/repositories/auth-identity.repository'; @@ -23,6 +24,7 @@ import { resolveBinaryAttributes, processUsers, mapLdapUserToDbUser, + saveLdapSynchronization, } from '../helpers.ee'; // Mock ldapts client @@ -45,6 +47,11 @@ jest.mock('../helpers.ee', () => ({ processUsers: jest.fn(), })); +jest.mock('n8n-workflow', () => ({ + ...jest.requireActual('n8n-workflow'), + randomString: jest.fn(), +})); + describe('LdapService', () => { const ldapConfig: LdapConfig = { loginEnabled: true, @@ -892,7 +899,12 @@ describe('LdapService', () => { }); }); - describe.only('runSync()', () => { + describe('runSync()', () => { + beforeEach(() => { + const mockedRandomString = randomString as jest.Mock; + mockedRandomString.mockReturnValue('nonRandomPassword'); + }); + it('should search for users with expected parameters', async () => { const settingsRepository = mock({ findOneByOrFail: jest.fn().mockResolvedValue({ @@ -907,7 +919,7 @@ describe('LdapService', () => { const mockedGetLdapIds = getLdapIds as jest.Mock; mockedGetLdapIds.mockResolvedValue([]); - const expectedParameter = createFilter( + const expectedFilter = createFilter( `(${ldapConfig.loginIdAttribute}=*)`, ldapConfig.userFilter, ); @@ -916,7 +928,7 @@ describe('LdapService', () => { await ldapService.runSync('dry'); expect(searchWithAdminBindingSpy).toHaveBeenCalledTimes(1); - expect(searchWithAdminBindingSpy).toHaveBeenCalledWith(expectedParameter); + expect(searchWithAdminBindingSpy).toHaveBeenCalledWith(expectedFilter); }); it('should resolve binary attributes for users', async () => { @@ -965,45 +977,222 @@ describe('LdapService', () => { await expect(ldapService.runSync('dry')).rejects.toThrowError('Error finding users'); }); - it.skip('should process users if mode is "live"', async () => { + it('should process expected users if mode is "live"', async () => { const settingsRepository = mock({ findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify({ ldapConfig }), + value: JSON.stringify(ldapConfig), }), }); const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); - const foundUsers = [ - // New user + + // Users that don't exist in memory + const newUsers = [ { - dn: 'uid=jdoe,ou=users,dc=example,dc=com', - cn: ['John Doe'], + dn: 'uid=johndoe,ou=users,dc=example,dc=com', + cn: 'John Doe', givenName: 'John', sn: 'Doe', - mail: ['jdoe@example.com'], - uid: ['jdoe'], + mail: 'john.doe@example.com', + uid: 'johndoe', + }, + { + dn: 'uid=janedoe,ou=users,dc=example,dc=com', + cn: 'Jane Doe', + givenName: 'Jane', + sn: 'Doe', + mail: 'jane.doe@example.com', + uid: 'janedoe', }, - // Existing user - // User to delete ]; + + // Users that exist in memory and in LDAP response + const updateUsers = [ + { + dn: 'uid=emilyclark,ou=users,dc=example,dc=com', + cn: 'Emily Clark', + givenName: 'Emily', + sn: 'Clark', + mail: 'emily.clark@example.com', + uid: 'emilyclark', + }, + ]; + + // Users that only exist in memory + const deleteUsers = ['santaclaus', 'jackfrost']; + + const foundUsers = [...newUsers, ...updateUsers]; Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: foundUsers }); const mockedGetLdapIds = getLdapIds as jest.Mock; - mockedGetLdapIds.mockResolvedValue([]); - const createDatabaseUser = mapLdapUserToDbUser(foundUsers[0], ldapConfig, true); + // Delete users that exist in memory but not in the LDAP response + mockedGetLdapIds.mockResolvedValue(['emilyclark', ...deleteUsers]); + + const newDbUsers = newUsers.map((user) => mapLdapUserToDbUser(user, ldapConfig, true)); + const updateDbUsers = updateUsers.map((user) => mapLdapUserToDbUser(user, ldapConfig)); await ldapService.init(); await ldapService.runSync('live'); expect(processUsers).toHaveBeenCalledTimes(1); - expect(processUsers).toHaveBeenCalledWith([createDatabaseUser], [], []); + expect(processUsers).toHaveBeenCalledWith(newDbUsers, updateDbUsers, deleteUsers); }); - it.todo('should write expected data to the database'); - it.todo( - 'should write expected data to the database with an error message if processing users fails', - ); + it('should sync expected LDAP data when no errors', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + // Users that don't exist in memory + const newUsers = [ + { + dn: 'uid=johndoe,ou=users,dc=example,dc=com', + cn: 'John Doe', + givenName: 'John', + sn: 'Doe', + mail: 'john.doe@example.com', + uid: 'johndoe', + }, + { + dn: 'uid=janedoe,ou=users,dc=example,dc=com', + cn: 'Jane Doe', + givenName: 'Jane', + sn: 'Doe', + mail: 'jane.doe@example.com', + uid: 'janedoe', + }, + ]; + + // Users that exist in memory and in LDAP response + const updateUsers = [ + { + dn: 'uid=emilyclark,ou=users,dc=example,dc=com', + cn: 'Emily Clark', + givenName: 'Emily', + sn: 'Clark', + mail: 'emily.clark@example.com', + uid: 'emilyclark', + }, + ]; + + // Users that only exist in memory + const deleteUsers = ['santaclaus', 'jackfrost']; + + const foundUsers = [...newUsers, ...updateUsers]; + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: foundUsers }); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + + // Delete users that exist in memory but not in the LDAP response + mockedGetLdapIds.mockResolvedValue(['emilyclark', ...deleteUsers]); + + const newDbUsers = newUsers.map((user) => mapLdapUserToDbUser(user, ldapConfig, true)); + const updateDbUsers = updateUsers.map((user) => mapLdapUserToDbUser(user, ldapConfig)); + + jest.setSystemTime(new Date('2024-12-25')); + const expectedDate = new Date(); + + await ldapService.init(); + await ldapService.runSync('live'); + + expect(saveLdapSynchronization).toHaveBeenCalledTimes(1); + expect(saveLdapSynchronization).toHaveBeenCalledWith({ + startedAt: expectedDate, + endedAt: expectedDate, + created: newDbUsers.length, + updated: updateDbUsers.length, + disabled: deleteUsers.length, + scanned: foundUsers.length, + runMode: 'live', + status: 'success', + error: '', + }); + }); + + it('should sync expected LDAP data when users fail to process', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + // Users that don't exist in memory + const newUsers = [ + { + dn: 'uid=johndoe,ou=users,dc=example,dc=com', + cn: 'John Doe', + givenName: 'John', + sn: 'Doe', + mail: 'john.doe@example.com', + uid: 'johndoe', + }, + { + dn: 'uid=janedoe,ou=users,dc=example,dc=com', + cn: 'Jane Doe', + givenName: 'Jane', + sn: 'Doe', + mail: 'jane.doe@example.com', + uid: 'janedoe', + }, + ]; + + // Users that exist in memory and in LDAP response + const updateUsers = [ + { + dn: 'uid=emilyclark,ou=users,dc=example,dc=com', + cn: 'Emily Clark', + givenName: 'Emily', + sn: 'Clark', + mail: 'emily.clark@example.com', + uid: 'emilyclark', + }, + ]; + + // Users that only exist in memory + const deleteUsers = ['santaclaus', 'jackfrost']; + + const foundUsers = [...newUsers, ...updateUsers]; + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: foundUsers }); + + const mockedProcessUsers = processUsers as jest.Mock; + mockedProcessUsers.mockRejectedValue( + new QueryFailedError('Query', [], new Error('Error processing users')), + ); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + + // Delete users that exist in memory but not in the LDAP response + mockedGetLdapIds.mockResolvedValue(['emilyclark', ...deleteUsers]); + + const newDbUsers = newUsers.map((user) => mapLdapUserToDbUser(user, ldapConfig, true)); + const updateDbUsers = updateUsers.map((user) => mapLdapUserToDbUser(user, ldapConfig)); + + jest.setSystemTime(new Date('2024-12-25')); + const expectedDate = new Date(); + + await ldapService.init(); + await ldapService.runSync('live'); + + expect(saveLdapSynchronization).toHaveBeenCalledTimes(1); + expect(saveLdapSynchronization).toHaveBeenCalledWith({ + startedAt: expectedDate, + endedAt: expectedDate, + created: newDbUsers.length, + updated: updateDbUsers.length, + disabled: deleteUsers.length, + scanned: foundUsers.length, + runMode: 'live', + status: 'error', + error: 'Error processing users', + }); + }); it('should emit expected event if synchronization is enabled', async () => { const settingsRepository = mock({ @@ -1074,7 +1263,7 @@ describe('LdapService', () => { }); }); - it('should emit expected event if processUsers fails', async () => { + it('should emit expected event with error message if processUsers fails', async () => { const settingsRepository = mock({ findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig), From f07bc7939a75ab1bf1f2fa55e96530dc407e2073 Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Mon, 9 Dec 2024 12:07:45 +0000 Subject: [PATCH 12/16] test: add tests for findAndAuthenticateLdapUser --- .../ldap.ee/__tests__/ldap.service.test.ts | 277 +++++++++++++++++- 1 file changed, 275 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index 20d179b425..b121b1b137 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -21,10 +21,12 @@ import { mockInstance, mockLogger } from '@test/mocking'; import { getLdapIds, createFilter, + escapeFilter, resolveBinaryAttributes, processUsers, mapLdapUserToDbUser, saveLdapSynchronization, + resolveEntryBinaryAttributes, } from '../helpers.ee'; // Mock ldapts client @@ -45,6 +47,7 @@ jest.mock('../helpers.ee', () => ({ saveLdapSynchronization: jest.fn(), resolveBinaryAttributes: jest.fn(), processUsers: jest.fn(), + resolveEntryBinaryAttributes: jest.fn(), })); jest.mock('n8n-workflow', () => ({ @@ -772,7 +775,277 @@ describe('LdapService', () => { }); }); - describe.skip('findAndAuthenticateLdapUser()', () => {}); + describe('findAndAuthenticateLdapUser()', () => { + it('should search for expected admin login ID', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const searchWithAdminBindingSpy = jest.spyOn(ldapService, 'searchWithAdminBinding'); + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + const expectedFilter = createFilter( + `(${ldapConfig.loginIdAttribute}=${escapeFilter('jdoe')})`, + ldapConfig.userFilter, + ); + + await ldapService.init(); + await ldapService.findAndAuthenticateLdapUser( + 'jdoe', + 'fakePassword', + ldapConfig.loginIdAttribute, + ldapConfig.userFilter, + ); + + expect(searchWithAdminBindingSpy).toHaveBeenCalledTimes(1); + expect(searchWithAdminBindingSpy).toHaveBeenCalledWith(expectedFilter); + }); + + it('should emit expected error if admin search fails', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const eventServiceMock = mock({ + emit: jest.fn(), + }); + + const ldapService = new LdapService( + mockLogger(), + settingsRepository, + mock(), + eventServiceMock, + ); + Client.prototype.search = jest.fn().mockRejectedValue(new Error('Failed to find admin user')); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + await ldapService.init(); + const result = await ldapService.findAndAuthenticateLdapUser( + 'jdoe', + 'fakePassword', + ldapConfig.loginIdAttribute, + ldapConfig.userFilter, + ); + + expect(eventServiceMock.emit).toBeCalledTimes(1); + expect(eventServiceMock.emit).toHaveBeenCalledWith('ldap-login-sync-failed', { + error: 'Failed to find admin user', + }); + expect(result).toBeUndefined(); + }); + + it('should return undefined if no user is found', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + await ldapService.init(); + const result = await ldapService.findAndAuthenticateLdapUser( + 'jdoe', + 'fakePassword', + ldapConfig.loginIdAttribute, + ldapConfig.userFilter, + ); + + expect(result).toBeUndefined(); + }); + + it('should validate found user', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + const foundUsers = [ + { + dn: 'uid=jdoe,ou=users,dc=example,dc=com', + cn: ['John Doe'], + mail: ['jdoe@example.com'], + uid: ['jdoe'], + }, + ]; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [...foundUsers] }); + + const validUserSpy = jest.spyOn(ldapService, 'validUser'); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + await ldapService.init(); + await ldapService.findAndAuthenticateLdapUser( + 'jdoe', + 'fakePassword', + ldapConfig.loginIdAttribute, + ldapConfig.userFilter, + ); + + expect(validUserSpy).toBeCalledTimes(1); + expect(validUserSpy).toHaveBeenCalledWith(foundUsers[0].dn, 'fakePassword'); + }); + + it('should validate last user if more than one is found', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + const foundUsers = [ + { + dn: 'uid=jdoe,ou=users,dc=example,dc=com', + cn: ['John Doe'], + mail: ['jdoe@example.com'], + uid: ['jdoe'], + }, + { + dn: 'uid=janedoe,ou=users,dc=example,dc=com', + cn: ['Jane Doe'], + mail: ['jane.doe@example.com'], + uid: ['janedoe'], + }, + ]; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [...foundUsers] }); + + const validUserSpy = jest.spyOn(ldapService, 'validUser'); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + await ldapService.init(); + await ldapService.findAndAuthenticateLdapUser( + 'jdoe', + 'fakePassword', + ldapConfig.loginIdAttribute, + ldapConfig.userFilter, + ); + + expect(validUserSpy).toBeCalledTimes(1); + expect(validUserSpy).toHaveBeenCalledWith(foundUsers[1].dn, 'fakePassword'); + }); + + it('should return undefined if invalid user is found', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + const foundUsers = [ + { + dn: 'uid=jdoe,ou=users,dc=example,dc=com', + cn: ['John Doe'], + mail: ['jdoe@example.com'], + uid: ['jdoe'], + }, + ]; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [...foundUsers] }); + + const validUserSpy = jest + .spyOn(ldapService, 'validUser') + .mockRejectedValue(new Error('Failed to validate user')); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + await ldapService.init(); + const result = await ldapService.findAndAuthenticateLdapUser( + 'jdoe', + 'fakePassword', + ldapConfig.loginIdAttribute, + ldapConfig.userFilter, + ); + + expect(validUserSpy).toHaveBeenCalledTimes(1); + expect(result).toBeUndefined(); + }); + + it('should resolve binary attributes for found user', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + const foundUsers = [ + { + dn: 'uid=jdoe,ou=users,dc=example,dc=com', + cn: ['John Doe'], + mail: ['jdoe@example.com'], + uid: ['jdoe'], + }, + ]; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [...foundUsers] }); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + await ldapService.init(); + await ldapService.findAndAuthenticateLdapUser( + 'jdoe', + 'fakePassword', + ldapConfig.loginIdAttribute, + ldapConfig.userFilter, + ); + + expect(resolveEntryBinaryAttributes).toHaveBeenCalledTimes(1); + expect(resolveEntryBinaryAttributes).toHaveBeenCalledWith(foundUsers[0]); + }); + + it('should return found user', async () => { + const settingsRepository = mock({ + findOneByOrFail: jest.fn().mockResolvedValue({ + value: JSON.stringify(ldapConfig), + }), + }); + const foundUsers = [ + { + dn: 'uid=jdoe,ou=users,dc=example,dc=com', + cn: ['John Doe'], + mail: ['jdoe@example.com'], + uid: ['jdoe'], + }, + ]; + + const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [...foundUsers] }); + + const mockedGetLdapIds = getLdapIds as jest.Mock; + mockedGetLdapIds.mockResolvedValue([]); + + await ldapService.init(); + const result = await ldapService.findAndAuthenticateLdapUser( + 'jdoe', + 'fakePassword', + ldapConfig.loginIdAttribute, + ldapConfig.userFilter, + ); + + expect(result).toEqual(foundUsers[0]); + }); + }); describe('testConnection()', () => { it('should throw expected error if init() is not called first', async () => { @@ -1322,7 +1595,7 @@ describe('LdapService', () => { const clearIntervalSpy = jest.spyOn(global, 'clearInterval'); await ldapService.init(); - await ldapService.stopSync(); + ldapService.stopSync(); expect(clearIntervalSpy).toHaveBeenCalledTimes(1); }); From a79a1bf4e20c05688e54be59de20f0aa1ba1efe0 Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Tue, 10 Dec 2024 12:27:06 +0000 Subject: [PATCH 13/16] test: refactor to simplify service setup --- .../ldap.ee/__tests__/ldap.service.test.ts | 433 +++++------------- 1 file changed, 119 insertions(+), 314 deletions(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index b121b1b137..167985b4e1 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -5,6 +5,7 @@ import type { Cipher } from 'n8n-core'; import { randomString } from 'n8n-workflow'; import config from '@/config'; +import type { Settings } from '@/databases/entities/settings'; import { AuthIdentityRepository } from '@/databases/repositories/auth-identity.repository'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; import type { EventService } from '@/events/event.service'; @@ -78,6 +79,8 @@ describe('LdapService', () => { searchTimeout: 6, }; + let settingsRepository = mockInstance(SettingsRepository); + beforeAll(() => { // Need fake timers to avoid setInterval // problems with the scheduled sync @@ -88,13 +91,17 @@ describe('LdapService', () => { jest.restoreAllMocks(); }); + const createDefaultLdapService = (config: LdapConfig) => { + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(config), + } as Settings); + + return new LdapService(mockLogger(), settingsRepository, mock(), mock()); + }; + describe('init()', () => { it('should load the LDAP configuration', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), - }); - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService(ldapConfig); const loadConfigSpy = jest.spyOn(ldapService, 'loadConfig'); @@ -104,11 +111,7 @@ describe('LdapService', () => { }); it('should set expected configuration variables from LDAP config if LDAP is enabled', async () => { - const settingsRepository = mockInstance(SettingsRepository, { - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), - }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService(ldapConfig); const configSetSpy = jest.spyOn(config, 'set'); @@ -126,11 +129,7 @@ describe('LdapService', () => { it('should set expected configuration variables from LDAP config if LDAP is disabled', async () => { const givenConfig = { ...ldapConfig, loginEnabled: false }; - const settingsRepository = mockInstance(SettingsRepository, { - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(givenConfig) }), - }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService(givenConfig); const configSetSpy = jest.spyOn(config, 'set'); @@ -146,9 +145,9 @@ describe('LdapService', () => { }); it('should show logger warning if authentication method is not ldap or email', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), - }); + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const logger = mockLogger(); @@ -172,11 +171,7 @@ describe('LdapService', () => { synchronizationInterval: 10, }; - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(givenConfig) }), - }); - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService(givenConfig); const setIntervalSpy = jest.spyOn(global, 'setInterval'); @@ -190,18 +185,12 @@ describe('LdapService', () => { }); it('should throw an error if config has enabled synchronization but no synchronizationInterval is set', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify({ - ...ldapConfig, - synchronizationEnabled: true, - synchronizationInterval: 0, - }), - }), + const ldapService = createDefaultLdapService({ + ...ldapConfig, + synchronizationEnabled: true, + synchronizationInterval: 0, }); - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); - const setIntervalSpy = jest.spyOn(global, 'setInterval'); await expect(ldapService.init()).rejects.toThrowError('Interval variable has to be defined'); @@ -211,13 +200,7 @@ describe('LdapService', () => { describe('loadConfig()', () => { it('should retrieve the LDAP configuration from the settings repository', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService(ldapConfig); await ldapService.loadConfig(); @@ -225,9 +208,9 @@ describe('LdapService', () => { }); it('should throw an expected error if the LDAP configuration is not found', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockRejectedValue(new Error('LDAP configuration not found')), - }); + settingsRepository.findOneByOrFail.mockRejectedValue( + new Error('LDAP configuration not found'), + ); const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); @@ -235,11 +218,9 @@ describe('LdapService', () => { }); it('should decipher the LDAP configuration admin password', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const cipherMock = mock({ decrypt: jest.fn(), @@ -254,11 +235,9 @@ describe('LdapService', () => { }); it('should return the expected LDAP configuration', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const cipherMock = mock({ decrypt: jest.fn().mockReturnValue('decryptedPassword'), @@ -274,26 +253,19 @@ describe('LdapService', () => { describe('updateConfig()', () => { it('should throw expected error if the LDAP configuration is invalid', async () => { - const settingsRepository = mockInstance(SettingsRepository, { - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), - }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + const ldapService = createDefaultLdapService(ldapConfig); const invalidLdapConfig = { ...ldapConfig, loginEnabled: 'notABoolean' }; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); - await expect( ldapService.updateConfig(invalidLdapConfig as unknown as LdapConfig), ).rejects.toThrowError('request.body.loginEnabled is not of a type(s) boolean'); }); it('should throw expected error if login is enabled and the current authentication method is "saml"', async () => { - const settingsRepository = mockInstance(SettingsRepository, { - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), - }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI - config.set('userManagement.authenticationMethod', 'saml'); - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + + const ldapService = createDefaultLdapService(ldapConfig); await expect(ldapService.updateConfig(ldapConfig)).rejects.toThrowError( 'LDAP cannot be enabled if SSO in enabled', @@ -301,9 +273,9 @@ describe('LdapService', () => { }); it('should encrypt the binding admin password', async () => { - const settingsRepository = mockInstance(SettingsRepository, { - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), - }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const cipherMock = mock({ encrypt: jest.fn().mockReturnValue('encryptedPassword'), @@ -321,9 +293,9 @@ describe('LdapService', () => { }); it('should delete all ldap identities if login is disabled and ldap users exist', async () => { - const settingsRepository = mockInstance(SettingsRepository, { - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), - }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const authIdentityRepository = mockInstance(AuthIdentityRepository, { find: jest.fn().mockResolvedValue([{ user: { id: 'userId' } }]), @@ -346,9 +318,9 @@ describe('LdapService', () => { }); it('should not delete ldap identities if login is disabled and there are no ldap identities', async () => { - const settingsRepository = mockInstance(SettingsRepository, { - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), - }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const authIdentityRepository = mockInstance(AuthIdentityRepository, { find: jest.fn().mockResolvedValue([]), @@ -371,10 +343,9 @@ describe('LdapService', () => { }); it('should update the LDAP configuration in the settings repository', async () => { - const settingsRepository = mockInstance(SettingsRepository, { - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), - update: jest.fn(), - }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); mockInstance(AuthIdentityRepository, { find: jest.fn().mockResolvedValue([{ user: { id: 'userId' } }]), @@ -399,10 +370,9 @@ describe('LdapService', () => { }); it('should update the LDAP login label in the config', async () => { - const settingsRepository = mockInstance(SettingsRepository, { - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(ldapConfig) }), - update: jest.fn(), - }); // set in container so `setCurrentAuthenticationMethod` does not fail - legacy LDAP code not using DI + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); mockInstance(AuthIdentityRepository, { find: jest.fn().mockResolvedValue([{ user: { id: 'userId' } }]), @@ -431,15 +401,10 @@ describe('LdapService', () => { describe('setConfig()', () => { it('should stop synchronization if the timer is running and the config is disabled', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); const updatedLdapConfig = { ...ldapConfig, synchronizationEnabled: false }; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); const clearIntervalSpy = jest.spyOn(global, 'clearInterval'); await ldapService.init(); @@ -449,11 +414,7 @@ describe('LdapService', () => { }); it('should schedule synchronization if the timer is not running and the config is enabled', () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); const updatedLdapConfig = { ...ldapConfig, @@ -461,7 +422,6 @@ describe('LdapService', () => { synchronizationInterval: 999, }; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); const clearIntervalSpy = jest.spyOn(global, 'clearInterval'); const setIntervalSpy = jest.spyOn(global, 'setInterval'); @@ -476,11 +436,7 @@ describe('LdapService', () => { }); it('should throw an error if the timer is not running and the config is enabled but the synchronizationInterval is not set', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); const updatedLdapConfig = { ...ldapConfig, @@ -488,7 +444,6 @@ describe('LdapService', () => { synchronizationInterval: 0, }; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); const clearIntervalSpy = jest.spyOn(global, 'clearInterval'); const setIntervalSpy = jest.spyOn(global, 'setInterval'); @@ -500,11 +455,7 @@ describe('LdapService', () => { }); it('should restart synchronization if the timer is running and the config is enabled', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); const updatedLdapConfig = { ...ldapConfig, @@ -512,7 +463,6 @@ describe('LdapService', () => { synchronizationInterval: 1234, }; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); const clearIntervalSpy = jest.spyOn(global, 'clearInterval'); const setIntervalSpy = jest.spyOn(global, 'setInterval'); @@ -536,11 +486,9 @@ describe('LdapService', () => { describe('searchWithAdminBinding()', () => { it('should bind admin client', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const cipherMock = mock({ decrypt: jest.fn().mockReturnValue('decryptedPassword'), @@ -563,11 +511,9 @@ describe('LdapService', () => { }); it('should call client search with expected parameters', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const cipherMock = mock({ decrypt: jest.fn().mockReturnValue('decryptedPassword'), @@ -599,11 +545,9 @@ describe('LdapService', () => { }); it('should call client search with expected parameters when searchPageSize is 0', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify({ ...ldapConfig, searchPageSize: 0 }), - }), - }); + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify({ ...ldapConfig, searchPageSize: 0 }), + } as Settings); const cipherMock = mock({ decrypt: jest.fn().mockReturnValue('decryptedPassword'), @@ -635,11 +579,9 @@ describe('LdapService', () => { }); it('should unbind client after search', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const cipherMock = mock({ decrypt: jest.fn().mockReturnValue('decryptedPassword'), @@ -658,11 +600,9 @@ describe('LdapService', () => { }); it('should return expected search entries', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const cipherMock = mock({ decrypt: jest.fn().mockReturnValue('decryptedPassword'), @@ -701,13 +641,7 @@ describe('LdapService', () => { describe('validUser()', () => { it('should throw expected error if no configuration has been set', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService(ldapConfig); await expect(ldapService.validUser('dn', 'password')).rejects.toThrowError( 'Service cannot be used without setting the property config', @@ -715,17 +649,11 @@ describe('LdapService', () => { }); it('should bind the ldap client with the expected distinguished name and password', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); const distinguishedName = 'uid=jdoe,ou=users,dc=example,dc=com'; const password = 'password'; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); - await ldapService.init(); await ldapService.validUser(distinguishedName, password); @@ -734,17 +662,11 @@ describe('LdapService', () => { }); it('should throw expected error if binding fails', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); const distinguishedName = 'uid=jdoe,ou=users,dc=example,dc=com'; const password = 'password'; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); - Client.prototype.bind = jest .fn() .mockRejectedValue(new Error('Error validating user against LDAP server')); @@ -757,17 +679,11 @@ describe('LdapService', () => { }); it('should unbind the client binding', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); const distinguishedName = 'uid=jdoe,ou=users,dc=example,dc=com'; const password = 'password'; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); - await ldapService.init(); await ldapService.validUser(distinguishedName, password); @@ -777,13 +693,8 @@ describe('LdapService', () => { describe('findAndAuthenticateLdapUser()', () => { it('should search for expected admin login ID', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); const searchWithAdminBindingSpy = jest.spyOn(ldapService, 'searchWithAdminBinding'); Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); @@ -808,11 +719,9 @@ describe('LdapService', () => { }); it('should emit expected error if admin search fails', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const eventServiceMock = mock({ emit: jest.fn(), @@ -845,13 +754,8 @@ describe('LdapService', () => { }); it('should return undefined if no user is found', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); const mockedGetLdapIds = getLdapIds as jest.Mock; @@ -869,11 +773,8 @@ describe('LdapService', () => { }); it('should validate found user', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); + const foundUsers = [ { dn: 'uid=jdoe,ou=users,dc=example,dc=com', @@ -883,7 +784,6 @@ describe('LdapService', () => { }, ]; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [...foundUsers] }); const validUserSpy = jest.spyOn(ldapService, 'validUser'); @@ -904,11 +804,8 @@ describe('LdapService', () => { }); it('should validate last user if more than one is found', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); + const foundUsers = [ { dn: 'uid=jdoe,ou=users,dc=example,dc=com', @@ -924,7 +821,6 @@ describe('LdapService', () => { }, ]; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [...foundUsers] }); const validUserSpy = jest.spyOn(ldapService, 'validUser'); @@ -945,11 +841,8 @@ describe('LdapService', () => { }); it('should return undefined if invalid user is found', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); + const foundUsers = [ { dn: 'uid=jdoe,ou=users,dc=example,dc=com', @@ -959,7 +852,6 @@ describe('LdapService', () => { }, ]; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [...foundUsers] }); const validUserSpy = jest @@ -982,11 +874,8 @@ describe('LdapService', () => { }); it('should resolve binary attributes for found user', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); + const foundUsers = [ { dn: 'uid=jdoe,ou=users,dc=example,dc=com', @@ -996,7 +885,6 @@ describe('LdapService', () => { }, ]; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [...foundUsers] }); const mockedGetLdapIds = getLdapIds as jest.Mock; @@ -1015,11 +903,8 @@ describe('LdapService', () => { }); it('should return found user', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); + const foundUsers = [ { dn: 'uid=jdoe,ou=users,dc=example,dc=com', @@ -1029,7 +914,6 @@ describe('LdapService', () => { }, ]; - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [...foundUsers] }); const mockedGetLdapIds = getLdapIds as jest.Mock; @@ -1049,13 +933,7 @@ describe('LdapService', () => { describe('testConnection()', () => { it('should throw expected error if init() is not called first', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService(ldapConfig); await expect(ldapService.testConnection()).rejects.toThrowError( 'Service cannot be used without setting the property config', @@ -1063,13 +941,7 @@ describe('LdapService', () => { }); it('should create a new client without TLS if connectionSecurity is set to "none"', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify({ ...ldapConfig, connectionSecurity: 'none' }), - }), - }); - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService({ ...ldapConfig, connectionSecurity: 'none' }); await ldapService.init(); await ldapService.testConnection(); @@ -1081,18 +953,12 @@ describe('LdapService', () => { }); it('should create a new client with TLS enabled if connectionSecurity is set to "tls" and allowing unauthorized certificates', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify({ - ...ldapConfig, - connectionSecurity: 'tls', - allowUnauthorizedCerts: true, - }), - }), + const ldapService = createDefaultLdapService({ + ...ldapConfig, + connectionSecurity: 'tls', + allowUnauthorizedCerts: true, }); - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); - await ldapService.init(); await ldapService.testConnection(); @@ -1106,18 +972,12 @@ describe('LdapService', () => { }); it('should create a new client with TLS enabled if connectionSecurity is set to "tls" and not allowing unauthorized certificates', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify({ - ...ldapConfig, - connectionSecurity: 'tls', - allowUnauthorizedCerts: false, - }), - }), + const ldapService = createDefaultLdapService({ + ...ldapConfig, + connectionSecurity: 'tls', + allowUnauthorizedCerts: false, }); - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); - await ldapService.init(); await ldapService.testConnection(); @@ -1131,18 +991,12 @@ describe('LdapService', () => { }); it('should create a new client and start TLS if connectionSecurity is set to "startTls"', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify({ - ...ldapConfig, - connectionSecurity: 'startTls', - allowUnauthorizedCerts: true, - }), - }), + const ldapService = createDefaultLdapService({ + ...ldapConfig, + connectionSecurity: 'startTls', + allowUnauthorizedCerts: true, }); - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); - await ldapService.init(); await ldapService.testConnection(); @@ -1154,13 +1008,7 @@ describe('LdapService', () => { }); it('should not create a new client if one has already been created', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService(ldapConfig); await ldapService.init(); await ldapService.testConnection(); @@ -1179,13 +1027,8 @@ describe('LdapService', () => { }); it('should search for users with expected parameters', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); const searchWithAdminBindingSpy = jest.spyOn(ldapService, 'searchWithAdminBinding'); Client.prototype.search = jest.fn().mockResolvedValue({ searchEntries: [] }); @@ -1205,13 +1048,8 @@ describe('LdapService', () => { }); it('should resolve binary attributes for users', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); const foundUsers = [ { dn: 'uid=jdoe,ou=users,dc=example,dc=com', @@ -1234,13 +1072,8 @@ describe('LdapService', () => { }); it('should throw expected error if search fails', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + const ldapService = createDefaultLdapService(ldapConfig); - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); Client.prototype.search = jest.fn().mockRejectedValue(new Error('Error finding users')); const mockedGetLdapIds = getLdapIds as jest.Mock; @@ -1251,13 +1084,7 @@ describe('LdapService', () => { }); it('should process expected users if mode is "live"', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService(ldapConfig); // Users that don't exist in memory const newUsers = [ @@ -1313,13 +1140,7 @@ describe('LdapService', () => { }); it('should sync expected LDAP data when no errors', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService(ldapConfig); // Users that don't exist in memory const newUsers = [ @@ -1388,13 +1209,7 @@ describe('LdapService', () => { }); it('should sync expected LDAP data when users fail to process', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService(ldapConfig); // Users that don't exist in memory const newUsers = [ @@ -1468,11 +1283,9 @@ describe('LdapService', () => { }); it('should emit expected event if synchronization is enabled', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const eventServiceMock = mock({ emit: jest.fn(), @@ -1502,11 +1315,9 @@ describe('LdapService', () => { }); it('should emit expected event if synchronization is disabled', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const eventServiceMock = mock({ emit: jest.fn(), @@ -1537,11 +1348,9 @@ describe('LdapService', () => { }); it('should emit expected event with error message if processUsers fails', async () => { - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ - value: JSON.stringify(ldapConfig), - }), - }); + settingsRepository.findOneByOrFail.mockResolvedValueOnce({ + value: JSON.stringify(ldapConfig), + } as Settings); const eventServiceMock = mock({ emit: jest.fn(), @@ -1586,11 +1395,7 @@ describe('LdapService', () => { synchronizationInterval: 10, }; - const settingsRepository = mock({ - findOneByOrFail: jest.fn().mockResolvedValue({ value: JSON.stringify(givenConfig) }), - }); - - const ldapService = new LdapService(mockLogger(), settingsRepository, mock(), mock()); + const ldapService = createDefaultLdapService(givenConfig); const clearIntervalSpy = jest.spyOn(global, 'clearInterval'); From cf0a23838e74d2a8959e50dde74cb7ebfadefa10 Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Tue, 10 Dec 2024 12:39:43 +0000 Subject: [PATCH 14/16] test: remove unnecessary variable --- packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index 167985b4e1..07abc0289e 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -1389,13 +1389,11 @@ describe('LdapService', () => { describe('stopSync()', () => { it('should clear the scheduled timer', async () => { - const givenConfig = { + const ldapService = createDefaultLdapService({ ...ldapConfig, synchronizationEnabled: true, synchronizationInterval: 10, - }; - - const ldapService = createDefaultLdapService(givenConfig); + }); const clearIntervalSpy = jest.spyOn(global, 'clearInterval'); From a8eaf4b8affd7f47f45107d85cf25ab39500987a Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Tue, 10 Dec 2024 14:52:52 +0000 Subject: [PATCH 15/16] test: clean up settings respository mocking --- .../ldap.ee/__tests__/ldap.service.test.ts | 74 ++++++------------- 1 file changed, 22 insertions(+), 52 deletions(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index 07abc0289e..d9e2ba9ca6 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -91,10 +91,14 @@ describe('LdapService', () => { jest.restoreAllMocks(); }); - const createDefaultLdapService = (config: LdapConfig) => { + const mockSettingsRespositoryFindOneByOrFail = (config: LdapConfig) => { settingsRepository.findOneByOrFail.mockResolvedValueOnce({ value: JSON.stringify(config), } as Settings); + }; + + const createDefaultLdapService = (config: LdapConfig) => { + mockSettingsRespositoryFindOneByOrFail(config); return new LdapService(mockLogger(), settingsRepository, mock(), mock()); }; @@ -145,9 +149,7 @@ describe('LdapService', () => { }); it('should show logger warning if authentication method is not ldap or email', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const logger = mockLogger(); @@ -218,9 +220,7 @@ describe('LdapService', () => { }); it('should decipher the LDAP configuration admin password', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const cipherMock = mock({ decrypt: jest.fn(), @@ -235,9 +235,7 @@ describe('LdapService', () => { }); it('should return the expected LDAP configuration', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const cipherMock = mock({ decrypt: jest.fn().mockReturnValue('decryptedPassword'), @@ -273,9 +271,7 @@ describe('LdapService', () => { }); it('should encrypt the binding admin password', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const cipherMock = mock({ encrypt: jest.fn().mockReturnValue('encryptedPassword'), @@ -293,9 +289,7 @@ describe('LdapService', () => { }); it('should delete all ldap identities if login is disabled and ldap users exist', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const authIdentityRepository = mockInstance(AuthIdentityRepository, { find: jest.fn().mockResolvedValue([{ user: { id: 'userId' } }]), @@ -318,9 +312,7 @@ describe('LdapService', () => { }); it('should not delete ldap identities if login is disabled and there are no ldap identities', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const authIdentityRepository = mockInstance(AuthIdentityRepository, { find: jest.fn().mockResolvedValue([]), @@ -343,9 +335,7 @@ describe('LdapService', () => { }); it('should update the LDAP configuration in the settings repository', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); mockInstance(AuthIdentityRepository, { find: jest.fn().mockResolvedValue([{ user: { id: 'userId' } }]), @@ -370,9 +360,7 @@ describe('LdapService', () => { }); it('should update the LDAP login label in the config', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); mockInstance(AuthIdentityRepository, { find: jest.fn().mockResolvedValue([{ user: { id: 'userId' } }]), @@ -486,9 +474,7 @@ describe('LdapService', () => { describe('searchWithAdminBinding()', () => { it('should bind admin client', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const cipherMock = mock({ decrypt: jest.fn().mockReturnValue('decryptedPassword'), @@ -511,9 +497,7 @@ describe('LdapService', () => { }); it('should call client search with expected parameters', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const cipherMock = mock({ decrypt: jest.fn().mockReturnValue('decryptedPassword'), @@ -545,9 +529,7 @@ describe('LdapService', () => { }); it('should call client search with expected parameters when searchPageSize is 0', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify({ ...ldapConfig, searchPageSize: 0 }), - } as Settings); + mockSettingsRespositoryFindOneByOrFail({ ...ldapConfig, searchPageSize: 0 }); const cipherMock = mock({ decrypt: jest.fn().mockReturnValue('decryptedPassword'), @@ -579,9 +561,7 @@ describe('LdapService', () => { }); it('should unbind client after search', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const cipherMock = mock({ decrypt: jest.fn().mockReturnValue('decryptedPassword'), @@ -600,9 +580,7 @@ describe('LdapService', () => { }); it('should return expected search entries', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const cipherMock = mock({ decrypt: jest.fn().mockReturnValue('decryptedPassword'), @@ -719,9 +697,7 @@ describe('LdapService', () => { }); it('should emit expected error if admin search fails', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const eventServiceMock = mock({ emit: jest.fn(), @@ -1283,9 +1259,7 @@ describe('LdapService', () => { }); it('should emit expected event if synchronization is enabled', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const eventServiceMock = mock({ emit: jest.fn(), @@ -1315,9 +1289,7 @@ describe('LdapService', () => { }); it('should emit expected event if synchronization is disabled', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const eventServiceMock = mock({ emit: jest.fn(), @@ -1348,9 +1320,7 @@ describe('LdapService', () => { }); it('should emit expected event with error message if processUsers fails', async () => { - settingsRepository.findOneByOrFail.mockResolvedValueOnce({ - value: JSON.stringify(ldapConfig), - } as Settings); + mockSettingsRespositoryFindOneByOrFail(ldapConfig); const eventServiceMock = mock({ emit: jest.fn(), From 4c5144012172791fc5891ad5a6b92e8fafb7e8d3 Mon Sep 17 00:00:00 2001 From: Marc Littlemore Date: Tue, 4 Mar 2025 14:33:11 +0000 Subject: [PATCH 16/16] rebase + fix test issues --- .../cli/src/ldap.ee/__tests__/ldap.service.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts index d9e2ba9ca6..209899cf5c 100644 --- a/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts +++ b/packages/cli/src/ldap.ee/__tests__/ldap.service.test.ts @@ -9,16 +9,14 @@ import type { Settings } from '@/databases/entities/settings'; import { AuthIdentityRepository } from '@/databases/repositories/auth-identity.repository'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; import type { EventService } from '@/events/event.service'; +import { mockInstance, mockLogger } from '@test/mocking'; + import { BINARY_AD_ATTRIBUTES, LDAP_LOGIN_ENABLED, LDAP_LOGIN_LABEL, LDAP_FEATURE_NAME, -} from '@/ldap/constants'; -import { LdapService } from '@/ldap/ldap.service.ee'; -import type { LdapConfig } from '@/ldap/types'; -import { mockInstance, mockLogger } from '@test/mocking'; - +} from '../constants'; import { getLdapIds, createFilter, @@ -29,6 +27,8 @@ import { saveLdapSynchronization, resolveEntryBinaryAttributes, } from '../helpers.ee'; +import { LdapService } from '../ldap.service.ee'; +import type { LdapConfig } from '../types'; // Mock ldapts client jest.mock('ldapts', () => {