refactor(membership): deduplicate repeated patterns

Behavior unchanged; call sites and public signatures stay the same.

- One resourcePolicyFilter builds resource-scoped policy filters;
  the duplicate policyFilterForResource is gone. removeAllPolicies now
  goes through it and reuses removePoliciesByFilter, so its filter also
  pins resource_type (redundant with the per-type ID fields in SQL).
- validateOrgRole/validateProjectRole/validateGroupRole are one-line
  wrappers over a shared validateRoleForScope.
- validateMinOwnerConstraint/validateMinGroupOwnerConstraint are
  wrappers over a shared validateMinRoleConstraint.
- The five near-identical org/group audit helpers delegate to one
  auditMemberChange that writes both audit stores.
- hasExactlyRole names the repeated "already has exactly this role"
  check.
- principalKey struct replaces the \x00-joined string map keys, and
  ListPrincipalsByResource's role enrichment moves into its own
  function.
- listOrgsForPrincipal/listGroupsForPrincipal reuse policyResourceIDs
  instead of hand-rolled pluck loops.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: whoAbhishekSah

core/membership/audit.go

@@ -49,82 +49,78 @@ func (s *Service) createAuditRecord(ctx context.Context, record auditrecord.Audi
 	}
 }
 
-func (s *Service) auditOrgMemberRoleChanged(ctx context.Context, org organization.Organization, p principalInfo, roleID string) {
+// auditMemberChange writes a membership change to both audit stores: an audit
+// record against the resource and a legacy auditor log with the given attrs.
+// The role ID and the principal's email go into the record's target metadata
+// when present.
+func (s *Service) auditMemberChange(ctx context.Context, event pkgAuditRecord.Event, legacyEvent audit.EventName, res auditrecord.Resource, orgID string, p principalInfo, roleID string, legacyAttrs map[string]string) {
 	targetType, _ := principalTypeToAuditType(p.Type)
-	meta := map[string]any{"role_id": roleID}
+	meta := map[string]any{}
+	if roleID != "" {
+		meta["role_id"] = roleID
+	}
 	if p.Email != "" {
 		meta["email"] = p.Email
 	}
 
 	s.createAuditRecord(ctx, auditrecord.AuditRecord{
-		Event: pkgAuditRecord.OrganizationMemberRoleChangedEvent,
-		Resource: auditrecord.Resource{
-			ID:   org.ID,
-			Type: pkgAuditRecord.OrganizationType,
-			Name: org.Title,
-		},
+		Event:    event,
+		Resource: res,
 		Target: &auditrecord.Target{
 			ID:       p.ID,
 			Type:     targetType,
 			Name:     p.Name,
 			Metadata: meta,
 		},
-		OrgID:      org.ID,
+		OrgID:      orgID,
 		OccurredAt: time.Now(),
 	})
 
-	if err := audit.GetAuditor(ctx, org.ID).LogWithAttrs(audit.OrgMemberRoleChangedEvent, audit.Target{
+	if err := audit.GetAuditor(ctx, orgID).LogWithAttrs(legacyEvent, audit.Target{
 		ID:   p.ID,
 		Type: p.Type,
-	}, map[string]string{
-		"role_id": roleID,
-	}); err != nil {
-		s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.OrgMemberRoleChangedEvent)
+	}, legacyAttrs); err != nil {
+		s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", legacyEvent)
 	}
 }
 
+func orgAuditResource(org organization.Organization) auditrecord.Resource {
+	return auditrecord.Resource{ID: org.ID, Type: pkgAuditRecord.OrganizationType, Name: org.Title}
+}
+
+func groupAuditResource(grp group.Group) auditrecord.Resource {
+	return auditrecord.Resource{ID: grp.ID, Type: pkgAuditRecord.GroupType, Name: grp.Title}
+}
+
 func (s *Service) auditOrgMemberAdded(ctx context.Context, org organization.Organization, p principalInfo, roleID string) {
-	targetType, _ := principalTypeToAuditType(p.Type)
-	meta := map[string]any{"role_id": roleID}
-	if p.Email != "" {
-		meta["email"] = p.Email
-	}
+	s.auditMemberChange(ctx, pkgAuditRecord.OrganizationMemberAddedEvent, audit.OrgMemberCreatedEvent,
+		orgAuditResource(org), org.ID, p, roleID, map[string]string{"role_id": roleID})
+}
 
-	s.createAuditRecord(ctx, auditrecord.AuditRecord{
-		Event: pkgAuditRecord.OrganizationMemberAddedEvent,
-		Resource: auditrecord.Resource{
-			ID:   org.ID,
-			Type: pkgAuditRecord.OrganizationType,
-			Name: org.Title,
-		},
-		Target: &auditrecord.Target{
-			ID:       p.ID,
-			Type:     targetType,
-			Name:     p.Name,
-			Metadata: meta,
-		},
-		OrgID:      org.ID,
-		OccurredAt: time.Now(),
-	})
+func (s *Service) auditOrgMemberRoleChanged(ctx context.Context, org organization.Organization, p principalInfo, roleID string) {
+	s.auditMemberChange(ctx, pkgAuditRecord.OrganizationMemberRoleChangedEvent, audit.OrgMemberRoleChangedEvent,
+		orgAuditResource(org), org.ID, p, roleID, map[string]string{"role_id": roleID})
+}
 
-	if err := audit.GetAuditor(ctx, org.ID).LogWithAttrs(audit.OrgMemberCreatedEvent, audit.Target{
-		ID:   p.ID,
-		Type: p.Type,
-	}, map[string]string{
-		"role_id": roleID,
-	}); err != nil {
-		s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.OrgMemberCreatedEvent)
-	}
+func (s *Service) auditGroupMemberAdded(ctx context.Context, grp group.Group, p principalInfo, roleID string) {
+	s.auditMemberChange(ctx, pkgAuditRecord.GroupMemberAddedEvent, audit.GroupMemberCreatedEvent,
+		groupAuditResource(grp), grp.OrganizationID, p, roleID, map[string]string{"role_id": roleID, "group_id": grp.ID})
+}
+
+func (s *Service) auditGroupMemberRoleChanged(ctx context.Context, grp group.Group, p principalInfo, roleID string) {
+	s.auditMemberChange(ctx, pkgAuditRecord.GroupMemberRoleChangedEvent, audit.GroupMemberRoleChangedEvent,
+		groupAuditResource(grp), grp.OrganizationID, p, roleID, map[string]string{"role_id": roleID, "group_id": grp.ID})
+}
+
+func (s *Service) auditGroupMemberRemoved(ctx context.Context, grp group.Group, p principalInfo) {
+	s.auditMemberChange(ctx, pkgAuditRecord.GroupMemberRemovedEvent, audit.GroupMemberRemovedEvent,
+		groupAuditResource(grp), grp.OrganizationID, p, "", map[string]string{"group_id": grp.ID})
 }
 
 func (s *Service) auditOrgMemberRemoved(ctx context.Context, org organization.Organization, targetID string, targetType pkgAuditRecord.EntityType) {
 	s.createAuditRecord(ctx, auditrecord.AuditRecord{
-		Event: pkgAuditRecord.OrganizationMemberRemovedEvent,
-		Resource: auditrecord.Resource{
-			ID:   org.ID,
-			Type: pkgAuditRecord.OrganizationType,
-			Name: org.Title,
-		},
+		Event:    pkgAuditRecord.OrganizationMemberRemovedEvent,
+		Resource: orgAuditResource(org),
 		Target: &auditrecord.Target{
 			ID:   targetID,
 			Type: targetType,
@@ -134,21 +130,6 @@ func (s *Service) auditOrgMemberRemoved(ctx context.Context, org organization.Or
 	})
 }
 
-func principalTypeToAuditType(principalType string) (pkgAuditRecord.EntityType, error) {
-	switch principalType {
-	case schema.ServiceUserPrincipal:
-		return pkgAuditRecord.ServiceUserType, nil
-	case schema.UserPrincipal:
-		return pkgAuditRecord.UserType, nil
-	case schema.GroupPrincipal:
-		return pkgAuditRecord.GroupType, nil
-	case schema.PATPrincipal:
-		return pkgAuditRecord.PATType, nil
-	default:
-		return "", ErrInvalidPrincipalType
-	}
-}
-
 func (s *Service) auditProjectMember(ctx context.Context, event pkgAuditRecord.Event, prj project.Project, principalID, principalType string, meta map[string]any) {
 	targetType, _ := principalTypeToAuditType(principalType)
 	if meta == nil {
@@ -172,106 +153,17 @@ func (s *Service) auditProjectMember(ctx context.Context, event pkgAuditRecord.E
 	})
 }
 
-func (s *Service) auditGroupMemberAdded(ctx context.Context, grp group.Group, p principalInfo, roleID string) {
-	targetType, _ := principalTypeToAuditType(p.Type)
-	meta := map[string]any{"role_id": roleID}
-	if p.Email != "" {
-		meta["email"] = p.Email
-	}
-
-	s.createAuditRecord(ctx, auditrecord.AuditRecord{
-		Event: pkgAuditRecord.GroupMemberAddedEvent,
-		Resource: auditrecord.Resource{
-			ID:   grp.ID,
-			Type: pkgAuditRecord.GroupType,
-			Name: grp.Title,
-		},
-		Target: &auditrecord.Target{
-			ID:       p.ID,
-			Type:     targetType,
-			Name:     p.Name,
-			Metadata: meta,
-		},
-		OrgID:      grp.OrganizationID,
-		OccurredAt: time.Now(),
-	})
-
-	if err := audit.GetAuditor(ctx, grp.OrganizationID).LogWithAttrs(audit.GroupMemberCreatedEvent, audit.Target{
-		ID:   p.ID,
-		Type: p.Type,
-	}, map[string]string{
-		"role_id":  roleID,
-		"group_id": grp.ID,
-	}); err != nil {
-		s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.GroupMemberCreatedEvent)
-	}
-}
-
-func (s *Service) auditGroupMemberRoleChanged(ctx context.Context, grp group.Group, p principalInfo, roleID string) {
-	targetType, _ := principalTypeToAuditType(p.Type)
-	meta := map[string]any{"role_id": roleID}
-	if p.Email != "" {
-		meta["email"] = p.Email
-	}
-
-	s.createAuditRecord(ctx, auditrecord.AuditRecord{
-		Event: pkgAuditRecord.GroupMemberRoleChangedEvent,
-		Resource: auditrecord.Resource{
-			ID:   grp.ID,
-			Type: pkgAuditRecord.GroupType,
-			Name: grp.Title,
-		},
-		Target: &auditrecord.Target{
-			ID:       p.ID,
-			Type:     targetType,
-			Name:     p.Name,
-			Metadata: meta,
-		},
-		OrgID:      grp.OrganizationID,
-		OccurredAt: time.Now(),
-	})
-
-	if err := audit.GetAuditor(ctx, grp.OrganizationID).LogWithAttrs(audit.GroupMemberRoleChangedEvent, audit.Target{
-		ID:   p.ID,
-		Type: p.Type,
-	}, map[string]string{
-		"role_id":  roleID,
-		"group_id": grp.ID,
-	}); err != nil {
-		s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.GroupMemberRoleChangedEvent)
-	}
-}
-
-func (s *Service) auditGroupMemberRemoved(ctx context.Context, grp group.Group, p principalInfo) {
-	targetType, _ := principalTypeToAuditType(p.Type)
-	meta := map[string]any{}
-	if p.Email != "" {
-		meta["email"] = p.Email
-	}
-
-	s.createAuditRecord(ctx, auditrecord.AuditRecord{
-		Event: pkgAuditRecord.GroupMemberRemovedEvent,
-		Resource: auditrecord.Resource{
-			ID:   grp.ID,
-			Type: pkgAuditRecord.GroupType,
-			Name: grp.Title,
-		},
-		Target: &auditrecord.Target{
-			ID:       p.ID,
-			Type:     targetType,
-			Name:     p.Name,
-			Metadata: meta,
-		},
-		OrgID:      grp.OrganizationID,
-		OccurredAt: time.Now(),
-	})
-
-	if err := audit.GetAuditor(ctx, grp.OrganizationID).LogWithAttrs(audit.GroupMemberRemovedEvent, audit.Target{
-		ID:   p.ID,
-		Type: p.Type,
-	}, map[string]string{
-		"group_id": grp.ID,
-	}); err != nil {
-		s.log.WarnContext(ctx, "failed to write audit log", "error", err, "event", audit.GroupMemberRemovedEvent)
+func principalTypeToAuditType(principalType string) (pkgAuditRecord.EntityType, error) {
+	switch principalType {
+	case schema.ServiceUserPrincipal:
+		return pkgAuditRecord.ServiceUserType, nil
+	case schema.UserPrincipal:
+		return pkgAuditRecord.UserType, nil
+	case schema.GroupPrincipal:
+		return pkgAuditRecord.GroupType, nil
+	case schema.PATPrincipal:
+		return pkgAuditRecord.PATType, nil
+	default:
+		return "", ErrInvalidPrincipalType
 	}
 }

core/membership/group.go

@@ -4,13 +4,11 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"slices"
 
 	"github.com/raystack/frontier/core/policy"
 	"github.com/raystack/frontier/core/relation"
 	"github.com/raystack/frontier/core/role"
 	"github.com/raystack/frontier/internal/bootstrap/schema"
-	"github.com/raystack/frontier/pkg/utils"
 )
 
 // removeGroupMemberRelation deletes the member relation for a principal on a group.
@@ -83,7 +81,7 @@ func (s *Service) SetGroupMemberRole(ctx context.Context, groupID, principalID,
 	}
 
 	// change path: skip if the principal already has exactly this role
-	if len(existing) == 1 && existing[0].RoleID == resolvedRoleID {
+	if hasExactlyRole(existing, resolvedRoleID) {
 		return nil
 	}
 
@@ -176,11 +174,11 @@ func (s *Service) RemoveAllGroupMembers(ctx context.Context, groupID string) err
 	// First pass: delete every policy. Track which principals had any
 	// delete failure so we don't strip their SpiceDB relations while a
 	// surviving policy still references them.
-	principals := make(map[string]policy.Policy, len(policies))
-	failed := make(map[string]struct{}, len(policies))
+	principals := make(map[principalKey]policy.Policy, len(policies))
+	failed := make(map[principalKey]struct{}, len(policies))
 	var errs error
 	for _, p := range policies {
-		key := p.PrincipalType + "\x00" + p.PrincipalID
+		key := policyPrincipalKey(p)
 		principals[key] = p
 		if delErr := s.policyService.Delete(ctx, p.ID); delErr != nil {
 			failed[key] = struct{}{}
@@ -311,56 +309,13 @@ func (s *Service) unlinkGroupFromOrg(ctx context.Context, groupID, orgID string)
 //   - a platform-wide role scoped to groups, or
 //   - a custom role created for the group's parent organization.
 func (s *Service) validateGroupRole(ctx context.Context, roleID, orgID string) (role.Role, error) {
-	fetchedRole, err := s.roleService.Get(ctx, roleID)
-	if err != nil {
-		return role.Role{}, err
-	}
-	if !slices.Contains(fetchedRole.Scopes, schema.GroupNamespace) {
-		return role.Role{}, ErrInvalidGroupRole
-	}
-	if fetchedRole.OrgID == orgID {
-		return fetchedRole, nil
-	}
-	if utils.IsNullUUID(fetchedRole.OrgID) {
-		return fetchedRole, nil
-	}
-	return role.Role{}, ErrInvalidGroupRole
+	return s.validateRoleForScope(ctx, roleID, orgID, schema.GroupNamespace, ErrInvalidGroupRole)
 }
 
 // validateMinGroupOwnerConstraint ensures the group keeps at least one owner
 // after the role change. Returns the resolved group owner role ID so the
 // caller can hand it to replacePolicy as a min-role guard, closing the TOCTOU
 // race between this pre-check and the policy delete.
 func (s *Service) validateMinGroupOwnerConstraint(ctx context.Context, groupID, newRoleID string, existing []policy.Policy) (string, error) {
-	ownerRole, err := s.roleService.Get(ctx, schema.GroupOwnerRole)
-	if err != nil {
-		return "", fmt.Errorf("get group owner role: %w", err)
-	}
-
-	if newRoleID == ownerRole.ID {
-		return ownerRole.ID, nil
-	}
-
-	isCurrentlyOwner := false
-	for _, p := range existing {
-		if p.RoleID == ownerRole.ID {
-			isCurrentlyOwner = true
-			break
-		}
-	}
-	if !isCurrentlyOwner {
-		return ownerRole.ID, nil
-	}
-
-	ownerPolicies, err := s.policyService.List(ctx, policy.Filter{
-		GroupID: groupID,
-		RoleID:  ownerRole.ID,
-	})
-	if err != nil {
-		return "", fmt.Errorf("list group owner policies: %w", err)
-	}
-	if len(ownerPolicies) <= 1 {
-		return "", ErrLastGroupOwnerRole
-	}
-	return ownerRole.ID, nil
+	return s.validateMinRoleConstraint(ctx, schema.GroupOwnerRole, policy.Filter{GroupID: groupID}, newRoleID, existing, ErrLastGroupOwnerRole)
 }