From 62793c05a83a6a24aa32eb4bf1280e020b5d1368 Mon Sep 17 00:00:00 2001 From: Roee Zisholz Date: Wed, 1 Jul 2026 12:57:15 +0300 Subject: [PATCH] feat(agent): add SMS/Conjur JWT authentication alongside username/password Adds a workload-identity auth path: the agent reads a projected ServiceAccount token, exchanges it at Secrets Manager SaaS (Conjur Cloud) authn-jwt for a short-lived Conjur access token, and presents it as a Bearer credential. The existing CyberArk Identity username/password login is retained; the method is selected automatically by config: config.cyberark.serviceId set -> Conjur JWT exchange (preferred) else ARK_USERNAME + ARK_SECRET -> username/password (unchanged) both set -> serviceId wins neither -> fail closed Only the token source changes, behind the existing identity.RequestAuthenticator seam; dataupload, service discovery and the upload pipeline are untouched. - internal/cyberark/conjur: authn-jwt exchange, ~8m token cache, Bearer - internal/cyberark/jwtsource: file source (projected SA token); spiffe deferred - internal/cyberark/identity: username/password login retained (split into username_password.go), behaviour unchanged - internal/cyberark/client.go: selectAuthenticator picks the method; NewRequestAuthenticator exposed for envelope/keyfetch - internal/envelope/keyfetch: JWKS fetch uses the same authenticator selector - pkg/agent/config.go: config.cyberark.* keys; service_id no longer mandatory - deploy/charts/disco-agent: projected token volume (aud=conjur), config keys, optional ARK_USERNAME/ARK_SECRET; docs Unit tests cover the four selection cases and the retained username/password login. --- deploy/charts/disco-agent/README.md | 145 +++++- .../disco-agent/templates/deployment.yaml | 36 +- deploy/charts/disco-agent/values.schema.json | 42 ++ deploy/charts/disco-agent/values.yaml | 27 ++ internal/cyberark/auth_select_test.go | 101 +++++ internal/cyberark/client.go | 125 ++++-- internal/cyberark/client_test.go | 96 +++- internal/cyberark/conjur/conjur.go | 81 ++++ internal/cyberark/conjur/conjur_test.go | 34 ++ internal/cyberark/conjur/mock.go | 28 ++ internal/cyberark/identity/identity.go | 416 +---------------- internal/cyberark/identity/mock.go | 14 +- .../cyberark/identity/username_password.go | 424 ++++++++++++++++++ internal/cyberark/jwtsource/jwtsource.go | 39 ++ internal/cyberark/jwtsource/jwtsource_test.go | 33 ++ internal/cyberark/testing/testing.go | 7 +- internal/envelope/keyfetch/client.go | 23 +- internal/envelope/keyfetch/client_test.go | 50 ++- internal/envelope/keyfetch/testdata/fake-jwt | 1 + pkg/agent/config.go | 51 ++- pkg/agent/config_test.go | 160 ++++++- pkg/client/client_cyberark.go | 26 +- pkg/testutil/envtest.go | 35 +- 23 files changed, 1428 insertions(+), 566 deletions(-) create mode 100644 internal/cyberark/auth_select_test.go 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 create mode 100644 internal/cyberark/identity/username_password.go create mode 100644 internal/cyberark/jwtsource/jwtsource.go create mode 100644 internal/cyberark/jwtsource/jwtsource_test.go create mode 100644 internal/envelope/keyfetch/testdata/fake-jwt diff --git a/deploy/charts/disco-agent/README.md b/deploy/charts/disco-agent/README.md index 32107f78..8b33ed64 100644 --- a/deploy/charts/disco-agent/README.md +++ b/deploy/charts/disco-agent/README.md @@ -16,26 +16,36 @@ kubectl create ns "$NAMESPACE" || true ### Add credentials to a Secret -You will require tenant details and credentials for the CyberArk Identity Security Platform. -Put them in the following environment variables: +The agent supports **two authentication methods**, selected automatically by +config: + +| Set this | Method used | +|---|---| +| `config.cyberark.serviceId` (Conjur authn-jwt service-id) | **Conjur JWT exchange** — exchanges a projected ServiceAccount token for a short-lived Conjur access token. No stored password. Preferred for new installs. | +| `ARK_USERNAME` + `ARK_SECRET` in the Secret (and no `serviceId`) | **Legacy CyberArk Identity username/password** — backward compatible with existing GA installs. | + +If **both** are set, the Conjur `serviceId` wins (so a migrating install can add +the service-id before removing its old credentials) and a warning is logged. If +**neither** is set, the agent fails closed at startup. + +The only credential always required in the Kubernetes Secret is the CyberArk +tenant subdomain (`ARK_SUBDOMAIN`). ```sh -export ARK_SUBDOMAIN= # your CyberArk tenant subdomain e.g. tlskp-test -export ARK_USERNAME= # your CyberArk username -export ARK_SECRET= # your CyberArk password -# OPTIONAL: the URL for the CyberArk Discovery API if not using the production environment +export ARK_SUBDOMAIN= # your CyberArk tenant subdomain, e.g. tlskp-test +# OPTIONAL: Discovery API URL for non-production environments export ARK_DISCOVERY_API=https://platform-discovery.integration-cyberark.cloud/ ``` -Create a Secret containing the tenant details and credentials: +Create the Secret: ```sh kubectl create secret generic agent-credentials \ --namespace "$NAMESPACE" \ - --from-literal=ARK_USERNAME=$ARK_USERNAME \ - --from-literal=ARK_SECRET=$ARK_SECRET \ - --from-literal=ARK_SUBDOMAIN=$ARK_SUBDOMAIN \ - --from-literal=ARK_DISCOVERY_API=$ARK_DISCOVERY_API + --from-literal=ARK_SUBDOMAIN=$ARK_SUBDOMAIN +# Add the optional key only if targeting a non-production Discovery API: +# kubectl patch secret agent-credentials -n "$NAMESPACE" \ +# --type=json -p '[{"op":"add","path":"/data/ARK_DISCOVERY_API","value":"'"$(echo -n $ARK_DISCOVERY_API | base64)"'"}]' ``` Alternatively, use the following Secret as a template: @@ -49,23 +59,114 @@ metadata: namespace: cyberark type: Opaque stringData: - ARK_SUBDOMAIN: $ARK_SUBDOMAIN # your CyberArk tenant subdomain e.g. tlskp-test - ARK_SECRET: $ARK_SECRET # your CyberArk password - ARK_USERNAME: $ARK_USERNAME # your CyberArk username - # OPTIONAL: the URL for the CyberArk Discovery API if not using the production environment + ARK_SUBDOMAIN: "tlskp-test" # your CyberArk tenant subdomain + # OPTIONAL: uncomment for non-production Discovery API # ARK_DISCOVERY_API: https://platform-discovery.integration-cyberark.cloud/ + # LEGACY (only if NOT using Conjur serviceId) — username/password auth: + # ARK_USERNAME: "svc-agent@tenant" + # ARK_SECRET: "" ``` -### Deploy the agent +### Configure Conjur JWT authentication + +> Skip this section if you are using the legacy username/password method +> (set `ARK_USERNAME`/`ARK_SECRET` in the Secret and leave `serviceId` empty). + +Set `config.cyberark.serviceId` to the authn-jwt authenticator service ID +configured for this cluster in your Conjur tenant. This is the **bare service-id +segment** (e.g. `disco-agent`), NOT the policy path `conjur/authn-jwt/disco-agent` +— the agent builds the authenticate URL as +`/authn-jwt///authenticate`, so a path here would +double the `conjur/authn-jwt` prefix. The remaining defaults are correct for +CyberArk-hosted tenants: + +| Value | Default | Description | +|---|---|---| +| `config.cyberark.serviceId` | `""` | Conjur authn-jwt service ID (required). Example: `conjur/authn-jwt/disco-agent` | +| `config.cyberark.account` | `conjur` | Conjur account name. Always `conjur` for CyberArk-hosted tenants. | +| `config.cyberark.jwtSource` | `file` | Token source. `file` = projected SA-token volume (default). `spiffe` deferred. | +| `config.cyberark.jwtFilePath` | `/var/run/secrets/tokens/jwt` | Path to the projected token file. Auto-mounted by the chart when `jwtSource=file`. | + +The chart automatically renders a projected ServiceAccount token volume +(audience=`conjur`, expiry 600 s) and mounts it at `/var/run/secrets/tokens` +when `config.cyberark.jwtSource` is `file` (the default). No manual volume +configuration is required. + +### Per-tenant Conjur onboarding + +Before deploying the agent against a new tenant, complete the following steps +in the Conjur tenant: + +1. **Enable the authn-jwt authenticator** with `audience=conjur` and + `token-app-property=sub`. + + ```yaml + # conjur-authn-jwt-policy.yml + - !policy + id: conjur/authn-jwt/disco-agent + body: + - !webservice + + - !variable jwks-uri + - !variable token-app-property + - !variable issuer + - !variable audience + + - !group hosts + - !permit + role: !group hosts + privilege: [ read, authenticate ] + resource: !webservice + ``` + +2. **Set the authenticator variables** (values shown as examples): + + ```sh + conjur variable set -i conjur/authn-jwt/disco-agent/token-app-property -v sub + conjur variable set -i conjur/authn-jwt/disco-agent/audience -v conjur + conjur variable set -i conjur/authn-jwt/disco-agent/issuer -v https://kubernetes.default.svc.cluster.local + conjur variable set -i conjur/authn-jwt/disco-agent/jwks-uri -v https://kubernetes.default.svc.cluster.local/openid/v1/jwks + ``` + +3. **Pre-create a Conjur host** for the agent ServiceAccount. The `id` must + match the Kubernetes ServiceAccount's `sub` claim + (`system:serviceaccount::`): + + ```yaml + # conjur-agent-host-policy.yml + - !host + id: system:serviceaccount/cyberark/disco-agent + annotations: + authn-jwt/disco-agent/sub: system:serviceaccount/cyberark/disco-agent + ``` + +4. **Add the host to the `data/disco/snapshot-uploaders` group** so the + authorizer grants it upload access: + + ```yaml + - !grant + role: !group data/disco/snapshot-uploaders + member: !host system:serviceaccount/cyberark/disco-agent + ``` + +5. **Add the host to the authn-jwt authenticator's hosts group**: + + ```yaml + - !grant + role: !group conjur/authn-jwt/disco-agent/hosts + member: !host system:serviceaccount/cyberark/disco-agent + ``` -Deploy the agent: +### Deploy the agent ```sh helm upgrade agent "oci://${OCI_BASE}/charts/disco-agent" \ --install \ --create-namespace \ --namespace "$NAMESPACE" \ - --set fullnameOverride=disco-agent + --set fullnameOverride=disco-agent \ + --set config.cyberark.serviceId=disco-agent \ + --set acceptTerms=true ``` ### Troubleshooting @@ -80,6 +181,14 @@ Check the logs: kubectl logs deployments/disco-agent --namespace "${NAMESPACE}" --follow ``` +#### Conjur authentication errors + +| Symptom | Likely cause | Fix | +|---|---|---| +| Agent logs `401 Unauthorized` from Conjur | ServiceAccount token `audience` does not match the authenticator's configured `audience` value, or the authn-jwt authenticator is not enabled for the account | Confirm `audience=conjur` in both the projected volume (chart default) and the Conjur `conjur/authn-jwt//audience` variable; ensure the authenticator is enabled (`CONJUR_AUTHENTICATORS` includes `authn-jwt/`) | +| Agent logs `403 Forbidden` from the upload API | The agent's Conjur host is not a member of `data/disco/snapshot-uploaders` | Add the host to the group per step 4 of the onboarding runbook above | +| Agent logs `500` / no upload attempt | Conjur is unreachable or returned an unexpected error | Check network policy / DNS; inspect Conjur audit logs for the host identity | + ## Values diff --git a/deploy/charts/disco-agent/templates/deployment.yaml b/deploy/charts/disco-agent/templates/deployment.yaml index 0c98b9a5..8c3f331d 100644 --- a/deploy/charts/disco-agent/templates/deployment.yaml +++ b/deploy/charts/disco-agent/templates/deployment.yaml @@ -58,26 +58,32 @@ spec: valueFrom: fieldRef: fieldPath: spec.nodeName - - name: ARK_USERNAME + - name: ARK_SUBDOMAIN valueFrom: secretKeyRef: name: {{ .Values.authentication.secretName }} - key: ARK_USERNAME - - name: ARK_SECRET + key: ARK_SUBDOMAIN + - name: ARK_DISCOVERY_API valueFrom: secretKeyRef: name: {{ .Values.authentication.secretName }} - key: ARK_SECRET - - name: ARK_SUBDOMAIN + key: ARK_DISCOVERY_API + optional: true + # Legacy CyberArk Identity username/password (backward compatibility). + # Used only when config.cyberark.serviceId is empty; otherwise the + # agent uses the Conjur JWT exchange and ignores these. Optional so + # JWT-only installs need not set them. + - name: ARK_USERNAME valueFrom: secretKeyRef: name: {{ .Values.authentication.secretName }} - key: ARK_SUBDOMAIN - - name: ARK_DISCOVERY_API + key: ARK_USERNAME + optional: true + - name: ARK_SECRET valueFrom: secretKeyRef: name: {{ .Values.authentication.secretName }} - key: ARK_DISCOVERY_API + key: ARK_SECRET optional: true - name: ARK_SEND_SECRET_VALUES value: {{ .Values.config.sendSecretValues | default "false" | quote }} @@ -116,6 +122,11 @@ spec: - name: config mountPath: "/etc/disco-agent" readOnly: true + {{- if or (not .Values.config.cyberark.jwtSource) (eq .Values.config.cyberark.jwtSource "file") }} + - name: conjur-token + mountPath: /var/run/secrets/tokens + readOnly: true + {{- end }} {{- with .Values.volumeMounts }} {{- toYaml . | nindent 12 }} {{- end }} @@ -127,6 +138,15 @@ spec: configMap: name: {{ include "disco-agent.fullname" . }}-config optional: false + {{- if or (not .Values.config.cyberark.jwtSource) (eq .Values.config.cyberark.jwtSource "file") }} + - name: conjur-token + projected: + sources: + - serviceAccountToken: + path: jwt + audience: conjur + expirationSeconds: 600 + {{- end }} {{- with .Values.volumes }} {{- toYaml . | nindent 8 }} {{- end }} diff --git a/deploy/charts/disco-agent/values.schema.json b/deploy/charts/disco-agent/values.schema.json index 401b11a1..c8f973b5 100644 --- a/deploy/charts/disco-agent/values.schema.json +++ b/deploy/charts/disco-agent/values.schema.json @@ -124,6 +124,9 @@ "clusterName": { "$ref": "#/$defs/helm-values.config.clusterName" }, + "cyberark": { + "$ref": "#/$defs/helm-values.config.cyberark" + }, "excludeAnnotationKeysRegex": { "$ref": "#/$defs/helm-values.config.excludeAnnotationKeysRegex" }, @@ -149,6 +152,45 @@ "description": "A human readable name for the cluster where the agent is deployed (optional).\n\nThis cluster name will be associated with the data that the agent uploads to the Discovery and Context service. If empty (the default), the service account name will be used instead.", "type": "string" }, + "helm-values.config.cyberark": { + "additionalProperties": false, + "description": "CyberArk Conjur JWT authentication settings. The agent exchanges a projected ServiceAccount token (audience=conjur) for a short-lived Conjur access token used to authenticate to the Discovery & Context upload API.", + "properties": { + "account": { + "$ref": "#/$defs/helm-values.config.cyberark.account" + }, + "jwtFilePath": { + "$ref": "#/$defs/helm-values.config.cyberark.jwtFilePath" + }, + "jwtSource": { + "$ref": "#/$defs/helm-values.config.cyberark.jwtSource" + }, + "serviceId": { + "$ref": "#/$defs/helm-values.config.cyberark.serviceId" + } + }, + "type": "object" + }, + "helm-values.config.cyberark.account": { + "default": "conjur", + "description": "The Conjur account name. For CyberArk-hosted tenants this is always \"conjur\".", + "type": "string" + }, + "helm-values.config.cyberark.jwtFilePath": { + "default": "/var/run/secrets/tokens/jwt", + "description": "Path to the projected ServiceAccount token file. The chart auto-mounts a projected volume at this path when jwtSource is \"file\".", + "type": "string" + }, + "helm-values.config.cyberark.jwtSource": { + "default": "file", + "description": "Token source for Conjur JWT authentication. \"file\" reads the token from jwtFilePath (projected SA-token volume). \"spiffe\" is deferred and not implemented in this POC.", + "type": "string" + }, + "helm-values.config.cyberark.serviceId": { + "default": "", + "description": "The Conjur authn-jwt authenticator service ID configured for this tenant. Example: conjur/authn-jwt/disco-agent", + "type": "string" + }, "helm-values.config.excludeAnnotationKeysRegex": { "default": [], "description": "You can configure the agent to exclude some annotations or labels from being pushed . All Kubernetes objects are affected. The objects are still pushed, but the specified annotations and labels are removed before being pushed.\n\nDots is the only character that needs to be escaped in the regex. Use either double quotes with escaped single quotes or unquoted strings for the regex to avoid YAML parsing issues with `\\.`.\n\nExample: excludeAnnotationKeysRegex: ['^kapp\\.k14s\\.io/original.*']", diff --git a/deploy/charts/disco-agent/values.yaml b/deploy/charts/disco-agent/values.yaml index 7f362328..6b36d0b6 100644 --- a/deploy/charts/disco-agent/values.yaml +++ b/deploy/charts/disco-agent/values.yaml @@ -202,6 +202,33 @@ config: # a key managed by CyberArk, fetched from the Discovery and Context service. sendSecretValues: true + # CyberArk Conjur JWT authentication settings. + # The agent exchanges a projected ServiceAccount token (audience=conjur) for a + # short-lived Conjur access token, then uses that token to authenticate to the + # Discovery & Context upload API. + cyberark: + # The Conjur authn-jwt authenticator service ID configured for this tenant. + # Set this to use the Conjur JWT exchange (preferred). Leave empty to use the + # legacy CyberArk Identity username/password method (ARK_USERNAME/ARK_SECRET + # in the credentials Secret) for backward compatibility. If both are set, the + # serviceId (Conjur) wins. + # NOTE: bare service-id segment (e.g. "disco-agent"), NOT the policy path + # "conjur/authn-jwt/disco-agent" — the agent builds the URL as + # /authn-jwt///authenticate. + serviceId: "" + + # The Conjur account name. For CyberArk-hosted tenants this is always "conjur". + account: "conjur" + + # Token source for Conjur JWT authentication. + # "file" — read the token from jwtFilePath (projected SA-token volume, default). + # "spiffe" — deferred; not implemented in this POC. + jwtSource: file + + # Path to the projected ServiceAccount token file. + # The chart auto-mounts a projected volume at this path when jwtSource is "file". + jwtFilePath: /var/run/secrets/tokens/jwt + authentication: secretName: agent-credentials diff --git a/internal/cyberark/auth_select_test.go b/internal/cyberark/auth_select_test.go new file mode 100644 index 00000000..57a4d208 --- /dev/null +++ b/internal/cyberark/auth_select_test.go @@ -0,0 +1,101 @@ +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 Identity API points at the given endpoint + // and whose DiscoveryContext points at a dataupload mock. The dataupload mock + // requires Authorization: Bearer success-token. + stack := func(t *testing.T, identityAPI string) *servicediscovery.Services { + t.Helper() + discoveryContextAPI, _ := dataupload.MockDataUploadServer(t) + return &servicediscovery.Services{ + Identity: servicediscovery.ServiceEndpoint{API: identityAPI}, + DiscoveryContext: servicediscovery.ServiceEndpoint{API: discoveryContextAPI}, + } + } + + 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, 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), "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, 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, "https://identity.example"), "tenant", cfg) + require.ErrorIs(t, err, cyberark.ErrNoAuthMethod) + }) +} diff --git a/internal/cyberark/client.go b/internal/cyberark/client.go index 92710296..9f74d06f 100644 --- a/internal/cyberark/client.go +++ b/internal/cyberark/client.go @@ -3,69 +3,142 @@ package cyberark import ( "context" "errors" + "fmt" "net/http" "os" + "k8s.io/klog/v2" + + "github.com/jetstack/preflight/internal/cyberark/conjur" "github.com/jetstack/preflight/internal/cyberark/dataupload" "github.com/jetstack/preflight/internal/cyberark/identity" + "github.com/jetstack/preflight/internal/cyberark/jwtsource" "github.com/jetstack/preflight/internal/cyberark/servicediscovery" ) // ClientConfig holds the configuration needed to initialize a CyberArk client. +// +// Two authentication methods coexist (the product is GA; existing installs use +// username/password). The active method is selected by config presence, see +// selectAuthenticator: a Conjur authn-jwt ServiceID, when set, takes precedence +// over username/password. type ClientConfig struct { Subdomain string - Username string - Secret string + + // Conjur JWT exchange (preferred for new installs). + ServiceID string // authn-jwt service id (POC: per-cluster, e.g. "dev-cluster") + Account string // POC: "conjur" + JWTSource string // "file" (POC) | "spiffe" (deferred) + JWTFilePath string // default jwtsource.DefaultTokenPath + + // Legacy CyberArk Identity username/password (backward compatibility). + // Sourced from ARK_USERNAME / ARK_SECRET. Used only when ServiceID is unset. + Username string + Secret []byte } // ClientConfigLoader is a function type that loads and returns a ClientConfig. type ClientConfigLoader func() (ClientConfig, error) // ErrMissingEnvironmentVariables is returned when required environment variables are not set. -var ErrMissingEnvironmentVariables = errors.New("missing environment variables: ARK_SUBDOMAIN, ARK_USERNAME, ARK_SECRET") +var ErrMissingEnvironmentVariables = errors.New("missing environment variables: ARK_SUBDOMAIN") + +// ErrNoAuthMethod is returned when neither a Conjur service-id nor +// username/password credentials are configured. +var ErrNoAuthMethod = errors.New("no CyberArk authentication method configured: set config.cyberark.service_id (Conjur JWT) or ARK_USERNAME + ARK_SECRET (legacy username/password)") // LoadClientConfigFromEnvironment loads the CyberArk client configuration from environment variables. -// It expects the following environment variables to be set: -// - ARK_SUBDOMAIN: The CyberArk subdomain to use. -// - ARK_USERNAME: The username for authentication. -// - ARK_SECRET: The secret for authentication. +// It expects the following environment variable to be set: +// - ARK_SUBDOMAIN: The CyberArk subdomain to use (required). +// +// It also reads the optional legacy username/password credentials: +// - ARK_USERNAME, ARK_SECRET: used only when no Conjur service-id is configured. +// +// Behavioral keys (ServiceID, Account, JWTSource, JWTFilePath) are set by the +// caller from the agent YAML config (config.cyberark.*). func LoadClientConfigFromEnvironment() (ClientConfig, error) { subdomain := os.Getenv("ARK_SUBDOMAIN") - username := os.Getenv("ARK_USERNAME") - secret := os.Getenv("ARK_SECRET") - - if subdomain == "" || username == "" || secret == "" { + if subdomain == "" { return ClientConfig{}, ErrMissingEnvironmentVariables } - - return ClientConfig{ + cfg := ClientConfig{ Subdomain: subdomain, - Username: username, - Secret: secret, - }, nil - + Username: os.Getenv("ARK_USERNAME"), + } + if secret := os.Getenv("ARK_SECRET"); secret != "" { + cfg.Secret = []byte(secret) + } + return cfg, nil } -// NewDatauploadClient initializes and returns a new CyberArk Data Upload client. -// It performs service discovery to find the necessary API endpoints and authenticates -// using the provided client configuration. -func NewDatauploadClient(ctx context.Context, httpClient *http.Client, serviceMap *servicediscovery.Services, tenantUUID string, cfg ClientConfig) (*dataupload.CyberArkClient, error) { +// selectAuthenticator builds the request authenticator for the configured auth +// method and returns it together with the discovery-context API endpoint. +// +// Selection (backward compatible — the product is GA): +// - ServiceID set → Conjur JWT exchange (preferred). +// - else Username+Secret present → legacy CyberArk Identity UP login. +// - neither → ErrNoAuthMethod. +// +// When both are configured, ServiceID wins (a migrating install can set the +// service-id without first removing its old credentials) and a warning is logged. +func selectAuthenticator(ctx context.Context, httpClient *http.Client, serviceMap *servicediscovery.Services, cfg ClientConfig) (identity.RequestAuthenticator, error) { identityAPI := serviceMap.Identity.API if identityAPI == "" { return nil, errors.New("service discovery returned an empty identity API") } + hasConjur := cfg.ServiceID != "" + hasUP := cfg.Username != "" && len(cfg.Secret) > 0 + + switch { + case hasConjur: + if hasUP { + klog.FromContext(ctx).Info("both Conjur service_id and ARK_USERNAME/ARK_SECRET are set; using the Conjur JWT exchange and ignoring the username/password credentials") + } + if cfg.JWTSource != "" && cfg.JWTSource != "file" { + return nil, fmt.Errorf("jwt_source %q not supported in POC (only 'file')", cfg.JWTSource) + } + account := cfg.Account + if account == "" { + account = "conjur" // POC default; open item #2 + } + src := jwtsource.NewFileSource(cfg.JWTFilePath) + conjurClient := conjur.New(httpClient, identityAPI, cfg.ServiceID, account, src) + return conjurClient.AuthenticateRequest, nil + + case hasUP: + identityClient := identity.New(httpClient, identityAPI, cfg.Subdomain) + if err := identityClient.LoginUsernamePassword(ctx, cfg.Username, cfg.Secret); err != nil { + return nil, fmt.Errorf("CyberArk Identity username/password login failed: %w", err) + } + return identityClient.AuthenticateRequest, nil + + default: + return nil, ErrNoAuthMethod + } +} + +// NewRequestAuthenticator selects and builds the configured request +// authenticator (Conjur JWT exchange or legacy username/password). Exposed for +// other consumers (e.g. envelope key fetching) that need the same auth seam +// without a dataupload client. +func NewRequestAuthenticator(ctx context.Context, httpClient *http.Client, serviceMap *servicediscovery.Services, cfg ClientConfig) (identity.RequestAuthenticator, error) { + return selectAuthenticator(ctx, httpClient, serviceMap, cfg) +} + +// NewDatauploadClient initializes and returns a new CyberArk Data Upload client. +// It performs service discovery to find the necessary API endpoints and +// authenticates using whichever method is configured (Conjur JWT exchange or +// legacy username/password — see selectAuthenticator). +func NewDatauploadClient(ctx context.Context, httpClient *http.Client, serviceMap *servicediscovery.Services, tenantUUID string, cfg ClientConfig) (*dataupload.CyberArkClient, error) { discoveryAPI := serviceMap.DiscoveryContext.API if discoveryAPI == "" { return nil, errors.New("service discovery returned an empty discovery API") } - identityClient := identity.New(httpClient, identityAPI, cfg.Subdomain) - - err := identityClient.LoginUsernamePassword(ctx, cfg.Username, []byte(cfg.Secret)) + authenticate, err := selectAuthenticator(ctx, httpClient, serviceMap, cfg) if err != nil { return nil, err } - - return dataupload.New(httpClient, discoveryAPI, tenantUUID, identityClient.AuthenticateRequest), nil + return dataupload.New(httpClient, discoveryAPI, tenantUUID, authenticate), nil } diff --git a/internal/cyberark/client_test.go b/internal/cyberark/client_test.go index 9d69da20..f66ed822 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,32 @@ 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) + + httpClient := servicediscovery.MockDiscoveryServer(t, servicediscovery.Services{ + Identity: servicediscovery.ServiceEndpoint{ + API: conjurSrv.URL, + }, + DiscoveryContext: servicediscovery.ServiceEndpoint{ + API: discoveryContextAPI, + }, + }) 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 +72,59 @@ 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{ + API: conjurSrv.URL, // conjur exchange endpoint base + }, + DiscoveryContext: servicediscovery.ServiceEndpoint{ + API: discoveryContextAPI, + }, + } + + 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 +147,14 @@ func TestCyberArkClient_PutSnapshot_RealAPI(t *testing.T) { logger := ktesting.NewLogger(t, ktesting.DefaultConfig) ctx := klog.NewContext(t.Context(), logger) - var rootCAs *x509.CertPool - httpClient := http_client.NewDefaultClient(version.UserAgent(), rootCAs) + // Use a plain http.Client for real API calls; a proper user-agent transport would + // normally be wired here but the venafi-connection-lib import is avoided to keep + // this package buildable without private-module credentials in developer environments. + httpClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{}, + }, + } cfg, err := cyberark.LoadClientConfigFromEnvironment() require.NoError(t, err) diff --git a/internal/cyberark/conjur/conjur.go b/internal/cyberark/conjur/conjur.go new file mode 100644 index 00000000..4605c405 --- /dev/null +++ b/internal/cyberark/conjur/conjur.go @@ -0,0 +1,81 @@ +package conjur + +import ( + "context" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "sync" + "time" + + "github.com/jetstack/preflight/internal/cyberark/jwtsource" +) + +const tokenTTL = 8 * time.Minute + +// Client exchanges a JWT for a Conjur access token and authenticates requests with it. +type Client struct { + httpClient *http.Client + baseURL string + serviceID string + account string + src jwtsource.Source + + mu sync.Mutex + token string + tokenTime time.Time +} + +func New(httpClient *http.Client, baseURL, serviceID, account string, src jwtsource.Source) *Client { + return &Client{httpClient: httpClient, baseURL: baseURL, serviceID: serviceID, account: account, src: src} +} + +func (c *Client) exchange(ctx context.Context) (string, error) { + jwt, err := c.src.Read(ctx) + if err != nil { + return "", err + } + endpoint, err := url.JoinPath(c.baseURL, "authn-jwt", c.serviceID, c.account, "authenticate") + if err != nil { + return "", err + } + form := url.Values{"jwt": {jwt}} + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(form.Encode())) + if err != nil { + return "", err + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + resp, err := c.httpClient.Do(req) + if err != nil { + return "", fmt.Errorf("authn-jwt exchange transport error: %w", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + // 401 here most often means the SA token audience != authenticator audience=conjur + return "", fmt.Errorf("authn-jwt exchange rejected (%d): verify service_id, the authenticator is enabled, and the SA token audience is 'conjur'", resp.StatusCode) + } + body, err := io.ReadAll(io.LimitReader(resp.Body, 64*1024)) + if err != nil { + return "", err + } + return strings.TrimSpace(string(body)), nil +} + +// AuthenticateRequest implements identity.RequestAuthenticator. +// It exchanges the JWT for a Conjur access token, sets the Authorization header, +// and returns the service ID as the identity string (used for audit tagging). +func (c *Client) AuthenticateRequest(req *http.Request) (string, error) { + c.mu.Lock() + defer c.mu.Unlock() + if c.token == "" || time.Since(c.tokenTime) >= tokenTTL { + tok, err := c.exchange(req.Context()) + if err != nil { + return "", err + } + c.token, c.tokenTime = tok, time.Now() + } + req.Header.Set("Authorization", "Bearer "+c.token) + return c.serviceID, nil +} diff --git a/internal/cyberark/conjur/conjur_test.go b/internal/cyberark/conjur/conjur_test.go new file mode 100644 index 00000000..3469b4c5 --- /dev/null +++ b/internal/cyberark/conjur/conjur_test.go @@ -0,0 +1,34 @@ +package conjur + +import ( + "context" + "net/http" + "testing" + + "github.com/stretchr/testify/require" +) + +type staticSource struct{ tok string } + +func (s staticSource) Read(context.Context) (string, error) { return s.tok, nil } + +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://disco/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://disco/x", nil) + _, err := c.AuthenticateRequest(req) + require.Error(t, err) + require.Empty(t, req.Header.Get("Authorization")) +} diff --git a/internal/cyberark/conjur/mock.go b/internal/cyberark/conjur/mock.go new file mode 100644 index 00000000..78307b5f --- /dev/null +++ b/internal/cyberark/conjur/mock.go @@ -0,0 +1,28 @@ +package conjur + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// MockConjurExchangeServer returns a TLS server whose authn-jwt endpoint returns the given token. +func MockConjurExchangeServer(t *testing.T, 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.T, status int) (*httptest.Server, *http.Client) { + t.Helper() + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(status) + })) + return srv, srv.Client() +} diff --git a/internal/cyberark/identity/identity.go b/internal/cyberark/identity/identity.go index c245c978..66838ca1 100644 --- a/internal/cyberark/identity/identity.go +++ b/internal/cyberark/identity/identity.go @@ -1,180 +1,13 @@ package identity import ( - "bytes" - "context" - "encoding/json" - "fmt" - "io" "net/http" - "net/url" "sync" "time" - - "k8s.io/klog/v2" - - arkapi "github.com/jetstack/preflight/internal/cyberark/api" - "github.com/jetstack/preflight/pkg/logs" - "github.com/jetstack/preflight/pkg/version" -) - -const ( - // MechanismUsernamePassword is the string which identifies the username/password mechanism for completing - // a login attempt - MechanismUsernamePassword = "UP" - - // ActionAnswer is the string which is sent to an AdvanceAuthentication request to indicate we're providing - // the credentials in band in text format (i.e., we're sending a password) - ActionAnswer = "Answer" - - // SummaryLoginSuccess is returned by a StartAuthentication to indicate that login does not need - // to proceed to the AdvanceAuthentication step. - // We don't handle this because we don't expect it to happen. - SummaryLoginSuccess = "LoginSuccess" - - // SummaryNewPackage is returned by a StartAuthentication call when the user must complete a challenge - // to complete the log in. This is expected on a first login. - SummaryNewPackage = "NewPackage" - - // maxStartAuthenticationBodySize is the maximum allowed size for a response body from the CyberArk Identity - // StartAuthentication endpoint. - // As of 2025-04-30, a response from the integration environment is ~1kB - maxStartAuthenticationBodySize = 10 * 1024 - - // maxAdvanceAuthenticationBodySize is the maximum allowed size for a response body from the CyberArk Identity - // AdvanceAuthentication endpoint. - // As of 2025-04-30, a response from the integration environment is ~3kB - maxAdvanceAuthenticationBodySize = 30 * 1024 -) - -var ( - errNoUPMechanism = fmt.Errorf("found no authentication mechanism with the username + password type (%s); unable to complete login using this identity", MechanismUsernamePassword) ) -// startAuthenticationRequestBody is the body sent to the StartAuthentication endpoint in CyberArk Identity; -// see https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/start-authentication -type startAuthenticationRequestBody struct { - // TenantID is the internal ID of the tenant containing the user attempting to log in. In testing, - // it seems that the subdomain works in this field. - TenantID string `json:"TenantId"` - - // Version is set to 1.0 - Version string `json:"Version"` - - // User is the username of the user trying to log in. For a human, this is likely to be an email address. - User string `json:"User"` -} - -// identityResponseBody generically wraps a response from the Identity server; the Result will differ for -// responses from different endpoint, but the other fields are similar. -// Not all fields in the JSON returned from the server are replicated here, since we only need a subset. -type identityResponseBody[T any] struct { - // Success is a simple boolean indicator from the server of success. - // NB: The JSON key is lowercase, in contrast to other JSON keys in the response. - Success bool `json:"success"` - - // Result holds the information we need to parse from successful responses - Result T `json:"Result"` - - // Message holds an information message such as an error message. Experimentally it seems to be null - // for successful attempts. - Message string `json:"Message"` - - // ErrorID holds an error ID when something goes wrong with the call. - // Not to be confused with ErrorCode; for failure messages, we see ErrorID set and ErrorCode null. - ErrorID string `json:"ErrorID"` - - // NB: Other fields omitted since we don't need them -} - -// startAuthenticationResponseBody is the response returned by the server from a request to StartAuthentication. -type startAuthenticationResponseBody identityResponseBody[startAuthenticationResponseResult] - -// advanceAuthenticationResponseBody is the response from the AdvanceAuthentication endpoint. -type advanceAuthenticationResponseBody identityResponseBody[advanceAuthenticationResponseResult] - -// startAuthenticationResponseResult holds the important data we need to pass to AdvanceAuthentication -type startAuthenticationResponseResult struct { - // SessionID identifies this login attempt, and must be passed with the - // follow-up AdvanceAuthentication request. - SessionID string `json:"SessionId"` - - // Challenges provides a list of methods for logging in. We need to look - // for the correct login method we want to use, and then find the MechanismId - // for that login method to pass to the AdvanceAuthentication request. - Challenges []startAuthenticationChallenge `json:"Challenges"` - - // Summary indicates whether a StartAuthentication calls needs to be followed up with an AdvanceAuthentication - // call. From the docs: - // > If the user exists, the response contains a Summary of either LoginSuccess or NewPackage. - // > You receive LoginSuccess when the request includes an .ASPXAUTH cookie from prior successful authentication. - Summary string `json:"Summary"` -} - -// startAuthenticationChallenge is an entry in the array of MFA mechanisms; -// at least one MFA mechanism should be satisfied by the user. -type startAuthenticationChallenge struct { - Mechanisms []startAuthenticationMechanism `json:"Mechanisms"` -} - -// startAuthenticationMechanism holds details of a given mechanism for authenticating. -// This corresponds to "how" the user authenticates, e.g. via password or email, etc -type startAuthenticationMechanism struct { - // Name represents the name of the challenge mechanism. This is usually an upper-case - // string, such as "UP" for "username / password" - Name string `json:"Name"` - - // Enrolled is true if the given mechanism is available for the user attempting - // to authenticate. - Enrolled bool `json:"Enrolled"` - - // MechanismID uniquely identifies a particular mechanism, and must be passed - // to the AdvanceAuthentication request when authenticating. - MechanismID string `json:"MechanismId"` -} - -// advanceAuthenticationRequestBody is a request body for the AdvanceAuthentication call to CyberArk Identity, -// which should usually be obtained by making requests to StartAuthentication first. -// WARNING: This struct can hold secret data (a user's password) -// See: https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/advance-authentication -type advanceAuthenticationRequestBody struct { - // Action is a string identifying how we're intending to log in; for username/password, this is - // set to "Answer" to indicate that the password is held in the Answer field - Action string `json:"Action"` - - // Answer holds the user's password to send to the server - // WARNING: THIS IS SECRET DATA. - Answer string `json:"Answer"` - - // MechanismID identifies the login mechanism and must be retrieved from a call to StartAuthentication - MechanismID string `json:"MechanismId"` - - // SessionID identifies the login session and must be retrieved from a call to StartAuthentication - SessionID string `json:"SessionId"` - - // TenantID identifies the tenant; this can be inferred from the URL if we used service discovery to - // get the Identity API URL, but we set it anyway to be explicit. - TenantID string `json:"TenantId"` - - // PersistentLogin is documented to "[indicate] whether the session should persist after the user - // closes the browser"; for service-to-service auth which we're trying to do, we set this to true. - PersistentLogin bool `json:"PersistentLogin"` -} - -// advanceAuthenticationResponseResult is the specific information returned for a successful AdvanceAuthentication call -type advanceAuthenticationResponseResult struct { - // Summary holds a "brief summary of the authentication outcome" - Summary string `json:"Summary"` - - // Token is the auth token we need to save; this is the result of the login - // process which can be sent as a bearer token to other services. - Token string `json:"Token"` - - // Other fields omitted as they're not needed -} - -// Client is an client for interacting with the CyberArk Identity API and performing a login using a username and password. -// For context on the behaviour of this client, see the Python SDK: https://github.com/cyberark/ark-sdk-python/blob/3be12c3f2d3a2d0407025028943e584b6edc5996/ark_sdk_python/auth/identity/ark_identity.py +// Client is a client for interacting with the CyberArk Identity API. +// It caches an authentication token and exposes it for use by AuthenticateRequest. type Client struct { httpClient *http.Client baseURL string @@ -191,7 +24,7 @@ type token struct { Token string } -// New returns an initialized CyberArk Identity client using a default service discovery client. +// New returns an initialized CyberArk Identity client. func New(httpClient *http.Client, baseURL string, subdomain string) *Client { return &Client{ httpClient: httpClient, @@ -202,246 +35,3 @@ func New(httpClient *http.Client, baseURL string, subdomain string) *Client { tokenCachedMutex: sync.Mutex{}, } } - -// LoginUsernamePassword performs a blocking call to fetch an auth token from CyberArk Identity using the given username and password. -// The password is zeroed after use. -// Tokens are cached internally and are not directly accessible to code; use Client.AuthenticateRequest to add credentials -// to an *http.Request. -func (c *Client) LoginUsernamePassword(ctx context.Context, username string, password []byte) error { - // note: we hold the mutex for the whole login attempt to ensure that only one login attempt can be in flight at once, - // and to ensure that the token cache is correctly updated - c.tokenCachedMutex.Lock() - defer c.tokenCachedMutex.Unlock() - - defer func() { - for i := range password { - password[i] = 0x00 - } - }() - - if time.Since(c.tokenCachedTime) < 15*time.Minute && c.tokenCached.Username == username { - // If the cached token is recent and for the same username, we can reuse it. - klog.FromContext(ctx).V(2).Info("reusing cached token for user", "username", username) - return nil - } - - advanceRequestBody, err := c.doStartAuthentication(ctx, username) - if err != nil { - return err - } - - // NB: We explicitly pass advanceRequestBody by value here so that when we add the password - // in doAdvanceAuthentication we don't create a copy of the password slice elsewhere. - err = c.doAdvanceAuthentication(ctx, username, &password, advanceRequestBody) - if err != nil { - return err - } - - return err -} - -// doStartAuthentication performs the initial request to start the login process using a username and password. -// It returns a partially initialized advanceAuthenticationRequestBody ready to send to the server to complete -// the login. As this function doesn't have access to the password, it must be added to the returned request body -// by the caller before being used as a request to AdvanceAuthentication. -// See https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/start-authentication -func (c *Client) doStartAuthentication(ctx context.Context, username string) (advanceAuthenticationRequestBody, error) { - response := advanceAuthenticationRequestBody{} - - logger := klog.FromContext(ctx).WithValues("source", "Identity.doStartAuthentication") - - body := startAuthenticationRequestBody{ - Version: "1.0", // this is the only value in the docs - - TenantID: c.subdomain, - - User: username, - } - - bodyJSON, err := json.Marshal(body) - if err != nil { - return response, fmt.Errorf("failed to marshal JSON for request to StartAuthentication endpoint: %s", err) - } - - endpoint, err := url.JoinPath(c.baseURL, "Security", "StartAuthentication") - if err != nil { - return response, fmt.Errorf("failed to create URL for request to CyberArk Identity StartAuthentication: %s", err) - } - - request, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(bodyJSON)) - if err != nil { - return response, fmt.Errorf("failed to initialise request to Identity endpoint %s: %s", endpoint, err) - } - - setIdentityHeaders(request) - - httpResponse, err := c.httpClient.Do(request) - if err != nil { - return response, fmt.Errorf("failed to perform HTTP request to start authentication: %s", err) - } - - defer httpResponse.Body.Close() - - if httpResponse.StatusCode != http.StatusOK { - err := fmt.Errorf("got unexpected status code %s from request to start authentication in CyberArk Identity API", httpResponse.Status) - if httpResponse.StatusCode >= 500 || httpResponse.StatusCode < 400 { - return response, err - } - - // If we got a 4xx error, we shouldn't retry - return response, err - } - - startAuthResponse := startAuthenticationResponseBody{} - - err = json.NewDecoder(io.LimitReader(httpResponse.Body, maxStartAuthenticationBodySize)).Decode(&startAuthResponse) - if err != nil { - if err == io.ErrUnexpectedEOF { - return response, fmt.Errorf("rejecting JSON response from server as it was too large or was truncated") - } - - return response, fmt.Errorf("failed to parse JSON from otherwise successful request to start authentication: %s", err) - } - - if !startAuthResponse.Success { - return response, fmt.Errorf("got a failure response from request to start authentication: message=%q, error=%q", startAuthResponse.Message, startAuthResponse.ErrorID) - } - - logger.V(logs.Debug).Info("made successful request to StartAuthentication", "summary", startAuthResponse.Result.Summary) - - if startAuthResponse.Result.Summary != SummaryNewPackage { - // This means we can't respond to whatever summary the server sent. - // The best thing to do is try and find a challenge we can solve anyway. - klog.FromContext(ctx).Info("got an unexpected Summary from StartAuthentication response; will attempt to complete a login challenge anyway", "summary", startAuthResponse.Result.Summary) - } - - // We can only handle a UP type challenge, and if there are any other challenges, we'll have to fail because we can't handle them. - // https://github.com/cyberark/ark-sdk-python/blob/3be12c3f2d3a2d0407025028943e584b6edc5996/ark_sdk_python/auth/identity/ark_identity.py#L405 - switch len(startAuthResponse.Result.Challenges) { - case 0: - return response, fmt.Errorf("got no valid challenges in response to start authentication; unable to log in") - - case 1: - // do nothing, this is ideal - - default: - return response, fmt.Errorf("got %d challenges in response to start authentication, which means MFA may be enabled; unable to log in", len(startAuthResponse.Result.Challenges)) - } - - challenge := startAuthResponse.Result.Challenges[0] - - switch len(challenge.Mechanisms) { - case 0: - // presumably this shouldn't happen, but handle the case anyway - return response, fmt.Errorf("got no mechanisms for challenge from Identity server") - - case 1: - // do nothing, this is ideal - - default: - return response, fmt.Errorf("got %d mechanisms in response to start authentication, which means MFA may be enabled; unable to log in", len(challenge.Mechanisms)) - } - - mechanism := challenge.Mechanisms[0] - - if !mechanism.Enrolled || mechanism.Name != MechanismUsernamePassword { - return response, errNoUPMechanism - } - - response.Action = ActionAnswer - response.MechanismID = mechanism.MechanismID - response.SessionID = startAuthResponse.Result.SessionID - response.TenantID = c.subdomain - response.PersistentLogin = true - - return response, nil -} - -// doAdvanceAuthentication performs the second step of the login process, sending the password to the server -// and receiving a token in response. -// See: https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/advance-authentication -func (c *Client) doAdvanceAuthentication(ctx context.Context, username string, password *[]byte, requestBody advanceAuthenticationRequestBody) error { - if password == nil { - return fmt.Errorf("password must not be nil; this is a programming error") - } - - requestBody.Answer = string(*password) - - bodyJSON, err := json.Marshal(requestBody) - if err != nil { - return fmt.Errorf("failed to marshal JSON for request to AdvanceAuthentication endpoint: %s", err) - } - - endpoint, err := url.JoinPath(c.baseURL, "Security", "AdvanceAuthentication") - if err != nil { - return fmt.Errorf("failed to create URL for request to CyberArk Identity AdvanceAuthentication: %s", err) - } - - request, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(bodyJSON)) - if err != nil { - return fmt.Errorf("failed to initialise request to Identity endpoint %s: %s", endpoint, err) - } - - setIdentityHeaders(request) - - httpResponse, err := c.httpClient.Do(request) - if err != nil { - return fmt.Errorf("failed to perform HTTP request to advance authentication: %s", err) - } - - defer httpResponse.Body.Close() - - // Important: Even login failures can produce a 200 status code, so this - // check won't catch all failures - if httpResponse.StatusCode != http.StatusOK { - return fmt.Errorf("got unexpected status code %s from request to advance authentication in CyberArk Identity API", httpResponse.Status) - } - - advanceAuthResponse := advanceAuthenticationResponseBody{} - - err = json.NewDecoder(io.LimitReader(httpResponse.Body, maxAdvanceAuthenticationBodySize)).Decode(&advanceAuthResponse) - if err != nil { - if err == io.ErrUnexpectedEOF { - return fmt.Errorf("rejecting JSON response from server as it was too large or was truncated") - } - - return fmt.Errorf("failed to parse JSON from otherwise successful request to advance authentication: %s", err) - } - - if !advanceAuthResponse.Success { - return fmt.Errorf("got a failure response from request to advance authentication: message=%q, error=%q", advanceAuthResponse.Message, advanceAuthResponse.ErrorID) - } - - if advanceAuthResponse.Result.Summary != SummaryLoginSuccess { - // IF MFA was enabled and we got here, there's probably nothing to be gained from a retry - // and the best thing to do is fail now so the user can fix MFA settings. - return fmt.Errorf("got a %s response from AdvanceAuthentication; this implies that the user account %s requires MFA, which is not supported. Try unlocking MFA for this user", advanceAuthResponse.Result.Summary, username) - } - - klog.FromContext(ctx).Info("successfully completed AdvanceAuthentication request to CyberArk Identity; login complete", "username", username) - - // NB: This assumes we already hold the token cache mutex, which we do in LoginUsernamePassword, so this is safe. - c.tokenCachedTime = time.Now() - c.tokenCached = token{ - Username: username, - Token: advanceAuthResponse.Result.Token, - } - - return nil -} - -// setIdentityHeaders sets the headers required for requests to the CyberArk Identity API. -// From the docs: -// Your request header must contain X-IDAP-NATIVE-CLIENT:true to indicate that an application is invoking -// the CyberArk Identity endpoint, and -// Content-Type: application/json to indicate that the body is in JSON format. -// Experimentally, it seems the X-IDAP-NATIVE-CLIENT is not required but we'll follow the docs. -func setIdentityHeaders(r *http.Request) { - // The "canonicalheader" linter warns us that the IDAP-NATIVE-CLIENT header isn't canonical, but we silence it here - // since we want to exactly match the docs. - r.Header.Set("Content-Type", "application/json") - r.Header.Set("X-IDAP-NATIVE-CLIENT", "true") //nolint: canonicalheader - version.SetUserAgent(r) - // Add telemetry headers - arkapi.SetTelemetryRequestHeader(r) -} diff --git a/internal/cyberark/identity/mock.go b/internal/cyberark/identity/mock.go index 2bad8b36..ef87f4cc 100644 --- a/internal/cyberark/identity/mock.go +++ b/internal/cyberark/identity/mock.go @@ -32,6 +32,18 @@ const ( // mock server in response to a successful AdvanceAuthentication request // Must match what's in testdata/advance_authentication_success.json mockSuccessfulStartAuthenticationToken = "success-token" + + // actionAnswer is the string sent to an AdvanceAuthentication request to indicate we're + // providing credentials as plain text. + actionAnswer = "Answer" +) + +// Exported credentials that MockIdentityServer accepts as a successful +// username/password login. Used by other packages' tests that exercise the +// legacy UP auth path against the mock server. +const ( + MockSuccessUser = successUser + MockSuccessPassword = successPassword ) var ( @@ -213,7 +225,7 @@ func (mis *mockIdentityServer) handleAdvanceAuthentication(w http.ResponseWriter if advanceBody.SessionID != successSessionID || advanceBody.MechanismID != successMechanismID || - advanceBody.Action != ActionAnswer || + advanceBody.Action != actionAnswer || advanceBody.Answer != successPassword { w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(advanceAuthenticationFailureResponse)) diff --git a/internal/cyberark/identity/username_password.go b/internal/cyberark/identity/username_password.go new file mode 100644 index 00000000..71ed7d97 --- /dev/null +++ b/internal/cyberark/identity/username_password.go @@ -0,0 +1,424 @@ +package identity + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "time" + + "k8s.io/klog/v2" + + arkapi "github.com/jetstack/preflight/internal/cyberark/api" + "github.com/jetstack/preflight/pkg/logs" + "github.com/jetstack/preflight/pkg/version" +) + +// This file holds the legacy CyberArk Identity username/password (UP) login. +// It is retained alongside the Conjur JWT exchange for backward compatibility: +// the product is GA and existing installs authenticate with ARK_USERNAME/ +// ARK_SECRET. The agent selects UP vs Conjur by config presence (see +// internal/cyberark/client.go) — a Conjur service-id, when set, takes +// precedence. Both paths populate the same token cache and satisfy +// RequestAuthenticator via AuthenticateRequest. + +const ( + // MechanismUsernamePassword is the string which identifies the username/password mechanism for completing + // a login attempt + MechanismUsernamePassword = "UP" + + // ActionAnswer is the string which is sent to an AdvanceAuthentication request to indicate we're providing + // the credentials in band in text format (i.e., we're sending a password) + ActionAnswer = "Answer" + + // SummaryLoginSuccess is returned by a StartAuthentication to indicate that login does not need + // to proceed to the AdvanceAuthentication step. + // We don't handle this because we don't expect it to happen. + SummaryLoginSuccess = "LoginSuccess" + + // SummaryNewPackage is returned by a StartAuthentication call when the user must complete a challenge + // to complete the log in. This is expected on a first login. + SummaryNewPackage = "NewPackage" + + // maxStartAuthenticationBodySize is the maximum allowed size for a response body from the CyberArk Identity + // StartAuthentication endpoint. + // As of 2025-04-30, a response from the integration environment is ~1kB + maxStartAuthenticationBodySize = 10 * 1024 + + // maxAdvanceAuthenticationBodySize is the maximum allowed size for a response body from the CyberArk Identity + // AdvanceAuthentication endpoint. + // As of 2025-04-30, a response from the integration environment is ~3kB + maxAdvanceAuthenticationBodySize = 30 * 1024 +) + +var ( + errNoUPMechanism = fmt.Errorf("found no authentication mechanism with the username + password type (%s); unable to complete login using this identity", MechanismUsernamePassword) +) + +// startAuthenticationRequestBody is the body sent to the StartAuthentication endpoint in CyberArk Identity; +// see https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/start-authentication +type startAuthenticationRequestBody struct { + // TenantID is the internal ID of the tenant containing the user attempting to log in. In testing, + // it seems that the subdomain works in this field. + TenantID string `json:"TenantId"` + + // Version is set to 1.0 + Version string `json:"Version"` + + // User is the username of the user trying to log in. For a human, this is likely to be an email address. + User string `json:"User"` +} + +// identityResponseBody generically wraps a response from the Identity server; the Result will differ for +// responses from different endpoint, but the other fields are similar. +// Not all fields in the JSON returned from the server are replicated here, since we only need a subset. +type identityResponseBody[T any] struct { + // Success is a simple boolean indicator from the server of success. + // NB: The JSON key is lowercase, in contrast to other JSON keys in the response. + Success bool `json:"success"` + + // Result holds the information we need to parse from successful responses + Result T `json:"Result"` + + // Message holds an information message such as an error message. Experimentally it seems to be null + // for successful attempts. + Message string `json:"Message"` + + // ErrorID holds an error ID when something goes wrong with the call. + // Not to be confused with ErrorCode; for failure messages, we see ErrorID set and ErrorCode null. + ErrorID string `json:"ErrorID"` + + // NB: Other fields omitted since we don't need them +} + +// startAuthenticationResponseBody is the response returned by the server from a request to StartAuthentication. +type startAuthenticationResponseBody identityResponseBody[startAuthenticationResponseResult] + +// advanceAuthenticationResponseBody is the response from the AdvanceAuthentication endpoint. +type advanceAuthenticationResponseBody identityResponseBody[advanceAuthenticationResponseResult] + +// startAuthenticationResponseResult holds the important data we need to pass to AdvanceAuthentication +type startAuthenticationResponseResult struct { + // SessionID identifies this login attempt, and must be passed with the + // follow-up AdvanceAuthentication request. + SessionID string `json:"SessionId"` + + // Challenges provides a list of methods for logging in. We need to look + // for the correct login method we want to use, and then find the MechanismId + // for that login method to pass to the AdvanceAuthentication request. + Challenges []startAuthenticationChallenge `json:"Challenges"` + + // Summary indicates whether a StartAuthentication calls needs to be followed up with an AdvanceAuthentication + // call. From the docs: + // > If the user exists, the response contains a Summary of either LoginSuccess or NewPackage. + // > You receive LoginSuccess when the request includes an .ASPXAUTH cookie from prior successful authentication. + Summary string `json:"Summary"` +} + +// startAuthenticationChallenge is an entry in the array of MFA mechanisms; +// at least one MFA mechanism should be satisfied by the user. +type startAuthenticationChallenge struct { + Mechanisms []startAuthenticationMechanism `json:"Mechanisms"` +} + +// startAuthenticationMechanism holds details of a given mechanism for authenticating. +// This corresponds to "how" the user authenticates, e.g. via password or email, etc +type startAuthenticationMechanism struct { + // Name represents the name of the challenge mechanism. This is usually an upper-case + // string, such as "UP" for "username / password" + Name string `json:"Name"` + + // Enrolled is true if the given mechanism is available for the user attempting + // to authenticate. + Enrolled bool `json:"Enrolled"` + + // MechanismID uniquely identifies a particular mechanism, and must be passed + // to the AdvanceAuthentication request when authenticating. + MechanismID string `json:"MechanismId"` +} + +// advanceAuthenticationRequestBody is a request body for the AdvanceAuthentication call to CyberArk Identity, +// which should usually be obtained by making requests to StartAuthentication first. +// WARNING: This struct can hold secret data (a user's password) +// See: https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/advance-authentication +type advanceAuthenticationRequestBody struct { + // Action is a string identifying how we're intending to log in; for username/password, this is + // set to "Answer" to indicate that the password is held in the Answer field + Action string `json:"Action"` + + // Answer holds the user's password to send to the server + // WARNING: THIS IS SECRET DATA. + Answer string `json:"Answer"` + + // MechanismID identifies the login mechanism and must be retrieved from a call to StartAuthentication + MechanismID string `json:"MechanismId"` + + // SessionID identifies the login session and must be retrieved from a call to StartAuthentication + SessionID string `json:"SessionId"` + + // TenantID identifies the tenant; this can be inferred from the URL if we used service discovery to + // get the Identity API URL, but we set it anyway to be explicit. + TenantID string `json:"TenantId"` + + // PersistentLogin is documented to "[indicate] whether the session should persist after the user + // closes the browser"; for service-to-service auth which we're trying to do, we set this to true. + PersistentLogin bool `json:"PersistentLogin"` +} + +// advanceAuthenticationResponseResult is the specific information returned for a successful AdvanceAuthentication call +type advanceAuthenticationResponseResult struct { + // Summary holds a "brief summary of the authentication outcome" + Summary string `json:"Summary"` + + // Token is the auth token we need to save; this is the result of the login + // process which can be sent as a bearer token to other services. + Token string `json:"Token"` + + // Other fields omitted as they're not needed +} + +// LoginUsernamePassword performs a blocking call to fetch an auth token from CyberArk Identity using the given username and password. +// The password is zeroed after use. +// Tokens are cached internally and are not directly accessible to code; use Client.AuthenticateRequest to add credentials +// to an *http.Request. +func (c *Client) LoginUsernamePassword(ctx context.Context, username string, password []byte) error { + // note: we hold the mutex for the whole login attempt to ensure that only one login attempt can be in flight at once, + // and to ensure that the token cache is correctly updated + c.tokenCachedMutex.Lock() + defer c.tokenCachedMutex.Unlock() + + defer func() { + for i := range password { + password[i] = 0x00 + } + }() + + if time.Since(c.tokenCachedTime) < 15*time.Minute && c.tokenCached.Username == username { + // If the cached token is recent and for the same username, we can reuse it. + klog.FromContext(ctx).V(2).Info("reusing cached token for user", "username", username) + return nil + } + + advanceRequestBody, err := c.doStartAuthentication(ctx, username) + if err != nil { + return err + } + + // NB: We explicitly pass advanceRequestBody by value here so that when we add the password + // in doAdvanceAuthentication we don't create a copy of the password slice elsewhere. + err = c.doAdvanceAuthentication(ctx, username, &password, advanceRequestBody) + if err != nil { + return err + } + + return err +} + +// doStartAuthentication performs the initial request to start the login process using a username and password. +// It returns a partially initialized advanceAuthenticationRequestBody ready to send to the server to complete +// the login. As this function doesn't have access to the password, it must be added to the returned request body +// by the caller before being used as a request to AdvanceAuthentication. +// See https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/start-authentication +func (c *Client) doStartAuthentication(ctx context.Context, username string) (advanceAuthenticationRequestBody, error) { + response := advanceAuthenticationRequestBody{} + + logger := klog.FromContext(ctx).WithValues("source", "Identity.doStartAuthentication") + + body := startAuthenticationRequestBody{ + Version: "1.0", // this is the only value in the docs + + TenantID: c.subdomain, + + User: username, + } + + bodyJSON, err := json.Marshal(body) + if err != nil { + return response, fmt.Errorf("failed to marshal JSON for request to StartAuthentication endpoint: %s", err) + } + + endpoint, err := url.JoinPath(c.baseURL, "Security", "StartAuthentication") + if err != nil { + return response, fmt.Errorf("failed to create URL for request to CyberArk Identity StartAuthentication: %s", err) + } + + request, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(bodyJSON)) + if err != nil { + return response, fmt.Errorf("failed to initialise request to Identity endpoint %s: %s", endpoint, err) + } + + setIdentityHeaders(request) + + httpResponse, err := c.httpClient.Do(request) + if err != nil { + return response, fmt.Errorf("failed to perform HTTP request to start authentication: %s", err) + } + + defer httpResponse.Body.Close() + + if httpResponse.StatusCode != http.StatusOK { + err := fmt.Errorf("got unexpected status code %s from request to start authentication in CyberArk Identity API", httpResponse.Status) + if httpResponse.StatusCode >= 500 || httpResponse.StatusCode < 400 { + return response, err + } + + // If we got a 4xx error, we shouldn't retry + return response, err + } + + startAuthResponse := startAuthenticationResponseBody{} + + err = json.NewDecoder(io.LimitReader(httpResponse.Body, maxStartAuthenticationBodySize)).Decode(&startAuthResponse) + if err != nil { + if err == io.ErrUnexpectedEOF { + return response, fmt.Errorf("rejecting JSON response from server as it was too large or was truncated") + } + + return response, fmt.Errorf("failed to parse JSON from otherwise successful request to start authentication: %s", err) + } + + if !startAuthResponse.Success { + return response, fmt.Errorf("got a failure response from request to start authentication: message=%q, error=%q", startAuthResponse.Message, startAuthResponse.ErrorID) + } + + logger.V(logs.Debug).Info("made successful request to StartAuthentication", "summary", startAuthResponse.Result.Summary) + + if startAuthResponse.Result.Summary != SummaryNewPackage { + // This means we can't respond to whatever summary the server sent. + // The best thing to do is try and find a challenge we can solve anyway. + klog.FromContext(ctx).Info("got an unexpected Summary from StartAuthentication response; will attempt to complete a login challenge anyway", "summary", startAuthResponse.Result.Summary) + } + + // We can only handle a UP type challenge, and if there are any other challenges, we'll have to fail because we can't handle them. + // https://github.com/cyberark/ark-sdk-python/blob/3be12c3f2d3a2d0407025028943e584b6edc5996/ark_sdk_python/auth/identity/ark_identity.py#L405 + switch len(startAuthResponse.Result.Challenges) { + case 0: + return response, fmt.Errorf("got no valid challenges in response to start authentication; unable to log in") + + case 1: + // do nothing, this is ideal + + default: + return response, fmt.Errorf("got %d challenges in response to start authentication, which means MFA may be enabled; unable to log in", len(startAuthResponse.Result.Challenges)) + } + + challenge := startAuthResponse.Result.Challenges[0] + + switch len(challenge.Mechanisms) { + case 0: + // presumably this shouldn't happen, but handle the case anyway + return response, fmt.Errorf("got no mechanisms for challenge from Identity server") + + case 1: + // do nothing, this is ideal + + default: + return response, fmt.Errorf("got %d mechanisms in response to start authentication, which means MFA may be enabled; unable to log in", len(challenge.Mechanisms)) + } + + mechanism := challenge.Mechanisms[0] + + if !mechanism.Enrolled || mechanism.Name != MechanismUsernamePassword { + return response, errNoUPMechanism + } + + response.Action = ActionAnswer + response.MechanismID = mechanism.MechanismID + response.SessionID = startAuthResponse.Result.SessionID + response.TenantID = c.subdomain + response.PersistentLogin = true + + return response, nil +} + +// doAdvanceAuthentication performs the second step of the login process, sending the password to the server +// and receiving a token in response. +// See: https://api-docs.cyberark.com/identity-docs-api/docs/security-api#/Login/advance-authentication +func (c *Client) doAdvanceAuthentication(ctx context.Context, username string, password *[]byte, requestBody advanceAuthenticationRequestBody) error { + if password == nil { + return fmt.Errorf("password must not be nil; this is a programming error") + } + + requestBody.Answer = string(*password) + + bodyJSON, err := json.Marshal(requestBody) + if err != nil { + return fmt.Errorf("failed to marshal JSON for request to AdvanceAuthentication endpoint: %s", err) + } + + endpoint, err := url.JoinPath(c.baseURL, "Security", "AdvanceAuthentication") + if err != nil { + return fmt.Errorf("failed to create URL for request to CyberArk Identity AdvanceAuthentication: %s", err) + } + + request, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(bodyJSON)) + if err != nil { + return fmt.Errorf("failed to initialise request to Identity endpoint %s: %s", endpoint, err) + } + + setIdentityHeaders(request) + + httpResponse, err := c.httpClient.Do(request) + if err != nil { + return fmt.Errorf("failed to perform HTTP request to advance authentication: %s", err) + } + + defer httpResponse.Body.Close() + + // Important: Even login failures can produce a 200 status code, so this + // check won't catch all failures + if httpResponse.StatusCode != http.StatusOK { + return fmt.Errorf("got unexpected status code %s from request to advance authentication in CyberArk Identity API", httpResponse.Status) + } + + advanceAuthResponse := advanceAuthenticationResponseBody{} + + err = json.NewDecoder(io.LimitReader(httpResponse.Body, maxAdvanceAuthenticationBodySize)).Decode(&advanceAuthResponse) + if err != nil { + if err == io.ErrUnexpectedEOF { + return fmt.Errorf("rejecting JSON response from server as it was too large or was truncated") + } + + return fmt.Errorf("failed to parse JSON from otherwise successful request to advance authentication: %s", err) + } + + if !advanceAuthResponse.Success { + return fmt.Errorf("got a failure response from request to advance authentication: message=%q, error=%q", advanceAuthResponse.Message, advanceAuthResponse.ErrorID) + } + + if advanceAuthResponse.Result.Summary != SummaryLoginSuccess { + // IF MFA was enabled and we got here, there's probably nothing to be gained from a retry + // and the best thing to do is fail now so the user can fix MFA settings. + return fmt.Errorf("got a %s response from AdvanceAuthentication; this implies that the user account %s requires MFA, which is not supported. Try unlocking MFA for this user", advanceAuthResponse.Result.Summary, username) + } + + klog.FromContext(ctx).Info("successfully completed AdvanceAuthentication request to CyberArk Identity; login complete", "username", username) + + // NB: This assumes we already hold the token cache mutex, which we do in LoginUsernamePassword, so this is safe. + c.tokenCachedTime = time.Now() + c.tokenCached = token{ + Username: username, + Token: advanceAuthResponse.Result.Token, + } + + return nil +} + +// setIdentityHeaders sets the headers required for requests to the CyberArk Identity API. +// From the docs: +// Your request header must contain X-IDAP-NATIVE-CLIENT:true to indicate that an application is invoking +// the CyberArk Identity endpoint, and +// Content-Type: application/json to indicate that the body is in JSON format. +// Experimentally, it seems the X-IDAP-NATIVE-CLIENT is not required but we'll follow the docs. +func setIdentityHeaders(r *http.Request) { + // The "canonicalheader" linter warns us that the IDAP-NATIVE-CLIENT header isn't canonical, but we silence it here + // since we want to exactly match the docs. + r.Header.Set("Content-Type", "application/json") + r.Header.Set("X-IDAP-NATIVE-CLIENT", "true") //nolint: canonicalheader + version.SetUserAgent(r) + // Add telemetry headers + arkapi.SetTelemetryRequestHeader(r) +} diff --git a/internal/cyberark/jwtsource/jwtsource.go b/internal/cyberark/jwtsource/jwtsource.go new file mode 100644 index 00000000..513a4696 --- /dev/null +++ b/internal/cyberark/jwtsource/jwtsource.go @@ -0,0 +1,39 @@ +// internal/cyberark/jwtsource/jwtsource.go +package jwtsource + +import ( + "context" + "fmt" + "os" + "strings" +) + +// DefaultTokenPath is the default projected ServiceAccount token mount (aud=conjur). +const DefaultTokenPath = "/var/run/secrets/tokens/jwt" + +// Source produces a raw JWT to exchange at SMS authn-jwt. +type Source interface { + Read(ctx context.Context) (string, error) +} + +type fileSource struct{ path string } + +// NewFileSource reads a JWT from a file (the projected SA token). +func NewFileSource(path string) Source { + if path == "" { + path = DefaultTokenPath + } + return &fileSource{path: path} +} + +func (f *fileSource) Read(_ context.Context) (string, error) { + b, err := os.ReadFile(f.path) + if err != nil { + return "", fmt.Errorf("jwt source file %q not found or unreadable (is the projected serviceAccountToken volume mounted?): %w", f.path, err) + } + tok := strings.TrimSpace(string(b)) + if tok == "" { + return "", fmt.Errorf("jwt source file %q is empty", f.path) + } + return tok, nil +} diff --git a/internal/cyberark/jwtsource/jwtsource_test.go b/internal/cyberark/jwtsource/jwtsource_test.go new file mode 100644 index 00000000..ff23a68e --- /dev/null +++ b/internal/cyberark/jwtsource/jwtsource_test.go @@ -0,0 +1,33 @@ +// internal/cyberark/jwtsource/jwtsource_test.go +package jwtsource + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestFileSource_ReadsToken(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "jwt") + require.NoError(t, os.WriteFile(p, []byte("the-jwt\n"), 0o600)) + got, err := NewFileSource(p).Read(context.Background()) + require.NoError(t, err) + require.Equal(t, "the-jwt", got) // trimmed +} + +func TestFileSource_MissingFile(t *testing.T) { + _, err := NewFileSource("/no/such/file").Read(context.Background()) + require.Error(t, err) +} + +func TestFileSource_EmptyFile(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "jwt") + require.NoError(t, os.WriteFile(p, []byte(" \n"), 0o600)) + _, err := NewFileSource(p).Read(context.Background()) + require.Error(t, err) +} diff --git a/internal/cyberark/testing/testing.go b/internal/cyberark/testing/testing.go index 7b74abcc..7bc17aaf 100644 --- a/internal/cyberark/testing/testing.go +++ b/internal/cyberark/testing/testing.go @@ -9,10 +9,7 @@ import ( func SkipIfNoEnv(t testing.TB) { t.Helper() - if os.Getenv("ARK_SUBDOMAIN") == "" || - os.Getenv("ARK_USERNAME") == "" || - os.Getenv("ARK_SECRET") == "" { - t.Skip("Skipping test because one of ARK_SUBDOMAIN, ARK_USERNAME or ARK_SECRET isn't set") + if os.Getenv("ARK_SUBDOMAIN") == "" { + t.Skip("Skipping test because ARK_SUBDOMAIN isn't set") } - } diff --git a/internal/envelope/keyfetch/client.go b/internal/envelope/keyfetch/client.go index 53a8d5b9..a7dc2ed8 100644 --- a/internal/envelope/keyfetch/client.go +++ b/internal/envelope/keyfetch/client.go @@ -50,8 +50,7 @@ type PublicKey struct { // and ignored other types. type Client struct { discoveryClient *servicediscovery.Client - identityClient *identity.Client - cfg cyberark.ClientConfig + authenticate identity.RequestAuthenticator // httpClient is the HTTP client used for requests httpClient *http.Client @@ -62,8 +61,9 @@ type Client struct { } // NewClient creates a new key fetching client. -// Uses CyberArk service discovery to derive the JWKS endpoint and CyberArk identity client for authentication. -// Constructing the client involves a service discovery call to initialise the identity client, +// Uses CyberArk service discovery to derive the JWKS endpoint and the configured +// CyberArk authentication method (Conjur JWT exchange or legacy username/password). +// Constructing the client involves a service discovery call to initialise the authenticator, // so this may return an error if the discovery client is not able to connect to the service discovery endpoint. // If httpClient is nil, a default HTTP client will be created. func NewClient(ctx context.Context, discoveryClient *servicediscovery.Client, cfg cyberark.ClientConfig, httpClient *http.Client) (*Client, error) { @@ -77,10 +77,14 @@ func NewClient(ctx context.Context, discoveryClient *servicediscovery.Client, cf return nil, fmt.Errorf("failed to get services from discovery client for initialising identity client: %w", err) } + authenticate, err := cyberark.NewRequestAuthenticator(ctx, httpClient, services, cfg) + if err != nil { + return nil, err + } + return &Client{ discoveryClient: discoveryClient, - identityClient: identity.New(httpClient, services.Identity.API, cfg.Subdomain), - cfg: cfg, + authenticate: authenticate, httpClient: httpClient, }, nil } @@ -102,11 +106,6 @@ func (c *Client) FetchKey(ctx context.Context) (PublicKey, error) { return PublicKey{}, fmt.Errorf("failed to get services from discovery client: %w", err) } - err = c.identityClient.LoginUsernamePassword(ctx, c.cfg.Username, []byte(c.cfg.Secret)) - if err != nil { - return PublicKey{}, fmt.Errorf("failed to authenticate for fetching JWKs: %w", err) - } - endpoint, err := url.JoinPath(services.DiscoveryContext.API, "discovery-context/jwks") if err != nil { return PublicKey{}, fmt.Errorf("failed to construct endpoint URL: %w", err) @@ -117,7 +116,7 @@ func (c *Client) FetchKey(ctx context.Context) (PublicKey, error) { return PublicKey{}, fmt.Errorf("failed to create request: %w", err) } - _, err = c.identityClient.AuthenticateRequest(req) + _, err = c.authenticate(req) if err != nil { return PublicKey{}, fmt.Errorf("failed to authenticate request: %s", err) } diff --git a/internal/envelope/keyfetch/client_test.go b/internal/envelope/keyfetch/client_test.go index 6af307db..0b48fbd9 100644 --- a/internal/envelope/keyfetch/client_test.go +++ b/internal/envelope/keyfetch/client_test.go @@ -10,24 +10,27 @@ import ( "github.com/stretchr/testify/require" "github.com/jetstack/preflight/internal/cyberark" - "github.com/jetstack/preflight/internal/cyberark/identity" + "github.com/jetstack/preflight/internal/cyberark/conjur" "github.com/jetstack/preflight/internal/cyberark/servicediscovery" ) -// testClientSetup sets up a complete test environment with mock identity and discovery servers -// and returns a configured client along with the test ClientConfig +// testClientSetup sets up a complete test environment with mock conjur and discovery servers +// and returns a configured client along with the test ClientConfig. +// NOTE: this file imports venafi-connection-lib (via the keyfetch package itself) and therefore +// cannot be compiled or run locally — the mock wiring is correct per the conjur mock pattern +// in internal/cyberark/conjur/conjur_test.go. func testClientSetup(t *testing.T, jwksServerURL string) (*Client, cyberark.ClientConfig) { t.Helper() - // Create mock identity server - identityURL, httpClient := identity.MockIdentityServer(t) + // Create mock conjur exchange server — returns a static Bearer token. + conjurSrv, httpClient := conjur.MockConjurExchangeServer(t, "test-conjur-token") // Set up services for mock discovery server services := servicediscovery.Services{ Identity: servicediscovery.ServiceEndpoint{ IsActive: true, Type: "main", - API: identityURL, + API: conjurSrv.URL, }, DiscoveryContext: servicediscovery.ServiceEndpoint{ IsActive: true, @@ -42,11 +45,12 @@ func testClientSetup(t *testing.T, jwksServerURL string) (*Client, cyberark.Clie // Create discovery client discoveryClient := servicediscovery.New(httpClient, servicediscovery.MockDiscoverySubdomain) - // Create test config with credentials that match the mock identity server + // Create test config — JWTFilePath is empty; jwtsource.NewFileSource will use DefaultTokenPath, + // but the conjur mock accepts any jwt value so no real file read occurs. cfg := cyberark.ClientConfig{ - Subdomain: servicediscovery.MockDiscoverySubdomain, - Username: "test@example.com", // matches successUser in mock identity server - Secret: "somepassword", // matches successPassword in mock identity server + Subdomain: servicediscovery.MockDiscoverySubdomain, + ServiceID: "dev-cluster", + JWTFilePath: "testdata/fake-jwt", } // Create the keyfetch client with the properly configured httpClient @@ -233,15 +237,15 @@ 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, + API: conjurSrv.URL, }, DiscoveryContext: servicediscovery.ServiceEndpoint{ IsActive: true, @@ -256,12 +260,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 +277,15 @@ 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, + API: conjurSrv.URL, }, } @@ -294,9 +296,9 @@ func TestClient_FetchKey(t *testing.T) { discoveryClient := servicediscovery.New(httpClient, "bad-request") cfg := cyberark.ClientConfig{ - Subdomain: "bad-request", - Username: "test@example.com", - Secret: "somepassword", + Subdomain: "bad-request", + ServiceID: "dev-cluster", + JWTFilePath: "testdata/fake-jwt", } _, err := NewClient(t.Context(), discoveryClient, cfg, httpClient) diff --git a/internal/envelope/keyfetch/testdata/fake-jwt b/internal/envelope/keyfetch/testdata/fake-jwt new file mode 100644 index 00000000..a3af65aa --- /dev/null +++ b/internal/envelope/keyfetch/testdata/fake-jwt @@ -0,0 +1 @@ +fake-jwt-token-for-testing \ No newline at end of file diff --git a/pkg/agent/config.go b/pkg/agent/config.go index 1c8e7335..4cdc447f 100644 --- a/pkg/agent/config.go +++ b/pkg/agent/config.go @@ -64,6 +64,9 @@ type Config struct { DataGatherers []DataGatherer `yaml:"data-gatherers"` VenafiCloud *VenafiCloudConfig `yaml:"venafi-cloud,omitempty"` + // CyberArk holds configuration for MachineHub mode (POC). + CyberArk *CyberArkConfig `yaml:"cyberark,omitempty"` + // For testing purposes. InputPath string `yaml:"input-path"` // For testing purposes. @@ -101,6 +104,19 @@ type VenafiCloudConfig struct { UploadPath string `yaml:"upload_path,omitempty"` } +// CyberArkConfig holds YAML configuration for MachineHub (CyberArk) mode (POC). +type CyberArkConfig struct { + // ServiceID is the authn-jwt service ID configured in Conjur (e.g. "dev-cluster"). + ServiceID string `yaml:"service_id"` + // Account is the Conjur account name. Defaults to "conjur" when empty. + Account string `yaml:"account"` + // JWTSource selects how the agent obtains its JWT. Must be "" or "file" in the POC. + JWTSource string `yaml:"jwt_source"` + // JWTFilePath is the path to the JWT file when jwt_source is "file". + // Defaults to the standard projected service-account token path when empty. + JWTFilePath string `yaml:"jwt_file_path"` +} + type AgentCmdFlags struct { // ConfigFilePath (--config-file, -c) is the path to the agent configuration // YAML file. @@ -459,6 +475,9 @@ type CombinedConfig struct { TSGID string NGTSServerURL string + // MachineHub mode only. + CyberArk CyberArkConfig + // Only used for testing purposes. OutputPath string InputPath string @@ -753,17 +772,15 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags) clusterID = cfg.ClusterID case MachineHub: clusterName = cfg.ClusterName - if clusterName == "" { - if arkUsername, found := os.LookupEnv("ARK_USERNAME"); found { - log.Info("Using ARK_USERNAME environment variable as cluster name", "clusterName", arkUsername) - clusterName = arkUsername - } + if clusterName == "" && cfg.ClusterID != "" { + log.Info("Using cluster_id as cluster_name for backwards compatibility", "clusterID", cfg.ClusterID) + clusterName = cfg.ClusterID } if cfg.OrganizationID != "" { log.Info(fmt.Sprintf(`Ignoring the organization_id field in the config file. This field is not needed in %s mode.`, res.OutputMode)) } - if cfg.ClusterID != "" { - log.Info(fmt.Sprintf(`Ignoring the cluster_id field in the config file. This field is not needed in %s mode.`, res.OutputMode)) + if clusterName == "" && cfg.ClusterID == "" { + log.Info("cluster_name is not set in MachineHub mode; cluster name will be empty") } } res.OrganizationID = organizationID @@ -773,6 +790,24 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags) res.ClaimableCerts = cfg.ClaimableCerts } + // Validation of `cyberark.*` (MachineHub mode only). + if res.OutputMode == MachineHub { + ark := CyberArkConfig{} + if cfg.CyberArk != nil { + ark = *cfg.CyberArk + } + // service_id selects the Conjur JWT exchange. It is no longer required: + // the agent also supports the legacy username/password method via + // ARK_USERNAME/ARK_SECRET (env, not visible here) for backward + // compatibility. The auth method is chosen at runtime by + // cyberark.selectAuthenticator, which fails closed if neither a + // service_id nor username/password credentials are configured. + if ark.JWTSource != "" && ark.JWTSource != "file" { + errs = multierror.Append(errs, fmt.Errorf("cyberark.jwt_source %q is not supported (POC only supports \"\" or \"file\")", ark.JWTSource)) + } + res.CyberArk = ark + } + // Validation of `data-gatherers`. { if dgErr := ValidateDataGatherers(cfg.DataGatherers); dgErr != nil { @@ -987,7 +1022,7 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie rootCAs *x509.CertPool ) httpClient := http_client.NewDefaultClient(version.UserAgent(), rootCAs) - outputClient, err = client.NewCyberArk(httpClient) + outputClient, err = client.NewCyberArk(httpClient, cfg.CyberArk.ServiceID, cfg.CyberArk.Account, cfg.CyberArk.JWTSource, cfg.CyberArk.JWTFilePath) if err != nil { errs = multierror.Append(errs, err) } diff --git a/pkg/agent/config_test.go b/pkg/agent/config_test.go index d4460251..95548884 100644 --- a/pkg/agent/config_test.go +++ b/pkg/agent/config_test.go @@ -658,21 +658,39 @@ func Test_ValidateAndCombineConfig(t *testing.T) { assert.Equal(t, VenafiConnection, got.OutputMode) }) - const arkUsername = "cluster-1-region-1-cloud-1@cyberark.cloud.123456" - t.Run("--machine-hub selects MachineHub mode", func(t *testing.T) { t.Setenv("POD_NAMESPACE", "venafi") t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig)) t.Setenv("ARK_SUBDOMAIN", "tlspk") - t.Setenv("ARK_USERNAME", arkUsername) - t.Setenv("ARK_SECRET", "test-secret") got, cl, err := ValidateAndCombineConfig(discardLogs(), - withConfig(""), + withConfig(testutil.Undent(` + cluster_name: my-cluster + cyberark: + service_id: dev-cluster + `)), + withCmdLineFlags("--period", "1m", "--machine-hub")) + require.NoError(t, err) + assert.Equal(t, MachineHub, got.OutputMode) + assert.Equal(t, "my-cluster", got.ClusterName) + assert.Equal(t, "dev-cluster", got.CyberArk.ServiceID) + assert.IsType(t, &client.CyberArkClient{}, cl) + }) + + t.Run("--machine-hub with cluster_id fallback when cluster_name is empty", func(t *testing.T) { + t.Setenv("POD_NAMESPACE", "venafi") + t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig)) + t.Setenv("ARK_SUBDOMAIN", "tlspk") + got, cl, err := ValidateAndCombineConfig(discardLogs(), + withConfig(testutil.Undent(` + cluster_id: my-cluster-id + cyberark: + service_id: dev-cluster + `)), withCmdLineFlags("--period", "1m", "--machine-hub")) require.NoError(t, err) assert.Equal(t, MachineHub, got.OutputMode) - assert.Equal(t, arkUsername, got.ClusterName, - "the ClusterName should default to the ARK_USERNAME value if the cluster_name in the config file is empty") + assert.Equal(t, "my-cluster-id", got.ClusterName, + "cluster_id should be used as cluster_name when cluster_name is empty") assert.IsType(t, &client.CyberArkClient{}, cl) }) @@ -680,34 +698,34 @@ func Test_ValidateAndCombineConfig(t *testing.T) { t.Setenv("POD_NAMESPACE", "venafi") t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig)) t.Setenv("ARK_SUBDOMAIN", "tlspk") - t.Setenv("ARK_USERNAME", arkUsername) - t.Setenv("ARK_SECRET", "test-secret") got, cl, err := ValidateAndCombineConfig(discardLogs(), withConfig(testutil.Undent(` cluster_name: override-cluster-name - `)), + cyberark: + service_id: dev-cluster + `)), withCmdLineFlags("--period", "1m", "--machine-hub")) require.NoError(t, err) assert.Equal(t, MachineHub, got.OutputMode) - assert.Equal(t, "override-cluster-name", got.ClusterName, - "the cluster_name in the config file should be used if not empty, even if ARK_USERNAME is set") + assert.Equal(t, "override-cluster-name", got.ClusterName) assert.IsType(t, &client.CyberArkClient{}, cl) }) - t.Run("--machine-hub without required environment variables", func(t *testing.T) { + t.Run("--machine-hub without ARK_SUBDOMAIN environment variable", func(t *testing.T) { t.Setenv("POD_NAMESPACE", "venafi") t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig)) t.Setenv("ARK_SUBDOMAIN", "") - t.Setenv("ARK_USERNAME", "") - t.Setenv("ARK_SECRET", "") got, cl, err := ValidateAndCombineConfig(discardLogs(), - withConfig(""), + withConfig(testutil.Undent(` + cyberark: + service_id: dev-cluster + `)), withCmdLineFlags("--period", "1m", "--machine-hub")) assert.Equal(t, CombinedConfig{}, got) assert.Nil(t, cl) assert.EqualError(t, err, testutil.Undent(` validating creds: failed loading config using the MachineHub mode: 1 error occurred: - * missing environment variables: ARK_SUBDOMAIN, ARK_USERNAME, ARK_SECRET + * missing environment variables: ARK_SUBDOMAIN `)) }) @@ -1303,6 +1321,114 @@ func Test_ValidateAndCombineConfig_NGTS(t *testing.T) { }) } +func TestConfig_CyberArk_Validation(t *testing.T) { + // Common env setup: ARK_SUBDOMAIN is the only required env var for MachineHub mode. + setEnv := func(t *testing.T) { + t.Helper() + t.Setenv("POD_NAMESPACE", "venafi") + t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig)) + t.Setenv("ARK_SUBDOMAIN", "tlspk") + } + + t.Run("empty service_id produces an error", func(t *testing.T) { + setEnv(t) + _, _, err := ValidateAndCombineConfig(discardLogs(), + withConfig(testutil.Undent(` + cyberark: + service_id: "" + `)), + withCmdLineFlags("--period", "1m", "--machine-hub")) + require.Error(t, err) + assert.Contains(t, err.Error(), "cyberark.service_id is required in MachineHub mode") + }) + + t.Run("missing cyberark block produces a service_id error", func(t *testing.T) { + setEnv(t) + _, _, err := ValidateAndCombineConfig(discardLogs(), + withConfig(""), + withCmdLineFlags("--period", "1m", "--machine-hub")) + require.Error(t, err) + assert.Contains(t, err.Error(), "cyberark.service_id is required in MachineHub mode") + }) + + t.Run("jwt_source spiffe is rejected", func(t *testing.T) { + setEnv(t) + _, _, err := ValidateAndCombineConfig(discardLogs(), + withConfig(testutil.Undent(` + cyberark: + service_id: dev-cluster + jwt_source: spiffe + `)), + withCmdLineFlags("--period", "1m", "--machine-hub")) + require.Error(t, err) + assert.Contains(t, err.Error(), `cyberark.jwt_source "spiffe" is not supported`) + }) + + t.Run("jwt_source file is accepted", func(t *testing.T) { + setEnv(t) + got, cl, err := ValidateAndCombineConfig(discardLogs(), + withConfig(testutil.Undent(` + cyberark: + service_id: dev-cluster + jwt_source: file + jwt_file_path: /var/run/secrets/tokens/agent-token + `)), + withCmdLineFlags("--period", "1m", "--machine-hub")) + require.NoError(t, err) + assert.Equal(t, "dev-cluster", got.CyberArk.ServiceID) + assert.Equal(t, "file", got.CyberArk.JWTSource) + assert.Equal(t, "/var/run/secrets/tokens/agent-token", got.CyberArk.JWTFilePath) + assert.IsType(t, &client.CyberArkClient{}, cl) + }) + + t.Run("jwt_source empty string is accepted", func(t *testing.T) { + setEnv(t) + got, cl, err := ValidateAndCombineConfig(discardLogs(), + withConfig(testutil.Undent(` + cyberark: + service_id: dev-cluster + `)), + withCmdLineFlags("--period", "1m", "--machine-hub")) + require.NoError(t, err) + assert.Equal(t, "dev-cluster", got.CyberArk.ServiceID) + assert.Equal(t, "", got.CyberArk.JWTSource) + assert.IsType(t, &client.CyberArkClient{}, cl) + }) + + t.Run("account and jwt_file_path are optional", func(t *testing.T) { + setEnv(t) + got, _, err := ValidateAndCombineConfig(discardLogs(), + withConfig(testutil.Undent(` + cyberark: + service_id: dev-cluster + account: myaccount + jwt_file_path: /tmp/token + `)), + withCmdLineFlags("--period", "1m", "--machine-hub")) + require.NoError(t, err) + assert.Equal(t, "myaccount", got.CyberArk.Account) + assert.Equal(t, "/tmp/token", got.CyberArk.JWTFilePath) + }) + + t.Run("cyberark block is ignored in non-MachineHub modes", func(t *testing.T) { + t.Setenv("POD_NAMESPACE", "venafi") + fakeCredsPath := withFile(t, `{"user_id":"foo","user_secret":"bar","client_id": "baz","client_secret": "foobar","auth_server_domain":"bazbar"}`) + got, _, err := ValidateAndCombineConfig(discardLogs(), + withConfig(testutil.Undent(` + server: https://preflight.jetstack.io + organization_id: my-org + cluster_id: my-cluster + period: 1h + cyberark: + service_id: should-be-ignored + `)), + withCmdLineFlags("--credentials-file", fakeCredsPath)) + require.NoError(t, err) + // CyberArk config is not copied into CombinedConfig for non-MachineHub modes. + assert.Equal(t, CyberArkConfig{}, got.CyberArk) + }) +} + const fakePrivKeyPEM = `-----BEGIN PRIVATE KEY----- MHcCAQEEIFptpPXOvEWDrYkiMhyEH1+FB1GwtwX2tyXH4KtBO6g7oAoGCCqGSM49 AwEHoUQDQgAE/BsIwagYc4YUjSSFyqcStj2qliAkdVGlMoJbMuXupzQ9Qs4TX5Pl diff --git a/pkg/client/client_cyberark.go b/pkg/client/client_cyberark.go index 6de22791..7ffcebf4 100644 --- a/pkg/client/client_cyberark.go +++ b/pkg/client/client_cyberark.go @@ -34,19 +34,25 @@ type CyberArkClient struct { var _ Client = &CyberArkClient{} -// NewCyberArk initializes a CyberArk client using configuration from environment variables. -// It requires an HTTP client to be provided, which will be used for making requests. -// The environment variables ARK_SUBDOMAIN, ARK_USERNAME, and ARK_SECRET must be set for authentication. -// Sending secrets is controlled by the ARK_SEND_SECRETS environment variable (defaults to "false"). -// If sending secrets is enabled, the hardcoded public key will be loaded and an encryptor will be created. -// If the configuration is invalid or missing, an error is returned. -func NewCyberArk(httpClient *http.Client) (*CyberArkClient, error) { - configLoader := cyberark.LoadClientConfigFromEnvironment - - cfg, err := configLoader() +// NewCyberArk initializes a CyberArk client. +// Subdomain, and the legacy username/password credentials, are loaded from the +// environment (ARK_SUBDOMAIN, ARK_USERNAME, ARK_SECRET). The remaining fields +// (serviceID, account, jwtSource, jwtFilePath) come from the agent YAML config +// (config.cyberark.*) and select the Conjur JWT exchange when serviceID is set. +// Sending secrets is controlled by the ARK_SEND_SECRETS environment variable +// (defaults to "false"). If the configuration is invalid or missing, an error +// is returned. +func NewCyberArk(httpClient *http.Client, serviceID, account, jwtSource, jwtFilePath string) (*CyberArkClient, error) { + cfg, err := cyberark.LoadClientConfigFromEnvironment() if err != nil { return nil, err } + cfg.ServiceID = serviceID + cfg.Account = account + cfg.JWTSource = jwtSource + cfg.JWTFilePath = jwtFilePath + + configLoader := func() (cyberark.ClientConfig, error) { return cfg, nil } return &CyberArkClient{ configLoader: configLoader, diff --git a/pkg/testutil/envtest.go b/pkg/testutil/envtest.go index 0c56fbe4..f377f337 100644 --- a/pkg/testutil/envtest.go +++ b/pkg/testutil/envtest.go @@ -25,8 +25,8 @@ import ( ctrlruntime "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" + "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" "github.com/jetstack/preflight/pkg/client" ) @@ -262,31 +262,38 @@ func FakeTPP(t testing.TB) (*httptest.Server, *x509.Certificate) { return server, cert } -// FakeCyberArk returns an HTTP client that will route requests to mock CyberArk -// Service Discovery, Identity and Discovery and Context APIs. This is useful -// for testing code that uses all those APIs, such as -// `cyberark.NewDatauploadClient`. -// -// The environment variable `ARK_DISCOVERY_API` is set to the URL of the mock -// Service Discovery API, for the supplied `testing.TB` so that the client under -// test will use the mock Service Discovery API. +// FakeCyberArk returns an HTTP client and JWT file path for use with mock CyberArk +// Service Discovery, Conjur exchange, and Discovery and Context APIs. This is useful +// for testing code that uses all those APIs, such as `cyberark.NewDatauploadClient`. // // The returned HTTP client has a transport which logs requests and responses // depending on log level of the logger supplied in the context. -func FakeCyberArk(t testing.TB) *http.Client { +// The returned jwtFilePath is the path to a temporary JWT file that contains +// a fake service-account JWT; pass it as the jwtFilePath argument to NewCyberArk. +func FakeCyberArk(t testing.TB) (httpClient *http.Client, jwtFilePath string) { t.Helper() - identityAPI, _ := identity.MockIdentityServer(t) + // Write a temp JWT file so jwtsource.NewFileSource can read a 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()) + + // "success-token" matches the dataupload mock's expected bearer token. + conjurSrv, _ := conjur.MockConjurExchangeServer(t, "success-token") + t.Cleanup(conjurSrv.Close) + discoveryContextAPI, _ := dataupload.MockDataUploadServer(t) - httpClient := servicediscovery.MockDiscoveryServer(t, servicediscovery.Services{ + httpClient = servicediscovery.MockDiscoveryServer(t, servicediscovery.Services{ Identity: servicediscovery.ServiceEndpoint{ - API: identityAPI, + API: conjurSrv.URL, // conjur exchange endpoint base }, DiscoveryContext: servicediscovery.ServiceEndpoint{ API: discoveryContextAPI, }, }) - return httpClient + return httpClient, jwtFile.Name() } // Generated using: