From 4860cc2670e89e95f09ab20253a8369325854dad Mon Sep 17 00:00:00 2001 From: einhornimmond Date: Thu, 24 Apr 2025 13:05:16 +0200 Subject: [PATCH] use old humhub username on update for finding humhub user to prevent new user if user has changed email in humhub and alias in wallet --- backend/src/apis/humhub/ExportUsers.ts | 4 ++-- backend/src/apis/humhub/__mocks__/syncUser.ts | 4 +++- backend/src/apis/humhub/syncUser.test.ts | 7 ++++--- backend/src/apis/humhub/syncUser.ts | 7 +++---- backend/src/graphql/resolver/UserResolver.ts | 20 ++++++++++++++----- .../graphql/resolver/util/syncHumhub.test.ts | 4 ++-- .../src/graphql/resolver/util/syncHumhub.ts | 13 ++++++------ backend/test/testSetup.ts | 2 ++ 8 files changed, 37 insertions(+), 24 deletions(-) diff --git a/backend/src/apis/humhub/ExportUsers.ts b/backend/src/apis/humhub/ExportUsers.ts index 5214fef5d..4ed6f2928 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() @@ -43,7 +43,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++ } 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/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..8057626af 100644 --- a/backend/src/apis/humhub/syncUser.ts +++ b/backend/src/apis/humhub/syncUser.ts @@ -21,9 +21,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 +30,7 @@ export async function syncUser( humhubUsers: Map, ): Promise { const postUser = new PostUser(user) - const humhubUser = humhubUsers.get(user.emailContact.email.trim()) + 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/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)