From 0aedb03996d7bdce88b1f0086151f8778b10c1a4 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 13 Nov 2024 16:58:09 +0800 Subject: [PATCH] Fix LFS route mock, realm, middleware names (#32488) 1. move "internal-lfs" route mock to "common-lfs" 2. fine tune tests 3. fix "realm" strings, according to RFC: https://datatracker.ietf.org/doc/html/rfc2617: * realm = "realm" "=" realm-value * realm-value = quoted-string 4. clarify some names of the middlewares, rename `ignXxx` to `optXxx` to match `reqXxx`, and rename ambiguous `requireSignIn` to `reqGitSignIn` --- go.mod | 1 + routers/common/lfs.go | 4 +- routers/private/internal.go | 5 +- routers/web/auth/oauth2_provider.go | 4 +- routers/web/githttp.go | 28 +++++----- routers/web/web.go | 73 +++++++++++++-------------- services/context/base.go | 5 ++ services/context/context.go | 3 ++ services/lfs/locks.go | 20 ++++---- services/lfs/server.go | 22 ++++---- tests/integration/git_lfs_ssh_test.go | 30 ++++++----- 11 files changed, 102 insertions(+), 93 deletions(-) diff --git a/go.mod b/go.mod index 60deb90222..bbd8186868 100644 --- a/go.mod +++ b/go.mod @@ -330,6 +330,7 @@ replace github.com/shurcooL/vfsgen => github.com/lunny/vfsgen v0.0.0-20220105142 replace github.com/nektos/act => gitea.com/gitea/act v0.261.3 +// TODO: the only difference is in `PutObject`: the fork doesn't use `NewVerifyingReader(r, sha256.New(), oid, expectedSize)`, need to figure out why replace github.com/charmbracelet/git-lfs-transfer => gitea.com/gitea/git-lfs-transfer v0.2.0 // TODO: This could be removed after https://github.com/mholt/archiver/pull/396 merged diff --git a/routers/common/lfs.go b/routers/common/lfs.go index ba6e1163f1..1d2b71394b 100644 --- a/routers/common/lfs.go +++ b/routers/common/lfs.go @@ -10,6 +10,8 @@ import ( "code.gitea.io/gitea/services/lfs" ) +const RouterMockPointCommonLFS = "common-lfs" + func AddOwnerRepoGitLFSRoutes(m *web.Router, middlewares ...any) { // shared by web and internal routers m.Group("/{username}/{reponame}/info/lfs", func() { @@ -25,5 +27,5 @@ func AddOwnerRepoGitLFSRoutes(m *web.Router, middlewares ...any) { m.Post("/{lid}/unlock", lfs.UnLockHandler) }, lfs.CheckAcceptMediaType) m.Any("/*", http.NotFound) - }, middlewares...) + }, append([]any{web.RouterMockPoint(RouterMockPointCommonLFS)}, middlewares...)...) } diff --git a/routers/private/internal.go b/routers/private/internal.go index db074238c6..1fb72f13d9 100644 --- a/routers/private/internal.go +++ b/routers/private/internal.go @@ -20,8 +20,6 @@ import ( chi_middleware "github.com/go-chi/chi/v5/middleware" ) -const RouterMockPointInternalLFS = "internal-lfs" - func authInternal(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { if setting.InternalToken == "" { @@ -87,10 +85,11 @@ func Routes() *web.Router { r.Group("/repo", func() { // FIXME: it is not right to use context.Contexter here because all routes here should use PrivateContext + // Fortunately, the LFS handlers are able to handle requests without a complete web context common.AddOwnerRepoGitLFSRoutes(r, func(ctx *context.PrivateContext) { webContext := &context.Context{Base: ctx.Base} ctx.AppendContextValue(context.WebContextKey, webContext) - }, web.RouterMockPoint(RouterMockPointInternalLFS)) + }) }) return r diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index 29827b062d..faea34959f 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -91,7 +91,7 @@ type userInfoResponse struct { // InfoOAuth manages request for userinfo endpoint func InfoOAuth(ctx *context.Context) { if ctx.Doer == nil || ctx.Data["AuthedMethod"] != (&auth_service.OAuth2{}).Name() { - ctx.Resp.Header().Set("WWW-Authenticate", `Bearer realm=""`) + ctx.Resp.Header().Set("WWW-Authenticate", `Bearer realm="Gitea OAuth2"`) ctx.PlainText(http.StatusUnauthorized, "no valid authorization") return } @@ -136,7 +136,7 @@ func IntrospectOAuth(ctx *context.Context) { clientIDValid = err == nil && app.ValidateClientSecret([]byte(clientSecret)) } if !clientIDValid { - ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm=""`) + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="Gitea OAuth2"`) ctx.PlainText(http.StatusUnauthorized, "no valid authorization") return } diff --git a/routers/web/githttp.go b/routers/web/githttp.go index 102c92e120..5831e6f523 100644 --- a/routers/web/githttp.go +++ b/routers/web/githttp.go @@ -12,21 +12,19 @@ import ( "code.gitea.io/gitea/services/context" ) -func requireSignIn(ctx *context.Context) { - if !setting.Service.RequireSignInView { - return +func addOwnerRepoGitHTTPRouters(m *web.Router) { + reqGitSignIn := func(ctx *context.Context) { + if !setting.Service.RequireSignInView { + return + } + // rely on the results of Contexter + if !ctx.IsSigned { + // TODO: support digit auth - which would be Authorization header with digit + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="Gitea"`) + ctx.Error(http.StatusUnauthorized) + } } - - // rely on the results of Contexter - if !ctx.IsSigned { - // TODO: support digit auth - which would be Authorization header with digit - ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="Gitea"`) - ctx.Error(http.StatusUnauthorized) - } -} - -func gitHTTPRouters(m *web.Router) { - m.Group("", func() { + m.Group("/{username}/{reponame}", func() { m.Methods("POST,OPTIONS", "/git-upload-pack", repo.ServiceUploadPack) m.Methods("POST,OPTIONS", "/git-receive-pack", repo.ServiceReceivePack) m.Methods("GET,OPTIONS", "/info/refs", repo.GetInfoRefs) @@ -38,5 +36,5 @@ func gitHTTPRouters(m *web.Router) { m.Methods("GET,OPTIONS", "/objects/{head:[0-9a-f]{2}}/{hash:[0-9a-f]{38,62}}", repo.GetLooseObject) m.Methods("GET,OPTIONS", "/objects/pack/pack-{file:[0-9a-f]{40,64}}.pack", repo.GetPackFile) m.Methods("GET,OPTIONS", "/objects/pack/pack-{file:[0-9a-f]{40,64}}.idx", repo.GetIdxFile) - }, ignSignInAndCsrf, requireSignIn, repo.HTTPGitEnabledHandler, repo.CorsHandler(), context.UserAssignmentWeb()) + }, optSignInIgnoreCsrf, reqGitSignIn, repo.HTTPGitEnabledHandler, repo.CorsHandler(), context.UserAssignmentWeb()) } diff --git a/routers/web/web.go b/routers/web/web.go index c56906c10d..137c677306 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -291,15 +291,16 @@ func Routes() *web.Router { return routes } -var ignSignInAndCsrf = verifyAuthWithOptions(&common.VerifyOptions{DisableCSRF: true}) +var optSignInIgnoreCsrf = verifyAuthWithOptions(&common.VerifyOptions{DisableCSRF: true}) // registerRoutes register routes func registerRoutes(m *web.Router) { + // required to be signed in or signed out reqSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: true}) reqSignOut := verifyAuthWithOptions(&common.VerifyOptions{SignOutRequired: true}) - // TODO: rename them to "optSignIn", which means that the "sign-in" could be optional, depends on the VerifyOptions (RequireSignInView) - ignSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: setting.Service.RequireSignInView}) - ignExploreSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: setting.Service.RequireSignInView || setting.Service.Explore.RequireSigninView}) + // optional sign in (if signed in, use the user as doer, if not, no doer) + optSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: setting.Service.RequireSignInView}) + optExploreSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: setting.Service.RequireSignInView || setting.Service.Explore.RequireSigninView}) validation.AddBindingRules() @@ -470,7 +471,7 @@ func registerRoutes(m *web.Router) { // Especially some AJAX requests, we can reduce middleware number to improve performance. m.Get("/", Home) - m.Get("/sitemap.xml", sitemapEnabled, ignExploreSignIn, HomeSitemap) + m.Get("/sitemap.xml", sitemapEnabled, optExploreSignIn, HomeSitemap) m.Group("/.well-known", func() { m.Get("/openid-configuration", auth.OIDCWellKnown) m.Group("", func() { @@ -500,7 +501,7 @@ func registerRoutes(m *web.Router) { } }, explore.Code) m.Get("/topics/search", explore.TopicSearch) - }, ignExploreSignIn) + }, optExploreSignIn) m.Group("/issues", func() { m.Get("", user.Issues) @@ -558,12 +559,12 @@ func registerRoutes(m *web.Router) { m.Post("/grant", web.Bind(forms.GrantApplicationForm{}), auth.GrantApplicationOAuth) // TODO manage redirection m.Post("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth) - }, ignSignInAndCsrf, reqSignIn) + }, optSignInIgnoreCsrf, reqSignIn) - m.Methods("GET, OPTIONS", "/userinfo", optionsCorsHandler(), ignSignInAndCsrf, auth.InfoOAuth) - m.Methods("POST, OPTIONS", "/access_token", optionsCorsHandler(), web.Bind(forms.AccessTokenForm{}), ignSignInAndCsrf, auth.AccessTokenOAuth) - m.Methods("GET, OPTIONS", "/keys", optionsCorsHandler(), ignSignInAndCsrf, auth.OIDCKeys) - m.Methods("POST, OPTIONS", "/introspect", optionsCorsHandler(), web.Bind(forms.IntrospectTokenForm{}), ignSignInAndCsrf, auth.IntrospectOAuth) + m.Methods("GET, OPTIONS", "/userinfo", optionsCorsHandler(), optSignInIgnoreCsrf, auth.InfoOAuth) + m.Methods("POST, OPTIONS", "/access_token", optionsCorsHandler(), web.Bind(forms.AccessTokenForm{}), optSignInIgnoreCsrf, auth.AccessTokenOAuth) + m.Methods("GET, OPTIONS", "/keys", optionsCorsHandler(), optSignInIgnoreCsrf, auth.OIDCKeys) + m.Methods("POST, OPTIONS", "/introspect", optionsCorsHandler(), web.Bind(forms.IntrospectTokenForm{}), optSignInIgnoreCsrf, auth.IntrospectOAuth) }, oauth2Enabled) m.Group("/user/settings", func() { @@ -685,7 +686,7 @@ func registerRoutes(m *web.Router) { m.Post("/forgot_password", auth.ForgotPasswdPost) m.Post("/logout", auth.SignOut) m.Get("/stopwatches", reqSignIn, user.GetStopwatches) - m.Get("/search_candidates", ignExploreSignIn, user.SearchCandidates) + m.Get("/search_candidates", optExploreSignIn, user.SearchCandidates) m.Group("/oauth2", func() { m.Get("/{provider}", auth.SignInOAuth) m.Get("/{provider}/callback", auth.SignInOAuthCallback) @@ -809,7 +810,7 @@ func registerRoutes(m *web.Router) { m.Group("", func() { m.Get("/{username}", user.UsernameSubRoute) m.Methods("GET, OPTIONS", "/attachments/{uuid}", optionsCorsHandler(), repo.GetAttachment) - }, ignSignIn) + }, optSignIn) m.Post("/{username}", reqSignIn, context.UserAssignmentWeb(), user.Action) @@ -860,7 +861,7 @@ func registerRoutes(m *web.Router) { m.Group("/{org}", func() { m.Get("/members", org.Members) }, context.OrgAssignment()) - }, ignSignIn) + }, optSignIn) // end "/org": members m.Group("/org", func() { @@ -1043,14 +1044,14 @@ func registerRoutes(m *web.Router) { m.Group("", func() { m.Get("/code", user.CodeSearch) }, reqUnitAccess(unit.TypeCode, perm.AccessModeRead, false), individualPermsChecker) - }, ignSignIn, context.UserAssignmentWeb(), context.OrgAssignment()) + }, optSignIn, context.UserAssignmentWeb(), context.OrgAssignment()) // end "/{username}/-": packages, projects, code m.Group("/{username}/{reponame}/-", func() { m.Group("/migrate", func() { m.Get("/status", repo.MigrateStatus) }) - }, ignSignIn, context.RepoAssignment, reqRepoCodeReader) + }, optSignIn, context.RepoAssignment, reqRepoCodeReader) // end "/{username}/{reponame}/-": migrate m.Group("/{username}/{reponame}/settings", func() { @@ -1145,10 +1146,10 @@ func registerRoutes(m *web.Router) { // end "/{username}/{reponame}/settings" // user/org home, including rss feeds - m.Get("/{username}/{reponame}", ignSignIn, context.RepoAssignment, context.RepoRef(), repo.SetEditorconfigIfExists, repo.Home) + m.Get("/{username}/{reponame}", optSignIn, context.RepoAssignment, context.RepoRef(), repo.SetEditorconfigIfExists, repo.Home) // TODO: maybe it should relax the permission to allow "any access" - m.Post("/{username}/{reponame}/markup", ignSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypeCode, unit.TypeIssues, unit.TypePullRequests, unit.TypeReleases, unit.TypeWiki), web.Bind(structs.MarkupOption{}), misc.Markup) + m.Post("/{username}/{reponame}/markup", optSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypeCode, unit.TypeIssues, unit.TypePullRequests, unit.TypeReleases, unit.TypeWiki), web.Bind(structs.MarkupOption{}), misc.Markup) m.Group("/{username}/{reponame}", func() { m.Get("/find/*", repo.FindFiles) @@ -1161,7 +1162,7 @@ func registerRoutes(m *web.Router) { m.Combo("/compare/*", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists). Get(repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff). Post(reqSignIn, context.RepoMustNotBeArchived(), reqRepoPullsReader, repo.MustAllowPulls, web.Bind(forms.CreateIssueForm{}), repo.SetWhitespaceBehavior, repo.CompareAndPullRequestPost) - }, ignSignIn, context.RepoAssignment, reqRepoCodeReader) + }, optSignIn, context.RepoAssignment, reqRepoCodeReader) // end "/{username}/{reponame}": find, compare, list (code related) m.Group("/{username}/{reponame}", func() { @@ -1184,7 +1185,7 @@ func registerRoutes(m *web.Router) { }) }, context.RepoRef()) m.Get("/issues/suggestions", repo.IssueSuggestions) - }, ignSignIn, context.RepoAssignment, reqRepoIssuesOrPullsReader) + }, optSignIn, context.RepoAssignment, reqRepoIssuesOrPullsReader) // end "/{username}/{reponame}": view milestone, label, issue, pull, etc m.Group("/{username}/{reponame}", func() { @@ -1194,7 +1195,7 @@ func registerRoutes(m *web.Router) { m.Get("", repo.ViewIssue) }) }) - }, ignSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypeIssues, unit.TypePullRequests, unit.TypeExternalTracker)) + }, optSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypeIssues, unit.TypePullRequests, unit.TypeExternalTracker)) // end "/{username}/{reponame}": issue/pull list, issue/pull view, external tracker m.Group("/{username}/{reponame}", func() { // edit issues, pulls, labels, milestones, etc @@ -1331,7 +1332,7 @@ func registerRoutes(m *web.Router) { repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, context.RepoRefByTypeOptions{IgnoreNotExistErr: true})) m.Post("/tags/delete", repo.DeleteTag, reqSignIn, repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoCodeWriter, context.RepoRef()) - }, ignSignIn, context.RepoAssignment, reqRepoCodeReader) + }, optSignIn, context.RepoAssignment, reqRepoCodeReader) // end "/{username}/{reponame}": repo tags m.Group("/{username}/{reponame}", func() { // repo releases @@ -1356,12 +1357,12 @@ func registerRoutes(m *web.Router) { m.Get("/edit/*", repo.EditRelease) m.Post("/edit/*", web.Bind(forms.EditReleaseForm{}), repo.EditReleasePost) }, reqSignIn, repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoReleaseWriter, repo.CommitInfoCache) - }, ignSignIn, context.RepoAssignment, reqRepoReleaseReader) + }, optSignIn, context.RepoAssignment, reqRepoReleaseReader) // end "/{username}/{reponame}": repo releases m.Group("/{username}/{reponame}", func() { // to maintain compatibility with old attachments m.Get("/attachments/{uuid}", repo.GetAttachment) - }, ignSignIn, context.RepoAssignment) + }, optSignIn, context.RepoAssignment) // end "/{username}/{reponame}": compatibility with old attachments m.Group("/{username}/{reponame}", func() { @@ -1372,7 +1373,7 @@ func registerRoutes(m *web.Router) { if setting.Packages.Enabled { m.Get("/packages", repo.Packages) } - }, ignSignIn, context.RepoAssignment) + }, optSignIn, context.RepoAssignment) m.Group("/{username}/{reponame}/projects", func() { m.Get("", repo.Projects) @@ -1397,7 +1398,7 @@ func registerRoutes(m *web.Router) { }) }) }, reqRepoProjectsWriter, context.RepoMustNotBeArchived()) - }, ignSignIn, context.RepoAssignment, reqRepoProjectsReader, repo.MustEnableRepoProjects) + }, optSignIn, context.RepoAssignment, reqRepoProjectsReader, repo.MustEnableRepoProjects) // end "/{username}/{reponame}/projects" m.Group("/{username}/{reponame}/actions", func() { @@ -1427,7 +1428,7 @@ func registerRoutes(m *web.Router) { m.Group("/workflows/{workflow_name}", func() { m.Get("/badge.svg", actions.GetWorkflowBadge) }) - }, ignSignIn, context.RepoAssignment, reqRepoActionsReader, actions.MustEnableActions) + }, optSignIn, context.RepoAssignment, reqRepoActionsReader, actions.MustEnableActions) // end "/{username}/{reponame}/actions" m.Group("/{username}/{reponame}/wiki", func() { @@ -1440,7 +1441,7 @@ func registerRoutes(m *web.Router) { m.Get("/commit/{sha:[a-f0-9]{7,64}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff) m.Get("/commit/{sha:[a-f0-9]{7,64}}.{ext:patch|diff}", repo.RawDiff) m.Get("/raw/*", repo.WikiRaw) - }, ignSignIn, context.RepoAssignment, repo.MustEnableWiki, reqRepoWikiReader, func(ctx *context.Context) { + }, optSignIn, context.RepoAssignment, repo.MustEnableWiki, reqRepoWikiReader, func(ctx *context.Context) { ctx.Data["PageIsWiki"] = true ctx.Data["CloneButtonOriginLink"] = ctx.Repo.Repository.WikiCloneLink() }) @@ -1462,7 +1463,7 @@ func registerRoutes(m *web.Router) { m.Get("/data", repo.RecentCommitsData) }) }, - ignSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypePullRequests, unit.TypeIssues, unit.TypeReleases), + optSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypePullRequests, unit.TypeIssues, unit.TypeReleases), context.RepoRef(), repo.MustBeNotEmpty, ) // end "/{username}/{reponame}/activity" @@ -1493,7 +1494,7 @@ func registerRoutes(m *web.Router) { }, context.RepoMustNotBeArchived()) }) }) - }, ignSignIn, context.RepoAssignment, repo.MustAllowPulls, reqRepoPullsReader) + }, optSignIn, context.RepoAssignment, repo.MustAllowPulls, reqRepoPullsReader) // end "/{username}/{reponame}/pulls/{index}": repo pull request m.Group("/{username}/{reponame}", func() { @@ -1593,7 +1594,7 @@ func registerRoutes(m *web.Router) { m.Get("/forks", context.RepoRef(), repo.Forks) m.Get("/commit/{sha:([a-f0-9]{7,64})}.{ext:patch|diff}", repo.MustBeNotEmpty, repo.RawDiff) m.Post("/lastcommit/*", context.RepoRefByType(context.RepoRefCommit), repo.LastCommit) - }, ignSignIn, context.RepoAssignment, reqRepoCodeReader) + }, optSignIn, context.RepoAssignment, reqRepoCodeReader) // end "/{username}/{reponame}": repo code m.Group("/{username}/{reponame}", func() { @@ -1601,13 +1602,11 @@ func registerRoutes(m *web.Router) { m.Get("/watchers", repo.Watchers) m.Get("/search", reqRepoCodeReader, repo.Search) m.Post("/action/{action}", reqSignIn, repo.Action) - }, ignSignIn, context.RepoAssignment, context.RepoRef()) + }, optSignIn, context.RepoAssignment, context.RepoRef()) - common.AddOwnerRepoGitLFSRoutes(m, ignSignInAndCsrf, lfsServerEnabled) - m.Group("/{username}/{reponame}", func() { - gitHTTPRouters(m) - }) - // end "/{username}/{reponame}.git": git support + common.AddOwnerRepoGitLFSRoutes(m, optSignInIgnoreCsrf, lfsServerEnabled) // "/{username}/{reponame}/{lfs-paths}": git-lfs support + + addOwnerRepoGitHTTPRouters(m) // "/{username}/{reponame}/{git-paths}": git http support m.Group("/notifications", func() { m.Get("", user.Notifications) diff --git a/services/context/base.go b/services/context/base.go index 68619bf067..d627095584 100644 --- a/services/context/base.go +++ b/services/context/base.go @@ -30,6 +30,10 @@ type contextValuePair struct { valueFn func() any } +type BaseContextKeyType struct{} + +var BaseContextKey BaseContextKeyType + type Base struct { originCtx context.Context contextValues []contextValuePair @@ -315,6 +319,7 @@ func NewBaseContext(resp http.ResponseWriter, req *http.Request) (b *Base, close Data: middleware.GetContextData(req.Context()), } b.Req = b.Req.WithContext(b) + b.AppendContextValue(BaseContextKey, b) b.AppendContextValue(translation.ContextKey, b.Locale) b.AppendContextValue(httplib.RequestContextKey, b.Req) return b, b.cleanUp diff --git a/services/context/context.go b/services/context/context.go index 6c7128ef68..812a8c27ee 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -65,6 +65,9 @@ type Context struct { type TemplateContext map[string]any func init() { + web.RegisterResponseStatusProvider[*Base](func(req *http.Request) web_types.ResponseStatusProvider { + return req.Context().Value(BaseContextKey).(*Base) + }) web.RegisterResponseStatusProvider[*Context](func(req *http.Request) web_types.ResponseStatusProvider { return req.Context().Value(WebContextKey).(*Context) }) diff --git a/services/lfs/locks.go b/services/lfs/locks.go index 4254c99383..1d464f4a66 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -51,7 +51,7 @@ func GetListLockHandler(ctx *context.Context) { repository, err := repo_model.GetRepositoryByOwnerAndName(ctx, rv.User, rv.Repo) if err != nil { log.Debug("Could not find repository: %s/%s - %s", rv.User, rv.Repo, err) - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have pull access to list locks", }) @@ -66,7 +66,7 @@ func GetListLockHandler(ctx *context.Context) { authenticated := authenticate(ctx, repository, rv.Authorization, true, false) if !authenticated { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have pull access to list locks", }) @@ -143,7 +143,7 @@ func PostLockHandler(ctx *context.Context) { repository, err := repo_model.GetRepositoryByOwnerAndName(ctx, userName, repoName) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", userName, repoName, err) - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to create locks", }) @@ -158,7 +158,7 @@ func PostLockHandler(ctx *context.Context) { authenticated := authenticate(ctx, repository, authorization, true, true) if !authenticated { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to create locks", }) @@ -191,7 +191,7 @@ func PostLockHandler(ctx *context.Context) { return } if git_model.IsErrLFSUnauthorizedAction(err) { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to create locks : " + err.Error(), }) @@ -215,7 +215,7 @@ func VerifyLockHandler(ctx *context.Context) { repository, err := repo_model.GetRepositoryByOwnerAndName(ctx, userName, repoName) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", userName, repoName, err) - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to verify locks", }) @@ -230,7 +230,7 @@ func VerifyLockHandler(ctx *context.Context) { authenticated := authenticate(ctx, repository, authorization, true, true) if !authenticated { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to verify locks", }) @@ -286,7 +286,7 @@ func UnLockHandler(ctx *context.Context) { repository, err := repo_model.GetRepositoryByOwnerAndName(ctx, userName, repoName) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", userName, repoName, err) - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to delete locks", }) @@ -301,7 +301,7 @@ func UnLockHandler(ctx *context.Context) { authenticated := authenticate(ctx, repository, authorization, true, true) if !authenticated { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to delete locks", }) @@ -324,7 +324,7 @@ func UnLockHandler(ctx *context.Context) { lock, err := git_model.DeleteLFSLockByID(ctx, ctx.PathParamInt64("lid"), repository, ctx.Doer, req.Force) if err != nil { if git_model.IsErrLFSUnauthorizedAction(err) { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ Message: "You must have push access to delete locks : " + err.Error(), }) diff --git a/services/lfs/server.go b/services/lfs/server.go index f8ef177387..a77623fdc1 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -21,7 +21,7 @@ import ( actions_model "code.gitea.io/gitea/models/actions" auth_model "code.gitea.io/gitea/models/auth" git_model "code.gitea.io/gitea/models/git" - "code.gitea.io/gitea/models/perm" + 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" @@ -77,7 +77,7 @@ func CheckAcceptMediaType(ctx *context.Context) { } } -var rangeHeaderRegexp = regexp.MustCompile(`bytes=(\d+)\-(\d*).*`) +var rangeHeaderRegexp = regexp.MustCompile(`bytes=(\d+)-(\d*).*`) // DownloadHandler gets the content from the content store func DownloadHandler(ctx *context.Context) { @@ -507,11 +507,11 @@ func writeStatusMessage(ctx *context.Context, status int, message string) { } // authenticate uses the authorization string to determine whether -// or not to proceed. This server assumes an HTTP Basic auth format. +// 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 { - accessMode := perm.AccessModeRead + accessMode := perm_model.AccessModeRead if requireWrite { - accessMode = perm.AccessModeWrite + accessMode = perm_model.AccessModeWrite } if ctx.Data["IsActionsToken"] == true { @@ -526,9 +526,9 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho } if task.IsForkPullRequest { - return accessMode <= perm.AccessModeRead + return accessMode <= perm_model.AccessModeRead } - return accessMode <= perm.AccessModeWrite + return accessMode <= perm_model.AccessModeWrite } // ctx.IsSigned is unnecessary here, this will be checked in perm.CanAccess @@ -553,7 +553,7 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho return true } -func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repository, mode perm.AccessMode) (*user_model.User, error) { +func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repository, mode perm_model.AccessMode) (*user_model.User, error) { if !strings.Contains(tokenSHA, ".") { return nil, nil } @@ -576,7 +576,7 @@ func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repo return nil, fmt.Errorf("invalid token claim") } - if mode == perm.AccessModeWrite && claims.Op != "upload" { + if mode == perm_model.AccessModeWrite && claims.Op != "upload" { return nil, fmt.Errorf("invalid token claim") } @@ -588,7 +588,7 @@ func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repo return u, nil } -func parseToken(ctx stdCtx.Context, authorization string, target *repo_model.Repository, mode perm.AccessMode) (*user_model.User, error) { +func parseToken(ctx stdCtx.Context, authorization string, target *repo_model.Repository, mode perm_model.AccessMode) (*user_model.User, error) { if authorization == "" { return nil, fmt.Errorf("no token") } @@ -608,6 +608,6 @@ func parseToken(ctx stdCtx.Context, authorization string, target *repo_model.Rep } func requireAuth(ctx *context.Context) { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) writeStatus(ctx, http.StatusUnauthorized) } diff --git a/tests/integration/git_lfs_ssh_test.go b/tests/integration/git_lfs_ssh_test.go index 33c2fba620..9cb7fd089b 100644 --- a/tests/integration/git_lfs_ssh_test.go +++ b/tests/integration/git_lfs_ssh_test.go @@ -4,14 +4,18 @@ package integration import ( + gocontext "context" "net/url" + "slices" + "strings" "sync" "testing" auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" - "code.gitea.io/gitea/routers/private" + "code.gitea.io/gitea/routers/common" "code.gitea.io/gitea/services/context" "github.com/stretchr/testify/assert" @@ -25,7 +29,7 @@ func TestGitLFSSSH(t *testing.T) { var mu sync.Mutex var routerCalls []string - web.RouteMock(private.RouterMockPointInternalLFS, func(ctx *context.PrivateContext) { + web.RouteMock(common.RouterMockPointCommonLFS, func(ctx *context.Base) { mu.Lock() routerCalls = append(routerCalls, ctx.Req.Method+" "+ctx.Req.URL.Path) mu.Unlock() @@ -42,20 +46,18 @@ func TestGitLFSSSH(t *testing.T) { setting.LFS.AllowPureSSH = true require.NoError(t, cfg.Save()) - // do LFS SSH transfer? + _, _, cmdErr := git.NewCommand(gocontext.Background(), "config", "lfs.sshtransfer", "always").RunStdString(&git.RunOpts{Dir: dstPath}) + assert.NoError(t, cmdErr) lfsCommitAndPushTest(t, dstPath, 10) }) - // FIXME: Here we only see the following calls, but actually there should be calls to "PUT"? - // 0 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" - // 1 = {string} "POST /api/internal/repo/user2/repo1.git/info/lfs/objects/batch" - // 2 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" - // 3 = {string} "POST /api/internal/repo/user2/repo1.git/info/lfs/locks" - // 4 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" - // 5 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" - // 6 = {string} "GET /api/internal/repo/user2/repo1.git/info/lfs/locks" - // 7 = {string} "POST /api/internal/repo/user2/repo1.git/info/lfs/locks/24/unlock" - assert.NotEmpty(t, routerCalls) - // assert.Contains(t, routerCalls, "PUT /api/internal/repo/user2/repo1.git/info/lfs/objects/....") + countBatch := slices.ContainsFunc(routerCalls, func(s string) bool { + return strings.Contains(s, "POST /api/internal/repo/user2/repo1.git/info/lfs/objects/batch") + }) + countUpload := slices.ContainsFunc(routerCalls, func(s string) bool { + return strings.Contains(s, "PUT /user2/repo1.git/info/lfs/objects/") + }) + assert.NotZero(t, countBatch) + assert.NotZero(t, countUpload) }) }