From c59144a287094d167d426e9cbed7c392c455154f Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@tailscale.com>
Date: Mon, 18 Nov 2024 15:58:42 +0100
Subject: [PATCH] only set username and email if valid

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
---
 hscontrol/policy/acls.go      |  2 +-
 hscontrol/policy/acls_test.go |  3 ++-
 hscontrol/types/users.go      | 24 ++++++++++++++++++------
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/hscontrol/policy/acls.go b/hscontrol/policy/acls.go
index 2c314e0e..3a0c3d9e 100644
--- a/hscontrol/policy/acls.go
+++ b/hscontrol/policy/acls.go
@@ -990,7 +990,7 @@ func filterNodesByUser(nodes types.Nodes, users []types.User, userToken string)
 
 	var potentialUsers []types.User
 	for _, user := range users {
-		if user.ProviderIdentifier == userToken {
+		if user.ProviderIdentifier.Valid && user.ProviderIdentifier.String == userToken {
 			// If a user is matching with a known unique field,
 			// disgard all other users and only keep the current
 			// user.
diff --git a/hscontrol/policy/acls_test.go b/hscontrol/policy/acls_test.go
index e2a1adc7..b00cec12 100644
--- a/hscontrol/policy/acls_test.go
+++ b/hscontrol/policy/acls_test.go
@@ -1,6 +1,7 @@
 package policy
 
 import (
+	"database/sql"
 	"errors"
 	"math/rand/v2"
 	"net/netip"
@@ -892,7 +893,7 @@ func Test_filterNodesByUser(t *testing.T) {
 			Model:              gorm.Model{ID: 3},
 			Name:               "mikael",
 			Email:              "mikael@headscale.net",
-			ProviderIdentifier: "http://oidc.org/1234",
+			ProviderIdentifier: sql.NullString{String: "http://oidc.org/1234", Valid: true},
 		},
 		{Model: gorm.Model{ID: 4}, Name: "mikael2", Email: "mikael@headscale.net"},
 		{Model: gorm.Model{ID: 5}, Name: "mikael", Email: "mikael2@headscale.net"},
diff --git a/hscontrol/types/users.go b/hscontrol/types/users.go
index f983d7f5..72cc9e1e 100644
--- a/hscontrol/types/users.go
+++ b/hscontrol/types/users.go
@@ -2,6 +2,8 @@ package types
 
 import (
 	"cmp"
+	"database/sql"
+	"net/mail"
 	"strconv"
 
 	v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
@@ -34,7 +36,7 @@ type User struct {
 	// Unique identifier of the user from OIDC,
 	// comes from `sub` claim in the OIDC token
 	// and is used to lookup the user.
-	ProviderIdentifier string `gorm:"index"`
+	ProviderIdentifier sql.NullString `gorm:"index"`
 
 	// Provider is the origin of the user account,
 	// same as RegistrationMethod, without authkey.
@@ -51,7 +53,7 @@ type User struct {
 // should be used throughout headscale, in information returned to the
 // user and the Policy engine.
 func (u *User) Username() string {
-	return cmp.Or(u.Email, u.Name, u.ProviderIdentifier, strconv.FormatUint(uint64(u.ID), 10))
+	return cmp.Or(u.Email, u.Name, u.ProviderIdentifier.String, strconv.FormatUint(uint64(u.ID), 10))
 }
 
 // DisplayNameOrUsername returns the DisplayName if it exists, otherwise
@@ -107,7 +109,7 @@ func (u *User) Proto() *v1.User {
 		CreatedAt:     timestamppb.New(u.CreatedAt),
 		DisplayName:   u.DisplayName,
 		Email:         u.Email,
-		ProviderId:    u.ProviderIdentifier,
+		ProviderId:    u.ProviderIdentifier.String,
 		Provider:      u.Provider,
 		ProfilePicUrl: u.ProfilePicURL,
 	}
@@ -129,10 +131,20 @@ type OIDCClaims struct {
 // FromClaim overrides a User from OIDC claims.
 // All fields will be updated, except for the ID.
 func (u *User) FromClaim(claims *OIDCClaims) {
-	u.ProviderIdentifier = claims.Sub
+	err := util.CheckForFQDNRules(claims.Username)
+	if err == nil {
+		u.Name = claims.Username
+	}
+
+	if claims.EmailVerified {
+		_, err = mail.ParseAddress(claims.Email)
+		if err == nil {
+			u.Email = claims.Email
+		}
+	}
+
+	u.ProviderIdentifier.String = claims.Sub
 	u.DisplayName = claims.Name
-	u.Email = claims.Email
-	u.Name = claims.Username
 	u.ProfilePicURL = claims.ProfilePictureURL
 	u.Provider = util.RegisterMethodOIDC
 }