Merge pull request '[gitea] week 2024-34-v7.0 cherry pick (release/v1.22 -> v7.0/forgejo)' (#4999) from earl-warren/wcp/2024-34-v7.0 into v7.0/forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4999
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

models/asymkey/ssh_key.go

@@ -229,35 +229,26 @@ func UpdatePublicKeyUpdated(ctx context.Context, id int64) error {
 
 // PublicKeysAreExternallyManaged returns whether the provided KeyID represents an externally managed Key
 func PublicKeysAreExternallyManaged(ctx context.Context, keys []*PublicKey) ([]bool, error) {
-	sources := make([]*auth.Source, 0, 5)
+	sourceCache := make(map[int64]*auth.Source, len(keys))
 	externals := make([]bool, len(keys))
-keyloop:
+
 	for i, key := range keys {
 		if key.LoginSourceID == 0 {
 			externals[i] = false
-			continue keyloop
+			continue
 		}
 
-		var source *auth.Source
+		source, ok := sourceCache[key.LoginSourceID]
-
+		if !ok {
-	sourceloop:
-		for _, s := range sources {
-			if s.ID == key.LoginSourceID {
-				source = s
-				break sourceloop
-			}
-		}
-
-		if source == nil {
 			var err error
 			source, err = auth.GetSourceByID(ctx, key.LoginSourceID)
 			if err != nil {
 				if auth.IsErrSourceNotExist(err) {
 					externals[i] = false
-					sources[i] = &auth.Source{
+					sourceCache[key.LoginSourceID] = &auth.Source{
 						ID: key.LoginSourceID,
 					}
-					continue keyloop
+					continue
 				}
 				return nil, err
 			}

models/asymkey/ssh_key_test.go

@@ -12,6 +12,8 @@ import (
 	"strings"
 	"testing"
 
+	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/unittest"
 	"code.gitea.io/gitea/modules/setting"
 
 	"github.com/42wim/sshsig"
@@ -501,3 +503,11 @@ func runErr(t *testing.T, stdin []byte, args ...string) {
 		t.Fatal("expected error")
 	}
 }
+
+func Test_PublicKeysAreExternallyManaged(t *testing.T) {
+	key1 := unittest.AssertExistsAndLoadBean(t, &PublicKey{ID: 1})
+	externals, err := PublicKeysAreExternallyManaged(db.DefaultContext, []*PublicKey{key1})
+	require.NoError(t, err)
+	assert.Len(t, externals, 1)
+	assert.False(t, externals[0])
+}

models/git/lfs_lock.go

@@ -21,11 +21,12 @@ import (
 
 // LFSLock represents a git lfs lock of repository.
 type LFSLock struct {
-	ID      int64     `xorm:"pk autoincr"`
+	ID      int64            `xorm:"pk autoincr"`
-	RepoID  int64     `xorm:"INDEX NOT NULL"`
+	RepoID  int64            `xorm:"INDEX NOT NULL"`
-	OwnerID int64     `xorm:"INDEX NOT NULL"`
+	OwnerID int64            `xorm:"INDEX NOT NULL"`
-	Path    string    `xorm:"TEXT"`
+	Owner   *user_model.User `xorm:"-"`
-	Created time.Time `xorm:"created"`
+	Path    string           `xorm:"TEXT"`
+	Created time.Time        `xorm:"created"`
 }
 
 func init() {
@@ -37,6 +38,35 @@ func (l *LFSLock) BeforeInsert() {
 	l.Path = util.PathJoinRel(l.Path)
 }
 
+// LoadAttributes loads attributes of the lock.
+func (l *LFSLock) LoadAttributes(ctx context.Context) error {
+	// Load owner
+	if err := l.LoadOwner(ctx); err != nil {
+		return fmt.Errorf("load owner: %w", err)
+	}
+
+	return nil
+}
+
+// LoadOwner loads owner of the lock.
+func (l *LFSLock) LoadOwner(ctx context.Context) error {
+	if l.Owner != nil {
+		return nil
+	}
+
+	owner, err := user_model.GetUserByID(ctx, l.OwnerID)
+	if err != nil {
+		if user_model.IsErrUserNotExist(err) {
+			l.Owner = user_model.NewGhostUser()
+			return nil
+		}
+		return err
+	}
+	l.Owner = owner
+
+	return nil
+}
+
 // CreateLFSLock creates a new lock.
 func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLock) (*LFSLock, error) {
 	dbCtx, committer, err := db.TxContext(ctx)
@@ -94,7 +124,7 @@ func GetLFSLockByID(ctx context.Context, id int64) (*LFSLock, error) {
 }
 
 // GetLFSLockByRepoID returns a list of locks of repository.
-func GetLFSLockByRepoID(ctx context.Context, repoID int64, page, pageSize int) ([]*LFSLock, error) {
+func GetLFSLockByRepoID(ctx context.Context, repoID int64, page, pageSize int) (LFSLockList, error) {
 	e := db.GetEngine(ctx)
 	if page >= 0 && pageSize > 0 {
 		start := 0
@@ -103,7 +133,7 @@ func GetLFSLockByRepoID(ctx context.Context, repoID int64, page, pageSize int) (
 		}
 		e.Limit(pageSize, start)
 	}
-	lfsLocks := make([]*LFSLock, 0, pageSize)
+	lfsLocks := make(LFSLockList, 0, pageSize)
 	return lfsLocks, e.Find(&lfsLocks, &LFSLock{RepoID: repoID})
 }
 

models/git/lfs_lock_list.go

@@ -0,0 +1,54 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package git
+
+import (
+	"context"
+	"fmt"
+
+	"code.gitea.io/gitea/models/db"
+	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/container"
+)
+
+// LFSLockList is a list of LFSLock
+type LFSLockList []*LFSLock
+
+// LoadAttributes loads the attributes for the given locks
+func (locks LFSLockList) LoadAttributes(ctx context.Context) error {
+	if len(locks) == 0 {
+		return nil
+	}
+
+	if err := locks.LoadOwner(ctx); err != nil {
+		return fmt.Errorf("load owner: %w", err)
+	}
+
+	return nil
+}
+
+// LoadOwner loads the owner of the locks
+func (locks LFSLockList) LoadOwner(ctx context.Context) error {
+	if len(locks) == 0 {
+		return nil
+	}
+
+	usersIDs := container.FilterSlice(locks, func(lock *LFSLock) (int64, bool) {
+		return lock.OwnerID, true
+	})
+	users := make(map[int64]*user_model.User, len(usersIDs))
+	if err := db.GetEngine(ctx).
+		In("id", usersIDs).
+		Find(&users); err != nil {
+		return fmt.Errorf("find users: %w", err)
+	}
+	for _, v := range locks {
+		v.Owner = users[v.OwnerID]
+		if v.Owner == nil { // not exist
+			v.Owner = user_model.NewGhostUser()
+		}
+	}
+
+	return nil
+}

modules/container/filter.go

@@ -0,0 +1,21 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package container
+
+import "slices"
+
+// FilterSlice ranges over the slice and calls include() for each element.
+// If the second returned value is true, the first returned value will be included in the resulting
+// slice (after deduplication).
+func FilterSlice[E any, T comparable](s []E, include func(E) (T, bool)) []T {
+	filtered := make([]T, 0, len(s)) // slice will be clipped before returning
+	seen := make(map[T]bool, len(s))
+	for i := range s {
+		if v, ok := include(s[i]); ok && !seen[v] {
+			filtered = append(filtered, v)
+			seen[v] = true
+		}
+	}
+	return slices.Clip(filtered)
+}

modules/container/filter_test.go

@@ -0,0 +1,28 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package container
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFilterMapUnique(t *testing.T) {
+	result := FilterSlice([]int{
+		0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
+	}, func(i int) (int, bool) {
+		switch i {
+		case 0:
+			return 0, true // included later
+		case 1:
+			return 0, true // duplicate of previous (should be ignored)
+		case 2:
+			return 2, false // not included
+		default:
+			return i, true
+		}
+	})
+	assert.Equal(t, []int{0, 3, 4, 5, 6, 7, 8, 9}, result)
+}

modules/lfs/http_client.go

@@ -211,7 +211,7 @@ func createRequest(ctx context.Context, method, url string, headers map[string]s
 	for key, value := range headers {
 		req.Header.Set(key, value)
 	}
-	req.Header.Set("Accept", MediaType)
+	req.Header.Set("Accept", AcceptHeader)
 
 	return req, nil
 }
@@ -251,6 +251,6 @@ func handleErrorResponse(resp *http.Response) error {
 		return err
 	}
 
-	log.Trace("ErrorResponse: %v", er)
+	log.Trace("ErrorResponse(%v): %v", resp.Status, er)
 	return errors.New(er.Message)
 }

modules/lfs/http_client_test.go

@@ -156,7 +156,7 @@ func TestHTTPClientDownload(t *testing.T) {
 	hc := &http.Client{Transport: RoundTripFunc(func(req *http.Request) *http.Response {
 		assert.Equal(t, "POST", req.Method)
 		assert.Equal(t, MediaType, req.Header.Get("Content-type"))
-		assert.Equal(t, MediaType, req.Header.Get("Accept"))
+		assert.Equal(t, AcceptHeader, req.Header.Get("Accept"))
 
 		var batchRequest BatchRequest
 		err := json.NewDecoder(req.Body).Decode(&batchRequest)
@@ -264,7 +264,7 @@ func TestHTTPClientUpload(t *testing.T) {
 	hc := &http.Client{Transport: RoundTripFunc(func(req *http.Request) *http.Response {
 		assert.Equal(t, "POST", req.Method)
 		assert.Equal(t, MediaType, req.Header.Get("Content-type"))
-		assert.Equal(t, MediaType, req.Header.Get("Accept"))
+		assert.Equal(t, AcceptHeader, req.Header.Get("Accept"))
 
 		var batchRequest BatchRequest
 		err := json.NewDecoder(req.Body).Decode(&batchRequest)

modules/lfs/shared.go

@@ -10,6 +10,8 @@ import (
 const (
 	// MediaType contains the media type for LFS server requests
 	MediaType = "application/vnd.git-lfs+json"
+	// Some LFS servers offer content with other types, so fallback to '*/*' if application/vnd.git-lfs+json cannot be served
+	AcceptHeader = "application/vnd.git-lfs+json;q=0.9, */*;q=0.8"
 )
 
 // BatchRequest contains multiple requests processed in one batch operation.

modules/lfs/transferadapter.go

@@ -37,6 +37,7 @@ func (a *BasicTransferAdapter) Download(ctx context.Context, l *Link) (io.ReadCl
 	if err != nil {
 		return nil, err
 	}
+	log.Debug("Download Request: %+v", req)
 	resp, err := performRequest(ctx, a.client, req)
 	if err != nil {
 		return nil, err

modules/lfs/transferadapter_test.go

@@ -27,7 +27,7 @@ func TestBasicTransferAdapter(t *testing.T) {
 	p := Pointer{Oid: "b5a2c96250612366ea272ffac6d9744aaf4b45aacd96aa7cfcb931ee3b558259", Size: 5}
 
 	roundTripHandler := func(req *http.Request) *http.Response {
-		assert.Equal(t, MediaType, req.Header.Get("Accept"))
+		assert.Equal(t, AcceptHeader, req.Header.Get("Accept"))
 		assert.Equal(t, "test-value", req.Header.Get("test-header"))
 
 		url := req.URL.String()

release-notes/4999.md

@@ -0,0 +1,2 @@
+fix: [commit](https://codeberg.org/forgejo/forgejo/commit/4c5e4e672da27a27dfdaabdc13bcd7b3d8af155b) Show lock owner instead of repo owner on LFS setting page.
+fix: [commit](https://codeberg.org/forgejo/forgejo/commit/22f23c0db00860f29d3a484a4e7fc82a62bfe8ac) Fix panic of ssh public key page after deletion of auth source.

routers/web/repo/setting/lfs.go

@@ -95,6 +95,11 @@ func LFSLocks(ctx *context.Context) {
 		ctx.ServerError("LFSLocks", err)
 		return
 	}
+	if err := lfsLocks.LoadAttributes(ctx); err != nil {
+		ctx.ServerError("LFSLocks", err)
+		return
+	}
+
 	ctx.Data["LFSLocks"] = lfsLocks
 
 	if len(lfsLocks) == 0 {

services/lfs/server.go

@@ -477,7 +477,7 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa
 			}
 
 			// This is only needed to workaround https://github.com/git-lfs/git-lfs/issues/3662
-			verifyHeader["Accept"] = lfs_module.MediaType
+			verifyHeader["Accept"] = lfs_module.AcceptHeader
 
 			rep.Actions["verify"] = &lfs_module.Link{Href: rc.VerifyLink(pointer), Header: verifyHeader}
 		}

templates/repo/settings/lfs_locks.tmpl

@@ -30,9 +30,9 @@
 								{{end}}
 							</td>
 							<td>
-								<a href="{{$.Owner.HomeLink}}">
+								<a href="{{$lock.Owner.HomeLink}}">
-									{{ctx.AvatarUtils.Avatar $.Owner}}
+									{{ctx.AvatarUtils.Avatar $lock.Owner}}
-									{{$.Owner.DisplayName}}
+									{{$lock.Owner.DisplayName}}
 								</a>
 							</td>
 							<td>{{TimeSince .Created ctx.Locale}}</td>

tests/integration/api_repo_lfs_locks_test.go

@@ -105,7 +105,7 @@ func TestAPILFSLocksLogged(t *testing.T) {
 	for _, test := range tests {
 		session := loginUser(t, test.user.Name)
 		req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s.git/info/lfs/locks", test.repo.FullName()), map[string]string{"path": test.path})
-		req.Header.Set("Accept", lfs.MediaType)
+		req.Header.Set("Accept", lfs.AcceptHeader)
 		req.Header.Set("Content-Type", lfs.MediaType)
 		resp := session.MakeRequest(t, req, test.httpResult)
 		if len(test.addTime) > 0 {
@@ -123,7 +123,7 @@ func TestAPILFSLocksLogged(t *testing.T) {
 	for _, test := range resultsTests {
 		session := loginUser(t, test.user.Name)
 		req := NewRequestf(t, "GET", "/%s.git/info/lfs/locks", test.repo.FullName())
-		req.Header.Set("Accept", lfs.MediaType)
+		req.Header.Set("Accept", lfs.AcceptHeader)
 		resp := session.MakeRequest(t, req, http.StatusOK)
 		var lfsLocks api.LFSLockList
 		DecodeJSON(t, resp, &lfsLocks)
@@ -135,7 +135,7 @@ func TestAPILFSLocksLogged(t *testing.T) {
 		}
 
 		req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s.git/info/lfs/locks/verify", test.repo.FullName()), map[string]string{})
-		req.Header.Set("Accept", lfs.MediaType)
+		req.Header.Set("Accept", lfs.AcceptHeader)
 		req.Header.Set("Content-Type", lfs.MediaType)
 		resp = session.MakeRequest(t, req, http.StatusOK)
 		var lfsLocksVerify api.LFSLockListVerify
@@ -159,7 +159,7 @@ func TestAPILFSLocksLogged(t *testing.T) {
 	for _, test := range deleteTests {
 		session := loginUser(t, test.user.Name)
 		req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s.git/info/lfs/locks/%s/unlock", test.repo.FullName(), test.lockID), map[string]string{})
-		req.Header.Set("Accept", lfs.MediaType)
+		req.Header.Set("Accept", lfs.AcceptHeader)
 		req.Header.Set("Content-Type", lfs.MediaType)
 		resp := session.MakeRequest(t, req, http.StatusOK)
 		var lfsLockRep api.LFSLockResponse
@@ -172,7 +172,7 @@ func TestAPILFSLocksLogged(t *testing.T) {
 	for _, test := range resultsTests {
 		session := loginUser(t, test.user.Name)
 		req := NewRequestf(t, "GET", "/%s.git/info/lfs/locks", test.repo.FullName())
-		req.Header.Set("Accept", lfs.MediaType)
+		req.Header.Set("Accept", lfs.AcceptHeader)
 		resp := session.MakeRequest(t, req, http.StatusOK)
 		var lfsLocks api.LFSLockList
 		DecodeJSON(t, resp, &lfsLocks)

tests/integration/api_repo_lfs_test.go

@@ -86,7 +86,7 @@ func TestAPILFSBatch(t *testing.T) {
 
 	newRequest := func(t testing.TB, br *lfs.BatchRequest) *RequestWrapper {
 		return NewRequestWithJSON(t, "POST", "/user2/lfs-batch-repo.git/info/lfs/objects/batch", br).
-			SetHeader("Accept", lfs.MediaType).
+			SetHeader("Accept", lfs.AcceptHeader).
 			SetHeader("Content-Type", lfs.MediaType)
 	}
 	decodeResponse := func(t *testing.T, b *bytes.Buffer) *lfs.BatchResponse {
@@ -449,7 +449,7 @@ func TestAPILFSVerify(t *testing.T) {
 
 	newRequest := func(t testing.TB, p *lfs.Pointer) *RequestWrapper {
 		return NewRequestWithJSON(t, "POST", "/user2/lfs-verify-repo.git/info/lfs/verify", p).
-			SetHeader("Accept", lfs.MediaType).
+			SetHeader("Accept", lfs.AcceptHeader).
 			SetHeader("Content-Type", lfs.MediaType)
 	}
 

tests/integration/lfs_view_test.go

@@ -4,12 +4,21 @@
 package integration
 
 import (
+	"context"
+	"fmt"
 	"net/http"
+	"strings"
 	"testing"
 
+	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/lfs"
+	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // check that files stored in LFS render properly in the web UI
@@ -103,3 +112,56 @@ func TestLFSRender(t *testing.T) {
 		assert.Equal(t, 1, doc.Find(`.sha.label[href="/user2/lfs/commit/73cf03db6ece34e12bf91e8853dc58f678f2f82d"]`).Length(), "could not find link to commit")
 	})
 }
+
+// TestLFSLockView tests the LFS lock view on settings page of repositories
+func TestLFSLockView(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})       // in org 3
+	repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // own by org 3
+	session := loginUser(t, user2.Name)
+
+	// create a lock
+	lockPath := "test_lfs_lock_view.zip"
+	lockID := ""
+	{
+		req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s.git/info/lfs/locks", repo3.FullName()), map[string]string{"path": lockPath})
+		req.Header.Set("Accept", lfs.AcceptHeader)
+		req.Header.Set("Content-Type", lfs.MediaType)
+		resp := session.MakeRequest(t, req, http.StatusCreated)
+		lockResp := &api.LFSLockResponse{}
+		DecodeJSON(t, resp, lockResp)
+		lockID = lockResp.Lock.ID
+	}
+	defer func() {
+		// release the lock
+		req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s.git/info/lfs/locks/%s/unlock", repo3.FullName(), lockID), map[string]string{})
+		req.Header.Set("Accept", lfs.AcceptHeader)
+		req.Header.Set("Content-Type", lfs.MediaType)
+		session.MakeRequest(t, req, http.StatusOK)
+	}()
+
+	t.Run("owner name", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		// make sure the display names are different, or the test is meaningless
+		require.NoError(t, repo3.LoadOwner(context.Background()))
+		require.NotEqual(t, user2.DisplayName(), repo3.Owner.DisplayName())
+
+		req := NewRequest(t, "GET", fmt.Sprintf("/%s/settings/lfs/locks", repo3.FullName()))
+		resp := session.MakeRequest(t, req, http.StatusOK)
+
+		doc := NewHTMLParser(t, resp.Body).doc
+
+		tr := doc.Find("table#lfs-files-locks-table tbody tr")
+		require.Equal(t, 1, tr.Length())
+
+		td := tr.First().Find("td")
+		require.Equal(t, 4, td.Length())
+
+		// path
+		assert.Equal(t, lockPath, strings.TrimSpace(td.Eq(0).Text()))
+		// owner name
+		assert.Equal(t, user2.DisplayName(), strings.TrimSpace(td.Eq(1).Text()))
+	})
+}