From 64356be59e45cd39ba7d01567d2558ec4213283b Mon Sep 17 00:00:00 2001 From: William Phetsinorath Date: Tue, 6 Oct 2026 10:09:21 +0200 Subject: [PATCH 1/3] feat(server-nestjs): consume adminRole events in keycloak and gitlab modules Design: bridge adminRole.upsert/delete events emitted by admin-role.service and user migration to keycloak (group membership reconcile via ensureAdminRoleGroup, delete revokes from payload members) and gitlab (admin/auditor flags from the role oidc group, mirrors legacy upsertAdminRole/deleteAdminRole plugin functions). Shared payload type AdminRoleEventPayload in app-events.service.ts. Rebased onto current main; drops the stray packages/ts-config artifact flagged in review. Refs #2723 Co-authored-by: Automata Signed-off-by: William Phetsinorath Change-Id: I5e020c8ea72c6ba782e73c62e29e9aad6a6a6964 --- .../modules/events/admin-role-chain.spec.ts | 79 +++++++++++++++++++ .../src/modules/events/app-events.service.ts | 13 +++ .../modules/gitlab/admin-role-bridge.spec.ts | 61 ++++++++++++++ .../src/modules/gitlab/gitlab.service.ts | 45 ++++++++++- .../keycloak/admin-role-bridge.spec.ts | 56 +++++++++++++ .../src/modules/keycloak/keycloak.service.ts | 46 ++++++++++- 6 files changed, 295 insertions(+), 5 deletions(-) create mode 100644 apps/server-nestjs/src/modules/events/admin-role-chain.spec.ts create mode 100644 apps/server-nestjs/src/modules/gitlab/admin-role-bridge.spec.ts create mode 100644 apps/server-nestjs/src/modules/keycloak/admin-role-bridge.spec.ts diff --git a/apps/server-nestjs/src/modules/events/admin-role-chain.spec.ts b/apps/server-nestjs/src/modules/events/admin-role-chain.spec.ts new file mode 100644 index 0000000000..bcc73965ee --- /dev/null +++ b/apps/server-nestjs/src/modules/events/admin-role-chain.spec.ts @@ -0,0 +1,79 @@ +import type { ConfigType } from '@nestjs/config' +import type { DeepMockProxy } from 'vitest-mock-extended' +import type { AdminRoleEventPayload } from './app-events.service' +import { EventEmitter2, EventEmitterModule } from '@nestjs/event-emitter' +import { Test } from '@nestjs/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { mockDeep } from 'vitest-mock-extended' +import { gitlabConfigFactory } from '../../config/gitlab.config' +import { GitlabClientService } from '../gitlab/gitlab-client.service' +import { GitlabDatastoreService } from '../gitlab/gitlab-datastore.service' +import { GitlabService } from '../gitlab/gitlab.service' +import { KeycloakClientService } from '../keycloak/keycloak-client.service' +import { KeycloakDatastoreService } from '../keycloak/keycloak-datastore.service' +import { makeGroupRepresentation } from '../keycloak/keycloak-testing.utils' +import { KeycloakService } from '../keycloak/keycloak.service' +import { VaultClientService } from '../vault/vault-client.service' + +describe('adminRole event chain', () => { + let eventEmitter: EventEmitter2 + let keycloak: DeepMockProxy + let gitlab: DeepMockProxy + + beforeEach(async () => { + keycloak = mockDeep({ + getOrCreateGroupByPath: vi.fn().mockResolvedValue(makeGroupRepresentation({ id: 'kc-group-id', name: 'admin' })), + getGroupMembers: vi.fn().mockResolvedValue([]), + }) + gitlab = mockDeep({ + upsertUser: vi.fn().mockResolvedValue({ id: 1 }), + }) + + const moduleRef = await Test.createTestingModule({ + imports: [EventEmitterModule.forRoot()], + providers: [ + KeycloakService, + GitlabService, + { provide: KeycloakClientService, useValue: keycloak }, + { provide: KeycloakDatastoreService, useValue: mockDeep({ + getAllAdminRoles: vi.fn().mockResolvedValue([{ id: 'role-1', oidcGroup: '/console/admin', type: 'global' }]), + getAllUsersWithAdminRoleIds: vi.fn().mockResolvedValue([{ id: 'user-1', adminRoleIds: ['role-1'] }]), + }) }, + { provide: GitlabClientService, useValue: gitlab }, + { provide: GitlabDatastoreService, useValue: mockDeep({ + getAdminPluginConfig: vi.fn().mockResolvedValue(null), + }) }, + { provide: VaultClientService, useValue: mockDeep() }, + { provide: gitlabConfigFactory.KEY, useValue: mockDeep>({}) }, + ], + }).compile() + + const app = moduleRef.createNestApplication() + await app.init() + + eventEmitter = moduleRef.get(EventEmitter2) + }) + + it('delivers the canonical AdminRoleEventPayload to both consumers on adminRole.upsert', async () => { + const payload: AdminRoleEventPayload = { + id: 'role-1', + oidcGroup: '/console/admin', + members: [{ id: 'u1', email: 'a@b.c', firstName: 'A', lastName: 'B' }], + } + + const results = await eventEmitter.emitAsync('adminRole.upsert', payload) + + expect(keycloak.getOrCreateGroupByPath).toHaveBeenCalledWith('/console/admin') + expect(keycloak.addUserToGroup).toHaveBeenCalledWith('user-1', 'kc-group-id') + + expect(gitlab.upsertUser).toHaveBeenCalledWith( + expect.objectContaining({ email: 'a@b.c', admin: true }), + { cpnUserId: 'u1' }, + ) + + expect(results).toEqual(expect.arrayContaining([ + { keycloak: expect.objectContaining({ status: 'OK' }) }, + { gitlab: expect.objectContaining({ status: 'OK' }) }, + ])) + }) +}) diff --git a/apps/server-nestjs/src/modules/events/app-events.service.ts b/apps/server-nestjs/src/modules/events/app-events.service.ts index ad3861f697..32e1e6d612 100644 --- a/apps/server-nestjs/src/modules/events/app-events.service.ts +++ b/apps/server-nestjs/src/modules/events/app-events.service.ts @@ -34,6 +34,19 @@ export type RepositorySyncEventPayload = { | { syncAllBranches: false, branchName: string } ) +export interface AdminRoleEventMember { + id: string + email: string + firstName: string + lastName: string +} + +export interface AdminRoleEventPayload { + id: string + oidcGroup: string | null + members: AdminRoleEventMember[] +} + /** Admin-log action labels (legacy hooks wording). */ export type EventLogAction = | 'Create Project' | 'Update Project' | 'Delete all project resources' diff --git a/apps/server-nestjs/src/modules/gitlab/admin-role-bridge.spec.ts b/apps/server-nestjs/src/modules/gitlab/admin-role-bridge.spec.ts new file mode 100644 index 0000000000..c3b5bda7fc --- /dev/null +++ b/apps/server-nestjs/src/modules/gitlab/admin-role-bridge.spec.ts @@ -0,0 +1,61 @@ +import type { GitlabConfig } from '../../config/gitlab.config' +import type { AdminRoleEventPayload } from '../events/app-events.service' +import type { VaultClientService } from '../vault/vault-client.service' +import type { GitlabClientService } from './gitlab-client.service' +import type { GitlabDatastoreService } from './gitlab-datastore.service' +import { describe, expect, it } from 'vitest' +import { mockDeep } from 'vitest-mock-extended' +import { GitlabService } from './gitlab.service' + +const gitlabConfig = { + token: 'token', + url: 'https://gitlab.example.com', + internalUrl: undefined, + secretExposeInternalUrl: false, + mirrorTokenExpirationDays: 365, + mirrorTokenRotationThresholdDays: 250, + projectRootDir: '/projects', +} satisfies GitlabConfig + +function buildService({ adminGroupPath, auditorGroupPath }: { adminGroupPath?: string, auditorGroupPath?: string } = {}) { + const datastore = mockDeep() + datastore.getAdminPluginConfig.mockImplementation(async (_plugin: string, key: string) => { + if (key === 'adminGroupPath') return adminGroupPath ?? null + if (key === 'auditorGroupPath') return auditorGroupPath ?? null + return null + }) + const gitlab = mockDeep() + const service = new GitlabService(datastore, gitlab, mockDeep(), gitlabConfig) + return { service, gitlab } +} + +function role(oidcGroup: string | null): AdminRoleEventPayload { + return { + id: 'role-1', + oidcGroup, + members: [{ id: 'u1', email: 'a@b.c', firstName: 'A', lastName: 'B' }], + } +} + +describe('gitlab adminRole event bridge', () => { + it('skips roles outside managed group paths', async () => { + const { service, gitlab } = buildService() + await service.handleAdminRoleUpsert(role('/other')) + expect(gitlab.upsertUser).not.toHaveBeenCalled() + }) + + it('flags admin for the admin group and auditor otherwise', async () => { + const { service, gitlab } = buildService({ adminGroupPath: '/console/admin' }) + await service.handleAdminRoleUpsert(role('/console/admin')) + expect(gitlab.upsertUser).toHaveBeenCalledWith(expect.objectContaining({ admin: true }), expect.anything()) + + await service.handleAdminRoleUpsert(role('/console/readonly')) + expect(gitlab.upsertUser).toHaveBeenLastCalledWith(expect.objectContaining({ auditor: true, admin: undefined }), expect.anything()) + }) + + it('revoke (delete) clears the flags', async () => { + const { service, gitlab } = buildService() + await service.handleAdminRoleDelete(role('/console/admin')) + expect(gitlab.upsertUser).toHaveBeenCalledWith(expect.objectContaining({ admin: false }), expect.anything()) + }) +}) diff --git a/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts b/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts index 2b54ae6062..09f0e12bb8 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 { RepositorySyncEventPayload } from '../events/app-events.service' +import type { AdminRoleEventPayload, RepositorySyncEventPayload } 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' @@ -85,6 +85,43 @@ export class GitlabService { return capturePluginResult('gitlab', () => this.cleanupProject(project)) } + @OnEvent('adminRole.upsert') + async handleAdminRoleUpsert(role: AdminRoleEventPayload): Promise> { + return capturePluginResult('gitlab', () => this.syncAdminRole(role)) + } + + @OnEvent('adminRole.delete') + async handleAdminRoleDelete(role: AdminRoleEventPayload): Promise> { + return capturePluginResult('gitlab', () => this.syncAdminRole(role, false)) + } + + @StartActiveSpan() + private async syncAdminRole(role: AdminRoleEventPayload, enabled = true) { + const span = trace.getActiveSpan() + span?.setAttribute('admin_role.id', role.id) + this.logger.log(`Handling an admin role ${enabled ? 'upsert' : 'delete'} event for ${role.id}`) + + const adminGroupPath = await this.getAdminGroupPath() + const auditorGroupPaths = parseGroupPaths(await this.getAuditorGroupPath()) + const isAuditorRole = auditorGroupPaths.includes(role.oidcGroup ?? '') + if (role.oidcGroup !== adminGroupPath && !isAuditorRole) { + this.logger.verbose(`Not a managed role for GitLab plugin (roleId=${role.id})`) + return + } + + for (const member of role.members) { + await this.gitlab.upsertUser({ + email: member.email, + username: generateUsername(member.email), + name: generateName(member.firstName, member.lastName), + admin: role.oidcGroup === adminGroupPath ? enabled : undefined, + auditor: isAuditorRole ? enabled : undefined, + }, { + cpnUserId: member.id, + }) + } + } + @OnEvent('repository.sync') async handleRepositorySync(payload: RepositorySyncEventPayload): Promise> { return capturePluginResult('gitlab', () => this.syncRepositoryMirror(payload)) @@ -258,15 +295,15 @@ export class GitlabService { return generateAdminRoleMapping(roles, adminGroupPaths, auditorGroupPaths) } - private async getAdminGroupPath(project: ProjectWithDetails): Promise { + private async getAdminGroupPath(project?: ProjectWithDetails): Promise { return await this.getAdminOrProjectPluginConfig(project, ADMIN_GROUP_PATH_PLUGIN_KEY) ?? DEFAULT_ADMIN_GROUP_PATH } - private async getAuditorGroupPath(project: ProjectWithDetails): Promise { + private async getAuditorGroupPath(project?: ProjectWithDetails): Promise { return await this.getAdminOrProjectPluginConfig(project, AUDITOR_GROUP_PATH_PLUGIN_KEY) ?? DEFAULT_AUDITOR_GROUP_PATH } - private async getAdminOrProjectPluginConfig(project: ProjectWithDetails, key: string): Promise { + private async getAdminOrProjectPluginConfig(project: ProjectWithDetails | undefined, key: string): Promise { const adminPluginConfig = await this.datastore.getAdminPluginConfig(PLUGIN_NAME, key) if (adminPluginConfig) return adminPluginConfig if (!project) return undefined diff --git a/apps/server-nestjs/src/modules/keycloak/admin-role-bridge.spec.ts b/apps/server-nestjs/src/modules/keycloak/admin-role-bridge.spec.ts new file mode 100644 index 0000000000..690005c75f --- /dev/null +++ b/apps/server-nestjs/src/modules/keycloak/admin-role-bridge.spec.ts @@ -0,0 +1,56 @@ +import type { DeepMockProxy } from 'vitest-mock-extended' +import type { AdminRoleWithDetails, UserWithAdminRoles } from './keycloak-datastore.service' +import { Test } from '@nestjs/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { mockDeep } from 'vitest-mock-extended' +import { KeycloakClientService } from './keycloak-client.service' +import { KeycloakDatastoreService } from './keycloak-datastore.service' +import { makeGroupRepresentation, makeUserRepresentation } from './keycloak-testing.utils' +import { KeycloakService } from './keycloak.service' + +describe('keycloak adminRole event bridge', () => { + let service: KeycloakService + let keycloak: DeepMockProxy + let datastore: DeepMockProxy + + beforeEach(async () => { + keycloak = mockDeep({ + getOrCreateGroupByPath: vi.fn().mockResolvedValue(makeGroupRepresentation({ id: 'kc-group-id', name: 'admin' })), + getGroupMembers: vi.fn().mockResolvedValue([]), + }) + datastore = mockDeep({}) + + const moduleRef = await Test.createTestingModule({ + providers: [ + KeycloakService, + { provide: KeycloakClientService, useValue: keycloak }, + { provide: KeycloakDatastoreService, useValue: datastore }, + ], + }).compile() + + service = moduleRef.get(KeycloakService) + }) + + it('syncs the impacted role group on adminRole.upsert', async () => { + const roles: AdminRoleWithDetails[] = [{ id: 'role-1', oidcGroup: '/console/admin', type: 'global' }] + const users: UserWithAdminRoles[] = [{ id: 'user-1', adminRoleIds: ['role-1'] }] + datastore.getAllAdminRoles.mockResolvedValue(roles) + datastore.getAllUsersWithAdminRoleIds.mockResolvedValue(users) + keycloak.getGroupMembers.mockResolvedValue([makeUserRepresentation({ id: 'user-2' })]) + + await service.handleAdminRoleUpsert({ id: 'role-1', oidcGroup: '/console/admin', members: [] }) + + expect(keycloak.getOrCreateGroupByPath).toHaveBeenCalledWith('/console/admin') + expect(keycloak.addUserToGroup).toHaveBeenCalledWith('user-1', 'kc-group-id') + expect(keycloak.removeUserFromGroup).toHaveBeenCalledWith('user-2', 'kc-group-id') + }) + + it('warns and no-ops when the role no longer exists', async () => { + datastore.getAllAdminRoles.mockResolvedValue([]) + datastore.getAllUsersWithAdminRoleIds.mockResolvedValue([]) + + await service.handleAdminRoleDelete({ id: 'gone', oidcGroup: '/console/admin', members: [] }) + + expect(keycloak.getOrCreateGroupByPath).not.toHaveBeenCalled() + }) +}) diff --git a/apps/server-nestjs/src/modules/keycloak/keycloak.service.ts b/apps/server-nestjs/src/modules/keycloak/keycloak.service.ts index 5947fbde1e..796c08e508 100644 --- a/apps/server-nestjs/src/modules/keycloak/keycloak.service.ts +++ b/apps/server-nestjs/src/modules/keycloak/keycloak.service.ts @@ -1,4 +1,5 @@ import type UserRepresentation from '@keycloak/keycloak-admin-client/lib/defs/userRepresentation' +import type { AdminRoleEventPayload } from '../events/app-events.service' import type { RequiredPluginResult } from '../plugin/plugin.utils' import type { AdminRoleWithDetails, ProjectWithDetails, UserWithAdminRoles } from './keycloak-datastore.service' import type { GroupRepresentationWith, GroupRepresentationWithIdNamePath } from './keycloak.utils' @@ -56,6 +57,49 @@ export class KeycloakService { this.logger.log(`Keycloak cleanup completed for project ${project.slug}`) } + @OnEvent('adminRole.upsert') + async handleAdminRoleUpsert(role: AdminRoleEventPayload): Promise> { + return capturePluginResult('keycloak', () => this.syncAdminRole(role.id)) + } + + @OnEvent('adminRole.delete') + async handleAdminRoleDelete(role: AdminRoleEventPayload): Promise> { + return capturePluginResult('keycloak', () => this.revokeAdminRoleGroup(role)) + } + + @StartActiveSpan() + private async revokeAdminRoleGroup(role: AdminRoleEventPayload) { + const span = trace.getActiveSpan() + span?.setAttribute('admin_role.id', role.id) + const roleGroupPath = toGroupPath(role.oidcGroup) + if (!roleGroupPath) return + const roleGroup = await this.keycloak.getGroupByPath(roleGroupPath) + if (!roleGroup?.id) { + this.logger.warn(`Keycloak group not found for deleted admin role (roleId=${role.id}, path=${roleGroupPath})`) + return + } + for (const member of role.members) { + await this.maybeRemoveUserFromGroup(member.id, roleGroup.id, roleGroup.name) + } + } + + @StartActiveSpan() + private async syncAdminRole(roleId: string) { + const span = trace.getActiveSpan() + span?.setAttribute('admin_role.id', roleId) + this.logger.log(`Handling an admin role event for ${roleId}`) + const [roles, users] = await Promise.all([ + this.datastore.getAllAdminRoles(), + this.datastore.getAllUsersWithAdminRoleIds(), + ]) + const role = roles.find(({ id }) => id === roleId) + if (!role) { + this.logger.warn(`Admin role not found for event (roleId=${roleId})`) + return + } + await this.ensureAdminRoleGroup(role, users) + } + // @Cron(CronExpression.EVERY_HOUR) @StartActiveSpan() async handleCron() { @@ -147,7 +191,7 @@ export class KeycloakService { span?.setAttribute('admin_role.id', role.id) span?.setAttribute('admin_role.oidc_group.present', isNonEmptyGroupPath(role.oidcGroup)) const roleGroupPath = toGroupPath(role.oidcGroup) - if (!roleGroupPath) return + if (!roleGroupPath || isExternalRoleType(role.type)) return span?.setAttribute('keycloak.group.path', roleGroupPath) const roleGroup = await this.keycloak.getOrCreateGroupByPath(roleGroupPath) From 5b251ae25b857ee020d227f676c77600eafd930a Mon Sep 17 00:00:00 2001 From: William Phetsinorath Date: Thu, 8 Oct 2026 10:26:37 +0200 Subject: [PATCH 2/3] fix(server-nestjs): honor multi-path admin config and skip absent GitLab accounts on revoke Co-authored-by: Automata Signed-off-by: William Phetsinorath Change-Id: I6b5e959fc804f5ea7d4cd9e06bfebabc6a6a6964 --- .../events/app-events-testing.utils.ts | 19 ++++++ ...hain.spec.ts => app-events.module.spec.ts} | 17 ++++-- .../modules/gitlab/admin-role-bridge.spec.ts | 61 ------------------- .../src/modules/gitlab/gitlab.service.spec.ts | 61 +++++++++++++++++++ .../src/modules/gitlab/gitlab.service.ts | 16 +++-- .../keycloak/admin-role-bridge.spec.ts | 56 ----------------- .../modules/keycloak/keycloak.service.spec.ts | 24 ++++++++ 7 files changed, 128 insertions(+), 126 deletions(-) create mode 100644 apps/server-nestjs/src/modules/events/app-events-testing.utils.ts rename apps/server-nestjs/src/modules/events/{admin-role-chain.spec.ts => app-events.module.spec.ts} (90%) delete mode 100644 apps/server-nestjs/src/modules/gitlab/admin-role-bridge.spec.ts delete mode 100644 apps/server-nestjs/src/modules/keycloak/admin-role-bridge.spec.ts diff --git a/apps/server-nestjs/src/modules/events/app-events-testing.utils.ts b/apps/server-nestjs/src/modules/events/app-events-testing.utils.ts new file mode 100644 index 0000000000..c4764dc7b3 --- /dev/null +++ b/apps/server-nestjs/src/modules/events/app-events-testing.utils.ts @@ -0,0 +1,19 @@ +import type { AdminRoleEventMember, AdminRoleEventPayload } from './app-events.service' +import { faker } from '@faker-js/faker' + +export function makeAdminRoleEventMember(overrides: { id?: string, email?: string, firstName?: string, lastName?: string } = {}): AdminRoleEventMember { + return { + id: overrides.id ?? faker.string.uuid(), + email: overrides.email ?? faker.internet.email(), + firstName: overrides.firstName ?? faker.person.firstName(), + lastName: overrides.lastName ?? faker.person.lastName(), + } +} + +export function makeAdminRoleEventPayload(overrides: { id?: string, oidcGroup?: string | null, members?: AdminRoleEventMember[] } = {}): AdminRoleEventPayload { + return { + id: overrides.id ?? faker.string.uuid(), + oidcGroup: overrides.oidcGroup ?? null, + members: overrides.members ?? [makeAdminRoleEventMember()], + } +} diff --git a/apps/server-nestjs/src/modules/events/admin-role-chain.spec.ts b/apps/server-nestjs/src/modules/events/app-events.module.spec.ts similarity index 90% rename from apps/server-nestjs/src/modules/events/admin-role-chain.spec.ts rename to apps/server-nestjs/src/modules/events/app-events.module.spec.ts index bcc73965ee..440697115c 100644 --- a/apps/server-nestjs/src/modules/events/admin-role-chain.spec.ts +++ b/apps/server-nestjs/src/modules/events/app-events.module.spec.ts @@ -1,6 +1,5 @@ import type { ConfigType } from '@nestjs/config' import type { DeepMockProxy } from 'vitest-mock-extended' -import type { AdminRoleEventPayload } from './app-events.service' import { EventEmitter2, EventEmitterModule } from '@nestjs/event-emitter' import { Test } from '@nestjs/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -14,8 +13,9 @@ import { KeycloakDatastoreService } from '../keycloak/keycloak-datastore.service import { makeGroupRepresentation } from '../keycloak/keycloak-testing.utils' import { KeycloakService } from '../keycloak/keycloak.service' import { VaultClientService } from '../vault/vault-client.service' +import { makeAdminRoleEventMember, makeAdminRoleEventPayload } from './app-events-testing.utils' -describe('adminRole event chain', () => { +describe('appEventsModule', () => { let eventEmitter: EventEmitter2 let keycloak: DeepMockProxy let gitlab: DeepMockProxy @@ -55,11 +55,18 @@ describe('adminRole event chain', () => { }) it('delivers the canonical AdminRoleEventPayload to both consumers on adminRole.upsert', async () => { - const payload: AdminRoleEventPayload = { + const payload = makeAdminRoleEventPayload({ id: 'role-1', oidcGroup: '/console/admin', - members: [{ id: 'u1', email: 'a@b.c', firstName: 'A', lastName: 'B' }], - } + members: [ + makeAdminRoleEventMember({ + id: 'u1', + email: 'a@b.c', + firstName: 'A', + lastName: 'B', + }), + ], + }) const results = await eventEmitter.emitAsync('adminRole.upsert', payload) diff --git a/apps/server-nestjs/src/modules/gitlab/admin-role-bridge.spec.ts b/apps/server-nestjs/src/modules/gitlab/admin-role-bridge.spec.ts deleted file mode 100644 index c3b5bda7fc..0000000000 --- a/apps/server-nestjs/src/modules/gitlab/admin-role-bridge.spec.ts +++ /dev/null @@ -1,61 +0,0 @@ -import type { GitlabConfig } from '../../config/gitlab.config' -import type { AdminRoleEventPayload } from '../events/app-events.service' -import type { VaultClientService } from '../vault/vault-client.service' -import type { GitlabClientService } from './gitlab-client.service' -import type { GitlabDatastoreService } from './gitlab-datastore.service' -import { describe, expect, it } from 'vitest' -import { mockDeep } from 'vitest-mock-extended' -import { GitlabService } from './gitlab.service' - -const gitlabConfig = { - token: 'token', - url: 'https://gitlab.example.com', - internalUrl: undefined, - secretExposeInternalUrl: false, - mirrorTokenExpirationDays: 365, - mirrorTokenRotationThresholdDays: 250, - projectRootDir: '/projects', -} satisfies GitlabConfig - -function buildService({ adminGroupPath, auditorGroupPath }: { adminGroupPath?: string, auditorGroupPath?: string } = {}) { - const datastore = mockDeep() - datastore.getAdminPluginConfig.mockImplementation(async (_plugin: string, key: string) => { - if (key === 'adminGroupPath') return adminGroupPath ?? null - if (key === 'auditorGroupPath') return auditorGroupPath ?? null - return null - }) - const gitlab = mockDeep() - const service = new GitlabService(datastore, gitlab, mockDeep(), gitlabConfig) - return { service, gitlab } -} - -function role(oidcGroup: string | null): AdminRoleEventPayload { - return { - id: 'role-1', - oidcGroup, - members: [{ id: 'u1', email: 'a@b.c', firstName: 'A', lastName: 'B' }], - } -} - -describe('gitlab adminRole event bridge', () => { - it('skips roles outside managed group paths', async () => { - const { service, gitlab } = buildService() - await service.handleAdminRoleUpsert(role('/other')) - expect(gitlab.upsertUser).not.toHaveBeenCalled() - }) - - it('flags admin for the admin group and auditor otherwise', async () => { - const { service, gitlab } = buildService({ adminGroupPath: '/console/admin' }) - await service.handleAdminRoleUpsert(role('/console/admin')) - expect(gitlab.upsertUser).toHaveBeenCalledWith(expect.objectContaining({ admin: true }), expect.anything()) - - await service.handleAdminRoleUpsert(role('/console/readonly')) - expect(gitlab.upsertUser).toHaveBeenLastCalledWith(expect.objectContaining({ auditor: true, admin: undefined }), expect.anything()) - }) - - it('revoke (delete) clears the flags', async () => { - const { service, gitlab } = buildService() - await service.handleAdminRoleDelete(role('/console/admin')) - expect(gitlab.upsertUser).toHaveBeenCalledWith(expect.objectContaining({ admin: false }), expect.anything()) - }) -}) 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 299b431fdc..afe79d6846 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts @@ -7,6 +7,7 @@ import { Test } from '@nestjs/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' import { mockDeep } from 'vitest-mock-extended' import { gitlabConfigFactory } from '../../config/gitlab.config' +import { makeAdminRoleEventMember, makeAdminRoleEventPayload } from '../events/app-events-testing.utils' import { OBSERVABILITY_REPOSITORY } from '../observability/observability.constants' import { VaultClientService } from '../vault/vault-client.service' import { GitlabClientService } from './gitlab-client.service' @@ -734,4 +735,64 @@ describe('gitlabService', () => { expect(gitlab.deleteGroup).not.toHaveBeenCalled() }) }) + + describe('handleAdminRoleUpsert', () => { + it('should skip roles outside managed group paths', async () => { + await service.handleAdminRoleUpsert(makeAdminRoleEventPayload({ oidcGroup: '/other' })) + + expect(gitlab.upsertUser).not.toHaveBeenCalled() + }) + + it('should flag admin for the admin group and auditor otherwise', async () => { + await service.handleAdminRoleUpsert(makeAdminRoleEventPayload({ + oidcGroup: '/console/admin', + members: [makeAdminRoleEventMember({ id: 'u1', email: 'a@b.c' })], + })) + expect(gitlab.upsertUser).toHaveBeenCalledWith(expect.objectContaining({ admin: true }), expect.anything()) + + await service.handleAdminRoleUpsert(makeAdminRoleEventPayload({ + oidcGroup: '/console/readonly', + members: [makeAdminRoleEventMember({ id: 'u1', email: 'a@b.c' })], + })) + expect(gitlab.upsertUser).toHaveBeenLastCalledWith(expect.objectContaining({ auditor: true, admin: undefined }), expect.anything()) + }) + + it('should recognize every configured admin group path', async () => { + vi.mocked(datastore.getAdminPluginConfig).mockResolvedValue('/console/admin,/console/ops') + try { + await service.handleAdminRoleUpsert(makeAdminRoleEventPayload({ + oidcGroup: '/console/ops', + members: [makeAdminRoleEventMember({ id: 'u1', email: 'a@b.c' })], + })) + } finally { + vi.mocked(datastore.getAdminPluginConfig).mockResolvedValue(undefined) + } + + expect(gitlab.upsertUser).toHaveBeenCalledWith(expect.objectContaining({ admin: true }), expect.anything()) + }) + }) + + describe('handleAdminRoleDelete', () => { + it('should clear the admin flag on revoke', async () => { + gitlab.getUserByEmail.mockResolvedValue(makeExpandedUserSchema({ id: 123, username: 'user' })) + + await service.handleAdminRoleDelete(makeAdminRoleEventPayload({ + oidcGroup: '/console/admin', + members: [makeAdminRoleEventMember({ id: 'u1', email: 'a@b.c' })], + })) + + expect(gitlab.upsertUser).toHaveBeenCalledWith(expect.objectContaining({ admin: false }), expect.anything()) + }) + + it('should skip absent GitLab accounts instead of provisioning them', async () => { + gitlab.getUserByEmail.mockResolvedValue(null) + + await service.handleAdminRoleDelete(makeAdminRoleEventPayload({ + oidcGroup: '/console/admin', + members: [makeAdminRoleEventMember({ id: 'u1', email: 'ghost@b.c' })], + })) + + expect(gitlab.upsertUser).not.toHaveBeenCalled() + }) + }) }) diff --git a/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts b/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts index 09f0e12bb8..b9a5e9b86a 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts @@ -101,20 +101,28 @@ export class GitlabService { span?.setAttribute('admin_role.id', role.id) this.logger.log(`Handling an admin role ${enabled ? 'upsert' : 'delete'} event for ${role.id}`) - const adminGroupPath = await this.getAdminGroupPath() + const adminGroupPaths = parseGroupPaths(await this.getAdminGroupPath()) const auditorGroupPaths = parseGroupPaths(await this.getAuditorGroupPath()) - const isAuditorRole = auditorGroupPaths.includes(role.oidcGroup ?? '') - if (role.oidcGroup !== adminGroupPath && !isAuditorRole) { + const oidcGroup = role.oidcGroup ?? '' + const isAdminRole = adminGroupPaths.includes(oidcGroup) + const isAuditorRole = auditorGroupPaths.includes(oidcGroup) + if (!isAdminRole && !isAuditorRole) { this.logger.verbose(`Not a managed role for GitLab plugin (roleId=${role.id})`) return } for (const member of role.members) { + if (!enabled && await this.gitlab.getUserByEmail(member.email) === null) { + // GitLab provisions users via OIDC at first login; a revoke must not + // create the account it is clearing flags on. + this.logger.verbose(`Skipping revoked member without a GitLab account (email=${member.email})`) + continue + } await this.gitlab.upsertUser({ email: member.email, username: generateUsername(member.email), name: generateName(member.firstName, member.lastName), - admin: role.oidcGroup === adminGroupPath ? enabled : undefined, + admin: isAdminRole ? enabled : undefined, auditor: isAuditorRole ? enabled : undefined, }, { cpnUserId: member.id, diff --git a/apps/server-nestjs/src/modules/keycloak/admin-role-bridge.spec.ts b/apps/server-nestjs/src/modules/keycloak/admin-role-bridge.spec.ts deleted file mode 100644 index 690005c75f..0000000000 --- a/apps/server-nestjs/src/modules/keycloak/admin-role-bridge.spec.ts +++ /dev/null @@ -1,56 +0,0 @@ -import type { DeepMockProxy } from 'vitest-mock-extended' -import type { AdminRoleWithDetails, UserWithAdminRoles } from './keycloak-datastore.service' -import { Test } from '@nestjs/testing' -import { beforeEach, describe, expect, it, vi } from 'vitest' -import { mockDeep } from 'vitest-mock-extended' -import { KeycloakClientService } from './keycloak-client.service' -import { KeycloakDatastoreService } from './keycloak-datastore.service' -import { makeGroupRepresentation, makeUserRepresentation } from './keycloak-testing.utils' -import { KeycloakService } from './keycloak.service' - -describe('keycloak adminRole event bridge', () => { - let service: KeycloakService - let keycloak: DeepMockProxy - let datastore: DeepMockProxy - - beforeEach(async () => { - keycloak = mockDeep({ - getOrCreateGroupByPath: vi.fn().mockResolvedValue(makeGroupRepresentation({ id: 'kc-group-id', name: 'admin' })), - getGroupMembers: vi.fn().mockResolvedValue([]), - }) - datastore = mockDeep({}) - - const moduleRef = await Test.createTestingModule({ - providers: [ - KeycloakService, - { provide: KeycloakClientService, useValue: keycloak }, - { provide: KeycloakDatastoreService, useValue: datastore }, - ], - }).compile() - - service = moduleRef.get(KeycloakService) - }) - - it('syncs the impacted role group on adminRole.upsert', async () => { - const roles: AdminRoleWithDetails[] = [{ id: 'role-1', oidcGroup: '/console/admin', type: 'global' }] - const users: UserWithAdminRoles[] = [{ id: 'user-1', adminRoleIds: ['role-1'] }] - datastore.getAllAdminRoles.mockResolvedValue(roles) - datastore.getAllUsersWithAdminRoleIds.mockResolvedValue(users) - keycloak.getGroupMembers.mockResolvedValue([makeUserRepresentation({ id: 'user-2' })]) - - await service.handleAdminRoleUpsert({ id: 'role-1', oidcGroup: '/console/admin', members: [] }) - - expect(keycloak.getOrCreateGroupByPath).toHaveBeenCalledWith('/console/admin') - expect(keycloak.addUserToGroup).toHaveBeenCalledWith('user-1', 'kc-group-id') - expect(keycloak.removeUserFromGroup).toHaveBeenCalledWith('user-2', 'kc-group-id') - }) - - it('warns and no-ops when the role no longer exists', async () => { - datastore.getAllAdminRoles.mockResolvedValue([]) - datastore.getAllUsersWithAdminRoleIds.mockResolvedValue([]) - - await service.handleAdminRoleDelete({ id: 'gone', oidcGroup: '/console/admin', members: [] }) - - expect(keycloak.getOrCreateGroupByPath).not.toHaveBeenCalled() - }) -}) diff --git a/apps/server-nestjs/src/modules/keycloak/keycloak.service.spec.ts b/apps/server-nestjs/src/modules/keycloak/keycloak.service.spec.ts index b557a04ccd..ba147431c6 100644 --- a/apps/server-nestjs/src/modules/keycloak/keycloak.service.spec.ts +++ b/apps/server-nestjs/src/modules/keycloak/keycloak.service.spec.ts @@ -3,6 +3,7 @@ import type { AdminRoleWithDetails, ProjectWithDetails, UserWithAdminRoles } fro import { Test } from '@nestjs/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' import { mockDeep } from 'vitest-mock-extended' +import { makeAdminRoleEventPayload } from '../events/app-events-testing.utils' import { KeycloakClientService } from './keycloak-client.service' import { KeycloakDatastoreService } from './keycloak-datastore.service' import { @@ -526,4 +527,27 @@ describe('keycloakService', () => { expect(keycloak.removeUserFromGroup).toHaveBeenCalledWith('user-2', 'system-managed-id') }) }) + + describe('handleAdminRoleUpsert', () => { + it('should sync the impacted role group on adminRole.upsert', async () => { + datastore.getAllAdminRoles.mockResolvedValue([{ id: 'role-1', oidcGroup: '/console/admin', type: 'global' }]) + datastore.getAllUsersWithAdminRoleIds.mockResolvedValue([{ id: 'user-1', adminRoleIds: ['role-1'] }]) + keycloak.getOrCreateGroupByPath.mockResolvedValue(makeGroupRepresentation({ id: 'kc-group-id', name: 'admin' })) + keycloak.getGroupMembers.mockResolvedValue([makeUserRepresentation({ id: 'user-2' })]) + + await service.handleAdminRoleUpsert(makeAdminRoleEventPayload({ id: 'role-1', oidcGroup: '/console/admin', members: [] })) + + expect(keycloak.getOrCreateGroupByPath).toHaveBeenCalledWith('/console/admin') + expect(keycloak.addUserToGroup).toHaveBeenCalledWith('user-1', 'kc-group-id') + expect(keycloak.removeUserFromGroup).toHaveBeenCalledWith('user-2', 'kc-group-id') + }) + }) + + describe('handleAdminRoleDelete', () => { + it('should warn and no-op when the role no longer exists', async () => { + await service.handleAdminRoleDelete(makeAdminRoleEventPayload({ id: 'gone', oidcGroup: '/console/admin', members: [] })) + + expect(keycloak.getOrCreateGroupByPath).not.toHaveBeenCalled() + }) + }) }) From 9f4be277e37f457d9fd857298edf73d2ad5eb17a Mon Sep 17 00:00:00 2001 From: William Phetsinorath Date: Thu, 8 Oct 2026 11:52:39 +0200 Subject: [PATCH 3/3] refactor(server-nestjs): sync admin flags on revoke Co-authored-by: Automata Signed-off-by: William Phetsinorath Change-Id: I3ac3c5df9f778e623f92e8383eb4ec906a6a6964 --- .../src/modules/gitlab/gitlab.service.spec.ts | 8 ++------ .../src/modules/gitlab/gitlab.service.ts | 20 +++++++++---------- 2 files changed, 11 insertions(+), 17 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 afe79d6846..6b40077400 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab.service.spec.ts @@ -774,8 +774,6 @@ describe('gitlabService', () => { describe('handleAdminRoleDelete', () => { it('should clear the admin flag on revoke', async () => { - gitlab.getUserByEmail.mockResolvedValue(makeExpandedUserSchema({ id: 123, username: 'user' })) - await service.handleAdminRoleDelete(makeAdminRoleEventPayload({ oidcGroup: '/console/admin', members: [makeAdminRoleEventMember({ id: 'u1', email: 'a@b.c' })], @@ -784,15 +782,13 @@ describe('gitlabService', () => { expect(gitlab.upsertUser).toHaveBeenCalledWith(expect.objectContaining({ admin: false }), expect.anything()) }) - it('should skip absent GitLab accounts instead of provisioning them', async () => { - gitlab.getUserByEmail.mockResolvedValue(null) - + it('should provision absent GitLab members on revoke', async () => { await service.handleAdminRoleDelete(makeAdminRoleEventPayload({ oidcGroup: '/console/admin', members: [makeAdminRoleEventMember({ id: 'u1', email: 'ghost@b.c' })], })) - expect(gitlab.upsertUser).not.toHaveBeenCalled() + expect(gitlab.upsertUser).toHaveBeenCalledWith(expect.objectContaining({ admin: false }), expect.anything()) }) }) }) diff --git a/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts b/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts index b9a5e9b86a..905efa4d6c 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab.service.ts @@ -101,23 +101,13 @@ export class GitlabService { span?.setAttribute('admin_role.id', role.id) this.logger.log(`Handling an admin role ${enabled ? 'upsert' : 'delete'} event for ${role.id}`) - const adminGroupPaths = parseGroupPaths(await this.getAdminGroupPath()) - const auditorGroupPaths = parseGroupPaths(await this.getAuditorGroupPath()) - const oidcGroup = role.oidcGroup ?? '' - const isAdminRole = adminGroupPaths.includes(oidcGroup) - const isAuditorRole = auditorGroupPaths.includes(oidcGroup) + const { isAdminRole, isAuditorRole } = await this.getAdminRoleFlags(role) if (!isAdminRole && !isAuditorRole) { this.logger.verbose(`Not a managed role for GitLab plugin (roleId=${role.id})`) return } for (const member of role.members) { - if (!enabled && await this.gitlab.getUserByEmail(member.email) === null) { - // GitLab provisions users via OIDC at first login; a revoke must not - // create the account it is clearing flags on. - this.logger.verbose(`Skipping revoked member without a GitLab account (email=${member.email})`) - continue - } await this.gitlab.upsertUser({ email: member.email, username: generateUsername(member.email), @@ -130,6 +120,14 @@ export class GitlabService { } } + private async getAdminRoleFlags(role: AdminRoleEventPayload) { + const oidcGroup = role.oidcGroup ?? '' + return { + isAdminRole: parseGroupPaths(await this.getAdminGroupPath()).includes(oidcGroup), + isAuditorRole: parseGroupPaths(await this.getAuditorGroupPath()).includes(oidcGroup), + } + } + @OnEvent('repository.sync') async handleRepositorySync(payload: RepositorySyncEventPayload): Promise> { return capturePluginResult('gitlab', () => this.syncRepositoryMirror(payload))