From 4b693a1320e061cf5625db485ea769d050e9791e Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@dalby.cc>
Date: Mon, 15 Jun 2026 10:14:35 +0000
Subject: [PATCH] oidc, types: validate pkce.method when OIDC is active

---
 hscontrol/oidc.go              |  7 +++++++
 hscontrol/types/config.go      |  6 +++++-
 hscontrol/types/config_test.go | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go
index 9fb9096d..95c7b739 100644
--- a/hscontrol/oidc.go
+++ b/hscontrol/oidc.go
@@ -54,6 +54,7 @@ var (
 		"authenticated principal does not match any allowed user",
 	)
 	errOIDCUnverifiedEmail = errors.New("authenticated principal has an unverified email")
+	errInvalidPKCEMethod   = errors.New("invalid pkce.method")
 )
 
 // AuthInfo contains both auth ID and verifier information for OIDC validation.
@@ -186,6 +187,12 @@ func (a *AuthProviderOIDC) authHandler(
 		case types.PKCEMethodPlain:
 			// oauth2 does not have a plain challenge option, so we add it manually
 			extras = append(extras, oauth2.SetAuthURLParam("code_challenge_method", "plain"), oauth2.SetAuthURLParam("code_challenge", verifier))
+		default:
+			// An unknown method must not silently emit no challenge: a
+			// verifier was generated and is sent at token exchange, so a
+			// missing challenge degrades to no-PKCE without anyone noticing.
+			httpError(writer, NewHTTPError(http.StatusInternalServerError, "internal server error", fmt.Errorf("%w: %q", errInvalidPKCEMethod, a.cfg.PKCE.Method)))
+			return
 		}
 	}
 
diff --git a/hscontrol/types/config.go b/hscontrol/types/config.go
index a09b1ebb..fb720169 100644
--- a/hscontrol/types/config.go
+++ b/hscontrol/types/config.go
@@ -558,7 +558,11 @@ func validateServerConfig() error {
 	// Removed: oidc.expiry -> node.expiry
 	depr.fatalIfSet("oidc.expiry", "node.expiry")
 
-	if viper.GetBool("oidc.enabled") {
+	// OIDC is activated by setting oidc.issuer (see app.go), not by a
+	// dedicated oidc.enabled key. Gate PKCE method validation on the real
+	// activation condition so an invalid method fails at startup instead of
+	// silently disabling PKCE at runtime.
+	if viper.GetString("oidc.issuer") != "" {
 		err := validatePKCEMethod(viper.GetString("oidc.pkce.method"))
 		if err != nil {
 			return err
diff --git a/hscontrol/types/config_test.go b/hscontrol/types/config_test.go
index 07756b55..8d7b286d 100644
--- a/hscontrol/types/config_test.go
+++ b/hscontrol/types/config_test.go
@@ -412,6 +412,38 @@ tls_letsencrypt_challenge_type: TLS-ALPN-01
 	require.NoError(t, err)
 }
 
+func TestPKCEMethodValidation(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// OIDC is active (issuer set) with PKCE enabled and an invalid method.
+	// There is no oidc.enabled key. validateServerConfig must reject the
+	// invalid method rather than silently skipping the check.
+	configYaml := []byte(`---
+noise:
+  private_key_path: noise_private.key
+server_url: http://127.0.0.1:8080
+dns:
+  override_local_dns: false
+oidc:
+  issuer: https://idp.example.com
+  client_id: headscale
+  pkce:
+    enabled: true
+    method: S256-typo
+`)
+
+	configFilePath := filepath.Join(tmpDir, "config.yaml")
+	err := os.WriteFile(configFilePath, configYaml, 0o600)
+	require.NoError(t, err)
+
+	err = LoadConfig(tmpDir, false)
+	require.NoError(t, err)
+
+	err = validateServerConfig()
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), errInvalidPKCEMethod.Error())
+}
+
 // OK
 // server_url: headscale.com, base: clients.headscale.com
 // server_url: headscale.com, base: headscale.net