handle unassignment of role

internal/permissions/assignments.go

@@ -45,6 +45,29 @@ func (c *Client) AssignRole(ctx context.Context, roleID gidx.PrefixedID, memberI
 	return nil
 }
 
+func (c *Client) UnassignRole(ctx context.Context, roleID gidx.PrefixedID, memberID gidx.PrefixedID) error {
+	path := fmt.Sprintf("/api/v1/roles/%s/assignments", roleID.String())
+
+	body, err := encodeJSON(RoleAssign{
+		SubjectID: memberID.String(),
+	})
+	if err != nil {
+		return err
+	}
+
+	var response RoleAssignResponse
+
+	if _, err = c.DoRequest(ctx, http.MethodDelete, path, body, &response); err != nil {
+		return err
+	}
+
+	if !response.Success {
+		return ErrUnassignmentFailed
+	}
+
+	return nil
+}
+
 func (c *Client) ListRoleAssignments(ctx context.Context, roleID gidx.PrefixedID) ([]gidx.PrefixedID, error) {
 	path := fmt.Sprintf("/api/v1/roles/%s/assignments", roleID.String())
 

internal/permissions/errors.go

@@ -3,6 +3,8 @@ package permissions
 import "errors"
 
 var (
-	ErrRoleNotFound     = errors.New("role not found")
-	ErrAssignmentFailed = errors.New("assignment failed")
+	ErrRoleNotFound               = errors.New("role not found")
+	ErrAssignmentFailed           = errors.New("assignment failed")
+	ErrUnassignmentFailed         = errors.New("unassignment failed")
+	ErrUnexpectedRoleDeleteFailed = errors.New("unknown role delete error")
 )

internal/permissions/roles.go

@@ -17,6 +17,10 @@ type ResourceRoleCreateResponse struct {
 	ID string `json:"id"`
 }
 
+type ResourceRoleDeleteResponse struct {
+	Success bool `json:"success"`
+}
+
 type ResourceRoles []ResourceRole
 
 type ResourceRole struct {
@@ -48,6 +52,22 @@ func (c *Client) CreateRole(ctx context.Context, resourceID gidx.PrefixedID, act
 	return roleID, nil
 }
 
+func (c *Client) DeleteRole(ctx context.Context, roleID gidx.PrefixedID) error {
+	path := fmt.Sprintf("/api/v1/roles/%s", roleID.String())
+
+	var response ResourceRoleDeleteResponse
+
+	if _, err := c.DoRequest(ctx, http.MethodDelete, path, nil, &response); err != nil {
+		return err
+	}
+
+	if !response.Success {
+		return ErrUnexpectedRoleDeleteFailed
+	}
+
+	return nil
+}
+
 func (c *Client) ListResourceRoles(ctx context.Context, resourceID gidx.PrefixedID) (ResourceRoles, error) {
 	path := fmt.Sprintf("/api/v1/resources/%s/roles", resourceID.String())
 

internal/pubsub/subscriber.go

@@ -206,7 +206,7 @@ func (s *Subscriber) handleDeleteEvent(ctx context.Context, msg *message.Message
 	}
 
 	if s.svc.IsUser(changeMsg.SubjectID) {
-		if err := s.svc.RemoveUser(ctx, changeMsg.SubjectID, changeMsg.AdditionalSubjectIDs...); err != nil {
+		if err := s.svc.UnassignUser(ctx, changeMsg.SubjectID, changeMsg.AdditionalSubjectIDs...); err != nil {
 			// TODO: only return errors on retryable errors
 			return err
 		}

internal/service/organizations.go

@@ -89,7 +89,7 @@ func (s *service) TouchOrganization(ctx context.Context, id gidx.PrefixedID) err
 	}
 
 	s.processRelationships(ctx, "metal-relation", relationships.Relationships)
-	s.processMemberships(ctx, relationships.Memberships)
+	s.syncMemberships(ctx, relationships.Memberships)
 
 	s.logger.Infow("organization sync complete", "relationships", len(relationships.Relationships), "memberships", len(relationships.Memberships))
 

internal/service/process.go

@@ -1,138 +0,0 @@
-package service
-
-import (
-	"context"
-
-	"go.infratographer.com/x/events"
-	"go.infratographer.com/x/gidx"
-)
-
-func (s *service) processRelationships(ctx context.Context, subjectType string, relationships []Relationship) {
-	var err error
-
-	for _, rel := range relationships {
-		err = s.publisher.PublishChange(ctx, subjectType, events.ChangeMessage{
-			SubjectID: rel.Resource.PrefixedID(),
-			EventType: string(events.CreateChangeType),
-			AdditionalSubjectIDs: []gidx.PrefixedID{
-				rel.RelatedResource.PrefixedID(),
-			},
-		})
-
-		if err != nil {
-			s.logger.Errorw("error publishing change",
-				"subject_type", subjectType,
-				"resource.id", rel.Resource.PrefixedID(),
-				"related_resource.id", rel.RelatedResource.PrefixedID(),
-				"error", err,
-			)
-		}
-	}
-}
-
-func (s *service) processMemberships(ctx context.Context, memberships []ResourceMemberships) {
-	resourceRoleID := make(map[gidx.PrefixedID]map[string]gidx.PrefixedID)
-	resourceRoleMembers := make(map[gidx.PrefixedID]map[string]map[gidx.PrefixedID]bool)
-	roleActions := make(map[string][]string)
-
-	for _, membership := range memberships {
-		resourceID := membership.Resource.PrefixedID()
-		role := membership.Role
-		memberID := membership.Member.PrefixedID()
-
-		if _, ok := resourceRoleMembers[resourceID]; !ok {
-			resourceRoleID[resourceID] = make(map[string]gidx.PrefixedID)
-			resourceRoleMembers[resourceID] = make(map[string]map[gidx.PrefixedID]bool)
-		}
-
-		if _, ok := resourceRoleMembers[resourceID][role]; !ok {
-			resourceRoleMembers[resourceID][role] = make(map[gidx.PrefixedID]bool)
-			roleActions[role] = s.roles[role]
-		}
-
-		resourceRoleID[resourceID][role] = gidx.NullPrefixedID
-		resourceRoleMembers[resourceID][role][memberID] = true
-	}
-
-	resourceRoleAssignments := make(map[gidx.PrefixedID]map[gidx.PrefixedID]map[gidx.PrefixedID]bool)
-
-	for resourceID, roles := range resourceRoleID {
-		resourceRoleAssignments[resourceID] = make(map[gidx.PrefixedID]map[gidx.PrefixedID]bool)
-
-		for role := range roles {
-			actions := roleActions[role]
-
-			resourceRole, err := s.perms.FindResourceRoleByActions(ctx, resourceID, actions)
-			if err != nil {
-				s.logger.Warnw("failed to find role by actions for resource", "resource.id", resourceID, "role", role, "actions", actions, "error", err)
-
-				continue
-			}
-
-			resourceRoleID[resourceID][role] = resourceRole.ID
-			resourceRoleAssignments[resourceID][resourceRole.ID] = make(map[gidx.PrefixedID]bool)
-
-			assignments, err := s.perms.ListRoleAssignments(ctx, resourceRole.ID)
-			if err != nil {
-				s.logger.Warnw("failed to get role assignments for resource", "resource.id", resourceID, "role", role, "error", err)
-
-				continue
-			}
-
-			for _, assignment := range assignments {
-				resourceRoleAssignments[resourceID][resourceRole.ID][assignment] = true
-			}
-		}
-	}
-
-	for resourceID, roles := range resourceRoleMembers {
-		for role, members := range roles {
-			roleID := resourceRoleID[resourceID][role]
-			actions := roleActions[role]
-
-			logger := s.logger.With("resource.id", resourceID, "role.name", role, "actions", actions)
-
-			var createdRole bool
-
-			if roleID == gidx.NullPrefixedID {
-				logger.Infow("creating role for resource")
-
-				resourceRoleID, err := s.perms.CreateRole(ctx, resourceID, actions)
-				if err != nil {
-					logger.Errorw("failed to create role for resource", "error", err)
-
-					continue
-				}
-
-				createdRole = true
-				roleID = resourceRoleID
-			}
-
-			logger = logger.With("role.id", roleID)
-
-			assignments := make(map[gidx.PrefixedID]bool)
-
-			if !createdRole {
-				assignments = resourceRoleAssignments[resourceID][roleID]
-			}
-
-			for memberID := range members {
-				mlogger := logger.With("member.id", memberID)
-
-				if _, ok := assignments[memberID]; ok {
-					mlogger.Infow("skipping already assigned member")
-
-					continue
-				}
-
-				if err := s.perms.AssignRole(ctx, roleID, memberID); err != nil {
-					mlogger.Errorw("failed to assign member to role", "error", err)
-
-					continue
-				}
-
-				mlogger.Infow("role assigned to member")
-			}
-		}
-	}
-}

internal/service/process_memberships.go

@@ -0,0 +1,245 @@
+package service
+
+import (
+	"context"
+	"strings"
+
+	"go.infratographer.com/x/gidx"
+	"golang.org/x/exp/slices"
+
+	"go.equinixmetal.net/infra9-metal-bridge/internal/permissions"
+)
+
+func (s *service) syncMemberships(ctx context.Context, memberships []ResourceMemberships) error {
+	if len(memberships) == 0 {
+		return nil
+	}
+
+	resourceMap := make(map[gidx.PrefixedID][]ResourceMemberships)
+
+	resourceRoleIDs := make(map[gidx.PrefixedID]map[string]gidx.PrefixedID)
+
+	resourceRoleCreations := make(map[gidx.PrefixedID]map[string][]string)
+	resourceRoleDeletions := make(map[gidx.PrefixedID][]gidx.PrefixedID)
+
+	resourceRoleAssignments := make(map[gidx.PrefixedID]map[string][]gidx.PrefixedID)
+	resourceRoleAssignmentRemovals := make(map[gidx.PrefixedID]map[string][]gidx.PrefixedID)
+
+	var (
+		totalRoleCreate, totalRoleDelete   int
+		totalRoleAssign, totalRoleUnassign int
+	)
+
+	for _, membership := range memberships {
+		resourceID := membership.Resource.PrefixedID()
+
+		resourceMap[resourceID] = append(resourceMap[resourceID], membership)
+	}
+
+	for resourceID, memberships := range resourceMap {
+		resourceRoleIDs[resourceID] = make(map[string]gidx.PrefixedID)
+
+		wantRoles, wantAssignments := s.mapResourceWants(memberships)
+
+		liveRoles, liveAssignments, err := s.mapResourceDetails(ctx, resourceID)
+		if err != nil {
+			return err
+		}
+
+		roleCreations := make(map[string][]string)
+		roleDeletions := make([]gidx.PrefixedID, 0)
+
+		for roleKey, actions := range wantRoles {
+			if _, ok := liveRoles[roleKey]; !ok {
+				roleCreations[roleKey] = actions
+			}
+		}
+
+		for roleKey, role := range liveRoles {
+			if _, ok := wantRoles[roleKey]; !ok {
+				roleDeletions = append(roleDeletions, role.ID)
+			}
+
+			resourceRoleIDs[resourceID][roleKey] = role.ID
+		}
+
+		roleAssignments := make(map[string][]gidx.PrefixedID)
+		roleAssignmentRemovals := make(map[string][]gidx.PrefixedID)
+
+		for roleKey, assignments := range wantAssignments {
+			for memberID := range assignments {
+				if _, ok := liveAssignments[roleKey]; ok {
+					if _, ok := liveAssignments[roleKey][memberID]; ok {
+						continue
+					}
+				}
+
+				roleAssignments[roleKey] = append(roleAssignments[roleKey], memberID)
+				totalRoleAssign++
+			}
+		}
+
+		for roleKey, assignments := range liveAssignments {
+			for memberID := range assignments {
+				if _, ok := wantAssignments[roleKey]; ok {
+					if _, ok := wantAssignments[roleKey][memberID]; ok {
+						continue
+					}
+				}
+
+				roleAssignmentRemovals[roleKey] = append(roleAssignmentRemovals[roleKey], memberID)
+				totalRoleUnassign++
+			}
+		}
+
+		resourceRoleCreations[resourceID] = roleCreations
+		resourceRoleDeletions[resourceID] = roleDeletions
+		resourceRoleAssignments[resourceID] = roleAssignments
+		resourceRoleAssignmentRemovals[resourceID] = roleAssignmentRemovals
+
+		totalRoleCreate += len(roleCreations)
+		totalRoleDelete += len(roleDeletions)
+	}
+
+	s.logger.Debugw("processing memberships",
+		"resources", len(resourceMap),
+		"role.create", totalRoleCreate,
+		"role.delete", totalRoleDelete,
+		"role.assign", totalRoleAssign,
+		"role.unassign", totalRoleUnassign,
+	)
+
+	var (
+		rolesCreated, rolesDeleted         int
+		roleAssignments, roleUnassignments int
+	)
+
+	for resourceID := range resourceMap {
+		rlogger := s.logger.With("resource.id", resourceID.String())
+		for roleKey, actions := range resourceRoleCreations[resourceID] {
+			roleID, err := s.perms.CreateRole(ctx, resourceID, actions)
+			if err != nil {
+				rlogger.Errorw("error creating role", "actions", actions, "error", err)
+
+				continue
+			}
+
+			resourceRoleIDs[resourceID][roleKey] = roleID
+			rolesCreated++
+		}
+
+		for _, roleID := range resourceRoleDeletions[resourceID] {
+			if err := s.perms.DeleteRole(ctx, roleID); err != nil {
+				rlogger.Errorw("error deleting role", "role.id", roleID, "error", err)
+
+				continue
+			}
+
+			rolesDeleted++
+		}
+
+		for roleKey, members := range resourceRoleAssignments[resourceID] {
+			roleID, ok := resourceRoleIDs[resourceID][roleKey]
+			if !ok {
+				rlogger.Errorw("role id not found for role actions key", "role_actions_key", roleKey, "members", len(members))
+
+				continue
+			}
+
+			for _, memberID := range members {
+				if err := s.perms.AssignRole(ctx, roleID, memberID); err != nil {
+					rlogger.Errorw("error assigning member to role", "role.id", roleID, "member.id", memberID, "error", err)
+
+					continue
+				}
+
+				roleAssignments++
+			}
+		}
+
+		for roleKey, members := range resourceRoleAssignmentRemovals[resourceID] {
+			roleID, ok := resourceRoleIDs[resourceID][roleKey]
+			if !ok {
+				rlogger.Errorw("role id not found for role actions key", "role_actions_key", roleKey, "members", len(members))
+
+				continue
+			}
+
+			for _, memberID := range members {
+				if err := s.perms.UnassignRole(ctx, roleID, memberID); err != nil {
+					rlogger.Errorw("error removing member from role", "role.id", roleID, "member.id", memberID, "error", err)
+
+					continue
+				}
+
+				roleUnassignments++
+			}
+		}
+	}
+
+	s.logger.Debugw("memberships processed",
+		"resources", len(resourceMap),
+		"role.create", rolesCreated,
+		"role.delete", rolesDeleted,
+		"role.assign", roleAssignments,
+		"role.unassign", roleUnassignments,
+	)
+
+	return nil
+}
+
+func (s *service) mapResourceWants(memberships []ResourceMemberships) (map[string][]string, map[string]map[gidx.PrefixedID]bool) {
+	roleActionsKey := make(map[string]string)
+
+	for role, actions := range s.roles {
+		slices.Sort(actions)
+
+		roleActionsKey[role] = strings.Join(actions, "|")
+	}
+
+	wantRoles := make(map[string][]string)
+	wantAssignments := make(map[string]map[gidx.PrefixedID]bool)
+
+	for _, membership := range memberships {
+		roleKey := roleActionsKey[membership.Role]
+
+		if _, ok := wantRoles[roleKey]; !ok {
+			wantRoles[roleKey] = s.roles[membership.Role]
+			wantAssignments[roleKey] = make(map[gidx.PrefixedID]bool)
+		}
+
+		wantAssignments[roleKey][membership.Member.PrefixedID()] = true
+	}
+
+	return wantRoles, wantAssignments
+}
+
+func (s *service) mapResourceDetails(ctx context.Context, resourceID gidx.PrefixedID) (map[string]permissions.ResourceRole, map[string]map[gidx.PrefixedID]bool, error) {
+	roles := make(map[string]permissions.ResourceRole)
+	assignments := make(map[string]map[gidx.PrefixedID]bool)
+
+	liveRoles, err := s.perms.ListResourceRoles(ctx, resourceID)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	for _, role := range liveRoles {
+		slices.Sort(role.Actions)
+
+		roleKey := strings.Join(role.Actions, "|")
+		roles[roleKey] = role
+
+		liveAssignments, err := s.perms.ListRoleAssignments(ctx, role.ID)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		assignments[roleKey] = make(map[gidx.PrefixedID]bool)
+
+		for _, assignment := range liveAssignments {
+			assignments[roleKey][assignment] = true
+		}
+	}
+
+	return roles, assignments, nil
+}

internal/service/process_relationships.go

@@ -0,0 +1,31 @@
+package service
+
+import (
+	"context"
+
+	"go.infratographer.com/x/events"
+	"go.infratographer.com/x/gidx"
+)
+
+func (s *service) processRelationships(ctx context.Context, subjectType string, relationships []Relationship) {
+	var err error
+
+	for _, rel := range relationships {
+		err = s.publisher.PublishChange(ctx, subjectType, events.ChangeMessage{
+			SubjectID: rel.Resource.PrefixedID(),
+			EventType: string(events.CreateChangeType),
+			AdditionalSubjectIDs: []gidx.PrefixedID{
+				rel.RelatedResource.PrefixedID(),
+			},
+		})
+
+		if err != nil {
+			s.logger.Errorw("error publishing change",
+				"subject_type", subjectType,
+				"resource.id", rel.Resource.PrefixedID(),
+				"related_resource.id", rel.RelatedResource.PrefixedID(),
+				"error", err,
+			)
+		}
+	}
+}

internal/service/projects.go

@@ -65,7 +65,7 @@ func (s *service) TouchProject(ctx context.Context, id gidx.PrefixedID) error {
 	}
 
 	s.processRelationships(ctx, "metal-relation", relationships.Relationships)
-	s.processMemberships(ctx, relationships.Memberships)
+	s.syncMemberships(ctx, relationships.Memberships)
 
 	s.logger.Infow("project sync complete", "relationships", len(relationships.Relationships), "memberships", len(relationships.Memberships))
 

internal/service/service.go

@@ -49,8 +49,8 @@ type Service interface {
 	IsUser(id gidx.PrefixedID) bool
 	// AssignUser assigns a user to the given resource.
 	AssignUser(ctx context.Context, userID gidx.PrefixedID, resourceIDs ...gidx.PrefixedID) error
-	// RemoveUser removes the users from the given resource.
-	RemoveUser(ctx context.Context, userID gidx.PrefixedID, resourceIDs ...gidx.PrefixedID) error
+	// UnassignUser removes the users from the given resource.
+	UnassignUser(ctx context.Context, userID gidx.PrefixedID, resourceIDs ...gidx.PrefixedID) error
 
 	// IsAssignableResource checks if the provided resource ID may have assigned users.
 	IsAssignableResource(id gidx.PrefixedID) bool

internal/service/users.go

@@ -31,22 +31,11 @@ func (s *service) AssignUser(ctx context.Context, userID gidx.PrefixedID, resour
 	var memberships []ResourceMemberships
 
 	for _, resourceID := range resourceIDs {
-		var (
-			role string
-			err  error
-		)
-
-		if idType, ok := s.idPrefixMap[resourceID.Prefix()]; ok {
-			switch idType {
-			case TypeOrganization:
-				role, err = s.metal.GetUserOrganizationRole(ctx, userID, resourceID)
-			case TypeProject:
-				role, err = s.metal.GetUserProjectRole(ctx, userID, resourceID)
-			}
-		}
-
+		role, err := s.getUserResourceRole(ctx, userID, resourceID)
 		if err != nil {
-			return err
+			s.logger.Warnw("failed to determine role for user resource", "error", err)
+
+			continue
 		}
 
 		if role == "" {
@@ -60,13 +49,82 @@ func (s *service) AssignUser(ctx context.Context, userID gidx.PrefixedID, resour
 		})
 	}
 
-	s.processMemberships(ctx, memberships)
+	s.syncMemberships(ctx, memberships)
 
 	s.logger.Infow("assignment sync complete", "memberships", len(memberships))
 
 	return nil
 }
 
-func (s *service) RemoveUser(ctx context.Context, userID gidx.PrefixedID, resourceIDs ...gidx.PrefixedID) error {
+func (s *service) UnassignUser(ctx context.Context, userID gidx.PrefixedID, resourceIDs ...gidx.PrefixedID) error {
+	for _, resourceID := range resourceIDs {
+		rlogger := s.logger.With("user.id", userID, "resource.id", resourceID)
+
+		role, err := s.getUserResourceRole(ctx, userID, resourceID)
+		if err != nil {
+			rlogger.Warnw("failed to determine role for user resource", "error", err)
+
+			continue
+		}
+
+		if role == "" {
+			continue
+		}
+
+		actions := s.roles[role]
+
+		rlogger = rlogger.With("role.name", role, "role.actions", actions)
+
+		resourceRole, err := s.perms.FindResourceRoleByActions(ctx, resourceID, actions)
+		if err != nil {
+			rlogger.Warnw("failed to find role by actions for resource", "error", err)
+
+			continue
+		}
+
+		rlogger = rlogger.With("role.id", resourceRole.ID)
+
+		assigned, err := s.perms.RoleHasAssignment(ctx, resourceRole.ID, userID)
+		if err != nil {
+			rlogger.Warnw("failed to check role assignment", "error", err)
+
+			continue
+		}
+
+		if !assigned {
+			rlogger.Warnw("unable to unassign member which is not assigned")
+
+			continue
+		}
+
+		if err = s.perms.UnassignRole(ctx, resourceRole.ID, userID); err != nil {
+			rlogger.Errorw("failed to unassign member from role", "error", err)
+
+			continue
+		}
+	}
+
 	return nil
 }
+
+func (s *service) getUserResourceRole(ctx context.Context, userID, resourceID gidx.PrefixedID) (string, error) {
+	var (
+		role string
+		err  error
+	)
+
+	if idType, ok := s.idPrefixMap[resourceID.Prefix()]; ok {
+		switch idType {
+		case TypeOrganization:
+			role, err = s.metal.GetUserOrganizationRole(ctx, userID, resourceID)
+		case TypeProject:
+			role, err = s.metal.GetUserProjectRole(ctx, userID, resourceID)
+		}
+	}
+
+	if err != nil {
+		return "", err
+	}
+
+	return role, nil
+}