gitea/gitea
0
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2025-06-21 21:48:00 +02:00
Code Issues Packages Projects Releases Wiki Activity

Fix Permission in API returned repository struct (#25388)

Browse Source 
The old code generates `structs.Repository.Permissions` with only
`access.Permission.AccessMode`, however, it should check the units too,
or the value could be incorrect. For example,
`structs.Repository.Permissions.Push` could be false even the doer has
write access to code unit.

Should fix
https://github.com/renovatebot/renovate/issues/14059#issuecomment-1047961128
(Not reported by it, I just found it when I was looking into this bug)

---

Review tips:

The major changes are
- `modules/structs/repo.go`
https://github.com/go-gitea/gitea/pull/25388/files#diff-870406f6857117f8b03611c43fca0ab9ed6d6e76a2d0069a7c1f17e8fa9092f7
- `services/convert/repository.go`
https://github.com/go-gitea/gitea/pull/25388/files#diff-7736f6d2ae894c9edb7729a80ab89aa183b888a26a811a0c1fdebd18726a7101

And other changes are passive.
This commit is contained in:
Jason Song
2023-06-22 21:08:08 +08:00
committed by GitHub
parent 7fb539677b
commit 174213530d
22 changed files with 174 additions and 153 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
services/convert/activity.go
View File

@ -28,7 +28,7 @@ func ToActivity(ctx context.Context, ac *activities_model.Action, doer *user_mod
ActUserID: ac.ActUserID,
ActUser: ToUser(ctx, ac.ActUser, doer),
RepoID: ac.RepoID,
Repo: ToRepo(ctx, ac.Repo, p.AccessMode),
Repo: ToRepo(ctx, ac.Repo, p),
RefName: ac.RefName,
IsPrivate: ac.IsPrivate,
Content: ac.Content,

3
services/convert/notification.go
View File

@ -9,6 +9,7 @@ import (
activities_model "code.gitea.io/gitea/models/activities"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
api "code.gitea.io/gitea/modules/structs"
)
@ -24,7 +25,7 @@ func ToNotificationThread(n *activities_model.Notification) *api.NotificationThr
// since user only get notifications when he has access to use minimal access mode
if n.Repository != nil {
result.Repository = ToRepo(db.DefaultContext, n.Repository, perm.AccessModeRead)
result.Repository = ToRepo(db.DefaultContext, n.Repository, access_model.Permission{AccessMode: perm.AccessModeRead})
// This permission is not correct and we should not be reporting it
for repository := result.Repository; repository != nil; repository = repository.Parent {

2
services/convert/package.go
View File

@ -22,7 +22,7 @@ func ToPackage(ctx context.Context, pd *packages.PackageDescriptor, doer *user_m
}
if permission.HasAccess() {
repo = ToRepo(ctx, pd.Repository, permission.AccessMode)
repo = ToRepo(ctx, pd.Repository, permission)
}
}

4
services/convert/pull.go
View File

@ -80,7 +80,7 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u
Name: pr.BaseBranch,
Ref: pr.BaseBranch,
RepoID: pr.BaseRepoID,
Repository: ToRepo(ctx, pr.BaseRepo, p.AccessMode),
Repository: ToRepo(ctx, pr.BaseRepo, p),
},
Head: &api.PRBranchInfo{
Name: pr.HeadBranch,
@ -152,7 +152,7 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u
}
apiPullRequest.Head.RepoID = pr.HeadRepo.ID
apiPullRequest.Head.Repository = ToRepo(ctx, pr.HeadRepo, p.AccessMode)
apiPullRequest.Head.Repository = ToRepo(ctx, pr.HeadRepo, p)
headGitRepo, err := git.OpenRepository(ctx, pr.HeadRepo.RepoPath())
if err != nil {

3
services/convert/pull_test.go
View File

@ -9,6 +9,7 @@ import (
"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/git"
@ -31,7 +32,7 @@ func TestPullRequest_APIFormat(t *testing.T) {
Ref: "refs/pull/2/head",
Sha: "4a357436d925b5c974181ff12a994538ddc5a269",
RepoID: 1,
Repository: ToRepo(db.DefaultContext, headRepo, perm.AccessModeRead),
Repository: ToRepo(db.DefaultContext, headRepo, access_model.Permission{AccessMode: perm.AccessModeRead}),
}, apiPullRequest.Head)
// withOut HeadRepo

30
services/convert/repository.go
View File

@ -9,6 +9,7 @@ import (
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
unit_model "code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/modules/log"
@ -16,18 +17,26 @@ import (
)
// ToRepo converts a Repository to api.Repository
func ToRepo(ctx context.Context, repo *repo_model.Repository, mode perm.AccessMode) *api.Repository {
return innerToRepo(ctx, repo, mode, false)
func ToRepo(ctx context.Context, repo *repo_model.Repository, permissionInRepo access_model.Permission) *api.Repository {
return innerToRepo(ctx, repo, permissionInRepo, false)
}
func innerToRepo(ctx context.Context, repo *repo_model.Repository, mode perm.AccessMode, isParent bool) *api.Repository {
func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInRepo access_model.Permission, isParent bool) *api.Repository {
var parent *api.Repository
if permissionInRepo.Units == nil && permissionInRepo.UnitsMode == nil {
// If Units and UnitsMode are both nil, it means that it's a hard coded permission,
// like access_model.Permission{AccessMode: perm.AccessModeAdmin}.
// So we need to load units for the repo, or UnitAccessMode will always return perm.AccessModeNone.
_ = repo.LoadUnits(ctx) // the error is not important, so ignore it
permissionInRepo.Units = repo.Units
}
cloneLink := repo.CloneLink()
permission := &api.Permission{
Admin: mode >= perm.AccessModeAdmin,
Push: mode >= perm.AccessModeWrite,
Pull: mode >= perm.AccessModeRead,
Admin: permissionInRepo.AccessMode >= perm.AccessModeAdmin,
Push: permissionInRepo.UnitAccessMode(unit_model.TypeCode) >= perm.AccessModeWrite,
Pull: permissionInRepo.UnitAccessMode(unit_model.TypeCode) >= perm.AccessModeRead,
}
if !isParent {
err := repo.GetBaseRepo(ctx)
@ -35,7 +44,12 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, mode perm.Acc
return nil
}
if repo.BaseRepo != nil {
parent = innerToRepo(ctx, repo.BaseRepo, mode, true)
// FIXME: The permission of the parent repo is not correct.
// It's the permission of the current repo, so it's probably different from the parent repo.
// But there isn't a good way to get the permission of the parent repo, because the doer is not passed in.
// Use the permission of the current repo to keep the behavior consistent with the old API.
// Maybe the right way is setting the permission of the parent repo to nil, empty is better than wrong.
parent = innerToRepo(ctx, repo.BaseRepo, permissionInRepo, true)
}
}
@ -154,7 +168,7 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, mode perm.Acc
return &api.Repository{
ID: repo.ID,
Owner: ToUserWithAccessMode(ctx, repo.Owner, mode),
Owner: ToUserWithAccessMode(ctx, repo.Owner, permissionInRepo.AccessMode),
Name: repo.Name,
FullName: repo.FullName(),
Description: repo.Description,