From f8ec54d8162847b1450a738cc28edfaef3b6e710 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 18 Nov 2024 15:58:42 +0100 Subject: [PATCH] only set username and email if valid Signed-off-by: Kristoffer Dalby --- 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 2dcbb88a..5d41d000 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 1e1a5860..1b9a3fd0 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" @@ -873,7 +874,7 @@ func Test_filterNodesByUser(t *testing.T) { users := []types.User{ {Model: gorm.Model{ID: 1}, Name: "marc"}, {Model: gorm.Model{ID: 2}, Name: "joe", Email: "joe@headscale.net"}, - {Model: gorm.Model{ID: 3}, Name: "mikael", Email: "mikael@headscale.net", ProviderIdentifier: "http://oidc.org/1234"}, + {Model: gorm.Model{ID: 3}, Name: "mikael", Email: "mikael@headscale.net", 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"}, {Model: gorm.Model{ID: 6}, Name: "http://oidc.org/1234", Email: "mikael@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 }