From 7b7b95341e73d0ac173e9ccef25cc129048b8701 Mon Sep 17 00:00:00 2001 From: gotjosh Date: Fri, 13 Sep 2019 16:26:25 +0100 Subject: [PATCH] LDAP: Allow an user to be synchronised against LDAP (#18976) * LDAP: Allow an user to be synchronised against LDAP This PR introduces the /ldap/sync/:id endpoint. It allows a user to be synchronized against LDAP on demand. A few things to note are: LDAP needs to be enabled for the sync to work It only works against users that originally authenticated against LDAP If the user is the Grafana admin and it needs to be disabled - it will not sync the information Includes a tiny refactor that favours the JSONEq assertion helper instead of manually parsing JSON strings. --- pkg/api/api.go | 1 + pkg/api/ldap_debug.go | 108 +++++++++++++++-- pkg/api/ldap_debug_test.go | 242 ++++++++++++++++++++++++++++++++----- pkg/api/login_test.go | 13 -- pkg/login/ldap_login.go | 6 +- 5 files changed, 313 insertions(+), 57 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 730efda4a98..6a590e28cfb 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -395,6 +395,7 @@ func (hs *HTTPServer) registerRoutes() { adminRoute.Post("/provisioning/datasources/reload", Wrap(hs.AdminProvisioningReloadDatasources)) adminRoute.Post("/provisioning/notifications/reload", Wrap(hs.AdminProvisioningReloadNotifications)) adminRoute.Post("/ldap/reload", Wrap(hs.ReloadLDAPCfg)) + adminRoute.Post("/ldap/sync/:id", Wrap(hs.PostSyncUserWithLDAP)) adminRoute.Get("/ldap/:username", Wrap(hs.GetUserFromLDAP)) adminRoute.Get("/ldap/status", Wrap(hs.GetLDAPStatus)) }, reqGrafanaAdmin) diff --git a/pkg/api/ldap_debug.go b/pkg/api/ldap_debug.go index 22252622e84..7ccbb8fbeca 100644 --- a/pkg/api/ldap_debug.go +++ b/pkg/api/ldap_debug.go @@ -1,20 +1,24 @@ package api import ( + "context" "fmt" "net/http" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/ldap" "github.com/grafana/grafana/pkg/services/multildap" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) var ( getLDAPConfig = multildap.GetConfig newLDAP = multildap.New + tokenService = AuthToken{}.TokenService logger = log.New("LDAP.debug") @@ -49,6 +53,22 @@ type LDAPUserDTO struct { Teams []models.TeamOrgGroupDTO `json:"teams"` } +// LDAPServerDTO is a serializer for LDAP server statuses +type LDAPServerDTO struct { + Host string `json:"host"` + Port int `json:"port"` + Available bool `json:"available"` + Error string `json:"error"` +} + +type AuthToken struct { + TokenService TokenRevoker `inject:""` +} + +type TokenRevoker interface { + RevokeAllUserTokens(context.Context, int64) error +} + // FetchOrgs fetches the organization(s) information by executing a single query to the database. Then, populating the DTO with the information retrieved. func (user *LDAPUserDTO) FetchOrgs() error { orgIds := []int64{} @@ -82,14 +102,6 @@ func (user *LDAPUserDTO) FetchOrgs() error { return nil } -// LDAPServerDTO is a serializer for LDAP server statuses -type LDAPServerDTO struct { - Host string `json:"host"` - Port int `json:"port"` - Available bool `json:"available"` - Error string `json:"error"` -} - // ReloadLDAPCfg reloads the LDAP configuration func (server *HTTPServer) ReloadLDAPCfg() Response { if !ldap.IsEnabled() { @@ -98,7 +110,7 @@ func (server *HTTPServer) ReloadLDAPCfg() Response { err := ldap.ReloadConfig() if err != nil { - return Error(http.StatusInternalServerError, "Failed to reload ldap config.", err) + return Error(http.StatusInternalServerError, "Failed to reload LDAP config", err) } return Success("LDAP config reloaded") } @@ -112,7 +124,7 @@ func (server *HTTPServer) GetLDAPStatus(c *models.ReqContext) Response { ldapConfig, err := getLDAPConfig() if err != nil { - return Error(http.StatusBadRequest, "Failed to obtain the LDAP configuration. Please verify the configuration and try again.", err) + return Error(http.StatusBadRequest, "Failed to obtain the LDAP configuration. Please verify the configuration and try again", err) } ldap := newLDAP(ldapConfig.Servers) @@ -141,6 +153,82 @@ func (server *HTTPServer) GetLDAPStatus(c *models.ReqContext) Response { return JSON(http.StatusOK, serverDTOs) } +// PostSyncUserWithLDAP enables a single Grafana user to be synchronized against LDAP +func (server *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) Response { + if !ldap.IsEnabled() { + return Error(http.StatusBadRequest, "LDAP is not enabled", nil) + } + + ldapConfig, err := getLDAPConfig() + + if err != nil { + return Error(http.StatusBadRequest, "Failed to obtain the LDAP configuration. Please verify the configuration and try again", err) + } + + userId := c.ParamsInt64(":id") + + query := models.GetUserByIdQuery{Id: userId} + + if err := bus.Dispatch(&query); err != nil { // validate the userId exists + if err == models.ErrUserNotFound { + return Error(404, models.ErrUserNotFound.Error(), nil) + } + + return Error(500, "Failed to get user", err) + } + + authModuleQuery := &models.GetAuthInfoQuery{UserId: query.Result.Id, AuthModule: models.AuthModuleLDAP} + + if err := bus.Dispatch(authModuleQuery); err != nil { // validate the userId comes from LDAP + if err == models.ErrUserNotFound { + return Error(404, models.ErrUserNotFound.Error(), nil) + } + + return Error(500, "Failed to get user", err) + } + + ldapServer := newLDAP(ldapConfig.Servers) + user, _, err := ldapServer.User(query.Result.Login) + + if err != nil { + if err == ldap.ErrCouldNotFindUser { // User was not in the LDAP server - we need to take action: + + if setting.AdminUser == query.Result.Login { // User is *the* Grafana Admin. We cannot disable it. + errMsg := fmt.Sprintf(`Refusing to sync grafana super admin "%s" - it would be disabled`, query.Result.Login) + logger.Error(errMsg) + return Error(http.StatusBadRequest, errMsg, err) + } + + // Since the user was not in the LDAP server. Let's disable it. + err := login.DisableExternalUser(query.Result.Login) + + if err != nil { + return Error(http.StatusInternalServerError, "Failed to disable the user", err) + } + + err = tokenService.RevokeAllUserTokens(context.TODO(), userId) + if err != nil { + return Error(http.StatusInternalServerError, "Failed to remove session tokens for the user", err) + } + + return Success("User disabled without any updates in the information") // should this be a success? + } + } + + upsertCmd := &models.UpsertUserCommand{ + ExternalUser: user, + SignupAllowed: setting.LDAPAllowSignup, + } + + err = bus.Dispatch(upsertCmd) + + if err != nil { + return Error(http.StatusInternalServerError, "Failed to udpate the user", err) + } + + return Success("User synced successfully") +} + // GetUserFromLDAP finds an user based on a username in LDAP. This helps illustrate how would the particular user be mapped in Grafana when synced. func (server *HTTPServer) GetUserFromLDAP(c *models.ReqContext) Response { if !ldap.IsEnabled() { diff --git a/pkg/api/ldap_debug_test.go b/pkg/api/ldap_debug_test.go index 46bfe57f7c7..938c3b1b228 100644 --- a/pkg/api/ldap_debug_test.go +++ b/pkg/api/ldap_debug_test.go @@ -1,7 +1,7 @@ package api import ( - "encoding/json" + "context" "errors" "net/http" "net/http/httptest" @@ -20,8 +20,12 @@ type LDAPMock struct { Results []*models.ExternalUserInfo } +type TokenServiceMock struct { +} + var userSearchResult *models.ExternalUserInfo var userSearchConfig ldap.ServerConfig +var userSearchError error var pingResult []*multildap.ServerStatus var pingError error @@ -39,7 +43,11 @@ func (m *LDAPMock) Users(logins []string) ([]*models.ExternalUserInfo, error) { } func (m *LDAPMock) User(login string) (*models.ExternalUserInfo, ldap.ServerConfig, error) { - return userSearchResult, userSearchConfig, nil + return userSearchResult, userSearchConfig, userSearchError +} + +func (ts *TokenServiceMock) RevokeAllUserTokens(ctx context.Context, userId int64) error { + return nil } //*** @@ -86,10 +94,7 @@ func TestGetUserFromLDAPApiEndpoint_UserNotFound(t *testing.T) { sc := getUserFromLDAPContext(t, "/api/admin/ldap/user-that-does-not-exist") require.Equal(t, sc.resp.Code, http.StatusNotFound) - responseString, err := getBody(sc.resp) - - assert.Nil(t, err) - assert.Equal(t, "{\"message\":\"No user was found on the LDAP server(s)\"}", responseString) + assert.JSONEq(t, "{\"message\":\"No user was found on the LDAP server(s)\"}", sc.resp.Body.String()) } func TestGetUserFromLDAPApiEndpoint_OrgNotfound(t *testing.T) { @@ -144,19 +149,13 @@ func TestGetUserFromLDAPApiEndpoint_OrgNotfound(t *testing.T) { require.Equal(t, sc.resp.Code, http.StatusBadRequest) - jsonResponse, err := getJSONbody(sc.resp) - assert.Nil(t, err) - expected := ` { "error": "Unable to find organization with ID '2'", "message": "An oganization was not found - Please verify your LDAP configuration" } ` - var expectedJSON interface{} - _ = json.Unmarshal([]byte(expected), &expectedJSON) - - assert.Equal(t, expectedJSON, jsonResponse) + assert.JSONEq(t, expected, sc.resp.Body.String()) } func TestGetUserFromLDAPApiEndpoint(t *testing.T) { @@ -206,9 +205,6 @@ func TestGetUserFromLDAPApiEndpoint(t *testing.T) { require.Equal(t, sc.resp.Code, http.StatusOK) - jsonResponse, err := getJSONbody(sc.resp) - assert.Nil(t, err) - expected := ` { "name": { @@ -231,10 +227,8 @@ func TestGetUserFromLDAPApiEndpoint(t *testing.T) { "teams": null } ` - var expectedJSON interface{} - _ = json.Unmarshal([]byte(expected), &expectedJSON) - assert.Equal(t, expectedJSON, jsonResponse) + assert.JSONEq(t, expected, sc.resp.Body.String()) } func TestGetUserFromLDAPApiEndpoint_WithTeamHandler(t *testing.T) { @@ -289,9 +283,6 @@ func TestGetUserFromLDAPApiEndpoint_WithTeamHandler(t *testing.T) { require.Equal(t, sc.resp.Code, http.StatusOK) - jsonResponse, err := getJSONbody(sc.resp) - assert.Nil(t, err) - expected := ` { "name": { @@ -314,10 +305,8 @@ func TestGetUserFromLDAPApiEndpoint_WithTeamHandler(t *testing.T) { "teams": [] } ` - var expectedJSON interface{} - _ = json.Unmarshal([]byte(expected), &expectedJSON) - assert.Equal(t, expectedJSON, jsonResponse) + assert.JSONEq(t, expected, sc.resp.Body.String()) } //*** @@ -369,8 +358,6 @@ func TestGetLDAPStatusApiEndpoint(t *testing.T) { sc := getLDAPStatusContext(t) require.Equal(t, http.StatusOK, sc.resp.Code) - jsonResponse, err := getJSONbody(sc.resp) - assert.Nil(t, err) expected := ` [ @@ -379,8 +366,201 @@ func TestGetLDAPStatusApiEndpoint(t *testing.T) { { "host": "10.0.0.5", "port": 361, "available": false, "error": "something is awfully wrong" } ] ` - var expectedJSON interface{} - _ = json.Unmarshal([]byte(expected), &expectedJSON) - - assert.Equal(t, expectedJSON, jsonResponse) + assert.JSONEq(t, expected, sc.resp.Body.String()) +} + +//*** +// PostSyncUserWithLDAP tests +//*** + +func postSyncUserWithLDAPContext(t *testing.T, requestURL string) *scenarioContext { + t.Helper() + + sc := setupScenarioContext(requestURL) + + ldap := setting.LDAPEnabled + setting.LDAPEnabled = true + defer func() { setting.LDAPEnabled = ldap }() + + hs := &HTTPServer{Cfg: setting.NewCfg()} + + sc.defaultHandler = Wrap(func(c *models.ReqContext) Response { + sc.context = c + return hs.PostSyncUserWithLDAP(c) + }) + + sc.m.Post("/api/admin/ldap/sync/:id", sc.defaultHandler) + + sc.resp = httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodPost, requestURL, nil) + sc.req = req + sc.exec() + + return sc +} + +func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) { + getLDAPConfig = func() (*ldap.Config, error) { + return &ldap.Config{}, nil + } + + newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { + return &LDAPMock{} + } + + userSearchResult = &models.ExternalUserInfo{ + Login: "ldap-daniel", + } + + bus.AddHandler("test", func(cmd *models.UpsertUserCommand) error { + require.Equal(t, "ldap-daniel", cmd.ExternalUser.Login) + return nil + }) + + bus.AddHandler("test", func(q *models.GetUserByIdQuery) error { + require.Equal(t, q.Id, int64(34)) + + q.Result = &models.User{Login: "ldap-daniel", Id: 34} + return nil + }) + + bus.AddHandler("test", func(q *models.GetAuthInfoQuery) error { + require.Equal(t, q.UserId, int64(34)) + require.Equal(t, q.AuthModule, models.AuthModuleLDAP) + + return nil + }) + + sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34") + + assert.Equal(t, http.StatusOK, sc.resp.Code) + + expected := ` + { + "message": "User synced successfully" + } + ` + + assert.JSONEq(t, expected, sc.resp.Body.String()) +} + +func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotFound(t *testing.T) { + getLDAPConfig = func() (*ldap.Config, error) { + return &ldap.Config{}, nil + } + + newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { + return &LDAPMock{} + } + + bus.AddHandler("test", func(q *models.GetUserByIdQuery) error { + require.Equal(t, q.Id, int64(34)) + + return models.ErrUserNotFound + }) + + sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34") + + assert.Equal(t, http.StatusNotFound, sc.resp.Code) + + expected := ` + { + "message": "User not found" + } + ` + + assert.JSONEq(t, expected, sc.resp.Body.String()) +} + +func TestPostSyncUserWithLDAPAPIEndpoint_WhenGrafanaAdmin(t *testing.T) { + getLDAPConfig = func() (*ldap.Config, error) { + return &ldap.Config{}, nil + } + + newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { + return &LDAPMock{} + } + + userSearchError = ldap.ErrCouldNotFindUser + + admin := setting.AdminUser + setting.AdminUser = "ldap-daniel" + defer func() { setting.AdminUser = admin }() + + bus.AddHandler("test", func(q *models.GetUserByIdQuery) error { + require.Equal(t, q.Id, int64(34)) + + q.Result = &models.User{Login: "ldap-daniel", Id: 34} + return nil + }) + + bus.AddHandler("test", func(q *models.GetAuthInfoQuery) error { + require.Equal(t, q.UserId, int64(34)) + require.Equal(t, q.AuthModule, models.AuthModuleLDAP) + + return nil + }) + + sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34") + + assert.Equal(t, http.StatusBadRequest, sc.resp.Code) + + expected := ` + { + "error": "Can't find user in LDAP", + "message": "Refusing to sync grafana super admin \"ldap-daniel\" - it would be disabled" + } + ` + + assert.JSONEq(t, expected, sc.resp.Body.String()) +} + +func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotInLDAP(t *testing.T) { + getLDAPConfig = func() (*ldap.Config, error) { + return &ldap.Config{}, nil + } + + tokenService = &TokenServiceMock{} + + newLDAP = func(_ []*ldap.ServerConfig) multildap.IMultiLDAP { + return &LDAPMock{} + } + + userSearchResult = nil + + bus.AddHandler("test", func(cmd *models.UpsertUserCommand) error { + require.Equal(t, "ldap-daniel", cmd.ExternalUser.Login) + return nil + }) + + bus.AddHandler("test", func(q *models.GetUserByIdQuery) error { + require.Equal(t, q.Id, int64(34)) + + q.Result = &models.User{Login: "ldap-daniel", Id: 34} + return nil + }) + + bus.AddHandler("test", func(q *models.GetExternalUserInfoByLoginQuery) error { + assert.Equal(t, "ldap-daniel", q.LoginOrEmail) + q.Result = &models.ExternalUserInfo{IsDisabled: true, UserId: 34} + + return nil + }) + + bus.AddHandler("test", func(cmd *models.DisableUserCommand) error { + assert.Equal(t, 34, cmd.UserId) + return nil + }) + + sc := postSyncUserWithLDAPContext(t, "/api/admin/ldap/sync/34") + + assert.Equal(t, http.StatusOK, sc.resp.Code) + + expected := ` + { + "message": "User disabled without any updates in the information" + } + ` + + assert.JSONEq(t, expected, sc.resp.Body.String()) } diff --git a/pkg/api/login_test.go b/pkg/api/login_test.go index 0d0983f433b..fd27bd3f656 100644 --- a/pkg/api/login_test.go +++ b/pkg/api/login_test.go @@ -2,7 +2,6 @@ package api import ( "encoding/hex" - "encoding/json" "errors" "io/ioutil" "net/http" @@ -52,18 +51,6 @@ func getBody(resp *httptest.ResponseRecorder) (string, error) { return string(responseData), nil } -func getJSONbody(resp *httptest.ResponseRecorder) (interface{}, error) { - var j interface{} - - err := json.Unmarshal(resp.Body.Bytes(), &j) - - if err != nil { - return nil, err - } - - return j, nil -} - func TestLoginErrorCookieApiEndpoint(t *testing.T) { mockSetIndexViewData() defer resetSetIndexViewData() diff --git a/pkg/login/ldap_login.go b/pkg/login/ldap_login.go index daf0dfe93f6..3077b21432a 100644 --- a/pkg/login/ldap_login.go +++ b/pkg/login/ldap_login.go @@ -40,7 +40,7 @@ var loginUsingLDAP = func(query *models.LoginUserQuery) (bool, error) { if err != nil { if err == ldap.ErrCouldNotFindUser { // Ignore the error since user might not be present anyway - disableExternalUser(query.Username) + DisableExternalUser(query.Username) return true, ldap.ErrInvalidCredentials } @@ -61,8 +61,8 @@ var loginUsingLDAP = func(query *models.LoginUserQuery) (bool, error) { return true, nil } -// disableExternalUser marks external user as disabled in Grafana db -func disableExternalUser(username string) error { +// DisableExternalUser marks external user as disabled in Grafana db +func DisableExternalUser(username string) error { // Check if external user exist in Grafana userQuery := &models.GetExternalUserInfoByLoginQuery{ LoginOrEmail: username,