Merge branch 'main' into lunny/allow_agit_force_push

Author: 6543

routers/api/packages/container/blob.go

@@ -10,20 +10,18 @@ import (
 	"fmt"
 	"os"
 	"strings"
-	"sync"
 
 	"code.gitea.io/gitea/models/db"
 	packages_model "code.gitea.io/gitea/models/packages"
 	container_model "code.gitea.io/gitea/models/packages/container"
+	"code.gitea.io/gitea/modules/globallock"
 	"code.gitea.io/gitea/modules/log"
 	packages_module "code.gitea.io/gitea/modules/packages"
 	container_module "code.gitea.io/gitea/modules/packages/container"
 	"code.gitea.io/gitea/modules/util"
 	packages_service "code.gitea.io/gitea/services/packages"
 )
 
-var uploadVersionMutex sync.Mutex
-
 // saveAsPackageBlob creates a package blob from an upload
 // The uploaded blob gets stored in a special upload version to link them to the package/image
 func saveAsPackageBlob(ctx context.Context, hsr packages_module.HashedSizeReader, pci *packages_service.PackageCreationInfo) (*packages_model.PackageBlob, error) { //nolint:unparam
@@ -90,13 +88,20 @@ func mountBlob(ctx context.Context, pi *packages_service.PackageInfo, pb *packag
 	})
 }
 
+func containerPkgName(piOwnerID int64, piName string) string {
+	return fmt.Sprintf("pkg_%d_container_%s", piOwnerID, strings.ToLower(piName))
+}
+
 func getOrCreateUploadVersion(ctx context.Context, pi *packages_service.PackageInfo) (*packages_model.PackageVersion, error) {
 	var uploadVersion *packages_model.PackageVersion
 
-	// FIXME: Replace usage of mutex with database transaction
-	// https://github.com/go-gitea/gitea/pull/21862
-	uploadVersionMutex.Lock()
-	err := db.WithTx(ctx, func(ctx context.Context) error {
+	releaser, err := globallock.Lock(ctx, containerPkgName(pi.Owner.ID, pi.Name))
+	if err != nil {
+		return nil, err
+	}
+	defer releaser()
+
+	err = db.WithTx(ctx, func(ctx context.Context) error {
 		created := true
 		p := &packages_model.Package{
 			OwnerID:   pi.Owner.ID,
@@ -140,7 +145,6 @@ func getOrCreateUploadVersion(ctx context.Context, pi *packages_service.PackageI
 
 		return nil
 	})
-	uploadVersionMutex.Unlock()
 
 	return uploadVersion, err
 }
@@ -173,6 +177,12 @@ func createFileForBlob(ctx context.Context, pv *packages_model.PackageVersion, p
 }
 
 func deleteBlob(ctx context.Context, ownerID int64, image, digest string) error {
+	releaser, err := globallock.Lock(ctx, containerPkgName(ownerID, image))
+	if err != nil {
+		return err
+	}
+	defer releaser()
+
 	return db.WithTx(ctx, func(ctx context.Context) error {
 		pfds, err := container_model.GetContainerBlobs(ctx, &container_model.BlobSearchOptions{
 			OwnerID: ownerID,

routers/web/repo/pull.go

@@ -887,8 +887,6 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
 		}
 
 		if pull.HeadRepo != nil {
-			ctx.Data["SourcePath"] = pull.HeadRepo.Link() + "/src/commit/" + endCommitID
-
 			if !pull.HasMerged && ctx.Doer != nil {
 				perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer)
 				if err != nil {

services/context/permission.go

@@ -58,6 +58,9 @@ func RequireRepoWriterOr(unitTypes ...unit.Type) func(ctx *Context) {
 func RequireRepoReader(unitType unit.Type) func(ctx *Context) {
 	return func(ctx *Context) {
 		if !ctx.Repo.CanRead(unitType) {
+			if unitType == unit.TypeCode && canWriteAsMaintainer(ctx) {
+				return
+			}
 			if log.IsTrace() {
 				if ctx.IsSigned {
 					log.Trace("Permission Denied: User %-v cannot read %-v in Repo %-v\n"+

services/context/repo.go

@@ -374,7 +374,7 @@ func repoAssignment(ctx *Context, repo *repo_model.Repository) {
 		return
 	}
 
-	if !ctx.Repo.Permission.HasAnyUnitAccessOrEveryoneAccess() {
+	if !ctx.Repo.Permission.HasAnyUnitAccessOrEveryoneAccess() && !canWriteAsMaintainer(ctx) {
 		if ctx.FormString("go-get") == "1" {
 			EarlyResponseForGoGetMeta(ctx)
 			return
@@ -1058,3 +1058,11 @@ func GitHookService() func(ctx *Context) {
 		}
 	}
 }
+
+// canWriteAsMaintainer check if the doer can write to a branch as a maintainer
+func canWriteAsMaintainer(ctx *Context) bool {
+	branchName := getRefNameFromPath(ctx.Repo, ctx.PathParam("*"), func(branchName string) bool {
+		return issues_model.CanMaintainerWriteToBranch(ctx, ctx.Repo.Permission, branchName, ctx.Doer)
+	})
+	return len(branchName) > 0
+}

tests/integration/pull_compare_test.go

@@ -14,6 +14,7 @@ import (
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/test"
 	repo_service "code.gitea.io/gitea/services/repository"
 	"code.gitea.io/gitea/tests"
 
@@ -73,3 +74,80 @@ func TestPullCompare(t *testing.T) {
 		assert.EqualValues(t, editButtonCount, 0, "Expected not to find a button to edit a file in the PR diff view because head repository has been deleted")
 	})
 }
+
+func TestPullCompare_EnableAllowEditsFromMaintainer(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		// repo3 is private
+		repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
+		assert.True(t, repo3.IsPrivate)
+
+		// user4 forks repo3
+		user4Session := loginUser(t, "user4")
+		forkedRepoName := "user4-forked-repo3"
+		testRepoFork(t, user4Session, repo3.OwnerName, repo3.Name, "user4", forkedRepoName, "")
+		forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user4", Name: forkedRepoName})
+		assert.True(t, forkedRepo.IsPrivate)
+
+		// user4 creates a new branch and a PR
+		testEditFileToNewBranch(t, user4Session, "user4", forkedRepoName, "master", "user4/update-readme", "README.md", "Hello, World\n(Edited by user4)\n")
+		resp := testPullCreateDirectly(t, user4Session, repo3.OwnerName, repo3.Name, "master", "user4", forkedRepoName, "user4/update-readme", "PR for user4 forked repo3")
+		prURL := test.RedirectURL(resp)
+
+		// user2 (admin of repo3) goes to the PR files page
+		user2Session := loginUser(t, "user2")
+		resp = user2Session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("%s/files", prURL)), http.StatusOK)
+		htmlDoc := NewHTMLParser(t, resp.Body)
+		nodes := htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .dropdown .menu a")
+		if assert.Equal(t, 1, nodes.Length()) {
+			// there is only "View File" button, no "Edit File" button
+			assert.Equal(t, "View File", nodes.First().Text())
+			viewFileLink, exists := nodes.First().Attr("href")
+			if assert.True(t, exists) {
+				user2Session.MakeRequest(t, NewRequest(t, "GET", viewFileLink), http.StatusOK)
+			}
+		}
+
+		// user4 goes to the PR page and enable "Allow maintainers to edit"
+		resp = user4Session.MakeRequest(t, NewRequest(t, "GET", prURL), http.StatusOK)
+		htmlDoc = NewHTMLParser(t, resp.Body)
+		dataURL, exists := htmlDoc.doc.Find("#allow-edits-from-maintainers").Attr("data-url")
+		assert.True(t, exists)
+		req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/set_allow_maintainer_edit", dataURL), map[string]string{
+			"_csrf":                 htmlDoc.GetCSRF(),
+			"allow_maintainer_edit": "true",
+		})
+		user4Session.MakeRequest(t, req, http.StatusOK)
+
+		// user2 (admin of repo3) goes to the PR files page again
+		resp = user2Session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("%s/files", prURL)), http.StatusOK)
+		htmlDoc = NewHTMLParser(t, resp.Body)
+		nodes = htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .dropdown .menu a")
+		if assert.Equal(t, 2, nodes.Length()) {
+			// there are "View File" button and "Edit File" button
+			assert.Equal(t, "View File", nodes.First().Text())
+			viewFileLink, exists := nodes.First().Attr("href")
+			if assert.True(t, exists) {
+				user2Session.MakeRequest(t, NewRequest(t, "GET", viewFileLink), http.StatusOK)
+			}
+
+			assert.Equal(t, "Edit File", nodes.Last().Text())
+			editFileLink, exists := nodes.Last().Attr("href")
+			if assert.True(t, exists) {
+				// edit the file
+				resp := user2Session.MakeRequest(t, NewRequest(t, "GET", editFileLink), http.StatusOK)
+				htmlDoc := NewHTMLParser(t, resp.Body)
+				lastCommit := htmlDoc.GetInputValueByName("last_commit")
+				assert.NotEmpty(t, lastCommit)
+				req := NewRequestWithValues(t, "POST", editFileLink, map[string]string{
+					"_csrf":          htmlDoc.GetCSRF(),
+					"last_commit":    lastCommit,
+					"tree_path":      "README.md",
+					"content":        "File is edited by the maintainer user2",
+					"commit_summary": "user2 updated the file",
+					"commit_choice":  "direct",
+				})
+				user2Session.MakeRequest(t, req, http.StatusSeeOther)
+			}
+		}
+	})
+}