diff --git a/internal/cyberark/auth_select_test.go b/internal/cyberark/auth_select_test.go new file mode 100644 index 00000000..b4e50457 --- /dev/null +++ b/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) + }) +} diff --git a/internal/cyberark/client.go b/internal/cyberark/client.go index 92710296..d8a8a50f 100644 --- a/internal/cyberark/client.go +++ b/internal/cyberark/client.go @@ -3,69 +3,151 @@ 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" ) // 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 // POC: "conjur" + JWTSource string // "file" (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 } -// 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. -func NewDatauploadClient(ctx context.Context, httpClient *http.Client, serviceMap *servicediscovery.Services, tenantUUID string, cfg ClientConfig) (*dataupload.CyberArkClient, error) { +// 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) { identityAPI := serviceMap.Identity.API if identityAPI == "" { return nil, errors.New("service discovery returned an empty identity API") } + 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 cfg.JWTSource != "" && cfg.JWTSource != "file" { + return nil, fmt.Errorf("jwt_source %q not supported in POC (only 'file')", cfg.JWTSource) + } + account := cfg.Account + if account == "" { + account = "conjur" + } + // 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, identityAPI, 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 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) { 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 } diff --git a/internal/cyberark/client_test.go b/internal/cyberark/client_test.go index 9d69da20..9b64d543 100644 --- a/internal/cyberark/client_test.go +++ b/internal/cyberark/client_test.go @@ -1,21 +1,21 @@ package cyberark_test import ( - "crypto/x509" + "crypto/tls" + "net/http" "os" "strings" "testing" - "github.com/jetstack/venafi-connection-lib/http_client" "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/servicediscovery" arktesting "github.com/jetstack/preflight/internal/cyberark/testing" - "github.com/jetstack/preflight/pkg/testutil" "github.com/jetstack/preflight/pkg/version" _ "k8s.io/klog/v2/ktesting/init" @@ -26,12 +26,39 @@ func TestCyberArkClient_PutSnapshot_MockAPI(t *testing.T) { logger := ktesting.NewLogger(t, ktesting.DefaultConfig) ctx := klog.NewContext(t.Context(), logger) - httpClient := testutil.FakeCyberArk(t) + const conjurToken = "success-token" // matches dataupload mock's expected bearer token + + jwtFile, err := os.CreateTemp(t.TempDir(), "jwt-*") + require.NoError(t, err) + _, err = jwtFile.WriteString("fake-service-account-jwt") + require.NoError(t, err) + require.NoError(t, jwtFile.Close()) + + conjurSrv, _ := conjur.MockConjurExchangeServer(t, conjurToken) + t.Cleanup(conjurSrv.Close) + + discoveryContextAPI, _ := dataupload.MockDataUploadServer(t) + + // Unused by the Conjur path, but service discovery requires it to be set. + const identitySrv = "https://identity.example.invalid" + + httpClient := servicediscovery.MockDiscoveryServer(t, servicediscovery.Services{ + Identity: servicediscovery.ServiceEndpoint{ + API: identitySrv, + }, + DiscoveryContext: servicediscovery.ServiceEndpoint{ + API: discoveryContextAPI, + }, + // The authn-jwt exchange lives on secrets_manager, not identity. + SecretsManager: servicediscovery.ServiceEndpoint{ + API: conjurSrv.URL, + }, + }) cfg := cyberark.ClientConfig{ - Subdomain: servicediscovery.MockDiscoverySubdomain, - Username: "test@example.com", - Secret: "somepassword", + Subdomain: servicediscovery.MockDiscoverySubdomain, + ServiceID: "dev-cluster", + JWTFilePath: jwtFile.Name(), } discoveryClient := servicediscovery.New(httpClient, cfg.Subdomain) @@ -52,9 +79,63 @@ func TestCyberArkClient_PutSnapshot_MockAPI(t *testing.T) { require.NoError(t, err) } +// TestNewDatauploadClient_UsesConjurExchanger asserts that NewDatauploadClient wires +// the conjur exchange as the dataupload RequestAuthenticator. It builds its own +// mock stack so that the Bearer token path can be verified end-to-end. +func TestNewDatauploadClient_UsesConjurExchanger(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 + + // Write a temp JWT file — NewFileSource reads it during AuthenticateRequest. + jwtFile, err := os.CreateTemp(t.TempDir(), "jwt-*") + require.NoError(t, err) + _, err = jwtFile.WriteString("fake-service-account-jwt") + require.NoError(t, err) + require.NoError(t, jwtFile.Close()) + + // Stand up a conjur exchange mock that validates the JWT and returns the token. + conjurSrv, _ := conjur.MockConjurExchangeServer(t, conjurToken) + defer conjurSrv.Close() + + // Stand up a dataupload mock. It expects Authorization: Bearer success-token. + // The returned httpClient trusts the TLS cert of the dataupload mock server; + // it can also reach plain-HTTP servers (the conjur mock) without issue. + discoveryContextAPI, httpClient := dataupload.MockDataUploadServer(t) + + serviceMap := &servicediscovery.Services{ + Identity: servicediscovery.ServiceEndpoint{ + // Unused by the Conjur path, but service discovery requires it. + API: "https://identity.example.invalid", + }, + DiscoveryContext: servicediscovery.ServiceEndpoint{ + API: discoveryContextAPI, + }, + SecretsManager: servicediscovery.ServiceEndpoint{ + API: conjurSrv.URL, // conjur authn-jwt exchange endpoint base + }, + } + + cfg := cyberark.ClientConfig{ + JWTFilePath: jwtFile.Name(), + ServiceID: "dev-cluster", + // Account defaults to "conjur" + } + + cl, err := cyberark.NewDatauploadClient(ctx, httpClient, serviceMap, "tenant-uuid-1234", cfg) + require.NoError(t, err) + + err = cl.PutSnapshot(ctx, dataupload.Snapshot{ + ClusterID: "ffffffff-ffff-ffff-ffff-ffffffffffff", + AgentVersion: version.PreflightVersion, + }) + require.NoError(t, err) +} + // TestCyberArkClient_PutSnapshot_RealAPI demonstrates that NewDatauploadClient works with the real inventory API. // -// An API token is obtained by authenticating with the ARK_USERNAME and ARK_SECRET from the environment. +// An API token is obtained by authenticating with the conjur JWT exchange from the environment. // ARK_SUBDOMAIN should be your tenant subdomain. // // To test against a tenant on the integration platform, also set: @@ -77,8 +158,14 @@ func TestCyberArkClient_PutSnapshot_RealAPI(t *testing.T) { logger := ktesting.NewLogger(t, ktesting.DefaultConfig) ctx := klog.NewContext(t.Context(), logger) - var rootCAs *x509.CertPool - httpClient := http_client.NewDefaultClient(version.UserAgent(), rootCAs) + // Use a plain http.Client for real API calls; a proper user-agent transport would + // normally be wired here but the venafi-connection-lib import is avoided to keep + // this package buildable without private-module credentials in developer environments. + httpClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{}, + }, + } cfg, err := cyberark.LoadClientConfigFromEnvironment() require.NoError(t, err) diff --git a/internal/cyberark/conjur/conjur.go b/internal/cyberark/conjur/conjur.go new file mode 100644 index 00000000..aef218ef --- /dev/null +++ b/internal/cyberark/conjur/conjur.go @@ -0,0 +1,161 @@ +package conjur + +import ( + "context" + "encoding/base64" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "sync" + "time" + + "k8s.io/klog/v2" + + "github.com/jetstack/preflight/internal/cyberark/jwtsource" +) + +const tokenTTL = 8 * time.Minute + +// Client exchanges a JWT for a Conjur access token and authenticates requests with it. +type Client struct { + httpClient *http.Client + baseURL string + serviceID string + account string + src jwtsource.Source + + mu sync.Mutex + token string + identity string + tokenTime time.Time +} + +func New(httpClient *http.Client, baseURL, serviceID, account string, src jwtsource.Source) *Client { + return &Client{httpClient: httpClient, baseURL: baseURL, serviceID: serviceID, account: account, src: src} +} + +func (c *Client) exchange(ctx context.Context) (string, error) { + jwt, err := c.src.Read(ctx) + if err != nil { + return "", err + } + endpoint, err := url.JoinPath(c.baseURL, "authn-jwt", c.serviceID, c.account, "authenticate") + if err != nil { + return "", err + } + form := url.Values{"jwt": {jwt}} + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(form.Encode())) + if err != nil { + return "", err + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + // Request the base64-encoded access token — Conjur's canonical wire form + // for the token, and the encoding this client's own decoding below + // expects. + req.Header.Set("Accept-Encoding", "base64") + resp, err := c.httpClient.Do(req) + if err != nil { + return "", fmt.Errorf("authn-jwt exchange transport error: %w", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + // 401 here most often means the SA token audience != authenticator audience=conjur + return "", fmt.Errorf("authn-jwt exchange rejected (%d): verify service_id, the authenticator is enabled, and the SA token audience is 'conjur'", resp.StatusCode) + } + body, err := io.ReadAll(io.LimitReader(resp.Body, 64*1024)) + if err != nil { + return "", err + } + return strings.TrimSpace(string(body)), nil +} + +// padBase64 adds the '=' padding base64.StdEncoding/URLEncoding require, +// for inputs that arrived without it. +func padBase64(s string) string { + return s + strings.Repeat("=", (4-len(s)%4)%4) +} + +// flattenedJWSJSON is the wire shape of a Conjur access token: a Flattened +// JWS JSON Serialization object, optionally base64-encoded on top (Conjur's +// `Accept-Encoding: base64`, which this client requests). +type flattenedJWSJSON struct { + Protected string `json:"protected"` + Payload string `json:"payload"` + Signature string `json:"signature"` +} + +// conjurTokenObject parses a Conjur access token into its Flattened-JWS-JSON +// object, tolerating the token being raw JSON, standard base64, or +// url-safe base64 (Conjur may return any of these depending on encoding). +func conjurTokenObject(token string) (*flattenedJWSJSON, bool) { + candidates := []string{token} + padded := padBase64(token) + if decoded, err := base64.StdEncoding.DecodeString(padded); err == nil { + candidates = append(candidates, string(decoded)) + } + if decoded, err := base64.URLEncoding.DecodeString(padded); err == nil { + candidates = append(candidates, string(decoded)) + } + for _, candidate := range candidates { + var obj flattenedJWSJSON + if err := json.Unmarshal([]byte(candidate), &obj); err != nil { + continue + } + if obj.Protected != "" && obj.Payload != "" && obj.Signature != "" { + return &obj, true + } + } + return nil, false +} + +// identityFromToken extracts the `sub` claim from a Conjur access token's +// payload. The payload segment is url-safe base64 without padding. Returns +// ("", false) if the token doesn't parse or has no `sub` claim. +func identityFromToken(token string) (string, bool) { + obj, ok := conjurTokenObject(token) + if !ok { + return "", false + } + payloadJSON, err := base64.URLEncoding.DecodeString(padBase64(obj.Payload)) + if err != nil { + return "", false + } + var payload struct { + Sub string `json:"sub"` + } + if err := json.Unmarshal(payloadJSON, &payload); err != nil { + return "", false + } + if payload.Sub == "" { + return "", false + } + return payload.Sub, true +} + +// AuthenticateRequest implements identity.RequestAuthenticator. +// It exchanges the JWT for a Conjur access token, sets the Authorization +// header, and returns an identity string for audit tagging. The identity is +// the token's own `sub` claim when it can be extracted; otherwise it falls +// back to the configured service ID so a token in an unexpected shape never +// fails the request. +func (c *Client) AuthenticateRequest(req *http.Request) (string, error) { + c.mu.Lock() + defer c.mu.Unlock() + if c.token == "" || time.Since(c.tokenTime) >= tokenTTL { + tok, err := c.exchange(req.Context()) + if err != nil { + return "", err + } + identity, ok := identityFromToken(tok) + if !ok { + klog.FromContext(req.Context()).V(2).Info("could not extract sub claim from Conjur access token; falling back to service ID as identity") + identity = c.serviceID + } + c.token, c.identity, c.tokenTime = tok, identity, time.Now() + } + req.Header.Set("Authorization", "Bearer "+c.token) + return c.identity, nil +} diff --git a/internal/cyberark/conjur/conjur_test.go b/internal/cyberark/conjur/conjur_test.go new file mode 100644 index 00000000..9684308f --- /dev/null +++ b/internal/cyberark/conjur/conjur_test.go @@ -0,0 +1,116 @@ +package conjur + +import ( + "context" + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" +) + +type staticSource struct{ tok string } + +func (s staticSource) Read(context.Context) (string, error) { return s.tok, nil } + +// mockConjurExchangeServerCountingExchanges is like MockConjurExchangeServer but +// also counts how many times the exchange endpoint was hit, to verify +// token/identity caching doesn't re-exchange on every AuthenticateRequest call. +func mockConjurExchangeServerCountingExchanges(t testing.TB, token string, count *int) *httptest.Server { + t.Helper() + return httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.FormValue("jwt") == "" { + w.WriteHeader(http.StatusBadRequest) + return + } + *count++ + _, _ = w.Write([]byte(token)) + })) +} + +// buildJWSToken builds a base64-encoded Flattened-JWS-JSON token (Conjur's +// wire form) with the given `sub` claim, for test use only. +func buildJWSToken(t testing.TB, sub string) string { + t.Helper() + payload, err := json.Marshal(map[string]string{"sub": sub}) + require.NoError(t, err) + obj := map[string]string{ + "protected": base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString([]byte(`{"alg":"conjur.v2"}`)), + "payload": base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(payload), + "signature": "sig", + } + raw, err := json.Marshal(obj) + require.NoError(t, err) + return base64.StdEncoding.EncodeToString(raw) +} + +func TestAuthenticateRequest_ExchangesAndSetsBearer(t *testing.T) { + srv, httpClient := MockConjurExchangeServer(t, "conjur-access-token") + defer srv.Close() + + c := New(httpClient, srv.URL, "dev-cluster", "conjur", staticSource{tok: "the-jwt"}) + req, _ := http.NewRequest(http.MethodGet, "https://example.com/snapshot-links", nil) + _, err := c.AuthenticateRequest(req) + require.NoError(t, err) + require.Equal(t, `Bearer conjur-access-token`, req.Header.Get("Authorization")) +} + +func TestAuthenticateRequest_ExchangeFailsClosed(t *testing.T) { + srv, httpClient := MockConjurExchangeServerStatus(t, http.StatusUnauthorized) + defer srv.Close() + c := New(httpClient, srv.URL, "dev-cluster", "conjur", staticSource{tok: "the-jwt"}) + req, _ := http.NewRequest(http.MethodGet, "https://example.com/x", nil) + _, err := c.AuthenticateRequest(req) + require.Error(t, err) + require.Empty(t, req.Header.Get("Authorization")) +} + +func TestAuthenticateRequest_ReturnsSubClaimFromToken(t *testing.T) { + const sub = "host/data/k8s/test-cluster-uuid/workloads/system:serviceaccount:test:test-agent" + token := buildJWSToken(t, sub) + srv, httpClient := MockConjurExchangeServer(t, token) + defer srv.Close() + + c := New(httpClient, srv.URL, "dev-cluster", "conjur", staticSource{tok: "the-jwt"}) + req, _ := http.NewRequest(http.MethodGet, "https://example.com/snapshot-links", nil) + identity, err := c.AuthenticateRequest(req) + require.NoError(t, err) + require.Equal(t, sub, identity) + require.Equal(t, "Bearer "+token, req.Header.Get("Authorization")) +} + +func TestAuthenticateRequest_OpaqueTokenFallsBackToServiceID(t *testing.T) { + // Opaque placeholder tokens (as used elsewhere in this repo's tests) are + // not JWS-JSON; extraction must fail gracefully, not error the request. + srv, httpClient := MockConjurExchangeServer(t, "success-token") + defer srv.Close() + + c := New(httpClient, srv.URL, "dev-cluster", "conjur", staticSource{tok: "the-jwt"}) + req, _ := http.NewRequest(http.MethodGet, "https://example.com/snapshot-links", nil) + identity, err := c.AuthenticateRequest(req) + require.NoError(t, err) + require.Equal(t, "dev-cluster", identity) +} + +func TestAuthenticateRequest_CachesIdentityWithToken(t *testing.T) { + const sub = "host/data/k8s/test-cluster-uuid/workloads/system:serviceaccount:test:test-agent" + token := buildJWSToken(t, sub) + var exchanges int + srv := mockConjurExchangeServerCountingExchanges(t, token, &exchanges) + defer srv.Close() + + c := New(srv.Client(), srv.URL, "dev-cluster", "conjur", staticSource{tok: "the-jwt"}) + req1, _ := http.NewRequest(http.MethodGet, "https://example.com/a", nil) + identity1, err := c.AuthenticateRequest(req1) + require.NoError(t, err) + + req2, _ := http.NewRequest(http.MethodGet, "https://example.com/b", nil) + identity2, err := c.AuthenticateRequest(req2) + require.NoError(t, err) + + require.Equal(t, sub, identity1) + require.Equal(t, identity1, identity2) + require.Equal(t, 1, exchanges, "expected only one exchange for two AuthenticateRequest calls within the token TTL") +} diff --git a/internal/cyberark/conjur/mock.go b/internal/cyberark/conjur/mock.go new file mode 100644 index 00000000..c1984108 --- /dev/null +++ b/internal/cyberark/conjur/mock.go @@ -0,0 +1,28 @@ +package conjur + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// MockConjurExchangeServer returns a TLS server whose authn-jwt endpoint returns the given token. +func MockConjurExchangeServer(t testing.TB, token string) (*httptest.Server, *http.Client) { + t.Helper() + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.FormValue("jwt") == "" { + w.WriteHeader(http.StatusBadRequest) + return + } + _, _ = w.Write([]byte(token)) + })) + return srv, srv.Client() +} + +func MockConjurExchangeServerStatus(t testing.TB, status int) (*httptest.Server, *http.Client) { + t.Helper() + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(status) + })) + return srv, srv.Client() +} diff --git a/internal/cyberark/identity/identity.go b/internal/cyberark/identity/identity.go index c245c978..66838ca1 100644 --- a/internal/cyberark/identity/identity.go +++ b/internal/cyberark/identity/identity.go @@ -1,180 +1,13 @@ package identity import ( - "bytes" - "context" - "encoding/json" - "fmt" - "io" "net/http" - "net/url" "sync" "time" - - "k8s.io/klog/v2" - - arkapi "github.com/jetstack/preflight/internal/cyberark/api" - "github.com/jetstack/preflight/pkg/logs" - "github.com/jetstack/preflight/pkg/version" -) - -const ( - // MechanismUsernamePassword is the string which identifies the username/password mechanism for completing - // a login attempt - MechanismUsernamePassword = "UP" - - // ActionAnswer is the string which is sent to an AdvanceAuthentication request to indicate we're providing - // the credentials in band in text format (i.e., we're sending a password) - ActionAnswer = "Answer" - - // SummaryLoginSuccess is returned by a StartAuthentication to indicate that login does not need - // to proceed to the AdvanceAuthentication step. - // We don't handle this because we don't expect it to happen. - SummaryLoginSuccess = "LoginSuccess" - - // SummaryNewPackage is returned by a StartAuthentication call when the user must complete a challenge - // to complete the log in. This is expected on a first login. - SummaryNewPackage = "NewPackage" - - // maxStartAuthenticationBodySize is the maximum allowed size for a response body from the CyberArk Identity - // StartAuthentication endpoint. - // As of 2025-04-30, a response from the integration environment is ~1kB - maxStartAuthenticationBodySize = 10 * 1024 - - // maxAdvanceAuthenticationBodySize is the maximum allowed size for a response body from the CyberArk Identity - // AdvanceAuthentication endpoint. - // As of 2025-04-30, a response from the integration environment is ~3kB - maxAdvanceAuthenticationBodySize = 30 * 1024 -) - -var ( - errNoUPMechanism = fmt.Errorf("found no authentication mechanism with the username + password type (%s); unable to complete login using this identity", MechanismUsernamePassword) ) -// startAuthenticationRequestBody is the body sent to the StartAuthentication endpoint in CyberArk Identity; -// see https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/start-authentication -type startAuthenticationRequestBody struct { - // TenantID is the internal ID of the tenant containing the user attempting to log in. In testing, - // it seems that the subdomain works in this field. - TenantID string `json:"TenantId"` - - // Version is set to 1.0 - Version string `json:"Version"` - - // User is the username of the user trying to log in. For a human, this is likely to be an email address. - User string `json:"User"` -} - -// identityResponseBody generically wraps a response from the Identity server; the Result will differ for -// responses from different endpoint, but the other fields are similar. -// Not all fields in the JSON returned from the server are replicated here, since we only need a subset. -type identityResponseBody[T any] struct { - // Success is a simple boolean indicator from the server of success. - // NB: The JSON key is lowercase, in contrast to other JSON keys in the response. - Success bool `json:"success"` - - // Result holds the information we need to parse from successful responses - Result T `json:"Result"` - - // Message holds an information message such as an error message. Experimentally it seems to be null - // for successful attempts. - Message string `json:"Message"` - - // ErrorID holds an error ID when something goes wrong with the call. - // Not to be confused with ErrorCode; for failure messages, we see ErrorID set and ErrorCode null. - ErrorID string `json:"ErrorID"` - - // NB: Other fields omitted since we don't need them -} - -// startAuthenticationResponseBody is the response returned by the server from a request to StartAuthentication. -type startAuthenticationResponseBody identityResponseBody[startAuthenticationResponseResult] - -// advanceAuthenticationResponseBody is the response from the AdvanceAuthentication endpoint. -type advanceAuthenticationResponseBody identityResponseBody[advanceAuthenticationResponseResult] - -// startAuthenticationResponseResult holds the important data we need to pass to AdvanceAuthentication -type startAuthenticationResponseResult struct { - // SessionID identifies this login attempt, and must be passed with the - // follow-up AdvanceAuthentication request. - SessionID string `json:"SessionId"` - - // Challenges provides a list of methods for logging in. We need to look - // for the correct login method we want to use, and then find the MechanismId - // for that login method to pass to the AdvanceAuthentication request. - Challenges []startAuthenticationChallenge `json:"Challenges"` - - // Summary indicates whether a StartAuthentication calls needs to be followed up with an AdvanceAuthentication - // call. From the docs: - // > If the user exists, the response contains a Summary of either LoginSuccess or NewPackage. - // > You receive LoginSuccess when the request includes an .ASPXAUTH cookie from prior successful authentication. - Summary string `json:"Summary"` -} - -// startAuthenticationChallenge is an entry in the array of MFA mechanisms; -// at least one MFA mechanism should be satisfied by the user. -type startAuthenticationChallenge struct { - Mechanisms []startAuthenticationMechanism `json:"Mechanisms"` -} - -// startAuthenticationMechanism holds details of a given mechanism for authenticating. -// This corresponds to "how" the user authenticates, e.g. via password or email, etc -type startAuthenticationMechanism struct { - // Name represents the name of the challenge mechanism. This is usually an upper-case - // string, such as "UP" for "username / password" - Name string `json:"Name"` - - // Enrolled is true if the given mechanism is available for the user attempting - // to authenticate. - Enrolled bool `json:"Enrolled"` - - // MechanismID uniquely identifies a particular mechanism, and must be passed - // to the AdvanceAuthentication request when authenticating. - MechanismID string `json:"MechanismId"` -} - -// advanceAuthenticationRequestBody is a request body for the AdvanceAuthentication call to CyberArk Identity, -// which should usually be obtained by making requests to StartAuthentication first. -// WARNING: This struct can hold secret data (a user's password) -// See: https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/advance-authentication -type advanceAuthenticationRequestBody struct { - // Action is a string identifying how we're intending to log in; for username/password, this is - // set to "Answer" to indicate that the password is held in the Answer field - Action string `json:"Action"` - - // Answer holds the user's password to send to the server - // WARNING: THIS IS SECRET DATA. - Answer string `json:"Answer"` - - // MechanismID identifies the login mechanism and must be retrieved from a call to StartAuthentication - MechanismID string `json:"MechanismId"` - - // SessionID identifies the login session and must be retrieved from a call to StartAuthentication - SessionID string `json:"SessionId"` - - // TenantID identifies the tenant; this can be inferred from the URL if we used service discovery to - // get the Identity API URL, but we set it anyway to be explicit. - TenantID string `json:"TenantId"` - - // PersistentLogin is documented to "[indicate] whether the session should persist after the user - // closes the browser"; for service-to-service auth which we're trying to do, we set this to true. - PersistentLogin bool `json:"PersistentLogin"` -} - -// advanceAuthenticationResponseResult is the specific information returned for a successful AdvanceAuthentication call -type advanceAuthenticationResponseResult struct { - // Summary holds a "brief summary of the authentication outcome" - Summary string `json:"Summary"` - - // Token is the auth token we need to save; this is the result of the login - // process which can be sent as a bearer token to other services. - Token string `json:"Token"` - - // Other fields omitted as they're not needed -} - -// Client is an client for interacting with the CyberArk Identity API and performing a login using a username and password. -// For context on the behaviour of this client, see the Python SDK: https://github.com/cyberark/ark-sdk-python/blob/3be12c3f2d3a2d0407025028943e584b6edc5996/ark_sdk_python/auth/identity/ark_identity.py +// Client is a client for interacting with the CyberArk Identity API. +// It caches an authentication token and exposes it for use by AuthenticateRequest. type Client struct { httpClient *http.Client baseURL string @@ -191,7 +24,7 @@ type token struct { Token string } -// New returns an initialized CyberArk Identity client using a default service discovery client. +// New returns an initialized CyberArk Identity client. func New(httpClient *http.Client, baseURL string, subdomain string) *Client { return &Client{ httpClient: httpClient, @@ -202,246 +35,3 @@ func New(httpClient *http.Client, baseURL string, subdomain string) *Client { tokenCachedMutex: sync.Mutex{}, } } - -// LoginUsernamePassword performs a blocking call to fetch an auth token from CyberArk Identity using the given username and password. -// The password is zeroed after use. -// Tokens are cached internally and are not directly accessible to code; use Client.AuthenticateRequest to add credentials -// to an *http.Request. -func (c *Client) LoginUsernamePassword(ctx context.Context, username string, password []byte) error { - // note: we hold the mutex for the whole login attempt to ensure that only one login attempt can be in flight at once, - // and to ensure that the token cache is correctly updated - c.tokenCachedMutex.Lock() - defer c.tokenCachedMutex.Unlock() - - defer func() { - for i := range password { - password[i] = 0x00 - } - }() - - if time.Since(c.tokenCachedTime) < 15*time.Minute && c.tokenCached.Username == username { - // If the cached token is recent and for the same username, we can reuse it. - klog.FromContext(ctx).V(2).Info("reusing cached token for user", "username", username) - return nil - } - - advanceRequestBody, err := c.doStartAuthentication(ctx, username) - if err != nil { - return err - } - - // NB: We explicitly pass advanceRequestBody by value here so that when we add the password - // in doAdvanceAuthentication we don't create a copy of the password slice elsewhere. - err = c.doAdvanceAuthentication(ctx, username, &password, advanceRequestBody) - if err != nil { - return err - } - - return err -} - -// doStartAuthentication performs the initial request to start the login process using a username and password. -// It returns a partially initialized advanceAuthenticationRequestBody ready to send to the server to complete -// the login. As this function doesn't have access to the password, it must be added to the returned request body -// by the caller before being used as a request to AdvanceAuthentication. -// See https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/start-authentication -func (c *Client) doStartAuthentication(ctx context.Context, username string) (advanceAuthenticationRequestBody, error) { - response := advanceAuthenticationRequestBody{} - - logger := klog.FromContext(ctx).WithValues("source", "Identity.doStartAuthentication") - - body := startAuthenticationRequestBody{ - Version: "1.0", // this is the only value in the docs - - TenantID: c.subdomain, - - User: username, - } - - bodyJSON, err := json.Marshal(body) - if err != nil { - return response, fmt.Errorf("failed to marshal JSON for request to StartAuthentication endpoint: %s", err) - } - - endpoint, err := url.JoinPath(c.baseURL, "Security", "StartAuthentication") - if err != nil { - return response, fmt.Errorf("failed to create URL for request to CyberArk Identity StartAuthentication: %s", err) - } - - request, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(bodyJSON)) - if err != nil { - return response, fmt.Errorf("failed to initialise request to Identity endpoint %s: %s", endpoint, err) - } - - setIdentityHeaders(request) - - httpResponse, err := c.httpClient.Do(request) - if err != nil { - return response, fmt.Errorf("failed to perform HTTP request to start authentication: %s", err) - } - - defer httpResponse.Body.Close() - - if httpResponse.StatusCode != http.StatusOK { - err := fmt.Errorf("got unexpected status code %s from request to start authentication in CyberArk Identity API", httpResponse.Status) - if httpResponse.StatusCode >= 500 || httpResponse.StatusCode < 400 { - return response, err - } - - // If we got a 4xx error, we shouldn't retry - return response, err - } - - startAuthResponse := startAuthenticationResponseBody{} - - err = json.NewDecoder(io.LimitReader(httpResponse.Body, maxStartAuthenticationBodySize)).Decode(&startAuthResponse) - if err != nil { - if err == io.ErrUnexpectedEOF { - return response, fmt.Errorf("rejecting JSON response from server as it was too large or was truncated") - } - - return response, fmt.Errorf("failed to parse JSON from otherwise successful request to start authentication: %s", err) - } - - if !startAuthResponse.Success { - return response, fmt.Errorf("got a failure response from request to start authentication: message=%q, error=%q", startAuthResponse.Message, startAuthResponse.ErrorID) - } - - logger.V(logs.Debug).Info("made successful request to StartAuthentication", "summary", startAuthResponse.Result.Summary) - - if startAuthResponse.Result.Summary != SummaryNewPackage { - // This means we can't respond to whatever summary the server sent. - // The best thing to do is try and find a challenge we can solve anyway. - klog.FromContext(ctx).Info("got an unexpected Summary from StartAuthentication response; will attempt to complete a login challenge anyway", "summary", startAuthResponse.Result.Summary) - } - - // We can only handle a UP type challenge, and if there are any other challenges, we'll have to fail because we can't handle them. - // https://github.com/cyberark/ark-sdk-python/blob/3be12c3f2d3a2d0407025028943e584b6edc5996/ark_sdk_python/auth/identity/ark_identity.py#L405 - switch len(startAuthResponse.Result.Challenges) { - case 0: - return response, fmt.Errorf("got no valid challenges in response to start authentication; unable to log in") - - case 1: - // do nothing, this is ideal - - default: - return response, fmt.Errorf("got %d challenges in response to start authentication, which means MFA may be enabled; unable to log in", len(startAuthResponse.Result.Challenges)) - } - - challenge := startAuthResponse.Result.Challenges[0] - - switch len(challenge.Mechanisms) { - case 0: - // presumably this shouldn't happen, but handle the case anyway - return response, fmt.Errorf("got no mechanisms for challenge from Identity server") - - case 1: - // do nothing, this is ideal - - default: - return response, fmt.Errorf("got %d mechanisms in response to start authentication, which means MFA may be enabled; unable to log in", len(challenge.Mechanisms)) - } - - mechanism := challenge.Mechanisms[0] - - if !mechanism.Enrolled || mechanism.Name != MechanismUsernamePassword { - return response, errNoUPMechanism - } - - response.Action = ActionAnswer - response.MechanismID = mechanism.MechanismID - response.SessionID = startAuthResponse.Result.SessionID - response.TenantID = c.subdomain - response.PersistentLogin = true - - return response, nil -} - -// doAdvanceAuthentication performs the second step of the login process, sending the password to the server -// and receiving a token in response. -// See: https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/advance-authentication -func (c *Client) doAdvanceAuthentication(ctx context.Context, username string, password *[]byte, requestBody advanceAuthenticationRequestBody) error { - if password == nil { - return fmt.Errorf("password must not be nil; this is a programming error") - } - - requestBody.Answer = string(*password) - - bodyJSON, err := json.Marshal(requestBody) - if err != nil { - return fmt.Errorf("failed to marshal JSON for request to AdvanceAuthentication endpoint: %s", err) - } - - endpoint, err := url.JoinPath(c.baseURL, "Security", "AdvanceAuthentication") - if err != nil { - return fmt.Errorf("failed to create URL for request to CyberArk Identity AdvanceAuthentication: %s", err) - } - - request, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(bodyJSON)) - if err != nil { - return fmt.Errorf("failed to initialise request to Identity endpoint %s: %s", endpoint, err) - } - - setIdentityHeaders(request) - - httpResponse, err := c.httpClient.Do(request) - if err != nil { - return fmt.Errorf("failed to perform HTTP request to advance authentication: %s", err) - } - - defer httpResponse.Body.Close() - - // Important: Even login failures can produce a 200 status code, so this - // check won't catch all failures - if httpResponse.StatusCode != http.StatusOK { - return fmt.Errorf("got unexpected status code %s from request to advance authentication in CyberArk Identity API", httpResponse.Status) - } - - advanceAuthResponse := advanceAuthenticationResponseBody{} - - err = json.NewDecoder(io.LimitReader(httpResponse.Body, maxAdvanceAuthenticationBodySize)).Decode(&advanceAuthResponse) - if err != nil { - if err == io.ErrUnexpectedEOF { - return fmt.Errorf("rejecting JSON response from server as it was too large or was truncated") - } - - return fmt.Errorf("failed to parse JSON from otherwise successful request to advance authentication: %s", err) - } - - if !advanceAuthResponse.Success { - return fmt.Errorf("got a failure response from request to advance authentication: message=%q, error=%q", advanceAuthResponse.Message, advanceAuthResponse.ErrorID) - } - - if advanceAuthResponse.Result.Summary != SummaryLoginSuccess { - // IF MFA was enabled and we got here, there's probably nothing to be gained from a retry - // and the best thing to do is fail now so the user can fix MFA settings. - return fmt.Errorf("got a %s response from AdvanceAuthentication; this implies that the user account %s requires MFA, which is not supported. Try unlocking MFA for this user", advanceAuthResponse.Result.Summary, username) - } - - klog.FromContext(ctx).Info("successfully completed AdvanceAuthentication request to CyberArk Identity; login complete", "username", username) - - // NB: This assumes we already hold the token cache mutex, which we do in LoginUsernamePassword, so this is safe. - c.tokenCachedTime = time.Now() - c.tokenCached = token{ - Username: username, - Token: advanceAuthResponse.Result.Token, - } - - return nil -} - -// setIdentityHeaders sets the headers required for requests to the CyberArk Identity API. -// From the docs: -// Your request header must contain X-IDAP-NATIVE-CLIENT:true to indicate that an application is invoking -// the CyberArk Identity endpoint, and -// Content-Type: application/json to indicate that the body is in JSON format. -// Experimentally, it seems the X-IDAP-NATIVE-CLIENT is not required but we'll follow the docs. -func setIdentityHeaders(r *http.Request) { - // The "canonicalheader" linter warns us that the IDAP-NATIVE-CLIENT header isn't canonical, but we silence it here - // since we want to exactly match the docs. - r.Header.Set("Content-Type", "application/json") - r.Header.Set("X-IDAP-NATIVE-CLIENT", "true") //nolint: canonicalheader - version.SetUserAgent(r) - // Add telemetry headers - arkapi.SetTelemetryRequestHeader(r) -} diff --git a/internal/cyberark/identity/mock.go b/internal/cyberark/identity/mock.go index 2bad8b36..ef87f4cc 100644 --- a/internal/cyberark/identity/mock.go +++ b/internal/cyberark/identity/mock.go @@ -32,6 +32,18 @@ const ( // mock server in response to a successful AdvanceAuthentication request // Must match what's in testdata/advance_authentication_success.json mockSuccessfulStartAuthenticationToken = "success-token" + + // actionAnswer is the string sent to an AdvanceAuthentication request to indicate we're + // providing credentials as plain text. + actionAnswer = "Answer" +) + +// Exported credentials that MockIdentityServer accepts as a successful +// username/password login. Used by other packages' tests that exercise the +// legacy UP auth path against the mock server. +const ( + MockSuccessUser = successUser + MockSuccessPassword = successPassword ) var ( @@ -213,7 +225,7 @@ func (mis *mockIdentityServer) handleAdvanceAuthentication(w http.ResponseWriter if advanceBody.SessionID != successSessionID || advanceBody.MechanismID != successMechanismID || - advanceBody.Action != ActionAnswer || + advanceBody.Action != actionAnswer || advanceBody.Answer != successPassword { w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(advanceAuthenticationFailureResponse)) diff --git a/internal/cyberark/identity/username_password.go b/internal/cyberark/identity/username_password.go new file mode 100644 index 00000000..71ed7d97 --- /dev/null +++ b/internal/cyberark/identity/username_password.go @@ -0,0 +1,424 @@ +package identity + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "time" + + "k8s.io/klog/v2" + + arkapi "github.com/jetstack/preflight/internal/cyberark/api" + "github.com/jetstack/preflight/pkg/logs" + "github.com/jetstack/preflight/pkg/version" +) + +// This file holds the legacy CyberArk Identity username/password (UP) login. +// It is retained alongside the Conjur JWT exchange for backward compatibility: +// the product is GA and existing installs authenticate with ARK_USERNAME/ +// ARK_SECRET. The agent selects UP vs Conjur by config presence (see +// internal/cyberark/client.go) — a Conjur service-id, when set, takes +// precedence. Both paths populate the same token cache and satisfy +// RequestAuthenticator via AuthenticateRequest. + +const ( + // MechanismUsernamePassword is the string which identifies the username/password mechanism for completing + // a login attempt + MechanismUsernamePassword = "UP" + + // ActionAnswer is the string which is sent to an AdvanceAuthentication request to indicate we're providing + // the credentials in band in text format (i.e., we're sending a password) + ActionAnswer = "Answer" + + // SummaryLoginSuccess is returned by a StartAuthentication to indicate that login does not need + // to proceed to the AdvanceAuthentication step. + // We don't handle this because we don't expect it to happen. + SummaryLoginSuccess = "LoginSuccess" + + // SummaryNewPackage is returned by a StartAuthentication call when the user must complete a challenge + // to complete the log in. This is expected on a first login. + SummaryNewPackage = "NewPackage" + + // maxStartAuthenticationBodySize is the maximum allowed size for a response body from the CyberArk Identity + // StartAuthentication endpoint. + // As of 2025-04-30, a response from the integration environment is ~1kB + maxStartAuthenticationBodySize = 10 * 1024 + + // maxAdvanceAuthenticationBodySize is the maximum allowed size for a response body from the CyberArk Identity + // AdvanceAuthentication endpoint. + // As of 2025-04-30, a response from the integration environment is ~3kB + maxAdvanceAuthenticationBodySize = 30 * 1024 +) + +var ( + errNoUPMechanism = fmt.Errorf("found no authentication mechanism with the username + password type (%s); unable to complete login using this identity", MechanismUsernamePassword) +) + +// startAuthenticationRequestBody is the body sent to the StartAuthentication endpoint in CyberArk Identity; +// see https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/start-authentication +type startAuthenticationRequestBody struct { + // TenantID is the internal ID of the tenant containing the user attempting to log in. In testing, + // it seems that the subdomain works in this field. + TenantID string `json:"TenantId"` + + // Version is set to 1.0 + Version string `json:"Version"` + + // User is the username of the user trying to log in. For a human, this is likely to be an email address. + User string `json:"User"` +} + +// identityResponseBody generically wraps a response from the Identity server; the Result will differ for +// responses from different endpoint, but the other fields are similar. +// Not all fields in the JSON returned from the server are replicated here, since we only need a subset. +type identityResponseBody[T any] struct { + // Success is a simple boolean indicator from the server of success. + // NB: The JSON key is lowercase, in contrast to other JSON keys in the response. + Success bool `json:"success"` + + // Result holds the information we need to parse from successful responses + Result T `json:"Result"` + + // Message holds an information message such as an error message. Experimentally it seems to be null + // for successful attempts. + Message string `json:"Message"` + + // ErrorID holds an error ID when something goes wrong with the call. + // Not to be confused with ErrorCode; for failure messages, we see ErrorID set and ErrorCode null. + ErrorID string `json:"ErrorID"` + + // NB: Other fields omitted since we don't need them +} + +// startAuthenticationResponseBody is the response returned by the server from a request to StartAuthentication. +type startAuthenticationResponseBody identityResponseBody[startAuthenticationResponseResult] + +// advanceAuthenticationResponseBody is the response from the AdvanceAuthentication endpoint. +type advanceAuthenticationResponseBody identityResponseBody[advanceAuthenticationResponseResult] + +// startAuthenticationResponseResult holds the important data we need to pass to AdvanceAuthentication +type startAuthenticationResponseResult struct { + // SessionID identifies this login attempt, and must be passed with the + // follow-up AdvanceAuthentication request. + SessionID string `json:"SessionId"` + + // Challenges provides a list of methods for logging in. We need to look + // for the correct login method we want to use, and then find the MechanismId + // for that login method to pass to the AdvanceAuthentication request. + Challenges []startAuthenticationChallenge `json:"Challenges"` + + // Summary indicates whether a StartAuthentication calls needs to be followed up with an AdvanceAuthentication + // call. From the docs: + // > If the user exists, the response contains a Summary of either LoginSuccess or NewPackage. + // > You receive LoginSuccess when the request includes an .ASPXAUTH cookie from prior successful authentication. + Summary string `json:"Summary"` +} + +// startAuthenticationChallenge is an entry in the array of MFA mechanisms; +// at least one MFA mechanism should be satisfied by the user. +type startAuthenticationChallenge struct { + Mechanisms []startAuthenticationMechanism `json:"Mechanisms"` +} + +// startAuthenticationMechanism holds details of a given mechanism for authenticating. +// This corresponds to "how" the user authenticates, e.g. via password or email, etc +type startAuthenticationMechanism struct { + // Name represents the name of the challenge mechanism. This is usually an upper-case + // string, such as "UP" for "username / password" + Name string `json:"Name"` + + // Enrolled is true if the given mechanism is available for the user attempting + // to authenticate. + Enrolled bool `json:"Enrolled"` + + // MechanismID uniquely identifies a particular mechanism, and must be passed + // to the AdvanceAuthentication request when authenticating. + MechanismID string `json:"MechanismId"` +} + +// advanceAuthenticationRequestBody is a request body for the AdvanceAuthentication call to CyberArk Identity, +// which should usually be obtained by making requests to StartAuthentication first. +// WARNING: This struct can hold secret data (a user's password) +// See: https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/advance-authentication +type advanceAuthenticationRequestBody struct { + // Action is a string identifying how we're intending to log in; for username/password, this is + // set to "Answer" to indicate that the password is held in the Answer field + Action string `json:"Action"` + + // Answer holds the user's password to send to the server + // WARNING: THIS IS SECRET DATA. + Answer string `json:"Answer"` + + // MechanismID identifies the login mechanism and must be retrieved from a call to StartAuthentication + MechanismID string `json:"MechanismId"` + + // SessionID identifies the login session and must be retrieved from a call to StartAuthentication + SessionID string `json:"SessionId"` + + // TenantID identifies the tenant; this can be inferred from the URL if we used service discovery to + // get the Identity API URL, but we set it anyway to be explicit. + TenantID string `json:"TenantId"` + + // PersistentLogin is documented to "[indicate] whether the session should persist after the user + // closes the browser"; for service-to-service auth which we're trying to do, we set this to true. + PersistentLogin bool `json:"PersistentLogin"` +} + +// advanceAuthenticationResponseResult is the specific information returned for a successful AdvanceAuthentication call +type advanceAuthenticationResponseResult struct { + // Summary holds a "brief summary of the authentication outcome" + Summary string `json:"Summary"` + + // Token is the auth token we need to save; this is the result of the login + // process which can be sent as a bearer token to other services. + Token string `json:"Token"` + + // Other fields omitted as they're not needed +} + +// LoginUsernamePassword performs a blocking call to fetch an auth token from CyberArk Identity using the given username and password. +// The password is zeroed after use. +// Tokens are cached internally and are not directly accessible to code; use Client.AuthenticateRequest to add credentials +// to an *http.Request. +func (c *Client) LoginUsernamePassword(ctx context.Context, username string, password []byte) error { + // note: we hold the mutex for the whole login attempt to ensure that only one login attempt can be in flight at once, + // and to ensure that the token cache is correctly updated + c.tokenCachedMutex.Lock() + defer c.tokenCachedMutex.Unlock() + + defer func() { + for i := range password { + password[i] = 0x00 + } + }() + + if time.Since(c.tokenCachedTime) < 15*time.Minute && c.tokenCached.Username == username { + // If the cached token is recent and for the same username, we can reuse it. + klog.FromContext(ctx).V(2).Info("reusing cached token for user", "username", username) + return nil + } + + advanceRequestBody, err := c.doStartAuthentication(ctx, username) + if err != nil { + return err + } + + // NB: We explicitly pass advanceRequestBody by value here so that when we add the password + // in doAdvanceAuthentication we don't create a copy of the password slice elsewhere. + err = c.doAdvanceAuthentication(ctx, username, &password, advanceRequestBody) + if err != nil { + return err + } + + return err +} + +// doStartAuthentication performs the initial request to start the login process using a username and password. +// It returns a partially initialized advanceAuthenticationRequestBody ready to send to the server to complete +// the login. As this function doesn't have access to the password, it must be added to the returned request body +// by the caller before being used as a request to AdvanceAuthentication. +// See https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/start-authentication +func (c *Client) doStartAuthentication(ctx context.Context, username string) (advanceAuthenticationRequestBody, error) { + response := advanceAuthenticationRequestBody{} + + logger := klog.FromContext(ctx).WithValues("source", "Identity.doStartAuthentication") + + body := startAuthenticationRequestBody{ + Version: "1.0", // this is the only value in the docs + + TenantID: c.subdomain, + + User: username, + } + + bodyJSON, err := json.Marshal(body) + if err != nil { + return response, fmt.Errorf("failed to marshal JSON for request to StartAuthentication endpoint: %s", err) + } + + endpoint, err := url.JoinPath(c.baseURL, "Security", "StartAuthentication") + if err != nil { + return response, fmt.Errorf("failed to create URL for request to CyberArk Identity StartAuthentication: %s", err) + } + + request, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(bodyJSON)) + if err != nil { + return response, fmt.Errorf("failed to initialise request to Identity endpoint %s: %s", endpoint, err) + } + + setIdentityHeaders(request) + + httpResponse, err := c.httpClient.Do(request) + if err != nil { + return response, fmt.Errorf("failed to perform HTTP request to start authentication: %s", err) + } + + defer httpResponse.Body.Close() + + if httpResponse.StatusCode != http.StatusOK { + err := fmt.Errorf("got unexpected status code %s from request to start authentication in CyberArk Identity API", httpResponse.Status) + if httpResponse.StatusCode >= 500 || httpResponse.StatusCode < 400 { + return response, err + } + + // If we got a 4xx error, we shouldn't retry + return response, err + } + + startAuthResponse := startAuthenticationResponseBody{} + + err = json.NewDecoder(io.LimitReader(httpResponse.Body, maxStartAuthenticationBodySize)).Decode(&startAuthResponse) + if err != nil { + if err == io.ErrUnexpectedEOF { + return response, fmt.Errorf("rejecting JSON response from server as it was too large or was truncated") + } + + return response, fmt.Errorf("failed to parse JSON from otherwise successful request to start authentication: %s", err) + } + + if !startAuthResponse.Success { + return response, fmt.Errorf("got a failure response from request to start authentication: message=%q, error=%q", startAuthResponse.Message, startAuthResponse.ErrorID) + } + + logger.V(logs.Debug).Info("made successful request to StartAuthentication", "summary", startAuthResponse.Result.Summary) + + if startAuthResponse.Result.Summary != SummaryNewPackage { + // This means we can't respond to whatever summary the server sent. + // The best thing to do is try and find a challenge we can solve anyway. + klog.FromContext(ctx).Info("got an unexpected Summary from StartAuthentication response; will attempt to complete a login challenge anyway", "summary", startAuthResponse.Result.Summary) + } + + // We can only handle a UP type challenge, and if there are any other challenges, we'll have to fail because we can't handle them. + // https://github.com/cyberark/ark-sdk-python/blob/3be12c3f2d3a2d0407025028943e584b6edc5996/ark_sdk_python/auth/identity/ark_identity.py#L405 + switch len(startAuthResponse.Result.Challenges) { + case 0: + return response, fmt.Errorf("got no valid challenges in response to start authentication; unable to log in") + + case 1: + // do nothing, this is ideal + + default: + return response, fmt.Errorf("got %d challenges in response to start authentication, which means MFA may be enabled; unable to log in", len(startAuthResponse.Result.Challenges)) + } + + challenge := startAuthResponse.Result.Challenges[0] + + switch len(challenge.Mechanisms) { + case 0: + // presumably this shouldn't happen, but handle the case anyway + return response, fmt.Errorf("got no mechanisms for challenge from Identity server") + + case 1: + // do nothing, this is ideal + + default: + return response, fmt.Errorf("got %d mechanisms in response to start authentication, which means MFA may be enabled; unable to log in", len(challenge.Mechanisms)) + } + + mechanism := challenge.Mechanisms[0] + + if !mechanism.Enrolled || mechanism.Name != MechanismUsernamePassword { + return response, errNoUPMechanism + } + + response.Action = ActionAnswer + response.MechanismID = mechanism.MechanismID + response.SessionID = startAuthResponse.Result.SessionID + response.TenantID = c.subdomain + response.PersistentLogin = true + + return response, nil +} + +// doAdvanceAuthentication performs the second step of the login process, sending the password to the server +// and receiving a token in response. +// See: https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/advance-authentication +func (c *Client) doAdvanceAuthentication(ctx context.Context, username string, password *[]byte, requestBody advanceAuthenticationRequestBody) error { + if password == nil { + return fmt.Errorf("password must not be nil; this is a programming error") + } + + requestBody.Answer = string(*password) + + bodyJSON, err := json.Marshal(requestBody) + if err != nil { + return fmt.Errorf("failed to marshal JSON for request to AdvanceAuthentication endpoint: %s", err) + } + + endpoint, err := url.JoinPath(c.baseURL, "Security", "AdvanceAuthentication") + if err != nil { + return fmt.Errorf("failed to create URL for request to CyberArk Identity AdvanceAuthentication: %s", err) + } + + request, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(bodyJSON)) + if err != nil { + return fmt.Errorf("failed to initialise request to Identity endpoint %s: %s", endpoint, err) + } + + setIdentityHeaders(request) + + httpResponse, err := c.httpClient.Do(request) + if err != nil { + return fmt.Errorf("failed to perform HTTP request to advance authentication: %s", err) + } + + defer httpResponse.Body.Close() + + // Important: Even login failures can produce a 200 status code, so this + // check won't catch all failures + if httpResponse.StatusCode != http.StatusOK { + return fmt.Errorf("got unexpected status code %s from request to advance authentication in CyberArk Identity API", httpResponse.Status) + } + + advanceAuthResponse := advanceAuthenticationResponseBody{} + + err = json.NewDecoder(io.LimitReader(httpResponse.Body, maxAdvanceAuthenticationBodySize)).Decode(&advanceAuthResponse) + if err != nil { + if err == io.ErrUnexpectedEOF { + return fmt.Errorf("rejecting JSON response from server as it was too large or was truncated") + } + + return fmt.Errorf("failed to parse JSON from otherwise successful request to advance authentication: %s", err) + } + + if !advanceAuthResponse.Success { + return fmt.Errorf("got a failure response from request to advance authentication: message=%q, error=%q", advanceAuthResponse.Message, advanceAuthResponse.ErrorID) + } + + if advanceAuthResponse.Result.Summary != SummaryLoginSuccess { + // IF MFA was enabled and we got here, there's probably nothing to be gained from a retry + // and the best thing to do is fail now so the user can fix MFA settings. + return fmt.Errorf("got a %s response from AdvanceAuthentication; this implies that the user account %s requires MFA, which is not supported. Try unlocking MFA for this user", advanceAuthResponse.Result.Summary, username) + } + + klog.FromContext(ctx).Info("successfully completed AdvanceAuthentication request to CyberArk Identity; login complete", "username", username) + + // NB: This assumes we already hold the token cache mutex, which we do in LoginUsernamePassword, so this is safe. + c.tokenCachedTime = time.Now() + c.tokenCached = token{ + Username: username, + Token: advanceAuthResponse.Result.Token, + } + + return nil +} + +// setIdentityHeaders sets the headers required for requests to the CyberArk Identity API. +// From the docs: +// Your request header must contain X-IDAP-NATIVE-CLIENT:true to indicate that an application is invoking +// the CyberArk Identity endpoint, and +// Content-Type: application/json to indicate that the body is in JSON format. +// Experimentally, it seems the X-IDAP-NATIVE-CLIENT is not required but we'll follow the docs. +func setIdentityHeaders(r *http.Request) { + // The "canonicalheader" linter warns us that the IDAP-NATIVE-CLIENT header isn't canonical, but we silence it here + // since we want to exactly match the docs. + r.Header.Set("Content-Type", "application/json") + r.Header.Set("X-IDAP-NATIVE-CLIENT", "true") //nolint: canonicalheader + version.SetUserAgent(r) + // Add telemetry headers + arkapi.SetTelemetryRequestHeader(r) +} diff --git a/internal/cyberark/jwtsource/jwtsource.go b/internal/cyberark/jwtsource/jwtsource.go new file mode 100644 index 00000000..513a4696 --- /dev/null +++ b/internal/cyberark/jwtsource/jwtsource.go @@ -0,0 +1,39 @@ +// internal/cyberark/jwtsource/jwtsource.go +package jwtsource + +import ( + "context" + "fmt" + "os" + "strings" +) + +// DefaultTokenPath is the default projected ServiceAccount token mount (aud=conjur). +const DefaultTokenPath = "/var/run/secrets/tokens/jwt" + +// Source produces a raw JWT to exchange at SMS authn-jwt. +type Source interface { + Read(ctx context.Context) (string, error) +} + +type fileSource struct{ path string } + +// NewFileSource reads a JWT from a file (the projected SA token). +func NewFileSource(path string) Source { + if path == "" { + path = DefaultTokenPath + } + return &fileSource{path: path} +} + +func (f *fileSource) Read(_ context.Context) (string, error) { + b, err := os.ReadFile(f.path) + if err != nil { + return "", fmt.Errorf("jwt source file %q not found or unreadable (is the projected serviceAccountToken volume mounted?): %w", f.path, err) + } + tok := strings.TrimSpace(string(b)) + if tok == "" { + return "", fmt.Errorf("jwt source file %q is empty", f.path) + } + return tok, nil +} diff --git a/internal/cyberark/jwtsource/jwtsource_test.go b/internal/cyberark/jwtsource/jwtsource_test.go new file mode 100644 index 00000000..ff23a68e --- /dev/null +++ b/internal/cyberark/jwtsource/jwtsource_test.go @@ -0,0 +1,33 @@ +// internal/cyberark/jwtsource/jwtsource_test.go +package jwtsource + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestFileSource_ReadsToken(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "jwt") + require.NoError(t, os.WriteFile(p, []byte("the-jwt\n"), 0o600)) + got, err := NewFileSource(p).Read(context.Background()) + require.NoError(t, err) + require.Equal(t, "the-jwt", got) // trimmed +} + +func TestFileSource_MissingFile(t *testing.T) { + _, err := NewFileSource("/no/such/file").Read(context.Background()) + require.Error(t, err) +} + +func TestFileSource_EmptyFile(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "jwt") + require.NoError(t, os.WriteFile(p, []byte(" \n"), 0o600)) + _, err := NewFileSource(p).Read(context.Background()) + require.Error(t, err) +} diff --git a/internal/cyberark/servicediscovery/discovery.go b/internal/cyberark/servicediscovery/discovery.go index 93598d5c..6c6ddfca 100644 --- a/internal/cyberark/servicediscovery/discovery.go +++ b/internal/cyberark/servicediscovery/discovery.go @@ -28,6 +28,13 @@ const ( // in responses from the Service Discovery API. DiscoveryContextServiceName = "discoverycontext" + // SecretsManagerServiceName is the name of the Secrets Manager (Conjur + // Cloud) API in responses from the Service Discovery API. This is the host + // that serves `authn-jwt///authenticate` — NOT the + // identity_administration host. The server that validates the resulting + // token resolves this same service name. + SecretsManagerServiceName = "secrets_manager" + // maxDiscoverBodySize is the maximum allowed size for a response body from the CyberArk Service Discovery subdomain endpoint // As of 2025-04-16, a response from the integration environment is ~4kB maxDiscoverBodySize = 2 * 1024 * 1024 @@ -101,11 +108,13 @@ type ServiceEndpoint struct { API string `json:"api"` } -// This is a convenience struct to hold the two ServiceEndpoints we care about. -// Currently, we only care about the Identity API and the Discovery Context API. +// This is a convenience struct to hold the ServiceEndpoints we care about: +// the Identity API, the Discovery Context API, and the Secrets Manager +// (Conjur Cloud) API used for the authn-jwt token exchange. type Services struct { Identity ServiceEndpoint DiscoveryContext ServiceEndpoint + SecretsManager ServiceEndpoint } // DiscoverServices fetches from the service discovery service for the configured subdomain @@ -163,7 +172,7 @@ func (c *Client) DiscoverServices(ctx context.Context) (*Services, string, error } return nil, "", fmt.Errorf("failed to parse JSON from otherwise successful request to service discovery endpoint: %s", err) } - var identityAPI, discoveryContextAPI string + var identityAPI, discoveryContextAPI, secretsManagerAPI string for _, svc := range discoveryResp.Services { switch svc.ServiceName { case IdentityServiceName: @@ -180,6 +189,13 @@ func (c *Client) DiscoverServices(ctx context.Context) (*Services, string, error break } } + case SecretsManagerServiceName: + for _, ep := range svc.Endpoints { + if ep.Type == "main" && ep.IsActive && ep.API != "" { + secretsManagerAPI = ep.API + break + } + } } } @@ -192,6 +208,7 @@ func (c *Client) DiscoverServices(ctx context.Context) (*Services, string, error services := &Services{ Identity: ServiceEndpoint{API: identityAPI}, DiscoveryContext: ServiceEndpoint{API: discoveryContextAPI}, + SecretsManager: ServiceEndpoint{API: secretsManagerAPI}, } c.cachedResponse = services diff --git a/internal/cyberark/servicediscovery/discovery_test.go b/internal/cyberark/servicediscovery/discovery_test.go index 618e63f9..23c2f1b6 100644 --- a/internal/cyberark/servicediscovery/discovery_test.go +++ b/internal/cyberark/servicediscovery/discovery_test.go @@ -62,6 +62,9 @@ func Test_DiscoverIdentityAPIURL(t *testing.T) { DiscoveryContext: ServiceEndpoint{ API: mockDiscoveryContextAPIURL, }, + SecretsManager: ServiceEndpoint{ + API: mockSecretsManagerAPIURL, + }, }) client := New(httpClient, testSpec.subdomain) @@ -76,6 +79,13 @@ func Test_DiscoverIdentityAPIURL(t *testing.T) { if services.Identity.API != testSpec.expectedURL { t.Errorf("expected API URL=%s\nobserved API URL=%s", testSpec.expectedURL, services.Identity.API) } + // The Conjur authn-jwt exchange is served by secrets_manager, not + // by identity_administration. Parsing it into the wrong field means + // every live token exchange 404s/401s, which the Conjur unit tests + // cannot catch because they point their mock at whichever field the + // code reads. + assert.Equal(t, mockSecretsManagerAPIURL, services.SecretsManager.API) + assert.NotEqual(t, services.Identity.API, services.SecretsManager.API) }) } } diff --git a/internal/cyberark/servicediscovery/mock.go b/internal/cyberark/servicediscovery/mock.go index 360c87a3..b784d8a4 100644 --- a/internal/cyberark/servicediscovery/mock.go +++ b/internal/cyberark/servicediscovery/mock.go @@ -25,6 +25,7 @@ const ( mockIdentityAPIURL = "https://ajp5871.id.integration-cyberark.cloud" mockDiscoveryContextAPIURL = "https://venafi-test.inventory.integration-cyberark.cloud/" + mockSecretsManagerAPIURL = "https://venafi-test.secretsmgr.integration-cyberark.cloud/api" prefix = "/api/public/tenant-discovery?bySubdomain=" ) diff --git a/internal/cyberark/servicediscovery/testdata/README.md b/internal/cyberark/servicediscovery/testdata/README.md index d6cf51a8..c511435c 100644 --- a/internal/cyberark/servicediscovery/testdata/README.md +++ b/internal/cyberark/servicediscovery/testdata/README.md @@ -9,6 +9,7 @@ NOTE: This API is not implemented yet as of 02.09.2025 but is expected to be fin curl -fsSL "${ARK_DISCOVERY_API}?bySubdomain=${ARK_SUBDOMAIN}" | jq ``` -Then replace `identity_administration.api` with `{{ .Identity.API }}` and -`discoverycontext.api` with `{{ .DiscoveryContext.API }}`. Those Go template +Then replace `identity_administration.api` with `{{ .Identity.API }}`, +`discoverycontext.api` with `{{ .DiscoveryContext.API }}`, and +`secrets_manager.api` with `{{ .SecretsManager.API }}`. Those Go template fields will be substituted in the tests. diff --git a/internal/cyberark/servicediscovery/testdata/discovery_success.json.template b/internal/cyberark/servicediscovery/testdata/discovery_success.json.template index ee04b067..2f50efaa 100644 --- a/internal/cyberark/servicediscovery/testdata/discovery_success.json.template +++ b/internal/cyberark/servicediscovery/testdata/discovery_success.json.template @@ -31,7 +31,7 @@ "is_active": true, "type": "main", "ui": "https://ui.test-conjur.cloud", - "api": "https://venafi-test.secretsmgr.integration-cyberark.cloud/api" + "api": "{{ .SecretsManager.API }}" } ] }, diff --git a/internal/cyberark/testing/testing.go b/internal/cyberark/testing/testing.go index 7b74abcc..7bc17aaf 100644 --- a/internal/cyberark/testing/testing.go +++ b/internal/cyberark/testing/testing.go @@ -9,10 +9,7 @@ import ( func SkipIfNoEnv(t testing.TB) { t.Helper() - if os.Getenv("ARK_SUBDOMAIN") == "" || - os.Getenv("ARK_USERNAME") == "" || - os.Getenv("ARK_SECRET") == "" { - t.Skip("Skipping test because one of ARK_SUBDOMAIN, ARK_USERNAME or ARK_SECRET isn't set") + if os.Getenv("ARK_SUBDOMAIN") == "" { + t.Skip("Skipping test because ARK_SUBDOMAIN isn't set") } - } diff --git a/internal/envelope/keyfetch/client.go b/internal/envelope/keyfetch/client.go index 53a8d5b9..a7dc2ed8 100644 --- a/internal/envelope/keyfetch/client.go +++ b/internal/envelope/keyfetch/client.go @@ -50,8 +50,7 @@ type PublicKey struct { // and ignored other types. type Client struct { discoveryClient *servicediscovery.Client - identityClient *identity.Client - cfg cyberark.ClientConfig + authenticate identity.RequestAuthenticator // httpClient is the HTTP client used for requests httpClient *http.Client @@ -62,8 +61,9 @@ type Client struct { } // NewClient creates a new key fetching client. -// Uses CyberArk service discovery to derive the JWKS endpoint and CyberArk identity client for authentication. -// Constructing the client involves a service discovery call to initialise the identity client, +// Uses CyberArk service discovery to derive the JWKS endpoint and the configured +// CyberArk authentication method (Conjur JWT exchange or legacy username/password). +// Constructing the client involves a service discovery call to initialise the authenticator, // so this may return an error if the discovery client is not able to connect to the service discovery endpoint. // If httpClient is nil, a default HTTP client will be created. func NewClient(ctx context.Context, discoveryClient *servicediscovery.Client, cfg cyberark.ClientConfig, httpClient *http.Client) (*Client, error) { @@ -77,10 +77,14 @@ func NewClient(ctx context.Context, discoveryClient *servicediscovery.Client, cf return nil, fmt.Errorf("failed to get services from discovery client for initialising identity client: %w", err) } + authenticate, err := cyberark.NewRequestAuthenticator(ctx, httpClient, services, cfg) + if err != nil { + return nil, err + } + return &Client{ discoveryClient: discoveryClient, - identityClient: identity.New(httpClient, services.Identity.API, cfg.Subdomain), - cfg: cfg, + authenticate: authenticate, httpClient: httpClient, }, nil } @@ -102,11 +106,6 @@ func (c *Client) FetchKey(ctx context.Context) (PublicKey, error) { return PublicKey{}, fmt.Errorf("failed to get services from discovery client: %w", err) } - err = c.identityClient.LoginUsernamePassword(ctx, c.cfg.Username, []byte(c.cfg.Secret)) - if err != nil { - return PublicKey{}, fmt.Errorf("failed to authenticate for fetching JWKs: %w", err) - } - endpoint, err := url.JoinPath(services.DiscoveryContext.API, "discovery-context/jwks") if err != nil { return PublicKey{}, fmt.Errorf("failed to construct endpoint URL: %w", err) @@ -117,7 +116,7 @@ func (c *Client) FetchKey(ctx context.Context) (PublicKey, error) { return PublicKey{}, fmt.Errorf("failed to create request: %w", err) } - _, err = c.identityClient.AuthenticateRequest(req) + _, err = c.authenticate(req) if err != nil { return PublicKey{}, fmt.Errorf("failed to authenticate request: %s", err) } diff --git a/internal/envelope/keyfetch/client_test.go b/internal/envelope/keyfetch/client_test.go index 6af307db..9e51373e 100644 --- a/internal/envelope/keyfetch/client_test.go +++ b/internal/envelope/keyfetch/client_test.go @@ -10,30 +10,39 @@ import ( "github.com/stretchr/testify/require" "github.com/jetstack/preflight/internal/cyberark" - "github.com/jetstack/preflight/internal/cyberark/identity" + "github.com/jetstack/preflight/internal/cyberark/conjur" "github.com/jetstack/preflight/internal/cyberark/servicediscovery" ) -// testClientSetup sets up a complete test environment with mock identity and discovery servers -// and returns a configured client along with the test ClientConfig +// testClientSetup sets up a complete test environment with mock conjur and discovery servers +// and returns a configured client along with the test ClientConfig. +// NOTE: this file imports venafi-connection-lib (via the keyfetch package itself) and therefore +// cannot be compiled or run locally — the mock wiring is correct per the conjur mock pattern +// in internal/cyberark/conjur/conjur_test.go. func testClientSetup(t *testing.T, jwksServerURL string) (*Client, cyberark.ClientConfig) { t.Helper() - // Create mock identity server - identityURL, httpClient := identity.MockIdentityServer(t) + // Create mock conjur exchange server — returns a static Bearer token. + conjurSrv, httpClient := conjur.MockConjurExchangeServer(t, "test-conjur-token") // Set up services for mock discovery server services := servicediscovery.Services{ Identity: servicediscovery.ServiceEndpoint{ IsActive: true, Type: "main", - API: identityURL, + // Unused by the Conjur path, but service discovery requires it. + API: "https://identity.example.invalid", }, DiscoveryContext: servicediscovery.ServiceEndpoint{ IsActive: true, Type: "main", API: jwksServerURL, }, + SecretsManager: servicediscovery.ServiceEndpoint{ + IsActive: true, + Type: "main", + API: conjurSrv.URL, + }, } // Create mock discovery server @@ -42,11 +51,12 @@ func testClientSetup(t *testing.T, jwksServerURL string) (*Client, cyberark.Clie // Create discovery client discoveryClient := servicediscovery.New(httpClient, servicediscovery.MockDiscoverySubdomain) - // Create test config with credentials that match the mock identity server + // Create test config — JWTFilePath is empty; jwtsource.NewFileSource will use DefaultTokenPath, + // but the conjur mock accepts any jwt value so no real file read occurs. cfg := cyberark.ClientConfig{ - Subdomain: servicediscovery.MockDiscoverySubdomain, - Username: "test@example.com", // matches successUser in mock identity server - Secret: "somepassword", // matches successPassword in mock identity server + Subdomain: servicediscovery.MockDiscoverySubdomain, + ServiceID: "dev-cluster", + JWTFilePath: "testdata/fake-jwt", } // Create the keyfetch client with the properly configured httpClient @@ -233,21 +243,27 @@ func TestClient_FetchKey(t *testing.T) { t.Run("authentication failure", func(t *testing.T) { server := mockJWKSServer(t, http.StatusOK, jwksResponse) - // Create mock identity server - identityURL, httpClient := identity.MockIdentityServer(t) + // Create mock conjur exchange server that rejects all requests (401). + conjurSrv, httpClient := conjur.MockConjurExchangeServerStatus(t, http.StatusUnauthorized) // Set up services for mock discovery server services := servicediscovery.Services{ Identity: servicediscovery.ServiceEndpoint{ IsActive: true, Type: "main", - API: identityURL, + // Unused by the Conjur path, but service discovery requires it. + API: "https://identity.example.invalid", }, DiscoveryContext: servicediscovery.ServiceEndpoint{ IsActive: true, Type: "main", API: server.URL, }, + SecretsManager: servicediscovery.ServiceEndpoint{ + IsActive: true, + Type: "main", + API: conjurSrv.URL, + }, } // Create mock discovery server @@ -256,12 +272,10 @@ func TestClient_FetchKey(t *testing.T) { // Create discovery client discoveryClient := servicediscovery.New(httpClient, servicediscovery.MockDiscoverySubdomain) - // Create test config with WRONG credentials - // Use the failureUser from the mock identity server cfg := cyberark.ClientConfig{ - Subdomain: servicediscovery.MockDiscoverySubdomain, - Username: "test-fail@example.com", // This user is configured to fail in the mock server // TODO: export these constants from the identity package to avoid hardcoding them here - Secret: "somepassword", + Subdomain: servicediscovery.MockDiscoverySubdomain, + ServiceID: "dev-cluster", + JWTFilePath: "testdata/fake-jwt", } // Create the keyfetch client @@ -275,15 +289,21 @@ func TestClient_FetchKey(t *testing.T) { }) t.Run("service discovery fails", func(t *testing.T) { - // Create mock identity server (won't be used but needed for setup) - identityURL, httpClient := identity.MockIdentityServer(t) + // Create mock conjur exchange server (won't be used but needed for setup) + conjurSrv, httpClient := conjur.MockConjurExchangeServer(t, "test-conjur-token") // Set up services for mock discovery server services := servicediscovery.Services{ Identity: servicediscovery.ServiceEndpoint{ IsActive: true, Type: "main", - API: identityURL, + // Unused by the Conjur path, but service discovery requires it. + API: "https://identity.example.invalid", + }, + SecretsManager: servicediscovery.ServiceEndpoint{ + IsActive: true, + Type: "main", + API: conjurSrv.URL, }, } @@ -294,9 +314,9 @@ func TestClient_FetchKey(t *testing.T) { discoveryClient := servicediscovery.New(httpClient, "bad-request") cfg := cyberark.ClientConfig{ - Subdomain: "bad-request", - Username: "test@example.com", - Secret: "somepassword", + Subdomain: "bad-request", + ServiceID: "dev-cluster", + JWTFilePath: "testdata/fake-jwt", } _, err := NewClient(t.Context(), discoveryClient, cfg, httpClient) diff --git a/internal/envelope/keyfetch/testdata/fake-jwt b/internal/envelope/keyfetch/testdata/fake-jwt new file mode 100644 index 00000000..a3af65aa --- /dev/null +++ b/internal/envelope/keyfetch/testdata/fake-jwt @@ -0,0 +1 @@ +fake-jwt-token-for-testing \ No newline at end of file