Redo OIDC configuration (#2020)

expand user, add claims to user

This commit expands the user table with additional fields that
can be retrieved from OIDC providers (and other places) and
uses this data in various tailscale response objects if it is
available.

This is the beginning of implementing
https://docs.google.com/document/d/1X85PMxIaVWDF6T_UPji3OeeUqVBcGj_uHRM5CI-AwlY/edit
trying to make OIDC more coherant and maintainable in addition
to giving the user a better experience and integration with a
provider.

remove usernames in magic dns, normalisation of emails

this commit removes the option to have usernames as part of MagicDNS
domains and headscale will now align with Tailscale, where there is a
root domain, and the machine name.

In addition, the various normalisation functions for dns names has been
made lighter not caring about username and special character that wont
occur.

Email are no longer normalised as part of the policy processing.

untagle oidc and regcache, use typed cache

This commits stops reusing the registration cache for oidc
purposes and switches the cache to be types and not use any
allowing the removal of a bunch of casting.

try to make reauth/register branches clearer in oidc

Currently there was a function that did a bunch of stuff,
finding the machine key, trying to find the node, reauthing
the node, returning some status, and it was called validate
which was very confusing.

This commit tries to split this into what to do if the node
exists, if it needs to register etc.

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
This commit is contained in:
Kristoffer Dalby 2024-10-02 14:50:17 +02:00 committed by GitHub
parent bc9e83b52e
commit 218138afee
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
32 changed files with 628 additions and 917 deletions

View file

@ -71,8 +71,7 @@ type Config struct {
ACMEURL string
ACMEEmail string
DNSConfig *tailcfg.DNSConfig
DNSUserNameInMagicDNS bool
DNSConfig *tailcfg.DNSConfig
UnixSocket string
UnixSocketPermission fs.FileMode
@ -90,12 +89,11 @@ type Config struct {
}
type DNSConfig struct {
MagicDNS bool `mapstructure:"magic_dns"`
BaseDomain string `mapstructure:"base_domain"`
Nameservers Nameservers
SearchDomains []string `mapstructure:"search_domains"`
ExtraRecords []tailcfg.DNSRecord `mapstructure:"extra_records"`
UserNameInMagicDNS bool `mapstructure:"use_username_in_magic_dns"`
MagicDNS bool `mapstructure:"magic_dns"`
BaseDomain string `mapstructure:"base_domain"`
Nameservers Nameservers
SearchDomains []string `mapstructure:"search_domains"`
ExtraRecords []tailcfg.DNSRecord `mapstructure:"extra_records"`
}
type Nameservers struct {
@ -164,7 +162,6 @@ type OIDCConfig struct {
AllowedDomains []string
AllowedUsers []string
AllowedGroups []string
StripEmaildomain bool
Expiry time.Duration
UseExpiryFromToken bool
}
@ -274,7 +271,6 @@ 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)
@ -321,8 +317,22 @@ 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")
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",
"dns_config.use_username_in_musername_in_magic_dns",
} {
if viper.IsSet(removed) {
log.Fatal().
Msgf("Fatal config error: %s has been removed. Please remove it from your config file", removed)
}
}
// Collect any validation errors and return them all at once
var errorText string
if (viper.GetString("tls_letsencrypt_hostname") != "") &&
@ -572,12 +582,9 @@ func dns() (DNSConfig, error) {
if err != nil {
return DNSConfig{}, fmt.Errorf("unmarshaling dns extra records: %w", err)
}
dns.ExtraRecords = extraRecords
}
dns.UserNameInMagicDNS = viper.GetBool("dns.use_username_in_magic_dns")
return dns, nil
}
@ -780,7 +787,12 @@ func LoadServerConfig() (*Config, error) {
case string(IPAllocationStrategyRandom):
alloc = IPAllocationStrategyRandom
default:
return nil, fmt.Errorf("config error, prefixes.allocation is set to %s, which is not a valid strategy, allowed options: %s, %s", allocStr, IPAllocationStrategySequential, IPAllocationStrategyRandom)
return nil, fmt.Errorf(
"config error, prefixes.allocation is set to %s, which is not a valid strategy, allowed options: %s, %s",
allocStr,
IPAllocationStrategySequential,
IPAllocationStrategyRandom,
)
}
dnsConfig, err := dns()
@ -814,10 +826,11 @@ func LoadServerConfig() (*Config, error) {
// - DERP run on their own domains
// - Control plane runs on login.tailscale.com/controlplane.tailscale.com
// - MagicDNS (BaseDomain) for users is on a *.ts.net domain per tailnet (e.g. tail-scale.ts.net)
//
// TODO(kradalby): remove dnsConfig.UserNameInMagicDNS check when removed.
if !dnsConfig.UserNameInMagicDNS && dnsConfig.BaseDomain != "" && strings.Contains(serverURL, dnsConfig.BaseDomain) {
return nil, errors.New("server_url cannot contain the base_domain, this will cause the headscale server and embedded DERP to become unreachable from the Tailscale node.")
if dnsConfig.BaseDomain != "" &&
strings.Contains(serverURL, dnsConfig.BaseDomain) {
return nil, errors.New(
"server_url cannot contain the base_domain, this will cause the headscale server and embedded DERP to become unreachable from the Tailscale node.",
)
}
return &Config{
@ -847,8 +860,7 @@ func LoadServerConfig() (*Config, error) {
TLS: tlsConfig(),
DNSConfig: dnsToTailcfgDNS(dnsConfig),
DNSUserNameInMagicDNS: dnsConfig.UserNameInMagicDNS,
DNSConfig: dnsToTailcfgDNS(dnsConfig),
ACMEEmail: viper.GetString("acme_email"),
ACMEURL: viper.GetString("acme_url"),
@ -860,15 +872,14 @@ 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"),
StripEmaildomain: viper.GetBool("oidc.strip_email_domain"),
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"),
Expiry: func() time.Duration {
// if set to 0, we assume no expiry
if value := viper.GetString("oidc.expiry"); value == "0" {
@ -903,9 +914,11 @@ func LoadServerConfig() (*Config, error) {
// TODO(kradalby): Document these settings when more stable
Tuning: Tuning{
NotifierSendTimeout: viper.GetDuration("tuning.notifier_send_timeout"),
BatchChangeDelay: viper.GetDuration("tuning.batch_change_delay"),
NodeMapSessionBufferedChanSize: viper.GetInt("tuning.node_mapsession_buffered_chan_size"),
NotifierSendTimeout: viper.GetDuration("tuning.notifier_send_timeout"),
BatchChangeDelay: viper.GetDuration("tuning.batch_change_delay"),
NodeMapSessionBufferedChanSize: viper.GetInt(
"tuning.node_mapsession_buffered_chan_size",
),
},
}, nil
}
@ -921,14 +934,26 @@ func (d *deprecator) warnWithAlias(newKey, oldKey string) {
// NOTE: RegisterAlias is called with NEW KEY -> OLD KEY
viper.RegisterAlias(newKey, oldKey)
if viper.IsSet(oldKey) {
d.warns.Add(fmt.Sprintf("The %q configuration key is deprecated. Please use %q instead. %q will be removed in the future.", oldKey, newKey, oldKey))
d.warns.Add(
fmt.Sprintf(
"The %q configuration key is deprecated. Please use %q instead. %q will be removed in the future.",
oldKey,
newKey,
oldKey,
),
)
}
}
// fatal deprecates and adds an entry to the fatal list of options if the oldKey is set.
func (d *deprecator) fatal(newKey, oldKey string) {
func (d *deprecator) fatal(oldKey string) {
if viper.IsSet(oldKey) {
d.fatals.Add(fmt.Sprintf("The %q configuration key is deprecated. Please use %q instead. %q has been removed.", oldKey, newKey, oldKey))
d.fatals.Add(
fmt.Sprintf(
"The %q configuration key has been removed. Please see the changelog for more details.",
oldKey,
),
)
}
}
@ -936,7 +961,14 @@ func (d *deprecator) fatal(newKey, oldKey string) {
// If the new key is set, a warning is emitted instead.
func (d *deprecator) fatalIfNewKeyIsNotUsed(newKey, oldKey string) {
if viper.IsSet(oldKey) && !viper.IsSet(newKey) {
d.fatals.Add(fmt.Sprintf("The %q configuration key is deprecated. Please use %q instead. %q has been removed.", oldKey, newKey, oldKey))
d.fatals.Add(
fmt.Sprintf(
"The %q configuration key is deprecated. Please use %q instead. %q has been removed.",
oldKey,
newKey,
oldKey,
),
)
} else if viper.IsSet(oldKey) {
d.warns.Add(fmt.Sprintf("The %q configuration key is deprecated. Please use %q instead. %q has been removed.", oldKey, newKey, oldKey))
}
@ -945,14 +977,26 @@ func (d *deprecator) fatalIfNewKeyIsNotUsed(newKey, oldKey string) {
// warn deprecates and adds an option to log a warning if the oldKey is set.
func (d *deprecator) warnNoAlias(newKey, oldKey string) {
if viper.IsSet(oldKey) {
d.warns.Add(fmt.Sprintf("The %q configuration key is deprecated. Please use %q instead. %q has been removed.", oldKey, newKey, oldKey))
d.warns.Add(
fmt.Sprintf(
"The %q configuration key is deprecated. Please use %q instead. %q has been removed.",
oldKey,
newKey,
oldKey,
),
)
}
}
// warn deprecates and adds an entry to the warn list of options if the oldKey is set.
func (d *deprecator) warn(oldKey string) {
if viper.IsSet(oldKey) {
d.warns.Add(fmt.Sprintf("The %q configuration key is deprecated and has been removed. Please see the changelog for more details.", oldKey))
d.warns.Add(
fmt.Sprintf(
"The %q configuration key is deprecated and has been removed. Please see the changelog for more details.",
oldKey,
),
)
}
}

View file

@ -42,8 +42,7 @@ func TestReadConfig(t *testing.T) {
{Name: "grafana.myvpn.example.com", Type: "A", Value: "100.64.0.3"},
{Name: "prometheus.myvpn.example.com", Type: "A", Value: "100.64.0.4"},
},
SearchDomains: []string{"test.com", "bar.com"},
UserNameInMagicDNS: true,
SearchDomains: []string{"test.com", "bar.com"},
},
},
{
@ -99,8 +98,7 @@ func TestReadConfig(t *testing.T) {
{Name: "grafana.myvpn.example.com", Type: "A", Value: "100.64.0.3"},
{Name: "prometheus.myvpn.example.com", Type: "A", Value: "100.64.0.4"},
},
SearchDomains: []string{"test.com", "bar.com"},
UserNameInMagicDNS: true,
SearchDomains: []string{"test.com", "bar.com"},
},
},
{
@ -234,11 +232,10 @@ func TestReadConfigFromEnv(t *testing.T) {
{
name: "unmarshal-dns-full-config",
configEnv: map[string]string{
"HEADSCALE_DNS_MAGIC_DNS": "true",
"HEADSCALE_DNS_BASE_DOMAIN": "example.com",
"HEADSCALE_DNS_NAMESERVERS_GLOBAL": `1.1.1.1 8.8.8.8`,
"HEADSCALE_DNS_SEARCH_DOMAINS": "test.com bar.com",
"HEADSCALE_DNS_USE_USERNAME_IN_MAGIC_DNS": "true",
"HEADSCALE_DNS_MAGIC_DNS": "true",
"HEADSCALE_DNS_BASE_DOMAIN": "example.com",
"HEADSCALE_DNS_NAMESERVERS_GLOBAL": `1.1.1.1 8.8.8.8`,
"HEADSCALE_DNS_SEARCH_DOMAINS": "test.com bar.com",
// TODO(kradalby): Figure out how to pass these as env vars
// "HEADSCALE_DNS_NAMESERVERS_SPLIT": `{foo.bar.com: ["1.1.1.1"]}`,
@ -266,8 +263,7 @@ func TestReadConfigFromEnv(t *testing.T) {
ExtraRecords: []tailcfg.DNSRecord{
// {Name: "prometheus.myvpn.example.com", Type: "A", Value: "100.64.0.4"},
},
SearchDomains: []string{"test.com", "bar.com"},
UserNameInMagicDNS: true,
SearchDomains: []string{"test.com", "bar.com"},
},
},
}

View file

@ -253,7 +253,7 @@ func (node *Node) Proto() *v1.Node {
return nodeProto
}
func (node *Node) GetFQDN(cfg *Config, baseDomain string) (string, error) {
func (node *Node) GetFQDN(baseDomain string) (string, error) {
if node.GivenName == "" {
return "", fmt.Errorf("failed to create valid FQDN: %w", ErrNodeHasNoGivenName)
}
@ -268,19 +268,6 @@ func (node *Node) GetFQDN(cfg *Config, baseDomain string) (string, error) {
)
}
if cfg.DNSUserNameInMagicDNS {
if node.User.Name == "" {
return "", fmt.Errorf("failed to create valid FQDN: %w", ErrNodeUserHasNoName)
}
hostname = fmt.Sprintf(
"%s.%s.%s",
node.GivenName,
node.User.Name,
baseDomain,
)
}
if len(hostname) > MaxHostnameLength {
return "", fmt.Errorf(
"failed to create valid FQDN (%s): %w",

View file

@ -1,7 +1,9 @@
package types
import (
"fmt"
"net/netip"
"strings"
"testing"
"github.com/google/go-cmp/cmp"
@ -127,76 +129,10 @@ func TestNodeFQDN(t *testing.T) {
tests := []struct {
name string
node Node
cfg Config
domain string
want string
wantErr string
}{
{
name: "all-set-with-username",
node: Node{
GivenName: "test",
User: User{
Name: "user",
},
},
cfg: Config{
DNSConfig: &tailcfg.DNSConfig{
Proxied: true,
},
DNSUserNameInMagicDNS: true,
},
domain: "example.com",
want: "test.user.example.com",
},
{
name: "no-given-name-with-username",
node: Node{
User: User{
Name: "user",
},
},
cfg: Config{
DNSConfig: &tailcfg.DNSConfig{
Proxied: true,
},
DNSUserNameInMagicDNS: true,
},
domain: "example.com",
wantErr: "failed to create valid FQDN: node has no given name",
},
{
name: "no-user-name-with-username",
node: Node{
GivenName: "test",
User: User{},
},
cfg: Config{
DNSConfig: &tailcfg.DNSConfig{
Proxied: true,
},
DNSUserNameInMagicDNS: true,
},
domain: "example.com",
wantErr: "failed to create valid FQDN: node user has no name",
},
{
name: "no-magic-dns-with-username",
node: Node{
GivenName: "test",
User: User{
Name: "user",
},
},
cfg: Config{
DNSConfig: &tailcfg.DNSConfig{
Proxied: false,
},
DNSUserNameInMagicDNS: true,
},
domain: "example.com",
want: "test.user.example.com",
},
{
name: "no-dnsconfig-with-username",
node: Node{
@ -216,12 +152,6 @@ func TestNodeFQDN(t *testing.T) {
Name: "user",
},
},
cfg: Config{
DNSConfig: &tailcfg.DNSConfig{
Proxied: true,
},
DNSUserNameInMagicDNS: false,
},
domain: "example.com",
want: "test.example.com",
},
@ -232,46 +162,16 @@ func TestNodeFQDN(t *testing.T) {
Name: "user",
},
},
cfg: Config{
DNSConfig: &tailcfg.DNSConfig{
Proxied: true,
},
DNSUserNameInMagicDNS: false,
},
domain: "example.com",
wantErr: "failed to create valid FQDN: node has no given name",
},
{
name: "no-user-name",
name: "too-long-username",
node: Node{
GivenName: "test",
User: User{},
GivenName: strings.Repeat("a", 256),
},
cfg: Config{
DNSConfig: &tailcfg.DNSConfig{
Proxied: true,
},
DNSUserNameInMagicDNS: false,
},
domain: "example.com",
want: "test.example.com",
},
{
name: "no-magic-dns",
node: Node{
GivenName: "test",
User: User{
Name: "user",
},
},
cfg: Config{
DNSConfig: &tailcfg.DNSConfig{
Proxied: false,
},
DNSUserNameInMagicDNS: false,
},
domain: "example.com",
want: "test.example.com",
domain: "example.com",
wantErr: fmt.Sprintf("failed to create valid FQDN (%s.example.com): hostname too long, cannot except 255 ASCII chars", strings.Repeat("a", 256)),
},
{
name: "no-dnsconfig",
@ -288,7 +188,9 @@ func TestNodeFQDN(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got, err := tc.node.GetFQDN(&tc.cfg, tc.domain)
got, err := tc.node.GetFQDN(tc.domain)
t.Logf("GOT: %q, %q", got, tc.domain)
if (err != nil) && (err.Error() != tc.wantErr) {
t.Errorf("GetFQDN() error = %s, wantErr %s", err, tc.wantErr)

View file

@ -1,6 +1,7 @@
package types
import (
"cmp"
"strconv"
v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
@ -10,25 +11,65 @@ import (
"tailscale.com/tailcfg"
)
type UserID uint64
// User is the way Headscale implements the concept of users in Tailscale
//
// At the end of the day, users in Tailscale are some kind of 'bubbles' or users
// that contain our machines.
type User struct {
gorm.Model
// Username for the user, is used if email is empty
// Should not be used, please use Username().
Name string `gorm:"unique"`
// Typically the full name of the user
DisplayName string
// Email of the user
// Should not be used, please use Username().
Email string
// Unique identifier of the user from OIDC,
// comes from `sub` claim in the OIDC token
// and is used to lookup the user.
ProviderIdentifier string `gorm:"index"`
// Provider is the origin of the user account,
// same as RegistrationMethod, without authkey.
Provider string
ProfilePicURL string
}
// Username is the main way to get the username of a user,
// it will return the email if it exists, the name if it exists,
// the OIDCIdentifier if it exists, and the ID if nothing else exists.
// Email and OIDCIdentifier will be set when the user has headscale
// enabled with OIDC, which means that there is a domain involved which
// should be used throughout headscale, in information returned to the
// user and the Policy engine.
func (u *User) Username() string {
return cmp.Or(u.Email, u.Name, u.ProviderIdentifier, strconv.FormatUint(uint64(u.ID), 10))
}
// DisplayNameOrUsername returns the DisplayName if it exists, otherwise
// it will return the Username.
func (u *User) DisplayNameOrUsername() string {
return cmp.Or(u.DisplayName, u.Username())
}
// TODO(kradalby): See if we can fill in Gravatar here.
func (u *User) profilePicURL() string {
return ""
return u.ProfilePicURL
}
func (u *User) TailscaleUser() *tailcfg.User {
user := tailcfg.User{
ID: tailcfg.UserID(u.ID),
LoginName: u.Name,
DisplayName: u.Name,
LoginName: u.Username(),
DisplayName: u.DisplayNameOrUsername(),
ProfilePicURL: u.profilePicURL(),
Logins: []tailcfg.LoginID{},
Created: u.CreatedAt,
@ -41,9 +82,9 @@ func (u *User) TailscaleLogin() *tailcfg.Login {
login := tailcfg.Login{
ID: tailcfg.LoginID(u.ID),
// TODO(kradalby): this should reflect registration method.
Provider: "",
LoginName: u.Name,
DisplayName: u.Name,
Provider: u.Provider,
LoginName: u.Username(),
DisplayName: u.DisplayNameOrUsername(),
ProfilePicURL: u.profilePicURL(),
}
@ -53,8 +94,8 @@ func (u *User) TailscaleLogin() *tailcfg.Login {
func (u *User) TailscaleUserProfile() tailcfg.UserProfile {
return tailcfg.UserProfile{
ID: tailcfg.UserID(u.ID),
LoginName: u.Name,
DisplayName: u.Name,
LoginName: u.Username(),
DisplayName: u.DisplayNameOrUsername(),
ProfilePicURL: u.profilePicURL(),
}
}
@ -66,3 +107,27 @@ func (n *User) Proto() *v1.User {
CreatedAt: timestamppb.New(n.CreatedAt),
}
}
type OIDCClaims struct {
// Sub is the user's unique identifier at the provider.
Sub string `json:"sub"`
// Name is the user's full name.
Name string `json:"name,omitempty"`
Groups []string `json:"groups,omitempty"`
Email string `json:"email,omitempty"`
EmailVerified bool `json:"email_verified,omitempty"`
ProfilePictureURL string `json:"picture,omitempty"`
Username string `json:"preferred_username,omitempty"`
}
// FromClaim overrides a User from OIDC claims.
// All fields will be updated, except for the ID.
func (u *User) FromClaim(claims *OIDCClaims) {
u.ProviderIdentifier = claims.Sub
u.DisplayName = claims.Name
u.Email = claims.Email
u.Name = claims.Username
u.ProfilePicURL = claims.ProfilePictureURL
u.Provider = util.RegisterMethodOIDC
}