From 3454af8a45b06126306a4a1d20b4453cef3e670e Mon Sep 17 00:00:00 2001 From: Sergio Palacio Date: Tue, 21 Apr 2026 13:11:39 +0200 Subject: [PATCH 1/3] feat(contrib/keyvault): add Azure Key Vault TokenStore Implements microsoft.TokenStore backed by Azure Key Vault, so distributors can persist MS SAM refresh tokens in the vault instead of local disk. Lives in a separate submodule (plugins/contrib/microsoft/keyvault) so the Azure SDK does not leak into the default chaperone binary or existing contrib consumers. Secret names are SHA-256 hex of the tenantID (Key Vault names forbid dots; a naive dot-to-hyphen mapping collides), with the original tenantID preserved as a tag for operator visibility. Distributors supply an azcore.TokenCredential of their choice. The onboarding CLI is unchanged -- operators pipe its stdout into az keyvault secret set using the same SHA-256 hex encoding, documented in the onboarding guide. --- docs/guides/onboarding-refresh-tokens.md | 50 ++ docs/reference/contrib-plugins.md | 112 ++++ go.mod | 6 +- go.sum | 10 +- go.work | 1 + go.work.sum | 51 ++ plugins/contrib/go.mod | 2 +- plugins/contrib/go.sum | 3 +- plugins/contrib/microsoft/keyvault/go.mod | 18 + plugins/contrib/microsoft/keyvault/go.sum | 42 ++ plugins/contrib/microsoft/keyvault/name.go | 21 + .../contrib/microsoft/keyvault/name_test.go | 85 +++ plugins/contrib/microsoft/keyvault/store.go | 208 +++++++ .../contrib/microsoft/keyvault/store_test.go | 513 ++++++++++++++++++ 14 files changed, 1110 insertions(+), 12 deletions(-) create mode 100644 go.work.sum create mode 100644 plugins/contrib/microsoft/keyvault/go.mod create mode 100644 plugins/contrib/microsoft/keyvault/go.sum create mode 100644 plugins/contrib/microsoft/keyvault/name.go create mode 100644 plugins/contrib/microsoft/keyvault/name_test.go create mode 100644 plugins/contrib/microsoft/keyvault/store.go create mode 100644 plugins/contrib/microsoft/keyvault/store_test.go diff --git a/docs/guides/onboarding-refresh-tokens.md b/docs/guides/onboarding-refresh-tokens.md index 40d0a49..af627d9 100644 --- a/docs/guides/onboarding-refresh-tokens.md +++ b/docs/guides/onboarding-refresh-tokens.md @@ -123,6 +123,56 @@ store := microsoft.NewFileStore("/var/lib/chaperone/tokens") Each tenant gets a single file. The base directory is created automatically at runtime when Microsoft rotates the token. +### Azure Key Vault + +[`microsoft.KeyVaultStore`](../reference/contrib-plugins.md#microsoft-keyvaultstore) persists each tenant's refresh token as a Key Vault secret. The secret name is `{prefix}{hex(sha256(tenantID))}`; the original tenantID is preserved on the `tenantID` tag. `chaperone-onboard` has no dedicated Key Vault subcommand — pipe stdout directly into `az keyvault secret set`. + +**Prerequisites:** + +- An Azure Key Vault with the identity running chaperone granted `Key Vault Secrets Officer` (or equivalent `get` + `set` access policy). +- The Azure CLI (`az`) logged in as a principal with `set` permission. +- `shasum` or `sha256sum` available locally (any POSIX system has one). + +**Seed one tenant:** + +```bash +TENANT="contoso.onmicrosoft.com" +PREFIX="chaperone-rt-" # must match keyvault.Config.Prefix (default shown) +HASH=$(printf '%s' "$TENANT" | shasum -a 256 | awk '{print $1}') +SECRET_NAME="${PREFIX}${HASH}" + +export CHAPERONE_ONBOARD_CLIENT_SECRET='your-app-secret' + +chaperone-onboard microsoft \ + -tenant "$TENANT" \ + -client-id 12345678-abcd-1234-abcd-1234567890ab \ + | az keyvault secret set \ + --vault-name myvault \ + --name "$SECRET_NAME" \ + --file /dev/stdin \ + --tags "tenantID=$TENANT" "managedBy=chaperone" \ + --output none +``` + +> **Keep the prefix aligned.** If you customize `keyvault.Config.Prefix` (e.g., `"env-staging-"` to namespace multiple environments in a shared vault), use the same prefix when computing `SECRET_NAME` above. Otherwise the proxy will look for a different secret name than the one you created and return `ErrTenantNotFound`. + +After seeding, point the proxy at the vault: + +```go +import ( + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" + "github.com/cloudblue/chaperone/plugins/contrib/microsoft/keyvault" +) + +cred, _ := azidentity.NewDefaultAzureCredential(nil) +store, _ := keyvault.NewStore(keyvault.Config{ + VaultURL: "https://myvault.vault.azure.net/", + Credential: cred, +}) +``` + +Rotations land as new secret versions; `Load` always reads the latest. + ### Other backends - **Vault:** Use the Vault CLI or API to write the token to the expected KV path. See the [Vault-backed skeleton](../reference/contrib-plugins.md#vault-backed-tokenstore) in the contrib reference. diff --git a/docs/reference/contrib-plugins.md b/docs/reference/contrib-plugins.md index bcd1260..27aca2d 100644 --- a/docs/reference/contrib-plugins.md +++ b/docs/reference/contrib-plugins.md @@ -727,6 +727,118 @@ Seed each tenant with [`chaperone-onboard microsoft`](../guides/onboarding-refre fabrikam.onmicrosoft.com ``` + +### Microsoft `KeyVaultStore` + +```go +type Store struct{ /* unexported */ } +``` + +An Azure Key Vault-backed [`TokenStore`](#tokenstoremicrosoft) that stores one refresh token per tenant as a Key Vault secret. Lives in the separate submodule `github.com/cloudblue/chaperone/plugins/contrib/microsoft/keyvault` so the Azure SDK dependencies do not leak into consumers that use only `FileStore` or other contrib building blocks. + +```go +import "github.com/cloudblue/chaperone/plugins/contrib/microsoft/keyvault" +``` + +#### `Config` + +```go +type Config struct { + VaultURL string // required, e.g. "https://myvault.vault.azure.net/" + Credential azcore.TokenCredential // required + Prefix string // default: "chaperone-rt-" + ClientOptions *azsecrets.ClientOptions + Logger *slog.Logger +} +``` + +| Field | Description | +|-------|-------------| +| `VaultURL` | The Key Vault URL. Standard vaults only (`*.vault.azure.net` or sovereign-cloud equivalents). Managed HSM is not tested. | +| `Credential` | Any [`azcore.TokenCredential`](https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azcore#TokenCredential) — the distributor constructs it from `azidentity` (`NewDefaultAzureCredential`, `NewManagedIdentityCredential`, `NewWorkloadIdentityCredential`, `NewClientSecretCredential`, etc.). | +| `Prefix` | Prepended to every secret name. Override to namespace multiple environments in a shared vault. | +| `ClientOptions` | Passed through to `azsecrets.NewClient`. Use to configure retry policy, transport, etc. | +| `Logger` | Structured logger for warnings. Resolved lazily — `nil` means `slog.Default()` is used at log-emit time. | + +#### `NewStore` + +```go +func NewStore(cfg Config) (*Store, error) +``` + +Constructs a `Store`. Returns an error if `VaultURL` or `Credential` is missing, or if the underlying `azsecrets` client fails to initialize. Unlike [`NewFileStore`](#newfilestore), it returns an error instead of panicking on misconfiguration — a Key Vault store typically runs far from the local disk, so fail-fast with an error is more operationally friendly. + +#### Secret naming + +Each tenant maps to a secret named `{Prefix}{hex(sha256(tenantID))}`. SHA-256 hex is used because: + +- Key Vault secret names are restricted to `^[0-9a-zA-Z-]{1,127}$` — no dots allowed. Valid tenantIDs may contain dots (`contoso.onmicrosoft.com`), so a naive encoding would fail. +- A naive "dot→hyphen" substitution would collide (`my-a.b` and `my.a-b` both map to `my-a-b`). SHA-256 guarantees collision resistance. +- Secret names appear in Azure Activity Log entries; hashing the tenantID keeps tenant identities out of the audit log. + +The original `tenantID` is preserved on each secret as the `tenantID` tag for operator visibility in the Azure portal. Chaperone also adds a `managedBy: chaperone` tag. + +#### RBAC + +The identity backing `Credential` needs Get and Set permissions on secrets in the vault. Either: + +- **Azure RBAC**: `Key Vault Secrets User` (read) + `Key Vault Secrets Officer` (write) — scope to the vault. If write access is already covered, `Key Vault Secrets Officer` alone suffices. +- **Access Policies**: `get` + `set` on secrets. + +`list`, `delete`, and `recover` are **not** required. + +#### Error behavior + +| Method | Condition | Error | +|--------|-----------|-------| +| `Load` | Secret does not exist (`SecretNotFound`, HTTP 404) | Wraps [`ErrTenantNotFound`](#sentinel-errors) | +| `Load` | Any other Key Vault failure (HTTP 403, 429, 5xx, network error) | Wrapped SDK error (not `ErrTenantNotFound`) | +| `Load` / `Save` | Invalid `tenantID` | Validation error (not `ErrTenantNotFound`) | +| `Save` | Empty `refreshToken` | Returns an error | + +Throttling (HTTP 429) is handled by the Azure SDK's default retry pipeline, which honors `Retry-After` automatically. + +#### Example: `DefaultAzureCredential` + +```go +import ( + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" + "github.com/cloudblue/chaperone/plugins/contrib/microsoft" + "github.com/cloudblue/chaperone/plugins/contrib/microsoft/keyvault" +) + +cred, err := azidentity.NewDefaultAzureCredential(nil) +if err != nil { /* ... */ } + +store, err := keyvault.NewStore(keyvault.Config{ + VaultURL: "https://myvault.vault.azure.net/", + Credential: cred, +}) +if err != nil { /* ... */ } + +source := microsoft.NewRefreshTokenSource(microsoft.Config{ + ClientID: os.Getenv("MS_CLIENT_ID"), + ClientSecret: os.Getenv("MS_CLIENT_SECRET"), + Store: store, +}) +``` + +#### Example: `ManagedIdentityCredential` (AKS pod with system-assigned identity) + +```go +cred, err := azidentity.NewManagedIdentityCredential(nil) +``` + +#### Example: `WorkloadIdentityCredential` (AKS workload identity) + +```go +cred, err := azidentity.NewWorkloadIdentityCredential(nil) +``` + +#### Seeding tenants + +See the [Azure Key Vault section](../guides/onboarding-refresh-tokens.md#azure-key-vault) of the onboarding guide for the operator workflow. + --- ## Sentinel errors diff --git a/go.mod b/go.mod index 683d4cf..0edc193 100644 --- a/go.mod +++ b/go.mod @@ -31,9 +31,9 @@ require ( go.opentelemetry.io/otel/metric v1.43.0 // indirect go.opentelemetry.io/proto/otlp v1.10.0 // indirect go.yaml.in/yaml/v2 v2.4.3 // indirect - golang.org/x/net v0.52.0 // indirect - golang.org/x/sys v0.42.0 // indirect - golang.org/x/text v0.35.0 // indirect + golang.org/x/net v0.53.0 // indirect + golang.org/x/sys v0.43.0 // indirect + golang.org/x/text v0.36.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 // indirect google.golang.org/grpc v1.80.0 // indirect diff --git a/go.sum b/go.sum index 0aa4dbe..941b6b6 100644 --- a/go.sum +++ b/go.sum @@ -67,12 +67,10 @@ go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.yaml.in/yaml/v2 v2.4.3 h1:6gvOSjQoTB3vt1l+CU+tSyi/HOjfOjRLJ4YwYZGwRO0= go.yaml.in/yaml/v2 v2.4.3/go.mod h1:zSxWcmIDjOzPXpjlTTbAsKokqkDNAVtZO0WOMiT90s8= -golang.org/x/net v0.52.0 h1:He/TN1l0e4mmR3QqHMT2Xab3Aj3L9qjbhRm78/6jrW0= -golang.org/x/net v0.52.0/go.mod h1:R1MAz7uMZxVMualyPXb+VaqGSa3LIaUqk0eEt3w36Sw= -golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= -golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/text v0.35.0 h1:JOVx6vVDFokkpaq1AEptVzLTpDe9KGpj5tR4/X+ybL8= -golang.org/x/text v0.35.0/go.mod h1:khi/HExzZJ2pGnjenulevKNX1W67CUy0AsXcNubPGCA= +golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= +golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= +golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= +golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg= gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9 h1:VPWxll4HlMw1Vs/qXtN7BvhZqsS9cdAittCNvVENElA= diff --git a/go.work b/go.work index 4fb25d9..6d124b1 100644 --- a/go.work +++ b/go.work @@ -3,5 +3,6 @@ go 1.26.2 use ( . ./plugins/contrib + ./plugins/contrib/microsoft/keyvault ./sdk ) diff --git a/go.work.sum b/go.work.sum new file mode 100644 index 0000000..b6297b0 --- /dev/null +++ b/go.work.sum @@ -0,0 +1,51 @@ +cel.dev/expr v0.25.1/go.mod h1:hrXvqGP6G6gyx8UAHSHJ5RGk//1Oj5nXQ2NI02Nrsg4= +cloud.google.com/go/compute/metadata v0.9.0/go.mod h1:E0bWwX5wTnLPedCKqk3pJmVgCBSM6qQI1yTBdEb3C10= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.19.1/go.mod h1:YD5h/ldMsG0XiIw7PdyNhLxaM317eFh5yNLccNfGdyw= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1 h1:jHb/wfvRikGdxMXYV3QG/SzUOPYN9KEUUuC0Yd0/vC0= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1/go.mod h1:pzBXCYn05zvYIrwLgtK8Ap8QcjRg+0i76tMQdWN6wOk= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0 h1:fhqpLE3UEXi9lPaBRpQ6XuRW0nU7hgg4zlmZZa+a9q4= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0/go.mod h1:7dCRMLwisfRH3dBupKeNCioWYUZ4SS09Z14H+7i8ZoY= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.4.0/go.mod h1:gpl+q95AzZlKVI3xSoseF9QPrypk0hQqBiJYeB/cR/I= +github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.31.0/go.mod h1:P4WPRUkOhJC13W//jWpyfJNDAIpvRbAUIYLX/4jtlE0= +github.com/alecthomas/kingpin/v2 v2.4.0/go.mod h1:0gyi0zQnjuFk8xrkNKamJoyUo382HRL7ATRpFZCw6tE= +github.com/alecthomas/units v0.0.0-20240927000941-0f3dac36c52b/go.mod h1:fvzegU4vN3H1qMT+8wDmzjAcDONcgo2/SZ/TyfdUOFs= +github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= +github.com/cloudblue/chaperone/plugins/contrib v0.1.0/go.mod h1:ZLjFwm8WU12NbjQTWHLMlKuw0Fn5SYZtuv8mMkzAhEI= +github.com/cloudblue/chaperone/plugins/contrib v0.1.1/go.mod h1:ZLjFwm8WU12NbjQTWHLMlKuw0Fn5SYZtuv8mMkzAhEI= +github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5/go.mod h1:KdCmV+x/BuvyMxRnYBlmVaq4OLiKW6iRQfvC62cvdkI= +github.com/envoyproxy/go-control-plane v0.14.0/go.mod h1:NcS5X47pLl/hfqxU70yPwL9ZMkUlwlKxtAohpi2wBEU= +github.com/envoyproxy/go-control-plane/envoy v1.36.0/go.mod h1:ty89S1YCCVruQAm9OtKeEkQLTb+Lkz0k8v9W0Oxsv98= +github.com/envoyproxy/go-control-plane/ratelimit v0.1.0/go.mod h1:Wk+tMFAFbCXaJPzVVHnPgRKdUdwW/KdbRt94AzgRee4= +github.com/envoyproxy/protoc-gen-validate v1.3.0/go.mod h1:HvYl7zwPa5mffgyeTUHA9zHIH36nmrm7oCbo4YKoSWA= +github.com/go-jose/go-jose/v4 v4.1.3/go.mod h1:x4oUasVrzR7071A4TnHLGSPpNOm2a21K9Kf04k1rs08= +github.com/golang-jwt/jwt/v5 v5.3.0/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= +github.com/golang/glog v1.2.5/go.mod h1:6AhwSGph0fcJtXVM/PEHPqZlFeoLxhs7/t5UDAwmO+w= +github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4= +github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= +github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM= +github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= +github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= +github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= +github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1:t/avpk3KcrXxUnYOhZhMXJlSEyie6gQbtLq5NM3loB8= +github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= +github.com/spiffe/go-spiffe/v2 v2.6.0/go.mod h1:gm2SeUoMZEtpnzPNs2Csc0D/gX33k1xIx7lEzqblHEs= +github.com/xhit/go-str2duration/v2 v2.1.0/go.mod h1:ohY8p+0f07DiV6Em5LKB0s2YpLtXVyJfNt1+BlmyAsU= +go.opentelemetry.io/contrib/detectors/gcp v1.39.0/go.mod h1:t/OGqzHBa5v6RHZwrDBJ2OirWc+4q/w2fTbLZwAKjTk= +go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= +golang.org/x/crypto v0.49.0/go.mod h1:ErX4dUh2UM+CFYiXZRTcMpEcN8b/1gxEuv3nODoYtCA= +golang.org/x/crypto v0.50.0/go.mod h1:3muZ7vA7PBCE6xgPX7nkzzjiUq87kRItoJQM1Yo8S+Q= +golang.org/x/mod v0.33.0/go.mod h1:swjeQEj+6r7fODbD2cqrnje9PnziFuw4bmLbBZFrQ5w= +golang.org/x/mod v0.34.0/go.mod h1:ykgH52iCZe79kzLLMhyCUzhMci+nQj+0XkbXpNYtVjY= +golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= +golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= +golang.org/x/oauth2 v0.35.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= +golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/term v0.41.0/go.mod h1:3pfBgksrReYfZ5lvYM0kSO0LIkAl4Yl2bXOkKP7Ec2A= +golang.org/x/term v0.42.0/go.mod h1:Dq/D+snpsbazcBG5+F9Q1n2rXV8Ma+71xEjTRufARgY= +golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg= +golang.org/x/text v0.36.0/go.mod h1:NIdBknypM8iqVmPiuco0Dh6P5Jcdk8lJL0CUebqK164= +golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0= +golang.org/x/tools v0.43.0/go.mod h1:uHkMso649BX2cZK6+RpuIPXS3ho2hZo4FVwfoy1vIk0= diff --git a/plugins/contrib/go.mod b/plugins/contrib/go.mod index 65b8f7f..13a3255 100644 --- a/plugins/contrib/go.mod +++ b/plugins/contrib/go.mod @@ -4,4 +4,4 @@ go 1.26.2 require github.com/cloudblue/chaperone/sdk v0.1.0 -require golang.org/x/sync v0.19.0 +require golang.org/x/sync v0.20.0 diff --git a/plugins/contrib/go.sum b/plugins/contrib/go.sum index daaec07..26cf7b3 100644 --- a/plugins/contrib/go.sum +++ b/plugins/contrib/go.sum @@ -1,4 +1,3 @@ github.com/cloudblue/chaperone/sdk v0.1.0 h1:OsrqjLfcaP35eSRLzsmL1r6wYf4IkmE/WG17GSCdgIE= github.com/cloudblue/chaperone/sdk v0.1.0/go.mod h1:p6JOMXPqVfm8EqvnyDAozgrmkvhfbs1O32am/dthnFc= -golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= -golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= diff --git a/plugins/contrib/microsoft/keyvault/go.mod b/plugins/contrib/microsoft/keyvault/go.mod new file mode 100644 index 0000000..64a2a0c --- /dev/null +++ b/plugins/contrib/microsoft/keyvault/go.mod @@ -0,0 +1,18 @@ +module github.com/cloudblue/chaperone/plugins/contrib/microsoft/keyvault + +go 1.26.2 + +require ( + github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1 + github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.4.0 + github.com/cloudblue/chaperone/plugins/contrib v0.1.1 +) + +require ( + github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0 // indirect + github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.2.0 // indirect + github.com/cloudblue/chaperone/sdk v0.1.0 // indirect + golang.org/x/net v0.53.0 // indirect + golang.org/x/sync v0.20.0 // indirect + golang.org/x/text v0.36.0 // indirect +) diff --git a/plugins/contrib/microsoft/keyvault/go.sum b/plugins/contrib/microsoft/keyvault/go.sum new file mode 100644 index 0000000..82dabc2 --- /dev/null +++ b/plugins/contrib/microsoft/keyvault/go.sum @@ -0,0 +1,42 @@ +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1 h1:jHb/wfvRikGdxMXYV3QG/SzUOPYN9KEUUuC0Yd0/vC0= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1/go.mod h1:pzBXCYn05zvYIrwLgtK8Ap8QcjRg+0i76tMQdWN6wOk= +github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1 h1:Hk5QBxZQC1jb2Fwj6mpzme37xbCDdNTxU7O9eb5+LB4= +github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1/go.mod h1:IYus9qsFobWIc2YVwe/WPjcnyCkPKtnHAqUYeebc8z0= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0 h1:fhqpLE3UEXi9lPaBRpQ6XuRW0nU7hgg4zlmZZa+a9q4= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0/go.mod h1:7dCRMLwisfRH3dBupKeNCioWYUZ4SS09Z14H+7i8ZoY= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.4.0 h1:/g8S6wk65vfC6m3FIxJ+i5QDyN9JWwXI8Hb0Img10hU= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.4.0/go.mod h1:gpl+q95AzZlKVI3xSoseF9QPrypk0hQqBiJYeB/cR/I= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.2.0 h1:nCYfgcSyHZXJI8J0IWE5MsCGlb2xp9fJiXyxWgmOFg4= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.2.0/go.mod h1:ucUjca2JtSZboY8IoUqyQyuuXvwbMBVwFOm0vdQPNhA= +github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0 h1:XRzhVemXdgvJqCH0sFfrBUTnUJSBrBf7++ypk+twtRs= +github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0/go.mod h1:HKpQxkWaGLJ+D/5H8QRpyQXA1eKjxkFlOMwck5+33Jk= +github.com/cloudblue/chaperone/plugins/contrib v0.1.1 h1:Kg6hrUt4QHBWLQDcv88eQRzLi4GtJq2mqAgPAKKRK/I= +github.com/cloudblue/chaperone/plugins/contrib v0.1.1/go.mod h1:ZLjFwm8WU12NbjQTWHLMlKuw0Fn5SYZtuv8mMkzAhEI= +github.com/cloudblue/chaperone/sdk v0.1.0 h1:OsrqjLfcaP35eSRLzsmL1r6wYf4IkmE/WG17GSCdgIE= +github.com/cloudblue/chaperone/sdk v0.1.0/go.mod h1:p6JOMXPqVfm8EqvnyDAozgrmkvhfbs1O32am/dthnFc= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/golang-jwt/jwt/v5 v5.3.0 h1:pv4AsKCKKZuqlgs5sUmn4x8UlGa0kEVt/puTpKx9vvo= +github.com/golang-jwt/jwt/v5 v5.3.0/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= +github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= +github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ= +github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +golang.org/x/crypto v0.50.0 h1:zO47/JPrL6vsNkINmLoo/PH1gcxpls50DNogFvB5ZGI= +golang.org/x/crypto v0.50.0/go.mod h1:3muZ7vA7PBCE6xgPX7nkzzjiUq87kRItoJQM1Yo8S+Q= +golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= +golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= +golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg= +golang.org/x/text v0.36.0/go.mod h1:NIdBknypM8iqVmPiuco0Dh6P5Jcdk8lJL0CUebqK164= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/plugins/contrib/microsoft/keyvault/name.go b/plugins/contrib/microsoft/keyvault/name.go new file mode 100644 index 0000000..ae373fa --- /dev/null +++ b/plugins/contrib/microsoft/keyvault/name.go @@ -0,0 +1,21 @@ +// Copyright 2026 CloudBlue LLC +// SPDX-License-Identifier: Apache-2.0 + +package keyvault + +import ( + "crypto/sha256" + "encoding/hex" +) + +// secretName maps a tenantID to a Key Vault secret name using SHA-256 hex. +// This guarantees a collision-free, always-valid name. Key Vault secret names +// allow only [0-9a-zA-Z-] and cap at 127 chars — a 64-char hex hash plus a +// short prefix fits comfortably and can never collide for distinct tenants. +// +// The original tenantID is preserved in the secret's "tenantID" tag for +// operator visibility in the Azure portal. +func secretName(prefix, tenantID string) string { + sum := sha256.Sum256([]byte(tenantID)) + return prefix + hex.EncodeToString(sum[:]) +} diff --git a/plugins/contrib/microsoft/keyvault/name_test.go b/plugins/contrib/microsoft/keyvault/name_test.go new file mode 100644 index 0000000..f93611f --- /dev/null +++ b/plugins/contrib/microsoft/keyvault/name_test.go @@ -0,0 +1,85 @@ +// Copyright 2026 CloudBlue LLC +// SPDX-License-Identifier: Apache-2.0 + +package keyvault + +import "testing" + +func TestSecretName_Deterministic(t *testing.T) { + const ( + prefix = "chaperone-rt-" + tenantID = "contoso.onmicrosoft.com" + ) + + first := secretName(prefix, tenantID) + second := secretName(prefix, tenantID) + + if first != second { + t.Errorf("secretName() not deterministic: %q != %q", first, second) + } +} + +func TestSecretName_CollisionSafety(t *testing.T) { + // These two tenants would collide under a naive "dot→hyphen" mapping + // but must map to distinct secret names under SHA-256 hex. + const ( + a = "my-a.b" + b = "my.a-b" + ) + + nameA := secretName(defaultPrefix, a) + nameB := secretName(defaultPrefix, b) + + if nameA == nameB { + t.Errorf("collision: secretName(%q) == secretName(%q) == %q", a, b, nameA) + } +} + +func TestSecretName_ValidKeyVaultName(t *testing.T) { + tenants := []string{ + "contoso.onmicrosoft.com", + "my-company.onmicrosoft.com", + "00000000-0000-0000-0000-000000000000", + "common", + "organizations", + "consumers", + "a", + "tenant.with.many.dots.example.com", + } + + for _, tenant := range tenants { + name := secretName(defaultPrefix, tenant) + if !isValidKeyVaultSecretName(name) { + t.Errorf("secretName(%q) = %q, not a valid Key Vault secret name", tenant, name) + } + } +} + +func TestSecretName_RespectsPrefix(t *testing.T) { + const tenantID = "contoso.onmicrosoft.com" + + envA := secretName("envA-", tenantID) + envB := secretName("envB-", tenantID) + + if envA == envB { + t.Errorf("prefixes ignored: %q == %q", envA, envB) + } + if got := envA[:5]; got != "envA-" { + t.Errorf("envA name = %q, want prefix %q", envA, "envA-") + } + if got := envB[:5]; got != "envB-" { + t.Errorf("envB name = %q, want prefix %q", envB, "envB-") + } +} + +func TestSecretName_LengthWithinKeyVaultLimit(t *testing.T) { + // Even with a long prefix and domain tenantID, the resulting name must + // fit within Key Vault's 127-character secret name limit. + const longPrefix = "chaperone-rt-very-long-prefix-for-testing-" + + name := secretName(longPrefix, "very-long-tenant-name.onmicrosoft.com") + + if len(name) > 127 { + t.Errorf("secretName length = %d, exceeds Key Vault limit of 127", len(name)) + } +} diff --git a/plugins/contrib/microsoft/keyvault/store.go b/plugins/contrib/microsoft/keyvault/store.go new file mode 100644 index 0000000..9e1e583 --- /dev/null +++ b/plugins/contrib/microsoft/keyvault/store.go @@ -0,0 +1,208 @@ +// Copyright 2026 CloudBlue LLC +// SPDX-License-Identifier: Apache-2.0 + +// Package keyvault provides an Azure Key Vault-backed implementation of +// microsoft.TokenStore for persisting Microsoft SAM refresh tokens. +// +// The store maps each tenant to a single Key Vault secret named +// prefix+hex(sha256(tenantID)). The original tenantID is preserved as a +// "tenantID" tag on the secret for operator visibility. +// +// Secret rotation is handled by Key Vault: each Save creates a new secret +// version, Load always reads the latest version. +package keyvault + +import ( + "context" + "errors" + "fmt" + "log/slog" + "net/http" + "regexp" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets" + + "github.com/cloudblue/chaperone/plugins/contrib" + "github.com/cloudblue/chaperone/plugins/contrib/microsoft" +) + +// defaultPrefix is prepended to every secret name. Override via Config.Prefix +// to namespace multiple environments in a shared vault. +const defaultPrefix = "chaperone-rt-" + +// secretNotFoundCode is the ErrorCode returned by Key Vault when a secret +// does not exist in the vault. +const secretNotFoundCode = "SecretNotFound" + +// validTenantID mirrors microsoft.validTenantID, which is unexported. Any +// change there should be reflected here. It matches Azure AD tenant +// identifiers (GUIDs, domain names) and rejects path separators, query +// strings, and fragments. +var validTenantID = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9.\-]*$`) + +// Config configures a Key Vault-backed TokenStore. +type Config struct { + // VaultURL is the Key Vault URL, e.g. "https://myvault.vault.azure.net/". + // Required. + VaultURL string + + // Credential authenticates to Key Vault. Any azcore.TokenCredential works; + // azidentity.NewDefaultAzureCredential, NewManagedIdentityCredential, and + // NewWorkloadIdentityCredential are typical. Required. + Credential azcore.TokenCredential + + // Prefix is prepended to every secret name. Defaults to "chaperone-rt-". + // Override to namespace multiple environments in a shared vault. + Prefix string + + // ClientOptions are passed through to azsecrets.NewClient. Optional. + ClientOptions *azsecrets.ClientOptions + + // Logger for debug/warning messages. If nil, slog.Default() is used + // (resolved lazily, matching the RefreshTokenSource pattern). + Logger *slog.Logger +} + +// secretsClient is the narrow subset of *azsecrets.Client that Store uses. +// Declaring it as a local interface enables fake-based unit testing without +// pulling in the full SDK client. +type secretsClient interface { + GetSecret(ctx context.Context, name, version string, + opts *azsecrets.GetSecretOptions) (azsecrets.GetSecretResponse, error) + SetSecret(ctx context.Context, name string, params azsecrets.SetSecretParameters, + opts *azsecrets.SetSecretOptions) (azsecrets.SetSecretResponse, error) +} + +// Store persists Microsoft SAM refresh tokens in Azure Key Vault. +// Safe for concurrent use. +type Store struct { + client secretsClient + prefix string + logger *slog.Logger +} + +// Compile-time check that Store implements microsoft.TokenStore. +var _ microsoft.TokenStore = (*Store)(nil) + +// NewStore constructs a Store from a Config. Returns an error if VaultURL or +// Credential is missing, or if the underlying azsecrets client fails to +// initialize. +func NewStore(cfg Config) (*Store, error) { + if cfg.VaultURL == "" { + return nil, errors.New("keyvault: VaultURL is required") + } + if cfg.Credential == nil { + return nil, errors.New("keyvault: Credential is required") + } + + client, err := azsecrets.NewClient(cfg.VaultURL, cfg.Credential, cfg.ClientOptions) + if err != nil { + return nil, fmt.Errorf("keyvault: creating azsecrets client: %w", err) + } + + prefix := cfg.Prefix + if prefix == "" { + prefix = defaultPrefix + } + + return &Store{ + client: client, + prefix: prefix, + logger: cfg.Logger, + }, nil +} + +// newStoreWithClient is the test-only constructor that accepts a preconstructed +// secretsClient. The public NewStore wraps azsecrets.NewClient and delegates +// here. Kept unexported so production callers can only reach it via NewStore. +func newStoreWithClient(client secretsClient, logger *slog.Logger) *Store { + return &Store{client: client, prefix: defaultPrefix, logger: logger} +} + +// log returns the configured logger, or slog.Default() if none was set. +// Called at log-emit time so the current global default is always used +// when no explicit logger is provided. +func (s *Store) log() *slog.Logger { + if s.logger != nil { + return s.logger + } + return slog.Default() +} + +// Load retrieves the current refresh token for the given tenant. +// Returns an error wrapping [contrib.ErrTenantNotFound] if the tenant has no +// stored token. +func (s *Store) Load(ctx context.Context, tenantID string) (string, error) { + if err := validateTenantID(tenantID); err != nil { + return "", err + } + + name := secretName(s.prefix, tenantID) + + resp, err := s.client.GetSecret(ctx, name, "", nil) + if err != nil { + var respErr *azcore.ResponseError + if errors.As(err, &respErr) && + respErr.StatusCode == http.StatusNotFound && + respErr.ErrorCode == secretNotFoundCode { + s.log().LogAttrs(ctx, slog.LevelDebug, "no refresh token in key vault for tenant", + slog.String("tenant_id", tenantID), + slog.String("secret_name", name)) + return "", fmt.Errorf("no token for tenant %s: %w", + tenantID, contrib.ErrTenantNotFound) + } + return "", fmt.Errorf("loading token for tenant %s: %w", tenantID, err) + } + + if resp.Value == nil { + return "", fmt.Errorf("key vault returned nil secret value for tenant %s", tenantID) + } + + return *resp.Value, nil +} + +// Save persists a rotated refresh token to Key Vault. Each call creates a +// new secret version; Load always reads the latest. +func (s *Store) Save(ctx context.Context, tenantID, refreshToken string) error { + if err := validateTenantID(tenantID); err != nil { + return err + } + if refreshToken == "" { + return errors.New("refusing to save empty refresh token") + } + + name := secretName(s.prefix, tenantID) + tenantIDCopy := tenantID + managedBy := "chaperone" + + params := azsecrets.SetSecretParameters{ + Value: &refreshToken, + Tags: map[string]*string{ + "tenantID": &tenantIDCopy, + "managedBy": &managedBy, + }, + } + + if _, err := s.client.SetSecret(ctx, name, params, nil); err != nil { + return fmt.Errorf("saving token for tenant %s: %w", tenantID, err) + } + + return nil +} + +// validateTenantID rejects tenant IDs that could cause path traversal or that +// don't match Azure AD's tenant ID grammar. Defense in depth: the outer +// RefreshTokenSource already validates, but Store is a public type and must +// not rely on callers for safety. +func validateTenantID(tenantID string) error { + if !validTenantID.MatchString(tenantID) { + display := tenantID + if len(display) > 64 { + display = display[:64] + "..." + } + return fmt.Errorf("keyvault: invalid tenant ID %q: must match %s", + display, validTenantID.String()) + } + return nil +} diff --git a/plugins/contrib/microsoft/keyvault/store_test.go b/plugins/contrib/microsoft/keyvault/store_test.go new file mode 100644 index 0000000..661c992 --- /dev/null +++ b/plugins/contrib/microsoft/keyvault/store_test.go @@ -0,0 +1,513 @@ +// Copyright 2026 CloudBlue LLC +// SPDX-License-Identifier: Apache-2.0 + +package keyvault + +import ( + "bytes" + "context" + "errors" + "fmt" + "log/slog" + "net/http" + "strings" + "sync" + "sync/atomic" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets" + + "github.com/cloudblue/chaperone/plugins/contrib" +) + +// fakeSecretsClient is an in-memory secretsClient implementation for tests. +// Safe for concurrent use. +type fakeSecretsClient struct { + mu sync.Mutex + secrets map[string]fakeSecret // key: secret name (post-encoding) + + getErr error + setErr error + + getCalls atomic.Int32 + setCalls atomic.Int32 +} + +type fakeSecret struct { + value string + tags map[string]*string +} + +func newFakeSecretsClient() *fakeSecretsClient { + return &fakeSecretsClient{secrets: make(map[string]fakeSecret)} +} + +func (f *fakeSecretsClient) GetSecret( + _ context.Context, name, _ string, _ *azsecrets.GetSecretOptions, +) (azsecrets.GetSecretResponse, error) { + f.getCalls.Add(1) + if f.getErr != nil { + return azsecrets.GetSecretResponse{}, f.getErr + } + f.mu.Lock() + defer f.mu.Unlock() + s, ok := f.secrets[name] + if !ok { + return azsecrets.GetSecretResponse{}, &azcore.ResponseError{ + StatusCode: http.StatusNotFound, + ErrorCode: secretNotFoundCode, + } + } + val := s.value + return azsecrets.GetSecretResponse{ + Secret: azsecrets.Secret{Value: &val, Tags: s.tags}, + }, nil +} + +func (f *fakeSecretsClient) SetSecret( + _ context.Context, name string, params azsecrets.SetSecretParameters, _ *azsecrets.SetSecretOptions, +) (azsecrets.SetSecretResponse, error) { + f.setCalls.Add(1) + if f.setErr != nil { + return azsecrets.SetSecretResponse{}, f.setErr + } + f.mu.Lock() + defer f.mu.Unlock() + f.secrets[name] = fakeSecret{value: *params.Value, tags: params.Tags} + return azsecrets.SetSecretResponse{ + Secret: azsecrets.Secret{Value: params.Value, Tags: params.Tags}, + }, nil +} + +func (f *fakeSecretsClient) get(name string) (fakeSecret, bool) { + f.mu.Lock() + defer f.mu.Unlock() + s, ok := f.secrets[name] + return s, ok +} + +func newTestStore(t *testing.T) (*Store, *fakeSecretsClient) { + t.Helper() + fake := newFakeSecretsClient() + return newStoreWithClient(fake, nil), fake +} + +func TestStore_SaveAndLoad_RoundTrip(t *testing.T) { + store, _ := newTestStore(t) + ctx := context.Background() + + const ( + tenant = "contoso.onmicrosoft.com" + token = "rt_abc123_secret" + ) + + if err := store.Save(ctx, tenant, token); err != nil { + t.Fatalf("Save() error = %v", err) + } + + got, err := store.Load(ctx, tenant) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if got != token { + t.Errorf("Load() = %q, want %q", got, token) + } +} + +func TestStore_Load_SecretNotFound_ReturnsErrTenantNotFound(t *testing.T) { + store, _ := newTestStore(t) + + _, err := store.Load(context.Background(), "contoso.onmicrosoft.com") + if err == nil { + t.Fatal("Load() expected error on empty vault, got nil") + } + if !errors.Is(err, contrib.ErrTenantNotFound) { + t.Errorf("Load() error should wrap contrib.ErrTenantNotFound, got: %v", err) + } +} + +func TestStore_Load_OtherAPIError_Propagated(t *testing.T) { + store, fake := newTestStore(t) + + sentinel := errors.New("transient network failure") + fake.getErr = sentinel + + _, err := store.Load(context.Background(), "contoso.onmicrosoft.com") + if err == nil { + t.Fatal("Load() expected error, got nil") + } + if !errors.Is(err, sentinel) { + t.Errorf("Load() should wrap sentinel error, got: %v", err) + } + if errors.Is(err, contrib.ErrTenantNotFound) { + t.Error("Load() should NOT wrap ErrTenantNotFound for a non-404 error") + } +} + +func TestStore_Load_WrongErrorCode_NotTreatedAsNotFound(t *testing.T) { + store, fake := newTestStore(t) + + // 404 with a different ErrorCode must NOT be mapped to ErrTenantNotFound + // — only the specific SecretNotFound code signals "no token for tenant". + fake.getErr = &azcore.ResponseError{ + StatusCode: http.StatusNotFound, + ErrorCode: "Forbidden", + } + + _, err := store.Load(context.Background(), "contoso.onmicrosoft.com") + if err == nil { + t.Fatal("Load() expected error, got nil") + } + if errors.Is(err, contrib.ErrTenantNotFound) { + t.Error("Load() should only map SecretNotFound to ErrTenantNotFound") + } +} + +func TestStore_Load_NilSecretValue_ReturnsError(t *testing.T) { + store := newStoreWithClient(nilValueSecretsClient{}, nil) + + _, err := store.Load(context.Background(), "contoso.onmicrosoft.com") + if err == nil { + t.Fatal("Load() expected error on nil secret value, got nil") + } +} + +// nilValueSecretsClient returns a GetSecret response whose Value pointer is +// nil — a possible defect in a live Key Vault response. Store.Load must +// detect this rather than panic on a nil dereference. +type nilValueSecretsClient struct{} + +func (nilValueSecretsClient) GetSecret( + _ context.Context, _, _ string, _ *azsecrets.GetSecretOptions, +) (azsecrets.GetSecretResponse, error) { + return azsecrets.GetSecretResponse{Secret: azsecrets.Secret{Value: nil}}, nil +} + +func (nilValueSecretsClient) SetSecret( + _ context.Context, _ string, _ azsecrets.SetSecretParameters, _ *azsecrets.SetSecretOptions, +) (azsecrets.SetSecretResponse, error) { + return azsecrets.SetSecretResponse{}, nil +} + +func TestStore_Save_EmptyToken_ReturnsError(t *testing.T) { + store, fake := newTestStore(t) + + err := store.Save(context.Background(), "contoso.onmicrosoft.com", "") + if err == nil { + t.Fatal("Save() expected error for empty token, got nil") + } + if fake.setCalls.Load() != 0 { + t.Errorf("Save() with empty token called SetSecret %d times; want 0", + fake.setCalls.Load()) + } +} + +func TestStore_Save_SetsTenantIDTag(t *testing.T) { + store, fake := newTestStore(t) + + const tenant = "contoso.onmicrosoft.com" + if err := store.Save(context.Background(), tenant, "rt_xyz"); err != nil { + t.Fatalf("Save() error = %v", err) + } + + sec, ok := fake.get(secretName(defaultPrefix, tenant)) + if !ok { + t.Fatal("secret not stored under expected name") + } + + gotTenant, ok := sec.tags["tenantID"] + if !ok || gotTenant == nil { + t.Fatal("secret missing tenantID tag") + } + if *gotTenant != tenant { + t.Errorf("tenantID tag = %q, want %q", *gotTenant, tenant) + } + + managedBy, ok := sec.tags["managedBy"] + if !ok || managedBy == nil || *managedBy != "chaperone" { + t.Errorf("managedBy tag = %v, want \"chaperone\"", managedBy) + } +} + +func TestStore_Save_SetSecretError_Propagated(t *testing.T) { + store, fake := newTestStore(t) + + sentinel := errors.New("throttled") + fake.setErr = sentinel + + err := store.Save(context.Background(), "contoso.onmicrosoft.com", "rt_abc") + if err == nil { + t.Fatal("Save() expected error when SetSecret fails, got nil") + } + if !errors.Is(err, sentinel) { + t.Errorf("Save() should wrap sentinel error, got: %v", err) + } +} + +func TestStore_ValidateTenantID_TruncatesLongInvalidID(t *testing.T) { + store, _ := newTestStore(t) + + // Invalid char triggers failure; length > 64 triggers the truncation branch. + longInvalid := strings.Repeat("a", 70) + "/bad" + + _, err := store.Load(context.Background(), longInvalid) + if err == nil { + t.Fatal("Load() expected validation error, got nil") + } + // Error message must not contain the full invalid string and must end + // with the truncation marker inside the quoted tenant ID. + if !strings.Contains(err.Error(), `..."`) { + t.Errorf("Load() error message missing truncation marker: %v", err) + } + if strings.Contains(err.Error(), "/bad") { + t.Error("Load() error message leaked the tail of a long invalid tenantID") + } +} + +func TestStore_Save_RotationCreatesNewValue(t *testing.T) { + store, _ := newTestStore(t) + ctx := context.Background() + const tenant = "contoso.onmicrosoft.com" + + if err := store.Save(ctx, tenant, "rt_v1"); err != nil { + t.Fatalf("first Save() error = %v", err) + } + if err := store.Save(ctx, tenant, "rt_v2"); err != nil { + t.Fatalf("second Save() error = %v", err) + } + + got, err := store.Load(ctx, tenant) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if got != "rt_v2" { + t.Errorf("Load() after rotation = %q, want %q", got, "rt_v2") + } +} + +func TestStore_LoadSave_ValidatesTenantID(t *testing.T) { + tests := []struct { + name string + tenantID string + }{ + {"empty", ""}, + {"path traversal", "../evil"}, + {"leading slash", "/etc"}, + {"contains slash", "tenant/sub"}, + {"contains space", "ten ant"}, + {"starts with dot", ".hidden"}, + {"null byte", "tenant\x00"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store, fake := newTestStore(t) + ctx := context.Background() + + if _, err := store.Load(ctx, tt.tenantID); err == nil { + t.Error("Load() expected validation error, got nil") + } + if err := store.Save(ctx, tt.tenantID, "rt_abc"); err == nil { + t.Error("Save() expected validation error, got nil") + } + if fake.getCalls.Load() != 0 { + t.Errorf("invalid tenantID reached GetSecret (%d calls)", fake.getCalls.Load()) + } + if fake.setCalls.Load() != 0 { + t.Errorf("invalid tenantID reached SetSecret (%d calls)", fake.setCalls.Load()) + } + }) + } +} + +func TestStore_SecretNameIsValidKeyVaultName(t *testing.T) { + // Tenant IDs with dots must produce names using only [0-9a-zA-Z-]. + tenants := []string{ + "contoso.onmicrosoft.com", + "my-company.onmicrosoft.com", + "00000000-0000-0000-0000-000000000000", + "common", + } + + for _, tenant := range tenants { + name := secretName(defaultPrefix, tenant) + if !isValidKeyVaultSecretName(name) { + t.Errorf("secretName(%q) = %q, not a valid Key Vault secret name", tenant, name) + } + } +} + +func TestStore_ConcurrentSaveLoad(t *testing.T) { + store, _ := newTestStore(t) + ctx := context.Background() + const tenant = "contoso.onmicrosoft.com" + + // Seed so Load never hits ErrTenantNotFound. + if err := store.Save(ctx, tenant, "rt_seed"); err != nil { + t.Fatalf("seed Save() error = %v", err) + } + + const ( + goroutines = 10 + iterations = 50 + ) + + allowedValues := map[string]bool{"rt_seed": true} + for g := range goroutines { + allowedValues[fmt.Sprintf("rt_v%d", g)] = true + } + + var wg sync.WaitGroup + wg.Add(goroutines * 2) + + for g := range goroutines { + go func() { + defer wg.Done() + value := fmt.Sprintf("rt_v%d", g) + for range iterations { + if err := store.Save(ctx, tenant, value); err != nil { + t.Errorf("Save() error = %v", err) + return + } + } + }() + go func() { + defer wg.Done() + for range iterations { + got, err := store.Load(ctx, tenant) + if err != nil { + t.Errorf("Load() error = %v", err) + return + } + if !allowedValues[got] { + t.Errorf("Load() = %q, not one of the written values", got) + return + } + } + }() + } + + wg.Wait() +} + +func TestNewStore_MissingVaultURL(t *testing.T) { + _, err := NewStore(Config{Credential: &fakeTokenCredential{}}) + if err == nil { + t.Fatal("NewStore() expected error for missing VaultURL, got nil") + } + if !strings.Contains(err.Error(), "VaultURL") { + t.Errorf("error = %q, want substring %q", err.Error(), "VaultURL") + } +} + +func TestNewStore_MissingCredential(t *testing.T) { + _, err := NewStore(Config{VaultURL: "https://myvault.vault.azure.net/"}) + if err == nil { + t.Fatal("NewStore() expected error for missing Credential, got nil") + } + if !strings.Contains(err.Error(), "Credential") { + t.Errorf("error = %q, want substring %q", err.Error(), "Credential") + } +} + +func TestNewStore_DefaultPrefix(t *testing.T) { + s, err := NewStore(Config{ + VaultURL: "https://myvault.vault.azure.net/", + Credential: &fakeTokenCredential{}, + }) + if err != nil { + t.Fatalf("NewStore() error = %v", err) + } + if s.prefix != defaultPrefix { + t.Errorf("prefix = %q, want %q", s.prefix, defaultPrefix) + } +} + +func TestNewStore_CustomPrefix(t *testing.T) { + const customPrefix = "env-staging-" + s, err := NewStore(Config{ + VaultURL: "https://myvault.vault.azure.net/", + Credential: &fakeTokenCredential{}, + Prefix: customPrefix, + }) + if err != nil { + t.Fatalf("NewStore() error = %v", err) + } + if s.prefix != customPrefix { + t.Errorf("prefix = %q, want %q", s.prefix, customPrefix) + } +} + +func TestStore_CustomLogger_EmitsOnNotFound(t *testing.T) { + var buf bytes.Buffer + custom := slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + store := newStoreWithClient(newFakeSecretsClient(), custom) + + if _, err := store.Load(context.Background(), "contoso.onmicrosoft.com"); err == nil { + t.Fatal("Load() expected ErrTenantNotFound, got nil") + } + + got := buf.String() + if got == "" { + t.Fatal("expected configured logger to receive a log line, got none") + } + if !strings.Contains(got, "contoso.onmicrosoft.com") { + t.Errorf("log output missing tenant_id; got: %s", got) + } +} + +func TestStore_NilLogger_LazyResolvesToDefault(t *testing.T) { + // This test mutates slog.Default() for the duration of the run. Do NOT + // mark it t.Parallel(): any sibling test that logs or reads slog.Default() + // would race with this one. + // + // Rewire slog.Default to capture output, run Load, and verify the + // message landed on the global default — confirming nil config.Logger + // means "use slog.Default() at emit time". + prev := slog.Default() + t.Cleanup(func() { slog.SetDefault(prev) }) + + var buf bytes.Buffer + slog.SetDefault(slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug}))) + + store := newStoreWithClient(newFakeSecretsClient(), nil) + + if _, err := store.Load(context.Background(), "contoso.onmicrosoft.com"); err == nil { + t.Fatal("Load() expected ErrTenantNotFound, got nil") + } + + if !strings.Contains(buf.String(), "contoso.onmicrosoft.com") { + t.Errorf("expected default logger to receive log line; got: %q", buf.String()) + } +} + +// fakeTokenCredential implements azcore.TokenCredential for NewStore tests +// that don't make any network calls. +type fakeTokenCredential struct{} + +func (*fakeTokenCredential) GetToken( + _ context.Context, _ policy.TokenRequestOptions, +) (azcore.AccessToken, error) { + return azcore.AccessToken{}, errors.New("fake credential not usable") +} + +// isValidKeyVaultSecretName validates that name matches Key Vault's secret +// name constraints: 1–127 chars of [0-9a-zA-Z-]. +func isValidKeyVaultSecretName(name string) bool { + if len(name) < 1 || len(name) > 127 { + return false + } + for _, r := range name { + switch { + case r >= '0' && r <= '9': + case r >= 'a' && r <= 'z': + case r >= 'A' && r <= 'Z': + case r == '-': + default: + return false + } + } + return true +} From ec69a3d67a78580d6124ba4c2ef2242bca4875d3 Mon Sep 17 00:00:00 2001 From: Sergio Palacio Date: Tue, 21 Apr 2026 13:11:44 +0200 Subject: [PATCH 2/3] chore(make): include keyvault submodule in multi-module targets The test, test-race, test-cover, lint, lint-fix, fmt, vet, and tidy targets iterate over each module explicitly -- add plugins/contrib/microsoft/keyvault so CI actually picks it up. gosec and govulncheck left unchanged: neither currently covers plugins/contrib either, so adding only keyvault would be inconsistent. --- Makefile | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ddfbb31..96720bf 100644 --- a/Makefile +++ b/Makefile @@ -104,18 +104,21 @@ test: ## Run tests (all modules) go test -v ./... cd sdk && go test -v ./... cd plugins/contrib && go test -v ./... + cd plugins/contrib/microsoft/keyvault && go test -v ./... .PHONY: test-race test-race: ## Run tests with race detector go test -race -v ./... cd sdk && go test -race -v ./... cd plugins/contrib && go test -race -v ./... + cd plugins/contrib/microsoft/keyvault && go test -race -v ./... .PHONY: test-cover test-cover: ## Run tests with coverage go test -coverprofile=coverage.out ./... cd sdk && go test -coverprofile=coverage-sdk.out ./... cd plugins/contrib && go test -coverprofile=coverage-contrib.out ./... + cd plugins/contrib/microsoft/keyvault && go test -coverprofile=coverage-keyvault.out ./... go tool cover -html=coverage.out -o coverage.html @echo "Coverage report: coverage.html" @@ -254,7 +257,8 @@ lint: ## Run linters (all modules) @if [ -x "$(GOLANGCI_LINT)" ]; then \ $(GOLANGCI_LINT) run && \ (cd sdk && $(GOLANGCI_LINT) run) && \ - (cd plugins/contrib && $(GOLANGCI_LINT) run); \ + (cd plugins/contrib && $(GOLANGCI_LINT) run) && \ + (cd plugins/contrib/microsoft/keyvault && $(GOLANGCI_LINT) run); \ else \ echo "golangci-lint not installed. Run: make tools"; \ exit 1; \ @@ -265,6 +269,7 @@ lint-fix: ## Run linters and fix issues $(GOLANGCI_LINT) run --fix cd sdk && $(GOLANGCI_LINT) run --fix cd plugins/contrib && $(GOLANGCI_LINT) run --fix + cd plugins/contrib/microsoft/keyvault && $(GOLANGCI_LINT) run --fix .PHONY: fmt fmt: ## Format code (all modules) @@ -274,12 +279,15 @@ fmt: ## Format code (all modules) cd sdk && gofmt -s -w . cd plugins/contrib && go fmt ./... cd plugins/contrib && gofmt -s -w . + cd plugins/contrib/microsoft/keyvault && go fmt ./... + cd plugins/contrib/microsoft/keyvault && gofmt -s -w . .PHONY: vet vet: ## Run go vet go vet ./... cd sdk && go vet ./... cd plugins/contrib && go vet ./... + cd plugins/contrib/microsoft/keyvault && go vet ./... .PHONY: tidy tidy: ## Tidy and verify go.mod (all modules) @@ -287,6 +295,7 @@ tidy: ## Tidy and verify go.mod (all modules) go mod verify cd sdk && go mod tidy cd plugins/contrib && go mod tidy && go mod verify + cd plugins/contrib/microsoft/keyvault && go mod tidy && go mod verify .PHONY: gosec gosec: ## Run gosec security scanner (all modules) From 7eda8bf2ae1bf2e3d3b22a9db35ff4cca99d5a5c Mon Sep 17 00:00:00 2001 From: Sergio Palacio Date: Tue, 5 May 2026 10:07:11 +0200 Subject: [PATCH 3/3] fix(contrib/keyvault): address review feedback on PR #83 - Validate Config.Prefix in NewStore against Key Vault's secret name grammar and the 127-char total-name budget. Catches typos like "env_staging-" and over-long prefixes at construction time instead of on every Save/Load with a 400 from Key Vault. - Export microsoft.ValidateTenantID and call it from FileStore and keyvault.Store, removing the duplicated regex in the keyvault submodule. - Drop the inconsistent "keyvault:" prefix from constructor errors so the package matches FileStore's error style. - Rename the contrib-plugins docs anchor to microsoft-keyvault-store and align the link in onboarding-refresh-tokens.md to keyvault.Store. --- docs/guides/onboarding-refresh-tokens.md | 2 +- docs/reference/contrib-plugins.md | 4 +- plugins/contrib/microsoft/filestore.go | 21 +----- plugins/contrib/microsoft/keyvault/store.go | 72 ++++++++++--------- .../contrib/microsoft/keyvault/store_test.go | 46 ++++++++++++ plugins/contrib/microsoft/token.go | 16 +++++ 6 files changed, 108 insertions(+), 53 deletions(-) diff --git a/docs/guides/onboarding-refresh-tokens.md b/docs/guides/onboarding-refresh-tokens.md index af627d9..fe9e904 100644 --- a/docs/guides/onboarding-refresh-tokens.md +++ b/docs/guides/onboarding-refresh-tokens.md @@ -125,7 +125,7 @@ Each tenant gets a single file. The base directory is created automatically at r ### Azure Key Vault -[`microsoft.KeyVaultStore`](../reference/contrib-plugins.md#microsoft-keyvaultstore) persists each tenant's refresh token as a Key Vault secret. The secret name is `{prefix}{hex(sha256(tenantID))}`; the original tenantID is preserved on the `tenantID` tag. `chaperone-onboard` has no dedicated Key Vault subcommand — pipe stdout directly into `az keyvault secret set`. +[`keyvault.Store`](../reference/contrib-plugins.md#microsoft-keyvault-store) persists each tenant's refresh token as a Key Vault secret. The secret name is `{prefix}{hex(sha256(tenantID))}`; the original tenantID is preserved on the `tenantID` tag. `chaperone-onboard` has no dedicated Key Vault subcommand — pipe stdout directly into `az keyvault secret set`. **Prerequisites:** diff --git a/docs/reference/contrib-plugins.md b/docs/reference/contrib-plugins.md index 27aca2d..5c3c026 100644 --- a/docs/reference/contrib-plugins.md +++ b/docs/reference/contrib-plugins.md @@ -727,8 +727,8 @@ Seed each tenant with [`chaperone-onboard microsoft`](../guides/onboarding-refre fabrikam.onmicrosoft.com ``` - -### Microsoft `KeyVaultStore` + +### Microsoft `keyvault.Store` ```go type Store struct{ /* unexported */ } diff --git a/plugins/contrib/microsoft/filestore.go b/plugins/contrib/microsoft/filestore.go index 3069c84..b34a22d 100644 --- a/plugins/contrib/microsoft/filestore.go +++ b/plugins/contrib/microsoft/filestore.go @@ -21,7 +21,7 @@ import ( // directory, synced to disk, and then renamed to the target path. This // prevents corruption from a crash mid-write. // -// TenantIDs are validated against [validTenantID] to prevent path traversal, +// TenantIDs are validated against [ValidateTenantID] to prevent path traversal, // regardless of whether the caller has already validated them. // // It is safe for concurrent use from multiple goroutines. @@ -41,7 +41,7 @@ func NewFileStore(baseDir string) *FileStore { // Load retrieves the current refresh token for the given tenant. // Returns [contrib.ErrTenantNotFound] if no token file exists. func (f *FileStore) Load(_ context.Context, tenantID string) (string, error) { - if err := validateTenantID(tenantID); err != nil { + if err := ValidateTenantID(tenantID); err != nil { return "", err } @@ -61,7 +61,7 @@ func (f *FileStore) Load(_ context.Context, tenantID string) (string, error) { // Save persists a rotated refresh token to disk atomically. func (f *FileStore) Save(_ context.Context, tenantID, refreshToken string) error { - if err := validateTenantID(tenantID); err != nil { + if err := ValidateTenantID(tenantID); err != nil { return err } if refreshToken == "" { @@ -113,18 +113,3 @@ func (f *FileStore) Save(_ context.Context, tenantID, refreshToken string) error func (f *FileStore) tokenPath(tenantID string) string { return filepath.Join(f.baseDir, tenantID) } - -// validateTenantID rejects tenant IDs that could cause path traversal. -// This is defense-in-depth: RefreshTokenSource also validates, but FileStore -// is a public type and must not rely on callers for safety. -func validateTenantID(tenantID string) error { - if !validTenantID.MatchString(tenantID) { - display := tenantID - if len(display) > 64 { - display = display[:64] + "..." - } - return fmt.Errorf("invalid tenant ID %q: must match %s", - display, validTenantID.String()) - } - return nil -} diff --git a/plugins/contrib/microsoft/keyvault/store.go b/plugins/contrib/microsoft/keyvault/store.go index 9e1e583..5852753 100644 --- a/plugins/contrib/microsoft/keyvault/store.go +++ b/plugins/contrib/microsoft/keyvault/store.go @@ -35,11 +35,16 @@ const defaultPrefix = "chaperone-rt-" // does not exist in the vault. const secretNotFoundCode = "SecretNotFound" -// validTenantID mirrors microsoft.validTenantID, which is unexported. Any -// change there should be reflected here. It matches Azure AD tenant -// identifiers (GUIDs, domain names) and rejects path separators, query -// strings, and fragments. -var validTenantID = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9.\-]*$`) +// keyVaultSecretNameMaxLen is the Key Vault secret name length limit. +const keyVaultSecretNameMaxLen = 127 + +// secretHashLen is the length of the hex-encoded SHA-256 suffix appended to +// every prefix when building a secret name (see [secretName]). +const secretHashLen = 64 + +// validPrefix matches the prefix grammar Key Vault accepts inside a secret +// name. The empty string is allowed; full-name length is checked separately. +var validPrefix = regexp.MustCompile(`^[0-9a-zA-Z-]*$`) // Config configures a Key Vault-backed TokenStore. type Config struct { @@ -86,25 +91,28 @@ type Store struct { var _ microsoft.TokenStore = (*Store)(nil) // NewStore constructs a Store from a Config. Returns an error if VaultURL or -// Credential is missing, or if the underlying azsecrets client fails to -// initialize. +// Credential is missing, the Prefix is incompatible with Key Vault's secret +// name grammar, or the underlying azsecrets client fails to initialize. func NewStore(cfg Config) (*Store, error) { if cfg.VaultURL == "" { - return nil, errors.New("keyvault: VaultURL is required") + return nil, errors.New("VaultURL is required") //nolint:staticcheck // VaultURL is a Config field identifier } if cfg.Credential == nil { - return nil, errors.New("keyvault: Credential is required") - } - - client, err := azsecrets.NewClient(cfg.VaultURL, cfg.Credential, cfg.ClientOptions) - if err != nil { - return nil, fmt.Errorf("keyvault: creating azsecrets client: %w", err) + return nil, errors.New("Credential is required") //nolint:staticcheck // Credential is a Config field identifier } prefix := cfg.Prefix if prefix == "" { prefix = defaultPrefix } + if err := validatePrefix(prefix); err != nil { + return nil, err + } + + client, err := azsecrets.NewClient(cfg.VaultURL, cfg.Credential, cfg.ClientOptions) + if err != nil { + return nil, fmt.Errorf("creating azsecrets client: %w", err) + } return &Store{ client: client, @@ -113,6 +121,22 @@ func NewStore(cfg Config) (*Store, error) { }, nil } +// validatePrefix enforces Key Vault's secret name grammar on the configured +// prefix and ensures the resulting "{prefix}{hex(sha256(tenantID))}" stays +// within Key Vault's 127-character limit. Catching this at startup avoids +// surfacing a 400 from Key Vault on every Save/Load. +func validatePrefix(prefix string) error { + if !validPrefix.MatchString(prefix) { + return fmt.Errorf("invalid Prefix %q: must match %s", + prefix, validPrefix.String()) + } + if len(prefix)+secretHashLen > keyVaultSecretNameMaxLen { + return fmt.Errorf("Prefix %q is too long: len(prefix) + %d must be <= %d", //nolint:staticcheck // Prefix is a Config field identifier + prefix, secretHashLen, keyVaultSecretNameMaxLen) + } + return nil +} + // newStoreWithClient is the test-only constructor that accepts a preconstructed // secretsClient. The public NewStore wraps azsecrets.NewClient and delegates // here. Kept unexported so production callers can only reach it via NewStore. @@ -134,7 +158,7 @@ func (s *Store) log() *slog.Logger { // Returns an error wrapping [contrib.ErrTenantNotFound] if the tenant has no // stored token. func (s *Store) Load(ctx context.Context, tenantID string) (string, error) { - if err := validateTenantID(tenantID); err != nil { + if err := microsoft.ValidateTenantID(tenantID); err != nil { return "", err } @@ -165,7 +189,7 @@ func (s *Store) Load(ctx context.Context, tenantID string) (string, error) { // Save persists a rotated refresh token to Key Vault. Each call creates a // new secret version; Load always reads the latest. func (s *Store) Save(ctx context.Context, tenantID, refreshToken string) error { - if err := validateTenantID(tenantID); err != nil { + if err := microsoft.ValidateTenantID(tenantID); err != nil { return err } if refreshToken == "" { @@ -190,19 +214,3 @@ func (s *Store) Save(ctx context.Context, tenantID, refreshToken string) error { return nil } - -// validateTenantID rejects tenant IDs that could cause path traversal or that -// don't match Azure AD's tenant ID grammar. Defense in depth: the outer -// RefreshTokenSource already validates, but Store is a public type and must -// not rely on callers for safety. -func validateTenantID(tenantID string) error { - if !validTenantID.MatchString(tenantID) { - display := tenantID - if len(display) > 64 { - display = display[:64] + "..." - } - return fmt.Errorf("keyvault: invalid tenant ID %q: must match %s", - display, validTenantID.String()) - } - return nil -} diff --git a/plugins/contrib/microsoft/keyvault/store_test.go b/plugins/contrib/microsoft/keyvault/store_test.go index 661c992..4779e79 100644 --- a/plugins/contrib/microsoft/keyvault/store_test.go +++ b/plugins/contrib/microsoft/keyvault/store_test.go @@ -440,6 +440,52 @@ func TestNewStore_CustomPrefix(t *testing.T) { } } +func TestNewStore_InvalidPrefix_RejectedAtConstruction(t *testing.T) { + tests := []struct { + name string + prefix string + }{ + {"underscore", "env_staging-"}, + {"dot", "env.staging-"}, + {"slash", "env/staging-"}, + {"space", "env staging-"}, + {"too long", strings.Repeat("a", keyVaultSecretNameMaxLen-secretHashLen+1)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := NewStore(Config{ + VaultURL: "https://myvault.vault.azure.net/", + Credential: &fakeTokenCredential{}, + Prefix: tt.prefix, + }) + if err == nil { + t.Fatalf("NewStore() expected error for prefix %q, got nil", tt.prefix) + } + if !strings.Contains(err.Error(), "Prefix") { + t.Errorf("error = %q, want substring %q", err.Error(), "Prefix") + } + }) + } +} + +func TestNewStore_PrefixAtMaxAllowedLength_Accepted(t *testing.T) { + // A prefix of exactly (127 - 64) = 63 chars must produce a 127-char name + // — the upper edge of Key Vault's grammar. + prefix := strings.Repeat("a", keyVaultSecretNameMaxLen-secretHashLen) + s, err := NewStore(Config{ + VaultURL: "https://myvault.vault.azure.net/", + Credential: &fakeTokenCredential{}, + Prefix: prefix, + }) + if err != nil { + t.Fatalf("NewStore() with max-length prefix unexpectedly failed: %v", err) + } + if got := len(secretName(s.prefix, "contoso.onmicrosoft.com")); got != keyVaultSecretNameMaxLen { + t.Errorf("secret name length = %d, want %d", got, keyVaultSecretNameMaxLen) + } +} + func TestStore_CustomLogger_EmitsOnNotFound(t *testing.T) { var buf bytes.Buffer custom := slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug})) diff --git a/plugins/contrib/microsoft/token.go b/plugins/contrib/microsoft/token.go index 045cf5c..691489b 100644 --- a/plugins/contrib/microsoft/token.go +++ b/plugins/contrib/microsoft/token.go @@ -25,6 +25,22 @@ import ( // "consumers". It rejects path separators, query strings, and fragments. var validTenantID = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9.\-]*$`) +// ValidateTenantID rejects tenant IDs that could cause path traversal or that +// don't match Azure AD's tenant ID grammar. Exported so sibling [TokenStore] +// implementations (e.g., keyvault.Store) can share the same rule without +// duplicating the regex. +func ValidateTenantID(tenantID string) error { + if !validTenantID.MatchString(tenantID) { + display := tenantID + if len(display) > 64 { + display = display[:64] + "..." + } + return fmt.Errorf("invalid tenant ID %q: must match %s", + display, validTenantID.String()) + } + return nil +} + const ( // defaultTokenEndpoint is the public Azure AD v1 token endpoint. defaultTokenEndpoint = "https://login.microsoftonline.com" // #nosec G101 -- URL endpoint, not a credential