diff --git a/src/Business/Grand.Business.Authentication/Services/ApiAuthenticationService.cs b/src/Business/Grand.Business.Authentication/Services/ApiAuthenticationService.cs index c4a521b65a..f87a24d45a 100644 --- a/src/Business/Grand.Business.Authentication/Services/ApiAuthenticationService.cs +++ b/src/Business/Grand.Business.Authentication/Services/ApiAuthenticationService.cs @@ -58,6 +58,11 @@ public virtual async Task GetAuthenticatedCustomer() if (!authenticateResult.Succeeded) return null; + //only a ticket of the Bearer scheme itself carries claims GrandNode signed with its SecretKey; a result the + //scheme forwarded (a provider token handed to the Oidc scheme) names its customer through the OIDC link + if (authenticateResult.Ticket?.AuthenticationScheme != JwtBearerDefaults.AuthenticationScheme) + return null; + //prefer the stable customer id (unambiguous with per-store identity), fall back to e-mail for old tokens var customerIdClaim = authenticateResult.Principal.Claims.FirstOrDefault(claim => claim.Type == "CustomerId"); if (customerIdClaim != null) diff --git a/src/Business/Grand.Business.Authentication/Services/JwtBearerAuthenticationService.cs b/src/Business/Grand.Business.Authentication/Services/JwtBearerAuthenticationService.cs index 0e9be1cc25..40c2df7bad 100644 --- a/src/Business/Grand.Business.Authentication/Services/JwtBearerAuthenticationService.cs +++ b/src/Business/Grand.Business.Authentication/Services/JwtBearerAuthenticationService.cs @@ -2,6 +2,8 @@ using Grand.Business.Core.Interfaces.Customers; using Microsoft.AspNetCore.Authentication.JwtBearer; +#pragma warning disable CS0618 //API users stay supported until removed + namespace Grand.Business.Authentication.Services; public class JwtBearerAuthenticationService : IJwtBearerAuthenticationService diff --git a/src/Business/Grand.Business.Authentication/Services/OidcCustomerResolver.cs b/src/Business/Grand.Business.Authentication/Services/OidcCustomerResolver.cs index bf09e11d67..3c0b146b42 100644 --- a/src/Business/Grand.Business.Authentication/Services/OidcCustomerResolver.cs +++ b/src/Business/Grand.Business.Authentication/Services/OidcCustomerResolver.cs @@ -159,7 +159,7 @@ private async Task Register(string sub, string email, string LastActivityDateUtc = DateTime.UtcNow }); - //the provider verified the e-mail and access comes from permissions an admin grants, so the store's e-mail + //access comes from permissions an admin grants (and the provider's e-mail check is configurable), so the store's e-mail //validation / admin approval does not apply; the account gets the Registered group only await _customerManagerService.RegisterCustomer(new RegistrationRequest(customer, normalizedEmail, normalizedEmail, CommonHelper.GenerateRandomDigitCode(20), PasswordFormat.Hashed, storeId)); diff --git a/src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs b/src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs index ab3818cfad..20bc77417f 100644 --- a/src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs +++ b/src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs @@ -6,6 +6,8 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +#pragma warning disable CS0618 //API users stay supported until removed + namespace Grand.Business.Authentication.Startup; public class StartupApplication : IStartupApplication diff --git a/src/Business/Grand.Business.Core/Interfaces/Authentication/IJwtBearerAuthenticationService.cs b/src/Business/Grand.Business.Core/Interfaces/Authentication/IJwtBearerAuthenticationService.cs index 6ae310b2a7..610c48f722 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Authentication/IJwtBearerAuthenticationService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Authentication/IJwtBearerAuthenticationService.cs @@ -2,6 +2,7 @@ namespace Grand.Business.Core.Interfaces.Authentication; +[Obsolete("API users with a shared SecretKey are deprecated - configure OidcResourceServer and use access tokens of your identity provider")] public interface IJwtBearerAuthenticationService { /// diff --git a/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcDisabledPathProvider.cs b/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcDisabledPathProvider.cs new file mode 100644 index 0000000000..85bad89373 --- /dev/null +++ b/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcDisabledPathProvider.cs @@ -0,0 +1,13 @@ +namespace Grand.Business.Core.Interfaces.Authentication; + +/// +/// Lets a module that can be switched off withdraw the paths it serves from OIDC protection while it is off. A +/// protected resource on, under or over one of these paths is left out of the registry: a token there is never +/// validated by the Oidc scheme, mapped to a customer or used to pin the store, and the resource is not published. +/// Read once, when the registry is built, so the switch must be a startup setting. Register the implementation +/// as a singleton. +/// +public interface IOidcDisabledPathProvider +{ + IEnumerable GetDisabledPaths(); +} diff --git a/src/Business/Grand.Business.Core/Interfaces/Customers/IUserApiService.cs b/src/Business/Grand.Business.Core/Interfaces/Customers/IUserApiService.cs index f84eee91dc..8a39ed82fa 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Customers/IUserApiService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Customers/IUserApiService.cs @@ -3,6 +3,7 @@ namespace Grand.Business.Core.Interfaces.Customers; +[Obsolete("API users with a shared SecretKey are deprecated - configure OidcResourceServer and use access tokens of your identity provider")] public interface IUserApiService { /// diff --git a/src/Business/Grand.Business.Customers/Services/UserApiService.cs b/src/Business/Grand.Business.Customers/Services/UserApiService.cs index ce6344e615..09461d9d98 100644 --- a/src/Business/Grand.Business.Customers/Services/UserApiService.cs +++ b/src/Business/Grand.Business.Customers/Services/UserApiService.cs @@ -5,6 +5,8 @@ using Grand.Infrastructure.Extensions; using Grand.Mediator; +#pragma warning disable CS0618 //API users stay supported until removed + namespace Grand.Business.Customers.Services; public class UserApiService : IUserApiService diff --git a/src/Business/Grand.Business.Customers/Startup/StartupApplication.cs b/src/Business/Grand.Business.Customers/Startup/StartupApplication.cs index 954aee0d82..3988285bef 100644 --- a/src/Business/Grand.Business.Customers/Startup/StartupApplication.cs +++ b/src/Business/Grand.Business.Customers/Startup/StartupApplication.cs @@ -11,6 +11,8 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +#pragma warning disable CS0618 //API users stay supported until removed + namespace Grand.Business.Customers.Startup; public class StartupApplication : IStartupApplication diff --git a/src/Core/Grand.Domain/Customers/UserApi.cs b/src/Core/Grand.Domain/Customers/UserApi.cs index ee8cc3a9bd..510d3ae312 100644 --- a/src/Core/Grand.Domain/Customers/UserApi.cs +++ b/src/Core/Grand.Domain/Customers/UserApi.cs @@ -1,5 +1,6 @@ namespace Grand.Domain.Customers; +[Obsolete("API users with a shared SecretKey are deprecated - configure OidcResourceServer and use access tokens of your identity provider")] public class UserApi : BaseEntity { public string Email { get; set; } diff --git a/src/Modules/Grand.Module.Api/Controllers/TokenController.cs b/src/Modules/Grand.Module.Api/Controllers/TokenController.cs index 36ee6c2b99..92bda40ca4 100644 --- a/src/Modules/Grand.Module.Api/Controllers/TokenController.cs +++ b/src/Modules/Grand.Module.Api/Controllers/TokenController.cs @@ -5,6 +5,8 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; +#pragma warning disable CS0618 //API users stay supported until removed + namespace Grand.Module.Api.Controllers; [ApiController] diff --git a/src/Modules/Grand.Module.Api/Filters/AuthorizeApiAdminAttribute.cs b/src/Modules/Grand.Module.Api/Filters/AuthorizeApiAdminAttribute.cs index 86ed8c47fd..080e4ba986 100644 --- a/src/Modules/Grand.Module.Api/Filters/AuthorizeApiAdminAttribute.cs +++ b/src/Modules/Grand.Module.Api/Filters/AuthorizeApiAdminAttribute.cs @@ -2,6 +2,7 @@ using Grand.Domain.Permissions; using Grand.Data; using Grand.Domain.Security; +using Grand.Infrastructure.Configuration; using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; @@ -31,7 +32,7 @@ public AuthorizeApiAdminAttribute(bool ignore = false) : base(typeof(AuthorizeAp /// Represents a filter that confirms access to the admin panel /// private class AuthorizeApiAdminFilter(bool ignoreFilter, IPermissionService permissionService, - SecuritySettings securitySettings) : IAsyncAuthorizationFilter + SecuritySettings securitySettings, BackendAPIConfig backendApiConfig) : IAsyncAuthorizationFilter { #region Ctor @@ -48,6 +49,14 @@ public async Task OnAuthorizationAsync(AuthorizationFilterContext filterContext) { ArgumentNullException.ThrowIfNull(filterContext); + //the kill switch also covers tokens forwarded to the OpenID Connect scheme, which skip the Bearer scheme's + //check; it applies to every action, including those that ignore the admin area checks below + if (!backendApiConfig.Enabled) + { + filterContext.Result = new ForbidResult(JwtBearerDefaults.AuthenticationScheme); + return; + } + //check whether this filter has been overridden for the action var actionFilter = filterContext.ActionDescriptor.FilterDescriptors .Where(f => f.Scope == FilterScope.Action) diff --git a/src/Modules/Grand.Module.Api/Infrastructure/ApiAuthenticationRegistrar.cs b/src/Modules/Grand.Module.Api/Infrastructure/ApiAuthenticationRegistrar.cs index e3c8400a3e..ba69f1fe57 100644 --- a/src/Modules/Grand.Module.Api/Infrastructure/ApiAuthenticationRegistrar.cs +++ b/src/Modules/Grand.Module.Api/Infrastructure/ApiAuthenticationRegistrar.cs @@ -9,6 +9,8 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.IdentityModel.Tokens; +#pragma warning disable CS0618 //API users stay supported until removed + namespace Grand.Module.Api.Infrastructure; public class ApiAuthenticationRegistrar : IAuthenticationBuilder diff --git a/src/Modules/Grand.Module.Api/Infrastructure/ApiOidcDisabledPathProvider.cs b/src/Modules/Grand.Module.Api/Infrastructure/ApiOidcDisabledPathProvider.cs new file mode 100644 index 0000000000..5754483019 --- /dev/null +++ b/src/Modules/Grand.Module.Api/Infrastructure/ApiOidcDisabledPathProvider.cs @@ -0,0 +1,15 @@ +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Infrastructure.Configuration; +using Grand.Module.Api.Constants; + +namespace Grand.Module.Api.Infrastructure; + +/// +/// While BackendAPI.Enabled is false the Web API's paths are withdrawn from OIDC protection, so an access token +/// there is refused by the Bearer scheme before it is mapped to a customer or pins the store +/// +public class ApiOidcDisabledPathProvider(BackendAPIConfig config) : IOidcDisabledPathProvider +{ + public IEnumerable GetDisabledPaths() => + config.Enabled ? [] : ["/" + Configurations.RestRoutePrefix]; +} diff --git a/src/Modules/Grand.Module.Api/Infrastructure/ApiSecurityStartup.cs b/src/Modules/Grand.Module.Api/Infrastructure/ApiSecurityStartup.cs index 6436707b90..56a58b6bf2 100644 --- a/src/Modules/Grand.Module.Api/Infrastructure/ApiSecurityStartup.cs +++ b/src/Modules/Grand.Module.Api/Infrastructure/ApiSecurityStartup.cs @@ -1,3 +1,4 @@ +using Grand.Business.Core.Interfaces.Authentication; using Grand.Infrastructure; using Grand.Infrastructure.Configuration; using Microsoft.AspNetCore.Builder; @@ -65,6 +66,8 @@ private static bool IsStrong(string secret) public void ConfigureServices(IServiceCollection services, IConfiguration configuration) { + //a disabled Web API also refuses access tokens of the identity provider, before any customer is resolved + services.AddSingleton(); } public int Priority => 0; diff --git a/src/Modules/Grand.Module.Api/Validators/Common/LoginValidator.cs b/src/Modules/Grand.Module.Api/Validators/Common/LoginValidator.cs index 08ea7710c4..674e784d8c 100644 --- a/src/Modules/Grand.Module.Api/Validators/Common/LoginValidator.cs +++ b/src/Modules/Grand.Module.Api/Validators/Common/LoginValidator.cs @@ -6,6 +6,8 @@ using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Validators; +#pragma warning disable CS0618 //API users stay supported until removed + namespace Grand.Module.Api.Validators.Common; public class LoginValidator : BaseGrandValidator diff --git a/src/Modules/Grand.Module.Installer/Services/InstallationService.cs b/src/Modules/Grand.Module.Installer/Services/InstallationService.cs index 50b31aa431..10f5fc5ca2 100644 --- a/src/Modules/Grand.Module.Installer/Services/InstallationService.cs +++ b/src/Modules/Grand.Module.Installer/Services/InstallationService.cs @@ -39,6 +39,8 @@ using Microsoft.Extensions.DependencyInjection; using System.Security.Cryptography; +#pragma warning disable CS0618 //API users stay supported until removed + namespace Grand.Module.Installer.Services; public partial class InstallationService : IInstallationService diff --git a/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationUpdateResourceString.cs b/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationUpdateResourceString.cs new file mode 100644 index 0000000000..67a1714b94 --- /dev/null +++ b/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationUpdateResourceString.cs @@ -0,0 +1,16 @@ +using Grand.Infrastructure.Migrations; + +namespace Grand.Module.Migration.Migrations._2._5; + +public class MigrationUpdateResourceString : IMigration +{ + public int Priority => 0; + public DbVersion Version => new(2, 5); + public Guid Identity => new("15EDC88E-A865-4BA2-9E43-FB97839F1B92"); + public string Name => "Update resource string for english language 2.5"; + + public bool UpgradeProcess(IServiceProvider serviceProvider) + { + return serviceProvider.ImportLanguageResourcesFromXml("App_Data/Resources/Upgrade/en_250.xml"); + } +} \ No newline at end of file diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/ApiAuthenticationServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/ApiAuthenticationServiceTests.cs index f51407e846..dcb944e458 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/ApiAuthenticationServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/ApiAuthenticationServiceTests.cs @@ -14,6 +14,8 @@ using Moq; using System.Security.Claims; +#pragma warning disable CS0618 //API users stay supported until removed + namespace Grand.Business.Authentication.Tests.Services; [TestClass] @@ -235,6 +237,25 @@ public async Task GetAuthenticatedCustomer_JwtSchemeRegisteredAndSucceeds_Return Assert.AreSame(customer, result); } + [TestMethod] + public async Task GetAuthenticatedCustomer_BearerForwardedToOidc_ReturnsNull() + { + //a provider token the Bearer scheme forwards to the Oidc scheme: its CustomerId claim was not issued by GrandNode + var customer = new Customer { Id = "c1", Active = true }; + _customerService.Setup(c => c.GetCustomerById("c1")).ReturnsAsync(customer); + _groupService.Setup(g => g.IsRegistered(customer)).ReturnsAsync(true); + var context = CreateHttpContext(b => b + .AddScheme(JwtBearerDefaults.AuthenticationScheme, + o => o.ForwardDefault = "Oidc") + .AddScheme("Oidc", null)); + + var result = await CreateService(context).GetAuthenticatedCustomer(); + + Assert.IsNull(result); + _customerService.Verify(c => c.GetCustomerById(It.IsAny()), Times.Never); + _customerService.Verify(c => c.GetCustomerByEmail(It.IsAny()), Times.Never); + } + private class OkHandler : AuthenticationHandler { public OkHandler(Microsoft.Extensions.Options.IOptionsMonitor options, diff --git a/src/Tests/Grand.Business.Customers.Tests/Services/UserApiServiceTests.cs b/src/Tests/Grand.Business.Customers.Tests/Services/UserApiServiceTests.cs index 11303c545a..7d7c91135c 100644 --- a/src/Tests/Grand.Business.Customers.Tests/Services/UserApiServiceTests.cs +++ b/src/Tests/Grand.Business.Customers.Tests/Services/UserApiServiceTests.cs @@ -6,6 +6,8 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; +#pragma warning disable CS0618 //API users stay supported until removed + namespace Grand.Business.Customers.Tests.Services; [TestClass] diff --git a/src/Tests/Grand.Mapping.Tests/AdminShared/CustomerMappingTests.cs b/src/Tests/Grand.Mapping.Tests/AdminShared/CustomerMappingTests.cs index edf927c7c9..b35d2e78f5 100644 --- a/src/Tests/Grand.Mapping.Tests/AdminShared/CustomerMappingTests.cs +++ b/src/Tests/Grand.Mapping.Tests/AdminShared/CustomerMappingTests.cs @@ -8,6 +8,8 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using VerifyMSTest; +#pragma warning disable CS0618 //API users stay supported until removed + namespace Grand.Mapping.Tests.AdminShared; [TestClass] diff --git a/src/Tests/Grand.Module.Api.Tests/Filters/AuthorizeApiAdminAttributeTests.cs b/src/Tests/Grand.Module.Api.Tests/Filters/AuthorizeApiAdminAttributeTests.cs new file mode 100644 index 0000000000..d2f86149b2 --- /dev/null +++ b/src/Tests/Grand.Module.Api.Tests/Filters/AuthorizeApiAdminAttributeTests.cs @@ -0,0 +1,88 @@ +using Grand.Business.Core.Interfaces.Common.Security; +using Grand.Data; +using Grand.Domain.Permissions; +using Grand.Domain.Security; +using Grand.Infrastructure.Configuration; +using Grand.Module.Api.Filters; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Module.Api.Tests.Filters; + +[TestClass] +public class AuthorizeApiAdminAttributeTests +{ + private Mock _permissionService; + + [TestInitialize] + public void Init() + { + DataSettingsManager.Initialize(Path.Join(Path.GetTempPath(), "grand-api-tests-settings.json")); + DataSettingsManager.Instance.LoadDataSettings(new DataSettings { ConnectionString = "mongodb://localhost" }); + _permissionService = new Mock(); + } + + private async Task Run(bool apiEnabled, bool ignore, bool permitted) + { + _permissionService.Setup(x => x.Authorize(It.IsAny())).ReturnsAsync(permitted); + var attribute = new AuthorizeApiAdminAttribute(ignore); + var filter = (IAsyncAuthorizationFilter)ActivatorUtilities.CreateInstance( + new ServiceCollection() + .AddSingleton(_permissionService.Object) + .AddSingleton(new SecuritySettings()) + .AddSingleton(new BackendAPIConfig { Enabled = apiEnabled }) + .BuildServiceProvider(), + attribute.ImplementationType, ignore); + var context = new AuthorizationFilterContext( + new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()), + [new FilterDescriptor(attribute, FilterScope.Controller).Filter]); + context.Filters.Add((IFilterMetadata)filter); + await filter.OnAuthorizationAsync(context); + return context; + } + + [TestMethod] + public async Task ApiDisabled_Forbid() + { + var context = await Run(apiEnabled: false, ignore: false, permitted: true); + Assert.IsInstanceOfType(context.Result); + _permissionService.Verify(x => x.Authorize(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task ApiDisabled_IgnoredAction_Forbid() + { + //ignore skips the admin panel checks, not the kill switch + var context = await Run(apiEnabled: false, ignore: true, permitted: true); + Assert.IsInstanceOfType(context.Result); + } + + [TestMethod] + public async Task ApiEnabled_IgnoredAction_NoPermissionCheck() + { + var context = await Run(apiEnabled: true, ignore: true, permitted: false); + Assert.IsNull(context.Result); + _permissionService.Verify(x => x.Authorize(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task ApiEnabled_PermissionDenied_Forbid() + { + var context = await Run(apiEnabled: true, ignore: false, permitted: false); + Assert.IsInstanceOfType(context.Result); + _permissionService.Verify(x => x.Authorize(It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task ApiEnabled_PermissionGranted_Allowed() + { + var context = await Run(apiEnabled: true, ignore: false, permitted: true); + Assert.IsNull(context.Result); + } +} diff --git a/src/Tests/Grand.Module.Api.Tests/Infrastructure/ApiOidcDisabledPathProviderTests.cs b/src/Tests/Grand.Module.Api.Tests/Infrastructure/ApiOidcDisabledPathProviderTests.cs new file mode 100644 index 0000000000..6db2c6adb5 --- /dev/null +++ b/src/Tests/Grand.Module.Api.Tests/Infrastructure/ApiOidcDisabledPathProviderTests.cs @@ -0,0 +1,19 @@ +using Grand.Infrastructure.Configuration; +using Grand.Module.Api.Infrastructure; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Grand.Module.Api.Tests.Infrastructure; + +[TestClass] +public class ApiOidcDisabledPathProviderTests +{ + [TestMethod] + public void ApiDisabled_ApiPathWithdrawn() => + CollectionAssert.AreEqual(new[] { "/api" }, + new ApiOidcDisabledPathProvider(new BackendAPIConfig { Enabled = false }).GetDisabledPaths().ToList()); + + [TestMethod] + public void ApiEnabled_NothingWithdrawn() => + Assert.AreEqual(0, + new ApiOidcDisabledPathProvider(new BackendAPIConfig { Enabled = true }).GetDisabledPaths().Count()); +} diff --git a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcForwardingPipelineTests.cs b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcForwardingPipelineTests.cs new file mode 100644 index 0000000000..eb47a7a15a --- /dev/null +++ b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcForwardingPipelineTests.cs @@ -0,0 +1,265 @@ +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Business.Core.Utilities.Authentication; +using Grand.Domain.Customers; +using Grand.Infrastructure.Configuration; +using Grand.Web.Common.Security.Oidc; +using Microsoft.AspNetCore.Authentication.JwtBearer; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.IdentityModel.JsonWebTokens; +using Microsoft.IdentityModel.Tokens; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using System.Net; +using System.Security.Claims; +using System.Text; + +namespace Grand.Web.Common.Tests.Security.Oidc; + +[TestClass] +public class OidcForwardingPipelineTests +{ + private const string LegacySecret = "legacy-secret-key-at-least-32-characters"; + private OidcTestHost _host; + private HttpClient _client; + private Mock _resolver; + + private async Task Start(OidcResourceServerConfig config, bool webApiDisabled = false) + { + if (_host != null) await _host.DisposeAsync(); + _host = new OidcTestHost(); + _client = await _host.Start(config, + services: s => + { + s.AddSingleton(_resolver.Object); + //what Grand.Module.Api registers while BackendAPI.Enabled is false + if (webApiDisabled) + s.AddSingleton(new DisabledPaths("/api")); + }, + afterAuthorization: app => app.UseMiddleware(), + auth: b => b.AddJwtBearer(JwtBearerDefaults.AuthenticationScheme, o => + { + o.TokenValidationParameters = new TokenValidationParameters { + ValidateIssuer = false, ValidateAudience = false, ValidateLifetime = true, ValidateIssuerSigningKey = true, + IssuerSigningKey = new SymmetricSecurityKey(Encoding.ASCII.GetBytes(LegacySecret)) + }; + }), + endpoints: e => + { + e.MapGet("/api/legacy", (HttpContext c) => + c.User.FindFirst("sub")?.Value ?? c.User.FindFirst("Email")?.Value ?? "none") + .RequireAuthorization(new AuthorizeAttribute { AuthenticationSchemes = JwtBearerDefaults.AuthenticationScheme }); + e.MapGet("/api/forbid", () => Results.Forbid(authenticationSchemes: [JwtBearerDefaults.AuthenticationScheme])) + .RequireAuthorization(new AuthorizeAttribute { AuthenticationSchemes = JwtBearerDefaults.AuthenticationScheme }); + //the store ContextMiddleware builds the work context in + e.MapGet("/api/legacy/store", (HttpContext c) => RequestStorePin.Get(c) ?? "none") + .RequireAuthorization(new AuthorizeAttribute { AuthenticationSchemes = JwtBearerDefaults.AuthenticationScheme }); + //no [Authorize]: the work context path + e.MapGet("/api/plain", async (HttpContext c) => + { + var resolver = c.RequestServices.GetServices().Single(); + return (await resolver.Resolve())?.Id ?? "none"; + }); + }); + } + + [TestInitialize] + public async Task Init() + { + _resolver = new Mock(); + _resolver.Setup(x => x.Resolve(It.IsAny(), It.IsAny())) + .ReturnsAsync(OidcCustomerResult.Success(new Customer { Id = "c1", Active = true })); + await Start(OidcTestHost.Config()); + } + + [TestCleanup] + public async Task Cleanup() => await _host.DisposeAsync(); + + private static string LegacyToken() => new JsonWebTokenHandler().CreateToken(new SecurityTokenDescriptor { + Subject = new ClaimsIdentity([new Claim("Email", "admin@example.com")]), + Expires = DateTime.UtcNow.AddMinutes(10), + SigningCredentials = new SigningCredentials(new SymmetricSecurityKey(Encoding.ASCII.GetBytes(LegacySecret)), SecurityAlgorithms.HmacSha256) + }); + + private Task Get(string path, string token = null) => OidcTestHost.Send(_client, path, token); + + [TestMethod] + public async Task IdpToken_OnBearerEndpoint_200() + { + using var response = await Get("/api/legacy", OidcTestHost.Token()); + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode); + Assert.AreEqual("sub-1", await response.Content.ReadAsStringAsync()); + } + + [TestMethod] + public async Task LegacySecretKeyToken_200() + { + using var response = await Get("/api/legacy", LegacyToken()); + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode); + Assert.AreEqual("admin@example.com", await response.Content.ReadAsStringAsync()); + _resolver.Verify(x => x.Resolve(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task IdpToken_OnBearerEndpoint_StorePinned() + { + using var response = await Get("/api/legacy/store", OidcTestHost.Token()); + Assert.AreEqual("store1", await response.Content.ReadAsStringAsync()); + } + + [TestMethod] + public async Task LegacySecretKeyToken_StoreNotPinned() + { + using var response = await Get("/api/legacy/store", LegacyToken()); + Assert.AreEqual("none", await response.Content.ReadAsStringAsync()); + } + + [TestMethod] + public async Task LegacySecretKeyToken_RequestCustomerResolver_NotOidc() + { + using var response = await Get("/api/plain", LegacyToken()); + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode); + Assert.AreEqual("none", await response.Content.ReadAsStringAsync()); + _resolver.Verify(x => x.Resolve(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task NoToken_OnBearerEndpoint_401_OidcChallenge() + { + using var response = await Get("/api/legacy"); + Assert.AreEqual(HttpStatusCode.Unauthorized, response.StatusCode); + StringAssert.Contains(response.Headers.WwwAuthenticate.ToString(), "resource_metadata"); + } + + [TestMethod] + public async Task ThreeSegmentNonJwt_OnBearerEndpoint_401_OidcChallenge() + { + using var response = await Get("/api/legacy", "x.y.z"); + Assert.AreEqual(HttpStatusCode.Unauthorized, response.StatusCode); + StringAssert.Contains(response.Headers.WwwAuthenticate.ToString(), "resource_metadata"); + } + + [TestMethod] + public async Task ForgedIssuer_WrongKey_401() + { + using var response = await Get("/api/legacy", OidcTestHost.Token(key: OidcTestHost.ForeignKey)); + Assert.AreEqual(HttpStatusCode.Unauthorized, response.StatusCode); + _resolver.Verify(x => x.Resolve(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task IdpToken_ForbidResult_403_NoError_Never500() + { + using var response = await Get("/api/forbid", OidcTestHost.Token()); + Assert.AreEqual(HttpStatusCode.Forbidden, response.StatusCode); + Assert.IsFalse(response.Headers.WwwAuthenticate.ToString().Contains("error="), response.Headers.WwwAuthenticate.ToString()); + } + + [TestMethod] + public async Task IdpSignedToken_OtherIssuer_401_InvalidToken_ResolverNotCalled() + { + using var response = await Get("/api/legacy", OidcTestHost.Token(issuer: "https://other.example.com")); + Assert.AreEqual(HttpStatusCode.Unauthorized, response.StatusCode); + StringAssert.Contains(response.Headers.WwwAuthenticate.ToString(), "error=\"invalid_token\""); + StringAssert.Contains(response.Headers.WwwAuthenticate.ToString(), "issuer"); + _resolver.Verify(x => x.Resolve(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task HmacToken_IdpIssuer_RejectedByBearer_ResolverNotCalled() + { + //HMAC-signed with a key that is not the SecretKey, claiming the provider as issuer: routed by its algorithm + var token = OidcTestHost.Token(key: new SymmetricSecurityKey(Encoding.ASCII.GetBytes(new string('k', 40))), + algorithm: SecurityAlgorithms.HmacSha256); + using var response = await Get("/api/legacy", token); + Assert.AreEqual(HttpStatusCode.Unauthorized, response.StatusCode); + Assert.IsFalse(response.Headers.WwwAuthenticate.ToString().Contains("resource_metadata"), + response.Headers.WwwAuthenticate.ToString()); + _resolver.Verify(x => x.Resolve(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + [DataRow("garbage")] + [DataRow("")] + public async Task Malformed_401_Never500(string token) + { + using var response = await Get("/api/legacy", token); + Assert.AreEqual(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [TestMethod] + public async Task Disabled_IdpTokenRejected_LegacyAccepted() + { + var config = OidcTestHost.Config(); + config.Enabled = false; + await Start(config); + + using var idpToken = await Get("/api/legacy", OidcTestHost.Token()); + Assert.AreEqual(HttpStatusCode.Unauthorized, idpToken.StatusCode); + using var legacyToken = await Get("/api/legacy", LegacyToken()); + Assert.AreEqual(HttpStatusCode.OK, legacyToken.StatusCode); + } + + private class DisabledPaths(params string[] paths) : IOidcDisabledPathProvider + { + public IEnumerable GetDisabledPaths() => paths; + } + + [TestMethod] + public async Task WebApiDisabled_IdpToken_RefusedBeforeCustomerResolution() + { + await Start(OidcTestHost.Config(), webApiDisabled: true); + + using var bearerEndpoint = await Get("/api/legacy", OidcTestHost.Token()); + Assert.AreEqual(HttpStatusCode.Unauthorized, bearerEndpoint.StatusCode); + using var pinned = await Get("/api/legacy/store", OidcTestHost.Token()); + Assert.AreEqual(HttpStatusCode.Unauthorized, pinned.StatusCode); + //no [Authorize]: neither OidcAuthenticationMiddleware nor the work context maps the token + using var plain = await Get("/api/plain", OidcTestHost.Token()); + Assert.AreEqual("none", await plain.Content.ReadAsStringAsync()); + + _resolver.Verify(x => x.Resolve(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task WebApiDisabled_ResourceNotPublished() + { + await Start(OidcTestHost.Config(), webApiDisabled: true); + + using var response = await Get("/.well-known/oauth-protected-resource/api"); + Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode); + } + + private static string Unsigned(string payloadJson) + { + static string B64(string json) => Convert.ToBase64String(Encoding.UTF8.GetBytes(json)).TrimEnd('=').Replace('+', '-').Replace('/', '_'); + return B64("{\"alg\":\"none\"}") + "." + B64(payloadJson) + "."; + } + + [TestMethod] + [DataRow("{\"iss\":123}")] + [DataRow("{\"iss\":{\"a\":1}}")] + [DataRow("{\"iss\":[\"a\",\"b\"]}")] + [DataRow("[1,2]")] + [DataRow("null")] + public async Task MalformedIssuerPayload_Never500(string payload) + { + var token = Unsigned(payload); + var legacy = await Get("/api/legacy", token); + Assert.IsTrue((int)legacy.StatusCode < 500, "legacy " + legacy.StatusCode); + var plain = await Get("/api/plain", token); + Assert.AreEqual(HttpStatusCode.OK, plain.StatusCode); + Assert.AreEqual("none", await plain.Content.ReadAsStringAsync()); + } + + [TestMethod] + public async Task Anonymous_PlainEndpoint_NoChallenge_ResolverNotCalled() + { + using var response = await Get("/api/plain"); + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode); + Assert.AreEqual("none", await response.Content.ReadAsStringAsync()); + _resolver.Verify(x => x.Resolve(It.IsAny(), It.IsAny()), Times.Never); + } +} diff --git a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcProtectedResourceRegistryTests.cs b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcProtectedResourceRegistryTests.cs index dc80467c01..8c0b0219ad 100644 --- a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcProtectedResourceRegistryTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcProtectedResourceRegistryTests.cs @@ -19,6 +19,11 @@ private class ThrowingProvider : IOidcProtectedResourceProvider public IEnumerable GetResources() => throw new InvalidOperationException("boom"); } + private class DisabledPaths(params string[] paths) : IOidcDisabledPathProvider + { + public IEnumerable GetDisabledPaths() => paths; + } + private static OidcResourceServerConfig Config(params OidcProtectedResource[] resources) => new() { Enabled = true, Authority = "https://idp.example.com", Resources = [.. resources] }; @@ -46,6 +51,42 @@ public void Resources_BuiltOnce_ReadOnly() Assert.IsNotInstanceOfType(registry.Resources, typeof(List)); } + [TestMethod] + public void DisabledPath_ResourcesOnOrUnderIt_Withdrawn() + { + var registry = new OidcProtectedResourceRegistry( + Config(new OidcProtectedResource { Name = "BackendApi", PathPrefix = "/api/" }, + new OidcProtectedResource { Name = "ApiV2", PathPrefix = "/API/v2" }, + new OidcProtectedResource { Name = "Apix", PathPrefix = "/apix" }), + [new Provider(new OidcProtectedResource { Name = "Tools", PathPrefix = "/tools" })], + [new DisabledPaths("/api")]); + + CollectionAssert.AreEquivalent(new[] { "Apix", "Tools" }, registry.Resources.Select(r => r.Name).ToList()); + CollectionAssert.AreEquivalent(new[] { "BackendApi", "ApiV2" }, registry.Withdrawn.Select(r => r.Name).ToList()); + Assert.AreEqual(0, registry.Errors.Count); + Assert.IsNull(registry.Match(new PathString("/api/Product"))); + } + + [TestMethod] + public void DisabledPath_UnderAResource_WithdrawsIt() + { + var registry = new OidcProtectedResourceRegistry( + Config(new OidcProtectedResource { Name = "Wide", PathPrefix = "/x" }), [], [new DisabledPaths("/x/api")]); + + Assert.AreEqual(0, registry.Resources.Count); + Assert.AreEqual("Wide", registry.Withdrawn.Single().Name); + } + + [TestMethod] + public void NoDisabledPaths_NothingWithdrawn() + { + var registry = new OidcProtectedResourceRegistry( + Config(new OidcProtectedResource { Name = "BackendApi", PathPrefix = "/api" }), [], [new DisabledPaths()]); + + Assert.AreEqual(1, registry.Resources.Count); + Assert.AreEqual(0, registry.Withdrawn.Count); + } + [TestMethod] public void Disabled_NoResources() { diff --git a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcResourceServerConfigValidatorTests.cs b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcResourceServerConfigValidatorTests.cs index a2b7a32090..c411149ec3 100644 --- a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcResourceServerConfigValidatorTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcResourceServerConfigValidatorTests.cs @@ -62,6 +62,21 @@ public void NoResources() Assert.AreEqual(1, Validate(config).Count); } + [TestMethod] + public void OnlyWithdrawnResources_NoError() + { + //the Web API is switched off: its resource is configured but withdrawn, which is not a misconfiguration + var config = Valid(); + var registry = new OidcProtectedResourceRegistry(config, [], [new DisabledApi()]); + Assert.AreEqual(0, registry.Resources.Count); + Assert.AreEqual(0, OidcResourceServerConfigValidator.Validate(config, registry, false).Count); + } + + private class DisabledApi : Grand.Business.Core.Interfaces.Authentication.IOidcDisabledPathProvider + { + public IEnumerable GetDisabledPaths() => ["/api"]; + } + [TestMethod] public void RegistryErrorsIncluded() { diff --git a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcSchemeSelectorTests.cs b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcSchemeSelectorTests.cs new file mode 100644 index 0000000000..c3bb67d0f8 --- /dev/null +++ b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcSchemeSelectorTests.cs @@ -0,0 +1,140 @@ +using Grand.Infrastructure.Configuration; +using Grand.Web.Common.Security.Oidc; +using Microsoft.AspNetCore.Authentication.JwtBearer; +using Microsoft.AspNetCore.Http; +using Microsoft.IdentityModel.JsonWebTokens; +using Microsoft.IdentityModel.Tokens; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System.Text; + +namespace Grand.Web.Common.Tests.Security.Oidc; + +[TestClass] +public class OidcSchemeSelectorTests +{ + private const string Issuer = "https://idp.example.com/realms/shop"; + private OidcSchemeSelector _selector; + + [TestInitialize] + public void Init() + { + var config = new OidcResourceServerConfig { + Enabled = true, Authority = Issuer + "/", + Resources = [new OidcProtectedResource { Name = "BackendApi", PathPrefix = "/api" }] + }; + _selector = new OidcSchemeSelector(new OidcProtectedResourceRegistry(config, [])); + } + + private static string Token(string issuer) => new JsonWebTokenHandler().CreateToken(new SecurityTokenDescriptor { + Issuer = issuer, + SigningCredentials = new SigningCredentials(new SymmetricSecurityKey(Encoding.ASCII.GetBytes(new string('k', 32))), SecurityAlgorithms.HmacSha256) + }); + + private static HttpContext Request(string path, string authorization = null) + { + var context = new DefaultHttpContext(); + context.Request.Path = path; + if (authorization != null) context.Request.Headers.Authorization = authorization; + return context; + } + + [TestMethod] public void NoToken_Resource_Oidc() => Assert.AreEqual("Oidc", _selector.Select(Request("/api/Product"))); + //routing reads the algorithm only: an HMAC token stays with Bearer whatever issuer it claims (Oidc accepts no HMAC) + [TestMethod] public void HmacToken_IdpIssuer_Null() => Assert.IsNull(_selector.Select(Request("/api/Product", "Bearer " + Token(Issuer)))); + [TestMethod] public void LegacyToken_NoIssuer_Null() => Assert.IsNull(_selector.Select(Request("/api/Product", "Bearer " + Token(null)))); + [TestMethod] public void ForeignIssuer_Null() => Assert.IsNull(_selector.Select(Request("/api/Product", "Bearer " + Token("grandnode")))); + [TestMethod] public void IdpToken_OutsideResources_Null() => Assert.IsNull(_selector.Select(Request("/customer/info", "Bearer " + OidcTestHost.Token()))); + + [TestMethod] + public void AsymmetricToken_ForeignIssuer_Oidc() + { + var token = OidcTestHost.Token(issuer: "https://other.example.com"); + Assert.AreEqual("Oidc", _selector.Select(Request("/api/Product", "Bearer " + token))); + } + + [TestMethod] + public void HmacToken_ForeignIssuer_Null() => Assert.IsNull(_selector.Select(Request("/api/Product", "Bearer " + Token("https://other.example.com")))); + + private static string WithAlgorithm(string algorithm) + { + static string B64(string json) => Base64UrlEncoder.Encode(Encoding.UTF8.GetBytes(json)); + return B64("{\"alg\":\"" + algorithm + "\",\"typ\":\"JWT\"}") + "." + B64("{\"sub\":\"1\"}") + ".c2ln"; + } + + [TestMethod] + [DataRow("RS256")] + [DataRow("PS384")] + [DataRow("ES512")] + public void AcceptedAsymmetricAlgorithm_Oidc(string algorithm) => + Assert.AreEqual("Oidc", _selector.Select(Request("/api/Product", "Bearer " + WithAlgorithm(algorithm)))); + + //the Oidc scheme's own list decides, not a prefix: these start with RS / ES but Oidc never accepts them + [TestMethod] + [DataRow("RSA-OAEP")] + [DataRow("ES256K")] + [DataRow("HS256")] + [DataRow("none")] + public void AlgorithmOutsideTheOidcList_Null(string algorithm) => + Assert.IsNull(_selector.Select(Request("/api/Product", "Bearer " + WithAlgorithm(algorithm)))); + + [TestMethod] + [DataRow("Bearer not-a-jwt")] + [DataRow("Bearer x.y.z")] + [DataRow("Bearer ")] + [DataRow("Basic dXNlcjpwYXNz")] + public void Unreadable_TreatedAsNoToken(string header) => Assert.AreEqual("Oidc", _selector.Select(Request("/api/x", header))); + + [TestMethod] + public void DecidedOncePerRequest() + { + var context = Request("/api/Product", "Bearer " + OidcTestHost.Token()); + Assert.AreEqual("Oidc", _selector.Select(context)); + Assert.IsTrue(context.Items.ContainsKey(OidcResourceServerDefaults.RouteItemKey)); + + //the token is not read again: the cached decision answers + context.Request.Headers.Authorization = "Bearer " + Token(null); + Assert.AreEqual("Oidc", _selector.Select(context)); + } + + [TestMethod] + public void PathChanged_DecidedAgain() + { + var context = Request("/api/Product", "Bearer " + OidcTestHost.Token()); + Assert.AreEqual("Oidc", _selector.Select(context)); + + context.Request.Path = "/other"; + Assert.IsNull(_selector.Select(context)); + } + + [TestMethod] + public void OversizedToken_TreatedAsNoToken() + { + var header = "Bearer " + WithAlgorithm("HS256") + new string('a', TokenValidationParameters.DefaultMaximumTokenSizeInBytes); + Assert.AreEqual("Oidc", _selector.Select(Request("/api/Product", header))); + } + + [TestMethod] + public void PostConfigure_Bearer_ChainsPreviousSelector() + { + var bearer = new JwtBearerOptions { ForwardDefaultSelector = _ => "Previous" }; + _selector.PostConfigure("Bearer", bearer); + Assert.AreEqual("Oidc", bearer.ForwardDefaultSelector(Request("/api/x"))); + Assert.AreEqual("Previous", bearer.ForwardDefaultSelector(Request("/other"))); + } + + [TestMethod] + [DataRow("Oidc")] + [DataRow("FrontAuthentication")] + [DataRow("PluginJwt")] + public void PostConfigure_OtherJwtBearerScheme_KeepsItsOwnSelector(string scheme) + { + Func own = _ => null; + var options = new JwtBearerOptions { ForwardDefaultSelector = own }; + _selector.PostConfigure(scheme, options); + Assert.AreSame(own, options.ForwardDefaultSelector); + + var none = new JwtBearerOptions(); + _selector.PostConfigure(scheme, none); + Assert.IsNull(none.ForwardDefaultSelector); + } +} 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 60ea005145..d1cee850de 100644 --- a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs @@ -16,6 +16,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.IdentityModel.Tokens; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using System.Security.Claims; @@ -86,7 +87,7 @@ private TestableSetter CreateSetter() new ApiUserCustomerResolver(_apiAuthentication.Object, _accessor.Object, _customerService.Object)); services.AddSingleton(new CookieCustomerResolver(_authentication.Object)); services.AddSingleton( - new OidcRequestCustomerResolver(_accessor.Object, _registry.Object)); + new OidcRequestCustomerResolver(_accessor.Object, new OidcSchemeSelector(_registry.Object))); services.AddSingleton( new GuestCookieCustomerResolver(_authentication.Object, _customerService.Object, Mock.Of())); services.AddSingleton( @@ -101,10 +102,28 @@ private TestableSetter CreateSetter() [TestMethod] public void Order_BeforeEveryBuiltInResolver() { - var resolver = new OidcRequestCustomerResolver(_accessor.Object, _registry.Object); + var resolver = new OidcRequestCustomerResolver(_accessor.Object, new OidcSchemeSelector(_registry.Object)); Assert.IsTrue(resolver.Order < RequestCustomerResolverOrder.AllowAnonymous); } + [TestMethod] + public async Task FollowsTheSchemeSelectorsDecisionForTheRequest() + { + //the authorization middleware already routed this request away from Oidc (a SecretKey token); the work + //context must not read the token again and route it differently + var selector = new OidcSchemeSelector(_registry.Object); + _httpContext.Request.Headers.Authorization = "Bearer " + OidcTestHost.Token( + key: new SymmetricSecurityKey(new byte[32]), algorithm: SecurityAlgorithms.HmacSha256); + Assert.IsNull(selector.Select(_httpContext)); + _httpContext.Request.Headers.Authorization = "Bearer " + OidcTestHost.Token(); + TokenValidatedFor(new Customer { Id = "token", Active = true }); + + var customer = await new OidcRequestCustomerResolver(_accessor.Object, selector).Resolve(); + + Assert.IsNull(customer); + _aspNetAuthentication.Verify(a => a.AuthenticateAsync(It.IsAny(), It.IsAny()), Times.Never); + } + [TestMethod] public async Task ValidToken_TokenCustomerWinsOverCookieCustomer() { @@ -176,6 +195,23 @@ public async Task PathOutsideProtectedResources_TokenNotAuthenticated() _aspNetAuthentication.Verify(a => a.AuthenticateAsync(It.IsAny(), It.IsAny()), Times.Never); } + [TestMethod] + public async Task TokenOfAnotherIssuer_LeftToTheWebApiScheme() + { + //the SecretKey JWT GrandNode issues itself: HMAC-signed, another issuer + var secretKey = new SymmetricSecurityKey(new byte[32]); + _httpContext.Request.Headers.Authorization = "Bearer " + OidcTestHost.Token(key: secretKey, + algorithm: SecurityAlgorithms.HmacSha256, issuer: "https://grandnode.api"); + _authentication.Setup(a => a.GetAuthenticatedCustomer()).ReturnsAsync((Customer)null); + _apiAuthentication.Setup(a => a.GetAuthenticatedCustomer()).ReturnsAsync(new Customer { Id = "api", Active = true }); + TokenValidatedFor(new Customer { Id = "token", Active = true }); + + var customer = await CreateSetter().CurrentCustomerForTest(new Store()); + + Assert.AreEqual("api", customer.Id); + _aspNetAuthentication.Verify(a => a.AuthenticateAsync(It.IsAny(), It.IsAny()), Times.Never); + } + private class TestableSetter( IHttpContextAccessor httpContextAccessor, IGrandAuthenticationService authenticationService, ICurrencyService currencyService, ICustomerService customerService, diff --git a/src/Web/Grand.Web.Admin/Areas/Admin/Views/ApiUser/Index.cshtml b/src/Web/Grand.Web.Admin/Areas/Admin/Views/ApiUser/Index.cshtml index 08a3ffb2d6..cec7ad9203 100644 --- a/src/Web/Grand.Web.Admin/Areas/Admin/Views/ApiUser/Index.cshtml +++ b/src/Web/Grand.Web.Admin/Areas/Admin/Views/ApiUser/Index.cshtml @@ -10,6 +10,8 @@ +
@Loc["Admin.System.APIUsers.Deprecated"]
+ Configuration problems found while merging (reported at startup) IReadOnlyList Errors { get; } + /// Configured resources left out because the module serving their path is switched off + IReadOnlyList Withdrawn { get; } + /// The resource covering the path, or null OidcProtectedResource Match(PathString path); } diff --git a/src/Web/Grand.Web.Common/Security/Oidc/OidcAuthenticationBuilderExtensions.cs b/src/Web/Grand.Web.Common/Security/Oidc/OidcAuthenticationBuilderExtensions.cs index 94ad99777c..49f7706ef7 100644 --- a/src/Web/Grand.Web.Common/Security/Oidc/OidcAuthenticationBuilderExtensions.cs +++ b/src/Web/Grand.Web.Common/Security/Oidc/OidcAuthenticationBuilderExtensions.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Options; using Microsoft.IdentityModel.Protocols; using Microsoft.IdentityModel.Protocols.OpenIdConnect; @@ -11,7 +12,10 @@ namespace Grand.Web.Common.Security.Oidc; public static class OidcAuthenticationBuilderExtensions { - private static readonly string[] AsymmetricAlgorithms = [ + /// + /// The signing algorithms the Oidc scheme accepts; the scheme selector routes a token here by the same list + /// + public static IReadOnlyList AsymmetricAlgorithms { get; } = [ SecurityAlgorithms.RsaSha256, SecurityAlgorithms.RsaSha384, SecurityAlgorithms.RsaSha512, SecurityAlgorithms.RsaSsaPssSha256, SecurityAlgorithms.RsaSsaPssSha384, SecurityAlgorithms.RsaSsaPssSha512, SecurityAlgorithms.EcdsaSha256, SecurityAlgorithms.EcdsaSha384, SecurityAlgorithms.EcdsaSha512 @@ -29,6 +33,11 @@ public static AuthenticationBuilder AddOidcResourceServer(this AuthenticationBui { ArgumentNullException.ThrowIfNull(config); var enabled = config.Enabled; + //the work context asks the selector too; it forwards the Bearer scheme only while the resource server is enabled + builder.Services.TryAddSingleton(); + if (enabled) + builder.Services.AddSingleton>( + services => services.GetRequiredService()); builder.AddJwtBearer(OidcResourceServerDefaults.AuthenticationScheme, options => { diff --git a/src/Web/Grand.Web.Common/Security/Oidc/OidcProtectedResourceRegistry.cs b/src/Web/Grand.Web.Common/Security/Oidc/OidcProtectedResourceRegistry.cs index 160fd1b56c..d7b5c7b0f8 100644 --- a/src/Web/Grand.Web.Common/Security/Oidc/OidcProtectedResourceRegistry.cs +++ b/src/Web/Grand.Web.Common/Security/Oidc/OidcProtectedResourceRegistry.cs @@ -6,20 +6,38 @@ namespace Grand.Web.Common.Security.Oidc; /// /// Protected resources from appsettings and from installed plugins, validated once: unique names, unique and -/// non-overlapping prefixes. Invalid entries are left out and reported in . +/// non-overlapping prefixes. Invalid entries are left out and reported in ; resources on the +/// paths of a switched-off module () are left out and listed in +/// . /// public class OidcProtectedResourceRegistry : IOidcProtectedResourceRegistry { private readonly List<(string Prefix, OidcProtectedResource Resource)> _entries = []; private readonly List _errors = []; + private readonly List _withdrawn = []; + private readonly List _disabledPrefixes = []; private readonly IReadOnlyList _resources = []; public OidcProtectedResourceRegistry(OidcResourceServerConfig config, - IEnumerable providers) + IEnumerable providers, + IEnumerable disabledPaths = null) { if (config is not { Enabled: true }) return; + foreach (var provider in disabledPaths ?? []) + { + try + { + _disabledPrefixes.AddRange((provider.GetDisabledPaths() ?? []).Select(OidcPath.Normalize) + .Where(x => x != null)); + } + catch (Exception ex) + { + _errors.Add($"{provider.GetType().FullName} failed to return its disabled paths: {ex.Message}"); + } + } + foreach (var resource in config.Resources ?? []) Add(resource, "configuration"); @@ -45,6 +63,7 @@ public OidcProtectedResourceRegistry(OidcResourceServerConfig config, public IReadOnlyList Resources => _resources; public IReadOnlyList Errors => _errors; + public IReadOnlyList Withdrawn => _withdrawn; public OidcProtectedResource Match(PathString path) { @@ -66,6 +85,14 @@ private void Add(OidcProtectedResource resource, string source) return; } + //the module serving the path is switched off: no token there reaches the Oidc scheme + if (_disabledPrefixes.Any(disabled => + OidcPath.Covers(disabled, new PathString(prefix)) || OidcPath.Covers(prefix, new PathString(disabled)))) + { + _withdrawn.Add(resource); + return; + } + if (_entries.Any(x => string.Equals(x.Resource.Name, resource.Name, StringComparison.OrdinalIgnoreCase))) { _errors.Add($"Protected resource '{resource.Name}' from {source}: the name is already used."); diff --git a/src/Web/Grand.Web.Common/Security/Oidc/OidcRequestCustomerResolver.cs b/src/Web/Grand.Web.Common/Security/Oidc/OidcRequestCustomerResolver.cs index ad63a0068d..ab5c1baf78 100644 --- a/src/Web/Grand.Web.Common/Security/Oidc/OidcRequestCustomerResolver.cs +++ b/src/Web/Grand.Web.Common/Security/Oidc/OidcRequestCustomerResolver.cs @@ -18,12 +18,12 @@ public class OidcRequestCustomerResolver : IRequestCustomerResolver private const int ResolverOrder = 100; private readonly IHttpContextAccessor _httpContextAccessor; - private readonly IOidcProtectedResourceRegistry _registry; + private readonly OidcSchemeSelector _selector; - public OidcRequestCustomerResolver(IHttpContextAccessor httpContextAccessor, IOidcProtectedResourceRegistry registry) + public OidcRequestCustomerResolver(IHttpContextAccessor httpContextAccessor, OidcSchemeSelector selector) { _httpContextAccessor = httpContextAccessor; - _registry = registry; + _selector = selector; } public int Order => ResolverOrder; @@ -31,7 +31,9 @@ public OidcRequestCustomerResolver(IHttpContextAccessor httpContextAccessor, IOi public async Task Resolve() { var httpContext = _httpContextAccessor.HttpContext; - if (httpContext == null || _registry.Match(httpContext.Request.Path) == null) + //the decision the Bearer scheme forwards by: a protected path, and no token or one an identity provider signed + //(an HMAC token - the SecretKey JWT of the Web API - belongs to its own scheme) + if (httpContext == null || _selector.Select(httpContext) == null) return null; //for [Authorize] endpoints the authorization middleware already did this - the result is cached per request diff --git a/src/Web/Grand.Web.Common/Security/Oidc/OidcResourceServerConfigValidator.cs b/src/Web/Grand.Web.Common/Security/Oidc/OidcResourceServerConfigValidator.cs index beed276e1d..cabeb3685f 100644 --- a/src/Web/Grand.Web.Common/Security/Oidc/OidcResourceServerConfigValidator.cs +++ b/src/Web/Grand.Web.Common/Security/Oidc/OidcResourceServerConfigValidator.cs @@ -34,7 +34,7 @@ public static IList Validate(OidcResourceServerConfig config, IOidcProte errors.Add("OidcResourceServer.Claims: Subject, Email and EmailVerified must name a claim."); errors.AddRange(registry.Errors); - if (registry.Resources.Count == 0 && registry.Errors.Count == 0) + if (registry.Resources.Count == 0 && registry.Errors.Count == 0 && registry.Withdrawn.Count == 0) errors.Add("OidcResourceServer is enabled but no protected resource is configured or registered by a plugin."); errors.AddRange(registry.Resources diff --git a/src/Web/Grand.Web.Common/Security/Oidc/OidcResourceServerDefaults.cs b/src/Web/Grand.Web.Common/Security/Oidc/OidcResourceServerDefaults.cs index aa207f0efd..c1d12d30fd 100644 --- a/src/Web/Grand.Web.Common/Security/Oidc/OidcResourceServerDefaults.cs +++ b/src/Web/Grand.Web.Common/Security/Oidc/OidcResourceServerDefaults.cs @@ -10,4 +10,5 @@ public static class OidcResourceServerDefaults public const string CustomerItemKey = "Grand.Oidc.Customer"; public const string FailureItemKey = "Grand.Oidc.Failure"; public const string ResourceItemKey = "Grand.Oidc.Resource"; + public const string RouteItemKey = "Grand.Oidc.Route"; } diff --git a/src/Web/Grand.Web.Common/Security/Oidc/OidcSchemeSelector.cs b/src/Web/Grand.Web.Common/Security/Oidc/OidcSchemeSelector.cs new file mode 100644 index 0000000000..e7baffe6d2 --- /dev/null +++ b/src/Web/Grand.Web.Common/Security/Oidc/OidcSchemeSelector.cs @@ -0,0 +1,89 @@ +using Microsoft.AspNetCore.Authentication.JwtBearer; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Options; +using Microsoft.IdentityModel.JsonWebTokens; +using Microsoft.IdentityModel.Tokens; +using Microsoft.Net.Http.Headers; + +namespace Grand.Web.Common.Security.Oidc; + +/// +/// Lets the Web API's JwtBearer scheme ("Bearer") hand requests on protected resources to the Oidc +/// scheme: requests without a readable bearer JWT (so the 401 is the OIDC challenge) and requests whose token is +/// signed asymmetrically, as identity providers sign. HMAC tokens - those GrandNode signs with its SecretKey - stay +/// with the original scheme, whatever issuer they claim (Oidc accepts no HMAC algorithm). The algorithm is read +/// unvalidated, for routing only; the scheme the request lands on validates the token. +/// +public class OidcSchemeSelector : IPostConfigureOptions +{ + private readonly IOidcProtectedResourceRegistry _registry; + + public OidcSchemeSelector(IOidcProtectedResourceRegistry registry) + { + _registry = registry; + } + + public void PostConfigure(string name, JwtBearerOptions options) + { + //registered only while the resource server is enabled (OidcAuthenticationBuilderExtensions); only the Web + //API's scheme: other JwtBearer schemes (FrontendAPI, plugins) keep their own validation and challenge + if (name != JwtBearerDefaults.AuthenticationScheme) + return; + + var previous = options.ForwardDefaultSelector; + options.ForwardDefaultSelector = context => Select(context) ?? previous?.Invoke(context); + } + + /// + /// "Oidc" for a request on a protected resource that the Oidc scheme should authenticate, otherwise null. + /// Decided once per request (the selector runs for every authenticate, challenge and forbid, and the work + /// context asks too) and kept in HttpContext.Items with the path it was made for. + /// + public string Select(HttpContext context) + { + var path = context.Request.Path; + if (context.Items.TryGetValue(OidcResourceServerDefaults.RouteItemKey, out var cached) && + cached is Route route && route.Path == path) + return route.Scheme; + + var scheme = _registry.Match(path) != null && IsOidcRequest(context.Request) + ? OidcResourceServerDefaults.AuthenticationScheme + : null; + context.Items[OidcResourceServerDefaults.RouteItemKey] = new Route(path, scheme); + return scheme; + } + + /// + /// True when the request carries no readable bearer JWT, or one signed with an asymmetric algorithm the Oidc + /// scheme accepts. The token is parsed once, unvalidated. + /// + private static bool IsOidcRequest(HttpRequest request) + { + string authorization = request.Headers[HeaderNames.Authorization]; + if (string.IsNullOrEmpty(authorization) || + !authorization.StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase)) + return true; + + var token = authorization["Bearer ".Length..].Trim(); + //the size limit JwtBearer applies itself + if (token.Length == 0 || token.Length > TokenValidationParameters.DefaultMaximumTokenSizeInBytes) + return true; + + string algorithm; + try + { + algorithm = new JsonWebToken(token).Alg; + } + catch (Exception ex) when (ex is ArgumentException or SecurityTokenMalformedException or FormatException) + { + //attacker-controlled input read for routing only: what the parser reports for a malformed value + //(not a JWS/JWE, undecodable segments, invalid JSON) counts as no token - never a 500 + return true; + } + + //identity providers sign asymmetrically; the tokens GrandNode issues itself are HMAC (HS*) + return algorithm != null && OidcAuthenticationBuilderExtensions.AsymmetricAlgorithms.Contains(algorithm); + } + + private sealed record Route(PathString Path, string Scheme); +} diff --git a/src/Web/Grand.Web.Common/Startup/OidcResourceServerStartup.cs b/src/Web/Grand.Web.Common/Startup/OidcResourceServerStartup.cs index 9e0d13b1dc..b1bafda3f9 100644 --- a/src/Web/Grand.Web.Common/Startup/OidcResourceServerStartup.cs +++ b/src/Web/Grand.Web.Common/Startup/OidcResourceServerStartup.cs @@ -34,16 +34,21 @@ public void Configure(WebApplication application, IWebHostEnvironment webHostEnv return; var registry = application.Services.GetRequiredService(); + var logger = application.Services.GetRequiredService().CreateLogger(nameof(OidcResourceServerStartup)); var errors = OidcResourceServerConfigValidator.Validate(config, registry, webHostEnvironment.IsDevelopment()); if (errors.Count > 0) { var message = string.Join(" ", errors); if (!webHostEnvironment.IsDevelopment()) throw new InvalidOperationException(message); - application.Services.GetRequiredService() - .CreateLogger(nameof(OidcResourceServerStartup)).LogWarning("{Message}", message); + logger.LogWarning("{Message}", message); } + foreach (var resource in registry.Withdrawn) + logger.LogInformation( + "Protected resource '{Name}' ({PathPrefix}) is withdrawn: the module serving its path is switched off", + resource.Name, resource.PathPrefix); + OidcProtectedResourceMetadata.Map(application); } diff --git a/src/Web/Grand.Web/App_Data/Resources/DefaultLanguage.xml b/src/Web/Grand.Web/App_Data/Resources/DefaultLanguage.xml index 1deef84754..878cea7858 100644 Binary files a/src/Web/Grand.Web/App_Data/Resources/DefaultLanguage.xml and b/src/Web/Grand.Web/App_Data/Resources/DefaultLanguage.xml differ diff --git a/src/Web/Grand.Web/App_Data/Resources/Upgrade/en_250.xml b/src/Web/Grand.Web/App_Data/Resources/Upgrade/en_250.xml new file mode 100644 index 0000000000..2ca6ecae76 --- /dev/null +++ b/src/Web/Grand.Web/App_Data/Resources/Upgrade/en_250.xml @@ -0,0 +1,6 @@ + + + + API users signed with a shared secret key are deprecated. Configure the OIDC resource server (OidcResourceServer in appsettings) and use access tokens issued by your identity provider. + + diff --git a/src/Web/Grand.Web/App_Data/appsettings.json b/src/Web/Grand.Web/App_Data/appsettings.json index 7c0dc1c110..09036bd4d1 100644 --- a/src/Web/Grand.Web/App_Data/appsettings.json +++ b/src/Web/Grand.Web/App_Data/appsettings.json @@ -229,6 +229,8 @@ //api for admin //when enabled (Development environment only), API documentation (Scalar) is available at: https://your-domain/scalar/v1 //locally: https://localhost:44350/scalar/v1 + //DEPRECATED: API users with tokens signed by SecretKey. Prefer OidcResourceServer; this keeps working until it is + //removed in a future major version. "BackendAPI": { "Enabled": true, //REQUIRED: replace with a strong, unique secret (>= 32 chars) kept out of source control (env var BackendAPI__SecretKey or a secret store). @@ -253,7 +255,23 @@ "RequireEmailVerified": true, //register an unknown user (customer group Registered only - no admin access until an admin grants it) "AutoRegister": false, - "Resources": [] + "Resources": [ + { + //the Web API (Grand.Module.Api); its existing SecretKey tokens keep working next to provider tokens + "Name": "BackendApi", + "PathPrefix": "/api", + "Resource": "", + "Audiences": [], + "AuthorizedParties": [], + "ScopesSupported": [ "grandnode.api" ], + "RequiredScopes": [ "grandnode.api" ], + "RequireAdminPanelAccess": true, + //the Web API's controllers apply no vendor or store scoping: store-wide data, so main administrators only + "DenyVendors": true, + "DenyStoreManagers": true + } + ] + }, "Logging": { "LogLevel": {