Skip to content

Bug: CreateGroup via PAT creates an orphan group row #1645

Description

@AmanGIT07

Summary

When CreateGroup is called by a PAT-authenticated principal, the Postgres groups row is written but the SpiceDB hierarchy links and owner policy are never wired. The row is left as an orphan.

Reproduction

  1. Authenticate as a PAT scoped to an org with the create-group permission.
  2. Call CreateGroup.
  3. Observe: response carries an error; a row exists in groups for the requested name; SpiceDB has no group#org / org#member@group relations and no owner policy in policies.

Root cause

  1. core/group/service.go:72-74s.repository.Create(ctx, grp) writes the Postgres row first.
  2. core/group/service.go:77s.membershipService.OnGroupCreated(ctx, newGroup.ID, newGroup.OrganizationID, principal.ID, principal.Type) passes the raw principal. For PAT auth this is (<pat-id>, "app/pat").
  3. core/membership/service.go:1302OnGroupCreated calls SetGroupMemberRole(..., creatorID, creatorType, schema.GroupOwnerRole).
  4. core/membership/service.go:1402-1420validateGroupPrincipal switches on principal type and accepts only app/user; everything else returns ErrInvalidPrincipalType.
  5. The error propagates back to the group Create caller, but repository.Create is not rolled back.

Impact

  • Orphan rows accumulate in groups on each failed PAT-driven CreateGroup call.
  • The orphan has no hierarchy in SpiceDB, no owner policy in Postgres, and is unreachable through normal listing (membership-based listing won't surface it either, since there's no policy row).

Suggested fix

Resolve the PAT to its underlying user before invoking OnGroupCreated, mirroring the pattern in CreateProject at core/project/service.go:113:

principal, err := s.authnService.GetPrincipal(ctx)
if err != nil {
    return Group{}, fmt.Errorf("%w: %s", authenticate.ErrInvalidID, err.Error())
}
...
subjectID, subjectType := principal.ResolveSubject()  // PAT → user
if err = s.membershipService.OnGroupCreated(ctx, newGroup.ID, newGroup.OrganizationID, subjectID, subjectType); err != nil {
    return Group{}, err
}

Two-line change. Independent of the ListByUserList migration in #1643 (this bug pre-dates that work).

Cleanup

Existing orphan rows should be identified and either backfilled with hierarchy + owner or deleted. Candidate query: groups whose id has no matching row in policies with resource_type = 'app/group'.

How surfaced

Found while testing the group listing migration in #1643 with PAT-authenticated calls. The listing bug there was fixed in 42a5c50; this write-side bug is independent.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions