Access Control: Add fine-grained access control to ldap handlers (#35525)

* Add new accesscontrol action for ldap config reload

* Update ldapAdminEditRole with new ldap config reload permission

* wrap /ldap/reload with accesscontrol authorize middleware

* document new action and update fixed:ldap:admin:edit with said action

* add fake accesscontrol implementation for tests

* Add accesscontrol tests for ldap handlers

Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com>

docs/sources/enterprise/access-control/fine-grained-access-control-references.md

@@ -21,7 +21,7 @@ Fixed roles | Permissions | Descriptions
 `fixed:users:org:read` | `org.users:read` | Allows to get user organizations.
 `fixed:users:org:edit` | All permissions from `fixed:users:org:read` and <br>`org.users:add`<br>`org.users:remove`<br>`org.users.role:update` | Allows every read action for user organizations and in addition allows to administer user organizations.
 `fixed:ldap:admin:read` | `ldap.user:read`<br>`ldap.status:read` | Allows to read LDAP information and status.
-`fixed:ldap:admin:edit` | All permissions from `fixed:ldap:admin:read` and <br>`ldap.user:sync` | Allows every read action for LDAP and in addition allows to administer LDAP.
+`fixed:ldap:admin:edit` | All permissions from `fixed:ldap:admin:read` and <br>`ldap.user:sync`<br>`ldap.config:reload` | Allows every read action for LDAP and in addition allows to administer LDAP.
 `fixed:settings:admin:edit` | `settings:write` | Update all settings
 
 ## Default built-in role assignments

docs/sources/enterprise/access-control/permissions.md

@@ -61,6 +61,7 @@ Actions | Applicable scopes | Descriptions
 `ldap.user:read` | n/a | Get a user via LDAP.
 `ldap.user:sync` | n/a | Sync a user via LDAP.
 `ldap.status:read` | n/a | Verify the LDAP servers’ availability.
+`ldap.config:reload` | n/a | Reload the LDAP configuration.
 `status:accesscontrol` | `service:accesscontrol` | Get access-control enabled status.
 `settings:write` | `settings:**`<br>`settings:auth.saml:*`<br>`settings:auth.saml:enabled` (property level) | Update settings
 

pkg/api/api.go

@@ -441,7 +441,7 @@ func (hs *HTTPServer) registerRoutes() {
 		adminRoute.Post("/provisioning/plugins/reload", reqGrafanaAdmin, routing.Wrap(hs.AdminProvisioningReloadPlugins))
 		adminRoute.Post("/provisioning/datasources/reload", reqGrafanaAdmin, routing.Wrap(hs.AdminProvisioningReloadDatasources))
 		adminRoute.Post("/provisioning/notifications/reload", reqGrafanaAdmin, routing.Wrap(hs.AdminProvisioningReloadNotifications))
-		adminRoute.Post("/ldap/reload", reqGrafanaAdmin, routing.Wrap(hs.ReloadLDAPCfg))
+		adminRoute.Post("/ldap/reload", authorize(reqGrafanaAdmin, accesscontrol.ActionLDAPConfigReload), routing.Wrap(hs.ReloadLDAPCfg))
 		adminRoute.Post("/ldap/sync/:id", authorize(reqGrafanaAdmin, accesscontrol.ActionLDAPUsersSync), routing.Wrap(hs.PostSyncUserWithLDAP))
 		adminRoute.Get("/ldap/:username", authorize(reqGrafanaAdmin, accesscontrol.ActionLDAPUsersRead), routing.Wrap(hs.GetUserFromLDAP))
 		adminRoute.Get("/ldap/status", authorize(reqGrafanaAdmin, accesscontrol.ActionLDAPStatusRead), routing.Wrap(hs.GetLDAPStatus))

pkg/api/common_test.go

@@ -1,12 +1,16 @@
 package api
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"path/filepath"
 	"testing"
 
+	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/evaluator"
+
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/bus"
@@ -229,3 +233,40 @@ type fakeRenderService struct {
 func (s *fakeRenderService) Init() error {
 	return nil
 }
+
+var _ accesscontrol.AccessControl = new(fakeAccessControl)
+
+type fakeAccessControl struct {
+	isDisabled  bool
+	permissions []*accesscontrol.Permission
+}
+
+func (f *fakeAccessControl) Evaluate(ctx context.Context, user *models.SignedInUser, permission string, scope ...string) (bool, error) {
+	return evaluator.Evaluate(ctx, f, user, permission, scope...)
+}
+
+func (f *fakeAccessControl) GetUserPermissions(ctx context.Context, user *models.SignedInUser) ([]*accesscontrol.Permission, error) {
+	return f.permissions, nil
+}
+
+func (f *fakeAccessControl) IsDisabled() bool {
+	return f.isDisabled
+}
+
+func setupAccessControlScenarioContext(t *testing.T, cfg *setting.Cfg, url string, permissions []*accesscontrol.Permission) *scenarioContext {
+	cfg.FeatureToggles = make(map[string]bool)
+	cfg.FeatureToggles["accesscontrol"] = true
+
+	hs := &HTTPServer{
+		Cfg:           cfg,
+		RouteRegister: routing.NewRouteRegister(),
+		AccessControl: &fakeAccessControl{permissions: permissions},
+	}
+
+	sc := setupScenarioContext(t, url)
+
+	hs.registerRoutes()
+	hs.RouteRegister.Register(sc.m.Router)
+
+	return sc
+}

pkg/api/ldap_debug_test.go

@@ -4,8 +4,11 @@ import (
 	"errors"
 	"net/http"
 	"net/http/httptest"
+	"path/filepath"
 	"testing"
 
+	"github.com/grafana/grafana/pkg/services/accesscontrol"
+
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/bus"
@@ -573,3 +576,142 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotInLDAP(t *testing.T) {
 
 	assert.JSONEq(t, expected, sc.resp.Body.String())
 }
+
+// ***
+// Access control tests for ldap endpoints
+// ***
+
+type ldapAccessControlTestCase struct {
+	expectedCode int
+	desc         string
+	url          string
+	method       string
+	permissions  []*accesscontrol.Permission
+}
+
+func TestLDAP_AccessControl(t *testing.T) {
+	tests := []ldapAccessControlTestCase{
+		{
+			url:          "/api/admin/ldap/reload",
+			method:       http.MethodPost,
+			desc:         "ReloadLDAPCfg should return 200 for user with correct permissions",
+			expectedCode: http.StatusOK,
+			permissions: []*accesscontrol.Permission{
+				{Action: accesscontrol.ActionLDAPConfigReload},
+			},
+		},
+		{
+			url:          "/api/admin/ldap/reload",
+			method:       http.MethodPost,
+			desc:         "ReloadLDAPCfg should return 403 for user without required permissions",
+			expectedCode: http.StatusForbidden,
+			permissions: []*accesscontrol.Permission{
+				{Action: "wrong"},
+			},
+		},
+		{
+			url:          "/api/admin/ldap/status",
+			method:       http.MethodGet,
+			desc:         "GetLDAPStatus should return 200 for user without required permissions",
+			expectedCode: http.StatusOK,
+			permissions: []*accesscontrol.Permission{
+				{Action: accesscontrol.ActionLDAPStatusRead},
+			},
+		},
+		{
+			url:          "/api/admin/ldap/status",
+			method:       http.MethodGet,
+			desc:         "GetLDAPStatus should return 200 for user without required permissions",
+			expectedCode: http.StatusForbidden,
+			permissions: []*accesscontrol.Permission{
+				{Action: "wrong"},
+			},
+		},
+		{
+			url:          "/api/admin/ldap/test",
+			method:       http.MethodGet,
+			desc:         "GetUserFromLDAP should return 200 for user with required permissions",
+			expectedCode: http.StatusOK,
+			permissions: []*accesscontrol.Permission{
+				{Action: accesscontrol.ActionLDAPUsersRead},
+			},
+		},
+		{
+			url:          "/api/admin/ldap/test",
+			method:       http.MethodGet,
+			desc:         "GetUserFromLDAP should return 403 for user without required permissions",
+			expectedCode: http.StatusForbidden,
+			permissions: []*accesscontrol.Permission{
+				{Action: "wrong"},
+			},
+		},
+		{
+			url:          "/api/admin/ldap/sync/test",
+			method:       http.MethodPost,
+			desc:         "PostSyncUserWithLDAP should return 200 for user without required permissions",
+			expectedCode: http.StatusOK,
+			permissions: []*accesscontrol.Permission{
+				{Action: accesscontrol.ActionLDAPUsersSync},
+			},
+		},
+		{
+			url:          "/api/admin/ldap/sync/test",
+			method:       http.MethodPost,
+			desc:         "PostSyncUserWithLDAP should return 200 for user without required permissions",
+			expectedCode: http.StatusForbidden,
+			permissions: []*accesscontrol.Permission{
+				{Action: "wrong"},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			t.Helper()
+
+			enabled := setting.LDAPEnabled
+			configFile := setting.LDAPConfigFile
+
+			t.Cleanup(func() {
+				setting.LDAPEnabled = enabled
+				setting.LDAPConfigFile = configFile
+			})
+
+			setting.LDAPEnabled = true
+			path, err := filepath.Abs("../../conf/ldap.toml")
+			assert.NoError(t, err)
+			setting.LDAPConfigFile = path
+
+			cfg := setting.NewCfg()
+			cfg.LDAPEnabled = true
+
+			sc := setupAccessControlScenarioContext(t, cfg, test.url, test.permissions)
+			sc.resp = httptest.NewRecorder()
+			sc.req, err = http.NewRequest(test.method, test.url, nil)
+			assert.NoError(t, err)
+
+			// Add minimal setup to pass handler
+			userSearchResult = &models.ExternalUserInfo{}
+			userSearchError = nil
+			newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP {
+				return &LDAPMock{}
+			}
+
+			bus.AddHandler("test", func(q *models.GetUserByIdQuery) error {
+				q.Result = &models.User{}
+				return nil
+			})
+
+			bus.AddHandler("test", func(q *models.GetAuthInfoQuery) error {
+				return nil
+			})
+
+			bus.AddHandler("test", func(cmd *models.UpsertUserCommand) error {
+				return nil
+			})
+
+			sc.exec()
+			assert.Equal(t, test.expectedCode, sc.resp.Code)
+		})
+	}
+}

pkg/services/accesscontrol/models.go

@@ -75,9 +75,10 @@ const (
 	ActionOrgUsersRoleUpdate = "org.users.role:update"
 
 	// LDAP actions
-	ActionLDAPUsersRead  = "ldap.user:read"
+	ActionLDAPUsersRead    = "ldap.user:read"
-	ActionLDAPUsersSync  = "ldap.user:sync"
+	ActionLDAPUsersSync    = "ldap.user:sync"
-	ActionLDAPStatusRead = "ldap.status:read"
+	ActionLDAPStatusRead   = "ldap.status:read"
+	ActionLDAPConfigReload = "ldap.config:reload"
 
 	// Global Scopes
 	ScopeGlobalUsersAll = "global:users:*"

pkg/services/accesscontrol/roles.go

@@ -17,11 +17,14 @@ var ldapAdminReadRole = RoleDTO{
 
 var ldapAdminEditRole = RoleDTO{
 	Name:    ldapAdminEdit,
-	Version: 1,
+	Version: 2,
 	Permissions: ConcatPermissions(ldapAdminReadRole.Permissions, []Permission{
 		{
 			Action: ActionLDAPUsersSync,
 		},
+		{
+			Action: ActionLDAPConfigReload,
+		},
 	}),
 }