Skip to content

Commit 6332979

Browse files
committed
only set username and email if valid
Signed-off-by: Kristoffer Dalby <[email protected]>
1 parent 4ba319a commit 6332979

File tree

3 files changed

+21
-8
lines changed

3 files changed

+21
-8
lines changed

hscontrol/policy/acls.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -990,7 +990,7 @@ func filterNodesByUser(nodes types.Nodes, users []types.User, userToken string)
990990

991991
var potentialUsers []types.User
992992
for _, user := range users {
993-
if user.ProviderIdentifier == userToken {
993+
if user.ProviderIdentifier.Valid && user.ProviderIdentifier.String == userToken {
994994
// If a user is matching with a known unique field,
995995
// disgard all other users and only keep the current
996996
// user.

hscontrol/policy/acls_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package policy
22

33
import (
4+
"database/sql"
45
"errors"
56
"math/rand/v2"
67
"net/netip"
@@ -873,7 +874,7 @@ func Test_filterNodesByUser(t *testing.T) {
873874
users := []types.User{
874875
{Model: gorm.Model{ID: 1}, Name: "marc"},
875876
{Model: gorm.Model{ID: 2}, Name: "joe", Email: "[email protected]"},
876-
{Model: gorm.Model{ID: 3}, Name: "mikael", Email: "[email protected]", ProviderIdentifier: "http://oidc.org/1234"},
877+
{Model: gorm.Model{ID: 3}, Name: "mikael", Email: "[email protected]", ProviderIdentifier: sql.NullString{String: "http://oidc.org/1234", Valid: true}},
877878
{Model: gorm.Model{ID: 4}, Name: "mikael2", Email: "[email protected]"},
878879
{Model: gorm.Model{ID: 5}, Name: "mikael", Email: "[email protected]"},
879880
{Model: gorm.Model{ID: 6}, Name: "http://oidc.org/1234", Email: "[email protected]"},

hscontrol/types/users.go

+18-6
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package types
22

33
import (
44
"cmp"
5+
"database/sql"
6+
"net/mail"
57
"strconv"
68

79
v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
@@ -34,7 +36,7 @@ type User struct {
3436
// Unique identifier of the user from OIDC,
3537
// comes from `sub` claim in the OIDC token
3638
// and is used to lookup the user.
37-
ProviderIdentifier string `gorm:"index"`
39+
ProviderIdentifier sql.NullString `gorm:"index"`
3840

3941
// Provider is the origin of the user account,
4042
// same as RegistrationMethod, without authkey.
@@ -51,7 +53,7 @@ type User struct {
5153
// should be used throughout headscale, in information returned to the
5254
// user and the Policy engine.
5355
func (u *User) Username() string {
54-
return cmp.Or(u.Email, u.Name, u.ProviderIdentifier, strconv.FormatUint(uint64(u.ID), 10))
56+
return cmp.Or(u.Email, u.Name, u.ProviderIdentifier.String, strconv.FormatUint(uint64(u.ID), 10))
5557
}
5658

5759
// DisplayNameOrUsername returns the DisplayName if it exists, otherwise
@@ -107,7 +109,7 @@ func (u *User) Proto() *v1.User {
107109
CreatedAt: timestamppb.New(u.CreatedAt),
108110
DisplayName: u.DisplayName,
109111
Email: u.Email,
110-
ProviderId: u.ProviderIdentifier,
112+
ProviderId: u.ProviderIdentifier.String,
111113
Provider: u.Provider,
112114
ProfilePicUrl: u.ProfilePicURL,
113115
}
@@ -129,10 +131,20 @@ type OIDCClaims struct {
129131
// FromClaim overrides a User from OIDC claims.
130132
// All fields will be updated, except for the ID.
131133
func (u *User) FromClaim(claims *OIDCClaims) {
132-
u.ProviderIdentifier = claims.Sub
134+
err := util.CheckForFQDNRules(claims.Username)
135+
if err == nil {
136+
u.Name = claims.Username
137+
}
138+
139+
if claims.EmailVerified {
140+
_, err = mail.ParseAddress(claims.Email)
141+
if err == nil {
142+
u.Email = claims.Email
143+
}
144+
}
145+
146+
u.ProviderIdentifier.String = claims.Sub
133147
u.DisplayName = claims.Name
134-
u.Email = claims.Email
135-
u.Name = claims.Username
136148
u.ProfilePicURL = claims.ProfilePictureURL
137149
u.Provider = util.RegisterMethodOIDC
138150
}

0 commit comments

Comments
 (0)