diff --git a/.github/workflows/test-integration-v2-TestOIDCExpireNodes.yaml b/.github/workflows/test-integration-v2-TestOIDCExpireNodesBasedOnTokenExpiry.yaml similarity index 92% rename from .github/workflows/test-integration-v2-TestOIDCExpireNodes.yaml rename to .github/workflows/test-integration-v2-TestOIDCExpireNodesBasedOnTokenExpiry.yaml index db2ea62b..287d129b 100644 --- a/.github/workflows/test-integration-v2-TestOIDCExpireNodes.yaml +++ b/.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' diff --git a/CHANGELOG.md b/CHANGELOG.md index 90971fd3..593c76e4 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/cmd/gh-action-integration-generator/main.go b/cmd/gh-action-integration-generator/main.go index af60a7fa..6231ec35 100644 --- a/cmd/gh-action-integration-generator/main.go +++ b/cmd/gh-action-integration-generator/main.go @@ -90,7 +90,7 @@ func main() { "TestHeadscale", "TestUserCommand", "TestOIDCAuthenticationPingAll", - "TestOIDCExpireNodes", + "TestOIDCExpireNodesBasedOnTokenExpiry", "TestPingAllByHostname", "TestPingAllByIP", "TestPreAuthKeyCommand", diff --git a/config-example.yaml b/config-example.yaml index 7b4398d0..249a0442 100644 --- a/config-example.yaml +++ b/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 diff --git a/config.go b/config.go index fed9b032..c0dd1c98 100644 --- a/config.go +++ b/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, diff --git a/integration/auth_oidc_test.go b/integration/auth_oidc_test.go index e9803457..f3ac040d 100644 --- a/integration/auth_oidc_test.go +++ b/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() @@ -125,10 +125,11 @@ func TestOIDCExpireNodes(t *testing.T) { } oidcMap := map[string]string{ - "HEADSCALE_OIDC_ISSUER": oidcConfig.Issuer, - "HEADSCALE_OIDC_CLIENT_ID": oidcConfig.ClientID, - "HEADSCALE_OIDC_CLIENT_SECRET": oidcConfig.ClientSecret, - "HEADSCALE_OIDC_STRIP_EMAIL_DOMAIN": fmt.Sprintf("%t", oidcConfig.StripEmaildomain), + "HEADSCALE_OIDC_ISSUER": oidcConfig.Issuer, + "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, diff --git a/integration_test/etc/alt-config.dump.gold.yaml b/integration_test/etc/alt-config.dump.gold.yaml index 6a51d904..c0665b30 100644 --- a/integration_test/etc/alt-config.dump.gold.yaml +++ b/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 diff --git a/integration_test/etc/alt-env-config.dump.gold.yaml b/integration_test/etc/alt-env-config.dump.gold.yaml index 908b6326..0be5d9ed 100644 --- a/integration_test/etc/alt-env-config.dump.gold.yaml +++ b/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 diff --git a/integration_test/etc/config.dump.gold.yaml b/integration_test/etc/config.dump.gold.yaml index 2e9675f7..e6a822a5 100644 --- a/integration_test/etc/config.dump.gold.yaml +++ b/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 diff --git a/oidc.go b/oidc.go index 4909ba99..5aa3ba62 100644 --- a/oidc.go +++ b/oidc.go @@ -27,8 +27,10 @@ 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") - errOIDCNodeKeyMissing = Error("could not get node key from cache") + errOIDCInvalidMachineState = Error( + "requested machine state key expired before authorisation completed", + ) + errOIDCNodeKeyMissing = Error("could not get node key from cache") ) type IDTokenClaims struct { @@ -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 }