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`

Author: wxiaoguang

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

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...)...)
 }

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

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
 	}

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())
 }

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,21 +1594,19 @@ 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() {
 		m.Get("/stars", repo.Stars)
 		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)

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

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)
 	})