From 3a263477de945d5590d5deca940ef9227e7b01f8 Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Mon, 28 Sep 2026 09:02:20 +0100 Subject: [PATCH 1/4] missed fixes :| --- .../EstateEndpointTests.cs | 17 ++++++- .../TenantContextEndpointTests.cs | 13 +++++ EstateReportingAPI/Common/EstateContext.cs | 50 +++++++++++++++---- 3 files changed, 67 insertions(+), 13 deletions(-) diff --git a/EstateReportingAPI.IntegrationTests/EstateEndpointTests.cs b/EstateReportingAPI.IntegrationTests/EstateEndpointTests.cs index b6c7b48..728a6ab 100644 --- a/EstateReportingAPI.IntegrationTests/EstateEndpointTests.cs +++ b/EstateReportingAPI.IntegrationTests/EstateEndpointTests.cs @@ -130,13 +130,26 @@ public async Task EstateEndpoint_EstateIdClaim_ReturnsEstate() } [Fact] - public async Task EstateEndpoint_MissingAuthentication_ReturnsUnauthorized() + public async Task EstateEndpoint_MissingAuthenticationAndEstateContext_ReturnsForbidden() { using HttpRequestMessage request = new(HttpMethod.Get, this.BaseRoute); using HttpResponseMessage response = await this.Client.SendAsync(request); - response.StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + response.StatusCode.ShouldBe(HttpStatusCode.Forbidden); + } + + [Fact] + public async Task EstateEndpoint_DisabledAuthorisationWithEstateHeader_ReturnsEstate() + { + await this.helper.AddEstate("Test Estate", "Ref1"); + + using HttpRequestMessage request = new(HttpMethod.Get, this.BaseRoute); + request.Headers.Add("estateId", this.TestId.ToString()); + + using HttpResponseMessage response = await this.Client.SendAsync(request); + + response.StatusCode.ShouldBe(HttpStatusCode.OK); } private async Task SendEstateRequest( diff --git a/EstateReportingAPI.IntegrationTests/TenantContextEndpointTests.cs b/EstateReportingAPI.IntegrationTests/TenantContextEndpointTests.cs index a369b40..26a1300 100644 --- a/EstateReportingAPI.IntegrationTests/TenantContextEndpointTests.cs +++ b/EstateReportingAPI.IntegrationTests/TenantContextEndpointTests.cs @@ -31,6 +31,19 @@ public async Task ProtectedEndpoint_DifferentEstateHeader_ReturnsForbidden(strin response.StatusCode.ShouldBe(HttpStatusCode.Forbidden); } + [Fact] + public async Task ProtectedEndpoint_RepeatedEstateHeaders_ReturnsForbidden() + { + using HttpRequestMessage request = new(HttpMethod.Get, "api/estates"); + request.Headers.Authorization = new AuthenticationHeaderValue("Test"); + request.Headers.Add("estateId", this.TestId.ToString()); + request.Headers.Add("estateId", Guid.NewGuid().ToString()); + + using HttpResponseMessage response = await this.Client.SendAsync(request); + + response.StatusCode.ShouldBe(HttpStatusCode.Forbidden); + } + [Theory] [InlineData("health")] [InlineData("healthui")] diff --git a/EstateReportingAPI/Common/EstateContext.cs b/EstateReportingAPI/Common/EstateContext.cs index 017e624..489f340 100644 --- a/EstateReportingAPI/Common/EstateContext.cs +++ b/EstateReportingAPI/Common/EstateContext.cs @@ -29,7 +29,10 @@ public EstateContextMiddleware(RequestDelegate next) _next = next; } - public async Task InvokeAsync(HttpContext httpContext,EstateContext estateContext) + public async Task InvokeAsync( + HttpContext httpContext, + EstateContext estateContext, + IConfiguration configuration) { // Health endpoints or anonymous endpoints do not need estate context. if (httpContext.GetEndpoint()?.Metadata @@ -39,12 +42,43 @@ public async Task InvokeAsync(HttpContext httpContext,EstateContext estateContex return; } - if (httpContext.User.Identity?.IsAuthenticated != true) + bool disableAuthorisation = configuration.GetValue( + "AppSettings:DisableAuthorisation"); + bool isAuthenticated = httpContext.User.Identity?.IsAuthenticated == true; + + if (isAuthenticated == false && disableAuthorisation == false) { httpContext.Response.StatusCode = StatusCodes.Status401Unauthorized; return; } + Guid? headerEstateId = null; + if (httpContext.Request.Headers.TryGetValue("estateId", out var headerValues)) + { + if (headerValues.Count != 1 || + !Guid.TryParse(headerValues[0], out Guid parsedHeaderEstateId) || + parsedHeaderEstateId == Guid.Empty) + { + httpContext.Response.StatusCode = StatusCodes.Status403Forbidden; + return; + } + + headerEstateId = parsedHeaderEstateId; + } + + if (isAuthenticated == false) + { + if (headerEstateId.HasValue == false) + { + httpContext.Response.StatusCode = StatusCodes.Status403Forbidden; + return; + } + + estateContext.SetEstate(headerEstateId.Value); + await _next(httpContext); + return; + } + string? estateClaim = httpContext.User.FindFirst("estateId")?.Value; if (!Guid.TryParse(estateClaim, out Guid authenticatedEstateId) || @@ -54,16 +88,10 @@ public async Task InvokeAsync(HttpContext httpContext,EstateContext estateContex return; } - // Backwards compatibility: if the header is supplied, it must agree - // with the authenticated claim. - if (httpContext.Request.Headers.TryGetValue("estateId", out var headerValue)) + if (headerEstateId.HasValue && headerEstateId.Value != authenticatedEstateId) { - if (!Guid.TryParse(headerValue.SingleOrDefault(), out Guid headerEstateId) || - headerEstateId != authenticatedEstateId) - { - httpContext.Response.StatusCode = StatusCodes.Status403Forbidden; - return; - } + httpContext.Response.StatusCode = StatusCodes.Status403Forbidden; + return; } estateContext.SetEstate(authenticatedEstateId); From 570e0d681b28859a6d17530fdc7cb34a0fa92a61 Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Mon, 28 Sep 2026 09:27:52 +0100 Subject: [PATCH 2/4] Configure disabled authorisation in integration tests --- .../CustomWebApplicationFactory.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/EstateReportingAPI.IntegrationTests/CustomWebApplicationFactory.cs b/EstateReportingAPI.IntegrationTests/CustomWebApplicationFactory.cs index 8b38112..2701db2 100644 --- a/EstateReportingAPI.IntegrationTests/CustomWebApplicationFactory.cs +++ b/EstateReportingAPI.IntegrationTests/CustomWebApplicationFactory.cs @@ -37,6 +37,14 @@ public CustomWebApplicationFactory(string databaseConnectionString) protected override void ConfigureWebHost(IWebHostBuilder builder) { + builder.ConfigureAppConfiguration((_, configurationBuilder) => + { + configurationBuilder.AddInMemoryCollection(new Dictionary + { + { "AppSettings:DisableAuthorisation", "true" } + }); + }); + builder.ConfigureServices(containerBuilder => { var createcontext = new EstateManagementContext(DatabaseConnectionString); From c3987c52311d1dbc51b2e46bc556fd483dad150f Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Mon, 28 Sep 2026 09:45:20 +0100 Subject: [PATCH 3/4] Set authorisation mode for integration tests --- .../CustomWebApplicationFactory.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/EstateReportingAPI.IntegrationTests/CustomWebApplicationFactory.cs b/EstateReportingAPI.IntegrationTests/CustomWebApplicationFactory.cs index 2701db2..2775dcf 100644 --- a/EstateReportingAPI.IntegrationTests/CustomWebApplicationFactory.cs +++ b/EstateReportingAPI.IntegrationTests/CustomWebApplicationFactory.cs @@ -30,6 +30,7 @@ public CustomWebApplicationFactory(string databaseConnectionString) { DatabaseConnectionString = databaseConnectionString; Environment.SetEnvironmentVariable("InTestMode", "true"); + Environment.SetEnvironmentVariable("AppSettings__DisableAuthorisation", "true"); } public string DefaultUserId { get; set; } = "1"; @@ -37,14 +38,6 @@ public CustomWebApplicationFactory(string databaseConnectionString) protected override void ConfigureWebHost(IWebHostBuilder builder) { - builder.ConfigureAppConfiguration((_, configurationBuilder) => - { - configurationBuilder.AddInMemoryCollection(new Dictionary - { - { "AppSettings:DisableAuthorisation", "true" } - }); - }); - builder.ConfigureServices(containerBuilder => { var createcontext = new EstateManagementContext(DatabaseConnectionString); From ac29c599bcf9832c836f4426116231bc2826c4e8 Mon Sep 17 00:00:00 2001 From: StuartFerguson Date: Mon, 28 Sep 2026 09:54:57 +0100 Subject: [PATCH 4/4] Reduce estate context middleware complexity --- EstateReportingAPI/Common/EstateContext.cs | 88 ++++++++++++++++------ 1 file changed, 63 insertions(+), 25 deletions(-) diff --git a/EstateReportingAPI/Common/EstateContext.cs b/EstateReportingAPI/Common/EstateContext.cs index 489f340..22a2b76 100644 --- a/EstateReportingAPI/Common/EstateContext.cs +++ b/EstateReportingAPI/Common/EstateContext.cs @@ -34,16 +34,13 @@ public async Task InvokeAsync( EstateContext estateContext, IConfiguration configuration) { - // Health endpoints or anonymous endpoints do not need estate context. - if (httpContext.GetEndpoint()?.Metadata - .GetMetadata() != null) + if (IsAnonymousEndpoint(httpContext)) { await _next(httpContext); return; } - bool disableAuthorisation = configuration.GetValue( - "AppSettings:DisableAuthorisation"); + bool disableAuthorisation = configuration.GetValue("AppSettings:DisableAuthorisation"); bool isAuthenticated = httpContext.User.Identity?.IsAuthenticated == true; if (isAuthenticated == false && disableAuthorisation == false) @@ -52,36 +49,77 @@ public async Task InvokeAsync( return; } - Guid? headerEstateId = null; - if (httpContext.Request.Headers.TryGetValue("estateId", out var headerValues)) + if (TryGetHeaderEstateId(httpContext, out Guid? headerEstateId) == false) { - if (headerValues.Count != 1 || - !Guid.TryParse(headerValues[0], out Guid parsedHeaderEstateId) || - parsedHeaderEstateId == Guid.Empty) - { - httpContext.Response.StatusCode = StatusCodes.Status403Forbidden; - return; - } - - headerEstateId = parsedHeaderEstateId; + httpContext.Response.StatusCode = StatusCodes.Status403Forbidden; + return; } - if (isAuthenticated == false) + if (isAuthenticated) { - if (headerEstateId.HasValue == false) - { - httpContext.Response.StatusCode = StatusCodes.Status403Forbidden; - return; - } + await HandleAuthenticatedRequestAsync(httpContext, estateContext, headerEstateId); + return; + } - estateContext.SetEstate(headerEstateId.Value); - await _next(httpContext); + await HandleUnauthenticatedRequestAsync(httpContext, estateContext, headerEstateId); + } + + private static bool IsAnonymousEndpoint(HttpContext httpContext) + { + return httpContext.GetEndpoint()?.Metadata.GetMetadata() != null; + } + + private static bool TryGetHeaderEstateId(HttpContext httpContext, out Guid? estateId) + { + estateId = null; + + if (httpContext.Request.Headers.TryGetValue("estateId", out var headerValues) == false) + { + return true; + } + + if (headerValues.Count != 1) + { + return false; + } + + if (Guid.TryParse(headerValues[0], out Guid parsedEstateId) == false) + { + return false; + } + + if (parsedEstateId == Guid.Empty) + { + return false; + } + + estateId = parsedEstateId; + return true; + } + + private async Task HandleUnauthenticatedRequestAsync( + HttpContext httpContext, + EstateContext estateContext, + Guid? headerEstateId) + { + if (headerEstateId.HasValue == false) + { + httpContext.Response.StatusCode = StatusCodes.Status403Forbidden; return; } + estateContext.SetEstate(headerEstateId.Value); + await _next(httpContext); + } + + private async Task HandleAuthenticatedRequestAsync( + HttpContext httpContext, + EstateContext estateContext, + Guid? headerEstateId) + { string? estateClaim = httpContext.User.FindFirst("estateId")?.Value; - if (!Guid.TryParse(estateClaim, out Guid authenticatedEstateId) || + if (Guid.TryParse(estateClaim, out Guid authenticatedEstateId) == false || authenticatedEstateId == Guid.Empty) { httpContext.Response.StatusCode = StatusCodes.Status403Forbidden;