Fix: Increase login_attempt.ip_address column length for IPv6 support (#107035)
* fix: increase login_attempt.ip_address column length for IPv6 support - Expand ip_address column from VARCHAR(30) to VARCHAR(50) to accommodate IPv6 addresses - Add database migration with support for PostgreSQL, MySQL, and SQLite - Add comprehensive integration tests for various IPv6 address formats - Resolves 500 errors when login fails over IPv6, now returns proper 401 errors Fixes #106362 * test: add missing test skip to TestIntegrationIPv6AddressSupport Skip integration test when running with -short flag to separate unit and integration tests * Update pkg/services/sqlstore/migrations/login_attempt_mig.go Co-authored-by: Victor Cinaglia <victor@grafana.com> * fix missing bracket * fix: resolve PostgreSQL timestamp overflow in IPv6 test - Use controlled time mock instead of time.Now() to avoid timestamp conversion issues - Follow existing test patterns with xormStore and mock time functions - Add proper Since parameter to GetIPLoginAttemptCount query - Fixes PostgreSQL error: 'pq: value "-62135596800" is out of range for type integer' * fix: resolve PostgreSQL UTF-8 encoding error in IPv6 test Replace string(rune(i)) with fmt.Sprintf to avoid null bytes (0x00) when i=0, which caused 'invalid byte sequence for encoding UTF8' error --------- Co-authored-by: Victor Cinaglia <victor@grafana.com>
This commit is contained in:
@@ -2,11 +2,14 @@ package loginattemptimpl
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
"github.com/grafana/grafana/pkg/infra/db"
|
||||
"github.com/grafana/grafana/pkg/infra/log"
|
||||
"github.com/grafana/grafana/pkg/services/loginattempt"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
)
|
||||
@@ -205,6 +208,71 @@ func TestIPLoginAttempts(t *testing.T) {
|
||||
assert.Nil(t, err)
|
||||
}
|
||||
|
||||
// TestIPv6AddressSupport verifies that various IPv6 address formats can be stored properly with the new column length, testing various IPv6 address formats that could be encountered.
|
||||
// This test validates that the ip_address column length is sufficient for IPv6 addresses
|
||||
func TestIntegrationIPv6AddressSupport(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("skipping integration test")
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
cfg := setting.NewCfg()
|
||||
cfg.DisableBruteForceLoginProtection = false
|
||||
cfg.BruteForceLoginProtectionMaxAttempts = 5
|
||||
|
||||
// Use controlled time like other tests to avoid timestamp conversion issues
|
||||
testTime := time.Date(2023, 10, 22, 8, 0, 0, 0, time.UTC)
|
||||
store := &xormStore{
|
||||
db: db.InitTestDB(t),
|
||||
now: func() time.Time { return testTime },
|
||||
}
|
||||
service := &Service{
|
||||
store: store,
|
||||
cfg: cfg,
|
||||
logger: log.New("test.login_attempt"),
|
||||
}
|
||||
|
||||
// Test various IPv6 address formats that should be supported
|
||||
ipv6Addresses := []string{
|
||||
"::1", // loopback (3 chars)
|
||||
"2001:db8::1", // shortened (12 chars)
|
||||
"[::1]", // bracketed loopback (5 chars)
|
||||
"[2001:db8::1]", // bracketed shortened (14 chars)
|
||||
"2001:0db8:85a3:0000:0000:8a2e:0370:7334", // full IPv6 (39 chars)
|
||||
"[2001:0db8:85a3:0000:0000:8a2e:0370:7334]", // bracketed full IPv6 (41 chars)
|
||||
"2001:db8:85a3:8d3:1319:8a2e:370:7348", // mixed case (34 chars)
|
||||
"[2001:db8:85a3:8d3:1319:8a2e:370:7348]", // bracketed mixed (36 chars)
|
||||
"aaaa:79c0:647:bd00:4c59:2f13:3da6:aaaa", // from the GitHub issue (35 chars)
|
||||
"[aaaa:79c0:647:bd00:4c59:2f13:3da6:aaaa]", // bracketed from issue (37 chars)
|
||||
}
|
||||
|
||||
for i, ipAddress := range ipv6Addresses {
|
||||
t.Run("IPv6_Address_"+ipAddress, func(t *testing.T) {
|
||||
username := fmt.Sprintf("testuser%d", i)
|
||||
|
||||
// Verify that the address length is within our new limit of 50 characters
|
||||
assert.LessOrEqual(t, len(ipAddress), 50, "IP address should fit in VARCHAR(50)")
|
||||
|
||||
// Test that we can add login attempts with this IPv6 address
|
||||
err := service.Add(ctx, username, ipAddress)
|
||||
assert.NoError(t, err, "Should be able to add login attempt with IPv6 address: %s", ipAddress)
|
||||
|
||||
// Verify that the login attempt was stored correctly
|
||||
count, err := store.GetIPLoginAttemptCount(ctx, GetIPLoginAttemptCountQuery{
|
||||
IPAddress: ipAddress,
|
||||
Since: testTime.Add(-time.Minute * 5),
|
||||
})
|
||||
assert.NoError(t, err, "Should be able to query login attempts for IPv6 address: %s", ipAddress)
|
||||
assert.Equal(t, int64(1), count, "Should have 1 login attempt for IPv6 address: %s", ipAddress)
|
||||
|
||||
// Test IP-based validation
|
||||
ok, err := service.ValidateIPAddress(ctx, ipAddress)
|
||||
assert.NoError(t, err, "Should be able to validate IPv6 address: %s", ipAddress)
|
||||
assert.True(t, ok, "IPv6 address should be valid: %s", ipAddress)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
var _ store = new(fakeStore)
|
||||
|
||||
type fakeStore struct {
|
||||
@@ -229,6 +297,6 @@ func (f fakeStore) DeleteOldLoginAttempts(ctx context.Context, command DeleteOld
|
||||
return f.ExpectedDeletedRows, f.ExpectedErr
|
||||
}
|
||||
|
||||
func (f fakeStore) DeleteLoginAttempts(ctx context.Context, cmd DeleteLoginAttemptsCommand) error {
|
||||
func (f fakeStore) DeleteLoginAttempts(ctx context.Context, command DeleteLoginAttemptsCommand) error {
|
||||
return f.ExpectedErr
|
||||
}
|
||||
|
||||
@@ -39,4 +39,9 @@ func addLoginAttemptMigrations(mg *Migrator) {
|
||||
"username": "username",
|
||||
"ip_address": "ip_address",
|
||||
})
|
||||
|
||||
// Increase ip_address column length to support IPv6 addresses
|
||||
mg.AddMigration("increase login_attempt.ip_address column length for IPv6 support", NewRawSQLMigration("").
|
||||
Postgres("ALTER TABLE login_attempt ALTER COLUMN ip_address TYPE VARCHAR(50);").
|
||||
Mysql("ALTER TABLE login_attempt MODIFY ip_address VARCHAR(50);"))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user