Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Data;

Check notice on line 1 in src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs

View check run for this annotation

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

✅ Getting better: Code Duplication

reduced similar code in: FindByNameAsync,FindUserAsync. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

Check notice on line 1 in src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs

View check run for this annotation

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

ℹ Getting worse: Overall Code Complexity

The mean cyclomatic complexity increases from 5.04 to 5.08, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
using System.Globalization;
using Microsoft.AspNetCore.Identity;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -281,7 +281,20 @@
cancellationToken.ThrowIfCancellationRequested();
ThrowIfDisposed();

IUser? user = _userService.GetUserById(UserIdToInt(userId));
// In the external login flow - see BackOfficeController.ExternalSignInAsync - we can have a situation where an
// error has occured but the user is signed in. For that reason, at the end of the process, if errors are
// recorded, the user is signed out.
// Before signing out, we request the user in order to update the security stamp - see UmbracoSignInManager.SignOutAsync.
// But we can have a situation where the signed in principal has the ID from the external provider, which may not be something
// we can parse to an integer.
// If that's the case, return null rather than throwing an exception. Without an Umbraco user, we can't update the security stamp,
// so no need to fail here.
if (!TryUserIdToInt(userId, out var userIdAsInt))
{
return Task.FromResult((BackOfficeIdentityUser?)null)!;
}

IUser? user = _userService.GetUserById(userIdAsInt);
if (user == null)
{
return Task.FromResult((BackOfficeIdentityUser?)null)!;
Expand Down
17 changes: 14 additions & 3 deletions src/Umbraco.Infrastructure/Security/UmbracoUserStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,29 @@ protected UmbracoUserStore(IdentityErrorDescriber describer)

protected static int UserIdToInt(string? userId)
{
if (int.TryParse(userId, NumberStyles.Integer, CultureInfo.InvariantCulture, out var result))
if (TryUserIdToInt(userId, out int 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
return BitConverter.ToInt32(key.ToByteArray(), 0);
result = BitConverter.ToInt32(key.ToByteArray(), 0);
return true;
}

throw new InvalidOperationException($"Unable to convert user ID ({userId})to int using InvariantCulture");
return false;
}

protected static string UserIdToString(int userId) => string.Intern(userId.ToString(CultureInfo.InvariantCulture));
Expand Down
Loading