Skip to content

Applied new [ValidateLaCode] attribute to actions and controllers that consume code #2339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 13, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion web/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ In a console window:
> run `dotnet user-secrets set "PLACEHOLDER" "PLACEHOLDER".` This will create a `secrets.json` file in the folder
> location
>
described [here](https://learn.microsoft.com/en-us/aspnet/core/security/app-secrets?view=aspnetcore-8.0&tabs=linux#how-the-secret-manager-tool-works).
described [in Microsoft docs](https://learn.microsoft.com/en-us/aspnet/core/security/app-secrets?view=aspnetcore-8.0&tabs=linux#how-the-secret-manager-tool-works).
> At this point `secrets.json` can be updated manually with the settings described below.

#### Platform APIs
Expand Down
89 changes: 70 additions & 19 deletions web/src/Web.App/Attributes/ValidateArgumentAttribute.cs
Original file line number Diff line number Diff line change
@@ -1,43 +1,94 @@
using System.Net;
using System.Text.RegularExpressions;
using FluentValidation;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Filters;
using Web.App.Validators;

namespace Web.App.Attributes;

[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)]
public class ValidateArgumentAttribute(string argumentName, Regex regex) : ActionFilterAttribute
public abstract class ValidateArgumentAttribute : TypeFilterAttribute
{
protected ValidateArgumentAttribute(string argumentName, string type) : this(argumentName, type, string.Empty)
{
}

protected ValidateArgumentAttribute(string argumentName, string type, string typeName) : base(typeof(ValidateArgumentFilter))
{
ArgumentName = argumentName;
Type = type;
TypeName = typeName;

// arguments list must match the `ValidateArgument` constructor arguments
Arguments =
[
ArgumentName,
Type,
TypeName
];
}

internal string ArgumentName { get; }
internal string Type { get; }
internal string TypeName { get; }
}

internal class ValidateArgumentFilter(
ILogger<ValidateArgumentFilter> logger,
IValidator<OrganisationIdentifier> validator,
string argumentName,
string type,
string typeName) : ActionFilterAttribute
{
public override void OnActionExecuting(ActionExecutingContext context)
{
if (context.ActionArguments.TryGetValue(argumentName, out var value))
if (context.ActionArguments.TryGetValue(argumentName, out var identifierValue))
{
var parsed = value?.ToString();
var parsed = identifierValue?.ToString();

// Only attempt to validate if non-null or empty value. This _should_ always be the case,
// otherwise ASP.NET action binding would fail and a `404` would be served by ASP.NET anyway.
if (!string.IsNullOrWhiteSpace(parsed) && !regex.IsMatch(parsed))
if (!string.IsNullOrWhiteSpace(parsed))
{
context.Result = new ViewResult
var parsedType = type;
if (string.IsNullOrWhiteSpace(parsedType) && !string.IsNullOrWhiteSpace(typeName))
{
if (context.ActionArguments.TryGetValue(typeName, out var typeValue))
{
parsedType = typeValue?.ToString();
}
}

// no type provided, so unable to validate identifier
if (string.IsNullOrWhiteSpace(parsedType))
{
logger.LogDebug("Unable to validate identifier {Identifier} because organisation type could not be resolved", parsed);
context.Result = new NotFoundResult();
}
else
{
ViewName = "../Error/NotFound",
StatusCode = (int)HttpStatusCode.BadRequest
};
var identifier = new OrganisationIdentifier
{
Value = parsed,
Type = parsedType
};

var result = validator.Validate(identifier);

logger.LogDebug("Validation of identifier {Identifier} of type {Type} returned {ErrorCount} error(s)", identifier.Value, identifier.Type, result.Errors.Count);
if (!result.IsValid)
{
context.Result = new NotFoundResult();
}
}
}
}

base.OnActionExecuting(context);
}
}

public static partial class ValidateArgumentAttributeRegex
{
[GeneratedRegex(@"^\d{6}$")]
public static partial Regex UrnRegex();
public class ValidateUrnAttribute() : ValidateArgumentAttribute("urn", OrganisationTypes.School);

[GeneratedRegex(@"^\d{8}$")]
public static partial Regex CompanyNumberRegex();
}
public class ValidateCompanyNumberAttribute() : ValidateArgumentAttribute("companyNumber", OrganisationTypes.Trust);

public class ValidateUrnAttribute() : ValidateArgumentAttribute("urn", ValidateArgumentAttributeRegex.UrnRegex());
public class ValidateCompanyNumberAttribute() : ValidateArgumentAttribute("companyNumber", ValidateArgumentAttributeRegex.CompanyNumberRegex());
public class ValidateLaCodeAttribute() : ValidateArgumentAttribute("code", OrganisationTypes.LocalAuthority);
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Mvc;
using Web.App.Attributes;
using Web.App.Controllers.Api.Mappers;
using Web.App.Controllers.Api.Responses;
using Web.App.Domain.NonFinancial;
Expand All @@ -23,6 +24,7 @@ public class EducationHealthCarePlansProxyController(
[ProducesResponseType<EducationHealthCarePlansComparisonResponse[]>(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
[Route("comparison")]
[ValidateLaCode]
public async Task<IActionResult> Comparison([FromQuery] string code, [FromQuery] string[]? set = null, CancellationToken cancellationToken = default)
{
try
Expand Down Expand Up @@ -53,6 +55,7 @@ public async Task<IActionResult> Comparison([FromQuery] string code, [FromQuery]
[ProducesResponseType<EducationHealthCarePlansHistoryResponse[]>(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
[Route("history")]
[ValidateLaCode]
public async Task<IActionResult> History([FromQuery] string code, CancellationToken cancellationToken = default)
{
try
Expand Down
3 changes: 3 additions & 0 deletions web/src/Web.App/Controllers/Api/HighNeedsProxyController.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Mvc;
using Web.App.Attributes;
using Web.App.Controllers.Api.Mappers;
using Web.App.Controllers.Api.Responses;
using Web.App.Domain.LocalAuthorities;
Expand All @@ -23,6 +24,7 @@ public class HighNeedsProxyController(
[ProducesResponseType<LocalAuthorityHighNeedsComparisonResponse[]>(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
[Route("comparison")]
[ValidateLaCode]
public async Task<IActionResult> Comparison([FromQuery] string code, [FromQuery] string[]? set = null, CancellationToken cancellationToken = default)
{
try
Expand Down Expand Up @@ -53,6 +55,7 @@ public async Task<IActionResult> Comparison([FromQuery] string code, [FromQuery]
[ProducesResponseType<LocalAuthorityHighNeedsHistoryResponse[]>(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
[Route("history")]
[ValidateLaCode]
public async Task<IActionResult> History([FromQuery] string code, CancellationToken cancellationToken = default)
{
try
Expand Down
2 changes: 2 additions & 0 deletions web/src/Web.App/Controllers/LocalAuthorityCensusController.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Mvc;
using Microsoft.FeatureManagement.Mvc;
using Web.App.Attributes;
using Web.App.Attributes.RequestTelemetry;
using Web.App.Domain;
using Web.App.Infrastructure.Apis;
Expand All @@ -12,6 +13,7 @@ namespace Web.App.Controllers;
[Controller]
[FeatureGate(FeatureFlags.LocalAuthorities)]
[Route("local-authority/{code}/census")]
[ValidateLaCode]
[LocalAuthorityRequestTelemetry(TrackedRequestFeature.BenchmarkWorkforce)]
public class LocalAuthorityCensusController(
IEstablishmentApi establishmentApi,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Mvc;
using Microsoft.FeatureManagement.Mvc;
using Web.App.Attributes;
using Web.App.Attributes.RequestTelemetry;
using Web.App.Domain;
using Web.App.Infrastructure.Apis;
Expand All @@ -12,6 +13,7 @@ namespace Web.App.Controllers;
[Controller]
[FeatureGate(FeatureFlags.LocalAuthorities)]
[Route("local-authority/{code}/comparison")]
[ValidateLaCode]
[LocalAuthorityRequestTelemetry(TrackedRequestFeature.BenchmarkCosts)]
public class LocalAuthorityComparisonController(
IEstablishmentApi establishmentApi,
Expand Down
2 changes: 2 additions & 0 deletions web/src/Web.App/Controllers/LocalAuthorityController.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Mvc;
using Microsoft.FeatureManagement.Mvc;
using Web.App.Attributes;
using Web.App.Attributes.RequestTelemetry;
using Web.App.Domain;
using Web.App.Infrastructure.Apis;
Expand All @@ -13,6 +14,7 @@ namespace Web.App.Controllers;
[Controller]
[FeatureGate(FeatureFlags.LocalAuthorities)]
[Route("local-authority/{code}")]
[ValidateLaCode]
public class LocalAuthorityController(
ILogger<LocalAuthorityController> logger,
IEstablishmentApi establishmentApi)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Mvc;
using Microsoft.FeatureManagement.Mvc;
using Web.App.Attributes;
using Web.App.Attributes.RequestTelemetry;
using Web.App.Domain;
using Web.App.Extensions;
Expand All @@ -15,6 +16,7 @@ namespace Web.App.Controllers;
[Controller]
[FeatureGate(FeatureFlags.LocalAuthorities, FeatureFlags.HighNeeds)]
[Route("local-authority/{code}/high-needs/benchmarking")]
[ValidateLaCode]
public class LocalAuthorityHighNeedsBenchmarkingController(
ILogger<LocalAuthorityHighNeedsBenchmarkingController> logger,
IEstablishmentApi establishmentApi,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Mvc;
using Microsoft.FeatureManagement.Mvc;
using Web.App.Attributes;
using Web.App.Attributes.RequestTelemetry;
using Web.App.Domain;
using Web.App.Infrastructure.Apis;
Expand All @@ -14,6 +15,7 @@ namespace Web.App.Controllers;
[Controller]
[FeatureGate(FeatureFlags.LocalAuthorities, FeatureFlags.HighNeeds)]
[Route("local-authority/{code}/high-needs")]
[ValidateLaCode]
public class LocalAuthorityHighNeedsController(
ILogger<LocalAuthorityHighNeedsController> logger,
IEstablishmentApi establishmentApi,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Mvc;
using Microsoft.FeatureManagement.Mvc;
using Web.App.Attributes;
using Web.App.Attributes.RequestTelemetry;
using Web.App.Domain;
using Web.App.Domain.LocalAuthorities;
Expand All @@ -15,6 +16,7 @@ namespace Web.App.Controllers;
[Controller]
[FeatureGate(FeatureFlags.LocalAuthorities, FeatureFlags.HighNeeds)]
[Route("local-authority/{code}/high-needs/history")]
[ValidateLaCode]
public class LocalAuthorityHighNeedsHistoricDataController(
ILogger<LocalAuthorityHighNeedsHistoricDataController> logger,
IEstablishmentApi establishmentApi,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Mvc;
using Microsoft.FeatureManagement.Mvc;
using Web.App.Attributes;
using Web.App.Attributes.RequestTelemetry;
using Web.App.Domain;
using Web.App.Infrastructure.Apis;
Expand All @@ -14,6 +15,7 @@ namespace Web.App.Controllers;
[Controller]
[FeatureGate(FeatureFlags.LocalAuthorities, FeatureFlags.HighNeeds)]
[Route("local-authority/{code}/high-needs/national-rank")]
[ValidateLaCode]
public class LocalAuthorityHighNeedsNationalRankingsController(
ILogger<LocalAuthorityHighNeedsNationalRankingsController> logger,
IEstablishmentApi establishmentApi,
Expand Down
20 changes: 16 additions & 4 deletions web/src/Web.App/Extensions/ServiceCollectionExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Security.Claims;
using FluentValidation;
using IdentityModel.Client;
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authentication.Cookies;
Expand All @@ -17,6 +18,7 @@
using Web.App.Infrastructure.Apis.NonFinancial;
using Web.App.Infrastructure.Storage;
using Web.App.Services;
using Web.App.Validators;

namespace Web.App.Extensions;

Expand Down Expand Up @@ -291,8 +293,18 @@ private static AuthenticationBuilder AddDfeSignInOpenIdConnect(this Authenticati
});
}

public static IServiceCollection AddActionResults(this IServiceCollection services) => services
.AddSingleton<IActionResultExecutor<CsvResult>, CsvResultActionResultExecutor>()
.AddSingleton<IActionResultExecutor<CsvResults>, CsvResultsActionResultExecutor>()
.AddSingleton<ICsvService, CsvService>();
public static IServiceCollection AddActionResults(this IServiceCollection services)
{
return services
.AddSingleton<IActionResultExecutor<CsvResult>, CsvResultActionResultExecutor>()
.AddSingleton<IActionResultExecutor<CsvResults>, CsvResultsActionResultExecutor>()
.AddSingleton<ICsvService, CsvService>();
}

public static IServiceCollection AddValidation(this IServiceCollection services)
{
return services
.AddScoped<IFinancialPlanStageValidator, FinancialPlanStageValidator>()
.AddScoped<IValidator<OrganisationIdentifier>, OrganisationIdentifierValidator>();
}
}
3 changes: 1 addition & 2 deletions web/src/Web.App/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
using Web.App.Middleware;
using Web.App.Services;
using Web.App.Telemetry;
using Web.App.Validators;

[assembly: InternalsVisibleTo("Web.Tests")]

Expand All @@ -40,12 +39,12 @@
.AddScoped<ISchoolComparatorSetService, SchoolComparatorSetService>()
.AddScoped<ITrustComparatorSetService, TrustComparatorSetService>()
.AddScoped<ISuggestService, SuggestService>()
.AddScoped<IFinancialPlanStageValidator, FinancialPlanStageValidator>()
.AddScoped<ICustomDataService, CustomDataService>()
.AddScoped<IUserDataService, UserDataService>()
.AddScoped<IClaimsIdentifierService, ClaimsIdentifierService>()
.AddScoped<ILocalAuthorityComparatorSetService, LocalAuthorityComparatorSetService>()
.AddScoped<ISearchService, SearchService>()
.AddValidation()
.AddActionResults();

builder.Services.AddHealthChecks()
Expand Down
Loading