reuse the same code for secrets in remote client and token exchange client (#2167)

* reuse the same code for secrets in remote client and token exchange client

Closes: #2166

* use constant

---------

Co-authored-by: taskbot <[email protected]>

Author: 2 people

cmd/thv/app/auth_flags.go

@@ -14,23 +14,54 @@ import (
 	"github.com/stacklok/toolhive/pkg/runner"
 )
 
+const (
+	// #nosec G101 - this is an environment variable name, not a credential
+	envTokenExchangeClientSecret = "TOOLHIVE_TOKEN_EXCHANGE_CLIENT_SECRET"
+)
+
 // readSecretFromFile reads a secret from a file, cleaning the path and trimming whitespace
-func readSecretFromFile(filePath, secretType string) (string, error) {
+func readSecretFromFile(filePath string) (string, error) {
 	// Clean the file path to prevent path traversal
 	cleanPath := filepath.Clean(filePath)
-	logger.Debugf("Reading %s from file: %s", secretType, cleanPath)
+	logger.Debugf("Reading secret from file: %s", cleanPath)
 	// #nosec G304 - file path is cleaned above
 	secretBytes, err := os.ReadFile(cleanPath)
 	if err != nil {
-		return "", fmt.Errorf("failed to read %s file %s: %w", secretType, cleanPath, err)
+		return "", fmt.Errorf("failed to read secret file %s: %w", cleanPath, err)
 	}
 	secret := strings.TrimSpace(string(secretBytes))
 	if secret == "" {
-		return "", fmt.Errorf("%s file %s is empty", secretType, cleanPath)
+		return "", fmt.Errorf("secret file %s is empty", cleanPath)
 	}
 	return secret, nil
 }
 
+// resolveSecret resolves a secret from multiple sources following a standard priority order.
+// Priority: 1. Flag value, 2. File, 3. Environment variable
+// Returns empty string (not an error) if no secret is found - this is acceptable for public client/PKCE flows.
+func resolveSecret(flagValue, filePath, envVarName string) (string, error) {
+	// 1. Check if provided directly via flag
+	if flagValue != "" {
+		logger.Debug("Using secret from command-line flag")
+		return flagValue, nil
+	}
+
+	// 2. Check if provided via file
+	if filePath != "" {
+		return readSecretFromFile(filePath)
+	}
+
+	// 3. Check environment variable
+	if secret := os.Getenv(envVarName); secret != "" {
+		logger.Debugf("Using secret from %s environment variable", envVarName)
+		return secret, nil
+	}
+
+	// No secret found - this is acceptable for PKCE flows
+	logger.Debug("No secret provided - using public client mode")
+	return "", nil
+}
+
 // RemoteAuthFlags holds the common remote authentication configuration
 type RemoteAuthFlags struct {
 	EnableRemoteAuth           bool
@@ -65,21 +96,14 @@ func (f *RemoteAuthFlags) BuildTokenExchangeConfig() (*tokenexchange.Config, err
 		return nil, nil
 	}
 
-	// Resolve token exchange client secret from flag or file only
-	// Environment variable is handled by the middleware for Kubernetes deployments
-	var clientSecret string
-	if f.TokenExchangeClientSecret != "" {
-		clientSecret = f.TokenExchangeClientSecret
-		logger.Debug("Using token exchange client secret from command-line flag")
-	} else if f.TokenExchangeClientSecretFile != "" {
-		var err error
-		clientSecret, err = readSecretFromFile(f.TokenExchangeClientSecretFile, "token exchange client secret")
-		if err != nil {
-			return nil, err
-		}
-	} else {
-		// Empty client secret is acceptable for public client mode
-		logger.Debug("No token exchange client secret provided - using public client mode")
+	// Resolve token exchange client secret using the same mechanism as remote-auth-client-secret
+	clientSecret, err := resolveSecret(
+		f.TokenExchangeClientSecret,
+		f.TokenExchangeClientSecretFile,
+		envTokenExchangeClientSecret,
+	)
+	if err != nil {
+		return nil, err
 	}
 
 	// Determine header strategy based on whether custom header name is provided

cmd/thv/app/proxy.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
-	"os"
 	"os/signal"
 	"syscall"
 	"time"
@@ -353,26 +352,11 @@ func handleOutgoingAuthentication(ctx context.Context) (*discovery.OAuthFlowResu
 // resolveClientSecret resolves the OAuth client secret from multiple sources
 // Priority: 1. Flag value, 2. File, 3. Environment variable
 func resolveClientSecret() (string, error) {
-	// 1. Check if provided directly via flag
-	if remoteAuthFlags.RemoteAuthClientSecret != "" {
-		logger.Debug("Using client secret from command-line flag")
-		return remoteAuthFlags.RemoteAuthClientSecret, nil
-	}
-
-	// 2. Check if provided via file
-	if remoteAuthFlags.RemoteAuthClientSecretFile != "" {
-		return readSecretFromFile(remoteAuthFlags.RemoteAuthClientSecretFile, "client secret")
-	}
-
-	// 3. Check environment variable
-	if secret := os.Getenv(envOAuthClientSecret); secret != "" {
-		logger.Debugf("Using client secret from %s environment variable", envOAuthClientSecret)
-		return secret, nil
-	}
-
-	// No client secret found - this is acceptable for PKCE flows
-	logger.Debug("No client secret provided - using PKCE flow")
-	return "", nil
+	return resolveSecret(
+		remoteAuthFlags.RemoteAuthClientSecret,
+		remoteAuthFlags.RemoteAuthClientSecretFile,
+		envOAuthClientSecret,
+	)
 }
 
 // createTokenInjectionMiddleware creates a middleware that injects the OAuth token into requests