From a437643a1494e59d07ca27b536a545c7dbf36b4b Mon Sep 17 00:00:00 2001 From: Andy Mantell <134642+andymantell@users.noreply.github.com> Date: Wed, 4 Dec 2024 12:58:31 +0000 Subject: [PATCH 1/4] Initial work receiving cookie preferences from FTS --- .../CookieAcceptanceMiddleware.cs | 27 ++++++ .../CookiePreferencesService.cs | 95 +++++++++++++++++++ .../Pages/Cookies.cshtml | 6 +- .../Pages/Cookies.cshtml.cs | 47 +++------ .../Pages/Shared/_CookieBanner.cshtml | 20 +++- .../Pages/Shared/_Layout.cshtml | 4 +- Frontend/CO.CDP.OrganisationApp/Program.cs | 4 + 7 files changed, 161 insertions(+), 42 deletions(-) create mode 100644 Frontend/CO.CDP.OrganisationApp/CookieAcceptanceMiddleware.cs create mode 100644 Frontend/CO.CDP.OrganisationApp/CookiePreferencesService.cs diff --git a/Frontend/CO.CDP.OrganisationApp/CookieAcceptanceMiddleware.cs b/Frontend/CO.CDP.OrganisationApp/CookieAcceptanceMiddleware.cs new file mode 100644 index 000000000..e873e2238 --- /dev/null +++ b/Frontend/CO.CDP.OrganisationApp/CookieAcceptanceMiddleware.cs @@ -0,0 +1,27 @@ +namespace CO.CDP.OrganisationApp; + +public class CookieAcceptanceMiddleware(ICookiePreferencesService cookiePreferencesService) : IMiddleware +{ + public async Task InvokeAsync(HttpContext context, RequestDelegate next) + { + if (context.Request.Query.TryGetValue(CookieSettings.FtsHandoverParameter, out var cookiesAcceptedValue)) + { + string cookiesAccepted = cookiesAcceptedValue.ToString().ToLower(); + + if (cookiesAccepted == "true") + { + cookiePreferencesService.Accept(); + } + else if (cookiesAccepted == "false") + { + cookiePreferencesService.Reject(); + } + else if (cookiesAccepted == "unknown") + { + cookiePreferencesService.Reset(); + } + } + + await next(context); + } +} diff --git a/Frontend/CO.CDP.OrganisationApp/CookiePreferencesService.cs b/Frontend/CO.CDP.OrganisationApp/CookiePreferencesService.cs new file mode 100644 index 000000000..b5726d0a7 --- /dev/null +++ b/Frontend/CO.CDP.OrganisationApp/CookiePreferencesService.cs @@ -0,0 +1,95 @@ +namespace CO.CDP.OrganisationApp; + +public interface ICookiePreferencesService +{ + bool IsAccepted(); + bool IsRejected(); + void Accept(); + void Reject(); + void Reset(); + void SetCookie(CookieAcceptanceValues value); + bool ValueIs(CookieAcceptanceValues value); + bool IsUnknown(); +} + +public class CookiePreferencesService : ICookiePreferencesService +{ + private readonly HttpContext _context; + + private CookieAcceptanceValues pendingAcceptanceValue; + + public CookiePreferencesService(IHttpContextAccessor httpContextAccessor) + { + _context = httpContextAccessor.HttpContext + ?? throw new InvalidOperationException("No active HTTP context."); + } + + public void Accept() + { + SetCookie(CookieAcceptanceValues.Accept); + } + + public void Reject() + { + SetCookie(CookieAcceptanceValues.Reject); + } + + public void Reset() + { + _context.Response.Cookies.Delete(CookieSettings.CookieName); + pendingAcceptanceValue = CookieAcceptanceValues.Unknown; + } + + public void SetCookie(CookieAcceptanceValues value) + { + _context.Response.Cookies.Append(CookieSettings.CookieName, value.ToString(), new CookieOptions + { + Expires = DateTimeOffset.UtcNow.AddDays(365), + IsEssential = true, + HttpOnly = true, + Secure = _context.Request.IsHttps + }); + pendingAcceptanceValue = value; + } + + public bool ValueIs(CookieAcceptanceValues value) + { + if(pendingAcceptanceValue == value) + { + return true; + } + + return _context.Request.Cookies.ContainsKey(CookieSettings.CookieName) + && _context.Request.Cookies[CookieSettings.CookieName] == value.ToString(); + } + + public bool IsAccepted() + { + return ValueIs(CookieAcceptanceValues.Accept); + } + + public bool IsRejected() + { + return ValueIs(CookieAcceptanceValues.Reject); + } + + public bool IsUnknown() + { + return ValueIs(CookieAcceptanceValues.Unknown); + } +} +public enum CookieAcceptanceValues +{ + Unknown, + Accept, + Reject +} + +public static class CookieSettings +{ + public const string CookieAcceptanceFieldName = "CookieAcceptance"; + public const string CookieSettingsPageReturnUrlFieldName = "ReturnUrl"; + public const string CookieBannerInteractionQueryString = "cookieBannerInteraction"; + public const string CookieName = "SIRSI_COOKIES_PREFERENCES_SET"; + public const string FtsHandoverParameter = "cookies_accepted"; +} \ No newline at end of file diff --git a/Frontend/CO.CDP.OrganisationApp/Pages/Cookies.cshtml b/Frontend/CO.CDP.OrganisationApp/Pages/Cookies.cshtml index fd1a24641..f6630a0b2 100644 --- a/Frontend/CO.CDP.OrganisationApp/Pages/Cookies.cshtml +++ b/Frontend/CO.CDP.OrganisationApp/Pages/Cookies.cshtml @@ -4,6 +4,8 @@ @using CO.CDP.Localization @using Microsoft.AspNetCore.Mvc.Localization +@inject ICookiePreferencesService cookiePreferencesService; + @{ ViewData["Title"] = StaticTextResource.Supplementary_Cookies_Title; var hasError = ((TagBuilder)Html.ValidationMessageFor(m => m.CookieAcceptance)).HasInnerHtml; @@ -45,13 +47,13 @@
- +
- + diff --git a/Frontend/CO.CDP.OrganisationApp/Pages/Cookies.cshtml.cs b/Frontend/CO.CDP.OrganisationApp/Pages/Cookies.cshtml.cs index 0c8d8ede7..3e5520105 100644 --- a/Frontend/CO.CDP.OrganisationApp/Pages/Cookies.cshtml.cs +++ b/Frontend/CO.CDP.OrganisationApp/Pages/Cookies.cshtml.cs @@ -9,8 +9,8 @@ namespace CO.CDP.OrganisationApp.Pages; [AuthenticatedSessionNotRequired] public class CookiesModel( - IWebHostEnvironment env, - ITempDataService tempDataService) : PageModel + ITempDataService tempDataService, + ICookiePreferencesService cookiePreferencesService) : PageModel { [BindProperty] [Required(ErrorMessage="Choose whether you accept cookies that measure website use")] @@ -38,45 +38,26 @@ public IActionResult OnPost() return Page(); } - Response.Cookies.Append(CookieSettings.CookieName, ((int)CookieAcceptance).ToString(), new CookieOptions{ - Expires = DateTimeOffset.UtcNow.AddDays(365), - IsEssential = true, - HttpOnly = false, - Secure = !env.IsDevelopment() - }); + switch(CookieAcceptance) + { + case CookieAcceptanceValues.Accept: + cookiePreferencesService.Accept(); + break; + + case CookieAcceptanceValues.Reject: + cookiePreferencesService.Reject(); + break; + } if (!string.IsNullOrEmpty(ReturnUrl) && Url.IsLocalUrl(ReturnUrl)) { - return LocalRedirect(QueryHelpers.AddQueryString(ReturnUrl, CookieSettings.CookiesAcceptedQueryString, "true")); + return LocalRedirect(QueryHelpers.AddQueryString(ReturnUrl, CookieSettings.CookieBannerInteractionQueryString, "true")); } + tempDataService.Put(FlashMessageTypes.Success, new FlashMessage( "You’ve set your cookie preferences." )); return RedirectToPage("/Cookies"); } - - public bool RadioIsChecked(CookieAcceptanceValues value) - { - return Request.Cookies.ContainsKey(CookieSettings.CookieName) && Request.Cookies[CookieSettings.CookieName] == ((int)value).ToString(); - } -} - -public enum CookieAcceptanceValues -{ - Accept=1, - Reject=2 -} - -public static class CookieSettings -{ - public const string CookieAcceptanceFieldName = "CookieAcceptance"; - public const string CookieSettingsPageReturnUrlFieldName = "ReturnUrl"; - public const string CookiesAcceptedQueryString = "cookiesAccepted"; - - // Cookie name in FTS is FT_COOKIES_PREFERENCES_SET - // Cookie values have been configured to match (See the 1 and 2 in the CookieAcceptanceValues enum). - // So if we're sharing a domain in production,we could switch to using the same cookie name and remove the frontend page - // (But keep the post handler for setting it - otherwise our cookie banner would need to post across to their subdomain) - public const string CookieName = "SIRSI_COOKIES_PREFERENCES_SET"; } \ No newline at end of file diff --git a/Frontend/CO.CDP.OrganisationApp/Pages/Shared/_CookieBanner.cshtml b/Frontend/CO.CDP.OrganisationApp/Pages/Shared/_CookieBanner.cshtml index 2ed94f1e9..ed34a74d8 100644 --- a/Frontend/CO.CDP.OrganisationApp/Pages/Shared/_CookieBanner.cshtml +++ b/Frontend/CO.CDP.OrganisationApp/Pages/Shared/_CookieBanner.cshtml @@ -1,8 +1,18 @@ +@using Microsoft.AspNetCore.Http.Extensions; +@using Microsoft.AspNetCore.Http; +@using Microsoft.AspNetCore.WebUtilities; +@inject ICookiePreferencesService cookiePreferencesService; + @{ - string currentUrl = Context.Request.PathBase + Context.Request.Path + Context.Request.QueryString; + var queryDict = QueryHelpers.ParseQuery(Context.Request.QueryString.Value); + queryDict.Remove(CookieSettings.FtsHandoverParameter); + var queryBuilder = new QueryBuilder(queryDict); + QueryString updatedQueryString = queryBuilder.ToQueryString(); + string currentUrl = Context.Request.PathBase + Context.Request.Path + updatedQueryString; } -@if (!Context.Request.Cookies.ContainsKey(CookieSettings.CookieName) && !Context.Request.Path.StartsWithSegments("/cookies")) { +@if (cookiePreferencesService.IsUnknown() && !Context.Request.Query.ContainsKey(CookieSettings.CookieBannerInteractionQueryString) && !Context.Request.Path.StartsWithSegments("/cookies")) +{
@@ -32,9 +42,9 @@
} -@if (Context.Request.Cookies.ContainsKey(CookieSettings.CookieName) && Context.Request.Query.ContainsKey(CookieSettings.CookiesAcceptedQueryString)) +@if (Context.Request.Cookies.ContainsKey(CookieSettings.CookieName) && Context.Request.Query.ContainsKey(CookieSettings.CookieBannerInteractionQueryString)) { - var acceptedState = (Context.Request.Cookies.ContainsKey(CookieSettings.CookieName) && Context.Request.Cookies[CookieSettings.CookieName] == ((int)CookieAcceptanceValues.Accept).ToString()) ? "accepted" : "rejected"; + var acceptedState = (Context.Request.Cookies.ContainsKey(CookieSettings.CookieName) && Context.Request.Cookies[CookieSettings.CookieName] == (CookieAcceptanceValues.Accept).ToString()) ? "accepted" : "rejected";
@@ -47,7 +57,7 @@
@{ var currentQueryString = Context.Request.QueryString.ToString(); - var cleanedQueryString = currentQueryString.Replace($"{CookieSettings.CookiesAcceptedQueryString}=true", ""); + var cleanedQueryString = currentQueryString.Replace($"{CookieSettings.CookieBannerInteractionQueryString}=true", ""); cleanedQueryString = cleanedQueryString == "?" ? "" : cleanedQueryString; } Hide cookie message diff --git a/Frontend/CO.CDP.OrganisationApp/Pages/Shared/_Layout.cshtml b/Frontend/CO.CDP.OrganisationApp/Pages/Shared/_Layout.cshtml index a39576046..74b16aba3 100644 --- a/Frontend/CO.CDP.OrganisationApp/Pages/Shared/_Layout.cshtml +++ b/Frontend/CO.CDP.OrganisationApp/Pages/Shared/_Layout.cshtml @@ -5,6 +5,7 @@ @inject Microsoft.Extensions.Configuration.IConfiguration config @inject IUserInfoService userInfoService +@inject ICookiePreferencesService cookiePreferencesService; @inject IHtmlLocalizer Localizer @@ -12,8 +13,7 @@ @{ var googleTagId = config["GoogleTagId"]; if (!string.IsNullOrWhiteSpace(googleTagId) - && Context.Request.Cookies.ContainsKey(CookieSettings.CookieName) - && Context.Request.Cookies[CookieSettings.CookieName] == ((int)CookieAcceptanceValues.Accept).ToString()) + && cookiePreferencesService.IsAccepted()) { diff --git a/Frontend/CO.CDP.OrganisationApp/Program.cs b/Frontend/CO.CDP.OrganisationApp/Program.cs index 19be14adb..1a9b5d76c 100644 --- a/Frontend/CO.CDP.OrganisationApp/Program.cs +++ b/Frontend/CO.CDP.OrganisationApp/Program.cs @@ -211,6 +211,9 @@ builder.Services.AddSingleton(); builder.Services.AddAuthorization(); +builder.Services.AddScoped(); +builder.Services.AddScoped(); + builder.Services.AddHealthChecks(); // @see DP-723 for details: https://noticingsystem.atlassian.net/browse/DP-723?focusedCommentId=27796 @@ -221,6 +224,7 @@ var app = builder.Build(); app.UseForwardedHeaders(); app.UseMiddleware(); +app.UseMiddleware(); // Configure the HTTP request pipeline. if (!app.Environment.IsDevelopment()) From d99e089eafec4c265deb8323b2ce72f4834f22c4 Mon Sep 17 00:00:00 2001 From: Andy Mantell <134642+andymantell@users.noreply.github.com> Date: Fri, 6 Dec 2024 12:57:15 +0000 Subject: [PATCH 2/4] Updating fts url service to send cookie values on outbound links --- .../CookieAcceptanceMiddlewareTests.cs | 74 +++++++++++++ .../CookiePreferencesServiceTests.cs | 103 ++++++++++++++++++ .../FtsUrlServiceTests.cs | 43 ++++++-- .../CookiePreferencesService.cs | 38 +++---- .../CO.CDP.OrganisationApp/FtsUrlService.cs | 20 ++-- .../ICookiePreferencesService.cs | 13 +++ .../CO.CDP.OrganisationApp/IFtsUrlService.cs | 6 + 7 files changed, 257 insertions(+), 40 deletions(-) create mode 100644 Frontend/CO.CDP.OrganisationApp.Tests/CookieAcceptanceMiddlewareTests.cs create mode 100644 Frontend/CO.CDP.OrganisationApp.Tests/CookiePreferencesServiceTests.cs create mode 100644 Frontend/CO.CDP.OrganisationApp/ICookiePreferencesService.cs create mode 100644 Frontend/CO.CDP.OrganisationApp/IFtsUrlService.cs diff --git a/Frontend/CO.CDP.OrganisationApp.Tests/CookieAcceptanceMiddlewareTests.cs b/Frontend/CO.CDP.OrganisationApp.Tests/CookieAcceptanceMiddlewareTests.cs new file mode 100644 index 000000000..77f1eacc7 --- /dev/null +++ b/Frontend/CO.CDP.OrganisationApp.Tests/CookieAcceptanceMiddlewareTests.cs @@ -0,0 +1,74 @@ +using Moq; +using Microsoft.AspNetCore.Http; + +namespace CO.CDP.OrganisationApp.Tests; + +public class CookieAcceptanceMiddlewareTests +{ + private readonly Mock _cookiePreferencesServiceMock; + private readonly Mock _nextDelegateMock; + private readonly CookieAcceptanceMiddleware _middleware; + private readonly DefaultHttpContext _httpContext; + + public CookieAcceptanceMiddlewareTests() + { + _cookiePreferencesServiceMock = new Mock(); + _nextDelegateMock = new Mock(); + _middleware = new CookieAcceptanceMiddleware(_cookiePreferencesServiceMock.Object); + _httpContext = new DefaultHttpContext(); + } + + [Fact] + public async Task InvokeAsync_ShouldCallAccept_WhenQueryParameterIsTrue() + { + _httpContext.Request.QueryString = new QueryString($"?{CookieSettings.FtsHandoverParameter}=true"); + + await _middleware.InvokeAsync(_httpContext, _nextDelegateMock.Object); + + _cookiePreferencesServiceMock.Verify(s => s.Accept(), Times.Once); + _nextDelegateMock.Verify(n => n(_httpContext), Times.Once); + } + + [Fact] + public async Task InvokeAsync_ShouldCallReject_WhenQueryParameterIsFalse() + { + _httpContext.Request.QueryString = new QueryString($"?{CookieSettings.FtsHandoverParameter}=false"); + + await _middleware.InvokeAsync(_httpContext, _nextDelegateMock.Object); + + _cookiePreferencesServiceMock.Verify(s => s.Reject(), Times.Once); + _nextDelegateMock.Verify(n => n(_httpContext), Times.Once); + } + + [Fact] + public async Task InvokeAsync_ShouldCallReset_WhenQueryParameterIsUnknown() + { + _httpContext.Request.QueryString = new QueryString($"?{CookieSettings.FtsHandoverParameter}=unknown"); + + await _middleware.InvokeAsync(_httpContext, _nextDelegateMock.Object); + + _cookiePreferencesServiceMock.Verify(s => s.Reset(), Times.Once); + _nextDelegateMock.Verify(n => n(_httpContext), Times.Once); + } + + [Fact] + public async Task InvokeAsync_ShouldNotCallAnyMethod_WhenQueryParameterIsMissing() + { + await _middleware.InvokeAsync(_httpContext, _nextDelegateMock.Object); + + _cookiePreferencesServiceMock.VerifyNoOtherCalls(); + _nextDelegateMock.Verify(n => n(_httpContext), Times.Once); + } + + [Fact] + public async Task InvokeAsync_ShouldNotCallAnyMethod_WhenQueryParameterHasInvalidValue() + { + _httpContext.Request.QueryString = new QueryString($"?{CookieSettings.FtsHandoverParameter}=invalid"); + + await _middleware.InvokeAsync(_httpContext, _nextDelegateMock.Object); + + _cookiePreferencesServiceMock.VerifyNoOtherCalls(); + _nextDelegateMock.Verify(n => n(_httpContext), Times.Once); + } +} + diff --git a/Frontend/CO.CDP.OrganisationApp.Tests/CookiePreferencesServiceTests.cs b/Frontend/CO.CDP.OrganisationApp.Tests/CookiePreferencesServiceTests.cs new file mode 100644 index 000000000..3ff0a560f --- /dev/null +++ b/Frontend/CO.CDP.OrganisationApp.Tests/CookiePreferencesServiceTests.cs @@ -0,0 +1,103 @@ +using Moq; +using FluentAssertions; +using Microsoft.AspNetCore.Http; + +namespace CO.CDP.OrganisationApp.Tests; + +public class CookiePreferencesServiceTests +{ + private readonly Mock _httpContextAccessorMock; + private readonly Mock _httpContextMock; + private readonly Mock _httpRequestMock; + private readonly Mock _httpResponseMock; + private readonly Mock _responseCookiesMock; + private readonly Mock _requestCookiesMock; + private readonly CookiePreferencesService _cookiePreferencesService; + + public CookiePreferencesServiceTests() + { + _httpContextAccessorMock = new Mock(); + _httpContextMock = new Mock(); + _httpRequestMock = new Mock(); + _httpResponseMock = new Mock(); + _responseCookiesMock = new Mock(); + _requestCookiesMock = new Mock(); + + _httpContextMock.Setup(c => c.Request).Returns(_httpRequestMock.Object); + _httpContextMock.Setup(c => c.Response).Returns(_httpResponseMock.Object); + _httpRequestMock.Setup(r => r.Cookies).Returns(_requestCookiesMock.Object); + _httpResponseMock.Setup(r => r.Cookies).Returns(_responseCookiesMock.Object); + + _httpContextAccessorMock.Setup(a => a.HttpContext).Returns(_httpContextMock.Object); + + _cookiePreferencesService = new CookiePreferencesService(_httpContextAccessorMock.Object); + } + + [Fact] + public void Constructor_ShouldThrowException_WhenHttpContextIsNull() + { + var accessor = new Mock(); + accessor.Setup(a => a.HttpContext).Returns((HttpContext)null!); + + Action act = () => new CookiePreferencesService(accessor.Object); + + act.Should().Throw() + .WithMessage("No active HTTP context."); + } + + [Fact] + public void Accept_ShouldSetCookieWithAcceptValue() + { + _cookiePreferencesService.Accept(); + + _responseCookiesMock.Verify(c => c.Append(CookieSettings.CookieName, "Accept", It.IsAny()), Times.Once); + _cookiePreferencesService.GetValue().Should().Be(CookieAcceptanceValues.Accept); + _cookiePreferencesService.IsAccepted().Should().BeTrue(); + } + + [Fact] + public void Reject_ShouldSetCookieWithRejectValue() + { + _cookiePreferencesService.Reject(); + + _responseCookiesMock.Verify(c => c.Append(CookieSettings.CookieName, "Reject", It.IsAny()), Times.Once); + _cookiePreferencesService.GetValue().Should().Be(CookieAcceptanceValues.Reject); + _cookiePreferencesService.IsRejected().Should().BeTrue(); + } + + [Fact] + public void Reset_ShouldDeleteCookie_AndSetPendingValueToUnknown() + { + _cookiePreferencesService.Reset(); + + _responseCookiesMock.Verify(c => c.Delete(CookieSettings.CookieName), Times.Once); + _cookiePreferencesService.GetValue().Should().Be(CookieAcceptanceValues.Unknown); + _cookiePreferencesService.IsUnknown().Should().BeTrue(); + } + + [Fact] + public void GetValue_ShouldReturnPendingValue_WhenSet() + { + _cookiePreferencesService.SetCookie(CookieAcceptanceValues.Accept); + + _cookiePreferencesService.GetValue().Should().Be(CookieAcceptanceValues.Accept); + } + + [Fact] + public void GetValue_ShouldReturnValueFromRequestCookie_WhenNoPendingValue() + { + _requestCookiesMock.Setup(c => c.ContainsKey(CookieSettings.CookieName)).Returns(true); + _requestCookiesMock.Setup(c => c[CookieSettings.CookieName]).Returns("Reject"); + + _cookiePreferencesService.GetValue().Should().Be(CookieAcceptanceValues.Reject); + } + + [Fact] + public void GetValue_ShouldReturnUnknown_WhenCookieIsNotPresent() + { + _requestCookiesMock.Setup(c => c.ContainsKey(CookieSettings.CookieName)).Returns(false); + + _cookiePreferencesService.GetValue().Should().Be(CookieAcceptanceValues.Unknown); + } +} + diff --git a/Frontend/CO.CDP.OrganisationApp.Tests/FtsUrlServiceTests.cs b/Frontend/CO.CDP.OrganisationApp.Tests/FtsUrlServiceTests.cs index 03991869a..6de546262 100644 --- a/Frontend/CO.CDP.OrganisationApp.Tests/FtsUrlServiceTests.cs +++ b/Frontend/CO.CDP.OrganisationApp.Tests/FtsUrlServiceTests.cs @@ -4,16 +4,19 @@ using FluentAssertions; namespace CO.CDP.OrganisationApp.Tests; + public class FtsUrlServiceTests { private readonly Mock _configurationMock; - private readonly FtsUrlService _service; + private readonly Mock _cookiePreferencesService; + private readonly IFtsUrlService _service; public FtsUrlServiceTests() { _configurationMock = new Mock(); + _cookiePreferencesService = new Mock(); _configurationMock.Setup(c => c["FtsService"]).Returns("https://example.com/"); - _service = new FtsUrlService(_configurationMock.Object); + _service = new FtsUrlService(_configurationMock.Object, _cookiePreferencesService.Object); } [Fact] @@ -21,23 +24,23 @@ public void Constructor_ShouldThrowException_WhenFtsServiceIsNotConfigured() { _configurationMock.Setup(c => c["FtsService"]).Returns((string?)null); - Action action = () => new FtsUrlService(_configurationMock.Object); + Action action = () => new FtsUrlService(_configurationMock.Object, _cookiePreferencesService.Object); action.Should().Throw() - .WithMessage("FtsService is not configured."); + .WithMessage("FtsService is not configured."); } [Fact] public void BuildUrl_ShouldTrimTrailingSlashFromBaseServiceUrl() { _configurationMock.Setup(c => c["FtsService"]).Returns("https://example.com/"); - var service = new FtsUrlService(_configurationMock.Object); + var service = new FtsUrlService(_configurationMock.Object, _cookiePreferencesService.Object); var endpoint = "test-endpoint"; CultureInfo.CurrentUICulture = new CultureInfo("en-GB"); var result = service.BuildUrl(endpoint); - result.Should().Be("https://example.com/test-endpoint?language=en_GB"); + result.Should().Be("https://example.com/test-endpoint?language=en_GB&cookies_accepted=unknown"); } [Fact] @@ -48,18 +51,18 @@ public void BuildUrl_ShouldConstructCorrectUrl_WhenOnlyEndpointIsProvided() var result = _service.BuildUrl(endpoint); - result.Should().Be("https://example.com/test-endpoint?language=en_GB"); + result.Should().Be("https://example.com/test-endpoint?language=en_GB&cookies_accepted=unknown"); } [Fact] - public void BuildUrl_ShouldConstructCorrectUrl_WhenLanguageisWelsh() + public void BuildUrl_ShouldConstructCorrectUrl_WhenLanguageIsWelsh() { var endpoint = "test-endpoint"; CultureInfo.CurrentUICulture = new CultureInfo("cy"); var result = _service.BuildUrl(endpoint); - result.Should().Be("https://example.com/test-endpoint?language=cy"); + result.Should().Be("https://example.com/test-endpoint?language=cy&cookies_accepted=unknown"); } [Fact] @@ -71,7 +74,7 @@ public void BuildUrl_ShouldIncludeOrganisationId_WhenProvided() var result = _service.BuildUrl(endpoint, organisationId); - result.Should().Be($"https://example.com/test-endpoint?language=en_GB&organisation_id={organisationId}"); + result.Should().Be($"https://example.com/test-endpoint?language=en_GB&organisation_id={organisationId}&cookies_accepted=unknown"); } [Fact] @@ -83,12 +86,13 @@ public void BuildUrl_ShouldIncludeRedirectUrl_WhenProvided() var result = _service.BuildUrl(endpoint, null, redirectUrl); - result.Should().Be("https://example.com/test-endpoint?language=en_GB&redirect_url=%2Fredirect-path"); + result.Should().Be("https://example.com/test-endpoint?language=en_GB&redirect_url=%2Fredirect-path&cookies_accepted=unknown"); } [Fact] public void BuildUrl_ShouldIncludeAllParameters_WhenAllAreProvided() { + _cookiePreferencesService.Setup(c => c.GetValue()).Returns(CookieAcceptanceValues.Accept); var endpoint = "test-endpoint"; var organisationId = Guid.NewGuid(); var redirectUrl = "/redirect-path"; @@ -96,6 +100,21 @@ public void BuildUrl_ShouldIncludeAllParameters_WhenAllAreProvided() var result = _service.BuildUrl(endpoint, organisationId, redirectUrl); - result.Should().Be($"https://example.com/test-endpoint?language=en_GB&organisation_id={organisationId}&redirect_url=%2Fredirect-path"); + result.Should().Be($"https://example.com/test-endpoint?language=en_GB&organisation_id={organisationId}&redirect_url=%2Fredirect-path&cookies_accepted=true"); + } + + [Theory] + [InlineData(CookieAcceptanceValues.Accept, "true")] + [InlineData(CookieAcceptanceValues.Reject, "false")] + [InlineData(CookieAcceptanceValues.Unknown, "unknown")] + public void BuildUrl_ShouldIncludeCookiesAcceptedParameter_BasedOnCookiePreferences(CookieAcceptanceValues preference, string expectedValue) + { + _cookiePreferencesService.Setup(c => c.GetValue()).Returns(preference); + var endpoint = "test-endpoint"; + CultureInfo.CurrentUICulture = new CultureInfo("en-GB"); + + var result = _service.BuildUrl(endpoint); + + result.Should().Be($"https://example.com/test-endpoint?language=en_GB&cookies_accepted={expectedValue}"); } } diff --git a/Frontend/CO.CDP.OrganisationApp/CookiePreferencesService.cs b/Frontend/CO.CDP.OrganisationApp/CookiePreferencesService.cs index b5726d0a7..469d4f0fe 100644 --- a/Frontend/CO.CDP.OrganisationApp/CookiePreferencesService.cs +++ b/Frontend/CO.CDP.OrganisationApp/CookiePreferencesService.cs @@ -1,22 +1,10 @@ namespace CO.CDP.OrganisationApp; -public interface ICookiePreferencesService -{ - bool IsAccepted(); - bool IsRejected(); - void Accept(); - void Reject(); - void Reset(); - void SetCookie(CookieAcceptanceValues value); - bool ValueIs(CookieAcceptanceValues value); - bool IsUnknown(); -} - public class CookiePreferencesService : ICookiePreferencesService { private readonly HttpContext _context; - private CookieAcceptanceValues pendingAcceptanceValue; + private CookieAcceptanceValues? pendingAcceptanceValue; public CookiePreferencesService(IHttpContextAccessor httpContextAccessor) { @@ -52,30 +40,38 @@ public void SetCookie(CookieAcceptanceValues value) pendingAcceptanceValue = value; } - public bool ValueIs(CookieAcceptanceValues value) + public CookieAcceptanceValues GetValue() { - if(pendingAcceptanceValue == value) + if (pendingAcceptanceValue != null) { - return true; + return (CookieAcceptanceValues)pendingAcceptanceValue; } - return _context.Request.Cookies.ContainsKey(CookieSettings.CookieName) - && _context.Request.Cookies[CookieSettings.CookieName] == value.ToString(); + if(_context.Request.Cookies.ContainsKey(CookieSettings.CookieName)) + { + if (Enum.TryParse(typeof(CookieAcceptanceValues), _context.Request.Cookies[CookieSettings.CookieName], true, out var result)) + { + return (CookieAcceptanceValues)result; + } + } + + return CookieAcceptanceValues.Unknown; + } public bool IsAccepted() { - return ValueIs(CookieAcceptanceValues.Accept); + return GetValue() == CookieAcceptanceValues.Accept; } public bool IsRejected() { - return ValueIs(CookieAcceptanceValues.Reject); + return GetValue() == CookieAcceptanceValues.Reject; } public bool IsUnknown() { - return ValueIs(CookieAcceptanceValues.Unknown); + return GetValue() == CookieAcceptanceValues.Unknown; } } public enum CookieAcceptanceValues diff --git a/Frontend/CO.CDP.OrganisationApp/FtsUrlService.cs b/Frontend/CO.CDP.OrganisationApp/FtsUrlService.cs index d6a512ab5..1410bb96c 100644 --- a/Frontend/CO.CDP.OrganisationApp/FtsUrlService.cs +++ b/Frontend/CO.CDP.OrganisationApp/FtsUrlService.cs @@ -1,17 +1,18 @@ using Microsoft.AspNetCore.Http.Extensions; using System.Globalization; -using System.Web; namespace CO.CDP.OrganisationApp; public class FtsUrlService : IFtsUrlService { private readonly string _ftsService; + private readonly ICookiePreferencesService _cookiePreferencesService; - public FtsUrlService(IConfiguration configuration) + public FtsUrlService(IConfiguration configuration, ICookiePreferencesService cookiePreferencesService) { var ftsService = configuration["FtsService"] ?? throw new InvalidOperationException("FtsService is not configured."); _ftsService = ftsService.TrimEnd('/'); + _cookiePreferencesService = cookiePreferencesService; } public string BuildUrl(string endpoint, Guid? organisationId = null, string? redirectUrl = null) @@ -36,13 +37,18 @@ public string BuildUrl(string endpoint, Guid? organisationId = null, string? red queryBuilder.Add("redirect_url", redirectUrl); } + CookieAcceptanceValues cookiesAccepted = _cookiePreferencesService.GetValue(); + string cookiesAcceptedValue = cookiesAccepted switch + { + CookieAcceptanceValues.Accept => "true", + CookieAcceptanceValues.Reject => "false", + _ => "unknown" + }; + + queryBuilder.Add(CookieSettings.FtsHandoverParameter, cookiesAcceptedValue); + uriBuilder.Query = queryBuilder.ToQueryString().Value; return uriBuilder.Uri.ToString(); } } - -public interface IFtsUrlService -{ - string BuildUrl(string endpoint, Guid? organisationId = null, string? redirectUrl = null); -} \ No newline at end of file diff --git a/Frontend/CO.CDP.OrganisationApp/ICookiePreferencesService.cs b/Frontend/CO.CDP.OrganisationApp/ICookiePreferencesService.cs new file mode 100644 index 000000000..11e6c5c8a --- /dev/null +++ b/Frontend/CO.CDP.OrganisationApp/ICookiePreferencesService.cs @@ -0,0 +1,13 @@ +namespace CO.CDP.OrganisationApp; + +public interface ICookiePreferencesService +{ + bool IsAccepted(); + bool IsRejected(); + void Accept(); + void Reject(); + void Reset(); + void SetCookie(CookieAcceptanceValues value); + bool IsUnknown(); + CookieAcceptanceValues GetValue(); +} \ No newline at end of file diff --git a/Frontend/CO.CDP.OrganisationApp/IFtsUrlService.cs b/Frontend/CO.CDP.OrganisationApp/IFtsUrlService.cs new file mode 100644 index 000000000..068501e2a --- /dev/null +++ b/Frontend/CO.CDP.OrganisationApp/IFtsUrlService.cs @@ -0,0 +1,6 @@ +namespace CO.CDP.OrganisationApp; + +public interface IFtsUrlService +{ + string BuildUrl(string endpoint, Guid? organisationId = null, string? redirectUrl = null); +} \ No newline at end of file From be732010c5b9267b42a855ad9d8bf615b879e389 Mon Sep 17 00:00:00 2001 From: Andy Mantell <134642+andymantell@users.noreply.github.com> Date: Fri, 6 Dec 2024 13:07:24 +0000 Subject: [PATCH 3/4] replace a couple more direct inspections of cookies with calls to the cookie service --- .../CO.CDP.OrganisationApp/Pages/Shared/_CookieBanner.cshtml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Frontend/CO.CDP.OrganisationApp/Pages/Shared/_CookieBanner.cshtml b/Frontend/CO.CDP.OrganisationApp/Pages/Shared/_CookieBanner.cshtml index ed34a74d8..1f15fb4d5 100644 --- a/Frontend/CO.CDP.OrganisationApp/Pages/Shared/_CookieBanner.cshtml +++ b/Frontend/CO.CDP.OrganisationApp/Pages/Shared/_CookieBanner.cshtml @@ -42,9 +42,9 @@
} -@if (Context.Request.Cookies.ContainsKey(CookieSettings.CookieName) && Context.Request.Query.ContainsKey(CookieSettings.CookieBannerInteractionQueryString)) +@if (!cookiePreferencesService.IsUnknown() && Context.Request.Query.ContainsKey(CookieSettings.CookieBannerInteractionQueryString)) { - var acceptedState = (Context.Request.Cookies.ContainsKey(CookieSettings.CookieName) && Context.Request.Cookies[CookieSettings.CookieName] == (CookieAcceptanceValues.Accept).ToString()) ? "accepted" : "rejected"; + var acceptedState = cookiePreferencesService.IsAccepted() ? "accepted" : "rejected";
From 4c4f0d64aee5f005f462ffda6fbb4dcb71e484e0 Mon Sep 17 00:00:00 2001 From: Andy Mantell <134642+andymantell@users.noreply.github.com> Date: Fri, 6 Dec 2024 13:47:35 +0000 Subject: [PATCH 4/4] code review updates --- .../CookiePreferencesServiceTests.cs | 2 +- .../CookieAcceptanceMiddleware.cs | 20 +++++++++---------- .../CookiePreferencesService.cs | 2 +- .../ICookiePreferencesService.cs | 1 - .../Pages/ContactUs.cshtml.cs | 1 - 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/Frontend/CO.CDP.OrganisationApp.Tests/CookiePreferencesServiceTests.cs b/Frontend/CO.CDP.OrganisationApp.Tests/CookiePreferencesServiceTests.cs index 3ff0a560f..76f1d074c 100644 --- a/Frontend/CO.CDP.OrganisationApp.Tests/CookiePreferencesServiceTests.cs +++ b/Frontend/CO.CDP.OrganisationApp.Tests/CookiePreferencesServiceTests.cs @@ -78,7 +78,7 @@ public void Reset_ShouldDeleteCookie_AndSetPendingValueToUnknown() [Fact] public void GetValue_ShouldReturnPendingValue_WhenSet() { - _cookiePreferencesService.SetCookie(CookieAcceptanceValues.Accept); + _cookiePreferencesService.Accept(); _cookiePreferencesService.GetValue().Should().Be(CookieAcceptanceValues.Accept); } diff --git a/Frontend/CO.CDP.OrganisationApp/CookieAcceptanceMiddleware.cs b/Frontend/CO.CDP.OrganisationApp/CookieAcceptanceMiddleware.cs index e873e2238..7546418e5 100644 --- a/Frontend/CO.CDP.OrganisationApp/CookieAcceptanceMiddleware.cs +++ b/Frontend/CO.CDP.OrganisationApp/CookieAcceptanceMiddleware.cs @@ -8,17 +8,17 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next) { string cookiesAccepted = cookiesAcceptedValue.ToString().ToLower(); - if (cookiesAccepted == "true") + switch (cookiesAccepted) { - cookiePreferencesService.Accept(); - } - else if (cookiesAccepted == "false") - { - cookiePreferencesService.Reject(); - } - else if (cookiesAccepted == "unknown") - { - cookiePreferencesService.Reset(); + case "true": + cookiePreferencesService.Accept(); + break; + case "false": + cookiePreferencesService.Reject(); + break; + case "unknown": + cookiePreferencesService.Reset(); + break; } } diff --git a/Frontend/CO.CDP.OrganisationApp/CookiePreferencesService.cs b/Frontend/CO.CDP.OrganisationApp/CookiePreferencesService.cs index 469d4f0fe..840a10750 100644 --- a/Frontend/CO.CDP.OrganisationApp/CookiePreferencesService.cs +++ b/Frontend/CO.CDP.OrganisationApp/CookiePreferencesService.cs @@ -28,7 +28,7 @@ public void Reset() pendingAcceptanceValue = CookieAcceptanceValues.Unknown; } - public void SetCookie(CookieAcceptanceValues value) + private void SetCookie(CookieAcceptanceValues value) { _context.Response.Cookies.Append(CookieSettings.CookieName, value.ToString(), new CookieOptions { diff --git a/Frontend/CO.CDP.OrganisationApp/ICookiePreferencesService.cs b/Frontend/CO.CDP.OrganisationApp/ICookiePreferencesService.cs index 11e6c5c8a..fce705800 100644 --- a/Frontend/CO.CDP.OrganisationApp/ICookiePreferencesService.cs +++ b/Frontend/CO.CDP.OrganisationApp/ICookiePreferencesService.cs @@ -7,7 +7,6 @@ public interface ICookiePreferencesService void Accept(); void Reject(); void Reset(); - void SetCookie(CookieAcceptanceValues value); bool IsUnknown(); CookieAcceptanceValues GetValue(); } \ No newline at end of file diff --git a/Frontend/CO.CDP.OrganisationApp/Pages/ContactUs.cshtml.cs b/Frontend/CO.CDP.OrganisationApp/Pages/ContactUs.cshtml.cs index f74bf4530..e4b08d9e7 100644 --- a/Frontend/CO.CDP.OrganisationApp/Pages/ContactUs.cshtml.cs +++ b/Frontend/CO.CDP.OrganisationApp/Pages/ContactUs.cshtml.cs @@ -8,7 +8,6 @@ using Microsoft.AspNetCore.Mvc.RazorPages; using System.ComponentModel; using System.ComponentModel.DataAnnotations; -using CO.CDP.OrganisationApp.Models; namespace CO.CDP.OrganisationApp.Pages;