Improvements/code (#584)

* Refactor cookie options creation with ICookieOptionsFactory
* Refactor nested if statements for readability
* Add environment-specific CORS policies
* Remove SonarQube workflow and add SonarCloud properties configuration
* Revert (#579) - Update ContactController

Author: KrzysztofPajak

.github/workflows/sonarcube.yml

@@ -0,0 +1,4 @@
+sonar.coverage.exclusions="**" `
+sonar.cpd.exclusions="**"
+sonar.language=cs
+sonar.exclusions=**/*.js,**/*.css,**/*.html,**/*.ts

.sonarcloud.properties

@@ -31,12 +31,14 @@ public CookieAuthenticationService(
         ICustomerService customerService,
         IGroupService groupService,
         IHttpContextAccessor httpContextAccessor,
+        ICookieOptionsFactory cookieOptionsFactory,
         SecurityConfig securityConfig)
     {
         _customerSettings = customerSettings;
         _customerService = customerService;
         _groupService = groupService;
         _httpContextAccessor = httpContextAccessor;
+        _cookieOptionsFactory = cookieOptionsFactory;
         _securityConfig = securityConfig;
     }
 
@@ -54,6 +56,8 @@ public CookieAuthenticationService(
     private readonly ICustomerService _customerService;
     private readonly IGroupService _groupService;
     private readonly IHttpContextAccessor _httpContextAccessor;
+    private readonly ICookieOptionsFactory _cookieOptionsFactory;
+
     private readonly SecurityConfig _securityConfig;
 
     #endregion
@@ -224,10 +228,7 @@ public virtual Task SetCustomerGuid(Guid customerGuid)
             return Task.CompletedTask;
 
         //set new cookie value
-        var options = new CookieOptions {
-            HttpOnly = true,
-            Expires = cookieExpiresDate
-        };
+        var options = _cookieOptionsFactory.Create(cookieExpiresDate);
         _httpContextAccessor.HttpContext.Response.Cookies.Append(CustomerCookieName, customerGuid.ToString(), options);
 
         return Task.CompletedTask;

src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs

@@ -0,0 +1,32 @@
+using Grand.Business.Core.Interfaces.Authentication;
+using Grand.Infrastructure.Configuration;
+using Microsoft.AspNetCore.Http;
+
+namespace Grand.Business.Authentication.Services;
+
+/// <summary>
+/// Factory for creating secure cookie options
+/// </summary>
+public class CookieOptionsFactory(SecurityConfig securityConfig) : ICookieOptionsFactory
+{
+    public string CookiePrefix => securityConfig.CookiePrefix;
+
+    /// <summary>
+    /// Creates cookie options with proper security settings
+    /// </summary>
+    /// <param name="expiresDate">Optional explicit expiration date</param>
+    /// <returns>Configured cookie options</returns>
+    public CookieOptions Create(DateTime? expiresDate = null)
+    {
+        var cookieExpiresDate = expiresDate ?? DateTime.UtcNow.AddHours(securityConfig.CookieAuthExpires);
+
+        var options = new CookieOptions {
+            HttpOnly = true,
+            Expires = cookieExpiresDate,
+            Secure = securityConfig.CookieSecurePolicyAlways,
+            SameSite = securityConfig.CookieSameSite,
+        };
+
+        return options;
+    }
+}

src/Business/Grand.Business.Authentication/Services/CookieOptionsFactory.cs

@@ -13,6 +13,7 @@ public class StartupApplication : IStartupApplication
     public void ConfigureServices(IServiceCollection services, IConfiguration configuration)
     {
         services.AddScoped<IGrandAuthenticationService, CookieAuthenticationService>();
+        services.AddScoped<ICookieOptionsFactory, CookieOptionsFactory>();
         services.AddScoped<IApiAuthenticationService, ApiAuthenticationService>();
         services.AddScoped<IJwtBearerAuthenticationService, JwtBearerAuthenticationService>();
         services.AddScoped<ITwoFactorAuthenticationService, TwoFactorAuthenticationService>();

src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs

@@ -0,0 +1,21 @@
+using Microsoft.AspNetCore.Http;
+
+namespace Grand.Business.Core.Interfaces.Authentication;
+
+// <summary>
+/// Interface for factory creating secure cookie options
+/// </summary>
+public interface ICookieOptionsFactory
+{
+    /// <summary>
+    /// Creates cookie options with proper security settings
+    /// </summary>
+    /// <param name="expiresDate">Optional explicit expiration date</param>
+    /// <returns>Configured cookie options</returns>
+    CookieOptions Create(DateTime? expiresDate = null);
+
+    /// <summary>
+    /// Gets the cookie prefix
+    /// </summary>
+    string CookiePrefix { get; }
+}

src/Business/Grand.Business.Core/Interfaces/Authentication/ICookieOptionsFactory.cs

@@ -3,6 +3,7 @@
 public static class Configurations
 {
     public const string RestRoutePrefix = "api";
-    public const string CorsPolicyName = "CorsPolicy";
+    public const string DevelopmentCorsPolicyName = "DevelopmentCorsPolicy";
+    public const string ProductionCorsPolicyName = "ProductionCorsPolicy";
     public const int MaxLimit = 100;
 }

src/Modules/Grand.Module.Api/Constants/Configurations.cs

@@ -4,23 +4,31 @@
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
 
 namespace Grand.Module.Api.Infrastructure;
 
 public class CorsStartup : IStartupApplication
 {
     public void Configure(WebApplication application, IWebHostEnvironment webHostEnvironment)
     {
-        application.UseCors(Configurations.CorsPolicyName);
+        if(webHostEnvironment.IsDevelopment())
+            application.UseCors(Configurations.DevelopmentCorsPolicyName);
+        else
+            application.UseCors(Configurations.ProductionCorsPolicyName);
     }
 
     public void ConfigureServices(IServiceCollection services,
         IConfiguration configuration)
     {
+        var allowedOrigins = configuration.GetSection("AllowedHostOrigins").Get<string[]>() ?? Array.Empty<string>();
+
         services.AddCors(options =>
         {
-            options.AddPolicy(Configurations.CorsPolicyName,
+            options.AddPolicy(Configurations.DevelopmentCorsPolicyName,
                 builder => builder.AllowAnyOrigin().AllowAnyMethod().AllowAnyHeader());
+            options.AddPolicy(Configurations.ProductionCorsPolicyName,
+                builder => builder.WithOrigins(allowedOrigins).AllowAnyMethod().AllowAnyHeader());
         });
     }
 

src/Modules/Grand.Module.Api/Infrastructure/CorsStartup.cs

@@ -1,4 +1,5 @@
 using Grand.Business.Authentication.Services;
+using Grand.Business.Core.Interfaces.Authentication;
 using Grand.Business.Core.Interfaces.Common.Directory;
 using Grand.Business.Core.Interfaces.Customers;
 using Grand.Business.Core.Utilities.Authentication;
@@ -24,6 +25,7 @@ public class CookieAuthenticationServiceTests
     private Mock<IHttpContextAccessor> _httpAccessorMock;
     private DefaultHttpContext _httpContext;
     private Mock<IServiceProvider> serviceProviderMock;
+    private CookieOptionsFactory _cookieOptionsFactory;
 
     [TestInitialize]
     public void Init()
@@ -37,8 +39,9 @@ public void Init()
             CookieClaimsIssuer = "grandnode",
             CookiePrefix = ".Grand."
         };
+        _cookieOptionsFactory = new CookieOptionsFactory(_config);
         _cookieAuthService = new CookieAuthenticationService(_customerSettings, _customerServiceMock.Object,
-            _groupServiceMock.Object, _httpAccessorMock.Object, _config);
+            _groupServiceMock.Object, _httpAccessorMock.Object, _cookieOptionsFactory, _config);
         //For mock HttpContext extension methods like SignOutAsync ,SignInAsync etc..
         _authServiceMock = new Mock<IAuthenticationService>();
         serviceProviderMock = new Mock<IServiceProvider>();

src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs

@@ -0,0 +1,98 @@
+using Grand.Business.Authentication.Services;
+using Grand.Infrastructure.Configuration;
+using Microsoft.AspNetCore.Http;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Grand.Business.Authentication.Tests.Services;
+
+[TestClass]
+public class CookieOptionsFactoryTests
+{
+    private SecurityConfig _securityConfig;
+    private CookieOptionsFactory _cookieOptionsFactory;
+
+    [TestInitialize]
+    public void Setup()
+    {
+        _securityConfig = new SecurityConfig {
+            CookieAuthExpires = 24,
+            CookieSecurePolicyAlways = true,
+        };
+        _cookieOptionsFactory = new CookieOptionsFactory(_securityConfig);
+    }
+
+    [TestMethod]
+    public void Create_WhenCalledWithoutExpiryDate_SetsDefaultExpiryDate()
+    {
+        // Arrange
+        var expectedExpiryDate = DateTime.UtcNow.AddHours(_securityConfig.CookieAuthExpires);
+
+        // Act
+        var options = _cookieOptionsFactory.Create();
+
+        // Assert
+        Assert.IsTrue(options.HttpOnly);
+        Assert.IsTrue(options.Secure);
+
+        // Verify expiration is close to expected (allowing for slight processing time differences)
+        var difference = (options.Expires.Value - expectedExpiryDate).TotalMinutes;
+        Assert.IsTrue(Math.Abs(difference) < 1, "Expiry time should be within 1 minute of expected value");
+    }
+
+    [TestMethod]
+    public void Create_WhenCalledWithExpiryDate_UsesProvidedExpiryDate()
+    {
+        // Arrange
+        var expiryDate = DateTime.UtcNow.AddDays(7);
+
+        // Act
+        var options = _cookieOptionsFactory.Create(expiryDate);
+
+        // Assert
+        Assert.IsTrue(options.HttpOnly);
+        Assert.IsTrue(options.Secure);
+        Assert.AreEqual(expiryDate, options.Expires);
+    }
+
+    [TestMethod]
+    public void Create_WhenSecurePolicyIsNone_SecureFlagIsFalse()
+    {
+        // Arrange
+        _securityConfig.CookieSecurePolicyAlways = true;
+        var factory = new CookieOptionsFactory(_securityConfig);
+
+        // Act
+        var options = factory.Create();
+
+        // Assert
+        Assert.IsTrue(options.Secure);
+    }
+
+    [TestMethod]
+    public void Create_WhenSecurePolicyIsSameAsRequest_UsesConfiguredPolicy()
+    {
+        // Arrange
+        _securityConfig.CookieSecurePolicyAlways = false;
+        var factory = new CookieOptionsFactory(_securityConfig);
+
+        // Act
+        var options = factory.Create();
+
+        // Assert
+        Assert.IsFalse(options.Secure);
+    }
+
+    [TestMethod]
+    public void Create_WhenSameSiteModeIsStrict_UseStrictMode()
+    {
+        // Arrange
+        _securityConfig.CookieSameSite = SameSiteMode.Strict;
+        var factory = new CookieOptionsFactory(_securityConfig);
+
+        // Act
+        var options = factory.Create();
+
+        // Assert
+        Assert.AreEqual(SameSiteMode.Strict, options.SameSite);
+    }
+}