From ffa53d1b99e4bfee6d3d25718ca225d2ac7f9779 Mon Sep 17 00:00:00 2001 From: rzisholz Date: Sun, 23 Aug 2026 13:48:30 +0300 Subject: [PATCH 1/5] CP-21164: add jwtsource package for reading projected SA tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces a small, isolated interface for reading a JWT from a file path — the first piece of the upcoming Conjur JWT authentication path, split out on its own since nothing else in this PR depends on it yet. --- internal/cyberark/jwtsource/jwtsource.go | 39 +++++++++++++++++++ internal/cyberark/jwtsource/jwtsource_test.go | 32 +++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 internal/cyberark/jwtsource/jwtsource.go create mode 100644 internal/cyberark/jwtsource/jwtsource_test.go 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..c3dd839a --- /dev/null +++ b/internal/cyberark/jwtsource/jwtsource_test.go @@ -0,0 +1,32 @@ +// internal/cyberark/jwtsource/jwtsource_test.go +package jwtsource + +import ( + "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(t.Context()) + require.NoError(t, err) + require.Equal(t, "the-jwt", got) // trimmed +} + +func TestFileSource_MissingFile(t *testing.T) { + _, err := NewFileSource("/no/such/file").Read(t.Context()) + 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(t.Context()) + require.Error(t, err) +} From 33e6b50f30f4ade7d4a78d1585beec97f3697d0d Mon Sep 17 00:00:00 2001 From: rzisholz Date: Sun, 23 Aug 2026 13:49:37 +0300 Subject: [PATCH 2/5] CP-21164: split legacy username/password login into its own file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit identity.go mixed the shared client/token-cache plumbing with the CyberArk Identity username/password (UP) login flow. Move the UP-specific code into username_password.go so the shared plumbing stays easy to find once a second login mechanism (Conjur JWT) is added alongside it. No behavior change — pure extraction, plus exporting the mock's success credentials for other packages' tests. --- internal/cyberark/identity/identity.go | 416 +---------------- internal/cyberark/identity/mock.go | 8 + .../cyberark/identity/username_password.go | 424 ++++++++++++++++++ 3 files changed, 435 insertions(+), 413 deletions(-) create mode 100644 internal/cyberark/identity/username_password.go 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..928ff868 100644 --- a/internal/cyberark/identity/mock.go +++ b/internal/cyberark/identity/mock.go @@ -34,6 +34,14 @@ const ( mockSuccessfulStartAuthenticationToken = "success-token" ) +// 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 ( //go:embed testdata/start_authentication_success.json startAuthenticationSuccessResponse string 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) +} From 0bd93b514e872e8be85f2ae3e07ad818dfbac1af Mon Sep 17 00:00:00 2001 From: rzisholz Date: Mon, 24 Aug 2026 14:34:05 +0300 Subject: [PATCH 3/5] CP-21164: resolve secrets_manager alongside identity_administration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Service Discovery API returns several independently-hosted services; the authn-jwt exchange this PR series adds is served by secrets_manager, a different host from identity_administration. Add a SecretsManager field to Services and parse it, so callers have it available — nothing reads it yet, that lands in a later PR alongside the client that needs it. secrets_manager and discoveryContext are deliberately not required here unlike identity, since not every caller needs them and requiring secrets_manager would break every existing username/password install on a tenant not yet onboarded to Conjur — each caller validates what it needs at its own point of use instead. Factor the repeated "find the first active main endpoint" loop into a mainActiveAPI helper now that there are three near-identical copies. --- .../cyberark/servicediscovery/discovery.go | 54 +++++++++++++------ .../servicediscovery/discovery_test.go | 10 ++++ internal/cyberark/servicediscovery/mock.go | 1 + .../servicediscovery/testdata/README.md | 5 +- .../testdata/discovery_success.json.template | 2 +- 5 files changed, 53 insertions(+), 19 deletions(-) diff --git a/internal/cyberark/servicediscovery/discovery.go b/internal/cyberark/servicediscovery/discovery.go index 93598d5c..b4b0c920 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 @@ -47,6 +54,17 @@ type Client struct { cachedResponseMutex sync.Mutex } +// mainActiveAPI returns the API URL of the first active "main" endpoint in +// eps, or "" if there isn't one. +func mainActiveAPI(eps []ServiceEndpoint) string { + for _, ep := range eps { + if ep.Type == "main" && ep.IsActive && ep.API != "" { + return ep.API + } + } + return "" +} + // New creates a new CyberArk Service Discovery client. If the ARK_DISCOVERY_API // environment variable is set, it is used as the base URL for the service // discovery API. Otherwise, the production URL is used. @@ -101,11 +119,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,23 +183,15 @@ 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: - for _, ep := range svc.Endpoints { - if ep.Type == "main" && ep.IsActive && ep.API != "" { - identityAPI = ep.API - break - } - } + identityAPI = mainActiveAPI(svc.Endpoints) case DiscoveryContextServiceName: - for _, ep := range svc.Endpoints { - if ep.Type == "main" && ep.IsActive && ep.API != "" { - discoveryContextAPI = ep.API - break - } - } + discoveryContextAPI = mainActiveAPI(svc.Endpoints) + case SecretsManagerServiceName: + secretsManagerAPI = mainActiveAPI(svc.Endpoints) } } @@ -187,11 +199,21 @@ func (c *Client) DiscoverServices(ctx context.Context) (*Services, string, error return nil, "", fmt.Errorf("didn't find %s in service discovery response, "+ "which may indicate a suspended tenant; unable to detect CyberArk Identity API URL", IdentityServiceName) } - //TODO: Should add a check for discoveryContextAPI too? + // discoveryContextAPI and secretsManagerAPI are deliberately not required + // here, unlike identityAPI above: not every caller needs both, and + // requiring secretsManagerAPI would break every existing + // username/password install on a tenant not yet onboarded to Conjur. + // Each caller that needs one validates it itself — e.g. + // cyberark.NewDatauploadClient rejects an empty discoveryContextAPI, and + // cyberark.selectAuthenticator rejects an empty secretsManagerAPI on the + // Conjur JWT path. Not every caller does this yet (keyfetch's client + // doesn't check discoveryContextAPI), so an empty value can still surface + // downstream as a less obvious 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 }}" } ] }, From b59e43d9aadece2947df64edbdd93eafc69ea259 Mon Sep 17 00:00:00 2001 From: rzisholz Date: Sun, 23 Aug 2026 13:52:12 +0300 Subject: [PATCH 4/5] CP-21164: add Conjur JWT authentication client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exchanges a projected ServiceAccount JWT (via jwtsource) for a Conjur access token through the authn-jwt endpoint, and authenticates requests with it as identity.RequestAuthenticator. Nothing wires this in yet — that's the next PR, once both this and the legacy identity client exist side by side. The identity returned for audit tagging is the token's own sub claim when it can be extracted, falling back to the configured service ID otherwise. The cache expiry is driven by the token's own exp claim when present, falling back to a guessed TTL only when it isn't — a fixed TTL stamped after the exchange returns would otherwise serve a token past its real expiry under latency or clock skew. Exposes Invalidate() so a caller that gets a 401 from the resource server can force a fresh exchange instead of waiting out the cache. --- internal/cyberark/conjur/conjur.go | 203 ++++++++++++++++++++++++ internal/cyberark/conjur/conjur_test.go | 203 ++++++++++++++++++++++++ internal/cyberark/conjur/mock.go | 37 +++++ 3 files changed, 443 insertions(+) create mode 100644 internal/cyberark/conjur/conjur.go create mode 100644 internal/cyberark/conjur/conjur_test.go create mode 100644 internal/cyberark/conjur/mock.go diff --git a/internal/cyberark/conjur/conjur.go b/internal/cyberark/conjur/conjur.go new file mode 100644 index 00000000..76fea72d --- /dev/null +++ b/internal/cyberark/conjur/conjur.go @@ -0,0 +1,203 @@ +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" +) + +// defaultTokenTTL is the fallback cache lifetime used when a token's own +// `exp` claim can't be read — Conjur access tokens default to an 8-minute +// lifetime. It's a Client field, not a const, so tests can shrink it. +const defaultTokenTTL = 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 + tokenTTL time.Duration + + mu sync.Mutex + token string + identity string + expiry 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, tokenTTL: defaultTokenTTL} +} + +// Invalidate clears the cached token, forcing the next AuthenticateRequest +// call to exchange a fresh one. Callers should call this after a 401 from +// the resource server the token was used against — the cache's own expiry +// tracking only catches a token aging out, not one rejected early (e.g. a +// Conjur restart or a toggled authenticator). +func (c *Client) Invalidate() { + c.mu.Lock() + defer c.mu.Unlock() + c.token, c.identity, c.expiry = "", "", time.Time{} +} + +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 { + // Conjur returns a JSON error body with the actual reason; include a + // bounded prefix so the operator doesn't have to go read Conjur's own + // audit log to find out why. 401 here most often means the SA token + // audience != authenticator audience=conjur. + errBody, _ := io.ReadAll(io.LimitReader(resp.Body, 4*1024)) + // Drain the rest so the connection can be reused, bounded so a + // misbehaving server can't make this read unboundedly. + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 1024*1024)) + return "", fmt.Errorf("authn-jwt exchange rejected (%d): %s; verify service_id, the authenticator is enabled, and the SA token audience is 'conjur'", + resp.StatusCode, strings.TrimSpace(string(errBody))) + } + 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 +} + +// tokenClaims is the subset of a Conjur access token's payload this client +// reads: `sub` (the caller's identity, used for audit tagging) and `exp` +// (unix seconds, used to drive the token cache off its real expiry instead +// of a guessed TTL). +type tokenClaims struct { + Sub string `json:"sub"` + Exp int64 `json:"exp"` +} + +// claimsFromToken extracts the payload claims from a Conjur access token. +// The payload segment is url-safe base64 without padding. Returns +// (zero value, false) if the token doesn't parse. +func claimsFromToken(token string) (tokenClaims, bool) { + obj, ok := conjurTokenObject(token) + if !ok { + return tokenClaims{}, false + } + payloadJSON, err := base64.URLEncoding.DecodeString(padBase64(obj.Payload)) + if err != nil { + return tokenClaims{}, false + } + var claims tokenClaims + if err := json.Unmarshal(payloadJSON, &claims); err != nil { + return tokenClaims{}, false + } + return claims, 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. +// +// The mutex is held across the exchange's network round-trip so concurrent +// callers share one exchange instead of a thundering herd; they're +// effectively serial at the current call sites. Whichever caller wins the +// race also controls the exchange's deadline via its own req.Context(), so +// an unrelated cancellation can fail a waiting caller — acceptable for now +// given the current call pattern. +func (c *Client) AuthenticateRequest(req *http.Request) (string, error) { + c.mu.Lock() + defer c.mu.Unlock() + if c.token == "" || !time.Now().Before(c.expiry) { + tok, err := c.exchange(req.Context()) + if err != nil { + return "", err + } + claims, ok := claimsFromToken(tok) + identity, expiry := c.serviceID, time.Now().Add(c.tokenTTL) + if !ok { + klog.FromContext(req.Context()).V(2).Info("could not parse Conjur access token; falling back to service ID as identity and a guessed expiry") + } else { + if claims.Sub != "" { + identity = claims.Sub + } + if claims.Exp > 0 { + expiry = time.Unix(claims.Exp, 0) + } + } + c.token, c.identity, c.expiry = tok, identity, expiry + } + 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..9880f7e9 --- /dev/null +++ b/internal/cyberark/conjur/conjur_test.go @@ -0,0 +1,203 @@ +package conjur + +import ( + "context" + "encoding/base64" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "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 and no `exp` claim, for test use only. +func buildJWSToken(t testing.TB, sub string) string { + t.Helper() + return buildJWSTokenClaims(t, sub, 0) +} + +// buildJWSTokenClaims is like buildJWSToken but also sets `exp` (unix +// seconds) when exp != 0. +func buildJWSTokenClaims(t testing.TB, sub string, exp int64) string { + t.Helper() + claims := map[string]any{"sub": sub} + if exp != 0 { + claims["exp"] = exp + } + payload, err := json.Marshal(claims) + 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") +} + +func TestAuthenticateRequest_ExpiredExpClaimTriggersReexchange(t *testing.T) { + const sub = "host/data/k8s/test-cluster-uuid/workloads/system:serviceaccount:test:test-agent" + token := buildJWSTokenClaims(t, sub, time.Now().Add(-time.Minute).Unix()) + var exchanges int + srv := mockConjurExchangeServerCountingExchanges(t, token, &exchanges) + defer srv.Close() + + // A long fallback TTL would keep this cached if exp weren't honored. + c := New(srv.Client(), srv.URL, "dev-cluster", "conjur", staticSource{tok: "the-jwt"}) + + req1, _ := http.NewRequest(http.MethodGet, "https://example.com/a", nil) + _, err := c.AuthenticateRequest(req1) + require.NoError(t, err) + + req2, _ := http.NewRequest(http.MethodGet, "https://example.com/b", nil) + _, err = c.AuthenticateRequest(req2) + require.NoError(t, err) + + require.Equal(t, 2, exchanges, "a token whose own exp claim has already passed must not be served from cache") +} + +func TestAuthenticateRequest_FutureExpClaimOverridesShortFallbackTTL(t *testing.T) { + const sub = "host/data/k8s/test-cluster-uuid/workloads/system:serviceaccount:test:test-agent" + token := buildJWSTokenClaims(t, sub, time.Now().Add(time.Hour).Unix()) + var exchanges int + srv := mockConjurExchangeServerCountingExchanges(t, token, &exchanges) + defer srv.Close() + + c := New(srv.Client(), srv.URL, "dev-cluster", "conjur", staticSource{tok: "the-jwt"}) + c.tokenTTL = time.Nanosecond // would force a re-exchange if exp weren't honored + + req1, _ := http.NewRequest(http.MethodGet, "https://example.com/a", nil) + _, err := c.AuthenticateRequest(req1) + require.NoError(t, err) + + req2, _ := http.NewRequest(http.MethodGet, "https://example.com/b", nil) + _, err = c.AuthenticateRequest(req2) + require.NoError(t, err) + + require.Equal(t, 1, exchanges, "a token's own future exp claim must be honored over a short fallback TTL") +} + +func TestInvalidate_ForcesReexchange(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) + _, err := c.AuthenticateRequest(req1) + require.NoError(t, err) + + c.Invalidate() + + req2, _ := http.NewRequest(http.MethodGet, "https://example.com/b", nil) + _, err = c.AuthenticateRequest(req2) + require.NoError(t, err) + + require.Equal(t, 2, exchanges, "Invalidate must force the next AuthenticateRequest call to re-exchange") +} + +func TestAuthenticateRequest_ExchangeErrorIncludesConjurResponseBody(t *testing.T) { + srv, httpClient := MockConjurExchangeServerStatusBody(t, http.StatusUnauthorized, []byte(`{"error":{"message":"CONJ00001E Invalid JWT token"}}`)) + 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.Contains(t, err.Error(), "CONJ00001E Invalid JWT token") +} diff --git a/internal/cyberark/conjur/mock.go b/internal/cyberark/conjur/mock.go new file mode 100644 index 00000000..c3846bd0 --- /dev/null +++ b/internal/cyberark/conjur/mock.go @@ -0,0 +1,37 @@ +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() + return MockConjurExchangeServerStatusBody(t, status, nil) +} + +// MockConjurExchangeServerStatusBody is like MockConjurExchangeServerStatus +// but also writes body, for asserting the client surfaces Conjur's own error +// message rather than discarding it. +func MockConjurExchangeServerStatusBody(t testing.TB, status int, body []byte) (*httptest.Server, *http.Client) { + t.Helper() + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(status) + _, _ = w.Write(body) + })) + return srv, srv.Client() +} From 0c06690b7daa714bb1fa9870298ee101dd8ad77c Mon Sep 17 00:00:00 2001 From: rzisholz Date: Sun, 23 Aug 2026 13:53:00 +0300 Subject: [PATCH 5/5] CP-21164: select Conjur JWT or legacy username/password authenticator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. 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 — the caller's own password slice is still zeroed as before. A refresh that fails falls back to whatever's cached rather than failing the request outright, since the next call will retry. 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. --- internal/cyberark/auth_select_test.go | 109 ++++++++++++ internal/cyberark/client.go | 158 ++++++++++++++---- internal/cyberark/client_test.go | 107 ++++++++++-- .../identity/authenticated_http_client.go | 24 ++- .../authenticated_http_client_test.go | 70 ++++++++ internal/cyberark/identity/identity.go | 17 ++ .../cyberark/identity/username_password.go | 22 ++- internal/cyberark/testing/testing.go | 7 +- internal/envelope/keyfetch/client.go | 31 ++-- internal/envelope/keyfetch/client_test.go | 151 ++++++++++++++--- internal/envelope/keyfetch/testdata/fake-jwt | 1 + 11 files changed, 608 insertions(+), 89 deletions(-) create mode 100644 internal/cyberark/auth_select_test.go create mode 100644 internal/cyberark/identity/authenticated_http_client_test.go create mode 100644 internal/envelope/keyfetch/testdata/fake-jwt 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..7a3feb27 100644 --- a/internal/cyberark/client.go +++ b/internal/cyberark/client.go @@ -3,69 +3,171 @@ 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: + identityAPI := serviceMap.Identity.API + if identityAPI == "" { + return nil, errors.New("service discovery returned an empty identity API") + } + 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 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 } 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/identity/authenticated_http_client.go b/internal/cyberark/identity/authenticated_http_client.go index c20d5bfb..3d552dbe 100644 --- a/internal/cyberark/identity/authenticated_http_client.go +++ b/internal/cyberark/identity/authenticated_http_client.go @@ -3,16 +3,36 @@ package identity import ( "fmt" "net/http" + + "k8s.io/klog/v2" ) type RequestAuthenticator func(req *http.Request) (string, error) -// AuthenticateRequest is a helper function that adds the Authorization header to an HTTP request using a cached token. -// It sets the Header directly, and if successful returns the username corresponding to the token. +// AuthenticateRequest is a helper function that adds the Authorization header +// to an HTTP request using a cached token, refreshing it first if it's aged +// past tokenTTL. It sets the Header directly, and if successful returns the +// username corresponding to the token. +// +// Refresh needs LoginUsernamePassword to have been called at least once — +// that's where the username/password this re-login uses gets captured. If a +// refresh attempt fails, this falls back to whatever's cached rather than +// failing the request outright: the failure is likely transient, and the +// next call will retry. func (c *Client) AuthenticateRequest(req *http.Request) (string, error) { c.tokenCachedMutex.Lock() defer c.tokenCachedMutex.Unlock() + if len(c.tokenCached.Token) == 0 && c.username == "" { + return "", fmt.Errorf("no token cached") + } + + if c.username != "" { + if err := c.login(req.Context(), c.username, c.secret); err != nil { + klog.FromContext(req.Context()).Error(err, "failed to refresh CyberArk Identity token; using cached token") + } + } + if len(c.tokenCached.Token) == 0 { return "", fmt.Errorf("no token cached") } diff --git a/internal/cyberark/identity/authenticated_http_client_test.go b/internal/cyberark/identity/authenticated_http_client_test.go new file mode 100644 index 00000000..8451d889 --- /dev/null +++ b/internal/cyberark/identity/authenticated_http_client_test.go @@ -0,0 +1,70 @@ +package identity + +import ( + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestAuthenticateRequest_NoTokenCached(t *testing.T) { + baseURL, httpClient := MockIdentityServer(t) + c := New(httpClient, baseURL, "subdomain-ignored-by-mock") + + 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_UsesCachedTokenWithinTTL(t *testing.T) { + baseURL, httpClient := MockIdentityServer(t) + c := New(httpClient, baseURL, "subdomain-ignored-by-mock") + require.NoError(t, c.LoginUsernamePassword(t.Context(), successUser, []byte(successPassword))) + + req, _ := http.NewRequest(http.MethodGet, "https://example.com/x", nil) + username, err := c.AuthenticateRequest(req) + require.NoError(t, err) + require.Equal(t, successUser, username) + require.NotEmpty(t, req.Header.Get("Authorization")) +} + +func TestAuthenticateRequest_RefreshesPastTokenTTL(t *testing.T) { + baseURL, httpClient := MockIdentityServer(t) + c := New(httpClient, baseURL, "subdomain-ignored-by-mock") + require.NoError(t, c.LoginUsernamePassword(t.Context(), successUser, []byte(successPassword))) + + // Force the cached token to look stale, then let AuthenticateRequest + // re-login on its own — this is the regression a caller that only calls + // LoginUsernamePassword once at startup (e.g. keyfetch) would otherwise hit. + c.tokenTTL = time.Nanosecond + c.tokenCachedTime = time.Now().Add(-time.Hour) + + req, _ := http.NewRequest(http.MethodGet, "https://example.com/x", nil) + username, err := c.AuthenticateRequest(req) + require.NoError(t, err) + require.Equal(t, successUser, username) + require.NotEmpty(t, req.Header.Get("Authorization")) + require.WithinDuration(t, time.Now(), c.tokenCachedTime, time.Second, "expected AuthenticateRequest to have refreshed the cached token") +} + +func TestAuthenticateRequest_FailedRefreshFallsBackToCachedToken(t *testing.T) { + baseURL, httpClient := MockIdentityServer(t) + c := New(httpClient, baseURL, "subdomain-ignored-by-mock") + require.NoError(t, c.LoginUsernamePassword(t.Context(), successUser, []byte(successPassword))) + cachedToken := c.tokenCached.Token + + // Simulate the password having changed server-side: a refresh attempt + // will now fail, but the previously cached token must still be served + // rather than the request failing outright. + c.secret = []byte("wrong-password") + c.tokenTTL = time.Nanosecond + c.tokenCachedTime = time.Now().Add(-time.Hour) + + req, _ := http.NewRequest(http.MethodGet, "https://example.com/x", nil) + username, err := c.AuthenticateRequest(req) + require.NoError(t, err) + require.Equal(t, successUser, username) + require.Equal(t, "Bearer "+cachedToken, req.Header.Get("Authorization")) +} diff --git a/internal/cyberark/identity/identity.go b/internal/cyberark/identity/identity.go index 66838ca1..88d26bac 100644 --- a/internal/cyberark/identity/identity.go +++ b/internal/cyberark/identity/identity.go @@ -6,16 +6,32 @@ import ( "time" ) +// defaultTokenTTL is the fallback lifetime this client assumes for a cached +// token before it re-logs-in. It's a Client field, not a const, so tests can +// shrink it. +const defaultTokenTTL = 15 * time.Minute + // 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 subdomain string + tokenTTL time.Duration tokenCached token tokenCachedMutex sync.Mutex tokenCachedTime time.Time + + // username and secret are a durable copy of the credentials most recently + // passed to LoginUsernamePassword, retained so AuthenticateRequest can + // re-login on its own once the cached token ages out — the caller's own + // password slice is wiped after that call returns and can't be reused. + // This doesn't meaningfully change the secret's exposure: it already + // lives in memory for the process's lifetime in the config this client + // was constructed from. + username string + secret []byte } // token is a wrapper type for holding auth tokens we want to cache. @@ -30,6 +46,7 @@ func New(httpClient *http.Client, baseURL string, subdomain string) *Client { httpClient: httpClient, baseURL: baseURL, subdomain: subdomain, + tokenTTL: defaultTokenTTL, tokenCached: token{}, tokenCachedMutex: sync.Mutex{}, diff --git a/internal/cyberark/identity/username_password.go b/internal/cyberark/identity/username_password.go index 71ed7d97..d25e0209 100644 --- a/internal/cyberark/identity/username_password.go +++ b/internal/cyberark/identity/username_password.go @@ -181,7 +181,9 @@ type advanceAuthenticationResponseResult struct { } // 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. +// The caller's password slice is zeroed after use — a separate internal copy +// is kept so AuthenticateRequest can re-login on its own once the token ages +// out, without needing the password passed in again. // 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 { @@ -190,13 +192,22 @@ func (c *Client) LoginUsernamePassword(ctx context.Context, username string, pas c.tokenCachedMutex.Lock() defer c.tokenCachedMutex.Unlock() + c.username = username + c.secret = append([]byte(nil), password...) + defer func() { for i := range password { password[i] = 0x00 } }() - if time.Since(c.tokenCachedTime) < 15*time.Minute && c.tokenCached.Username == username { + return c.login(ctx, username, password) +} + +// login performs the actual login flow, reusing the cached token if it's +// still fresh. Callers must hold tokenCachedMutex. +func (c *Client) login(ctx context.Context, username string, password []byte) error { + if time.Since(c.tokenCachedTime) < c.tokenTTL && c.tokenCached.Username == username && c.tokenCached.Token != "" { // 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 @@ -209,12 +220,7 @@ func (c *Client) LoginUsernamePassword(ctx context.Context, username string, pas // 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 + return c.doAdvanceAuthentication(ctx, username, &password, advanceRequestBody) } // doStartAuthentication performs the initial request to start the login process using a username and password. 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..902cd119 100644 --- a/internal/envelope/keyfetch/client.go +++ b/internal/envelope/keyfetch/client.go @@ -25,6 +25,10 @@ const ( // minRSAKeySize is the minimum RSA key size in bits; we'd expect that keys will be larger but 2048 is a sane floor // to enforce to ensure that a weak key can't accidentally be used minRSAKeySize = 2048 + + // defaultCachedKeyTTL is the fallback JWKS cache lifetime. It's a Client + // field, not a const, so tests can shrink it. + defaultCachedKeyTTL = 15 * time.Minute ) // KeyFetcher is an interface for fetching public keys. @@ -50,8 +54,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 @@ -59,11 +62,13 @@ type Client struct { cachedKey PublicKey cachedKeyMutex sync.Mutex cachedKeyTime time.Time + cachedKeyTTL time.Duration } // 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,11 +82,16 @@ 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, + cachedKeyTTL: defaultCachedKeyTTL, }, nil } @@ -92,7 +102,7 @@ func (c *Client) FetchKey(ctx context.Context) (PublicKey, error) { c.cachedKeyMutex.Lock() defer c.cachedKeyMutex.Unlock() - if time.Since(c.cachedKeyTime) < 15*time.Minute { + if time.Since(c.cachedKeyTime) < c.cachedKeyTTL { klog.FromContext(ctx).WithName("keyfetch").V(2).Info("using cached key", "fetchedAt", c.cachedKeyTime.Format(time.RFC3339Nano), "kid", c.cachedKey.KeyID) return c.cachedKey, nil } @@ -102,11 +112,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 +122,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..f8406ff0 100644 --- a/internal/envelope/keyfetch/client_test.go +++ b/internal/envelope/keyfetch/client_test.go @@ -5,35 +5,45 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/jetstack/preflight/internal/cyberark" + "github.com/jetstack/preflight/internal/cyberark/conjur" "github.com/jetstack/preflight/internal/cyberark/identity" "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, using the Conjur JWT auth path. See testKeyfetchClientWithIdentityAuth +// for the legacy username/password path. 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 +52,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 @@ -56,6 +67,41 @@ func testClientSetup(t *testing.T, jwksServerURL string) (*Client, cyberark.Clie return client, cfg } +// testKeyfetchClientWithIdentityAuth is like testClientSetup but wires up the +// legacy username/password path instead of Conjur — bypasses NewClient/ +// selectAuthenticator to get back the underlying *identity.Client so tests +// can shrink its token TTL directly, the same way conjur.Client's tests do. +func testKeyfetchClientWithIdentityAuth(t *testing.T, jwksServerURL string) (*Client, *identity.Client) { + t.Helper() + + identityBaseURL, httpClient := identity.MockIdentityServer(t) + identityClient := identity.New(httpClient, identityBaseURL, "subdomain-ignored-by-mock") + require.NoError(t, identityClient.LoginUsernamePassword(t.Context(), identity.MockSuccessUser, []byte(identity.MockSuccessPassword))) + + services := servicediscovery.Services{ + Identity: servicediscovery.ServiceEndpoint{ + IsActive: true, + Type: "main", + API: identityBaseURL, + }, + DiscoveryContext: servicediscovery.ServiceEndpoint{ + IsActive: true, + Type: "main", + API: jwksServerURL, + }, + } + _ = servicediscovery.MockDiscoveryServer(t, services) + discoveryClient := servicediscovery.New(httpClient, servicediscovery.MockDiscoverySubdomain) + + client := &Client{ + discoveryClient: discoveryClient, + authenticate: identityClient.AuthenticateRequest, + httpClient: httpClient, + cachedKeyTTL: defaultCachedKeyTTL, + } + return client, identityClient +} + func mockJWKSServer(t *testing.T, statusCode int, jwksResponse string) *httptest.Server { t.Helper() @@ -233,21 +279,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 +308,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 +325,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 +350,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) @@ -394,3 +450,52 @@ func TestClient_FetchKey(t *testing.T) { assert.Contains(t, err.Error(), "no valid RSA keys found") }) } + +// TestClient_FetchKey_UsernamePasswordAuth covers the legacy auth path, which +// testClientSetup/TestClient_FetchKey above doesn't exercise at all (it only +// wires up Conjur) — this is the path a self-refresh regression in +// identity.Client would silently break while every other test here stays +// green. +func TestClient_FetchKey_UsernamePasswordAuth(t *testing.T) { + jwksResponse := `{ +"keys": [ + { + "kty": "RSA", + "use": "enc", + "kid": "test-key-1", + "alg": "RSA-OAEP-256", + "n": "vDdioGpDuAEQDd4WRXyWa4sZ5EeS9OPsRrU_jU3PbZdDcANxfh_WSeSvSBKGfGXGC3fIzu0Ernk9VjXcs3LeFdRq2N4nNRZvCzsd_MjBtn7CWgjM_Sk9DXEGn3cHHilcJUJQ4i2YgX9bHu0odNgE6cSVIUEMIC2EGuGk_I7lwroinAAwXpNLLQkV_25kv_QQof2i5f7AocY6QTd0SAo8ZUqFBzanupkeFpl3-Bsz6_zdt_N0x9k5XHQn42Q2oTupTwvXFbE1x8XtCpiaP3_fsQ9dN7t4z6HtwlNUJB2tFfF6PgdKZ9LuJpYjFPYzJQ6Rv28fuc8YHcF7Jittjyzmew", + "e": "AQAB" + } + ] + }` + + t.Run("successful fetch", func(t *testing.T) { + server := mockJWKSServer(t, http.StatusOK, jwksResponse) + + client, _ := testKeyfetchClientWithIdentityAuth(t, server.URL) + key, err := client.FetchKey(t.Context()) + + require.NoError(t, err) + assert.Equal(t, "test-key-1", key.KeyID) + }) + + t.Run("second fetch past the JWKS cache TTL re-authenticates and still succeeds", func(t *testing.T) { + server := mockJWKSServer(t, http.StatusOK, jwksResponse) + + client, _ := testKeyfetchClientWithIdentityAuth(t, server.URL) + _, err := client.FetchKey(t.Context()) + require.NoError(t, err) + + // Force the JWKS cache to look stale so this fetch does a real round + // trip through c.authenticate again rather than returning the cached + // key — this is exactly the seam a dropped per-fetch authentication + // call would break, since nothing else in this test file re-drives + // the legacy username/password path after the first login. + client.cachedKeyTime = time.Now().Add(-time.Hour) + + key, err := client.FetchKey(t.Context()) + require.NoError(t, err) + assert.Equal(t, "test-key-1", key.KeyID) + }) +} 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