AccessControl: Protect org users lookup (#38981)

* Move legacy accesscontrol to middleware layer

* Remove bus usage for this endpoint

* Add tests for legacy accesscontrol

* Fix tests for org user and remove one more bus usage

* Added test for FolderAdmin as suggested in the review

pkg/api/api.go

@@ -28,6 +28,7 @@ func (hs *HTTPServer) registerRoutes() {
 	reqGrafanaAdmin := middleware.ReqGrafanaAdmin
 	reqEditorRole := middleware.ReqEditorRole
 	reqOrgAdmin := middleware.ReqOrgAdmin
+	reqOrgAdminFolderAdminOrTeamAdmin := middleware.OrgAdminFolderAdminOrTeamAdmin
 	reqCanAccessTeams := middleware.AdminOrFeatureEnabled(hs.Cfg.EditorsCanAdmin)
 	reqSnapshotPublicModeOrSignedIn := middleware.SnapshotPublicModeOrSignedIn(hs.Cfg)
 	redirectFromLegacyPanelEditURL := middleware.RedirectFromLegacyPanelEditURL(hs.Cfg)
@@ -226,7 +227,7 @@ func (hs *HTTPServer) registerRoutes() {
 
 		// current org without requirement of user to be org admin
 		apiRoute.Group("/org", func(orgRoute routing.RouteRegister) {
-			orgRoute.Get("/users/lookup", routing.Wrap(hs.GetOrgUsersForCurrentOrgLookup))
+			orgRoute.Get("/users/lookup", authorize(reqOrgAdminFolderAdminOrTeamAdmin, ac.EvalPermission(ac.ActionOrgUsersRead, ac.ScopeUsersAll)), routing.Wrap(hs.GetOrgUsersForCurrentOrgLookup))
 		})
 
 		// create new org

pkg/api/org_users.go

@@ -7,6 +7,7 @@ import (
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/util"
 )
 
@@ -70,15 +71,6 @@ func (hs *HTTPServer) GetOrgUsersForCurrentOrg(c *models.ReqContext) response.Re
 
 // GET /api/org/users/lookup
 func (hs *HTTPServer) GetOrgUsersForCurrentOrgLookup(c *models.ReqContext) response.Response {
-	isAdmin, err := isOrgAdminFolderAdminOrTeamAdmin(c)
-	if err != nil {
-		return response.Error(500, "Failed to get users for current organization", err)
-	}
-
-	if !isAdmin {
-		return response.Error(403, "Permission denied", nil)
-	}
-
 	orgUsers, err := hs.getOrgUsersHelper(&models.GetOrgUsersQuery{
 		OrgId: c.OrgId,
 		Query: c.Query("query"),
@@ -102,28 +94,6 @@ func (hs *HTTPServer) GetOrgUsersForCurrentOrgLookup(c *models.ReqContext) respo
 	return response.JSON(200, result)
 }
 
-func isOrgAdminFolderAdminOrTeamAdmin(c *models.ReqContext) (bool, error) {
-	if c.OrgRole == models.ROLE_ADMIN {
-		return true, nil
-	}
-
-	hasAdminPermissionInFoldersQuery := models.HasAdminPermissionInFoldersQuery{SignedInUser: c.SignedInUser}
-	if err := bus.Dispatch(&hasAdminPermissionInFoldersQuery); err != nil {
-		return false, err
-	}
-
-	if hasAdminPermissionInFoldersQuery.Result {
-		return true, nil
-	}
-
-	isAdminOfTeamsQuery := models.IsAdminOfTeamsQuery{SignedInUser: c.SignedInUser}
-	if err := bus.Dispatch(&isAdminOfTeamsQuery); err != nil {
-		return false, err
-	}
-
-	return isAdminOfTeamsQuery.Result, nil
-}
-
 // GET /api/orgs/:orgId/users
 func (hs *HTTPServer) GetOrgUsers(c *models.ReqContext) response.Response {
 	result, err := hs.getOrgUsersHelper(&models.GetOrgUsersQuery{
@@ -140,7 +110,7 @@ func (hs *HTTPServer) GetOrgUsers(c *models.ReqContext) response.Response {
 }
 
 func (hs *HTTPServer) getOrgUsersHelper(query *models.GetOrgUsersQuery, signedInUser *models.SignedInUser) ([]*models.OrgUserDTO, error) {
-	if err := bus.Dispatch(query); err != nil {
+	if err := sqlstore.GetOrgUsers(query); err != nil {
 		return nil, err
 	}
 

pkg/api/org_users_test.go

@@ -4,33 +4,28 @@ import (
 	"context"
 	"encoding/json"
 	"net/http"
+	"net/http/httptest"
 	"testing"
+	"time"
 
 	"github.com/grafana/grafana/pkg/api/dtos"
-	"github.com/grafana/grafana/pkg/bus"
+	"github.com/grafana/grafana/pkg/api/routing"
+	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
+	"github.com/grafana/grafana/pkg/services/quota"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-func setUpGetOrgUsersHandler() {
-	bus.AddHandler("test", func(query *models.GetOrgUsersQuery) error {
-		query.Result = []*models.OrgUserDTO{
-			{Email: "testUser@grafana.com", Login: testUserLogin},
-			{Email: "user1@grafana.com", Login: "user1"},
-			{Email: "user2@grafana.com", Login: "user2"},
-		}
-		return nil
-	})
-}
-
 func setUpGetOrgUsersDB(t *testing.T, sqlStore *sqlstore.SQLStore) {
 	setting.AutoAssignOrg = true
-	setting.AutoAssignOrgId = 1
+	setting.AutoAssignOrgId = int(testOrgID)
 
-	_, err := sqlStore.CreateUser(context.Background(), models.CreateUserCommand{Email: "testUser@grafana.com", Login: "testUserLogin"})
+	_, err := sqlStore.CreateUser(context.Background(), models.CreateUserCommand{Email: "testUser@grafana.com", Login: testUserLogin})
 	require.NoError(t, err)
 	_, err = sqlStore.CreateUser(context.Background(), models.CreateUserCommand{Email: "user1@grafana.com", Login: "user1"})
 	require.NoError(t, err)
@@ -45,7 +40,7 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) {
 	sqlStore := sqlstore.InitTestDB(t)
 
 	loggedInUserScenario(t, "When calling GET on", "api/org/users", func(sc *scenarioContext) {
-		setUpGetOrgUsersHandler()
+		setUpGetOrgUsersDB(t, sqlStore)
 
 		sc.handlerFunc = hs.GetOrgUsersForCurrentOrg
 		sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
@@ -94,48 +89,7 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) {
 		assert.Equal(t, 2, resp.Page)
 	})
 
-	loggedInUserScenario(t, "When calling GET as an editor with no team / folder permissions on",
+	t.Run("Given there are two hidden users", func(t *testing.T) {
-		"api/org/users/lookup", func(sc *scenarioContext) {
-			setUpGetOrgUsersHandler()
-			bus.AddHandler("test", func(query *models.HasAdminPermissionInFoldersQuery) error {
-				query.Result = false
-				return nil
-			})
-			bus.AddHandler("test", func(query *models.IsAdminOfTeamsQuery) error {
-				query.Result = false
-				return nil
-			})
-
-			sc.handlerFunc = hs.GetOrgUsersForCurrentOrgLookup
-			sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
-
-			assert.Equal(t, http.StatusForbidden, sc.resp.Code)
-
-			var resp struct {
-				Message string
-			}
-			err := json.Unmarshal(sc.resp.Body.Bytes(), &resp)
-			require.NoError(t, err)
-
-			assert.Equal(t, "Permission denied", resp.Message)
-		})
-
-	loggedInUserScenarioWithRole(t, "When calling GET as an admin on", "GET", "api/org/users/lookup",
-		"api/org/users/lookup", models.ROLE_ADMIN, func(sc *scenarioContext) {
-			setUpGetOrgUsersHandler()
-
-			sc.handlerFunc = hs.GetOrgUsersForCurrentOrgLookup
-			sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
-
-			require.Equal(t, http.StatusOK, sc.resp.Code)
-
-			var resp []dtos.UserLookupDTO
-			err := json.Unmarshal(sc.resp.Body.Bytes(), &resp)
-			require.NoError(t, err)
-			assert.Len(t, resp, 3)
-		})
-
-	t.Run("Given there is two hidden users", func(t *testing.T) {
 		settings.HiddenUsers = map[string]struct{}{
 			"user1":       {},
 			testUserLogin: {},
@@ -143,7 +97,7 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) {
 		t.Cleanup(func() { settings.HiddenUsers = make(map[string]struct{}) })
 
 		loggedInUserScenario(t, "When calling GET on", "api/org/users", func(sc *scenarioContext) {
-			setUpGetOrgUsersHandler()
+			setUpGetOrgUsersDB(t, sqlStore)
 
 			sc.handlerFunc = hs.GetOrgUsersForCurrentOrg
 			sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
@@ -160,7 +114,7 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) {
 
 		loggedInUserScenarioWithRole(t, "When calling GET as an admin on", "GET", "api/org/users/lookup",
 			"api/org/users/lookup", models.ROLE_ADMIN, func(sc *scenarioContext) {
-				setUpGetOrgUsersHandler()
+				setUpGetOrgUsersDB(t, sqlStore)
 
 				sc.handlerFunc = hs.GetOrgUsersForCurrentOrgLookup
 				sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec()
@@ -176,3 +130,164 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) {
 			})
 	})
 }
+
+func setupOrgUsersAPIcontext(t *testing.T, role models.RoleType) (*scenarioContext, *sqlstore.SQLStore) {
+	cfg := setting.NewCfg()
+	db := sqlstore.InitTestDB(t)
+
+	hs := &HTTPServer{
+		Cfg:           cfg,
+		QuotaService:  &quota.QuotaService{Cfg: cfg},
+		RouteRegister: routing.NewRouteRegister(),
+		AccessControl: accesscontrolmock.New().WithDisabled(),
+		SQLStore:      db,
+	}
+
+	sc := setupScenarioContext(t, "/api/org/users/lookup")
+	// 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 = role
+		sc.context.IsSignedIn = true
+	}
+	sc.m.Use(pretendSignInMiddleware)
+
+	hs.registerRoutes()
+	hs.RouteRegister.Register(sc.m.Router)
+
+	return sc, db
+}
+
+func TestOrgUsersAPIEndpoint_LegacyAccessControl_FolderAdmin(t *testing.T) {
+	sc, db := setupOrgUsersAPIcontext(t, models.ROLE_VIEWER)
+
+	// Create a dashboard folder
+	cmd := models.SaveDashboardCommand{
+		OrgId:    testOrgID,
+		FolderId: 1,
+		IsFolder: true,
+		Dashboard: simplejson.NewFromAny(map[string]interface{}{
+			"id":    nil,
+			"title": "1 test dash folder",
+			"tags":  "prod",
+		}),
+	}
+	folder, err := db.SaveDashboard(cmd)
+	require.NoError(t, err)
+	require.NotNil(t, folder)
+
+	// Grant our test Viewer with permission to admin the folder
+	acls := []*models.DashboardAcl{
+		{
+			DashboardID: folder.Id,
+			OrgID:       testOrgID,
+			UserID:      testUserID,
+			Permission:  models.PERMISSION_ADMIN,
+			Created:     time.Now(),
+			Updated:     time.Now(),
+		},
+	}
+	err = db.UpdateDashboardACL(folder.Id, acls)
+	require.NoError(t, err)
+
+	sc.resp = httptest.NewRecorder()
+
+	sc.req, err = http.NewRequest(http.MethodGet, "/api/org/users/lookup", nil)
+	require.NoError(t, err)
+
+	sc.exec()
+	assert.Equal(t, http.StatusOK, sc.resp.Code)
+}
+
+func TestOrgUsersAPIEndpoint_LegacyAccessControl_TeamAdmin(t *testing.T) {
+	sc, db := setupOrgUsersAPIcontext(t, models.ROLE_VIEWER)
+
+	// Setup store teams
+	team1, err := db.CreateTeam("testteam1", "testteam1@example.org", testOrgID)
+	require.NoError(t, err)
+	err = db.AddTeamMember(testUserID, testOrgID, team1.Id, false, models.PERMISSION_ADMIN)
+	require.NoError(t, err)
+
+	sc.resp = httptest.NewRecorder()
+
+	sc.req, err = http.NewRequest(http.MethodGet, "/api/org/users/lookup", nil)
+	require.NoError(t, err)
+
+	sc.exec()
+	assert.Equal(t, http.StatusOK, sc.resp.Code)
+}
+
+func TestOrgUsersAPIEndpoint_LegacyAccessControl_Admin(t *testing.T) {
+	sc, _ := setupOrgUsersAPIcontext(t, models.ROLE_ADMIN)
+	sc.resp = httptest.NewRecorder()
+
+	var err error
+	sc.req, err = http.NewRequest(http.MethodGet, "/api/org/users/lookup", nil)
+	require.NoError(t, err)
+
+	sc.exec()
+	assert.Equal(t, http.StatusOK, sc.resp.Code)
+}
+
+func TestOrgUsersAPIEndpoint_LegacyAccessControl_Viewer(t *testing.T) {
+	sc, _ := setupOrgUsersAPIcontext(t, models.ROLE_VIEWER)
+	sc.resp = httptest.NewRecorder()
+
+	var err error
+	sc.req, err = http.NewRequest(http.MethodGet, "/api/org/users/lookup", nil)
+	require.NoError(t, err)
+
+	sc.exec()
+	assert.Equal(t, http.StatusForbidden, sc.resp.Code)
+}
+
+func TestOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
+	tests := []accessControlTestCase{
+		{
+			expectedCode: http.StatusOK,
+			desc:         "UsersLookupGet should return 200 for user with correct permissions",
+			url:          "/api/org/users/lookup",
+			method:       http.MethodGet,
+			permissions:  []*accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll}},
+		},
+		{
+
+			expectedCode: http.StatusForbidden,
+			desc:         "UsersLookupGet should return 403 for user without required permissions",
+			url:          "/api/org/users/lookup",
+			method:       http.MethodGet,
+			permissions:  []*accesscontrol.Permission{{Action: "wrong"}},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			cfg := setting.NewCfg()
+			sc, hs := setupAccessControlScenarioContext(t, cfg, test.url, test.permissions)
+
+			// 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 = models.ROLE_VIEWER
+				sc.context.IsSignedIn = true
+			}
+			sc.m.Use(pretendSignInMiddleware)
+
+			sc.resp = httptest.NewRecorder()
+			hs.SettingsProvider = &setting.OSSImpl{Cfg: cfg}
+
+			var err error
+			sc.req, err = http.NewRequest(test.method, test.url, nil)
+			require.NoError(t, err)
+
+			sc.exec()
+			assert.Equal(t, test.expectedCode, sc.resp.Code)
+		})
+	}
+}

pkg/middleware/auth.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/middleware/cookies"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
@@ -188,3 +189,29 @@ func shouldForceLogin(c *models.ReqContext) bool {
 
 	return forceLogin
 }
+
+func OrgAdminFolderAdminOrTeamAdmin(c *models.ReqContext) {
+	if c.OrgRole == models.ROLE_ADMIN {
+		return
+	}
+
+	hasAdminPermissionInFoldersQuery := models.HasAdminPermissionInFoldersQuery{SignedInUser: c.SignedInUser}
+	if err := sqlstore.HasAdminPermissionInFolders(&hasAdminPermissionInFoldersQuery); err != nil {
+		c.JsonApiErr(500, "Failed to check if user is a folder admin", err)
+	}
+
+	if hasAdminPermissionInFoldersQuery.Result {
+		return
+	}
+
+	isAdminOfTeamsQuery := models.IsAdminOfTeamsQuery{SignedInUser: c.SignedInUser}
+	if err := sqlstore.IsAdminOfTeams(&isAdminOfTeamsQuery); err != nil {
+		c.JsonApiErr(500, "Failed to check if user is a team admin", err)
+	}
+
+	if isAdminOfTeamsQuery.Result {
+		return
+	}
+
+	accessForbidden(c)
+}