AccessControl: Remove acmock.New from accesscontrol service tests (#71942)

* remove mock ac provider from service accounts

* remove mock ac provider from accesscontrol tests

* remove mock ac from ac service tests

pkg/services/accesscontrol/middleware_test.go

@@ -9,37 +9,37 @@ import (
 
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
-	"github.com/grafana/grafana/pkg/services/accesscontrol/mock"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
 	"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/user"
+	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/web"
 )
 
 type middlewareTestCase struct {
-	desc           string
-	expectEndpoint bool
-	evaluator      accesscontrol.Evaluator
-	ac             accesscontrol.AccessControl
+	desc            string
+	expectEndpoint  bool
+	evaluator       accesscontrol.Evaluator
+	ctxSignedInUser *user.SignedInUser
 }
 
 func TestMiddleware(t *testing.T) {
+	cfg := setting.NewCfg()
+	ac := acimpl.ProvideAccessControl(cfg)
+
 	tests := []middlewareTestCase{
 		{
-			desc: "should pass middleware for correct permissions",
-			ac: mock.New().WithPermissions(
-				[]accesscontrol.Permission{{Action: "users:read", Scope: "users:*"}},
-			),
-			evaluator:      accesscontrol.EvalPermission("users:read", "users:*"),
-			expectEndpoint: true,
+			desc:            "should pass middleware for correct permissions",
+			evaluator:       accesscontrol.EvalPermission("users:read", "users:*"),
+			ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:*"}}}},
+			expectEndpoint:  true,
 		},
 		{
-			desc: "should not reach endpoint when missing permissions",
-			ac: mock.New().WithPermissions(
-				[]accesscontrol.Permission{{Action: "users:read", Scope: "users:1"}},
-			),
-			evaluator:      accesscontrol.EvalPermission("users:read", "users:*"),
-			expectEndpoint: false,
+			desc:            "should not reach endpoint when missing permissions",
+			ctxSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {"users:read": {"users:1"}}}},
+			evaluator:       accesscontrol.EvalPermission("users:read", "users:*"),
+			expectEndpoint:  false,
 		},
 	}
 
@@ -48,8 +48,12 @@ func TestMiddleware(t *testing.T) {
 			server := web.New()
 			server.UseMiddleware(web.Renderer("../../public/views", "[[", "]]"))
 
-			server.Use(contextProvider())
-			server.Use(accesscontrol.Middleware(test.ac)(test.evaluator))
+			server.Use(contextProvider(
+				func(c *contextmodel.ReqContext) {
+					c.SignedInUser = test.ctxSignedInUser
+				},
+			))
+			server.Use(accesscontrol.Middleware(ac)(test.evaluator))
 
 			endpointCalled := false
 			server.Get("/", func(c *contextmodel.ReqContext) {
@@ -78,39 +82,49 @@ func TestMiddleware_forceLogin(t *testing.T) {
 		{url: "/endpoint"},
 	}
 
+	cfg := setting.NewCfg()
+	ac := acimpl.ProvideAccessControl(cfg)
+
 	for _, tc := range tests {
-		var endpointCalled bool
+		t.Run(tc.url, func(t *testing.T) {
+			var endpointCalled bool
 
-		server := web.New()
-		server.UseMiddleware(web.Renderer("../../public/views", "[[", "]]"))
+			server := web.New()
+			server.UseMiddleware(web.Renderer("../../public/views", "[[", "]]"))
 
-		server.Get("/endpoint", func(c *contextmodel.ReqContext) {
-			endpointCalled = true
-			c.Resp.WriteHeader(http.StatusOK)
+			server.Get("/endpoint", func(c *contextmodel.ReqContext) {
+				endpointCalled = true
+				c.Resp.WriteHeader(http.StatusOK)
+			})
+
+			user := &user.SignedInUser{UserID: 1,
+				OrgID:       1,
+				IsAnonymous: true,
+				Permissions: map[int64]map[string][]string{1: {"endpoint:read": {"endpoint:1"}}}}
+
+			server.Use(contextProvider(func(c *contextmodel.ReqContext) {
+				c.AllowAnonymous = true
+				c.SignedInUser = user
+				c.IsSignedIn = false
+			}))
+
+			server.Use(
+				accesscontrol.Middleware(ac)(accesscontrol.EvalPermission("endpoint:read", "endpoint:1")),
+			)
+
+			request, err := http.NewRequest(http.MethodGet, tc.url, nil)
+			assert.NoError(t, err)
+			recorder := httptest.NewRecorder()
+
+			server.ServeHTTP(recorder, request)
+
+			expectedCode := http.StatusOK
+			if tc.redirectToLogin {
+				expectedCode = http.StatusFound
+			}
+			assert.Equal(t, expectedCode, recorder.Code)
+			assert.Equal(t, !tc.redirectToLogin, endpointCalled, "/endpoint should be called")
 		})
-
-		ac := mock.New().WithPermissions([]accesscontrol.Permission{{Action: "endpoint:read", Scope: "endpoint:1"}})
-		server.Use(contextProvider(func(c *contextmodel.ReqContext) {
-			c.AllowAnonymous = true
-			c.SignedInUser.IsAnonymous = true
-			c.IsSignedIn = false
-		}))
-		server.Use(
-			accesscontrol.Middleware(ac)(accesscontrol.EvalPermission("endpoint:read", "endpoint:1")),
-		)
-
-		request, err := http.NewRequest(http.MethodGet, tc.url, nil)
-		assert.NoError(t, err)
-		recorder := httptest.NewRecorder()
-
-		server.ServeHTTP(recorder, request)
-
-		expectedCode := http.StatusOK
-		if tc.redirectToLogin {
-			expectedCode = http.StatusFound
-		}
-		assert.Equal(t, expectedCode, recorder.Code)
-		assert.Equal(t, !tc.redirectToLogin, endpointCalled, "/endpoint should be called?")
 	}
 }
 

pkg/services/accesscontrol/resourcepermissions/api_test.go

@@ -117,8 +117,8 @@ func TestApi_getDescription(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, _, _ := setupTestEnvironment(t, tt.permissions, tt.options)
-			server := setupTestServer(t, &user.SignedInUser{OrgID: 1}, service)
+			service, _, _ := setupTestEnvironment(t, tt.options)
+			server := setupTestServer(t, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service)
 
 			req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("/api/access-control/%s/description", tt.options.Resource), nil)
 			require.NoError(t, err)
@@ -164,7 +164,7 @@ func TestApi_getPermissions(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, sql, _ := setupTestEnvironment(t, tt.permissions, testOptions)
+			service, sql, _ := setupTestEnvironment(t, testOptions)
 			server := setupTestServer(t, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service)
 
 			seedPermissions(t, tt.resourceID, sql, service)
@@ -241,7 +241,7 @@ func TestApi_setBuiltinRolePermission(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, _, _ := setupTestEnvironment(t, tt.permissions, testOptions)
+			service, _, _ := setupTestEnvironment(t, testOptions)
 			server := setupTestServer(t, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service)
 
 			recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "builtInRoles", tt.builtInRole)
@@ -319,7 +319,7 @@ func TestApi_setTeamPermission(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, _, teamSvc := setupTestEnvironment(t, tt.permissions, testOptions)
+			service, _, teamSvc := setupTestEnvironment(t, testOptions)
 			server := setupTestServer(t, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service)
 
 			// seed team
@@ -402,7 +402,7 @@ func TestApi_setUserPermission(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, sql, _ := setupTestEnvironment(t, tt.permissions, testOptions)
+			service, sql, _ := setupTestEnvironment(t, testOptions)
 			server := setupTestServer(t, &user.SignedInUser{
 				OrgID:       1,
 				Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)},

pkg/services/accesscontrol/resourcepermissions/service_test.go

@@ -10,7 +10,8 @@ import (
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
-	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
 	"github.com/grafana/grafana/pkg/services/licensing/licensingtest"
 	"github.com/grafana/grafana/pkg/services/org/orgimpl"
 	"github.com/grafana/grafana/pkg/services/quota/quotatest"
@@ -42,7 +43,7 @@ func TestService_SetUserPermission(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, sql, _ := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{
+			service, sql, _ := setupTestEnvironment(t, Options{
 				Resource:             "dashboards",
 				Assignments:          Assignments{Users: true},
 				PermissionsToActions: nil,
@@ -90,7 +91,7 @@ func TestService_SetTeamPermission(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, _, teamSvc := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{
+			service, _, teamSvc := setupTestEnvironment(t, Options{
 				Resource:             "dashboards",
 				Assignments:          Assignments{Teams: true},
 				PermissionsToActions: nil,
@@ -134,7 +135,7 @@ func TestService_SetBuiltInRolePermission(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, _, _ := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{
+			service, _, _ := setupTestEnvironment(t, Options{
 				Resource:             "dashboards",
 				Assignments:          Assignments{BuiltInRoles: true},
 				PermissionsToActions: nil,
@@ -207,7 +208,7 @@ func TestService_SetPermissions(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, sql, teamSvc := setupTestEnvironment(t, []accesscontrol.Permission{}, tt.options)
+			service, sql, teamSvc := setupTestEnvironment(t, tt.options)
 
 			// seed user
 			orgSvc, err := orgimpl.ProvideService(sql, sql.Cfg, quotatest.New(false, nil))
@@ -230,7 +231,7 @@ func TestService_SetPermissions(t *testing.T) {
 	}
 }
 
-func setupTestEnvironment(t *testing.T, permissions []accesscontrol.Permission, ops Options) (*Service, *sqlstore.SQLStore, team.Service) {
+func setupTestEnvironment(t *testing.T, ops Options) (*Service, *sqlstore.SQLStore, team.Service) {
 	t.Helper()
 
 	sql := db.InitTestDB(t)
@@ -240,10 +241,11 @@ func setupTestEnvironment(t *testing.T, permissions []accesscontrol.Permission,
 	require.NoError(t, err)
 	license := licensingtest.NewFakeLicensing()
 	license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe()
-	mock := accesscontrolmock.New().WithPermissions(permissions)
+	ac := acimpl.ProvideAccessControl(cfg)
+	acService := &actest.FakeService{}
 	service, err := New(
 		ops, cfg, routing.NewRouteRegister(), license,
-		accesscontrolmock.New().WithPermissions(permissions), mock, sql, teamSvc, userSvc,
+		ac, acService, sql, teamSvc, userSvc,
 	)
 	require.NoError(t, err)
 

pkg/services/publicdashboards/service/service_test.go

@@ -10,10 +10,14 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	dashboardsDB "github.com/grafana/grafana/pkg/services/dashboards/database"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
@@ -22,14 +26,11 @@ import (
 	. "github.com/grafana/grafana/pkg/services/publicdashboards/models"
 	"github.com/grafana/grafana/pkg/services/publicdashboards/validation"
 	"github.com/grafana/grafana/pkg/services/quota/quotatest"
-	"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
 	"github.com/grafana/grafana/pkg/services/tag/tagimpl"
 	"github.com/grafana/grafana/pkg/services/user"
+	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/tsdb/intervalv2"
 	"github.com/grafana/grafana/pkg/util"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/mock"
-	"github.com/stretchr/testify/require"
 )
 
 var timeSettings = &TimeSettings{From: "now-12h", To: "now"}
@@ -989,12 +990,7 @@ func TestPublicDashboardServiceImpl_ListPublicDashboards(t *testing.T) {
 		},
 	}
 
-	ac := tests.SetupMockAccesscontrol(t,
-		func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-			return []accesscontrol.Permission{}, nil
-		},
-		false,
-	)
+	ac := acimpl.ProvideAccessControl(setting.NewCfg())
 
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {

pkg/services/serviceaccounts/tests/common.go

@@ -7,8 +7,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/infra/db"
-	"github.com/grafana/grafana/pkg/services/accesscontrol"
-	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
 	"github.com/grafana/grafana/pkg/services/apikey"
 	"github.com/grafana/grafana/pkg/services/apikey/apikeyimpl"
 	"github.com/grafana/grafana/pkg/services/org"
@@ -104,15 +102,3 @@ func SetupApiKey(t *testing.T, sqlStore *sqlstore.SQLStore, testKey TestApiKey)
 
 	return key
 }
-
-func SetupMockAccesscontrol(t *testing.T,
-	userpermissionsfunc func(c context.Context, siu *user.SignedInUser, opt accesscontrol.Options) ([]accesscontrol.Permission, error),
-	disableAccessControl bool) *accesscontrolmock.Mock {
-	t.Helper()
-	acmock := accesscontrolmock.New()
-	if disableAccessControl {
-		acmock = acmock.WithDisabled()
-	}
-	acmock.GetUserPermissionsFunc = userpermissionsfunc
-	return acmock
-}