Skip to content
4 changes: 2 additions & 2 deletions src/Umbraco.Core/Configuration/Models/SecuritySettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ public class SecuritySettings

internal const int StaticMemberDefaultLockoutTimeInMinutes = 30 * 24 * 60;
internal const int StaticUserDefaultLockoutTimeInMinutes = 30 * 24 * 60;
private const long StaticUserDefaultFailedLoginDurationInMilliseconds = 1000;
private const long StaticUserMinimumFailedLoginDurationInMilliseconds = 250;
internal const long StaticUserDefaultFailedLoginDurationInMilliseconds = 1000;
internal const long StaticUserMinimumFailedLoginDurationInMilliseconds = 250;

/// <summary>
/// Gets or sets a value indicating whether to keep the user logged in.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,29 @@
namespace Umbraco.Cms.Core.Persistence.Repositories;

/// <summary>
/// Repository for external logins with Guid as key, so it can be shared for members and users
/// Repository for external logins with Guid as key, so it can be shared for members and users.
/// </summary>
public interface IExternalLoginWithKeyRepository : IReadWriteQueryRepository<int, IIdentityUserLogin>,
IQueryRepository<IIdentityUserToken>
{
/// <summary>
/// Replaces all external login providers for the user/member key
/// Replaces all external login providers for the user/member key.
/// </summary>
void Save(Guid userOrMemberKey, IEnumerable<IExternalLogin> logins);

/// <summary>
/// Replaces all external login provider tokens for the providers specified for the user/member key
/// Replaces all external login provider tokens for the providers specified for the user/member key.
/// </summary>
void Save(Guid userOrMemberKey, IEnumerable<IExternalLoginToken> tokens);

/// <summary>
/// Deletes all external logins for the specified the user/member key
/// Deletes all external logins for the specified the user/member key.
/// </summary>
void DeleteUserLogins(Guid userOrMemberKey);

/// <summary>
/// Deletes external logins that aren't associated with the current collection of providers.
/// </summary>
/// <param name="currentLoginProviders">The names of the currently configured providers.</param>
void DeleteUserLoginsForRemovedProviders(IEnumerable<string> currentLoginProviders) { }
}
8 changes: 7 additions & 1 deletion src/Umbraco.Core/Persistence/Repositories/IUserRepository.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Linq.Expressions;
using System.Linq.Expressions;
using Umbraco.Cms.Core.Models.Membership;
using Umbraco.Cms.Core.Persistence.Querying;

Expand Down Expand Up @@ -110,4 +110,10 @@ IEnumerable<IUser> GetPagedResultsByQuery(
void ClearLoginSession(Guid sessionId);

IEnumerable<IUser> GetNextUsers(int id, int count);

/// <summary>
/// Invalidates sessions for users that aren't associated with the current collection of providers.
/// </summary>
/// <param name="currentProviderKeys">The keys for the currently configured providers.</param>
void InvalidateSessionsForRemovedProviders(IEnumerable<string> currentProviderKeys) { }
}
10 changes: 10 additions & 0 deletions src/Umbraco.Core/Services/ExternalLoginService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,14 @@ public void DeleteUserLogins(Guid userOrMemberKey)
scope.Complete();
}
}

/// <inheritdoc />
public void DeleteUserLoginsForRemovedProviders(IEnumerable<string> currentLoginProviders)
{
using (ICoreScope scope = ScopeProvider.CreateCoreScope())
{
_externalLoginRepository.DeleteUserLoginsForRemovedProviders(currentLoginProviders);
scope.Complete();
}
}
}
26 changes: 16 additions & 10 deletions src/Umbraco.Core/Services/IExternalLoginWithKeyService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,47 +5,53 @@ namespace Umbraco.Cms.Core.Services;
public interface IExternalLoginWithKeyService : IService
{
/// <summary>
/// Returns all user logins assigned
/// Returns all user logins assigned.
/// </summary>
IEnumerable<IIdentityUserLogin> GetExternalLogins(Guid userOrMemberKey);

/// <summary>
/// Returns all user login tokens assigned
/// Returns all user login tokens assigned.
/// </summary>
IEnumerable<IIdentityUserToken> GetExternalLoginTokens(Guid userOrMemberKey);

/// <summary>
/// Returns all logins matching the login info - generally there should only be one but in some cases
/// there might be more than one depending on if an administrator has been editing/removing members
/// there might be more than one depending on if an administrator has been editing/removing members.
/// </summary>
IEnumerable<IIdentityUserLogin> Find(string loginProvider, string providerKey);

/// <summary>
/// Saves the external logins associated with the user
/// Saves the external logins associated with the user.
/// </summary>
/// <param name="userOrMemberKey">
/// The user or member key associated with the logins
/// The user or member key associated with the logins.
/// </param>
/// <param name="logins"></param>
/// <remarks>
/// This will replace all external login provider information for the user
/// This will replace all external login provider information for the user.
/// </remarks>
void Save(Guid userOrMemberKey, IEnumerable<IExternalLogin> logins);

/// <summary>
/// Saves the external login tokens associated with the user
/// Saves the external login tokens associated with the user.
/// </summary>
/// <param name="userOrMemberKey">
/// The user or member key associated with the logins
/// The user or member key associated with the logins.
/// </param>
/// <param name="tokens"></param>
/// <remarks>
/// This will replace all external login tokens for the user
/// This will replace all external login tokens for the user.
/// </remarks>
void Save(Guid userOrMemberKey, IEnumerable<IExternalLoginToken> tokens);

/// <summary>
/// Deletes all user logins - normally used when a member is deleted
/// Deletes all user logins - normally used when a member is deleted.
/// </summary>
void DeleteUserLogins(Guid userOrMemberKey);

/// <summary>
/// Deletes external logins that aren't associated with the current collection of providers.
/// </summary>
/// <param name="currentLoginProviders">The names of the currently configured providers.</param>
void DeleteUserLoginsForRemovedProviders(IEnumerable<string> currentLoginProviders) { }
}
6 changes: 6 additions & 0 deletions src/Umbraco.Core/Services/IUserService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,12 @@ IEnumerable<IUser> GetAll(

IEnumerable<IUser> GetNextUsers(int id, int count);

/// <summary>
/// Invalidates sessions for users that aren't associated with the current collection of providers.
/// </summary>
/// <param name="currentProviderKeys">The keys for the currently configured providers.</param>
void InvalidateSessionsForRemovedProviders(IEnumerable<string> currentProviderKeys) { }

#region User groups

/// <summary>
Expand Down
10 changes: 10 additions & 0 deletions src/Umbraco.Core/Services/UserService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Data.Common;

Check notice on line 1 in src/Umbraco.Core/Services/UserService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v13/dev)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 68.00% to 67.46%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using System.Globalization;
using System.Linq.Expressions;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -720,6 +720,16 @@
}
}

/// <inheritdoc />
public void InvalidateSessionsForRemovedProviders(IEnumerable<string> currentProviderKeys)
{
using (ICoreScope scope = ScopeProvider.CreateCoreScope())
{
_userRepository.InvalidateSessionsForRemovedProviders(currentProviderKeys);
scope.Complete();
}
}

/// <summary>
/// Gets a list of <see cref="IUser" /> objects associated with a given group
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ public int Count(IQuery<IIdentityUserToken> query)
public void DeleteUserLogins(Guid userOrMemberKey) =>
Database.Delete<ExternalLoginDto>("WHERE userOrMemberKey=@userOrMemberKey", new { userOrMemberKey });

/// <inheritdoc />
public void DeleteUserLoginsForRemovedProviders(IEnumerable<string> currentLoginProviders) =>
Database.Execute(Sql()
.Delete<ExternalLoginDto>()
.WhereNotIn<ExternalLoginDto>(x => x.LoginProvider, currentLoginProviders));

/// <inheritdoc />
public void Save(Guid userOrMemberKey, IEnumerable<IExternalLogin> logins)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Linq.Expressions;

Check notice on line 1 in src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v13/dev)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 53.52% to 53.42%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using System.Reflection;
using System.Text;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -1070,5 +1070,45 @@
: GetMany(ids).OrderBy(x => x.Id) ?? Enumerable.Empty<IUser>();
}

/// <inheritdoc />
public void InvalidateSessionsForRemovedProviders(IEnumerable<string> currentProviderKeys)
{
// Get all the user or member keys associated with the removed providers.
Sql<ISqlContext> idsQuery = SqlContext.Sql()
.Select<ExternalLoginDto>(x => x.UserOrMemberKey)
.From<ExternalLoginDto>()
.WhereNotIn<ExternalLoginDto>(x => x.ProviderKey, currentProviderKeys);
List<Guid> userAndMemberKeysAssociatedWithRemovedProviders = Database.Fetch<Guid>(idsQuery);

// Filter for actual users and convert to integer IDs.
var userIdsAssociatedWithRemovedProviders = userAndMemberKeysAssociatedWithRemovedProviders
.Select(ConvertUserKeyToUserId)
.Where(x => x.HasValue)
.Select(x => x!.Value)
.ToArray();

// Invalidate the security stamps on the users associated with the removed providers.
Sql<ISqlContext> updateQuery = Sql()
.Update<UserDto>(u => u.Set(x => x.SecurityStampToken, "0".PadLeft(32, '0')))
.WhereIn<UserDto>(x => x.Id, userIdsAssociatedWithRemovedProviders);
Database.Execute(updateQuery);
}

// Marked as internal to expose for unit testing.
internal static int? ConvertUserKeyToUserId(Guid userOrMemberKey)
{
// User Ids are stored as integers in the umbracoUser table, but as a GUID representation
// of that integer in umbracoExternalLogin (converted via IntExtensions.ToGuid()).
// We need to parse that to get the user Ids to invalidate.
// Note also that umbracoExternalLogin contains members too, as proper GUIDs, so we need to ignore them.
if (userOrMemberKey.ToString().EndsWith("-0000-0000-0000-000000000000") is false)
{
// We have a proper GUID, not an integer conversion, hence a member, not a user.
return null;
}

return BitConverter.ToInt32(userOrMemberKey.ToByteArray());
}

#endregion
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Umbraco.Cms.Core.Security;
using Umbraco.Cms.Web.BackOffice.Authorization;
using Umbraco.Cms.Web.BackOffice.Middleware;
using Umbraco.Cms.Web.BackOffice.NotificationHandlers;
using Umbraco.Cms.Web.BackOffice.Security;
using Umbraco.Cms.Web.Common.Authorization;
using Umbraco.Cms.Web.Common.Security;
Expand Down Expand Up @@ -65,6 +66,8 @@ public static IUmbracoBuilder AddBackOfficeAuthentication(this IUmbracoBuilder b
builder.AddNotificationHandler<UserPasswordChangedNotification, BackOfficeUserManagerAuditer>();
builder.AddNotificationHandler<UserPasswordResetNotification, BackOfficeUserManagerAuditer>();

builder.AddNotificationHandler<UmbracoApplicationStartingNotification, ExternalLoginProviderStartupHandler>();

return builder;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Web.BackOffice.Security;

namespace Umbraco.Cms.Web.BackOffice.NotificationHandlers;

/// <summary>
/// Invalidates backoffice sessions and clears external logins for removed providers if the external login
/// provider setup has changed.
/// </summary>
public class ExternalLoginProviderStartupHandler : INotificationHandler<UmbracoApplicationStartingNotification>
{
private readonly IBackOfficeExternalLoginProviders _backOfficeExternalLoginProviders;
private readonly IRuntimeState _runtimeState;

public ExternalLoginProviderStartupHandler(
IBackOfficeExternalLoginProviders backOfficeExternalLoginProviders,
IRuntimeState runtimeState)
{
_backOfficeExternalLoginProviders = backOfficeExternalLoginProviders;
_runtimeState = runtimeState;
}

public void Handle(UmbracoApplicationStartingNotification notification)
{
if (_runtimeState.Level == RuntimeLevel.Run)
{
_backOfficeExternalLoginProviders.InvalidateSessionsIfExternalLoginProvidersChanged();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
using Microsoft.AspNetCore.Authentication;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Services;

namespace Umbraco.Cms.Web.BackOffice.Security;

Expand All @@ -7,13 +11,41 @@
{
private readonly IAuthenticationSchemeProvider _authenticationSchemeProvider;
private readonly Dictionary<string, BackOfficeExternalLoginProvider> _externalLogins;
private readonly IKeyValueService _keyValueService;
private readonly IExternalLoginWithKeyService _externalLoginWithKeyService;
private readonly IUserService _userService;
private readonly ILogger<BackOfficeExternalLoginProviders> _logger;

private const string ExternalLoginProvidersKey = "Umbraco.Cms.Web.BackOffice.Security.BackOfficeExternalLoginProviders";

[Obsolete("Please use the constructor taking all parameters. Scheduled for removal in Umbraco 17.")]
public BackOfficeExternalLoginProviders(
IEnumerable<BackOfficeExternalLoginProvider> externalLogins,
IAuthenticationSchemeProvider authenticationSchemeProvider)
: this(
externalLogins,
authenticationSchemeProvider,
StaticServiceProvider.Instance.GetRequiredService<IKeyValueService>(),
StaticServiceProvider.Instance.GetRequiredService<IExternalLoginWithKeyService>(),
StaticServiceProvider.Instance.GetRequiredService<IUserService>(),
StaticServiceProvider.Instance.GetRequiredService<ILogger<BackOfficeExternalLoginProviders>>())
{
}

public BackOfficeExternalLoginProviders(
IEnumerable<BackOfficeExternalLoginProvider> externalLogins,
IAuthenticationSchemeProvider authenticationSchemeProvider,
IKeyValueService keyValueService,
IExternalLoginWithKeyService externalLoginWithKeyService,
IUserService userService,
ILogger<BackOfficeExternalLoginProviders> logger)
{
_externalLogins = externalLogins.ToDictionary(x => x.AuthenticationType);
_authenticationSchemeProvider = authenticationSchemeProvider;
_keyValueService = keyValueService;
_externalLoginWithKeyService = externalLoginWithKeyService;
_userService = userService;
_logger = logger;

Check warning on line 48 in src/Umbraco.Web.BackOffice/Security/BackOfficeExternalLoginProviders.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v13/dev)

❌ New issue: Constructor Over-Injection

BackOfficeExternalLoginProviders has 6 arguments, threshold = 5. This constructor has too many arguments, indicating an object with low cohesion or missing function argument abstraction. Avoid adding more arguments.
}

/// <inheritdoc />
Expand Down Expand Up @@ -66,4 +98,26 @@
var found = _externalLogins.Values.Where(x => x.Options.DenyLocalLogin).ToList();
return found.Count > 0;
}

/// <inheritdoc />
public void InvalidateSessionsIfExternalLoginProvidersChanged()
{
var previousExternalLoginProvidersValue = _keyValueService.GetValue(ExternalLoginProvidersKey);
var currentExternalLoginProvidersValue = string.Join("|", _externalLogins.Keys);

if ((previousExternalLoginProvidersValue ?? string.Empty) != currentExternalLoginProvidersValue)
{
_logger.LogWarning(
"The configured external login providers have changed. Existing backoffice sessions using the removed providers will be invalidated and external login data removed.");

_userService.InvalidateSessionsForRemovedProviders(_externalLogins.Keys);
_externalLoginWithKeyService.DeleteUserLoginsForRemovedProviders(_externalLogins.Keys);

_keyValueService.SetValue(ExternalLoginProvidersKey, currentExternalLoginProvidersValue);
}
else if (previousExternalLoginProvidersValue is null)
{
_keyValueService.SetValue(ExternalLoginProvidersKey, string.Empty);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,11 @@ public interface IBackOfficeExternalLoginProviders
/// </summary>
/// <returns></returns>
bool HasDenyLocalLogin();

/// <summary>
/// Used during startup to see if the configured external login providers is different from the persisted information.
/// If they are different, this will invalidate backoffice sessions and clear external logins for removed providers
/// if the external login provider setup has changed.
/// </summary>
void InvalidateSessionsIfExternalLoginProvidersChanged() { }
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public static void ConfigureOptions(SecurityStampValidatorOptions options, Secur
// Adjust the security stamp validation interval to a shorter duration
// when concurrent logins are not allowed and the duration has the default interval value
// (currently defaults to 30 minutes), ensuring quicker re-validation.
if (securitySettings.AllowConcurrentLogins is false && options.ValidationInterval == TimeSpan.FromMinutes(30))
if (securitySettings.AllowConcurrentLogins is false && options.ValidationInterval == new SecurityStampValidatorOptions().ValidationInterval)
{
options.ValidationInterval = TimeSpan.FromSeconds(30);
}
Expand Down
Loading
Loading