restore strip_email_domain for migration

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
This commit is contained in:
Kristoffer Dalby 2024-10-17 06:22:44 -06:00
parent eae63ec562
commit 14d678c0de
No known key found for this signature in database
3 changed files with 75 additions and 17 deletions

View file

@ -445,25 +445,29 @@ func (a *AuthProviderOIDC) createOrUpdateUserFromClaim(
// 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.
// TODO(kradalby): Remove when strip_email_domain and migration is removed
// after #2170 is cleaned up.
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)
}
if oldUsername, err := getUserName(claims, a.cfg.StripEmaildomain); err == nil {
user, err = a.db.GetUserByName(oldUsername)
if err != nil && !errors.Is(err, db.ErrUserNotFound) {
return nil, fmt.Errorf("creating or updating user: %w", err)
}
// if the user is still not found, create a new empty user.
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 != nil && 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{}
}
}
}
// 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{}
}
// if the user is still not found, create a new empty user.
if user == nil {
user = &types.User{}
}
user.FromClaim(claims)
@ -513,3 +517,21 @@ func renderOIDCCallbackTemplate(
return &content, nil
}
// TODO(kradalby): Reintroduce when strip_email_domain is removed
// after #2170 is cleaned up
// DEPRECATED: DO NOT USE
func getUserName(
claims *types.OIDCClaims,
stripEmaildomain bool,
) (string, error) {
userName, err := util.NormalizeToFQDNRules(
claims.Email,
stripEmaildomain,
)
if err != nil {
return "", err
}
return userName, nil
}

View file

@ -162,6 +162,7 @@ type OIDCConfig struct {
AllowedDomains []string
AllowedUsers []string
AllowedGroups []string
StripEmaildomain bool
Expiry time.Duration
UseExpiryFromToken bool
MapLegacyUsers bool
@ -273,6 +274,7 @@ func LoadConfig(path string, isFile bool) error {
viper.SetDefault("database.sqlite.write_ahead_log", true)
viper.SetDefault("oidc.scope", []string{oidc.ScopeOpenID, "profile", "email"})
viper.SetDefault("oidc.strip_email_domain", true)
viper.SetDefault("oidc.only_start_if_oidc_is_available", true)
viper.SetDefault("oidc.expiry", "180d")
viper.SetDefault("oidc.use_expiry_from_token", false)
@ -320,14 +322,18 @@ func validateServerConfig() error {
depr.warn("dns_config.use_username_in_magic_dns")
depr.warn("dns.use_username_in_magic_dns")
depr.fatal("oidc.strip_email_domain")
// TODO(kradalby): Reintroduce when strip_email_domain is removed
// after #2170 is cleaned up
// depr.fatal("oidc.strip_email_domain")
depr.fatal("dns.use_username_in_musername_in_magic_dns")
depr.fatal("dns_config.use_username_in_musername_in_magic_dns")
depr.Log()
for _, removed := range []string{
"oidc.strip_email_domain",
// TODO(kradalby): Reintroduce when strip_email_domain is removed
// after #2170 is cleaned up
// "oidc.strip_email_domain",
"dns_config.use_username_in_musername_in_magic_dns",
} {
if viper.IsSet(removed) {

View file

@ -182,3 +182,33 @@ func GenerateIPv6DNSRootDomain(ipPrefix netip.Prefix) []dnsname.FQDN {
return fqdns
}
// TODO(kradalby): Reintroduce when strip_email_domain is removed
// after #2170 is cleaned up
// DEPRECATED: DO NOT USE
// NormalizeToFQDNRules will replace forbidden chars in user
// it can also return an error if the user doesn't respect RFC 952 and 1123.
func NormalizeToFQDNRules(name string, stripEmailDomain bool) (string, error) {
name = strings.ToLower(name)
name = strings.ReplaceAll(name, "'", "")
atIdx := strings.Index(name, "@")
if stripEmailDomain && atIdx > 0 {
name = name[:atIdx]
} else {
name = strings.ReplaceAll(name, "@", ".")
}
name = invalidCharsInUserRegex.ReplaceAllString(name, "-")
for _, elt := range strings.Split(name, ".") {
if len(elt) > LabelHostnameLength {
return "", fmt.Errorf(
"label %v is more than 63 chars: %w",
elt,
ErrInvalidUserName,
)
}
}
return name, nil
}