oidc, types: validate pkce.method when OIDC is active

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
 		}
 	}
 

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

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