mirror of
https://github.com/juanfont/headscale.git
synced 2024-11-29 18:33:05 +00:00
Harden OIDC migration and make optional
This commit hardens the migration part of the OIDC from the old username based approach to the new sub based approach and makes it possible for the operator to opt out entirely. Fixes #1990 Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
This commit is contained in:
parent
a6b19e85db
commit
1f73616f90
3 changed files with 27 additions and 7 deletions
|
@ -364,12 +364,18 @@ unix_socket_permission: "0770"
|
|||
# allowed_users:
|
||||
# - alice@example.com
|
||||
#
|
||||
# # If `strip_email_domain` is set to `true`, the domain part of the username email address will be removed.
|
||||
# # This will transform `first-name.last-name@example.com` to the user `first-name.last-name`
|
||||
# # If `strip_email_domain` is set to `false` the domain part will NOT be removed resulting to the following
|
||||
# user: `first-name.last-name.example.com`
|
||||
#
|
||||
# strip_email_domain: true
|
||||
# # Map legacy users from pre-0.24.0 versions of headscale to the new OIDC users
|
||||
# # by taking the username from the legacy user and matching it with the username
|
||||
# # provided by the OIDC. This is useful when migrating from legacy users to OIDC
|
||||
# # to force them using the unique identifier from the OIDC and to give them a
|
||||
# # proper display name and picture if available.
|
||||
# # Note that this will only work if the username from the legacy user is the same
|
||||
# # and ther is a posibility for account takeover should a username have changed
|
||||
# # with the provider.
|
||||
# # Disabling this feature will cause all new logins to be created as new users.
|
||||
# # Note this option will be removed in the future and should be set to false
|
||||
# # on all new installations, or when all users have logged in with OIDC once.
|
||||
# map_legacy_users: true
|
||||
|
||||
# Logtail configuration
|
||||
# Logtail is Tailscales logging and auditing infrastructure, it allows the control panel
|
||||
|
|
|
@ -443,7 +443,9 @@ func (a *AuthProviderOIDC) createOrUpdateUserFromClaim(
|
|||
|
||||
// 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.
|
||||
if user == nil {
|
||||
// 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)
|
||||
if err != nil && !errors.Is(err, db.ErrUserNotFound) {
|
||||
return nil, fmt.Errorf("creating or updating user: %w", err)
|
||||
|
@ -453,6 +455,15 @@ func (a *AuthProviderOIDC) createOrUpdateUserFromClaim(
|
|||
if user == nil {
|
||||
user = &types.User{}
|
||||
}
|
||||
|
||||
// If the user exists, but it already has a provider identifier (OIDC sub), create a new user.
|
||||
// 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{}
|
||||
}
|
||||
}
|
||||
|
||||
user.FromClaim(claims)
|
||||
|
|
|
@ -165,6 +165,7 @@ type OIDCConfig struct {
|
|||
AllowedGroups []string
|
||||
Expiry time.Duration
|
||||
UseExpiryFromToken bool
|
||||
MapLegacyUsers bool
|
||||
}
|
||||
|
||||
type DERPConfig struct {
|
||||
|
@ -276,6 +277,7 @@ func LoadConfig(path string, isFile bool) error {
|
|||
viper.SetDefault("oidc.only_start_if_oidc_is_available", true)
|
||||
viper.SetDefault("oidc.expiry", "180d")
|
||||
viper.SetDefault("oidc.use_expiry_from_token", false)
|
||||
viper.SetDefault("oidc.map_legacy_users", true)
|
||||
|
||||
viper.SetDefault("logtail.enabled", false)
|
||||
viper.SetDefault("randomize_client_port", false)
|
||||
|
@ -897,6 +899,7 @@ func LoadServerConfig() (*Config, error) {
|
|||
}
|
||||
}(),
|
||||
UseExpiryFromToken: viper.GetBool("oidc.use_expiry_from_token"),
|
||||
MapLegacyUsers: viper.GetBool("oidc.map_legacy_users"),
|
||||
},
|
||||
|
||||
LogTail: logTailConfig,
|
||||
|
|
Loading…
Reference in a new issue