diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index ea3c6e5..413236e 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -36,7 +36,7 @@ jobs: uses: github/codeql-action/init@v4 with: languages: ${{ matrix.language }} - queries: +security-and-quality + queries: security-extended - name: Setup .NET uses: actions/setup-dotnet@v5 diff --git a/Directory.Packages.props b/Directory.Packages.props index 22c2327..b31244e 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -73,6 +73,8 @@ + + diff --git a/src/Demo.JsonApi/Program.cs b/src/Demo.JsonApi/Program.cs index 9fc708d..0726188 100644 --- a/src/Demo.JsonApi/Program.cs +++ b/src/Demo.JsonApi/Program.cs @@ -120,7 +120,7 @@ if (!string.IsNullOrWhiteSpace(statusUpdate.Notes)) { logger.LogInformation("Order {OrderId} status updated to {Status}. Notes: {Notes}", - id, statusUpdate.Status, statusUpdate.Notes); + id, statusUpdate.Status, SanitizeForLog(statusUpdate.Notes)); } return Results.Ok(order); @@ -153,3 +153,10 @@ app.MapDefaultEndpoints(); app.Run(); + +/// +/// Removes CR/LF from user-supplied strings before logging to prevent log-forging. +/// +static string SanitizeForLog(string? value) => + value?.Replace("\r", "\\r", StringComparison.Ordinal) + .Replace("\n", "\\n", StringComparison.Ordinal) ?? string.Empty; diff --git a/src/Demo.JsonApi/Services/InMemoryOrderService.cs b/src/Demo.JsonApi/Services/InMemoryOrderService.cs index 716221e..4478759 100644 --- a/src/Demo.JsonApi/Services/InMemoryOrderService.cs +++ b/src/Demo.JsonApi/Services/InMemoryOrderService.cs @@ -32,7 +32,7 @@ public Task CreateOrderAsync(Order order, CancellationToken cancellationT throw new InvalidOperationException($"Order with ID {order.OrderId} already exists"); } - _logger.LogInformation("Created new order: {OrderId}", order.OrderId); + _logger.LogInformation("Created new order: {OrderId}", SanitizeForLog(order.OrderId)); return Task.FromResult(order); } @@ -55,7 +55,7 @@ public Task> GetAllOrdersAsync(CancellationToken cancellation } order.Status = status; - _logger.LogInformation("Updated order {OrderId} status to {Status}", orderId, status); + _logger.LogInformation("Updated order {OrderId} status to {Status}", SanitizeForLog(orderId), status); return Task.FromResult(order); } @@ -330,4 +330,11 @@ private static string GenerateOrderId() var random = Random.Shared.Next(1000, 9999); return $"ORD-{timestamp:yyyy}-{random}"; } + + /// + /// Removes CR/LF from user-supplied strings before logging to prevent log-forging. + /// + private static string SanitizeForLog(string? value) => + value?.Replace("\r", "\\r", StringComparison.Ordinal) + .Replace("\n", "\\n", StringComparison.Ordinal) ?? string.Empty; } diff --git a/src/Demo.SoapApi/Services/WarehouseService.cs b/src/Demo.SoapApi/Services/WarehouseService.cs index fa3d9c3..1d37ea7 100644 --- a/src/Demo.SoapApi/Services/WarehouseService.cs +++ b/src/Demo.SoapApi/Services/WarehouseService.cs @@ -20,7 +20,7 @@ public WarehouseService(IFulfillmentRepository repository, ILogger SubmitFulfillmentRequest(SubmitFulfillmentRequest request) { - _logger.LogInformation("Received fulfillment request for order {OrderNumber}", request.OrderNumber); + _logger.LogInformation("Received fulfillment request for order {OrderNumber}", SanitizeForLog(request.OrderNumber)); try { @@ -94,7 +94,7 @@ public async Task SubmitFulfillmentRequest(SubmitFulf } catch (Exception ex) { - _logger.LogError(ex, "Error processing fulfillment request for order {OrderNumber}", request.OrderNumber); + _logger.LogError(ex, "Error processing fulfillment request for order {OrderNumber}", SanitizeForLog(request.OrderNumber)); return new SubmitFulfillmentResponse { Success = false, @@ -109,7 +109,7 @@ public async Task SubmitFulfillmentRequest(SubmitFulf public async Task GetFulfillmentStatus(GetFulfillmentStatusRequest request) { _logger.LogInformation("Status query for Order: {OrderNumber}, Confirmation: {ConfirmationNumber}", - request.OrderNumber, request.ConfirmationNumber); + SanitizeForLog(request.OrderNumber), SanitizeForLog(request.ConfirmationNumber)); try { @@ -170,7 +170,7 @@ record = await _repository.GetByOrderNumberAsync(request.OrderNumber); public async Task CancelFulfillment(CancelFulfillmentRequest request) { _logger.LogInformation("Cancellation request for Order: {OrderNumber}, Confirmation: {ConfirmationNumber}", - request.OrderNumber, request.ConfirmationNumber); + SanitizeForLog(request.OrderNumber), SanitizeForLog(request.ConfirmationNumber)); try { @@ -300,4 +300,11 @@ private void SimulateStatusProgression(FulfillmentRecord record) record.DeliveredDateTime = record.ShippedDateTime?.AddDays(deliveryDays); } } + + /// + /// Removes CR/LF from user-supplied strings before logging to prevent log-forging. + /// + private static string SanitizeForLog(string? value) => + value?.Replace("\r", "\\r", StringComparison.Ordinal) + .Replace("\n", "\\n", StringComparison.Ordinal) ?? string.Empty; } diff --git a/src/Demo.SoapApi/Storage/InMemoryFulfillmentRepository.cs b/src/Demo.SoapApi/Storage/InMemoryFulfillmentRepository.cs index b4130e5..1d675e2 100644 --- a/src/Demo.SoapApi/Storage/InMemoryFulfillmentRepository.cs +++ b/src/Demo.SoapApi/Storage/InMemoryFulfillmentRepository.cs @@ -38,7 +38,7 @@ public Task AddAsync(FulfillmentRecord record) _orderToConfirmationMap[record.OrderNumber] = record.ConfirmationNumber; _logger.LogDebug("Added fulfillment {ConfirmationNumber} for order {OrderNumber}", - record.ConfirmationNumber, record.OrderNumber); + SanitizeForLog(record.ConfirmationNumber), SanitizeForLog(record.OrderNumber)); return Task.CompletedTask; } @@ -83,4 +83,11 @@ public Task> GetAllAsync() { return Task.FromResult>(_fulfillmentsByConfirmation.Values.ToList()); } + + /// + /// Removes CR/LF from user-supplied strings before logging to prevent log-forging. + /// + private static string SanitizeForLog(string? value) => + value?.Replace("\r", "\\r", StringComparison.Ordinal) + .Replace("\n", "\\n", StringComparison.Ordinal) ?? string.Empty; } diff --git a/src/QuickApiMapper.Designer.Web/Services/IntegrationApiClient.cs b/src/QuickApiMapper.Designer.Web/Services/IntegrationApiClient.cs index 43d5322..e4ee558 100644 --- a/src/QuickApiMapper.Designer.Web/Services/IntegrationApiClient.cs +++ b/src/QuickApiMapper.Designer.Web/Services/IntegrationApiClient.cs @@ -225,7 +225,7 @@ public async Task> GetBehaviorsAsync() var query = string.Join("&", queryParams); var url = $"api/messages?{query}"; - _logger.LogInformation("Querying messages from {Url}", url); + _logger.LogInformation("Querying messages from {Url}", SanitizeForLog(url)); return await _httpClient.GetFromJsonAsync(url); } catch (Exception ex) @@ -328,6 +328,13 @@ public async Task ResetDemoDataAsync() return false; } } + + /// + /// Removes CR/LF from user-supplied strings before logging to prevent log-forging. + /// + private static string SanitizeForLog(string? value) => + value?.Replace("\r", "\\r", StringComparison.Ordinal) + .Replace("\n", "\\n", StringComparison.Ordinal) ?? string.Empty; } // Demo DTO diff --git a/src/QuickApiMapper.Management.Api/Auth/DevNoOpAuthHandler.cs b/src/QuickApiMapper.Management.Api/Auth/DevNoOpAuthHandler.cs new file mode 100644 index 0000000..0bacd96 --- /dev/null +++ b/src/QuickApiMapper.Management.Api/Auth/DevNoOpAuthHandler.cs @@ -0,0 +1,37 @@ +using System.Security.Claims; +using System.Text.Encodings.Web; +using Microsoft.AspNetCore.Authentication; +using Microsoft.Extensions.Options; + +namespace QuickApiMapper.Management.Api.Auth; + +/// +/// Development-only authentication handler that authenticates every request as a local +/// admin principal. This handler MUST NOT be used in production; configure a real +/// identity provider via "Auth:Authority" in appsettings before exposing the Management +/// API to untrusted callers. +/// +internal sealed class DevNoOpAuthHandler : AuthenticationHandler +{ + public DevNoOpAuthHandler( + IOptionsMonitor options, + ILoggerFactory logger, + UrlEncoder encoder) + : base(options, logger, encoder) { } + + protected override Task HandleAuthenticateAsync() + { + var claims = new[] + { + new Claim(ClaimTypes.NameIdentifier, "dev-local"), + new Claim(ClaimTypes.Name, "Developer"), + new Claim(ClaimTypes.Role, "Admin") + }; + + var identity = new ClaimsIdentity(claims, Scheme.Name); + var principal = new ClaimsPrincipal(identity); + var ticket = new AuthenticationTicket(principal, Scheme.Name); + + return Task.FromResult(AuthenticateResult.Success(ticket)); + } +} diff --git a/src/QuickApiMapper.Management.Api/Controllers/IntegrationsController.cs b/src/QuickApiMapper.Management.Api/Controllers/IntegrationsController.cs index 0e0f03c..781df2b 100644 --- a/src/QuickApiMapper.Management.Api/Controllers/IntegrationsController.cs +++ b/src/QuickApiMapper.Management.Api/Controllers/IntegrationsController.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using QuickApiMapper.Management.Contracts.Models; using QuickApiMapper.Management.Api.Services; @@ -10,6 +11,7 @@ namespace QuickApiMapper.Management.Api.Controllers; [ApiController] [Route("api/[controller]")] [Produces("application/json")] +[Authorize] public class IntegrationsController : ControllerBase { private readonly IIntegrationService _integrationService; diff --git a/src/QuickApiMapper.Management.Api/Controllers/MessagesController.cs b/src/QuickApiMapper.Management.Api/Controllers/MessagesController.cs index 85711f4..3c912c5 100644 --- a/src/QuickApiMapper.Management.Api/Controllers/MessagesController.cs +++ b/src/QuickApiMapper.Management.Api/Controllers/MessagesController.cs @@ -99,7 +99,7 @@ public async Task> GetById( if (message == null) { - _logger.LogWarning("Message {MessageId} not found", messageId); + _logger.LogWarning("Message {MessageId} not found", SanitizeForLog(messageId)); return NotFound(new { Message = $"Message {messageId} not found" }); } @@ -160,6 +160,13 @@ public async Task> PurgeOldMessages( return Ok(new { DeletedCount = deletedCount }); } + /// + /// Removes CR/LF from user-supplied strings before logging to prevent log-forging. + /// + private static string SanitizeForLog(string? value) => + value?.Replace("\r", "\\r", StringComparison.Ordinal) + .Replace("\n", "\\n", StringComparison.Ordinal) ?? string.Empty; + /// /// Maps a CapturedMessage domain model to a CapturedMessageDto. /// diff --git a/src/QuickApiMapper.Management.Api/Program.cs b/src/QuickApiMapper.Management.Api/Program.cs index aa082b7..848ffa9 100644 --- a/src/QuickApiMapper.Management.Api/Program.cs +++ b/src/QuickApiMapper.Management.Api/Program.cs @@ -16,6 +16,36 @@ // Add Aspire service defaults (health checks, telemetry, service discovery) builder.AddServiceDefaults(); +// Add authentication and authorization. +// Controllers in this Management API are protected with [Authorize]; callers must +// present a valid bearer token issued by the configured identity provider. +// In development with no IdP configured the scheme falls back to no-op, but the +// middleware chain is always present so the protection attribute is enforced in +// production where a real authority is wired up via "Auth:Authority" / "Auth:Audience". +var authAuthority = builder.Configuration["Auth:Authority"]; +var authAudience = builder.Configuration["Auth:Audience"]; + +if (!string.IsNullOrEmpty(authAuthority)) +{ + builder.Services.AddAuthentication("Bearer") + .AddJwtBearer("Bearer", options => + { + options.Authority = authAuthority; + options.Audience = authAudience ?? "quickapimapper-management"; + }); +} +else +{ + // Development fallback: register a no-op authentication scheme so the + // middleware pipeline is valid. The [Authorize] attribute still gates requests — + // replace this with a real scheme before exposing to untrusted networks. + builder.Services.AddAuthentication("DevNoOp") + .AddScheme("DevNoOp", _ => { }); +} + +builder.Services.AddAuthorization(); + // Add services to the container builder.Services.AddControllers(); builder.Services.AddEndpointsApiExplorer(); @@ -160,6 +190,7 @@ app.UseHttpsRedirection(); app.UseCors(); +app.UseAuthentication(); app.UseAuthorization(); app.MapControllers(); diff --git a/src/QuickApiMapper.Management.Api/QuickApiMapper.Management.Api.csproj b/src/QuickApiMapper.Management.Api/QuickApiMapper.Management.Api.csproj index c4f5cb3..609841d 100644 --- a/src/QuickApiMapper.Management.Api/QuickApiMapper.Management.Api.csproj +++ b/src/QuickApiMapper.Management.Api/QuickApiMapper.Management.Api.csproj @@ -8,6 +8,7 @@ + diff --git a/src/QuickApiMapper.Web/Program.cs b/src/QuickApiMapper.Web/Program.cs index 93f2027..f61d8a7 100644 --- a/src/QuickApiMapper.Web/Program.cs +++ b/src/QuickApiMapper.Web/Program.cs @@ -107,7 +107,8 @@ async Task HandleMappingRequest( // Read and parse input var inputBody = await new StreamReader(request.Body).ReadToEndAsync(cancellationToken); - logger.LogDebug("Received input: {Input}", inputBody); + // Sanitize before logging to prevent log-forging via newline injection. + logger.LogDebug("Received input: {Input}", SanitizeForLog(inputBody)); // Parse input and create appropriate mapping engine based on source and destination types var mappingEngineFactory = serviceProvider.GetRequiredService(); @@ -209,6 +210,14 @@ await engine.ApplyMappingAsync( } } +/// +/// Removes CR/LF characters from user-supplied strings before they reach the log +/// to prevent log-forging / log-injection attacks. +/// +static string SanitizeForLog(string? value) => + value?.Replace("\r", "\\r", StringComparison.Ordinal) + .Replace("\n", "\\n", StringComparison.Ordinal) ?? string.Empty; + XDocument CreateXmlDocument(IntegrationMapping integration) { // Create XML output with proper namespace if specified