RBAC: Rewrite data source api tests (#61783)

RBAC: Rewrite datasource rbac api tests

pkg/api/common_test.go

@@ -26,7 +26,6 @@ import (
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
-	"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
 	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol"
 	"github.com/grafana/grafana/pkg/services/annotations/annotationstest"
@@ -42,7 +41,6 @@ import (
 	dashver "github.com/grafana/grafana/pkg/services/dashboardversion"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/guardian"
-	"github.com/grafana/grafana/pkg/services/ldap"
 	"github.com/grafana/grafana/pkg/services/licensing"
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/login/loginservice"
@@ -56,7 +54,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/searchusers"
 	"github.com/grafana/grafana/pkg/services/searchusers/filters"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
-	"github.com/grafana/grafana/pkg/services/stats/statsimpl"
 	"github.com/grafana/grafana/pkg/services/tag/tagimpl"
 	"github.com/grafana/grafana/pkg/services/team"
 	"github.com/grafana/grafana/pkg/services/team/teamimpl"
@@ -252,31 +249,6 @@ func (s *fakeRenderService) Init() error {
 	return nil
 }
 
-func setupAccessControlScenarioContext(t *testing.T, cfg *setting.Cfg, url string, permissions []accesscontrol.Permission) (*scenarioContext, *HTTPServer) {
-	store := sqlstore.InitTestDB(t)
-	statsService := statsimpl.ProvideService(store)
-	hs := &HTTPServer{
-		Cfg:                  cfg,
-		Live:                 newTestLive(t, store),
-		License:              &licensing.OSSLicensingService{},
-		Features:             featuremgmt.WithFeatures(),
-		QuotaService:         quotatest.New(false, nil),
-		RouteRegister:        routing.NewRouteRegister(),
-		AccessControl:        accesscontrolmock.New().WithPermissions(permissions),
-		searchUsersService:   searchusers.ProvideUsersService(filters.ProvideOSSSearchUserFilter(), usertest.NewUserServiceFake()),
-		ldapGroups:           ldap.ProvideGroupsService(),
-		accesscontrolService: actest.FakeService{},
-		statsService:         statsService,
-	}
-
-	sc := setupScenarioContext(t, url)
-
-	hs.registerRoutes()
-	hs.RouteRegister.Register(sc.m.Router)
-
-	return sc, hs
-}
-
 type accessControlTestCase struct {
 	expectedCode int
 	desc         string

pkg/api/dashboard_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/registry/corekind"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
 	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
 	"github.com/grafana/grafana/pkg/services/alerting"
@@ -115,7 +116,7 @@ func newTestLive(t *testing.T, store db.DB) *live.GrafanaLive {
 		nil,
 		&usagestats.UsageStatsMock{T: t},
 		nil,
-		features, accesscontrolmock.New(), &dashboards.FakeDashboardService{}, annotationstest.NewFakeAnnotationsRepo(), nil)
+		features, acimpl.ProvideAccessControl(cfg), &dashboards.FakeDashboardService{}, annotationstest.NewFakeAnnotationsRepo(), nil)
 	require.NoError(t, err)
 	return gLive
 }

pkg/api/datasources_test.go

@@ -1,15 +1,14 @@
 package api
 
 import (
-	"bytes"
 	"context"
 	"encoding/json"
-	"fmt"
 	"io"
 	"net/http"
-	"net/http/httptest"
+	"strings"
 	"testing"
 
+	"github.com/grafana/grafana/pkg/web/webtest"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -24,7 +23,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
 	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/datasources/permissions"
-	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
@@ -250,369 +248,107 @@ func TestUpdateDataSource_URLWithoutProtocol(t *testing.T) {
 	assert.Equal(t, 200, sc.resp.Code)
 }
 
-func TestAPI_Datasources_AccessControl(t *testing.T) {
-	testDatasource := datasources.DataSource{
-		Id:     3,
-		Uid:    "testUID",
-		OrgId:  testOrgID,
-		Name:   "test",
-		Url:    "http://localhost:5432",
-		Type:   "postgresql",
-		Access: "Proxy",
-	}
-	testDatasourceReadOnly := datasources.DataSource{
-		Id:       4,
-		Uid:      "testUID",
-		OrgId:    testOrgID,
-		Name:     "test",
-		Url:      "http://localhost:5432",
-		Type:     "postgresql",
-		Access:   "Proxy",
-		ReadOnly: true,
+func TestAPI_datasources_AccessControl(t *testing.T) {
+	type testCase struct {
+		desc         string
+		urls         []string
+		method       string
+		body         string
+		permission   []ac.Permission
+		expectedCode int
 	}
 
-	addDatasourceBody := func() io.Reader {
-		s, _ := json.Marshal(datasources.AddDataSourceCommand{
-			Name:   "test",
-			Url:    "http://localhost:5432",
-			Type:   "postgresql",
-			Access: "Proxy",
-		})
-		return bytes.NewReader(s)
-	}
-
-	dsServiceMock := &dataSourcesServiceMock{
-		expectedDatasource: &testDatasource,
-	}
-	dsPermissionService := permissions.NewMockDatasourcePermissionService()
-	dsPermissionService.DsResult = []*datasources.DataSource{
-		&testDatasource,
-	}
-
-	updateDatasourceBody := func() io.Reader {
-		s, _ := json.Marshal(datasources.UpdateDataSourceCommand{
-			Name:   "test",
-			Url:    "http://localhost:5432",
-			Type:   "postgresql",
-			Access: "Proxy",
-		})
-		return bytes.NewReader(s)
-	}
-	type acTestCaseWithHandler struct {
-		body func() io.Reader
-		accessControlTestCase
-		expectedDS       *datasources.DataSource
-		expectedSQLError error
-	}
-	tests := []acTestCaseWithHandler{
+	tests := []testCase{
 		{
-			body: updateDatasourceBody,
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusNotFound,
-				desc:         "DatasourcesPut should return 404 if datasource not found",
-				url:          fmt.Sprintf("/api/datasources/%v", "12345678"),
-				method:       http.MethodPut,
-				permissions: []ac.Permission{
-					{
-						Action: datasources.ActionWrite,
-						Scope:  datasources.ScopeAll,
-					},
-				},
+			desc:   "should be able to update datasource with correct permission",
+			urls:   []string{"api/datasources/1", "/api/datasources/uid/1"},
+			method: http.MethodPut,
+			body:   `{"name": "test", "url": "http://localhost:5432", "type": "postgresql", "access": "Proxy"}`,
+			permission: []ac.Permission{
+				{Action: datasources.ActionWrite, Scope: datasources.ScopeProvider.GetResourceScope("1")},
+				{Action: datasources.ActionWrite, Scope: datasources.ScopeProvider.GetResourceScopeUID("1")},
 			},
-			expectedSQLError: datasources.ErrDataSourceNotFound,
+			expectedCode: http.StatusOK,
 		},
 		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusOK,
-				desc:         "DatasourcesGet should return 200 for user with correct permissions",
-				url:          "/api/datasources/",
-				method:       http.MethodGet,
-				permissions:  []ac.Permission{{Action: datasources.ActionRead, Scope: datasources.ScopeAll}},
-			},
+			desc:         "should not be able to update datasource without correct permission",
+			urls:         []string{"api/datasources/1", "/api/datasources/uid/1"},
+			method:       http.MethodPut,
+			permission:   []ac.Permission{},
+			expectedCode: http.StatusForbidden,
 		},
 		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusForbidden,
-				desc:         "DatasourcesGet should return 403 for user without required permissions",
-				url:          "/api/datasources/",
-				method:       http.MethodGet,
-				permissions:  []ac.Permission{{Action: "wrong"}},
+			desc:   "should be able to fetch datasource with correct permission",
+			urls:   []string{"api/datasources/1", "/api/datasources/uid/1", "/api/datasources/name/test"},
+			method: http.MethodGet,
+			permission: []ac.Permission{
+				{Action: datasources.ActionRead, Scope: datasources.ScopeProvider.GetResourceScope("1")},
+				{Action: datasources.ActionRead, Scope: datasources.ScopeProvider.GetResourceScopeUID("1")},
+				{Action: datasources.ActionRead, Scope: datasources.ScopeProvider.GetResourceScopeName("test")},
 			},
+			expectedCode: http.StatusOK,
 		},
 		{
-			body: addDatasourceBody,
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusOK,
-				desc:         "DatasourcesPost should return 200 for user with correct permissions",
-				url:          "/api/datasources/",
-				method:       http.MethodPost,
-				permissions:  []ac.Permission{{Action: datasources.ActionCreate}},
-			},
-			expectedDS: &testDatasource,
+			desc:         "should not be able to fetch datasource without correct permission",
+			urls:         []string{"api/datasources/1", "/api/datasources/uid/1"},
+			method:       http.MethodGet,
+			permission:   []ac.Permission{},
+			expectedCode: http.StatusForbidden,
 		},
 		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusForbidden,
-				desc:         "DatasourcesPost should return 403 for user without required permissions",
-				url:          "/api/datasources/",
-				method:       http.MethodPost,
-				permissions:  []ac.Permission{{Action: "wrong"}},
-			},
+			desc:         "should be able to create datasource with correct permission",
+			urls:         []string{"/api/datasources"},
+			method:       http.MethodPost,
+			body:         `{"name": "test", "url": "http://localhost:5432", "type": "postgresql", "access": "Proxy"}`,
+			permission:   []ac.Permission{{Action: datasources.ActionCreate}},
+			expectedCode: http.StatusOK,
 		},
 		{
-			body: updateDatasourceBody,
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusOK,
-				desc:         "DatasourcesPut should return 200 for user with correct permissions",
-				url:          fmt.Sprintf("/api/datasources/%v", testDatasource.Id),
-				method:       http.MethodPut,
-				permissions: []ac.Permission{
-					{
-						Action: datasources.ActionWrite,
-						Scope:  fmt.Sprintf("datasources:id:%v", testDatasource.Id),
-					},
-				},
-			},
-			expectedDS: &testDatasource,
+			desc:         "should not be able to create datasource without correct permission",
+			urls:         []string{"/api/datasources"},
+			method:       http.MethodPost,
+			permission:   []ac.Permission{},
+			expectedCode: http.StatusForbidden,
 		},
 		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusForbidden,
-				desc:         "DatasourcesPut should return 403 for user without required permissions",
-				url:          fmt.Sprintf("/api/datasources/%v", testDatasource.Id),
-				method:       http.MethodPut,
-				permissions:  []ac.Permission{{Action: "wrong"}},
+			desc:   "should be able to delete datasource with correct permission",
+			urls:   []string{"/api/datasources/1", "/api/datasources/uid/1"},
+			method: http.MethodDelete,
+			permission: []ac.Permission{
+				{Action: datasources.ActionDelete, Scope: datasources.ScopeProvider.GetResourceScope("1")},
+				{Action: datasources.ActionDelete, Scope: datasources.ScopeProvider.GetResourceScopeUID("1")},
 			},
+			expectedCode: http.StatusOK,
 		},
 		{
-			body: updateDatasourceBody,
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusForbidden,
-				desc:         "DatasourcesPut should return 403 for read only datasource",
-				url:          fmt.Sprintf("/api/datasources/%v", testDatasourceReadOnly.Id),
-				method:       http.MethodPut,
-				permissions: []ac.Permission{
-					{
-						Action: datasources.ActionWrite,
-						Scope:  fmt.Sprintf("datasources:id:%v", testDatasourceReadOnly.Id),
-					},
-				},
-			},
-			expectedDS: &testDatasourceReadOnly,
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusOK,
-				desc:         "DatasourcesDeleteByID should return 200 for user with correct permissions",
-				url:          fmt.Sprintf("/api/datasources/%v", testDatasource.Id),
-				method:       http.MethodDelete,
-				permissions: []ac.Permission{
-					{
-						Action: datasources.ActionDelete,
-						Scope:  fmt.Sprintf("datasources:id:%v", testDatasource.Id),
-					},
-				},
-			},
-			expectedDS: &testDatasource,
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusForbidden,
-				desc:         "DatasourcesDeleteByID should return 403 for user without required permissions",
-				url:          fmt.Sprintf("/api/datasources/%v", testDatasource.Id),
-				method:       http.MethodDelete,
-				permissions:  []ac.Permission{{Action: "wrong"}},
-			},
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusOK,
-				desc:         "DatasourcesDeleteByUID should return 200 for user with correct permissions",
-				url:          fmt.Sprintf("/api/datasources/uid/%v", testDatasource.Uid),
-				method:       http.MethodDelete,
-				permissions: []ac.Permission{
-					{
-						Action: datasources.ActionDelete,
-						Scope:  fmt.Sprintf("datasources:uid:%v", testDatasource.Uid),
-					},
-				},
-			},
-			expectedDS: &testDatasource,
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusForbidden,
-				desc:         "DatasourcesDeleteByUID should return 403 for user without required permissions",
-				url:          fmt.Sprintf("/api/datasources/uid/%v", testDatasource.Uid),
-				method:       http.MethodDelete,
-				permissions:  []ac.Permission{{Action: "wrong"}},
-			},
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusOK,
-				desc:         "DatasourcesDeleteByName should return 200 for user with correct permissions",
-				url:          fmt.Sprintf("/api/datasources/name/%v", testDatasource.Name),
-				method:       http.MethodDelete,
-				permissions: []ac.Permission{
-					{
-						Action: datasources.ActionDelete,
-						Scope:  fmt.Sprintf("datasources:name:%v", testDatasource.Name),
-					},
-				},
-			},
-			expectedDS: &testDatasource,
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusForbidden,
-				desc:         "DatasourcesDeleteByName should return 403 for user without required permissions",
-				url:          fmt.Sprintf("/api/datasources/name/%v", testDatasource.Name),
-				method:       http.MethodDelete,
-				permissions:  []ac.Permission{{Action: "wrong"}},
-			},
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusOK,
-				desc:         "DatasourcesGetByID should return 200 for user with correct permissions",
-				url:          fmt.Sprintf("/api/datasources/%v", testDatasource.Id),
-				method:       http.MethodGet,
-				permissions: []ac.Permission{
-					{
-						Action: datasources.ActionRead,
-						Scope:  fmt.Sprintf("datasources:id:%v", testDatasource.Id),
-					},
-				},
-			},
-			expectedDS: &testDatasource,
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusForbidden,
-				desc:         "DatasourcesGetByID should return 403 for user without required permissions",
-				url:          fmt.Sprintf("/api/datasources/%v", testDatasource.Id),
-				method:       http.MethodGet,
-				permissions:  []ac.Permission{{Action: "wrong"}},
-			},
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusOK,
-				desc:         "DatasourcesGetByUID should return 200 for user with correct permissions",
-				url:          fmt.Sprintf("/api/datasources/uid/%v", testDatasource.Uid),
-				method:       http.MethodGet,
-				permissions: []ac.Permission{
-					{
-						Action: datasources.ActionRead,
-						Scope:  fmt.Sprintf("datasources:uid:%v", testDatasource.Uid),
-					},
-				},
-			},
-			expectedDS: &testDatasource,
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusForbidden,
-				desc:         "DatasourcesGetByUID should return 403 for user without required permissions",
-				url:          fmt.Sprintf("/api/datasources/uid/%v", testDatasource.Uid),
-				method:       http.MethodGet,
-				permissions:  []ac.Permission{{Action: "wrong"}},
-			},
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusOK,
-				desc:         "DatasourcesGetByName should return 200 for user with correct permissions",
-				url:          fmt.Sprintf("/api/datasources/name/%v", testDatasource.Name),
-				method:       http.MethodGet,
-				permissions: []ac.Permission{
-					{
-						Action: datasources.ActionRead,
-						Scope:  fmt.Sprintf("datasources:name:%v", testDatasource.Name),
-					},
-				},
-			},
-			expectedDS: &testDatasource,
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusForbidden,
-				desc:         "DatasourcesGetByName should return 403 for user without required permissions",
-				url:          fmt.Sprintf("/api/datasources/name/%v", testDatasource.Name),
-				method:       http.MethodGet,
-				permissions:  []ac.Permission{{Action: "wrong"}},
-			},
-			expectedDS: &testDatasource,
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusOK,
-				desc:         "DatasourcesGetIdByName should return 200 for user with correct permissions",
-				url:          fmt.Sprintf("/api/datasources/id/%v", testDatasource.Name),
-				method:       http.MethodGet,
-				permissions: []ac.Permission{
-					{
-						Action: datasources.ActionIDRead,
-						Scope:  fmt.Sprintf("datasources:name:%v", testDatasource.Name),
-					},
-				},
-			},
-			expectedDS: &testDatasource,
-		},
-		{
-			accessControlTestCase: accessControlTestCase{
-				expectedCode: http.StatusForbidden,
-				desc:         "DatasourcesGetIdByName should return 403 for user without required permissions",
-				url:          fmt.Sprintf("/api/datasources/id/%v", testDatasource.Name),
-				method:       http.MethodGet,
-				permissions:  []ac.Permission{{Action: "wrong"}},
-			},
-			expectedDS: &testDatasource,
+			desc:         "should not be able to delete datasource without correct permission",
+			urls:         []string{"/api/datasources/1", "/api/datasources/uid/1"},
+			method:       http.MethodDelete,
+			permission:   []ac.Permission{},
+			expectedCode: http.StatusForbidden,
 		},
 	}
 
-	for _, test := range tests {
-		t.Run(test.desc, func(t *testing.T) {
-			cfg := setting.NewCfg()
-			sc, hs := setupAccessControlScenarioContext(t, cfg, test.url, test.permissions)
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			server := SetupAPITestServer(t, func(hs *HTTPServer) {
+				hs.Cfg = setting.NewCfg()
+				hs.DataSourcesService = &dataSourcesServiceMock{expectedDatasource: &datasources.DataSource{}}
+				hs.accesscontrolService = actest.FakeService{}
+				hs.Live = newTestLive(t, hs.SQLStore)
+			})
 
-			// mock sqlStore and datasource permission service
-			dsServiceMock.expectedError = test.expectedSQLError
-			dsServiceMock.expectedDatasource = test.expectedDS
-			dsPermissionService.DsResult = []*datasources.DataSource{test.expectedDS}
-			if test.expectedDS == nil {
-				dsPermissionService.DsResult = nil
+			for _, url := range tt.urls {
+				var body io.Reader
+				if tt.body != "" {
+					body = strings.NewReader(tt.body)
+				}
+
+				res, err := server.SendJSON(webtest.RequestWithSignedInUser(server.NewRequest(tt.method, url, body), userWithPermissions(1, tt.permission)))
+				require.NoError(t, err)
+				assert.Equal(t, tt.expectedCode, res.StatusCode)
+				require.NoError(t, res.Body.Close())
 			}
-			hs.DataSourcesService = dsServiceMock
-			hs.DatasourcePermissionsService = dsPermissionService
-
-			// Create a middleware to pretend user is logged in
-			pretendSignInMiddleware := func(c *models.ReqContext) {
-				sc.context = c
-				sc.context.UserID = testUserID
-				sc.context.OrgID = testOrgID
-				sc.context.Login = testUserLogin
-				sc.context.OrgRole = org.RoleViewer
-				sc.context.IsSignedIn = true
-			}
-			sc.m.Use(pretendSignInMiddleware)
-
-			sc.resp = httptest.NewRecorder()
-			hs.SettingsProvider = &setting.OSSImpl{Cfg: cfg}
-
-			var err error
-			if test.body != nil {
-				sc.req, err = http.NewRequest(test.method, test.url, test.body())
-				sc.req.Header.Add("Content-Type", "application/json")
-			} else {
-				sc.req, err = http.NewRequest(test.method, test.url, nil)
-			}
-
-			assert.NoError(t, err)
-
-			sc.exec()
-			assert.Equal(t, test.expectedCode, sc.resp.Code)
 		})
 	}
 }