From 4ce66fbdcef28ffd1e631f5e0be05fc9a10a82ef Mon Sep 17 00:00:00 2001 From: spb <1661003+spbsoluble@users.noreply.github.com> Date: Wed, 1 Jul 2026 12:54:35 -0700 Subject: [PATCH] fix: correct JSON serialization and error handling in VaultHttp (#10) * updated integration manifest to reflect prod release * fix: correct JSON serialization and error handling in VaultHttp AddJsonBody(string) in RestSharp re-serializes the argument, wrapping a pre-serialized JSON string in quotes and escaping it. Vault receives a JSON string literal instead of a JSON object and returns HTTP 400. Fixed by using AddStringBody(string, ContentType.Json) which sends the raw body without re-encoding. ThrowOnAnyError = true caused RestSharp to throw before the explicit BadRequest handler ran, hiding the Vault error body from logs. Removed ThrowOnAnyError and added response.ThrowIfError() after the BadRequest block so all other non-2xx responses still surface as exceptions. Also drops the EOL net6.0 target; project now multi-targets net8.0 and net10.0. * chore: target v1.0.3 in CHANGELOG * fix: replace unsafe dictionary indexers in ValidateCAConnectionInfo and ValidateProductInfo Direct Dictionary.get_Item calls throw KeyNotFoundException when the gateway does not populate all parameter keys before calling validation (observed on PUT/POST config/configuration). Replace with TryGetValue throughout. Also relaxes the ValidateProductInfo RoleName check: RoleName is optional since the Enroll path already falls back to ProductID when RoleName is absent, so the validator no longer rejects configs that omit it. * chore: document dictionary indexer fix in CHANGELOG * fix(ci): `AppendTargetFrameworkToOutputPath=true` --------- Co-authored-by: Mikey Henderson <4452096+fiddlermikey@users.noreply.github.com> Co-authored-by: Joe VanWanzeele --- CHANGELOG.md | 31 ++++++++++++++++++- hashicorp-vault-cagateway/Client/VaultHttp.cs | 6 ++-- .../HashicorpVaultCAConnector.cs | 21 ++++++++----- .../hashicorp-vault-caplugin.csproj | 6 ++-- integration-manifest.json | 4 +-- 5 files changed, 53 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2786a1f..e8e2258 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,32 @@ +# Changelog + +## [1.0.3] + +### Fixed + +- **`VaultHttp.PostAsync`: JSON double-encoding caused HTTP 400 from Vault/OpenBao** + + `PostAsync` manually serialized the request body to a JSON string using `JsonSerializer.Serialize`, then passed that string to `request.AddJsonBody()`. In RestSharp ≥ 106, `AddJsonBody` re-serializes whatever object it receives — when the argument is a `string`, it encodes it as a JSON string literal, wrapping the content in quotes and escaping the inner characters. Vault/OpenBao received `"{\\"csr\\":\\"...\\"}"` (a JSON-encoded string) instead of `{"csr":"..."}` (a JSON object) and returned HTTP 400 "error parsing JSON". + + Fixed by replacing `request.AddJsonBody(serializedParams)` with `request.AddStringBody(serializedParams, ContentType.Json)`. `AddStringBody` sends the string as the raw request body without re-encoding it. + +- **`ThrowOnAnyError = true` made the `BadRequest` error-parsing block dead code** + + `RestClientOptions` was constructed with `ThrowOnAnyError = true`, which causes RestSharp to throw an exception on any non-2xx response before returning to the caller. The `PostAsync` method had explicit handling for `HttpStatusCode.BadRequest` that deserialized Vault error messages and threw a descriptive exception — but that block was never reached because RestSharp threw first, and the actual Vault error body was lost. + + Fixed by removing `ThrowOnAnyError = true` from `RestClientOptions` and adding `response.ThrowIfError()` after the explicit `BadRequest` handler, so non-2xx responses that are not `BadRequest` still surface as exceptions while `BadRequest` responses are handled with full Vault error body parsing. + +- **`ValidateCAConnectionInfo` and `ValidateProductInfo`: `KeyNotFoundException` on gateway config PUT/POST** + + Both validation methods used direct `Dictionary.get_Item` indexers (`connectionInfo[key]`) to read parameters. The gateway does not always pre-populate every parameter key before calling validation, so any absent key threw `KeyNotFoundException` and surfaced as an opaque HTTP 500 from the gateway config endpoint. + + Fixed by replacing all direct indexers with `TryGetValue` calls throughout both methods. + + Additionally relaxed the `RoleName` requirement in `ValidateProductInfo`: the `Enroll` path already falls back to `ProductID` when `RoleName` is absent, so the validator no longer rejects configurations that omit it. The check now only errors if `RoleName` is explicitly present but empty. + +### Changed + +- Dropped .NET 6.0 target (EOL). The project now targets `net8.0` and `net10.0`. ## 1.0.2 * bug fix: _certDataReader is now initialized in the Initialize method @@ -5,4 +34,4 @@ * added retrieval of roles associated with enrolled certificates via metadata for Vault Enterprise users ## 1.0.0 -* initial release \ No newline at end of file +* initial release diff --git a/hashicorp-vault-cagateway/Client/VaultHttp.cs b/hashicorp-vault-cagateway/Client/VaultHttp.cs index e40e3f8..23d9857 100644 --- a/hashicorp-vault-cagateway/Client/VaultHttp.cs +++ b/hashicorp-vault-cagateway/Client/VaultHttp.cs @@ -42,7 +42,7 @@ public VaultHttp(string host, string mountPoint, string authToken, string nameSp PreferredObjectCreationHandling = JsonObjectCreationHandling.Replace }; - var restClientOptions = new RestClientOptions($"{host.TrimEnd('/')}/v1") { ThrowOnAnyError = true }; + var restClientOptions = new RestClientOptions($"{host.TrimEnd('/')}/v1"); _restClient = new RestClient(restClientOptions, configureSerialization: s => s.UseSystemTextJson(_serializerOptions)); _mountPoint = mountPoint.TrimStart('/').TrimEnd('/'); // remove leading and trailing slashes @@ -124,7 +124,7 @@ public async Task PostAsync(string path, dynamic parameters = default) { string serializedParams = JsonSerializer.Serialize(parameters); logger.LogTrace($"serialized parameters (from {parameters.GetType()?.Name}): {serializedParams}"); - request.AddJsonBody(serializedParams); + request.AddStringBody(serializedParams, ContentType.Json); } logger.LogTrace($"full url for the request: {_restClient.Options.BaseUrl}/{request.Resource}"); @@ -150,6 +150,8 @@ public async Task PostAsync(string path, dynamic parameters = default) logger.LogTrace($"errors: {allErrors}"); throw new Exception(allErrors); } + + response.ThrowIfError(); return response.Data; } catch (Exception ex) diff --git a/hashicorp-vault-cagateway/HashicorpVaultCAConnector.cs b/hashicorp-vault-cagateway/HashicorpVaultCAConnector.cs index 21ce226..b9021a9 100644 --- a/hashicorp-vault-cagateway/HashicorpVaultCAConnector.cs +++ b/hashicorp-vault-cagateway/HashicorpVaultCAConnector.cs @@ -384,22 +384,23 @@ public async Task ValidateCAConnectionInfo(Dictionary connection List errors = new List(); // then, we make sure required fields are defined.. - if (string.IsNullOrEmpty(connectionInfo[Constants.CAConfig.HOST] as string)) + connectionInfo.TryGetValue(Constants.CAConfig.HOST, out var hostVal); + if (string.IsNullOrEmpty(hostVal as string)) { errors.Add($"The '{Constants.CAConfig.HOST}' is required."); } - if (string.IsNullOrEmpty(connectionInfo[Constants.CAConfig.MOUNTPOINT] as string)) + connectionInfo.TryGetValue(Constants.CAConfig.MOUNTPOINT, out var mountVal); + if (string.IsNullOrEmpty(mountVal as string)) { errors.Add($"The '{Constants.CAConfig.MOUNTPOINT}' is required."); } // make sure an authentication mechanism is defined (either certificate or token) - var token = connectionInfo[Constants.CAConfig.TOKEN] as string; - - //var cert = connectionInfo[Constants.CAConfig.CLIENTCERT] as string; - - var cert = string.Empty; // temporary until client cert auth into vault is implemented + connectionInfo.TryGetValue(Constants.CAConfig.TOKEN, out var tokenVal); + connectionInfo.TryGetValue(Constants.CAConfig.CLIENTCERT, out var certVal); + var token = tokenVal as string; + var cert = certVal as string; if (string.IsNullOrEmpty(token) && string.IsNullOrEmpty(cert)) { @@ -489,6 +490,12 @@ public Task ValidateProductInfo(EnrollmentProductInfo productInfo, Dictionary - net6.0 + net8.0;net10.0 Keyfactor.Extensions.CAPlugin.HashicorpVault disable warnings @@ -9,7 +9,7 @@ False True 12.0 - False + True False @@ -18,7 +18,7 @@ False True bin - False + True False diff --git a/integration-manifest.json b/integration-manifest.json index 30206d5..7906d6e 100644 --- a/integration-manifest.json +++ b/integration-manifest.json @@ -2,8 +2,8 @@ "$schema": "https://keyfactor.github.io/v2/integration-manifest-schema.json", "integration_type": "anyca-plugin", "name": "Hashicorp Vault AnyCA REST Gateway Plugin", - "status": "prototype", - "support_level": "community", + "status": "production", + "support_level": "kf-supported", "link_github": true, "update_catalog": false, "description": "Hashicorp Vault plugin for the AnyCA REST Gateway Framework",