From 24c9f3c21f8bd08868be4c3ddd264b9688bd1208 Mon Sep 17 00:00:00 2001 From: Samuel Date: Mon, 18 Nov 2024 13:31:25 +0100 Subject: [PATCH] refactor: made sync service abstract for any type --- src/cron.ts | 21 +- .../{user-sync-service.ts => sync-manager.ts} | 46 ++-- src/service/sync/sync-service.ts | 44 ++- .../sync/{ => user}/ldap-sync-service.ts | 18 +- src/service/sync/user/user-sync-manager.ts | 36 +++ src/service/sync/user/user-sync-service.ts | 42 +++ test/unit/service/sync/ldap-sync-service.ts | 257 +++++++++++------- 7 files changed, 307 insertions(+), 157 deletions(-) rename src/service/sync/{user-sync-service.ts => sync-manager.ts} (64%) rename src/service/sync/{ => user}/ldap-sync-service.ts (93%) create mode 100644 src/service/sync/user/user-sync-manager.ts create mode 100644 src/service/sync/user/user-sync-service.ts diff --git a/src/cron.ts b/src/cron.ts index e5920d8e..02d30a77 100644 --- a/src/cron.ts +++ b/src/cron.ts @@ -34,8 +34,9 @@ import RoleManager from './rbac/role-manager'; import Gewis from './gewis/gewis'; import EventService from './service/event-service'; import DefaultRoles from './rbac/default-roles'; -import { SyncService } from './service/sync/sync-service'; -import LdapSyncService from './service/sync/ldap-sync-service'; +import LdapSyncService from './service/sync/user/ldap-sync-service'; +import { UserSyncService } from './service/sync/user/user-sync-service'; +import UserSyncManager from './service/sync/user/user-sync-manager'; class CronApplication { logger: Logger; @@ -101,7 +102,7 @@ async function createCronTasks(): Promise { // INJECT GEWIS BINDINGS Gewis.overwriteBindings(); - const syncServices: SyncService[] = []; + const syncServices: UserSyncService[] = []; if (process.env.ENABLE_LDAP === 'true') { const ldapSyncService = new LdapSyncService(application.roleManager, null, AppDataSource.manager); @@ -119,9 +120,17 @@ async function createCronTasks(): Promise { // application.tasks.push(syncGewis); // } - // TODO sensible cron schedule - // const runSyncer = cron.schedule('*/10 * * * *', async () => { - // } + if (syncServices.length !== 0) { + const syncManager = new UserSyncManager(syncServices); + + // TODO sensible cron schedule + const userSyncer = cron.schedule('*/10 * * * *', async () => { + logger.debug('Syncing users.'); + await syncManager.run(); + await syncManager.fetch(); + }); + application.tasks.push(userSyncer); + } application.logger.info('Tasks registered'); } diff --git a/src/service/sync/user-sync-service.ts b/src/service/sync/sync-manager.ts similarity index 64% rename from src/service/sync/user-sync-service.ts rename to src/service/sync/sync-manager.ts index 94540411..8617b060 100644 --- a/src/service/sync/user-sync-service.ts +++ b/src/service/sync/sync-manager.ts @@ -18,60 +18,58 @@ * @license */ -import User from '../../entity/user/user'; import WithManager from '../../database/with-manager'; import { SyncResult, SyncService } from './sync-service'; -import { In } from 'typeorm'; import log4js, { Logger } from 'log4js'; -export default class UserSyncService extends WithManager { +export default abstract class SyncManager> extends WithManager { - private readonly services: SyncService[]; + protected readonly services: S[]; - private logger: Logger = log4js.getLogger('UserSync'); + protected logger: Logger = log4js.getLogger('SyncManager'); - constructor(services: SyncService[]) { + constructor(services: S[]) { super(); this.logger.level = process.env.LOG_LEVEL; this.services = services; } - async syncUsers() { - const userTypes = this.services.flatMap((s) => s.targets); - this.logger.trace('Syncing users of types', userTypes); - await this.pre(); + abstract getTargets(): Promise; + + async run() { + this.logger.trace('Start sync job'); + const entities = await this.getTargets(); - const users = await this.manager.find(User, { where: { type: In(userTypes) } }); - for (const user of users) { + await this.pre(); + for (const entity of entities) { try { - const result = await this.sync(user); + const result = await this.sync(entity); if (result.skipped) { - this.logger.trace('Syncing skipped for user', user.id, user.firstName, user.type); + this.logger.trace('Syncing skipped for', entity); continue; } if (result.result === false) { - this.logger.warn('Sync result: false for user', user.id); - await this.down(user); + this.logger.warn('Sync result: false for', entity); + await this.down(entity); } else { - this.logger.trace('Sync result: true for user', user.id); + this.logger.trace('Sync result: true for', entity); } } catch (error) { - this.logger.error('Syncing error for user', user.id, error); + this.logger.error('Syncing error for', entities, error); } } - await this.post(); } - async sync(user: User): Promise { + async sync(entity: T): Promise { const syncResult: SyncResult = { skipped: true, result: false }; // Aggregate results from all services for (const service of this.services) { - const result = await service.up(user); + const result = await service.up(entity); if (!result.skipped) syncResult.skipped = false; if (result.result) syncResult.result = true; @@ -80,12 +78,12 @@ export default class UserSyncService extends WithManager { return syncResult; } - async down(user: User): Promise { + async down(entity: T): Promise { for (const service of this.services) { try { - await service.down(user); + await service.down(entity); } catch (error) { - this.logger.error('Could not down user', user.id); + this.logger.error('Could not down', entity); } } } diff --git a/src/service/sync/sync-service.ts b/src/service/sync/sync-service.ts index ad0174dd..4f671c17 100644 --- a/src/service/sync/sync-service.ts +++ b/src/service/sync/sync-service.ts @@ -21,10 +21,9 @@ /** * This is the module page of the abstract sync-service. * - * @module internal/user-sync + * @module internal/sync-service */ -import User, { UserType } from '../../entity/user/user'; import WithManager from '../../database/with-manager'; export interface SyncResult { @@ -35,53 +34,44 @@ export interface SyncResult { /** * SyncService interface. * - * SyncService is the abstract class which is used to sync user data. + * SyncService is the abstract class which is used to sync entity data. * This can be used to integrate external data sources into the SudoSOS back-end. */ -export abstract class SyncService extends WithManager { +export abstract class SyncService extends WithManager { /** - * Targets is the list of user types that this sync service is responsible for. - * - * Used to improve performance by only syncing the relevant user types. - */ - targets: UserType[]; - - /** - * Guard determines whether the user should be synced using this sync service. + * Guard determines whether the entity should be synced using this sync service. * * Not passing the guard will result in the user being skipped. * A skipped sync does not count as a failure. * - * @param user The user to check. - * @returns {Promise} True if the user should be synced, false otherwise. + * @param entity The entity to check. + * @returns {Promise} True if the entity should be synced, false otherwise. */ - protected guard(user: User): Promise { - return Promise.resolve(this.targets.includes(user.type)); - } + abstract guard(entity: T): Promise; /** * Up is a wrapper around `sync` that handles the guard. * - * @param user + * @param entity * * @returns {Promise} The result of the sync. */ - async up(user: User): Promise { - const guardResult = await this.guard(user); + async up(entity: T): Promise { + const guardResult = await this.guard(entity); if (!guardResult) return { skipped: true, result: false }; - const result = await this.sync(user); + const result = await this.sync(entity); return { skipped: false, result }; } /** * Synchronizes the user data with the external data source. * - * @param user The user to synchronize. + * @param entity The user to synchronize. * @returns {Promise} True if the user was synchronized, false otherwise. */ - protected abstract sync(user: User): Promise; + protected abstract sync(entity: T): Promise; /** * Fetches the user data from the external data source. @@ -91,14 +81,14 @@ export abstract class SyncService extends WithManager { abstract fetch(): Promise; /** - * Down is called when the SyncService decides that the user is no longer connected to this sync service be removed. - * This can be used to remove the user from the database or clean up entities. + * Down is called when the SyncService decides that the entity is no longer connected to this sync service be removed. + * This can be used to remove the entity from the database or clean up entities. * * This should be revertible and idempotent! * - * @param user + * @param entity */ - abstract down(user: User): Promise; + abstract down(entity: T): Promise; /** * Called before a sync batch is started. diff --git a/src/service/sync/ldap-sync-service.ts b/src/service/sync/user/ldap-sync-service.ts similarity index 93% rename from src/service/sync/ldap-sync-service.ts rename to src/service/sync/user/ldap-sync-service.ts index 3da59905..baa21049 100644 --- a/src/service/sync/ldap-sync-service.ts +++ b/src/service/sync/user/ldap-sync-service.ts @@ -21,21 +21,21 @@ /** * This is the module page of the ldap-sync-service. * - * @module internal/user-sync + * @module internal/ldap-sync-service */ -import { SyncService } from './sync-service'; -import User, { TermsOfServiceStatus, UserType } from '../../entity/user/user'; +import User, { TermsOfServiceStatus, UserType } from '../../../entity/user/user'; import { Client } from 'ldapts'; -import ADService from '../ad-service'; -import LDAPAuthenticator from '../../entity/authenticator/ldap-authenticator'; -import RoleManager from '../../rbac/role-manager'; +import ADService from '../../ad-service'; +import LDAPAuthenticator from '../../../entity/authenticator/ldap-authenticator'; +import RoleManager from '../../../rbac/role-manager'; import { EntityManager } from 'typeorm'; -import { getLDAPConnection, LDAPGroup, LDAPUser } from '../../helpers/ad'; -import RBACService from '../rbac-service'; +import { getLDAPConnection, LDAPGroup, LDAPUser } from '../../../helpers/ad'; +import RBACService from '../../rbac-service'; import log4js, { Logger } from 'log4js'; +import { UserSyncService } from './user-sync-service'; -export default class LdapSyncService extends SyncService { +export default class LdapSyncService extends UserSyncService { // We only sync organs, members and integrations. targets = [UserType.ORGAN, UserType.MEMBER, UserType.INTEGRATION]; diff --git a/src/service/sync/user/user-sync-manager.ts b/src/service/sync/user/user-sync-manager.ts new file mode 100644 index 00000000..3f1daf88 --- /dev/null +++ b/src/service/sync/user/user-sync-manager.ts @@ -0,0 +1,36 @@ +/** + * SudoSOS back-end API service. + * Copyright (C) 2024 Study association GEWIS + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + * @license + */ + +import User from '../../../entity/user/user'; +import { In } from 'typeorm'; +import log4js, { Logger } from 'log4js'; +import SyncManager from '../sync-manager'; +import { UserSyncService } from './user-sync-service'; + +export default class UserSyncManager extends SyncManager { + + protected logger: Logger = log4js.getLogger('UserSyncManager'); + + async getTargets(): Promise { + const userTypes = this.services.flatMap((s) => s.targets); + this.logger.trace('Syncing users of types', userTypes); + return this.manager.find(User, { where: { type: In(userTypes) } }); + } +} diff --git a/src/service/sync/user/user-sync-service.ts b/src/service/sync/user/user-sync-service.ts new file mode 100644 index 00000000..2cccc698 --- /dev/null +++ b/src/service/sync/user/user-sync-service.ts @@ -0,0 +1,42 @@ +/** + * SudoSOS back-end API service. + * Copyright (C) 2024 Study association GEWIS + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + * @license + */ + +/** + * This is the module page of the abstract sync-service. + * + * @module internal/user-user-service + */ + +import User, { UserType } from '../../../entity/user/user'; +import { SyncService } from '../sync-service'; + +/** + * UserSyncService interface. + * + * Specific sync service for users. + */ +export abstract class UserSyncService extends SyncService { + + targets: UserType[]; + + guard(user: User): Promise { + return Promise.resolve(this.targets.includes(user.type)); + } +} diff --git a/test/unit/service/sync/ldap-sync-service.ts b/test/unit/service/sync/ldap-sync-service.ts index a4ac5436..e210e471 100644 --- a/test/unit/service/sync/ldap-sync-service.ts +++ b/test/unit/service/sync/ldap-sync-service.ts @@ -27,7 +27,7 @@ import { setDefaultLDAPEnv, storeLDAPEnv, } from '../../../helpers/test-helpers'; -import LdapSyncService from '../../../../src/service/sync/ldap-sync-service'; +import LdapSyncService from '../../../../src/service/sync/user/ldap-sync-service'; import { expect } from 'chai'; import LDAPAuthenticator from '../../../../src/entity/authenticator/ldap-authenticator'; import sinon from 'sinon'; @@ -98,7 +98,7 @@ describe('LdapSyncService', () => { }); }); - describe('sync function', () => { + describe('ldap functions', () => { let ldapSyncService: LdapSyncService; before(async () => { @@ -112,8 +112,24 @@ describe('LdapSyncService', () => { beforeEach(() => { stubs.push(sinon.stub(Client.prototype, 'bind').resolves(null)); }); + + function stubGroupSearch(stub: sinon.SinonStub, baseDN: string, returns: any) { + stub.withArgs(baseDN, { + filter: '(CN=*)', + explicitBufferAttributes: ['objectGUID'], + }).resolves(returns); + stubs.push(stub); + } + + function stubMemberSearch(stub: sinon.SinonStub, dn: string, returns: any) { + stub.withArgs(process.env.LDAP_BASE, { + filter: `(&(objectClass=user)(objectCategory=person)(memberOf:1.2.840.113556.1.4.1941:=${dn}))`, + explicitBufferAttributes: ['objectGUID'], + }).resolves(returns); + stubs.push(stub); + } - function stubSearch(stub: sinon.SinonStub, value: Buffer, returns: any) { + function stubGUIDSearch(stub: sinon.SinonStub, value: Buffer, returns: any) { stub.withArgs(process.env.LDAP_BASE, { filter: new EqualityFilter({ attribute: 'objectGUID', @@ -143,104 +159,163 @@ describe('LdapSyncService', () => { expect(user.active).to.be.true; } - it('should return false if user no AD entry matching the LDAPAuthenticator UUID', async () => { - await inUserContext( - await (await UserFactory()).clone(1), - async (member: User) => { - const UUID = Buffer.from('4321', 'hex'); - expect(await ldapSyncService.guard(member)).to.be.false; - const stub = sinon.stub(Client.prototype, 'search'); - stubSearch(stub, UUID, { searchReferences: [], searchEntries: [] }); - await ldapSyncService.pre(); + describe('sync function', () => { + it('should return false if user no AD entry matching the LDAPAuthenticator UUID', async () => { + await inUserContext( + await (await UserFactory()).clone(1), + async (member: User) => { + const UUID = Buffer.from('4321', 'hex'); + expect(await ldapSyncService.guard(member)).to.be.false; + const stub = sinon.stub(Client.prototype, 'search'); + stubGUIDSearch(stub, UUID, { searchReferences: [], searchEntries: [] }); + await ldapSyncService.pre(); - await addLDAPAuthenticator(UUID, member); - expect(await ldapSyncService.sync(member)).to.be.false; - }, - ); - }); - it('should return true if user has AD entry matching the LDAPAuthenticator UUID', async () => { - await inUserContext( - await (await UserFactory()).clone(1), - async (member: User) => { - const UUID = Buffer.from('4444', 'hex'); - await addLDAPAuthenticator(UUID, member); + await addLDAPAuthenticator(UUID, member); + expect(await ldapSyncService.sync(member)).to.be.false; + }, + ); + }); + it('should return true if user has AD entry matching the LDAPAuthenticator UUID', async () => { + await inUserContext( + await (await UserFactory()).clone(1), + async (member: User) => { + const UUID = Buffer.from('4444', 'hex'); + await addLDAPAuthenticator(UUID, member); - const stub = sinon.stub(Client.prototype, 'search'); - stubSearch(stub, UUID, { searchReferences: [], searchEntries: [{ member }] }); - await ldapSyncService.pre(); + const stub = sinon.stub(Client.prototype, 'search'); + stubGUIDSearch(stub, UUID, { searchReferences: [], searchEntries: [{ member }] }); + await ldapSyncService.pre(); - expect(await ldapSyncService.sync(member)).to.be.true; - }, - ); - }); - it('should return true and update user if user is of type ORGAN with known LDAPAuthenticator', async () => { - await inUserContext( - await (await UserFactory(await ORGAN_USER())).clone(1), - async (organ: User) => { - const UUID = Buffer.from('8989', 'hex'); - await addLDAPAuthenticator(UUID, organ); - - // Intentionally "mess up" the user - await AppDataSource.manager.update(User, organ.id, { - firstName: 'Wrong', - lastName: 'Wrong', - canGoIntoDebt: true, - acceptedToS: TermsOfServiceStatus.ACCEPTED, - active: false, - }); + expect(await ldapSyncService.sync(member)).to.be.true; + }, + ); + }); + it('should return true and update user if user is of type ORGAN with known LDAPAuthenticator', async () => { + await inUserContext( + await (await UserFactory(await ORGAN_USER())).clone(1), + async (organ: User) => { + const UUID = Buffer.from('8989', 'hex'); + await addLDAPAuthenticator(UUID, organ); - const displayName = `${organ.firstName} Updated`; - const stub = sinon.stub(Client.prototype, 'search'); - stubSearch(stub, UUID, { searchReferences: [], searchEntries: [{ - displayName, - }] }); + // Intentionally "mess up" the user + await AppDataSource.manager.update(User, organ.id, { + firstName: 'Wrong', + lastName: 'Wrong', + canGoIntoDebt: true, + acceptedToS: TermsOfServiceStatus.ACCEPTED, + active: false, + }); - await ldapSyncService.pre(); - expect(await ldapSyncService.sync(organ)).to.be.true; - const dbUser = await AppDataSource.manager.findOne(User, { where: { id: organ.id } }); - userIsAsExpected(dbUser, { displayName } as LDAPUser); - }, - ); + const displayName = `${organ.firstName} Updated`; + const stub = sinon.stub(Client.prototype, 'search'); + stubGUIDSearch(stub, UUID, { + searchReferences: [], searchEntries: [{ + displayName, + }], + }); + + await ldapSyncService.pre(); + expect(await ldapSyncService.sync(organ)).to.be.true; + const dbUser = await AppDataSource.manager.findOne(User, { where: { id: organ.id } }); + userIsAsExpected(dbUser, { displayName } as LDAPUser); + }, + ); + }); + it('should return true and update user if user is of type INTEGRATION with known LDAPAuthenticator', async () => { + await inUserContext( + await (await UserFactory(await INTEGRATION_USER())).clone(1), + async (organ: User) => { + const UUID = Buffer.from('4141', 'hex'); + await addLDAPAuthenticator(UUID, organ); + + // Intentionally "mess up" the user + await AppDataSource.manager.update(User, organ.id, { + firstName: 'Wrong', + lastName: 'Wrong', + canGoIntoDebt: true, + acceptedToS: TermsOfServiceStatus.ACCEPTED, + active: false, + }); + + const displayName = `${organ.firstName} Updated`; + const stub = sinon.stub(Client.prototype, 'search'); + stubGUIDSearch(stub, UUID, { + searchReferences: [], searchEntries: [{ + displayName, + }], + }); + + await ldapSyncService.pre(); + expect(await ldapSyncService.sync(organ)).to.be.true; + const dbUser = await AppDataSource.manager.findOne(User, { where: { id: organ.id } }); + userIsAsExpected(dbUser, { displayName } as LDAPUser); + }, + ); + }); + it('should return false if user has no LDAPAuthenticator', async () => { + await inUserContext( + await (await UserFactory()).clone(1), + async (member: User) => { + expect(await ldapSyncService.sync(member)).to.be.false; + }, + ); + }); }); - it('should return true and update user if user is of type INTEGRATION with known LDAPAuthenticator', async () => { - await inUserContext( - await (await UserFactory(await INTEGRATION_USER())).clone(1), - async (organ: User) => { - const UUID = Buffer.from('4141', 'hex'); - await addLDAPAuthenticator(UUID, organ); - - // Intentionally "mess up" the user - await AppDataSource.manager.update(User, organ.id, { - firstName: 'Wrong', - lastName: 'Wrong', - canGoIntoDebt: true, - acceptedToS: TermsOfServiceStatus.ACCEPTED, - active: false, - }); - const displayName = `${organ.firstName} Updated`; - const stub = sinon.stub(Client.prototype, 'search'); - stubSearch(stub, UUID, { searchReferences: [], searchEntries: [{ - displayName, - }] }); + describe('down function', () => { + it('should remove the LDAPAuthenticator for the given user', async () => { + await inUserContext( + await (await UserFactory()).clone(1), + async (member: User) => { + const UUID = Buffer.from('4321'.repeat(member.id), 'hex'); + expect(await ldapSyncService.guard(member)).to.be.false; + const stub = sinon.stub(Client.prototype, 'search'); + stubGUIDSearch(stub, UUID, { searchReferences: [], searchEntries: [] }); + await ldapSyncService.pre(); - await ldapSyncService.pre(); - expect(await ldapSyncService.sync(organ)).to.be.true; - const dbUser = await AppDataSource.manager.findOne(User, { where: { id: organ.id } }); - userIsAsExpected(dbUser, { displayName } as LDAPUser); - }, - ); + await addLDAPAuthenticator(UUID, member); + expect(await ldapSyncService.sync(member)).to.be.false; + + await ldapSyncService.down(member); + const auth = await LDAPAuthenticator.findOne({ where: { UUID } }); + expect(auth).to.be.null; + }, + ); + }); + it('should set INTEGRATION and ORGAN users to deleted and inactive', async () => { + await inUserContext( + await (await UserFactory(await ORGAN_USER())).clone(1), + async (organ: User) => { + const UUID = Buffer.from('4141'.repeat(organ.id), 'hex'); + expect(await ldapSyncService.guard(organ)).to.be.true; + const stub = sinon.stub(Client.prototype, 'search'); + stubGUIDSearch(stub, UUID, { searchReferences: [], searchEntries: [] }); + await ldapSyncService.pre(); + + await addLDAPAuthenticator(UUID, organ); + expect(await ldapSyncService.sync(organ)).to.be.false; + + await ldapSyncService.down(organ); + const auth = await LDAPAuthenticator.findOne({ where: { UUID } }); + expect(auth).to.be.null; + + const dbUser = await AppDataSource.manager.findOne(User, { where: { id: organ.id } }); + expect(dbUser.deleted).to.be.true; + expect(dbUser.active).to.be.false; + }, + ); + }); }); - it('should return false if user has no LDAPAuthenticator', async () => { - await inUserContext( - await (await UserFactory()).clone(1), - async (member: User) => { - expect(await ldapSyncService.sync(member)).to.be.false; - }, - ); + + describe('fetch functions', () => { + describe('fetchSharedAccounts function', () => { + it('should create an account for new shared accounts', async () => { + }); + }); }); }); + // TODO: move these test cases to ad-sync-service test cases // describe('syncSharedAccounts functions', () => { // async function createAccountsFromLDAP(accounts: any[]) { @@ -514,7 +589,7 @@ describe('LdapSyncService', () => { // expect(await AssignedRole.findOne({ where: { role: { name: 'SudoSOS - Test' }, user: { id: users[1].id } } })).to.exist; // }); // }); - // describe('syncUsers function', () => { + // describe('run function', () => { // it('should create new users if needed', async () => { // process.env.ENABLE_LDAP = 'true'; // @@ -531,7 +606,7 @@ describe('LdapSyncService', () => { // stubs.push(clientBindStub); // stubs.push(clientSearchStub); // - // await new ADService().syncUsers(); + // await new ADService().run(); // const auth = (await LDAPAuthenticator.findOne( // { where: { UUID: newUser.objectGUID }, relations: ['user'] }, // ));