From aa6e2e47d8e7ea32e8828c7d6d9d5ad125d3363e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 14 Jan 2026 16:13:28 -0800 Subject: [PATCH 1/3] Fix bug when push to prs with allow edits from maintainers and lfs usage --- models/fixtures/pull_request.yml | 1 + services/lfs/server.go | 127 ++++++++++++++++++++++++++++--- services/lfs/server_test.go | 18 +++++ 3 files changed, 134 insertions(+), 12 deletions(-) diff --git a/models/fixtures/pull_request.yml b/models/fixtures/pull_request.yml index 9a16316e5a..eccbae945f 100644 --- a/models/fixtures/pull_request.yml +++ b/models/fixtures/pull_request.yml @@ -38,6 +38,7 @@ base_branch: master merge_base: 0abcb056019adb83 has_merged: false + allow_maintainer_edit: true - id: 4 diff --git a/services/lfs/server.go b/services/lfs/server.go index 3455b4b9bd..708569cc62 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -21,6 +21,7 @@ import ( auth_model "code.gitea.io/gitea/models/auth" git_model "code.gitea.io/gitea/models/git" + issues_model "code.gitea.io/gitea/models/issues" perm_model "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" @@ -47,6 +48,66 @@ type requestContext struct { RepoGitURL string } +const ( + lfsRefContextKey = "LFSRefName" + lfsRefQueryKey = "lfs-ref" +) + +func normalizeLFSRefName(raw string) string { + ref := strings.TrimSpace(raw) + if ref == "" { + return "" + } + prefixes := []string{"refs/heads/", "refs/remotes/", "refs/"} + for _, prefix := range prefixes { + if trimmed, ok := strings.CutPrefix(ref, prefix); ok { + ref = trimmed + break + } + } + return ref +} + +func refNameFromBatchRequest(br *lfs_module.BatchRequest) string { + if br == nil || br.Ref == nil { + return "" + } + return normalizeLFSRefName(br.Ref.Name) +} + +func setLFSRefInContext(ctx *context.Context, ref string) { + ref = normalizeLFSRefName(ref) + if ref == "" { + return + } + if ctx.Data == nil { + ctx.Data = make(map[string]any) + } + ctx.Data[lfsRefContextKey] = ref +} + +func getLFSRefFromContext(ctx *context.Context) string { + if ctx.Data == nil { + return "" + } + if ref, ok := ctx.Data[lfsRefContextKey].(string); ok { + return ref + } + return "" +} + +func setLFSRefFromQuery(ctx *context.Context) { + ref := ctx.Req.URL.Query().Get(lfsRefQueryKey) + setLFSRefInContext(ctx, ref) +} + +func appendRefQuery(baseURL, ref string) string { + if ref == "" { + return baseURL + } + return baseURL + "?" + lfsRefQueryKey + "=" + url.QueryEscape(ref) +} + // Claims is a JWT Token Claims type Claims struct { RepoID int64 @@ -88,13 +149,14 @@ func (rc *requestContext) DownloadLink(p lfs_module.Pointer) string { } // UploadLink builds a URL to upload the object. -func (rc *requestContext) UploadLink(p lfs_module.Pointer) string { - return rc.RepoGitURL + "/info/lfs/objects/" + url.PathEscape(p.Oid) + "/" + strconv.FormatInt(p.Size, 10) +func (rc *requestContext) UploadLink(p lfs_module.Pointer, ref string) string { + base := rc.RepoGitURL + "/info/lfs/objects/" + url.PathEscape(p.Oid) + "/" + strconv.FormatInt(p.Size, 10) + return appendRefQuery(base, ref) } // VerifyLink builds a URL for verifying the object. -func (rc *requestContext) VerifyLink(p lfs_module.Pointer) string { - return rc.RepoGitURL + "/info/lfs/verify" +func (rc *requestContext) VerifyLink(p lfs_module.Pointer, ref string) string { + return appendRefQuery(rc.RepoGitURL+"/info/lfs/verify", ref) } // CheckAcceptMediaType checks if the client accepts the LFS media type. @@ -111,6 +173,7 @@ func CheckAcceptMediaType(ctx *context.Context) { var rangeHeaderRegexp = regexp.MustCompile(`bytes=(\d+)-(\d*).*`) // DownloadHandler gets the content from the content store + func DownloadHandler(ctx *context.Context) { rc := getRequestContext(ctx) p := lfs_module.Pointer{Oid: ctx.PathParam("oid")} @@ -207,6 +270,10 @@ func BatchHandler(ctx *context.Context) { } rc := getRequestContext(ctx) + refName := refNameFromBatchRequest(&br) + if refName != "" { + setLFSRefInContext(ctx, refName) + } repository := getAuthenticatedRepository(ctx, rc, isUpload) if repository == nil { @@ -227,7 +294,7 @@ func BatchHandler(ctx *context.Context) { responseObjects = append(responseObjects, buildObjectResponse(rc, p, false, false, &lfs_module.ObjectError{ Code: http.StatusUnprocessableEntity, Message: "Oid or size are invalid", - })) + }, refName)) continue } @@ -249,7 +316,7 @@ func BatchHandler(ctx *context.Context) { responseObjects = append(responseObjects, buildObjectResponse(rc, p, false, false, &lfs_module.ObjectError{ Code: http.StatusUnprocessableEntity, Message: fmt.Sprintf("Object %s is not %d bytes", p.Oid, p.Size), - })) + }, refName)) continue } @@ -282,7 +349,7 @@ func BatchHandler(ctx *context.Context) { } } - responseObject = buildObjectResponse(rc, p, false, !exists, err) + responseObject = buildObjectResponse(rc, p, false, !exists, err, refName) } else { var err *lfs_module.ObjectError if !exists || meta == nil { @@ -292,7 +359,7 @@ func BatchHandler(ctx *context.Context) { } } - responseObject = buildObjectResponse(rc, p, true, false, err) + responseObject = buildObjectResponse(rc, p, true, false, err, refName) } responseObjects = append(responseObjects, responseObject) } @@ -309,6 +376,7 @@ func BatchHandler(ctx *context.Context) { // UploadHandler receives data from the client and puts it into the content store func UploadHandler(ctx *context.Context) { + setLFSRefFromQuery(ctx) rc := getRequestContext(ctx) p := lfs_module.Pointer{Oid: ctx.PathParam("oid")} @@ -394,6 +462,7 @@ func VerifyHandler(ctx *context.Context) { return } + setLFSRefFromQuery(ctx) rc := getRequestContext(ctx) meta := getAuthenticatedMeta(ctx, rc, p, true) @@ -481,7 +550,7 @@ func getAuthenticatedRepository(ctx *context.Context, rc *requestContext, requir return repository } -func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, download, upload bool, err *lfs_module.ObjectError) *lfs_module.ObjectResponse { +func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, download, upload bool, err *lfs_module.ObjectError, ref string) *lfs_module.ObjectResponse { rep := &lfs_module.ObjectResponse{Pointer: pointer} if err != nil { rep.Error = err @@ -512,7 +581,7 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa rep.Actions["download"] = link } if upload { - rep.Actions["upload"] = &lfs_module.Link{Href: rc.UploadLink(pointer), Header: header} + rep.Actions["upload"] = &lfs_module.Link{Href: rc.UploadLink(pointer, ref), Header: header} verifyHeader := make(map[string]string) maps.Copy(verifyHeader, header) @@ -520,7 +589,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.AcceptHeader - rep.Actions["verify"] = &lfs_module.Link{Href: rc.VerifyLink(pointer), Header: verifyHeader} + rep.Actions["verify"] = &lfs_module.Link{Href: rc.VerifyLink(pointer, ref), Header: verifyHeader} } } return rep @@ -542,6 +611,17 @@ func writeStatusMessage(ctx *context.Context, status int, message string) { } } +func maintainerHasLFSAccess(ctx *context.Context, perm access_model.Permission, user *user_model.User) bool { + if user == nil { + return false + } + branch := getLFSRefFromContext(ctx) + if branch == "" { + return false + } + return issues_model.CanMaintainerWriteToBranch(ctx, perm, branch, user) +} + // authenticate uses the authorization string to determine whether // to proceed. This server assumes an HTTP Basic auth format. func authenticate(ctx *context.Context, repository *repo_model.Repository, authorization string, requireSigned, requireWrite bool) bool { @@ -568,6 +648,9 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho } canAccess := perm.CanAccess(accessMode, unit.TypeCode) + if requireWrite && !canAccess && maintainerHasLFSAccess(ctx, perm, ctx.Doer) { + canAccess = true + } // if it doesn't require sign-in and anonymous user has access, return true // if the user is already signed in (for example: by session auth method), and the doer can access, return true if canAccess && (!requireSigned || ctx.IsSigned) { @@ -584,7 +667,27 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho return false } ctx.Doer = user - return true + + if !requireWrite { + return true + } + + perm, err = access_model.GetUserRepoPermission(ctx, repository, ctx.Doer) + if err != nil { + log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", ctx.Doer, repository, err) + return false + } + + canAccess = perm.CanAccess(accessMode, unit.TypeCode) + if !canAccess && maintainerHasLFSAccess(ctx, perm, ctx.Doer) { + canAccess = true + } + if canAccess { + return true + } + + log.Warn("Authentication failure for provided token: insufficient permissions for %s/%s", repository.OwnerName, repository.Name) + return false } func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repository, mode perm_model.AccessMode) (*user_model.User, error) { diff --git a/services/lfs/server_test.go b/services/lfs/server_test.go index e66e48cee8..0e02fc306b 100644 --- a/services/lfs/server_test.go +++ b/services/lfs/server_test.go @@ -8,8 +8,11 @@ import ( "testing" perm_model "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/unit" "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/services/contexttest" "github.com/stretchr/testify/assert" @@ -23,6 +26,7 @@ func TestMain(m *testing.M) { func TestAuthenticate(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + repoFork := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 11}) token2, _ := GetLFSAuthTokenWithBearer(AuthTokenOptions{Op: "download", UserID: 2, RepoID: 1}) _, token2, _ = strings.Cut(token2, " ") @@ -48,4 +52,18 @@ func TestAuthenticate(t *testing.T) { assert.False(t, authenticate(ctx, repo1, prefixBearer+"invalid", true, false)) assert.True(t, authenticate(ctx, repo1, prefixBearer+token2, true, false)) }) + + t.Run("maintainer edits", func(t *testing.T) { + maintainer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 12}) + ctx2, _ := contexttest.MockContext(t, "/") + ctx2.Doer = maintainer + if ctx2.Data == nil { + ctx2.Data = make(map[string]any) + } + ctx2.Data[lfsRefContextKey] = "branch2" + perm, err := access_model.GetUserRepoPermission(ctx2, repoFork, maintainer) + require.NoError(t, err) + require.False(t, perm.CanWrite(unit.TypeCode)) + assert.True(t, authenticate(ctx2, repoFork, "", false, true)) + }) } From ee2ca5d28a8a6b8be838bd1280b4d53d6b20b591 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 16 Jan 2026 23:39:28 -0800 Subject: [PATCH 2/3] improve the functions --- services/lfs/server.go | 106 ++++++++++++++++------------------------- 1 file changed, 41 insertions(+), 65 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index 708569cc62..81a68649c3 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -28,6 +28,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" @@ -53,61 +54,6 @@ const ( lfsRefQueryKey = "lfs-ref" ) -func normalizeLFSRefName(raw string) string { - ref := strings.TrimSpace(raw) - if ref == "" { - return "" - } - prefixes := []string{"refs/heads/", "refs/remotes/", "refs/"} - for _, prefix := range prefixes { - if trimmed, ok := strings.CutPrefix(ref, prefix); ok { - ref = trimmed - break - } - } - return ref -} - -func refNameFromBatchRequest(br *lfs_module.BatchRequest) string { - if br == nil || br.Ref == nil { - return "" - } - return normalizeLFSRefName(br.Ref.Name) -} - -func setLFSRefInContext(ctx *context.Context, ref string) { - ref = normalizeLFSRefName(ref) - if ref == "" { - return - } - if ctx.Data == nil { - ctx.Data = make(map[string]any) - } - ctx.Data[lfsRefContextKey] = ref -} - -func getLFSRefFromContext(ctx *context.Context) string { - if ctx.Data == nil { - return "" - } - if ref, ok := ctx.Data[lfsRefContextKey].(string); ok { - return ref - } - return "" -} - -func setLFSRefFromQuery(ctx *context.Context) { - ref := ctx.Req.URL.Query().Get(lfsRefQueryKey) - setLFSRefInContext(ctx, ref) -} - -func appendRefQuery(baseURL, ref string) string { - if ref == "" { - return baseURL - } - return baseURL + "?" + lfsRefQueryKey + "=" + url.QueryEscape(ref) -} - // Claims is a JWT Token Claims type Claims struct { RepoID int64 @@ -148,6 +94,13 @@ func (rc *requestContext) DownloadLink(p lfs_module.Pointer) string { return rc.RepoGitURL + "/info/lfs/objects/" + url.PathEscape(p.Oid) } +func appendRefQuery(baseURL, ref string) string { + if ref == "" { + return baseURL + } + return baseURL + "?" + lfsRefQueryKey + "=" + url.QueryEscape(ref) +} + // UploadLink builds a URL to upload the object. func (rc *requestContext) UploadLink(p lfs_module.Pointer, ref string) string { base := rc.RepoGitURL + "/info/lfs/objects/" + url.PathEscape(p.Oid) + "/" + strconv.FormatInt(p.Size, 10) @@ -173,7 +126,6 @@ func CheckAcceptMediaType(ctx *context.Context) { var rangeHeaderRegexp = regexp.MustCompile(`bytes=(\d+)-(\d*).*`) // DownloadHandler gets the content from the content store - func DownloadHandler(ctx *context.Context) { rc := getRequestContext(ctx) p := lfs_module.Pointer{Oid: ctx.PathParam("oid")} @@ -248,6 +200,32 @@ func DownloadHandler(ctx *context.Context) { } } +func refNameFromBatchRequest(br *lfs_module.BatchRequest) string { + if br.Ref == nil { + return "" + } + return br.Ref.Name +} + +func setLFSRefInContext(ctx *context.Context, ref string) { + if ref == "" { + return + } + ctx.Data[lfsRefContextKey] = ref +} + +func getLFSRefFromContext(ctx *context.Context) string { + if ref, ok := ctx.Data[lfsRefContextKey].(string); ok { + return ref + } + return "" +} + +func setLFSRefFromQuery(ctx *context.Context) { + ref := ctx.Req.URL.Query().Get(lfsRefQueryKey) + setLFSRefInContext(ctx, ref) +} + // BatchHandler provides the batch api func BatchHandler(ctx *context.Context) { var br lfs_module.BatchRequest @@ -271,9 +249,7 @@ func BatchHandler(ctx *context.Context) { rc := getRequestContext(ctx) refName := refNameFromBatchRequest(&br) - if refName != "" { - setLFSRefInContext(ctx, refName) - } + setLFSRefInContext(ctx, refName) repository := getAuthenticatedRepository(ctx, rc, isUpload) if repository == nil { @@ -611,15 +587,15 @@ func writeStatusMessage(ctx *context.Context, status int, message string) { } } -func maintainerHasLFSAccess(ctx *context.Context, perm access_model.Permission, user *user_model.User) bool { +func canMaintainerWriteLFS(ctx *context.Context, perm access_model.Permission, user *user_model.User) bool { if user == nil { return false } - branch := getLFSRefFromContext(ctx) - if branch == "" { + ref := git.RefName(getLFSRefFromContext(ctx)) + if ref == "" || !ref.IsBranch() { return false } - return issues_model.CanMaintainerWriteToBranch(ctx, perm, branch, user) + return issues_model.CanMaintainerWriteToBranch(ctx, perm, ref.BranchName(), user) } // authenticate uses the authorization string to determine whether @@ -648,7 +624,7 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho } canAccess := perm.CanAccess(accessMode, unit.TypeCode) - if requireWrite && !canAccess && maintainerHasLFSAccess(ctx, perm, ctx.Doer) { + if requireWrite && !canAccess && canMaintainerWriteLFS(ctx, perm, ctx.Doer) { canAccess = true } // if it doesn't require sign-in and anonymous user has access, return true @@ -679,7 +655,7 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho } canAccess = perm.CanAccess(accessMode, unit.TypeCode) - if !canAccess && maintainerHasLFSAccess(ctx, perm, ctx.Doer) { + if !canAccess && canMaintainerWriteLFS(ctx, perm, ctx.Doer) { canAccess = true } if canAccess { From c72075010367e93c82ca256920f1c457c81b2f12 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 27 Jan 2026 17:40:10 -0800 Subject: [PATCH 3/3] improvement --- services/lfs/locks.go | 8 ++--- services/lfs/server.go | 65 +++++++++++++++---------------------- services/lfs/server_test.go | 12 +++---- 3 files changed, 34 insertions(+), 51 deletions(-) diff --git a/services/lfs/locks.go b/services/lfs/locks.go index c2279edaf0..9d656d66e6 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -64,7 +64,7 @@ func GetListLockHandler(ctx *context.Context) { return } - authenticated := authenticate(ctx, repository, rv.Authorization, true, false) + authenticated := authenticate(ctx, repository, rv.Authorization, true, false, "") if !authenticated { ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ @@ -153,7 +153,7 @@ func PostLockHandler(ctx *context.Context) { return } - authenticated := authenticate(ctx, repository, authorization, true, true) + authenticated := authenticate(ctx, repository, authorization, true, true, "") if !authenticated { ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ @@ -218,7 +218,7 @@ func VerifyLockHandler(ctx *context.Context) { return } - authenticated := authenticate(ctx, repository, authorization, true, true) + authenticated := authenticate(ctx, repository, authorization, true, true, "") if !authenticated { ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ @@ -286,7 +286,7 @@ func UnLockHandler(ctx *context.Context) { return } - authenticated := authenticate(ctx, repository, authorization, true, true) + authenticated := authenticate(ctx, repository, authorization, true, true, "") if !authenticated { ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ diff --git a/services/lfs/server.go b/services/lfs/server.go index 81a68649c3..dba5b58617 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -50,8 +50,7 @@ type requestContext struct { } const ( - lfsRefContextKey = "LFSRefName" - lfsRefQueryKey = "lfs-ref" + lfsRefQueryKey = "lfs-ref" ) // Claims is a JWT Token Claims @@ -130,7 +129,7 @@ func DownloadHandler(ctx *context.Context) { rc := getRequestContext(ctx) p := lfs_module.Pointer{Oid: ctx.PathParam("oid")} - meta := getAuthenticatedMeta(ctx, rc, p, false) + meta := getAuthenticatedMeta(ctx, rc, p, false, "") if meta == nil { return } @@ -207,25 +206,6 @@ func refNameFromBatchRequest(br *lfs_module.BatchRequest) string { return br.Ref.Name } -func setLFSRefInContext(ctx *context.Context, ref string) { - if ref == "" { - return - } - ctx.Data[lfsRefContextKey] = ref -} - -func getLFSRefFromContext(ctx *context.Context) string { - if ref, ok := ctx.Data[lfsRefContextKey].(string); ok { - return ref - } - return "" -} - -func setLFSRefFromQuery(ctx *context.Context) { - ref := ctx.Req.URL.Query().Get(lfsRefQueryKey) - setLFSRefInContext(ctx, ref) -} - // BatchHandler provides the batch api func BatchHandler(ctx *context.Context) { var br lfs_module.BatchRequest @@ -249,9 +229,8 @@ func BatchHandler(ctx *context.Context) { rc := getRequestContext(ctx) refName := refNameFromBatchRequest(&br) - setLFSRefInContext(ctx, refName) - repository := getAuthenticatedRepository(ctx, rc, isUpload) + repository := getAuthenticatedRepository(ctx, rc, isUpload, refName) if repository == nil { return } @@ -352,7 +331,7 @@ func BatchHandler(ctx *context.Context) { // UploadHandler receives data from the client and puts it into the content store func UploadHandler(ctx *context.Context) { - setLFSRefFromQuery(ctx) + ref := ctx.Req.URL.Query().Get(lfsRefQueryKey) rc := getRequestContext(ctx) p := lfs_module.Pointer{Oid: ctx.PathParam("oid")} @@ -367,7 +346,7 @@ func UploadHandler(ctx *context.Context) { return } - repository := getAuthenticatedRepository(ctx, rc, true) + repository := getAuthenticatedRepository(ctx, rc, true, ref) if repository == nil { return } @@ -438,10 +417,10 @@ func VerifyHandler(ctx *context.Context) { return } - setLFSRefFromQuery(ctx) + ref := ctx.Req.URL.Query().Get(lfsRefQueryKey) rc := getRequestContext(ctx) - meta := getAuthenticatedMeta(ctx, rc, p, true) + meta := getAuthenticatedMeta(ctx, rc, p, true, ref) if meta == nil { return } @@ -478,14 +457,14 @@ func getRequestContext(ctx *context.Context) *requestContext { } } -func getAuthenticatedMeta(ctx *context.Context, rc *requestContext, p lfs_module.Pointer, requireWrite bool) *git_model.LFSMetaObject { +func getAuthenticatedMeta(ctx *context.Context, rc *requestContext, p lfs_module.Pointer, requireWrite bool, ref string) *git_model.LFSMetaObject { if !p.IsValid() { log.Info("Attempt to access invalid LFS OID[%s] in %s/%s", p.Oid, rc.User, rc.Repo) writeStatusMessage(ctx, http.StatusUnprocessableEntity, "Oid or size are invalid") return nil } - repository := getAuthenticatedRepository(ctx, rc, requireWrite) + repository := getAuthenticatedRepository(ctx, rc, requireWrite, ref) if repository == nil { return nil } @@ -500,7 +479,7 @@ func getAuthenticatedMeta(ctx *context.Context, rc *requestContext, p lfs_module return meta } -func getAuthenticatedRepository(ctx *context.Context, rc *requestContext, requireWrite bool) *repo_model.Repository { +func getAuthenticatedRepository(ctx *context.Context, rc *requestContext, requireWrite bool, ref string) *repo_model.Repository { repository, err := repo_model.GetRepositoryByOwnerAndName(ctx, rc.User, rc.Repo) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", rc.User, rc.Repo, err) @@ -508,7 +487,7 @@ func getAuthenticatedRepository(ctx *context.Context, rc *requestContext, requir return nil } - if !authenticate(ctx, repository, rc.Authorization, false, requireWrite) { + if !authenticate(ctx, repository, rc.Authorization, false, requireWrite, ref) { requireAuth(ctx) return nil } @@ -587,20 +566,28 @@ func writeStatusMessage(ctx *context.Context, status int, message string) { } } -func canMaintainerWriteLFS(ctx *context.Context, perm access_model.Permission, user *user_model.User) bool { +func canMaintainerWriteLFS(ctx *context.Context, perm access_model.Permission, user *user_model.User, ref string) bool { if user == nil { return false } - ref := git.RefName(getLFSRefFromContext(ctx)) - if ref == "" || !ref.IsBranch() { + refName := git.RefName(ref) + if refName == "" { return false } - return issues_model.CanMaintainerWriteToBranch(ctx, perm, ref.BranchName(), user) + + branchName := refName.BranchName() + if branchName == "" { + if strings.HasPrefix(ref, "refs/") { + return false + } + branchName = ref + } + return issues_model.CanMaintainerWriteToBranch(ctx, perm, branchName, user) } // authenticate uses the authorization string to determine whether // to proceed. This server assumes an HTTP Basic auth format. -func authenticate(ctx *context.Context, repository *repo_model.Repository, authorization string, requireSigned, requireWrite bool) bool { +func authenticate(ctx *context.Context, repository *repo_model.Repository, authorization string, requireSigned, requireWrite bool, ref string) bool { accessMode := perm_model.AccessModeRead if requireWrite { accessMode = perm_model.AccessModeWrite @@ -624,7 +611,7 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho } canAccess := perm.CanAccess(accessMode, unit.TypeCode) - if requireWrite && !canAccess && canMaintainerWriteLFS(ctx, perm, ctx.Doer) { + if requireWrite && !canAccess && canMaintainerWriteLFS(ctx, perm, ctx.Doer, ref) { canAccess = true } // if it doesn't require sign-in and anonymous user has access, return true @@ -655,7 +642,7 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho } canAccess = perm.CanAccess(accessMode, unit.TypeCode) - if !canAccess && canMaintainerWriteLFS(ctx, perm, ctx.Doer) { + if !canAccess && canMaintainerWriteLFS(ctx, perm, ctx.Doer, ref) { canAccess = true } if canAccess { diff --git a/services/lfs/server_test.go b/services/lfs/server_test.go index 0e02fc306b..37b795bbe7 100644 --- a/services/lfs/server_test.go +++ b/services/lfs/server_test.go @@ -48,22 +48,18 @@ func TestAuthenticate(t *testing.T) { t.Run("authenticate", func(t *testing.T) { const prefixBearer = "Bearer " - assert.False(t, authenticate(ctx, repo1, "", true, false)) - assert.False(t, authenticate(ctx, repo1, prefixBearer+"invalid", true, false)) - assert.True(t, authenticate(ctx, repo1, prefixBearer+token2, true, false)) + assert.False(t, authenticate(ctx, repo1, "", true, false, "")) + assert.False(t, authenticate(ctx, repo1, prefixBearer+"invalid", true, false, "")) + assert.True(t, authenticate(ctx, repo1, prefixBearer+token2, true, false, "")) }) t.Run("maintainer edits", func(t *testing.T) { maintainer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 12}) ctx2, _ := contexttest.MockContext(t, "/") ctx2.Doer = maintainer - if ctx2.Data == nil { - ctx2.Data = make(map[string]any) - } - ctx2.Data[lfsRefContextKey] = "branch2" perm, err := access_model.GetUserRepoPermission(ctx2, repoFork, maintainer) require.NoError(t, err) require.False(t, perm.CanWrite(unit.TypeCode)) - assert.True(t, authenticate(ctx2, repoFork, "", false, true)) + assert.True(t, authenticate(ctx2, repoFork, "", false, true, "branch2")) }) }