From 71c293320b7bbc8c72c8268a95434c11b696ba2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Zieli=C5=84ski?= Date: Fri, 5 Dec 2025 21:37:15 +0100 Subject: [PATCH] Security: controllers and stack traces in logs --- DiunaBI.API/Attributes/ApiKeyAuthAttribute.cs | 63 +++++++++++++++++++ .../Controllers/DataInboxController.cs | 6 +- DiunaBI.API/Controllers/JobsController.cs | 55 ++++++---------- DiunaBI.API/Controllers/LayersController.cs | 16 ++--- 4 files changed, 93 insertions(+), 47 deletions(-) create mode 100644 DiunaBI.API/Attributes/ApiKeyAuthAttribute.cs diff --git a/DiunaBI.API/Attributes/ApiKeyAuthAttribute.cs b/DiunaBI.API/Attributes/ApiKeyAuthAttribute.cs new file mode 100644 index 0000000..f911748 --- /dev/null +++ b/DiunaBI.API/Attributes/ApiKeyAuthAttribute.cs @@ -0,0 +1,63 @@ +using System.Security.Cryptography; +using System.Text; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; + +namespace DiunaBI.API.Attributes; + +/// +/// Authorization attribute that validates API key from X-API-Key header. +/// Uses constant-time comparison to prevent timing attacks. +/// +[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)] +public class ApiKeyAuthAttribute : Attribute, IAuthorizationFilter +{ + private const string ApiKeyHeaderName = "X-API-Key"; + + public void OnAuthorization(AuthorizationFilterContext context) + { + var configuration = context.HttpContext.RequestServices.GetRequiredService(); + var logger = context.HttpContext.RequestServices.GetRequiredService>(); + + // Get expected API key from configuration + var expectedApiKey = configuration["apiKey"]; + if (string.IsNullOrEmpty(expectedApiKey)) + { + logger.LogError("API key not configured in appsettings"); + context.Result = new StatusCodeResult(StatusCodes.Status500InternalServerError); + return; + } + + // Get API key from header + if (!context.HttpContext.Request.Headers.TryGetValue(ApiKeyHeaderName, out var extractedApiKey)) + { + logger.LogWarning("API key missing from request header"); + context.Result = new UnauthorizedObjectResult(new { error = "API key is required" }); + return; + } + + // Constant-time comparison to prevent timing attacks + if (!IsApiKeyValid(extractedApiKey!, expectedApiKey)) + { + logger.LogWarning("Invalid API key provided from {RemoteIp}", context.HttpContext.Connection.RemoteIpAddress); + context.Result = new UnauthorizedObjectResult(new { error = "Invalid API key" }); + return; + } + + // API key is valid - allow the request to proceed + } + + /// + /// Constant-time string comparison to prevent timing attacks. + /// + private static bool IsApiKeyValid(string providedKey, string expectedKey) + { + if (providedKey == null || expectedKey == null) + return false; + + var providedBytes = Encoding.UTF8.GetBytes(providedKey); + var expectedBytes = Encoding.UTF8.GetBytes(expectedKey); + + return CryptographicOperations.FixedTimeEquals(providedBytes, expectedBytes); + } +} diff --git a/DiunaBI.API/Controllers/DataInboxController.cs b/DiunaBI.API/Controllers/DataInboxController.cs index cf113f4..9ad6ab3 100644 --- a/DiunaBI.API/Controllers/DataInboxController.cs +++ b/DiunaBI.API/Controllers/DataInboxController.cs @@ -87,7 +87,7 @@ public class DataInboxController : Controller catch (Exception e) { _logger.LogError(e, "DataInbox: Insert error for source {Source}, name {Name}", dataInbox.Source, dataInbox.Name); - return BadRequest(e.ToString()); + return BadRequest("An error occurred processing your request"); } } @@ -137,7 +137,7 @@ public class DataInboxController : Controller catch (Exception e) { _logger.LogError(e, "GetAll: Error retrieving data inbox items"); - return BadRequest(e.ToString()); + return BadRequest("An error occurred processing your request"); } } @@ -172,7 +172,7 @@ public class DataInboxController : Controller catch (Exception e) { _logger.LogError(e, "Get: Error retrieving data inbox item {Id}", id); - return BadRequest(e.ToString()); + return BadRequest("An error occurred processing your request"); } } diff --git a/DiunaBI.API/Controllers/JobsController.cs b/DiunaBI.API/Controllers/JobsController.cs index e95fad0..4fa0a3b 100644 --- a/DiunaBI.API/Controllers/JobsController.cs +++ b/DiunaBI.API/Controllers/JobsController.cs @@ -1,3 +1,4 @@ +using DiunaBI.API.Attributes; using DiunaBI.Application.DTOModels.Common; using DiunaBI.Domain.Entities; using DiunaBI.Infrastructure.Data; @@ -82,7 +83,7 @@ public class JobsController : Controller catch (Exception ex) { _logger.LogError(ex, "GetAll: Error retrieving jobs"); - return BadRequest(ex.ToString()); + return BadRequest("An error occurred while retrieving jobs"); } } @@ -108,21 +109,15 @@ public class JobsController : Controller catch (Exception ex) { _logger.LogError(ex, "Get: Error retrieving job {JobId}", id); - return BadRequest(ex.ToString()); + return BadRequest("An error occurred processing your request"); } } [HttpPost] - [Route("schedule/{apiKey}")] - [AllowAnonymous] - public async Task ScheduleJobs(string apiKey, [FromQuery] string? nameFilter = null) + [Route("schedule")] + [ApiKeyAuth] + public async Task ScheduleJobs([FromQuery] string? nameFilter = null) { - if (apiKey != _configuration["apiKey"]) - { - _logger.LogWarning("ScheduleJobs: Unauthorized request with apiKey {ApiKey}", apiKey); - return Unauthorized(); - } - try { var jobsCreated = await _jobScheduler.ScheduleAllJobsAsync(nameFilter); @@ -139,21 +134,15 @@ public class JobsController : Controller catch (Exception ex) { _logger.LogError(ex, "ScheduleJobs: Error scheduling jobs"); - return BadRequest(ex.ToString()); + return BadRequest("An error occurred processing your request"); } } [HttpPost] - [Route("schedule/imports/{apiKey}")] - [AllowAnonymous] - public async Task ScheduleImportJobs(string apiKey, [FromQuery] string? nameFilter = null) + [Route("schedule/imports")] + [ApiKeyAuth] + public async Task ScheduleImportJobs([FromQuery] string? nameFilter = null) { - if (apiKey != _configuration["apiKey"]) - { - _logger.LogWarning("ScheduleImportJobs: Unauthorized request with apiKey {ApiKey}", apiKey); - return Unauthorized(); - } - try { var jobsCreated = await _jobScheduler.ScheduleImportJobsAsync(nameFilter); @@ -170,21 +159,15 @@ public class JobsController : Controller catch (Exception ex) { _logger.LogError(ex, "ScheduleImportJobs: Error scheduling import jobs"); - return BadRequest(ex.ToString()); + return BadRequest("An error occurred processing your request"); } } [HttpPost] - [Route("schedule/processes/{apiKey}")] - [AllowAnonymous] - public async Task ScheduleProcessJobs(string apiKey) + [Route("schedule/processes")] + [ApiKeyAuth] + public async Task ScheduleProcessJobs() { - if (apiKey != _configuration["apiKey"]) - { - _logger.LogWarning("ScheduleProcessJobs: Unauthorized request with apiKey {ApiKey}", apiKey); - return Unauthorized(); - } - try { var jobsCreated = await _jobScheduler.ScheduleProcessJobsAsync(); @@ -201,7 +184,7 @@ public class JobsController : Controller catch (Exception ex) { _logger.LogError(ex, "ScheduleProcessJobs: Error scheduling process jobs"); - return BadRequest(ex.ToString()); + return BadRequest("An error occurred processing your request"); } } @@ -243,7 +226,7 @@ public class JobsController : Controller catch (Exception ex) { _logger.LogError(ex, "RetryJob: Error retrying job {JobId}", id); - return BadRequest(ex.ToString()); + return BadRequest("An error occurred processing your request"); } } @@ -290,7 +273,7 @@ public class JobsController : Controller catch (Exception ex) { _logger.LogError(ex, "CancelJob: Error cancelling job {JobId}", id); - return BadRequest(ex.ToString()); + return BadRequest("An error occurred processing your request"); } } @@ -317,7 +300,7 @@ public class JobsController : Controller catch (Exception ex) { _logger.LogError(ex, "GetStats: Error retrieving job statistics"); - return BadRequest(ex.ToString()); + return BadRequest("An error occurred processing your request"); } } @@ -429,7 +412,7 @@ public class JobsController : Controller catch (Exception ex) { _logger.LogError(ex, "CreateJobForLayer: Error creating job for layer {LayerId}", layerId); - return BadRequest(ex.ToString()); + return BadRequest("An error occurred processing your request"); } } } diff --git a/DiunaBI.API/Controllers/LayersController.cs b/DiunaBI.API/Controllers/LayersController.cs index 353b71f..0c3a958 100644 --- a/DiunaBI.API/Controllers/LayersController.cs +++ b/DiunaBI.API/Controllers/LayersController.cs @@ -99,7 +99,7 @@ public class LayersController : Controller catch (Exception e) { _logger.LogError(e, "GetAll: Error retrieving layers"); - return BadRequest(e.ToString()); + return BadRequest("An error occurred processing your request"); } } [HttpGet] @@ -119,7 +119,7 @@ public class LayersController : Controller catch (Exception e) { _logger.LogError(e, "Get: Error retrieving layer {LayerId}", id); - return BadRequest(e.ToString()); + return BadRequest("An error occurred processing your request"); } } [HttpGet] @@ -396,7 +396,7 @@ public class LayersController : Controller catch (Exception e) { _logger.LogError(e, "AutoImport: Process error"); - return BadRequest(e.ToString()); + return BadRequest("An error occurred processing your request"); } } @@ -808,7 +808,7 @@ public class LayersController : Controller catch (Exception e) { _logger.LogError(e, "CreateRecord: Error creating record in layer {LayerId}", layerId); - return BadRequest(e.ToString()); + return BadRequest("An error occurred processing your request"); } } @@ -889,7 +889,7 @@ public class LayersController : Controller catch (Exception e) { _logger.LogError(e, "UpdateRecord: Error updating record {RecordId} in layer {LayerId}", recordId, layerId); - return BadRequest(e.ToString()); + return BadRequest("An error occurred processing your request"); } } @@ -944,7 +944,7 @@ public class LayersController : Controller catch (Exception e) { _logger.LogError(e, "DeleteRecord: Error deleting record {RecordId} from layer {LayerId}", recordId, layerId); - return BadRequest(e.ToString()); + return BadRequest("An error occurred processing your request"); } } @@ -983,7 +983,7 @@ public class LayersController : Controller catch (Exception e) { _logger.LogError(e, "GetRecordHistory: Error retrieving history for record {RecordId}", recordId); - return BadRequest(e.ToString()); + return BadRequest("An error occurred processing your request"); } } @@ -1033,7 +1033,7 @@ public class LayersController : Controller catch (Exception e) { _logger.LogError(e, "GetDeletedRecords: Error retrieving deleted records for layer {LayerId}", layerId); - return BadRequest(e.ToString()); + return BadRequest("An error occurred processing your request"); } }