Nicely handle missing user in collaborations

It is possible to have a collaboration in a repository which refers to a no-longer
existing user. This causes the repository transfer to fail with an unusual error.

This PR makes `repo.getCollaborators()` nicely handle the missing user by ghosting
the collaboration but also adds consistency check. It also adds an
Access consistency check.

Fix go-gitea#17044

Signed-off-by: Andrew Thornton <[email protected]>

Author: zeripath

models/access.go

@@ -225,6 +225,9 @@ func (repo *Repository) refreshCollaboratorAccesses(e Engine, accessMap map[int6
 		return fmt.Errorf("getCollaborations: %v", err)
 	}
 	for _, c := range collaborators {
+		if c.User.IsGhost() {
+			continue
+		}
 		updateUserAccess(accessMap, c.User, c.Collaboration.Mode)
 	}
 	return nil

models/repo_collaboration.go

@@ -8,6 +8,7 @@ package models
 import (
 	"fmt"
 
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/timeutil"
 
 	"xorm.io/builder"
@@ -83,16 +84,24 @@ func (repo *Repository) getCollaborators(e Engine, listOptions ListOptions) ([]*
 		return nil, fmt.Errorf("getCollaborations: %v", err)
 	}
 
-	collaborators := make([]*Collaborator, len(collaborations))
-	for i, c := range collaborations {
+	collaborators := make([]*Collaborator, 0, len(collaborations))
+	for _, c := range collaborations {
 		user, err := getUserByID(e, c.UserID)
 		if err != nil {
+			if IsErrUserNotExist(err) {
+				log.Warn("Inconsistent DB: User: %d is listed as collaborator of %-v but does not exist", c.UserID, repo)
+				collaborators = append(collaborators, &Collaborator{
+					User:          NewGhostUser(),
+					Collaboration: c,
+				})
+				continue
+			}
 			return nil, err
 		}
-		collaborators[i] = &Collaborator{
+		collaborators = append(collaborators, &Collaborator{
 			User:          user,
 			Collaboration: c,
-		}
+		})
 	}
 	return collaborators, nil
 }

models/repo_transfer.go

@@ -269,6 +269,14 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e
 	// Dummy object.
 	collaboration := &Collaboration{RepoID: repo.ID}
 	for _, c := range collaborators {
+		if c.IsGhost() {
+			collaboration.ID = c.Collaboration.ID
+			if _, err := sess.Delete(collaboration); err != nil {
+				return fmt.Errorf("remove collaborator '%d': %v", c.ID, err)
+			}
+			collaboration.ID = 0
+		}
+
 		if c.ID != newOwner.ID {
 			isMember, err := isOrganizationMember(sess, newOwner.ID, c.ID)
 			if err != nil {
@@ -281,6 +289,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e
 		if _, err := sess.Delete(collaboration); err != nil {
 			return fmt.Errorf("remove collaborator '%d': %v", c.ID, err)
 		}
+		collaboration.UserID = 0
 	}
 
 	// Remove old team-repository relations.

modules/doctor/dbconsistency.go

@@ -262,6 +262,78 @@ func checkDBConsistency(logger log.Logger, autofix bool) error {
 		}
 	}
 
+	// find collaborations without users
+	count, err = models.CountOrphanedObjects("collaboration", "user", "collaboration.user_id=user.id")
+	if err != nil {
+		logger.Critical("Error: %v whilst counting orphaned collaborations", err)
+		return err
+	}
+	if count > 0 {
+		if autofix {
+			if err = models.DeleteOrphanedObjects("collaboration", "user", "collaboration.user_id=user.id"); err != nil {
+				logger.Critical("Error: %v whilst deleting orphaned collaborations", err)
+				return err
+			}
+			logger.Info("%d collaborations without existing users deleted", count)
+		} else {
+			logger.Warn("%d collaborations without existing users", count)
+		}
+	}
+
+	// find collaborations without repository
+	count, err = models.CountOrphanedObjects("collaboration", "repository", "collaboration.repo_id=repository.id")
+	if err != nil {
+		logger.Critical("Error: %v whilst counting orphaned collaborations", err)
+		return err
+	}
+	if count > 0 {
+		if autofix {
+			if err = models.DeleteOrphanedObjects("collaboration", "repository", "collaboration.repo_id=repository.id"); err != nil {
+				logger.Critical("Error: %v whilst deleting orphaned collaborations", err)
+				return err
+			}
+			logger.Info("%d collaborations without existing repository deleted", count)
+		} else {
+			logger.Warn("%d collaborations without existing repository", count)
+		}
+	}
+
+	// find access without users
+	count, err = models.CountOrphanedObjects("access", "user", "access.user_id=user.id")
+	if err != nil {
+		logger.Critical("Error: %v whilst counting orphaned access", err)
+		return err
+	}
+	if count > 0 {
+		if autofix {
+			if err = models.DeleteOrphanedObjects("access", "user", "access.user_id=user.id"); err != nil {
+				logger.Critical("Error: %v whilst deleting orphaned access", err)
+				return err
+			}
+			logger.Info("%d access without existing users deleted", count)
+		} else {
+			logger.Warn("%d access without existing users", count)
+		}
+	}
+
+	// find access without repository
+	count, err = models.CountOrphanedObjects("access", "repository", "access.repo_id=repository.id")
+	if err != nil {
+		logger.Critical("Error: %v whilst counting orphaned access", err)
+		return err
+	}
+	if count > 0 {
+		if autofix {
+			if err = models.DeleteOrphanedObjects("access", "repository", "access.repo_id=repository.id"); err != nil {
+				logger.Critical("Error: %v whilst deleting orphaned access", err)
+				return err
+			}
+			logger.Info("%d access without existing repository deleted", count)
+		} else {
+			logger.Warn("%d access without existing repository", count)
+		}
+	}
+
 	return nil
 }