only set username and email if valid

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
This commit is contained in:
Kristoffer Dalby 2024-11-18 15:58:42 +01:00
parent 31d398c793
commit f8ec54d816
No known key found for this signature in database
3 changed files with 21 additions and 8 deletions

View file

@ -990,7 +990,7 @@ func filterNodesByUser(nodes types.Nodes, users []types.User, userToken string)
var potentialUsers []types.User var potentialUsers []types.User
for _, user := range users { 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, // If a user is matching with a known unique field,
// disgard all other users and only keep the current // disgard all other users and only keep the current
// user. // user.

View file

@ -1,6 +1,7 @@
package policy package policy
import ( import (
"database/sql"
"errors" "errors"
"math/rand/v2" "math/rand/v2"
"net/netip" "net/netip"
@ -873,7 +874,7 @@ func Test_filterNodesByUser(t *testing.T) {
users := []types.User{ users := []types.User{
{Model: gorm.Model{ID: 1}, Name: "marc"}, {Model: gorm.Model{ID: 1}, Name: "marc"},
{Model: gorm.Model{ID: 2}, Name: "joe", Email: "joe@headscale.net"}, {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: 4}, Name: "mikael2", Email: "mikael@headscale.net"},
{Model: gorm.Model{ID: 5}, Name: "mikael", Email: "mikael2@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"}, {Model: gorm.Model{ID: 6}, Name: "http://oidc.org/1234", Email: "mikael@headscale.net"},

View file

@ -2,6 +2,8 @@ package types
import ( import (
"cmp" "cmp"
"database/sql"
"net/mail"
"strconv" "strconv"
v1 "github.com/juanfont/headscale/gen/go/headscale/v1" v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
@ -34,7 +36,7 @@ type User struct {
// Unique identifier of the user from OIDC, // Unique identifier of the user from OIDC,
// comes from `sub` claim in the OIDC token // comes from `sub` claim in the OIDC token
// and is used to lookup the user. // and is used to lookup the user.
ProviderIdentifier string `gorm:"index"` ProviderIdentifier sql.NullString `gorm:"index"`
// Provider is the origin of the user account, // Provider is the origin of the user account,
// same as RegistrationMethod, without authkey. // same as RegistrationMethod, without authkey.
@ -51,7 +53,7 @@ type User struct {
// should be used throughout headscale, in information returned to the // should be used throughout headscale, in information returned to the
// user and the Policy engine. // user and the Policy engine.
func (u *User) Username() string { 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 // DisplayNameOrUsername returns the DisplayName if it exists, otherwise
@ -107,7 +109,7 @@ func (u *User) Proto() *v1.User {
CreatedAt: timestamppb.New(u.CreatedAt), CreatedAt: timestamppb.New(u.CreatedAt),
DisplayName: u.DisplayName, DisplayName: u.DisplayName,
Email: u.Email, Email: u.Email,
ProviderId: u.ProviderIdentifier, ProviderId: u.ProviderIdentifier.String,
Provider: u.Provider, Provider: u.Provider,
ProfilePicUrl: u.ProfilePicURL, ProfilePicUrl: u.ProfilePicURL,
} }
@ -129,10 +131,20 @@ type OIDCClaims struct {
// FromClaim overrides a User from OIDC claims. // FromClaim overrides a User from OIDC claims.
// All fields will be updated, except for the ID. // All fields will be updated, except for the ID.
func (u *User) FromClaim(claims *OIDCClaims) { 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.DisplayName = claims.Name
u.Email = claims.Email
u.Name = claims.Username
u.ProfilePicURL = claims.ProfilePictureURL u.ProfilePicURL = claims.ProfilePictureURL
u.Provider = util.RegisterMethodOIDC u.Provider = util.RegisterMethodOIDC
} }