Merge pull request #3138 from dolthub/elianddb/9624-fix-wildcard-user-auth

dolthub/dolt#9624 - Fix wildcard user authentication for IP patterns

Author: elianddb

sql/mysql_db/mysql_db.go

@@ -20,6 +20,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net"
+	"regexp"
 	"sort"
 	"strings"
 	"sync"
@@ -591,6 +592,22 @@ func (db *MySQLDb) AddLockedSuperUser(ed *Editor, username string, host string,
 	}
 }
 
+// matchesHostPattern checks if a host matches a host pattern with wildcards.
+func matchesHostPattern(host, pattern string) bool {
+	// No wildcard, not a pattern
+	if !strings.Contains(pattern, "%") {
+		return false
+	}
+
+	// Escape regex metacharacters, then replace % with .*
+	regexPattern := regexp.QuoteMeta(pattern)
+	regexPattern = strings.ReplaceAll(regexPattern, "%", ".*")
+	regexPattern = "^" + regexPattern + "$"
+
+	matched, err := regexp.MatchString(regexPattern, host)
+	return err == nil && matched
+}
+
 // GetUser returns a user matching the given user and host if it exists. Due to the slight difference between users and
 // roles, roleSearch changes whether the search matches against user or role rules.
 func (db *MySQLDb) GetUser(fetcher UserFetcher, user string, host string, roleSearch bool) *User {
@@ -607,6 +624,9 @@ func (db *MySQLDb) GetUser(fetcher UserFetcher, user string, host string, roleSe
 	//TODO: Allow for CIDR notation in hostnames
 	//TODO: Which user do we choose when multiple host names match (e.g. host name with most characters matched, etc.)
 
+	// Store the original host for pattern matching against IP patterns
+	originalHost := host
+
 	if "127.0.0.1" == host || "::1" == host {
 		host = "localhost"
 	}
@@ -626,7 +646,9 @@ func (db *MySQLDb) GetUser(fetcher UserFetcher, user string, host string, roleSe
 			if host == user.Host ||
 				(host == "localhost" && user.Host == "::1") ||
 				(host == "localhost" && user.Host == "127.0.0.1") ||
-				(user.Host == "%" && (!roleSearch || host == "")) {
+				(user.Host == "%" && (!roleSearch || host == "")) ||
+				matchesHostPattern(host, user.Host) ||
+				(originalHost != host && matchesHostPattern(originalHost, user.Host)) {
 				return user
 			}
 		}

sql/mysql_db/mysql_db_test.go

@@ -86,3 +86,125 @@ func TestMySQLDbOverwriteUsersAndGrantsData(t *testing.T) {
 
 	rd.Close()
 }
+
+func TestMatchesHostPattern(t *testing.T) {
+	tests := []struct {
+		name     string
+		host     string
+		pattern  string
+		expected bool
+	}{
+		// Basic wildcard patterns
+		{"IP wildcard - exact match", "127.0.0.1", "127.0.0.%", true},
+		{"IP wildcard - different last octet", "127.0.0.255", "127.0.0.%", true},
+		{"IP wildcard - no match", "192.168.1.1", "127.0.0.%", false},
+		{"IP wildcard - partial match", "127.0.1.1", "127.0.0.%", false},
+
+		// Multiple wildcards
+		{"Multiple wildcards", "192.168.1.100", "192.168.%.%", true},
+		{"Multiple wildcards - no match", "10.0.1.100", "192.168.%.%", false},
+
+		// Single wildcard at different positions
+		{"Wildcard first octet", "10.0.0.1", "%.0.0.1", true},
+		{"Wildcard middle octet", "192.168.50.1", "192.%.50.1", true},
+		{"Wildcard last octet", "192.168.1.255", "192.168.1.%", true},
+
+		// Non-IP patterns
+		{"Hostname wildcard", "server1.example.com", "server%.example.com", true},
+		{"Hostname wildcard - no match", "db1.example.com", "server%.example.com", false},
+		{"Domain wildcard", "host.subdomain.example.com", "%.example.com", true},
+
+		// Edge cases
+		{"Empty pattern", "127.0.0.1", "", false},
+		{"Pattern without wildcard", "127.0.0.1", "127.0.0.1", false}, // Should return false as it's not a wildcard pattern
+		{"Just wildcard", "anything", "%", true},
+		{"Multiple wildcards together", "test", "%%", true},
+
+		// Special characters in patterns (should be escaped)
+		{"Pattern with dots", "test.host", "test.%", true},
+		{"Pattern with regex chars", "test[1]", "test[%]", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := matchesHostPattern(tt.host, tt.pattern)
+			require.Equal(t, tt.expected, result, "matchesHostPattern(%q, %q) = %v, want %v", tt.host, tt.pattern, result, tt.expected)
+		})
+	}
+}
+
+func TestGetUserWithWildcardAuthentication(t *testing.T) {
+	ctx := sql.NewEmptyContext()
+	db := CreateEmptyMySQLDb()
+	p := &capturingPersistence{}
+	db.SetPersister(p)
+
+	// Add test users with various host patterns
+	ed := db.Editor()
+	db.AddSuperUser(ed, "testuser", "127.0.0.1", "password")
+	db.AddSuperUser(ed, "localhost_user", "localhost", "password")
+	db.AddSuperUser(ed, "wildcard_user", "127.0.0.%", "password")
+	db.AddSuperUser(ed, "subnet_user", "192.168.1.%", "password")
+	db.AddSuperUser(ed, "hostname_user", "%.example.com", "password")
+	db.AddSuperUser(ed, "any_user", "%", "password")
+	db.Persist(ctx, ed)
+	ed.Close()
+
+	rd := db.Reader()
+	defer rd.Close()
+
+	tests := []struct {
+		name         string
+		username     string
+		host         string
+		expectedUser string
+		shouldFind   bool
+	}{
+		// Specific IP tests
+		{"Specific IP - exact match", "testuser", "127.0.0.1", "testuser", true},
+		{"Localhost user - normalized", "localhost_user", "127.0.0.1", "localhost_user", true},
+		{"Localhost user - ::1", "localhost_user", "::1", "localhost_user", true},
+		{"Non-existent user", "nonexistent", "127.0.0.1", "", false},
+
+		// IP wildcard tests
+		{"Wildcard IP - 127.0.0.% matches 127.0.0.1", "wildcard_user", "127.0.0.1", "wildcard_user", true},
+		{"Wildcard IP - 127.0.0.% matches 127.0.0.100", "wildcard_user", "127.0.0.100", "wildcard_user", true},
+		{"Wildcard IP - 127.0.0.% matches 127.0.0.255", "wildcard_user", "127.0.0.255", "wildcard_user", true},
+		{"Wildcard IP - 127.0.0.% does not match 127.0.1.1", "wildcard_user", "127.0.1.1", "", false},
+		{"Wildcard IP - 127.0.0.% does not match 192.168.1.1", "wildcard_user", "192.168.1.1", "", false},
+
+		// Subnet wildcard tests
+		{"Subnet wildcard - 192.168.1.% matches 192.168.1.1", "subnet_user", "192.168.1.1", "subnet_user", true},
+		{"Subnet wildcard - 192.168.1.% matches 192.168.1.100", "subnet_user", "192.168.1.100", "subnet_user", true},
+		{"Subnet wildcard - 192.168.1.% does not match 192.168.2.1", "subnet_user", "192.168.2.1", "", false},
+		{"Subnet wildcard - 192.168.1.% does not match 10.0.0.1", "subnet_user", "10.0.0.1", "", false},
+
+		// Hostname wildcard tests
+		{"Hostname wildcard - %.example.com matches host.example.com", "hostname_user", "host.example.com", "hostname_user", true},
+		{"Hostname wildcard - %.example.com matches www.example.com", "hostname_user", "www.example.com", "hostname_user", true},
+		{"Hostname wildcard - %.example.com does not match example.com", "hostname_user", "example.com", "", false},
+		{"Hostname wildcard - %.example.com does not match host.other.com", "hostname_user", "host.other.com", "", false},
+
+		// Global wildcard tests
+		{"Global wildcard - % matches any IP", "any_user", "10.0.0.1", "any_user", true},
+		{"Global wildcard - % matches any hostname", "any_user", "any.hostname.com", "any_user", true},
+		{"Global wildcard - % matches localhost", "any_user", "localhost", "any_user", true},
+
+		// Issue #9624 scenario
+		{"Customer scenario - matches connecting IP in range", "wildcard_user", "127.0.0.50", "wildcard_user", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			user := db.GetUser(rd, tt.username, tt.host, false)
+
+			if !tt.shouldFind {
+				require.Nil(t, user, "Expected no user to be found for %s@%s", tt.username, tt.host)
+				return
+			}
+
+			require.NotNil(t, user, "Expected user to be found for %s@%s", tt.username, tt.host)
+			require.Equal(t, tt.expectedUser, user.User, "Expected username %s, got %s", tt.expectedUser, user.User)
+		})
+	}
+}