From c5c64f2b54bfeb3f49d07fbca827b4719db6f95c Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Fri, 21 Feb 2025 09:48:34 +0100 Subject: [PATCH] Ported changes from Umbraco 13 around not throwing exceptions with identity IDs that can't be parsed to an integer or GUID (#18389) * Ported changes from Umbraco 13 around not throwing exceptions with identity IDs that can't be parsed to an integer or GUID. * Revert async changes (better done systemically in a separate update). --- .../Security/BackOfficeUserStore.cs | 21 +++++++----- .../Security/MemberUserStore.cs | 32 ++++++++++++++----- .../Security/UmbracoUserStore.cs | 17 ++-------- 3 files changed, 40 insertions(+), 30 deletions(-) diff --git a/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs b/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs index 03d1c35c56..588aed7f63 100644 --- a/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs @@ -1,5 +1,6 @@ using System.Data; using System.Data.Common; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; @@ -436,8 +437,7 @@ public class BackOfficeUserStore : throw new ArgumentNullException(nameof(user)); } - IUser? found = FindUserFromString(user.Id); - if (found is not null) + if (TryFindUserFromString(user.Id, out IUser? found)) { DisableAsync(found).GetAwaiter().GetResult(); } @@ -469,8 +469,7 @@ public class BackOfficeUserStore : cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); - IUser? user = FindUserFromString(userId); - if (user == null) + if (TryFindUserFromString(userId, out IUser? user) is false) { return Task.FromResult((BackOfficeIdentityUser?)null)!; } @@ -478,22 +477,28 @@ public class BackOfficeUserStore : return Task.FromResult(AssignLoginsCallback(_mapper.Map(user)))!; } - private IUser? FindUserFromString(string userId) + private bool TryFindUserFromString(string userId, [NotNullWhen(true)] out IUser? user) { // We could use ResolveEntityIdFromIdentityId here, but that would require multiple DB calls, so let's not. if (TryConvertIdentityIdToInt(userId, out var id)) { - return GetAsync(id).GetAwaiter().GetResult(); + user = GetAsync(id).GetAwaiter().GetResult(); + return user is not null; } // We couldn't directly convert the ID to an int, this is because the user logged in with external login. // So we need to look up the user by key. if (Guid.TryParse(userId, out Guid key)) { - return GetAsync(key).GetAwaiter().GetResult(); + user = GetAsync(key).GetAwaiter().GetResult(); + return user is not null; } - throw new InvalidOperationException($"Unable to resolve user with ID {userId}"); + // Maybe we have some other format of user Id from an external login flow, so don't throw but return null. + // We won't be able to find the user via this ID in a local database lookup so we'll handle the same as if they don't exist. + + user = null; + return false; } protected override async Task ResolveEntityIdFromIdentityId(string? identityId) diff --git a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs index 929ab55a3d..21c4207224 100644 --- a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs @@ -2,7 +2,6 @@ using System.Data; using System.Globalization; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; -using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Mapping; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.PublishedContent; @@ -322,9 +321,15 @@ public class MemberUserStore : UmbracoUserStore(user)))!; } - protected override Task ResolveEntityIdFromIdentityId(string? identityId) + private bool TryResolveEntityIdFromIdentityId(string? identityId, out int entityId) { - if (TryConvertIdentityIdToInt(identityId, out var id)) + if (TryConvertIdentityIdToInt(identityId, out entityId)) { - return Task.FromResult(id); + return true; } if (Guid.TryParse(identityId, out Guid key)) @@ -346,10 +351,21 @@ public class MemberUserStore : UmbracoUserStore ResolveEntityIdFromIdentityId(string? identityId) + { + if (TryResolveEntityIdFromIdentityId(identityId, out var entityId)) + { + return Task.FromResult(entityId); + } + throw new InvalidOperationException($"Unable to resolve user with ID {identityId}"); } diff --git a/src/Umbraco.Infrastructure/Security/UmbracoUserStore.cs b/src/Umbraco.Infrastructure/Security/UmbracoUserStore.cs index 51bda8a863..6b015bda58 100644 --- a/src/Umbraco.Infrastructure/Security/UmbracoUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/UmbracoUserStore.cs @@ -35,29 +35,18 @@ public abstract class UmbracoUserStore [Obsolete("Use TryConvertIdentityIdToInt instead. Scheduled for removal in V15.")] protected static int UserIdToInt(string? userId) { - if (TryUserIdToInt(userId, out int result)) + if (int.TryParse(userId, NumberStyles.Integer, CultureInfo.InvariantCulture, out var result)) { return result; } - throw new InvalidOperationException($"Unable to convert user ID ({userId})to int using InvariantCulture"); - } - - protected static bool TryUserIdToInt(string? userId, out int result) - { - if (int.TryParse(userId, NumberStyles.Integer, CultureInfo.InvariantCulture, out result)) - { - return true; - } - if (Guid.TryParse(userId, out Guid key)) { // Reverse the IntExtensions.ToGuid - result = BitConverter.ToInt32(key.ToByteArray(), 0); - return true; + return BitConverter.ToInt32(key.ToByteArray(), 0); } - return false; + throw new InvalidOperationException($"Unable to convert user ID ({userId})to int using InvariantCulture"); } protected abstract Task ResolveEntityIdFromIdentityId(string? identityId);