Remove routes without a node_id (#2386)

The routes table has a NOT NULL constraint on node_id.

Fixes: #2376

CHANGELOG.md

@@ -1,8 +1,29 @@
 # CHANGELOG
 
-## Next
 
-## 0.24.0 (2024-01-17)
+## 0.24.2 (2025-01-30)
+
+### Changes
+
+- Fix issue where email and username being equal fails to match in Policy
+  [#2388](https://github.com/juanfont/headscale/pull/2388)
+- Delete invalid routes before adding a NOT NULL constraint on node_id
+  [#2386](https://github.com/juanfont/headscale/pull/2386)
+
+## 0.24.1 (2025-01-23)
+
+### Changes
+
+- Fix migration issue with user table for PostgreSQL
+  [#2367](https://github.com/juanfont/headscale/pull/2367)
+- Relax username validation to allow emails
+  [#2364](https://github.com/juanfont/headscale/pull/2364)
+- Remove invalid routes and add stronger constraints for routes to avoid API panic
+  [#2371](https://github.com/juanfont/headscale/pull/2371)
+- Fix panic when `derp.update_frequency` is 0
+  [#2368](https://github.com/juanfont/headscale/pull/2368)
+
+## 0.24.0 (2025-01-17)
 
 ### Security fix: OIDC changes in Headscale 0.24.0
 

cmd/headscale/cli/routes.go

@@ -251,10 +251,15 @@ func routesToPtables(routes []*v1.Route) pterm.TableData {
 			isPrimaryStr = strconv.FormatBool(route.GetIsPrimary())
 		}
 
+		var nodeName string
+		if route.GetNode() != nil {
+			nodeName = route.GetNode().GetGivenName()
+		}
+
 		tableData = append(tableData,
 			[]string{
 				strconv.FormatUint(route.GetId(), Base10),
-				route.GetNode().GetGivenName(),
+				nodeName,
 				route.GetPrefix(),
 				strconv.FormatBool(route.GetAdvertised()),
 				strconv.FormatBool(route.GetEnabled()),

hscontrol/app.go

@@ -245,11 +245,11 @@ func (h *Headscale) scheduledTasks(ctx context.Context) {
 
 	lastExpiryCheck := time.Unix(0, 0)
 
-	derpTicker := time.NewTicker(h.cfg.DERP.UpdateFrequency)
+	derpTickerChan := make(<-chan time.Time)
-	defer derpTicker.Stop()
+	if h.cfg.DERP.AutoUpdate && h.cfg.DERP.UpdateFrequency != 0 {
-	// If we dont want auto update, just stop the ticker
+		derpTicker := time.NewTicker(h.cfg.DERP.UpdateFrequency)
-	if !h.cfg.DERP.AutoUpdate {
+		defer derpTicker.Stop()
-		derpTicker.Stop()
+		derpTickerChan = derpTicker.C
 	}
 
 	var extraRecordsUpdate <-chan []tailcfg.DNSRecord
@@ -285,7 +285,7 @@ func (h *Headscale) scheduledTasks(ctx context.Context) {
 				h.nodeNotifier.NotifyAll(ctx, update)
 			}
 
-		case <-derpTicker.C:
+		case <-derpTickerChan:
 			log.Info().Msg("Fetching DERPMap updates")
 			h.DERPMap = derp.GetDERPMap(h.cfg.DERP)
 			if h.cfg.DERP.ServerEnabled && h.cfg.DERP.AutomaticallyAddEmbeddedDerpRegion {

hscontrol/db/db.go

@@ -478,6 +478,38 @@ func NewHeadscaleDatabase(
 				// populate the user with more interesting information.
 				ID: "202407191627",
 				Migrate: func(tx *gorm.DB) error {
+					// Fix an issue where the automigration in GORM expected a constraint to
+					// exists that didnt, and add the one it wanted.
+					// Fixes https://github.com/juanfont/headscale/issues/2351
+					if cfg.Type == types.DatabasePostgres {
+						err := tx.Exec(`
+BEGIN;
+DO $$
+BEGIN
+    IF NOT EXISTS (
+        SELECT 1 FROM pg_constraint
+        WHERE conname = 'uni_users_name'
+    ) THEN
+        ALTER TABLE users ADD CONSTRAINT uni_users_name UNIQUE (name);
+    END IF;
+END $$;
+
+DO $$
+BEGIN
+    IF EXISTS (
+        SELECT 1 FROM pg_constraint
+        WHERE conname = 'users_name_key'
+    ) THEN
+        ALTER TABLE users DROP CONSTRAINT users_name_key;
+    END IF;
+END $$;
+COMMIT;
+`).Error
+						if err != nil {
+							return fmt.Errorf("failed to rename constraint: %w", err)
+						}
+					}
+
 					err := tx.AutoMigrate(&types.User{})
 					if err != nil {
 						return err
@@ -521,6 +553,35 @@ func NewHeadscaleDatabase(
 				},
 				Rollback: func(db *gorm.DB) error { return nil },
 			},
+			{
+				// Add a constraint to routes ensuring they cannot exist without a node.
+				ID: "202501221827",
+				Migrate: func(tx *gorm.DB) error {
+					// Remove any invalid routes associated with a node that does not exist.
+					if tx.Migrator().HasTable(&types.Route{}) && tx.Migrator().HasTable(&types.Node{}) {
+						err := tx.Exec("delete from routes where node_id not in (select id from nodes)").Error
+						if err != nil {
+							return err
+						}
+					}
+
+					// Remove any invalid routes without a node_id.
+					if tx.Migrator().HasTable(&types.Route{}) {
+						err := tx.Exec("delete from routes where node_id is null").Error
+						if err != nil {
+							return err
+						}
+					}
+
+					err := tx.AutoMigrate(&types.Route{})
+					if err != nil {
+						return err
+					}
+
+					return nil
+				},
+				Rollback: func(db *gorm.DB) error { return nil },
+			},
 		},
 	)
 

hscontrol/db/db_test.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"net/netip"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"slices"
 	"sort"
@@ -23,7 +24,10 @@ import (
 	"zgo.at/zcache/v2"
 )
 
-func TestMigrations(t *testing.T) {
+// TestMigrationsSQLite is the main function for testing migrations,
+// we focus on SQLite correctness as it is the main database used in headscale.
+// All migrations that are worth testing should be added here.
+func TestMigrationsSQLite(t *testing.T) {
 	ipp := func(p string) netip.Prefix {
 		return netip.MustParsePrefix(p)
 	}
@@ -375,3 +379,58 @@ func TestConstraints(t *testing.T) {
 		})
 	}
 }
+
+func TestMigrationsPostgres(t *testing.T) {
+	tests := []struct {
+		name     string
+		dbPath   string
+		wantFunc func(*testing.T, *HSDatabase)
+	}{
+		{
+			name:   "user-idx-breaking",
+			dbPath: "testdata/pre-24-postgresdb.pssql.dump",
+			wantFunc: func(t *testing.T, h *HSDatabase) {
+				users, err := Read(h.DB, func(rx *gorm.DB) ([]types.User, error) {
+					return ListUsers(rx)
+				})
+				require.NoError(t, err)
+
+				for _, user := range users {
+					assert.NotEmpty(t, user.Name)
+					assert.Empty(t, user.ProfilePicURL)
+					assert.Empty(t, user.Email)
+				}
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			u := newPostgresDBForTest(t)
+
+			pgRestorePath, err := exec.LookPath("pg_restore")
+			if err != nil {
+				t.Fatal("pg_restore not found in PATH. Please install it and ensure it is accessible.")
+			}
+
+			// Construct the pg_restore command
+			cmd := exec.Command(pgRestorePath, "--verbose", "--if-exists", "--clean", "--no-owner", "--dbname", u.String(), tt.dbPath)
+
+			// Set the output streams
+			cmd.Stdout = os.Stdout
+			cmd.Stderr = os.Stderr
+
+			// Execute the command
+			err = cmd.Run()
+			if err != nil {
+				t.Fatalf("failed to restore postgres database: %s", err)
+			}
+
+			db = newHeadscaleDBFromPostgresURL(t, u)
+
+			if tt.wantFunc != nil {
+				tt.wantFunc(t, db)
+			}
+		})
+	}
+}

hscontrol/db/suite_test.go

@@ -78,13 +78,11 @@ func newSQLiteTestDB() (*HSDatabase, error) {
 func newPostgresTestDB(t *testing.T) *HSDatabase {
 	t.Helper()
 
-	var err error
+	return newHeadscaleDBFromPostgresURL(t, newPostgresDBForTest(t))
-	tmpDir, err = os.MkdirTemp("", "headscale-db-test-*")
+}
-	if err != nil {
-		t.Fatal(err)
-	}
 
-	log.Printf("database path: %s", tmpDir+"/headscale_test.db")
+func newPostgresDBForTest(t *testing.T) *url.URL {
+	t.Helper()
 
 	ctx := context.Background()
 	srv, err := postgrestest.Start(ctx)
@@ -100,10 +98,16 @@ func newPostgresTestDB(t *testing.T) *HSDatabase {
 	t.Logf("created local postgres: %s", u)
 	pu, _ := url.Parse(u)
 
+	return pu
+}
+
+func newHeadscaleDBFromPostgresURL(t *testing.T, pu *url.URL) *HSDatabase {
+	t.Helper()
+
 	pass, _ := pu.User.Password()
 	port, _ := strconv.Atoi(pu.Port())
 
-	db, err = NewHeadscaleDatabase(
+	db, err := NewHeadscaleDatabase(
 		types.DatabaseConfig{
 			Type: types.DatabasePostgres,
 			Postgres: types.PostgresConfig{

hscontrol/policy/acls.go

@@ -381,7 +381,7 @@ func (pol *ACLPolicy) CompileSSHPolicy(
 				}
 
 				for _, userStr := range usersFromGroup {
-					user, err := findUserFromTokenOrErr(users, userStr)
+					user, err := findUserFromToken(users, userStr)
 					if err != nil {
 						log.Trace().Err(err).Msg("user not found")
 						continue
@@ -400,7 +400,7 @@ func (pol *ACLPolicy) CompileSSHPolicy(
 			// corresponds with the User info in the netmap.
 			// TODO(kradalby): This is a bit of a hack, and it should go
 			// away with the new policy where users can be reliably determined.
-			if user, err := findUserFromTokenOrErr(users, srcToken); err == nil {
+			if user, err := findUserFromToken(users, srcToken); err == nil {
 				principals = append(principals, &tailcfg.SSHPrincipal{
 					UserLogin: user.Username(),
 				})
@@ -1001,7 +1001,7 @@ func (pol *ACLPolicy) TagsOfNode(
 			}
 			var found bool
 			for _, owner := range owners {
-				user, err := findUserFromTokenOrErr(users, owner)
+				user, err := findUserFromToken(users, owner)
 				if err != nil {
 					log.Trace().Caller().Err(err).Msg("could not determine user to filter tags by")
 				}
@@ -1038,7 +1038,7 @@ func (pol *ACLPolicy) TagsOfNode(
 func filterNodesByUser(nodes types.Nodes, users []types.User, userToken string) types.Nodes {
 	var out types.Nodes
 
-	user, err := findUserFromTokenOrErr(users, userToken)
+	user, err := findUserFromToken(users, userToken)
 	if err != nil {
 		log.Trace().Caller().Err(err).Msg("could not determine user to filter nodes by")
 		return out
@@ -1058,24 +1058,19 @@ var (
 	ErrorMultipleUserMatching = errors.New("multiple users matching")
 )
 
-func findUserFromTokenOrErr(
+// findUserFromToken finds and returns a user based on the given token, prioritizing matches by ProviderIdentifier, followed by email or name.
-	users []types.User,
+// If no matching user is found, it returns an error of type ErrorNoUserMatching.
-	token string,
+// If multiple users match the token, it returns an error indicating multiple matches.
-) (types.User, error) {
+func findUserFromToken(users []types.User, token string) (types.User, error) {
 	var potentialUsers []types.User
+
 	for _, user := range users {
 		if user.ProviderIdentifier.Valid && user.ProviderIdentifier.String == token {
-			// If a user is matching with a known unique field,
+			// Prioritize ProviderIdentifier match and exit early
-			// disgard all other users and only keep the current
+			return user, nil
-			// user.
+		}
-			potentialUsers = []types.User{user}
 
-			break
+		if user.Email == token || user.Name == token {
-		}
-		if user.Email == token {
-			potentialUsers = append(potentialUsers, user)
-		}
-		if user.Name == token {
 			potentialUsers = append(potentialUsers, user)
 		}
 	}

hscontrol/policy/acls_test.go

@@ -4046,3 +4046,315 @@ func TestValidTagInvalidUser(t *testing.T) {
 		t.Errorf("TestValidTagInvalidUser() unexpected result (-want +got):\n%s", diff)
 	}
 }
+
+func TestFindUserByToken(t *testing.T) {
+	tests := []struct {
+		name    string
+		users   []types.User
+		token   string
+		want    types.User
+		wantErr bool
+	}{
+		{
+			name: "exact match by ProviderIdentifier",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "token1"}},
+				{Email: "user2@example.com"},
+			},
+			token:   "token1",
+			want:    types.User{ProviderIdentifier: sql.NullString{Valid: true, String: "token1"}},
+			wantErr: false,
+		},
+		{
+			name: "no matches found",
+			users: []types.User{
+				{Email: "user1@example.com"},
+				{Name: "username"},
+			},
+			token:   "nonexistent-token",
+			want:    types.User{},
+			wantErr: true,
+		},
+		{
+			name: "multiple matches by email and name",
+			users: []types.User{
+				{Email: "token2", Name: "notoken"},
+				{Name: "token2", Email: "notoken@example.com"},
+			},
+			token:   "token2",
+			want:    types.User{},
+			wantErr: true,
+		},
+		{
+			name: "match by email",
+			users: []types.User{
+				{Email: "token3@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "othertoken"}},
+			},
+			token:   "token3@example.com",
+			want:    types.User{Email: "token3@example.com"},
+			wantErr: false,
+		},
+		{
+			name: "match by name",
+			users: []types.User{
+				{Name: "token4"},
+				{Email: "user5@example.com"},
+			},
+			token:   "token4",
+			want:    types.User{Name: "token4"},
+			wantErr: false,
+		},
+		{
+			name: "provider identifier takes precedence over email and name matches",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "token5"}},
+				{Email: "token5@example.com", Name: "token5"},
+			},
+			token:   "token5",
+			want:    types.User{ProviderIdentifier: sql.NullString{Valid: true, String: "token5"}},
+			wantErr: false,
+		},
+		{
+			name: "empty token finds no users",
+			users: []types.User{
+				{Email: "user6@example.com"},
+				{Name: "username6"},
+			},
+			token:   "",
+			want:    types.User{},
+			wantErr: true,
+		},
+		// Test case 1: Duplicate Emails with Unique ProviderIdentifiers
+		{
+			name: "duplicate emails with unique provider identifiers",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid1"}, Email: "user@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid2"}, Email: "user@example.com"},
+			},
+			token:   "user@example.com",
+			want:    types.User{},
+			wantErr: true,
+		},
+
+		// Test case 2: Duplicate Names with Unique ProviderIdentifiers
+		{
+			name: "duplicate names with unique provider identifiers",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid3"}, Name: "John Doe"},
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid4"}, Name: "John Doe"},
+			},
+			token:   "John Doe",
+			want:    types.User{},
+			wantErr: true,
+		},
+
+		// Test case 3: Duplicate Emails and Names with Unique ProviderIdentifiers
+		{
+			name: "duplicate emails and names with unique provider identifiers",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid5"}, Email: "user@example.com", Name: "John Doe"},
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid6"}, Email: "user@example.com", Name: "John Doe"},
+			},
+			token:   "user@example.com",
+			want:    types.User{},
+			wantErr: true,
+		},
+
+		// Test case 4: Unique Names without ProviderIdentifiers
+		{
+			name: "unique names without provider identifiers",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "johndoe@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "Jane Smith", Email: "janesmith@example.com"},
+			},
+			token:   "John Doe",
+			want:    types.User{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "johndoe@example.com"},
+			wantErr: false,
+		},
+
+		// Test case 5: Duplicate Emails without ProviderIdentifiers but Unique Names
+		{
+			name: "duplicate emails without provider identifiers but unique names",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "user@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "Jane Smith", Email: "user@example.com"},
+			},
+			token:   "John Doe",
+			want:    types.User{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "user@example.com"},
+			wantErr: false,
+		},
+
+		// Test case 6: Duplicate Names and Emails without ProviderIdentifiers
+		{
+			name: "duplicate names and emails without provider identifiers",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "user@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "user@example.com"},
+			},
+			token:   "John Doe",
+			want:    types.User{},
+			wantErr: true,
+		},
+
+		// Test case 7: Multiple Users with the Same Email but Different Names and Unique ProviderIdentifiers
+		{
+			name: "multiple users with same email, different names, unique provider identifiers",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid7"}, Email: "user@example.com", Name: "John Doe"},
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid8"}, Email: "user@example.com", Name: "Jane Smith"},
+			},
+			token:   "user@example.com",
+			want:    types.User{},
+			wantErr: true,
+		},
+
+		// Test case 8: Multiple Users with the Same Name but Different Emails and Unique ProviderIdentifiers
+		{
+			name: "multiple users with same name, different emails, unique provider identifiers",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid9"}, Email: "johndoe@example.com", Name: "John Doe"},
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid10"}, Email: "janedoe@example.com", Name: "John Doe"},
+			},
+			token:   "John Doe",
+			want:    types.User{},
+			wantErr: true,
+		},
+
+		// Test case 9: Multiple Users with Same Email and Name but Unique ProviderIdentifiers
+		{
+			name: "multiple users with same email and name, unique provider identifiers",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid11"}, Email: "user@example.com", Name: "John Doe"},
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid12"}, Email: "user@example.com", Name: "John Doe"},
+			},
+			token:   "user@example.com",
+			want:    types.User{},
+			wantErr: true,
+		},
+
+		// Test case 10: Multiple Users without ProviderIdentifiers but with Unique Names and Emails
+		{
+			name: "multiple users without provider identifiers, unique names and emails",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "johndoe@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "Jane Smith", Email: "janesmith@example.com"},
+			},
+			token:   "John Doe",
+			want:    types.User{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "johndoe@example.com"},
+			wantErr: false,
+		},
+
+		// Test case 11: Multiple Users without ProviderIdentifiers and Duplicate Emails but Unique Names
+		{
+			name: "multiple users without provider identifiers, duplicate emails but unique names",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "user@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "Jane Smith", Email: "user@example.com"},
+			},
+			token:   "John Doe",
+			want:    types.User{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "user@example.com"},
+			wantErr: false,
+		},
+
+		// Test case 12: Multiple Users without ProviderIdentifiers and Duplicate Names but Unique Emails
+		{
+			name: "multiple users without provider identifiers, duplicate names but unique emails",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "johndoe@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "janedoe@example.com"},
+			},
+			token:   "John Doe",
+			want:    types.User{},
+			wantErr: true,
+		},
+
+		// Test case 13: Multiple Users without ProviderIdentifiers and Duplicate Both Names and Emails
+		{
+			name: "multiple users without provider identifiers, duplicate names and emails",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "user@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "user@example.com"},
+			},
+			token:   "John Doe",
+			want:    types.User{},
+			wantErr: true,
+		},
+
+		// Test case 14: Multiple Users with Same Email Without ProviderIdentifiers
+		{
+			name: "multiple users with same email without provider identifiers",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "user@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "Jane Smith", Email: "user@example.com"},
+			},
+			token:   "user@example.com",
+			want:    types.User{},
+			wantErr: true,
+		},
+
+		// Test case 15: Multiple Users with Same Name Without ProviderIdentifiers
+		{
+			name: "multiple users with same name without provider identifiers",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "johndoe@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "John Doe", Email: "janedoe@example.com"},
+			},
+			token:   "John Doe",
+			want:    types.User{},
+			wantErr: true,
+		},
+		{
+			name: "Name field used as email address match",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid3"}, Name: "user@example.com", Email: "another@example.com"},
+			},
+			token:   "user@example.com",
+			want:    types.User{ProviderIdentifier: sql.NullString{Valid: true, String: "pid3"}, Name: "user@example.com", Email: "another@example.com"},
+			wantErr: false,
+		},
+		{
+			name: "multiple users with same name as email and unique provider identifiers",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid4"}, Name: "user@example.com", Email: "user1@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: true, String: "pid5"}, Name: "user@example.com", Email: "user2@example.com"},
+			},
+			token:   "user@example.com",
+			want:    types.User{},
+			wantErr: true,
+		},
+		{
+			name: "no provider identifier and duplicate names as emails",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "user@example.com", Email: "another1@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "user@example.com", Email: "another2@example.com"},
+			},
+			token:   "user@example.com",
+			want:    types.User{},
+			wantErr: true,
+		},
+		{
+			name: "name as email with multiple matches when provider identifier is not set",
+			users: []types.User{
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "user@example.com", Email: "another1@example.com"},
+				{ProviderIdentifier: sql.NullString{Valid: false, String: ""}, Name: "user@example.com", Email: "another2@example.com"},
+			},
+			token:   "user@example.com",
+			want:    types.User{},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotUser, err := findUserFromToken(tt.users, tt.token)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("findUserFromToken() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if diff := cmp.Diff(tt.want, gotUser, util.Comparers...); diff != "" {
+				t.Errorf("findUserFromToken() unexpected result (-want +got):\n%s", diff)
+			}
+		})
+	}
+}

hscontrol/types/routes.go

@@ -13,7 +13,7 @@ import (
 type Route struct {
 	gorm.Model
 
-	NodeID uint64
+	NodeID uint64 `gorm:"not null"`
 	Node   *Node
 
 	// TODO(kradalby): change this custom type to netip.Prefix
@@ -79,7 +79,6 @@ func (rs Routes) Proto() []*v1.Route {
 	for _, route := range rs {
 		protoRoute := v1.Route{
 			Id:         uint64(route.ID),
-			Node:       route.Node.Proto(),
 			Prefix:     route.Prefix.String(),
 			Advertised: route.Advertised,
 			Enabled:    route.Enabled,
@@ -88,6 +87,10 @@ func (rs Routes) Proto() []*v1.Route {
 			UpdatedAt:  timestamppb.New(route.UpdatedAt),
 		}
 
+		if route.Node != nil {
+			protoRoute.Node = route.Node.Proto()
+		}
+
 		if route.DeletedAt.Valid {
 			protoRoute.DeletedAt = timestamppb.New(route.DeletedAt.Time)
 		}

hscontrol/types/users.go

@@ -29,8 +29,9 @@ type User struct {
 	// you can have multiple users with the same name in OIDC,
 	// but not if you only run with CLI users.
 
-	// Username for the user, is used if email is empty
+	// Name (username) for the user, is used if email is empty
 	// Should not be used, please use Username().
+	// It is unique if ProviderIdentifier is not set.
 	Name string
 
 	// Typically the full name of the user
@@ -40,9 +41,11 @@ type User struct {
 	// Should not be used, please use Username().
 	Email string
 
-	// Unique identifier of the user from OIDC,
+	// ProviderIdentifier is a unique or not set identifier of the
-	// comes from `sub` claim in the OIDC token
+	// user from OIDC. It is the combination of `iss`
-	// and is used to lookup the user.
+	// and `sub` claim in the OIDC token.
+	// It is unique if set.
+	// It is unique together with Name.
 	ProviderIdentifier sql.NullString
 
 	// Provider is the origin of the user account,

hscontrol/types/users_test.go

@@ -1,10 +1,12 @@
 package types
 
 import (
+	"database/sql"
 	"encoding/json"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/juanfont/headscale/hscontrol/util"
 )
 
 func TestUnmarshallOIDCClaims(t *testing.T) {
@@ -73,3 +75,149 @@ func TestUnmarshallOIDCClaims(t *testing.T) {
 		})
 	}
 }
+
+func TestOIDCClaimsJSONToUser(t *testing.T) {
+	tests := []struct {
+		name    string
+		jsonstr string
+		want    User
+	}{
+		{
+			name: "normal-bool",
+			jsonstr: `
+{
+  "sub": "test",
+  "email": "test@test.no",
+  "email_verified": true
+}
+			`,
+			want: User{
+				Provider: util.RegisterMethodOIDC,
+				Email:    "test@test.no",
+				ProviderIdentifier: sql.NullString{
+					String: "/test",
+					Valid:  true,
+				},
+			},
+		},
+		{
+			name: "string-bool-true",
+			jsonstr: `
+{
+  "sub": "test2",
+  "email": "test2@test.no",
+  "email_verified": "true"
+}
+			`,
+			want: User{
+				Provider: util.RegisterMethodOIDC,
+				Email:    "test2@test.no",
+				ProviderIdentifier: sql.NullString{
+					String: "/test2",
+					Valid:  true,
+				},
+			},
+		},
+		{
+			name: "string-bool-false",
+			jsonstr: `
+{
+  "sub": "test3",
+  "email": "test3@test.no",
+  "email_verified": "false"
+}
+			`,
+			want: User{
+				Provider: util.RegisterMethodOIDC,
+				ProviderIdentifier: sql.NullString{
+					String: "/test3",
+					Valid:  true,
+				},
+			},
+		},
+		{
+			// From https://github.com/juanfont/headscale/issues/2333
+			name: "okta-oidc-claim-20250121",
+			jsonstr: `
+{
+  "sub": "00u7dr4qp7XXXXXXXXXX",
+  "name": "Tim Horton",
+  "email": "tim.horton@company.com",
+  "ver": 1,
+  "iss": "https://sso.company.com/oauth2/default",
+  "aud": "0oa8neto4tXXXXXXXXXX",
+  "iat": 1737455152,
+  "exp": 1737458752,
+  "jti": "ID.zzJz93koTunMKv5Bq-XXXXXXXXXXXXXXXXXXXXXXXXX",
+  "amr": [
+    "pwd"
+  ],
+  "idp": "00o42r3s2cXXXXXXXX",
+  "nonce": "nonce",
+  "preferred_username": "tim.horton@company.com",
+  "auth_time": 1000,
+  "at_hash": "preview_at_hash"
+}
+			`,
+			want: User{
+				Provider:    util.RegisterMethodOIDC,
+				DisplayName: "Tim Horton",
+				Name:        "tim.horton@company.com",
+				ProviderIdentifier: sql.NullString{
+					String: "https://sso.company.com/oauth2/default/00u7dr4qp7XXXXXXXXXX",
+					Valid:  true,
+				},
+			},
+		},
+		{
+			// From https://github.com/juanfont/headscale/issues/2333
+			name: "okta-oidc-claim-20250121",
+			jsonstr: `
+{
+  "aud": "79xxxxxx-xxxx-xxxx-xxxx-892146xxxxxx",
+  "iss": "https://login.microsoftonline.com//v2.0",
+  "iat": 1737346441,
+  "nbf": 1737346441,
+  "exp": 1737350341,
+  "aio": "AWQAm/8ZAAAABKne9EWr6ygVO2DbcRmoPIpRM819qqlP/mmK41AAWv/C2tVkld4+znbG8DaXFdLQa9jRUzokvsT7rt9nAT6Fg7QC+/ecDWsF5U+QX11f9Ox7ZkK4UAIWFcIXpuZZvRS7",
+  "email": "user@domain.com",
+  "name": "XXXXXX XXXX",
+  "oid": "54c2323d-5052-4130-9588-ad751909003f",
+  "preferred_username": "user@domain.com",
+  "rh": "1.AXUAXdg0Rfc11UifLDJv67ChfSluoXmD9z1EmK-JIUYuSK9cAQl1AA.",
+  "sid": "5250a0a2-0b4e-4e68-8652-b4e97866411d",
+  "sub": "I-70OQnj3TogrNSfkZQqB3f7dGwyBWSm1dolHNKrMzQ",
+  "tid": "<redacted>",
+  "uti": "zAuXeEtMM0GwcTAcOsBZAA",
+  "ver": "2.0"
+}
+			`,
+			want: User{
+				Provider:    util.RegisterMethodOIDC,
+				DisplayName: "XXXXXX XXXX",
+				Name:        "user@domain.com",
+				ProviderIdentifier: sql.NullString{
+					String: "https://login.microsoftonline.com//v2.0/I-70OQnj3TogrNSfkZQqB3f7dGwyBWSm1dolHNKrMzQ",
+					Valid:  true,
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var got OIDCClaims
+			if err := json.Unmarshal([]byte(tt.jsonstr), &got); err != nil {
+				t.Errorf("TestOIDCClaimsJSONToUser() error = %v", err)
+				return
+			}
+
+			var user User
+
+			user.FromClaim(&got)
+			if diff := cmp.Diff(user, tt.want); diff != "" {
+				t.Errorf("TestOIDCClaimsJSONToUser() mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}

hscontrol/util/dns.go

@@ -26,6 +26,11 @@ var invalidCharsInUserRegex = regexp.MustCompile("[^a-z0-9-.]+")
 
 var ErrInvalidUserName = errors.New("invalid user name")
 
+// ValidateUsername checks if a username is valid.
+// It must be at least 2 characters long, start with a letter, and contain
+// only letters, numbers, hyphens, dots, and underscores.
+// It cannot contain more than one '@'.
+// It cannot contain invalid characters.
 func ValidateUsername(username string) error {
 	// Ensure the username meets the minimum length requirement
 	if len(username) < 2 {
@@ -40,7 +45,11 @@ func ValidateUsername(username string) error {
 	atCount := 0
 	for _, char := range username {
 		switch {
-		case unicode.IsLetter(char), unicode.IsDigit(char), char == '-':
+		case unicode.IsLetter(char),
+			unicode.IsDigit(char),
+			char == '-',
+			char == '.',
+			char == '_':
 			// Valid characters
 		case char == '@':
 			atCount++