diff --git a/apps/server-nestjs/src/modules/keycloak/keycloak-client.service.ts b/apps/server-nestjs/src/modules/keycloak/keycloak-client.service.ts index 22e42eaf09..ed0cc872a0 100644 --- a/apps/server-nestjs/src/modules/keycloak/keycloak-client.service.ts +++ b/apps/server-nestjs/src/modules/keycloak/keycloak-client.service.ts @@ -12,7 +12,7 @@ import { keycloakConfigFactory } from '../../config/keycloak.config' import { getErrorResponseStatus } from '../../utils/http.utils' import { StartActiveSpan } from '../infrastructure/telemetry/telemetry.decorator' import { ADMIN_AUTH_REALM, ADMIN_TOKEN_REFRESH_INTERVAL_MS, CONSOLE_GROUP_NAME, PASSWORD_GRANT_TYPE, REFRESH_TOKEN_GRANT_TYPE, SUBGROUPS_PAGINATE_QUERY_MAX } from './keycloak.constants' -import { groupSchema, splitGroupPath } from './keycloak.utils' +import { ensure, groupSchema, splitGroupPath } from './keycloak.utils' export const KEYCLOAK_ADMIN_CLIENT = Symbol('KEYCLOAK_ADMIN_CLIENT') @@ -129,21 +129,15 @@ export class KeycloakClientService implements OnModuleInit { const span = trace.getActiveSpan() span?.setAttribute('group.name', name) this.logger.debug(`Creating Keycloak group ${name}`) - try { - await this.client.groups.create({ name }) - } catch (err) { - // A concurrent reconciliation (e.g. the cron sync and a project upsert) - // may have created the root group between the read and the create; treat - // the 409 as "already exists" and re-fetch it. - if (getErrorResponseStatus(err) !== 409) throw err - this.logger.verbose(`Keycloak group ${name} was created concurrently, fetching it`) - const existing = await this.getRootGroupByName(name) - if (!existing) throw err - return existing - } - const created = await this.getRootGroupByName(name) - if (!created) throw new Error(`Created Keycloak group ${name} but could not fetch it back`) - return created + const group = await ensure({ + create: async () => { + await this.client.groups.create({ name }) + }, + reload: () => this.getRootGroupByName(name), + onCollision: () => this.logger.verbose(`Keycloak group ${name} was created concurrently, fetching it`), + }) + if (!group) throw new Error(`Keycloak group "${name}" could not be fetched back after creation`) + return group } async addUserToGroup(userId: string, groupId: string) { @@ -213,22 +207,15 @@ export class KeycloakClientService implements OnModuleInit { if (existing) return existing this.logger.debug(`Creating Keycloak subgroup ${name} under parentId=${parentId}`) - try { - // createChildGroup only returns the new id; re-list the parent to hand - // back the representation Keycloak computed (name, path, subGroups...) - await this.client.groups.createChildGroup({ id: parentId }, { name }) - const created = await this.getSubGroupByName(parentId, name) - if (!created) throw new Error(`Created Keycloak subgroup ${name} under parentId=${parentId} but could not fetch it back`) - return created - } catch (err) { - // A concurrent reconciliation may have created the subgroup between the - // scan and the create; treat the 409 as "already exists" and re-fetch it - if (getErrorResponseStatus(err) !== 409) throw err - this.logger.verbose(`Keycloak subgroup ${name} was created concurrently under parentId=${parentId}, fetching it`) - const subgroup = await this.getSubGroupByName(parentId, name) - if (!subgroup) throw err - return subgroup - } + const group = await ensure({ + create: async () => { + await this.client.groups.createChildGroup({ id: parentId }, { name }) + }, + reload: () => this.getSubGroupByName(parentId, name), + onCollision: () => this.logger.verbose(`Keycloak subgroup ${name} was created concurrently under parentId=${parentId}, fetching it`), + }) + if (!group) throw new Error(`Keycloak subgroup "${name}" under parentId=${parentId} could not be fetched back after creation`) + return group } async getOrCreateConsoleGroup(projectGroup: GroupRepresentationWith<'id'>) { diff --git a/apps/server-nestjs/src/modules/keycloak/keycloak.utils.ts b/apps/server-nestjs/src/modules/keycloak/keycloak.utils.ts index fb61ce9893..3b26aadca7 100644 --- a/apps/server-nestjs/src/modules/keycloak/keycloak.utils.ts +++ b/apps/server-nestjs/src/modules/keycloak/keycloak.utils.ts @@ -1,7 +1,9 @@ import type GroupRepresentation from '@keycloak/keycloak-admin-client/lib/defs/groupRepresentation' import type UserRepresentation from '@keycloak/keycloak-admin-client/lib/defs/userRepresentation' import type { ProjectWithDetails } from './keycloak-datastore.service' +import { HttpStatus } from '@nestjs/common' import z from 'zod' +import { getErrorResponseStatus } from '../../utils/http.utils' import { CONSOLE_GROUP_NAME } from './keycloak.constants' type With = T & Required> @@ -57,3 +59,37 @@ export function isAdminRole( ): boolean { return role.oidcGroup === `/${project.slug}/${CONSOLE_GROUP_NAME}/admin` } + +// Whether a Keycloak admin-client error signals an entity already existing +// (race collision): a 409 conflict on the create call. +export function isKeycloakConflict(error: unknown): boolean { + return getErrorResponseStatus(error) === HttpStatus.CONFLICT +} + +// Runs an idempotent create: `create` only performs the write; the entity is +// always read back through `reload` so callers get the representation Keycloak +// computed, never a locally synthesized one. On a race collision (409), +// `onCollision` is invoked once and `reload` fetches the concurrently created +// entity; if `reload` still finds nothing there, the original error is +// rethrown so genuine failures are not swallowed. Returns undefined if +// `reload` does not find the entity after the write. +export async function ensure({ + create, + reload, + onCollision, +}: { + create: () => Promise + reload: () => Promise + onCollision?: (error: unknown) => void +}): Promise { + try { + await create() + } catch (error) { + if (!isKeycloakConflict(error)) throw error + onCollision?.(error) + const existing = await reload() + if (!existing) throw error + return existing + } + return reload() +}