Auth: Add IP address login attempt validation (#98123)

* Auth: Add IP address login attempt validation

* LoginAttempt struct IpAddress field must be camelCase to match db ip_address column

* add setting DisableIPAddressLoginProtection

* lint

* add DisableIPAddressLoginProtection setting to tests

* add request object to authenticate password test

* nit suggestions & rename tests

* add login attempt on failed password authentication

* dont need to reset login attempts if successful

* don't change error message

* revert go.work.sum

* Update pkg/services/authn/clients/password.go

Co-authored-by: Misi <mgyongyosi@users.noreply.github.com>

---------

Co-authored-by: Misi <mgyongyosi@users.noreply.github.com>

conf/defaults.ini

@@ -354,6 +354,9 @@ disable_brute_force_login_protection = false
 # max number of failed login attempts before user gets locked
 brute_force_login_protection_max_attempts = 5
 
+# disable protection against brute force login attempts by IP address
+disable_ip_address_login_protection = true
+
 # set to true if you host Grafana behind HTTPS. default is false.
 cookie_secure = false
 

conf/sample.ini

@@ -353,6 +353,9 @@
 # max number of failed login attempts before user gets locked
 ;brute_force_login_protection_max_attempts = 5
 
+# disable protection against brute force login attempts by IP address
+; disable_ip_address_login_protection = true
+
 # set to true if you host Grafana behind HTTPS. default is false.
 ;cookie_secure = false
 

docs/sources/setup-grafana/configure-grafana/_index.md

@@ -700,6 +700,10 @@ An existing user's account is unable to login for five minutes if all login atte
 Configure how many login attempts a user can have within a five minute window before their account is locked.
 Default is `5`.
 
+#### `disable_ip_address_login_protection`
+
+Set to `true` to disable [brute force login protection by IP address](https://cheatsheetseries.owasp.org/cheatsheets/Authentication_Cheat_Sheet.html#account-lockout). Default is `true`. Anyone from the IP address will be unable to login for 5 minutes if all login attempts are spent within a 5 minute window.
+
 #### `cookie_secure`
 
 Set to `true` if you host Grafana behind HTTPS. Default is `false`.

pkg/services/authn/clients/password.go

@@ -39,6 +39,14 @@ func (c *Password) AuthenticatePassword(ctx context.Context, r *authn.Request, u
 		return nil, errPasswordAuthFailed.Errorf("too many consecutive incorrect login attempts for user - login for user temporarily blocked")
 	}
 
+	ok, err = c.loginAttempts.ValidateIPAddress(ctx, web.RemoteAddr(r.HTTPRequest))
+	if err != nil {
+		return nil, err
+	}
+	if !ok {
+		return nil, errPasswordlessClientTooManyLoginAttempts.Errorf("too many consecutive incorrect login attempts for IP address - login for IP address temporarily blocked")
+	}
+
 	if len(password) == 0 {
 		return nil, errPasswordAuthFailed.Errorf("no password provided")
 	}
@@ -56,8 +64,9 @@ func (c *Password) AuthenticatePassword(ctx context.Context, r *authn.Request, u
 		return identity, nil
 	}
 
-	if errors.Is(clientErrs, errInvalidPassword) {
-		_ = c.loginAttempts.Add(ctx, username, web.RemoteAddr(r.HTTPRequest))
+	err = c.loginAttempts.Add(ctx, username, web.RemoteAddr(r.HTTPRequest))
+	if err != nil {
+		return nil, err
 	}
 
 	return nil, errPasswordAuthFailed.Errorf("failed to authenticate identity: %w", clientErrs)

pkg/services/authn/clients/password_test.go

@@ -2,6 +2,8 @@ package clients
 
 import (
 	"context"
+	"net/http"
+	"net/url"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -18,7 +20,6 @@ func TestPassword_AuthenticatePassword(t *testing.T) {
 		desc             string
 		username         string
 		password         string
-		req              *authn.Request
 		blockLogin       bool
 		clients          []authn.PasswordClient
 		expectedErr      error
@@ -30,7 +31,6 @@ func TestPassword_AuthenticatePassword(t *testing.T) {
 			desc:             "should success when password client return identity",
 			username:         "test",
 			password:         "test",
-			req:              &authn.Request{},
 			clients:          []authn.PasswordClient{authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: "1", Type: claims.TypeUser}}},
 			expectedIdentity: &authn.Identity{ID: "1", Type: claims.TypeUser},
 		},
@@ -38,7 +38,6 @@ func TestPassword_AuthenticatePassword(t *testing.T) {
 			desc:             "should success when found in second client",
 			username:         "test",
 			password:         "test",
-			req:              &authn.Request{},
 			clients:          []authn.PasswordClient{authntest.FakePasswordClient{ExpectedErr: errIdentityNotFound}, authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: "2", Type: claims.TypeUser}}},
 			expectedIdentity: &authn.Identity{ID: "2", Type: claims.TypeUser},
 		},
@@ -46,14 +45,12 @@ func TestPassword_AuthenticatePassword(t *testing.T) {
 			desc:        "should fail for empty password",
 			username:    "test",
 			password:    "",
-			req:         &authn.Request{},
 			expectedErr: errPasswordAuthFailed,
 		},
 		{
 			desc:        "should if login is blocked by to many attempts",
 			username:    "test",
 			password:    "test",
-			req:         &authn.Request{},
 			blockLogin:  true,
 			expectedErr: errPasswordAuthFailed,
 		},
@@ -61,7 +58,6 @@ func TestPassword_AuthenticatePassword(t *testing.T) {
 			desc:        "should fail when not found in any clients",
 			username:    "test",
 			password:    "test",
-			req:         &authn.Request{},
 			clients:     []authn.PasswordClient{authntest.FakePasswordClient{ExpectedErr: errIdentityNotFound}, authntest.FakePasswordClient{ExpectedErr: errIdentityNotFound}},
 			expectedErr: errPasswordAuthFailed,
 		},
@@ -70,8 +66,22 @@ func TestPassword_AuthenticatePassword(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
 			c := ProvidePassword(loginattempttest.FakeLoginAttemptService{ExpectedValid: !tt.blockLogin}, tt.clients...)
-
-			identity, err := c.AuthenticatePassword(context.Background(), tt.req, tt.username, tt.password)
+			r := &authn.Request{
+				OrgID: 12345,
+				HTTPRequest: &http.Request{
+					Method: "GET",
+					URL: &url.URL{
+						Scheme: "https",
+						Host:   "example.com",
+						Path:   "/api/v1/resource",
+					},
+					Header: http.Header{
+						"Content-Type": []string{"application/json"},
+						"User-Agent":   []string{"MyApp/1.0"},
+					},
+				},
+			}
+			identity, err := c.AuthenticatePassword(context.Background(), r, tt.username, tt.password)
 			if tt.expectedErr != nil {
 				assert.ErrorIs(t, err, tt.expectedErr)
 				assert.Nil(t, identity)

pkg/services/authn/clients/passwordless.go

@@ -105,7 +105,6 @@ func (c *Passwordless) RedirectURL(ctx context.Context, r *authn.Request) (*auth
 		return nil, err
 	}
 
-	// TODO: add IP address validation
 	ok, err := c.loginAttempts.Validate(ctx, form.Email)
 	if err != nil {
 		return nil, err
@@ -115,6 +114,15 @@ func (c *Passwordless) RedirectURL(ctx context.Context, r *authn.Request) (*auth
 		return nil, errPasswordlessClientTooManyLoginAttempts.Errorf("too many consecutive incorrect login attempts for user - login for user temporarily blocked")
 	}
 
+	ok, err = c.loginAttempts.ValidateIPAddress(ctx, web.RemoteAddr(r.HTTPRequest))
+	if err != nil {
+		return nil, err
+	}
+
+	if !ok {
+		return nil, errPasswordlessClientTooManyLoginAttempts.Errorf("too many consecutive incorrect login attempts for IP address - login for IP address temporarily blocked")
+	}
+
 	err = c.loginAttempts.Add(ctx, form.Email, web.RemoteAddr(r.HTTPRequest))
 	if err != nil {
 		return nil, err

pkg/services/loginattempt/login_attempt.go

@@ -6,10 +6,13 @@ import (
 
 type Service interface {
 	// Add adds a new login attempt record for provided username
-	Add(ctx context.Context, username, IPAddress string) error
+	Add(ctx context.Context, username, ipAddress string) error
 	// Validate checks if username has to many login attempts inside a window.
 	// Will return true if provided username do not have too many attempts.
 	Validate(ctx context.Context, username string) (bool, error)
+	// Validate checks if IP address has to many login attempts inside a window.
+	// Will return true if provided IP address do not have too many attempts.
+	ValidateIPAddress(ctx context.Context, ipAddress string) (bool, error)
 	// Reset resets all login attempts attached to username
 	Reset(ctx context.Context, username string) error
 }

pkg/services/loginattempt/loginattemptimpl/login_attempt.go

@@ -53,7 +53,7 @@ func (s *Service) Add(ctx context.Context, username, IPAddress string) error {
 
 	_, err := s.store.CreateLoginAttempt(ctx, CreateLoginAttemptCommand{
 		Username:  strings.ToLower(username),
-		IpAddress: IPAddress,
+		IPAddress: IPAddress,
 	})
 	return err
 }
@@ -84,6 +84,28 @@ func (s *Service) Validate(ctx context.Context, username string) (bool, error) {
 	return true, nil
 }
 
+func (s *Service) ValidateIPAddress(ctx context.Context, IPAddress string) (bool, error) {
+	if s.cfg.DisableIPAddressLoginProtection {
+		return true, nil
+	}
+
+	loginAttemptCountQuery := GetIPLoginAttemptCountQuery{
+		IPAddress: IPAddress,
+		Since:     time.Now().Add(-loginAttemptsWindow),
+	}
+
+	count, err := s.store.GetIPLoginAttemptCount(ctx, loginAttemptCountQuery)
+	if err != nil {
+		return false, err
+	}
+
+	if count >= s.cfg.BruteForceLoginProtectionMaxAttempts {
+		return false, nil
+	}
+
+	return true, nil
+}
+
 func (s *Service) cleanup(ctx context.Context) {
 	err := s.lock.LockAndExecute(ctx, "delete old login attempts", time.Minute*10, func(context.Context) {
 		cmd := DeleteOldLoginAttemptsCommand{

pkg/services/loginattempt/loginattemptimpl/login_attempt_test.go

@@ -22,40 +22,40 @@ func TestService_Validate(t *testing.T) {
 		expectedErr   error
 	}{
 		{
-			name:          "When brute force protection enabled and user login attempt count is less than max",
+			name:          "Should be valid when brute force protection enabled and user login attempt count is less than max",
 			loginAttempts: maxInvalidLoginAttempts - 1,
 			expected:      true,
 			expectedErr:   nil,
 		},
 		{
-			name:          "When brute force protection enabled and user login attempt count equals max",
+			name:          "Should be invalid when brute force protection enabled and user login attempt count equals max",
 			loginAttempts: maxInvalidLoginAttempts,
 			expected:      false,
 			expectedErr:   nil,
 		},
 		{
-			name:          "When brute force protection enabled and user login attempt count is greater than max",
+			name:          "Should be invalid when brute force protection enabled and user login attempt count is greater than max",
 			loginAttempts: maxInvalidLoginAttempts + 1,
 			expected:      false,
 			expectedErr:   nil,
 		},
 
 		{
-			name:          "When brute force protection disabled and user login attempt count is less than max",
+			name:          "Should be valid when brute force protection disabled and user login attempt count is less than max",
 			loginAttempts: maxInvalidLoginAttempts - 1,
 			disabled:      true,
 			expected:      true,
 			expectedErr:   nil,
 		},
 		{
-			name:          "When brute force protection disabled and user login attempt count equals max",
+			name:          "Should be valid when brute force protection disabled and user login attempt count equals max",
 			loginAttempts: maxInvalidLoginAttempts,
 			disabled:      true,
 			expected:      true,
 			expectedErr:   nil,
 		},
 		{
-			name:          "When brute force protection disabled and user login attempt count is greater than max",
+			name:          "Should be valid when brute force protection disabled and user login attempt count is greater than max",
 			loginAttempts: maxInvalidLoginAttempts + 1,
 			disabled:      true,
 			expected:      true,
@@ -83,7 +83,7 @@ func TestService_Validate(t *testing.T) {
 	}
 }
 
-func TestLoginAttempts(t *testing.T) {
+func TestUserLoginAttempts(t *testing.T) {
 	ctx := context.Background()
 	cfg := setting.NewCfg()
 	cfg.DisableBruteForceLoginProtection = false
@@ -109,6 +109,102 @@ func TestLoginAttempts(t *testing.T) {
 	assert.Nil(t, err)
 }
 
+func TestService_ValidateIPAddress(t *testing.T) {
+	const maxInvalidLoginAttempts = 5
+
+	testCases := []struct {
+		name          string
+		loginAttempts int64
+		disabled      bool
+		expected      bool
+		expectedErr   error
+	}{
+		{
+			name:          "Should be valid when brute force protection enabled and IP address login attempt count is less than max",
+			loginAttempts: maxInvalidLoginAttempts - 1,
+			expected:      true,
+			expectedErr:   nil,
+		},
+		{
+			name:          "Should be invalid when brute force protection enabled and IP address login attempt count equals max",
+			loginAttempts: maxInvalidLoginAttempts,
+			expected:      false,
+			expectedErr:   nil,
+		},
+		{
+			name:          "Should be invalid when brute force protection enabled and IP address login attempt count is greater than max",
+			loginAttempts: maxInvalidLoginAttempts + 1,
+			expected:      false,
+			expectedErr:   nil,
+		},
+
+		{
+			name:          "Should be valid when brute force protection disabled and IP address login attempt count is less than max",
+			loginAttempts: maxInvalidLoginAttempts - 1,
+			disabled:      true,
+			expected:      true,
+			expectedErr:   nil,
+		},
+		{
+			name:          "Should be valid when brute force protection disabled and IP address login attempt count equals max",
+			loginAttempts: maxInvalidLoginAttempts,
+			disabled:      true,
+			expected:      true,
+			expectedErr:   nil,
+		},
+		{
+			name:          "Should be valid when brute force protection disabled and IP address login attempt count is greater than max",
+			loginAttempts: maxInvalidLoginAttempts + 1,
+			disabled:      true,
+			expected:      true,
+			expectedErr:   nil,
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := setting.NewCfg()
+			cfg.BruteForceLoginProtectionMaxAttempts = maxInvalidLoginAttempts
+			cfg.DisableIPAddressLoginProtection = tt.disabled
+			service := &Service{
+				store: fakeStore{
+					ExpectedCount: tt.loginAttempts,
+					ExpectedErr:   tt.expectedErr,
+				},
+				cfg: cfg,
+			}
+
+			ok, err := service.ValidateIPAddress(context.Background(), "192.168.1.1")
+			assert.Equal(t, tt.expected, ok)
+			assert.Equal(t, tt.expectedErr, err)
+		})
+	}
+}
+
+func TestIPLoginAttempts(t *testing.T) {
+	ctx := context.Background()
+	cfg := setting.NewCfg()
+	cfg.DisableIPAddressLoginProtection = false
+	cfg.BruteForceLoginProtectionMaxAttempts = 3
+	db := db.InitTestDB(t)
+	service := ProvideService(db, cfg, nil)
+
+	_ = service.Add(ctx, "user1", "192.168.1.1")
+	_ = service.Add(ctx, "user2", "10.0.0.123")
+	_ = service.Add(ctx, "user3", "192.168.1.1")
+	_ = service.Add(ctx, "user4", "[::1]")
+	_ = service.Add(ctx, "user5", "192.168.1.1")
+	_ = service.Add(ctx, "user6", "192.168.1.1")
+
+	count, err := service.store.GetIPLoginAttemptCount(ctx, GetIPLoginAttemptCountQuery{IPAddress: "192.168.1.1"})
+	assert.Nil(t, err)
+	assert.Equal(t, int64(4), count)
+
+	ok, err := service.ValidateIPAddress(ctx, "192.168.1.1")
+	assert.False(t, ok)
+	assert.Nil(t, err)
+}
+
 var _ store = new(fakeStore)
 
 type fakeStore struct {
@@ -121,6 +217,10 @@ func (f fakeStore) GetUserLoginAttemptCount(ctx context.Context, query GetUserLo
 	return f.ExpectedCount, f.ExpectedErr
 }
 
+func (f fakeStore) GetIPLoginAttemptCount(ctx context.Context, query GetIPLoginAttemptCountQuery) (int64, error) {
+	return f.ExpectedCount, f.ExpectedErr
+}
+
 func (f fakeStore) CreateLoginAttempt(ctx context.Context, command CreateLoginAttemptCommand) (loginattempt.LoginAttempt, error) {
 	return loginattempt.LoginAttempt{}, f.ExpectedErr
 }

pkg/services/loginattempt/loginattemptimpl/models.go

@@ -6,7 +6,7 @@ import (
 
 type CreateLoginAttemptCommand struct {
 	Username  string
-	IpAddress string
+	IPAddress string
 }
 
 type GetUserLoginAttemptCountQuery struct {
@@ -14,6 +14,11 @@ type GetUserLoginAttemptCountQuery struct {
 	Since    time.Time
 }
 
+type GetIPLoginAttemptCountQuery struct {
+	IPAddress string
+	Since     time.Time
+}
+
 type DeleteOldLoginAttemptsCommand struct {
 	OlderThan time.Time
 }

pkg/services/loginattempt/loginattemptimpl/store.go

@@ -18,13 +18,14 @@ type store interface {
 	DeleteOldLoginAttempts(ctx context.Context, cmd DeleteOldLoginAttemptsCommand) (int64, error)
 	DeleteLoginAttempts(ctx context.Context, cmd DeleteLoginAttemptsCommand) error
 	GetUserLoginAttemptCount(ctx context.Context, query GetUserLoginAttemptCountQuery) (int64, error)
+	GetIPLoginAttemptCount(ctx context.Context, query GetIPLoginAttemptCountQuery) (int64, error)
 }
 
 func (xs *xormStore) CreateLoginAttempt(ctx context.Context, cmd CreateLoginAttemptCommand) (result loginattempt.LoginAttempt, err error) {
 	err = xs.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
 		loginAttempt := loginattempt.LoginAttempt{
 			Username:  cmd.Username,
-			IpAddress: cmd.IpAddress,
+			IpAddress: cmd.IPAddress,
 			Created:   xs.now().Unix(),
 		}
 
@@ -73,6 +74,22 @@ func (xs *xormStore) GetUserLoginAttemptCount(ctx context.Context, query GetUser
 			And("created >= ?", query.Since.Unix()).
 			Count(loginAttempt)
 
+		return queryErr
+	})
+
+	return total, err
+}
+
+func (xs *xormStore) GetIPLoginAttemptCount(ctx context.Context, query GetIPLoginAttemptCountQuery) (int64, error) {
+	var total int64
+	err := xs.db.WithDbSession(ctx, func(dbSession *db.Session) error {
+		var queryErr error
+		loginAttempt := new(loginattempt.LoginAttempt)
+		total, queryErr = dbSession.
+			Where("ip_address = ?", query.IPAddress).
+			And("created >= ?", query.Since.Unix()).
+			Count(loginAttempt)
+
 		if queryErr != nil {
 			return queryErr
 		}

pkg/services/loginattempt/loginattemptimpl/store_test.go

@@ -60,21 +60,21 @@ func TestIntegrationLoginAttemptsQuery(t *testing.T) {
 
 		_, err := s.CreateLoginAttempt(context.Background(), CreateLoginAttemptCommand{
 			Username:  user,
-			IpAddress: "192.168.0.1",
+			IPAddress: "192.168.0.1",
 		})
 		require.Nil(t, err)
 
 		mockTime = timePlusOneMinute
 		_, err = s.CreateLoginAttempt(context.Background(), CreateLoginAttemptCommand{
 			Username:  user,
-			IpAddress: "192.168.0.1",
+			IPAddress: "192.168.0.1",
 		})
 		require.Nil(t, err)
 
 		mockTime = timePlusTwoMinutes
 		_, err = s.CreateLoginAttempt(context.Background(), CreateLoginAttemptCommand{
 			Username:  user,
-			IpAddress: "192.168.0.1",
+			IPAddress: "192.168.0.1",
 		})
 		require.Nil(t, err)
 
@@ -125,21 +125,21 @@ func TestIntegrationLoginAttemptsDelete(t *testing.T) {
 
 		_, err := s.CreateLoginAttempt(context.Background(), CreateLoginAttemptCommand{
 			Username:  user,
-			IpAddress: "192.168.0.1",
+			IPAddress: "192.168.0.1",
 		})
 		require.Nil(t, err)
 
 		mockTime = timePlusOneMinute
 		_, err = s.CreateLoginAttempt(context.Background(), CreateLoginAttemptCommand{
 			Username:  user,
-			IpAddress: "192.168.0.1",
+			IPAddress: "192.168.0.1",
 		})
 		require.Nil(t, err)
 
 		mockTime = timePlusTwoMinutes
 		_, err = s.CreateLoginAttempt(context.Background(), CreateLoginAttemptCommand{
 			Username:  user,
-			IpAddress: "192.168.0.1",
+			IPAddress: "192.168.0.1",
 		})
 		require.Nil(t, err)
 

pkg/services/loginattempt/loginattempttest/fake.go

@@ -24,3 +24,7 @@ func (f FakeLoginAttemptService) Reset(ctx context.Context, username string) err
 func (f FakeLoginAttemptService) Validate(ctx context.Context, username string) (bool, error) {
 	return f.ExpectedValid, f.ExpectedErr
 }
+
+func (f FakeLoginAttemptService) ValidateIPAddress(ctx context.Context, IpAddress string) (bool, error) {
+	return f.ExpectedValid, f.ExpectedErr
+}

pkg/services/loginattempt/loginattempttest/mock.go

@@ -17,7 +17,7 @@ type MockLoginAttemptService struct {
 	ExpectedErr   error
 }
 
-func (f *MockLoginAttemptService) Add(ctx context.Context, username, IPAddress string) error {
+func (f *MockLoginAttemptService) Add(ctx context.Context, username, ipAddress string) error {
 	f.AddCalled = true
 	return f.ExpectedErr
 }
@@ -31,3 +31,8 @@ func (f *MockLoginAttemptService) Validate(ctx context.Context, username string)
 	f.ValidateCalled = true
 	return f.ExpectedValid, f.ExpectedErr
 }
+
+func (f *MockLoginAttemptService) ValidateIPAddress(ctx context.Context, ipAddress string) (bool, error) {
+	f.ValidateCalled = true
+	return f.ExpectedValid, f.ExpectedErr
+}

pkg/setting/setting.go

@@ -156,6 +156,7 @@ type Cfg struct {
 	DisableInitAdminCreation             bool
 	DisableBruteForceLoginProtection     bool
 	BruteForceLoginProtectionMaxAttempts int64
+	DisableIPAddressLoginProtection      bool
 	CookieSecure                         bool
 	CookieSameSiteDisabled               bool
 	CookieSameSiteMode                   http.SameSite
@@ -1527,6 +1528,7 @@ func readSecuritySettings(iniFile *ini.File, cfg *Cfg) error {
 
 	cfg.DisableBruteForceLoginProtection = security.Key("disable_brute_force_login_protection").MustBool(false)
 	cfg.BruteForceLoginProtectionMaxAttempts = security.Key("brute_force_login_protection_max_attempts").MustInt64(5)
+	cfg.DisableIPAddressLoginProtection = security.Key("disable_ip_address_login_protection").MustBool(true)
 
 	// Ensure at least one login attempt can be performed.
 	if cfg.BruteForceLoginProtectionMaxAttempts <= 0 {