diff --git a/backend/src/apis/humhub/ExportUsers.ts b/backend/src/apis/humhub/ExportUsers.ts index b3af27f7d..d4515b3ce 100644 --- a/backend/src/apis/humhub/ExportUsers.ts +++ b/backend/src/apis/humhub/ExportUsers.ts @@ -26,7 +26,7 @@ function getUsersPage(page: number, limit: number): Promise<[User[], number]> { /** * @param client - * @returns user map indices with email + * @returns user map indices with username */ async function loadUsersFromHumHub(client: HumHubClient): Promise> { const start = new Date().getTime() @@ -42,7 +42,7 @@ async function loadUsersFromHumHub(client: HumHubClient): Promise { // deleted users have empty emails if (user.account.email) { - humhubUsers.set(user.account.email.trim(), user) + humhubUsers.set(user.account.username, user) } else { skippedUsersCount++ } @@ -52,6 +52,7 @@ async function loadUsersFromHumHub(client: HumHubClient): Promise[] = [] - users.forEach((user: User) => promises.push(syncUser(user, humhubUsers))) - const executedActions = await Promise.all(promises) - executedActions.forEach((executedAction: ExecutedHumhubAction) => { - executedHumhubActionsCount[executedAction as number]++ - }) - // using process.stdout.write here so that carriage-return is working analog to c - // printf("\rchecked user: %d/%d", dbUserCount, totalUsers); - process.stdout.write(`checked user: ${dbUserCount}/${totalUsers}\r`) + try { + const [users, totalUsers] = await getUsersPage(page, USER_BULK_SIZE) + dbUserCount += users.length + userCount = users.length + page++ + const promises: Promise[] = [] + users.forEach((user: User) => promises.push(syncUser(user, humhubUsers))) + const executedActions = await Promise.all(promises) + executedActions.forEach((executedAction: ExecutedHumhubAction) => { + executedHumhubActionsCount[executedAction as number]++ + }) + // using process.stdout.write here so that carriage-return is working analog to c + // printf("\rchecked user: %d/%d", dbUserCount, totalUsers); + process.stdout.write(`checked user: ${dbUserCount}/${totalUsers}\r`) + } catch (e) { + process.stdout.write('\n') + throw e + } } while (userCount === USER_BULK_SIZE) + process.stdout.write('\n') await con.destroy() const elapsed = new Date().getTime() - start @@ -114,12 +121,13 @@ async function main() { updatedCount: executedHumhubActionsCount[ExecutedHumhubAction.UPDATE], skippedCount: executedHumhubActionsCount[ExecutedHumhubAction.SKIP], deletedCount: executedHumhubActionsCount[ExecutedHumhubAction.DELETE], + validationErrorCount: executedHumhubActionsCount[ExecutedHumhubAction.VALIDATION_ERROR], }) } main().catch((e) => { // eslint-disable-next-line no-console - console.error(e) + console.error(JSON.stringify(e, null, 2)) // eslint-disable-next-line n/no-process-exit process.exit(1) }) diff --git a/backend/src/apis/humhub/__mocks__/syncUser.ts b/backend/src/apis/humhub/__mocks__/syncUser.ts index 7e0660da4..1cb2edfbf 100644 --- a/backend/src/apis/humhub/__mocks__/syncUser.ts +++ b/backend/src/apis/humhub/__mocks__/syncUser.ts @@ -2,6 +2,7 @@ import { User } from '@entity/User' import { isHumhubUserIdenticalToDbUser } from '@/apis/humhub/compareHumhubUserDbUser' import { GetUser } from '@/apis/humhub/model/GetUser' +import { PostUser } from '@/apis/humhub/model/PostUser' export enum ExecutedHumhubAction { UPDATE, @@ -26,7 +27,8 @@ export async function syncUser( user: User, humhubUsers: Map, ): Promise { - const humhubUser = humhubUsers.get(user.emailContact.email.trim()) + const postUser = new PostUser(user) + const humhubUser = humhubUsers.get(postUser.account.username) if (humhubUser) { if (!user.humhubAllowed) { return Promise.resolve(ExecutedHumhubAction.DELETE) diff --git a/backend/src/apis/humhub/compareHumhubUserDbUser.test.ts b/backend/src/apis/humhub/compareHumhubUserDbUser.test.ts index cc636d17a..a3d6c7044 100644 --- a/backend/src/apis/humhub/compareHumhubUserDbUser.test.ts +++ b/backend/src/apis/humhub/compareHumhubUserDbUser.test.ts @@ -1,4 +1,7 @@ /* eslint-disable prettier/prettier */ +// eslint-disable-next-line import/no-unassigned-import +import 'reflect-metadata' +import { PublishNameType } from '@/graphql/enum/PublishNameType' import { communityDbUser } from '@/util/communityUser' import { isHumhubUserIdenticalToDbUser } from './compareHumhubUserDbUser' @@ -13,6 +16,7 @@ describe('isHumhubUserIdenticalToDbUser', () => { defaultUser.alias = 'alias' defaultUser.emailContact.email = 'email@gmail.com' defaultUser.language = 'en' + defaultUser.gradidoID = 'gradidoID' }) it('Should return true because humhubUser was created from entity user', () => { @@ -21,6 +25,20 @@ describe('isHumhubUserIdenticalToDbUser', () => { expect(result).toBe(true) }) + it('Should return false, because last name differ because of publish name type', () => { + const humhubUser = new GetUser(defaultUser, 1) + defaultUser.humhubPublishName = PublishNameType.PUBLISH_NAME_FIRST + const result = isHumhubUserIdenticalToDbUser(humhubUser, defaultUser) + expect(result).toBe(false) + }) + + it('Should return true, even if alias is empty', () => { + defaultUser.alias = '' + const humhubUser = new GetUser(defaultUser, 1) + const result = isHumhubUserIdenticalToDbUser(humhubUser, defaultUser) + expect(result).toBe(true) + }) + it('Should return false because first name differ', () => { const humhubUser = new GetUser(defaultUser, 1) humhubUser.profile.firstname = 'changed first name' diff --git a/backend/src/apis/humhub/syncUser.test.ts b/backend/src/apis/humhub/syncUser.test.ts index 20a6b2c33..00c125632 100644 --- a/backend/src/apis/humhub/syncUser.test.ts +++ b/backend/src/apis/humhub/syncUser.test.ts @@ -28,7 +28,8 @@ describe('syncUser function', () => { it('When humhubUser exists and user.humhubAllowed is false, should return DELETE action', async () => { const humhubUsers = new Map() - humhubUsers.set(defaultUser.emailContact.email, new GetUser(defaultUser, 1)) + const humhubUser = new GetUser(defaultUser, 1) + humhubUsers.set(humhubUser.account.username, humhubUser) defaultUser.humhubAllowed = false const result = await syncUser(defaultUser, humhubUsers) @@ -39,8 +40,8 @@ describe('syncUser function', () => { it('When humhubUser exists and user.humhubAllowed is true and there are changes in user data, should return UPDATE action', async () => { const humhubUsers = new Map() const humhubUser = new GetUser(defaultUser, 1) + humhubUsers.set(humhubUser.account.username, humhubUser) humhubUser.account.username = 'test username' - humhubUsers.set(defaultUser.emailContact.email, humhubUser) defaultUser.humhubAllowed = true const result = await syncUser(defaultUser, humhubUsers) @@ -51,7 +52,7 @@ describe('syncUser function', () => { it('When humhubUser exists and user.humhubAllowed is true and there are no changes in user data, should return SKIP action', async () => { const humhubUsers = new Map() const humhubUser = new GetUser(defaultUser, 1) - humhubUsers.set(defaultUser.emailContact.email, humhubUser) + humhubUsers.set(humhubUser.account.username, humhubUser) defaultUser.humhubAllowed = true const result = await syncUser(defaultUser, humhubUsers) diff --git a/backend/src/apis/humhub/syncUser.ts b/backend/src/apis/humhub/syncUser.ts index fc6fcc99b..e5b793ee3 100644 --- a/backend/src/apis/humhub/syncUser.ts +++ b/backend/src/apis/humhub/syncUser.ts @@ -1,6 +1,7 @@ import { User } from '@entity/User' import { LogError } from '@/server/LogError' +import { backendLogger as logger } from '@/server/logger' import { isHumhubUserIdenticalToDbUser } from './compareHumhubUserDbUser' import { HumHubClient } from './HumHubClient' @@ -12,7 +13,22 @@ export enum ExecutedHumhubAction { CREATE, SKIP, DELETE, + VALIDATION_ERROR, } + +// todo: replace with full validation (schema) +function isValid(postUser: PostUser, userId: number): boolean { + if (postUser.profile.firstname.length > 20) { + logger.error('firstname too long for humhub, for user with id:', userId) + return false + } + if (postUser.profile.lastname.length > 20) { + logger.error('lastname too long for humhub, for user with id:', userId) + return false + } + return true +} + /** * Trigger action according to conditions * | User exist on humhub | export to humhub allowed | changes in user data | ACTION @@ -21,9 +37,8 @@ export enum ExecutedHumhubAction { * | true | true | false | SKIP * | false | false | ignored | SKIP * | false | true | ignored | CREATE - * @param user - * @param humHubClient - * @param humhubUsers + * @param user user entity + * @param humhubUsers user map indices with username * @returns */ export async function syncUser( @@ -31,7 +46,10 @@ export async function syncUser( humhubUsers: Map, ): Promise { const postUser = new PostUser(user) - const humhubUser = humhubUsers.get(user.emailContact.email.trim()) + if (!isValid(postUser, user.id)) { + return ExecutedHumhubAction.VALIDATION_ERROR + } + const humhubUser = humhubUsers.get(postUser.account.username) const humHubClient = HumHubClient.getInstance() if (!humHubClient) { throw new LogError('Error creating humhub client') diff --git a/backend/src/data/PublishName.logic.ts b/backend/src/data/PublishName.logic.ts index a78627d49..b70a29bb3 100644 --- a/backend/src/data/PublishName.logic.ts +++ b/backend/src/data/PublishName.logic.ts @@ -35,12 +35,16 @@ export class PublishNameLogic { * @returns user.firstName for PUBLISH_NAME_FIRST, PUBLISH_NAME_FIRST_INITIAL or PUBLISH_NAME_FULL */ public getFirstName(publishNameType: PublishNameType): string { + let firstName = '' + if (this.user && typeof this.user.firstName === 'string') { + firstName = this.user.firstName + } return [ PublishNameType.PUBLISH_NAME_FIRST, PublishNameType.PUBLISH_NAME_FIRST_INITIAL, PublishNameType.PUBLISH_NAME_FULL, ].includes(publishNameType) - ? this.user.firstName + ? firstName.slice(0, 20) : '' } @@ -51,10 +55,14 @@ export class PublishNameLogic { * first initial from user.lastName for PUBLISH_NAME_FIRST_INITIAL */ public getLastName(publishNameType: PublishNameType): string { + let lastName = '' + if (this.user && typeof this.user.lastName === 'string') { + lastName = this.user.lastName + } return publishNameType === PublishNameType.PUBLISH_NAME_FULL - ? this.user.lastName - : publishNameType === PublishNameType.PUBLISH_NAME_FIRST_INITIAL - ? this.user.lastName.charAt(0) + ? lastName.slice(0, 20) + : publishNameType === PublishNameType.PUBLISH_NAME_FIRST_INITIAL && lastName.length > 0 + ? lastName.charAt(0) : '' } diff --git a/backend/src/graphql/resolver/UserResolver.ts b/backend/src/graphql/resolver/UserResolver.ts index fea150338..66e6a546b 100644 --- a/backend/src/graphql/resolver/UserResolver.ts +++ b/backend/src/graphql/resolver/UserResolver.ts @@ -37,6 +37,7 @@ import { UpdateUserInfosArgs } from '@arg/UpdateUserInfosArgs' import { OptInType } from '@enum/OptInType' import { Order } from '@enum/Order' import { PasswordEncryptionType } from '@enum/PasswordEncryptionType' +import { PublishNameType } from '@enum/PublishNameType' import { UserContactType } from '@enum/UserContactType' import { SearchAdminUsersResult } from '@model/AdminUser' // import { Location } from '@model/Location' @@ -54,6 +55,7 @@ import { subscribe } from '@/apis/KlicktippController' import { encode } from '@/auth/JWT' import { RIGHTS } from '@/auth/RIGHTS' import { CONFIG } from '@/config' +import { PublishNameLogic } from '@/data/PublishName.logic' import { sendAccountActivationEmail, sendAccountMultiRegistrationEmail, @@ -246,12 +248,12 @@ export class UserResolver { try { const result = await humhubUserPromise user.humhubAllowed = result?.result?.account.status === 1 - if (user.humhubAllowed) { + if (user.humhubAllowed && result?.result?.account?.username) { let spaceId = null if (projectBranding) { spaceId = projectBranding.spaceId } - void syncHumhub(null, dbUser, spaceId) + await syncHumhub(null, dbUser, result.result.account.username, spaceId) } } catch (e) { logger.error("couldn't reach out to humhub, disable for now", e) @@ -448,7 +450,11 @@ export class UserResolver { if (projectBranding) { spaceId = projectBranding.spaceId } - void syncHumhub(null, dbUser, spaceId) + try { + await syncHumhub(null, dbUser, dbUser.gradidoID, spaceId) + } catch (e) { + logger.error("createUser: couldn't reach out to humhub, disable for now", e) + } } if (redeemCode) { @@ -659,6 +665,10 @@ export class UserResolver { ) const user = getUser(context) const updateUserInGMS = compareGmsRelevantUserSettings(user, updateUserInfosArgs) + const publishNameLogic = new PublishNameLogic(user) + const oldHumhubUsername = publishNameLogic.getUserIdentifier( + user.humhubPublishName as PublishNameType, + ) // try { if (firstName) { @@ -764,7 +774,7 @@ export class UserResolver { } try { if (CONFIG.HUMHUB_ACTIVE) { - await syncHumhub(updateUserInfosArgs, user) + await syncHumhub(updateUserInfosArgs, user, oldHumhubUsername) } } catch (e) { logger.error('error sync user with humhub', e) @@ -839,7 +849,7 @@ export class UserResolver { } const humhubUserAccount = new HumhubAccount(dbUser) const autoLoginUrlPromise = humhubClient.createAutoLoginUrl(humhubUserAccount.username, project) - const humhubUser = await syncHumhub(null, dbUser) + const humhubUser = await syncHumhub(null, dbUser, humhubUserAccount.username) if (!humhubUser) { throw new LogError("user don't exist (any longer) on humhub and couldn't be created") } diff --git a/backend/src/graphql/resolver/util/syncHumhub.test.ts b/backend/src/graphql/resolver/util/syncHumhub.test.ts index c25eb52a8..d8a61af23 100644 --- a/backend/src/graphql/resolver/util/syncHumhub.test.ts +++ b/backend/src/graphql/resolver/util/syncHumhub.test.ts @@ -32,7 +32,7 @@ describe('syncHumhub', () => { }) it('Should not sync if no relevant changes', async () => { - await syncHumhub(mockUpdateUserInfosArg, new User()) + await syncHumhub(mockUpdateUserInfosArg, new User(), 'username') expect(HumHubClient.getInstance).not.toBeCalled() // language logging from some other place expect(logger.debug).toBeCalledTimes(5) @@ -42,7 +42,7 @@ describe('syncHumhub', () => { it('Should retrieve user from humhub and sync if relevant changes', async () => { mockUpdateUserInfosArg.firstName = 'New' // Relevant changes mockUser.firstName = 'New' - await syncHumhub(mockUpdateUserInfosArg, mockUser) + await syncHumhub(mockUpdateUserInfosArg, mockUser, 'username') expect(logger.debug).toHaveBeenCalledTimes(8) // Four language logging calls, two debug calls in function, one for not syncing expect(logger.info).toHaveBeenLastCalledWith('finished sync user with humhub', { localId: mockUser.id, diff --git a/backend/src/graphql/resolver/util/syncHumhub.ts b/backend/src/graphql/resolver/util/syncHumhub.ts index 90500bbc5..23ba6d0dd 100644 --- a/backend/src/graphql/resolver/util/syncHumhub.ts +++ b/backend/src/graphql/resolver/util/syncHumhub.ts @@ -2,10 +2,9 @@ import { User } from '@entity/User' import { HumHubClient } from '@/apis/humhub/HumHubClient' import { GetUser } from '@/apis/humhub/model/GetUser' +import { PostUser } from '@/apis/humhub/model/PostUser' import { ExecutedHumhubAction, syncUser } from '@/apis/humhub/syncUser' -import { PublishNameLogic } from '@/data/PublishName.logic' import { UpdateUserInfosArgs } from '@/graphql/arg/UpdateUserInfosArgs' -import { PublishNameType } from '@/graphql/enum/PublishNameType' import { backendLogger as logger } from '@/server/logger' /** @@ -17,6 +16,7 @@ import { backendLogger as logger } from '@/server/logger' export async function syncHumhub( updateUserInfosArg: UpdateUserInfosArgs | null, user: User, + oldHumhubUsername: string, spaceId?: number | null, ): Promise { // check for humhub relevant changes @@ -38,15 +38,13 @@ export async function syncHumhub( return } logger.debug('retrieve user from humhub') - const userNameLogic = new PublishNameLogic(user) - const username = userNameLogic.getUserIdentifier(user.humhubPublishName as PublishNameType) - let humhubUser = await humhubClient.userByUsername(username) + let humhubUser = await humhubClient.userByUsername(oldHumhubUsername) if (!humhubUser) { humhubUser = await humhubClient.userByEmail(user.emailContact.email) } const humhubUsers = new Map() if (humhubUser) { - humhubUsers.set(user.emailContact.email, humhubUser) + humhubUsers.set(humhubUser.account.username, humhubUser) } logger.debug('update user at humhub') const result = await syncUser(user, humhubUsers) @@ -63,7 +61,8 @@ export async function syncHumhub( logger.debug(`user added to space ${spaceId}`) } if (result !== ExecutedHumhubAction.SKIP) { - return await humhubClient.userByUsername(username) + const getUser = new PostUser(user) + return await humhubClient.userByUsername(getUser.account.username) } return humhubUser } diff --git a/backend/test/testSetup.ts b/backend/test/testSetup.ts index 8cf4ff084..74021f0a3 100644 --- a/backend/test/testSetup.ts +++ b/backend/test/testSetup.ts @@ -6,6 +6,8 @@ import { backendLogger as logger } from '@/server/logger' CONFIG.EMAIL = true CONFIG.EMAIL_TEST_MODUS = false +CONFIG.HUMHUB_ACTIVE = false +CONFIG.GMS_ACTIVE = false jest.setTimeout(1000000)