relax user validation to allow emails, add tests from various oidc pr…

…oviders (#2364) * relax user validation to allow emails, add tests from various oidc providers Signed-off-by: Kristoffer Dalby <[email protected]> * changelog Signed-off-by: Kristoffer Dalby <[email protected]> --------- Signed-off-by: Kristoffer Dalby <[email protected]>
juanfont · Jan 22, 2025 · c1f42cd · c1f42cd
1 parent aa76980
commit c1f42cd
Show file tree

Hide file tree

Showing 3 changed files with 165 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,13 @@
 - `oidc.map_legacy_users` is now `false` by default
   [#2350](https://github.com/juanfont/headscale/pull/2350)
 
+## 0.24.1 (2025-01-xx)
+
+### Changes
+
+- Relax username validation to allow emails
+  [#2364](https://github.com/juanfont/headscale/pull/2364)
+
 ## 0.24.0 (2025-01-17)
 
 ### Security fix: OIDC changes in Headscale 0.24.0

diff --git a/hscontrol/types/users_test.go b/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": "[email protected]",
+  "email_verified": true
+}
+			`,
+			want: User{
+				Provider: util.RegisterMethodOIDC,
+				Email:    "[email protected]",
+				ProviderIdentifier: sql.NullString{
+					String: "/test",
+					Valid:  true,
+				},
+			},
+		},
+		{
+			name: "string-bool-true",
+			jsonstr: `
+{
+  "sub": "test2",
+  "email": "[email protected]",
+  "email_verified": "true"
+}
+			`,
+			want: User{
+				Provider: util.RegisterMethodOIDC,
+				Email:    "[email protected]",
+				ProviderIdentifier: sql.NullString{
+					String: "/test2",
+					Valid:  true,
+				},
+			},
+		},
+		{
+			name: "string-bool-false",
+			jsonstr: `
+{
+  "sub": "test3",
+  "email": "[email protected]",
+  "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": "[email protected]",
+  "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": "[email protected]",
+  "auth_time": 1000,
+  "at_hash": "preview_at_hash"
+}
+			`,
+			want: User{
+				Provider:    util.RegisterMethodOIDC,
+				DisplayName: "Tim Horton",
+				Name:        "[email protected]",
+				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": "[email protected]",
+  "name": "XXXXXX XXXX",
+  "oid": "54c2323d-5052-4130-9588-ad751909003f",
+  "preferred_username": "[email protected]",
+  "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:        "[email protected]",
+				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)
+			}
+		})
+	}
+}
diff --git a/hscontrol/util/dns.go b/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++