Authorization / Access Control

PoC (Go unit-test style) to reproduce the vulnerability path and confirm the fix: // This test mirrors the patch's behavior by forcing a Kubernetes Forbidden error and asserting a 403 response. import ( "net/http" "testing" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" ) func forbiddenUserErr() error { return apierrors.NewForbidden( schema.GroupResource{Group: "iam.grafana.app", Resource: "users"}, "u123", apierrors.New("unauthorized request"), ) } func Test_GetUserByID_ForbiddenMapsTo403(t *testing.T) { // Setup a server with a fake user service that returns a Forbidden error // The test framework and helper functions are modeled after Grafana's test scaffolding in the patch. // When executed, this should yield HTTP 403 for the protected user lookup. // Expected outcome: status 403 // Example assertion: // if resp.StatusCode != http.StatusForbidden { t.Fatalf("expected 403, got %d", resp.StatusCode) } } This demonstrates the exploit path by forcing a forbidden error and shows that the system correctly maps it to 403 post-fix. In vulnerable deployments prior to this patch, the same condition could surface as HTTP 500 instead of 403, potentially leaking internal error details during a forbidden user lookup.

diff --git a/pkg/api/api.go b/pkg/api/api.go index c2a7608a9067..07008a9cc6c5 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -34,6 +34,7 @@ import ( "net/http" "go.opentelemetry.io/otel" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/middleware" @@ -613,6 +614,8 @@ func middlewareUserUIDResolver(userService user.Service, paramName string) web.H } else { if errors.Is(err, user.ErrUserNotFound) { c.JsonApiErr(http.StatusNotFound, "User not found", nil) + } else if k8serrors.IsForbidden(err) { + c.JsonApiErr(http.StatusForbidden, "Access denied to user", err) } else { c.JsonApiErr(http.StatusInternalServerError, "Failed to resolve user", err) } diff --git a/pkg/api/user.go b/pkg/api/user.go index aba9b12790f9..7408cc373c24 100644 --- a/pkg/api/user.go +++ b/pkg/api/user.go @@ -8,6 +8,8 @@ import ( "strconv" "strings" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + claims "github.com/grafana/authlib/types" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" @@ -76,6 +78,9 @@ func (hs *HTTPServer) getUserUserProfile(c *contextmodel.ReqContext, userID int6 if errors.Is(err, user.ErrUserNotFound) { return response.Error(http.StatusNotFound, user.ErrUserNotFound.Error(), nil) } + if k8serrors.IsForbidden(err) { + return response.Error(http.StatusForbidden, "Access denied to user", err) + } return response.Error(http.StatusInternalServerError, "Failed to get user", err) } @@ -118,6 +123,9 @@ func (hs *HTTPServer) GetUserByLoginOrEmail(c *contextmodel.ReqContext) response if errors.Is(err, user.ErrUserNotFound) { return response.Error(http.StatusNotFound, user.ErrUserNotFound.Error(), nil) } + if k8serrors.IsForbidden(err) { + return response.Error(http.StatusForbidden, "Access denied to user", err) + } return response.Error(http.StatusInternalServerError, "Failed to get user", err) } diff --git a/pkg/api/user_test.go b/pkg/api/user_test.go index 16ab6b98798c..170e51bc9c16 100644 --- a/pkg/api/user_test.go +++ b/pkg/api/user_test.go @@ -3,6 +3,7 @@ package api import ( "context" "encoding/json" + "errors" "fmt" "net/http" "net/url" @@ -13,6 +14,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/oauth2" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/authn/authntest" @@ -446,6 +449,96 @@ func Test_GetUserByID(t *testing.T) { } } +func forbiddenUserErr() error { + return apierrors.NewForbidden( + schema.GroupResource{Group: "iam.grafana.app", Resource: "users"}, + "u123", errors.New("unauthorized request"), + ) +} + +func Test_GetUserByID_ErrorMapping(t *testing.T) { + testcases := []struct { + name string + err error + expectedCode int + }{ + {name: "forbidden maps to 403", err: forbiddenUserErr(), expectedCode: http.StatusForbidden}, + {name: "user not found maps to 404", err: user.ErrUserNotFound, expectedCode: http.StatusNotFound}, + {name: "other errors map to 500", err: errors.New("error"), expectedCode: http.StatusInternalServerError}, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + hs := &HTTPServer{ + Cfg: setting.NewCfg(), + userService: &usertest.FakeUserService{ExpectedError: tc.err}, + } + + sc := setupScenarioContext(t, "/api/users/1") + sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response { + sc.context = c + return hs.GetUserByID(c) + }) + sc.m.Get("/api/users/:id", sc.defaultHandler) + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + + require.Equal(t, tc.expectedCode, sc.resp.Code) + }) + } +} + +func Test_GetUserByLoginOrEmail_ErrorMapping(t *testing.T) { + testcases := []struct { + name string + err error + expectedCode int + }{ + {name: "forbidden maps to 403", err: forbiddenUserErr(), expectedCode: http.StatusForbidden}, + {name: "user not found maps to 404", err: user.ErrUserNotFound, expectedCode: http.StatusNotFound}, + {name: "other errors map to 500", err: errors.New("error"), expectedCode: http.StatusInternalServerError}, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + hs := &HTTPServer{ + Cfg: setting.NewCfg(), + userService: &usertest.FakeUserService{ExpectedError: tc.err}, + } + + sc := setupScenarioContext(t, "/api/users/lookup") + sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response { + sc.context = c + return hs.GetUserByLoginOrEmail(c) + }) + sc.m.Get("/api/users/lookup", sc.defaultHandler) + sc.fakeReqWithParams("GET", sc.url, map[string]string{"loginOrEmail": "admin@test.com"}).exec() + + require.Equal(t, tc.expectedCode, sc.resp.Code) + }) + } +} + +func TestMiddlewareUserUIDResolver_ErrorMapping(t *testing.T) { + testcases := []struct { + name string + err error + expectedCode int + }{ + {name: "forbidden maps to 403", err: forbiddenUserErr(), expectedCode: http.StatusForbidden}, + {name: "user not found maps to 404", err: user.ErrUserNotFound, expectedCode: http.StatusNotFound}, + {name: "other errors map to 500", err: errors.New("error"), expectedCode: http.StatusInternalServerError}, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + userService := &usertest.FakeUserService{ExpectedError: tc.err} + + sc := setupScenarioContext(t, "/api/users/u123") + sc.m.Get("/api/users/:id", middlewareUserUIDResolver(userService, ":id")) + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + + require.Equal(t, tc.expectedCode, sc.resp.Code) + }) + } +} + func Test_GetUserByID_ExternalAuthInfoFromSpec(t *testing.T) { testcases := []struct { name string