[LDAP] Disable removed users on login (#74016)

* [LDAP] Disable removed users on login

* Fix tests

* Add test for user disabling

* Add tests for disabling user behind auth proxy

* Linting.

* Rename setup func

* Account for reviews comments

Co-authored-by: Kalle Persson <kalle.persson@grafana.com>

---------

Co-authored-by: Kalle Persson <kalle.persson@grafana.com>

pkg/services/authn/authnimpl/service.go

@@ -97,7 +97,7 @@ func ProvideService(
 	var proxyClients []authn.ProxyClient
 	var passwordClients []authn.PasswordClient
 	if s.cfg.LDAPAuthEnabled {
-		ldap := clients.ProvideLDAP(cfg, ldapService)
+		ldap := clients.ProvideLDAP(cfg, ldapService, userService, authInfoService)
 		proxyClients = append(proxyClients, ldap)
 		passwordClients = append(passwordClients, ldap)
 	}

pkg/services/authn/clients/ldap.go

@@ -4,9 +4,11 @@ import (
 	"context"
 	"errors"
 
+	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/authn"
 	"github.com/grafana/grafana/pkg/services/ldap/multildap"
 	"github.com/grafana/grafana/pkg/services/login"
+	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
@@ -18,13 +20,16 @@ type ldapService interface {
 	User(username string) (*login.ExternalUserInfo, error)
 }
 
-func ProvideLDAP(cfg *setting.Cfg, ldapService ldapService) *LDAP {
+func ProvideLDAP(cfg *setting.Cfg, ldapService ldapService, userService user.Service, authInfoService login.AuthInfoService) *LDAP {
-	return &LDAP{cfg, ldapService}
+	return &LDAP{cfg, log.New("authn.ldap"), ldapService, userService, authInfoService}
 }
 
 type LDAP struct {
-	cfg     *setting.Cfg
+	cfg             *setting.Cfg
-	service ldapService
+	logger          log.Logger
+	service         ldapService
+	userService     user.Service
+	authInfoService login.AuthInfoService
 }
 
 func (c *LDAP) String() string {
@@ -34,7 +39,7 @@ func (c *LDAP) String() string {
 func (c *LDAP) AuthenticateProxy(ctx context.Context, r *authn.Request, username string, _ map[string]string) (*authn.Identity, error) {
 	info, err := c.service.User(username)
 	if errors.Is(err, multildap.ErrDidNotFindUser) {
-		return nil, errIdentityNotFound.Errorf("no user found: %w", err)
+		return c.disableUser(ctx, username)
 	}
 
 	if err != nil {
@@ -51,8 +56,7 @@ func (c *LDAP) AuthenticatePassword(ctx context.Context, r *authn.Request, usern
 	})
 
 	if errors.Is(err, multildap.ErrCouldNotFindUser) {
-		// FIXME: disable user in grafana if not found
+		return c.disableUser(ctx, username)
-		return nil, errIdentityNotFound.Errorf("no user found: %w", err)
 	}
 
 	// user was found so set auth module in req metadata
@@ -69,6 +73,39 @@ func (c *LDAP) AuthenticatePassword(ctx context.Context, r *authn.Request, usern
 	return c.identityFromLDAPInfo(r.OrgID, info), nil
 }
 
+// disableUser will disable users if they logged in via LDAP previously
+func (c *LDAP) disableUser(ctx context.Context, username string) (*authn.Identity, error) {
+	c.logger.Debug("user was not found in the LDAP directory tree", "username", username)
+	retErr := errIdentityNotFound.Errorf("no user found: %w", multildap.ErrDidNotFindUser)
+
+	// Retrieve the user from store based on the login
+	dbUser, errGet := c.userService.GetByLogin(ctx, &user.GetUserByLoginQuery{
+		LoginOrEmail: username,
+	})
+	if errors.Is(errGet, user.ErrUserNotFound) {
+		return nil, retErr
+	} else if errGet != nil {
+		return nil, errGet
+	}
+
+	// Check if the user logged in via LDAP
+	query := &login.GetAuthInfoQuery{UserId: dbUser.ID, AuthModule: login.LDAPAuthModule}
+	authinfo, errGetAuthInfo := c.authInfoService.GetAuthInfo(ctx, query)
+	if errors.Is(errGetAuthInfo, user.ErrUserNotFound) {
+		return nil, retErr
+	} else if errGetAuthInfo != nil {
+		return nil, errGetAuthInfo
+	}
+
+	// Disable the user
+	c.logger.Debug("user was removed from the LDAP directory tree, disabling it", "username", username, "authID", authinfo.AuthId)
+	if errDisable := c.userService.Disable(ctx, &user.DisableUserCommand{UserID: dbUser.ID, IsDisabled: true}); errDisable != nil {
+		return nil, errDisable
+	}
+
+	return nil, retErr
+}
+
 func (c *LDAP) identityFromLDAPInfo(orgID int64, info *login.ExternalUserInfo) *authn.Identity {
 	return &authn.Identity{
 		OrgID:           orgID,

pkg/services/authn/clients/ldap_test.go

@@ -6,26 +6,39 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
+	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/authn"
 	"github.com/grafana/grafana/pkg/services/ldap"
 	"github.com/grafana/grafana/pkg/services/ldap/multildap"
 	"github.com/grafana/grafana/pkg/services/ldap/service"
 	"github.com/grafana/grafana/pkg/services/login"
+	"github.com/grafana/grafana/pkg/services/login/logintest"
 	"github.com/grafana/grafana/pkg/services/org"
+	"github.com/grafana/grafana/pkg/services/user"
+	"github.com/grafana/grafana/pkg/services/user/usertest"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
-func TestLDAP_AuthenticateProxy(t *testing.T) {
+type ldapTestCase struct {
-	type testCase struct {
+	desc             string
-		desc             string
+	username         string
-		username         string
+	password         string
-		expectedLDAPErr  error
+	expectedErr      error
-		expectedLDAPInfo *login.ExternalUserInfo
+	expectedLDAPErr  error
-		expectedErr      error
+	expectedLDAPInfo *login.ExternalUserInfo
-		expectedIdentity *authn.Identity
+	expectedIdentity *authn.Identity
-	}
 
-	tests := []testCase{
+	// Disabling User
+	expectedUser        user.User
+	expectedUserErr     error
+	expectedAuthInfo    login.UserAuth
+	expectedAuthInfoErr error
+	disableCalled       bool
+	expectDisable       bool
+}
+
+func TestLDAP_AuthenticateProxy(t *testing.T) {
+	tests := []ldapTestCase{
 		{
 			desc:     "should return valid identity when found by ldap service",
 			username: "test",
@@ -65,32 +78,35 @@ func TestLDAP_AuthenticateProxy(t *testing.T) {
 			desc:            "should return error when user is not found",
 			username:        "test",
 			expectedLDAPErr: multildap.ErrDidNotFindUser,
+			expectedUserErr: user.ErrUserNotFound,
 			expectedErr:     errIdentityNotFound,
 		},
+		{
+			desc:             "should disable user when user is not found",
+			username:         "test",
+			expectedLDAPErr:  multildap.ErrDidNotFindUser,
+			expectedUser:     user.User{ID: 11, Login: "test"},
+			expectedAuthInfo: login.UserAuth{UserId: 11, AuthId: "cn=test,ou=users,dc=example,dc=org", AuthModule: login.LDAPAuthModule},
+			expectDisable:    true,
+			expectedErr:      errIdentityNotFound,
+		},
 	}
 
-	for _, tt := range tests {
+	for i := range tests {
+		tt := tests[i]
 		t.Run(tt.desc, func(t *testing.T) {
-			c := &LDAP{cfg: setting.NewCfg(), service: &service.LDAPFakeService{ExpectedUser: tt.expectedLDAPInfo, ExpectedError: tt.expectedLDAPErr}}
+			c := setupLDAPTestCase(&tt)
+
 			identity, err := c.AuthenticateProxy(context.Background(), &authn.Request{OrgID: 1}, tt.username, nil)
 			assert.ErrorIs(t, err, tt.expectedErr)
 			assert.EqualValues(t, tt.expectedIdentity, identity)
+			assert.Equal(t, tt.expectDisable, tt.disableCalled)
 		})
 	}
 }
 
 func TestLDAP_AuthenticatePassword(t *testing.T) {
-	type testCase struct {
+	tests := []ldapTestCase{
-		desc             string
-		username         string
-		password         string
-		expectedErr      error
-		expectedLDAPErr  error
-		expectedLDAPInfo *login.ExternalUserInfo
-		expectedIdentity *authn.Identity
-	}
-
-	tests := []testCase{
 		{
 			desc:     "should successfully authenticate with correct username and password",
 			username: "test",
@@ -140,20 +156,58 @@ func TestLDAP_AuthenticatePassword(t *testing.T) {
 			password:        "wrong",
 			expectedErr:     errIdentityNotFound,
 			expectedLDAPErr: ldap.ErrCouldNotFindUser,
+			expectedUserErr: user.ErrUserNotFound,
+		},
+		{
+			desc:             "should disable user if not found",
+			username:         "test",
+			password:         "wrong",
+			expectedErr:      errIdentityNotFound,
+			expectedLDAPErr:  ldap.ErrCouldNotFindUser,
+			expectedUser:     user.User{ID: 11, Login: "test"},
+			expectedAuthInfo: login.UserAuth{UserId: 11, AuthId: "cn=test,ou=users,dc=example,dc=org", AuthModule: login.LDAPAuthModule},
+			expectDisable:    true,
 		},
 	}
 
-	for _, tt := range tests {
+	for i := range tests {
+		tt := tests[i]
 		t.Run(tt.desc, func(t *testing.T) {
-			c := &LDAP{cfg: setting.NewCfg(), service: &service.LDAPFakeService{ExpectedUser: tt.expectedLDAPInfo, ExpectedError: tt.expectedLDAPErr}}
+			c := setupLDAPTestCase(&tt)
 
 			identity, err := c.AuthenticatePassword(context.Background(), &authn.Request{OrgID: 1}, tt.username, tt.password)
 			assert.ErrorIs(t, err, tt.expectedErr)
 			assert.EqualValues(t, tt.expectedIdentity, identity)
+			assert.Equal(t, tt.expectDisable, tt.disableCalled)
 		})
 	}
 }
 
+func setupLDAPTestCase(tt *ldapTestCase) *LDAP {
+	userService := &usertest.FakeUserService{
+		ExpectedError: tt.expectedUserErr,
+		ExpectedUser:  &tt.expectedUser,
+		DisableFn: func(ctx context.Context, cmd *user.DisableUserCommand) error {
+			tt.disableCalled = true
+			return nil
+		},
+	}
+	authInfoService := &logintest.AuthInfoServiceFake{
+		ExpectedUserAuth: &tt.expectedAuthInfo,
+		ExpectedError:    tt.expectedAuthInfoErr,
+	}
+
+	c := &LDAP{
+		cfg:             setting.NewCfg(),
+		logger:          log.New("authn.ldap.test"),
+		service:         &service.LDAPFakeService{ExpectedUser: tt.expectedLDAPInfo, ExpectedError: tt.expectedLDAPErr},
+		userService:     userService,
+		authInfoService: authInfoService,
+	}
+
+	return c
+}
+
 func strPtr(s string) *string {
 	return &s
 }