Set OpenID Connect Expiry

This commit adds a default OpenID Connect expiry to 180d to align with
Tailscale SaaS (previously infinite or based on token expiry).

In addition, it adds an option use the expiry time from the Token sent
by the OpenID provider. This will typically cause really short expiry
and you should only turn on this option if you know what you are
desiring.

This fixes #1176.

Co-authored-by: Even Holthe <even.holthe@bekk.no>
Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

.github/workflows/test-integration-v2-TestOIDCExpireNodesBasedOnTokenExpiry.yaml

@@ -2,7 +2,7 @@
 # DO NOT EDIT, generated with cmd/gh-action-integration-generator/main.go
 # To regenerate, run "go generate" in cmd/gh-action-integration-generator/
 
-name: Integration Test v2 - TestOIDCExpireNodes
+name: Integration Test v2 - TestOIDCExpireNodesBasedOnTokenExpiry
 
 on: [pull_request]
 
@@ -49,7 +49,7 @@ jobs:
                   -failfast \
                   -timeout 120m \
                   -parallel 1 \
-                  -run "^TestOIDCExpireNodes$"
+                  -run "^TestOIDCExpireNodesBasedOnTokenExpiry$"
 
       - uses: actions/upload-artifact@v3
         if: always() && steps.changed-files.outputs.any_changed == 'true'

CHANGELOG.md

@@ -6,6 +6,9 @@
 
 - Fix wrong behaviour in exit nodes [#1159](https://github.com/juanfont/headscale/pull/1159)
 - Align behaviour of `dns_config.restricted_nameservers` to tailscale [#1162](https://github.com/juanfont/headscale/pull/1162)
+- Make OpenID Connect authenticated client expiry time configurable [#1191](https://github.com/juanfont/headscale/pull/1191)
+  - defaults to 180 days like Tailscale SaaS
+  - adds option to use the expiry time from the OpenID token for the node (see config-example.yaml)
 
 ## 0.19.0 (2023-01-29)
 

cmd/gh-action-integration-generator/main.go

@@ -90,7 +90,7 @@ func main() {
 		"TestHeadscale",
 		"TestUserCommand",
 		"TestOIDCAuthenticationPingAll",
-		"TestOIDCExpireNodes",
+		"TestOIDCExpireNodesBasedOnTokenExpiry",
 		"TestPingAllByHostname",
 		"TestPingAllByIP",
 		"TestPreAuthKeyCommand",

config-example.yaml

@@ -282,27 +282,38 @@ unix_socket_permission: "0770"
 #   client_secret_path: "${CREDENTIALS_DIRECTORY}/oidc_client_secret"
 #   # client_secret and client_secret_path are mutually exclusive.
 #
-#   Customize the scopes used in the OIDC flow, defaults to "openid", "profile" and "email" and add custom query
-#   parameters to the Authorize Endpoint request. Scopes default to "openid", "profile" and "email".
+#   # The amount of time from a node is authenticated with OpenID until it
+#   # expires and needs to reauthenticate.
+#   # Setting the value to "0" will mean no expiry.
+#   expiry: 180d
+#
+#   # Use the expiry from the token received from OpenID when the user logged
+#   # in, this will typically lead to frequent need to reauthenticate and should
+#   # only been enabled if you know what you are doing.
+#   # Note: enabling this will cause `oidc.expiry` to be ignored.
+#   use_expiry_from_token: false
+#
+#   # Customize the scopes used in the OIDC flow, defaults to "openid", "profile" and "email" and add custom query
+#   # parameters to the Authorize Endpoint request. Scopes default to "openid", "profile" and "email".
 #
 #   scope: ["openid", "profile", "email", "custom"]
 #   extra_params:
 #     domain_hint: example.com
 #
-#   List allowed principal domains and/or users. If an authenticated user's domain is not in this list, the
-#   authentication request will be rejected.
+#   # List allowed principal domains and/or users. If an authenticated user's domain is not in this list, the
+#   # authentication request will be rejected.
 #
 #   allowed_domains:
 #     - example.com
-# Groups from keycloak have a leading '/'
+#   # Note: Groups from keycloak have a leading '/'
 #   allowed_groups:
 #     - /headscale
 #   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
+#   # 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

config.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/coreos/go-oidc/v3/oidc"
+	"github.com/prometheus/common/model"
 	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
 	"github.com/spf13/viper"
@@ -25,9 +26,14 @@ const (
 
 	JSONLogFormat = "json"
 	TextLogFormat = "text"
+
+	defaultOIDCExpiryTime               = 180 * 24 * time.Hour // 180 Days
+	maxDuration           time.Duration = 1<<63 - 1
 )
 
-var errOidcMutuallyExclusive = errors.New("oidc_client_secret and oidc_client_secret_path are mutually exclusive")
+var errOidcMutuallyExclusive = errors.New(
+	"oidc_client_secret and oidc_client_secret_path are mutually exclusive",
+)
 
 // Config contains the initial Headscale configuration.
 type Config struct {
@@ -101,6 +107,8 @@ type OIDCConfig struct {
 	AllowedUsers               []string
 	AllowedGroups              []string
 	StripEmaildomain           bool
+	Expiry                     time.Duration
+	UseExpiryFromToken         bool
 }
 
 type DERPConfig struct {
@@ -180,6 +188,8 @@ func LoadConfig(path string, isFile bool) error {
 	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)
 
 	viper.SetDefault("logtail.enabled", false)
 	viper.SetDefault("randomize_client_port", false)
@@ -601,6 +611,22 @@ func GetHeadscaleConfig() (*Config, error) {
 			AllowedUsers:     viper.GetStringSlice("oidc.allowed_users"),
 			AllowedGroups:    viper.GetStringSlice("oidc.allowed_groups"),
 			StripEmaildomain: viper.GetBool("oidc.strip_email_domain"),
+			Expiry: func() time.Duration {
+				// if set to 0, we assume no expiry
+				if value := viper.GetString("oidc.expiry"); value == "0" {
+					return maxDuration
+				} else {
+					expiry, err := model.ParseDuration(value)
+					if err != nil {
+						log.Warn().Msg("failed to parse oidc.expiry, defaulting back to 180 days")
+
+						return defaultOIDCExpiryTime
+					}
+
+					return time.Duration(expiry)
+				}
+			}(),
+			UseExpiryFromToken: viper.GetBool("oidc.use_expiry_from_token"),
 		},
 
 		LogTail:             logConfig,

integration/auth_oidc_test.go

@@ -100,7 +100,7 @@ func TestOIDCAuthenticationPingAll(t *testing.T) {
 	}
 }
 
-func TestOIDCExpireNodes(t *testing.T) {
+func TestOIDCExpireNodesBasedOnTokenExpiry(t *testing.T) {
 	IntegrationSkip(t)
 	t.Parallel()
 
@@ -129,6 +129,7 @@ func TestOIDCExpireNodes(t *testing.T) {
 		"HEADSCALE_OIDC_CLIENT_ID":             oidcConfig.ClientID,
 		"HEADSCALE_OIDC_CLIENT_SECRET":         oidcConfig.ClientSecret,
 		"HEADSCALE_OIDC_STRIP_EMAIL_DOMAIN":    fmt.Sprintf("%t", oidcConfig.StripEmaildomain),
+		"HEADSCALE_OIDC_USE_EXPIRY_FROM_TOKEN": "1",
 	}
 
 	err = scenario.CreateHeadscaleEnv(
@@ -278,7 +279,10 @@ func (s *AuthOIDCScenario) runMockOIDC(accessTTL time.Duration) (*headscale.OIDC
 	log.Printf("headscale mock oidc is ready for tests at %s", hostEndpoint)
 
 	return &headscale.OIDCConfig{
-		Issuer:                     fmt.Sprintf("http://%s/oidc", net.JoinHostPort(s.mockOIDC.GetIPInNetwork(s.network), strconv.Itoa(port))),
+		Issuer: fmt.Sprintf(
+			"http://%s/oidc",
+			net.JoinHostPort(s.mockOIDC.GetIPInNetwork(s.network), strconv.Itoa(port)),
+		),
 		ClientID:                   "superclient",
 		ClientSecret:               "supersecret",
 		StripEmaildomain:           true,

integration_test/etc/alt-config.dump.gold.yaml

@@ -37,12 +37,14 @@ logtail:
   enabled: false
 metrics_listen_addr: 127.0.0.1:19090
 oidc:
+  expiry: 180d
   only_start_if_oidc_is_available: true
   scope:
     - openid
     - profile
     - email
   strip_email_domain: true
+  use_expiry_from_token: false
 private_key_path: private.key
 noise:
   private_key_path: noise_private.key

integration_test/etc/alt-env-config.dump.gold.yaml

@@ -36,12 +36,14 @@ logtail:
   enabled: false
 metrics_listen_addr: 127.0.0.1:19090
 oidc:
+  expiry: 180d
   only_start_if_oidc_is_available: true
   scope:
     - openid
     - profile
     - email
   strip_email_domain: true
+  use_expiry_from_token: false
 private_key_path: private.key
 noise:
   private_key_path: noise_private.key

integration_test/etc/config.dump.gold.yaml

@@ -37,12 +37,14 @@ logtail:
   enabled: false
 metrics_listen_addr: 127.0.0.1:9090
 oidc:
+  expiry: 180d
   only_start_if_oidc_is_available: true
   scope:
     - openid
     - profile
     - email
   strip_email_domain: true
+  use_expiry_from_token: false
 private_key_path: private.key
 noise:
   private_key_path: noise_private.key

oidc.go

@@ -27,7 +27,9 @@ const (
 	errOIDCAllowedDomains      = Error("authenticated principal does not match any allowed domain")
 	errOIDCAllowedGroups       = Error("authenticated principal is not in any allowed group")
 	errOIDCAllowedUsers        = Error("authenticated principal does not match any allowed user")
-	errOIDCInvalidMachineState = Error("requested machine state key expired before authorisation completed")
+	errOIDCInvalidMachineState = Error(
+		"requested machine state key expired before authorisation completed",
+	)
 	errOIDCNodeKeyMissing = Error("could not get node key from cache")
 )
 
@@ -68,6 +70,14 @@ func (h *Headscale) initOIDC() error {
 	return nil
 }
 
+func (h *Headscale) determineTokenExpiration(idTokenExpiration time.Time) time.Time {
+	if h.cfg.OIDC.UseExpiryFromToken {
+		return idTokenExpiration
+	}
+
+	return time.Now().Add(h.cfg.OIDC.Expiry)
+}
+
 // RegisterOIDC redirects to the OIDC provider for authentication
 // Puts NodeKey in cache so the callback can retrieve it using the oidc state param
 // Listens in /oidc/register/:nKey.
@@ -193,6 +203,7 @@ func (h *Headscale) OIDCCallback(
 	if err != nil {
 		return
 	}
+	idTokenExpiry := h.determineTokenExpiration(idToken.Expiry)
 
 	// TODO: we can use userinfo at some point to grab additional information about the user (groups membership, etc)
 	// userInfo, err := oidcProvider.UserInfo(context.Background(), oauth2.StaticTokenSource(oauth2Token))
@@ -218,7 +229,12 @@ func (h *Headscale) OIDCCallback(
 		return
 	}
 
-	nodeKey, machineExists, err := h.validateMachineForOIDCCallback(writer, state, claims, idToken.Expiry)
+	nodeKey, machineExists, err := h.validateMachineForOIDCCallback(
+		writer,
+		state,
+		claims,
+		idTokenExpiry,
+	)
 	if err != nil || machineExists {
 		return
 	}
@@ -236,7 +252,7 @@ func (h *Headscale) OIDCCallback(
 		return
 	}
 
-	if err := h.registerMachineForOIDCCallback(writer, user, nodeKey, idToken.Expiry); err != nil {
+	if err := h.registerMachineForOIDCCallback(writer, user, nodeKey, idTokenExpiry); err != nil {
 		return
 	}