Merge pull request #1038 from TechnologyEnhancedLearning/Dev/Feature/TD-3723-LH-Pen-Test-Fixes

Pen test Fixes to RC branch-EnglishIvy

Author: AnjuJose011

LearningHub.Nhs.WebUI/Controllers/HomeController.cs

@@ -15,13 +15,13 @@ namespace LearningHub.Nhs.WebUI.Controllers
     using LearningHub.Nhs.WebUI.Helpers;
     using LearningHub.Nhs.WebUI.Interfaces;
     using LearningHub.Nhs.WebUI.Models;
-    using Microsoft.ApplicationInsights.AspNetCore;
     using Microsoft.AspNetCore.Authentication;
     using Microsoft.AspNetCore.Authentication.Cookies;
     using Microsoft.AspNetCore.Authorization;
     using Microsoft.AspNetCore.Diagnostics;
     using Microsoft.AspNetCore.Hosting;
     using Microsoft.AspNetCore.Mvc;
+    using Microsoft.Extensions.Configuration;
     using Microsoft.Extensions.Logging;
     using Microsoft.Extensions.Options;
     using Microsoft.FeatureManagement;
@@ -39,6 +39,7 @@ public class HomeController : BaseController
         private readonly IDashboardService dashboardService;
         private readonly IContentService contentService;
         private readonly IFeatureManager featureManager;
+        private readonly Microsoft.Extensions.Configuration.IConfiguration configuration;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="HomeController"/> class.
@@ -53,6 +54,7 @@ public class HomeController : BaseController
         /// <param name="dashboardService">Dashboard service.</param>
         /// <param name="contentService">Content service.</param>
         /// <param name="featureManager"> featureManager.</param>
+        /// <param name="configuration"> config.</param>
         public HomeController(
             IHttpClientFactory httpClientFactory,
             IWebHostEnvironment hostingEnvironment,
@@ -63,7 +65,8 @@ public HomeController(
             LearningHubAuthServiceConfig authConfig,
             IDashboardService dashboardService,
             IContentService contentService,
-            IFeatureManager featureManager)
+            IFeatureManager featureManager,
+            Microsoft.Extensions.Configuration.IConfiguration configuration)
         : base(hostingEnvironment, httpClientFactory, logger, settings.Value)
         {
             this.authConfig = authConfig;
@@ -72,6 +75,7 @@ public HomeController(
             this.dashboardService = dashboardService;
             this.contentService = contentService;
             this.featureManager = featureManager;
+            this.configuration = configuration;
         }
 
         /// <summary>
@@ -133,11 +137,12 @@ public IActionResult CreateAccount()
         public IActionResult Error(int? httpStatusCode)
         {
             string originalPathUrlMessage = null;
-
+            string originalPath = null;
             if (httpStatusCode.HasValue && httpStatusCode.Value == 404)
             {
                 var exceptionHandlerPathFeature = this.HttpContext.Features.Get<IStatusCodeReExecuteFeature>();
-                originalPathUrlMessage = $"Page Not Found url: {exceptionHandlerPathFeature?.OriginalPath}. ";
+                originalPath = exceptionHandlerPathFeature?.OriginalPath;
+                originalPathUrlMessage = $"Page Not Found url: {originalPath}. ";
             }
 
             if (this.User.Identity.IsAuthenticated)
@@ -165,16 +170,26 @@ public IActionResult Error(int? httpStatusCode)
             }
             else
             {
-                this.ViewBag.ErrorHeader = httpStatusCode.Value switch
+                if (originalPath == "/TooManyRequests")
                 {
-                    401 => "You do not have permission to access this page",
-                    404 => "We cannot find the page you are looking for",
-                    _ => "We cannot find the page you are looking for",
-                };
+                    this.ViewBag.Period = this.configuration["IpRateLimiting:GeneralRules:0:Period"];
+                    this.ViewBag.Limit = this.configuration["IpRateLimiting:GeneralRules:0:Limit"];
 
-                this.ViewBag.HttpStatusCode = httpStatusCode.Value;
-                this.ViewBag.HomePageUrl = "/home";
-                return this.View("CustomError");
+                    return this.View("TooManyRequests");
+                }
+                else
+                {
+                    this.ViewBag.ErrorHeader = httpStatusCode.Value switch
+                    {
+                        401 => "You do not have permission to access this page",
+                        404 => "We cannot find the page you are looking for",
+                        _ => "We cannot find the page you are looking for",
+                    };
+
+                    this.ViewBag.HttpStatusCode = httpStatusCode.Value;
+                    this.ViewBag.HomePageUrl = "/home";
+                    return this.View("CustomError");
+                }
             }
         }
 
@@ -320,14 +335,19 @@ public IActionResult NhsSites()
         }
 
         /// <summary>
-        /// The Logout.
-        /// This is directly referenced in the LoginWizardFilter to allow
-        /// logouts to bypass LoginWizard redirects.
-        /// If the name is changed, the LoginWizardFilter MUST be updated.
+        /// The ChangePasswordAcknowledgement.
         /// </summary>
         /// <returns>The <see cref="IActionResult"/>.</returns>
-        [AllowAnonymous]
-        public IActionResult Logout()
+        public IActionResult ChangePasswordAcknowledgement()
+        {
+            return this.View();
+        }
+
+        /// <summary>
+        /// StatusUpdate.
+        /// </summary>
+        /// <returns>Actionresult.</returns>
+        public IActionResult UserLogout()
         {
             if (!(this.User?.Identity.IsAuthenticated ?? false))
             {
@@ -337,6 +357,20 @@ public IActionResult Logout()
             return new SignOutResult(new[] { CookieAuthenticationDefaults.AuthenticationScheme, "oidc" });
         }
 
+        /// <summary>
+        /// The Logout.
+        /// This is directly referenced in the LoginWizardFilter to allow
+        /// logouts to bypass LoginWizard redirects.
+        /// If the name is changed, the LoginWizardFilter MUST be updated.
+        /// </summary>
+        /// <returns>The <see cref="IActionResult"/>.</returns>
+        [AllowAnonymous]
+        public IActionResult Logout()
+        {
+            var redirectUri = $"{this.configuration["LearningHubAuthServiceConfig:Authority"]}/Home/SetIsPasswordUpdate?isLogout=true";
+            return this.Redirect(redirectUri);
+        }
+
         /// <summary>
         /// The SessionTimeout.
         /// </summary>

LearningHub.Nhs.WebUI/Controllers/MyAccountController.cs

@@ -21,10 +21,12 @@
     using Microsoft.AspNetCore.Hosting;
     using Microsoft.AspNetCore.Mvc;
     using Microsoft.AspNetCore.Routing;
+    using Microsoft.Extensions.Configuration;
     using Microsoft.Extensions.Logging;
     using Microsoft.Extensions.Options;
     using NHSUKViewComponents.Web.ViewModels;
     using ChangePasswordViewModel = LearningHub.Nhs.WebUI.Models.UserProfile.ChangePasswordViewModel;
+    using IConfiguration = Microsoft.Extensions.Configuration.IConfiguration;
 
     /// <summary>
     /// The UserController.
@@ -43,6 +45,7 @@ public partial class MyAccountController : BaseController
         private readonly ISpecialtyService specialtyService;
         private readonly ILocationService locationService;
         private readonly ICacheService cacheService;
+        private readonly IConfiguration configuration;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="MyAccountController"/> class.
@@ -61,6 +64,7 @@ public partial class MyAccountController : BaseController
         /// <param name="locationService">The locationService.</param>
         /// <param name="multiPageFormService">The multiPageFormService<see cref="IMultiPageFormService"/>.</param>
         /// <param name="cacheService">The cacheService<see cref="ICacheService"/>.</param>
+        /// <param name="configuration">The cacheService<see cref="IConfiguration"/>.</param>
         public MyAccountController(
                 IWebHostEnvironment hostingEnvironment,
                 ILogger<ResourceController> logger,
@@ -75,7 +79,8 @@ public MyAccountController(
                 ISpecialtyService specialtyService,
                 ILocationService locationService,
                 IMultiPageFormService multiPageFormService,
-                ICacheService cacheService)
+                ICacheService cacheService,
+                IConfiguration configuration)
                 : base(hostingEnvironment, httpClientFactory, logger, settings.Value)
         {
             this.userService = userService;
@@ -88,6 +93,7 @@ public MyAccountController(
             this.locationService = locationService;
             this.multiPageFormService = multiPageFormService;
             this.cacheService = cacheService;
+            this.configuration = configuration;
         }
 
         private string LoginWizardCacheKey => $"{this.CurrentUserId}:LoginWizard";
@@ -452,9 +458,8 @@ public async Task<IActionResult> UpdatePassword(ChangePasswordViewModel model)
             if (this.ModelState.IsValid)
             {
                 await this.userService.UpdatePassword(model.NewPassword);
-
-                this.ViewBag.SuccessMessage = CommonValidationErrorMessages.PasswordSuccessMessage;
-                return this.View("SuccessMessage");
+                var redirectUri = $"{this.configuration["LearningHubAuthServiceConfig:Authority"]}/Home/SetIsPasswordUpdate?isLogout=false";
+                return this.Redirect(redirectUri);
             }
             else
             {

LearningHub.Nhs.WebUI/Helpers/InMemoryTicketStore.cs

@@ -0,0 +1,104 @@
+namespace LearningHub.Nhs.WebUI.Helpers
+{
+    using System;
+    using System.Collections.Concurrent;
+    using System.Linq;
+    using System.Threading.Tasks;
+    using Microsoft.AspNetCore.Authentication;
+    using Microsoft.AspNetCore.Authentication.Cookies;
+
+    /// <summary>
+    /// Defines the <see cref="InMemoryTicketStore" />.
+    /// </summary>
+    public class InMemoryTicketStore : ITicketStore
+    {
+        private readonly ConcurrentDictionary<string, AuthenticationTicket> cache;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="InMemoryTicketStore"/> class.
+        /// The InMemoryTicketStore.
+        /// </summary>
+        /// <param name="cache">the cache.</param>
+        public InMemoryTicketStore(ConcurrentDictionary<string, AuthenticationTicket> cache)
+        {
+            this.cache = cache;
+        }
+
+        /// <summary>
+        /// The StoreAsync.
+        /// </summary>
+        /// <param name="ticket">The ticket.</param>
+        /// <returns>The key.</returns>
+        public async Task<string> StoreAsync(AuthenticationTicket ticket)
+        {
+            var ticketUserId = ticket.Principal.Claims.Where(c => c.Type == "sub")
+                .FirstOrDefault()
+                .Value;
+            var matchingAuthTicket = this.cache.Values.FirstOrDefault(
+                t => t.Principal.Claims.FirstOrDefault(
+                    c => c.Type == "sub"
+                    && c.Value == ticketUserId) != null);
+            if (matchingAuthTicket != null)
+            {
+                var cacheKey = this.cache.Where(
+                    entry => entry.Value == matchingAuthTicket)
+                    .Select(entry => entry.Key)
+                    .FirstOrDefault();
+                this.cache.TryRemove(
+                    cacheKey,
+                    out _);
+            }
+
+            var key = Guid
+                .NewGuid()
+                .ToString();
+            await this.RenewAsync(
+                key,
+                ticket);
+            return key;
+        }
+
+        /// <summary>
+        /// The RenewAsync.
+        /// </summary>
+        /// <param name="key">The key.</param>
+        /// <param name="ticket">The ticket.</param>
+        /// <returns>The Task.</returns>
+        public Task RenewAsync(
+            string key,
+            AuthenticationTicket ticket)
+        {
+            this.cache.AddOrUpdate(
+                key,
+                ticket,
+                (_, _) => ticket);
+            return Task.CompletedTask;
+        }
+
+        /// <summary>
+        /// The RetrieveAsync.
+        /// </summary>
+        /// <param name="key">The Key.</param>
+        /// <returns>The Task.</returns>
+        public Task<AuthenticationTicket> RetrieveAsync(string key)
+        {
+            this.cache.TryGetValue(
+                key,
+                out var ticket);
+            return Task.FromResult(ticket);
+        }
+
+        /// <summary>
+        /// The RemoveAsync.
+        /// </summary>
+        /// <param name="key">The key.</param>
+        /// <returns>The Task.</returns>
+        public Task RemoveAsync(string key)
+        {
+            this.cache.TryRemove(
+                key,
+                out _);
+            return Task.CompletedTask;
+        }
+    }
+}

LearningHub.Nhs.WebUI/LearningHub.Nhs.WebUI.csproj

@@ -104,6 +104,7 @@
 
 
 	<ItemGroup>
+		<PackageReference Include="AspNetCoreRateLimit" Version="5.0.0" />
 		<PackageReference Include="AutoMapper.Extensions.Microsoft.DependencyInjection" Version="8.1.1" />
 		<PackageReference Include="Azure.Storage.Blobs" Version="12.23.0" />
 		<PackageReference Include="elfhHub.Nhs.Models" Version="3.0.9" />

LearningHub.Nhs.WebUI/Middleware/LHIPRateLimitMiddleware.cs

@@ -0,0 +1,57 @@
+namespace LearningHub.Nhs.WebUI.Middleware
+{
+    using System.Threading.Tasks;
+    using AspNetCoreRateLimit;
+    using Microsoft.AspNetCore.Http;
+    using Microsoft.Extensions.Logging;
+    using Microsoft.Extensions.Options;
+
+    /// <summary>
+    /// Defines the <see cref="LHIPRateLimitMiddleware" />.
+    /// </summary>
+    public class LHIPRateLimitMiddleware : IpRateLimitMiddleware
+    {
+        /// <summary>
+        /// Initializes a new instance of the <see cref="LHIPRateLimitMiddleware"/> class.
+        /// </summary>
+        /// <param name="next">The next.</param>
+        /// <param name="processingStrategy">The processingStrategy.</param>
+        /// <param name="options">The options.</param>
+        /// <param name="policyStore">The policyStore.</param>
+        /// <param name="config">The config.</param>
+        /// <param name="logger">The logger.</param>
+        public LHIPRateLimitMiddleware(
+            RequestDelegate next,
+            IProcessingStrategy processingStrategy,
+            IOptions<IpRateLimitOptions> options,
+            IIpPolicyStore policyStore,
+            IRateLimitConfiguration config,
+            ILogger<IpRateLimitMiddleware> logger)
+            : base(
+                  next,
+                  processingStrategy,
+                  options,
+                  policyStore,
+                  config,
+                  logger)
+        {
+        }
+
+        /// <summary>
+        /// The ReturnQuotaExceededResponse method.
+        /// </summary>
+        /// <param name="httpContext">The httpContext.</param>
+        /// <param name="rule">The rule.</param>
+        /// <param name="retryAfter">The retryAfter.</param>
+        /// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
+        public override Task ReturnQuotaExceededResponse(
+            HttpContext httpContext,
+            RateLimitRule rule,
+            string retryAfter)
+        {
+            httpContext.Response.Headers["Location"] = "/TooManyRequests";
+            httpContext.Response.StatusCode = 302;
+            return httpContext.Response.WriteAsync(string.Empty);
+        }
+    }
+}

LearningHub.Nhs.WebUI/Program.cs

@@ -84,7 +84,7 @@
     app.UseAuthorization();
 
     app.UseMiddleware<NLogMiddleware>();
-
+    app.UseMiddleware<LHIPRateLimitMiddleware>();
     app.UseStaticFiles();
 
     app.Map(TimezoneInfoMiddleware.TimezoneInfoUrl, b => b.UseMiddleware<TimezoneInfoMiddleware>());