From 7bb4ff822d2d7742c0f655123077e6b51eb341c5 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 1 Feb 2022 22:39:19 +0000 Subject: [PATCH 1/2] Collaborator trust model should trust collaborators There was an unintended regression in #17917 which leads to only repository admin commits being trusted. This PR restores the old logic. Fix #18501 Signed-off-by: Andrew Thornton --- models/asymkey/gpg_key_commit_verification.go | 10 +++++----- models/commit.go | 2 +- models/repo_collaboration.go | 8 ++++---- modules/gitgraph/graph_models.go | 2 +- routers/web/repo/commit.go | 2 +- routers/web/repo/view.go | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/models/asymkey/gpg_key_commit_verification.go b/models/asymkey/gpg_key_commit_verification.go index 4b9d0cfda4b1..2f66863091ae 100644 --- a/models/asymkey/gpg_key_commit_verification.go +++ b/models/asymkey/gpg_key_commit_verification.go @@ -71,7 +71,7 @@ const ( ) // ParseCommitsWithSignature checks if signaute of commits are corresponding to users gpg keys. -func ParseCommitsWithSignature(oldCommits []*user_model.UserCommit, repoTrustModel repo_model.TrustModelType, isCodeReader func(*user_model.User) (bool, error)) []*SignCommit { +func ParseCommitsWithSignature(oldCommits []*user_model.UserCommit, repoTrustModel repo_model.TrustModelType, isOwnerMemberCollaborator func(*user_model.User) (bool, error)) []*SignCommit { newCommits := make([]*SignCommit, 0, len(oldCommits)) keyMap := map[string]bool{} @@ -81,7 +81,7 @@ func ParseCommitsWithSignature(oldCommits []*user_model.UserCommit, repoTrustMod Verification: ParseCommitWithSignature(c.Commit), } - _ = CalculateTrustStatus(signCommit.Verification, repoTrustModel, isCodeReader, &keyMap) + _ = CalculateTrustStatus(signCommit.Verification, repoTrustModel, isOwnerMemberCollaborator, &keyMap) newCommits = append(newCommits, signCommit) } @@ -455,7 +455,7 @@ func hashAndVerifyForKeyID(sig *packet.Signature, payload string, committer *use // CalculateTrustStatus will calculate the TrustStatus for a commit verification within a repository // There are several trust models in Gitea -func CalculateTrustStatus(verification *CommitVerification, repoTrustModel repo_model.TrustModelType, isCodeReader func(*user_model.User) (bool, error), keyMap *map[string]bool) (err error) { +func CalculateTrustStatus(verification *CommitVerification, repoTrustModel repo_model.TrustModelType, isOwnerMemberCollaborator func(*user_model.User) (bool, error), keyMap *map[string]bool) (err error) { if !verification.Verified { return } @@ -500,11 +500,11 @@ func CalculateTrustStatus(verification *CommitVerification, repoTrustModel repo_ var has bool isMember, has = (*keyMap)[verification.SigningKey.KeyID] if !has { - isMember, err = isCodeReader(verification.SigningUser) + isMember, err = isOwnerMemberCollaborator(verification.SigningUser) (*keyMap)[verification.SigningKey.KeyID] = isMember } } else { - isMember, err = isCodeReader(verification.SigningUser) + isMember, err = isOwnerMemberCollaborator(verification.SigningUser) } if !isMember { diff --git a/models/commit.go b/models/commit.go index 5df6964a1d06..8911e500ae20 100644 --- a/models/commit.go +++ b/models/commit.go @@ -18,7 +18,7 @@ func ConvertFromGitCommit(commits []*git.Commit, repo *repo_model.Repository) [] user_model.ValidateCommitsWithEmails(commits), repo.GetTrustModel(), func(user *user_model.User) (bool, error) { - return IsUserRepoAdmin(repo, user) + return IsOwnerMemberCollaborator(repo, user) }, ), repo, diff --git a/models/repo_collaboration.go b/models/repo_collaboration.go index 3aca1023e62f..be6dedcdb0eb 100644 --- a/models/repo_collaboration.go +++ b/models/repo_collaboration.go @@ -274,15 +274,15 @@ func GetRepoTeams(repo *repo_model.Repository) ([]*Team, error) { } // IsOwnerMemberCollaborator checks if a provided user is the owner, a collaborator or a member of a team in a repository -func IsOwnerMemberCollaborator(repo *repo_model.Repository, userID int64) (bool, error) { - if repo.OwnerID == userID { +func IsOwnerMemberCollaborator(repo *repo_model.Repository, user *user_model.User) (bool, error) { + if repo.OwnerID == user.ID { return true, nil } teamMember, err := db.GetEngine(db.DefaultContext).Join("INNER", "team_repo", "team_repo.team_id = team_user.team_id"). Join("INNER", "team_unit", "team_unit.team_id = team_user.team_id"). Where("team_repo.repo_id = ?", repo.ID). And("team_unit.`type` = ?", unit.TypeCode). - And("team_user.uid = ?", userID).Table("team_user").Exist(&TeamUser{}) + And("team_user.uid = ?", user.ID).Table("team_user").Exist(&TeamUser{}) if err != nil { return false, err } @@ -290,5 +290,5 @@ func IsOwnerMemberCollaborator(repo *repo_model.Repository, userID int64) (bool, return true, nil } - return db.GetEngine(db.DefaultContext).Get(&Collaboration{RepoID: repo.ID, UserID: userID}) + return db.GetEngine(db.DefaultContext).Get(&Collaboration{RepoID: repo.ID, UserID: user.ID}) } diff --git a/modules/gitgraph/graph_models.go b/modules/gitgraph/graph_models.go index 44773a3b9a31..3d78df7bfebe 100644 --- a/modules/gitgraph/graph_models.go +++ b/modules/gitgraph/graph_models.go @@ -117,7 +117,7 @@ func (graph *Graph) LoadAndProcessCommits(repository *repo_model.Repository, git c.Verification = asymkey_model.ParseCommitWithSignature(c.Commit) _ = asymkey_model.CalculateTrustStatus(c.Verification, repository.GetTrustModel(), func(user *user_model.User) (bool, error) { - return models.IsUserRepoAdmin(repository, user) + return models.IsOwnerMemberCollaborator(repository, user) }, &keyMap) statuses, _, err := models.GetLatestCommitStatus(repository.ID, c.Commit.ID.String(), db.ListOptions{}) diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 12457e45ee70..24524af699b1 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -351,7 +351,7 @@ func Diff(ctx *context.Context) { ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0 if err := asymkey_model.CalculateTrustStatus(verification, ctx.Repo.Repository.GetTrustModel(), func(user *user_model.User) (bool, error) { - return models.IsUserRepoAdmin(ctx.Repo.Repository, user) + return models.IsOwnerMemberCollaborator(ctx.Repo.Repository, user) }, nil); err != nil { ctx.ServerError("CalculateTrustStatus", err) return diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 150ace212b32..a1e6ff9c9f16 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -799,7 +799,7 @@ func renderDirectoryFiles(ctx *context.Context, timeout time.Duration) git.Entri verification := asymkey_model.ParseCommitWithSignature(latestCommit) if err := asymkey_model.CalculateTrustStatus(verification, ctx.Repo.Repository.GetTrustModel(), func(user *user_model.User) (bool, error) { - return models.IsUserRepoAdmin(ctx.Repo.Repository, user) + return models.IsOwnerMemberCollaborator(ctx.Repo.Repository, user) }, nil); err != nil { ctx.ServerError("CalculateTrustStatus", err) return nil From a6fee5e595bd79457b39492983d9d3cc327e5f9e Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 2 Feb 2022 09:25:25 +0000 Subject: [PATCH 2/2] as per lunny Signed-off-by: Andrew Thornton --- models/commit.go | 2 +- models/repo_collaboration.go | 8 ++++---- modules/gitgraph/graph_models.go | 2 +- routers/web/repo/commit.go | 2 +- routers/web/repo/view.go | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/models/commit.go b/models/commit.go index 8911e500ae20..92a839b7808f 100644 --- a/models/commit.go +++ b/models/commit.go @@ -18,7 +18,7 @@ func ConvertFromGitCommit(commits []*git.Commit, repo *repo_model.Repository) [] user_model.ValidateCommitsWithEmails(commits), repo.GetTrustModel(), func(user *user_model.User) (bool, error) { - return IsOwnerMemberCollaborator(repo, user) + return IsOwnerMemberCollaborator(repo, user.ID) }, ), repo, diff --git a/models/repo_collaboration.go b/models/repo_collaboration.go index be6dedcdb0eb..3aca1023e62f 100644 --- a/models/repo_collaboration.go +++ b/models/repo_collaboration.go @@ -274,15 +274,15 @@ func GetRepoTeams(repo *repo_model.Repository) ([]*Team, error) { } // IsOwnerMemberCollaborator checks if a provided user is the owner, a collaborator or a member of a team in a repository -func IsOwnerMemberCollaborator(repo *repo_model.Repository, user *user_model.User) (bool, error) { - if repo.OwnerID == user.ID { +func IsOwnerMemberCollaborator(repo *repo_model.Repository, userID int64) (bool, error) { + if repo.OwnerID == userID { return true, nil } teamMember, err := db.GetEngine(db.DefaultContext).Join("INNER", "team_repo", "team_repo.team_id = team_user.team_id"). Join("INNER", "team_unit", "team_unit.team_id = team_user.team_id"). Where("team_repo.repo_id = ?", repo.ID). And("team_unit.`type` = ?", unit.TypeCode). - And("team_user.uid = ?", user.ID).Table("team_user").Exist(&TeamUser{}) + And("team_user.uid = ?", userID).Table("team_user").Exist(&TeamUser{}) if err != nil { return false, err } @@ -290,5 +290,5 @@ func IsOwnerMemberCollaborator(repo *repo_model.Repository, user *user_model.Use return true, nil } - return db.GetEngine(db.DefaultContext).Get(&Collaboration{RepoID: repo.ID, UserID: user.ID}) + return db.GetEngine(db.DefaultContext).Get(&Collaboration{RepoID: repo.ID, UserID: userID}) } diff --git a/modules/gitgraph/graph_models.go b/modules/gitgraph/graph_models.go index 3d78df7bfebe..653384252dac 100644 --- a/modules/gitgraph/graph_models.go +++ b/modules/gitgraph/graph_models.go @@ -117,7 +117,7 @@ func (graph *Graph) LoadAndProcessCommits(repository *repo_model.Repository, git c.Verification = asymkey_model.ParseCommitWithSignature(c.Commit) _ = asymkey_model.CalculateTrustStatus(c.Verification, repository.GetTrustModel(), func(user *user_model.User) (bool, error) { - return models.IsOwnerMemberCollaborator(repository, user) + return models.IsOwnerMemberCollaborator(repository, user.ID) }, &keyMap) statuses, _, err := models.GetLatestCommitStatus(repository.ID, c.Commit.ID.String(), db.ListOptions{}) diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 24524af699b1..36cc005cec91 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -351,7 +351,7 @@ func Diff(ctx *context.Context) { ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0 if err := asymkey_model.CalculateTrustStatus(verification, ctx.Repo.Repository.GetTrustModel(), func(user *user_model.User) (bool, error) { - return models.IsOwnerMemberCollaborator(ctx.Repo.Repository, user) + return models.IsOwnerMemberCollaborator(ctx.Repo.Repository, user.ID) }, nil); err != nil { ctx.ServerError("CalculateTrustStatus", err) return diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index a1e6ff9c9f16..7c6f031907db 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -799,7 +799,7 @@ func renderDirectoryFiles(ctx *context.Context, timeout time.Duration) git.Entri verification := asymkey_model.ParseCommitWithSignature(latestCommit) if err := asymkey_model.CalculateTrustStatus(verification, ctx.Repo.Repository.GetTrustModel(), func(user *user_model.User) (bool, error) { - return models.IsOwnerMemberCollaborator(ctx.Repo.Repository, user) + return models.IsOwnerMemberCollaborator(ctx.Repo.Repository, user.ID) }, nil); err != nil { ctx.ServerError("CalculateTrustStatus", err) return nil