From b11e7caa865f8eee5155cddbd98d445717b84ef5 Mon Sep 17 00:00:00 2001 From: Stuart Meeks Date: Sun, 3 May 2026 03:18:25 +0000 Subject: [PATCH] Patch all three providers; centralise package management; bump core to 0.6.1 Adobe 0.2.0 -> 0.2.1, Airtable 0.2.0 -> 0.2.1, SoftwareOne 0.3.0 -> 0.3.1. Security: - Reject plain http for credential-bearing endpoints. The Adobe and SoftwareOne collectors and authentication services now require https for the IMS URL, the Adobe API base URL, and the SoftwareOne API base URL. http is accepted only for loopback hosts so local mock servers and proxies still work during development. The same check runs in the auth services on every call, so a hand-edited keystore that downgrades a stored credential to http is rejected before any request is sent. Closes the specific risk that the SoftwareOne collector ships the API token in the URL query (eq(token,'...')) -- over plain http that token would otherwise traverse the network in cleartext and land in any intermediate access log. - Sanitise response bodies before they reach exception messages. The SoftwareOne lookup and the Adobe IMS exchange now truncate error bodies to 512 chars; the SoftwareOne path additionally redacts the literal token value from the body. Defends against a misbehaving upstream proxy that echoes the request URL (which carries the token in the query) into an error page that would otherwise reach exception aggregators and log files. Changed: - Register the IAuthenticationService interface mapping in all three providers' ServiceCollectionExtensions. The registration forwards to the same singleton as the concrete type so existing consumers depending on the concrete are unaffected. - Bump core library reference from [0.5.0,1.0.0) to [0.6.1,1.0.0). The 0.6.x line of the core package added ICredentialManager.GetCredentialByIdAsync(string, string); the test FakeCredentialManager doubles implement it as NotSupportedException to match the existing "fail loudly on accidental reliance" pattern. Repo housekeeping: - Centralise package management in /Directory.Packages.props with CentralPackageVersionOverrideEnabled=false so per-project version pins fail the build instead of silently overriding the central pin. Every csproj's now omits its Version attribute and resolves through the central manifest. - Stop auto-packing on every build. The csprojs previously set GeneratePackageOnBuild=true with a hardcoded Windows-only PackageOutputPath (C:\nuget-local\). Both properties are removed; CI now invokes dotnet pack explicitly per provider. Tests: 119 -> 155 (Adobe 60, Airtable 36, SoftwareOne 59), all green. --- .github/workflows/ci.yml | 15 ++- CHANGELOG.md | 23 ++++ Directory.Packages.props | 43 ++++++++ .../AdobeAuthenticationService.cs | 52 ++++++++- .../AdobeCredentialCollector.cs | 35 ++++-- ...SpectreConsole.Auth.Providers.Adobe.csproj | 25 ++--- .../ServiceCollectionExtensions.cs | 11 +- ...ctreConsole.Auth.Providers.Airtable.csproj | 19 +--- .../ServiceCollectionExtensions.cs | 11 +- ...eConsole.Auth.Providers.SoftwareOne.csproj | 21 ++-- .../ServiceCollectionExtensions.cs | 11 +- .../SoftwareOneAuthenticationService.cs | 23 ++++ .../SoftwareOneCredentialCollector.cs | 72 +++++++++++- .../AdobeAuthenticationServiceTests.cs | 65 +++++++++++ .../AdobeCredentialCollectorTests.cs | 44 ++++++++ .../FakeCredentialManager.cs | 3 + ...eConsole.Auth.Providers.Adobe.Tests.csproj | 13 ++- .../ServiceCollectionExtensionsTests.cs | 35 ++++++ .../FakeCredentialManager.cs | 3 + ...nsole.Auth.Providers.Airtable.Tests.csproj | 11 +- .../ServiceCollectionExtensionsTests.cs | 28 +++++ .../FakeCredentialManager.cs | 3 + ...le.Auth.Providers.SoftwareOne.Tests.csproj | 11 +- .../ServiceCollectionExtensionsTests.cs | 30 +++++ .../SoftwareOneAuthenticationServiceTests.cs | 46 ++++++++ ...ftwareOneCredentialCollectorLookupTests.cs | 104 ++++++++++++++++++ 26 files changed, 680 insertions(+), 77 deletions(-) create mode 100644 Directory.Packages.props diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 223eadd..4f5e204 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,13 +30,22 @@ jobs: run: dotnet restore - name: Build - # Override PackageOutputPath so the auto-generated .nupkg lands in - # ./artifacts instead of the Windows-path the csprojs hardcode. - run: dotnet build --configuration Release --no-restore -p:PackageOutputPath=${{ github.workspace }}/artifacts + run: dotnet build --configuration Release --no-restore - name: Test run: dotnet test --configuration Release --no-build --verbosity normal + - name: Pack + # Pack each provider explicitly. The csprojs no longer auto-pack on + # build, so this step is the single point at which nupkgs / snupkgs + # are produced for downstream consumption (publish job + uploaded + # artifact). --no-build is safe because the build step above ran in + # the same Release configuration. + run: | + dotnet pack src/NextIteration.SpectreConsole.Auth.Providers.Adobe --configuration Release --no-build --output ${{ github.workspace }}/artifacts + dotnet pack src/NextIteration.SpectreConsole.Auth.Providers.Airtable --configuration Release --no-build --output ${{ github.workspace }}/artifacts + dotnet pack src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne --configuration Release --no-build --output ${{ github.workspace }}/artifacts + - name: Upload package artifacts uses: actions/upload-artifact@v6 with: diff --git a/CHANGELOG.md b/CHANGELOG.md index 5867154..cb8e04e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,28 @@ and each package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 --- +## [0.2.1 / 0.2.1 / 0.3.1] — 2026-05-03 + +_Adobe → 0.2.1, Airtable → 0.2.1, SoftwareOne → 0.3.1. Coordinated patch release driven by an external security review._ + +### Security +- **Reject plain `http` for credential-bearing endpoints.** The Adobe and SoftwareOne collectors and authentication services now require `https` for the IMS URL, the Adobe API base URL, and the SoftwareOne API base URL. `http` is accepted only when the host is a loopback address, so local mock servers and proxies still work during development. The same check runs in the authentication services on every call, so a hand-edited keystore that downgrades a stored credential to `http` is rejected before any request is sent. This closes the specific risk that the SoftwareOne collector ships the API token in the URL query (`eq(token,'…')`) — over plain `http` that token would otherwise traverse the network in cleartext and land in any intermediate access log. +- **Sanitise response bodies before they reach exception messages.** When the SoftwareOne lookup or the Adobe IMS exchange returns a non-success status, the error path used to inline the raw response body verbatim. Both providers now truncate the body to 512 characters before constructing the exception, and the SoftwareOne path additionally redacts the literal token value out of the body — defending against a misbehaving upstream proxy that echoes the request URL (which carries the token in the query string) into an error page that would otherwise reach exception aggregators and log files. +- Airtable carries no URL prompt and made no IMS-style call, so its 0.2.1 picks up the cross-cutting DI / packaging fixes only. + +### Changed +- **Register the `IAuthenticationService` interface mapping** for all three providers. Previously only the concrete `XxxAuthenticationService` was registered with DI, so consumers depending on the abstraction got a runtime resolution failure. The interface registration forwards to the same singleton instance, so this is purely additive — existing consumers depending on the concrete type are unaffected. +- **Core library reference bumped from `[0.5.0,1.0.0)` to `[0.6.1,1.0.0)`.** Picks up the latest core release published to nuget.org. The cap stays on the next major so a breaking 1.0 of the core package doesn't auto-flow into provider consumers — bump the upper bound deliberately when validating against the next major. + +### Fixed +- **Stop auto-packing on every build.** The csprojs previously set `GeneratePackageOnBuild=true` with a hardcoded Windows-only `PackageOutputPath` (`C:\nuget-local\`). On non-Windows hosts that path was interpreted as a project-relative directory called `C:\nuget-local\`; on every host the auto-pack ran during `dotnet test`, which is wasteful and meant ordinary local development was producing release nupkgs as a side effect. Both properties are removed; CI now invokes `dotnet pack` explicitly per provider. + +### Migration notes +- Consumer apps need no source changes. The DI registration is purely additive; the `https`-only enforcement only rejects URLs you should not have been using to begin with. +- If you were relying on the auto-generated nupkgs landing in the project tree from a local build, switch to `dotnet pack --output ./artifacts` instead. + +--- + ## SoftwareOne — [0.3.0] — 2026-04-18 _Applies to `NextIteration.SpectreConsole.Auth.Providers.SoftwareOne` only. Adobe and Airtable remain at 0.2.0._ @@ -96,6 +118,7 @@ _SoftwareOne Marketplace API token — pass-through._ - Per-package NuGet metadata: MIT license expression, SourceLink, deterministic builds, embedded symbols, snupkg, capped version ranges for cross-package dependencies. - GitHub Actions CI with per-package tag-triggered publishing (`adobe-v*` → publishes Adobe only, etc.). +[0.2.1 / 0.2.1 / 0.3.1]: https://github.com/StuartMeeks/NextIteration.SpectreConsole.Auth.Providers/releases [0.2.0]: https://github.com/StuartMeeks/NextIteration.SpectreConsole.Auth.Providers/releases [0.1.1]: https://github.com/StuartMeeks/NextIteration.SpectreConsole.Auth.Providers/releases [0.1.0]: https://github.com/StuartMeeks/NextIteration.SpectreConsole.Auth.Providers/releases diff --git a/Directory.Packages.props b/Directory.Packages.props new file mode 100644 index 0000000..4535ad7 --- /dev/null +++ b/Directory.Packages.props @@ -0,0 +1,43 @@ + + + + true + + false + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/AdobeAuthenticationService.cs b/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/AdobeAuthenticationService.cs index 434b746..b9c9519 100644 --- a/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/AdobeAuthenticationService.cs +++ b/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/AdobeAuthenticationService.cs @@ -30,6 +30,11 @@ public sealed class AdobeAuthenticationService : IAuthenticationService AuthenticateAsync(AdobeCredential credential) { // Include the IMS response body in the error so callers can // see e.g. {"error":"invalid_client","error_description":"..."} - // rather than just a bare status code. + // rather than just a bare status code — but truncate so an + // upstream proxy that echoes the full request can't bloat + // logs (and bound the unlikely case of credential material + // making it back into the error body). + var safeBody = TruncateErrorBody(responseBody); throw new HttpRequestException( - $"Adobe IMS token request failed: {(int)response.StatusCode} {response.StatusCode}. Body: {responseBody}"); + $"Adobe IMS token request failed: {(int)response.StatusCode} {response.StatusCode}. Body: {safeBody}"); } var dto = JsonSerializer.Deserialize(responseBody) @@ -151,6 +160,45 @@ private static void ValidateCredential(AdobeCredential credential) $"{nameof(AdobeCredential.Environment)} is required and must not be whitespace.", nameof(credential)); } + + // The collector enforces https-or-loopback on input, but a + // hand-edited keystore could downgrade either URL to plain + // http. Re-check before sending the client secret over the + // wire — refusing here closes the gap. + RequireSecureUrl(credential.ImsUrl, nameof(AdobeCredential.ImsUrl)); + RequireSecureUrl(credential.BaseUrl, nameof(AdobeCredential.BaseUrl)); + + static void RequireSecureUrl(Uri url, string fieldName) + { + if (url.Scheme == Uri.UriSchemeHttps) + { + return; + } + + if (url.Scheme == Uri.UriSchemeHttp && url.IsLoopback) + { + return; + } + + throw new ArgumentException( + $"{fieldName} must use https (http is only accepted for loopback addresses).", + fieldName); + } + } + + /// + /// Truncate the IMS error body to + /// before it lands in an exception message. Bounds the size of + /// anything that gets logged downstream. + /// + internal static string TruncateErrorBody(string body) + { + if (string.IsNullOrEmpty(body) || body.Length <= ErrorBodyMaxChars) + { + return body; + } + + return string.Concat(body.AsSpan(0, ErrorBodyMaxChars), "… [truncated]"); } /// diff --git a/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/AdobeCredentialCollector.cs b/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/AdobeCredentialCollector.cs index 41037c5..181d930 100644 --- a/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/AdobeCredentialCollector.cs +++ b/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/AdobeCredentialCollector.cs @@ -29,7 +29,7 @@ public sealed class AdobeCredentialCollector : ICredentialCollector var imsUrlInput = await AnsiConsole.PromptAsync( new TextPrompt("Enter IMS URL:") .DefaultValue(DefaultImsUrl) - .Validate(ValidateHttpUrl)).ConfigureAwait(false); + .Validate(ValidateSecureUrl)).ConfigureAwait(false); var apiKey = await AnsiConsole.PromptAsync( new TextPrompt("Enter API Key (OAuth2 client_id):") @@ -43,7 +43,7 @@ public sealed class AdobeCredentialCollector : ICredentialCollector var baseUrlInput = await AnsiConsole.PromptAsync( new TextPrompt("Enter Base URL:") .DefaultValue(DefaultBaseUrl) - .Validate(ValidateHttpUrl)).ConfigureAwait(false); + .Validate(ValidateSecureUrl)).ConfigureAwait(false); var environment = await AnsiConsole.PromptAsync( new SelectionPrompt() @@ -62,11 +62,32 @@ public sealed class AdobeCredentialCollector : ICredentialCollector return (JsonSerializer.Serialize(credential, AdobeCredential.JsonOptions), credential.Environment); } - private static ValidationResult ValidateHttpUrl(string value) - => Uri.TryCreate(value, UriKind.Absolute, out var parsed) - && (parsed.Scheme == Uri.UriSchemeHttp || parsed.Scheme == Uri.UriSchemeHttps) - ? ValidationResult.Success() - : ValidationResult.Error("Must be a valid absolute http(s) URL"); + /// + /// Accept the URL only if it's an absolute https URI, or an http + /// loopback (so devs can point the collector at a local mock or + /// proxy without compromising the OAuth2 client secret over the + /// wire in real deployments). + /// + internal static ValidationResult ValidateSecureUrl(string value) + { + if (!Uri.TryCreate(value, UriKind.Absolute, out var parsed)) + { + return ValidationResult.Error("Must be a valid absolute http(s) URL"); + } + + if (parsed.Scheme == Uri.UriSchemeHttps) + { + return ValidationResult.Success(); + } + + if (parsed.Scheme == Uri.UriSchemeHttp && parsed.IsLoopback) + { + return ValidationResult.Success(); + } + + return ValidationResult.Error( + "Must use https. http is only accepted for loopback addresses (the OAuth2 client secret is POSTed to this URL and must not traverse the network in cleartext)."); + } private static Func ValidateNonEmpty(string fieldName) => value => string.IsNullOrWhiteSpace(value) diff --git a/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/NextIteration.SpectreConsole.Auth.Providers.Adobe.csproj b/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/NextIteration.SpectreConsole.Auth.Providers.Adobe.csproj index 0664067..50ac474 100644 --- a/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/NextIteration.SpectreConsole.Auth.Providers.Adobe.csproj +++ b/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/NextIteration.SpectreConsole.Auth.Providers.Adobe.csproj @@ -11,10 +11,8 @@ NextIteration.SpectreConsole.Auth.Providers.Adobe - 0.2.0 + 0.2.1 Adobe VIP Marketplace credential provider for NextIteration.SpectreConsole.Auth. Ships AdobeCredential, AdobeToken, AdobeAuthenticationService (OAuth2 client-credentials against Adobe IMS), and the Spectre.Console collector that drives the accounts-add prompt. - true - C:\nuget-local\ true MIT README.md @@ -34,17 +32,16 @@ - - - - - - + + + + + + + + + + diff --git a/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/ServiceCollectionExtensions.cs b/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/ServiceCollectionExtensions.cs index 3ca680e..5afa920 100644 --- a/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/ServiceCollectionExtensions.cs +++ b/src/NextIteration.SpectreConsole.Auth.Providers.Adobe/ServiceCollectionExtensions.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.DependencyInjection; using NextIteration.SpectreConsole.Auth.Commands; +using NextIteration.SpectreConsole.Auth.Services; namespace NextIteration.SpectreConsole.Auth.Providers.Adobe { @@ -11,11 +12,19 @@ public static class ServiceCollectionExtensions /// /// Registers and the Adobe /// so it appears in the - /// accounts add provider-selection prompt. + /// accounts add provider-selection prompt. The auth service + /// is also registered against the + /// abstraction + /// so consumers that depend on the interface (rather than the concrete + /// type) can resolve it. /// public static IServiceCollection AddAdobeAuthProvider(this IServiceCollection services) { services.AddSingleton(); + // Forward the interface registration to the concrete singleton so + // both resolution shapes return the same instance. + services.AddSingleton>( + sp => sp.GetRequiredService()); services.AddSingleton(); services.AddSingleton(); return services; diff --git a/src/NextIteration.SpectreConsole.Auth.Providers.Airtable/NextIteration.SpectreConsole.Auth.Providers.Airtable.csproj b/src/NextIteration.SpectreConsole.Auth.Providers.Airtable/NextIteration.SpectreConsole.Auth.Providers.Airtable.csproj index bdf61b4..6006fe0 100644 --- a/src/NextIteration.SpectreConsole.Auth.Providers.Airtable/NextIteration.SpectreConsole.Auth.Providers.Airtable.csproj +++ b/src/NextIteration.SpectreConsole.Auth.Providers.Airtable/NextIteration.SpectreConsole.Auth.Providers.Airtable.csproj @@ -11,10 +11,8 @@ NextIteration.SpectreConsole.Auth.Providers.Airtable - 0.2.0 + 0.2.1 Airtable credential provider for NextIteration.SpectreConsole.Auth. Ships AirtableCredential, AirtableToken, AirtableAuthenticationService (pass-through personal access token), and the Spectre.Console collector that drives the accounts-add prompt. - true - C:\nuget-local\ true MIT README.md @@ -34,16 +32,11 @@ - - - - - + + + + + diff --git a/src/NextIteration.SpectreConsole.Auth.Providers.Airtable/ServiceCollectionExtensions.cs b/src/NextIteration.SpectreConsole.Auth.Providers.Airtable/ServiceCollectionExtensions.cs index c67bcd5..60c4a4a 100644 --- a/src/NextIteration.SpectreConsole.Auth.Providers.Airtable/ServiceCollectionExtensions.cs +++ b/src/NextIteration.SpectreConsole.Auth.Providers.Airtable/ServiceCollectionExtensions.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.DependencyInjection; using NextIteration.SpectreConsole.Auth.Commands; +using NextIteration.SpectreConsole.Auth.Services; namespace NextIteration.SpectreConsole.Auth.Providers.Airtable { @@ -11,11 +12,19 @@ public static class ServiceCollectionExtensions /// /// Registers and the Airtable /// so it appears in the - /// accounts add provider-selection prompt. + /// accounts add provider-selection prompt. The auth service + /// is also registered against the + /// abstraction + /// so consumers that depend on the interface (rather than the concrete + /// type) can resolve it. /// public static IServiceCollection AddAirtableAuthProvider(this IServiceCollection services) { services.AddSingleton(); + // Forward the interface registration to the concrete singleton so + // both resolution shapes return the same instance. + services.AddSingleton>( + sp => sp.GetRequiredService()); services.AddSingleton(); services.AddSingleton(); return services; diff --git a/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.csproj b/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.csproj index 0182f06..5e34d4f 100644 --- a/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.csproj +++ b/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.csproj @@ -11,10 +11,8 @@ NextIteration.SpectreConsole.Auth.Providers.SoftwareOne - 0.3.0 + 0.3.1 SoftwareOne Marketplace credential provider for NextIteration.SpectreConsole.Auth. Ships SoftwareOneCredential, SoftwareOneToken, SoftwareOneAuthenticationService, and the Spectre.Console collector that drives the accounts-add prompt. The collector performs a live lookup against the Marketplace API at add-time to validate the token and enrich the credential with the account and token metadata. - true - C:\nuget-local\ true MIT README.md @@ -38,17 +36,12 @@ - - - - - - + + + + + + diff --git a/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/ServiceCollectionExtensions.cs b/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/ServiceCollectionExtensions.cs index 3e46ef7..149cbc1 100644 --- a/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/ServiceCollectionExtensions.cs +++ b/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/ServiceCollectionExtensions.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.DependencyInjection; using NextIteration.SpectreConsole.Auth.Commands; +using NextIteration.SpectreConsole.Auth.Services; namespace NextIteration.SpectreConsole.Auth.Providers.SoftwareOne { @@ -11,11 +12,19 @@ public static class ServiceCollectionExtensions /// /// Registers and the /// SoftwareOne so it appears in the - /// accounts add provider-selection prompt. + /// accounts add provider-selection prompt. The auth service + /// is also registered against the + /// abstraction + /// so consumers that depend on the interface (rather than the concrete + /// type) can resolve it. /// public static IServiceCollection AddSoftwareOneAuthProvider(this IServiceCollection services) { services.AddSingleton(); + // Forward the interface registration to the concrete singleton so + // both resolution shapes return the same instance. + services.AddSingleton>( + sp => sp.GetRequiredService()); services.AddSingleton(); services.AddSingleton(); return services; diff --git a/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/SoftwareOneAuthenticationService.cs b/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/SoftwareOneAuthenticationService.cs index a0346a5..c002234 100644 --- a/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/SoftwareOneAuthenticationService.cs +++ b/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/SoftwareOneAuthenticationService.cs @@ -82,6 +82,12 @@ private static void ValidateCredential(SoftwareOneCredential credential) RequireNonWhitespace(credential.AccountName, nameof(SoftwareOneCredential.AccountName)); RequireNonWhitespace(credential.AccountType, nameof(SoftwareOneCredential.AccountType)); + // Re-check the URL scheme even on the auth path. The collector + // already enforces https-or-loopback, but a hand-edited keystore + // file could downgrade BaseUrl to plain http. Refusing here + // prevents the token from being shipped over a cleartext channel. + RequireSecureUrl(credential.BaseUrl, nameof(SoftwareOneCredential.BaseUrl)); + // Local helper — `fieldName` is the paramName so CA2208 is happy. // The field shows up as the ArgumentException.ParamName, which // matches what consumers would expect ("which field was bad?"). @@ -94,6 +100,23 @@ static void RequireNonWhitespace(string value, string fieldName) fieldName); } } + + static void RequireSecureUrl(Uri url, string fieldName) + { + if (url.Scheme == Uri.UriSchemeHttps) + { + return; + } + + if (url.Scheme == Uri.UriSchemeHttp && url.IsLoopback) + { + return; + } + + throw new ArgumentException( + $"{fieldName} must use https (http is only accepted for loopback addresses).", + fieldName); + } } } } diff --git a/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/SoftwareOneCredentialCollector.cs b/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/SoftwareOneCredentialCollector.cs index 4bcd9c2..accc229 100644 --- a/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/SoftwareOneCredentialCollector.cs +++ b/src/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne/SoftwareOneCredentialCollector.cs @@ -52,6 +52,12 @@ private static readonly CompositeFormat TokenLookupPathFormat private static readonly JsonSerializerOptions TokenLookupJsonOptions = new() { PropertyNameCaseInsensitive = true }; + // Hard cap on the response-body slice we surface in exceptions. + // Big enough to keep useful "invalid_token"-style payloads, small + // enough that an upstream proxy echoing the full request can't bloat + // logs or sneak a credential past our redaction. + internal const int ErrorBodyMaxChars = 512; + private readonly IHttpClientFactory _httpClientFactory; /// DI constructor. @@ -77,10 +83,7 @@ public SoftwareOneCredentialCollector(IHttpClientFactory httpClientFactory) var baseUrlInput = await AnsiConsole.PromptAsync( new TextPrompt("Enter Base URL:") .DefaultValue(DefaultBaseUrl) - .Validate(value => Uri.TryCreate(value, UriKind.Absolute, out var parsed) - && (parsed.Scheme == Uri.UriSchemeHttp || parsed.Scheme == Uri.UriSchemeHttps) - ? ValidationResult.Success() - : ValidationResult.Error("Must be a valid absolute http(s) URL"))).ConfigureAwait(false); + .Validate(ValidateSecureBaseUrl)).ConfigureAwait(false); var environment = await AnsiConsole.PromptAsync( new SelectionPrompt() @@ -143,8 +146,14 @@ internal async Task LookupTokenAsync(Uri baseUrl, string ap if (!response.IsSuccessStatusCode) { + // Sanitise before surfacing: a misbehaving upstream proxy can + // echo the request URL — which carries the token in the + // eq(token,'…') query — back into an error page. Redact the + // literal token and cap the slice we keep so neither logs + // nor exception aggregators receive credential material. + var safeBody = SanitiseErrorBody(responseBody, apiToken); throw new InvalidOperationException( - $"SoftwareOne token lookup failed: {(int)response.StatusCode} {response.StatusCode}. Body: {responseBody}"); + $"SoftwareOne token lookup failed: {(int)response.StatusCode} {response.StatusCode}. Body: {safeBody}"); } var result = JsonSerializer.Deserialize(responseBody, TokenLookupJsonOptions) @@ -165,5 +174,58 @@ internal async Task LookupTokenAsync(Uri baseUrl, string ap return result.Data[0]; } + + /// + /// Accept the URL only if it's an absolute https URI, or an + /// http loopback (so devs can point the collector at a local + /// mock or proxy without compromising the token over the wire + /// in real deployments). + /// + internal static ValidationResult ValidateSecureBaseUrl(string value) + { + if (!Uri.TryCreate(value, UriKind.Absolute, out var parsed)) + { + return ValidationResult.Error("Must be a valid absolute http(s) URL"); + } + + if (parsed.Scheme == Uri.UriSchemeHttps) + { + return ValidationResult.Success(); + } + + if (parsed.Scheme == Uri.UriSchemeHttp && parsed.IsLoopback) + { + return ValidationResult.Success(); + } + + return ValidationResult.Error( + "Must use https. http is only accepted for loopback addresses (the SoftwareOne API token is sent in the URL query and must not traverse the network in cleartext)."); + } + + /// + /// Strip the literal token value from the response body and + /// truncate to . The token is + /// what we're trying to prevent from leaking into logs; the + /// truncation bounds size for everything else (memory pressure, + /// noise in aggregators). + /// + internal static string SanitiseErrorBody(string body, string apiToken) + { + if (string.IsNullOrEmpty(body)) + { + return body; + } + + var redacted = string.IsNullOrEmpty(apiToken) + ? body + : body.Replace(apiToken, "", StringComparison.Ordinal); + + if (redacted.Length <= ErrorBodyMaxChars) + { + return redacted; + } + + return string.Concat(redacted.AsSpan(0, ErrorBodyMaxChars), "… [truncated]"); + } } } diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/AdobeAuthenticationServiceTests.cs b/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/AdobeAuthenticationServiceTests.cs index 84b39f6..9b9f638 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/AdobeAuthenticationServiceTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/AdobeAuthenticationServiceTests.cs @@ -153,6 +153,71 @@ public async Task AuthenticateAsync_WhenImsReturnsError_IncludesResponseBodyInEx Assert.Contains("BadRequest", ex.Message, StringComparison.Ordinal); } + [Fact] + public async Task AuthenticateAsync_WhenImsReturnsLargeErrorBody_TruncatesItInException() + { + // Build a body well over the 512-char cap so the exception message + // ends up bounded in size — keeps log noise low if an upstream proxy + // echoes the full request back. + var bigBody = new string('x', 4000); + var http = StubHttpClientFactory.ReturningJson(bigBody, HttpStatusCode.BadGateway); + var service = new AdobeAuthenticationService(new FakeCredentialManager(), http); + + var ex = await Assert.ThrowsAsync( + () => service.AuthenticateAsync(NewCredential())); + + Assert.Contains("[truncated]", ex.Message, StringComparison.Ordinal); + // The status preamble plus "[truncated]" suffix push the message a + // bit past 512, but it should be far short of the 4000-char body. + Assert.True(ex.Message.Length < 1024, $"Expected truncated message, was {ex.Message.Length} chars"); + } + + [Theory] + [InlineData("http://ims-na1.adobelogin.com/", "https://partners.adobe.io/")] + [InlineData("https://ims-na1.adobelogin.com/", "http://partners.adobe.io/")] + public async Task AuthenticateAsync_RejectsCredentialWithCleartextHttpUrl(string imsUrl, string baseUrl) + { + // A hand-edited keystore that downgrades either URL to cleartext + // http should be rejected before any token leaves the process. + var http = StubHttpClientFactory.ReturningJson("{}"); + var service = new AdobeAuthenticationService(new FakeCredentialManager(), http); + var credential = new AdobeCredential + { + ImsUrl = new Uri(imsUrl, UriKind.Absolute), + ApiKey = "abc-api-key", + ClientSecret = "super-secret", + BaseUrl = new Uri(baseUrl, UriKind.Absolute), + Environment = "Production", + }; + + await Assert.ThrowsAsync(() => service.AuthenticateAsync(credential)); + Assert.Null(http.LastRequest); + } + + [Fact] + public async Task AuthenticateAsync_AllowsHttpLoopback() + { + // Loopback http stays usable so devs can point the auth service at + // a local mock IMS — the credential never leaves the box. + var http = StubHttpClientFactory.ReturningJson(""" + { "access_token": "at", "token_type": "bearer", "expires_in": 3600 } + """); + var service = new AdobeAuthenticationService(new FakeCredentialManager(), http); + var credential = new AdobeCredential + { + ImsUrl = new Uri("http://127.0.0.1:5000/"), + ApiKey = "abc-api-key", + ClientSecret = "super-secret", + BaseUrl = new Uri("http://localhost:6000/"), + Environment = "Sandbox", + }; + + var token = await service.AuthenticateAsync(credential); + + Assert.Equal("at", token.AccessToken); + Assert.Equal(new Uri("http://localhost:6000/"), token.BaseUrl); + } + [Fact] public async Task AuthenticateAsync_NoCredentialSelected_Throws() { diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/AdobeCredentialCollectorTests.cs b/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/AdobeCredentialCollectorTests.cs index 6c41a60..5cb1e02 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/AdobeCredentialCollectorTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/AdobeCredentialCollectorTests.cs @@ -1,3 +1,4 @@ +using Spectre.Console; using Xunit; namespace NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests; @@ -17,4 +18,47 @@ public void ProviderName_MatchesCredential() Assert.Equal(AdobeCredential.ProviderName, collector.ProviderName); Assert.Equal("Adobe", collector.ProviderName); } + + [Theory] + [InlineData("https://ims-na1.adobelogin.com/")] + [InlineData("https://partners.adobe.io/")] + [InlineData("https://example.com:8443/")] + public void ValidateSecureUrl_AcceptsHttps(string url) + { + var result = AdobeCredentialCollector.ValidateSecureUrl(url); + Assert.True(result.Successful); + } + + [Theory] + [InlineData("http://127.0.0.1:8080/")] + [InlineData("http://localhost:5000/")] + [InlineData("http://[::1]:9000/")] + public void ValidateSecureUrl_AcceptsHttpLoopback(string url) + { + // Loopback http allowed so devs can point the collector at a local + // mock IMS without compromising the credential in real deployments. + var result = AdobeCredentialCollector.ValidateSecureUrl(url); + Assert.True(result.Successful); + } + + [Theory] + [InlineData("http://ims-na1.adobelogin.com/")] + [InlineData("http://example.com/")] + [InlineData("http://10.0.0.1/")] + public void ValidateSecureUrl_RejectsHttpForNonLoopback(string url) + { + var result = AdobeCredentialCollector.ValidateSecureUrl(url); + Assert.False(result.Successful); + Assert.Contains("https", result.Message ?? string.Empty, StringComparison.Ordinal); + } + + [Theory] + [InlineData("ftp://example.com/")] + [InlineData("not-a-url")] + [InlineData("")] + public void ValidateSecureUrl_RejectsNonHttpSchemesAndGarbage(string url) + { + var result = AdobeCredentialCollector.ValidateSecureUrl(url); + Assert.False(result.Successful); + } } diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/FakeCredentialManager.cs b/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/FakeCredentialManager.cs index f71445e..fe53ad1 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/FakeCredentialManager.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/FakeCredentialManager.cs @@ -15,6 +15,9 @@ internal sealed class FakeCredentialManager : ICredentialManager public Task GetSelectedCredentialAsync(string providerName) => Task.FromResult(SelectedCredentialJson); + public Task GetCredentialByIdAsync(string providerName, string accountId) + => throw new NotSupportedException(); + public Task> ListCredentialsAsync(string providerName) => throw new NotSupportedException(); diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests.csproj b/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests.csproj index d4e0572..491bcc5 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests.csproj +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests.csproj @@ -22,18 +22,19 @@ - - - + + + + runtime; build; native; contentfiles; analyzers; buildtransitive all - + runtime; build; native; contentfiles; analyzers; buildtransitive all - - + + diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/ServiceCollectionExtensionsTests.cs b/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/ServiceCollectionExtensionsTests.cs index 96c923b..5eb4598 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/ServiceCollectionExtensionsTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests/ServiceCollectionExtensionsTests.cs @@ -1,6 +1,7 @@ using Microsoft.Extensions.DependencyInjection; using NextIteration.SpectreConsole.Auth.Commands; using NextIteration.SpectreConsole.Auth.Persistence; +using NextIteration.SpectreConsole.Auth.Services; using Xunit; namespace NextIteration.SpectreConsole.Auth.Providers.Adobe.Tests; @@ -64,4 +65,38 @@ public void AddAdobeAuthProvider_RegistersAsSingletons() var b = sp.GetRequiredService(); Assert.Same(a, b); } + + [Fact] + public void AddAdobeAuthProvider_ResolvesAuthenticationServiceViaInterface() + { + // Consumers depending on the IAuthenticationService<,> abstraction + // (rather than the concrete type) must be able to resolve it. + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddHttpClient(); + + services.AddAdobeAuthProvider(); + + using var sp = services.BuildServiceProvider(); + var viaInterface = sp.GetRequiredService>(); + Assert.IsType(viaInterface); + } + + [Fact] + public void AddAdobeAuthProvider_InterfaceForwardsToSameSingletonAsConcrete() + { + // The interface registration must forward to the concrete singleton + // (not register a second instance), so resolving by either shape + // returns the same object. + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddHttpClient(); + + services.AddAdobeAuthProvider(); + + using var sp = services.BuildServiceProvider(); + var concrete = sp.GetRequiredService(); + var viaInterface = sp.GetRequiredService>(); + Assert.Same(concrete, viaInterface); + } } diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests/FakeCredentialManager.cs b/tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests/FakeCredentialManager.cs index d34b7f1..ec38032 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests/FakeCredentialManager.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests/FakeCredentialManager.cs @@ -15,6 +15,9 @@ internal sealed class FakeCredentialManager : ICredentialManager public Task GetSelectedCredentialAsync(string providerName) => Task.FromResult(SelectedCredentialJson); + public Task GetCredentialByIdAsync(string providerName, string accountId) + => throw new NotSupportedException(); + public Task> ListCredentialsAsync(string providerName) => throw new NotSupportedException(); diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests.csproj b/tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests.csproj index 62d4a6d..97f88db 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests.csproj +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests.csproj @@ -22,17 +22,18 @@ - - - + + + + runtime; build; native; contentfiles; analyzers; buildtransitive all - + runtime; build; native; contentfiles; analyzers; buildtransitive all - + diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests/ServiceCollectionExtensionsTests.cs b/tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests/ServiceCollectionExtensionsTests.cs index 18f17fb..0413c32 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests/ServiceCollectionExtensionsTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests/ServiceCollectionExtensionsTests.cs @@ -1,6 +1,7 @@ using Microsoft.Extensions.DependencyInjection; using NextIteration.SpectreConsole.Auth.Commands; using NextIteration.SpectreConsole.Auth.Persistence; +using NextIteration.SpectreConsole.Auth.Services; using Xunit; namespace NextIteration.SpectreConsole.Auth.Providers.Airtable.Tests; @@ -62,4 +63,31 @@ public void AddAirtableAuthProvider_RegistersAsSingletons() var b = sp.GetRequiredService(); Assert.Same(a, b); } + + [Fact] + public void AddAirtableAuthProvider_ResolvesAuthenticationServiceViaInterface() + { + var services = new ServiceCollection(); + services.AddSingleton(); + + services.AddAirtableAuthProvider(); + + using var sp = services.BuildServiceProvider(); + var viaInterface = sp.GetRequiredService>(); + Assert.IsType(viaInterface); + } + + [Fact] + public void AddAirtableAuthProvider_InterfaceForwardsToSameSingletonAsConcrete() + { + var services = new ServiceCollection(); + services.AddSingleton(); + + services.AddAirtableAuthProvider(); + + using var sp = services.BuildServiceProvider(); + var concrete = sp.GetRequiredService(); + var viaInterface = sp.GetRequiredService>(); + Assert.Same(concrete, viaInterface); + } } diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/FakeCredentialManager.cs b/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/FakeCredentialManager.cs index c12be40..ea829c8 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/FakeCredentialManager.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/FakeCredentialManager.cs @@ -15,6 +15,9 @@ internal sealed class FakeCredentialManager : ICredentialManager public Task GetSelectedCredentialAsync(string providerName) => Task.FromResult(SelectedCredentialJson); + public Task GetCredentialByIdAsync(string providerName, string accountId) + => throw new NotSupportedException(); + public Task> ListCredentialsAsync(string providerName) => throw new NotSupportedException(); diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests.csproj b/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests.csproj index 2f443c8..705cb6e 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests.csproj +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests.csproj @@ -22,17 +22,18 @@ - - - + + + + runtime; build; native; contentfiles; analyzers; buildtransitive all - + runtime; build; native; contentfiles; analyzers; buildtransitive all - + diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/ServiceCollectionExtensionsTests.cs b/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/ServiceCollectionExtensionsTests.cs index 7a6be61..c6de443 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/ServiceCollectionExtensionsTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/ServiceCollectionExtensionsTests.cs @@ -1,6 +1,7 @@ using Microsoft.Extensions.DependencyInjection; using NextIteration.SpectreConsole.Auth.Commands; using NextIteration.SpectreConsole.Auth.Persistence; +using NextIteration.SpectreConsole.Auth.Services; using Xunit; namespace NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests; @@ -64,4 +65,33 @@ public void AddSoftwareOneAuthProvider_RegistersAsSingletons() var b = sp.GetRequiredService(); Assert.Same(a, b); } + + [Fact] + public void AddSoftwareOneAuthProvider_ResolvesAuthenticationServiceViaInterface() + { + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddHttpClient(); + + services.AddSoftwareOneAuthProvider(); + + using var sp = services.BuildServiceProvider(); + var viaInterface = sp.GetRequiredService>(); + Assert.IsType(viaInterface); + } + + [Fact] + public void AddSoftwareOneAuthProvider_InterfaceForwardsToSameSingletonAsConcrete() + { + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddHttpClient(); + + services.AddSoftwareOneAuthProvider(); + + using var sp = services.BuildServiceProvider(); + var concrete = sp.GetRequiredService(); + var viaInterface = sp.GetRequiredService>(); + Assert.Same(concrete, viaInterface); + } } diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/SoftwareOneAuthenticationServiceTests.cs b/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/SoftwareOneAuthenticationServiceTests.cs index 81c2477..d16544a 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/SoftwareOneAuthenticationServiceTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/SoftwareOneAuthenticationServiceTests.cs @@ -138,6 +138,52 @@ public async Task AuthenticateAsync_WithStoredCredential_ReturnsProjectedToken() Assert.Equal("Vendor", token.Actor); } + [Fact] + public async Task AuthenticateAsync_RejectsCredentialWithCleartextHttpBaseUrl() + { + // A hand-edited keystore that downgrades BaseUrl to plain http + // should be rejected before any token leaves the process. + var template = NewCredential(); + var bad = new SoftwareOneCredential + { + ApiToken = template.ApiToken, + BaseUrl = new Uri("http://api.softwareone.com/"), + Environment = template.Environment, + Actor = template.Actor, + TokenId = template.TokenId, + TokenName = template.TokenName, + AccountId = template.AccountId, + AccountName = template.AccountName, + AccountType = template.AccountType, + }; + var service = new SoftwareOneAuthenticationService(new FakeCredentialManager()); + + await Assert.ThrowsAsync(() => service.AuthenticateAsync(bad)); + } + + [Fact] + public async Task AuthenticateAsync_AllowsHttpLoopback() + { + var template = NewCredential(); + var loopback = new SoftwareOneCredential + { + ApiToken = template.ApiToken, + BaseUrl = new Uri("http://127.0.0.1:6000/"), + Environment = template.Environment, + Actor = template.Actor, + TokenId = template.TokenId, + TokenName = template.TokenName, + AccountId = template.AccountId, + AccountName = template.AccountName, + AccountType = template.AccountType, + }; + var service = new SoftwareOneAuthenticationService(new FakeCredentialManager()); + + var token = await service.AuthenticateAsync(loopback); + + Assert.Equal(new Uri("http://127.0.0.1:6000/"), token.BaseUrl); + } + [Fact] public async Task ValidateTokenAsync_ReturnsTrue_ForNonExpiredToken() { diff --git a/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/SoftwareOneCredentialCollectorLookupTests.cs b/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/SoftwareOneCredentialCollectorLookupTests.cs index 5ff8ad9..ecdfd6d 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/SoftwareOneCredentialCollectorLookupTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Providers.SoftwareOne.Tests/SoftwareOneCredentialCollectorLookupTests.cs @@ -121,6 +121,110 @@ public async Task LookupTokenAsync_HttpError_Throws_WithBodyInMessage() Assert.Contains("unauthorized", ex.Message, StringComparison.Ordinal); } + [Fact] + public async Task LookupTokenAsync_HttpError_RedactsTokenFromBody() + { + // Simulate a misbehaving upstream proxy that echoes the request URL + // (which carries the token in the eq(token,'…') query) back into the + // error body. The collector must redact the token before it lands in + // the exception message — otherwise the credential leaks via logs. + const string tokenValue = "tok-secret-12345"; + var http = StubHttpClientFactory.ReturningJson( + $$"""{ "error": "bad gateway", "request_url": "/v1/accounts/api-tokens?eq(token,'{{tokenValue}}')&limit=2" }""", + HttpStatusCode.BadGateway); + var collector = new SoftwareOneCredentialCollector(http); + + var ex = await Assert.ThrowsAsync( + () => collector.LookupTokenAsync(BaseUrl, tokenValue)); + + Assert.DoesNotContain(tokenValue, ex.Message, StringComparison.Ordinal); + Assert.Contains("", ex.Message, StringComparison.Ordinal); + // The non-credential context ("bad gateway") should still be visible + // — diagnostics aren't sacrificed for sanitisation. + Assert.Contains("bad gateway", ex.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task LookupTokenAsync_HttpError_TruncatesLargeBody() + { + var bigBody = new string('y', 4000); + var http = StubHttpClientFactory.ReturningJson(bigBody, HttpStatusCode.InternalServerError); + var collector = new SoftwareOneCredentialCollector(http); + + var ex = await Assert.ThrowsAsync( + () => collector.LookupTokenAsync(BaseUrl, "abc-123")); + + Assert.Contains("[truncated]", ex.Message, StringComparison.Ordinal); + Assert.True(ex.Message.Length < 1024, $"Expected truncated message, was {ex.Message.Length} chars"); + } + + [Fact] + public void SanitiseErrorBody_RedactsTokenWithinKeptWindowAndTruncates() + { + // Token sits well inside the first 512 chars so it's both redacted + // and visible after truncation. Covers the common case: a small + // error envelope wrapped in a much larger HTML page from a proxy. + var body = new string('a', 200) + "tok-xyz" + new string('b', 1000); + var sanitised = SoftwareOneCredentialCollector.SanitiseErrorBody(body, "tok-xyz"); + + Assert.DoesNotContain("tok-xyz", sanitised, StringComparison.Ordinal); + Assert.Contains("", sanitised, StringComparison.Ordinal); + Assert.EndsWith("[truncated]", sanitised, StringComparison.Ordinal); + } + + [Fact] + public void SanitiseErrorBody_TokenPastTruncationBoundary_StillRemovedFromOutput() + { + // Pathological case: token sits past the 512-char cap. Redaction + // happens before truncation, but the truncated tail is dropped + // entirely — so even though the literal "" marker isn't + // visible in the kept window, the token itself MUST NOT be either. + var body = new string('a', 600) + "tok-xyz" + new string('b', 600); + var sanitised = SoftwareOneCredentialCollector.SanitiseErrorBody(body, "tok-xyz"); + + Assert.DoesNotContain("tok-xyz", sanitised, StringComparison.Ordinal); + Assert.EndsWith("[truncated]", sanitised, StringComparison.Ordinal); + } + + [Theory] + [InlineData("https://api.softwareone.com/")] + [InlineData("https://test.softwareone.com:8443/")] + public void ValidateSecureBaseUrl_AcceptsHttps(string url) + { + var result = SoftwareOneCredentialCollector.ValidateSecureBaseUrl(url); + Assert.True(result.Successful); + } + + [Theory] + [InlineData("http://127.0.0.1:8080/")] + [InlineData("http://localhost:5000/")] + public void ValidateSecureBaseUrl_AcceptsHttpLoopback(string url) + { + var result = SoftwareOneCredentialCollector.ValidateSecureBaseUrl(url); + Assert.True(result.Successful); + } + + [Theory] + [InlineData("http://api.softwareone.com/")] + [InlineData("http://example.com/")] + public void ValidateSecureBaseUrl_RejectsCleartextHttpForNonLoopback(string url) + { + // The token rides in the URL query — cleartext http would expose it + // on every hop and in any access log. + var result = SoftwareOneCredentialCollector.ValidateSecureBaseUrl(url); + Assert.False(result.Successful); + Assert.Contains("https", result.Message ?? string.Empty, StringComparison.Ordinal); + } + + [Theory] + [InlineData("ftp://example.com/")] + [InlineData("not-a-url")] + public void ValidateSecureBaseUrl_RejectsNonHttpSchemesAndGarbage(string url) + { + var result = SoftwareOneCredentialCollector.ValidateSecureBaseUrl(url); + Assert.False(result.Successful); + } + [Fact] public async Task LookupTokenAsync_MalformedJson_Throws() {