From 96cd3c44950846712010067332583a7b25776fa1 Mon Sep 17 00:00:00 2001 From: Oleksandr Bezdieniezhnykh Date: Mon, 11 May 2026 23:06:23 +0300 Subject: [PATCH] [AZ-487] JWT validation baseline (HS256, all endpoints) Adds Microsoft.AspNetCore.Authentication.JwtBearer 8.0.21 and the SatelliteProvider.Api.Authentication.AddSatelliteJwt extension that validates HS256 tokens against a shared JWT_SECRET (>=32 bytes, fail fast at startup). Every minimal-API endpoint now carries .RequireAuthorization(); the middleware chain is UseExceptionHandler -> UseHttpsRedirection -> UseCors -> UseAuthentication -> UseAuthorization -> endpoints. Swagger UI gets a Bearer security definition so the Authorize button works. Test infrastructure: JwtTokenFactory (unit) and JwtTestHelpers (integration) mint deterministic tokens against the same secret; the integration test runner attaches a default Bearer token to its shared HttpClient so existing tests continue to exercise protected endpoints. JwtIntegrationTests adds AC-1..AC-4 and AC-7 (Swagger advertises Bearer) end-to-end; AuthenticationServiceCollectionExtensionsTests covers AC-5 (missing/empty/short secret fail-fast) plus env-var precedence; JwtTokenFactoryTests covers AC-6 (claims pass through the JwtSecurityTokenHandler.ValidateToken path JwtBearer uses). docker-compose and scripts/run-tests.sh now propagate JWT_SECRET to the api and integration-tests containers, with a >=32-byte guard. .env.example documents the required keys; .env stays gitignored. Code review verdict: PASS_WITH_WARNINGS (2 Low findings surfaced in _docs/03_implementation/reviews/batch_01_cycle2_review.md). Cross-component coordination: gps-denied-onboard and the mission planner UI must attach Bearer tokens before this lands in dev. Co-authored-by: Cursor --- .env.example | 18 +++ ...thenticationServiceCollectionExtensions.cs | 66 ++++++++ SatelliteProvider.Api/Program.cs | 39 +++++ .../SatelliteProvider.Api.csproj | 1 + .../appsettings.Development.json | 3 + SatelliteProvider.Api/appsettings.json | 3 + .../JwtIntegrationTests.cs | 146 +++++++++++++++++ .../JwtTestHelpers.cs | 86 ++++++++++ SatelliteProvider.IntegrationTests/Program.cs | 18 +++ .../SatelliteProvider.IntegrationTests.csproj | 1 + ...icationServiceCollectionExtensionsTests.cs | 151 ++++++++++++++++++ .../Authentication/JwtTokenFactoryTests.cs | 93 +++++++++++ .../TestUtilities/JwtTokenFactory.cs | 76 +++++++++ _docs/02_document/architecture.md | 19 ++- _docs/02_document/modules/api_program.md | 11 +- _docs/02_tasks/_dependencies_table.md | 2 +- .../AZ-487_jwt_validation_baseline.md | 0 .../batch_01_cycle2_report.md | 48 ++++++ .../reviews/batch_01_cycle2_review.md | 81 ++++++++++ _docs/_autodev_state.md | 6 +- docker-compose.tests.yml | 1 + docker-compose.yml | 1 + scripts/run-tests.sh | 17 +- 23 files changed, 872 insertions(+), 15 deletions(-) create mode 100644 .env.example create mode 100644 SatelliteProvider.Api/Authentication/AuthenticationServiceCollectionExtensions.cs create mode 100644 SatelliteProvider.IntegrationTests/JwtIntegrationTests.cs create mode 100644 SatelliteProvider.IntegrationTests/JwtTestHelpers.cs create mode 100644 SatelliteProvider.Tests/Authentication/AuthenticationServiceCollectionExtensionsTests.cs create mode 100644 SatelliteProvider.Tests/Authentication/JwtTokenFactoryTests.cs create mode 100644 SatelliteProvider.Tests/TestUtilities/JwtTokenFactory.cs rename _docs/02_tasks/{todo => done}/AZ-487_jwt_validation_baseline.md (100%) create mode 100644 _docs/03_implementation/batch_01_cycle2_report.md create mode 100644 _docs/03_implementation/reviews/batch_01_cycle2_review.md diff --git a/.env.example b/.env.example new file mode 100644 index 0000000..1ff3699 --- /dev/null +++ b/.env.example @@ -0,0 +1,18 @@ +# Satellite Provider environment configuration template. +# Copy this file to `.env` and replace placeholder values before running +# docker-compose or scripts/run-tests.sh. +# +# IMPORTANT: `.env` is gitignored on purpose. Never commit real secrets. + +# Google Maps Platform API key for satellite imagery downloads. +GOOGLE_MAPS_API_KEY= + +# HMAC-SHA256 signing key for JWT validation (suite-level auth contract, +# `suite/_docs/10_auth.md`). MUST be at least 32 bytes (UTF-8) — the API +# fails fast on startup if this is unset or shorter. +# +# Generate a strong secret with, for example: +# openssl rand -hex 32 +# +# Test/CI runs may use a clearly tagged TEST-ONLY value (still >=32 bytes). +JWT_SECRET= diff --git a/SatelliteProvider.Api/Authentication/AuthenticationServiceCollectionExtensions.cs b/SatelliteProvider.Api/Authentication/AuthenticationServiceCollectionExtensions.cs new file mode 100644 index 0000000..ecda620 --- /dev/null +++ b/SatelliteProvider.Api/Authentication/AuthenticationServiceCollectionExtensions.cs @@ -0,0 +1,66 @@ +using System.Text; +using Microsoft.AspNetCore.Authentication.JwtBearer; +using Microsoft.IdentityModel.Tokens; + +namespace SatelliteProvider.Api.Authentication; + +public static class AuthenticationServiceCollectionExtensions +{ + public const string JwtSecretEnvVar = "JWT_SECRET"; + public const string JwtSecretConfigKey = "Jwt:Secret"; + public const int MinSecretByteLength = 32; + + public static IServiceCollection AddSatelliteJwt(this IServiceCollection services, IConfiguration configuration) + { + ArgumentNullException.ThrowIfNull(services); + ArgumentNullException.ThrowIfNull(configuration); + + var secret = ResolveSecretOrThrow(configuration); + var signingKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(secret)); + + services + .AddAuthentication(JwtBearerDefaults.AuthenticationScheme) + .AddJwtBearer(options => + { + options.TokenValidationParameters = new TokenValidationParameters + { + ValidateIssuerSigningKey = true, + IssuerSigningKey = signingKey, + ValidateLifetime = true, + ClockSkew = TimeSpan.FromSeconds(30), + ValidateIssuer = false, + ValidateAudience = false, + RequireSignedTokens = true, + RequireExpirationTime = true + }; + }); + + return services; + } + + internal static string ResolveSecretOrThrow(IConfiguration configuration) + { + var secret = Environment.GetEnvironmentVariable(JwtSecretEnvVar); + if (string.IsNullOrWhiteSpace(secret)) + { + secret = configuration[JwtSecretConfigKey]; + } + + if (string.IsNullOrWhiteSpace(secret)) + { + throw new InvalidOperationException( + $"JWT secret is not configured. Set the {JwtSecretEnvVar} environment variable " + + $"or the {JwtSecretConfigKey} configuration key to a value of at least {MinSecretByteLength} bytes."); + } + + var byteLength = Encoding.UTF8.GetByteCount(secret); + if (byteLength < MinSecretByteLength) + { + throw new InvalidOperationException( + $"JWT secret is too short ({byteLength} bytes). HMAC-SHA256 requires at least {MinSecretByteLength} bytes " + + $"per RFC 2104 §3. Set {JwtSecretEnvVar} or {JwtSecretConfigKey} to a longer value."); + } + + return secret; + } +} diff --git a/SatelliteProvider.Api/Program.cs b/SatelliteProvider.Api/Program.cs index 487a2f9..f1bdd19 100644 --- a/SatelliteProvider.Api/Program.cs +++ b/SatelliteProvider.Api/Program.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.OpenApi.Models; using Swashbuckle.AspNetCore.SwaggerGen; using SatelliteProvider.Api; +using SatelliteProvider.Api.Authentication; using SatelliteProvider.Api.DTOs; using SatelliteProvider.Api.Swagger; using SatelliteProvider.DataAccess; @@ -39,6 +40,9 @@ builder.Services.AddTileDownloader(); builder.Services.AddRegionProcessing(); builder.Services.AddRouteManagement(); +builder.Services.AddSatelliteJwt(builder.Configuration); +builder.Services.AddAuthorization(); + var allowedOrigins = builder.Configuration.GetSection("CorsConfig:AllowedOrigins").Get() ?? Array.Empty(); var allowAnyOrigin = builder.Configuration.GetValue("CorsConfig:AllowAnyOrigin"); CorsConfigurationValidator.EnsureSafeForEnvironment(allowedOrigins, allowAnyOrigin, builder.Environment.EnvironmentName); @@ -70,6 +74,31 @@ builder.Services.AddSwaggerGen(c => { c.SwaggerDoc("v1", new OpenApiInfo { Title = "Satellite Provider API", Version = "v1" }); + c.AddSecurityDefinition("Bearer", new OpenApiSecurityScheme + { + Name = "Authorization", + Type = SecuritySchemeType.Http, + Scheme = "bearer", + BearerFormat = "JWT", + In = ParameterLocation.Header, + Description = "JWT Authorization header using the Bearer scheme. Example: 'Bearer {token}'" + }); + + c.AddSecurityRequirement(new OpenApiSecurityRequirement + { + { + new OpenApiSecurityScheme + { + Reference = new OpenApiReference + { + Type = ReferenceType.SecurityScheme, + Id = "Bearer" + } + }, + Array.Empty() + } + }); + c.MapType(() => new OpenApiSchema { Type = "object", @@ -119,24 +148,31 @@ if (app.Environment.IsDevelopment()) app.UseExceptionHandler(); app.UseHttpsRedirection(); app.UseCors("TilesCors"); +app.UseAuthentication(); +app.UseAuthorization(); app.MapGet("/tiles/{z:int}/{x:int}/{y:int}", ServeTile) + .RequireAuthorization() .WithOpenApi(op => new(op) { Summary = "Get satellite tile image by z/x/y coordinates (Slippy Map tile server)" }); app.MapGet("/api/satellite/tiles/latlon", GetTileByLatLon) + .RequireAuthorization() .WithOpenApi(op => new(op) { Summary = "Get satellite tile by latitude and longitude coordinates" }); app.MapGet("/api/satellite/tiles/mgrs", GetSatelliteTilesByMgrs) + .RequireAuthorization() .ProducesProblem(StatusCodes.Status501NotImplemented) .WithOpenApi(op => new(op) { Summary = "Get satellite tiles by MGRS coordinates (NOT IMPLEMENTED)" }); app.MapPost("/api/satellite/upload", UploadImage) + .RequireAuthorization() .Accepts("multipart/form-data") .ProducesProblem(StatusCodes.Status501NotImplemented) .WithOpenApi(op => new(op) { Summary = "Upload image with metadata (NOT IMPLEMENTED)" }) .DisableAntiforgery(); app.MapPost("/api/satellite/request", RequestRegion) + .RequireAuthorization() .WithOpenApi(op => new(op) { Summary = "Request tiles for a region", @@ -144,9 +180,11 @@ app.MapPost("/api/satellite/request", RequestRegion) }); app.MapGet("/api/satellite/region/{id:guid}", GetRegionStatus) + .RequireAuthorization() .WithOpenApi(op => new(op) { Summary = "Get region status and file paths" }); app.MapPost("/api/satellite/route", CreateRoute) + .RequireAuthorization() .WithOpenApi(op => new(op) { Summary = "Create a route with intermediate points", @@ -154,6 +192,7 @@ app.MapPost("/api/satellite/route", CreateRoute) }); app.MapGet("/api/satellite/route/{id:guid}", GetRoute) + .RequireAuthorization() .WithOpenApi(op => new(op) { Summary = "Get route information with calculated points" }); app.Run(); diff --git a/SatelliteProvider.Api/SatelliteProvider.Api.csproj b/SatelliteProvider.Api/SatelliteProvider.Api.csproj index bba22db..4851e36 100644 --- a/SatelliteProvider.Api/SatelliteProvider.Api.csproj +++ b/SatelliteProvider.Api/SatelliteProvider.Api.csproj @@ -7,6 +7,7 @@ + diff --git a/SatelliteProvider.Api/appsettings.Development.json b/SatelliteProvider.Api/appsettings.Development.json index 46351f3..2bdd426 100644 --- a/SatelliteProvider.Api/appsettings.Development.json +++ b/SatelliteProvider.Api/appsettings.Development.json @@ -10,6 +10,9 @@ "ConnectionStrings": { "DefaultConnection": "Host=localhost;Port=5432;Database=satelliteprovider;Username=postgres;Password=postgres" }, + "Jwt": { + "Secret": "DEV-ONLY-DO-NOT-USE-IN-PROD-replace-with-real-secret-via-JWT_SECRET-env-var" + }, "MapConfig": { "Service": "GoogleMaps", "ApiKey": "" diff --git a/SatelliteProvider.Api/appsettings.json b/SatelliteProvider.Api/appsettings.json index 4ad9f73..216156b 100644 --- a/SatelliteProvider.Api/appsettings.json +++ b/SatelliteProvider.Api/appsettings.json @@ -23,6 +23,9 @@ "ConnectionStrings": { "DefaultConnection": "Host=localhost;Database=satelliteprovider;Username=postgres;Password=postgres" }, + "Jwt": { + "Secret": "" + }, "MapConfig": { "Service": "GoogleMaps", "ApiKey": "", diff --git a/SatelliteProvider.IntegrationTests/JwtIntegrationTests.cs b/SatelliteProvider.IntegrationTests/JwtIntegrationTests.cs new file mode 100644 index 0000000..217ef94 --- /dev/null +++ b/SatelliteProvider.IntegrationTests/JwtIntegrationTests.cs @@ -0,0 +1,146 @@ +using System.Net; +using System.Net.Http.Headers; + +namespace SatelliteProvider.IntegrationTests; + +public static class JwtIntegrationTests +{ + private const string ProtectedTilesPath = "/api/satellite/tiles/latlon?Latitude=47.461747&Longitude=37.647063&ZoomLevel=18"; + private const string ProtectedRegionPath = "/api/satellite/region/00000000-0000-0000-0000-000000000000"; + + public static async Task RunAll(string apiUrl, string secret) + { + RouteTestHelpers.PrintTestHeader("Test: JWT auth baseline (AZ-487)"); + + await AnonymousRequest_To_AnyEndpoint_Returns401(apiUrl); + await ExpiredToken_Returns401(apiUrl, secret); + await InvalidSignature_Returns401(apiUrl, secret); + await ValidToken_Returns200_OnHealthyEndpoint(apiUrl, secret); + await SwaggerDocument_AdvertisesBearerSecurityScheme(apiUrl); + + Console.WriteLine("✓ JWT integration tests: PASSED"); + } + + private static async Task AnonymousRequest_To_AnyEndpoint_Returns401(string apiUrl) + { + Console.WriteLine(); + Console.WriteLine("AZ-487 AC-1: Anonymous request to a protected endpoint returns 401"); + + using var anon = new HttpClient { BaseAddress = new Uri(apiUrl), Timeout = TimeSpan.FromMinutes(1) }; + var response = await anon.GetAsync(ProtectedTilesPath); + var status = (int)response.StatusCode; + + if (status != 401) + { + throw new Exception($"Expected 401 for anonymous request, got {status}"); + } + + Console.WriteLine($" ✓ Anonymous request rejected with HTTP {status}"); + } + + private static async Task ExpiredToken_Returns401(string apiUrl, string secret) + { + Console.WriteLine(); + Console.WriteLine("AZ-487 AC-2: Expired token returns 401"); + + using var client = new HttpClient { BaseAddress = new Uri(apiUrl), Timeout = TimeSpan.FromMinutes(1) }; + var expired = JwtTestHelpers.MintExpiredToken(secret); + client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", expired); + + var response = await client.GetAsync(ProtectedTilesPath); + var status = (int)response.StatusCode; + + if (status != 401) + { + throw new Exception($"Expected 401 for expired token, got {status}"); + } + + Console.WriteLine($" ✓ Expired token rejected with HTTP {status}"); + } + + private static async Task InvalidSignature_Returns401(string apiUrl, string secret) + { + Console.WriteLine(); + Console.WriteLine("AZ-487 AC-3: Tampered signature returns 401"); + + using var client = new HttpClient { BaseAddress = new Uri(apiUrl), Timeout = TimeSpan.FromMinutes(1) }; + var valid = JwtTestHelpers.MintValidToken(secret); + var tampered = JwtTestHelpers.TamperSignature(valid); + client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", tampered); + + var response = await client.GetAsync(ProtectedRegionPath); + var status = (int)response.StatusCode; + + if (status != 401) + { + throw new Exception($"Expected 401 for tampered signature, got {status}"); + } + + Console.WriteLine($" ✓ Tampered signature rejected with HTTP {status}"); + } + + private static async Task ValidToken_Returns200_OnHealthyEndpoint(string apiUrl, string secret) + { + Console.WriteLine(); + Console.WriteLine("AZ-487 AC-4: Valid token reaches handler unchanged"); + + using var client = new HttpClient { BaseAddress = new Uri(apiUrl), Timeout = TimeSpan.FromMinutes(2) }; + var valid = JwtTestHelpers.MintValidToken(secret); + client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", valid); + + var response = await client.GetAsync(ProtectedTilesPath); + var status = (int)response.StatusCode; + + // The endpoint may legitimately return 200 (tile available) or a tile-download-related + // error; what we care about for AZ-487 is that the request reached the handler at all + // (i.e. NOT 401 / 403). Treat 200 as confirmation; treat anything other than 401/403 as + // "passed auth" — the handler decided the outcome. + if (status == 401 || status == 403) + { + var body = await response.Content.ReadAsStringAsync(); + throw new Exception($"Expected valid-token request to bypass auth, got {status}. Body: {body}"); + } + + Console.WriteLine($" ✓ Valid-token request reached handler (HTTP {status})"); + } + + private static async Task SwaggerDocument_AdvertisesBearerSecurityScheme(string apiUrl) + { + // AC-7: Swagger UI accepts a Bearer token via the "Authorize" button. The button is + // rendered only when the OpenAPI document declares a `Bearer` security scheme — so the + // existence of that scheme in the document is the automatable signal for AC-7. + Console.WriteLine(); + Console.WriteLine("AZ-487 AC-7: Swagger document advertises Bearer security scheme"); + + using var anon = new HttpClient { BaseAddress = new Uri(apiUrl), Timeout = TimeSpan.FromMinutes(1) }; + var response = await anon.GetAsync("/swagger/v1/swagger.json"); + if (!response.IsSuccessStatusCode) + { + throw new Exception($"Expected Swagger document to be reachable, got HTTP {(int)response.StatusCode}"); + } + + var body = await response.Content.ReadAsStringAsync(); + using var doc = System.Text.Json.JsonDocument.Parse(body); + var root = doc.RootElement; + + if (!root.TryGetProperty("components", out var components) || + !components.TryGetProperty("securitySchemes", out var schemes) || + !schemes.TryGetProperty("Bearer", out var bearer)) + { + throw new Exception("Swagger document is missing `components.securitySchemes.Bearer`."); + } + + var type = bearer.GetProperty("type").GetString(); + var scheme = bearer.GetProperty("scheme").GetString(); + var format = bearer.TryGetProperty("bearerFormat", out var bf) ? bf.GetString() : null; + + if (!string.Equals(type, "http", StringComparison.OrdinalIgnoreCase) || + !string.Equals(scheme, "bearer", StringComparison.OrdinalIgnoreCase) || + !string.Equals(format, "JWT", StringComparison.OrdinalIgnoreCase)) + { + throw new Exception($"Bearer scheme has wrong shape: type={type}, scheme={scheme}, bearerFormat={format}"); + } + + Console.WriteLine($" ✓ Swagger document declares Bearer (http, bearer, JWT)"); + } +} diff --git a/SatelliteProvider.IntegrationTests/JwtTestHelpers.cs b/SatelliteProvider.IntegrationTests/JwtTestHelpers.cs new file mode 100644 index 0000000..c49a7c3 --- /dev/null +++ b/SatelliteProvider.IntegrationTests/JwtTestHelpers.cs @@ -0,0 +1,86 @@ +using System.IdentityModel.Tokens.Jwt; +using System.Net.Http.Headers; +using System.Security.Claims; +using System.Text; +using Microsoft.IdentityModel.Tokens; + +namespace SatelliteProvider.IntegrationTests; + +public static class JwtTestHelpers +{ + public const string JwtSecretEnvVar = "JWT_SECRET"; + public const string DefaultSubject = "integration-tests"; + + public static string ResolveSecretOrThrow() + { + var secret = Environment.GetEnvironmentVariable(JwtSecretEnvVar); + if (string.IsNullOrWhiteSpace(secret)) + { + throw new InvalidOperationException( + $"{JwtSecretEnvVar} is not set in the integration test environment. " + + "It must match the JWT_SECRET configured for the API container."); + } + + var byteLength = Encoding.UTF8.GetByteCount(secret); + if (byteLength < 32) + { + throw new InvalidOperationException( + $"{JwtSecretEnvVar} is {byteLength} bytes; the test runner requires at least 32 bytes to match API validation."); + } + + return secret; + } + + public static string MintValidToken(string secret, string subject = DefaultSubject, TimeSpan? lifetime = null, IEnumerable? extraClaims = null) + { + ArgumentNullException.ThrowIfNull(secret); + + var signingKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(secret)); + var credentials = new SigningCredentials(signingKey, SecurityAlgorithms.HmacSha256); + + var now = DateTime.UtcNow; + var claims = new List + { + new(JwtRegisteredClaimNames.Sub, subject), + new(JwtRegisteredClaimNames.Jti, Guid.NewGuid().ToString("N")) + }; + if (extraClaims is not null) + { + claims.AddRange(extraClaims); + } + + var token = new JwtSecurityToken( + issuer: null, + audience: null, + claims: claims, + notBefore: now, + expires: now.Add(lifetime ?? TimeSpan.FromHours(1)), + signingCredentials: credentials); + + return new JwtSecurityTokenHandler().WriteToken(token); + } + + public static string MintExpiredToken(string secret, string subject = DefaultSubject) + { + return MintValidToken(secret, subject, lifetime: TimeSpan.FromMinutes(-10)); + } + + public static string TamperSignature(string token) + { + var parts = token.Split('.'); + if (parts.Length != 3) + { + throw new ArgumentException("JWT must have three dot-separated segments.", nameof(token)); + } + + var signature = parts[2]; + var firstChar = signature[0]; + parts[2] = (firstChar == 'A' ? 'B' : 'A') + signature[1..]; + return string.Join('.', parts); + } + + public static void AttachDefaultAuthorization(HttpClient httpClient, string token) + { + httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token); + } +} diff --git a/SatelliteProvider.IntegrationTests/Program.cs b/SatelliteProvider.IntegrationTests/Program.cs index 3ce3273..a05570c 100644 --- a/SatelliteProvider.IntegrationTests/Program.cs +++ b/SatelliteProvider.IntegrationTests/Program.cs @@ -17,10 +17,23 @@ class Program TestRunMode.Smoke = modeEnv == "smoke"; } + string jwtSecret; + try + { + jwtSecret = JwtTestHelpers.ResolveSecretOrThrow(); + } + catch (InvalidOperationException ex) + { + Console.WriteLine("❌ Integration tests cannot start without a valid JWT secret."); + Console.WriteLine($" {ex.Message}"); + return 1; + } + Console.WriteLine("Starting Integration Tests"); Console.WriteLine("========================="); Console.WriteLine($"API URL : {apiUrl}"); Console.WriteLine($"Mode : {(TestRunMode.Smoke ? "smoke (fast subset, tightened timeouts)" : "full")}"); + Console.WriteLine($"Auth : JWT_SECRET resolved ({System.Text.Encoding.UTF8.GetByteCount(jwtSecret)} bytes)"); Console.WriteLine(); using var httpClient = new HttpClient @@ -29,6 +42,9 @@ class Program Timeout = TimeSpan.FromMinutes(15) }; + var defaultToken = JwtTestHelpers.MintValidToken(jwtSecret); + JwtTestHelpers.AttachDefaultAuthorization(httpClient, defaultToken); + try { Console.WriteLine("Waiting for API to be ready..."); @@ -36,6 +52,8 @@ class Program Console.WriteLine("✓ API is ready"); Console.WriteLine(); + await JwtIntegrationTests.RunAll(apiUrl, jwtSecret); + if (TestRunMode.Smoke) { await RunSmokeSuite(httpClient); diff --git a/SatelliteProvider.IntegrationTests/SatelliteProvider.IntegrationTests.csproj b/SatelliteProvider.IntegrationTests/SatelliteProvider.IntegrationTests.csproj index 7f966bb..cb549a2 100644 --- a/SatelliteProvider.IntegrationTests/SatelliteProvider.IntegrationTests.csproj +++ b/SatelliteProvider.IntegrationTests/SatelliteProvider.IntegrationTests.csproj @@ -9,6 +9,7 @@ + diff --git a/SatelliteProvider.Tests/Authentication/AuthenticationServiceCollectionExtensionsTests.cs b/SatelliteProvider.Tests/Authentication/AuthenticationServiceCollectionExtensionsTests.cs new file mode 100644 index 0000000..893321b --- /dev/null +++ b/SatelliteProvider.Tests/Authentication/AuthenticationServiceCollectionExtensionsTests.cs @@ -0,0 +1,151 @@ +using System.IdentityModel.Tokens.Jwt; +using FluentAssertions; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.JwtBearer; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Microsoft.IdentityModel.Tokens; +using SatelliteProvider.Api.Authentication; +using SatelliteProvider.Tests.TestUtilities; + +namespace SatelliteProvider.Tests.Authentication; + +public class AuthenticationServiceCollectionExtensionsTests : IDisposable +{ + private const string ValidSecret = "test-secret-that-is-definitely-longer-than-32-bytes"; + private readonly string? _originalEnv; + + public AuthenticationServiceCollectionExtensionsTests() + { + _originalEnv = Environment.GetEnvironmentVariable(AuthenticationServiceCollectionExtensions.JwtSecretEnvVar); + Environment.SetEnvironmentVariable(AuthenticationServiceCollectionExtensions.JwtSecretEnvVar, null); + } + + public void Dispose() + { + Environment.SetEnvironmentVariable(AuthenticationServiceCollectionExtensions.JwtSecretEnvVar, _originalEnv); + GC.SuppressFinalize(this); + } + + [Fact] + public void AddSatelliteJwt_RegistersJwtBearerScheme() + { + // Arrange + var services = new ServiceCollection(); + services.AddLogging(); + var configuration = BuildConfiguration(("Jwt:Secret", ValidSecret)); + + // Act + services.AddSatelliteJwt(configuration); + var provider = services.BuildServiceProvider(); + var schemeProvider = provider.GetRequiredService(); + var scheme = schemeProvider.GetSchemeAsync(JwtBearerDefaults.AuthenticationScheme).GetAwaiter().GetResult(); + + // Assert + scheme.Should().NotBeNull("JwtBearer scheme should be registered"); + scheme!.HandlerType.Should().Be(typeof(JwtBearerHandler)); + } + + [Fact] + public void AddSatelliteJwt_ConfiguresTokenValidationParameters_AsPerContract() + { + // Arrange + var services = new ServiceCollection(); + services.AddLogging(); + var configuration = BuildConfiguration(("Jwt:Secret", ValidSecret)); + + // Act + services.AddSatelliteJwt(configuration); + var provider = services.BuildServiceProvider(); + var options = provider.GetRequiredService>().Get(JwtBearerDefaults.AuthenticationScheme); + + // Assert + var p = options.TokenValidationParameters; + p.ValidateIssuerSigningKey.Should().BeTrue(); + p.ValidateLifetime.Should().BeTrue(); + p.ValidateIssuer.Should().BeFalse(); + p.ValidateAudience.Should().BeFalse(); + p.RequireSignedTokens.Should().BeTrue(); + p.RequireExpirationTime.Should().BeTrue(); + p.ClockSkew.Should().Be(TimeSpan.FromSeconds(30)); + p.IssuerSigningKey.Should().BeOfType(); + } + + [Fact] + public void AddSatelliteJwt_ThrowsOnMissingSecret() + { + // Arrange + var services = new ServiceCollection(); + var configuration = BuildConfiguration(); + + // Act + var act = () => services.AddSatelliteJwt(configuration); + + // Assert + act.Should().Throw() + .WithMessage("*JWT secret is not configured*"); + } + + [Fact] + public void AddSatelliteJwt_ThrowsOnEmptySecret() + { + // Arrange + var services = new ServiceCollection(); + var configuration = BuildConfiguration(("Jwt:Secret", "")); + + // Act + var act = () => services.AddSatelliteJwt(configuration); + + // Assert + act.Should().Throw() + .WithMessage("*JWT secret is not configured*"); + } + + [Fact] + public void AddSatelliteJwt_ThrowsOnShortSecret() + { + // Arrange + var services = new ServiceCollection(); + var configuration = BuildConfiguration(("Jwt:Secret", "too-short-secret")); + + // Act + var act = () => services.AddSatelliteJwt(configuration); + + // Assert + act.Should().Throw() + .WithMessage("*at least 32 bytes*"); + } + + [Fact] + public void AddSatelliteJwt_PrefersEnvironmentVariableOverConfiguration() + { + // Arrange + const string envSecret = "env-secret-also-longer-than-thirty-two-bytes-for-hmac"; + Environment.SetEnvironmentVariable(AuthenticationServiceCollectionExtensions.JwtSecretEnvVar, envSecret); + var services = new ServiceCollection(); + services.AddLogging(); + var configuration = BuildConfiguration(("Jwt:Secret", "config-secret-also-32-bytes-long-aaaaaaaaaa")); + + // Act + services.AddSatelliteJwt(configuration); + var provider = services.BuildServiceProvider(); + var options = provider.GetRequiredService>().Get(JwtBearerDefaults.AuthenticationScheme); + var token = JwtTokenFactory.Create(envSecret); + var handler = new JwtSecurityTokenHandler(); + var act = () => handler.ValidateToken(token, options.TokenValidationParameters, out _); + + // Assert + act.Should().NotThrow("token signed with env secret must validate when env secret takes precedence"); + } + + private static IConfiguration BuildConfiguration(params (string Key, string Value)[] pairs) + { + var builder = new ConfigurationBuilder(); + if (pairs.Length > 0) + { + builder.AddInMemoryCollection(pairs.Select(p => new KeyValuePair(p.Key, p.Value))); + } + return builder.Build(); + } +} diff --git a/SatelliteProvider.Tests/Authentication/JwtTokenFactoryTests.cs b/SatelliteProvider.Tests/Authentication/JwtTokenFactoryTests.cs new file mode 100644 index 0000000..591acfc --- /dev/null +++ b/SatelliteProvider.Tests/Authentication/JwtTokenFactoryTests.cs @@ -0,0 +1,93 @@ +using System.IdentityModel.Tokens.Jwt; +using System.Security.Claims; +using System.Text; +using FluentAssertions; +using Microsoft.IdentityModel.Tokens; +using SatelliteProvider.Tests.TestUtilities; + +namespace SatelliteProvider.Tests.Authentication; + +public class JwtTokenFactoryTests +{ + private const string Secret = "factory-secret-that-is-longer-than-thirty-two-bytes-bytes"; + + [Fact] + public void Create_ProducesTokenValidatedByMatchingParameters() + { + // Arrange + var token = JwtTokenFactory.Create(Secret, subject: "alice"); + var parameters = BuildParameters(Secret); + var handler = new JwtSecurityTokenHandler(); + + // Act + var principal = handler.ValidateToken(token, parameters, out var validatedToken); + + // Assert + principal.Identity!.IsAuthenticated.Should().BeTrue(); + principal.FindFirst(JwtRegisteredClaimNames.Sub)!.Value.Should().Be("alice"); + validatedToken.Should().BeOfType(); + } + + [Fact] + public void Create_WithExtraClaims_PropagatesClaimsThroughValidation() + { + // Arrange + var claims = new[] + { + new Claim("email", "alice@example.com"), + new Claim("role", "operator"), + new Claim("permissions", "GPS"), + new Claim("permissions", "FL") + }; + var token = JwtTokenFactory.Create(Secret, extraClaims: claims); + var handler = new JwtSecurityTokenHandler(); + + // Act + var principal = handler.ValidateToken(token, BuildParameters(Secret), out _); + + // Assert + principal.FindAll("permissions").Select(c => c.Value).Should().BeEquivalentTo(new[] { "GPS", "FL" }); + principal.FindFirst("email")!.Value.Should().Be("alice@example.com"); + } + + [Fact] + public void CreateExpired_TokenFailsValidationWithLifetimeException() + { + // Arrange + var token = JwtTokenFactory.CreateExpired(Secret); + var handler = new JwtSecurityTokenHandler(); + + // Act + var act = () => handler.ValidateToken(token, BuildParameters(Secret), out _); + + // Assert + act.Should().Throw(); + } + + [Fact] + public void TamperSignature_TokenFailsValidationWithSignatureException() + { + // Arrange + var token = JwtTokenFactory.Create(Secret); + var tampered = JwtTokenFactory.TamperSignature(token); + var handler = new JwtSecurityTokenHandler(); + + // Act + var act = () => handler.ValidateToken(tampered, BuildParameters(Secret), out _); + + // Assert + act.Should().Throw(); + } + + private static TokenValidationParameters BuildParameters(string secret) => new() + { + ValidateIssuerSigningKey = true, + IssuerSigningKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(secret)), + ValidateLifetime = true, + ClockSkew = TimeSpan.FromSeconds(30), + ValidateIssuer = false, + ValidateAudience = false, + RequireSignedTokens = true, + RequireExpirationTime = true + }; +} diff --git a/SatelliteProvider.Tests/TestUtilities/JwtTokenFactory.cs b/SatelliteProvider.Tests/TestUtilities/JwtTokenFactory.cs new file mode 100644 index 0000000..668f5e1 --- /dev/null +++ b/SatelliteProvider.Tests/TestUtilities/JwtTokenFactory.cs @@ -0,0 +1,76 @@ +using System.IdentityModel.Tokens.Jwt; +using System.Security.Claims; +using System.Text; +using Microsoft.IdentityModel.Tokens; + +namespace SatelliteProvider.Tests.TestUtilities; + +public static class JwtTokenFactory +{ + public const string DefaultSubject = "test-user"; + + public static string Create( + string secret, + string subject = DefaultSubject, + TimeSpan? lifetime = null, + IEnumerable? extraClaims = null, + string algorithm = SecurityAlgorithms.HmacSha256) + { + ArgumentNullException.ThrowIfNull(secret); + + var keyBytes = Encoding.UTF8.GetBytes(secret); + var signingKey = new SymmetricSecurityKey(keyBytes); + var credentials = new SigningCredentials(signingKey, algorithm); + + var now = DateTime.UtcNow; + var expires = now.Add(lifetime ?? TimeSpan.FromHours(1)); + + var claims = new List + { + new(JwtRegisteredClaimNames.Sub, subject), + new(JwtRegisteredClaimNames.Jti, Guid.NewGuid().ToString("N")) + }; + + if (extraClaims is not null) + { + claims.AddRange(extraClaims); + } + + var token = new JwtSecurityToken( + issuer: null, + audience: null, + claims: claims, + notBefore: now, + expires: expires, + signingCredentials: credentials); + + return new JwtSecurityTokenHandler().WriteToken(token); + } + + public static string CreateExpired(string secret, string subject = DefaultSubject) + { + return Create(secret, subject, lifetime: TimeSpan.FromMinutes(-5)); + } + + public static string TamperSignature(string token) + { + ArgumentException.ThrowIfNullOrEmpty(token); + + var parts = token.Split('.'); + if (parts.Length != 3) + { + throw new ArgumentException("JWT must have three dot-separated parts.", nameof(token)); + } + + var signature = parts[2]; + if (signature.Length == 0) + { + throw new ArgumentException("JWT signature segment is empty.", nameof(token)); + } + + var firstChar = signature[0]; + var replacement = firstChar == 'A' ? 'B' : 'A'; + parts[2] = replacement + signature[1..]; + return string.Join('.', parts); + } +} diff --git a/_docs/02_document/architecture.md b/_docs/02_document/architecture.md index 4f1a006..e34b5cd 100644 --- a/_docs/02_document/architecture.md +++ b/_docs/02_document/architecture.md @@ -22,7 +22,14 @@ The three Layer-3 service components are compile-time siblings: each only refere - Single-instance deployment, no horizontal scaling requirements (`inferred-from: Channel-based queue, no distributed state`) - Append-by-source tile storage — multiple producers (Google Maps, UAV upload, future SatAR, …) can each persist a row per `(latitude, longitude, tile_zoom, tile_size_meters)` cell. Reads return the most-recent row across sources, ordered by `captured_at DESC` with deterministic `(updated_at DESC, id DESC)` tie-breaks. The single-row-per-cell-per-source invariant is enforced by the 5-column unique index `idx_tiles_unique_location_source` introduced in migration 013 (AZ-484). The `tiles.version` column is vestigial since AZ-357 dropped year-based cache invalidation in favour of cell-level overwrite. (`inferred-from: tiles table + AZ-484/AZ-357 migrations + tile-storage contract v1.0.0`) - Fire-and-forget async processing with status polling (`inferred-from: queue + background service + status endpoint`) -- No authentication layer — designed as an internal/trusted network service (`inferred-from: no auth middleware in Program.cs`) +- JWT-validated callers only — every HTTP endpoint requires a valid HS256-signed Bearer token, validated locally against a shared `JWT_SECRET` per the suite-level auth contract (`suite/_docs/10_auth.md`). Issuer/audience are intentionally not validated yet; signature + lifetime + ≥32-byte key are. Per-endpoint permission claims (e.g. `permissions: ["GPS"]` on the UAV upload) layer on top of this baseline. + +**Authentication & Authorization** (AZ-487): +- Validation library: `Microsoft.AspNetCore.Authentication.JwtBearer` 8.0.21 (matches the rest of the ASP.NET Core 8 package set). +- Signing key: read from the `JWT_SECRET` environment variable (preferred) or the `Jwt:Secret` configuration key. Startup fails fast if the resolved secret is unset, empty, or shorter than 32 bytes (HMAC-SHA256 minimum per RFC 2104 §3). +- Token contract: `ValidateIssuerSigningKey = true`, `ValidateLifetime = true`, `RequireSignedTokens = true`, `RequireExpirationTime = true`, `ValidateIssuer/Audience = false`, `ClockSkew = 30s`. The 5-minute JwtBearer default is intentionally tightened. +- Authorization model: every endpoint registered in `Program.cs` is decorated with `.RequireAuthorization()`. AZ-488 adds `permissions`-claim policies on top of this baseline (UAV upload requires `GPS`). +- Test infrastructure: `JwtTokenFactory` (unit tests) and `JwtTestHelpers` (integration tests) mint deterministic tokens against the same `JWT_SECRET`; the integration test runner attaches a default Bearer token to its shared `HttpClient` so legacy non-auth tests continue to exercise the protected endpoints unchanged. **Planned features** (confirmed by user, currently stubs): - MGRS endpoint — tile access via Military Grid Reference System coordinates @@ -132,16 +139,16 @@ The N-source storage contract is authoritative in `_docs/02_document/contracts/d ## 7. Security Architecture -**Authentication**: None (internal service, no auth layer) +**Authentication**: HS256 JWT Bearer tokens (AZ-487). Signing key from `JWT_SECRET` env var (≥ 32 bytes, validated at startup). `Microsoft.AspNetCore.Authentication.JwtBearer` validates signature, lifetime, and signing key; issuer and audience are intentionally not validated (suite contract does not specify expected values). ClockSkew tightened from JwtBearer default (5 min) to 30 s. Tokens are minted by the centralized Admin API per `suite/_docs/10_auth.md`. -**Authorization**: None (all endpoints are open) +**Authorization**: Every endpoint requires authentication via `.RequireAuthorization()`. Permission-claim enforcement (e.g. `permissions: ["GPS"]`) is added per-endpoint where needed — AZ-488 introduces it on `POST /api/satellite/upload`. Other endpoints accept any authenticated principal. **Data protection**: - At rest: No encryption (tiles stored as plain JPEG files) -- In transit: HTTPS for Google Maps calls; API itself on HTTP -- Secrets management: Google Maps session token in appsettings / env vars +- In transit: HTTPS for Google Maps calls; API itself runs HTTP behind Kestrel (TLS termination is a deployment-layer concern) +- Secrets management: `JWT_SECRET` and `GOOGLE_MAPS_API_KEY` from environment variables / `.env` (gitignored); `.env.example` documents the required keys. Production deployments MUST supply both via the host environment, never via the appsettings files. -**Audit logging**: Serilog writes to file; logs exceptions and processing state transitions +**Audit logging**: Serilog writes to file; logs exceptions and processing state transitions. 401/403 responses are emitted by the JwtBearer middleware via the `WWW-Authenticate` header; no body leakage of internal details. ## 8. Key Architectural Decisions diff --git a/_docs/02_document/modules/api_program.md b/_docs/02_document/modules/api_program.md index 921a12e..9eacf25 100644 --- a/_docs/02_document/modules/api_program.md +++ b/_docs/02_document/modules/api_program.md @@ -36,12 +36,14 @@ Application entry point. Configures DI container, sets up middleware, defines mi 6. Hosted services: `RegionProcessingService`, `RouteProcessingService` 7. CORS policy: `TilesCors` — configured origins from `CorsConfig:AllowedOrigins`, falls back to allow-any 8. JSON options: camelCase, case-insensitive +9. **JWT authentication (AZ-487)**: `AddSatelliteJwt(builder.Configuration)` (extension in `SatelliteProvider.Api.Authentication`) registers `JwtBearer` with `TokenValidationParameters` set per the suite auth contract (signature + lifetime, no issuer/audience validation, 30 s clock skew, ≥ 32-byte HMAC key). Followed by `AddAuthorization()`. ### Startup 1. Database migration via `DatabaseMigrator.RunMigrations()` — throws on failure 2. Creates tiles and ready directories 3. Swagger enabled in Development mode -4. HTTPS redirection, CORS applied +4. Middleware chain (order matters): `UseExceptionHandler` → `UseHttpsRedirection` → `UseCors("TilesCors")` → `UseAuthentication` → `UseAuthorization` → endpoint mapping. +5. Every `MapGet`/`MapPost` endpoint is decorated with `.RequireAuthorization()`; the framework returns 401 before the handler runs for any anonymous, expired, or invalid-signature request. ### ServeTile Handler 1. Checks `IMemoryCache` for tile bytes (1h absolute, 30min sliding expiration) @@ -57,7 +59,7 @@ Validates size (100–10000m), delegates to `IRegionService.RequestRegionAsync`. ## Dependencies All project references: Common, DataAccess, Services. -NuGet: `Serilog.AspNetCore`, `Swashbuckle.AspNetCore`, `Microsoft.AspNetCore.OpenApi`, `SixLabors.ImageSharp`, `Newtonsoft.Json`. +NuGet: `Serilog.AspNetCore`, `Swashbuckle.AspNetCore`, `Microsoft.AspNetCore.OpenApi`, `Microsoft.AspNetCore.Authentication.JwtBearer` (8.0.21, AZ-487), `SixLabors.ImageSharp`, `Newtonsoft.Json`. ## Consumers - HTTP clients (external) @@ -71,6 +73,7 @@ All configuration sections are consumed here: - `ConnectionStrings:DefaultConnection` - `MapConfig`, `StorageConfig`, `ProcessingConfig` - `CorsConfig:AllowedOrigins` +- `Jwt:Secret` — HMAC-SHA256 signing key for JWT validation (AZ-487). Resolution: `JWT_SECRET` env var (preferred, opaque production secret) → `Jwt:Secret` configuration key (`appsettings.Development.json` placeholder only). Startup fails fast if the resolved value is unset, empty, or shorter than 32 bytes. - `Serilog` section ## External Integrations @@ -80,9 +83,9 @@ All configuration sections are consumed here: ## Security - CORS configured (permissive by default when no origins specified) -- Swagger only in Development +- Swagger only in Development; Bearer token "Authorize" button registered via `AddSecurityDefinition`/`AddSecurityRequirement` (AZ-487) - HTTPS redirection enabled -- No authentication/authorization implemented +- JWT bearer authentication (AZ-487) — every endpoint requires a valid HS256-signed token. Anonymous, expired, or signature-tampered requests return 401 before the handler runs. Per-endpoint permission claims are layered on top in subsequent PBIs (e.g. AZ-488 requires `permissions: ["GPS"]` on the upload endpoint). ## Tests Integration tests exercise all endpoints. Unit test project has only a dummy test. diff --git a/_docs/02_tasks/_dependencies_table.md b/_docs/02_tasks/_dependencies_table.md index 3344985..01413a5 100644 --- a/_docs/02_tasks/_dependencies_table.md +++ b/_docs/02_tasks/_dependencies_table.md @@ -68,7 +68,7 @@ Roadmap: `_docs/04_refactoring/03-code-quality-refactoring/analysis/refactoring_ | Task | Title | Depends On | Points | Status | |------|-------|-----------|--------|--------| -| AZ-487 | JWT validation baseline (HS256, JWT_SECRET, all endpoints) | — (consumes suite-level contract `suite/_docs/10_auth.md`) | 2 | To Do | +| AZ-487 | JWT validation baseline (HS256, JWT_SECRET, all endpoints) | — (consumes suite-level contract `suite/_docs/10_auth.md`) | 2 | Done (In Testing) | | AZ-488 | UAV tile upload endpoint with batch + 5-rule quality gate | AZ-487 (hard prereq), AZ-484 contract `tile-storage.md` v1.0.0 | 8 (over-cap, user-accepted) | To Do | ## Execution Order diff --git a/_docs/02_tasks/todo/AZ-487_jwt_validation_baseline.md b/_docs/02_tasks/done/AZ-487_jwt_validation_baseline.md similarity index 100% rename from _docs/02_tasks/todo/AZ-487_jwt_validation_baseline.md rename to _docs/02_tasks/done/AZ-487_jwt_validation_baseline.md diff --git a/_docs/03_implementation/batch_01_cycle2_report.md b/_docs/03_implementation/batch_01_cycle2_report.md new file mode 100644 index 0000000..4313f52 --- /dev/null +++ b/_docs/03_implementation/batch_01_cycle2_report.md @@ -0,0 +1,48 @@ +# Batch Report — Batch 01 cycle 2 + +**Batch**: 01 (cycle 2) +**Tasks**: AZ-487 (JWT validation baseline) +**Date**: 2026-05-11 + +## Task Results + +| Task | Status | Files Modified | Tests | AC Coverage | Issues | +|------|--------|---------------|-------|-------------|--------| +| AZ-487_jwt_validation_baseline | Done | 12 modified + 6 added (`.env.example`, `SatelliteProvider.Api/Authentication/AuthenticationServiceCollectionExtensions.cs`, `SatelliteProvider.IntegrationTests/JwtIntegrationTests.cs`, `JwtTestHelpers.cs`, `SatelliteProvider.Tests/Authentication/AuthenticationServiceCollectionExtensionsTests.cs`, `JwtTokenFactoryTests.cs`, `SatelliteProvider.Tests/TestUtilities/JwtTokenFactory.cs`) | To run (Step 11) | 8/8 ACs covered | 0 blockers; 2 Low findings (see review) | + +## AC Test Coverage: All covered (8 of 8) +## Code Review Verdict: PASS_WITH_WARNINGS +## Auto-Fix Attempts: 0 +## Stuck Agents: None + +## What was implemented + +- New `AuthenticationServiceCollectionExtensions.AddSatelliteJwt(IConfiguration)` — registers `Microsoft.AspNetCore.Authentication.JwtBearer 8.0.21` with `TokenValidationParameters` matching the suite-level auth contract (HS256, `ValidateLifetime`, `RequireSignedTokens`, `RequireExpirationTime`, no issuer/audience validation, 30 s clock skew). Secret resolution prefers `JWT_SECRET` env var, falls back to `Jwt:Secret` configuration, fails fast on missing/short (<32-byte) secret. +- `Program.cs` wires `AddSatelliteJwt` + `AddAuthorization` into DI, adds `UseAuthentication` + `UseAuthorization` middleware after `UseCors`, and decorates every minimal-API endpoint with `.RequireAuthorization()`. Swagger UI gets a Bearer security definition + global security requirement so the "Authorize" button renders. +- Configuration: `appsettings.json` adds an empty `Jwt:Secret` placeholder; `appsettings.Development.json` ships a clearly-tagged dev placeholder (≥32 bytes). The `JWT_SECRET` env var overrides both when set. +- Test infrastructure: + - `SatelliteProvider.Tests.TestUtilities.JwtTokenFactory` — mints, expires, and tampers tokens using the same secret as the API. + - `SatelliteProvider.Tests.Authentication.AuthenticationServiceCollectionExtensionsTests` — 6 tests covering registration, parameter shape, missing/empty/short secret handling, and env-var-precedence semantics. + - `SatelliteProvider.Tests.Authentication.JwtTokenFactoryTests` — 4 tests covering token mint + claims propagation + expired + tampered. + - `SatelliteProvider.IntegrationTests.JwtTestHelpers` — runner-side helpers for token mint + bearer attachment. + - `SatelliteProvider.IntegrationTests.JwtIntegrationTests` — 5 end-to-end checks (anon → 401, expired → 401, tampered → 401, valid → handler, Swagger advertises Bearer). + - `SatelliteProvider.IntegrationTests.Program` — resolves `JWT_SECRET` at startup, mints a default token, attaches it to the shared `HttpClient` so legacy non-auth tests continue to pass against protected endpoints. +- Docker / scripts: + - `docker-compose.yml`: api gets `JWT_SECRET=${JWT_SECRET}`. + - `docker-compose.tests.yml`: integration-tests gets the same env var (api inherits via `extends`). + - `scripts/run-tests.sh`: loads `JWT_SECRET` from `.env` (alongside `GOOGLE_MAPS_API_KEY`), rejects missing/short values, exports for compose runs. + - `.env.example` (new): documents `GOOGLE_MAPS_API_KEY` and `JWT_SECRET` with generation guidance. + - `.env` (gitignored): dev `JWT_SECRET` added locally. +- Docs: + - `architecture.md` § Architecture Vision: added Authentication & Authorization sub-section. + - `architecture.md` § Security Architecture: replaced "no auth" prose with JWT description; documented placeholder semantics and operator obligations. + - `modules/api_program.md`: updated DI Registration (#9), Startup (middleware chain), Dependencies (JwtBearer 8.0.21), Configuration (`Jwt:Secret` + env var precedence), Security (per-endpoint `.RequireAuthorization()`). + +## Open follow-ups (non-blocking) + +- **Doc-folder choice** (F1 in review): the task spec referenced `_docs/02_document/components/01_web_api/description.md`, which does not exist. Updated `modules/api_program.md` + `architecture.md` instead. User should decide whether to add a stub `01_web_api` component folder for symmetry, or to formalize the "WebApi has no `components/*` folder" pattern. +- **Cross-component coordination**: per the AZ-487 spec, `gps-denied-onboard` and the mission planner UI must attach Bearer tokens to their outbound calls before this change ships to `dev`. Flagged for the operator to coordinate before deploy (Step 16). + +## Next Batch: AZ-488 (UAV tile upload endpoint with batch + 5-rule quality gate) + +AZ-488 is an 8 SP task (user-accepted over-cap). It introduces a new endpoint, DTOs, quality gate, configuration class, and ~15 unit + integration tests. Recommend a fresh conversation for context freshness before starting (per `protocols.md` Context Budget Heuristic). diff --git a/_docs/03_implementation/reviews/batch_01_cycle2_review.md b/_docs/03_implementation/reviews/batch_01_cycle2_review.md new file mode 100644 index 0000000..43a837b --- /dev/null +++ b/_docs/03_implementation/reviews/batch_01_cycle2_review.md @@ -0,0 +1,81 @@ +# Code Review Report — Batch 01 cycle 2 + +**Batch**: AZ-487 (JWT validation baseline) +**Date**: 2026-05-11 +**Verdict**: PASS_WITH_WARNINGS + +## Findings + +| # | Severity | Category | File:Line | Title | +|---|----------|----------|-----------|-------| +| 1 | Low | Style | _docs/02_document/components/01_web_api/description.md | Task spec referenced a doc path that does not exist in the codebase | +| 2 | Low | Security | SatelliteProvider.Api/appsettings.Development.json | Dev-only JWT secret placeholder is committed (intentional per spec) | + +### Finding Details + +**F1: Task spec referenced a doc path that does not exist in the codebase** (Low / Style) +- Location: `_docs/02_document/components/01_web_api/description.md` (referenced; does not exist) +- Description: The AZ-487 task spec lists `_docs/02_document/components/01_web_api/description.md` as a doc to update. The codebase's component-doc folders are `01_common`, `02_data_access`, `03_tile_downloader`, `04_region_processing`, `05_route_management` — there is no `01_web_api` folder. The WebApi component's documentation lives in `_docs/02_document/modules/api_program.md`. +- Suggestion: Either (a) create the missing folder with a brief stub that defers to `api_program.md`, or (b) update the task spec for AZ-488 to point at `modules/api_program.md` and acknowledge that WebApi has no `components/*` folder. This batch chose (b) — updated `architecture.md` § Architecture Vision + § Security Architecture and `modules/api_program.md`. Surface to user for a doc-organization decision. +- Task: AZ-487 + +**F2: Dev-only JWT secret placeholder is committed** (Low / Security) +- Location: `SatelliteProvider.Api/appsettings.Development.json` +- Description: The dev placeholder `DEV-ONLY-DO-NOT-USE-IN-PROD-replace-with-real-secret-via-JWT_SECRET-env-var` (78 bytes) is committed to the repo. Anyone with read access could mint a token signed with that secret. The mitigations (per task spec) are: (i) only loaded when `ASPNETCORE_ENVIRONMENT=Development`, (ii) the `JWT_SECRET` env var overrides it whenever set, (iii) the placeholder text itself signals it must be replaced. +- Suggestion: Accept as-is for dev ergonomics; production environments must set `JWT_SECRET` (validated in `scripts/run-tests.sh` and at startup). Document on the README that this dev placeholder is intentional. +- Task: AZ-487 + +## Phase Notes + +### Phase 1 — Context Loading +- Task spec read: `_docs/02_tasks/todo/AZ-487_jwt_validation_baseline.md` +- Suite contract referenced: `suite/_docs/10_auth.md` (consumed via implementation, not modified) +- Module layout, architecture doc, existing test patterns reviewed. + +### Phase 2 — Spec Compliance +All 8 acceptance criteria have at least one automated test: + +| AC | Description | Test | +|----|-------------|------| +| AC-1 | Anonymous → 401 | `JwtIntegrationTests.AnonymousRequest_To_AnyEndpoint_Returns401` | +| AC-2 | Expired → 401 | `JwtIntegrationTests.ExpiredToken_Returns401` | +| AC-3 | Invalid signature → 401 | `JwtIntegrationTests.InvalidSignature_Returns401` | +| AC-4 | Valid token reaches handler | `JwtIntegrationTests.ValidToken_Returns200_OnHealthyEndpoint` | +| AC-5 | Missing/short secret → fail fast | `AuthenticationServiceCollectionExtensionsTests.AddSatelliteJwt_ThrowsOnMissingSecret/Empty/Short` | +| AC-6 | `HttpContext.User` exposes claims | `JwtTokenFactoryTests.Create_WithExtraClaims_PropagatesClaimsThroughValidation` (exercises same `JwtSecurityTokenHandler.ValidateToken` path JwtBearer uses to populate `HttpContext.User`) | +| AC-7 | Swagger Authorize button | `JwtIntegrationTests.SwaggerDocument_AdvertisesBearerSecurityScheme` (OpenAPI document declares `Bearer` security scheme) | +| AC-8 | All existing tests pass | Verified at Step 11 (test-run gate); shared `HttpClient` attaches default Bearer token in `IntegrationTests/Program.cs` | + +Contract verification: AZ-487 is a consumer of `suite/_docs/10_auth.md`. The implementation matches the consumed contract (HS256, ≥32-byte HMAC key, no issuer/audience validation, expiration required). + +### Phase 3 — Code Quality +- New code follows SRP (`AuthenticationServiceCollectionExtensions` does one thing). +- Tests follow AAA pattern with explicit comments (matches project style). +- No bare catches introduced. The pre-existing empty catch in `IntegrationTests/Program.cs::WaitForApiReady` is out of scope (pre-existing, not modified). +- No comments narrating obvious code. + +### Phase 4 — Security Quick-Scan +- No SQL/command-injection vectors introduced. +- No sensitive data in logs (error messages mention secret length, not value). +- Dev placeholder secret: see F2. + +### Phase 5 — Performance Scan +- JWT validation per request is microsecond-scale HMAC + claims parsing; no I/O, no caching needed (per NFR). +- No N+1, no unbounded fetches, no new blocking calls. + +### Phase 6 — Cross-Task Consistency +Single-task batch; not applicable. + +### Phase 7 — Architecture Compliance +- All new code in `SatelliteProvider.Api/Authentication/` — owned by the WebApi component per `module-layout.md`. +- Imports stay within the allowed dependency table (`Microsoft.AspNetCore.Authentication.JwtBearer`, `Microsoft.IdentityModel.Tokens` are external NuGet packages, not other components). +- Test code lives under `SatelliteProvider.Tests/` and `SatelliteProvider.IntegrationTests/` (separate test projects per layout rule 5). +- No new cross-sibling ProjectReferences. No new cyclic dependencies. No duplicate cross-component symbols. + +## Baseline Delta + +`_docs/02_document/architecture_compliance_baseline.md` is not present in the repo; baseline delta section omitted. + +## Verdict Rationale + +Two Low findings, both already accepted in the task spec or surfacable as doc-organization decisions. No Critical, High, or Medium findings. Verdict: **PASS_WITH_WARNINGS** — proceed to commit per implement skill Step 10. diff --git a/_docs/_autodev_state.md b/_docs/_autodev_state.md index 321a32a..bbb09c5 100644 --- a/_docs/_autodev_state.md +++ b/_docs/_autodev_state.md @@ -6,9 +6,9 @@ step: 10 name: Implement status: in_progress sub_step: - phase: 0 - name: awaiting-invocation - detail: "" + phase: 7 + name: batch-loop + detail: "batch 1 of 2 done (AZ-487); batch 2 (AZ-488) pending" retry_count: 0 cycle: 2 tracker: jira diff --git a/docker-compose.tests.yml b/docker-compose.tests.yml index b1a0987..7b54e09 100644 --- a/docker-compose.tests.yml +++ b/docker-compose.tests.yml @@ -18,6 +18,7 @@ services: - API_URL=http://api:8080 - INTEGRATION_TESTS_MODE=${INTEGRATION_TESTS_MODE:-full} - DB_CONNECTION_STRING=Host=postgres;Port=5432;Database=satelliteprovider;Username=postgres;Password=postgres + - JWT_SECRET=${JWT_SECRET} volumes: - ./ready:/app/ready - ./tiles:/app/tiles diff --git a/docker-compose.yml b/docker-compose.yml index 00de48f..d81c45a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -29,6 +29,7 @@ services: - ASPNETCORE_URLS=http://+:8080 - ConnectionStrings__DefaultConnection=Host=postgres;Port=5432;Database=satelliteprovider;Username=postgres;Password=postgres - MapConfig__ApiKey=${GOOGLE_MAPS_API_KEY} + - JWT_SECRET=${JWT_SECRET} volumes: - ./tiles:/app/tiles - ./ready:/app/ready diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index 7171e41..1ce2fcc 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -23,6 +23,9 @@ Flags: Environment: GOOGLE_MAPS_API_KEY Required for any integration test mode (loaded from .env or shell env). + JWT_SECRET Required for any integration test mode. Shared HMAC secret used by the + API and the integration test runner; must be at least 32 bytes (UTF-8). + Loaded from .env or shell env. EOF } @@ -66,7 +69,7 @@ if [[ "$mode" == "unit" ]]; then exit 0 fi -if [[ -z "${GOOGLE_MAPS_API_KEY:-}" ]] && [[ -f "$PROJECT_ROOT/.env" ]]; then +if { [[ -z "${GOOGLE_MAPS_API_KEY:-}" ]] || [[ -z "${JWT_SECRET:-}" ]]; } && [[ -f "$PROJECT_ROOT/.env" ]]; then set -o allexport # shellcheck disable=SC1091 source "$PROJECT_ROOT/.env" @@ -78,6 +81,18 @@ if [[ -z "${GOOGLE_MAPS_API_KEY:-}" ]]; then exit 3 fi +if [[ -z "${JWT_SECRET:-}" ]]; then + echo "ERROR: JWT_SECRET is not set (export it or add to .env). API will fail to start without it." + exit 3 +fi + +jwt_secret_bytes=${#JWT_SECRET} +if (( jwt_secret_bytes < 32 )); then + echo "ERROR: JWT_SECRET is ${jwt_secret_bytes} bytes; HMAC-SHA256 requires at least 32 bytes." + exit 3 +fi +export JWT_SECRET + echo "Step 1: Unit tests" docker run --rm -v "$PROJECT_ROOT:/src" -w /src mcr.microsoft.com/dotnet/sdk:8.0 \ sh -c "dotnet restore SatelliteProvider.sln && dotnet test SatelliteProvider.Tests/SatelliteProvider.Tests.csproj --no-restore --configuration Release --collect:'XPlat Code Coverage' --results-directory /src/TestResults --logger 'console;verbosity=normal'"