CreateTeamDto.createdById is taken from the request body, so a caller can create a team owned by someone else

Abierto
  • ¿Algo no está bien?
#742 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
75/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
typescript
Área
api, backend, security

Línea de trabajo

Start with TeamsService.create in src/teams/teams.service.ts, then read CreateTeamDto and the controller in the paths named in the issue. Check the existing team service specs and how member IDs are validated. Done means a request cannot choose an arbitrary team owner, and a service spec covers a body-supplied createdById that differs from the caller.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

bug security

Location — src/teams/teams.service.ts:38-43 (TeamsService.create), DTO field src/teams/dto/create-team.dto.ts:49-52 (CreateTeamDto.createdById), controller src/teams/teams.controller.ts:43.

Problem — The authenticated caller's id is used only as a fallback for a client-supplied value:

const team = await this.teamRepo.save(
  this.teamRepo.create({
    name: dto.name?.trim() ?? dto.name,
    // Fall back to the authenticated caller's id when the client omits createdById.
    createdById: dto.createdById ?? callerUserId,
  }),
);

CreateTeamDto.createdById is validated as @IsOptional() @IsUUID() and nothing else. There is no check that it equals user.userId from @CurrentUser(), and no check that the referenced user even exists (the existence loop above only validates members[].userId, not createdById). So any caller holding SPONSOR or MAINTAINER can set createdById to an arbitrary UUID.

Why it matters — Team.createdById is the only ownership field on the entity. Issue #357's fix made it the fallback owner precisely so a team cannot be created with no owner, and it is the field any ownership check on PATCH /teams/:id/splits or POST /teams/:id/assign/:bountyId must key on. Letting the client choose it means:

  • The audit trail is wrong: a team whose splits pay out real money is attributed to a user who never created it, and the real creator leaves no record anywhere.
  • It pre-breaks the ownership checks this field exists to support. Once a check of the form team.createdById === callerId is added, a caller can still hand ownership to an account they control, or — in the other direction — plant a team attributed to a victim so the victim appears responsible for its payout terms.
  • Passing an unknown UUID is also accepted; because Team.createdBy is onDelete: 'SET NULL' / nullable: true, there is no FK rejection path that reliably catches it at insert time for an id that does not exist, so the team can end up with a dangling owner id (the condition #357 set out to prevent).

Reproduction / failure path

  1. Authenticate as user A (SPONSOR). Note A's id.
  2. POST /teams with body {"name":"t","createdById":"<user B's uuid>","members":[{"userId":"<A>","percentage":100}]}.
  3. Returns 201 with createdById set to B. Nothing links the team to A.
  4. Repeat with a random UUID that belongs to no user — also accepted.

Suggested fix

  • Remove createdById from CreateTeamDto entirely and set createdById: callerUserId unconditionally in the service. The caller id is already available and is the only trustworthy source.
  • If an admin-on-behalf-of flow is genuinely needed, keep the field but accept it only when the caller holds UserRole.MAINTAINER, and validate the referenced user exists with the same batched existence check used for members; otherwise reject with ForbiddenException.
  • Add a service spec asserting that a body-supplied createdById different from the caller's id does not end up on the saved entity.
Lenguaje dominante
TypeScript
Estrellas
2
Forks
90
Merge medio
31 min
PR fusionados (30 d)
20

Preparar el entorno

  • Incluye un Dockerfile o un archivo de Docker Compose
  • Sin plantilla de pull request
  • Sin guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de MergeFi/backend

Todos los issues de MergeFi/backend

Issues similares

Más issues de TypeScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.