Przeglądaj źródła

Users: Disable users removed from LDAP (#16820)

* Users: add is_disabled column

* Users: disable users removed from LDAP

* Auth: return ErrInvalidCredentials for failed LDAP auth

* User: return isDisabled flag in user search api

* User: mark disabled users at the server admin page

* Chore: refactor according to review

* Auth: prevent disabled user from login

* Auth: re-enable user when it found in ldap

* User: add api endpoint for disabling user

* User: use separate endpoints to disable/enable user

* User: disallow disabling external users

* User: able do disable users from admin UI

* Chore: refactor based on review

* Chore: use more clear error check when disabling user

* Fix login tests

* Tests for disabling user during the LDAP login

* Tests for disable user API

* Tests for login with disabled user

* Remove disable user UI stub

* Sync with latest LDAP refactoring
Alexander Zobnin 6 lat temu
rodzic
commit
2d03815770

+ 56 - 16
pkg/api/admin_users.go

@@ -4,12 +4,12 @@ import (
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/infra/metrics"
-	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/util"
 )
 
-func AdminCreateUser(c *m.ReqContext, form dtos.AdminCreateUserForm) {
-	cmd := m.CreateUserCommand{
+func AdminCreateUser(c *models.ReqContext, form dtos.AdminCreateUserForm) {
+	cmd := models.CreateUserCommand{
 		Login:    form.Login,
 		Email:    form.Email,
 		Password: form.Password,
@@ -38,7 +38,7 @@ func AdminCreateUser(c *m.ReqContext, form dtos.AdminCreateUserForm) {
 
 	user := cmd.Result
 
-	result := m.UserIdDTO{
+	result := models.UserIdDTO{
 		Message: "User created",
 		Id:      user.Id,
 	}
@@ -46,7 +46,7 @@ func AdminCreateUser(c *m.ReqContext, form dtos.AdminCreateUserForm) {
 	c.JSON(200, result)
 }
 
-func AdminUpdateUserPassword(c *m.ReqContext, form dtos.AdminUpdateUserPasswordForm) {
+func AdminUpdateUserPassword(c *models.ReqContext, form dtos.AdminUpdateUserPasswordForm) {
 	userID := c.ParamsInt64(":id")
 
 	if len(form.Password) < 4 {
@@ -54,7 +54,7 @@ func AdminUpdateUserPassword(c *m.ReqContext, form dtos.AdminUpdateUserPasswordF
 		return
 	}
 
-	userQuery := m.GetUserByIdQuery{Id: userID}
+	userQuery := models.GetUserByIdQuery{Id: userID}
 
 	if err := bus.Dispatch(&userQuery); err != nil {
 		c.JsonApiErr(500, "Could not read user from database", err)
@@ -63,7 +63,7 @@ func AdminUpdateUserPassword(c *m.ReqContext, form dtos.AdminUpdateUserPasswordF
 
 	passwordHashed := util.EncodePassword(form.Password, userQuery.Result.Salt)
 
-	cmd := m.ChangeUserPasswordCommand{
+	cmd := models.ChangeUserPasswordCommand{
 		UserId:      userID,
 		NewPassword: passwordHashed,
 	}
@@ -77,17 +77,17 @@ func AdminUpdateUserPassword(c *m.ReqContext, form dtos.AdminUpdateUserPasswordF
 }
 
 // PUT /api/admin/users/:id/permissions
-func AdminUpdateUserPermissions(c *m.ReqContext, form dtos.AdminUpdateUserPermissionsForm) {
+func AdminUpdateUserPermissions(c *models.ReqContext, form dtos.AdminUpdateUserPermissionsForm) {
 	userID := c.ParamsInt64(":id")
 
-	cmd := m.UpdateUserPermissionsCommand{
+	cmd := models.UpdateUserPermissionsCommand{
 		UserId:         userID,
 		IsGrafanaAdmin: form.IsGrafanaAdmin,
 	}
 
 	if err := bus.Dispatch(&cmd); err != nil {
-		if err == m.ErrLastGrafanaAdmin {
-			c.JsonApiErr(400, m.ErrLastGrafanaAdmin.Error(), nil)
+		if err == models.ErrLastGrafanaAdmin {
+			c.JsonApiErr(400, models.ErrLastGrafanaAdmin.Error(), nil)
 			return
 		}
 
@@ -98,10 +98,10 @@ func AdminUpdateUserPermissions(c *m.ReqContext, form dtos.AdminUpdateUserPermis
 	c.JsonOK("User permissions updated")
 }
 
-func AdminDeleteUser(c *m.ReqContext) {
+func AdminDeleteUser(c *models.ReqContext) {
 	userID := c.ParamsInt64(":id")
 
-	cmd := m.DeleteUserCommand{UserId: userID}
+	cmd := models.DeleteUserCommand{UserId: userID}
 
 	if err := bus.Dispatch(&cmd); err != nil {
 		c.JsonApiErr(500, "Failed to delete user", err)
@@ -111,8 +111,48 @@ func AdminDeleteUser(c *m.ReqContext) {
 	c.JsonOK("User deleted")
 }
 
+// POST /api/admin/users/:id/disable
+func AdminDisableUser(c *models.ReqContext) {
+	userID := c.ParamsInt64(":id")
+
+	// External users shouldn't be disabled from API
+	authInfoQuery := &models.GetAuthInfoQuery{UserId: userID}
+	if err := bus.Dispatch(authInfoQuery); err != models.ErrUserNotFound {
+		c.JsonApiErr(500, "Could not disable external user", nil)
+		return
+	}
+
+	disableCmd := models.DisableUserCommand{UserId: userID, IsDisabled: true}
+	if err := bus.Dispatch(&disableCmd); err != nil {
+		c.JsonApiErr(500, "Failed to disable user", err)
+		return
+	}
+
+	c.JsonOK("User disabled")
+}
+
+// POST /api/admin/users/:id/enable
+func AdminEnableUser(c *models.ReqContext) {
+	userID := c.ParamsInt64(":id")
+
+	// External users shouldn't be disabled from API
+	authInfoQuery := &models.GetAuthInfoQuery{UserId: userID}
+	if err := bus.Dispatch(authInfoQuery); err != models.ErrUserNotFound {
+		c.JsonApiErr(500, "Could not enable external user", nil)
+		return
+	}
+
+	disableCmd := models.DisableUserCommand{UserId: userID, IsDisabled: false}
+	if err := bus.Dispatch(&disableCmd); err != nil {
+		c.JsonApiErr(500, "Failed to enable user", err)
+		return
+	}
+
+	c.JsonOK("User enabled")
+}
+
 // POST /api/admin/users/:id/logout
-func (server *HTTPServer) AdminLogoutUser(c *m.ReqContext) Response {
+func (server *HTTPServer) AdminLogoutUser(c *models.ReqContext) Response {
 	userID := c.ParamsInt64(":id")
 
 	if c.UserId == userID {
@@ -123,13 +163,13 @@ func (server *HTTPServer) AdminLogoutUser(c *m.ReqContext) Response {
 }
 
 // GET /api/admin/users/:id/auth-tokens
-func (server *HTTPServer) AdminGetUserAuthTokens(c *m.ReqContext) Response {
+func (server *HTTPServer) AdminGetUserAuthTokens(c *models.ReqContext) Response {
 	userID := c.ParamsInt64(":id")
 	return server.getUserAuthTokensInternal(c, userID)
 }
 
 // POST /api/admin/users/:id/revoke-auth-token
-func (server *HTTPServer) AdminRevokeUserAuthToken(c *m.ReqContext, cmd m.RevokeAuthTokenCmd) Response {
+func (server *HTTPServer) AdminRevokeUserAuthToken(c *models.ReqContext, cmd models.RevokeAuthTokenCmd) Response {
 	userID := c.ParamsInt64(":id")
 	return server.revokeUserAuthTokenInternal(c, userID, cmd)
 }

+ 53 - 0
pkg/api/admin_users_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/bus"
+	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/auth"
 
@@ -84,6 +85,36 @@ func TestAdminApiEndpoint(t *testing.T) {
 			So(userId, ShouldEqual, 200)
 		})
 	})
+
+	Convey("When a server admin attempts to disable/enable external user", t, func() {
+		userId := int64(0)
+		bus.AddHandler("test", func(cmd *m.GetAuthInfoQuery) error {
+			userId = cmd.UserId
+			return nil
+		})
+
+		adminDisableUserScenario("Should return Could not disable external user error", "disable", "/api/admin/users/42/disable", "/api/admin/users/:id/disable", func(sc *scenarioContext) {
+			sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
+			So(sc.resp.Code, ShouldEqual, 500)
+
+			respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
+			So(err, ShouldBeNil)
+			So(respJSON.Get("message").MustString(), ShouldEqual, "Could not disable external user")
+
+			So(userId, ShouldEqual, 42)
+		})
+
+		adminDisableUserScenario("Should return Could not enable external user error", "enable", "/api/admin/users/42/enable", "/api/admin/users/:id/enable", func(sc *scenarioContext) {
+			sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
+			So(sc.resp.Code, ShouldEqual, 500)
+
+			respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
+			So(err, ShouldBeNil)
+			So(respJSON.Get("message").MustString(), ShouldEqual, "Could not enable external user")
+
+			So(userId, ShouldEqual, 42)
+		})
+	})
 }
 
 func putAdminScenario(desc string, url string, routePattern string, role m.RoleType, cmd dtos.AdminUpdateUserPermissionsForm, fn scenarioFunc) {
@@ -186,3 +217,25 @@ func adminGetUserAuthTokensScenario(desc string, url string, routePattern string
 		fn(sc)
 	})
 }
+
+func adminDisableUserScenario(desc string, action string, url string, routePattern string, fn scenarioFunc) {
+	Convey(desc+" "+url, func() {
+		defer bus.ClearBusHandlers()
+
+		sc := setupScenarioContext(url)
+		sc.defaultHandler = Wrap(func(c *m.ReqContext) {
+			sc.context = c
+			sc.context.UserId = TestUserID
+
+			if action == "enable" {
+				AdminEnableUser(c)
+			} else {
+				AdminDisableUser(c)
+			}
+		})
+
+		sc.m.Post(routePattern, sc.defaultHandler)
+
+		fn(sc)
+	})
+}

+ 2 - 0
pkg/api/api.go

@@ -381,6 +381,8 @@ func (hs *HTTPServer) registerRoutes() {
 		adminRoute.Put("/users/:id/password", bind(dtos.AdminUpdateUserPasswordForm{}), AdminUpdateUserPassword)
 		adminRoute.Put("/users/:id/permissions", bind(dtos.AdminUpdateUserPermissionsForm{}), AdminUpdateUserPermissions)
 		adminRoute.Delete("/users/:id", AdminDeleteUser)
+		adminRoute.Post("/users/:id/disable", AdminDisableUser)
+		adminRoute.Post("/users/:id/enable", AdminEnableUser)
 		adminRoute.Get("/users/:id/quotas", Wrap(GetUserQuotas))
 		adminRoute.Put("/users/:id/quotas/:target", bind(m.UpdateUserQuotaCmd{}), Wrap(UpdateUserQuota))
 		adminRoute.Get("/stats", AdminGetStats)

+ 4 - 0
pkg/api/login.go

@@ -105,6 +105,10 @@ func (hs *HTTPServer) LoginPost(c *m.ReqContext, cmd dtos.LoginCommand) Response
 			return Error(401, "Invalid username or password", err)
 		}
 
+		if err == login.ErrUserDisabled {
+			return Error(401, "User is disabled", err)
+		}
+
 		return Error(500, "Error while trying to authenticate user", err)
 	}
 

+ 7 - 2
pkg/login/auth.go

@@ -19,6 +19,7 @@ var (
 	ErrPasswordEmpty         = errors.New("No password provided")
 	ErrUsersQuotaReached     = errors.New("Users quota reached")
 	ErrGettingUserQuota      = errors.New("Error getting user quota")
+	ErrUserDisabled          = errors.New("User is disabled")
 )
 
 func Init() {
@@ -36,7 +37,7 @@ func AuthenticateUser(query *models.LoginUserQuery) error {
 	}
 
 	err := loginUsingGrafanaDB(query)
-	if err == nil || (err != models.ErrUserNotFound && err != ErrInvalidCredentials) {
+	if err == nil || (err != models.ErrUserNotFound && err != ErrInvalidCredentials && err != ErrUserDisabled) {
 		return err
 	}
 
@@ -46,11 +47,14 @@ func AuthenticateUser(query *models.LoginUserQuery) error {
 			return ldapErr
 		}
 
-		err = ldapErr
+		if err != ErrUserDisabled || ldapErr != ldap.ErrInvalidCredentials {
+			err = ldapErr
+		}
 	}
 
 	if err == ErrInvalidCredentials || err == ldap.ErrInvalidCredentials {
 		saveInvalidLoginAttempt(query)
+		return ErrInvalidCredentials
 	}
 
 	if err == models.ErrUserNotFound {
@@ -59,6 +63,7 @@ func AuthenticateUser(query *models.LoginUserQuery) error {
 
 	return err
 }
+
 func validatePasswordSet(password string) error {
 	if len(password) == 0 {
 		return ErrPasswordEmpty

+ 2 - 2
pkg/login/auth_test.go

@@ -108,7 +108,7 @@ func TestAuthenticateUser(t *testing.T) {
 			err := AuthenticateUser(sc.loginUserQuery)
 
 			Convey("it should result in", func() {
-				So(err, ShouldEqual, ldap.ErrInvalidCredentials)
+				So(err, ShouldEqual, ErrInvalidCredentials)
 				So(sc.loginAttemptValidationWasCalled, ShouldBeTrue)
 				So(sc.grafanaLoginWasCalled, ShouldBeTrue)
 				So(sc.ldapLoginWasCalled, ShouldBeTrue)
@@ -160,7 +160,7 @@ func TestAuthenticateUser(t *testing.T) {
 			err := AuthenticateUser(sc.loginUserQuery)
 
 			Convey("it should result in", func() {
-				So(err, ShouldEqual, ldap.ErrInvalidCredentials)
+				So(err, ShouldEqual, ErrInvalidCredentials)
 				So(sc.loginAttemptValidationWasCalled, ShouldBeTrue)
 				So(sc.grafanaLoginWasCalled, ShouldBeTrue)
 				So(sc.ldapLoginWasCalled, ShouldBeTrue)

+ 4 - 0
pkg/login/grafana_login.go

@@ -26,6 +26,10 @@ var loginUsingGrafanaDB = func(query *m.LoginUserQuery) error {
 
 	user := userQuery.Result
 
+	if user.IsDisabled {
+		return ErrUserDisabled
+	}
+
 	if err := validatePassword(query.Password, user.Password, user.Salt); err != nil {
 		return err
 	}

+ 23 - 0
pkg/login/grafana_login_test.go

@@ -63,6 +63,23 @@ func TestGrafanaLogin(t *testing.T) {
 				So(sc.loginUserQuery.User.Password, ShouldEqual, sc.loginUserQuery.Password)
 			})
 		})
+
+		grafanaLoginScenario("When login with disabled user", func(sc *grafanaLoginScenarioContext) {
+			sc.withDisabledUser()
+			err := loginUsingGrafanaDB(sc.loginUserQuery)
+
+			Convey("it should return user is disabled error", func() {
+				So(err, ShouldEqual, ErrUserDisabled)
+			})
+
+			Convey("it should not call password validation", func() {
+				So(sc.validatePasswordCalled, ShouldBeFalse)
+			})
+
+			Convey("it should not pupulate user object", func() {
+				So(sc.loginUserQuery.User, ShouldBeNil)
+			})
+		})
 	})
 }
 
@@ -138,3 +155,9 @@ func (sc *grafanaLoginScenarioContext) withInvalidPassword() {
 	})
 	mockPasswordValidation(false, sc)
 }
+
+func (sc *grafanaLoginScenarioContext) withDisabledUser() {
+	sc.getUserByLoginQueryReturns(&m.User{
+		IsDisabled: true,
+	})
+}

+ 8 - 0
pkg/models/user.go

@@ -30,6 +30,7 @@ type User struct {
 	EmailVerified bool
 	Theme         string
 	HelpFlags1    HelpFlags1
+	IsDisabled    bool
 
 	IsAdmin bool
 	OrgId   int64
@@ -88,6 +89,11 @@ type UpdateUserPermissionsCommand struct {
 	UserId         int64 `json:"-"`
 }
 
+type DisableUserCommand struct {
+	UserId     int64
+	IsDisabled bool
+}
+
 type DeleteUserCommand struct {
 	UserId int64
 }
@@ -203,6 +209,7 @@ type UserProfileDTO struct {
 	Theme          string `json:"theme"`
 	OrgId          int64  `json:"orgId"`
 	IsGrafanaAdmin bool   `json:"isGrafanaAdmin"`
+	IsDisabled     bool   `json:"isDisabled"`
 }
 
 type UserSearchHitDTO struct {
@@ -212,6 +219,7 @@ type UserSearchHitDTO struct {
 	Email         string    `json:"email"`
 	AvatarUrl     string    `json:"avatarUrl"`
 	IsAdmin       bool      `json:"isAdmin"`
+	IsDisabled    bool      `json:"isDisabled"`
 	LastSeenAt    time.Time `json:"lastSeenAt"`
 	LastSeenAtAge string    `json:"lastSeenAtAge"`
 }

+ 11 - 0
pkg/models/user_auth.go

@@ -6,6 +6,10 @@ import (
 	"golang.org/x/oauth2"
 )
 
+const (
+	AuthModuleLDAP = "ldap"
+)
+
 type UserAuth struct {
 	Id                int64
 	UserId            int64
@@ -29,6 +33,7 @@ type ExternalUserInfo struct {
 	Groups         []string
 	OrgRoles       map[int64]RoleType
 	IsGrafanaAdmin *bool // This is a pointer to know if we should sync this or not (nil = ignore sync)
+	IsDisabled     bool
 }
 
 // ---------------------
@@ -81,6 +86,12 @@ type GetUserByAuthInfoQuery struct {
 	Result *User
 }
 
+type GetExternalUserInfoByLoginQuery struct {
+	LoginOrEmail string
+
+	Result *ExternalUserInfo
+}
+
 type GetAuthInfoQuery struct {
 	UserId     int64
 	AuthModule string

+ 32 - 1
pkg/services/ldap/ldap.go

@@ -10,6 +10,7 @@ import (
 
 	"gopkg.in/ldap.v3"
 
+	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
 )
@@ -48,6 +49,7 @@ var (
 
 	// ErrInvalidCredentials is returned if username and password do not match
 	ErrInvalidCredentials = errors.New("Invalid Username or Password")
+	ErrLDAPUserNotFound   = errors.New("LDAP user not found")
 )
 
 var dial = func(network, addr string) (IConnection, error) {
@@ -142,6 +144,7 @@ func (server *Server) Login(query *models.LoginUserQuery) (
 	// If we couldn't find the user -
 	// we should show incorrect credentials err
 	if len(users) == 0 {
+		server.disableExternalUser(query.Username)
 		return nil, ErrInvalidCredentials
 	}
 
@@ -263,6 +266,34 @@ func (server *Server) validateGrafanaUser(user *models.ExternalUserInfo) error {
 	return nil
 }
 
+// disableExternalUser marks external user as disabled in Grafana db
+func (server *Server) disableExternalUser(username string) error {
+	// Check if external user exist in Grafana
+	userQuery := &models.GetExternalUserInfoByLoginQuery{
+		LoginOrEmail: username,
+	}
+
+	if err := bus.Dispatch(userQuery); err != nil {
+		return err
+	}
+
+	userInfo := userQuery.Result
+	if !userInfo.IsDisabled {
+		server.log.Debug("Disabling external user", "user", userQuery.Result.Login)
+		// Mark user as disabled in grafana db
+		disableUserCmd := &models.DisableUserCommand{
+			UserId:     userQuery.Result.UserId,
+			IsDisabled: true,
+		}
+
+		if err := bus.Dispatch(disableUserCmd); err != nil {
+			server.log.Debug("Error disabling external user", "user", userQuery.Result.Login, "message", err.Error())
+			return err
+		}
+	}
+	return nil
+}
+
 // getSearchRequest returns LDAP search request for users
 func (server *Server) getSearchRequest(
 	base string,
@@ -305,7 +336,7 @@ func (server *Server) getSearchRequest(
 // buildGrafanaUser extracts info from UserInfo model to ExternalUserInfo
 func (server *Server) buildGrafanaUser(user *UserInfo) *models.ExternalUserInfo {
 	extUser := &models.ExternalUserInfo{
-		AuthModule: "ldap",
+		AuthModule: models.AuthModuleLDAP,
 		AuthId:     user.DN,
 		Name: strings.TrimSpace(
 			fmt.Sprintf("%s %s", user.FirstName, user.LastName),

+ 97 - 0
pkg/services/ldap/ldap_login_test.go

@@ -148,5 +148,102 @@ func TestLDAPLogin(t *testing.T) {
 			So(err, ShouldBeNil)
 			So(resp.Login, ShouldEqual, "markelog")
 		})
+
+		authScenario("When user not found in LDAP, but exist in Grafana", func(scenario *scenarioContext) {
+			connection := &mockConnection{}
+			result := ldap.SearchResult{Entries: []*ldap.Entry{}}
+			connection.setSearchResult(&result)
+
+			externalUser := &models.ExternalUserInfo{UserId: 42, IsDisabled: false}
+			scenario.getExternalUserInfoByLoginQueryReturns(externalUser)
+
+			connection.bindProvider = func(username, password string) error {
+				return nil
+			}
+			auth := &Server{
+				config: &ServerConfig{
+					SearchBaseDNs: []string{"BaseDNHere"},
+				},
+				connection: connection,
+				log:        log.New("test-logger"),
+			}
+
+			_, err := auth.Login(scenario.loginUserQuery)
+
+			Convey("it should disable user", func() {
+				So(scenario.disableExternalUserCalled, ShouldBeTrue)
+				So(scenario.disableUserCmd.IsDisabled, ShouldBeTrue)
+				So(scenario.disableUserCmd.UserId, ShouldEqual, 42)
+			})
+
+			Convey("it should return invalid credentials error", func() {
+				So(err, ShouldEqual, ErrInvalidCredentials)
+			})
+		})
+
+		authScenario("When user not found in LDAP, and disabled in Grafana already", func(scenario *scenarioContext) {
+			connection := &mockConnection{}
+			result := ldap.SearchResult{Entries: []*ldap.Entry{}}
+			connection.setSearchResult(&result)
+
+			externalUser := &models.ExternalUserInfo{UserId: 42, IsDisabled: true}
+			scenario.getExternalUserInfoByLoginQueryReturns(externalUser)
+
+			connection.bindProvider = func(username, password string) error {
+				return nil
+			}
+			auth := &Server{
+				config: &ServerConfig{
+					SearchBaseDNs: []string{"BaseDNHere"},
+				},
+				connection: connection,
+				log:        log.New("test-logger"),
+			}
+
+			_, err := auth.Login(scenario.loginUserQuery)
+
+			Convey("it should't call disable function", func() {
+				So(scenario.disableExternalUserCalled, ShouldBeFalse)
+			})
+
+			Convey("it should return invalid credentials error", func() {
+				So(err, ShouldEqual, ErrInvalidCredentials)
+			})
+		})
+
+		authScenario("When user found in LDAP, and disabled in Grafana", func(scenario *scenarioContext) {
+			connection := &mockConnection{}
+			entry := ldap.Entry{}
+			result := ldap.SearchResult{Entries: []*ldap.Entry{&entry}}
+			connection.setSearchResult(&result)
+			scenario.userQueryReturns(&models.User{Id: 42, IsDisabled: true})
+
+			connection.bindProvider = func(username, password string) error {
+				return nil
+			}
+			auth := &Server{
+				config: &ServerConfig{
+					SearchBaseDNs: []string{"BaseDNHere"},
+				},
+				connection: connection,
+				log:        log.New("test-logger"),
+			}
+
+			extUser, _ := auth.Login(scenario.loginUserQuery)
+			_, err := user.Upsert(&user.UpsertArgs{
+				SignupAllowed: true,
+				ExternalUser:  extUser,
+			})
+
+			Convey("it should re-enable user", func() {
+				So(scenario.disableExternalUserCalled, ShouldBeTrue)
+				So(scenario.disableUserCmd.IsDisabled, ShouldBeFalse)
+				So(scenario.disableUserCmd.UserId, ShouldEqual, 42)
+			})
+
+			Convey("it should not return error", func() {
+				So(err, ShouldBeNil)
+			})
+		})
 	})
 }

+ 36 - 10
pkg/services/ldap/test.go

@@ -115,6 +115,18 @@ func authScenario(desc string, fn scenarioFunc) {
 			return nil
 		})
 
+		bus.AddHandler("test", func(cmd *models.GetExternalUserInfoByLoginQuery) error {
+			sc.getExternalUserInfoByLoginQuery = cmd
+			sc.getExternalUserInfoByLoginQuery.Result = &models.ExternalUserInfo{UserId: 42, IsDisabled: false}
+			return nil
+		})
+
+		bus.AddHandler("test", func(cmd *models.DisableUserCommand) error {
+			sc.disableExternalUserCalled = true
+			sc.disableUserCmd = cmd
+			return nil
+		})
+
 		bus.AddHandler("test", func(cmd *models.AddOrgUserCommand) error {
 			sc.addOrgUserCmd = cmd
 			return nil
@@ -145,16 +157,19 @@ func authScenario(desc string, fn scenarioFunc) {
 }
 
 type scenarioContext struct {
-	loginUserQuery           *models.LoginUserQuery
-	getUserByAuthInfoQuery   *models.GetUserByAuthInfoQuery
-	getUserOrgListQuery      *models.GetUserOrgListQuery
-	createUserCmd            *models.CreateUserCommand
-	addOrgUserCmd            *models.AddOrgUserCommand
-	updateOrgUserCmd         *models.UpdateOrgUserCommand
-	removeOrgUserCmd         *models.RemoveOrgUserCommand
-	updateUserCmd            *models.UpdateUserCommand
-	setUsingOrgCmd           *models.SetUsingOrgCommand
-	updateUserPermissionsCmd *models.UpdateUserPermissionsCommand
+	loginUserQuery                  *models.LoginUserQuery
+	getUserByAuthInfoQuery          *models.GetUserByAuthInfoQuery
+	getExternalUserInfoByLoginQuery *models.GetExternalUserInfoByLoginQuery
+	getUserOrgListQuery             *models.GetUserOrgListQuery
+	createUserCmd                   *models.CreateUserCommand
+	disableUserCmd                  *models.DisableUserCommand
+	addOrgUserCmd                   *models.AddOrgUserCommand
+	updateOrgUserCmd                *models.UpdateOrgUserCommand
+	removeOrgUserCmd                *models.RemoveOrgUserCommand
+	updateUserCmd                   *models.UpdateUserCommand
+	setUsingOrgCmd                  *models.SetUsingOrgCommand
+	updateUserPermissionsCmd        *models.UpdateUserPermissionsCommand
+	disableExternalUserCalled       bool
 }
 
 func (sc *scenarioContext) userQueryReturns(user *models.User) {
@@ -177,4 +192,15 @@ func (sc *scenarioContext) userOrgsQueryReturns(orgs []*models.UserOrgDTO) {
 	})
 }
 
+func (sc *scenarioContext) getExternalUserInfoByLoginQueryReturns(externalUser *models.ExternalUserInfo) {
+	bus.AddHandler("test", func(cmd *models.GetExternalUserInfoByLoginQuery) error {
+		sc.getExternalUserInfoByLoginQuery = cmd
+		sc.getExternalUserInfoByLoginQuery.Result = &models.ExternalUserInfo{
+			UserId:     externalUser.UserId,
+			IsDisabled: externalUser.IsDisabled,
+		}
+		return nil
+	})
+}
+
 type scenarioFunc func(c *scenarioContext)

+ 27 - 20
pkg/services/login/login.go

@@ -3,7 +3,7 @@ package login
 import (
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/infra/log"
-	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/registry"
 	"github.com/grafana/grafana/pkg/services/quota"
 )
@@ -27,10 +27,10 @@ func (ls *LoginService) Init() error {
 	return nil
 }
 
-func (ls *LoginService) UpsertUser(cmd *m.UpsertUserCommand) error {
+func (ls *LoginService) UpsertUser(cmd *models.UpsertUserCommand) error {
 	extUser := cmd.ExternalUser
 
-	userQuery := &m.GetUserByAuthInfoQuery{
+	userQuery := &models.GetUserByAuthInfoQuery{
 		AuthModule: extUser.AuthModule,
 		AuthId:     extUser.AuthId,
 		UserId:     extUser.UserId,
@@ -39,7 +39,7 @@ func (ls *LoginService) UpsertUser(cmd *m.UpsertUserCommand) error {
 	}
 
 	err := bus.Dispatch(userQuery)
-	if err != m.ErrUserNotFound && err != nil {
+	if err != models.ErrUserNotFound && err != nil {
 		return err
 	}
 
@@ -64,7 +64,7 @@ func (ls *LoginService) UpsertUser(cmd *m.UpsertUserCommand) error {
 		}
 
 		if extUser.AuthModule != "" {
-			cmd2 := &m.SetAuthInfoCommand{
+			cmd2 := &models.SetAuthInfoCommand{
 				UserId:     cmd.Result.Id,
 				AuthModule: extUser.AuthModule,
 				AuthId:     extUser.AuthId,
@@ -90,6 +90,13 @@ func (ls *LoginService) UpsertUser(cmd *m.UpsertUserCommand) error {
 				return err
 			}
 		}
+
+		if extUser.AuthModule == models.AuthModuleLDAP && userQuery.Result.IsDisabled {
+			// Re-enable user when it found in LDAP
+			if err := ls.Bus.Dispatch(&models.DisableUserCommand{UserId: cmd.Result.Id, IsDisabled: false}); err != nil {
+				return err
+			}
+		}
 	}
 
 	err = syncOrgRoles(cmd.Result, extUser)
@@ -100,12 +107,12 @@ func (ls *LoginService) UpsertUser(cmd *m.UpsertUserCommand) error {
 
 	// Sync isGrafanaAdmin permission
 	if extUser.IsGrafanaAdmin != nil && *extUser.IsGrafanaAdmin != cmd.Result.IsAdmin {
-		if err := ls.Bus.Dispatch(&m.UpdateUserPermissionsCommand{UserId: cmd.Result.Id, IsGrafanaAdmin: *extUser.IsGrafanaAdmin}); err != nil {
+		if err := ls.Bus.Dispatch(&models.UpdateUserPermissionsCommand{UserId: cmd.Result.Id, IsGrafanaAdmin: *extUser.IsGrafanaAdmin}); err != nil {
 			return err
 		}
 	}
 
-	err = ls.Bus.Dispatch(&m.SyncTeamsCommand{
+	err = ls.Bus.Dispatch(&models.SyncTeamsCommand{
 		User:         cmd.Result,
 		ExternalUser: extUser,
 	})
@@ -117,8 +124,8 @@ func (ls *LoginService) UpsertUser(cmd *m.UpsertUserCommand) error {
 	return err
 }
 
-func createUser(extUser *m.ExternalUserInfo) (*m.User, error) {
-	cmd := &m.CreateUserCommand{
+func createUser(extUser *models.ExternalUserInfo) (*models.User, error) {
+	cmd := &models.CreateUserCommand{
 		Login:        extUser.Login,
 		Email:        extUser.Email,
 		Name:         extUser.Name,
@@ -132,9 +139,9 @@ func createUser(extUser *m.ExternalUserInfo) (*m.User, error) {
 	return &cmd.Result, nil
 }
 
-func updateUser(user *m.User, extUser *m.ExternalUserInfo) error {
+func updateUser(user *models.User, extUser *models.ExternalUserInfo) error {
 	// sync user info
-	updateCmd := &m.UpdateUserCommand{
+	updateCmd := &models.UpdateUserCommand{
 		UserId: user.Id,
 	}
 
@@ -165,8 +172,8 @@ func updateUser(user *m.User, extUser *m.ExternalUserInfo) error {
 	return bus.Dispatch(updateCmd)
 }
 
-func updateUserAuth(user *m.User, extUser *m.ExternalUserInfo) error {
-	updateCmd := &m.UpdateAuthInfoCommand{
+func updateUserAuth(user *models.User, extUser *models.ExternalUserInfo) error {
+	updateCmd := &models.UpdateAuthInfoCommand{
 		AuthModule: extUser.AuthModule,
 		AuthId:     extUser.AuthId,
 		UserId:     user.Id,
@@ -177,13 +184,13 @@ func updateUserAuth(user *m.User, extUser *m.ExternalUserInfo) error {
 	return bus.Dispatch(updateCmd)
 }
 
-func syncOrgRoles(user *m.User, extUser *m.ExternalUserInfo) error {
+func syncOrgRoles(user *models.User, extUser *models.ExternalUserInfo) error {
 	// don't sync org roles if none are specified
 	if len(extUser.OrgRoles) == 0 {
 		return nil
 	}
 
-	orgsQuery := &m.GetUserOrgListQuery{UserId: user.Id}
+	orgsQuery := &models.GetUserOrgListQuery{UserId: user.Id}
 	if err := bus.Dispatch(orgsQuery); err != nil {
 		return err
 	}
@@ -199,7 +206,7 @@ func syncOrgRoles(user *m.User, extUser *m.ExternalUserInfo) error {
 			deleteOrgIds = append(deleteOrgIds, org.OrgId)
 		} else if extUser.OrgRoles[org.OrgId] != org.Role {
 			// update role
-			cmd := &m.UpdateOrgUserCommand{OrgId: org.OrgId, UserId: user.Id, Role: extUser.OrgRoles[org.OrgId]}
+			cmd := &models.UpdateOrgUserCommand{OrgId: org.OrgId, UserId: user.Id, Role: extUser.OrgRoles[org.OrgId]}
 			if err := bus.Dispatch(cmd); err != nil {
 				return err
 			}
@@ -213,16 +220,16 @@ func syncOrgRoles(user *m.User, extUser *m.ExternalUserInfo) error {
 		}
 
 		// add role
-		cmd := &m.AddOrgUserCommand{UserId: user.Id, Role: orgRole, OrgId: orgId}
+		cmd := &models.AddOrgUserCommand{UserId: user.Id, Role: orgRole, OrgId: orgId}
 		err := bus.Dispatch(cmd)
-		if err != nil && err != m.ErrOrgNotFound {
+		if err != nil && err != models.ErrOrgNotFound {
 			return err
 		}
 	}
 
 	// delete any removed org roles
 	for _, orgId := range deleteOrgIds {
-		cmd := &m.RemoveOrgUserCommand{OrgId: orgId, UserId: user.Id}
+		cmd := &models.RemoveOrgUserCommand{OrgId: orgId, UserId: user.Id}
 		if err := bus.Dispatch(cmd); err != nil {
 			return err
 		}
@@ -235,7 +242,7 @@ func syncOrgRoles(user *m.User, extUser *m.ExternalUserInfo) error {
 			break
 		}
 
-		return bus.Dispatch(&m.SetUsingOrgCommand{
+		return bus.Dispatch(&models.SetUsingOrgCommand{
 			UserId: user.Id,
 			OrgId:  user.OrgId,
 		})

+ 6 - 0
pkg/services/sqlstore/migrations/user_mig.go

@@ -116,6 +116,12 @@ func addUserMigrations(mg *Migrator) {
 
 	// Adds salt & rands for old users who used ldap or oauth
 	mg.AddMigration("Add missing user data", &AddMissingUserSaltAndRandsMigration{})
+
+	// is_disabled indicates whether user disabled or not. Disabled user should not be able to log in.
+	// This field used in couple with LDAP auth to disable users removed from LDAP rather than delete it immediately.
+	mg.AddMigration("Add is_disabled column to user", NewAddColumnMigration(userV2, &Column{
+		Name: "is_disabled", Type: DB_Bool, Nullable: false, Default: "0",
+	}))
 }
 
 type AddMissingUserSaltAndRandsMigration struct {

+ 15 - 1
pkg/services/sqlstore/user.go

@@ -27,6 +27,7 @@ func (ss *SqlStore) addUserQueryAndCommandHandlers() {
 	bus.AddHandler("sql", GetUserProfile)
 	bus.AddHandler("sql", SearchUsers)
 	bus.AddHandler("sql", GetUserOrgList)
+	bus.AddHandler("sql", DisableUser)
 	bus.AddHandler("sql", DeleteUser)
 	bus.AddHandler("sql", UpdateUserPermissions)
 	bus.AddHandler("sql", SetUserHelpFlag)
@@ -326,6 +327,7 @@ func GetUserProfile(query *m.GetUserProfileQuery) error {
 		Login:          user.Login,
 		Theme:          user.Theme,
 		IsGrafanaAdmin: user.IsAdmin,
+		IsDisabled:     user.IsDisabled,
 		OrgId:          user.OrgId,
 	}
 
@@ -450,7 +452,7 @@ func SearchUsers(query *m.SearchUsersQuery) error {
 
 	offset := query.Limit * (query.Page - 1)
 	sess.Limit(query.Limit, offset)
-	sess.Cols("id", "email", "name", "login", "is_admin", "last_seen_at")
+	sess.Cols("id", "email", "name", "login", "is_admin", "is_disabled", "last_seen_at")
 	if err := sess.Find(&query.Result.Users); err != nil {
 		return err
 	}
@@ -473,6 +475,18 @@ func SearchUsers(query *m.SearchUsersQuery) error {
 	return err
 }
 
+func DisableUser(cmd *m.DisableUserCommand) error {
+	user := m.User{}
+	sess := x.Table("user")
+	sess.ID(cmd.UserId).Get(&user)
+
+	user.IsDisabled = cmd.IsDisabled
+	sess.UseBool("is_disabled")
+
+	_, err := sess.ID(cmd.UserId).Update(&user)
+	return err
+}
+
 func DeleteUser(cmd *m.DeleteUserCommand) error {
 	return inTransaction(func(sess *DBSession) error {
 		return deleteUserInTransaction(sess, cmd)

+ 45 - 20
pkg/services/sqlstore/user_auth.go

@@ -5,7 +5,7 @@ import (
 	"time"
 
 	"github.com/grafana/grafana/pkg/bus"
-	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
 )
@@ -14,17 +14,18 @@ var getTime = time.Now
 
 func init() {
 	bus.AddHandler("sql", GetUserByAuthInfo)
+	bus.AddHandler("sql", GetExternalUserInfoByLogin)
 	bus.AddHandler("sql", GetAuthInfo)
 	bus.AddHandler("sql", SetAuthInfo)
 	bus.AddHandler("sql", UpdateAuthInfo)
 	bus.AddHandler("sql", DeleteAuthInfo)
 }
 
-func GetUserByAuthInfo(query *m.GetUserByAuthInfoQuery) error {
-	user := &m.User{}
+func GetUserByAuthInfo(query *models.GetUserByAuthInfoQuery) error {
+	user := &models.User{}
 	has := false
 	var err error
-	authQuery := &m.GetAuthInfoQuery{}
+	authQuery := &models.GetAuthInfoQuery{}
 
 	// Try to find the user by auth module and id first
 	if query.AuthModule != "" && query.AuthId != "" {
@@ -32,14 +33,14 @@ func GetUserByAuthInfo(query *m.GetUserByAuthInfoQuery) error {
 		authQuery.AuthId = query.AuthId
 
 		err = GetAuthInfo(authQuery)
-		if err != m.ErrUserNotFound {
+		if err != models.ErrUserNotFound {
 			if err != nil {
 				return err
 			}
 
 			// if user id was specified and doesn't match the user_auth entry, remove it
 			if query.UserId != 0 && query.UserId != authQuery.Result.UserId {
-				err = DeleteAuthInfo(&m.DeleteAuthInfoCommand{
+				err = DeleteAuthInfo(&models.DeleteAuthInfoCommand{
 					UserAuth: authQuery.Result,
 				})
 				if err != nil {
@@ -55,7 +56,7 @@ func GetUserByAuthInfo(query *m.GetUserByAuthInfoQuery) error {
 
 				if !has {
 					// if the user has been deleted then remove the entry
-					err = DeleteAuthInfo(&m.DeleteAuthInfoCommand{
+					err = DeleteAuthInfo(&models.DeleteAuthInfoCommand{
 						UserAuth: authQuery.Result,
 					})
 					if err != nil {
@@ -78,7 +79,7 @@ func GetUserByAuthInfo(query *m.GetUserByAuthInfoQuery) error {
 
 	// If not found, try to find the user by email address
 	if !has && query.Email != "" {
-		user = &m.User{Email: query.Email}
+		user = &models.User{Email: query.Email}
 		has, err = x.Get(user)
 		if err != nil {
 			return err
@@ -87,7 +88,7 @@ func GetUserByAuthInfo(query *m.GetUserByAuthInfoQuery) error {
 
 	// If not found, try to find the user by login
 	if !has && query.Login != "" {
-		user = &m.User{Login: query.Login}
+		user = &models.User{Login: query.Login}
 		has, err = x.Get(user)
 		if err != nil {
 			return err
@@ -96,12 +97,12 @@ func GetUserByAuthInfo(query *m.GetUserByAuthInfoQuery) error {
 
 	// No user found
 	if !has {
-		return m.ErrUserNotFound
+		return models.ErrUserNotFound
 	}
 
 	// create authInfo record to link accounts
 	if authQuery.Result == nil && query.AuthModule != "" {
-		cmd2 := &m.SetAuthInfoCommand{
+		cmd2 := &models.SetAuthInfoCommand{
 			UserId:     user.Id,
 			AuthModule: query.AuthModule,
 			AuthId:     query.AuthId,
@@ -115,8 +116,32 @@ func GetUserByAuthInfo(query *m.GetUserByAuthInfoQuery) error {
 	return nil
 }
 
-func GetAuthInfo(query *m.GetAuthInfoQuery) error {
-	userAuth := &m.UserAuth{
+func GetExternalUserInfoByLogin(query *models.GetExternalUserInfoByLoginQuery) error {
+	userQuery := models.GetUserByLoginQuery{LoginOrEmail: query.LoginOrEmail}
+	err := bus.Dispatch(&userQuery)
+	if err != nil {
+		return err
+	}
+
+	authInfoQuery := &models.GetAuthInfoQuery{UserId: userQuery.Result.Id}
+	if err := bus.Dispatch(authInfoQuery); err != nil {
+		return err
+	}
+
+	query.Result = &models.ExternalUserInfo{
+		UserId:     userQuery.Result.Id,
+		Login:      userQuery.Result.Login,
+		Email:      userQuery.Result.Email,
+		Name:       userQuery.Result.Name,
+		IsDisabled: userQuery.Result.IsDisabled,
+		AuthModule: authInfoQuery.Result.AuthModule,
+		AuthId:     authInfoQuery.Result.AuthId,
+	}
+	return nil
+}
+
+func GetAuthInfo(query *models.GetAuthInfoQuery) error {
+	userAuth := &models.UserAuth{
 		UserId:     query.UserId,
 		AuthModule: query.AuthModule,
 		AuthId:     query.AuthId,
@@ -126,7 +151,7 @@ func GetAuthInfo(query *m.GetAuthInfoQuery) error {
 		return err
 	}
 	if !has {
-		return m.ErrUserNotFound
+		return models.ErrUserNotFound
 	}
 
 	secretAccessToken, err := decodeAndDecrypt(userAuth.OAuthAccessToken)
@@ -149,9 +174,9 @@ func GetAuthInfo(query *m.GetAuthInfoQuery) error {
 	return nil
 }
 
-func SetAuthInfo(cmd *m.SetAuthInfoCommand) error {
+func SetAuthInfo(cmd *models.SetAuthInfoCommand) error {
 	return inTransaction(func(sess *DBSession) error {
-		authUser := &m.UserAuth{
+		authUser := &models.UserAuth{
 			UserId:     cmd.UserId,
 			AuthModule: cmd.AuthModule,
 			AuthId:     cmd.AuthId,
@@ -183,9 +208,9 @@ func SetAuthInfo(cmd *m.SetAuthInfoCommand) error {
 	})
 }
 
-func UpdateAuthInfo(cmd *m.UpdateAuthInfoCommand) error {
+func UpdateAuthInfo(cmd *models.UpdateAuthInfoCommand) error {
 	return inTransaction(func(sess *DBSession) error {
-		authUser := &m.UserAuth{
+		authUser := &models.UserAuth{
 			UserId:     cmd.UserId,
 			AuthModule: cmd.AuthModule,
 			AuthId:     cmd.AuthId,
@@ -212,7 +237,7 @@ func UpdateAuthInfo(cmd *m.UpdateAuthInfoCommand) error {
 			authUser.OAuthExpiry = cmd.OAuthToken.Expiry
 		}
 
-		cond := &m.UserAuth{
+		cond := &models.UserAuth{
 			UserId:     cmd.UserId,
 			AuthModule: cmd.AuthModule,
 		}
@@ -222,7 +247,7 @@ func UpdateAuthInfo(cmd *m.UpdateAuthInfoCommand) error {
 	})
 }
 
-func DeleteAuthInfo(cmd *m.DeleteAuthInfoCommand) error {
+func DeleteAuthInfo(cmd *models.DeleteAuthInfoCommand) error {
 	return inTransaction(func(sess *DBSession) error {
 		_, err := sess.Delete(cmd.UserAuth)
 		return err