diff --git a/docs/sources/enterprise/access-control/fine-grained-access-control-references.md b/docs/sources/enterprise/access-control/fine-grained-access-control-references.md index f7fb0094650..65e1d7f7cc2 100644 --- a/docs/sources/enterprise/access-control/fine-grained-access-control-references.md +++ b/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
`org.users:add`
`org.users:remove`
`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`
`ldap.status:read` | Allows to read LDAP information and status. -`fixed:ldap:admin:edit` | All permissions from `fixed:ldap:admin:read` and
`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
`ldap.user:sync`
`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 diff --git a/docs/sources/enterprise/access-control/permissions.md b/docs/sources/enterprise/access-control/permissions.md index 1eb9d66652e..25f87ddcd45 100644 --- a/docs/sources/enterprise/access-control/permissions.md +++ b/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:**`
`settings:auth.saml:*`
`settings:auth.saml:enabled` (property level) | Update settings diff --git a/pkg/api/api.go b/pkg/api/api.go index 97f242228e1..8e0724bd1ff 100644 --- a/pkg/api/api.go +++ b/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)) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 10c8e6499e0..9da77f27b10 100644 --- a/pkg/api/common_test.go +++ b/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 +} diff --git a/pkg/api/ldap_debug_test.go b/pkg/api/ldap_debug_test.go index b94df9c95b1..5868d9118cd 100644 --- a/pkg/api/ldap_debug_test.go +++ b/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) + }) + } +} diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index 8dfeb898f68..2d1e91edc78 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -75,9 +75,10 @@ const ( ActionOrgUsersRoleUpdate = "org.users.role:update" // LDAP actions - ActionLDAPUsersRead = "ldap.user:read" - ActionLDAPUsersSync = "ldap.user:sync" - ActionLDAPStatusRead = "ldap.status:read" + ActionLDAPUsersRead = "ldap.user:read" + ActionLDAPUsersSync = "ldap.user:sync" + ActionLDAPStatusRead = "ldap.status:read" + ActionLDAPConfigReload = "ldap.config:reload" // Global Scopes ScopeGlobalUsersAll = "global:users:*" diff --git a/pkg/services/accesscontrol/roles.go b/pkg/services/accesscontrol/roles.go index 44fd9a53e05..270c3915d3d 100644 --- a/pkg/services/accesscontrol/roles.go +++ b/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, + }, }), }