PublicDashboards: Add RBAC to secured endpoints (#54544)

pkg/api/accesscontrol.go

@@ -419,6 +419,19 @@ func (hs *HTTPServer) declareFixedRoles() error {
 		Grants: []string{"Admin"},
 	}
 
+	publicDashboardsWriterRole := ac.RoleRegistration{
+		Role: ac.RoleDTO{
+			Name:        "fixed:dashboards.public:writer",
+			DisplayName: "Public Dashboard writer",
+			Description: "Create, write or disable a public dashboard.",
+			Group:       "Dashboards",
+			Permissions: []ac.Permission{
+				{Action: dashboards.ActionDashboardPublicWrite, Scope: dashboards.ScopeDashboardsAll},
+			},
+		},
+		Grants: []string{"Admin"},
+	}
+
 	return hs.accesscontrolService.DeclareFixedRoles(
 		provisioningWriterRole, datasourcesReaderRole, builtInDatasourceReader, datasourcesWriterRole,
 		datasourcesIdReaderRole, orgReaderRole, orgWriterRole,
@@ -426,6 +439,7 @@ func (hs *HTTPServer) declareFixedRoles() error {
 		annotationsReaderRole, dashboardAnnotationsWriterRole, annotationsWriterRole,
 		dashboardsCreatorRole, dashboardsReaderRole, dashboardsWriterRole,
 		foldersCreatorRole, foldersReaderRole, foldersWriterRole, apikeyReaderRole, apikeyWriterRole,
+		publicDashboardsWriterRole,
 	)
 }
 

pkg/services/dashboards/accesscontrol.go

@@ -28,6 +28,8 @@ const (
 	ActionDashboardsDelete           = "dashboards:delete"
 	ActionDashboardsPermissionsRead  = "dashboards.permissions:read"
 	ActionDashboardsPermissionsWrite = "dashboards.permissions:write"
+
+	ActionDashboardPublicWrite = "dashboards.public:write"
 )
 
 var (

pkg/services/publicdashboards/api/api.go

@@ -51,18 +51,25 @@ func ProvideApi(
 //Registers Endpoints on Grafana Router
 func (api *Api) RegisterAPIEndpoints() {
 	auth := accesscontrol.Middleware(api.AccessControl)
-	reqSignedIn := middleware.ReqSignedIn
 
 	// Anonymous access to public dashboard route is configured in pkg/api/api.go
 	// because it is deeply dependent on the HTTPServer.Index() method and would result in a
 	// circular dependency
 
+	// public endpoints
 	api.RouteRegister.Get("/api/public/dashboards/:accessToken", routing.Wrap(api.GetPublicDashboard))
 	api.RouteRegister.Post("/api/public/dashboards/:accessToken/panels/:panelId/query", routing.Wrap(api.QueryPublicDashboard))
 
 	// Create/Update Public Dashboard
-	api.RouteRegister.Get("/api/dashboards/uid/:uid/public-config", auth(reqSignedIn, accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite)), routing.Wrap(api.GetPublicDashboardConfig))
+	uidScope := dashboards.ScopeDashboardsProvider.GetResourceScopeUID(accesscontrol.Parameter(":uid"))
-	api.RouteRegister.Post("/api/dashboards/uid/:uid/public-config", auth(reqSignedIn, accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite)), routing.Wrap(api.SavePublicDashboardConfig))
+
+	api.RouteRegister.Get("/api/dashboards/uid/:uid/public-config",
+		auth(middleware.ReqSignedIn, accesscontrol.EvalPermission(dashboards.ActionDashboardsRead, uidScope)),
+		routing.Wrap(api.GetPublicDashboardConfig))
+
+	api.RouteRegister.Post("/api/dashboards/uid/:uid/public-config",
+		auth(middleware.ReqOrgAdmin, accesscontrol.EvalPermission(dashboards.ActionDashboardPublicWrite, uidScope)),
+		routing.Wrap(api.SavePublicDashboardConfig))
 }
 
 // Gets public dashboard
@@ -72,7 +79,7 @@ func (api *Api) GetPublicDashboard(c *models.ReqContext) response.Response {
 
 	pubdash, dash, err := api.PublicDashboardService.GetPublicDashboard(
 		c.Req.Context(),
-		web.Params(c.Req)[":accessToken"],
+		accessToken,
 	)
 
 	if err != nil {
@@ -92,7 +99,7 @@ func (api *Api) GetPublicDashboard(c *models.ReqContext) response.Response {
 		Version:                    dash.Version,
 		IsFolder:                   false,
 		FolderId:                   dash.FolderId,
-		PublicDashboardAccessToken: accessToken,
+		PublicDashboardAccessToken: pubdash.AccessToken,
 		PublicDashboardUID:         pubdash.Uid,
 	}
 

pkg/services/publicdashboards/api/api_test.go

@@ -24,8 +24,9 @@ import (
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	dashboardStore "github.com/grafana/grafana/pkg/services/dashboards/database"
 	"github.com/grafana/grafana/pkg/services/datasources"
+	datasourcesService "github.com/grafana/grafana/pkg/services/datasources/service"
+	"github.com/grafana/grafana/pkg/services/org"
 
-	"github.com/grafana/grafana/pkg/services/datasources/service"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/publicdashboards"
 	publicdashboardsStore "github.com/grafana/grafana/pkg/services/publicdashboards/database"
@@ -37,6 +38,12 @@ import (
 	"github.com/grafana/grafana/pkg/web"
 )
 
+var userAdmin = &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: org.RoleAdmin, Login: "testAdminUser"}
+var userAdminRBAC = &user.SignedInUser{UserID: 2, OrgID: 1, OrgRole: org.RoleAdmin, Login: "testAdminUserRBAC", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardPublicWrite: {dashboards.ScopeDashboardsAll}}}}
+var userViewer = &user.SignedInUser{UserID: 3, OrgID: 1, OrgRole: org.RoleViewer, Login: "testViewerUser"}
+var userViewerRBAC = &user.SignedInUser{UserID: 4, OrgID: 1, OrgRole: org.RoleViewer, Login: "testViewerUserRBAC", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}}}}
+var anonymousUser *user.SignedInUser
+
 func TestAPIGetPublicDashboard(t *testing.T) {
 	t.Run("It should 404 if featureflag is not enabled", func(t *testing.T) {
 		cfg := setting.NewCfg()
@@ -47,7 +54,7 @@ func TestAPIGetPublicDashboard(t *testing.T) {
 		service.On("GetPublicDashboardConfig", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).
 			Return(&PublicDashboard{}, nil).Maybe()
 
-		testServer := setupTestServer(t, cfg, featuremgmt.WithFeatures(), service, nil)
+		testServer := setupTestServer(t, cfg, featuremgmt.WithFeatures(), service, nil, anonymousUser)
 
 		response := callAPI(testServer, http.MethodGet, "/api/public/dashboards", nil, t)
 		assert.Equal(t, http.StatusNotFound, response.Code)
@@ -56,7 +63,7 @@ func TestAPIGetPublicDashboard(t *testing.T) {
 		assert.Equal(t, http.StatusNotFound, response.Code)
 
 		// control set. make sure routes are mounted
-		testServer = setupTestServer(t, cfg, featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards), service, nil)
+		testServer = setupTestServer(t, cfg, featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards), service, nil, userAdmin)
 		response = callAPI(testServer, http.MethodGet, "/api/public/dashboards/asdf", nil, t)
 		assert.NotEqual(t, http.StatusNotFound, response.Code)
 	})
@@ -108,6 +115,7 @@ func TestAPIGetPublicDashboard(t *testing.T) {
 				featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards),
 				service,
 				nil,
+				anonymousUser,
 			)
 
 			response := callAPI(testServer, http.MethodGet,
@@ -148,6 +156,9 @@ func TestAPIGetPublicDashboardConfig(t *testing.T) {
 		ExpectedHttpResponse  int
 		PublicDashboardResult *PublicDashboard
 		PublicDashboardErr    error
+		User                  *user.SignedInUser
+		AccessControlEnabled  bool
+		ShouldCallService     bool
 	}{
 		{
 			Name:                  "retrieves public dashboard config when dashboard is found",
@@ -155,6 +166,9 @@ func TestAPIGetPublicDashboardConfig(t *testing.T) {
 			ExpectedHttpResponse:  http.StatusOK,
 			PublicDashboardResult: pubdash,
 			PublicDashboardErr:    nil,
+			User:                  userViewer,
+			AccessControlEnabled:  false,
+			ShouldCallService:     true,
 		},
 		{
 			Name:                  "returns 404 when dashboard not found",
@@ -162,6 +176,9 @@ func TestAPIGetPublicDashboardConfig(t *testing.T) {
 			ExpectedHttpResponse:  http.StatusNotFound,
 			PublicDashboardResult: nil,
 			PublicDashboardErr:    dashboards.ErrDashboardNotFound,
+			User:                  userViewer,
+			AccessControlEnabled:  false,
+			ShouldCallService:     true,
 		},
 		{
 			Name:                  "returns 500 when internal server error",
@@ -169,17 +186,42 @@ func TestAPIGetPublicDashboardConfig(t *testing.T) {
 			ExpectedHttpResponse:  http.StatusInternalServerError,
 			PublicDashboardResult: nil,
 			PublicDashboardErr:    errors.New("database broken"),
+			User:                  userViewer,
+			AccessControlEnabled:  false,
+			ShouldCallService:     true,
+		},
+		{
+			Name:                  "retrieves public dashboard config when dashboard is found RBAC on",
+			DashboardUid:          "1",
+			ExpectedHttpResponse:  http.StatusOK,
+			PublicDashboardResult: pubdash,
+			PublicDashboardErr:    nil,
+			User:                  userViewerRBAC,
+			AccessControlEnabled:  true,
+			ShouldCallService:     true,
+		},
+		{
+			Name:                  "returns 403 when no permissions RBAC on",
+			ExpectedHttpResponse:  http.StatusForbidden,
+			PublicDashboardResult: pubdash,
+			PublicDashboardErr:    nil,
+			User:                  userViewer,
+			AccessControlEnabled:  true,
+			ShouldCallService:     false,
 		},
 	}
 
 	for _, test := range testCases {
 		t.Run(test.Name, func(t *testing.T) {
 			service := publicdashboards.NewFakePublicDashboardService(t)
+
+			if test.ShouldCallService {
 				service.On("GetPublicDashboardConfig", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).
 					Return(test.PublicDashboardResult, test.PublicDashboardErr)
+			}
 
 			cfg := setting.NewCfg()
-			cfg.RBACEnabled = false
+			cfg.RBACEnabled = test.AccessControlEnabled
 
 			testServer := setupTestServer(
 				t,
@@ -187,6 +229,7 @@ func TestAPIGetPublicDashboardConfig(t *testing.T) {
 				featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards),
 				service,
 				nil,
+				test.User,
 			)
 
 			response := callAPI(
@@ -216,6 +259,9 @@ func TestApiSavePublicDashboardConfig(t *testing.T) {
 		publicDashboardConfig *PublicDashboard
 		ExpectedHttpResponse  int
 		SaveDashboardErr      error
+		User                  *user.SignedInUser
+		AccessControlEnabled  bool
+		ShouldCallService     bool
 	}{
 		{
 			Name:                  "returns 200 when update persists",
@@ -223,29 +269,70 @@ func TestApiSavePublicDashboardConfig(t *testing.T) {
 			publicDashboardConfig: &PublicDashboard{IsEnabled: true},
 			ExpectedHttpResponse:  http.StatusOK,
 			SaveDashboardErr:      nil,
+			User:                  userAdmin,
+			AccessControlEnabled:  false,
+			ShouldCallService:     true,
 		},
 		{
 			Name:                  "returns 500 when not persisted",
 			ExpectedHttpResponse:  http.StatusInternalServerError,
 			publicDashboardConfig: &PublicDashboard{},
 			SaveDashboardErr:      errors.New("backend failed to save"),
+			User:                  userAdmin,
+			AccessControlEnabled:  false,
+			ShouldCallService:     true,
 		},
 		{
 			Name:                  "returns 404 when dashboard not found",
 			ExpectedHttpResponse:  http.StatusNotFound,
 			publicDashboardConfig: &PublicDashboard{},
 			SaveDashboardErr:      dashboards.ErrDashboardNotFound,
+			User:                  userAdmin,
+			AccessControlEnabled:  false,
+			ShouldCallService:     true,
+		},
+		{
+			Name:                  "returns 200 when update persists RBAC on",
+			DashboardUid:          "1",
+			publicDashboardConfig: &PublicDashboard{IsEnabled: true},
+			ExpectedHttpResponse:  http.StatusOK,
+			SaveDashboardErr:      nil,
+			User:                  userAdminRBAC,
+			AccessControlEnabled:  true,
+			ShouldCallService:     true,
+		},
+		{
+			Name:                  "returns 403 when no permissions",
+			ExpectedHttpResponse:  http.StatusForbidden,
+			publicDashboardConfig: &PublicDashboard{IsEnabled: true},
+			SaveDashboardErr:      nil,
+			User:                  userViewer,
+			AccessControlEnabled:  false,
+			ShouldCallService:     false,
+		},
+		{
+			Name:                  "returns 403 when no permissions RBAC on",
+			ExpectedHttpResponse:  http.StatusForbidden,
+			publicDashboardConfig: &PublicDashboard{IsEnabled: true},
+			SaveDashboardErr:      nil,
+			User:                  userAdmin,
+			AccessControlEnabled:  true,
+			ShouldCallService:     false,
 		},
 	}
 
 	for _, test := range testCases {
 		t.Run(test.Name, func(t *testing.T) {
 			service := publicdashboards.NewFakePublicDashboardService(t)
+
+			// this is to avoid AssertExpectations fail at t.Cleanup when the middleware returns before calling the service
+			if test.ShouldCallService {
 				service.On("SavePublicDashboardConfig", mock.Anything, mock.Anything, mock.AnythingOfType("*models.SavePublicDashboardConfigDTO")).
 					Return(&PublicDashboard{IsEnabled: true}, test.SaveDashboardErr)
+			}
 
 			cfg := setting.NewCfg()
-			cfg.RBACEnabled = false
+			cfg.RBACEnabled = test.AccessControlEnabled
 
 			testServer := setupTestServer(
 				t,
@@ -253,6 +340,7 @@ func TestApiSavePublicDashboardConfig(t *testing.T) {
 				featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards),
 				service,
 				nil,
+				test.User,
 			)
 
 			response := callAPI(
@@ -339,6 +427,7 @@ func TestAPIQueryPublicDashboard(t *testing.T) {
 			featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards, enabled),
 			service,
 			nil,
+			anonymousUser,
 		)
 
 		return testServer, service
@@ -396,7 +485,7 @@ func TestAPIQueryPublicDashboard(t *testing.T) {
 func TestIntegrationUnauthenticatedUserCanGetPubdashPanelQueryData(t *testing.T) {
 	db := sqlstore.InitTestDB(t)
 
-	cacheService := service.ProvideCacheService(localcache.ProvideService(), db)
+	cacheService := datasourcesService.ProvideCacheService(localcache.ProvideService(), db)
 	qds := buildQueryDataService(t, cacheService, nil, db)
 
 	_ = db.AddDataSource(context.Background(), &datasources.AddDataSourceCommand{
@@ -436,8 +525,8 @@ func TestIntegrationUnauthenticatedUserCanGetPubdashPanelQueryData(t *testing.T)
 	}
 
 	// create dashboard
-	dashboardStore := dashboardStore.ProvideDashboardStore(db, featuremgmt.WithFeatures())
+	dashboardStoreService := dashboardStore.ProvideDashboardStore(db, featuremgmt.WithFeatures())
-	dashboard, err := dashboardStore.SaveDashboard(saveDashboardCmd)
+	dashboard, err := dashboardStoreService.SaveDashboard(saveDashboardCmd)
 	require.NoError(t, err)
 
 	// Create public dashboard
@@ -463,6 +552,7 @@ func TestIntegrationUnauthenticatedUserCanGetPubdashPanelQueryData(t *testing.T)
 		featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards),
 		service,
 		db,
+		anonymousUser,
 	)
 
 	resp := callAPI(server, http.MethodPost,

pkg/services/publicdashboards/api/common_test.go

@@ -21,7 +21,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
 	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
-	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/publicdashboards"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/services/user"
@@ -34,18 +33,13 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-type Server struct {
-	Mux           *web.Mux
-	RouteRegister routing.RouteRegister
-	TestServer    *httptest.Server
-}
-
 func setupTestServer(
 	t *testing.T,
 	cfg *setting.Cfg,
 	features *featuremgmt.FeatureManager,
 	service publicdashboards.Service,
 	db *sqlstore.SQLStore,
+	user *user.SignedInUser,
 ) *web.Mux {
 	// build router to register routes
 	rr := routing.NewRouteRegister()
@@ -69,18 +63,7 @@ func setupTestServer(
 	m := web.New()
 
 	// set initial context
-	m.Use(func(c *web.Context) {
+	m.Use(contextProvider(&testContext{user}))
-		ctx := &models.ReqContext{
-			Context:    c,
-			IsSignedIn: true, // FIXME need to be able to change this for tests
-			SkipCache:  true, // hardcoded to make sure query service doesnt hit the cache
-			Logger:     log.New("publicdashboards-test"),
-
-			// Set signed in user. We might not actually need to do this.
-			SignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: org.RoleAdmin, Login: "testUser"},
-		}
-		c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), ctx))
-	})
 
 	// build api, this will mount the routes at the same time if
 	// featuremgmt.FlagPublicDashboard is enabled
@@ -92,6 +75,24 @@ func setupTestServer(
 	return m
 }
 
+type testContext struct {
+	user *user.SignedInUser
+}
+
+func contextProvider(tc *testContext) web.Handler {
+	return func(c *web.Context) {
+		signedIn := tc.user != nil
+		reqCtx := &models.ReqContext{
+			Context:      c,
+			SignedInUser: tc.user,
+			IsSignedIn:   signedIn,
+			SkipCache:    true,
+			Logger:       log.New("publicdashboards-test"),
+		}
+		c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), reqCtx))
+	}
+}
+
 func callAPI(server *web.Mux, method, path string, body io.Reader, t *testing.T) *httptest.ResponseRecorder {
 	req, err := http.NewRequest(method, path, body)
 	require.NoError(t, err)