diff --git a/.gitignore b/.gitignore index aabccddee..174d32e0d 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ # Agent AI development scratch docs (plans, smoke-test guides); not committed PLAN-*.md SMOKETEST-*.md +.review-pr-ignored-* # User-specific files *.rsuser diff --git a/test/Classes/AuthorizationFallbackTests.cs b/test/Classes/AuthorizationFallbackTests.cs new file mode 100644 index 000000000..b1a775a5c --- /dev/null +++ b/test/Classes/AuthorizationFallbackTests.cs @@ -0,0 +1,27 @@ +using Microsoft.AspNetCore.Authorization; +using Viper.Areas.CMS.Controllers; +using Viper.Controllers; + +namespace Viper.test.Classes +{ + /// + /// Asserts that the controllers which answer anonymously by design still carry [AllowAnonymous]. + /// The FallbackPolicy in Program.cs is not asserted here: this only pins the opt-outs, because + /// losing one would deny the endpoint at runtime and break sign-in or the public nav rather + /// than fail a build. + /// + public class AuthorizationFallbackTests + { + public static TheoryData AnonymousControllers() => + new(typeof(CMSController), typeof(LayoutController), typeof(LoggedInUserController)); + + [Theory] + [MemberData(nameof(AnonymousControllers))] + public void AnonymousControllers_CarryAllowAnonymous(Type controller) + { + Assert.True(controller.GetCustomAttributes(typeof(AllowAnonymousAttribute), inherit: true).Length > 0, + $"{controller.Name} answers anonymously by design and must carry [AllowAnonymous], " + + "otherwise the FallbackPolicy denies it"); + } + } +} diff --git a/web/Areas/CMS/Controllers/CMSController.cs b/web/Areas/CMS/Controllers/CMSController.cs index 0223a9fc9..bca024a3b 100644 --- a/web/Areas/CMS/Controllers/CMSController.cs +++ b/web/Areas/CMS/Controllers/CMSController.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.RateLimiting; using Viper.Areas.CMS.Services; @@ -7,6 +8,9 @@ namespace Viper.Areas.CMS.Controllers { [Route("/CMS/[action]")] + // Serves public files and enforces per-file permissions itself, so it opts out of the + // fallback policy rather than sitting behind a blanket authenticated-user check. + [AllowAnonymous] public class CMSController : Controller { private readonly VIPERContext _viperContext; diff --git a/web/Classes/HealthChecks/HealthCheckExtensions.cs b/web/Classes/HealthChecks/HealthCheckExtensions.cs index bd34f2e87..f05e56725 100644 --- a/web/Classes/HealthChecks/HealthCheckExtensions.cs +++ b/web/Classes/HealthChecks/HealthCheckExtensions.cs @@ -263,7 +263,7 @@ public static WebApplication UseViperHealthChecks(this WebApplication app) app.MapHealthChecks("/health", new HealthCheckOptions { Predicate = _ => false, - }); + }).AllowAnonymous(); // /health/detail - per-check JSON (UI format), IP-allowlisted to SVM // infra via InternalAllowlist. Intentionally not CAS-gated so the @@ -286,7 +286,7 @@ public static WebApplication UseViperHealthChecks(this WebApplication app) return null; } return await next(ctx); - }); + }).AllowAnonymous(); // IP-gate every UI sub-path (HTML page, API, resource files, webhook config). app.UseWhen( @@ -358,7 +358,7 @@ public static WebApplication UseViperHealthChecks(this WebApplication app) options.AddCustomStylesheet(Path.Join( app.Environment.ContentRootPath, "wwwroot", "css", "healthchecks-ui-branding.css")); - }); + }).AllowAnonymous(); return app; } diff --git a/web/Controllers/LayoutController.cs b/web/Controllers/LayoutController.cs index b2024fafa..20ace4192 100644 --- a/web/Controllers/LayoutController.cs +++ b/web/Controllers/LayoutController.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Viper.Areas.ClinicalScheduler.Services; using Viper.Areas.CMS.Data; @@ -9,6 +10,8 @@ namespace Viper.Controllers { [Route("/api/layout")] + // Returns a permission-filtered nav to anonymous visitors, who get the public subset. + [AllowAnonymous] public class LayoutController : ApiController { private readonly RAPSContext _context; diff --git a/web/Controllers/LoggedInUserController.cs b/web/Controllers/LoggedInUserController.cs index 14c9b64db..7951b39d2 100644 --- a/web/Controllers/LoggedInUserController.cs +++ b/web/Controllers/LoggedInUserController.cs @@ -1,4 +1,5 @@ using Microsoft.AspNetCore.Antiforgery; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Viper.Classes; using Viper.Classes.SQLContext; @@ -6,7 +7,9 @@ namespace Viper.Controllers { [Route("/api/loggedInUser")] - //[Permission(Allow = "SVMSecure")] + // Answers anonymously by design: the front end asks who is logged in before it knows + // whether anyone is, and gets a null user rather than a 401. + [AllowAnonymous] public class LoggedInUserController : ApiController { private readonly IAntiforgery _antiforgery; diff --git a/web/Program.cs b/web/Program.cs index 636320a04..c4ff81f24 100644 --- a/web/Program.cs +++ b/web/Program.cs @@ -169,6 +169,12 @@ .RequireAuthenticatedUser() .AddRequirements(new AuthorizationPolicyBuilder().RequireClaim(ClaimTypes.AuthenticationMethod, "CAS").Build().Requirements.ToArray()) .Build(); + + // Attribute-routed controllers are not covered by RequireAuthorization() on the conventional + // routes, so a controller that forgets [Authorize] is public. Deny by default instead. The + // handful that answer anonymously opt out explicitly: [AllowAnonymous] on controllers, + // .AllowAnonymous() on endpoint mappings such as the health checks. + options.FallbackPolicy = options.DefaultPolicy; }); // Add services necessary for nonces in CSP, 32-byte nonces