From 7be20912f5861ef9d6fa42a615e44e6c0b0f2c17 Mon Sep 17 00:00:00 2001 From: Justin Angel Date: Thu, 18 Dec 2025 06:42:32 -0500 Subject: [PATCH] oidc: make email verification configurable Co-authored-by: Kristoffer Dalby --- CHANGELOG.md | 3 + hscontrol/mapper/batcher_lockfree.go | 11 +- hscontrol/oidc.go | 80 +++++++++---- hscontrol/oidc_test.go | 173 +++++++++++++++++++++++++++ hscontrol/types/config.go | 19 +-- hscontrol/types/users.go | 4 +- hscontrol/types/users_test.go | 48 ++++++-- 7 files changed, 292 insertions(+), 46 deletions(-) create mode 100644 hscontrol/oidc_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 5aa502c8..0ef22ff2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,9 @@ sequentially through each stable release, selecting the latest patch version ava - Smarter change notifications send partial map updates and node removals instead of full maps [#2961](https://github.com/juanfont/headscale/pull/2961) - Send lightweight endpoint and DERP region updates instead of full maps [#2856](https://github.com/juanfont/headscale/pull/2856) +- Add `oidc.email_verified_required` config option to control email verification requirement [#2860](https://github.com/juanfont/headscale/pull/2860) + - When `true` (default), only verified emails can authenticate via OIDC with `allowed_domains` or `allowed_users` + - When `false`, unverified emails are allowed for OIDC authentication - Add NixOS module in repository for faster iteration [#2857](https://github.com/juanfont/headscale/pull/2857) - Add favicon to webpages [#2858](https://github.com/juanfont/headscale/pull/2858) - Redesign OIDC callback and registration web templates [#2832](https://github.com/juanfont/headscale/pull/2832) diff --git a/hscontrol/mapper/batcher_lockfree.go b/hscontrol/mapper/batcher_lockfree.go index 87284aa3..e00512b6 100644 --- a/hscontrol/mapper/batcher_lockfree.go +++ b/hscontrol/mapper/batcher_lockfree.go @@ -31,6 +31,7 @@ type LockFreeBatcher struct { workCh chan work workChOnce sync.Once // Ensures workCh is only closed once done chan struct{} + doneOnce sync.Once // Ensures done is only closed once // Batching state pendingChanges *xsync.Map[types.NodeID, []change.Change] @@ -151,10 +152,12 @@ func (b *LockFreeBatcher) Start() { } func (b *LockFreeBatcher) Close() { - // Signal shutdown to all goroutines - if b.done != nil { - close(b.done) - } + // Signal shutdown to all goroutines, only once + b.doneOnce.Do(func() { + if b.done != nil { + close(b.done) + } + }) // Only close workCh once using sync.Once to prevent races b.workChOnce.Do(func() { diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go index 5059d07c..7013b8ed 100644 --- a/hscontrol/oidc.go +++ b/hscontrol/oidc.go @@ -41,6 +41,7 @@ var ( errOIDCAllowedUsers = errors.New( "authenticated principal does not match any allowed user", ) + errOIDCUnverifiedEmail = errors.New("authenticated principal has an unverified email") ) // RegistrationInfo contains both machine key and verifier information for OIDC validation. @@ -264,17 +265,8 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler( // The user claims are now updated from the userinfo endpoint so we can verify the user // against allowed emails, email domains, and groups. - if err := validateOIDCAllowedDomains(a.cfg.AllowedDomains, &claims); err != nil { - httpError(writer, err) - return - } - - if err := validateOIDCAllowedGroups(a.cfg.AllowedGroups, &claims); err != nil { - httpError(writer, err) - return - } - - if err := validateOIDCAllowedUsers(a.cfg.AllowedUsers, &claims); err != nil { + err = doOIDCAuthorization(a.cfg, &claims) + if err != nil { httpError(writer, err) return } @@ -434,17 +426,13 @@ func validateOIDCAllowedGroups( allowedGroups []string, claims *types.OIDCClaims, ) error { - if len(allowedGroups) > 0 { - for _, group := range allowedGroups { - if slices.Contains(claims.Groups, group) { - return nil - } + for _, group := range allowedGroups { + if slices.Contains(claims.Groups, group) { + return nil } - - return NewHTTPError(http.StatusUnauthorized, "unauthorised group", errOIDCAllowedGroups) } - return nil + return NewHTTPError(http.StatusUnauthorized, "unauthorised group", errOIDCAllowedGroups) } // validateOIDCAllowedUsers checks that if AllowedUsers is provided, @@ -453,14 +441,62 @@ func validateOIDCAllowedUsers( allowedUsers []string, claims *types.OIDCClaims, ) error { - if len(allowedUsers) > 0 && - !slices.Contains(allowedUsers, claims.Email) { + if !slices.Contains(allowedUsers, claims.Email) { return NewHTTPError(http.StatusUnauthorized, "unauthorised user", errOIDCAllowedUsers) } return nil } +// doOIDCAuthorization applies authorization tests to claims. +// +// The following tests are always applied: +// +// - validateOIDCAllowedGroups +// +// The following tests are applied if cfg.EmailVerifiedRequired=false +// or claims.email_verified=true: +// +// - validateOIDCAllowedDomains +// - validateOIDCAllowedUsers +// +// NOTE that, contrary to the function name, validateOIDCAllowedUsers +// only checks the email address -- not the username. +func doOIDCAuthorization( + cfg *types.OIDCConfig, + claims *types.OIDCClaims, +) error { + if len(cfg.AllowedGroups) > 0 { + err := validateOIDCAllowedGroups(cfg.AllowedGroups, claims) + if err != nil { + return err + } + } + + trustEmail := !cfg.EmailVerifiedRequired || bool(claims.EmailVerified) + + hasEmailTests := len(cfg.AllowedDomains) > 0 || len(cfg.AllowedUsers) > 0 + if !trustEmail && hasEmailTests { + return NewHTTPError(http.StatusUnauthorized, "unverified email", errOIDCUnverifiedEmail) + } + + if len(cfg.AllowedDomains) > 0 { + err := validateOIDCAllowedDomains(cfg.AllowedDomains, claims) + if err != nil { + return err + } + } + + if len(cfg.AllowedUsers) > 0 { + err := validateOIDCAllowedUsers(cfg.AllowedUsers, claims) + if err != nil { + return err + } + } + + return nil +} + // getRegistrationIDFromState retrieves the registration ID from the state. func (a *AuthProviderOIDC) getRegistrationIDFromState(state string) *types.RegistrationID { regInfo, ok := a.registrationCache.Get(state) @@ -493,7 +529,7 @@ func (a *AuthProviderOIDC) createOrUpdateUserFromClaim( user = &types.User{} } - user.FromClaim(claims) + user.FromClaim(claims, a.cfg.EmailVerifiedRequired) if newUser { user, c, err = a.h.state.CreateUser(*user) diff --git a/hscontrol/oidc_test.go b/hscontrol/oidc_test.go new file mode 100644 index 00000000..c6ce382c --- /dev/null +++ b/hscontrol/oidc_test.go @@ -0,0 +1,173 @@ +package hscontrol + +import ( + "testing" + + "github.com/juanfont/headscale/hscontrol/types" +) + +func TestDoOIDCAuthorization(t *testing.T) { + testCases := []struct { + name string + cfg *types.OIDCConfig + claims *types.OIDCClaims + wantErr bool + }{ + { + name: "verified email domain", + wantErr: false, + cfg: &types.OIDCConfig{ + EmailVerifiedRequired: true, + AllowedDomains: []string{"test.com"}, + AllowedUsers: []string{}, + AllowedGroups: []string{}, + }, + claims: &types.OIDCClaims{ + Email: "user@test.com", + EmailVerified: true, + }, + }, + { + name: "verified email user", + wantErr: false, + cfg: &types.OIDCConfig{ + EmailVerifiedRequired: true, + AllowedDomains: []string{}, + AllowedUsers: []string{"user@test.com"}, + AllowedGroups: []string{}, + }, + claims: &types.OIDCClaims{ + Email: "user@test.com", + EmailVerified: true, + }, + }, + { + name: "unverified email domain", + wantErr: true, + cfg: &types.OIDCConfig{ + EmailVerifiedRequired: true, + AllowedDomains: []string{"test.com"}, + AllowedUsers: []string{}, + AllowedGroups: []string{}, + }, + claims: &types.OIDCClaims{ + Email: "user@test.com", + EmailVerified: false, + }, + }, + { + name: "group member", + wantErr: false, + cfg: &types.OIDCConfig{ + EmailVerifiedRequired: true, + AllowedDomains: []string{}, + AllowedUsers: []string{}, + AllowedGroups: []string{"test"}, + }, + claims: &types.OIDCClaims{Groups: []string{"test"}}, + }, + { + name: "non group member", + wantErr: true, + cfg: &types.OIDCConfig{ + EmailVerifiedRequired: true, + AllowedDomains: []string{}, + AllowedUsers: []string{}, + AllowedGroups: []string{"nope"}, + }, + claims: &types.OIDCClaims{Groups: []string{"testo"}}, + }, + { + name: "group member but bad domain", + wantErr: true, + cfg: &types.OIDCConfig{ + EmailVerifiedRequired: true, + AllowedDomains: []string{"user@good.com"}, + AllowedUsers: []string{}, + AllowedGroups: []string{"test group"}, + }, + claims: &types.OIDCClaims{Groups: []string{"test group"}, Email: "bad@bad.com", EmailVerified: true}, + }, + { + name: "all checks pass", + wantErr: false, + cfg: &types.OIDCConfig{ + EmailVerifiedRequired: true, + AllowedDomains: []string{"test.com"}, + AllowedUsers: []string{"user@test.com"}, + AllowedGroups: []string{"test group"}, + }, + claims: &types.OIDCClaims{Groups: []string{"test group"}, Email: "user@test.com", EmailVerified: true}, + }, + { + name: "all checks pass with unverified email", + wantErr: false, + cfg: &types.OIDCConfig{ + EmailVerifiedRequired: false, + AllowedDomains: []string{"test.com"}, + AllowedUsers: []string{"user@test.com"}, + AllowedGroups: []string{"test group"}, + }, + claims: &types.OIDCClaims{Groups: []string{"test group"}, Email: "user@test.com", EmailVerified: false}, + }, + { + name: "fail on unverified email", + wantErr: true, + cfg: &types.OIDCConfig{ + EmailVerifiedRequired: true, + AllowedDomains: []string{"test.com"}, + AllowedUsers: []string{"user@test.com"}, + AllowedGroups: []string{"test group"}, + }, + claims: &types.OIDCClaims{Groups: []string{"test group"}, Email: "user@test.com", EmailVerified: false}, + }, + { + name: "unverified email user only", + wantErr: true, + cfg: &types.OIDCConfig{ + EmailVerifiedRequired: true, + AllowedDomains: []string{}, + AllowedUsers: []string{"user@test.com"}, + AllowedGroups: []string{}, + }, + claims: &types.OIDCClaims{ + Email: "user@test.com", + EmailVerified: false, + }, + }, + { + name: "no filters configured", + wantErr: false, + cfg: &types.OIDCConfig{ + EmailVerifiedRequired: true, + AllowedDomains: []string{}, + AllowedUsers: []string{}, + AllowedGroups: []string{}, + }, + claims: &types.OIDCClaims{ + Email: "anyone@anywhere.com", + EmailVerified: false, + }, + }, + { + name: "multiple allowed groups second matches", + wantErr: false, + cfg: &types.OIDCConfig{ + EmailVerifiedRequired: true, + AllowedDomains: []string{}, + AllowedUsers: []string{}, + AllowedGroups: []string{"group1", "group2", "group3"}, + }, + claims: &types.OIDCClaims{Groups: []string{"group2"}}, + }, + } + + for _, tC := range testCases { + t.Run(tC.name, func(t *testing.T) { + err := doOIDCAuthorization(tC.cfg, tC.claims) + if ((err != nil) && !tC.wantErr) || ((err == nil) && tC.wantErr) { + t.Errorf("bad authorization: %s > want=%v | got=%v", tC.name, tC.wantErr, err) + } + }) + } +} diff --git a/hscontrol/types/config.go b/hscontrol/types/config.go index 13621c0a..4068d72e 100644 --- a/hscontrol/types/config.go +++ b/hscontrol/types/config.go @@ -185,6 +185,7 @@ type OIDCConfig struct { AllowedDomains []string AllowedUsers []string AllowedGroups []string + EmailVerifiedRequired bool Expiry time.Duration UseExpiryFromToken bool PKCE PKCEConfig @@ -384,6 +385,7 @@ func LoadConfig(path string, isFile bool) error { viper.SetDefault("oidc.use_expiry_from_token", false) viper.SetDefault("oidc.pkce.enabled", false) viper.SetDefault("oidc.pkce.method", "S256") + viper.SetDefault("oidc.email_verified_required", true) viper.SetDefault("logtail.enabled", false) viper.SetDefault("randomize_client_port", false) @@ -1022,14 +1024,15 @@ func LoadServerConfig() (*Config, error) { OnlyStartIfOIDCIsAvailable: viper.GetBool( "oidc.only_start_if_oidc_is_available", ), - Issuer: viper.GetString("oidc.issuer"), - ClientID: viper.GetString("oidc.client_id"), - ClientSecret: oidcClientSecret, - Scope: viper.GetStringSlice("oidc.scope"), - ExtraParams: viper.GetStringMapString("oidc.extra_params"), - AllowedDomains: viper.GetStringSlice("oidc.allowed_domains"), - AllowedUsers: viper.GetStringSlice("oidc.allowed_users"), - AllowedGroups: viper.GetStringSlice("oidc.allowed_groups"), + Issuer: viper.GetString("oidc.issuer"), + ClientID: viper.GetString("oidc.client_id"), + ClientSecret: oidcClientSecret, + Scope: viper.GetStringSlice("oidc.scope"), + ExtraParams: viper.GetStringMapString("oidc.extra_params"), + AllowedDomains: viper.GetStringSlice("oidc.allowed_domains"), + AllowedUsers: viper.GetStringSlice("oidc.allowed_users"), + AllowedGroups: viper.GetStringSlice("oidc.allowed_groups"), + EmailVerifiedRequired: viper.GetBool("oidc.email_verified_required"), Expiry: func() time.Duration { // if set to 0, we assume no expiry if value := viper.GetString("oidc.expiry"); value == "0" { diff --git a/hscontrol/types/users.go b/hscontrol/types/users.go index 2e78386c..36702146 100644 --- a/hscontrol/types/users.go +++ b/hscontrol/types/users.go @@ -353,7 +353,7 @@ type OIDCUserInfo struct { // FromClaim overrides a User from OIDC claims. // All fields will be updated, except for the ID. -func (u *User) FromClaim(claims *OIDCClaims) { +func (u *User) FromClaim(claims *OIDCClaims, emailVerifiedRequired bool) { err := util.ValidateUsername(claims.Username) if err == nil { u.Name = claims.Username @@ -361,7 +361,7 @@ func (u *User) FromClaim(claims *OIDCClaims) { log.Debug().Caller().Err(err).Msgf("Username %s is not valid", claims.Username) } - if claims.EmailVerified { + if claims.EmailVerified || !FlexibleBoolean(emailVerifiedRequired) { _, err = mail.ParseAddress(claims.Email) if err == nil { u.Email = claims.Email diff --git a/hscontrol/types/users_test.go b/hscontrol/types/users_test.go index f36489a3..15386553 100644 --- a/hscontrol/types/users_test.go +++ b/hscontrol/types/users_test.go @@ -291,12 +291,14 @@ func TestCleanIdentifier(t *testing.T) { func TestOIDCClaimsJSONToUser(t *testing.T) { tests := []struct { - name string - jsonstr string - want User + name string + jsonstr string + emailVerifiedRequired bool + want User }{ { - name: "normal-bool", + name: "normal-bool", + emailVerifiedRequired: true, jsonstr: ` { "sub": "test", @@ -314,7 +316,8 @@ func TestOIDCClaimsJSONToUser(t *testing.T) { }, }, { - name: "string-bool-true", + name: "string-bool-true", + emailVerifiedRequired: true, jsonstr: ` { "sub": "test2", @@ -332,7 +335,8 @@ func TestOIDCClaimsJSONToUser(t *testing.T) { }, }, { - name: "string-bool-false", + name: "string-bool-false", + emailVerifiedRequired: true, jsonstr: ` { "sub": "test3", @@ -348,9 +352,29 @@ func TestOIDCClaimsJSONToUser(t *testing.T) { }, }, }, + { + name: "allow-unverified-email", + emailVerifiedRequired: false, + jsonstr: ` +{ + "sub": "test4", + "email": "test4@test.no", + "email_verified": "false" +} + `, + want: User{ + Provider: util.RegisterMethodOIDC, + Email: "test4@test.no", + ProviderIdentifier: sql.NullString{ + String: "/test4", + Valid: true, + }, + }, + }, { // From https://github.com/juanfont/headscale/issues/2333 - name: "okta-oidc-claim-20250121", + name: "okta-oidc-claim-20250121", + emailVerifiedRequired: true, jsonstr: ` { "sub": "00u7dr4qp7XXXXXXXXXX", @@ -375,6 +399,7 @@ func TestOIDCClaimsJSONToUser(t *testing.T) { want: User{ Provider: util.RegisterMethodOIDC, DisplayName: "Tim Horton", + Email: "", Name: "tim.horton@company.com", ProviderIdentifier: sql.NullString{ String: "https://sso.company.com/oauth2/default/00u7dr4qp7XXXXXXXXXX", @@ -384,7 +409,8 @@ func TestOIDCClaimsJSONToUser(t *testing.T) { }, { // From https://github.com/juanfont/headscale/issues/2333 - name: "okta-oidc-claim-20250121", + name: "okta-oidc-claim-20250121", + emailVerifiedRequired: true, jsonstr: ` { "aud": "79xxxxxx-xxxx-xxxx-xxxx-892146xxxxxx", @@ -409,6 +435,7 @@ func TestOIDCClaimsJSONToUser(t *testing.T) { Provider: util.RegisterMethodOIDC, DisplayName: "XXXXXX XXXX", Name: "user@domain.com", + Email: "", ProviderIdentifier: sql.NullString{ String: "https://login.microsoftonline.com/v2.0/I-70OQnj3TogrNSfkZQqB3f7dGwyBWSm1dolHNKrMzQ", Valid: true, @@ -417,7 +444,8 @@ func TestOIDCClaimsJSONToUser(t *testing.T) { }, { // From https://github.com/juanfont/headscale/issues/2333 - name: "casby-oidc-claim-20250513", + name: "casby-oidc-claim-20250513", + emailVerifiedRequired: true, jsonstr: ` { "sub": "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", @@ -458,7 +486,7 @@ func TestOIDCClaimsJSONToUser(t *testing.T) { var user User - user.FromClaim(&got) + user.FromClaim(&got, tt.emailVerifiedRequired) if diff := cmp.Diff(user, tt.want); diff != "" { t.Errorf("TestOIDCClaimsJSONToUser() mismatch (-want +got):\n%s", diff) }