Merge d8490031b2bd50a57277abf30f98e1d383f2418c into a5d81d9ce230aaa6e1021b6236ca01cb6d2b56c3

routers/private/serv.go

@@ -333,8 +333,9 @@ func ServCommand(ctx *context.PrivateContext) {
 			// Because of the special ref "refs/for" (AGit) we will need to delay write permission check,
 			// AGit flow needs to write its own ref when the doer has "reader" permission (allowing to create PR).
 			// The real permission check is done in HookPreReceive (routers/private/hook_pre_receive.go).
-			// Here it should relax the permission check for "git push (git-receive-pack)", but not for others like LFS operations.
-			if git.DefaultFeatures().SupportProcReceive && unitType == unit.TypeCode && verb == git.CmdVerbReceivePack {
+			// Here it should relax the permission check for "git push (git-receive-pack)" and LFS upload operations.
+			if git.DefaultFeatures().SupportProcReceive && unitType == unit.TypeCode &&
+				(verb == git.CmdVerbReceivePack || verb == git.CmdVerbLfsAuthenticate || verb == git.CmdVerbLfsTransfer) {
 				mode = perm.AccessModeRead
 			}
 

services/lfs/server.go

@@ -26,6 +26,7 @@ import (
 	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/auth/httpauth"
+	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/httplib"
 	"code.gitea.io/gitea/modules/json"
 	lfs_module "code.gitea.io/gitea/modules/lfs"
@@ -460,12 +461,19 @@ func getAuthenticatedRepository(ctx *context.Context, rc *requestContext, requir
 		return nil
 	}
 
-	if !authenticate(ctx, repository, rc.Authorization, false, requireWrite) {
+	// Because of special ref "refs/for" (AGit), we need to delay/relax write permission check
+	// for LFS object uploads. LFS locks still require full write access.
+	lfsWriteMode := requireWrite
+	if lfsWriteMode && git.DefaultFeatures().SupportProcReceive {
+		lfsWriteMode = false
+	}
+
+	if !authenticate(ctx, repository, rc.Authorization, false, lfsWriteMode) {
 		requireAuth(ctx)
 		return nil
 	}
 
-	if requireWrite {
+	if lfsWriteMode {
 		context.CheckRepoScopedToken(ctx, repository, auth_model.Write)
 	} else {
 		context.CheckRepoScopedToken(ctx, repository, auth_model.Read)

tests/integration/agit_lfs_test.go

@@ -0,0 +1,119 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package integration
+
+import (
+	"net/http"
+	"net/url"
+	"os"
+	"path/filepath"
+	"testing"
+
+	auth_model "code.gitea.io/gitea/models/auth"
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unittest"
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/git/gitcmd"
+	"code.gitea.io/gitea/modules/setting"
+	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/tests"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func testAPISaveUserPublicKey(t *testing.T, session *TestSession, username, keyname, content string) {
+	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteUser)
+	req := NewRequestWithJSON(t, "POST", "/api/v1/user/keys", &api.CreateKeyOption{
+		Title: keyname,
+		Key:   content,
+	}).AddTokenAuth(token)
+	MakeRequest(t, req, http.StatusCreated)
+}
+
+func TestAgitLFS(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		// Enable LFS
+		defer tests.PrepareTestEnv(t)()
+		setting.LFS.StartServer = true
+		setting.LFS.Storage.Path = filepath.Join(setting.AppDataPath, "lfs")
+
+		t.Run("HTTP", func(t *testing.T) {
+			dstPath := t.TempDir()
+
+			// user4 has read access to repo1 (owned by user2)
+			u.Path = "user2/repo1.git"
+			u.User = url.UserPassword("user4", userPassword)
+
+			doGitClone(dstPath, u)(t)
+
+			// Setup LFS in the repo
+			_, _, err := gitcmd.NewCommand("lfs", "install").WithDir(dstPath).RunStdString(t.Context())
+			assert.NoError(t, err)
+
+			_, _, err = gitcmd.NewCommand("lfs", "track", "*.bin").WithDir(dstPath).RunStdString(t.Context())
+			assert.NoError(t, err)
+
+			assert.NoError(t, os.WriteFile(filepath.Join(dstPath, "large.bin"), []byte("this is a large file"), 0o644))
+			assert.NoError(t, git.AddChanges(t.Context(), dstPath, true))
+
+			signature := git.Signature{
+				Email: "user4@example.com",
+				Name:  "user4",
+			}
+			assert.NoError(t, git.CommitChanges(t.Context(), dstPath, git.CommitChangesOptions{
+				Committer: &signature,
+				Author:    &signature,
+				Message:   "Add LFS file",
+			}))
+
+			// push to create an agit pull request
+			assert.NoError(t, gitcmd.NewCommand("push", "origin", "HEAD:refs/for/master/test-agit-lfs-http").
+				WithDir(dstPath).
+				Run(t.Context()))
+		})
+
+		t.Run("SSH", func(t *testing.T) {
+			dstPath := t.TempDir()
+
+			// user4 has read access to repo1 (owned by user2)
+			repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"})
+			sshURL := createSSHUrl(repo.FullName()+".git", u)
+
+			withKeyFile(t, "id_rsa", func(keyFile string) {
+				t.Run("AddKey", func(t *testing.T) {
+					session := loginUser(t, "user4")
+					content, _ := os.ReadFile(keyFile + ".pub")
+					testAPISaveUserPublicKey(t, session, "user4", "user4-agit-lfs", string(content))
+				})
+
+				doGitClone(dstPath, sshURL)(t)
+
+				// Setup LFS in the repo
+				_, _, err := gitcmd.NewCommand("lfs", "install").WithDir(dstPath).RunStdString(t.Context())
+				assert.NoError(t, err)
+
+				_, _, err = gitcmd.NewCommand("lfs", "track", "*.bin").WithDir(dstPath).RunStdString(t.Context())
+				assert.NoError(t, err)
+
+				assert.NoError(t, os.WriteFile(filepath.Join(dstPath, "large-ssh.bin"), []byte("this is a large file via ssh"), 0o644))
+				assert.NoError(t, git.AddChanges(t.Context(), dstPath, true))
+
+				signature := git.Signature{
+					Email: "user4@example.com",
+					Name:  "user4",
+				}
+				assert.NoError(t, git.CommitChanges(t.Context(), dstPath, git.CommitChangesOptions{
+					Committer: &signature,
+					Author:    &signature,
+					Message:   "Add LFS file via SSH",
+				}))
+
+				// push to create an agit pull request
+				assert.NoError(t, gitcmd.NewCommand("push", "origin", "HEAD:refs/for/master/test-agit-lfs-ssh").
+					WithDir(dstPath).
+					Run(t.Context()))
+			})
+		})
+	})
+}

tests/integration/git_general_test.go

@@ -166,13 +166,18 @@ func doSSHLFSAccessTest(_ APITestContext, keyID int64) func(*testing.T) {
 		t.Run("User2AccessOther", func(t *testing.T) {
 			sshCmdUser2Other := append(slices.Clone(sshCmdParts),
 				"-p", strconv.Itoa(setting.SSH.ListenPort), "git@"+setting.SSH.ListenHost,
-				"git-lfs-authenticate", "user5/repo4.git", "upload", // inaccessible to other's (user5/repo4)
+				"git-lfs-authenticate", "user5/repo4.git", "upload", // accessible to other's (user5/repo4) if AGit is supported
 			)
 			cmd := exec.CommandContext(t.Context(), sshCmdUser2Other[0], sshCmdUser2Other[1:]...)
 			_, err := cmd.Output()
-			var errExit *exec.ExitError
-			require.ErrorAs(t, err, &errExit) // inaccessible, error
-			assert.Contains(t, string(errExit.Stderr), fmt.Sprintf("User: 2:user2 with Key: %d:test-key is not authorized to write to user5/repo4.", keyID))
+
+			if git.DefaultFeatures().SupportProcReceive {
+				assert.NoError(t, err) // relaxed for AGit
+			} else {
+				var errExit *exec.ExitError
+				require.ErrorAs(t, err, &errExit) // inaccessible, error
+				assert.Contains(t, string(errExit.Stderr), fmt.Sprintf("User: 2:user2 with Key: %d:test-key is not authorized to write to user5/repo4.", keyID))
+			}
 		})
 	}
 }