More hardening:

* Look up users by email, not `preferred_username` (though, this field
  didn't exist before #2020)

* Check OIDC claim for `email_verified`, and reject it if false or unset.
  This is can be disabled with `oidc.allow_unverified_email = true`.

* Don't create a new user if one with the same `email` exists and has an
  OIDC identifier set.

hscontrol/oidc.go

@@ -441,12 +441,16 @@ func (a *AuthProviderOIDC) createOrUpdateUserFromClaim(
 		return nil, fmt.Errorf("creating or updating user: %w", err)
 	}
 
+    if !claims.EmailVerified && !a.cfg.AllowUnverifiedEmail {
+		return nil, fmt.Errorf("OIDC provider does not report email address is verified")
+	}
+
 	// This check is for legacy, if the user cannot be found by the OIDC identifier
 	// look it up by username. This should only be needed once.
 	// This branch will presist for a number of versions after the OIDC migration and
 	// then be removed following a deprecation.
 	if a.cfg.MapLegacyUsers && user == nil {
-		user, err = a.db.GetUserByName(claims.Username)
+		user, err = a.db.GetUserByEmail(claims.Email)
 		if err != nil && !errors.Is(err, db.ErrUserNotFound) {
 			return nil, fmt.Errorf("creating or updating user: %w", err)
 		}
@@ -456,13 +460,12 @@ func (a *AuthProviderOIDC) createOrUpdateUserFromClaim(
 			user = &types.User{}
 		}
 
-		// If the user exists, but it already has a provider identifier (OIDC sub), create a new user.
+		// If the user exists, but it already has a provider identifier (OIDC sub), reject it.
 		// This is to prevent users that have already been migrated to the new OIDC format
 		// to be updated with the new OIDC identifier inexplicitly which might be the cause of an
 		// account takeover.
 		if user.ProviderIdentifier != "" {
-			log.Info().Str("username", claims.Username).Str("sub", claims.Sub).Msg("user found by username, but has provider identifier, creating new user.")
-			user = &types.User{}
+			return nil, fmt.Errorf("user with that email already exists, but has a different provider identifier")
 		}
 	}
 

hscontrol/types/config.go

@@ -165,6 +165,7 @@ type OIDCConfig struct {
 	Expiry                     time.Duration
 	UseExpiryFromToken         bool
 	MapLegacyUsers             bool
+	AllowUnverifiedEmail       bool
 }
 
 type DERPConfig struct {
@@ -276,6 +277,7 @@ func LoadConfig(path string, isFile bool) error {
 	viper.SetDefault("oidc.expiry", "180d")
 	viper.SetDefault("oidc.use_expiry_from_token", false)
 	viper.SetDefault("oidc.map_legacy_users", false)
+	viper.SetDefault("oidc.allow_unverified_email", false)
 
 	viper.SetDefault("logtail.enabled", false)
 	viper.SetDefault("randomize_client_port", false)
@@ -897,8 +899,9 @@ func LoadServerConfig() (*Config, error) {
 					return time.Duration(expiry)
 				}
 			}(),
-			UseExpiryFromToken: viper.GetBool("oidc.use_expiry_from_token"),
-			MapLegacyUsers:     viper.GetBool("oidc.map_legacy_users"),
+			UseExpiryFromToken:   viper.GetBool("oidc.use_expiry_from_token"),
+			MapLegacyUsers:       viper.GetBool("oidc.map_legacy_users"),
+			AllowUnverifiedEmail: viper.GetBool("oidc.allow_unverified_email"),
 		},
 
 		LogTail:             logTailConfig,