github-forks
/
gitea
mirror of https://github.com/go-gitea/gitea.git
1
0
Fork 0
Code Projects Releases Activity

Nicely handle missing user in collaborations () ()

Browse Source 
Backport 

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 

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
pull/17172/head
zeripath committed by GitHub
parent 4b8b214108
commit 4707d4b8a9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 155 additions and 222 deletions

3
models/access.go
Unescape Escape View File

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

14
models/repo_collaboration.go
Unescape Escape View File

@ -8,6 +8,7 @@ package models
import ( import (
"fmt" "fmt"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
"xorm.io/builder" "xorm.io/builder"
@ -83,16 +84,21 @@ func (repo *Repository) getCollaborators(e Engine, listOptions ListOptions) ([]*
return nil, fmt.Errorf("getCollaborations: %v", err) return nil, fmt.Errorf("getCollaborations: %v", err)
} }
collaborators := make([]*Collaborator, len(collaborations)) collaborators := make([]*Collaborator, 0, len(collaborations))
for i, c := range collaborations { for _, c := range collaborations {
user, err := getUserByID(e, c.UserID) user, err := getUserByID(e, c.UserID)
if err != nil { 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)
user = NewGhostUser()
} else {
return nil, err return nil, err
} }
collaborators[i] = &Collaborator{ }
collaborators = append(collaborators, &Collaborator{
User: user, User: user,
Collaboration: c, Collaboration: c,
} })
} }
return collaborators, nil return collaborators, nil
} }

9
models/repo_transfer.go
Unescape Escape View File

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

327
modules/doctor/dbconsistency.go
Unescape Escape View File

@ -13,253 +13,168 @@ import (
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
) )
func checkDBConsistency(logger log.Logger, autofix bool) error { type consistencyCheck struct {
// make sure DB version is uptodate Name string
if err := models.NewEngine(context.Background(), migrations.EnsureUpToDate); err != nil { Counter func() (int64, error)
logger.Critical("Model version on the database does not match the current Gitea version. Model consistency will not be checked until the database is upgraded") Fixer func() (int64, error)
return err FixedMessage string
} }
// find labels without existing repo or org func (c *consistencyCheck) Run(logger log.Logger, autofix bool) error {
count, err := models.CountOrphanedLabels() count, err := c.Counter()
if err != nil { if err != nil {
logger.Critical("Error: %v whilst counting orphaned labels", err) logger.Critical("Error: %v whilst counting %s", err, c.Name)
return err return err
} }
if count > 0 { if count > 0 {
if autofix { if autofix {
if err = models.DeleteOrphanedLabels(); err != nil { var fixed int64
logger.Critical("Error: %v whilst deleting orphaned labels", err) if fixed, err = c.Fixer(); err != nil {
logger.Critical("Error: %v whilst fixing %s", err, c.Name)
return err return err
} }
logger.Info("%d labels without existing repository/organisation deleted", count)
} else {
logger.Warn("%d labels without existing repository/organisation", count)
}
}
// find IssueLabels without existing label prompt := "Deleted"
count, err = models.CountOrphanedIssueLabels() if c.FixedMessage != "" {
if err != nil { prompt = c.FixedMessage
logger.Critical("Error: %v whilst counting orphaned issue_labels", err)
return err
}
if count > 0 {
if autofix {
if err = models.DeleteOrphanedIssueLabels(); err != nil {
logger.Critical("Error: %v whilst deleting orphaned issue_labels", err)
return err
}
logger.Info("%d issue_labels without existing label deleted", count)
} else {
logger.Warn("%d issue_labels without existing label", count)
}
} }
// find issues without existing repository if fixed < 0 {
count, err = models.CountOrphanedIssues() logger.Info(prompt+" %d %s", count, c.Name)
if err != nil {
logger.Critical("Error: %v whilst counting orphaned issues", err)
return err
}
if count > 0 {
if autofix {
if err = models.DeleteOrphanedIssues(); err != nil {
logger.Critical("Error: %v whilst deleting orphaned issues", err)
return err
}
logger.Info("%d issues without existing repository deleted", count)
} else { } else {
logger.Warn("%d issues without existing repository", count) logger.Info(prompt+" %d/%d %s", fixed, count, c.Name)
}
}
// find pulls without existing issues
count, err = models.CountOrphanedObjects("pull_request", "issue", "pull_request.issue_id=issue.id")
if err != nil {
logger.Critical("Error: %v whilst counting orphaned objects", err)
return err
}
if count > 0 {
if autofix {
if err = models.DeleteOrphanedObjects("pull_request", "issue", "pull_request.issue_id=issue.id"); err != nil {
logger.Critical("Error: %v whilst deleting orphaned objects", err)
return err
} }
logger.Info("%d pull requests without existing issue deleted", count)
} else { } else {
logger.Warn("%d pull requests without existing issue", count) logger.Warn("Found %d %s", count, c.Name)
}
}
// find tracked times without existing issues/pulls
count, err = models.CountOrphanedObjects("tracked_time", "issue", "tracked_time.issue_id=issue.id")
if err != nil {
logger.Critical("Error: %v whilst counting orphaned objects", err)
return err
}
if count > 0 {
if autofix {
if err = models.DeleteOrphanedObjects("tracked_time", "issue", "tracked_time.issue_id=issue.id"); err != nil {
logger.Critical("Error: %v whilst deleting orphaned objects", err)
return err
} }
logger.Info("%d tracked times without existing issue deleted", count)
} else {
logger.Warn("%d tracked times without existing issue", count)
} }
return nil
} }
// find null archived repositories func asFixer(fn func() error) func() (int64, error) {
count, err = models.CountNullArchivedRepository() return func() (int64, error) {
if err != nil { err := fn()
logger.Critical("Error: %v whilst counting null archived repositories", err) return -1, err
return err
}
if count > 0 {
if autofix {
updatedCount, err := models.FixNullArchivedRepository()
if err != nil {
logger.Critical("Error: %v whilst fixing null archived repositories", err)
return err
}
logger.Info("%d repositories with null is_archived updated", updatedCount)
} else {
logger.Warn("%d repositories with null is_archived", count)
} }
} }
// find label comments with empty labels func genericOrphanCheck(name, subject, refobject, joincond string) consistencyCheck {
count, err = models.CountCommentTypeLabelWithEmptyLabel() return consistencyCheck{
if err != nil { Name: name,
logger.Critical("Error: %v whilst counting label comments with empty labels", err) Counter: func() (int64, error) {
return err return models.CountOrphanedObjects(subject, refobject, joincond)
} },
if count > 0 { Fixer: func() (int64, error) {
if autofix { err := models.DeleteOrphanedObjects(subject, refobject, joincond)
updatedCount, err := models.FixCommentTypeLabelWithEmptyLabel() return -1, err
if err != nil { },
logger.Critical("Error: %v whilst removing label comments with empty labels", err)
return err
}
logger.Info("%d label comments with empty labels removed", updatedCount)
} else {
logger.Warn("%d label comments with empty labels", count)
} }
} }
// find label comments with labels from outside the repository func checkDBConsistency(logger log.Logger, autofix bool) error {
count, err = models.CountCommentTypeLabelWithOutsideLabels() // make sure DB version is uptodate
if err != nil { if err := models.NewEngine(context.Background(), migrations.EnsureUpToDate); err != nil {
logger.Critical("Error: %v whilst counting label comments with outside labels", err) logger.Critical("Model version on the database does not match the current Gitea version. Model consistency will not be checked until the database is upgraded")
return err
}
if count > 0 {
if autofix {
updatedCount, err := models.FixCommentTypeLabelWithOutsideLabels()
if err != nil {
logger.Critical("Error: %v whilst removing label comments with outside labels", err)
return err return err
} }
log.Info("%d label comments with outside labels removed", updatedCount)
} else {
log.Warn("%d label comments with outside labels", count)
}
}
consistencyChecks := []consistencyCheck{
{
// find labels without existing repo or org
Name: "Orphaned Labels without existing repository or organisation",
Counter: models.CountOrphanedLabels,
Fixer: asFixer(models.DeleteOrphanedLabels),
},
{
// find IssueLabels without existing label
Name: "Orphaned Issue Labels without existing label",
Counter: models.CountOrphanedIssueLabels,
Fixer: asFixer(models.DeleteOrphanedIssueLabels),
},
{
// find issues without existing repository
Name: "Orphaned Issues without existing repository",
Counter: models.CountOrphanedIssues,
Fixer: asFixer(models.DeleteOrphanedIssues),
},
// find releases without existing repository
genericOrphanCheck("Orphaned Releases without existing repository",
"release", "repository", "release.repo_id=repository.id"),
// find pulls without existing issues
genericOrphanCheck("Orphaned PullRequests without existing issue",
"pull_request", "issue", "pull_request.issue_id=issue.id"),
// find tracked times without existing issues/pulls
genericOrphanCheck("Orphaned TrackedTimes without existing issue",
"tracked_time", "issue", "tracked_time.issue_id=issue.id"),
// find null archived repositories
{
Name: "Repositories with is_archived IS NULL",
Counter: models.CountNullArchivedRepository,
Fixer: models.FixNullArchivedRepository,
FixedMessage: "Fixed",
},
// find label comments with empty labels
{
Name: "Label comments with empty labels",
Counter: models.CountCommentTypeLabelWithEmptyLabel,
Fixer: models.FixCommentTypeLabelWithEmptyLabel,
FixedMessage: "Fixed",
},
// find label comments with labels from outside the repository
{
Name: "Label comments with labels from outside the repository",
Counter: models.CountCommentTypeLabelWithOutsideLabels,
Fixer: models.FixCommentTypeLabelWithOutsideLabels,
FixedMessage: "Removed",
},
// find issue_label with labels from outside the repository // find issue_label with labels from outside the repository
count, err = models.CountIssueLabelWithOutsideLabels() {
if err != nil { Name: "IssueLabels with Labels from outside the repository",
logger.Critical("Error: %v whilst counting issue_labels from outside the repository or organisation", err) Counter: models.CountIssueLabelWithOutsideLabels,
return err Fixer: models.FixIssueLabelWithOutsideLabels,
} FixedMessage: "Removed",
if count > 0 { },
if autofix {
updatedCount, err := models.FixIssueLabelWithOutsideLabels()
if err != nil {
logger.Critical("Error: %v whilst removing issue_labels from outside the repository or organisation", err)
return err
}
logger.Info("%d issue_labels from outside the repository or organisation removed", updatedCount)
} else {
logger.Warn("%d issue_labels from outside the repository or organisation", count)
}
} }
// TODO: function to recalc all counters // TODO: function to recalc all counters
if setting.Database.UsePostgreSQL { if setting.Database.UsePostgreSQL {
count, err = models.CountBadSequences() consistencyChecks = append(consistencyChecks, consistencyCheck{
if err != nil { Name: "Sequence values",
logger.Critical("Error: %v whilst checking sequence values", err) Counter: models.CountBadSequences,
return err Fixer: asFixer(models.FixBadSequences),
} FixedMessage: "Updated",
if count > 0 { })
if autofix {
err := models.FixBadSequences()
if err != nil {
logger.Critical("Error: %v whilst attempting to fix sequences", err)
return err
}
logger.Info("%d sequences updated", count)
} else {
logger.Warn("%d sequences with incorrect values", count)
}
}
} }
consistencyChecks = append(consistencyChecks,
// find protected branches without existing repository // find protected branches without existing repository
count, err = models.CountOrphanedObjects("protected_branch", "repository", "protected_branch.repo_id=repository.id") genericOrphanCheck("Protected Branches without existing repository",
if err != nil { "protected_branch", "repository", "protected_branch.repo_id=repository.id"),
logger.Critical("Error: %v whilst counting orphaned objects", err)
return err
}
if count > 0 {
if autofix {
if err = models.DeleteOrphanedObjects("protected_branch", "repository", "protected_branch.repo_id=repository.id"); err != nil {
logger.Critical("Error: %v whilst deleting orphaned objects", err)
return err
}
logger.Info("%d protected branches without existing repository deleted", count)
} else {
logger.Warn("%d protected branches without existing repository", count)
}
}
// find deleted branches without existing repository // find deleted branches without existing repository
count, err = models.CountOrphanedObjects("deleted_branch", "repository", "deleted_branch.repo_id=repository.id") genericOrphanCheck("Deleted Branches without existing repository",
if err != nil { "deleted_branch", "repository", "deleted_branch.repo_id=repository.id"),
logger.Critical("Error: %v whilst counting orphaned objects", err)
return err
}
if count > 0 {
if autofix {
if err = models.DeleteOrphanedObjects("deleted_branch", "repository", "deleted_branch.repo_id=repository.id"); err != nil {
logger.Critical("Error: %v whilst deleting orphaned objects", err)
return err
}
logger.Info("%d deleted branches without existing repository deleted", count)
} else {
logger.Warn("%d deleted branches without existing repository", count)
}
}
// find LFS locks without existing repository // find LFS locks without existing repository
count, err = models.CountOrphanedObjects("lfs_lock", "repository", "lfs_lock.repo_id=repository.id") genericOrphanCheck("LFS locks without existing repository",
if err != nil { "lfs_lock", "repository", "lfs_lock.repo_id=repository.id"),
logger.Critical("Error: %v whilst counting orphaned objects", err) // find collaborations without users
return err genericOrphanCheck("Collaborations without existing user",
} "collaboration", "user", "collaboration.user_id=user.id"),
if count > 0 { // find collaborations without repository
if autofix { genericOrphanCheck("Collaborations without existing repository",
if err = models.DeleteOrphanedObjects("lfs_lock", "repository", "lfs_lock.repo_id=repository.id"); err != nil { "collaboration", "repository", "collaboration.repo_id=repository.id"),
logger.Critical("Error: %v whilst deleting orphaned objects", err) // find access without users
genericOrphanCheck("Access entries without existing user",
"access", "user", "access.user_id=user.id"),
// find access without repository
genericOrphanCheck("Access entries without existing repository",
"access", "repository", "access.repo_id=repository.id"),
)
for _, c := range consistencyChecks {
if err := c.Run(logger, autofix); err != nil {
return err return err
} }
logger.Info("%d LFS locks without existing repository deleted", count)
} else {
logger.Warn("%d LFS locks without existing repository", count)
}
} }
return nil return nil

Write Preview
Loading…
Cancel
Save