yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Only update security stamp once per request (#15562)

Browse Source 
* Add item in requestcache when security stamp is already updated in request

* Propagate constructur obsoletion to implementing services and fix unit tests

---------

Co-authored-by: kjac <kja@umbraco.dk>
This commit is contained in:
Bjarke Berg
2024-01-11 10:20:58 +01:00
committed by GitHub
parent b916c85738
commit d3dc85f31f
4 changed files with 104 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
src/Umbraco.Web.BackOffice/Security/BackOfficeSignInManager.cs
View File

@@ -6,6 +6,7 @@ using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Events;
@@ -37,8 +38,9 @@ public class BackOfficeSignInManager : UmbracoSignInManager<BackOfficeIdentityUs
IAuthenticationSchemeProvider schemes,
IUserConfirmation<BackOfficeIdentityUser> confirmation,
IEventAggregator eventAggregator,
IOptions<SecuritySettings> securitySettings)
: base(userManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation, securitySettings)
IOptions<SecuritySettings> securitySettings,
IRequestCache requestCache)
: base(userManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation, securitySettings, requestCache)
{
_userManager = userManager;
_externalLogins = externalLogins;
@@ -46,6 +48,35 @@ public class BackOfficeSignInManager : UmbracoSignInManager<BackOfficeIdentityUs
_globalSettings = globalSettings.Value;
}
[Obsolete("Use non-obsolete constructor. This is scheduled for removal in V15.")]
public BackOfficeSignInManager(
BackOfficeUserManager userManager,
IHttpContextAccessor contextAccessor,
IBackOfficeExternalLoginProviders externalLogins,
IUserClaimsPrincipalFactory<BackOfficeIdentityUser> claimsFactory,
IOptions<IdentityOptions> optionsAccessor,
IOptions<GlobalSettings> globalSettings,
ILogger<SignInManager<BackOfficeIdentityUser>> logger,
IAuthenticationSchemeProvider schemes,
IUserConfirmation<BackOfficeIdentityUser> confirmation,
IEventAggregator eventAggregator,
IOptions<SecuritySettings> securitySettings)
: this(
userManager,
contextAccessor,
externalLogins,
claimsFactory,
optionsAccessor,
globalSettings,
logger,
schemes,
confirmation,
eventAggregator,
securitySettings,
StaticServiceProvider.Instance.GetRequiredService<IRequestCache>())
{
}
[Obsolete("Use non-obsolete constructor. This is scheduled for removal in V14.")]
public BackOfficeSignInManager(
BackOfficeUserManager userManager,

33
src/Umbraco.Web.Common/Security/MemberSignInManager.cs
View File

@@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Identity;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Events;
@@ -32,13 +33,41 @@ public class MemberSignInManager : UmbracoSignInManager<MemberIdentityUser>, IMe
IUserConfirmation<MemberIdentityUser> confirmation,
IMemberExternalLoginProviders memberExternalLoginProviders,
IEventAggregator eventAggregator,
IOptions<SecuritySettings> securitySettings)
: base(memberManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation, securitySettings)
IOptions<SecuritySettings> securitySettings,
IRequestCache requestCache)
: base(memberManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation, securitySettings, requestCache)
{
_memberExternalLoginProviders = memberExternalLoginProviders;
_eventAggregator = eventAggregator;
}
[Obsolete("Use non-obsolete constructor. This is scheduled for removal in V15.")]
public MemberSignInManager(
UserManager<MemberIdentityUser> memberManager,
IHttpContextAccessor contextAccessor,
IUserClaimsPrincipalFactory<MemberIdentityUser> claimsFactory,
IOptions<IdentityOptions> optionsAccessor,
ILogger<SignInManager<MemberIdentityUser>> logger,
IAuthenticationSchemeProvider schemes,
IUserConfirmation<MemberIdentityUser> confirmation,
IMemberExternalLoginProviders memberExternalLoginProviders,
IEventAggregator eventAggregator,
IOptions<SecuritySettings> securitySettings)
: this(
memberManager,
contextAccessor,
claimsFactory,
optionsAccessor,
logger,
schemes,
confirmation,
memberExternalLoginProviders,
eventAggregator,
securitySettings,
StaticServiceProvider.Instance.GetRequiredService<IRequestCache>())
{
}
[Obsolete("Use non-obsolete constructor. This is scheduled for removal in V14.")]
public MemberSignInManager(
UserManager<MemberIdentityUser> memberManager,

40
src/Umbraco.Web.Common/Security/UmbracoSignInManager.cs
View File

@@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Identity;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Security;
@@ -19,6 +20,7 @@ namespace Umbraco.Cms.Web.Common.Security;
public abstract class UmbracoSignInManager<TUser> : SignInManager<TUser>
where TUser : UmbracoIdentityUser
{
private readonly IRequestCache _requestCache;
private SecuritySettings _securitySettings;
// borrowed from https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Core/src/SignInManager.cs
@@ -44,7 +46,31 @@ public abstract class UmbracoSignInManager<TUser> : SignInManager<TUser>
logger,
schemes,
confirmation,
StaticServiceProvider.Instance.GetRequiredService<IOptions<SecuritySettings>>())
StaticServiceProvider.Instance.GetRequiredService<IOptions<SecuritySettings>>(),
StaticServiceProvider.Instance.GetRequiredService<IRequestCache>())
{
}
[Obsolete("Use non-obsolete constructor. This is scheduled for removal in V15.")]
public UmbracoSignInManager(
UserManager<TUser> userManager,
IHttpContextAccessor contextAccessor,
IUserClaimsPrincipalFactory<TUser> claimsFactory,
IOptions<IdentityOptions> optionsAccessor,
ILogger<SignInManager<TUser>> logger,
IAuthenticationSchemeProvider schemes,
IUserConfirmation<TUser> confirmation,
IOptions<SecuritySettings> securitySettingsOptions)
: this(
userManager,
contextAccessor,
claimsFactory,
optionsAccessor,
logger,
schemes,
confirmation,
securitySettingsOptions,
StaticServiceProvider.Instance.GetRequiredService<IRequestCache>())
{
}
@@ -56,9 +82,11 @@ public abstract class UmbracoSignInManager<TUser> : SignInManager<TUser>
ILogger<SignInManager<TUser>> logger,
IAuthenticationSchemeProvider schemes,
IUserConfirmation<TUser> confirmation,
IOptions<SecuritySettings> securitySettingsOptions)
IOptions<SecuritySettings> securitySettingsOptions,
IRequestCache requestCache)
: base(userManager, contextAccessor, claimsFactory, optionsAccessor, logger, schemes, confirmation)
{
_requestCache = requestCache;
_securitySettings = securitySettingsOptions.Value;
}
@@ -370,7 +398,13 @@ public abstract class UmbracoSignInManager<TUser> : SignInManager<TUser>
if (_securitySettings.AllowConcurrentLogins is false)
{
await UserManager.UpdateSecurityStampAsync(user);
if (_requestCache.Get("SecurityStampUpdated") is null)
{
await UserManager.UpdateSecurityStampAsync(user);
_requestCache.Set("SecurityStampUpdated", true);
}
}
Logger.LogInformation("User: {UserName} logged in from IP address {IpAddress}", username, Context.Connection.RemoteIpAddress);

4
tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Security/MemberSignInManagerTests.cs
View File

@@ -10,6 +10,7 @@ using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Net;
@@ -71,7 +72,8 @@ public class MemberSignInManagerTests
Mock.Of<IUserConfirmation<MemberIdentityUser>>(),
Mock.Of<IMemberExternalLoginProviders>(),
Mock.Of<IEventAggregator>(),
Mock.Of<IOptions<SecuritySettings>>(x => x.Value == new SecuritySettings()));
Mock.Of<IOptions<SecuritySettings>>(x => x.Value == new SecuritySettings()),
new DictionaryAppCache());
}
private static Mock<MemberManager> MockMemberManager()