Skip to content

Commit 940d0fa

Browse files
feat(membership): add group membership management
Introduces the SetGroupMemberRole RPC and three service methods on the membership package: AddGroupMember, SetGroupMemberRole, and OnGroupCreated. These manage policy + SpiceDB relation atomically and keep them in sync, fixing the leaky-relation pattern at the group layer. - AddGroupMember validates org membership of the principal and rejects duplicates with ErrAlreadyMember (service-only, no proto). - SetGroupMemberRole rejects non-members with ErrNotMember and enforces a min-owner constraint (ErrLastGroupOwnerRole) on demotion. - OnGroupCreated bundles the group<->org hierarchy relations with the initial owner add, so group.Create can wire SpiceDB with one call. - Principal validation is restricted to app/user; the switch is kept extensible for future principal types. Audit events are added for both the added and role-changed cases. No call sites are migrated yet — group.Create, AddGroupUsers, and the deletion of legacy group service methods will follow in subsequent PRs. PROTON_COMMIT is temporarily pinned to the feature-branch SHA on raystack/proton#485; it will be re-pinned to the merge commit once that PR lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f4d3398 commit 940d0fa

10 files changed

Lines changed: 1060 additions & 4 deletions

File tree

core/audit/audit.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,12 @@ const (
5555
ServiceUserCreatedEvent EventName = "app.serviceuser.created"
5656
ServiceUserDeletedEvent EventName = "app.serviceuser.deleted"
5757

58-
GroupCreatedEvent EventName = "app.group.created"
59-
GroupUpdatedEvent EventName = "app.group.updated"
60-
GroupDeletedEvent EventName = "app.group.deleted"
61-
GroupMemberRemovedEvent EventName = "app.group.members.removed"
58+
GroupCreatedEvent EventName = "app.group.created"
59+
GroupUpdatedEvent EventName = "app.group.updated"
60+
GroupDeletedEvent EventName = "app.group.deleted"
61+
GroupMemberCreatedEvent EventName = "app.group.member.created"
62+
GroupMemberRoleChangedEvent EventName = "app.group.member.role_changed"
63+
GroupMemberRemovedEvent EventName = "app.group.members.removed"
6264

6365
RoleCreatedEvent EventName = "app.role.created"
6466
RoleUpdatedEvent EventName = "app.role.updated"

core/membership/errors.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,6 @@ var (
1313
ErrNotOrgMember = errors.New("principal is not a member of the organization")
1414
ErrInvalidProjectRole = errors.New("role is not valid for project scope")
1515
ErrInvalidResourceType = errors.New("unsupported resource type")
16+
ErrInvalidGroupRole = errors.New("role is not valid for group scope")
17+
ErrLastGroupOwnerRole = errors.New("cannot change role: this is the last owner of the group")
1618
)

core/membership/service.go

Lines changed: 314 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,3 +1019,317 @@ func (s *Service) ListPrincipalsByResource(ctx context.Context, resourceID, reso
10191019

10201020
return members, nil
10211021
}
1022+
1023+
// AddGroupMember adds a principal as a member of a group with an explicit role.
1024+
// Returns ErrAlreadyMember if the principal already has a policy on this group.
1025+
// The principal must be a member of the group's parent organization.
1026+
func (s *Service) AddGroupMember(ctx context.Context, groupID, principalID, principalType, roleID string) error {
1027+
grp, err := s.groupService.Get(ctx, groupID)
1028+
if err != nil {
1029+
return err
1030+
}
1031+
1032+
principal, err := s.validateGroupPrincipal(ctx, principalID, principalType)
1033+
if err != nil {
1034+
return err
1035+
}
1036+
1037+
fetchedRole, err := s.validateGroupRole(ctx, roleID, grp.OrganizationID)
1038+
if err != nil {
1039+
return err
1040+
}
1041+
1042+
if err := s.validateOrgMembership(ctx, grp.OrganizationID, principalID, principalType); err != nil {
1043+
return err
1044+
}
1045+
1046+
existing, err := s.policyService.List(ctx, policy.Filter{
1047+
GroupID: groupID,
1048+
PrincipalID: principalID,
1049+
PrincipalType: principalType,
1050+
})
1051+
if err != nil {
1052+
return fmt.Errorf("list existing policies: %w", err)
1053+
}
1054+
if len(existing) > 0 {
1055+
return ErrAlreadyMember
1056+
}
1057+
1058+
createdPolicy, err := s.createPolicy(ctx, groupID, schema.GroupNamespace, principalID, principalType, fetchedRole.ID)
1059+
if err != nil {
1060+
return err
1061+
}
1062+
1063+
relationName := groupRoleToRelation(fetchedRole)
1064+
if err := s.createRelation(ctx, groupID, schema.GroupNamespace, principalID, principalType, relationName); err != nil {
1065+
if deleteErr := s.policyService.Delete(ctx, createdPolicy.ID); deleteErr != nil {
1066+
s.log.WarnContext(ctx, "orphaned policy: relation creation failed and policy cleanup also failed",
1067+
"policy_id", createdPolicy.ID,
1068+
"group_id", groupID,
1069+
"principal_id", principalID,
1070+
"policy_delete_error", deleteErr,
1071+
)
1072+
}
1073+
return err
1074+
}
1075+
1076+
s.auditGroupMemberAdded(ctx, grp, principal, fetchedRole.ID)
1077+
return nil
1078+
}
1079+
1080+
// SetGroupMemberRole changes an existing member's role in a group.
1081+
// Returns ErrNotMember if the principal has no existing policy on the group.
1082+
// Enforces the min-owner constraint: demoting the last owner returns ErrLastGroupOwnerRole.
1083+
func (s *Service) SetGroupMemberRole(ctx context.Context, groupID, principalID, principalType, roleID string) error {
1084+
grp, err := s.groupService.Get(ctx, groupID)
1085+
if err != nil {
1086+
return err
1087+
}
1088+
1089+
principal, err := s.validateGroupPrincipal(ctx, principalID, principalType)
1090+
if err != nil {
1091+
return err
1092+
}
1093+
1094+
fetchedRole, err := s.validateGroupRole(ctx, roleID, grp.OrganizationID)
1095+
if err != nil {
1096+
return err
1097+
}
1098+
resolvedRoleID := fetchedRole.ID
1099+
1100+
existing, err := s.policyService.List(ctx, policy.Filter{
1101+
GroupID: groupID,
1102+
PrincipalID: principalID,
1103+
PrincipalType: principalType,
1104+
})
1105+
if err != nil {
1106+
return fmt.Errorf("list existing policies: %w", err)
1107+
}
1108+
if len(existing) == 0 {
1109+
return ErrNotMember
1110+
}
1111+
1112+
// skip if the user already has exactly this role
1113+
if len(existing) == 1 && existing[0].RoleID == resolvedRoleID {
1114+
return nil
1115+
}
1116+
1117+
if err := s.validateMinGroupOwnerConstraint(ctx, groupID, resolvedRoleID, existing); err != nil {
1118+
return err
1119+
}
1120+
1121+
if err := s.replacePolicy(ctx, groupID, schema.GroupNamespace, principalID, principalType, resolvedRoleID, existing); err != nil {
1122+
return err
1123+
}
1124+
1125+
newRelation := groupRoleToRelation(fetchedRole)
1126+
oldRelations := []string{schema.OwnerRelationName, schema.MemberRelationName}
1127+
if err := s.replaceRelation(ctx, groupID, schema.GroupNamespace, principalID, principalType, oldRelations, newRelation); err != nil {
1128+
s.log.ErrorContext(ctx, "membership state inconsistent: policy replaced but group relation update failed, needs manual fix",
1129+
"group_id", groupID,
1130+
"principal_id", principalID,
1131+
"principal_type", principalType,
1132+
"new_role_id", resolvedRoleID,
1133+
"expected_relation", newRelation,
1134+
"error", err,
1135+
)
1136+
return err
1137+
}
1138+
1139+
s.auditGroupMemberRoleChanged(ctx, grp, principal, resolvedRoleID)
1140+
return nil
1141+
}
1142+
1143+
// OnGroupCreated wires up SpiceDB relations for a newly-created group:
1144+
// links the group to its parent organization (both directions) and adds the
1145+
// creator as owner via AddGroupMember.
1146+
func (s *Service) OnGroupCreated(ctx context.Context, groupID, orgID, creatorID, creatorType string) error {
1147+
if err := s.linkGroupToOrg(ctx, groupID, orgID); err != nil {
1148+
return err
1149+
}
1150+
if err := s.AddGroupMember(ctx, groupID, creatorID, creatorType, schema.GroupOwnerRole); err != nil {
1151+
return err
1152+
}
1153+
return nil
1154+
}
1155+
1156+
// linkGroupToOrg creates the two hierarchy relations between a group and its org:
1157+
// - group#org@organization (identity link from group to org)
1158+
// - organization#member@group#member (lets org#member traverse to group members)
1159+
func (s *Service) linkGroupToOrg(ctx context.Context, groupID, orgID string) error {
1160+
if _, err := s.relationService.Create(ctx, relation.Relation{
1161+
Object: relation.Object{ID: groupID, Namespace: schema.GroupNamespace},
1162+
Subject: relation.Subject{ID: orgID, Namespace: schema.OrganizationNamespace},
1163+
RelationName: schema.OrganizationRelationName,
1164+
}); err != nil {
1165+
return fmt.Errorf("link group to org: %w", err)
1166+
}
1167+
1168+
if _, err := s.relationService.Create(ctx, relation.Relation{
1169+
Object: relation.Object{ID: orgID, Namespace: schema.OrganizationNamespace},
1170+
Subject: relation.Subject{
1171+
ID: groupID,
1172+
Namespace: schema.GroupNamespace,
1173+
SubRelationName: schema.MemberRelationName,
1174+
},
1175+
RelationName: schema.MemberRelationName,
1176+
}); err != nil {
1177+
return fmt.Errorf("add group as org member: %w", err)
1178+
}
1179+
1180+
return nil
1181+
}
1182+
1183+
// validateGroupRole checks that the role is valid for group scope:
1184+
// - a platform-wide role scoped to groups, or
1185+
// - a custom role created for the group's parent organization.
1186+
func (s *Service) validateGroupRole(ctx context.Context, roleID, orgID string) (role.Role, error) {
1187+
fetchedRole, err := s.roleService.Get(ctx, roleID)
1188+
if err != nil {
1189+
return role.Role{}, err
1190+
}
1191+
if !slices.Contains(fetchedRole.Scopes, schema.GroupNamespace) {
1192+
return role.Role{}, ErrInvalidGroupRole
1193+
}
1194+
if fetchedRole.OrgID == orgID {
1195+
return fetchedRole, nil
1196+
}
1197+
if utils.IsNullUUID(fetchedRole.OrgID) {
1198+
return fetchedRole, nil
1199+
}
1200+
return role.Role{}, ErrInvalidGroupRole
1201+
}
1202+
1203+
// validateGroupPrincipal fetches and validates the principal for group operations.
1204+
// Currently only app/user is supported; the switch is structured so future principal
1205+
// types (e.g. serviceuser) can be enabled here without touching call sites.
1206+
func (s *Service) validateGroupPrincipal(ctx context.Context, principalID, principalType string) (principalInfo, error) {
1207+
switch principalType {
1208+
case schema.UserPrincipal:
1209+
usr, err := s.userService.GetByID(ctx, principalID)
1210+
if err != nil {
1211+
return principalInfo{}, err
1212+
}
1213+
if usr.State == user.Disabled {
1214+
return principalInfo{}, user.ErrDisabled
1215+
}
1216+
return principalInfo{
1217+
ID: usr.ID,
1218+
Type: schema.UserPrincipal,
1219+
Name: usr.Title,
1220+
Email: usr.Email,
1221+
}, nil
1222+
default:
1223+
return principalInfo{}, ErrInvalidPrincipalType
1224+
}
1225+
}
1226+
1227+
// validateMinGroupOwnerConstraint ensures the group keeps at least one owner
1228+
// after the role change. Mirrors the org-level constraint.
1229+
func (s *Service) validateMinGroupOwnerConstraint(ctx context.Context, groupID, newRoleID string, existing []policy.Policy) error {
1230+
ownerRole, err := s.roleService.Get(ctx, schema.GroupOwnerRole)
1231+
if err != nil {
1232+
return fmt.Errorf("get group owner role: %w", err)
1233+
}
1234+
1235+
if newRoleID == ownerRole.ID {
1236+
return nil
1237+
}
1238+
1239+
isCurrentlyOwner := false
1240+
for _, p := range existing {
1241+
if p.RoleID == ownerRole.ID {
1242+
isCurrentlyOwner = true
1243+
break
1244+
}
1245+
}
1246+
if !isCurrentlyOwner {
1247+
return nil
1248+
}
1249+
1250+
ownerPolicies, err := s.policyService.List(ctx, policy.Filter{
1251+
GroupID: groupID,
1252+
RoleID: ownerRole.ID,
1253+
})
1254+
if err != nil {
1255+
return fmt.Errorf("list group owner policies: %w", err)
1256+
}
1257+
if len(ownerPolicies) <= 1 {
1258+
return ErrLastGroupOwnerRole
1259+
}
1260+
return nil
1261+
}
1262+
1263+
// groupRoleToRelation maps a group role to the matching SpiceDB relation name.
1264+
func groupRoleToRelation(r role.Role) string {
1265+
if r.Name == schema.GroupOwnerRole {
1266+
return schema.OwnerRelationName
1267+
}
1268+
return schema.MemberRelationName
1269+
}
1270+
1271+
func (s *Service) auditGroupMemberAdded(ctx context.Context, grp group.Group, p principalInfo, roleID string) {
1272+
targetType, _ := principalTypeToAuditType(p.Type)
1273+
meta := map[string]any{"role_id": roleID}
1274+
if p.Email != "" {
1275+
meta["email"] = p.Email
1276+
}
1277+
1278+
s.auditRecordRepository.Create(ctx, auditrecord.AuditRecord{
1279+
Event: pkgAuditRecord.GroupMemberAddedEvent,
1280+
Resource: auditrecord.Resource{
1281+
ID: grp.ID,
1282+
Type: pkgAuditRecord.GroupType,
1283+
Name: grp.Title,
1284+
},
1285+
Target: &auditrecord.Target{
1286+
ID: p.ID,
1287+
Type: targetType,
1288+
Name: p.Name,
1289+
Metadata: meta,
1290+
},
1291+
OrgID: grp.OrganizationID,
1292+
OccurredAt: time.Now(),
1293+
})
1294+
1295+
audit.GetAuditor(ctx, grp.OrganizationID).LogWithAttrs(audit.GroupMemberCreatedEvent, audit.Target{
1296+
ID: p.ID,
1297+
Type: p.Type,
1298+
}, map[string]string{
1299+
"role_id": roleID,
1300+
"group_id": grp.ID,
1301+
})
1302+
}
1303+
1304+
func (s *Service) auditGroupMemberRoleChanged(ctx context.Context, grp group.Group, p principalInfo, roleID string) {
1305+
targetType, _ := principalTypeToAuditType(p.Type)
1306+
meta := map[string]any{"role_id": roleID}
1307+
if p.Email != "" {
1308+
meta["email"] = p.Email
1309+
}
1310+
1311+
s.auditRecordRepository.Create(ctx, auditrecord.AuditRecord{
1312+
Event: pkgAuditRecord.GroupMemberRoleChangedEvent,
1313+
Resource: auditrecord.Resource{
1314+
ID: grp.ID,
1315+
Type: pkgAuditRecord.GroupType,
1316+
Name: grp.Title,
1317+
},
1318+
Target: &auditrecord.Target{
1319+
ID: p.ID,
1320+
Type: targetType,
1321+
Name: p.Name,
1322+
Metadata: meta,
1323+
},
1324+
OrgID: grp.OrganizationID,
1325+
OccurredAt: time.Now(),
1326+
})
1327+
1328+
audit.GetAuditor(ctx, grp.OrganizationID).LogWithAttrs(audit.GroupMemberRoleChangedEvent, audit.Target{
1329+
ID: p.ID,
1330+
Type: p.Type,
1331+
}, map[string]string{
1332+
"role_id": roleID,
1333+
"group_id": grp.ID,
1334+
})
1335+
}

0 commit comments

Comments
 (0)