Chore: Remove untyped data map from macaron context (#39077)

pkg/macaron/context.go

@@ -45,7 +45,6 @@ type Context struct {
 	Resp     ResponseWriter
 	params   Params
 	template *template.Template
-	Data     map[string]interface{}
 }
 
 func (ctx *Context) handler() Handler {

pkg/macaron/macaron.go

@@ -157,7 +157,6 @@ func (m *Macaron) createContext(rw http.ResponseWriter, req *http.Request) *Cont
 		index:    0,
 		Router:   m.Router,
 		Resp:     NewResponseWriter(req.Method, rw),
-		Data:     make(map[string]interface{}),
 	}
 	req = req.WithContext(context.WithValue(req.Context(), macaronContextKey{}, c))
 	c.Map(c)

pkg/middleware/auth.go

@@ -111,9 +111,8 @@ func Auth(options *AuthOptions) macaron.Handler {
 		requireLogin := !c.AllowAnonymous || forceLogin || options.ReqNoAnonynmous
 
 		if !c.IsSignedIn && options.ReqSignedIn && requireLogin {
-			lookupTokenErr, hasTokenErr := c.Data["lookupTokenErr"].(error)
 			var revokedErr *models.TokenRevokedError
-			if hasTokenErr && errors.As(lookupTokenErr, &revokedErr) {
+			if errors.As(c.LookupTokenErr, &revokedErr) {
 				tokenRevoked(c, revokedErr)
 				return
 			}

pkg/middleware/logger.go

@@ -19,9 +19,8 @@ import (
 	"net/http"
 	"time"
 
-	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/contexthandler"
 	"github.com/grafana/grafana/pkg/setting"
-	"github.com/prometheus/client_golang/prometheus"
 	cw "github.com/weaveworks/common/middleware"
 	"gopkg.in/macaron.v1"
 )
@@ -29,16 +28,15 @@ import (
 func Logger(cfg *setting.Cfg) macaron.Handler {
 	return func(res http.ResponseWriter, req *http.Request, c *macaron.Context) {
 		start := time.Now()
-		c.Data["perfmon.start"] = start
 
 		rw := res.(macaron.ResponseWriter)
 		c.Next()
 
 		timeTaken := time.Since(start) / time.Millisecond
 
-		if timer, ok := c.Data["perfmon.timer"]; ok {
+		ctx := contexthandler.FromContext(c.Req.Context())
-			timerTyped := timer.(prometheus.Summary)
+		if ctx != nil && ctx.PerfmonTimer != nil {
-			timerTyped.Observe(float64(timeTaken))
+			ctx.PerfmonTimer.Observe(float64(timeTaken))
 		}
 
 		status := rw.Status()
@@ -48,9 +46,7 @@ func Logger(cfg *setting.Cfg) macaron.Handler {
 			}
 		}
 
-		if ctx, ok := c.Data["ctx"]; ok {
+		if ctx != nil {
-			ctxTyped := ctx.(*models.ReqContext)
-
 			logParams := []interface{}{
 				"method", req.Method,
 				"path", req.URL.Path,
@@ -61,15 +57,15 @@ func Logger(cfg *setting.Cfg) macaron.Handler {
 				"referer", req.Referer(),
 			}
 
-			traceID, exist := cw.ExtractTraceID(ctxTyped.Req.Context())
+			traceID, exist := cw.ExtractTraceID(ctx.Req.Context())
 			if exist {
 				logParams = append(logParams, "traceID", traceID)
 			}
 
 			if status >= 500 {
-				ctxTyped.Logger.Error("Request Completed", logParams...)
+				ctx.Logger.Error("Request Completed", logParams...)
 			} else {
-				ctxTyped.Logger.Info("Request Completed", logParams...)
+				ctx.Logger.Info("Request Completed", logParams...)
 			}
 		}
 	}

pkg/middleware/org_redirect.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/contexthandler"
 	"github.com/grafana/grafana/pkg/setting"
 	"gopkg.in/macaron.v1"
 )
@@ -23,8 +24,8 @@ func OrgRedirect(cfg *setting.Cfg) macaron.Handler {
 			return
 		}
 
-		ctx, ok := c.Data["ctx"].(*models.ReqContext)
+		ctx := contexthandler.FromContext(req.Context())
-		if !ok || !ctx.IsSignedIn {
+		if !ctx.IsSignedIn {
 			return
 		}
 

pkg/middleware/recovery.go

@@ -26,7 +26,7 @@ import (
 	"gopkg.in/macaron.v1"
 
 	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/contexthandler"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
@@ -109,9 +109,9 @@ func Recovery(cfg *setting.Cfg) macaron.Handler {
 			if r := recover(); r != nil {
 				panicLogger := log.Root
 				// try to get request logger
-				if ctx, ok := c.Data["ctx"]; ok {
+				ctx := contexthandler.FromContext(c.Req.Context())
-					ctxTyped := ctx.(*models.ReqContext)
+				if ctx != nil {
-					panicLogger = ctxTyped.Logger
+					panicLogger = ctx.Logger
 				}
 
 				if err, ok := r.(error); ok {
@@ -132,33 +132,34 @@ func Recovery(cfg *setting.Cfg) macaron.Handler {
 					return
 				}
 
-				c.Data["Title"] = "Server Error"
+				data := struct {
-				c.Data["AppSubUrl"] = cfg.AppSubURL
+					Title     string
-				c.Data["Theme"] = cfg.DefaultTheme
+					AppSubUrl string
+					Theme     string
+					ErrorMsg  string
+				}{"Server Error", cfg.AppSubURL, cfg.DefaultTheme, ""}
 
 				if setting.Env == setting.Dev {
 					if err, ok := r.(error); ok {
-						c.Data["Title"] = err.Error()
+						data.Title = err.Error()
 					}
 
-					c.Data["ErrorMsg"] = string(stack)
+					data.ErrorMsg = string(stack)
 				}
 
-				ctx, ok := c.Data["ctx"].(*models.ReqContext)
+				if ctx != nil && ctx.IsApiRequest() {
-
-				if ok && ctx.IsApiRequest() {
 					resp := make(map[string]interface{})
 					resp["message"] = "Internal Server Error - Check the Grafana server logs for the detailed error message."
 
-					if c.Data["ErrorMsg"] != nil {
+					if data.ErrorMsg != "" {
-						resp["error"] = fmt.Sprintf("%v - %v", c.Data["Title"], c.Data["ErrorMsg"])
+						resp["error"] = fmt.Sprintf("%v - %v", data.Title, data.ErrorMsg)
 					} else {
-						resp["error"] = c.Data["Title"]
+						resp["error"] = data.Title
 					}
 
 					c.JSON(500, resp)
 				} else {
-					c.HTML(500, cfg.ErrTemplateName, c.Data)
+					c.HTML(500, cfg.ErrTemplateName, data)
 				}
 			}
 		}()

pkg/models/context.go

@@ -21,22 +21,27 @@ type ReqContext struct {
 	Logger         log.Logger
 	// RequestNonce is a cryptographic request identifier for use with Content Security Policy.
 	RequestNonce string
+
+	PerfmonTimer   prometheus.Summary
+	LookupTokenErr error
 }
 
 // Handle handles and logs error by given status.
 func (ctx *ReqContext) Handle(cfg *setting.Cfg, status int, title string, err error) {
+	data := struct {
+		Title     string
+		AppSubUrl string
+		Theme     string
+		ErrorMsg  error
+	}{title, cfg.AppSubURL, "dark", nil}
 	if err != nil {
 		ctx.Logger.Error(title, "error", err)
 		if setting.Env != setting.Prod {
-			ctx.Data["ErrorMsg"] = err
+			data.ErrorMsg = err
 		}
 	}
 
-	ctx.Data["Title"] = title
+	ctx.HTML(status, cfg.ErrTemplateName, data)
-	ctx.Data["AppSubUrl"] = cfg.AppSubURL
-	ctx.Data["Theme"] = "dark"
-
-	ctx.HTML(status, cfg.ErrTemplateName, ctx.Data)
 }
 
 func (ctx *ReqContext) IsApiRequest() bool {
@@ -76,7 +81,7 @@ func (ctx *ReqContext) HasHelpFlag(flag HelpFlags1) bool {
 }
 
 func (ctx *ReqContext) TimeRequest(timer prometheus.Summary) {
-	ctx.Data["perfmon.timer"] = timer
+	ctx.PerfmonTimer = timer
 }
 
 // QueryBoolWithDefault extracts a value from the request query params and applies a bool default if not present.

pkg/services/contexthandler/auth_proxy_test.go

@@ -58,10 +58,7 @@ func TestInitContextWithAuthProxy_CachedInvalidUserID(t *testing.T) {
 	req, err := http.NewRequest("POST", "http://example.com", nil)
 	require.NoError(t, err)
 	ctx := &models.ReqContext{
-		Context: &macaron.Context{
+		Context: &macaron.Context{Req: req},
-			Req:  req,
-			Data: map[string]interface{}{},
-		},
 		Logger:  log.New("Test"),
 	}
 	req.Header.Set(svc.Cfg.AuthProxyHeaderName, name)

pkg/services/contexthandler/contexthandler.go

@@ -121,7 +121,6 @@ func (h *ContextHandler) Middleware(mContext *macaron.Context) {
 	}
 
 	reqContext.Logger = log.New("context", "userId", reqContext.UserId, "orgId", reqContext.OrgId, "uname", reqContext.Login)
-	reqContext.Data["ctx"] = reqContext
 
 	span.LogFields(
 		ol.String("uname", reqContext.Login),
@@ -299,7 +298,7 @@ func (h *ContextHandler) initContextWithToken(reqContext *models.ReqContext, org
 	token, err := h.AuthTokenService.LookupToken(ctx, rawToken)
 	if err != nil {
 		reqContext.Logger.Error("Failed to look up user based on cookie", "error", err)
-		reqContext.Data["lookupTokenErr"] = err
+		reqContext.LookupTokenErr = err
 		return false
 	}