Fix `Permission` in API returned repository struct (#25388) (#25441)

Backport #25388 by @wolfogre

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.

Co-authored-by: Jason Song <i@wolfogre.com>

2 files changed, 37 insertions, 37 deletions

diff --git a/services/actions/notifier.go b/services/actions/notifier.go
index da870bb84c..507eeaacf6 100644
--- a/services/actions/notifier.go
+++ b/services/actions/notifier.go
@@ -45,13 +45,13 @@ func (n *actionsNotifier) NotifyNewIssue(ctx context.Context, issue *issues_mode
 		log.Error("issue.LoadPoster: %v", err)
 		return
 	}
-	mode, _ := access_model.AccessLevel(ctx, issue.Poster, issue.Repo)
+	permission, _ := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster)
 
 	newNotifyInputFromIssue(issue, webhook_module.HookEventIssues).WithPayload(&api.IssuePayload{
 		Action:     api.HookIssueOpened,
 		Index:      issue.Index,
 		Issue:      convert.ToAPIIssue(ctx, issue),
-		Repository: convert.ToRepo(ctx, issue.Repo, mode),
+		Repository: convert.ToRepo(ctx, issue.Repo, permission),
 		Sender:     convert.ToUser(ctx, issue.Poster, nil),
 	}).Notify(withMethod(ctx, "NotifyNewIssue"))
 }
@@ -59,7 +59,7 @@ func (n *actionsNotifier) NotifyNewIssue(ctx context.Context, issue *issues_mode
 // NotifyIssueChangeStatus notifies close or reopen issue to notifiers
 func (n *actionsNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *user_model.User, commitID string, issue *issues_model.Issue, _ *issues_model.Comment, isClosed bool) {
 	ctx = withMethod(ctx, "NotifyIssueChangeStatus")
-	mode, _ := access_model.AccessLevel(ctx, issue.Poster, issue.Repo)
+	permission, _ := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster)
 	if issue.IsPull {
 		if err := issue.LoadPullRequest(ctx); err != nil {
 			log.Error("LoadPullRequest: %v", err)
@@ -69,7 +69,7 @@ func (n *actionsNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *use
 		apiPullRequest := &api.PullRequestPayload{
 			Index:       issue.Index,
 			PullRequest: convert.ToAPIPullRequest(db.DefaultContext, issue.PullRequest, nil),
-			Repository:  convert.ToRepo(ctx, issue.Repo, mode),
+			Repository:  convert.ToRepo(ctx, issue.Repo, permission),
 			Sender:      convert.ToUser(ctx, doer, nil),
 			CommitID:    commitID,
 		}
@@ -88,7 +88,7 @@ func (n *actionsNotifier) NotifyIssueChangeStatus(ctx context.Context, doer *use
 	apiIssue := &api.IssuePayload{
 		Index:      issue.Index,
 		Issue:      convert.ToAPIIssue(ctx, issue),
-		Repository: convert.ToRepo(ctx, issue.Repo, mode),
+		Repository: convert.ToRepo(ctx, issue.Repo, permission),
 		Sender:     convert.ToUser(ctx, doer, nil),
 	}
 	if isClosed {
@@ -118,7 +118,7 @@ func (n *actionsNotifier) NotifyIssueChangeLabels(ctx context.Context, doer *use
 		return
 	}
 
-	mode, _ := access_model.AccessLevel(ctx, issue.Poster, issue.Repo)
+	permission, _ := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster)
 	if issue.IsPull {
 		if err = issue.LoadPullRequest(ctx); err != nil {
 			log.Error("loadPullRequest: %v", err)
@@ -134,7 +134,7 @@ func (n *actionsNotifier) NotifyIssueChangeLabels(ctx context.Context, doer *use
 				Action:      api.HookIssueLabelUpdated,
 				Index:       issue.Index,
 				PullRequest: convert.ToAPIPullRequest(ctx, issue.PullRequest, nil),
-				Repository:  convert.ToRepo(ctx, issue.Repo, perm_model.AccessModeNone),
+				Repository:  convert.ToRepo(ctx, issue.Repo, access_model.Permission{AccessMode: perm_model.AccessModeNone}),
 				Sender:      convert.ToUser(ctx, doer, nil),
 			}).
 			WithPullRequest(issue.PullRequest).
@@ -147,7 +147,7 @@ func (n *actionsNotifier) NotifyIssueChangeLabels(ctx context.Context, doer *use
 			Action:     api.HookIssueLabelUpdated,
 			Index:      issue.Index,
 			Issue:      convert.ToAPIIssue(ctx, issue),
-			Repository: convert.ToRepo(ctx, issue.Repo, mode),
+			Repository: convert.ToRepo(ctx, issue.Repo, permission),
 			Sender:     convert.ToUser(ctx, doer, nil),
 		}).
 		Notify(ctx)
@@ -159,7 +159,7 @@ func (n *actionsNotifier) NotifyCreateIssueComment(ctx context.Context, doer *us
 ) {
 	ctx = withMethod(ctx, "NotifyCreateIssueComment")
 
-	mode, _ := access_model.AccessLevel(ctx, doer, repo)
+	permission, _ := access_model.GetUserRepoPermission(ctx, repo, doer)
 
 	if issue.IsPull {
 		if err := issue.LoadPullRequest(ctx); err != nil {
@@ -172,7 +172,7 @@ func (n *actionsNotifier) NotifyCreateIssueComment(ctx context.Context, doer *us
 				Action:     api.HookIssueCommentCreated,
 				Issue:      convert.ToAPIIssue(ctx, issue),
 				Comment:    convert.ToComment(ctx, comment),
-				Repository: convert.ToRepo(ctx, repo, mode),
+				Repository: convert.ToRepo(ctx, repo, permission),
 				Sender:     convert.ToUser(ctx, doer, nil),
 				IsPull:     true,
 			}).
@@ -186,7 +186,7 @@ func (n *actionsNotifier) NotifyCreateIssueComment(ctx context.Context, doer *us
 			Action:     api.HookIssueCommentCreated,
 			Issue:      convert.ToAPIIssue(ctx, issue),
 			Comment:    convert.ToComment(ctx, comment),
-			Repository: convert.ToRepo(ctx, repo, mode),
+			Repository: convert.ToRepo(ctx, repo, permission),
 			Sender:     convert.ToUser(ctx, doer, nil),
 			IsPull:     false,
 		}).
@@ -209,14 +209,14 @@ func (n *actionsNotifier) NotifyNewPullRequest(ctx context.Context, pull *issues
 		return
 	}
 
-	mode, _ := access_model.AccessLevel(ctx, pull.Issue.Poster, pull.Issue.Repo)
+	permission, _ := access_model.GetUserRepoPermission(ctx, pull.Issue.Repo, pull.Issue.Poster)
 
 	newNotifyInputFromIssue(pull.Issue, webhook_module.HookEventPullRequest).
 		WithPayload(&api.PullRequestPayload{
 			Action:      api.HookIssueOpened,
 			Index:       pull.Issue.Index,
 			PullRequest: convert.ToAPIPullRequest(ctx, pull, nil),
-			Repository:  convert.ToRepo(ctx, pull.Issue.Repo, mode),
+			Repository:  convert.ToRepo(ctx, pull.Issue.Repo, permission),
 			Sender:      convert.ToUser(ctx, pull.Issue.Poster, nil),
 		}).
 		WithPullRequest(pull).
@@ -228,7 +228,7 @@ func (n *actionsNotifier) NotifyCreateRepository(ctx context.Context, doer, u *u
 
 	newNotifyInput(repo, doer, webhook_module.HookEventRepository).WithPayload(&api.RepositoryPayload{
 		Action:       api.HookRepoCreated,
-		Repository:   convert.ToRepo(ctx, repo, perm_model.AccessModeOwner),
+		Repository:   convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeOwner}),
 		Organization: convert.ToUser(ctx, u, nil),
 		Sender:       convert.ToUser(ctx, doer, nil),
 	}).Notify(ctx)
@@ -237,13 +237,13 @@ func (n *actionsNotifier) NotifyCreateRepository(ctx context.Context, doer, u *u
 func (n *actionsNotifier) NotifyForkRepository(ctx context.Context, doer *user_model.User, oldRepo, repo *repo_model.Repository) {
 	ctx = withMethod(ctx, "NotifyForkRepository")
 
-	oldMode, _ := access_model.AccessLevel(ctx, doer, oldRepo)
-	mode, _ := access_model.AccessLevel(ctx, doer, repo)
+	oldPermission, _ := access_model.GetUserRepoPermission(ctx, oldRepo, doer)
+	permission, _ := access_model.GetUserRepoPermission(ctx, repo, doer)
 
 	// forked webhook
 	newNotifyInput(oldRepo, doer, webhook_module.HookEventFork).WithPayload(&api.ForkPayload{
-		Forkee: convert.ToRepo(ctx, oldRepo, oldMode),
-		Repo:   convert.ToRepo(ctx, repo, mode),
+		Forkee: convert.ToRepo(ctx, oldRepo, oldPermission),
+		Repo:   convert.ToRepo(ctx, repo, permission),
 		Sender: convert.ToUser(ctx, doer, nil),
 	}).Notify(ctx)
 
@@ -255,7 +255,7 @@ func (n *actionsNotifier) NotifyForkRepository(ctx context.Context, doer *user_m
 			WithRef(oldRepo.DefaultBranch).
 			WithPayload(&api.RepositoryPayload{
 				Action:       api.HookRepoCreated,
-				Repository:   convert.ToRepo(ctx, repo, perm_model.AccessModeOwner),
+				Repository:   convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeOwner}),
 				Organization: convert.ToUser(ctx, u, nil),
 				Sender:       convert.ToUser(ctx, doer, nil),
 			}).Notify(ctx)
@@ -285,9 +285,9 @@ func (n *actionsNotifier) NotifyPullRequestReview(ctx context.Context, pr *issue
 		return
 	}
 
-	mode, err := access_model.AccessLevel(ctx, review.Issue.Poster, review.Issue.Repo)
+	permission, err := access_model.GetUserRepoPermission(ctx, review.Issue.Repo, review.Issue.Poster)
 	if err != nil {
-		log.Error("models.AccessLevel: %v", err)
+		log.Error("models.GetUserRepoPermission: %v", err)
 		return
 	}
 
@@ -297,7 +297,7 @@ func (n *actionsNotifier) NotifyPullRequestReview(ctx context.Context, pr *issue
 			Action:      api.HookIssueReviewed,
 			Index:       review.Issue.Index,
 			PullRequest: convert.ToAPIPullRequest(db.DefaultContext, pr, nil),
-			Repository:  convert.ToRepo(ctx, review.Issue.Repo, mode),
+			Repository:  convert.ToRepo(ctx, review.Issue.Repo, permission),
 			Sender:      convert.ToUser(ctx, review.Reviewer, nil),
 			Review: &api.ReviewPayload{
 				Type:    string(reviewHookType),
@@ -325,9 +325,9 @@ func (*actionsNotifier) NotifyMergePullRequest(ctx context.Context, doer *user_m
 		return
 	}
 
-	mode, err := access_model.AccessLevel(ctx, doer, pr.Issue.Repo)
+	permission, err := access_model.GetUserRepoPermission(ctx, pr.Issue.Repo, doer)
 	if err != nil {
-		log.Error("models.AccessLevel: %v", err)
+		log.Error("models.GetUserRepoPermission: %v", err)
 		return
 	}
 
@@ -335,7 +335,7 @@ func (*actionsNotifier) NotifyMergePullRequest(ctx context.Context, doer *user_m
 	apiPullRequest := &api.PullRequestPayload{
 		Index:       pr.Issue.Index,
 		PullRequest: convert.ToAPIPullRequest(db.DefaultContext, pr, nil),
-		Repository:  convert.ToRepo(ctx, pr.Issue.Repo, mode),
+		Repository:  convert.ToRepo(ctx, pr.Issue.Repo, permission),
 		Sender:      convert.ToUser(ctx, doer, nil),
 		Action:      api.HookIssueClosed,
 	}
@@ -366,7 +366,7 @@ func (n *actionsNotifier) NotifyPushCommits(ctx context.Context, pusher *user_mo
 			CompareURL: setting.AppURL + commits.CompareURL,
 			Commits:    apiCommits,
 			HeadCommit: apiHeadCommit,
-			Repo:       convert.ToRepo(ctx, repo, perm_model.AccessModeOwner),
+			Repo:       convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeOwner}),
 			Pusher:     apiPusher,
 			Sender:     apiPusher,
 		}).
@@ -377,7 +377,7 @@ func (n *actionsNotifier) NotifyCreateRef(ctx context.Context, pusher *user_mode
 	ctx = withMethod(ctx, "NotifyCreateRef")
 
 	apiPusher := convert.ToUser(ctx, pusher, nil)
-	apiRepo := convert.ToRepo(ctx, repo, perm_model.AccessModeNone)
+	apiRepo := convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeNone})
 
 	newNotifyInput(repo, pusher, webhook_module.HookEventCreate).
 		WithRef(refFullName.ShortName()). // FIXME: should we use a full ref name
@@ -395,7 +395,7 @@ func (n *actionsNotifier) NotifyDeleteRef(ctx context.Context, pusher *user_mode
 	ctx = withMethod(ctx, "NotifyDeleteRef")
 
 	apiPusher := convert.ToUser(ctx, pusher, nil)
-	apiRepo := convert.ToRepo(ctx, repo, perm_model.AccessModeNone)
+	apiRepo := convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeNone})
 
 	newNotifyInput(repo, pusher, webhook_module.HookEventDelete).
 		WithRef(refFullName.ShortName()). // FIXME: should we use a full ref name
@@ -429,7 +429,7 @@ func (n *actionsNotifier) NotifySyncPushCommits(ctx context.Context, pusher *use
 			Commits:      apiCommits,
 			TotalCommits: commits.Len,
 			HeadCommit:   apiHeadCommit,
-			Repo:         convert.ToRepo(ctx, repo, perm_model.AccessModeOwner),
+			Repo:         convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeOwner}),
 			Pusher:       apiPusher,
 			Sender:       apiPusher,
 		}).
@@ -494,7 +494,7 @@ func (n *actionsNotifier) NotifyPullRequestSynchronized(ctx context.Context, doe
 			Action:      api.HookIssueSynchronized,
 			Index:       pr.Issue.Index,
 			PullRequest: convert.ToAPIPullRequest(ctx, pr, nil),
-			Repository:  convert.ToRepo(ctx, pr.Issue.Repo, perm_model.AccessModeNone),
+			Repository:  convert.ToRepo(ctx, pr.Issue.Repo, access_model.Permission{AccessMode: perm_model.AccessModeNone}),
 			Sender:      convert.ToUser(ctx, doer, nil),
 		}).
 		WithPullRequest(pr).
@@ -514,7 +514,7 @@ func (n *actionsNotifier) NotifyPullRequestChangeTargetBranch(ctx context.Contex
 		return
 	}
 
-	mode, _ := access_model.AccessLevel(ctx, pr.Issue.Poster, pr.Issue.Repo)
+	permission, _ := access_model.GetUserRepoPermission(ctx, pr.Issue.Repo, pr.Issue.Poster)
 	newNotifyInput(pr.Issue.Repo, doer, webhook_module.HookEventPullRequest).
 		WithPayload(&api.PullRequestPayload{
 			Action: api.HookIssueEdited,
@@ -525,7 +525,7 @@ func (n *actionsNotifier) NotifyPullRequestChangeTargetBranch(ctx context.Contex
 				},
 			},
 			PullRequest: convert.ToAPIPullRequest(ctx, pr, nil),
-			Repository:  convert.ToRepo(ctx, pr.Issue.Repo, mode),
+			Repository:  convert.ToRepo(ctx, pr.Issue.Repo, permission),
 			Sender:      convert.ToUser(ctx, doer, nil),
 		}).
 		WithPullRequest(pr).
@@ -537,7 +537,7 @@ func (n *actionsNotifier) NotifyNewWikiPage(ctx context.Context, doer *user_mode
 
 	newNotifyInput(repo, doer, webhook_module.HookEventWiki).WithPayload(&api.WikiPayload{
 		Action:     api.HookWikiCreated,
-		Repository: convert.ToRepo(ctx, repo, perm_model.AccessModeOwner),
+		Repository: convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeOwner}),
 		Sender:     convert.ToUser(ctx, doer, nil),
 		Page:       page,
 		Comment:    comment,
@@ -549,7 +549,7 @@ func (n *actionsNotifier) NotifyEditWikiPage(ctx context.Context, doer *user_mod
 
 	newNotifyInput(repo, doer, webhook_module.HookEventWiki).WithPayload(&api.WikiPayload{
 		Action:     api.HookWikiEdited,
-		Repository: convert.ToRepo(ctx, repo, perm_model.AccessModeOwner),
+		Repository: convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeOwner}),
 		Sender:     convert.ToUser(ctx, doer, nil),
 		Page:       page,
 		Comment:    comment,
@@ -561,7 +561,7 @@ func (n *actionsNotifier) NotifyDeleteWikiPage(ctx context.Context, doer *user_m
 
 	newNotifyInput(repo, doer, webhook_module.HookEventWiki).WithPayload(&api.WikiPayload{
 		Action:     api.HookWikiDeleted,
-		Repository: convert.ToRepo(ctx, repo, perm_model.AccessModeOwner),
+		Repository: convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm_model.AccessModeOwner}),
 		Sender:     convert.ToUser(ctx, doer, nil),
 		Page:       page,
 	}).Notify(ctx)
diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go
index 5e41241d18..69c23656f2 100644
--- a/services/actions/notifier_helper.go
+++ b/services/actions/notifier_helper.go
@@ -222,14 +222,14 @@ func notifyRelease(ctx context.Context, doer *user_model.User, rel *repo_model.R
 		return
 	}
 
-	mode, _ := access_model.AccessLevel(ctx, doer, rel.Repo)
+	permission, _ := access_model.GetUserRepoPermission(ctx, rel.Repo, doer)
 
 	newNotifyInput(rel.Repo, doer, webhook_module.HookEventRelease).
 		WithRef(git.RefNameFromTag(rel.TagName).String()).
 		WithPayload(&api.ReleasePayload{
 			Action:     action,
 			Release:    convert.ToRelease(ctx, rel),
-			Repository: convert.ToRepo(ctx, rel.Repo, mode),
+			Repository: convert.ToRepo(ctx, rel.Repo, permission),
 			Sender:     convert.ToUser(ctx, doer, nil),
 		}).
 		Notify(ctx)