Input validation / Resource existence check (Dashboard)

PoC steps (illustrative): 1) Prerequisites: You have a Grafana deployment with the Kubernetes API exposed and authenticated access to create dashboard snapshots via the Kubernetes-based API (namespace-scoped). 2) Attempt to create a snapshot that references a non-existent dashboard UID. Example request (payload may vary by deployment): curl -k -X POST https://grafana.example.com/apis/dashboard.k8s/v1/namespaces/default/snapshots \ -H 'Content-Type: application/json' \ -d '{ "apiVersion": "dashboard.k8s/v1alpha1", "kind": "Snapshot", "metadata": {"name": "poc-snap"}, "spec": {"dashboard": {"uid": "does-not-exist"}, "name": "Poc Snapshot"} }' 3) Expected behavior before the fix (hypothetical): The API may allow creating the snapshot even though the referenced dashboard UID does not exist, resulting in a snapshot that points to a non-existent dashboard. 4) Expected behavior after the fix: The API validates the dashboard existence and returns HTTP 400 with a message like: dashboard with UID "does-not-exist" not found 5) Also test missing UID: send a payload where dashboard.uid is missing or empty and expect HTTP 400 with message: dashboard UID is required. 6) Verification notes: - If your environment previously accepted non-existent dashboard UIDs, this PoC would demonstrate that behavior switching to a 400 error post-fix. - Ensure you test with an authenticated user possessing the appropriate namespace-scoped permissions. - Adapt API path and payload to your Grafana deployment if the path differs; the key is that the dashboard UID is either missing or non-existent and the API returns a 400 with a descriptive error.

diff --git a/pkg/registry/apis/dashboard/register.go b/pkg/registry/apis/dashboard/register.go index 2abcc1730b972..c1c4b3fd76987 100644 --- a/pkg/registry/apis/dashboard/register.go +++ b/pkg/registry/apis/dashboard/register.go @@ -1062,7 +1062,7 @@ func (b *DashboardsAPIBuilder) GetAPIRoutes(gv schema.GroupVersion) *builder.API searchAPIRoutes := b.search.GetAPIRoutes(defs) snapshotAPIRoutes := snapshot.GetRoutes(b.snapshotService, b.snapshotOptions, defs, func() rest.Storage { return b.snapshotStorage - }) + }, b.dashboardService) return &builder.APIRoutes{ Namespace: append(searchAPIRoutes.Namespace, snapshotAPIRoutes.Namespace...), diff --git a/pkg/registry/apis/dashboard/snapshot/routes.go b/pkg/registry/apis/dashboard/snapshot/routes.go index c4ed5df270fad..a9e424d2955b4 100644 --- a/pkg/registry/apis/dashboard/snapshot/routes.go +++ b/pkg/registry/apis/dashboard/snapshot/routes.go @@ -19,6 +19,7 @@ import ( "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/services/apiserver/builder" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" + "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboardsnapshots" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" @@ -26,7 +27,7 @@ import ( "github.com/grafana/grafana/pkg/web" ) -func GetRoutes(service dashboardsnapshots.Service, options dashv0.SnapshotSharingOptions, defs map[string]common.OpenAPIDefinition, storageGetter func() rest.Storage) *builder.APIRoutes { +func GetRoutes(service dashboardsnapshots.Service, options dashv0.SnapshotSharingOptions, defs map[string]common.OpenAPIDefinition, storageGetter func() rest.Storage, dashboardService dashboards.DashboardService) *builder.APIRoutes { prefix := dashv0.SnapshotResourceInfo.GroupResource().Resource tags := []string{dashv0.SnapshotResourceInfo.GroupVersionKind().Kind} @@ -147,7 +148,20 @@ func GetRoutes(service dashboardsnapshots.Service, options dashv0.SnapshotSharin return } - // TODO: validate dashboard exists. Need to call dashboards api, Maybe in a validation hook? + // Validate that the dashboard exists + dashboardUID, _ := cmd.Dashboard.Object["uid"].(string) + if dashboardUID == "" { + wrap.JsonApiErr(http.StatusBadRequest, "dashboard UID is required", nil) + return + } + _, err = dashboardService.GetDashboard(ctx, &dashboards.GetDashboardQuery{ + UID: dashboardUID, + OrgID: user.GetOrgID(), + }) + if err != nil { + wrap.JsonApiErr(http.StatusBadRequest, fmt.Sprintf("dashboard with UID %q not found", dashboardUID), nil) + return + } cmd.OrgID = user.GetOrgID() cmd.UserID, _ = identity.UserIdentifier(user.GetID()) diff --git a/pkg/registry/apis/dashboard/snapshot/routes_test.go b/pkg/registry/apis/dashboard/snapshot/routes_test.go new file mode 100644 index 0000000000000..2e3885c5d549c --- /dev/null +++ b/pkg/registry/apis/dashboard/snapshot/routes_test.go @@ -0,0 +1,154 @@ +package snapshot + +import ( + "bytes" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gorilla/mux" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "k8s.io/apiserver/pkg/registry/rest" + "k8s.io/kube-openapi/pkg/common" + + authlib "github.com/grafana/authlib/types" + dashv0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" + "github.com/grafana/grafana/pkg/apimachinery/identity" + grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" + "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/dashboardsnapshots" + "github.com/grafana/grafana/pkg/services/user" +) + +func TestCreateSnapshotDashboardValidation(t *testing.T) { + const orgID int64 = 1 + namespace := authlib.OrgNamespaceFormatter(orgID) + + testUser := &user.SignedInUser{ + UserID: 1, + OrgID: orgID, + } + + tests := []struct { + name string + body map[string]any + setupDashboardMock func(*dashboards.FakeDashboardService) + setupStorageMock func(t *testing.T) func() rest.Storage + expectedStatus int + expectedMessage string + }{ + { + name: "missing dashboard UID returns 400", + body: map[string]any{ + "dashboard": map[string]any{ + "title": "test", + }, + "name": "test snapshot", + }, + setupDashboardMock: func(m *dashboards.FakeDashboardService) {}, + setupStorageMock: func(t *testing.T) func() rest.Storage { return func() rest.Storage { return nil } }, + expectedStatus: http.StatusBadRequest, + expectedMessage: "dashboard UID is required", + }, + { + name: "empty dashboard UID returns 400", + body: map[string]any{ + "dashboard": map[string]any{ + "uid": "", + "title": "test", + }, + "name": "test snapshot", + }, + setupDashboardMock: func(m *dashboards.FakeDashboardService) {}, + setupStorageMock: func(t *testing.T) func() rest.Storage { return func() rest.Storage { return nil } }, + expectedStatus: http.StatusBadRequest, + expectedMessage: "dashboard UID is required", + }, + { + name: "non-existent dashboard UID returns 400", + body: map[string]any{ + "dashboard": map[string]any{ + "uid": "does-not-exist", + "title": "test", + }, + "name": "test snapshot", + }, + setupDashboardMock: func(m *dashboards.FakeDashboardService) { + m.On("GetDashboard", mock.Anything, &dashboards.GetDashboardQuery{ + UID: "does-not-exist", + OrgID: orgID, + }).Return(nil, dashboards.ErrDashboardNotFound) + }, + setupStorageMock: func(t *testing.T) func() rest.Storage { return func() rest.Storage { return nil } }, + expectedStatus: http.StatusBadRequest, + expectedMessage: `dashboard with UID "does-not-exist" not found`, + }, + { + name: "existing dashboard UID passes validation", + body: map[string]any{ + "dashboard": map[string]any{ + "uid": "valid-uid", + "title": "test", + }, + "name": "test snapshot", + }, + setupDashboardMock: func(m *dashboards.FakeDashboardService) { + m.On("GetDashboard", mock.Anything, &dashboards.GetDashboardQuery{ + UID: "valid-uid", + OrgID: orgID, + }).Return(&dashboards.Dashboard{UID: "valid-uid", OrgID: orgID}, nil) + }, + setupStorageMock: func(t *testing.T) func() rest.Storage { + mockStorage := grafanarest.NewMockStorage(t) + mockStorage.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(&dashv0.Snapshot{}, nil) + return func() rest.Storage { return mockStorage } + }, + expectedStatus: http.StatusOK, + expectedMessage: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + snapshotService := dashboardsnapshots.NewMockService(t) + dashboardService := dashboards.NewFakeDashboardService(t) + tt.setupDashboardMock(dashboardService) + + routes := GetRoutes( + snapshotService, + dashv0.SnapshotSharingOptions{SnapshotsEnabled: true}, + map[string]common.OpenAPIDefinition{}, + tt.setupStorageMock(t), + dashboardService, + ) + + // Find the create handler (first namespace route) + require.NotEmpty(t, routes.Namespace) + handler := routes.Namespace[0].Handler + + bodyBytes, err := json.Marshal(tt.body) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodPost, "/snapshots/create", bytes.NewReader(bodyBytes)) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(identity.WithRequester(req.Context(), testUser)) + req = mux.SetURLVars(req, map[string]string{"namespace": namespace}) + + recorder := httptest.NewRecorder() + handler(recorder, req) + + assert.Equal(t, tt.expectedStatus, recorder.Code) + if tt.expectedMessage != "" { + var resp map[string]any + err := json.Unmarshal(recorder.Body.Bytes(), &resp) + require.NoError(t, err) + assert.Contains(t, fmt.Sprintf("%v", resp["message"]), tt.expectedMessage) + } + }) + } +} diff --git a/pkg/tests/apis/dashboard/snapshot_test.go b/pkg/tests/apis/dashboard/snapshot_test.go index 01f68d8f77d3a..ffff03ed4ea97 100644 --- a/pkg/tests/apis/dashboard/snapshot_test.go +++ b/pkg/tests/apis/dashboard/snapshot_test.go @@ -10,10 +10,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" dashv0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" + dashv1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/setting" @@ -68,6 +70,10 @@ func TestIntegrationSnapshotDualWrite(t *testing.T) { }) ns := client.Args.Namespace + // Create a dashboard with the UID referenced by snapshot tests, + // so the dashboard validation in the create handler passes. + createTestDashboard(t, helper, ns) + t.Log("Testing:", tc.description) t.Run("create and get snapshot", func(t *testing.T) { @@ -175,6 +181,7 @@ func createSnapshotViaSubresource(t *testing.T, helper *apis.K8sTestHelper, ns s Object: map[string]interface{}{ "title": "Test Dashboard", "panels": []interface{}{}, + "uid": "a-valid-uid", "schemaVersion": 39, "time": map[string]interface{}{ "from": "now-6h", @@ -206,3 +213,33 @@ func createSnapshotViaSubresource(t *testing.T, helper *apis.K8sTestHelper, ns s return rsp.Result } + +// createTestDashboard creates a dashboard with the UID "a-valid-uid" so that +// snapshot creation (which validates the dashboard exists) can succeed. +func createTestDashboard(t *testing.T, helper *apis.K8sTestHelper, ns string) { + t.Helper() + + dashClient := helper.GetResourceClient(apis.ResourceClientArgs{ + User: helper.Org1.Admin, + Namespace: ns, + GVR: dashv1.DashboardResourceInfo.GroupVersionResource(), + }) + + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": dashv1.DashboardResourceInfo.GroupVersion().String(), + "kind": "Dashboard", + "metadata": map[string]interface{}{ + "name": "a-valid-uid", + "namespace": ns, + }, + "spec": map[string]interface{}{ + "title": "Test Dashboard", + "schemaVersion": 42, + }, + }, + } + + _, err := dashClient.Resource.Create(context.Background(), obj, metav1.CreateOptions{}) + require.NoError(t, err, "failed to create test dashboard") +}