CP-21164: select Conjur JWT or legacy username/password authenticator

Adds NewRequestAuthenticator, choosing between the new Conjur JWT
exchange and the legacy CyberArk Identity username/password login based
on which config is present — Conjur JWT takes priority when both are
set. Conjur JWT requires service_id and resolves its base URL from the
secrets_manager service discovered in the prior PR, not
identity_administration; the two are different hosts. The identity API
is only required on the username/password path now; Conjur JWT never
uses it. Since service discovery already errors on a missing
identity_administration API before selectAuthenticator ever runs, the
username/password branch no longer re-checks it — that check could
never actually fire.

Switches keyfetch's client over to the new authenticator selection
instead of constructing a username/password identity client directly.
Doing so dropped keyfetch's own per-fetch LoginUsernamePassword call, which
was the only thing keeping the cached identity token from aging out —
identity.Client.AuthenticateRequest never refreshed on its own. Give it
the same self-refreshing behavior conjur.Client already has: it now
re-logs-in internally once its cached token passes tokenTTL (a field,
not a const, so tests can shrink it), using a durable copy of the
credentials it captured at the last LoginUsernamePassword call. A
refresh that fails falls back to whatever's cached rather than failing
the request outright, since the next call will retry.

LoginUsernamePassword no longer zeroes the caller's password slice: it
already keeps its own durable copy (needed for the self-refresh above),
so wiping the caller's copy bought nothing and actively broke callers
that reuse one ClientConfig across multiple logins — cfg.Secret is a
[]byte shared across every upload cycle via NewCyberArk's configLoader
closure, so the second call received an already-zeroed password and
failed to authenticate. Reproduced live before fixing: a second
PostDataReadingsWithOptions call on the username/password path failed
with "Authentication ... has failed" every time.

Hoists the jwt_source validation and the "file"/"conjur" literals this
and the agent config layer both encode separately into shared
JWTSourceFile/DefaultAccount consts and a ValidateJWTSource helper, and
drops the "POC" wording from the operator-facing error.

Author: rzisholz

internal/cyberark/auth_select_test.go

@@ -0,0 +1,109 @@
+package cyberark_test
+
+import (
+	"net/http"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"k8s.io/klog/v2"
+	"k8s.io/klog/v2/ktesting"
+
+	"github.com/jetstack/preflight/internal/cyberark"
+	"github.com/jetstack/preflight/internal/cyberark/conjur"
+	"github.com/jetstack/preflight/internal/cyberark/dataupload"
+	"github.com/jetstack/preflight/internal/cyberark/identity"
+	"github.com/jetstack/preflight/internal/cyberark/servicediscovery"
+
+	_ "k8s.io/klog/v2/ktesting/init"
+)
+
+// The agent supports two coexisting auth methods (the product is GA). These
+// tests pin the selection rule in NewDatauploadClient / selectAuthenticator:
+//   - ServiceID set                → Conjur JWT exchange
+//   - else Username+Secret present → legacy username/password
+//   - both set                     → Conjur wins
+//   - neither                      → ErrNoAuthMethod
+func TestNewDatauploadClient_AuthMethodSelection(t *testing.T) {
+	logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
+	ctx := klog.NewContext(t.Context(), logger)
+
+	const conjurToken = "success-token" // matches dataupload mock's expected bearer token
+
+	writeJWT := func(t *testing.T) string {
+		t.Helper()
+		f, err := os.CreateTemp(t.TempDir(), "jwt-*")
+		require.NoError(t, err)
+		_, err = f.WriteString("fake-service-account-jwt")
+		require.NoError(t, err)
+		require.NoError(t, f.Close())
+		return f.Name()
+	}
+
+	// stack builds a service map whose DiscoveryContext points at a dataupload
+	// mock (which requires Authorization: Bearer success-token). The Identity
+	// and SecretsManager endpoints are supplied separately and deliberately
+	// differ: the username/password path must use Identity and the Conjur
+	// authn-jwt exchange must use SecretsManager, so pointing a mock at only
+	// one of them proves which endpoint the code actually called.
+	stack := func(t *testing.T, identityAPI, smsAPI string) *servicediscovery.Services {
+		t.Helper()
+		discoveryContextAPI, _ := dataupload.MockDataUploadServer(t)
+		return &servicediscovery.Services{
+			Identity:         servicediscovery.ServiceEndpoint{API: identityAPI},
+			DiscoveryContext: servicediscovery.ServiceEndpoint{API: discoveryContextAPI},
+			SecretsManager:   servicediscovery.ServiceEndpoint{API: smsAPI},
+		}
+	}
+
+	// Endpoints that must never be dialled by the path under test.
+	const unusedIdentity = "https://identity.example.invalid"
+	const unusedSMS = "https://secretsmgr.example.invalid"
+
+	t.Run("serviceID set -> conjur path", func(t *testing.T) {
+		conjurSrv, _ := conjur.MockConjurExchangeServer(t, conjurToken)
+		t.Cleanup(conjurSrv.Close)
+
+		cfg := cyberark.ClientConfig{
+			ServiceID:   "dev-cluster",
+			JWTFilePath: writeJWT(t),
+		}
+		_, err := cyberark.NewDatauploadClient(ctx, conjurSrv.Client(), stack(t, unusedIdentity, conjurSrv.URL), "tenant", cfg)
+		require.NoError(t, err)
+	})
+
+	t.Run("username/password only -> identity path", func(t *testing.T) {
+		identityURL, httpClient := identity.MockIdentityServer(t)
+
+		cfg := cyberark.ClientConfig{
+			Subdomain: "tenant-sub",
+			Username:  identity.MockSuccessUser,
+			Secret:    []byte(identity.MockSuccessPassword),
+		}
+		// Login happens during construction; success proves the UP path ran.
+		_, err := cyberark.NewDatauploadClient(ctx, httpClient, stack(t, identityURL, unusedSMS), "tenant", cfg)
+		require.NoError(t, err)
+	})
+
+	t.Run("both set -> conjur wins", func(t *testing.T) {
+		conjurSrv, _ := conjur.MockConjurExchangeServer(t, conjurToken)
+		t.Cleanup(conjurSrv.Close)
+
+		cfg := cyberark.ClientConfig{
+			ServiceID:   "dev-cluster",
+			JWTFilePath: writeJWT(t),
+			// UP creds present too — must be ignored. Deliberately bogus so that
+			// if the identity path were taken, login would fail.
+			Username: "should-not-be-used@example.com",
+			Secret:   []byte("wrong-password"),
+		}
+		_, err := cyberark.NewDatauploadClient(ctx, conjurSrv.Client(), stack(t, unusedIdentity, conjurSrv.URL), "tenant", cfg)
+		require.NoError(t, err) // conjur path used; bogus UP creds never exercised
+	})
+
+	t.Run("neither set -> ErrNoAuthMethod", func(t *testing.T) {
+		cfg := cyberark.ClientConfig{Subdomain: "tenant-sub"}
+		_, err := cyberark.NewDatauploadClient(ctx, &http.Client{}, stack(t, unusedIdentity, unusedSMS), "tenant", cfg)
+		require.ErrorIs(t, err, cyberark.ErrNoAuthMethod)
+	})
+}

internal/cyberark/client.go

@@ -3,69 +3,167 @@ package cyberark
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net/http"
 	"os"
 
+	"k8s.io/klog/v2"
+
+	"github.com/jetstack/preflight/internal/cyberark/conjur"
 	"github.com/jetstack/preflight/internal/cyberark/dataupload"
 	"github.com/jetstack/preflight/internal/cyberark/identity"
+	"github.com/jetstack/preflight/internal/cyberark/jwtsource"
 	"github.com/jetstack/preflight/internal/cyberark/servicediscovery"
 )
 
+const (
+	// JWTSourceFile is the only currently-supported JWTSource value (besides
+	// the empty string, which also means "file").
+	JWTSourceFile = "file"
+
+	// DefaultAccount is the Conjur account name used when ClientConfig.Account
+	// is unset.
+	DefaultAccount = "conjur"
+)
+
+// ValidateJWTSource returns an error if source is set to something other than
+// the empty string or JWTSourceFile — the only supported jwt_source values.
+// Shared so the CLI/config-file validation path and the client construction
+// path can't drift on what's accepted.
+func ValidateJWTSource(source string) error {
+	if source != "" && source != JWTSourceFile {
+		return fmt.Errorf("%q is not supported; supported values are \"\" and %q", source, JWTSourceFile)
+	}
+	return nil
+}
+
 // ClientConfig holds the configuration needed to initialize a CyberArk client.
+//
+// Two authentication methods coexist (the product is GA; existing installs use
+// username/password). The active method is selected by config presence, see
+// selectAuthenticator: a Conjur authn-jwt ServiceID, when set, takes precedence
+// over username/password.
 type ClientConfig struct {
 	Subdomain string
-	Username  string
-	Secret    string
+
+	// Conjur JWT exchange (preferred for new installs).
+	ServiceID   string // authn-jwt service id (POC: per-cluster, e.g. "dev-cluster")
+	Account     string // defaults to DefaultAccount
+	JWTSource   string // "" or JWTSourceFile (POC) | "spiffe" (deferred)
+	JWTFilePath string // default jwtsource.DefaultTokenPath
+
+	// Legacy CyberArk Identity username/password (backward compatibility).
+	// Sourced from ARK_USERNAME / ARK_SECRET. Used only when ServiceID is unset.
+	Username string
+	Secret   []byte
 }
 
 // ClientConfigLoader is a function type that loads and returns a ClientConfig.
 type ClientConfigLoader func() (ClientConfig, error)
 
 // ErrMissingEnvironmentVariables is returned when required environment variables are not set.
-var ErrMissingEnvironmentVariables = errors.New("missing environment variables: ARK_SUBDOMAIN, ARK_USERNAME, ARK_SECRET")
+var ErrMissingEnvironmentVariables = errors.New("missing environment variables: ARK_SUBDOMAIN")
+
+// ErrNoAuthMethod is returned when neither a Conjur service-id nor
+// username/password credentials are configured.
+var ErrNoAuthMethod = errors.New("no CyberArk authentication method configured: set config.cyberark.service_id (Conjur JWT) or ARK_USERNAME + ARK_SECRET (legacy username/password)")
 
 // LoadClientConfigFromEnvironment loads the CyberArk client configuration from environment variables.
-// It expects the following environment variables to be set:
-// - ARK_SUBDOMAIN: The CyberArk subdomain to use.
-// - ARK_USERNAME: The username for authentication.
-// - ARK_SECRET: The secret for authentication.
+// It expects the following environment variable to be set:
+//   - ARK_SUBDOMAIN: The CyberArk subdomain to use (required).
+//
+// It also reads the optional legacy username/password credentials:
+//   - ARK_USERNAME, ARK_SECRET: used only when no Conjur service-id is configured.
+//
+// Behavioral keys (ServiceID, Account, JWTSource, JWTFilePath) are set by the
+// caller from the agent YAML config (config.cyberark.*).
 func LoadClientConfigFromEnvironment() (ClientConfig, error) {
 	subdomain := os.Getenv("ARK_SUBDOMAIN")
-	username := os.Getenv("ARK_USERNAME")
-	secret := os.Getenv("ARK_SECRET")
-
-	if subdomain == "" || username == "" || secret == "" {
+	if subdomain == "" {
 		return ClientConfig{}, ErrMissingEnvironmentVariables
 	}
-
-	return ClientConfig{
+	cfg := ClientConfig{
 		Subdomain: subdomain,
-		Username:  username,
-		Secret:    secret,
-	}, nil
+		Username:  os.Getenv("ARK_USERNAME"),
+	}
+	if secret := os.Getenv("ARK_SECRET"); secret != "" {
+		cfg.Secret = []byte(secret)
+	}
+	return cfg, nil
+}
+
+// selectAuthenticator builds the request authenticator for the configured auth
+// method and returns it together with the discovery-context API endpoint.
+//
+// Selection (backward compatible — the product is GA):
+//   - ServiceID set                  → Conjur JWT exchange (preferred).
+//   - else Username+Secret present   → legacy CyberArk Identity UP login.
+//   - neither                        → ErrNoAuthMethod.
+//
+// When both are configured, ServiceID wins (a migrating install can set the
+// service-id without first removing its old credentials) and a warning is logged.
+func selectAuthenticator(ctx context.Context, httpClient *http.Client, serviceMap *servicediscovery.Services, cfg ClientConfig) (identity.RequestAuthenticator, error) {
+	hasConjur := cfg.ServiceID != ""
+	hasUP := cfg.Username != "" && len(cfg.Secret) > 0
+
+	switch {
+	case hasConjur:
+		if hasUP {
+			klog.FromContext(ctx).Info("both Conjur service_id and ARK_USERNAME/ARK_SECRET are set; using the Conjur JWT exchange and ignoring the username/password credentials")
+		}
+		if err := ValidateJWTSource(cfg.JWTSource); err != nil {
+			return nil, fmt.Errorf("jwt_source %w", err)
+		}
+		account := cfg.Account
+		if account == "" {
+			account = DefaultAccount
+		}
+		// The authn-jwt exchange is served by Secrets Manager (Conjur Cloud),
+		// not by identity_administration — those are different hosts. Tenant
+		// onboarding registers the authenticator on the Secrets Manager host,
+		// and the server that later validates the resulting token resolves the
+		// same service from service discovery.
+		smsAPI := serviceMap.SecretsManager.API
+		if smsAPI == "" {
+			return nil, errors.New("service discovery returned an empty secrets_manager API, which is required for the Conjur JWT exchange")
+		}
+		src := jwtsource.NewFileSource(cfg.JWTFilePath)
+		conjurClient := conjur.New(httpClient, smsAPI, cfg.ServiceID, account, src)
+		return conjurClient.AuthenticateRequest, nil
+
+	case hasUP:
+		identityClient := identity.New(httpClient, serviceMap.Identity.API, cfg.Subdomain)
+		if err := identityClient.LoginUsernamePassword(ctx, cfg.Username, cfg.Secret); err != nil {
+			return nil, fmt.Errorf("CyberArk Identity username/password login failed: %w", err)
+		}
+		return identityClient.AuthenticateRequest, nil
+
+	default:
+		return nil, ErrNoAuthMethod
+	}
+}
 
+// NewRequestAuthenticator selects and builds the configured request
+// authenticator (Conjur JWT exchange or legacy username/password). Exposed for
+// other consumers (e.g. envelope key fetching) that need the same auth seam
+// without a dataupload client.
+func NewRequestAuthenticator(ctx context.Context, httpClient *http.Client, serviceMap *servicediscovery.Services, cfg ClientConfig) (identity.RequestAuthenticator, error) {
+	return selectAuthenticator(ctx, httpClient, serviceMap, cfg)
 }
 
 // NewDatauploadClient initializes and returns a new CyberArk Data Upload client.
-// It performs service discovery to find the necessary API endpoints and authenticates
-// using the provided client configuration.
+// It performs service discovery to find the necessary API endpoints and
+// authenticates using whichever method is configured (Conjur JWT exchange or
+// legacy username/password — see selectAuthenticator).
 func NewDatauploadClient(ctx context.Context, httpClient *http.Client, serviceMap *servicediscovery.Services, tenantUUID string, cfg ClientConfig) (*dataupload.CyberArkClient, error) {
-	identityAPI := serviceMap.Identity.API
-	if identityAPI == "" {
-		return nil, errors.New("service discovery returned an empty identity API")
-	}
-
 	discoveryAPI := serviceMap.DiscoveryContext.API
 	if discoveryAPI == "" {
 		return nil, errors.New("service discovery returned an empty discovery API")
 	}
 
-	identityClient := identity.New(httpClient, identityAPI, cfg.Subdomain)
-
-	err := identityClient.LoginUsernamePassword(ctx, cfg.Username, []byte(cfg.Secret))
+	authenticate, err := selectAuthenticator(ctx, httpClient, serviceMap, cfg)
 	if err != nil {
 		return nil, err
 	}
-
-	return dataupload.New(httpClient, discoveryAPI, tenantUUID, identityClient.AuthenticateRequest), nil
+	return dataupload.New(httpClient, discoveryAPI, tenantUUID, authenticate), nil
 }