From a51cc6dea41b154b946e982fde6cc1a600210a71 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 28 Jan 2021 01:46:35 +0800 Subject: [PATCH] Fix access log (#14475) Fix #14121, #14478. The `AccessLog` middleware has to be after `Contexter` or `APIContexter` so that we can get `LoginUserName` if possible. And also there is a **BREAK** change that it removed internal API access log. --- modules/auth/sso/oauth2.go | 3 +- modules/auth/sso/sso.go | 10 ------ modules/auth/sso/sspi_windows.go | 7 ++-- modules/context/access_log.go | 60 ++++++++++++++++++++++++++++++++ modules/context/context.go | 25 +++++++++++++ modules/context/response.go | 10 +++++- modules/middlewares/request.go | 20 +++++++++++ routers/api/v1/api.go | 5 +++ routers/routes/base.go | 53 ---------------------------- routers/routes/web.go | 8 ++--- 10 files changed, 129 insertions(+), 72 deletions(-) create mode 100644 modules/context/access_log.go create mode 100644 modules/middlewares/request.go diff --git a/modules/auth/sso/oauth2.go b/modules/auth/sso/oauth2.go index c3f6f08fb..b6f59dc92 100644 --- a/modules/auth/sso/oauth2.go +++ b/modules/auth/sso/oauth2.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/middlewares" "code.gitea.io/gitea/modules/timeutil" ) @@ -121,7 +122,7 @@ func (o *OAuth2) VerifyAuthData(req *http.Request, w http.ResponseWriter, store return nil } - if isInternalPath(req) || !isAPIPath(req) && !isAttachmentDownload(req) { + if middlewares.IsInternalPath(req) || !middlewares.IsAPIPath(req) && !isAttachmentDownload(req) { return nil } diff --git a/modules/auth/sso/sso.go b/modules/auth/sso/sso.go index d54310168..f3788e4c9 100644 --- a/modules/auth/sso/sso.go +++ b/modules/auth/sso/sso.go @@ -94,16 +94,6 @@ func SessionUser(sess SessionStore) *models.User { return user } -// isAPIPath returns true if the specified URL is an API path -func isAPIPath(req *http.Request) bool { - return strings.HasPrefix(req.URL.Path, "/api/") -} - -// isInternalPath returns true if the specified URL is an internal API path -func isInternalPath(req *http.Request) bool { - return strings.HasPrefix(req.URL.Path, "/api/internal/") -} - // isAttachmentDownload check if request is a file download (GET) with URL to an attachment func isAttachmentDownload(req *http.Request) bool { return strings.HasPrefix(req.URL.Path, "/attachments/") && req.Method == "GET" diff --git a/modules/auth/sso/sspi_windows.go b/modules/auth/sso/sspi_windows.go index 448336c07..10571d67c 100644 --- a/modules/auth/sso/sspi_windows.go +++ b/modules/auth/sso/sspi_windows.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/middlewares" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" @@ -135,7 +136,7 @@ func (s *SSPI) VerifyAuthData(req *http.Request, w http.ResponseWriter, store Da } // Make sure requests to API paths and PWA resources do not create a new session - if !isAPIPath(req) && !isAttachmentDownload(req) { + if !middlewares.IsAPIPath(req) && !isAttachmentDownload(req) { handleSignIn(w, req, sess, user) } @@ -166,9 +167,9 @@ func (s *SSPI) shouldAuthenticate(req *http.Request) (shouldAuth bool) { } else if req.FormValue("auth_with_sspi") == "1" { shouldAuth = true } - } else if isInternalPath(req) { + } else if middlewares.IsInternalPath(req) { shouldAuth = false - } else if isAPIPath(req) || isAttachmentDownload(req) { + } else if middlewares.IsAPIPath(req) || isAttachmentDownload(req) { shouldAuth = true } return diff --git a/modules/context/access_log.go b/modules/context/access_log.go new file mode 100644 index 000000000..97bb32f4c --- /dev/null +++ b/modules/context/access_log.go @@ -0,0 +1,60 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package context + +import ( + "bytes" + "html/template" + "net/http" + "time" + + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" +) + +type routerLoggerOptions struct { + req *http.Request + Identity *string + Start *time.Time + ResponseWriter http.ResponseWriter + Ctx map[string]interface{} +} + +// AccessLogger returns a middleware to log access logger +func AccessLogger() func(http.Handler) http.Handler { + logger := log.GetLogger("access") + logTemplate, _ := template.New("log").Parse(setting.AccessLogTemplate) + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + start := time.Now() + next.ServeHTTP(w, req) + identity := "-" + if val := SignedUserName(req); val != "" { + identity = val + } + rw := w.(ResponseWriter) + + buf := bytes.NewBuffer([]byte{}) + err := logTemplate.Execute(buf, routerLoggerOptions{ + req: req, + Identity: &identity, + Start: &start, + ResponseWriter: rw, + Ctx: map[string]interface{}{ + "RemoteAddr": req.RemoteAddr, + "Req": req, + }, + }) + if err != nil { + log.Error("Could not set up chi access logger: %v", err.Error()) + } + + err = logger.SendLog(log.INFO, "", "", 0, buf.String(), "") + if err != nil { + log.Error("Could not set up chi access logger: %v", err.Error()) + } + }) + } +} diff --git a/modules/context/context.go b/modules/context/context.go index e5025205c..d02339d5b 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -485,6 +485,31 @@ func GetContext(req *http.Request) *Context { return req.Context().Value(contextKey).(*Context) } +// SignedUserName returns signed user's name via context +func SignedUserName(req *http.Request) string { + if middlewares.IsInternalPath(req) { + return "" + } + if middlewares.IsAPIPath(req) { + ctx, ok := req.Context().Value(apiContextKey).(*APIContext) + if ok { + v := ctx.Data["SignedUserName"] + if res, ok := v.(string); ok { + return res + } + } + } else { + ctx, ok := req.Context().Value(contextKey).(*Context) + if ok { + v := ctx.Data["SignedUserName"] + if res, ok := v.(string); ok { + return res + } + } + } + return "" +} + func getCsrfOpts() CsrfOptions { return CsrfOptions{ Secret: setting.SecretKey, diff --git a/modules/context/response.go b/modules/context/response.go index 1881ec7b3..bdbbb97af 100644 --- a/modules/context/response.go +++ b/modules/context/response.go @@ -12,6 +12,7 @@ type ResponseWriter interface { Flush() Status() int Before(func(ResponseWriter)) + Size() int } var ( @@ -21,11 +22,17 @@ var ( // Response represents a response type Response struct { http.ResponseWriter + written int status int befores []func(ResponseWriter) beforeExecuted bool } +// Size return written size +func (r *Response) Size() int { + return r.written +} + // Write writes bytes to HTTP endpoint func (r *Response) Write(bs []byte) (int, error) { if !r.beforeExecuted { @@ -35,8 +42,9 @@ func (r *Response) Write(bs []byte) (int, error) { r.beforeExecuted = true } size, err := r.ResponseWriter.Write(bs) + r.written += size if err != nil { - return 0, err + return size, err } if r.status == 0 { r.WriteHeader(200) diff --git a/modules/middlewares/request.go b/modules/middlewares/request.go new file mode 100644 index 000000000..aa3d60be2 --- /dev/null +++ b/modules/middlewares/request.go @@ -0,0 +1,20 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package middlewares + +import ( + "net/http" + "strings" +) + +// IsAPIPath returns true if the specified URL is an API path +func IsAPIPath(req *http.Request) bool { + return strings.HasPrefix(req.URL.Path, "/api/") +} + +// IsInternalPath returns true if the specified URL is an internal API path +func IsInternalPath(req *http.Request) bool { + return strings.HasPrefix(req.URL.Path, "/api/internal/") +} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 593551d77..b78c55269 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -553,6 +553,11 @@ func Routes() *web.Route { })) } m.Use(context.APIContexter()) + + if setting.EnableAccessLog { + m.Use(context.AccessLogger()) + } + m.Use(context.ToggleAPI(&context.ToggleOptions{ SignInRequired: setting.Service.RequireSignInView, })) diff --git a/routers/routes/base.go b/routers/routes/base.go index a313032a8..4efb7f01d 100644 --- a/routers/routes/base.go +++ b/routers/routes/base.go @@ -5,7 +5,6 @@ package routes import ( - "bytes" "errors" "fmt" "io" @@ -13,7 +12,6 @@ import ( "os" "path" "strings" - "text/template" "time" "code.gitea.io/gitea/modules/auth/sso" @@ -28,57 +26,6 @@ import ( "gitea.com/go-chi/session" ) -type routerLoggerOptions struct { - req *http.Request - Identity *string - Start *time.Time - ResponseWriter http.ResponseWriter -} - -// SignedUserName returns signed user's name via context -func SignedUserName(req *http.Request) string { - ctx := context.GetContext(req) - if ctx != nil { - v := ctx.Data["SignedUserName"] - if res, ok := v.(string); ok { - return res - } - } - return "" -} - -func accessLogger() func(http.Handler) http.Handler { - logger := log.GetLogger("access") - logTemplate, _ := template.New("log").Parse(setting.AccessLogTemplate) - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - start := time.Now() - next.ServeHTTP(w, req) - identity := "-" - if val := SignedUserName(req); val != "" { - identity = val - } - rw := w - - buf := bytes.NewBuffer([]byte{}) - err := logTemplate.Execute(buf, routerLoggerOptions{ - req: req, - Identity: &identity, - Start: &start, - ResponseWriter: rw, - }) - if err != nil { - log.Error("Could not set up chi access logger: %v", err.Error()) - } - - err = logger.SendLog(log.INFO, "", "", 0, buf.String(), "") - if err != nil { - log.Error("Could not set up chi access logger: %v", err.Error()) - } - }) - } -} - // LoggerHandler is a handler that will log the routing to the default gitea log func LoggerHandler(level log.Level) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { diff --git a/routers/routes/web.go b/routers/routes/web.go index cbd7c0b7c..3fecb4dbb 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -88,10 +88,6 @@ func commonMiddlewares() []func(http.Handler) http.Handler { next.ServeHTTP(resp, req) }) }) - - if setting.EnableAccessLog { - handlers = append(handlers, accessLogger()) - } return handlers } @@ -168,6 +164,10 @@ func WebRoutes() *web.Route { r.Use(context.Contexter()) // Removed: SetAutoHead allow a get request redirect to head if get method is not exist + if setting.EnableAccessLog { + r.Use(context.AccessLogger()) + } + r.Use(user.GetNotificationCount) r.Use(repo.GetActiveStopwatch) r.Use(func(ctx *context.Context) {