From 8e53d3b40fb8fe4c3b7a863364bc588f6eb9240f Mon Sep 17 00:00:00 2001 From: Shikanime Deva <22115108+shikanime@users.noreply.github.com> Date: Fri, 11 Sep 2026 17:55:23 +0200 Subject: [PATCH 1/3] fix(server-nestjs): consume projectMember events in gitlab module projectMember.upsert/delete were emitted with no @OnEvent consumer, so GitLab group member sync silently stopped on the /api/v1/projects routes already cut over to server-nestjs. Add a gitlab consumer that reconciles the project group members on upsert (parity with the legacy main step) and removes the group member on delete (parity with the legacy post step; absent user or group tolerated as OK). Refs #2590 Co-authored-by: Automata Signed-off-by: William Phetsinorath Change-Id: I258b7d5035b1d05946e851297ec3a1376a6a6964 Signed-off-by: Shikanime Deva <22115108+shikanime@users.noreply.github.com> --- .../src/modules/gitlab/gitlab.service.spec.ts | 71 ++++++++++++++++++- .../src/modules/gitlab/gitlab.service.ts | 55 +++++++++++++- 2 files changed, 124 insertions(+), 2 deletions(-) diff --git a/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts b/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts index bfd72e6bf5..e451b9ea8e 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts @@ -12,7 +12,7 @@ import { OBSERVABILITY_REPOSITORY } from '../observability/observability.constan import { VaultClientService } from '../vault/vault-client.service' import { GitlabClientService } from './gitlab-client.service' import { GitlabDatastoreService } from './gitlab-datastore.service' -import { makeAccessTokenExposedSchema, makeExpandedUserSchema, makeGroupSchema, makeMemberSchema, makePipeline, makePipelineTriggerToken, makeProjectSchema, makeProjectWithDetails, makeVaultSecret } from './gitlab-testing.utils' +import { makeAccessTokenExposedSchema, makeExpandedUserSchema, makeGroupSchema, makeMemberSchema, makePipeline, makePipelineTriggerToken, makeProjectSchema, makeProjectWithDetails, makeUser, makeVaultSecret } from './gitlab-testing.utils' import { INFRA_APPS_REPO_NAME, MIRROR_REPO_NAME, PLUGIN_NAME, TOPIC_PLUGIN_MANAGED, TOPIC_SYSTEM_MANAGED } from './gitlab.constants' import { GitlabService } from './gitlab.service' @@ -56,6 +56,75 @@ describe('gitlabService', () => { expect(service).toBeDefined() }) + describe('projectMember events', () => { + it('should reconcile project members on projectMember.upsert', async () => { + const project = makeProjectWithDetails({ + members: [{ user: { id: 'u1', email: 'member@example.com', firstName: 'New', lastName: 'User', adminRoleIds: [] }, roleIds: [] }], + }) + const group = makeGroupSchema({ id: 123, name: 'project-1', path: 'project-1', full_path: 'forge/console/project-1', full_name: 'forge/console/project-1', parent_id: 1 }) + datastore.getProject.mockResolvedValue(project) + gitlab.getOrCreateProjectSubGroup.mockResolvedValue(group) + gitlab.getGroupMembers.mockResolvedValue([]) + gitlab.upsertUser.mockImplementation(async user => makeExpandedUserSchema({ + id: user.email === 'member@example.com' ? 999 : 998, + email: user.email, + username: user.email.split('@')[0] ?? user.email, + name: user.name, + })) + + const result = await service.handleProjectMemberUpsert({ projectId: project.id, userId: 'u1' }) + + expect(result.gitlab.status).toBe('OK') + expect(gitlab.addGroupMember).toHaveBeenCalledWith(group, 999, AccessLevel.GUEST) + expect(gitlab.addGroupMember).toHaveBeenCalledWith(group, 998, AccessLevel.OWNER) + }) + + it('should fail with KO when the project no longer exists on projectMember.upsert', async () => { + datastore.getProject.mockResolvedValue(null) + + const result = await service.handleProjectMemberUpsert({ projectId: 'missing', userId: 'u1' }) + + expect(result.gitlab.status).toBe('KO') + }) + + it('should remove the group member on projectMember.delete', async () => { + const project = makeProjectWithDetails() + const group = makeGroupSchema({ id: 321, name: 'project-1', path: 'project-1', full_path: 'forge/console/project-1', full_name: 'forge/console/project-1', parent_id: 1 }) + datastore.getProject.mockResolvedValue(project) + datastore.getUser.mockResolvedValue(makeUser({ id: 'u1', email: 'leaver@example.com' })) + gitlab.getUserByEmail.mockResolvedValue(makeExpandedUserSchema({ id: 555, email: 'leaver@example.com', username: 'leaver', name: 'Leaver User' })) + gitlab.getProjectGroup.mockResolvedValue(group) + + const result = await service.handleProjectMemberDelete({ projectId: project.id, userId: 'u1' }) + + expect(result.gitlab.status).toBe('OK') + expect(gitlab.removeGroupMember).toHaveBeenCalledWith(group, 555) + }) + + it('should tolerate a user absent from GitLab on projectMember.delete', async () => { + datastore.getProject.mockResolvedValue(makeProjectWithDetails()) + datastore.getUser.mockResolvedValue(makeUser({ id: 'u1', email: 'ghost@example.com' })) + gitlab.getUserByEmail.mockResolvedValue(null) + + const result = await service.handleProjectMemberDelete({ projectId: 'p1', userId: 'u1' }) + + expect(result.gitlab.status).toBe('OK') + expect(gitlab.removeGroupMember).not.toHaveBeenCalled() + }) + + it('should tolerate a missing GitLab group on projectMember.delete', async () => { + datastore.getProject.mockResolvedValue(makeProjectWithDetails()) + datastore.getUser.mockResolvedValue(makeUser({ id: 'u1', email: 'leaver@example.com' })) + gitlab.getUserByEmail.mockResolvedValue(makeExpandedUserSchema({ id: 555, email: 'leaver@example.com', username: 'leaver', name: 'Leaver User' })) + gitlab.getProjectGroup.mockResolvedValue(undefined) + + const result = await service.handleProjectMemberDelete({ projectId: 'p1', userId: 'u1' }) + + expect(result.gitlab.status).toBe('OK') + expect(gitlab.removeGroupMember).not.toHaveBeenCalled() + }) + }) + describe('handleUpsert', () => { it('should reconcile project members and repositories', async () => { const project = makeProjectWithDetails() diff --git a/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts b/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts index a5b0f91611..c41bfc76b7 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts @@ -1,6 +1,6 @@ import type { MemberSchema } from '@gitbeaker/core' import type { ConfigType } from '@nestjs/config' -import type { AdminRoleEventPayload, RepositorySyncEventPayload, ZoneEventPayload } from '../events/app-events.service' +import type { AdminRoleEventPayload, ProjectMemberEventPayload, RepositorySyncEventPayload, ZoneEventPayload } from '../events/app-events.service' import type { RequiredPluginResult } from '../plugin/plugin.utils' import type { MirrorUserSecret, VaultSecret } from '../vault/vault-client.service' import type { GroupSchemaWith } from './gitlab-client.service' @@ -161,6 +161,16 @@ export class GitlabService { this.logger.log(`GitLab zone cleanup completed for ${payload.slug}`) } + @OnEvent('projectMember.upsert') + async handleProjectMemberUpsert(payload: ProjectMemberEventPayload): Promise> { + return capturePluginResult('gitlab', () => this.syncProjectMembers(payload.projectId)) + } + + @OnEvent('projectMember.delete') + async handleProjectMemberDelete(payload: ProjectMemberEventPayload): Promise> { + return capturePluginResult('gitlab', () => this.removeProjectMember(payload)) + } + @StartActiveSpan() private async syncRepositoryMirror(payload: RepositorySyncEventPayload) { const { projectSlug, internalRepoName, syncAllBranches } = payload @@ -192,6 +202,49 @@ export class GitlabService { this.logger.log(`GitLab cleanup completed for project ${project.slug}`) } + @StartActiveSpan() + private async syncProjectMembers(projectId: string) { + const span = trace.getActiveSpan() + span?.setAttribute('project.id', projectId) + this.logger.log(`Handling a project member upsert event (projectId=${projectId})`) + const project = await this.datastore.getProject(projectId) + if (!project) { + throw new Error(`Project not found for member sync (projectId=${projectId})`) + } + const group = await this.gitlab.getOrCreateProjectSubGroup(project.slug) + const members = await this.gitlab.getGroupMembers(group) + await this.ensureProjectGroupMembers(project, group, members) + this.logger.log(`GitLab member sync completed (projectId=${projectId}, slug=${project.slug})`) + } + + @StartActiveSpan() + private async removeProjectMember(payload: ProjectMemberEventPayload) { + const span = trace.getActiveSpan() + span?.setAttribute('project.id', payload.projectId) + span?.setAttribute('project.member.userId', payload.userId) + this.logger.log(`Handling a project member delete event (projectId=${payload.projectId}, userId=${payload.userId})`) + const user = await this.datastore.getUser(payload.userId) + if (!user) { + throw new Error(`User not found for member removal (userId=${payload.userId})`) + } + const project = await this.datastore.getProject(payload.projectId) + if (!project) { + throw new Error(`Project not found for member removal (projectId=${payload.projectId})`) + } + const gitlabUser = await this.gitlab.getUserByEmail(user.email) + if (!gitlabUser) { + this.logger.log(`User absent from GitLab, nothing to remove (projectId=${payload.projectId}, userId=${payload.userId})`) + return + } + const group = await this.gitlab.getProjectGroup(project.slug) + if (!group) { + this.logger.log(`No GitLab group for project, nothing to remove (projectId=${payload.projectId}, slug=${project.slug})`) + return + } + await this.gitlab.removeGroupMember(group, gitlabUser.id) + this.logger.log(`GitLab member removed (projectId=${payload.projectId}, gitlabUserId=${gitlabUser.id})`) + } + // @Cron(CronExpression.EVERY_HOUR) @StartActiveSpan() async handleCron() { From b2ef474162a2dc5ce3a0ac84ebeb64f2f49722d8 Mon Sep 17 00:00:00 2001 From: Shikanime Deva <22115108+shikanime@users.noreply.github.com> Date: Fri, 25 Sep 2026 17:36:50 +0200 Subject: [PATCH 2/3] fix(gitlab): sync project members on upsert and delete events Co-authored-by: Automata Signed-off-by: Shikanime Deva <22115108+shikanime@users.noreply.github.com> --- .../src/modules/gitlab/gitlab.service.spec.ts | 4 +++- .../src/modules/gitlab/gitlab.service.ts | 15 ++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts b/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts index e451b9ea8e..8a2c42719f 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts @@ -56,7 +56,7 @@ describe('gitlabService', () => { expect(service).toBeDefined() }) - describe('projectMember events', () => { + describe('handleProjectMemberUpsert', () => { it('should reconcile project members on projectMember.upsert', async () => { const project = makeProjectWithDetails({ members: [{ user: { id: 'u1', email: 'member@example.com', firstName: 'New', lastName: 'User', adminRoleIds: [] }, roleIds: [] }], @@ -86,7 +86,9 @@ describe('gitlabService', () => { expect(result.gitlab.status).toBe('KO') }) + }) + describe('handleProjectMemberDelete', () => { it('should remove the group member on projectMember.delete', async () => { const project = makeProjectWithDetails() const group = makeGroupSchema({ id: 321, name: 'project-1', path: 'project-1', full_path: 'forge/console/project-1', full_name: 'forge/console/project-1', parent_id: 1 }) diff --git a/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts b/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts index c41bfc76b7..c28c155090 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts @@ -163,7 +163,7 @@ export class GitlabService { @OnEvent('projectMember.upsert') async handleProjectMemberUpsert(payload: ProjectMemberEventPayload): Promise> { - return capturePluginResult('gitlab', () => this.syncProjectMembers(payload.projectId)) + return capturePluginResult('gitlab', () => this.syncProjectMembers(payload)) } @OnEvent('projectMember.delete') @@ -203,18 +203,19 @@ export class GitlabService { } @StartActiveSpan() - private async syncProjectMembers(projectId: string) { + private async syncProjectMembers(payload: ProjectMemberEventPayload) { const span = trace.getActiveSpan() - span?.setAttribute('project.id', projectId) - this.logger.log(`Handling a project member upsert event (projectId=${projectId})`) - const project = await this.datastore.getProject(projectId) + span?.setAttribute('project.id', payload.projectId) + span?.setAttribute('project.member.userId', payload.userId) + this.logger.log(`Handling a project member upsert event (projectId=${payload.projectId}, userId=${payload.userId})`) + const project = await this.datastore.getProject(payload.projectId) if (!project) { - throw new Error(`Project not found for member sync (projectId=${projectId})`) + throw new Error(`Project not found for member sync (projectId=${payload.projectId})`) } const group = await this.gitlab.getOrCreateProjectSubGroup(project.slug) const members = await this.gitlab.getGroupMembers(group) await this.ensureProjectGroupMembers(project, group, members) - this.logger.log(`GitLab member sync completed (projectId=${projectId}, slug=${project.slug})`) + this.logger.log(`GitLab member sync completed (projectId=${payload.projectId}, slug=${project.slug})`) } @StartActiveSpan() From 44af7c5f399de3c7ddda39ce2afb77a18241fdd5 Mon Sep 17 00:00:00 2001 From: William Phetsinorath Date: Fri, 16 Oct 2026 16:38:30 +0200 Subject: [PATCH 3/3] fix(gitlab): match users by exact email and repair the member-sync spec Co-authored-by: Automata Signed-off-by: William Phetsinorath Change-Id: I3a35122664f8d2616ce94fe6d073a80e6a6a6964 --- .../src/modules/gitlab/gitlab-client.service.ts | 5 +++-- apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts | 4 ++-- apps/server-nestjs/src/modules/gitlab/gitlab.service.ts | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/apps/server-nestjs/src/modules/gitlab/gitlab-client.service.ts b/apps/server-nestjs/src/modules/gitlab/gitlab-client.service.ts index dd5b92274a..277f293937 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab-client.service.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab-client.service.ts @@ -432,9 +432,10 @@ export class GitlabClientService { } async getUserByEmail(email: string) { + // Fuzzy search matches username and display name too; only an exact + // email hit is safe on destructive paths (member removal). const users = await this.client.Users.all({ search: email, orderBy: 'username' }) - if (users.length === 0) return undefined - return users[0] as UserSchema + return users.find(user => user.email === email) as UserSchema | undefined } async createUser(user: EditUserOptions) { diff --git a/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts b/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts index 8a2c42719f..49ec9b9b15 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts @@ -63,9 +63,9 @@ describe('gitlabService', () => { }) const group = makeGroupSchema({ id: 123, name: 'project-1', path: 'project-1', full_path: 'forge/console/project-1', full_name: 'forge/console/project-1', parent_id: 1 }) datastore.getProject.mockResolvedValue(project) - gitlab.getOrCreateProjectSubGroup.mockResolvedValue(group) + gitlab.ensureProjectSubGroup.mockResolvedValue(group) gitlab.getGroupMembers.mockResolvedValue([]) - gitlab.upsertUser.mockImplementation(async user => makeExpandedUserSchema({ + gitlab.ensureUser.mockImplementation(async user => makeExpandedUserSchema({ id: user.email === 'member@example.com' ? 999 : 998, email: user.email, username: user.email.split('@')[0] ?? user.email, diff --git a/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts b/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts index c28c155090..695e5949fe 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts @@ -6,13 +6,13 @@ import type { MirrorUserSecret, VaultSecret } from '../vault/vault-client.servic import type { GroupSchemaWith } from './gitlab-client.service' import type { ProjectWithDetails } from './gitlab-datastore.service' import { specificallyEnabled } from '@cpn-console/hooks' +import { daysAgo } from '@cpn-console/shared' import { AccessLevel } from '@gitbeaker/core' import { Inject, Injectable, Logger } from '@nestjs/common' import { OnEvent } from '@nestjs/event-emitter' import { trace } from '@opentelemetry/api' import { gitlabConfigFactory } from '../../config/gitlab.config' import { getAll } from '../../utils/iterable.utils' -import { daysAgo } from '@cpn-console/shared' import { StartActiveSpan } from '../infrastructure/telemetry/telemetry.decorator' import { capturePluginResult } from '../plugin/plugin.utils' import { VaultClientService } from '../vault/vault-client.service' @@ -212,7 +212,7 @@ export class GitlabService { if (!project) { throw new Error(`Project not found for member sync (projectId=${payload.projectId})`) } - const group = await this.gitlab.getOrCreateProjectSubGroup(project.slug) + const group = await this.gitlab.ensureProjectSubGroup(project.slug) const members = await this.gitlab.getGroupMembers(group) await this.ensureProjectGroupMembers(project, group, members) this.logger.log(`GitLab member sync completed (projectId=${payload.projectId}, slug=${project.slug})`)