diff --git a/pkg/services/auth/identity/requester.go b/pkg/services/auth/identity/requester.go index 1eb72bb5a90..b88120e88f2 100644 --- a/pkg/services/auth/identity/requester.go +++ b/pkg/services/auth/identity/requester.go @@ -67,12 +67,22 @@ type Requester interface { GetIDToken() string } +// IsNamespace returns true if namespace matches any expected namespace +func IsNamespace(namespace string, expected ...string) bool { + for _, e := range expected { + if namespace == e { + return true + } + } + + return false +} + // IntIdentifier converts a string identifier to an int64. // Applicable for users, service accounts, api keys and renderer service. // Errors if the identifier is not initialized or if namespace is not recognized. func IntIdentifier(namespace, identifier string) (int64, error) { - switch namespace { - case NamespaceUser, NamespaceAPIKey, NamespaceServiceAccount, NamespaceRenderService: + if IsNamespace(namespace, NamespaceUser, NamespaceAPIKey, NamespaceServiceAccount, NamespaceRenderService) { id, err := strconv.ParseInt(identifier, 10, 64) if err != nil { return 0, fmt.Errorf("unrecognized format for valid namespace %s: %w", namespace, err) @@ -98,8 +108,7 @@ func UserIdentifier(namespace, identifier string) (int64, error) { return 0, nil } - switch namespace { - case NamespaceUser, NamespaceServiceAccount: + if IsNamespace(namespace, NamespaceUser, NamespaceServiceAccount) { return userID, nil } diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index 6d9ef076911..49b91f45f4f 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "net/http" - "strconv" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -15,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/auth" + "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/contexthandler/ctxkey" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" @@ -138,38 +138,26 @@ func (h *ContextHandler) Middleware(next http.Handler) http.Handler { )) if h.Cfg.IDResponseHeaderEnabled && reqContext.SignedInUser != nil { - namespace, id := getNamespaceAndID(reqContext.SignedInUser) - reqContext.Resp.Before(h.addIDHeaderEndOfRequestFunc(namespace, id)) + reqContext.Resp.Before(h.addIDHeaderEndOfRequestFunc(reqContext.SignedInUser)) } next.ServeHTTP(w, r) }) } -// TODO(kalleep): Refactor to user identity.Requester interface and methods after we have backported this -func getNamespaceAndID(user *user.SignedInUser) (string, string) { - var namespace, id string - if user.UserID > 0 && user.IsServiceAccount { - id = strconv.Itoa(int(user.UserID)) - namespace = "service-account" - } else if user.UserID > 0 { - id = strconv.Itoa(int(user.UserID)) - namespace = "user" - } else if user.ApiKeyID > 0 { - id = strconv.Itoa(int(user.ApiKeyID)) - namespace = "api-key" - } - - return namespace, id -} - -func (h *ContextHandler) addIDHeaderEndOfRequestFunc(namespace, id string) web.BeforeFunc { +func (h *ContextHandler) addIDHeaderEndOfRequestFunc(ident identity.Requester) web.BeforeFunc { return func(w web.ResponseWriter) { if w.Written() { return } - if namespace == "" || id == "" { + namespace, id := ident.GetNamespacedID() + if !identity.IsNamespace( + namespace, + identity.NamespaceUser, + identity.NamespaceServiceAccount, + identity.NamespaceAPIKey, + ) || id == "0" { return } diff --git a/pkg/services/contexthandler/contexthandler_test.go b/pkg/services/contexthandler/contexthandler_test.go index ca9a003231c..47f5adc4816 100644 --- a/pkg/services/contexthandler/contexthandler_test.go +++ b/pkg/services/contexthandler/contexthandler_test.go @@ -2,6 +2,7 @@ package contexthandler_test import ( "errors" + "net/http" "testing" "github.com/stretchr/testify/assert" @@ -38,8 +39,9 @@ func TestContextHandler(t *testing.T) { require.Error(t, c.LookupTokenErr) }) - _, err := server.Send(server.NewGetRequest("/api/handler")) + res, err := server.Send(server.NewGetRequest("/api/handler")) require.NoError(t, err) + require.NoError(t, res.Body.Close()) }) t.Run("should set identity on successful authentication", func(t *testing.T) { @@ -59,8 +61,9 @@ func TestContextHandler(t *testing.T) { require.NoError(t, c.LookupTokenErr) }) - _, err := server.Send(server.NewGetRequest("/api/handler")) + res, err := server.Send(server.NewGetRequest("/api/handler")) require.NoError(t, err) + require.NoError(t, res.Body.Close()) }) t.Run("should not set IsSignedIn on anonymous identity", func(t *testing.T) { @@ -80,8 +83,9 @@ func TestContextHandler(t *testing.T) { require.NoError(t, c.LookupTokenErr) }) - _, err := server.Send(server.NewGetRequest("/api/handler")) + res, err := server.Send(server.NewGetRequest("/api/handler")) require.NoError(t, err) + require.NoError(t, res.Body.Close()) }) t.Run("should set IsRenderCall when authenticated by render client", func(t *testing.T) { @@ -102,8 +106,9 @@ func TestContextHandler(t *testing.T) { require.NoError(t, c.LookupTokenErr) }) - _, err := server.Send(server.NewGetRequest("/api/handler")) + res, err := server.Send(server.NewGetRequest("/api/handler")) require.NoError(t, err) + require.NoError(t, res.Body.Close()) }) t.Run("should delete session cookie on invalid session", func(t *testing.T) { @@ -175,7 +180,72 @@ func TestContextHandler(t *testing.T) { assert.Contains(t, list.Items, "Authorization") }) - _, err := server.Send(server.NewGetRequest("/api/handler")) + res, err := server.Send(server.NewGetRequest("/api/handler")) require.NoError(t, err) + require.NoError(t, res.Body.Close()) + }) + + t.Run("id response headers", func(t *testing.T) { + run := func(cfg *setting.Cfg, id string) *http.Response { + handler := contexthandler.ProvideService( + cfg, + tracing.InitializeTracerForTest(), + featuremgmt.WithFeatures(), + &authntest.FakeService{ExpectedIdentity: &authn.Identity{ID: id}}, + ) + + server := webtest.NewServer(t, routing.NewRouteRegister()) + server.Mux.Use(handler.Middleware) + server.Mux.Get("/api/handler", func(c *contextmodel.ReqContext) {}) + + res, err := server.Send(server.NewGetRequest("/api/handler")) + require.NoError(t, err) + + return res + } + + t.Run("should add id header for user", func(t *testing.T) { + cfg := setting.NewCfg() + cfg.IDResponseHeaderEnabled = true + cfg.IDResponseHeaderPrefix = "X-Grafana" + cfg.IDResponseHeaderNamespaces = map[string]struct{}{"user": {}} + res := run(cfg, "user:1") + + require.Equal(t, "user:1", res.Header.Get("X-Grafana-Identity-Id")) + require.NoError(t, res.Body.Close()) + }) + + t.Run("should not add id header for user when id is 0", func(t *testing.T) { + cfg := setting.NewCfg() + cfg.IDResponseHeaderEnabled = true + cfg.IDResponseHeaderPrefix = "X-Grafana" + cfg.IDResponseHeaderNamespaces = map[string]struct{}{"user": {}} + res := run(cfg, "user:0") + + require.Empty(t, res.Header.Get("X-Grafana-Identity-Id")) + require.NoError(t, res.Body.Close()) + }) + + t.Run("should add id header for service account", func(t *testing.T) { + cfg := setting.NewCfg() + cfg.IDResponseHeaderEnabled = true + cfg.IDResponseHeaderPrefix = "X-Grafana" + cfg.IDResponseHeaderNamespaces = map[string]struct{}{"service-account": {}} + res := run(cfg, "service-account:1") + + require.Equal(t, "service-account:1", res.Header.Get("X-Grafana-Identity-Id")) + require.NoError(t, res.Body.Close()) + }) + + t.Run("should not add id header for service account when not configured", func(t *testing.T) { + cfg := setting.NewCfg() + cfg.IDResponseHeaderEnabled = true + cfg.IDResponseHeaderPrefix = "X-Grafana" + cfg.IDResponseHeaderNamespaces = map[string]struct{}{"user": {}} + res := run(cfg, "service-account:1") + + require.Empty(t, res.Header.Get("X-Grafana-Identity-Id")) + require.NoError(t, res.Body.Close()) + }) }) } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 77678629047..63aed72c8e6 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -1546,7 +1546,7 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { // ID response header cfg.IDResponseHeaderEnabled = auth.Key("id_response_header_enabled").MustBool(false) - cfg.IDResponseHeaderPrefix = auth.Key("id_response_header_prefix").MustString("X-Grafana-") + cfg.IDResponseHeaderPrefix = auth.Key("id_response_header_prefix").MustString("X-Grafana") idHeaderNamespaces := util.SplitString(auth.Key("id_response_header_namespaces").MustString("")) cfg.IDResponseHeaderNamespaces = make(map[string]struct{}, len(idHeaderNamespaces))