From 3cd62de97b4a484c10433e0eff1def16dcd2f7d2 Mon Sep 17 00:00:00 2001 From: Codewriter90x Date: Fri, 10 Jul 2026 09:08:47 +0200 Subject: [PATCH] security: fix csrf token validation alerts --- src/OpenCashFlow.API/AppStart/05_Auth.cs | 8 ---- .../Controllers/AuditLogController.cs | 1 + .../Controllers/AuthenticationController.cs | 1 + .../Controllers/CompanyController.cs | 1 + .../Controllers/DevEmailController.cs | 1 + .../Controllers/EmployeeController.cs | 1 + .../Controllers/PaymentController.Cash.cs | 1 + .../Controllers/PaymentController.cs | 1 + .../Controllers/RolesController.cs | 2 +- .../Controllers/SetupController.cs | 1 + .../Controllers/UsersController.cs | 1 + .../Controllers/AccountController.cs | 44 +++++++++++++++++++ .../Controllers/CompanyController.Cash.cs | 1 + .../Controllers/Internal/PaymentController.cs | 5 +++ .../Views/Company/Company_CashLedger.cshtml | 3 ++ .../Views/Payments/Index.cshtml | 9 ++++ .../Payments/Partials/_PaymentsList.cshtml | 3 ++ .../Views/Shared/Sections/_Scripts.cshtml | 3 +- .../Shared/Sections/_ScriptsFront.cshtml | 3 +- .../Views/Shared/_CommonMasterLayout.cshtml | 3 ++ .../wwwroot/js/components/antiforgery.js | 19 ++++++++ .../wwwroot/js/components/session-watchdog.js | 5 ++- .../Tests/API/Payments_Tests.cs | 29 ++++++++++++ 23 files changed, 134 insertions(+), 12 deletions(-) create mode 100644 src/OpenCashFlow.WebApp/wwwroot/js/components/antiforgery.js diff --git a/src/OpenCashFlow.API/AppStart/05_Auth.cs b/src/OpenCashFlow.API/AppStart/05_Auth.cs index a1d1b5e..4469f34 100644 --- a/src/OpenCashFlow.API/AppStart/05_Auth.cs +++ b/src/OpenCashFlow.API/AppStart/05_Auth.cs @@ -45,14 +45,6 @@ public static WebApplicationBuilder AppStartConfigureAuth(this WebApplicationBui options.Events = new JwtBearerEvents { - OnMessageReceived = context => - { - if (string.IsNullOrEmpty(context.Token)) - { - context.Token = context.Request.Cookies[OpenCashFlow.Contracts.Core.Configuration.AuthCookieName]; - } - return Task.CompletedTask; - }, OnAuthenticationFailed = context => { context.Response.StatusCode = 401; diff --git a/src/OpenCashFlow.API/Controllers/AuditLogController.cs b/src/OpenCashFlow.API/Controllers/AuditLogController.cs index 1705c1b..5651e4a 100644 --- a/src/OpenCashFlow.API/Controllers/AuditLogController.cs +++ b/src/OpenCashFlow.API/Controllers/AuditLogController.cs @@ -11,6 +11,7 @@ namespace OpenCashFlow.API.Controllers { [ApiController] + [IgnoreAntiforgeryToken] [Authorize(Roles = "CompanyAdmin,InstanceAdmin")] [Route("v{version:apiVersion}/Admin/AuditLog")] [ApiVersion("1.0")] diff --git a/src/OpenCashFlow.API/Controllers/AuthenticationController.cs b/src/OpenCashFlow.API/Controllers/AuthenticationController.cs index 0b028be..cfaa3d7 100644 --- a/src/OpenCashFlow.API/Controllers/AuthenticationController.cs +++ b/src/OpenCashFlow.API/Controllers/AuthenticationController.cs @@ -10,6 +10,7 @@ namespace OpenCashFlow.API.Controllers { [ApiController] + [IgnoreAntiforgeryToken] [Route("v{version:apiVersion}/[controller]")] [ApiVersion("1.0")] [ApiVersion("2.0")] diff --git a/src/OpenCashFlow.API/Controllers/CompanyController.cs b/src/OpenCashFlow.API/Controllers/CompanyController.cs index 68677ed..444ea32 100644 --- a/src/OpenCashFlow.API/Controllers/CompanyController.cs +++ b/src/OpenCashFlow.API/Controllers/CompanyController.cs @@ -9,6 +9,7 @@ namespace OpenCashFlow.API.Controllers { [ApiController, Authorize(Policy = "CompanyMember")] + [IgnoreAntiforgeryToken] [Route("v{version:apiVersion}/")] [ApiVersion("1.0")] public partial class CompanyController(ICompanyService CompanyService, ILogger logger) : Controller diff --git a/src/OpenCashFlow.API/Controllers/DevEmailController.cs b/src/OpenCashFlow.API/Controllers/DevEmailController.cs index 4b45b0a..6686b9e 100644 --- a/src/OpenCashFlow.API/Controllers/DevEmailController.cs +++ b/src/OpenCashFlow.API/Controllers/DevEmailController.cs @@ -6,6 +6,7 @@ namespace OpenCashFlow.API.Controllers { #if DEBUG [ApiController] + [IgnoreAntiforgeryToken] [Route("dev/email")] [AllowAnonymous] [ApiExplorerSettings(IgnoreApi = true)] diff --git a/src/OpenCashFlow.API/Controllers/EmployeeController.cs b/src/OpenCashFlow.API/Controllers/EmployeeController.cs index 0d0915d..f5fa210 100644 --- a/src/OpenCashFlow.API/Controllers/EmployeeController.cs +++ b/src/OpenCashFlow.API/Controllers/EmployeeController.cs @@ -8,6 +8,7 @@ namespace OpenCashFlow.API.Controllers { [ApiController, Authorize] + [IgnoreAntiforgeryToken] [Route("v{version:apiVersion}/")] [ApiVersion("1.0")] public partial class EmployeeController(IEmployeeService EmployeeService, ILogger logger) : Controller diff --git a/src/OpenCashFlow.API/Controllers/PaymentController.Cash.cs b/src/OpenCashFlow.API/Controllers/PaymentController.Cash.cs index 641b3a7..c5e6a53 100644 --- a/src/OpenCashFlow.API/Controllers/PaymentController.Cash.cs +++ b/src/OpenCashFlow.API/Controllers/PaymentController.Cash.cs @@ -11,6 +11,7 @@ namespace OpenCashFlow.API.Controllers { [ApiController] + [IgnoreAntiforgeryToken] [Authorize(Roles = "CompanyAdmin,InstanceAdmin")] [Route("v{version:apiVersion}/admin/cash")] [ApiVersion("1.0")] diff --git a/src/OpenCashFlow.API/Controllers/PaymentController.cs b/src/OpenCashFlow.API/Controllers/PaymentController.cs index a315e3c..0cffb56 100644 --- a/src/OpenCashFlow.API/Controllers/PaymentController.cs +++ b/src/OpenCashFlow.API/Controllers/PaymentController.cs @@ -12,6 +12,7 @@ namespace OpenCashFlow.API.Controllers { [ApiController, Authorize(Policy = "CompanyMember")] + [IgnoreAntiforgeryToken] [Route("v{version:apiVersion}/")] [ApiVersion("1.0")] public partial class PaymentController(IPaymentService PaymentService, IAuditLogService auditLogService, ILogger logger) : Controller diff --git a/src/OpenCashFlow.API/Controllers/RolesController.cs b/src/OpenCashFlow.API/Controllers/RolesController.cs index 0ae055c..f5947d7 100644 --- a/src/OpenCashFlow.API/Controllers/RolesController.cs +++ b/src/OpenCashFlow.API/Controllers/RolesController.cs @@ -7,6 +7,7 @@ namespace OpenCashFlow.API.Controllers { [ApiController, Authorize] + [IgnoreAntiforgeryToken] [Route("v{version:apiVersion}/")] [ApiVersion("1.0")] public class RolesController(IRoleService roleService) : Controller @@ -21,4 +22,3 @@ public async Task>> GetVisibleRoles(Canc } } } - diff --git a/src/OpenCashFlow.API/Controllers/SetupController.cs b/src/OpenCashFlow.API/Controllers/SetupController.cs index bd2768f..c3841e2 100644 --- a/src/OpenCashFlow.API/Controllers/SetupController.cs +++ b/src/OpenCashFlow.API/Controllers/SetupController.cs @@ -9,6 +9,7 @@ namespace OpenCashFlow.API.Controllers { [ApiController] + [IgnoreAntiforgeryToken] [AllowAnonymous] [Route("v{version:apiVersion}/[controller]")] [ApiVersion("1.0")] diff --git a/src/OpenCashFlow.API/Controllers/UsersController.cs b/src/OpenCashFlow.API/Controllers/UsersController.cs index e8f0c56..1414051 100644 --- a/src/OpenCashFlow.API/Controllers/UsersController.cs +++ b/src/OpenCashFlow.API/Controllers/UsersController.cs @@ -11,6 +11,7 @@ namespace OpenCashFlow.API.Controllers { [ApiController] + [IgnoreAntiforgeryToken] [Authorize(Roles = "CompanyAdmin")] [Route("v{version:apiVersion}/Admin/Users")] [ApiVersion("1.0")] diff --git a/src/OpenCashFlow.WebApp/Controllers/AccountController.cs b/src/OpenCashFlow.WebApp/Controllers/AccountController.cs index a289ddf..81e8bbb 100644 --- a/src/OpenCashFlow.WebApp/Controllers/AccountController.cs +++ b/src/OpenCashFlow.WebApp/Controllers/AccountController.cs @@ -194,6 +194,50 @@ public async Task ChangePassword(PasswordVM vm) return await LoadEditAccountView(userId.Value); } + [HttpPost] + [ValidateAntiForgeryToken] + [Route("Account/RefreshSession")] + public async Task RefreshSession(CancellationToken cancellationToken) + { + var tokenResult = await _authAPIService.RegenerateTokenAsync(cancellationToken); + if (!tokenResult.Success || tokenResult.Data == null || string.IsNullOrWhiteSpace(tokenResult.Data.Token)) + { + _logger.LogWarning("Session refresh failed: {Message}", tokenResult.Message); + return Unauthorized(new { success = false }); + } + + var newToken = tokenResult.Data.Token; + var expirationTime = TryGetJwtExpiration(newToken) + ?? DateTimeOffset.UtcNow.AddMinutes(OpenCashFlow.Contracts.Core.Configuration.WebSessionDurationMinutes); + + var authCookieOptions = new CookieOptions + { + Domain = _configuration["Account:CookieDomain"], + HttpOnly = true, + Secure = true, + SameSite = SameSiteMode.Lax, + Expires = expirationTime + }; + + HttpContext.Response.Cookies.Append(OpenCashFlow.Contracts.Core.Configuration.AuthCookieName, newToken, authCookieOptions); + + var infoCookieOptions = new CookieOptions + { + Domain = _configuration["Account:CookieDomain"], + HttpOnly = false, + Secure = true, + SameSite = SameSiteMode.Lax, + Expires = expirationTime + }; + + HttpContext.Response.Cookies.Append( + OpenCashFlow.Contracts.Core.Configuration.AuthCookieName + ".Info", + expirationTime.ToUnixTimeSeconds().ToString(), + infoCookieOptions); + + return Ok(new { success = true }); + } + private async Task LoadEditAccountView(Guid userId) { var detail = await _employeeAPIService.GetEmployeeByIDAsync(userId); diff --git a/src/OpenCashFlow.WebApp/Controllers/CompanyController.Cash.cs b/src/OpenCashFlow.WebApp/Controllers/CompanyController.Cash.cs index 0f834b5..8bf45ac 100644 --- a/src/OpenCashFlow.WebApp/Controllers/CompanyController.Cash.cs +++ b/src/OpenCashFlow.WebApp/Controllers/CompanyController.Cash.cs @@ -94,6 +94,7 @@ public async Task GetCashLedger(int skip = 0, int take = 50, Canc public class AdjustRequest { public decimal? Delta { get; set; } public string Reason { get; set; } = string.Empty; } [HttpPost("CashLedger/Adjust")] + [ValidateAntiForgeryToken] public async Task Adjust([FromBody] AdjustRequest request, CancellationToken ct) { var giClaim = User?.FindFirst("TenantID")?.Value; diff --git a/src/OpenCashFlow.WebApp/Controllers/Internal/PaymentController.cs b/src/OpenCashFlow.WebApp/Controllers/Internal/PaymentController.cs index 6318702..8d78de2 100644 --- a/src/OpenCashFlow.WebApp/Controllers/Internal/PaymentController.cs +++ b/src/OpenCashFlow.WebApp/Controllers/Internal/PaymentController.cs @@ -34,6 +34,7 @@ public IActionResult GetPaymentsList() } [HttpPost] + [ValidateAntiForgeryToken] public async Task FilterPayments(Payment_Filter_DTO filters) { try @@ -65,6 +66,7 @@ public IActionResult EditPaymentModal(Guid paymentID) } [HttpPost] + [ValidateAntiForgeryToken] public async Task CreatePayment(Payment_Create_DTO dto) { if (!ModelState.IsValid) @@ -99,6 +101,7 @@ public async Task CreatePayment(Payment_Create_DTO dto) } [HttpPost] + [ValidateAntiForgeryToken] public async Task UpdatePayment(Payment_Update_DTO editDto) { if (!ModelState.IsValid) @@ -118,6 +121,7 @@ public async Task UpdatePayment(Payment_Update_DTO editDto) } [HttpPost] + [ValidateAntiForgeryToken] public async Task DeletePayment(Guid paymentID) { if (paymentID == Guid.Empty) @@ -162,6 +166,7 @@ public async Task GetCashBalance(CancellationToken cancellationTo } [HttpPost] + [ValidateAntiForgeryToken] public async Task GetCalendarEvents([FromBody] Payment_Filter_DTO filters) { try diff --git a/src/OpenCashFlow.WebApp/Views/Company/Company_CashLedger.cshtml b/src/OpenCashFlow.WebApp/Views/Company/Company_CashLedger.cshtml index 941a330..623616a 100644 --- a/src/OpenCashFlow.WebApp/Views/Company/Company_CashLedger.cshtml +++ b/src/OpenCashFlow.WebApp/Views/Company/Company_CashLedger.cshtml @@ -252,6 +252,9 @@ $.ajax({ url: '/Company/CashLedger/Adjust', method: 'POST', + headers: window.openCashFlowAntiForgeryHeaders + ? window.openCashFlowAntiForgeryHeaders() + : {}, contentType: 'application/json', data: JSON.stringify({ delta: delta, reason: reason }), success: function (resp) { diff --git a/src/OpenCashFlow.WebApp/Views/Payments/Index.cshtml b/src/OpenCashFlow.WebApp/Views/Payments/Index.cshtml index e9d9744..36bc0a9 100644 --- a/src/OpenCashFlow.WebApp/Views/Payments/Index.cshtml +++ b/src/OpenCashFlow.WebApp/Views/Payments/Index.cshtml @@ -22,6 +22,12 @@ @* *@ + @if (User?.Identity?.IsAuthenticated ?? false) { @@ -14,5 +15,5 @@ data-disconnect-url="@Url.Content("~/Account/Disconnect")" data-auth-info-cookie-name="@(Configuration.AuthCookieName).Info" data-inactivity-minutes="@(Configuration.WebSessionDurationMinutes)" - data-refresh-url="@($"{_configuration["Account:API"]}/v1/Authentication/refresh")"> + data-refresh-url="@Url.Content("~/Account/RefreshSession")"> } diff --git a/src/OpenCashFlow.WebApp/Views/Shared/Sections/_ScriptsFront.cshtml b/src/OpenCashFlow.WebApp/Views/Shared/Sections/_ScriptsFront.cshtml index ff139fe..42882a3 100644 --- a/src/OpenCashFlow.WebApp/Views/Shared/Sections/_ScriptsFront.cshtml +++ b/src/OpenCashFlow.WebApp/Views/Shared/Sections/_ScriptsFront.cshtml @@ -6,6 +6,7 @@ + @if (User?.Identity?.IsAuthenticated ?? false) { @@ -14,5 +15,5 @@ data-disconnect-url="@Url.Content("~/Account/Disconnect")" data-auth-info-cookie-name="@(Configuration.AuthCookieName).Info" data-inactivity-minutes="@(Configuration.WebSessionDurationMinutes)" - data-refresh-url="@($"{_configuration["Account:API"]}/v1/Authentication/refresh")"> + data-refresh-url="@Url.Content("~/Account/RefreshSession")"> } diff --git a/src/OpenCashFlow.WebApp/Views/Shared/_CommonMasterLayout.cshtml b/src/OpenCashFlow.WebApp/Views/Shared/_CommonMasterLayout.cshtml index 6cae2e1..2a8d893 100644 --- a/src/OpenCashFlow.WebApp/Views/Shared/_CommonMasterLayout.cshtml +++ b/src/OpenCashFlow.WebApp/Views/Shared/_CommonMasterLayout.cshtml @@ -1,11 +1,13 @@ @inject IHttpContextAccessor httpContextAccessor @inject IWebHostEnvironment Env +@inject Microsoft.AspNetCore.Antiforgery.IAntiforgery Antiforgery @{ bool isFront = ViewData["isFront"] is bool isFrontValue && isFrontValue; ViewData["main"] = isFront ? "front-" : ""; ViewData["frontVar"] = isFront ? "Front" : ""; string bodyClass = isFront ? "body-marketing body-gradient" : ""; + string? requestVerificationToken = Antiforgery.GetAndStoreTokens(Context).RequestToken; } @@ -25,6 +27,7 @@ } + diff --git a/src/OpenCashFlow.WebApp/wwwroot/js/components/antiforgery.js b/src/OpenCashFlow.WebApp/wwwroot/js/components/antiforgery.js new file mode 100644 index 0000000..0c8de08 --- /dev/null +++ b/src/OpenCashFlow.WebApp/wwwroot/js/components/antiforgery.js @@ -0,0 +1,19 @@ +(function () { + "use strict"; + + function getToken() { + const meta = document.querySelector('meta[name="request-verification-token"]'); + if (meta && meta.content) { + return meta.content; + } + + const input = document.querySelector('input[name="__RequestVerificationToken"]'); + return input ? input.value : ""; + } + + window.openCashFlowAntiForgeryToken = getToken; + window.openCashFlowAntiForgeryHeaders = function () { + const token = getToken(); + return token ? { "RequestVerificationToken": token } : {}; + }; +})(); diff --git a/src/OpenCashFlow.WebApp/wwwroot/js/components/session-watchdog.js b/src/OpenCashFlow.WebApp/wwwroot/js/components/session-watchdog.js index 143a2ea..0a75737 100644 --- a/src/OpenCashFlow.WebApp/wwwroot/js/components/session-watchdog.js +++ b/src/OpenCashFlow.WebApp/wwwroot/js/components/session-watchdog.js @@ -104,7 +104,10 @@ try { const response = await fetch(refreshUrl, { method: "POST", - credentials: "include" + credentials: "same-origin", + headers: window.openCashFlowAntiForgeryHeaders + ? window.openCashFlowAntiForgeryHeaders() + : {} }); if (!response.ok) { diff --git a/tests/OpenCashFlow.Test/Tests/API/Payments_Tests.cs b/tests/OpenCashFlow.Test/Tests/API/Payments_Tests.cs index 5a3b738..5c528ea 100644 --- a/tests/OpenCashFlow.Test/Tests/API/Payments_Tests.cs +++ b/tests/OpenCashFlow.Test/Tests/API/Payments_Tests.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.DependencyInjection; using OpenCashFlow.Infrastructure.Persistence; using OpenCashFlow.Contracts.DTOs; +using OpenCashFlow.Contracts.Core; using OpenCashFlow.Infrastructure.Persistence.Entities; using System.Net; using System.Net.Http.Headers; @@ -116,6 +117,34 @@ public async Task CreatePayment_WithoutAuthentication_ShouldFail() } + [Trait("Layer", "API")] + [Trait("Feature", "Payments")] + [Trait("Type", "Security")] + [Trait("Priority", "High")] + [Fact(DisplayName = "POST /v1/payments should reject JWT supplied only by cookie")] + public async Task CreatePayment_WithJwtOnlyInCookie_ShouldFail() + { + var userId = Guid.Parse("00000000-0000-0000-0000-000000000001"); + var token = await _factory.GenerateJwtTokenAsync(userId); + var client = _factory.CreateClient(); + client.DefaultRequestHeaders.Add("Cookie", $"{Configuration.AuthCookieName}={token}"); + + var payment = new Payment_Create_DTO + { + TenantID = Guid.Parse("00000000-0000-0000-0000-000000000001"), + UserID = userId, + Amount = 150, + EntryType = nameof(EntryTypeEnum.Income), + PaymentMethodID = Guid.Parse("00000000-0000-0000-0000-000000000001"), + DocumentTypeID = Guid.Parse("00000000-0000-0000-0000-000000000001"), + Description = "Cookie-only JWT should not authenticate API requests" + }; + + var response = await client.PostAsJsonAsync("/v1/Payment", payment); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + // Payment creation [FAIL] (using an unauthorized employee) [Trait("Layer", "API")] [Trait("Feature", "Payments")]