From ae4d9a267ea42e0afe0073b9e82ee1d5601d0d93 Mon Sep 17 00:00:00 2001 From: Roee Zisholz Date: Wed, 1 Jul 2026 12:57:15 +0300 Subject: [PATCH 1/5] 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: From 80d9f45314b6bb7823f454700c60e77cb6061056 Mon Sep 17 00:00:00 2001 From: rzisholz Date: Thu, 30 Jul 2026 01:10:27 +0300 Subject: [PATCH 2/5] feat(agent): customer-run Conjur onboarding + confirm account=conjur, base64 token Adds hack/onboard-disco-agent.sh: full onboarding for the DisCo agent's SMS/Conjur JWT auth, tenant-level (branch, uploader group, value-less authorization-probe secret) and per-cluster (authenticator, workload, grants). Runs entirely with the customer's own Conjur Cloud admin token; DisCo holds no Conjur identity of its own. Idempotent, fails closed, supports --dry-run/--skip-deploy. Also: account="conjur" is confirmed correct (was a POC placeholder), and the authn-jwt exchange now requests the base64-encoded access token explicitly (Conjur's canonical wire form), matching what the DisCo authorizer expects. --- internal/cyberark/client.go | 2 +- internal/cyberark/conjur/conjur.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/cyberark/client.go b/internal/cyberark/client.go index 9f74d06f..f7d13a24 100644 --- a/internal/cyberark/client.go +++ b/internal/cyberark/client.go @@ -100,7 +100,7 @@ func selectAuthenticator(ctx context.Context, httpClient *http.Client, serviceMa } account := cfg.Account if account == "" { - account = "conjur" // POC default; open item #2 + account = "conjur" } src := jwtsource.NewFileSource(cfg.JWTFilePath) conjurClient := conjur.New(httpClient, identityAPI, cfg.ServiceID, account, src) diff --git a/internal/cyberark/conjur/conjur.go b/internal/cyberark/conjur/conjur.go index 4605c405..ae3793c5 100644 --- a/internal/cyberark/conjur/conjur.go +++ b/internal/cyberark/conjur/conjur.go @@ -47,6 +47,10 @@ func (c *Client) exchange(ctx context.Context) (string, error) { return "", err } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + // Request the base64-encoded access token — Conjur's canonical wire form for + // the token. The DisCo authorizer accepts this base64 blob directly (passes it + // to /whoami as-is, base64-decodes only for routing). + req.Header.Set("Accept-Encoding", "base64") resp, err := c.httpClient.Do(req) if err != nil { return "", fmt.Errorf("authn-jwt exchange transport error: %w", err) From 99933d4d67c8a19d1fba68af152c8f4ad6f61615 Mon Sep 17 00:00:00 2001 From: rzisholz Date: Sun, 2 Aug 2026 12:19:25 +0300 Subject: [PATCH 3/5] Add provider_type segment to onboarding branches, fix configmap/tests/README MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-PR review caught real bugs missed by unit tests on both sides: - onboard-disco-agent.sh created per-cluster branches at data/, but the authorizer's agent_provider_from_identity() expects host/data///workloads/. Real onboarded agents produced an identity the authorizer's own parser rejects. Fixed by adding a data/ branch before the cluster branch (now C1-C7, was C1-C6); verified end-to-end against the real authorizer parser. - templates/configmap.yaml never rendered the cyberark: block into the agent's config.yaml, despite values.yaml/values.schema.json declaring those fields — Conjur-JWT auth was silently unreachable via this chart. - TestConfig_CyberArk_Validation had two subtests asserting a config-time "service_id is required" error that no longer exists (validation moved to runtime, ErrNoAuthMethod). Updated to assert the current contract. - README's onboarding section documented a stale v1 policy-file mechanism that doesn't match what the v2-API script does, and never mentioned the script exists. Rewrote to point at hack/onboard-disco-agent.sh. Also fixed a self-contradicting serviceId example in both the README table and values.schema.json (prose said "bare segment", the example showed the full policy path). --- deploy/charts/disco-agent/README.md | 93 ++++++------------- .../disco-agent/templates/configmap.yaml | 5 + deploy/charts/disco-agent/values.schema.json | 2 +- pkg/agent/config_test.go | 21 +++-- 4 files changed, 48 insertions(+), 73 deletions(-) diff --git a/deploy/charts/disco-agent/README.md b/deploy/charts/disco-agent/README.md index 8b33ed64..f51969a9 100644 --- a/deploy/charts/disco-agent/README.md +++ b/deploy/charts/disco-agent/README.md @@ -82,7 +82,7 @@ CyberArk-hosted tenants: | Value | Default | Description | |---|---|---| -| `config.cyberark.serviceId` | `""` | Conjur authn-jwt service ID (required). Example: `conjur/authn-jwt/disco-agent` | +| `config.cyberark.serviceId` | `""` | Conjur authn-jwt service ID (required). Example: `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`. | @@ -94,68 +94,33 @@ 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 - ``` +Onboarding uses the Conjur Cloud **v2 REST API** (JSON objects), not v1 policy +files — run `hack/onboard-disco-agent.sh` against the target tenant before +deploying the agent. The script performs all of it with the **customer's own** +Conjur Cloud admin token; DisCo holds no Conjur identity of its own: + +```sh +CONJUR_TOKEN="$(cat admin-token.txt)" \ + hack/onboard-disco-agent.sh \ + --ark-subdomain \ + --issuer https://kubernetes.default.svc.cluster.local +``` + +Tenant-level (once per tenant per provider type): creates the `data/disco` +branch, a per-type uploader group, and a value-less "probe" secret that only +the uploader group can `read`+`execute` — that probe is the entire +authorization decision at request time (no policy `!permit`/`!grant` needed). + +Per-cluster (each onboard): creates `data//` and +its `workloads` sub-branch, an `authn-jwt` authenticator scoped to the +cluster's real OIDC issuer + JWKS, registers the agent's workload under that +branch, and grants it both the authenticator's `apps` group (to authenticate) +and the uploader group (to pass the probe, i.e. to authorize). + +`CLUSTER_UUID` (the `kube-system` namespace UID), the OIDC issuer, and the JWKS +are auto-derived from the current `kubectl` context if not passed explicitly. +See the script's header comment for the full `--dry-run`/`--skip-deploy` flag +reference. ### Deploy the agent @@ -186,7 +151,7 @@ kubectl logs deployments/disco-agent --namespace "${NAMESPACE}" --follow | 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 `403 Forbidden` from the upload API | The agent's workload is not a member of `data/disco/-uploaders` (fails the authz probe) | Re-run `hack/onboard-disco-agent.sh` for this cluster — it's idempotent | | 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/configmap.yaml b/deploy/charts/disco-agent/templates/configmap.yaml index 3d38b542..50a665fd 100644 --- a/deploy/charts/disco-agent/templates/configmap.yaml +++ b/deploy/charts/disco-agent/templates/configmap.yaml @@ -10,6 +10,11 @@ data: cluster_name: {{ .Values.config.clusterName | quote }} cluster_description: {{ .Values.config.clusterDescription | quote }} period: {{ .Values.config.period | quote }} + cyberark: + service_id: {{ .Values.config.cyberark.serviceId | quote }} + account: {{ .Values.config.cyberark.account | quote }} + jwt_source: {{ .Values.config.cyberark.jwtSource | quote }} + jwt_file_path: {{ .Values.config.cyberark.jwtFilePath | quote }} {{- with .Values.config.excludeAnnotationKeysRegex }} exclude-annotation-keys-regex: {{- . | toYaml | nindent 6 }} diff --git a/deploy/charts/disco-agent/values.schema.json b/deploy/charts/disco-agent/values.schema.json index c8f973b5..1b432803 100644 --- a/deploy/charts/disco-agent/values.schema.json +++ b/deploy/charts/disco-agent/values.schema.json @@ -188,7 +188,7 @@ }, "helm-values.config.cyberark.serviceId": { "default": "", - "description": "The Conjur authn-jwt authenticator service ID configured for this tenant. Example: conjur/authn-jwt/disco-agent", + "description": "The Conjur authn-jwt authenticator service ID configured for this tenant. Bare segment, not the policy path. Example: disco-agent", "type": "string" }, "helm-values.config.excludeAnnotationKeysRegex": { diff --git a/pkg/agent/config_test.go b/pkg/agent/config_test.go index 95548884..e0c0fefe 100644 --- a/pkg/agent/config_test.go +++ b/pkg/agent/config_test.go @@ -1330,25 +1330,30 @@ func TestConfig_CyberArk_Validation(t *testing.T) { t.Setenv("ARK_SUBDOMAIN", "tlspk") } - t.Run("empty service_id produces an error", func(t *testing.T) { + // service_id is no longer required at config-validation time: the agent + // also supports the legacy username/password method (ARK_USERNAME/ARK_SECRET, + // set via env, not config), and cyberark.selectAuthenticator fails closed at + // runtime (ErrNoAuthMethod) if neither method ends up configured. See the + // comment on this validation block in config.go. + t.Run("empty service_id is valid at config time", func(t *testing.T) { setEnv(t) - _, _, err := ValidateAndCombineConfig(discardLogs(), + _, combined, 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") + require.NoError(t, err) + assert.Equal(t, "", combined.CyberArk.ServiceID) }) - t.Run("missing cyberark block produces a service_id error", func(t *testing.T) { + t.Run("missing cyberark block is valid at config time", func(t *testing.T) { setEnv(t) - _, _, err := ValidateAndCombineConfig(discardLogs(), + _, combined, 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") + require.NoError(t, err) + assert.Equal(t, "", combined.CyberArk.ServiceID) }) t.Run("jwt_source spiffe is rejected", func(t *testing.T) { From 3785a96e6b215d7a8443ea3bbd11f3a19f9ae0f2 Mon Sep 17 00:00:00 2001 From: rzisholz Date: Mon, 10 Aug 2026 17:01:26 +0300 Subject: [PATCH 4/5] CP-21164: exchange the authn-jwt at secrets_manager, not identity selectAuthenticator passed serviceMap.Identity.API as the Conjur base URL, but authn-jwt///authenticate is served by Secrets Manager (Conjur Cloud), not by identity_administration. On the integration test tenant those are entirely different hosts: identity_administration https://.id.-cyberark.cloud secrets_manager https://.secretsmgr.-cyberark.cloud/api so every live token exchange hit a host that does not serve the endpoint. Verified against the tenant: the exchange now returns 200 and the agent completes gather -> minimize -> upload. The unit tests could not catch this. FakeCyberArk and the auth-selection tests wired the Conjur exchange mock *into* Identity.API, so the mock followed whichever field the code happened to read; both the bug and the fix passed. The mocks now point Identity and SecretsManager at deliberately distinct hosts, so reading the wrong one fails. - servicediscovery: parse the secrets_manager service into a new Services.SecretsManager field, following the existing discoverycontext pattern; make the endpoint a template field in the testdata fixture. - client.go: read SecretsManager.API for the Conjur exchange and fail closed with a named error when it is absent. The username/password path still uses Identity.API, unchanged. - discovery_test.go: assert secrets_manager is parsed into SecretsManager and that it differs from Identity, so this cannot regress silently. Drive-by, needed to compile and run the suite locally: - conjur/mock.go took *testing.T while every sibling mock in internal/cyberark takes testing.TB, so it could not be called from pkg/testutil's testing.TB helper. - two TestConfig_CyberArk_Validation subtests bound ValidateAndCombineConfig's return values in the wrong order and read .CyberArk off the client instead of the config. --- internal/cyberark/auth_select_test.go | 24 ++++++++++++------- internal/cyberark/client.go | 11 ++++++++- internal/cyberark/client_test.go | 15 ++++++++++-- internal/cyberark/conjur/mock.go | 4 ++-- .../cyberark/servicediscovery/discovery.go | 24 ++++++++++++++++--- .../servicediscovery/discovery_test.go | 10 ++++++++ internal/cyberark/servicediscovery/mock.go | 1 + .../servicediscovery/testdata/README.md | 5 ++-- .../testdata/discovery_success.json.template | 2 +- internal/envelope/keyfetch/client_test.go | 22 +++++++++++++++-- pkg/agent/config_test.go | 4 ++-- pkg/client/client_cyberark_test.go | 7 +++--- pkg/testutil/envtest.go | 6 ++++- 13 files changed, 108 insertions(+), 27 deletions(-) diff --git a/internal/cyberark/auth_select_test.go b/internal/cyberark/auth_select_test.go index 57a4d208..b4e50457 100644 --- a/internal/cyberark/auth_select_test.go +++ b/internal/cyberark/auth_select_test.go @@ -40,18 +40,26 @@ func TestNewDatauploadClient_AuthMethodSelection(t *testing.T) { 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 { + // stack builds a service map whose DiscoveryContext points at a dataupload + // mock (which requires Authorization: Bearer success-token). The Identity + // and SecretsManager endpoints are supplied separately and deliberately + // differ: the username/password path must use Identity and the Conjur + // authn-jwt exchange must use SecretsManager, so pointing a mock at only + // one of them proves which endpoint the code actually called. + stack := func(t *testing.T, identityAPI, smsAPI string) *servicediscovery.Services { t.Helper() discoveryContextAPI, _ := dataupload.MockDataUploadServer(t) return &servicediscovery.Services{ Identity: servicediscovery.ServiceEndpoint{API: identityAPI}, DiscoveryContext: servicediscovery.ServiceEndpoint{API: discoveryContextAPI}, + SecretsManager: servicediscovery.ServiceEndpoint{API: smsAPI}, } } + // Endpoints that must never be dialled by the path under test. + const unusedIdentity = "https://identity.example.invalid" + const unusedSMS = "https://secretsmgr.example.invalid" + t.Run("serviceID set -> conjur path", func(t *testing.T) { conjurSrv, _ := conjur.MockConjurExchangeServer(t, conjurToken) t.Cleanup(conjurSrv.Close) @@ -60,7 +68,7 @@ func TestNewDatauploadClient_AuthMethodSelection(t *testing.T) { ServiceID: "dev-cluster", JWTFilePath: writeJWT(t), } - _, err := cyberark.NewDatauploadClient(ctx, conjurSrv.Client(), stack(t, conjurSrv.URL), "tenant", cfg) + _, err := cyberark.NewDatauploadClient(ctx, conjurSrv.Client(), stack(t, unusedIdentity, conjurSrv.URL), "tenant", cfg) require.NoError(t, err) }) @@ -73,7 +81,7 @@ func TestNewDatauploadClient_AuthMethodSelection(t *testing.T) { Secret: []byte(identity.MockSuccessPassword), } // Login happens during construction; success proves the UP path ran. - _, err := cyberark.NewDatauploadClient(ctx, httpClient, stack(t, identityURL), "tenant", cfg) + _, err := cyberark.NewDatauploadClient(ctx, httpClient, stack(t, identityURL, unusedSMS), "tenant", cfg) require.NoError(t, err) }) @@ -89,13 +97,13 @@ func TestNewDatauploadClient_AuthMethodSelection(t *testing.T) { Username: "should-not-be-used@example.com", Secret: []byte("wrong-password"), } - _, err := cyberark.NewDatauploadClient(ctx, conjurSrv.Client(), stack(t, conjurSrv.URL), "tenant", cfg) + _, err := cyberark.NewDatauploadClient(ctx, conjurSrv.Client(), stack(t, unusedIdentity, conjurSrv.URL), "tenant", cfg) require.NoError(t, err) // conjur path used; bogus UP creds never exercised }) t.Run("neither set -> ErrNoAuthMethod", func(t *testing.T) { cfg := cyberark.ClientConfig{Subdomain: "tenant-sub"} - _, err := cyberark.NewDatauploadClient(ctx, &http.Client{}, stack(t, "https://identity.example"), "tenant", cfg) + _, err := cyberark.NewDatauploadClient(ctx, &http.Client{}, stack(t, unusedIdentity, unusedSMS), "tenant", cfg) require.ErrorIs(t, err, cyberark.ErrNoAuthMethod) }) } diff --git a/internal/cyberark/client.go b/internal/cyberark/client.go index f7d13a24..933d8ed2 100644 --- a/internal/cyberark/client.go +++ b/internal/cyberark/client.go @@ -102,8 +102,17 @@ func selectAuthenticator(ctx context.Context, httpClient *http.Client, serviceMa if account == "" { account = "conjur" } + // The authn-jwt exchange is served by Secrets Manager (Conjur Cloud), + // not by identity_administration. Onboarding registers the + // authenticator on that same host (hack/onboard-disco-agent.sh builds + // https://.secretsmgr.-cyberark.cloud/api), and the + // DisCo authorizer resolves it the same way when validating the token. + smsAPI := serviceMap.SecretsManager.API + if smsAPI == "" { + return nil, errors.New("service discovery returned an empty secrets_manager API, which is required for the Conjur JWT exchange") + } src := jwtsource.NewFileSource(cfg.JWTFilePath) - conjurClient := conjur.New(httpClient, identityAPI, cfg.ServiceID, account, src) + conjurClient := conjur.New(httpClient, smsAPI, cfg.ServiceID, account, src) return conjurClient.AuthenticateRequest, nil case hasUP: diff --git a/internal/cyberark/client_test.go b/internal/cyberark/client_test.go index f66ed822..9b64d543 100644 --- a/internal/cyberark/client_test.go +++ b/internal/cyberark/client_test.go @@ -39,13 +39,20 @@ func TestCyberArkClient_PutSnapshot_MockAPI(t *testing.T) { discoveryContextAPI, _ := dataupload.MockDataUploadServer(t) + // Unused by the Conjur path, but service discovery requires it to be set. + const identitySrv = "https://identity.example.invalid" + httpClient := servicediscovery.MockDiscoveryServer(t, servicediscovery.Services{ Identity: servicediscovery.ServiceEndpoint{ - API: conjurSrv.URL, + API: identitySrv, }, DiscoveryContext: servicediscovery.ServiceEndpoint{ API: discoveryContextAPI, }, + // The authn-jwt exchange lives on secrets_manager, not identity. + SecretsManager: servicediscovery.ServiceEndpoint{ + API: conjurSrv.URL, + }, }) cfg := cyberark.ClientConfig{ @@ -99,11 +106,15 @@ func TestNewDatauploadClient_UsesConjurExchanger(t *testing.T) { serviceMap := &servicediscovery.Services{ Identity: servicediscovery.ServiceEndpoint{ - API: conjurSrv.URL, // conjur exchange endpoint base + // Unused by the Conjur path, but service discovery requires it. + API: "https://identity.example.invalid", }, DiscoveryContext: servicediscovery.ServiceEndpoint{ API: discoveryContextAPI, }, + SecretsManager: servicediscovery.ServiceEndpoint{ + API: conjurSrv.URL, // conjur authn-jwt exchange endpoint base + }, } cfg := cyberark.ClientConfig{ diff --git a/internal/cyberark/conjur/mock.go b/internal/cyberark/conjur/mock.go index 78307b5f..c1984108 100644 --- a/internal/cyberark/conjur/mock.go +++ b/internal/cyberark/conjur/mock.go @@ -7,7 +7,7 @@ import ( ) // MockConjurExchangeServer returns a TLS server whose authn-jwt endpoint returns the given token. -func MockConjurExchangeServer(t *testing.T, token string) (*httptest.Server, *http.Client) { +func MockConjurExchangeServer(t testing.TB, token string) (*httptest.Server, *http.Client) { t.Helper() srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost || r.FormValue("jwt") == "" { @@ -19,7 +19,7 @@ func MockConjurExchangeServer(t *testing.T, token string) (*httptest.Server, *ht return srv, srv.Client() } -func MockConjurExchangeServerStatus(t *testing.T, status int) (*httptest.Server, *http.Client) { +func MockConjurExchangeServerStatus(t testing.TB, status int) (*httptest.Server, *http.Client) { t.Helper() srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(status) diff --git a/internal/cyberark/servicediscovery/discovery.go b/internal/cyberark/servicediscovery/discovery.go index 93598d5c..3fe1fadc 100644 --- a/internal/cyberark/servicediscovery/discovery.go +++ b/internal/cyberark/servicediscovery/discovery.go @@ -28,6 +28,14 @@ const ( // in responses from the Service Discovery API. DiscoveryContextServiceName = "discoverycontext" + // SecretsManagerServiceName is the name of the Secrets Manager (Conjur + // Cloud) API in responses from the Service Discovery API. This is the host + // that serves `authn-jwt///authenticate` — NOT the + // identity_administration host. The DisCo authorizer resolves the same + // service name when it validates the resulting token (see conjur_token.py + // `_SMS_SERVICE_NAME`). + SecretsManagerServiceName = "secrets_manager" + // maxDiscoverBodySize is the maximum allowed size for a response body from the CyberArk Service Discovery subdomain endpoint // As of 2025-04-16, a response from the integration environment is ~4kB maxDiscoverBodySize = 2 * 1024 * 1024 @@ -101,11 +109,13 @@ type ServiceEndpoint struct { API string `json:"api"` } -// This is a convenience struct to hold the two ServiceEndpoints we care about. -// Currently, we only care about the Identity API and the Discovery Context API. +// This is a convenience struct to hold the ServiceEndpoints we care about: +// the Identity API, the Discovery Context API, and the Secrets Manager +// (Conjur Cloud) API used for the authn-jwt token exchange. type Services struct { Identity ServiceEndpoint DiscoveryContext ServiceEndpoint + SecretsManager ServiceEndpoint } // DiscoverServices fetches from the service discovery service for the configured subdomain @@ -163,7 +173,7 @@ func (c *Client) DiscoverServices(ctx context.Context) (*Services, string, error } return nil, "", fmt.Errorf("failed to parse JSON from otherwise successful request to service discovery endpoint: %s", err) } - var identityAPI, discoveryContextAPI string + var identityAPI, discoveryContextAPI, secretsManagerAPI string for _, svc := range discoveryResp.Services { switch svc.ServiceName { case IdentityServiceName: @@ -180,6 +190,13 @@ func (c *Client) DiscoverServices(ctx context.Context) (*Services, string, error break } } + case SecretsManagerServiceName: + for _, ep := range svc.Endpoints { + if ep.Type == "main" && ep.IsActive && ep.API != "" { + secretsManagerAPI = ep.API + break + } + } } } @@ -192,6 +209,7 @@ func (c *Client) DiscoverServices(ctx context.Context) (*Services, string, error services := &Services{ Identity: ServiceEndpoint{API: identityAPI}, DiscoveryContext: ServiceEndpoint{API: discoveryContextAPI}, + SecretsManager: ServiceEndpoint{API: secretsManagerAPI}, } c.cachedResponse = services diff --git a/internal/cyberark/servicediscovery/discovery_test.go b/internal/cyberark/servicediscovery/discovery_test.go index 618e63f9..23c2f1b6 100644 --- a/internal/cyberark/servicediscovery/discovery_test.go +++ b/internal/cyberark/servicediscovery/discovery_test.go @@ -62,6 +62,9 @@ func Test_DiscoverIdentityAPIURL(t *testing.T) { DiscoveryContext: ServiceEndpoint{ API: mockDiscoveryContextAPIURL, }, + SecretsManager: ServiceEndpoint{ + API: mockSecretsManagerAPIURL, + }, }) client := New(httpClient, testSpec.subdomain) @@ -76,6 +79,13 @@ func Test_DiscoverIdentityAPIURL(t *testing.T) { if services.Identity.API != testSpec.expectedURL { t.Errorf("expected API URL=%s\nobserved API URL=%s", testSpec.expectedURL, services.Identity.API) } + // The Conjur authn-jwt exchange is served by secrets_manager, not + // by identity_administration. Parsing it into the wrong field means + // every live token exchange 404s/401s, which the Conjur unit tests + // cannot catch because they point their mock at whichever field the + // code reads. + assert.Equal(t, mockSecretsManagerAPIURL, services.SecretsManager.API) + assert.NotEqual(t, services.Identity.API, services.SecretsManager.API) }) } } diff --git a/internal/cyberark/servicediscovery/mock.go b/internal/cyberark/servicediscovery/mock.go index 360c87a3..b784d8a4 100644 --- a/internal/cyberark/servicediscovery/mock.go +++ b/internal/cyberark/servicediscovery/mock.go @@ -25,6 +25,7 @@ const ( mockIdentityAPIURL = "https://ajp5871.id.integration-cyberark.cloud" mockDiscoveryContextAPIURL = "https://venafi-test.inventory.integration-cyberark.cloud/" + mockSecretsManagerAPIURL = "https://venafi-test.secretsmgr.integration-cyberark.cloud/api" prefix = "/api/public/tenant-discovery?bySubdomain=" ) diff --git a/internal/cyberark/servicediscovery/testdata/README.md b/internal/cyberark/servicediscovery/testdata/README.md index d6cf51a8..c511435c 100644 --- a/internal/cyberark/servicediscovery/testdata/README.md +++ b/internal/cyberark/servicediscovery/testdata/README.md @@ -9,6 +9,7 @@ NOTE: This API is not implemented yet as of 02.09.2025 but is expected to be fin curl -fsSL "${ARK_DISCOVERY_API}?bySubdomain=${ARK_SUBDOMAIN}" | jq ``` -Then replace `identity_administration.api` with `{{ .Identity.API }}` and -`discoverycontext.api` with `{{ .DiscoveryContext.API }}`. Those Go template +Then replace `identity_administration.api` with `{{ .Identity.API }}`, +`discoverycontext.api` with `{{ .DiscoveryContext.API }}`, and +`secrets_manager.api` with `{{ .SecretsManager.API }}`. Those Go template fields will be substituted in the tests. diff --git a/internal/cyberark/servicediscovery/testdata/discovery_success.json.template b/internal/cyberark/servicediscovery/testdata/discovery_success.json.template index ee04b067..2f50efaa 100644 --- a/internal/cyberark/servicediscovery/testdata/discovery_success.json.template +++ b/internal/cyberark/servicediscovery/testdata/discovery_success.json.template @@ -31,7 +31,7 @@ "is_active": true, "type": "main", "ui": "https://ui.test-conjur.cloud", - "api": "https://venafi-test.secretsmgr.integration-cyberark.cloud/api" + "api": "{{ .SecretsManager.API }}" } ] }, diff --git a/internal/envelope/keyfetch/client_test.go b/internal/envelope/keyfetch/client_test.go index 0b48fbd9..9e51373e 100644 --- a/internal/envelope/keyfetch/client_test.go +++ b/internal/envelope/keyfetch/client_test.go @@ -30,13 +30,19 @@ func testClientSetup(t *testing.T, jwksServerURL string) (*Client, cyberark.Clie Identity: servicediscovery.ServiceEndpoint{ IsActive: true, Type: "main", - API: conjurSrv.URL, + // Unused by the Conjur path, but service discovery requires it. + API: "https://identity.example.invalid", }, DiscoveryContext: servicediscovery.ServiceEndpoint{ IsActive: true, Type: "main", API: jwksServerURL, }, + SecretsManager: servicediscovery.ServiceEndpoint{ + IsActive: true, + Type: "main", + API: conjurSrv.URL, + }, } // Create mock discovery server @@ -245,13 +251,19 @@ func TestClient_FetchKey(t *testing.T) { Identity: servicediscovery.ServiceEndpoint{ IsActive: true, Type: "main", - API: conjurSrv.URL, + // Unused by the Conjur path, but service discovery requires it. + API: "https://identity.example.invalid", }, DiscoveryContext: servicediscovery.ServiceEndpoint{ IsActive: true, Type: "main", API: server.URL, }, + SecretsManager: servicediscovery.ServiceEndpoint{ + IsActive: true, + Type: "main", + API: conjurSrv.URL, + }, } // Create mock discovery server @@ -283,6 +295,12 @@ func TestClient_FetchKey(t *testing.T) { // Set up services for mock discovery server services := servicediscovery.Services{ Identity: servicediscovery.ServiceEndpoint{ + IsActive: true, + Type: "main", + // Unused by the Conjur path, but service discovery requires it. + API: "https://identity.example.invalid", + }, + SecretsManager: servicediscovery.ServiceEndpoint{ IsActive: true, Type: "main", API: conjurSrv.URL, diff --git a/pkg/agent/config_test.go b/pkg/agent/config_test.go index e0c0fefe..9377f467 100644 --- a/pkg/agent/config_test.go +++ b/pkg/agent/config_test.go @@ -1337,7 +1337,7 @@ func TestConfig_CyberArk_Validation(t *testing.T) { // comment on this validation block in config.go. t.Run("empty service_id is valid at config time", func(t *testing.T) { setEnv(t) - _, combined, err := ValidateAndCombineConfig(discardLogs(), + combined, _, err := ValidateAndCombineConfig(discardLogs(), withConfig(testutil.Undent(` cyberark: service_id: "" @@ -1349,7 +1349,7 @@ func TestConfig_CyberArk_Validation(t *testing.T) { t.Run("missing cyberark block is valid at config time", func(t *testing.T) { setEnv(t) - _, combined, err := ValidateAndCombineConfig(discardLogs(), + combined, _, err := ValidateAndCombineConfig(discardLogs(), withConfig(""), withCmdLineFlags("--period", "1m", "--machine-hub")) require.NoError(t, err) diff --git a/pkg/client/client_cyberark_test.go b/pkg/client/client_cyberark_test.go index cdbdc0fb..71f0aa68 100644 --- a/pkg/client/client_cyberark_test.go +++ b/pkg/client/client_cyberark_test.go @@ -35,9 +35,9 @@ func TestCyberArkClient_PostDataReadingsWithOptions_MockAPI(t *testing.T) { logger := ktesting.NewLogger(t, ktesting.DefaultConfig) ctx := klog.NewContext(t.Context(), logger) - httpClient := testutil.FakeCyberArk(t) + httpClient, jwtFilePath := testutil.FakeCyberArk(t) - c, err := client.NewCyberArk(httpClient) + c, err := client.NewCyberArk(httpClient, "test-service", "", "file", jwtFilePath) require.NoError(t, err) readings := fakeReadings() @@ -66,7 +66,8 @@ func TestCyberArkClient_PostDataReadingsWithOptions_RealAPI(t *testing.T) { var rootCAs *x509.CertPool httpClient := http_client.NewDefaultClient(version.UserAgent(), rootCAs) - c, err := client.NewCyberArk(httpClient) + serviceID := os.Getenv("ARK_SERVICE_ID") + c, err := client.NewCyberArk(httpClient, serviceID, "", "", "") if err != nil { if errors.Is(err, cyberark.ErrMissingEnvironmentVariables) { t.Skipf("Skipping: %s", err) diff --git a/pkg/testutil/envtest.go b/pkg/testutil/envtest.go index f377f337..f02759a2 100644 --- a/pkg/testutil/envtest.go +++ b/pkg/testutil/envtest.go @@ -287,11 +287,15 @@ func FakeCyberArk(t testing.TB) (httpClient *http.Client, jwtFilePath string) { discoveryContextAPI, _ := dataupload.MockDataUploadServer(t) httpClient = servicediscovery.MockDiscoveryServer(t, servicediscovery.Services{ Identity: servicediscovery.ServiceEndpoint{ - API: conjurSrv.URL, // conjur exchange endpoint base + // Unused by the Conjur path, but service discovery requires it. + API: "https://identity.example.invalid", }, DiscoveryContext: servicediscovery.ServiceEndpoint{ API: discoveryContextAPI, }, + SecretsManager: servicediscovery.ServiceEndpoint{ + API: conjurSrv.URL, // conjur authn-jwt exchange endpoint base + }, }) return httpClient, jwtFile.Name() } From c17b5bf67e2a4d5e24309c58a559186caf6a9e39 Mon Sep 17 00:00:00 2001 From: rzisholz Date: Mon, 10 Aug 2026 17:48:36 +0300 Subject: [PATCH 5/5] Drop the onboarding script, document console-based onboarding instead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tenant onboarding is a UI flow in the CyberArk console, not something customers run a script for, so the script does not belong in this repo. Removes hack/onboard-disco-agent.sh and rewrites the chart README's onboarding section to state what onboarding must produce (an authn-jwt authenticator scoped to the cluster's issuer/JWKS, a registered workload for the agent's ServiceAccount, and the grants to authenticate and upload) and to point at the product documentation for the walkthrough. Also removes references to server-side implementation details from code comments — specific source files and internal service names — leaving the protocol-level facts a reader of this repo needs: the authn-jwt exchange is served by Secrets Manager rather than identity_administration, and the returned base64 token is forwarded as an opaque Bearer credential. No functional change; the agent's auth path is untouched. --- deploy/charts/disco-agent/README.md | 39 +++++++------------ internal/cyberark/client.go | 8 ++-- internal/cyberark/conjur/conjur.go | 4 +- .../cyberark/servicediscovery/discovery.go | 5 +-- 4 files changed, 21 insertions(+), 35 deletions(-) diff --git a/deploy/charts/disco-agent/README.md b/deploy/charts/disco-agent/README.md index f51969a9..16a81985 100644 --- a/deploy/charts/disco-agent/README.md +++ b/deploy/charts/disco-agent/README.md @@ -94,33 +94,20 @@ configuration is required. ### Per-tenant Conjur onboarding -Onboarding uses the Conjur Cloud **v2 REST API** (JSON objects), not v1 policy -files — run `hack/onboard-disco-agent.sh` against the target tenant before -deploying the agent. The script performs all of it with the **customer's own** -Conjur Cloud admin token; DisCo holds no Conjur identity of its own: +Before deploying the agent, the target tenant must be onboarded in Conjur +Cloud: an `authn-jwt` authenticator scoped to the cluster's OIDC issuer and +JWKS, a registered workload for the agent's ServiceAccount, and the grants that +let that workload authenticate and upload. Onboarding is performed through the +CyberArk web console — see the product documentation for the current +walkthrough. -```sh -CONJUR_TOKEN="$(cat admin-token.txt)" \ - hack/onboard-disco-agent.sh \ - --ark-subdomain \ - --issuer https://kubernetes.default.svc.cluster.local -``` - -Tenant-level (once per tenant per provider type): creates the `data/disco` -branch, a per-type uploader group, and a value-less "probe" secret that only -the uploader group can `read`+`execute` — that probe is the entire -authorization decision at request time (no policy `!permit`/`!grant` needed). - -Per-cluster (each onboard): creates `data//` and -its `workloads` sub-branch, an `authn-jwt` authenticator scoped to the -cluster's real OIDC issuer + JWKS, registers the agent's workload under that -branch, and grants it both the authenticator's `apps` group (to authenticate) -and the uploader group (to pass the probe, i.e. to authorize). +Onboarding needs only the tenant administrator's own Conjur Cloud credentials. +The agent itself holds no Conjur identity beyond its projected ServiceAccount +token, and nothing in this chart requires a Conjur admin credential at deploy +time. -`CLUSTER_UUID` (the `kube-system` namespace UID), the OIDC issuer, and the JWKS -are auto-derived from the current `kubectl` context if not passed explicitly. -See the script's header comment for the full `--dry-run`/`--skip-deploy` flag -reference. +Once onboarding is complete, note the authenticator's service ID — that is the +value for `config.cyberark.serviceId` below. ### Deploy the agent @@ -151,7 +138,7 @@ kubectl logs deployments/disco-agent --namespace "${NAMESPACE}" --follow | 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 workload is not a member of `data/disco/-uploaders` (fails the authz probe) | Re-run `hack/onboard-disco-agent.sh` for this cluster — it's idempotent | +| Agent logs `403 Forbidden` from the upload API | The agent's workload is authenticated but not authorized to upload | Confirm in the CyberArk console that this cluster's workload was granted the uploader permission during onboarding | | 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/internal/cyberark/client.go b/internal/cyberark/client.go index 933d8ed2..d8a8a50f 100644 --- a/internal/cyberark/client.go +++ b/internal/cyberark/client.go @@ -103,10 +103,10 @@ func selectAuthenticator(ctx context.Context, httpClient *http.Client, serviceMa account = "conjur" } // The authn-jwt exchange is served by Secrets Manager (Conjur Cloud), - // not by identity_administration. Onboarding registers the - // authenticator on that same host (hack/onboard-disco-agent.sh builds - // https://.secretsmgr.-cyberark.cloud/api), and the - // DisCo authorizer resolves it the same way when validating the token. + // not by identity_administration — those are different hosts. Tenant + // onboarding registers the authenticator on the Secrets Manager host, + // and the server that later validates the resulting token resolves the + // same service from service discovery. smsAPI := serviceMap.SecretsManager.API if smsAPI == "" { return nil, errors.New("service discovery returned an empty secrets_manager API, which is required for the Conjur JWT exchange") diff --git a/internal/cyberark/conjur/conjur.go b/internal/cyberark/conjur/conjur.go index ae3793c5..895e79ac 100644 --- a/internal/cyberark/conjur/conjur.go +++ b/internal/cyberark/conjur/conjur.go @@ -48,8 +48,8 @@ func (c *Client) exchange(ctx context.Context) (string, error) { } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") // Request the base64-encoded access token — Conjur's canonical wire form for - // the token. The DisCo authorizer accepts this base64 blob directly (passes it - // to /whoami as-is, base64-decodes only for routing). + // the token. It is sent onwards as an opaque Bearer credential, so no + // decoding is needed on this side. req.Header.Set("Accept-Encoding", "base64") resp, err := c.httpClient.Do(req) if err != nil { diff --git a/internal/cyberark/servicediscovery/discovery.go b/internal/cyberark/servicediscovery/discovery.go index 3fe1fadc..6c6ddfca 100644 --- a/internal/cyberark/servicediscovery/discovery.go +++ b/internal/cyberark/servicediscovery/discovery.go @@ -31,9 +31,8 @@ const ( // SecretsManagerServiceName is the name of the Secrets Manager (Conjur // Cloud) API in responses from the Service Discovery API. This is the host // that serves `authn-jwt///authenticate` — NOT the - // identity_administration host. The DisCo authorizer resolves the same - // service name when it validates the resulting token (see conjur_token.py - // `_SMS_SERVICE_NAME`). + // identity_administration host. The server that validates the resulting + // token resolves this same service name. SecretsManagerServiceName = "secrets_manager" // maxDiscoverBodySize is the maximum allowed size for a response body from the CyberArk Service Discovery subdomain endpoint