From 3848b74fd272d237bd5aba62847ffec19197eba7 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Wed, 5 Jun 2024 12:25:00 +0100 Subject: [PATCH 01/28] All controllers redirect to challenge url. --- .../AccessibilityStatementController.cs | 3 ++- .../Controllers/AdviceController.cs | 5 ++-- .../Controllers/Base/ServiceController.cs | 9 ++++++++ .../Controllers/ChallengeController.cs | 16 +++++++++++++ .../Controllers/CookiesController.cs | 12 ++++++---- .../Controllers/HealthController.cs | 3 ++- .../Controllers/HomeController.cs | 3 ++- .../QualificationDetailsController.cs | 3 ++- .../Controllers/QuestionsController.cs | 3 ++- .../ChallengeResourceFilterAttribute.cs | 23 +++++++++++++++++++ 10 files changed, 68 insertions(+), 12 deletions(-) create mode 100644 src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs create mode 100644 src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs create mode 100644 src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/AccessibilityStatementController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/AccessibilityStatementController.cs index e27a62ef..b6d0d9f3 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/AccessibilityStatementController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/AccessibilityStatementController.cs @@ -1,6 +1,7 @@ using Dfe.EarlyYearsQualification.Content.Entities; using Dfe.EarlyYearsQualification.Content.Renderers.Entities; using Dfe.EarlyYearsQualification.Content.Services; +using Dfe.EarlyYearsQualification.Web.Controllers.Base; using Dfe.EarlyYearsQualification.Web.Models.Content; using Microsoft.AspNetCore.Mvc; @@ -11,7 +12,7 @@ public class AccessibilityStatementController( ILogger logger, IContentService contentService, IHtmlRenderer renderer) - : Controller + : ServiceController { [HttpGet] public async Task Index() diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/AdviceController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/AdviceController.cs index c58ad539..559e4c8f 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/AdviceController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/AdviceController.cs @@ -2,6 +2,7 @@ using Dfe.EarlyYearsQualification.Content.Entities; using Dfe.EarlyYearsQualification.Content.Renderers.Entities; using Dfe.EarlyYearsQualification.Content.Services; +using Dfe.EarlyYearsQualification.Web.Controllers.Base; using Dfe.EarlyYearsQualification.Web.Models.Content; using Microsoft.AspNetCore.Mvc; @@ -9,7 +10,7 @@ namespace Dfe.EarlyYearsQualification.Web.Controllers; [Route("/advice")] public class AdviceController(ILogger logger, IContentService contentService, IHtmlRenderer renderer) - : Controller + : ServiceController { [HttpGet("qualification-outside-the-united-kingdom")] public async Task QualificationOutsideTheUnitedKingdom() @@ -22,7 +23,7 @@ private async Task GetView(string advicePageId) var advicePage = await contentService.GetAdvicePage(advicePageId); if (advicePage is null) { - logger.LogError("No content for the advice page"); + logger.LogError("No content for the advice page"); return RedirectToAction("Error", "Home"); } diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs new file mode 100644 index 00000000..bbe81a10 --- /dev/null +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs @@ -0,0 +1,9 @@ +using Dfe.EarlyYearsQualification.Web.Filters; +using Microsoft.AspNetCore.Mvc; + +namespace Dfe.EarlyYearsQualification.Web.Controllers.Base; + +[ChallengeResourceFilter] +public class ServiceController : Controller +{ +} \ No newline at end of file diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs new file mode 100644 index 00000000..20bfaf06 --- /dev/null +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs @@ -0,0 +1,16 @@ +using Microsoft.AspNetCore.Mvc; + +namespace Dfe.EarlyYearsQualification.Web.Controllers; + +[Route("/challenge")] +public class ChallengeController( + ILogger logger) + : Controller +{ + [HttpGet] + public Task Index() + { + logger.LogWarning("Challenge page invoked"); + return Task.FromResult(new OkResult()); + } +} \ No newline at end of file diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/CookiesController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/CookiesController.cs index 7e930317..3d7c6975 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/CookiesController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/CookiesController.cs @@ -1,6 +1,7 @@ using Dfe.EarlyYearsQualification.Content.Entities; using Dfe.EarlyYearsQualification.Content.Renderers.Entities; using Dfe.EarlyYearsQualification.Content.Services; +using Dfe.EarlyYearsQualification.Web.Controllers.Base; using Dfe.EarlyYearsQualification.Web.Models.Content; using Dfe.EarlyYearsQualification.Web.Services.CookieService; using Microsoft.AspNetCore.Mvc; @@ -15,7 +16,7 @@ public class CookiesController( ISuccessBannerRenderer successBannerRenderer, ICookieService cookieService, IUrlHelper urlHelper) - : Controller + : ServiceController { [HttpGet] public async Task Index() @@ -34,21 +35,21 @@ public async Task Index() } [HttpPost("accept")] - public IActionResult Accept([FromForm]string? returnUrl) + public IActionResult Accept([FromForm] string? returnUrl) { cookieService.SetPreference(true); return Redirect(CheckUrl(returnUrl)); } [HttpPost("reject")] - public IActionResult Reject([FromForm]string? returnUrl) + public IActionResult Reject([FromForm] string? returnUrl) { cookieService.RejectCookies(); return Redirect(CheckUrl(returnUrl)); } [HttpPost("hidebanner")] - public IActionResult HideBanner([FromForm]string? returnUrl) + public IActionResult HideBanner([FromForm] string? returnUrl) { cookieService.SetVisibility(false); return Redirect(CheckUrl(returnUrl)); @@ -65,13 +66,14 @@ public IActionResult CookiePreference(string cookiesAnswer) { cookieService.RejectCookies(); } + TempData["UserPreferenceRecorded"] = true; return Redirect("/cookies"); } private string CheckUrl(string? url) { - return urlHelper.IsLocalUrl(url) ? url : "/cookies"; + return urlHelper.IsLocalUrl(url) ? url : "/cookies"; } private async Task Map(CookiesPage content) diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/HealthController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/HealthController.cs index c5b9c5af..b1311e87 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/HealthController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/HealthController.cs @@ -1,10 +1,11 @@ +using Dfe.EarlyYearsQualification.Web.Controllers.Base; using Microsoft.AspNetCore.Mvc; namespace Dfe.EarlyYearsQualification.Web.Controllers; [ApiController] [Route("[controller]")] -public class HealthController : Controller +public class HealthController : ServiceController { [HttpGet] public IActionResult Get() diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/HomeController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/HomeController.cs index 00365434..5d9c30a0 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/HomeController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/HomeController.cs @@ -2,6 +2,7 @@ using Dfe.EarlyYearsQualification.Content.Entities; using Dfe.EarlyYearsQualification.Content.Renderers.Entities; using Dfe.EarlyYearsQualification.Content.Services; +using Dfe.EarlyYearsQualification.Web.Controllers.Base; using Dfe.EarlyYearsQualification.Web.Models; using Dfe.EarlyYearsQualification.Web.Models.Content; using Microsoft.AspNetCore.Mvc; @@ -13,7 +14,7 @@ public class HomeController( IContentService contentService, IHtmlRenderer htmlRenderer, ISideContentRenderer sideContentRenderer) - : Controller + : ServiceController { [HttpGet] public async Task Index() diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs index 88358db8..1fcba4b8 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs @@ -1,6 +1,7 @@ using Dfe.EarlyYearsQualification.Content.Entities; using Dfe.EarlyYearsQualification.Content.Renderers.Entities; using Dfe.EarlyYearsQualification.Content.Services; +using Dfe.EarlyYearsQualification.Web.Controllers.Base; using Dfe.EarlyYearsQualification.Web.Models.Content; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; @@ -12,7 +13,7 @@ public class QualificationDetailsController( ILogger logger, IContentService contentService, IGovUkInsetTextRenderer renderer) - : Controller + : ServiceController { [HttpGet] public IActionResult Get() diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs index abd04c8d..0b33c575 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs @@ -3,6 +3,7 @@ using Dfe.EarlyYearsQualification.Content.Renderers.Entities; using Dfe.EarlyYearsQualification.Content.Services; using Dfe.EarlyYearsQualification.Web.Constants; +using Dfe.EarlyYearsQualification.Web.Controllers.Base; using Dfe.EarlyYearsQualification.Web.Models.Content; using Microsoft.AspNetCore.Mvc; @@ -13,7 +14,7 @@ public class QuestionsController( ILogger logger, IContentService contentService, IHtmlRenderer renderer) - : Controller + : ServiceController { private const string Questions = "Questions"; diff --git a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs new file mode 100644 index 00000000..fd3ba6fb --- /dev/null +++ b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs @@ -0,0 +1,23 @@ +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; + +namespace Dfe.EarlyYearsQualification.Web.Filters; + +public class ChallengeResourceFilterAttribute : Attribute, IResourceFilter +{ + private const string ChallengeSecret = "CX"; + + public void OnResourceExecuting(ResourceExecutingContext context) + { + if (!context.HttpContext.Request.Cookies.ContainsKey("auth-secret") + || !context.HttpContext.Request.Cookies["auth-secret"]!.Equals(ChallengeSecret)) + { + context.Result = new RedirectResult("/challenge"); + } + } + + public void OnResourceExecuted(ResourceExecutedContext context) + { + // do nothing + } +} \ No newline at end of file From 0f28df957619f7261ae997fc3fdb5bba8fbd3b64 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Thu, 6 Jun 2024 11:55:47 +0100 Subject: [PATCH 02/28] Simple cookie-based solution working with hard-coded secret. --- .../Controllers/Base/ServiceController.cs | 2 +- .../Controllers/ChallengeController.cs | 34 ++++++++++-- .../ChallengeResourceFilterAttribute.cs | 33 ++++++++++-- .../Program.cs | 16 +++--- .../Controllers/ChallengeControllerTests.cs | 52 +++++++++++++++++++ 5 files changed, 122 insertions(+), 15 deletions(-) create mode 100644 tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs index bbe81a10..5b90b8ec 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs @@ -3,7 +3,7 @@ namespace Dfe.EarlyYearsQualification.Web.Controllers.Base; -[ChallengeResourceFilter] +[ServiceFilter] public class ServiceController : Controller { } \ No newline at end of file diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs index 20bfaf06..77317b81 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs @@ -1,16 +1,44 @@ +using Dfe.EarlyYearsQualification.Web.Filters; using Microsoft.AspNetCore.Mvc; namespace Dfe.EarlyYearsQualification.Web.Controllers; -[Route("/challenge")] public class ChallengeController( ILogger logger) : Controller { + [Route("/challenge")] [HttpGet] - public Task Index() + public Task Index([FromQuery(Name = "from")] string? from, + [FromQuery(Name = "access-value")] string? accessValue) { + from ??= "/"; + + if (accessValue != null) + { + logger.LogInformation("Challenge secret access value entered successfully"); + + HttpContext.Response.Cookies.Append(ChallengeResourceFilterAttribute.AuthSecretCookieName, accessValue); + return Task.FromResult(new RedirectResult(from)); + } + logger.LogWarning("Challenge page invoked"); - return Task.FromResult(new OkResult()); + + return Task.FromResult(Content($""" + + +

+ If you have been invited to view this preview service, you will have been + given a value that will grant access. Please type in the value and click 'Submit'. +

+
+ + + +
+ + + """, + "text/html")); } } \ No newline at end of file diff --git a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs index fd3ba6fb..d89147a9 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs @@ -1,19 +1,42 @@ +using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; namespace Dfe.EarlyYearsQualification.Web.Filters; -public class ChallengeResourceFilterAttribute : Attribute, IResourceFilter +[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true)] +public class ChallengeResourceFilterAttribute(ILogger logger) + : Attribute, IResourceFilter { - private const string ChallengeSecret = "CX"; + public const string AuthSecretCookieName = "auth-secret"; + public const string Challenge = "CX"; + + private const bool RedirectIsPermanent = false; + private const bool RedirectPreservesMethod = true; public void OnResourceExecuting(ResourceExecutingContext context) { - if (!context.HttpContext.Request.Cookies.ContainsKey("auth-secret") - || !context.HttpContext.Request.Cookies["auth-secret"]!.Equals(ChallengeSecret)) + if (context.HttpContext.Request.Cookies.ContainsKey(AuthSecretCookieName) + && context.HttpContext.Request.Cookies[AuthSecretCookieName]!.Equals(Challenge)) { - context.Result = new RedirectResult("/challenge"); + return; } + + logger.LogWarning($"Access denied by {nameof(ChallengeResourceFilterAttribute)}"); + + var requestedUri = context.HttpContext.Request.GetEncodedUrl(); + + var uriBuilder = new UriBuilder(requestedUri) + { + Path = "/challenge", + Query = $"from={requestedUri}" + }; + + var redirectUri = uriBuilder.Uri; + + context.Result = new RedirectResult(redirectUri.ToString(), + RedirectIsPermanent, + RedirectPreservesMethod); } public void OnResourceExecuted(ResourceExecutedContext context) diff --git a/src/Dfe.EarlyYearsQualification.Web/Program.cs b/src/Dfe.EarlyYearsQualification.Web/Program.cs index a7e0a8af..6f76db45 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Program.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Program.cs @@ -4,6 +4,7 @@ using Dfe.EarlyYearsQualification.Content.Extensions; using Dfe.EarlyYearsQualification.Content.Services; using Dfe.EarlyYearsQualification.Mock.Extensions; +using Dfe.EarlyYearsQualification.Web.Filters; using Dfe.EarlyYearsQualification.Web.Security; using Dfe.EarlyYearsQualification.Web.Services.CookieService; using Microsoft.AspNetCore.DataProtection; @@ -51,11 +52,14 @@ builder.Services.AddTransient(); builder.Services.AddSingleton(); -builder.Services.AddScoped(x => { - var actionContext = x.GetRequiredService().ActionContext; - var factory = x.GetRequiredService(); - return factory.GetUrlHelper(actionContext!); -}); +builder.Services.AddScoped(x => + { + var actionContext = x.GetRequiredService().ActionContext; + var factory = x.GetRequiredService(); + return factory.GetUrlHelper(actionContext!); + }); + +builder.Services.AddScoped(); builder.Services.AddStaticRobotsTxt(robotsTxtOptions => robotsTxtOptions.DenyAll()); @@ -84,7 +88,7 @@ "default", "{controller=Home}/{action=Index}/{id?}"); -app.Run(); +await app.RunAsync(); [ExcludeFromCodeCoverage] diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs new file mode 100644 index 00000000..b661f59f --- /dev/null +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs @@ -0,0 +1,52 @@ +using Dfe.EarlyYearsQualification.Web.Controllers; +using Dfe.EarlyYearsQualification.Web.Filters; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; + +namespace Dfe.EarlyYearsQualification.UnitTests.Controllers; + +[TestClass] +public class ChallengeControllerTests +{ + [TestMethod] + public async Task GetChallenge_Returns_Page() + { + var controller = new ChallengeController(NullLogger.Instance); + + var result = await controller.Index("/", null); + + result.Should().BeAssignableTo(); + } + + [TestMethod] + public async Task GetChallenge_WithCorrectValue_RedirectsWithCookie() + { + var cookies = new Dictionary(); + + var cookiesMock = new Mock(); + cookiesMock.Setup(c => + c.Append(ChallengeResourceFilterAttribute.AuthSecretCookieName, + ChallengeResourceFilterAttribute.Challenge)) + .Callback((string k, string v) => cookies.Add(k, v)); + + var mockContext = new Mock(); + mockContext.SetupGet(c => c.Response.Cookies).Returns(cookiesMock.Object); + + var controller = new ChallengeController(NullLogger.Instance); + controller.ControllerContext = new ControllerContext + { + HttpContext = mockContext.Object + }; + + var result = await controller.Index("/url", ChallengeResourceFilterAttribute.Challenge); + + result.Should().BeAssignableTo(); + + cookies.Should().ContainKey(ChallengeResourceFilterAttribute.AuthSecretCookieName); + cookies[ChallengeResourceFilterAttribute.AuthSecretCookieName].Should() + .Be(ChallengeResourceFilterAttribute.Challenge); + } +} \ No newline at end of file From 8e35a35e58ff5bd8eb770c34a91c1b64ee287fbc Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Fri, 7 Jun 2024 18:32:21 +0100 Subject: [PATCH 03/28] Postback not quite working yet --- .../Controllers/ChallengeController.cs | 74 ++++--- .../ChallengeResourceFilterAttribute.cs | 38 ++-- .../Models/ChallengeModel.cs | 8 + .../Views/Challenge/EntryForm.cshtml | 24 ++ .../Controllers/ChallengeControllerTests.cs | 116 +++++++++- .../ChallengeResourceFilterAttributeTests.cs | 208 ++++++++++++++++++ 6 files changed, 422 insertions(+), 46 deletions(-) create mode 100644 src/Dfe.EarlyYearsQualification.Web/Models/ChallengeModel.cs create mode 100644 src/Dfe.EarlyYearsQualification.Web/Views/Challenge/EntryForm.cshtml create mode 100644 tests/Dfe.EarlyYearsQualification.UnitTests/Filters/ChallengeResourceFilterAttributeTests.cs diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs index 77317b81..51fc8a6a 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs @@ -1,44 +1,68 @@ using Dfe.EarlyYearsQualification.Web.Filters; +using Dfe.EarlyYearsQualification.Web.Models; using Microsoft.AspNetCore.Mvc; namespace Dfe.EarlyYearsQualification.Web.Controllers; public class ChallengeController( - ILogger logger) + ILogger logger, + IUrlHelper urlHelper) : Controller { - [Route("/challenge")] + private const string DefaultRedirectAddress = "/"; + [HttpGet] - public Task Index([FromQuery(Name = "from")] string? from, - [FromQuery(Name = "access-value")] string? accessValue) + public Task Index([FromQuery] ChallengeModel model) { - from ??= "/"; + if (!ModelState.IsValid) + { + logger.LogWarning("Invalid challenge model (get)"); + } + + model.RedirectAddress = SanitiseReferralAddress(model.RedirectAddress); + + logger.LogWarning("Challenge page invoked"); - if (accessValue != null) + return Task.FromResult(View("EntryForm", model)); + } + + [HttpPost] + public Task Post([FromForm] ChallengeModel model) + { + if (!ModelState.IsValid) + { + logger.LogWarning("Invalid challenge model (post)"); + } + + var referralAddress = SanitiseReferralAddress(model.RedirectAddress); + + if (model.Value != null) { logger.LogInformation("Challenge secret access value entered successfully"); - HttpContext.Response.Cookies.Append(ChallengeResourceFilterAttribute.AuthSecretCookieName, accessValue); - return Task.FromResult(new RedirectResult(from)); + SetAuthSecretCookie(model.Value); + return Task.FromResult(new RedirectResult(referralAddress)); } - logger.LogWarning("Challenge page invoked"); + return Index(model); + } + + private void SetAuthSecretCookie(string accessValue) + { + HttpContext.Response + .Cookies + .Append(ChallengeResourceFilterAttribute.AuthSecretCookieName, accessValue); + } + + private string SanitiseReferralAddress(string? from) + { + var redirectAddress = from ?? DefaultRedirectAddress; + + if (!urlHelper.IsLocalUrl(redirectAddress)) + { + redirectAddress = DefaultRedirectAddress; + } - return Task.FromResult(Content($""" - - -

- If you have been invited to view this preview service, you will have been - given a value that will grant access. Please type in the value and click 'Submit'. -

-
- - - -
- - - """, - "text/html")); + return redirectAddress; } } \ No newline at end of file diff --git a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs index d89147a9..7c1e4a8a 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs @@ -9,34 +9,42 @@ public class ChallengeResourceFilterAttribute(ILogger + + + + Challenge + + +

+ If you have been invited to view this preview service, you will have been + given a value that will grant access. Please type in the value and click 'Submit'. +

+
+ + + +
+ + \ No newline at end of file diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs index b661f59f..825867aa 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs @@ -1,5 +1,6 @@ using Dfe.EarlyYearsQualification.Web.Controllers; using Dfe.EarlyYearsQualification.Web.Filters; +using Dfe.EarlyYearsQualification.Web.Models; using FluentAssertions; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -14,16 +15,111 @@ public class ChallengeControllerTests [TestMethod] public async Task GetChallenge_Returns_Page() { - var controller = new ChallengeController(NullLogger.Instance); + const string from = "/"; - var result = await controller.Index("/", null); + var mockUrlHelper = new Mock(); + mockUrlHelper.Setup(u => u.IsLocalUrl(It.IsAny())) + .Returns(true); - result.Should().BeAssignableTo(); + var controller = new ChallengeController(NullLogger.Instance, + mockUrlHelper.Object); + + controller.ControllerContext = new ControllerContext + { + HttpContext = new DefaultHttpContext() + }; + + var result = await controller.Index(new ChallengeModel { RedirectAddress = "/", Value = null }); + + result.Should().BeAssignableTo(); + + var content = (ViewResult)result; + + content.Model.Should().BeAssignableTo() + .Which + .RedirectAddress.Should().Be(from); + } + + [TestMethod] + public async Task GetChallenge_NonLocalFrom_Returns_Page_With_BaseFrom() + { + const string from = "https://google.co.uk"; + + var mockUrlHelper = new Mock(); + mockUrlHelper.Setup(u => u.IsLocalUrl(It.IsAny())) + .Returns(false); + + var controller = new ChallengeController(NullLogger.Instance, + mockUrlHelper.Object); + + controller.ControllerContext = new ControllerContext + { + HttpContext = new DefaultHttpContext() + }; + + var result = await controller.Index(new ChallengeModel { RedirectAddress = from, Value = null }); + + result.Should().BeAssignableTo(); + + var content = (ViewResult)result; + + content.Model.Should().BeAssignableTo() + .Which + .RedirectAddress.Should().Be(from); + } + + [TestMethod] + public async Task PostChallenge_WithCorrectValue_RedirectsWithCookie() + { + const string from = "/cookies"; + + var mockUrlHelper = new Mock(); + mockUrlHelper.Setup(u => u.IsLocalUrl(It.IsAny())) + .Returns(true); + + var cookies = new Dictionary(); + + var cookiesMock = new Mock(); + cookiesMock.Setup(c => + c.Append(ChallengeResourceFilterAttribute.AuthSecretCookieName, + ChallengeResourceFilterAttribute.Challenge)) + .Callback((string k, string v) => cookies.Add(k, v)); + + var mockContext = new Mock(); + mockContext.SetupGet(c => c.Response.Cookies).Returns(cookiesMock.Object); + + var controller = new ChallengeController(NullLogger.Instance, + mockUrlHelper.Object); + controller.ControllerContext = new ControllerContext + { + HttpContext = mockContext.Object + }; + + var result = await controller.Post(new ChallengeModel + { + RedirectAddress = from, + Value = ChallengeResourceFilterAttribute.Challenge + }); + + result.Should().BeAssignableTo(); + + var redirect = (RedirectResult)result; + redirect.Url.Should().Be("/cookies"); + + cookies.Should().ContainKey(ChallengeResourceFilterAttribute.AuthSecretCookieName); + cookies[ChallengeResourceFilterAttribute.AuthSecretCookieName].Should() + .Be(ChallengeResourceFilterAttribute.Challenge); } [TestMethod] - public async Task GetChallenge_WithCorrectValue_RedirectsWithCookie() + public async Task PostChallenge_WithCorrectValue_ButNonLocalFrom_RedirectsWithCookie_ToBaseUrl() { + const string from = "https://google.co.uk"; + + var mockUrlHelper = new Mock(); + mockUrlHelper.Setup(u => u.IsLocalUrl(It.IsAny())) + .Returns(false); // NB: behaviour relies on UrlHelper correctly determining non-local URLs + var cookies = new Dictionary(); var cookiesMock = new Mock(); @@ -35,16 +131,24 @@ public async Task GetChallenge_WithCorrectValue_RedirectsWithCookie() var mockContext = new Mock(); mockContext.SetupGet(c => c.Response.Cookies).Returns(cookiesMock.Object); - var controller = new ChallengeController(NullLogger.Instance); + var controller = new ChallengeController(NullLogger.Instance, + mockUrlHelper.Object); controller.ControllerContext = new ControllerContext { HttpContext = mockContext.Object }; - var result = await controller.Index("/url", ChallengeResourceFilterAttribute.Challenge); + var result = await controller.Post(new ChallengeModel + { + RedirectAddress = from, + Value = ChallengeResourceFilterAttribute.Challenge + }); result.Should().BeAssignableTo(); + var redirect = (RedirectResult)result; + redirect.Url.Should().Be("/"); + cookies.Should().ContainKey(ChallengeResourceFilterAttribute.AuthSecretCookieName); cookies[ChallengeResourceFilterAttribute.AuthSecretCookieName].Should() .Be(ChallengeResourceFilterAttribute.Challenge); diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Filters/ChallengeResourceFilterAttributeTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Filters/ChallengeResourceFilterAttributeTests.cs new file mode 100644 index 00000000..ee2fc707 --- /dev/null +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Filters/ChallengeResourceFilterAttributeTests.cs @@ -0,0 +1,208 @@ +using Dfe.EarlyYearsQualification.UnitTests.Extensions; +using Dfe.EarlyYearsQualification.Web.Filters; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; + +namespace Dfe.EarlyYearsQualification.UnitTests.Filters; + +[TestClass] +public class ChallengeResourceFilterAttributeTests +{ + [TestMethod] + public void ExecuteFilter_NoSecretValue_RedirectsToChallenge() + { + var filter = new ChallengeResourceFilterAttribute(NullLogger.Instance); + + var httpContext = new DefaultHttpContext + { + Request = + { + Scheme = "https", + Host = new HostString("localhost"), + Path = "/start" + } + }; + + var actionContext = new ActionContext(httpContext, + new RouteData(), + new ActionDescriptor(), + new ModelStateDictionary()); + + var resourceExecutingContext = new ResourceExecutingContext(actionContext, + new List(), + new List()); + + filter.OnResourceExecuting(resourceExecutingContext); + + var result = resourceExecutingContext.Result; + + result.Should().BeAssignableTo(); + + var redirect = (RedirectToActionResult)result!; + + redirect.ActionName.Should().Be("Index"); + redirect.ControllerName.Should().Be("Challenge"); + redirect.RouteValues.Should().ContainKey("redirectAddress"); + redirect.Permanent.Should().BeFalse(); + redirect.PreserveMethod.Should().BeFalse(); + } + + [TestMethod] + public void ExecuteFilter_NoSecretValue_LogsWarning() + { + var mockLogger = new Mock>(); + + var filter = new ChallengeResourceFilterAttribute(mockLogger.Object); + + var httpContext = new DefaultHttpContext + { + Request = + { + Scheme = "https", + Host = new HostString("localhost"), + Path = "/start" + } + }; + + var actionContext = new ActionContext(httpContext, + new RouteData(), + new ActionDescriptor(), + new ModelStateDictionary()); + + var resourceExecutingContext = new ResourceExecutingContext(actionContext, + new List(), + new List()); + + filter.OnResourceExecuting(resourceExecutingContext); + + mockLogger.VerifyWarning($"Access denied by {nameof(ChallengeResourceFilterAttribute)}"); + } + + [TestMethod] + public void ExecuteFilter_CorrectSecretValue_PassesThrough() + { + var filter = new ChallengeResourceFilterAttribute(NullLogger.Instance); + + var httpContext = new DefaultHttpContext + { + Request = + { + Scheme = "https", + Host = new HostString("localhost"), + Path = "/start" + } + }; + + var cookie = new[] + { + $"{ChallengeResourceFilterAttribute.AuthSecretCookieName}={ChallengeResourceFilterAttribute.Challenge}" + }; + + httpContext.Request.Headers["Cookie"] = cookie; + + var actionContext = new ActionContext(httpContext, + new RouteData(), + new ActionDescriptor(), + new ModelStateDictionary()); + + var resourceExecutingContext = new ResourceExecutingContext(actionContext, + new List(), + new List()); + + filter.OnResourceExecuting(resourceExecutingContext); + + resourceExecutingContext.Result.Should().BeNull(); + } + + [TestMethod] + public void ExecuteFilter_IncorrectSecretValue_RedirectsToChallenge() + { + var filter = new ChallengeResourceFilterAttribute(NullLogger.Instance); + + var httpContext = new DefaultHttpContext + { + Request = + { + Scheme = "https", + Host = new HostString("localhost"), + Path = "/start" + } + }; + + var cookie = new[] + { + $"{ChallengeResourceFilterAttribute.AuthSecretCookieName}=not-{ChallengeResourceFilterAttribute.Challenge}" + }; + + httpContext.Request.Headers["Cookie"] = cookie; + + var actionContext = new ActionContext(httpContext, + new RouteData(), + new ActionDescriptor(), + new ModelStateDictionary()); + + var resourceExecutingContext = new ResourceExecutingContext(actionContext, + new List(), + new List()); + + filter.OnResourceExecuting(resourceExecutingContext); + + var result = resourceExecutingContext.Result; + + result.Should().BeAssignableTo(); + + var redirect = (RedirectToActionResult)result!; + + redirect.ActionName.Should().Be("Index"); + redirect.ControllerName.Should().Be("Challenge"); + redirect.RouteValues.Should().ContainKey("redirectAddress"); + redirect.Permanent.Should().BeFalse(); + redirect.PreserveMethod.Should().BeFalse(); + } + + [TestMethod] + public void ExecuteFilter_IncorrectSecretValue_LogsWarning() + { + var logger = new Mock>(); + + var filter = new ChallengeResourceFilterAttribute(logger.Object); + + var httpContext = new DefaultHttpContext + { + Request = + { + Scheme = "https", + Host = new HostString("localhost"), + Path = "/start" + } + }; + + var cookie = new[] + { + $"{ChallengeResourceFilterAttribute.AuthSecretCookieName}=not-{ChallengeResourceFilterAttribute.Challenge}" + }; + + httpContext.Request.Headers["Cookie"] = cookie; + + var actionContext = new ActionContext(httpContext, + new RouteData(), + new ActionDescriptor(), + new ModelStateDictionary()); + + var resourceExecutingContext = new ResourceExecutingContext(actionContext, + new List(), + new List()); + + filter.OnResourceExecuting(resourceExecutingContext); + + logger.VerifyWarning($"Access denied by {nameof(ChallengeResourceFilterAttribute)} (incorrect value submitted)"); + } +} \ No newline at end of file From 551496a58f3a4b03438cbe52fb32c6ed85c3fa9b Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Fri, 7 Jun 2024 18:33:55 +0100 Subject: [PATCH 04/28] Fix assertion --- .../Controllers/ChallengeControllerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs index 825867aa..2fc97411 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs @@ -65,7 +65,7 @@ public async Task GetChallenge_NonLocalFrom_Returns_Page_With_BaseFrom() content.Model.Should().BeAssignableTo() .Which - .RedirectAddress.Should().Be(from); + .RedirectAddress.Should().Be("/"); } [TestMethod] From b9e66210155bbd178ff1fda446a346ce333e2010 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Mon, 10 Jun 2024 14:25:59 +0100 Subject: [PATCH 05/28] Converted raw form to cshtml form helper. Only relative path addresses are considered "local" URLs. --- .../Filters/ChallengeResourceFilterAttribute.cs | 5 ++--- .../Views/Challenge/EntryForm.cshtml | 7 ++++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs index 7c1e4a8a..f663ab37 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs @@ -1,4 +1,3 @@ -using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; @@ -36,12 +35,12 @@ public void OnResourceExecuting(ResourceExecutingContext context) logger.LogWarning(warningMessage); - var requestedUri = context.HttpContext.Request.GetEncodedUrl(); + var requestedPath = context.HttpContext.Request.Path; context.Result = new RedirectToActionResult("Index", "Challenge", new { - redirectAddress = requestedUri + redirectAddress = requestedPath }, RedirectIsPermanent, RedirectPreservesMethod); diff --git a/src/Dfe.EarlyYearsQualification.Web/Views/Challenge/EntryForm.cshtml b/src/Dfe.EarlyYearsQualification.Web/Views/Challenge/EntryForm.cshtml index dc0d728c..ed70ddde 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Views/Challenge/EntryForm.cshtml +++ b/src/Dfe.EarlyYearsQualification.Web/Views/Challenge/EntryForm.cshtml @@ -15,10 +15,11 @@ If you have been invited to view this preview service, you will have been given a value that will grant access. Please type in the value and click 'Submit'.

-
- +@using (Html.BeginForm("Post", "Challenge", new { }, FormMethod.Post)) +{ + -
+} \ No newline at end of file From c2cd1c237165070d241f28599de1d2a535951da8 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Mon, 10 Jun 2024 16:52:48 +0100 Subject: [PATCH 06/28] Allow multiple secret values. Secret values configurable. --- .../ChallengeResourceFilterAttribute.cs | 26 +++- .../Controllers/ChallengeControllerTests.cs | 14 +- .../ChallengeResourceFilterAttributeTests.cs | 126 ++++++++++++++++-- 3 files changed, 147 insertions(+), 19 deletions(-) diff --git a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs index f663ab37..23566c27 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs @@ -4,7 +4,9 @@ namespace Dfe.EarlyYearsQualification.Web.Filters; [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true)] -public class ChallengeResourceFilterAttribute(ILogger logger) +public class ChallengeResourceFilterAttribute( + ILogger logger, + IConfiguration configuration) : Attribute, IResourceFilter { public const string AuthSecretCookieName = "auth-secret"; @@ -12,16 +14,32 @@ public class ChallengeResourceFilterAttribute(ILogger(); + } } public void OnResourceExecuting(ResourceExecutingContext context) { var cookieIsPresent = context.HttpContext.Request.Cookies.ContainsKey(AuthSecretCookieName); - if (cookieIsPresent && context.HttpContext.Request.Cookies[AuthSecretCookieName]!.Equals(Challenge)) + if (ChallengeValues == null || ChallengeValues.Length == 0) + { + logger.LogError("Service access keys not configured"); + context.Result = new RedirectToActionResult("Error", + "Home", + new { }, + RedirectIsPermanent, + RedirectPreservesMethod); + } + + if (cookieIsPresent && ChallengeValues!.Contains(context.HttpContext.Request.Cookies[AuthSecretCookieName])) { return; } diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs index 2fc97411..56e15275 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ChallengeControllerTests.cs @@ -72,6 +72,7 @@ public async Task GetChallenge_NonLocalFrom_Returns_Page_With_BaseFrom() public async Task PostChallenge_WithCorrectValue_RedirectsWithCookie() { const string from = "/cookies"; + const string accessKey = "CX"; var mockUrlHelper = new Mock(); mockUrlHelper.Setup(u => u.IsLocalUrl(It.IsAny())) @@ -82,7 +83,7 @@ public async Task PostChallenge_WithCorrectValue_RedirectsWithCookie() var cookiesMock = new Mock(); cookiesMock.Setup(c => c.Append(ChallengeResourceFilterAttribute.AuthSecretCookieName, - ChallengeResourceFilterAttribute.Challenge)) + accessKey)) .Callback((string k, string v) => cookies.Add(k, v)); var mockContext = new Mock(); @@ -98,7 +99,7 @@ public async Task PostChallenge_WithCorrectValue_RedirectsWithCookie() var result = await controller.Post(new ChallengeModel { RedirectAddress = from, - Value = ChallengeResourceFilterAttribute.Challenge + Value = accessKey }); result.Should().BeAssignableTo(); @@ -108,13 +109,14 @@ public async Task PostChallenge_WithCorrectValue_RedirectsWithCookie() cookies.Should().ContainKey(ChallengeResourceFilterAttribute.AuthSecretCookieName); cookies[ChallengeResourceFilterAttribute.AuthSecretCookieName].Should() - .Be(ChallengeResourceFilterAttribute.Challenge); + .Be(accessKey); } [TestMethod] public async Task PostChallenge_WithCorrectValue_ButNonLocalFrom_RedirectsWithCookie_ToBaseUrl() { const string from = "https://google.co.uk"; + const string accessKey = "CX"; var mockUrlHelper = new Mock(); mockUrlHelper.Setup(u => u.IsLocalUrl(It.IsAny())) @@ -125,7 +127,7 @@ public async Task PostChallenge_WithCorrectValue_ButNonLocalFrom_RedirectsWithCo var cookiesMock = new Mock(); cookiesMock.Setup(c => c.Append(ChallengeResourceFilterAttribute.AuthSecretCookieName, - ChallengeResourceFilterAttribute.Challenge)) + accessKey)) .Callback((string k, string v) => cookies.Add(k, v)); var mockContext = new Mock(); @@ -141,7 +143,7 @@ public async Task PostChallenge_WithCorrectValue_ButNonLocalFrom_RedirectsWithCo var result = await controller.Post(new ChallengeModel { RedirectAddress = from, - Value = ChallengeResourceFilterAttribute.Challenge + Value = accessKey }); result.Should().BeAssignableTo(); @@ -151,6 +153,6 @@ public async Task PostChallenge_WithCorrectValue_ButNonLocalFrom_RedirectsWithCo cookies.Should().ContainKey(ChallengeResourceFilterAttribute.AuthSecretCookieName); cookies[ChallengeResourceFilterAttribute.AuthSecretCookieName].Should() - .Be(ChallengeResourceFilterAttribute.Challenge); + .Be(accessKey); } } \ No newline at end of file diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Filters/ChallengeResourceFilterAttributeTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Filters/ChallengeResourceFilterAttributeTests.cs index ee2fc707..f6aa6fcf 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Filters/ChallengeResourceFilterAttributeTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Filters/ChallengeResourceFilterAttributeTests.cs @@ -7,6 +7,7 @@ using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Moq; @@ -19,7 +20,19 @@ public class ChallengeResourceFilterAttributeTests [TestMethod] public void ExecuteFilter_NoSecretValue_RedirectsToChallenge() { - var filter = new ChallengeResourceFilterAttribute(NullLogger.Instance); + const string accessKey = "CX"; + + var dic = new Dictionary + { + { "ServiceAccess:Keys:0", accessKey } + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(dic) + .Build(); + + var filter = new ChallengeResourceFilterAttribute(NullLogger.Instance, + configuration); var httpContext = new DefaultHttpContext { @@ -58,9 +71,20 @@ public void ExecuteFilter_NoSecretValue_RedirectsToChallenge() [TestMethod] public void ExecuteFilter_NoSecretValue_LogsWarning() { + const string accessKey = "CX"; + + var dic = new Dictionary + { + { "ServiceAccess:Keys:0", accessKey } + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(dic) + .Build(); + var mockLogger = new Mock>(); - var filter = new ChallengeResourceFilterAttribute(mockLogger.Object); + var filter = new ChallengeResourceFilterAttribute(mockLogger.Object, configuration); var httpContext = new DefaultHttpContext { @@ -87,9 +111,21 @@ public void ExecuteFilter_NoSecretValue_LogsWarning() } [TestMethod] - public void ExecuteFilter_CorrectSecretValue_PassesThrough() + public void ExecuteFilter_CorrectSecretValue1_PassesThrough() { - var filter = new ChallengeResourceFilterAttribute(NullLogger.Instance); + const string accessKey = "CX"; + + var dic = new Dictionary + { + { "ServiceAccess:Keys:0", accessKey } + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(dic) + .Build(); + + var filter = new ChallengeResourceFilterAttribute(NullLogger.Instance, + configuration); var httpContext = new DefaultHttpContext { @@ -103,7 +139,56 @@ public void ExecuteFilter_CorrectSecretValue_PassesThrough() var cookie = new[] { - $"{ChallengeResourceFilterAttribute.AuthSecretCookieName}={ChallengeResourceFilterAttribute.Challenge}" + $"{ChallengeResourceFilterAttribute.AuthSecretCookieName}={accessKey}" + }; + + httpContext.Request.Headers["Cookie"] = cookie; + + var actionContext = new ActionContext(httpContext, + new RouteData(), + new ActionDescriptor(), + new ModelStateDictionary()); + + var resourceExecutingContext = new ResourceExecutingContext(actionContext, + new List(), + new List()); + + filter.OnResourceExecuting(resourceExecutingContext); + + resourceExecutingContext.Result.Should().BeNull(); + } + + [TestMethod] + public void ExecuteFilter_CorrectSecretValue2_PassesThrough() + { + const string accessKey = "CX"; + + var dic = new Dictionary + { + { "ServiceAccess:Keys:0", "SomeKey" }, // <== NB, not using the first key in the array + { "ServiceAccess:Keys:1", accessKey } + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(dic) + .Build(); + + var filter = new ChallengeResourceFilterAttribute(NullLogger.Instance, + configuration); + + var httpContext = new DefaultHttpContext + { + Request = + { + Scheme = "https", + Host = new HostString("localhost"), + Path = "/start" + } + }; + + var cookie = new[] + { + $"{ChallengeResourceFilterAttribute.AuthSecretCookieName}={accessKey}" }; httpContext.Request.Headers["Cookie"] = cookie; @@ -125,7 +210,19 @@ public void ExecuteFilter_CorrectSecretValue_PassesThrough() [TestMethod] public void ExecuteFilter_IncorrectSecretValue_RedirectsToChallenge() { - var filter = new ChallengeResourceFilterAttribute(NullLogger.Instance); + const string accessKey = "CX"; + + var dic = new Dictionary + { + { "ServiceAccess:Keys:0", accessKey } + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(dic) + .Build(); + + var filter = new ChallengeResourceFilterAttribute(NullLogger.Instance, + configuration); var httpContext = new DefaultHttpContext { @@ -139,7 +236,7 @@ public void ExecuteFilter_IncorrectSecretValue_RedirectsToChallenge() var cookie = new[] { - $"{ChallengeResourceFilterAttribute.AuthSecretCookieName}=not-{ChallengeResourceFilterAttribute.Challenge}" + $"{ChallengeResourceFilterAttribute.AuthSecretCookieName}=not-{accessKey}" }; httpContext.Request.Headers["Cookie"] = cookie; @@ -171,9 +268,20 @@ public void ExecuteFilter_IncorrectSecretValue_RedirectsToChallenge() [TestMethod] public void ExecuteFilter_IncorrectSecretValue_LogsWarning() { + const string accessKey = "CX"; + + var dic = new Dictionary + { + { "ServiceAccess:Keys:0", accessKey } + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(dic) + .Build(); + var logger = new Mock>(); - var filter = new ChallengeResourceFilterAttribute(logger.Object); + var filter = new ChallengeResourceFilterAttribute(logger.Object, configuration); var httpContext = new DefaultHttpContext { @@ -187,7 +295,7 @@ public void ExecuteFilter_IncorrectSecretValue_LogsWarning() var cookie = new[] { - $"{ChallengeResourceFilterAttribute.AuthSecretCookieName}=not-{ChallengeResourceFilterAttribute.Challenge}" + $"{ChallengeResourceFilterAttribute.AuthSecretCookieName}=not-{accessKey}" }; httpContext.Request.Headers["Cookie"] = cookie; From 450fcd0544b7a27ff8183548806a814d6e9db85c Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Mon, 10 Jun 2024 17:51:33 +0100 Subject: [PATCH 07/28] Only add the challenge filter if configured. New no-op "no challenge" filter if service access unchallenged. --- .../Controllers/Base/ServiceController.cs | 2 +- .../Filters/ChallengeResourceFilterAttribute.cs | 6 +++--- .../IChallengeResourceFilterAttribute.cs | 5 +++++ .../NoChallengeResourceFilterAttribute.cs | 17 +++++++++++++++++ src/Dfe.EarlyYearsQualification.Web/Program.cs | 12 +++++++++++- 5 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 src/Dfe.EarlyYearsQualification.Web/Filters/IChallengeResourceFilterAttribute.cs create mode 100644 src/Dfe.EarlyYearsQualification.Web/Filters/NoChallengeResourceFilterAttribute.cs diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs index 5b90b8ec..993eeb3e 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs @@ -3,7 +3,7 @@ namespace Dfe.EarlyYearsQualification.Web.Controllers.Base; -[ServiceFilter] +[ServiceFilter] public class ServiceController : Controller { } \ No newline at end of file diff --git a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs index 23566c27..c3805418 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs @@ -7,7 +7,7 @@ namespace Dfe.EarlyYearsQualification.Web.Filters; public class ChallengeResourceFilterAttribute( ILogger logger, IConfiguration configuration) - : Attribute, IResourceFilter + : Attribute, IChallengeResourceFilterAttribute { public const string AuthSecretCookieName = "auth-secret"; @@ -27,8 +27,6 @@ private string[]? ChallengeValues public void OnResourceExecuting(ResourceExecutingContext context) { - var cookieIsPresent = context.HttpContext.Request.Cookies.ContainsKey(AuthSecretCookieName); - if (ChallengeValues == null || ChallengeValues.Length == 0) { logger.LogError("Service access keys not configured"); @@ -39,6 +37,8 @@ public void OnResourceExecuting(ResourceExecutingContext context) RedirectPreservesMethod); } + var cookieIsPresent = context.HttpContext.Request.Cookies.ContainsKey(AuthSecretCookieName); + if (cookieIsPresent && ChallengeValues!.Contains(context.HttpContext.Request.Cookies[AuthSecretCookieName])) { return; diff --git a/src/Dfe.EarlyYearsQualification.Web/Filters/IChallengeResourceFilterAttribute.cs b/src/Dfe.EarlyYearsQualification.Web/Filters/IChallengeResourceFilterAttribute.cs new file mode 100644 index 00000000..42558448 --- /dev/null +++ b/src/Dfe.EarlyYearsQualification.Web/Filters/IChallengeResourceFilterAttribute.cs @@ -0,0 +1,5 @@ +using Microsoft.AspNetCore.Mvc.Filters; + +namespace Dfe.EarlyYearsQualification.Web.Filters; + +public interface IChallengeResourceFilterAttribute : IResourceFilter; \ No newline at end of file diff --git a/src/Dfe.EarlyYearsQualification.Web/Filters/NoChallengeResourceFilterAttribute.cs b/src/Dfe.EarlyYearsQualification.Web/Filters/NoChallengeResourceFilterAttribute.cs new file mode 100644 index 00000000..978a46c9 --- /dev/null +++ b/src/Dfe.EarlyYearsQualification.Web/Filters/NoChallengeResourceFilterAttribute.cs @@ -0,0 +1,17 @@ +using Microsoft.AspNetCore.Mvc.Filters; + +namespace Dfe.EarlyYearsQualification.Web.Filters; + +[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true)] +public class NoChallengeResourceFilterAttribute : Attribute, IChallengeResourceFilterAttribute +{ + public void OnResourceExecuting(ResourceExecutingContext context) + { + // do nothing + } + + public void OnResourceExecuted(ResourceExecutedContext context) + { + // do nothing + } +} \ No newline at end of file diff --git a/src/Dfe.EarlyYearsQualification.Web/Program.cs b/src/Dfe.EarlyYearsQualification.Web/Program.cs index 6f76db45..9de33f8b 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Program.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Program.cs @@ -59,7 +59,17 @@ return factory.GetUrlHelper(actionContext!); }); -builder.Services.AddScoped(); +var accessIsChallenged = !builder.Configuration.GetValue("ServiceAccess:Unchallenged"); +// ...by default, challenge the user for the secret value unless that's explicitly turned off + +if (accessIsChallenged) +{ + builder.Services.AddScoped(); +} +else +{ + builder.Services.AddSingleton(); +} builder.Services.AddStaticRobotsTxt(robotsTxtOptions => robotsTxtOptions.DenyAll()); From 236ce517bf2525a331effa3715a04d748cfbf6cd Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Tue, 11 Jun 2024 10:34:52 +0100 Subject: [PATCH 08/28] Test that only the correct controllers are unguarded. Move Error endpoint to its own new unguarded controller. --- .../AccessibilityStatementController.cs | 2 +- .../Controllers/AdviceController.cs | 2 +- .../Controllers/Base/ServiceController.cs | 11 +++++-- .../Controllers/CookiesController.cs | 2 +- .../Controllers/ErrorController.cs | 17 ++++++++++ .../Controllers/HealthController.cs | 3 +- .../Controllers/HomeController.cs | 10 +----- .../QualificationDetailsController.cs | 4 +-- .../Controllers/QuestionsController.cs | 2 +- .../ChallengeResourceFilterAttribute.cs | 20 ++++++------ .../Program.cs | 2 +- .../AccessibilityStatementControllerTests.cs | 2 +- .../Controllers/AdviceControllerTests.cs | 4 +-- .../Controllers/ControllerAccessTests.cs | 32 +++++++++++++++++++ .../Controllers/CookiesControllerTests.cs | 2 +- .../Controllers/HomeControllerTests.cs | 3 +- .../QualificationDetailsControllerTests.cs | 8 ++--- .../Controllers/QuestionsControllerTests.cs | 8 ++--- 18 files changed, 91 insertions(+), 43 deletions(-) create mode 100644 src/Dfe.EarlyYearsQualification.Web/Controllers/ErrorController.cs create mode 100644 tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ControllerAccessTests.cs diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/AccessibilityStatementController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/AccessibilityStatementController.cs index b6d0d9f3..80013dc3 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/AccessibilityStatementController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/AccessibilityStatementController.cs @@ -22,7 +22,7 @@ public async Task Index() if (content is null) { logger.LogError("No content for the accessibility statement page"); - return RedirectToAction("Error", "Home"); + return RedirectToAction("Index", "Error"); } var model = await Map(content); diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/AdviceController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/AdviceController.cs index 559e4c8f..5671af63 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/AdviceController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/AdviceController.cs @@ -24,7 +24,7 @@ private async Task GetView(string advicePageId) if (advicePage is null) { logger.LogError("No content for the advice page"); - return RedirectToAction("Error", "Home"); + return RedirectToAction("Index", "Error"); } var model = await Map(advicePage); diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs index 993eeb3e..d2c5c0d2 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/Base/ServiceController.cs @@ -3,7 +3,12 @@ namespace Dfe.EarlyYearsQualification.Web.Controllers.Base; +/// +/// Controller class that is guarded by a +/// so that it is possible, while in private beta and in non-production environments, +/// to configure a secret that must be entered to gain access to the service. +/// All controllers except , +/// and should derive from this type. +/// [ServiceFilter] -public class ServiceController : Controller -{ -} \ No newline at end of file +public class ServiceController : Controller; \ No newline at end of file diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/CookiesController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/CookiesController.cs index 3d7c6975..522e91aa 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/CookiesController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/CookiesController.cs @@ -26,7 +26,7 @@ public async Task Index() if (content is null) { logger.LogError("No content for the cookies page"); - return RedirectToAction("Error", "Home"); + return RedirectToAction("Index", "Error"); } var model = await Map(content); diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/ErrorController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/ErrorController.cs new file mode 100644 index 00000000..e8611a5b --- /dev/null +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/ErrorController.cs @@ -0,0 +1,17 @@ +using System.Diagnostics; +using Dfe.EarlyYearsQualification.Web.Models; +using Microsoft.AspNetCore.Mvc; + +namespace Dfe.EarlyYearsQualification.Web.Controllers; + +[Route("/error")] +public class ErrorController( + ILogger logger) : Controller +{ + [HttpGet] + [ResponseCache(Duration = 0, Location = ResponseCacheLocation.None, NoStore = true)] + public IActionResult Index() + { + return View("Error", new ErrorViewModel { RequestId = Activity.Current?.Id ?? HttpContext.TraceIdentifier }); + } +} \ No newline at end of file diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/HealthController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/HealthController.cs index b1311e87..c5b9c5af 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/HealthController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/HealthController.cs @@ -1,11 +1,10 @@ -using Dfe.EarlyYearsQualification.Web.Controllers.Base; using Microsoft.AspNetCore.Mvc; namespace Dfe.EarlyYearsQualification.Web.Controllers; [ApiController] [Route("[controller]")] -public class HealthController : ServiceController +public class HealthController : Controller { [HttpGet] public IActionResult Get() diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/HomeController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/HomeController.cs index 5d9c30a0..9f08ec12 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/HomeController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/HomeController.cs @@ -1,9 +1,7 @@ -using System.Diagnostics; using Dfe.EarlyYearsQualification.Content.Entities; using Dfe.EarlyYearsQualification.Content.Renderers.Entities; using Dfe.EarlyYearsQualification.Content.Services; using Dfe.EarlyYearsQualification.Web.Controllers.Base; -using Dfe.EarlyYearsQualification.Web.Models; using Dfe.EarlyYearsQualification.Web.Models.Content; using Microsoft.AspNetCore.Mvc; @@ -23,19 +21,13 @@ public async Task Index() if (startPageContent is null) { logger.LogCritical("Start page content not found"); - return RedirectToAction("Error"); + return RedirectToAction("Index", "Error"); } var model = await Map(startPageContent); return View(model); } - [ResponseCache(Duration = 0, Location = ResponseCacheLocation.None, NoStore = true)] - public IActionResult Error() - { - return View(new ErrorViewModel { RequestId = Activity.Current?.Id ?? HttpContext.TraceIdentifier }); - } - private async Task Map(StartPage startPageContent) { return new StartPageModel diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs index 1fcba4b8..6423c2fa 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/QualificationDetailsController.cs @@ -33,7 +33,7 @@ public async Task Index(string qualificationId) if (detailsPageContent is null) { logger.LogError("No content for the qualification details page"); - return RedirectToAction("Error", "Home"); + return RedirectToAction("Index", "Error"); } var qualification = await contentService.GetQualificationById(qualificationId); @@ -43,7 +43,7 @@ public async Task Index(string qualificationId) logger.LogError("Could not find details for qualification with ID: {QualificationId}", loggedQualificationId); - return RedirectToAction("Error", "Home"); + return RedirectToAction("Index", "Error"); } var model = await Map(qualification, detailsPageContent); diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs index 0b33c575..8a8ee765 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/QuestionsController.cs @@ -100,7 +100,7 @@ private async Task GetView(string questionPageId, string actionNa if (questionPage is null) { logger.LogError("No content for the question page"); - return RedirectToAction("Error", "Home"); + return RedirectToAction("Index", "Error"); } var model = await Map(new QuestionModel(), questionPage, actionName, controllerName); diff --git a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs index c3805418..60dfa21e 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs @@ -11,8 +11,8 @@ public class ChallengeResourceFilterAttribute( { public const string AuthSecretCookieName = "auth-secret"; - private const bool RedirectIsPermanent = false; - private const bool RedirectPreservesMethod = false; + private const bool RedirectsArePermanent = false; + private const bool RedirectsPreserveMethod = false; private string[]? ChallengeValues { @@ -30,11 +30,12 @@ public void OnResourceExecuting(ResourceExecutingContext context) if (ChallengeValues == null || ChallengeValues.Length == 0) { logger.LogError("Service access keys not configured"); - context.Result = new RedirectToActionResult("Error", - "Home", + context.Result = new RedirectToActionResult("Index", + "Error", new { }, - RedirectIsPermanent, - RedirectPreservesMethod); + RedirectsArePermanent, + RedirectsPreserveMethod); + return; } var cookieIsPresent = context.HttpContext.Request.Cookies.ContainsKey(AuthSecretCookieName); @@ -55,13 +56,14 @@ public void OnResourceExecuting(ResourceExecutingContext context) var requestedPath = context.HttpContext.Request.Path; - context.Result = new RedirectToActionResult("Index", "Challenge", + context.Result = new RedirectToActionResult("Index", + "Challenge", new { redirectAddress = requestedPath }, - RedirectIsPermanent, - RedirectPreservesMethod); + RedirectsArePermanent, + RedirectsPreserveMethod); } public void OnResourceExecuted(ResourceExecutedContext context) diff --git a/src/Dfe.EarlyYearsQualification.Web/Program.cs b/src/Dfe.EarlyYearsQualification.Web/Program.cs index 9de33f8b..b2f54546 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Program.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Program.cs @@ -59,7 +59,7 @@ return factory.GetUrlHelper(actionContext!); }); -var accessIsChallenged = !builder.Configuration.GetValue("ServiceAccess:Unchallenged"); +var accessIsChallenged = !builder.Configuration.GetValue("ServiceAccess:IsPublic"); // ...by default, challenge the user for the secret value unless that's explicitly turned off if (accessIsChallenged) diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/AccessibilityStatementControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/AccessibilityStatementControllerTests.cs index 6f1e3efb..81ae1220 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/AccessibilityStatementControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/AccessibilityStatementControllerTests.cs @@ -33,7 +33,7 @@ public async Task Index_NoContent_NavigatesToErrorPageAsync() var result = await controller.Index(); result.Should().NotBeNull(); - result.Should().BeEquivalentTo(new RedirectToActionResult("Error", "Home", null)); + result.Should().BeEquivalentTo(new RedirectToActionResult("Index", "Error", null)); mockLogger.VerifyError("No content for the accessibility statement page"); } diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/AdviceControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/AdviceControllerTests.cs index a31977f9..25d0fbf0 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/AdviceControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/AdviceControllerTests.cs @@ -36,8 +36,8 @@ public async Task QualificationOutsideTheUnitedKingdom_ContentServiceReturnsNoAd resultType.Should().NotBeNull(); - resultType!.ActionName.Should().Be("Error"); - resultType.ControllerName.Should().Be("Home"); + resultType!.ActionName.Should().Be("Index"); + resultType.ControllerName.Should().Be("Error"); mockLogger.VerifyError("No content for the advice page"); } diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ControllerAccessTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ControllerAccessTests.cs new file mode 100644 index 00000000..49d5e1e7 --- /dev/null +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/ControllerAccessTests.cs @@ -0,0 +1,32 @@ +using System.Reflection; +using Dfe.EarlyYearsQualification.Web.Controllers; +using Dfe.EarlyYearsQualification.Web.Controllers.Base; +using FluentAssertions; +using Microsoft.AspNetCore.Mvc; + +namespace Dfe.EarlyYearsQualification.UnitTests.Controllers; + +[TestClass] +public class ControllerAccessTests +{ + [TestMethod] + public void OnlyPubliclyAccessibleControllers_Are_Challenge_Error_And_Health() + { + var unguardedControllerTypes = + Assembly.GetAssembly(typeof(HomeController))! + .GetTypes() + .Where(c => + c.IsSubclassOf(typeof(Controller)) + && !c.IsSubclassOf(typeof(ServiceController)) + && c != typeof(ServiceController)); + + unguardedControllerTypes.Should().HaveCount(3) + .And.Contain([ + typeof(ChallengeController), + typeof(ErrorController), + typeof(HealthController) + ], + $"To enable guarding access to the service, controllers should inherit {typeof(ServiceController).FullName}" + ); + } +} \ No newline at end of file diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/CookiesControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/CookiesControllerTests.cs index 952d34b5..4135d4cc 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/CookiesControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/CookiesControllerTests.cs @@ -37,7 +37,7 @@ public async Task Index_NoContent_NavigatesToErrorPageAsync() var result = await controller.Index(); result.Should().NotBeNull(); - result.Should().BeEquivalentTo(new RedirectToActionResult("Error", "Home", null)); + result.Should().BeEquivalentTo(new RedirectToActionResult("Index", "Error", null)); mockLogger.VerifyError("No content for the cookies page"); } diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/HomeControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/HomeControllerTests.cs index dce65861..f8792f6f 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/HomeControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/HomeControllerTests.cs @@ -33,7 +33,8 @@ public async Task Index_ContentServiceReturnsNoContent_RedirectsToErrorPage() var resultType = result as RedirectToActionResult; resultType.Should().NotBeNull(); - resultType!.ActionName.Should().Be("Error"); + resultType!.ActionName.Should().Be("Index"); + resultType.ControllerName.Should().Be("Error"); mockLogger.VerifyCritical("Start page content not found"); } diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs index e3f40999..d81119b2 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QualificationDetailsControllerTests.cs @@ -66,8 +66,8 @@ public async Task Index_ContentServiceReturnsNullDetailsPage_RedirectsToHomeErro var actionResult = (RedirectToActionResult)result; - actionResult.ActionName.Should().Be("Error"); - actionResult.ControllerName.Should().Be("Home"); + actionResult.ActionName.Should().Be("Index"); + actionResult.ControllerName.Should().Be("Error"); mockLogger.VerifyError("No content for the qualification details page"); } @@ -97,8 +97,8 @@ public async Task Index_ContentServiceReturnsNoQualification_RedirectsToErrorPag var resultType = result as RedirectToActionResult; resultType.Should().NotBeNull(); - resultType!.ActionName.Should().Be("Error"); - resultType.ControllerName.Should().Be("Home"); + resultType!.ActionName.Should().Be("Index"); + resultType.ControllerName.Should().Be("Error"); mockLogger.VerifyError("Could not find details for qualification with ID: eyq-145"); } diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QuestionsControllerTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QuestionsControllerTests.cs index df89e2dc..e236c10f 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QuestionsControllerTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Controllers/QuestionsControllerTests.cs @@ -39,8 +39,8 @@ public async Task WhereWasTheQualificationAwarded_ContentServiceReturnsNoQuestio var resultType = result as RedirectToActionResult; result.Should().NotBeNull(); - resultType!.ActionName.Should().Be("Error"); - resultType.ControllerName.Should().Be("Home"); + resultType!.ActionName.Should().Be("Index"); + resultType.ControllerName.Should().Be("Error"); mockLogger.VerifyError("No content for the question page"); } @@ -208,8 +208,8 @@ public async Task WhatLevelIsTheQualification_ContentServiceReturnsNoQuestionPag var resultType = result as RedirectToActionResult; result.Should().NotBeNull(); - resultType!.ActionName.Should().Be("Error"); - resultType.ControllerName.Should().Be("Home"); + resultType!.ActionName.Should().Be("Index"); + resultType.ControllerName.Should().Be("Error"); mockLogger.VerifyError("No content for the question page"); } From 166cca1656928238a8442d44b97ff48c4deb023e Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Tue, 11 Jun 2024 11:15:22 +0100 Subject: [PATCH 09/28] Filter passes through if public access allowed. Doc comments (point to no-op filter for public access). More tests. --- .../ChallengeResourceFilterAttribute.cs | 46 +++- .../ChallengeResourceFilterAttributeTests.cs | 209 ++++++++++++++++++ 2 files changed, 248 insertions(+), 7 deletions(-) diff --git a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs index 60dfa21e..d90d101c 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Filters/ChallengeResourceFilterAttribute.cs @@ -3,6 +3,26 @@ namespace Dfe.EarlyYearsQualification.Web.Filters; +/// +/// Filter attribute that will check the HTTP request for a cookie whose value matches +/// one of the configured keys. Up to four allowable keys are set up in the service config: +/// +/// "ServiceAccess": +/// { +/// "IsPublic": false, +/// "Keys": [ +/// "Key-value-1", +/// "Key-value-2" +/// "Key-value-3" +/// "Key-value-4" +/// ] +/// } +/// +/// "IsPublic" defaults to false. If "IsPublic" is true, it is more efficient to +/// add to the pipeline instead. +/// +/// +/// [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true)] public class ChallengeResourceFilterAttribute( ILogger logger, @@ -14,20 +34,32 @@ public class ChallengeResourceFilterAttribute( private const bool RedirectsArePermanent = false; private const bool RedirectsPreserveMethod = false; - private string[]? ChallengeValues + private bool AllowPublicAccess + { + get { return configuration.GetValue("ServiceAccess:IsPublic"); } + } + + private IEnumerable ConfiguredKeys { get { - return configuration - .GetSection("ServiceAccess") - .GetSection("Keys") - .Get(); + var keys = configuration + .GetSection("ServiceAccess") + .GetSection("Keys") + .Get(); + + return keys == null ? [] : keys.Where(k => !string.IsNullOrWhiteSpace(k)); } } public void OnResourceExecuting(ResourceExecutingContext context) { - if (ChallengeValues == null || ChallengeValues.Length == 0) + if (AllowPublicAccess) + { + return; + } + + if (!ConfiguredKeys.Any()) { logger.LogError("Service access keys not configured"); context.Result = new RedirectToActionResult("Index", @@ -40,7 +72,7 @@ public void OnResourceExecuting(ResourceExecutingContext context) var cookieIsPresent = context.HttpContext.Request.Cookies.ContainsKey(AuthSecretCookieName); - if (cookieIsPresent && ChallengeValues!.Contains(context.HttpContext.Request.Cookies[AuthSecretCookieName])) + if (cookieIsPresent && ConfiguredKeys.Contains(context.HttpContext.Request.Cookies[AuthSecretCookieName])) { return; } diff --git a/tests/Dfe.EarlyYearsQualification.UnitTests/Filters/ChallengeResourceFilterAttributeTests.cs b/tests/Dfe.EarlyYearsQualification.UnitTests/Filters/ChallengeResourceFilterAttributeTests.cs index f6aa6fcf..1a4ac54a 100644 --- a/tests/Dfe.EarlyYearsQualification.UnitTests/Filters/ChallengeResourceFilterAttributeTests.cs +++ b/tests/Dfe.EarlyYearsQualification.UnitTests/Filters/ChallengeResourceFilterAttributeTests.cs @@ -313,4 +313,213 @@ public void ExecuteFilter_IncorrectSecretValue_LogsWarning() logger.VerifyWarning($"Access denied by {nameof(ChallengeResourceFilterAttribute)} (incorrect value submitted)"); } + + [TestMethod] + public void Filter_NoSecretsConfigured_RedirectsToError() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary()) + .Build(); + + var logger = new Mock>(); + + var filter = new ChallengeResourceFilterAttribute(logger.Object, configuration); + + + var httpContext = new DefaultHttpContext + { + Request = + { + Scheme = "https", + Host = new HostString("localhost"), + Path = "/start" + } + }; + + var actionContext = new ActionContext(httpContext, + new RouteData(), + new ActionDescriptor(), + new ModelStateDictionary()); + + var resourceExecutingContext = new ResourceExecutingContext(actionContext, + new List(), + new List()); + + filter.OnResourceExecuting(resourceExecutingContext); + + var result = resourceExecutingContext.Result; + + result.Should().BeAssignableTo(); + + var redirect = (RedirectToActionResult)result!; + + redirect.ActionName.Should().Be("Index"); + redirect.ControllerName.Should().Be("Error"); + } + + [TestMethod] + public void Filter_OnlyEmptySecretsConfigured_RedirectsToError() + { + var dic = new Dictionary + { + { "ServiceAccess:Keys:0", " " }, + { "ServiceAccess:Keys:1", "\t" } + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(dic) + .Build(); + + var logger = new Mock>(); + + var filter = new ChallengeResourceFilterAttribute(logger.Object, configuration); + + + var httpContext = new DefaultHttpContext + { + Request = + { + Scheme = "https", + Host = new HostString("localhost"), + Path = "/start" + } + }; + + var actionContext = new ActionContext(httpContext, + new RouteData(), + new ActionDescriptor(), + new ModelStateDictionary()); + + var resourceExecutingContext = new ResourceExecutingContext(actionContext, + new List(), + new List()); + + filter.OnResourceExecuting(resourceExecutingContext); + + var result = resourceExecutingContext.Result; + + result.Should().BeAssignableTo(); + + var redirect = (RedirectToActionResult)result!; + + redirect.ActionName.Should().Be("Index"); + redirect.ControllerName.Should().Be("Error"); + } + + [TestMethod] + public void Filter_NoSecretsConfigured_LogsError() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary()) + .Build(); + + var mockLogger = new Mock>(); + + var filter = new ChallengeResourceFilterAttribute(mockLogger.Object, configuration); + + var httpContext = new DefaultHttpContext + { + Request = + { + Scheme = "https", + Host = new HostString("localhost"), + Path = "/start" + } + }; + + var actionContext = new ActionContext(httpContext, + new RouteData(), + new ActionDescriptor(), + new ModelStateDictionary()); + + var resourceExecutingContext = new ResourceExecutingContext(actionContext, + new List(), + new List()); + + filter.OnResourceExecuting(resourceExecutingContext); + + mockLogger.VerifyError("Service access keys not configured"); + } + + [TestMethod] + public void Filter_OnlyEmptySecretsConfigured_LogsError() + { + var dic = new Dictionary + { + { "ServiceAccess:Keys:0", " " }, + { "ServiceAccess:Keys:1", "\t" } + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(dic) + .Build(); + + var mockLogger = new Mock>(); + + var filter = new ChallengeResourceFilterAttribute(mockLogger.Object, configuration); + + var httpContext = new DefaultHttpContext + { + Request = + { + Scheme = "https", + Host = new HostString("localhost"), + Path = "/start" + } + }; + + var actionContext = new ActionContext(httpContext, + new RouteData(), + new ActionDescriptor(), + new ModelStateDictionary()); + + var resourceExecutingContext = new ResourceExecutingContext(actionContext, + new List(), + new List()); + + filter.OnResourceExecuting(resourceExecutingContext); + + mockLogger.VerifyError("Service access keys not configured"); + } + + [TestMethod] + public void ExecuteFilter_AllowPublicAccess_PassesThrough() + { + var dic = new Dictionary + { + { "ServiceAccess:IsPublic", true.ToString() } + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(dic) + .Build(); + + var filter = new ChallengeResourceFilterAttribute(NullLogger.Instance, + configuration); + + var httpContext = new DefaultHttpContext + { + Request = + { + Scheme = "https", + Host = new HostString("localhost"), + Path = "/start" + } + }; + + var actionContext = new ActionContext(httpContext, + new RouteData(), + new ActionDescriptor(), + new ModelStateDictionary()); + + var resourceExecutingContext = new ResourceExecutingContext(actionContext, + new List(), + new List()); + + filter.OnResourceExecuting(resourceExecutingContext); + + var result = resourceExecutingContext.Result; + + result.Should().BeNull(); + } } \ No newline at end of file From a349039cf0d48e58d77181afb27cbb7b1708c8a9 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Tue, 11 Jun 2024 12:20:18 +0100 Subject: [PATCH 10/28] Don't cache the challenge response. --- .../Controllers/ChallengeController.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs index 51fc8a6a..401b4aa3 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/ChallengeController.cs @@ -12,6 +12,7 @@ public class ChallengeController( private const string DefaultRedirectAddress = "/"; [HttpGet] + [ResponseCache(Duration = 0, Location = ResponseCacheLocation.None, NoStore = true)] public Task Index([FromQuery] ChallengeModel model) { if (!ModelState.IsValid) From dfa8e17e4ee83eace9f6d28602b865b2d24e1f5e Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Tue, 11 Jun 2024 12:25:46 +0100 Subject: [PATCH 11/28] Logger in the ErrorController is not used. --- .../Controllers/ErrorController.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Dfe.EarlyYearsQualification.Web/Controllers/ErrorController.cs b/src/Dfe.EarlyYearsQualification.Web/Controllers/ErrorController.cs index e8611a5b..03bf3d42 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Controllers/ErrorController.cs +++ b/src/Dfe.EarlyYearsQualification.Web/Controllers/ErrorController.cs @@ -5,8 +5,7 @@ namespace Dfe.EarlyYearsQualification.Web.Controllers; [Route("/error")] -public class ErrorController( - ILogger logger) : Controller +public class ErrorController : Controller { [HttpGet] [ResponseCache(Duration = 0, Location = ResponseCacheLocation.None, NoStore = true)] From 0f9b01ae83b1afa590720ba77d69e86383947239 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Tue, 11 Jun 2024 15:53:42 +0100 Subject: [PATCH 12/28] End-to-end tests now need an auth secret from env. Will come from pipeline. ...or can come from command line on dev machines. --- .../cypress.config.js | 11 ++++++- .../e2e/journey/deny-public-access.cy.js | 15 +++++++++ .../cypress/e2e/journey/journey-spec.cy.js | 1 + .../accessibility-specification-spec.cy.js | 5 ++- .../cypress/e2e/pages/advice-spec.cy.js | 5 ++- .../cypress/e2e/pages/cookies-spec.cy.js | 21 ++++++------ .../cypress/e2e/pages/home-spec.cy.js | 6 +++- .../pages/qualification-details-spec.cy.js | 32 ++++++++++--------- .../cypress/e2e/pages/question-spec.cy.js | 9 ++++-- .../e2e/shared/cookies-banner-spec.cy.js | 10 ++++-- .../e2e/shared/phase-banner-spec.cy.js | 8 +++-- .../e2e/shared/security-header-spec.cy.js | 3 ++ 12 files changed, 89 insertions(+), 37 deletions(-) create mode 100644 tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/journey/deny-public-access.cy.js diff --git a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress.config.js b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress.config.js index 871b69f4..10e64fa0 100644 --- a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress.config.js +++ b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress.config.js @@ -1,10 +1,19 @@ const { defineConfig } = require("cypress"); module.exports = defineConfig({ + env: { + auth_secret: 'CX' // dummy value: pass in using Cypress command line --env auth_secret=an-acceptable-secret-value + }, e2e: { baseUrl: "http://127.0.0.1:5025/", setupNodeEvents(on, config) { + on('task', { + log(message) { + console.log(message) + return null + } + }) // implement node event listeners here - }, + } }, }); diff --git a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/journey/deny-public-access.cy.js b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/journey/deny-public-access.cy.js new file mode 100644 index 00000000..054d60cc --- /dev/null +++ b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/journey/deny-public-access.cy.js @@ -0,0 +1,15 @@ +describe('A spec used to check a new user is challenged to enter the secret', () => { + + it("should redirect the user to the challenge page", () => { + + cy.visit("/"); + + cy.location().should((loc) => { + expect(loc.pathname).to.eq('/Challenge'); + expect(loc.search).to.eq('?redirectAddress=%2F') + }) + + cy.get('#redirectAddress').should("have.value", '/'); + cy.get('#value').should("be.empty"); + }) +}) \ No newline at end of file diff --git a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/journey/journey-spec.cy.js b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/journey/journey-spec.cy.js index e70152af..dace06b0 100644 --- a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/journey/journey-spec.cy.js +++ b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/journey/journey-spec.cy.js @@ -1,5 +1,6 @@ describe('A spec used to test the various routes through the journey', () => { beforeEach(() => { + cy.setCookie('auth-secret', Cypress.env('auth_secret')); cy.visit("/"); cy.get('.govuk-button--start').should('exist'); }) diff --git a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/accessibility-specification-spec.cy.js b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/accessibility-specification-spec.cy.js index 732b31b4..f4775f40 100644 --- a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/accessibility-specification-spec.cy.js +++ b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/accessibility-specification-spec.cy.js @@ -1,5 +1,8 @@ describe("A spec that tests the accessibility statement page", () => { - + beforeEach(() => { + cy.setCookie('auth-secret', Cypress.env('auth_secret')); + }) + // Mock details found in Dfe.EarlyYearsQualification.Mock.Content.MockContentfulService. it("Checks the heading and content are present", () => { cy.visit("/accessibility-statement"); diff --git a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/advice-spec.cy.js b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/advice-spec.cy.js index 696356ef..28c4500d 100644 --- a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/advice-spec.cy.js +++ b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/advice-spec.cy.js @@ -1,5 +1,8 @@ describe("A spec that tests advice pages", () => { - + beforeEach(() => { + cy.setCookie('auth-secret', Cypress.env('auth_secret')); + }) + // Mock details found in Dfe.EarlyYearsQualification.Mock.Content.MockContentfulService. it("Checks the qualification details are on the page", () => { cy.visit("/advice/qualification-outside-the-united-kingdom"); diff --git a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/cookies-spec.cy.js b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/cookies-spec.cy.js index e9e71873..9d47ae50 100644 --- a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/cookies-spec.cy.js +++ b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/cookies-spec.cy.js @@ -2,6 +2,7 @@ describe("A spec that tests the cookies page", () => { // Mock details found in Dfe.EarlyYearsQualification.Mock.Content.MockContentfulService. beforeEach(() => { + cy.setCookie('auth-secret', Cypress.env('auth_secret')); cy.visit("/cookies"); }); @@ -14,8 +15,8 @@ describe("A spec that tests the cookies page", () => { cy.get("#test-option-value-1").should("exist"); cy.get("#test-option-value-2").should("exist"); - cy.get("label[for='test-option-value-1']").should("contain.text","Test Option Label 1"); - cy.get("label[for='test-option-value-2']").should("contain.text","Test Option Label 2"); + cy.get("label[for='test-option-value-1']").should("contain.text", "Test Option Label 1"); + cy.get("label[for='test-option-value-2']").should("contain.text", "Test Option Label 2"); cy.get("#cookies-choice-error") .should("not.be.visible") @@ -27,25 +28,25 @@ describe("A spec that tests the cookies page", () => { describe("Check the functionality of the page", () => { it("Checks that the radio button validation is working", () => { cy.get('button[id="cookies-button"]').click(); - + cy.get("#cookies-set-banner").should("not.exist"); - + cy.get("#cookies-choice-error").should("be.visible"); }); - + ["test-option-value-1", "test-option-value-2"].forEach((option) => { it(`Checks that selecting ${option} reveals success banner`, () => { cy.get(`#${option}`).click(); cy.get('button[id="cookies-button"]').click(); - + cy.get("#cookies-set-banner").should("be.visible"); - + // Seen as the success banner doesn't exist in the rendered HTML by default; we have to check the content once we expect the heading to be visible. - cy.get("#cookies-set-banner-heading").should("contain.text","Test Success Banner Heading"); - cy.get("#cookies-set-banner-content").should("contain.text","Test Success Banner Content"); - + cy.get("#cookies-set-banner-heading").should("contain.text", "Test Success Banner Heading"); + cy.get("#cookies-set-banner-content").should("contain.text", "Test Success Banner Content"); + cy.get("#cookies-choice-error").should("not.be.visible"); }); }); diff --git a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/home-spec.cy.js b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/home-spec.cy.js index 538833c2..4d2030e5 100644 --- a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/home-spec.cy.js +++ b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/home-spec.cy.js @@ -1,11 +1,13 @@ describe("A spec used to test the home page", () => { beforeEach(() => { - cy.visit("/"); + cy.setCookie('auth-secret', Cypress.env('auth_secret')); }) // Mock details found in Dfe.EarlyYearsQualification.Mock.Content.MockContentfulService. it("Checks the page contains the relevant components", () => { + cy.visit("/"); + cy.get(".govuk-heading-xl").should("contain.text", "Test Header"); cy.get("#pre-cta-content p").should("contain.text", "This is the pre cta content"); cy.get(".govuk-button--start").should("contain.text", "Start Button Text"); @@ -15,6 +17,8 @@ describe("A spec used to test the home page", () => { }) it("Checks Crown copyright link text", () => { + cy.visit("/"); + cy.get(".govuk-footer__copyright-logo").first().should("contain.text", "Crown copyright") }) }) \ No newline at end of file diff --git a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/qualification-details-spec.cy.js b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/qualification-details-spec.cy.js index 66c04bb5..002f193f 100644 --- a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/qualification-details-spec.cy.js +++ b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/qualification-details-spec.cy.js @@ -1,17 +1,19 @@ describe("A spec used to test the qualification details page", () => { - beforeEach(() => { - cy.visit("/qualifications/qualification-details/eyq-240"); - }) - - // Mock details found in Dfe.EarlyYearsQualification.Mock.Content.MockContentfulService. - it("Checks the qualification details are on the page", () => { - cy.get("#qualification-name-value").should("contain.text", "T Level Technical Qualification in Education and Childcare (Specialism - Early Years Educator)"); - cy.get("#awarding-qualification-value").should("contain.text", "NCFE"); - cy.get("#qualification-level-value").should("contain.text", "3"); - cy.get("#qualification-number-value").should("contain.text", "603/5829/4"); - cy.get("#from-which-year-value").should("contain.text", "2020"); - cy.get("#notes-value").should("contain.text", "The course must be assessed within the EYFS in an Early Years setting in England. Please note that the name of this qualification changed in February 2023. Qualifications achieved under either name are full and relevant provided that the start date for the qualification aligns with the date of the name change."); - cy.get("#additional-requirements-value").should("contain.text", "Additional notes"); - }) - }) \ No newline at end of file + beforeEach(() => { + cy.setCookie('auth-secret', Cypress.env('auth_secret')); + }) + + // Mock details found in Dfe.EarlyYearsQualification.Mock.Content.MockContentfulService. + it("Checks the qualification details are on the page", () => { + cy.visit("/qualifications/qualification-details/eyq-240"); + + cy.get("#qualification-name-value").should("contain.text", "T Level Technical Qualification in Education and Childcare (Specialism - Early Years Educator)"); + cy.get("#awarding-qualification-value").should("contain.text", "NCFE"); + cy.get("#qualification-level-value").should("contain.text", "3"); + cy.get("#qualification-number-value").should("contain.text", "603/5829/4"); + cy.get("#from-which-year-value").should("contain.text", "2020"); + cy.get("#notes-value").should("contain.text", "The course must be assessed within the EYFS in an Early Years setting in England. Please note that the name of this qualification changed in February 2023. Qualifications achieved under either name are full and relevant provided that the start date for the qualification aligns with the date of the name change."); + cy.get("#additional-requirements-value").should("contain.text", "Additional notes"); + }) +}) \ No newline at end of file diff --git a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/question-spec.cy.js b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/question-spec.cy.js index e5902197..00e2504d 100644 --- a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/question-spec.cy.js +++ b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/pages/question-spec.cy.js @@ -1,5 +1,8 @@ describe("A spec that tests question pages", () => { - + beforeEach(() => { + cy.setCookie('auth-secret', Cypress.env('auth_secret')); + }) + // Mock details found in Dfe.EarlyYearsQualification.Mock.Content.MockContentfulService. it("Checks the content on where-was-the-qualification-awarded page", () => { cy.visit("/questions/where-was-the-qualification-awarded"); @@ -17,7 +20,7 @@ describe("A spec that tests question pages", () => { cy.get('button[id="question-submit"]').click(); cy.location().should((loc) => { - expect(loc.pathname).to.eq("/questions/where-was-the-qualification-awarded"); + expect(loc.pathname).to.eq("/questions/where-was-the-qualification-awarded"); }) cy.get('#option-error').should("exist"); @@ -47,7 +50,7 @@ describe("A spec that tests question pages", () => { cy.get('button[id="question-submit"]').click(); cy.location().should((loc) => { - expect(loc.pathname).to.eq("/questions/what-level-is-the-qualification"); + expect(loc.pathname).to.eq("/questions/what-level-is-the-qualification"); }) cy.get('#option-error').should("exist"); diff --git a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/shared/cookies-banner-spec.cy.js b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/shared/cookies-banner-spec.cy.js index 918fcb09..ee137cd3 100644 --- a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/shared/cookies-banner-spec.cy.js +++ b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/shared/cookies-banner-spec.cy.js @@ -1,12 +1,16 @@ import { pages } from "./urls-to-check"; describe("A spec that tests that the cookies banner shows on all pages", () => { + beforeEach(() => { + cy.setCookie('auth-secret', Cypress.env('auth_secret')); + }) + // Mock details found in Dfe.EarlyYearsQualification.Mock.Content.MockContentfulService. pages.forEach((option) => { it(`Checks that the cookies banner is present at the URL: ${option}`, () => { cy.visit(option); - + cy.get("#choose-cookies-preference").should("be.visible"); cy.get("#cookies-preference-chosen").should("not.exist"); @@ -26,7 +30,7 @@ describe("A spec that tests that the cookies banner shows on all pages", () => { .should('have.property', 'value', "%7B%22HasApproved%22%3Atrue%2C%22IsVisible%22%3Atrue%2C%22IsRejected%22%3Afalse%7D"); cy.get("#cookies-banner-pref-chosen-content").should("contain.text", "This is the accepted cookie content"); - + cy.get('button[id="hide-cookie-banner-button"]').click(); cy.get("#choose-cookies-preference").should("not.exist"); @@ -48,7 +52,7 @@ describe("A spec that tests that the cookies banner shows on all pages", () => { .should('have.property', 'value', "%7B%22HasApproved%22%3Afalse%2C%22IsVisible%22%3Atrue%2C%22IsRejected%22%3Atrue%7D"); cy.get("#cookies-banner-pref-chosen-content").should("contain.text", "This is the rejected cookie content"); - + cy.get('button[id="hide-cookie-banner-button"]').click(); cy.get("#choose-cookies-preference").should("not.exist"); diff --git a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/shared/phase-banner-spec.cy.js b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/shared/phase-banner-spec.cy.js index bc9a83d2..cd323431 100644 --- a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/shared/phase-banner-spec.cy.js +++ b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/shared/phase-banner-spec.cy.js @@ -1,14 +1,18 @@ import { pages } from "./urls-to-check"; describe("A spec that tests the phase banner is showing on all pages", () => { - + // Mock details found in Dfe.EarlyYearsQualification.Mock.Content.MockContentfulService. pages.forEach((option) => { + beforeEach(() => { + cy.setCookie('auth-secret', Cypress.env('auth_secret')); + }) + it(`Checks that the phase banner is present at the URL: ${option}`, () => { cy.visit(option); - + cy.get(".govuk-phase-banner").should("be.visible"); cy.get(".govuk-phase-banner__content__tag").should("contain.text", "Test phase banner name"); diff --git a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/shared/security-header-spec.cy.js b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/shared/security-header-spec.cy.js index d70ec823..40084939 100644 --- a/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/shared/security-header-spec.cy.js +++ b/tests/Dfe.EarlyYearsQualification.E2ETests/cypress/e2e/shared/security-header-spec.cy.js @@ -1,6 +1,9 @@ import { pagesWithForms, pagesWithoutForms } from "./urls-to-check"; describe("A spec that checks for security headers in the response", () => { + beforeEach(() => { + cy.setCookie('auth-secret', Cypress.env('auth_secret')); + }) pagesWithForms.forEach((page) => { it(`pages with forms and no cookie banner - ${page} contains the expected response headers`, () => { From e4a92c0ceae6950a065dcf3741c16db84a592bab Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Tue, 11 Jun 2024 16:22:57 +0100 Subject: [PATCH 13/28] Tf added gateway exclusion for auth-secret cookie. Tf added variables for access challenge. Tf configure service on deployment for access challenge. Secret values will come from command line / pipeline. --- terraform/main.tf | 8 +++++- terraform/modules/azure-web/app-gateway.tf | 8 +++++- terraform/modules/azure-web/variables.tf | 33 +++++++++++++++++++++- terraform/modules/azure-web/web-app.tf | 7 ++++- 4 files changed, 52 insertions(+), 4 deletions(-) diff --git a/terraform/main.tf b/terraform/main.tf index 0071589d..66d1cf10 100644 --- a/terraform/main.tf +++ b/terraform/main.tf @@ -75,10 +75,16 @@ module "webapp" { webapp_docker_registry_url = var.webapp_docker_registry_url webapp_session_cookie_name = "_early_years_qualification_session" webapp_cookie_preference_name = "cookies_preferences_set" + webapp_cookie_auth_secret_name = "auth-secret" webapp_custom_domain_name = var.custom_domain_name webapp_custom_domain_cert_secret_label = var.kv_certificate_label webapp_health_check_path = "/health" webapp_health_check_eviction_time_in_min = 10 + webapp_accessIsPublic = false + webapp_e2e_accessKey = "" + webapp_team_accessKey = "" + webapp_accessKey1 = "" + webapp_accessKey2 = "" agw_subnet_id = module.network.agw_subnet_id agw_pip_id = module.network.agw_pip_id kv_id = module.network.kv_id @@ -86,4 +92,4 @@ module "webapp" { kv_mi_id = module.network.kv_mi_id tags = local.common_tags depends_on = [module.network] -} \ No newline at end of file +} diff --git a/terraform/modules/azure-web/app-gateway.tf b/terraform/modules/azure-web/app-gateway.tf index 52dd2e15..afbe122e 100644 --- a/terraform/modules/azure-web/app-gateway.tf +++ b/terraform/modules/azure-web/app-gateway.tf @@ -37,6 +37,12 @@ resource "azurerm_web_application_firewall_policy" "agw_wafp" { selector = var.webapp_cookie_preference_name selector_match_operator = "Equals" } + + exclusion { + match_variable = "RequestCookieNames" + selector = var.webapp_cookie_auth_secret_name + selector_match_operator = "Equals" + } } policy_settings { @@ -192,4 +198,4 @@ resource "azurerm_monitor_diagnostic_setting" "agw_logs_monitor" { lifecycle { ignore_changes = [metric] } -} \ No newline at end of file +} diff --git a/terraform/modules/azure-web/variables.tf b/terraform/modules/azure-web/variables.tf index c6bef645..de45f1e7 100644 --- a/terraform/modules/azure-web/variables.tf +++ b/terraform/modules/azure-web/variables.tf @@ -45,6 +45,32 @@ variable "webapp_name" { type = string } +variable "webapp_accessIsPublic" { + description = "Web app service is public, and access is unchallenged" + default = false + type = bool +} + +variable "webapp_e2e_accessKey" { + description = "Web app access key for automated end-to-end tests" + type = string +} + +variable "webapp_team_accessKey" { + description = "Web app access key for the service team" + type = string +} + +variable "webapp_accessKey1" { + description = "Web app access key for invited access 1" + type = string +} + +variable "webapp_accessKey2" { + description = "Web app access key for invited access 2" + type = string +} + variable "webapp_subnet_id" { description = "ID of the delegated Subnet for the Web Application" type = string @@ -85,6 +111,11 @@ variable "webapp_cookie_preference_name" { type = string } +variable "webapp_cookie_auth_secret_name" { + description = "Name of the cookie holding the auth secret" + type = string +} + variable "webapp_health_check_path" { default = null description = "Path to health check endpoint" @@ -141,4 +172,4 @@ variable "kv_mi_id" { variable "tags" { description = "Resource tags" type = map(string) -} \ No newline at end of file +} diff --git a/terraform/modules/azure-web/web-app.tf b/terraform/modules/azure-web/web-app.tf index 06673cef..28cae372 100644 --- a/terraform/modules/azure-web/web-app.tf +++ b/terraform/modules/azure-web/web-app.tf @@ -136,6 +136,11 @@ resource "azurerm_linux_web_app_slot" "webapp_slot" { "APPINSIGHTS_INSTRUMENTATIONKEY" = azurerm_application_insights.web.instrumentation_key "APPLICATIONINSIGHTS_CONNECTION_STRING" = azurerm_application_insights.web.connection_string "ApplicationInsightsAgent_EXTENSION_VERSION" = "~3" + "ServiceAccess__IsPublic" = false + "ServiceAccess__Keys__0" = var.webapp_e2e_accessKey + "ServiceAccess__Keys__1" = var.webapp_team_accessKey + "ServiceAccess__Keys__2" = var.webapp_accessKey1 + "ServiceAccess__Keys__3" = var.webapp_accessKey2 }, var.webapp_slot_app_settings) site_config { @@ -391,4 +396,4 @@ resource "azurerm_app_service_certificate_binding" "webapp_custom_domain_cert_bi hostname_binding_id = azurerm_app_service_custom_hostname_binding.webapp_custom_domain[0].id certificate_id = azurerm_app_service_certificate.webapp_custom_domain_cert[0].id ssl_state = "SniEnabled" -} \ No newline at end of file +} From d9e220d4db40a27464a30b3eb34f3b30ab00bfb1 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 11 Jun 2024 15:23:25 +0000 Subject: [PATCH 14/28] terraform-docs: automated action --- terraform/modules/azure-web/README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/terraform/modules/azure-web/README.md b/terraform/modules/azure-web/README.md index 29b7c9e2..d9094f6c 100644 --- a/terraform/modules/azure-web/README.md +++ b/terraform/modules/azure-web/README.md @@ -56,14 +56,19 @@ No modules. | [resource\_group](#input\_resource\_group) | Name of the Azure Resource Group to deploy resources | `string` | n/a | yes | | [resource\_name\_prefix](#input\_resource\_name\_prefix) | Prefix for resource names | `string` | n/a | yes | | [tags](#input\_tags) | Resource tags | `map(string)` | n/a | yes | +| [webapp\_accessIsPublic](#input\_webapp\_accessIsPublic) | Web app service is public, and access is unchallenged | `bool` | `false` | no | +| [webapp\_accessKey1](#input\_webapp\_accessKey1) | Web app access key for invited access 1 | `string` | n/a | yes | +| [webapp\_accessKey2](#input\_webapp\_accessKey2) | Web app access key for invited access 2 | `string` | n/a | yes | | [webapp\_admin\_email\_address](#input\_webapp\_admin\_email\_address) | Email Address of the Admin | `string` | n/a | yes | | [webapp\_app\_settings](#input\_webapp\_app\_settings) | App Settings are exposed as environment variables | `map(string)` | n/a | yes | +| [webapp\_cookie\_auth\_secret\_name](#input\_webapp\_cookie\_auth\_secret\_name) | Name of the cookie holding the auth secret | `string` | n/a | yes | | [webapp\_cookie\_preference\_name](#input\_webapp\_cookie\_preference\_name) | Name of the user's cookie preference cookie | `string` | n/a | yes | | [webapp\_custom\_domain\_cert\_secret\_label](#input\_webapp\_custom\_domain\_cert\_secret\_label) | Label for the Certificate | `string` | n/a | yes | | [webapp\_custom\_domain\_name](#input\_webapp\_custom\_domain\_name) | Custom domain hostname | `string` | n/a | yes | | [webapp\_docker\_image](#input\_webapp\_docker\_image) | Docker Image to deploy | `string` | n/a | yes | | [webapp\_docker\_image\_tag](#input\_webapp\_docker\_image\_tag) | Tag for the Docker Image | `string` | n/a | yes | | [webapp\_docker\_registry\_url](#input\_webapp\_docker\_registry\_url) | URL to the Docker Registry | `string` | n/a | yes | +| [webapp\_e2e\_accessKey](#input\_webapp\_e2e\_accessKey) | Web app access key for automated end-to-end tests | `string` | n/a | yes | | [webapp\_health\_check\_eviction\_time\_in\_min](#input\_webapp\_health\_check\_eviction\_time\_in\_min) | Minutes before considering an instance unhealthy | `number` | `null` | no | | [webapp\_health\_check\_path](#input\_webapp\_health\_check\_path) | Path to health check endpoint | `string` | `null` | no | | [webapp\_name](#input\_webapp\_name) | Name for the Web Application | `string` | n/a | yes | @@ -71,6 +76,7 @@ No modules. | [webapp\_slot\_app\_settings](#input\_webapp\_slot\_app\_settings) | App Settings are exposed as environment variables | `map(string)` | n/a | yes | | [webapp\_startup\_command](#input\_webapp\_startup\_command) | Startup command to pass into the Web Application | `string` | `null` | no | | [webapp\_subnet\_id](#input\_webapp\_subnet\_id) | ID of the delegated Subnet for the Web Application | `string` | n/a | yes | +| [webapp\_team\_accessKey](#input\_webapp\_team\_accessKey) | Web app access key for the service team | `string` | n/a | yes | | [webapp\_worker\_count](#input\_webapp\_worker\_count) | Number of Workers for the App Service Plan | `string` | n/a | yes | ## Outputs From fbe970b3cb20e143ef19f60f1e72abc6e7fe30b0 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Wed, 12 Jun 2024 09:50:42 +0100 Subject: [PATCH 15/28] First attempt at hooking up challenge to pipelines, e2e tests etc. --- .github/actions/run-app-for-testing/action.yml | 8 +++++++- .github/actions/run-e2e-tests/action.yml | 2 ++ terraform/main.tf | 10 +++++----- terraform/modules/azure-web/variables.tf | 10 +++++----- terraform/modules/azure-web/web-app.tf | 10 +++++----- 5 files changed, 24 insertions(+), 16 deletions(-) diff --git a/.github/actions/run-app-for-testing/action.yml b/.github/actions/run-app-for-testing/action.yml index f88e5352..d5e002a7 100644 --- a/.github/actions/run-app-for-testing/action.yml +++ b/.github/actions/run-app-for-testing/action.yml @@ -12,4 +12,10 @@ runs: steps: - name: Run .Net Project shell: bash - run: dotnet run --urls "${{ inputs.url }}" --project="src/Dfe.EarlyYearsQualification.Web" --UseMockContentful=true & + run: | + dotnet run --urls "${{ inputs.url }}" --project="src/Dfe.EarlyYearsQualification.Web" \ + --UseMockContentful=true \ + --ServiceAccess:Keys:0={{ secrets.WEBAPP_E2E_ACCESS_KEY }} \ + --ServiceAccess:Keys:1={{ secrets.WEBAPP_TEAM_ACCESS_KEY }} \ + --ServiceAccess:Keys:2={{ secrets.WEBAPP_ACCESS_KEY_1 }} \ + --ServiceAccess:Keys:3={{ secrets.WEBAPP_ACCESS_KEY_2 }} & diff --git a/.github/actions/run-e2e-tests/action.yml b/.github/actions/run-e2e-tests/action.yml index fa165d07..280e9c5b 100644 --- a/.github/actions/run-e2e-tests/action.yml +++ b/.github/actions/run-e2e-tests/action.yml @@ -14,6 +14,7 @@ runs: uses: cypress-io/github-action@v6 if: success() || failure() with: + env: auth_secret=${{ secrets.WEBAPP_E2E_ACCESS_KEY }} working-directory: ./tests/Dfe.EarlyYearsQualification.E2ETests/ browser: chrome config: baseUrl="${{ inputs.url }}" @@ -22,6 +23,7 @@ runs: uses: cypress-io/github-action@v6 if: success() || failure() with: + env: auth_secret=${{ secrets.WEBAPP_E2E_ACCESS_KEY }} working-directory: ./tests/Dfe.EarlyYearsQualification.E2ETests/ browser: firefox config: baseUrl="${{ inputs.url }}" diff --git a/terraform/main.tf b/terraform/main.tf index 66d1cf10..37ac7644 100644 --- a/terraform/main.tf +++ b/terraform/main.tf @@ -80,11 +80,11 @@ module "webapp" { webapp_custom_domain_cert_secret_label = var.kv_certificate_label webapp_health_check_path = "/health" webapp_health_check_eviction_time_in_min = 10 - webapp_accessIsPublic = false - webapp_e2e_accessKey = "" - webapp_team_accessKey = "" - webapp_accessKey1 = "" - webapp_accessKey2 = "" + webapp_access_is_public = false + webapp_e2e_access_key = "" + webapp_team_access_key = "" + webapp_access_key_1 = "" + webapp_access_key_2 = "" agw_subnet_id = module.network.agw_subnet_id agw_pip_id = module.network.agw_pip_id kv_id = module.network.kv_id diff --git a/terraform/modules/azure-web/variables.tf b/terraform/modules/azure-web/variables.tf index de45f1e7..52f3223d 100644 --- a/terraform/modules/azure-web/variables.tf +++ b/terraform/modules/azure-web/variables.tf @@ -45,28 +45,28 @@ variable "webapp_name" { type = string } -variable "webapp_accessIsPublic" { +variable "webapp_access_is_public" { description = "Web app service is public, and access is unchallenged" default = false type = bool } -variable "webapp_e2e_accessKey" { +variable "webapp_e2e_access_key" { description = "Web app access key for automated end-to-end tests" type = string } -variable "webapp_team_accessKey" { +variable "webapp_team_access_key" { description = "Web app access key for the service team" type = string } -variable "webapp_accessKey1" { +variable "webapp_access_key_1" { description = "Web app access key for invited access 1" type = string } -variable "webapp_accessKey2" { +variable "webapp_access_key_2" { description = "Web app access key for invited access 2" type = string } diff --git a/terraform/modules/azure-web/web-app.tf b/terraform/modules/azure-web/web-app.tf index 28cae372..b37a6e1d 100644 --- a/terraform/modules/azure-web/web-app.tf +++ b/terraform/modules/azure-web/web-app.tf @@ -136,11 +136,11 @@ resource "azurerm_linux_web_app_slot" "webapp_slot" { "APPINSIGHTS_INSTRUMENTATIONKEY" = azurerm_application_insights.web.instrumentation_key "APPLICATIONINSIGHTS_CONNECTION_STRING" = azurerm_application_insights.web.connection_string "ApplicationInsightsAgent_EXTENSION_VERSION" = "~3" - "ServiceAccess__IsPublic" = false - "ServiceAccess__Keys__0" = var.webapp_e2e_accessKey - "ServiceAccess__Keys__1" = var.webapp_team_accessKey - "ServiceAccess__Keys__2" = var.webapp_accessKey1 - "ServiceAccess__Keys__3" = var.webapp_accessKey2 + "ServiceAccess__IsPublic" = var.webapp_access_is_public + "ServiceAccess__Keys__0" = var.webapp_e2e_access_key + "ServiceAccess__Keys__1" = var.webapp_team_access_key + "ServiceAccess__Keys__2" = var.webapp_access_key_1 + "ServiceAccess__Keys__3" = var.webapp_access_key_2 }, var.webapp_slot_app_settings) site_config { From dc44afe9a7785402893d3bd48ef27fb686fabdbc Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 12 Jun 2024 08:52:18 +0000 Subject: [PATCH 16/28] terraform-docs: automated action --- terraform/modules/azure-web/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/terraform/modules/azure-web/README.md b/terraform/modules/azure-web/README.md index d9094f6c..1b5a9833 100644 --- a/terraform/modules/azure-web/README.md +++ b/terraform/modules/azure-web/README.md @@ -56,9 +56,9 @@ No modules. | [resource\_group](#input\_resource\_group) | Name of the Azure Resource Group to deploy resources | `string` | n/a | yes | | [resource\_name\_prefix](#input\_resource\_name\_prefix) | Prefix for resource names | `string` | n/a | yes | | [tags](#input\_tags) | Resource tags | `map(string)` | n/a | yes | -| [webapp\_accessIsPublic](#input\_webapp\_accessIsPublic) | Web app service is public, and access is unchallenged | `bool` | `false` | no | -| [webapp\_accessKey1](#input\_webapp\_accessKey1) | Web app access key for invited access 1 | `string` | n/a | yes | -| [webapp\_accessKey2](#input\_webapp\_accessKey2) | Web app access key for invited access 2 | `string` | n/a | yes | +| [webapp\_access\_is\_public](#input\_webapp\_access\_is\_public) | Web app service is public, and access is unchallenged | `bool` | `false` | no | +| [webapp\_access\_key\_1](#input\_webapp\_access\_key\_1) | Web app access key for invited access 1 | `string` | n/a | yes | +| [webapp\_access\_key\_2](#input\_webapp\_access\_key\_2) | Web app access key for invited access 2 | `string` | n/a | yes | | [webapp\_admin\_email\_address](#input\_webapp\_admin\_email\_address) | Email Address of the Admin | `string` | n/a | yes | | [webapp\_app\_settings](#input\_webapp\_app\_settings) | App Settings are exposed as environment variables | `map(string)` | n/a | yes | | [webapp\_cookie\_auth\_secret\_name](#input\_webapp\_cookie\_auth\_secret\_name) | Name of the cookie holding the auth secret | `string` | n/a | yes | @@ -68,7 +68,7 @@ No modules. | [webapp\_docker\_image](#input\_webapp\_docker\_image) | Docker Image to deploy | `string` | n/a | yes | | [webapp\_docker\_image\_tag](#input\_webapp\_docker\_image\_tag) | Tag for the Docker Image | `string` | n/a | yes | | [webapp\_docker\_registry\_url](#input\_webapp\_docker\_registry\_url) | URL to the Docker Registry | `string` | n/a | yes | -| [webapp\_e2e\_accessKey](#input\_webapp\_e2e\_accessKey) | Web app access key for automated end-to-end tests | `string` | n/a | yes | +| [webapp\_e2e\_access\_key](#input\_webapp\_e2e\_access\_key) | Web app access key for automated end-to-end tests | `string` | n/a | yes | | [webapp\_health\_check\_eviction\_time\_in\_min](#input\_webapp\_health\_check\_eviction\_time\_in\_min) | Minutes before considering an instance unhealthy | `number` | `null` | no | | [webapp\_health\_check\_path](#input\_webapp\_health\_check\_path) | Path to health check endpoint | `string` | `null` | no | | [webapp\_name](#input\_webapp\_name) | Name for the Web Application | `string` | n/a | yes | @@ -76,7 +76,7 @@ No modules. | [webapp\_slot\_app\_settings](#input\_webapp\_slot\_app\_settings) | App Settings are exposed as environment variables | `map(string)` | n/a | yes | | [webapp\_startup\_command](#input\_webapp\_startup\_command) | Startup command to pass into the Web Application | `string` | `null` | no | | [webapp\_subnet\_id](#input\_webapp\_subnet\_id) | ID of the delegated Subnet for the Web Application | `string` | n/a | yes | -| [webapp\_team\_accessKey](#input\_webapp\_team\_accessKey) | Web app access key for the service team | `string` | n/a | yes | +| [webapp\_team\_access\_key](#input\_webapp\_team\_access\_key) | Web app access key for the service team | `string` | n/a | yes | | [webapp\_worker\_count](#input\_webapp\_worker\_count) | Number of Workers for the App Service Plan | `string` | n/a | yes | ## Outputs From e598888259f0b5c32ad9f29cd48359eabb27828b Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Wed, 12 Jun 2024 12:41:40 +0100 Subject: [PATCH 17/28] Make the challenge box a little wider. --- .../Views/Challenge/EntryForm.cshtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Dfe.EarlyYearsQualification.Web/Views/Challenge/EntryForm.cshtml b/src/Dfe.EarlyYearsQualification.Web/Views/Challenge/EntryForm.cshtml index ed70ddde..95844598 100644 --- a/src/Dfe.EarlyYearsQualification.Web/Views/Challenge/EntryForm.cshtml +++ b/src/Dfe.EarlyYearsQualification.Web/Views/Challenge/EntryForm.cshtml @@ -18,7 +18,7 @@ @using (Html.BeginForm("Post", "Challenge", new { }, FormMethod.Post)) { - + } From 65a54ed8149e9f42dae4d62c3cecceb3fe03ebbd Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Wed, 12 Jun 2024 13:34:57 +0100 Subject: [PATCH 18/28] Investigate what happens if I pass secrets in. --- .github/actions/run-e2e-tests/action.yml | 7 +++++-- .github/workflows/code-pr-check.yml | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/actions/run-e2e-tests/action.yml b/.github/actions/run-e2e-tests/action.yml index 280e9c5b..eae51e7f 100644 --- a/.github/actions/run-e2e-tests/action.yml +++ b/.github/actions/run-e2e-tests/action.yml @@ -5,6 +5,9 @@ inputs: url: required: true type: string + auth_secret: + required: true + type: string runs: using: composite @@ -14,7 +17,7 @@ runs: uses: cypress-io/github-action@v6 if: success() || failure() with: - env: auth_secret=${{ secrets.WEBAPP_E2E_ACCESS_KEY }} + env: auth_secret=${{ inputs.auth_secret }} working-directory: ./tests/Dfe.EarlyYearsQualification.E2ETests/ browser: chrome config: baseUrl="${{ inputs.url }}" @@ -23,7 +26,7 @@ runs: uses: cypress-io/github-action@v6 if: success() || failure() with: - env: auth_secret=${{ secrets.WEBAPP_E2E_ACCESS_KEY }} + env: auth_secret=${{ inputs.auth_secret }} working-directory: ./tests/Dfe.EarlyYearsQualification.E2ETests/ browser: firefox config: baseUrl="${{ inputs.url }}" diff --git a/.github/workflows/code-pr-check.yml b/.github/workflows/code-pr-check.yml index dfe50f74..c77e948c 100644 --- a/.github/workflows/code-pr-check.yml +++ b/.github/workflows/code-pr-check.yml @@ -59,6 +59,7 @@ jobs: uses: ./.github/actions/run-e2e-tests with: url: ${{ env.URL }} + auth_secret: ${{ secrets.WEBAPP_E2E_ACCESS_KEY }} - name: Run Accessibility tests uses: ./.github/actions/run-accessibility-tests From 2735793e8681d00a7f23f2afeb74c031188da86b Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Wed, 12 Jun 2024 13:46:34 +0100 Subject: [PATCH 19/28] Secret parameter passing to run-app-for-testing too. --- .github/actions/run-app-for-testing/action.yml | 9 +++++---- .github/actions/run-e2e-tests/action.yml | 6 +++--- .github/workflows/code-pr-check.yml | 3 ++- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/.github/actions/run-app-for-testing/action.yml b/.github/actions/run-app-for-testing/action.yml index d5e002a7..bc1d701a 100644 --- a/.github/actions/run-app-for-testing/action.yml +++ b/.github/actions/run-app-for-testing/action.yml @@ -5,6 +5,10 @@ inputs: url: required: true type: string + e2e_auth_secret: + required: true + type: string + runs: using: composite @@ -15,7 +19,4 @@ runs: run: | dotnet run --urls "${{ inputs.url }}" --project="src/Dfe.EarlyYearsQualification.Web" \ --UseMockContentful=true \ - --ServiceAccess:Keys:0={{ secrets.WEBAPP_E2E_ACCESS_KEY }} \ - --ServiceAccess:Keys:1={{ secrets.WEBAPP_TEAM_ACCESS_KEY }} \ - --ServiceAccess:Keys:2={{ secrets.WEBAPP_ACCESS_KEY_1 }} \ - --ServiceAccess:Keys:3={{ secrets.WEBAPP_ACCESS_KEY_2 }} & + --ServiceAccess:Keys:0={{ inputs.e2e_auth_secret }} & diff --git a/.github/actions/run-e2e-tests/action.yml b/.github/actions/run-e2e-tests/action.yml index eae51e7f..6fd0f91e 100644 --- a/.github/actions/run-e2e-tests/action.yml +++ b/.github/actions/run-e2e-tests/action.yml @@ -5,7 +5,7 @@ inputs: url: required: true type: string - auth_secret: + e2e_auth_secret: required: true type: string @@ -17,7 +17,7 @@ runs: uses: cypress-io/github-action@v6 if: success() || failure() with: - env: auth_secret=${{ inputs.auth_secret }} + env: auth_secret=${{ inputs.e2e_auth_secret }} working-directory: ./tests/Dfe.EarlyYearsQualification.E2ETests/ browser: chrome config: baseUrl="${{ inputs.url }}" @@ -26,7 +26,7 @@ runs: uses: cypress-io/github-action@v6 if: success() || failure() with: - env: auth_secret=${{ inputs.auth_secret }} + env: auth_secret=${{ inputs.e2e_auth_secret }} working-directory: ./tests/Dfe.EarlyYearsQualification.E2ETests/ browser: firefox config: baseUrl="${{ inputs.url }}" diff --git a/.github/workflows/code-pr-check.yml b/.github/workflows/code-pr-check.yml index c77e948c..1c967de6 100644 --- a/.github/workflows/code-pr-check.yml +++ b/.github/workflows/code-pr-check.yml @@ -54,12 +54,13 @@ jobs: uses: ./.github/actions/run-app-for-testing with: url: ${{ env.URL }} + e2e_auth_secret: ${{ secrets.WEBAPP_E2E_ACCESS_KEY }} - name: Run e2e tests uses: ./.github/actions/run-e2e-tests with: url: ${{ env.URL }} - auth_secret: ${{ secrets.WEBAPP_E2E_ACCESS_KEY }} + e2e_auth_secret: ${{ secrets.WEBAPP_E2E_ACCESS_KEY }} - name: Run Accessibility tests uses: ./.github/actions/run-accessibility-tests From 9cb3f9585eb9be027a54942b63f7526b1bd105b5 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Wed, 12 Jun 2024 13:57:34 +0100 Subject: [PATCH 20/28] Fix use of input --- .github/actions/run-app-for-testing/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/run-app-for-testing/action.yml b/.github/actions/run-app-for-testing/action.yml index bc1d701a..37d17ee0 100644 --- a/.github/actions/run-app-for-testing/action.yml +++ b/.github/actions/run-app-for-testing/action.yml @@ -19,4 +19,4 @@ runs: run: | dotnet run --urls "${{ inputs.url }}" --project="src/Dfe.EarlyYearsQualification.Web" \ --UseMockContentful=true \ - --ServiceAccess:Keys:0={{ inputs.e2e_auth_secret }} & + --ServiceAccess:Keys:0="${{ inputs.e2e_auth_secret }}" & From 43867fafcad1c6811cb5620a3278c505478804fd Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Wed, 12 Jun 2024 14:13:45 +0100 Subject: [PATCH 21/28] Another attempt at passing the secrets. --- .github/actions/run-app-for-testing/action.yml | 6 ++++-- .github/actions/run-e2e-tests/action.yml | 6 +++--- .github/workflows/code-pr-check.yml | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/.github/actions/run-app-for-testing/action.yml b/.github/actions/run-app-for-testing/action.yml index 37d17ee0..a19f542d 100644 --- a/.github/actions/run-app-for-testing/action.yml +++ b/.github/actions/run-app-for-testing/action.yml @@ -3,9 +3,11 @@ description: Run the app within the action so we can run E2E and Accessibility t inputs: url: + description: "URL used by the app to listen" required: true type: string - e2e_auth_secret: + auth_secret: + description: "Auth secret used by end-to-end tests" required: true type: string @@ -19,4 +21,4 @@ runs: run: | dotnet run --urls "${{ inputs.url }}" --project="src/Dfe.EarlyYearsQualification.Web" \ --UseMockContentful=true \ - --ServiceAccess:Keys:0="${{ inputs.e2e_auth_secret }}" & + --ServiceAccess:Keys:0="${{ inputs.auth_secret }}" & diff --git a/.github/actions/run-e2e-tests/action.yml b/.github/actions/run-e2e-tests/action.yml index 6fd0f91e..fc136f7c 100644 --- a/.github/actions/run-e2e-tests/action.yml +++ b/.github/actions/run-e2e-tests/action.yml @@ -5,7 +5,7 @@ inputs: url: required: true type: string - e2e_auth_secret: + auth_secret: required: true type: string @@ -17,7 +17,7 @@ runs: uses: cypress-io/github-action@v6 if: success() || failure() with: - env: auth_secret=${{ inputs.e2e_auth_secret }} + env: auth_secret="${{ inputs.auth_secret }}" working-directory: ./tests/Dfe.EarlyYearsQualification.E2ETests/ browser: chrome config: baseUrl="${{ inputs.url }}" @@ -26,7 +26,7 @@ runs: uses: cypress-io/github-action@v6 if: success() || failure() with: - env: auth_secret=${{ inputs.e2e_auth_secret }} + env: auth_secret="${{ inputs.auth_secret }}" working-directory: ./tests/Dfe.EarlyYearsQualification.E2ETests/ browser: firefox config: baseUrl="${{ inputs.url }}" diff --git a/.github/workflows/code-pr-check.yml b/.github/workflows/code-pr-check.yml index 1c967de6..a2f56351 100644 --- a/.github/workflows/code-pr-check.yml +++ b/.github/workflows/code-pr-check.yml @@ -54,13 +54,13 @@ jobs: uses: ./.github/actions/run-app-for-testing with: url: ${{ env.URL }} - e2e_auth_secret: ${{ secrets.WEBAPP_E2E_ACCESS_KEY }} + auth_secret: ${{ secrets.WEBAPP_E2E_ACCESS_KEY }} - name: Run e2e tests uses: ./.github/actions/run-e2e-tests with: url: ${{ env.URL }} - e2e_auth_secret: ${{ secrets.WEBAPP_E2E_ACCESS_KEY }} + auth_secret: ${{ secrets.WEBAPP_E2E_ACCESS_KEY }} - name: Run Accessibility tests uses: ./.github/actions/run-accessibility-tests From 91964321c2e136379d9fb7a733b28d22edf26838 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Wed, 12 Jun 2024 17:07:18 +0100 Subject: [PATCH 22/28] Screenshot zips are corrupt in v3: try v4 of the action. --- .github/actions/run-e2e-tests/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/run-e2e-tests/action.yml b/.github/actions/run-e2e-tests/action.yml index fc136f7c..cd3d36ba 100644 --- a/.github/actions/run-e2e-tests/action.yml +++ b/.github/actions/run-e2e-tests/action.yml @@ -32,7 +32,7 @@ runs: config: baseUrl="${{ inputs.url }}" - name: Store screenshots on test failure - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 if: failure() with: name: cypress-screenshots From 58cf097b2552b21ac1226e26b695ccf7b2604955 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Thu, 13 Jun 2024 10:42:43 +0100 Subject: [PATCH 23/28] Set a cookie on the Pa11y tests. --- .../.pa11yci-ubuntu.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.json b/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.json index e15a859c..dfbea517 100644 --- a/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.json +++ b/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.json @@ -2,6 +2,9 @@ "defaults": { "chromeLaunchConfig": { "executablePath": "/usr/bin/google-chrome" + }, + "headers": { + "Cookie": "auth-secret=E2ETest" } }, "urls": [ From 6e599b7827afa50e6622ef6d40dc23388549faac Mon Sep 17 00:00:00 2001 From: Sam Carter Date: Thu, 13 Jun 2024 11:41:25 +0100 Subject: [PATCH 24/28] Updated to add auth secret to pa11y config --- .github/actions/run-accessibility-tests/action.yml | 5 ++++- .github/workflows/code-pr-check.yml | 1 + .../.pa11yci-ubuntu.json | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/actions/run-accessibility-tests/action.yml b/.github/actions/run-accessibility-tests/action.yml index c5d0feea..6129b4b5 100644 --- a/.github/actions/run-accessibility-tests/action.yml +++ b/.github/actions/run-accessibility-tests/action.yml @@ -5,6 +5,9 @@ inputs: url: required: true type: string + auth_secret: + required: true + type: string runs: using: composite @@ -12,7 +15,7 @@ runs: steps: - name: Run Pa11y shell: bash - run: npm install -g pa11y-ci && pa11y-ci ${{ inputs.url }} --config .pa11yci-ubuntu.json + run: npm install -g pa11y-ci && export AUTH_SECRET=${{ inputs.auth_secret }} && pa11y-ci ${{ inputs.url }} --config .pa11yci-ubuntu.json working-directory: ./tests/Dfe.EarlyYearsQualification.AccessibilityTests/ \ No newline at end of file diff --git a/.github/workflows/code-pr-check.yml b/.github/workflows/code-pr-check.yml index a2f56351..d628f0d6 100644 --- a/.github/workflows/code-pr-check.yml +++ b/.github/workflows/code-pr-check.yml @@ -66,3 +66,4 @@ jobs: uses: ./.github/actions/run-accessibility-tests with: url: ${{ env.URL }} + auth_secret: ${{ secrets.WEBAPP_E2E_ACCESS_KEY }} diff --git a/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.json b/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.json index dfbea517..81e7b513 100644 --- a/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.json +++ b/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.json @@ -4,7 +4,7 @@ "executablePath": "/usr/bin/google-chrome" }, "headers": { - "Cookie": "auth-secret=E2ETest" + "Cookie": "auth-secret=${AUTH_SECRET}" } }, "urls": [ From add4048456594cd19d8b4b279a0e1ade701457c1 Mon Sep 17 00:00:00 2001 From: Sam Carter Date: Thu, 13 Jun 2024 11:53:47 +0100 Subject: [PATCH 25/28] Added js pa11y-ci implementation --- .../run-accessibility-tests/action.yml | 2 +- .../.pa11yci-ubuntu.js | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.js diff --git a/.github/actions/run-accessibility-tests/action.yml b/.github/actions/run-accessibility-tests/action.yml index 6129b4b5..d35e2133 100644 --- a/.github/actions/run-accessibility-tests/action.yml +++ b/.github/actions/run-accessibility-tests/action.yml @@ -15,7 +15,7 @@ runs: steps: - name: Run Pa11y shell: bash - run: npm install -g pa11y-ci && export AUTH_SECRET=${{ inputs.auth_secret }} && pa11y-ci ${{ inputs.url }} --config .pa11yci-ubuntu.json + run: npm install -g pa11y-ci && export AUTH_SECRET=${{ inputs.auth_secret }} && pa11y-ci ${{ inputs.url }} --config .pa11yci-ubuntu.js working-directory: ./tests/Dfe.EarlyYearsQualification.AccessibilityTests/ \ No newline at end of file diff --git a/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.js b/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.js new file mode 100644 index 00000000..f20c793a --- /dev/null +++ b/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.js @@ -0,0 +1,34 @@ +var config = { + defaults: { + standard: 'WCAG2AA', + chromeLaunchConfig: { + executablePath: "/usr/bin/google-chrome" + }, + headers: { + Cookie: 'auth-secret=${AUTH_SECRET}' + } + }, + urls: [ + "http://localhost:5000/", + "http://localhost:5000/qualifications/qualification-details/eyq-240", + "http://localhost:5000/questions/where-was-the-qualification-awarded", + "http://localhost:5000/advice/qualification-outside-the-united-kingdom", + "http://localhost:5000/questions/when-was-the-qualification-started", + "http://localhost:5000/questions/what-level-is-the-qualification" + ] +}; + +function createPa11yCiConfiguration(urls, defaults) { + + console.error('Env:', process.env.AUTH_SECRET); + + defaults.headers.Cookie = defaults.headers.Cookie.replace('${AUTH_SECRET}', process.env.AUTH_SECRET) + + return { + defaults: defaults, + urls: urls + } +}; + +// Important ~ call the function, don't just return a reference to it! +module.exports = createPa11yCiConfiguration(config.urls, config.defaults); \ No newline at end of file From 35ae94ec4d11ce2f3cf8ad12b6f3aca3a1bccb11 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Thu, 13 Jun 2024 13:59:50 +0100 Subject: [PATCH 26/28] Deleted redundant config file. --- .../.pa11yci-ubuntu.json | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.json diff --git a/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.json b/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.json deleted file mode 100644 index 81e7b513..00000000 --- a/tests/Dfe.EarlyYearsQualification.AccessibilityTests/.pa11yci-ubuntu.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "defaults": { - "chromeLaunchConfig": { - "executablePath": "/usr/bin/google-chrome" - }, - "headers": { - "Cookie": "auth-secret=${AUTH_SECRET}" - } - }, - "urls": [ - "http://localhost:5000/", - "http://localhost:5000/qualifications/qualification-details/eyq-240", - "http://localhost:5000/questions/where-was-the-qualification-awarded", - "http://localhost:5000/advice/qualification-outside-the-united-kingdom", - "http://localhost:5000/questions/when-was-the-qualification-started", - "http://localhost:5000/questions/what-level-is-the-qualification" - ] -} \ No newline at end of file From e225706db3ef12d70183d2dfd91edecf5c9fdef4 Mon Sep 17 00:00:00 2001 From: RobertGHippo Date: Thu, 13 Jun 2024 14:31:18 +0100 Subject: [PATCH 27/28] Moved tf settings to locals. --- terraform/local.tf | 12 +++++++++- terraform/main.tf | 5 ----- terraform/modules/azure-web/variables.tf | 26 ---------------------- terraform/modules/azure-web/web-app.tf | 5 ----- terraform/variables.tf | 28 +++++++++++++++++++++++- 5 files changed, 38 insertions(+), 38 deletions(-) diff --git a/terraform/local.tf b/terraform/local.tf index 9fe8f079..c487db2d 100644 --- a/terraform/local.tf +++ b/terraform/local.tf @@ -16,6 +16,11 @@ locals { "KeyVault__Endpoint" = "https://${var.resource_name_prefix}-kv.vault.azure.net/" "ContentfulOptions__UsePreviewApi" = var.contentful_use_preview_api "WEBSITES_PORT" = "8080" + "ServiceAccess__IsPublic" = var.webapp_access_is_public + "ServiceAccess__Keys__0" = var.webapp_e2e_access_key + "ServiceAccess__Keys__1" = var.webapp_team_access_key + "ServiceAccess__Keys__2" = var.webapp_access_key_1 + "ServiceAccess__Keys__3" = var.webapp_access_key_2 } webapp_slot_app_settings = { @@ -26,5 +31,10 @@ locals { "KeyVault__Endpoint" = "https://${var.resource_name_prefix}-kv.vault.azure.net/" "ContentfulOptions__UsePreviewApi" = var.contentful_use_preview_api "WEBSITES_PORT" = "8080" + "ServiceAccess__IsPublic" = var.webapp_access_is_public + "ServiceAccess__Keys__0" = var.webapp_e2e_access_key + "ServiceAccess__Keys__1" = var.webapp_team_access_key + "ServiceAccess__Keys__2" = var.webapp_access_key_1 + "ServiceAccess__Keys__3" = var.webapp_access_key_2 } -} \ No newline at end of file +} diff --git a/terraform/main.tf b/terraform/main.tf index 639bcdda..6e660340 100644 --- a/terraform/main.tf +++ b/terraform/main.tf @@ -84,11 +84,6 @@ module "webapp" { webapp_custom_domain_cert_secret_label = var.kv_certificate_label webapp_health_check_path = "/health" webapp_health_check_eviction_time_in_min = 10 - webapp_access_is_public = false - webapp_e2e_access_key = "" - webapp_team_access_key = "" - webapp_access_key_1 = "" - webapp_access_key_2 = "" agw_subnet_id = module.network.agw_subnet_id agw_pip_id = module.network.agw_pip_id kv_id = module.network.kv_id diff --git a/terraform/modules/azure-web/variables.tf b/terraform/modules/azure-web/variables.tf index baa70b2c..e6e3c475 100644 --- a/terraform/modules/azure-web/variables.tf +++ b/terraform/modules/azure-web/variables.tf @@ -45,32 +45,6 @@ variable "webapp_name" { type = string } -variable "webapp_access_is_public" { - description = "Web app service is public, and access is unchallenged" - default = false - type = bool -} - -variable "webapp_e2e_access_key" { - description = "Web app access key for automated end-to-end tests" - type = string -} - -variable "webapp_team_access_key" { - description = "Web app access key for the service team" - type = string -} - -variable "webapp_access_key_1" { - description = "Web app access key for invited access 1" - type = string -} - -variable "webapp_access_key_2" { - description = "Web app access key for invited access 2" - type = string -} - variable "webapp_slot_name" { description = "Name for the slot for the Web Application" type = string diff --git a/terraform/modules/azure-web/web-app.tf b/terraform/modules/azure-web/web-app.tf index eec9e3a1..7d0a599d 100644 --- a/terraform/modules/azure-web/web-app.tf +++ b/terraform/modules/azure-web/web-app.tf @@ -136,11 +136,6 @@ resource "azurerm_linux_web_app_slot" "webapp_slot" { "APPINSIGHTS_INSTRUMENTATIONKEY" = azurerm_application_insights.web.instrumentation_key "APPLICATIONINSIGHTS_CONNECTION_STRING" = azurerm_application_insights.web.connection_string "ApplicationInsightsAgent_EXTENSION_VERSION" = "~3" - "ServiceAccess__IsPublic" = var.webapp_access_is_public - "ServiceAccess__Keys__0" = var.webapp_e2e_access_key - "ServiceAccess__Keys__1" = var.webapp_team_access_key - "ServiceAccess__Keys__2" = var.webapp_access_key_1 - "ServiceAccess__Keys__3" = var.webapp_access_key_2 }, var.webapp_slot_app_settings) site_config { diff --git a/terraform/variables.tf b/terraform/variables.tf index 099604c3..faf8b9e9 100644 --- a/terraform/variables.tf +++ b/terraform/variables.tf @@ -90,6 +90,32 @@ variable "webapp_slot_name" { type = string } +variable "webapp_access_is_public" { + description = "Web app service is public, and access is unchallenged" + default = false + type = bool +} + +variable "webapp_e2e_access_key" { + description = "Web app access key for automated end-to-end tests" + type = string +} + +variable "webapp_team_access_key" { + description = "Web app access key for the service team" + type = string +} + +variable "webapp_access_key_1" { + description = "Web app access key for invited access 1" + type = string +} + +variable "webapp_access_key_2" { + description = "Web app access key for invited access 2" + type = string +} + variable "webapp_docker_registry_url" { description = "URL to the Docker Registry" type = string @@ -134,4 +160,4 @@ variable "contentful_space_id" { variable "contentful_use_preview_api" { description = "Boolean used to set whether content is preview or published" type = bool -} \ No newline at end of file +} From 35fde42cf3ab9035ec6623417e4ff947a1f155d8 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 13 Jun 2024 13:31:47 +0000 Subject: [PATCH 28/28] terraform-docs: automated action --- terraform/README.md | 5 +++++ terraform/modules/azure-web/README.md | 5 ----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/terraform/README.md b/terraform/README.md index 34c748d3..c67654db 100644 --- a/terraform/README.md +++ b/terraform/README.md @@ -56,11 +56,16 @@ This module provisions a new Azure Resource Group that assembles together the in | [kv\_certificate\_subject](#input\_kv\_certificate\_subject) | Subject of the Certificate | `string` | n/a | yes | | [resource\_name\_prefix](#input\_resource\_name\_prefix) | Prefix for resource names | `string` | n/a | yes | | [tracking\_id](#input\_tracking\_id) | Google Tag Manager tracking ID | `string` | n/a | yes | +| [webapp\_access\_is\_public](#input\_webapp\_access\_is\_public) | Web app service is public, and access is unchallenged | `bool` | `false` | no | +| [webapp\_access\_key\_1](#input\_webapp\_access\_key\_1) | Web app access key for invited access 1 | `string` | n/a | yes | +| [webapp\_access\_key\_2](#input\_webapp\_access\_key\_2) | Web app access key for invited access 2 | `string` | n/a | yes | | [webapp\_docker\_image](#input\_webapp\_docker\_image) | Docker Image to deploy | `string` | n/a | yes | | [webapp\_docker\_image\_tag](#input\_webapp\_docker\_image\_tag) | Tag for the Docker Image | `string` | `"latest"` | no | | [webapp\_docker\_registry\_url](#input\_webapp\_docker\_registry\_url) | URL to the Docker Registry | `string` | n/a | yes | +| [webapp\_e2e\_access\_key](#input\_webapp\_e2e\_access\_key) | Web app access key for automated end-to-end tests | `string` | n/a | yes | | [webapp\_name](#input\_webapp\_name) | Name for the Web Application | `string` | n/a | yes | | [webapp\_slot\_name](#input\_webapp\_slot\_name) | Name for the slot for the Web Application | `string` | `"green"` | no | +| [webapp\_team\_access\_key](#input\_webapp\_team\_access\_key) | Web app access key for the service team | `string` | n/a | yes | | [webapp\_worker\_count](#input\_webapp\_worker\_count) | Number of Workers for the App Service Plan | `string` | `1` | no | ## Outputs diff --git a/terraform/modules/azure-web/README.md b/terraform/modules/azure-web/README.md index 864e942b..3919dec0 100644 --- a/terraform/modules/azure-web/README.md +++ b/terraform/modules/azure-web/README.md @@ -57,9 +57,6 @@ No modules. | [resource\_group](#input\_resource\_group) | Name of the Azure Resource Group to deploy resources | `string` | n/a | yes | | [resource\_name\_prefix](#input\_resource\_name\_prefix) | Prefix for resource names | `string` | n/a | yes | | [tags](#input\_tags) | Resource tags | `map(string)` | n/a | yes | -| [webapp\_access\_is\_public](#input\_webapp\_access\_is\_public) | Web app service is public, and access is unchallenged | `bool` | `false` | no | -| [webapp\_access\_key\_1](#input\_webapp\_access\_key\_1) | Web app access key for invited access 1 | `string` | n/a | yes | -| [webapp\_access\_key\_2](#input\_webapp\_access\_key\_2) | Web app access key for invited access 2 | `string` | n/a | yes | | [webapp\_admin\_email\_address](#input\_webapp\_admin\_email\_address) | Email Address of the Admin | `string` | n/a | yes | | [webapp\_app\_settings](#input\_webapp\_app\_settings) | App Settings are exposed as environment variables | `map(string)` | n/a | yes | | [webapp\_cookie\_auth\_secret\_name](#input\_webapp\_cookie\_auth\_secret\_name) | Name of the cookie holding the auth secret | `string` | n/a | yes | @@ -69,7 +66,6 @@ No modules. | [webapp\_docker\_image](#input\_webapp\_docker\_image) | Docker Image to deploy | `string` | n/a | yes | | [webapp\_docker\_image\_tag](#input\_webapp\_docker\_image\_tag) | Tag for the Docker Image | `string` | n/a | yes | | [webapp\_docker\_registry\_url](#input\_webapp\_docker\_registry\_url) | URL to the Docker Registry | `string` | n/a | yes | -| [webapp\_e2e\_access\_key](#input\_webapp\_e2e\_access\_key) | Web app access key for automated end-to-end tests | `string` | n/a | yes | | [webapp\_health\_check\_eviction\_time\_in\_min](#input\_webapp\_health\_check\_eviction\_time\_in\_min) | Minutes before considering an instance unhealthy | `number` | `null` | no | | [webapp\_health\_check\_path](#input\_webapp\_health\_check\_path) | Path to health check endpoint | `string` | `null` | no | | [webapp\_name](#input\_webapp\_name) | Name for the Web Application | `string` | n/a | yes | @@ -78,7 +74,6 @@ No modules. | [webapp\_slot\_name](#input\_webapp\_slot\_name) | Name for the slot for the Web Application | `string` | n/a | yes | | [webapp\_startup\_command](#input\_webapp\_startup\_command) | Startup command to pass into the Web Application | `string` | `null` | no | | [webapp\_subnet\_id](#input\_webapp\_subnet\_id) | ID of the delegated Subnet for the Web Application | `string` | n/a | yes | -| [webapp\_team\_access\_key](#input\_webapp\_team\_access\_key) | Web app access key for the service team | `string` | n/a | yes | | [webapp\_worker\_count](#input\_webapp\_worker\_count) | Number of Workers for the App Service Plan | `string` | n/a | yes | ## Outputs