From 7a6fb21774130288e71dca5916d97ce31b61e694 Mon Sep 17 00:00:00 2001 From: support Date: Sun, 18 Oct 2026 13:40:08 +0200 Subject: [PATCH 1/2] Split ExternalAuthenticationService into a Business result and a Web sign-in ExternalAuthenticationService.Authenticate returns ExternalAuthResult instead of IActionResult and no longer signs in. The new ExternalSignInService in Web.Common signs the customer in and maps the result to the same redirects as before. IGrandAuthenticationService has no Business user left and moves to Web.Common. --- .../Services/ExternalAuthenticationService.cs | 85 +++++--------- .../IExternalAuthenticationService.cs | 9 +- .../Authentication/ExternalAuthResult.cs | 59 ++++++++++ .../FacebookAuthenticationController.cs | 6 +- .../GoogleAuthenticationController.cs | 6 +- .../Controllers/OidcLoginController.cs | 1 + .../ExternalAuthenticationServiceTests.cs | 94 ++++++++++++++-- .../OpenIdConnect/OidcLoginControllerTests.cs | 1 + .../LoginControllerExternalSignInTests.cs | 1 + .../LoginControllerProviderOnlyTests.cs | 1 + .../RequestCustomerResolversTests.cs | 1 - .../ExternalSignInServiceTests.cs | 104 ++++++++++++++++++ ...GrandAuthenticationServiceDefaultsTests.cs | 4 +- .../ProviderSignOutTests.cs | 1 + .../Security/Oidc/OidcWorkContextTests.cs | 1 - .../WorkContextSetterRequestResolverTests.cs | 1 - .../WorkContextSetterTests.cs | 2 +- .../UpdateCustomerInfoCommandHandlerTests.cs | 2 +- .../AccountControllerExternalRegisterTests.cs | 1 + .../AccountControllerProviderOnlyTests.cs | 1 + ...ccountControllerTwoFactorReturnUrlTests.cs | 1 + .../Controllers/LoginController.cs | 4 +- .../Controllers/BaseLoginController.cs | 1 + .../CookieCustomerResolver.cs | 2 +- .../GuestCookieCustomerResolver.cs | 2 +- .../Authentication/ExternalSignInService.cs | 53 +++++++++ .../Authentication/IExternalSignInService.cs | 25 +++++ .../IGrandAuthenticationService.cs | 2 +- .../ExternalRegistration/ProviderSignOut.cs | 1 + .../Startup/StartupApplication.cs | 2 +- src/Web/Grand.Web.Common/WorkContextSetter.cs | 4 +- .../Controllers/LoginController.cs | 4 +- .../Controllers/LoginController.cs | 4 +- .../UpdateCustomerInfoCommandHandler.cs | 4 +- .../Controllers/AccountController.cs | 1 + 35 files changed, 395 insertions(+), 96 deletions(-) create mode 100644 src/Business/Grand.Business.Core/Utilities/Authentication/ExternalAuthResult.cs create mode 100644 src/Tests/Grand.Web.Common.Tests/Security/Authentication/ExternalSignInServiceTests.cs rename src/Tests/{Grand.Business.Authentication.Tests/Services => Grand.Web.Common.Tests/Security/Authentication}/GrandAuthenticationServiceDefaultsTests.cs (90%) create mode 100644 src/Web/Grand.Web.Common/Security/Authentication/ExternalSignInService.cs create mode 100644 src/Web/Grand.Web.Common/Security/Authentication/IExternalSignInService.cs rename src/{Business/Grand.Business.Core/Interfaces => Web/Grand.Web.Common/Security}/Authentication/IGrandAuthenticationService.cs (96%) diff --git a/src/Business/Grand.Business.Authentication/Services/ExternalAuthenticationService.cs b/src/Business/Grand.Business.Authentication/Services/ExternalAuthenticationService.cs index 2cb6fd3b4..63d65086c 100644 --- a/src/Business/Grand.Business.Authentication/Services/ExternalAuthenticationService.cs +++ b/src/Business/Grand.Business.Authentication/Services/ExternalAuthenticationService.cs @@ -13,7 +13,6 @@ using Grand.Infrastructure.Extensions; using Grand.SharedKernel.Extensions; using Grand.Mediator; -using Microsoft.AspNetCore.Mvc; namespace Grand.Business.Authentication.Services; @@ -25,7 +24,6 @@ public class ExternalAuthenticationService : IExternalAuthenticationService #region Ctor public ExternalAuthenticationService( - IGrandAuthenticationService authenticationService, ICustomerManagerService customerManagerService, ICustomerService customerService, IGroupService groupService, @@ -37,7 +35,6 @@ public ExternalAuthenticationService( { _customerSettings = customerSettings; _externalAuthenticationSettings = externalAuthenticationSettings; - _authenticationService = authenticationService; _customerManagerService = customerManagerService; _customerService = customerService; _groupService = groupService; @@ -50,7 +47,6 @@ public ExternalAuthenticationService( #region Fields - private readonly IGrandAuthenticationService _authenticationService; private readonly ICustomerManagerService _customerManagerService; private readonly ICustomerService _customerService; private readonly IGroupService _groupService; @@ -72,22 +68,19 @@ public ExternalAuthenticationService( /// Customer of the request (a guest when nobody is logged in) /// Store identifier /// Working language of the request - /// URL to which the user will return after authentication /// Result of an authentication - protected virtual async Task AuthenticateExistingUser(Customer associatedUser, - Customer currentLoggedInUser, Customer customer, string storeId, Language language, string returnUrl) + protected virtual async Task AuthenticateExistingUser(Customer associatedUser, + Customer currentLoggedInUser, Customer customer, string storeId, Language language) { //log in guest user if (currentLoggedInUser == null) - return await LoginUser(associatedUser, customer, storeId, language, returnUrl); + return await LoginUser(associatedUser, customer, storeId, language); //account is already assigned to another user if (currentLoggedInUser.Id != associatedUser.Id) - return Error(["Account is already assigned"]); + return ExternalAuthResult.AlreadyAssigned(); - if (string.IsNullOrEmpty(returnUrl)) - return new RedirectToRouteResult("HomePage", new { area = "" }); - return new RedirectResult(returnUrl); + return ExternalAuthResult.Associated(currentLoggedInUser); } /// @@ -98,26 +91,23 @@ protected virtual async Task AuthenticateExistingUser(Customer as /// Customer of the request (a guest when nobody is logged in) /// Store identifier /// Working language of the request - /// URL to which the user will return after authentication /// Result of an authentication - protected virtual async Task AuthenticateNewUser(Customer currentLoggedInUser, - ExternalAuthParam parameters, Customer customer, string storeId, Language language, string returnUrl) + protected virtual async Task AuthenticateNewUser(Customer currentLoggedInUser, + ExternalAuthParam parameters, Customer customer, string storeId, Language language) { //associate external account with logged-in user if (currentLoggedInUser != null) { await AssociateCustomer(currentLoggedInUser, parameters); - if (string.IsNullOrEmpty(returnUrl)) - return new RedirectToRouteResult("HomePage", new { area = "" }); - return new RedirectResult(returnUrl); + return ExternalAuthResult.Associated(currentLoggedInUser); } //or try to register new user if (_customerSettings.UserRegistrationType != UserRegistrationType.Disabled) - return await RegisterNewUser(parameters, customer, storeId, language, returnUrl); + return await RegisterNewUser(parameters, customer, storeId, language); //registration is disabled - return Error(["Registration is disabled"]); + return ExternalAuthResult.RegistrationDisabled(); } /// @@ -127,10 +117,9 @@ protected virtual async Task AuthenticateNewUser(Customer current /// Guest customer of the request; registration turns it into the new user /// Store identifier /// Working language of the request - /// URL to which the user will return after authentication /// Result of an authentication - protected virtual async Task RegisterNewUser(ExternalAuthParam parameters, Customer customer, - string storeId, Language language, string returnUrl) + protected virtual async Task RegisterNewUser(ExternalAuthParam parameters, Customer customer, + string storeId, Language language) { var approved = _customerSettings.UserRegistrationType is UserRegistrationType.Standard @@ -156,15 +145,12 @@ _customerSettings.UserRegistrationType is UserRegistrationType.Standard //associate external account with registered user await AssociateCustomer(customer, parameters); - //authenticate - if (!approved) - return _customerSettings.UserRegistrationType == UserRegistrationType.AdminApproval - ? new RedirectToRouteResult("RegisterResult", - new { resultId = (int)UserRegistrationType.AdminApproval }) - : Error(["Error on registration"]); - await _authenticationService.SignIn(customer, false); + if (approved) + return ExternalAuthResult.Registered(customer); - return new RedirectToRouteResult("RegisterResult", new { resultId = (int)UserRegistrationType.Standard }); + return _customerSettings.UserRegistrationType == UserRegistrationType.AdminApproval + ? ExternalAuthResult.AwaitingApproval(customer) + : ExternalAuthResult.Error("Error on registration"); } /// @@ -174,31 +160,14 @@ _customerSettings.UserRegistrationType is UserRegistrationType.Standard /// Customer of the request before the login (the guest whose cart is merged) /// Store identifier /// Working language of the request - /// URL to which the user will return after authentication /// Result of an authentication - protected virtual async Task LoginUser(Customer user, Customer previousCustomer, string storeId, - Language language, string returnUrl) + protected virtual async Task LoginUser(Customer user, Customer previousCustomer, + string storeId, Language language) { - //raise event + //raise event; the caller signs the user in afterwards await _mediator.Publish(new CustomerLoggedInEvent(user, previousCustomer, storeId, language)); - //authenticate - await _authenticationService.SignIn(user, false); - - if (string.IsNullOrEmpty(returnUrl)) - return new RedirectToRouteResult("HomePage", new { area = "" }); - - return new RedirectResult(returnUrl); - } - - /// - /// Add errors that occurred during authentication - /// - /// Collection of errors - /// Result of an authentication - protected virtual IActionResult Error(IEnumerable errors) - { - return new RedirectToActionResult("ExternalAuthenticationError", "Common", new { Errors = errors }); + return ExternalAuthResult.LoggedIn(user); } #endregion @@ -276,10 +245,9 @@ public virtual bool AuthenticationProviderIsAvailable(string systemName, Custome /// Customer of the request (a guest when nobody is logged in) /// Store of the request /// Working language of the request - /// URL to which the user will return after authentication /// Result of an authentication - public virtual async Task Authenticate(ExternalAuthParam parameters, Customer customer, Store store, - Language language, string returnUrl = null) + public virtual async Task Authenticate(ExternalAuthParam parameters, Customer customer, + Store store, Language language) { ArgumentNullException.ThrowIfNull(parameters); ArgumentNullException.ThrowIfNull(customer); @@ -287,7 +255,7 @@ public virtual async Task Authenticate(ExternalAuthParam paramete ArgumentNullException.ThrowIfNull(language); if (!AuthenticationProviderIsAvailable(parameters.ProviderSystemName, customer, store)) - return Error(["External authentication method cannot be loaded"]); + return ExternalAuthResult.Error("External authentication method cannot be loaded"); //get current logged-in user var currentLoggedInUser = await _groupService.IsRegistered(customer) ? customer : null; @@ -295,11 +263,10 @@ public virtual async Task Authenticate(ExternalAuthParam paramete //authenticate associated user if already exists var associatedUser = await GetCustomer(parameters); if (associatedUser != null) - return await AuthenticateExistingUser(associatedUser, currentLoggedInUser, customer, store.Id, language, - returnUrl); + return await AuthenticateExistingUser(associatedUser, currentLoggedInUser, customer, store.Id, language); //or associate and authenticate new user - return await AuthenticateNewUser(currentLoggedInUser, parameters, customer, store.Id, language, returnUrl); + return await AuthenticateNewUser(currentLoggedInUser, parameters, customer, store.Id, language); } #endregion diff --git a/src/Business/Grand.Business.Core/Interfaces/Authentication/IExternalAuthenticationService.cs b/src/Business/Grand.Business.Core/Interfaces/Authentication/IExternalAuthenticationService.cs index 4d76f2e63..64e989320 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Authentication/IExternalAuthenticationService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Authentication/IExternalAuthenticationService.cs @@ -2,7 +2,6 @@ using Grand.Domain.Customers; using Grand.Domain.Localization; using Grand.Domain.Stores; -using Microsoft.AspNetCore.Mvc; namespace Grand.Business.Core.Interfaces.Authentication; @@ -14,16 +13,16 @@ public interface IExternalAuthenticationService #region Authentication /// - /// Use passed parameters to authenticate customer + /// Use passed parameters to find, associate or register the customer; the caller signs in the customer of a + /// LoggedIn or Registered result /// /// External authentication parameters /// Customer of the request (a guest when nobody is logged in) /// Store of the request /// Working language of the request - /// Defines the URL of user return, when successfully authenticated /// Result of an authentication - Task Authenticate(ExternalAuthParam parameters, Customer customer, Store store, Language language, - string returnUrl = null); + Task Authenticate(ExternalAuthParam parameters, Customer customer, Store store, + Language language); #endregion diff --git a/src/Business/Grand.Business.Core/Utilities/Authentication/ExternalAuthResult.cs b/src/Business/Grand.Business.Core/Utilities/Authentication/ExternalAuthResult.cs new file mode 100644 index 000000000..403e91add --- /dev/null +++ b/src/Business/Grand.Business.Core/Utilities/Authentication/ExternalAuthResult.cs @@ -0,0 +1,59 @@ +using Grand.Domain.Customers; + +namespace Grand.Business.Core.Utilities.Authentication; + +public enum ExternalAuthStatus +{ + /// An existing customer associated with the external account is to be signed in + LoggedIn, + + /// A new customer was registered, approved and is to be signed in + Registered, + + /// The external account is associated with the customer already signed in; nothing to sign in + Associated, + + /// A new customer was registered and waits for admin approval + AwaitingApproval, + + /// The external account belongs to another customer than the one signed in + AlreadyAssigned, + + RegistrationDisabled, + Error +} + +/// +/// Outcome of an external authentication; signing in and redirecting is left to the caller +/// +public class ExternalAuthResult +{ + private ExternalAuthResult(ExternalAuthStatus status, Customer customer, IList errors) + { + Status = status; + Customer = customer; + Errors = errors; + } + + public ExternalAuthStatus Status { get; } + + /// The customer to sign in (LoggedIn, Registered) or the one the account was associated with + public Customer Customer { get; } + + public IList Errors { get; } + + public static ExternalAuthResult LoggedIn(Customer customer) => new(ExternalAuthStatus.LoggedIn, customer, []); + public static ExternalAuthResult Registered(Customer customer) => new(ExternalAuthStatus.Registered, customer, []); + public static ExternalAuthResult Associated(Customer customer) => new(ExternalAuthStatus.Associated, customer, []); + + public static ExternalAuthResult AwaitingApproval(Customer customer) => + new(ExternalAuthStatus.AwaitingApproval, customer, []); + + public static ExternalAuthResult AlreadyAssigned() => + new(ExternalAuthStatus.AlreadyAssigned, null, ["Account is already assigned"]); + + public static ExternalAuthResult RegistrationDisabled() => + new(ExternalAuthStatus.RegistrationDisabled, null, ["Registration is disabled"]); + + public static ExternalAuthResult Error(string error) => new(ExternalAuthStatus.Error, null, [error]); +} diff --git a/src/Plugins/Authentication.Facebook/Controllers/FacebookAuthenticationController.cs b/src/Plugins/Authentication.Facebook/Controllers/FacebookAuthenticationController.cs index 5dec5505b..1d7dbab80 100644 --- a/src/Plugins/Authentication.Facebook/Controllers/FacebookAuthenticationController.cs +++ b/src/Plugins/Authentication.Facebook/Controllers/FacebookAuthenticationController.cs @@ -4,6 +4,7 @@ using Grand.Infrastructure; using Grand.SharedKernel; using Grand.Web.Common.Controllers; +using Grand.Web.Common.Security.Authentication; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Facebook; using Microsoft.AspNetCore.Mvc; @@ -18,10 +19,12 @@ public class FacebookAuthenticationController : BasePluginController public FacebookAuthenticationController( IExternalAuthenticationService externalAuthenticationService, + IExternalSignInService externalSignInService, IConfiguration configuration, IContextAccessor contextAccessor) { _externalAuthenticationService = externalAuthenticationService; + _externalSignInService = externalSignInService; _configuration = configuration; _contextAccessor = contextAccessor; } @@ -31,6 +34,7 @@ public FacebookAuthenticationController( #region Fields private readonly IExternalAuthenticationService _externalAuthenticationService; + private readonly IExternalSignInService _externalSignInService; private readonly IConfiguration _configuration; private readonly IContextAccessor _contextAccessor; @@ -76,7 +80,7 @@ public async Task FacebookLoginCallback(string returnUrl) }; //authenticate Grand user - return await _externalAuthenticationService.Authenticate(authenticationParameters, + return await _externalSignInService.Authenticate(authenticationParameters, _contextAccessor.WorkContext.CurrentCustomer, _contextAccessor.StoreContext.CurrentStore, _contextAccessor.WorkContext.WorkingLanguage, returnUrl); } diff --git a/src/Plugins/Authentication.Google/Controllers/GoogleAuthenticationController.cs b/src/Plugins/Authentication.Google/Controllers/GoogleAuthenticationController.cs index 686bec888..82b84725e 100644 --- a/src/Plugins/Authentication.Google/Controllers/GoogleAuthenticationController.cs +++ b/src/Plugins/Authentication.Google/Controllers/GoogleAuthenticationController.cs @@ -4,6 +4,7 @@ using Grand.Infrastructure; using Grand.SharedKernel; using Grand.Web.Common.Controllers; +using Grand.Web.Common.Security.Authentication; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Google; using Microsoft.AspNetCore.Mvc; @@ -17,10 +18,12 @@ public class GoogleAuthenticationController : BasePluginController #region Ctor public GoogleAuthenticationController(IExternalAuthenticationService externalAuthenticationService, + IExternalSignInService externalSignInService, IConfiguration configuration, IContextAccessor contextAccessor) { _externalAuthenticationService = externalAuthenticationService; + _externalSignInService = externalSignInService; _configuration = configuration; _contextAccessor = contextAccessor; } @@ -30,6 +33,7 @@ public GoogleAuthenticationController(IExternalAuthenticationService externalAut #region Fields private readonly IExternalAuthenticationService _externalAuthenticationService; + private readonly IExternalSignInService _externalSignInService; private readonly IConfiguration _configuration; private readonly IContextAccessor _contextAccessor; @@ -75,7 +79,7 @@ public async Task GoogleLoginCallback(string returnUrl) }; //authenticate grand user - return await _externalAuthenticationService.Authenticate(authenticationParameters, + return await _externalSignInService.Authenticate(authenticationParameters, _contextAccessor.WorkContext.CurrentCustomer, _contextAccessor.StoreContext.CurrentStore, _contextAccessor.WorkContext.WorkingLanguage, returnUrl); } diff --git a/src/Plugins/Authentication.OpenIdConnect/Controllers/OidcLoginController.cs b/src/Plugins/Authentication.OpenIdConnect/Controllers/OidcLoginController.cs index d37e88bc3..f538c9610 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Controllers/OidcLoginController.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Controllers/OidcLoginController.cs @@ -8,6 +8,7 @@ using Grand.Infrastructure.Configuration; using Grand.Mediator; using Grand.Web.Common.Controllers; +using Grand.Web.Common.Security.Authentication; using Grand.Web.Common.Security.ExternalRegistration; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/ExternalAuthenticationServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/ExternalAuthenticationServiceTests.cs index 3a9e78621..ae64ebf36 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/ExternalAuthenticationServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/ExternalAuthenticationServiceTests.cs @@ -19,7 +19,6 @@ namespace Grand.Business.Authentication.Tests.Services; [TestClass] public class ExternalAuthenticationServiceTests { - private Mock _authenticationServiceMock; private Mock _customerManagerServiceMock; private Mock _customerServiceMock; private IEnumerable _externalAuthenticationProviders; @@ -34,24 +33,27 @@ public void Init() { _customerServiceMock = new Mock(); _groupServiceMock = new Mock(); - _authenticationServiceMock = new Mock(); _customerManagerServiceMock = new Mock(); _mediatorMock = new Mock(); _externalAuthenticationRecordRepository = new MongoDBRepositoryTest(); _externalAuthenticationProviders = new List { new ExternalAuthenticationProviderTest() }; + _externalAuthenticationService = CreateService(UserRegistrationType.Standard); + } + + private ExternalAuthenticationService CreateService(UserRegistrationType userRegistrationType) + { var externalAuthenticationSetting = new ExternalAuthenticationSettings { ActiveAuthenticationMethodSystemNames = [ "ExternalAuthenticationProviderTest" ] }; - _externalAuthenticationService = new ExternalAuthenticationService(_authenticationServiceMock.Object, - _customerManagerServiceMock.Object, _customerServiceMock.Object, + return new ExternalAuthenticationService(_customerManagerServiceMock.Object, _customerServiceMock.Object, _groupServiceMock.Object, _mediatorMock.Object, _externalAuthenticationRecordRepository, _externalAuthenticationProviders, - new CustomerSettings { UserRegistrationType = UserRegistrationType.Standard }, externalAuthenticationSetting); + new CustomerSettings { UserRegistrationType = userRegistrationType }, externalAuthenticationSetting); } @@ -133,12 +135,14 @@ public async Task Authenticate_AssociatedUserFromGuest_PublishesLoggedInEventWit var parameters = new ExternalAuthParam { ProviderSystemName = "ExternalAuthenticationProviderTest", Identifier = "1" }; await _externalAuthenticationService.AssociateCustomer(user, parameters); - await _externalAuthenticationService.Authenticate(parameters, guest, new Store { Id = "store1" }, language); + var result = + await _externalAuthenticationService.Authenticate(parameters, guest, new Store { Id = "store1" }, language); _mediatorMock.Verify(m => m.Publish(It.Is(e => e.Customer == user && e.PreviousCustomer == guest && e.StoreId == "store1" && e.Language == language), default), Times.Once); - _authenticationServiceMock.Verify(a => a.SignIn(user, false), Times.Once); + Assert.AreEqual(ExternalAuthStatus.LoggedIn, result.Status); + Assert.AreSame(user, result.Customer); } [TestMethod] @@ -150,15 +154,87 @@ public async Task Authenticate_UnknownIdentity_RegistersPassedCustomerInPassedSt ProviderSystemName = "ExternalAuthenticationProviderTest", Identifier = "new", Email = "new@test.com" }; - await _externalAuthenticationService.Authenticate(parameters, guest, new Store { Id = "store1" }, language); + var result = + await _externalAuthenticationService.Authenticate(parameters, guest, new Store { Id = "store1" }, language); _customerManagerServiceMock.Verify(m => m.RegisterCustomer(It.Is(r => r.Customer == guest && r.StoreId == "store1" && r.Email == "new@test.com")), Times.Once); _mediatorMock.Verify(m => m.Publish(It.Is(e => e.Customer == guest && e.Language == language), default), Times.Once); - _authenticationServiceMock.Verify(a => a.SignIn(guest, false), Times.Once); Assert.AreEqual("guest", _externalAuthenticationRecordRepository.Table.Single().CustomerId); + Assert.AreEqual(ExternalAuthStatus.Registered, result.Status); + Assert.AreSame(guest, result.Customer); + } + + [TestMethod] + public async Task Authenticate_UnknownIdentityWithAdminApproval_IsAwaitingApproval() + { + var guest = new Customer { Id = "guest" }; + var parameters = new ExternalAuthParam { + ProviderSystemName = "ExternalAuthenticationProviderTest", Identifier = "new", Email = "new@test.com" + }; + + var result = await CreateService(UserRegistrationType.AdminApproval) + .Authenticate(parameters, guest, new Store { Id = "store1" }, new Language()); + + _customerManagerServiceMock.Verify(m => m.RegisterCustomer(It.Is(r => + r.Customer == guest && !r.IsApproved)), Times.Once); + Assert.AreEqual(ExternalAuthStatus.AwaitingApproval, result.Status); + } + + [TestMethod] + public async Task Authenticate_UnknownIdentityWithRegistrationDisabled_RegistersNobody() + { + var parameters = new ExternalAuthParam { ProviderSystemName = "ExternalAuthenticationProviderTest", Identifier = "new" }; + + var result = await CreateService(UserRegistrationType.Disabled) + .Authenticate(parameters, new Customer { Id = "guest" }, new Store(), new Language()); + + _customerManagerServiceMock.Verify(m => m.RegisterCustomer(It.IsAny()), Times.Never); + Assert.AreEqual(ExternalAuthStatus.RegistrationDisabled, result.Status); + Assert.IsFalse(_externalAuthenticationRecordRepository.Table.Any()); + } + + [TestMethod] + public async Task Authenticate_UnknownIdentityWhileLoggedIn_AssociatesItWithLoggedInCustomer() + { + var customer = new Customer { Id = "registered" }; + _groupServiceMock.Setup(g => g.IsRegistered(customer)).ReturnsAsync(true); + var parameters = new ExternalAuthParam { ProviderSystemName = "ExternalAuthenticationProviderTest", Identifier = "new" }; + + var result = await _externalAuthenticationService.Authenticate(parameters, customer, new Store(), new Language()); + + Assert.AreEqual(ExternalAuthStatus.Associated, result.Status); + Assert.AreEqual("registered", _externalAuthenticationRecordRepository.Table.Single().CustomerId); + _mediatorMock.Verify(m => m.Publish(It.IsAny(), default), Times.Never); + } + + [TestMethod] + public async Task Authenticate_IdentityOfAnotherCustomerWhileLoggedIn_IsAlreadyAssigned() + { + var owner = new Customer { Id = "owner" }; + var customer = new Customer { Id = "registered" }; + _groupServiceMock.Setup(g => g.IsRegistered(customer)).ReturnsAsync(true); + _customerServiceMock.Setup(c => c.GetCustomerById("owner")).ReturnsAsync(owner); + var parameters = new ExternalAuthParam { ProviderSystemName = "ExternalAuthenticationProviderTest", Identifier = "1" }; + await _externalAuthenticationService.AssociateCustomer(owner, parameters); + + var result = await _externalAuthenticationService.Authenticate(parameters, customer, new Store(), new Language()); + + Assert.AreEqual(ExternalAuthStatus.AlreadyAssigned, result.Status); + CollectionAssert.AreEqual(new[] { "Account is already assigned" }, result.Errors.ToArray()); + } + + [TestMethod] + public async Task Authenticate_UnavailableProvider_IsError() + { + var parameters = new ExternalAuthParam { ProviderSystemName = "Unknown", Identifier = "1" }; + + var result = await _externalAuthenticationService.Authenticate(parameters, new Customer(), new Store(), new Language()); + + Assert.AreEqual(ExternalAuthStatus.Error, result.Status); + _customerManagerServiceMock.Verify(m => m.RegisterCustomer(It.IsAny()), Times.Never); } [TestMethod] diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginControllerTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginControllerTests.cs index 214a2b85f..a5964643f 100644 --- a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginControllerTests.cs +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginControllerTests.cs @@ -14,6 +14,7 @@ using Grand.Domain.Stores; using Grand.Infrastructure; using Grand.Mediator; +using Grand.Web.Common.Security.Authentication; using Grand.Web.Common.Security.ExternalRegistration; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; diff --git a/src/Tests/Grand.Web.Admin.Tests/Controllers/LoginControllerExternalSignInTests.cs b/src/Tests/Grand.Web.Admin.Tests/Controllers/LoginControllerExternalSignInTests.cs index b214fbc41..aff51054d 100644 --- a/src/Tests/Grand.Web.Admin.Tests/Controllers/LoginControllerExternalSignInTests.cs +++ b/src/Tests/Grand.Web.Admin.Tests/Controllers/LoginControllerExternalSignInTests.cs @@ -9,6 +9,7 @@ using Grand.Mediator; using Grand.Web.Admin.Controllers; using Grand.Web.AdminShared.Models.Common; +using Grand.Web.Common.Security.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; diff --git a/src/Tests/Grand.Web.Admin.Tests/Controllers/LoginControllerProviderOnlyTests.cs b/src/Tests/Grand.Web.Admin.Tests/Controllers/LoginControllerProviderOnlyTests.cs index 103bacd20..f46550f2c 100644 --- a/src/Tests/Grand.Web.Admin.Tests/Controllers/LoginControllerProviderOnlyTests.cs +++ b/src/Tests/Grand.Web.Admin.Tests/Controllers/LoginControllerProviderOnlyTests.cs @@ -9,6 +9,7 @@ using Grand.Mediator; using Grand.Web.Admin.Controllers; using Grand.Web.AdminShared.Models.Common; +using Grand.Web.Common.Security.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs index dce7834e5..7c86e47a0 100644 --- a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs @@ -1,4 +1,3 @@ -using Grand.Business.Core.Interfaces.Authentication; using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Customers; using Grand.Domain.Customers; diff --git a/src/Tests/Grand.Web.Common.Tests/Security/Authentication/ExternalSignInServiceTests.cs b/src/Tests/Grand.Web.Common.Tests/Security/Authentication/ExternalSignInServiceTests.cs new file mode 100644 index 000000000..de31f8b41 --- /dev/null +++ b/src/Tests/Grand.Web.Common.Tests/Security/Authentication/ExternalSignInServiceTests.cs @@ -0,0 +1,104 @@ +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Business.Core.Utilities.Authentication; +using Grand.Domain.Customers; +using Grand.Domain.Localization; +using Grand.Domain.Stores; +using Grand.Web.Common.Security.Authentication; +using Microsoft.AspNetCore.Mvc; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Web.Common.Tests.Security.Authentication; + +[TestClass] +public class ExternalSignInServiceTests +{ + private readonly Customer _customer = new() { Id = "customer" }; + private Mock _authenticationServiceMock; + private Mock _externalAuthenticationServiceMock; + private ExternalSignInService _service; + + [TestInitialize] + public void Init() + { + _externalAuthenticationServiceMock = new Mock(); + _authenticationServiceMock = new Mock(); + _service = new ExternalSignInService(_externalAuthenticationServiceMock.Object, + _authenticationServiceMock.Object); + } + + private Task Authenticate(ExternalAuthResult result, string returnUrl = null) + { + _externalAuthenticationServiceMock + .Setup(s => s.Authenticate(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny())) + .ReturnsAsync(result); + return _service.Authenticate(new ExternalAuthParam(), new Customer(), new Store(), new Language(), returnUrl); + } + + [TestMethod] + public async Task LoggedIn_SignsInAndRedirectsToReturnUrl() + { + var result = await Authenticate(ExternalAuthResult.LoggedIn(_customer), "/cart"); + + _authenticationServiceMock.Verify(a => a.SignIn(_customer, false), Times.Once); + Assert.AreEqual("/cart", ((RedirectResult)result).Url); + } + + [TestMethod] + public async Task LoggedIn_WithoutReturnUrl_RedirectsToHomePage() + { + var result = await Authenticate(ExternalAuthResult.LoggedIn(_customer)); + + Assert.AreEqual("HomePage", ((RedirectToRouteResult)result).RouteName); + } + + [TestMethod] + public async Task Registered_SignsInAndShowsStandardRegisterResult() + { + var result = await Authenticate(ExternalAuthResult.Registered(_customer), "/cart"); + + _authenticationServiceMock.Verify(a => a.SignIn(_customer, false), Times.Once); + var redirect = (RedirectToRouteResult)result; + Assert.AreEqual("RegisterResult", redirect.RouteName); + Assert.AreEqual((int)UserRegistrationType.Standard, redirect.RouteValues!["resultId"]); + } + + [TestMethod] + public async Task Associated_RedirectsToReturnUrlWithoutSignIn() + { + var result = await Authenticate(ExternalAuthResult.Associated(_customer), "/account"); + + _authenticationServiceMock.Verify(a => a.SignIn(It.IsAny(), It.IsAny()), Times.Never); + Assert.AreEqual("/account", ((RedirectResult)result).Url); + } + + [TestMethod] + public async Task AwaitingApproval_ShowsAdminApprovalRegisterResultWithoutSignIn() + { + var result = await Authenticate(ExternalAuthResult.AwaitingApproval(_customer)); + + _authenticationServiceMock.Verify(a => a.SignIn(It.IsAny(), It.IsAny()), Times.Never); + var redirect = (RedirectToRouteResult)result; + Assert.AreEqual("RegisterResult", redirect.RouteName); + Assert.AreEqual((int)UserRegistrationType.AdminApproval, redirect.RouteValues!["resultId"]); + } + + [TestMethod] + public async Task Refusals_RedirectToExternalAuthenticationErrorWithoutSignIn() + { + foreach (var refusal in new[] { + ExternalAuthResult.AlreadyAssigned(), ExternalAuthResult.RegistrationDisabled(), + ExternalAuthResult.Error("failed") + }) + { + var result = (RedirectToActionResult)await Authenticate(refusal); + + Assert.AreEqual("ExternalAuthenticationError", result.ActionName); + Assert.AreEqual("Common", result.ControllerName); + Assert.AreSame(refusal.Errors, result.RouteValues!["Errors"]); + } + + _authenticationServiceMock.Verify(a => a.SignIn(It.IsAny(), It.IsAny()), Times.Never); + } +} diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/GrandAuthenticationServiceDefaultsTests.cs b/src/Tests/Grand.Web.Common.Tests/Security/Authentication/GrandAuthenticationServiceDefaultsTests.cs similarity index 90% rename from src/Tests/Grand.Business.Authentication.Tests/Services/GrandAuthenticationServiceDefaultsTests.cs rename to src/Tests/Grand.Web.Common.Tests/Security/Authentication/GrandAuthenticationServiceDefaultsTests.cs index 119b1616a..315af4d34 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/GrandAuthenticationServiceDefaultsTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Security/Authentication/GrandAuthenticationServiceDefaultsTests.cs @@ -1,8 +1,8 @@ -using Grand.Business.Core.Interfaces.Authentication; using Grand.Domain.Customers; +using Grand.Web.Common.Security.Authentication; using Microsoft.VisualStudio.TestTools.UnitTesting; -namespace Grand.Business.Authentication.Tests.Services; +namespace Grand.Web.Common.Tests.Security.Authentication; [TestClass] public class GrandAuthenticationServiceDefaultsTests diff --git a/src/Tests/Grand.Web.Common.Tests/Security/ExternalRegistration/ProviderSignOutTests.cs b/src/Tests/Grand.Web.Common.Tests/Security/ExternalRegistration/ProviderSignOutTests.cs index 79a1081c2..fa897659e 100644 --- a/src/Tests/Grand.Web.Common.Tests/Security/ExternalRegistration/ProviderSignOutTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Security/ExternalRegistration/ProviderSignOutTests.cs @@ -2,6 +2,7 @@ using Grand.Domain.Stores; using Grand.Infrastructure; using Grand.Infrastructure.Configuration; +using Grand.Web.Common.Security.Authentication; using Grand.Web.Common.Security.ExternalRegistration; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging.Abstractions; diff --git a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs index 7d3bc8fe5..1afc271b9 100644 --- a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs @@ -1,4 +1,3 @@ -using Grand.Business.Core.Interfaces.Authentication; using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Interfaces.Common.Security; diff --git a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs index ca9548022..c3842ac01 100644 --- a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs @@ -1,5 +1,4 @@ using System.Reflection; -using Grand.Business.Core.Interfaces.Authentication; using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Interfaces.Common.Security; diff --git a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterTests.cs b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterTests.cs index dcc8ce78e..74f4f13e1 100644 --- a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterTests.cs @@ -1,4 +1,3 @@ -using Grand.Business.Core.Interfaces.Authentication; using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Interfaces.Common.Security; @@ -8,6 +7,7 @@ using Grand.Domain.Stores; using Grand.Domain.Tax; using Grand.Infrastructure.Configuration; +using Grand.Web.Common.Security.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; diff --git a/src/Tests/Grand.Web.Tests/Commands/Customers/UpdateCustomerInfoCommandHandlerTests.cs b/src/Tests/Grand.Web.Tests/Commands/Customers/UpdateCustomerInfoCommandHandlerTests.cs index 3fa9f7e34..b5b033ed8 100644 --- a/src/Tests/Grand.Web.Tests/Commands/Customers/UpdateCustomerInfoCommandHandlerTests.cs +++ b/src/Tests/Grand.Web.Tests/Commands/Customers/UpdateCustomerInfoCommandHandlerTests.cs @@ -1,4 +1,3 @@ -using Grand.Business.Core.Interfaces.Authentication; using Grand.Business.Core.Interfaces.Catalog.Tax; using Grand.Business.Core.Interfaces.Customers; using Grand.Business.Core.Interfaces.Marketing.Newsletters; @@ -7,6 +6,7 @@ using Grand.Infrastructure.Configuration; using Grand.Web.Commands.Handler.Customers; using Grand.Web.Commands.Models.Customers; +using Grand.Web.Common.Security.Authentication; using Grand.Web.Models.Customer; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; diff --git a/src/Tests/Grand.Web.Tests/Controllers/AccountControllerExternalRegisterTests.cs b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerExternalRegisterTests.cs index 74f3b7679..7f9d5d0f0 100644 --- a/src/Tests/Grand.Web.Tests/Controllers/AccountControllerExternalRegisterTests.cs +++ b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerExternalRegisterTests.cs @@ -10,6 +10,7 @@ using Grand.Infrastructure; using Grand.Infrastructure.Configuration; using Grand.Mediator; +using Grand.Web.Common.Security.Authentication; using Grand.Web.Common.Security.ExternalRegistration; using Grand.Web.Controllers; using Grand.Web.Features.Models.Customers; diff --git a/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs index 2e5b9f4de..45fe8eff2 100644 --- a/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs +++ b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs @@ -9,6 +9,7 @@ using Grand.Infrastructure; using Grand.Infrastructure.Configuration; using Grand.Mediator; +using Grand.Web.Common.Security.Authentication; using Grand.Web.Common.Security.ExternalRegistration; using Grand.Web.Controllers; using Grand.Web.Features.Models.Customers; diff --git a/src/Tests/Grand.Web.Tests/Controllers/AccountControllerTwoFactorReturnUrlTests.cs b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerTwoFactorReturnUrlTests.cs index f9ebadfae..433002287 100644 --- a/src/Tests/Grand.Web.Tests/Controllers/AccountControllerTwoFactorReturnUrlTests.cs +++ b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerTwoFactorReturnUrlTests.cs @@ -9,6 +9,7 @@ using Grand.Infrastructure; using Grand.Infrastructure.Configuration; using Grand.Mediator; +using Grand.Web.Common.Security.Authentication; using Grand.Web.Controllers; using Grand.Web.Models.Customer; using Microsoft.AspNetCore.Http; diff --git a/src/Web/Grand.Web.Admin/Controllers/LoginController.cs b/src/Web/Grand.Web.Admin/Controllers/LoginController.cs index e5844fb6a..ed8a66edd 100644 --- a/src/Web/Grand.Web.Admin/Controllers/LoginController.cs +++ b/src/Web/Grand.Web.Admin/Controllers/LoginController.cs @@ -1,5 +1,4 @@ -using Grand.Business.Core.Interfaces.Authentication; -using Grand.Business.Core.Interfaces.Customers; +using Grand.Business.Core.Interfaces.Customers; using Grand.Business.Core.Interfaces.Messages; using Grand.Domain.Common; using Grand.Domain.Customers; @@ -7,6 +6,7 @@ using Grand.Web.Admin.Extensions; using Grand.Web.AdminShared.Controllers; using Grand.Mediator; +using Grand.Web.Common.Security.Authentication; using Microsoft.AspNetCore.Mvc; using Grand.Web.Common.Localization; diff --git a/src/Web/Grand.Web.AdminShared/Controllers/BaseLoginController.cs b/src/Web/Grand.Web.AdminShared/Controllers/BaseLoginController.cs index c8a990e4c..21333964d 100644 --- a/src/Web/Grand.Web.AdminShared/Controllers/BaseLoginController.cs +++ b/src/Web/Grand.Web.AdminShared/Controllers/BaseLoginController.cs @@ -12,6 +12,7 @@ using Grand.Web.AdminShared.Models.Common; using Grand.Web.Common.Controllers; using Grand.Mediator; +using Grand.Web.Common.Security.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Grand.Web.Common.Localization; diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/CookieCustomerResolver.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/CookieCustomerResolver.cs index 5416d4588..c5d11782a 100644 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/CookieCustomerResolver.cs +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/CookieCustomerResolver.cs @@ -1,5 +1,5 @@ -using Grand.Business.Core.Interfaces.Authentication; using Grand.Domain.Customers; +using Grand.Web.Common.Security.Authentication; namespace Grand.Web.Common.RequestCustomerResolvers; diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCookieCustomerResolver.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCookieCustomerResolver.cs index 66ea5f1c6..d0c02bfeb 100644 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCookieCustomerResolver.cs +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCookieCustomerResolver.cs @@ -1,7 +1,7 @@ -using Grand.Business.Core.Interfaces.Authentication; using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Customers; using Grand.Domain.Customers; +using Grand.Web.Common.Security.Authentication; namespace Grand.Web.Common.RequestCustomerResolvers; diff --git a/src/Web/Grand.Web.Common/Security/Authentication/ExternalSignInService.cs b/src/Web/Grand.Web.Common/Security/Authentication/ExternalSignInService.cs new file mode 100644 index 000000000..b4d4f2eb0 --- /dev/null +++ b/src/Web/Grand.Web.Common/Security/Authentication/ExternalSignInService.cs @@ -0,0 +1,53 @@ +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Business.Core.Utilities.Authentication; +using Grand.Domain.Customers; +using Grand.Domain.Localization; +using Grand.Domain.Stores; +using Microsoft.AspNetCore.Mvc; + +namespace Grand.Web.Common.Security.Authentication; + +public class ExternalSignInService( + IExternalAuthenticationService externalAuthenticationService, + IGrandAuthenticationService authenticationService) : IExternalSignInService +{ + public virtual async Task Authenticate(ExternalAuthParam parameters, Customer customer, + Store store, Language language, string returnUrl = null) + { + //the logged-in or registered event is already published; sign in after it, as before + var result = await externalAuthenticationService.Authenticate(parameters, customer, store, language); + switch (result.Status) + { + case ExternalAuthStatus.LoggedIn: + await authenticationService.SignIn(result.Customer, false); + return ReturnUrl(returnUrl); + case ExternalAuthStatus.Registered: + await authenticationService.SignIn(result.Customer, false); + return RegisterResult(UserRegistrationType.Standard); + case ExternalAuthStatus.Associated: + return ReturnUrl(returnUrl); + case ExternalAuthStatus.AwaitingApproval: + return RegisterResult(UserRegistrationType.AdminApproval); + default: + return Error(result.Errors); + } + } + + protected virtual IActionResult ReturnUrl(string returnUrl) + { + if (string.IsNullOrEmpty(returnUrl)) + return new RedirectToRouteResult("HomePage", new { area = "" }); + + return new RedirectResult(returnUrl); + } + + protected virtual IActionResult RegisterResult(UserRegistrationType registrationType) + { + return new RedirectToRouteResult("RegisterResult", new { resultId = (int)registrationType }); + } + + protected virtual IActionResult Error(IEnumerable errors) + { + return new RedirectToActionResult("ExternalAuthenticationError", "Common", new { Errors = errors }); + } +} diff --git a/src/Web/Grand.Web.Common/Security/Authentication/IExternalSignInService.cs b/src/Web/Grand.Web.Common/Security/Authentication/IExternalSignInService.cs new file mode 100644 index 000000000..3e564b3a1 --- /dev/null +++ b/src/Web/Grand.Web.Common/Security/Authentication/IExternalSignInService.cs @@ -0,0 +1,25 @@ +using Grand.Business.Core.Utilities.Authentication; +using Grand.Domain.Customers; +using Grand.Domain.Localization; +using Grand.Domain.Stores; +using Microsoft.AspNetCore.Mvc; + +namespace Grand.Web.Common.Security.Authentication; + +/// +/// Completes a sign-in by an external authentication method: signs the customer in and picks where to go next +/// +public interface IExternalSignInService +{ + /// + /// Use passed parameters to authenticate customer + /// + /// External authentication parameters + /// Customer of the request (a guest when nobody is logged in) + /// Store of the request + /// Working language of the request + /// Defines the URL of user return, when successfully authenticated + /// Result of an authentication + Task Authenticate(ExternalAuthParam parameters, Customer customer, Store store, Language language, + string returnUrl = null); +} diff --git a/src/Business/Grand.Business.Core/Interfaces/Authentication/IGrandAuthenticationService.cs b/src/Web/Grand.Web.Common/Security/Authentication/IGrandAuthenticationService.cs similarity index 96% rename from src/Business/Grand.Business.Core/Interfaces/Authentication/IGrandAuthenticationService.cs rename to src/Web/Grand.Web.Common/Security/Authentication/IGrandAuthenticationService.cs index 01cd7254e..ac038041c 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Authentication/IGrandAuthenticationService.cs +++ b/src/Web/Grand.Web.Common/Security/Authentication/IGrandAuthenticationService.cs @@ -1,6 +1,6 @@ using Grand.Domain.Customers; -namespace Grand.Business.Core.Interfaces.Authentication; +namespace Grand.Web.Common.Security.Authentication; /// /// Authentication service interface diff --git a/src/Web/Grand.Web.Common/Security/ExternalRegistration/ProviderSignOut.cs b/src/Web/Grand.Web.Common/Security/ExternalRegistration/ProviderSignOut.cs index 609da5cc8..a854309ae 100644 --- a/src/Web/Grand.Web.Common/Security/ExternalRegistration/ProviderSignOut.cs +++ b/src/Web/Grand.Web.Common/Security/ExternalRegistration/ProviderSignOut.cs @@ -1,6 +1,7 @@ using Grand.Business.Core.Interfaces.Authentication; using Grand.Infrastructure; using Grand.Infrastructure.Configuration; +using Grand.Web.Common.Security.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; diff --git a/src/Web/Grand.Web.Common/Startup/StartupApplication.cs b/src/Web/Grand.Web.Common/Startup/StartupApplication.cs index 32341410d..1366816e1 100644 --- a/src/Web/Grand.Web.Common/Startup/StartupApplication.cs +++ b/src/Web/Grand.Web.Common/Startup/StartupApplication.cs @@ -1,4 +1,3 @@ -using Grand.Business.Core.Interfaces.Authentication; using Grand.Business.Core.Interfaces.Common.Pdf; using Grand.Business.Core.Interfaces.System.Admin; using Grand.Data; @@ -125,6 +124,7 @@ private static void RegisterFramework(IServiceCollection serviceCollection) serviceCollection.AddScoped(); serviceCollection.AddScoped(); serviceCollection.AddScoped(); + serviceCollection.AddScoped(); serviceCollection.AddScoped(); serviceCollection.AddScoped(); serviceCollection.AddScoped(); diff --git a/src/Web/Grand.Web.Common/WorkContextSetter.cs b/src/Web/Grand.Web.Common/WorkContextSetter.cs index aeba6b8a8..5aafafffb 100644 --- a/src/Web/Grand.Web.Common/WorkContextSetter.cs +++ b/src/Web/Grand.Web.Common/WorkContextSetter.cs @@ -1,5 +1,4 @@ -using Grand.Business.Core.Interfaces.Authentication; -using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Interfaces.Common.Security; using Grand.Business.Core.Interfaces.Common.Stores; @@ -14,6 +13,7 @@ using Grand.Infrastructure; using Grand.Infrastructure.Configuration; using Grand.Web.Common.RequestCustomerResolvers; +using Grand.Web.Common.Security.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Localization; using Microsoft.Extensions.DependencyInjection; diff --git a/src/Web/Grand.Web.Store/Controllers/LoginController.cs b/src/Web/Grand.Web.Store/Controllers/LoginController.cs index 72650e744..667be5ad1 100644 --- a/src/Web/Grand.Web.Store/Controllers/LoginController.cs +++ b/src/Web/Grand.Web.Store/Controllers/LoginController.cs @@ -1,10 +1,10 @@ -using Grand.Business.Core.Interfaces.Authentication; -using Grand.Business.Core.Interfaces.Customers; +using Grand.Business.Core.Interfaces.Customers; using Grand.Business.Core.Interfaces.Messages; using Grand.Domain.Common; using Grand.Domain.Customers; using Grand.Infrastructure; using Grand.Web.AdminShared.Controllers; +using Grand.Web.Common.Security.Authentication; using Grand.Web.Store.Extensions; using Grand.Mediator; using Microsoft.AspNetCore.Mvc; diff --git a/src/Web/Grand.Web.Vendor/Controllers/LoginController.cs b/src/Web/Grand.Web.Vendor/Controllers/LoginController.cs index d963d6493..4aef6cbde 100644 --- a/src/Web/Grand.Web.Vendor/Controllers/LoginController.cs +++ b/src/Web/Grand.Web.Vendor/Controllers/LoginController.cs @@ -1,10 +1,10 @@ -using Grand.Business.Core.Interfaces.Authentication; -using Grand.Business.Core.Interfaces.Customers; +using Grand.Business.Core.Interfaces.Customers; using Grand.Business.Core.Interfaces.Messages; using Grand.Domain.Common; using Grand.Domain.Customers; using Grand.Infrastructure; using Grand.Web.AdminShared.Controllers; +using Grand.Web.Common.Security.Authentication; using Grand.Web.Vendor.Extensions; using Grand.Mediator; using Microsoft.AspNetCore.Mvc; diff --git a/src/Web/Grand.Web/Commands/Handler/Customers/UpdateCustomerInfoCommandHandler.cs b/src/Web/Grand.Web/Commands/Handler/Customers/UpdateCustomerInfoCommandHandler.cs index 353491b7c..892b9acd5 100644 --- a/src/Web/Grand.Web/Commands/Handler/Customers/UpdateCustomerInfoCommandHandler.cs +++ b/src/Web/Grand.Web/Commands/Handler/Customers/UpdateCustomerInfoCommandHandler.cs @@ -1,5 +1,4 @@ -using Grand.Business.Core.Interfaces.Authentication; -using Grand.Business.Core.Interfaces.Catalog.Tax; +using Grand.Business.Core.Interfaces.Catalog.Tax; using Grand.Business.Core.Interfaces.Customers; using Grand.Business.Core.Interfaces.Marketing.Newsletters; using Grand.Domain.Common; @@ -9,6 +8,7 @@ using Grand.Infrastructure.Configuration; using Grand.Web.Commands.Models.Customers; using Grand.Mediator; +using Grand.Web.Common.Security.Authentication; namespace Grand.Web.Commands.Handler.Customers; diff --git a/src/Web/Grand.Web/Controllers/AccountController.cs b/src/Web/Grand.Web/Controllers/AccountController.cs index 92f98fbc0..cc7321e4e 100644 --- a/src/Web/Grand.Web/Controllers/AccountController.cs +++ b/src/Web/Grand.Web/Controllers/AccountController.cs @@ -16,6 +16,7 @@ using Grand.Web.Commands.Models.Customers; using Grand.Web.Common.Controllers; using Grand.Web.Common.Filters; +using Grand.Web.Common.Security.Authentication; using Grand.Web.Common.Security.Authorization; using Grand.Web.Common.Security.Captcha; using Grand.Web.Common.Security.ExternalRegistration; From 911d1f41b7b94a8f52f5786775f7cac9711133f9 Mon Sep 17 00:00:00 2001 From: support Date: Sun, 18 Oct 2026 14:01:42 +0200 Subject: [PATCH 2/2] Harden the external sign-in result handling - Redirect to the home page when returnUrl is not a local URL - Map every ExternalAuthStatus explicitly and throw on an unknown one - Translate the already-assigned and registration-disabled messages in Web (new resources in DefaultLanguage.xml and Upgrade/en_250.xml) - Expose ExternalAuthResult.Errors as IReadOnlyList and drop the unreachable registration error branch - Document that callers of IExternalAuthenticationService.Authenticate must sign in a LoggedIn or Registered customer --- .../Services/ExternalAuthenticationService.cs | 8 +- .../IExternalAuthenticationService.cs | 6 +- .../Authentication/ExternalAuthResult.cs | 15 ++-- .../ExternalAuthenticationServiceTests.cs | 2 +- .../ExternalSignInServiceTests.cs | 72 ++++++++++++++---- .../Authentication/ExternalSignInService.cs | 21 ++++- .../App_Data/Resources/DefaultLanguage.xml | Bin 1574920 -> 1575718 bytes .../App_Data/Resources/Upgrade/en_250.xml | 6 ++ 8 files changed, 99 insertions(+), 31 deletions(-) diff --git a/src/Business/Grand.Business.Authentication/Services/ExternalAuthenticationService.cs b/src/Business/Grand.Business.Authentication/Services/ExternalAuthenticationService.cs index 63d65086c..f83b15e2a 100644 --- a/src/Business/Grand.Business.Authentication/Services/ExternalAuthenticationService.cs +++ b/src/Business/Grand.Business.Authentication/Services/ExternalAuthenticationService.cs @@ -145,12 +145,8 @@ _customerSettings.UserRegistrationType is UserRegistrationType.Standard //associate external account with registered user await AssociateCustomer(customer, parameters); - if (approved) - return ExternalAuthResult.Registered(customer); - - return _customerSettings.UserRegistrationType == UserRegistrationType.AdminApproval - ? ExternalAuthResult.AwaitingApproval(customer) - : ExternalAuthResult.Error("Error on registration"); + //Disabled never gets here, so the only unapproved registration type is AdminApproval + return approved ? ExternalAuthResult.Registered(customer) : ExternalAuthResult.AwaitingApproval(customer); } /// diff --git a/src/Business/Grand.Business.Core/Interfaces/Authentication/IExternalAuthenticationService.cs b/src/Business/Grand.Business.Core/Interfaces/Authentication/IExternalAuthenticationService.cs index 64e989320..e012ec8fb 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Authentication/IExternalAuthenticationService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Authentication/IExternalAuthenticationService.cs @@ -13,8 +13,10 @@ public interface IExternalAuthenticationService #region Authentication /// - /// Use passed parameters to find, associate or register the customer; the caller signs in the customer of a - /// LoggedIn or Registered result + /// Use passed parameters to find, associate or register the customer. Nothing is signed in here: the caller + /// must sign in the customer of a LoggedIn or Registered result (IExternalSignInService in Grand.Web.Common + /// does it). A LoggedIn result has already published CustomerLoggedInEvent, which merges the guest's cart into + /// the customer, so a caller that skips the sign-in leaves the request as a guest with an emptied cart. /// /// External authentication parameters /// Customer of the request (a guest when nobody is logged in) diff --git a/src/Business/Grand.Business.Core/Utilities/Authentication/ExternalAuthResult.cs b/src/Business/Grand.Business.Core/Utilities/Authentication/ExternalAuthResult.cs index 403e91add..17547ec3e 100644 --- a/src/Business/Grand.Business.Core/Utilities/Authentication/ExternalAuthResult.cs +++ b/src/Business/Grand.Business.Core/Utilities/Authentication/ExternalAuthResult.cs @@ -19,16 +19,20 @@ public enum ExternalAuthStatus /// The external account belongs to another customer than the one signed in AlreadyAssigned, + /// The external account is unknown and registration of new customers is disabled RegistrationDisabled, + + /// The authentication failed; says why Error } /// -/// Outcome of an external authentication; signing in and redirecting is left to the caller +/// Outcome of an external authentication; signing in, redirecting and the texts shown to the customer are left +/// to the caller /// public class ExternalAuthResult { - private ExternalAuthResult(ExternalAuthStatus status, Customer customer, IList errors) + private ExternalAuthResult(ExternalAuthStatus status, Customer customer, IReadOnlyList errors) { Status = status; Customer = customer; @@ -40,7 +44,8 @@ private ExternalAuthResult(ExternalAuthStatus status, Customer customer, IListThe customer to sign in (LoggedIn, Registered) or the one the account was associated with public Customer Customer { get; } - public IList Errors { get; } + /// Technical reasons of an Error result; empty for every other status + public IReadOnlyList Errors { get; } public static ExternalAuthResult LoggedIn(Customer customer) => new(ExternalAuthStatus.LoggedIn, customer, []); public static ExternalAuthResult Registered(Customer customer) => new(ExternalAuthStatus.Registered, customer, []); @@ -50,10 +55,10 @@ public static ExternalAuthResult AwaitingApproval(Customer customer) => new(ExternalAuthStatus.AwaitingApproval, customer, []); public static ExternalAuthResult AlreadyAssigned() => - new(ExternalAuthStatus.AlreadyAssigned, null, ["Account is already assigned"]); + new(ExternalAuthStatus.AlreadyAssigned, null, []); public static ExternalAuthResult RegistrationDisabled() => - new(ExternalAuthStatus.RegistrationDisabled, null, ["Registration is disabled"]); + new(ExternalAuthStatus.RegistrationDisabled, null, []); public static ExternalAuthResult Error(string error) => new(ExternalAuthStatus.Error, null, [error]); } diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/ExternalAuthenticationServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/ExternalAuthenticationServiceTests.cs index ae64ebf36..e6eb88022 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/ExternalAuthenticationServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/ExternalAuthenticationServiceTests.cs @@ -223,7 +223,7 @@ public async Task Authenticate_IdentityOfAnotherCustomerWhileLoggedIn_IsAlreadyA var result = await _externalAuthenticationService.Authenticate(parameters, customer, new Store(), new Language()); Assert.AreEqual(ExternalAuthStatus.AlreadyAssigned, result.Status); - CollectionAssert.AreEqual(new[] { "Account is already assigned" }, result.Errors.ToArray()); + Assert.IsEmpty(result.Errors); } [TestMethod] diff --git a/src/Tests/Grand.Web.Common.Tests/Security/Authentication/ExternalSignInServiceTests.cs b/src/Tests/Grand.Web.Common.Tests/Security/Authentication/ExternalSignInServiceTests.cs index de31f8b41..5cfa4d456 100644 --- a/src/Tests/Grand.Web.Common.Tests/Security/Authentication/ExternalSignInServiceTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Security/Authentication/ExternalSignInServiceTests.cs @@ -1,4 +1,5 @@ using Grand.Business.Core.Interfaces.Authentication; +using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Utilities.Authentication; using Grand.Domain.Customers; using Grand.Domain.Localization; @@ -16,6 +17,7 @@ public class ExternalSignInServiceTests private readonly Customer _customer = new() { Id = "customer" }; private Mock _authenticationServiceMock; private Mock _externalAuthenticationServiceMock; + private Mock _translationServiceMock; private ExternalSignInService _service; [TestInitialize] @@ -23,8 +25,11 @@ public void Init() { _externalAuthenticationServiceMock = new Mock(); _authenticationServiceMock = new Mock(); + _translationServiceMock = new Mock(); + _translationServiceMock.Setup(t => t.GetResource(It.IsAny(), "lang1", It.IsAny(), + It.IsAny())).Returns((key, _, _, _) => $"{key}@lang1"); _service = new ExternalSignInService(_externalAuthenticationServiceMock.Object, - _authenticationServiceMock.Object); + _authenticationServiceMock.Object, _translationServiceMock.Object); } private Task Authenticate(ExternalAuthResult result, string returnUrl = null) @@ -33,7 +38,8 @@ private Task Authenticate(ExternalAuthResult result, string retur .Setup(s => s.Authenticate(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync(result); - return _service.Authenticate(new ExternalAuthParam(), new Customer(), new Store(), new Language(), returnUrl); + return _service.Authenticate(new ExternalAuthParam(), new Customer(), new Store(), new Language { Id = "lang1" }, + returnUrl); } [TestMethod] @@ -85,20 +91,58 @@ public async Task AwaitingApproval_ShowsAdminApprovalRegisterResultWithoutSignIn } [TestMethod] - public async Task Refusals_RedirectToExternalAuthenticationErrorWithoutSignIn() + [DataRow("https://evil.example")] + [DataRow("//evil.example")] + [DataRow("/\evil.example")] + [DataRow("javascript:alert(1)")] + public async Task LoggedIn_WithExternalReturnUrl_RedirectsToHomePage(string returnUrl) { - foreach (var refusal in new[] { - ExternalAuthResult.AlreadyAssigned(), ExternalAuthResult.RegistrationDisabled(), - ExternalAuthResult.Error("failed") - }) - { - var result = (RedirectToActionResult)await Authenticate(refusal); - - Assert.AreEqual("ExternalAuthenticationError", result.ActionName); - Assert.AreEqual("Common", result.ControllerName); - Assert.AreSame(refusal.Errors, result.RouteValues!["Errors"]); - } + var result = await Authenticate(ExternalAuthResult.LoggedIn(_customer), returnUrl); + _authenticationServiceMock.Verify(a => a.SignIn(_customer, false), Times.Once); + Assert.AreEqual("HomePage", ((RedirectToRouteResult)result).RouteName); + } + + [TestMethod] + public async Task Associated_WithExternalReturnUrl_RedirectsToHomePage() + { + var result = await Authenticate(ExternalAuthResult.Associated(_customer), "https://evil.example"); + + Assert.AreEqual("HomePage", ((RedirectToRouteResult)result).RouteName); + } + + [TestMethod] + public async Task AlreadyAssigned_ShowsTranslatedErrorWithoutSignIn() + { + var result = (RedirectToActionResult)await Authenticate(ExternalAuthResult.AlreadyAssigned()); + + Assert.AreEqual("ExternalAuthenticationError", result.ActionName); + Assert.AreEqual("Common", result.ControllerName); + CollectionAssert.AreEqual(new[] { "Account.AssociatedExternalAuth.AlreadyAssigned@lang1" }, + ((IEnumerable)result.RouteValues!["Errors"]).ToArray()); + _authenticationServiceMock.Verify(a => a.SignIn(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task RegistrationDisabled_ShowsTranslatedErrorWithoutSignIn() + { + var result = (RedirectToActionResult)await Authenticate(ExternalAuthResult.RegistrationDisabled()); + + Assert.AreEqual("ExternalAuthenticationError", result.ActionName); + CollectionAssert.AreEqual(new[] { "Account.AssociatedExternalAuth.RegistrationDisabled@lang1" }, + ((IEnumerable)result.RouteValues!["Errors"]).ToArray()); + _authenticationServiceMock.Verify(a => a.SignIn(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task Error_ShowsErrorsOfTheResultWithoutSignIn() + { + var error = ExternalAuthResult.Error("failed"); + + var result = (RedirectToActionResult)await Authenticate(error); + + Assert.AreEqual("ExternalAuthenticationError", result.ActionName); + Assert.AreSame(error.Errors, result.RouteValues!["Errors"]); _authenticationServiceMock.Verify(a => a.SignIn(It.IsAny(), It.IsAny()), Times.Never); } } diff --git a/src/Web/Grand.Web.Common/Security/Authentication/ExternalSignInService.cs b/src/Web/Grand.Web.Common/Security/Authentication/ExternalSignInService.cs index b4d4f2eb0..88e8f7061 100644 --- a/src/Web/Grand.Web.Common/Security/Authentication/ExternalSignInService.cs +++ b/src/Web/Grand.Web.Common/Security/Authentication/ExternalSignInService.cs @@ -1,15 +1,18 @@ using Grand.Business.Core.Interfaces.Authentication; +using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Utilities.Authentication; using Grand.Domain.Customers; using Grand.Domain.Localization; using Grand.Domain.Stores; +using Microsoft.AspNetCore.Http.HttpResults; using Microsoft.AspNetCore.Mvc; namespace Grand.Web.Common.Security.Authentication; public class ExternalSignInService( IExternalAuthenticationService externalAuthenticationService, - IGrandAuthenticationService authenticationService) : IExternalSignInService + IGrandAuthenticationService authenticationService, + ITranslationService translationService) : IExternalSignInService { public virtual async Task Authenticate(ExternalAuthParam parameters, Customer customer, Store store, Language language, string returnUrl = null) @@ -28,14 +31,26 @@ public virtual async Task Authenticate(ExternalAuthParam paramete return ReturnUrl(returnUrl); case ExternalAuthStatus.AwaitingApproval: return RegisterResult(UserRegistrationType.AdminApproval); - default: + case ExternalAuthStatus.AlreadyAssigned: + return Error([ + translationService.GetResource("Account.AssociatedExternalAuth.AlreadyAssigned", language.Id) + ]); + case ExternalAuthStatus.RegistrationDisabled: + return Error([ + translationService.GetResource("Account.AssociatedExternalAuth.RegistrationDisabled", language.Id) + ]); + case ExternalAuthStatus.Error: return Error(result.Errors); + default: + throw new ArgumentOutOfRangeException(nameof(result.Status), result.Status, + "Unhandled external authentication status"); } } protected virtual IActionResult ReturnUrl(string returnUrl) { - if (string.IsNullOrEmpty(returnUrl)) + //the return URL comes from the query string of the login request; never leave the store with it + if (string.IsNullOrEmpty(returnUrl) || !RedirectHttpResult.IsLocalUrl(returnUrl)) return new RedirectToRouteResult("HomePage", new { area = "" }); return new RedirectResult(returnUrl); diff --git a/src/Web/Grand.Web/App_Data/Resources/DefaultLanguage.xml b/src/Web/Grand.Web/App_Data/Resources/DefaultLanguage.xml index f3f1813fd387728d7b4e7e65326191eb715fd691..82316f2f3ae7d460068ec246aa6cd05cd7801fc3 100644 GIT binary patch delta 274 zcmWm5&nrYx7{>AU&fIxj$93o2ABH3yEXKmZA7CXbD_fh}6;qUqj0hVwiRINPZ^=$s z$YM%Svapb~e}NgAQdYLgWAS-b-{&OHuFkSn%Cf%4BhtMLw6sp+Ny(2&Y&-37yLrbl z|Hayx&ajN`vzF&`FOkLZXny^ngf?n8^xkHqBCe?NWSKf2E^<@%mKulK+FZi+(66&#JoTZ(#H?P0zfPX#6mzU48$TpEDFS8+okx#dHw The account links now belong to the configured identity provider. + + This external account is already connected to another customer account. + + + Registration of new customers is disabled, so this external account cannot be used to create one. +