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

Cleans up IdentityAuditEventArgs and handling of the current user since we cannot resolve it from the thread.

Browse Source
This commit is contained in:
Shannon
2020-05-21 16:33:24 +10:00
parent cf58782093
commit d89337e3d5
7 changed files with 57 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs
View File

@@ -35,6 +35,7 @@ namespace Umbraco.Extensions
}
catch (InvalidOperationException)
{
// TODO: Look into this? Why did we do this, see git history and add some notes
}
}

57
src/Umbraco.Core/BackOffice/IdentityAuditEventArgs.cs
View File

@@ -41,7 +41,7 @@ namespace Umbraco.Core.BackOffice
/// <summary>
/// This property is always empty except in the LoginFailed event for an unknown user trying to login
/// </summary>
public string Username { get; private set; }
public string AffectedUsername { get; private set; }
/// <summary>
@@ -52,65 +52,22 @@ namespace Umbraco.Core.BackOffice
/// <param name="comment"></param>
/// <param name="performingUser"></param>
/// <param name="affectedUser"></param>
public IdentityAuditEventArgs(AuditEvent action, string ipAddress, string comment = null, int performingUser = -1, int affectedUser = -1)
public IdentityAuditEventArgs(AuditEvent action, string ipAddress, int performingUser, string comment, int affectedUser, string affectedUsername)
{
DateTimeUtc = DateTime.UtcNow;
Action = action;
IpAddress = ipAddress;
Comment = comment;
Comment = comment;
PerformingUser = performingUser;
AffectedUsername = affectedUsername;
AffectedUser = affectedUser;
PerformingUser = performingUser == -1
? GetCurrentRequestBackofficeUserId()
: performingUser;
}
/// <summary>
/// Creates an instance without a performing or affected user (the id will be set to -1)
/// </summary>
/// <param name="action"></param>
/// <param name="ipAddress"></param>
/// <param name="username"></param>
/// <param name="comment"></param>
public IdentityAuditEventArgs(AuditEvent action, string ipAddress, string username, string comment)
public IdentityAuditEventArgs(AuditEvent action, string ipAddress, int performingUser, string comment, string affectedUsername)
: this(action, ipAddress, performingUser, comment, -1, affectedUsername)
{
DateTimeUtc = DateTime.UtcNow;
Action = action;
IpAddress = ipAddress;
Username = username;
Comment = comment;
PerformingUser = -1;
}
public IdentityAuditEventArgs(AuditEvent action, string ipAddress, string username, string comment, int performingUser)
{
DateTimeUtc = DateTime.UtcNow;
Action = action;
IpAddress = ipAddress;
Username = username;
Comment = comment;
PerformingUser = performingUser == -1
? GetCurrentRequestBackofficeUserId()
: performingUser;
}
/// <summary>
/// Returns the current logged in backoffice user's Id logging if there is one
/// </summary>
/// <returns></returns>
protected int GetCurrentRequestBackofficeUserId()
{
var userId = -1;
/*var backOfficeIdentity = Thread.CurrentPrincipal.GetUmbracoIdentity();
if (backOfficeIdentity != null)
int.TryParse(backOfficeIdentity.Id.ToString(), out userId);*/
return userId;
}
}
public enum AuditEvent

7
src/Umbraco.Core/ClaimsIdentityExtensions.cs
View File

@@ -6,6 +6,13 @@ namespace Umbraco.Core
{
public static class ClaimsIdentityExtensions
{
public static T GetUserId<T>(this IIdentity identity)
{
var strId = identity.GetUserId();
var converted = strId.TryConvertTo<T>();
return converted.ResultOr(default);
}
/// <summary>
/// Returns the user id from the <see cref="IIdentity"/> of either the claim type <see cref="ClaimTypes.NameIdentifier"/> or "sub"
/// </summary>

81
src/Umbraco.Infrastructure/BackOffice/BackOfficeUserManager.cs
View File

@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Security.Principal;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Identity;
@@ -7,6 +8,7 @@ using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Umbraco.Core.Configuration;
using Umbraco.Core.Security;
using Umbraco.Extensions;
using Umbraco.Net;
namespace Umbraco.Core.BackOffice
@@ -201,14 +203,16 @@ namespace Umbraco.Core.BackOffice
if (user == null) throw new InvalidOperationException("Could not find user");
var result = await base.ResetPasswordAsync(user, token, newPassword);
if (result.Succeeded) RaisePasswordChangedEvent(userId);
if (result.Succeeded)
RaisePasswordChangedEvent(null, userId); // TODO: How can we get the current user? we have not HttpContext (netstandard), we can make our own IPrincipalAccessor?
return result;
}
public override async Task<IdentityResult> ChangePasswordAsync(T user, string currentPassword, string newPassword)
{
var result = await base.ChangePasswordAsync(user, currentPassword, newPassword);
if (result.Succeeded) RaisePasswordChangedEvent(user.Id);
if (result.Succeeded)
RaisePasswordChangedEvent(null, user.Id); // TODO: How can we get the current user? we have not HttpContext (netstandard), we can make our own IPrincipalAccessor?
return result;
}
@@ -286,11 +290,11 @@ namespace Umbraco.Core.BackOffice
// The way we unlock is by setting the lockoutEnd date to the current datetime
if (result.Succeeded && lockoutEnd >= DateTimeOffset.UtcNow)
{
RaiseAccountLockedEvent(user.Id);
RaiseAccountLockedEvent(null, user.Id); // TODO: How can we get the current user? we have not HttpContext (netstandard), we can make our own IPrincipalAccessor?
}
else
{
RaiseAccountUnlockedEvent(user.Id);
RaiseAccountUnlockedEvent(null, user.Id); // TODO: How can we get the current user? we have not HttpContext (netstandard), we can make our own IPrincipalAccessor?
//Resets the login attempt fails back to 0 when unlock is clicked
await ResetAccessFailedCountAsync(user);
}
@@ -310,7 +314,7 @@ namespace Umbraco.Core.BackOffice
await lockoutStore.ResetAccessFailedCountAsync(user, CancellationToken.None);
//raise the event now that it's reset
RaiseResetAccessFailedCountEvent(user.Id);
RaiseResetAccessFailedCountEvent(null, user.Id); // TODO: How can we get the current user? we have not HttpContext (netstandard), we can make our own IPrincipalAccessor?
return await UpdateAsync(user);
}
@@ -344,65 +348,44 @@ namespace Umbraco.Core.BackOffice
var result = await UpdateAsync(user);
//Slightly confusing: this will return a Success if we successfully update the AccessFailed count
if (result.Succeeded) RaiseLoginFailedEvent(user.Id);
if (result.Succeeded)
{
// TODO: This may no longer be the case in netcore, we'll need to see about that
RaiseLoginFailedEvent(null, user.Id); // TODO: How can we get the current user? we have not HttpContext (netstandard), we can make our own IPrincipalAccessor?
}
return result;
}
public void RaiseAccountLockedEvent(int userId)
private IdentityAuditEventArgs CreateArgs(AuditEvent auditEvent, IPrincipal currentUser, int affectedUserId, string affectedUsername)
{
OnAccountLocked(new IdentityAuditEventArgs(AuditEvent.AccountLocked, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId));
}
var umbIdentity = currentUser?.GetUmbracoIdentity();
var currentUserId = umbIdentity?.GetUserId<int?>() ?? Constants.Security.SuperUserId;
var ip = IpResolver.GetCurrentRequestIpAddress();
return new IdentityAuditEventArgs(auditEvent, ip, currentUserId, string.Empty, affectedUserId, affectedUsername);
}
public void RaiseAccountUnlockedEvent(int userId)
{
OnAccountUnlocked(new IdentityAuditEventArgs(AuditEvent.AccountUnlocked, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId));
}
public void RaiseAccountLockedEvent(IPrincipal currentUser, int userId) => OnAccountLocked(CreateArgs(AuditEvent.AccountLocked, currentUser, userId, string.Empty));
public void RaiseForgotPasswordRequestedEvent(int userId)
{
OnForgotPasswordRequested(new IdentityAuditEventArgs(AuditEvent.ForgotPasswordRequested, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId));
}
public void RaiseAccountUnlockedEvent(IPrincipal currentUser, int userId) => OnAccountUnlocked(CreateArgs(AuditEvent.AccountUnlocked, currentUser, userId, string.Empty));
public void RaiseForgotPasswordChangedSuccessEvent(int userId)
{
OnForgotPasswordChangedSuccess(new IdentityAuditEventArgs(AuditEvent.ForgotPasswordChangedSuccess, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId));
}
public void RaiseForgotPasswordRequestedEvent(IPrincipal currentUser, int userId) => OnForgotPasswordRequested(CreateArgs(AuditEvent.ForgotPasswordRequested, currentUser, userId, string.Empty));
public void RaiseLoginFailedEvent(int userId)
{
OnLoginFailed(new IdentityAuditEventArgs(AuditEvent.LoginFailed, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId));
}
public void RaiseForgotPasswordChangedSuccessEvent(IPrincipal currentUser, int userId) => OnForgotPasswordChangedSuccess(CreateArgs(AuditEvent.ForgotPasswordChangedSuccess, currentUser, userId, string.Empty));
public void RaiseInvalidLoginAttemptEvent(string username)
{
OnLoginFailed(new IdentityAuditEventArgs(AuditEvent.LoginFailed, IpResolver.GetCurrentRequestIpAddress(), username, string.Format("Attempted login for username '{0}' failed", username)));
}
public void RaiseLoginFailedEvent(IPrincipal currentUser, int userId) => OnLoginFailed(CreateArgs(AuditEvent.LoginFailed, currentUser, userId, string.Empty));
public void RaiseLoginRequiresVerificationEvent(int userId)
{
OnLoginRequiresVerification(new IdentityAuditEventArgs(AuditEvent.LoginRequiresVerification, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId));
}
public void RaiseInvalidLoginAttemptEvent(IPrincipal currentUser, string username) => OnLoginFailed(CreateArgs(AuditEvent.LoginFailed, currentUser, Constants.Security.SuperUserId, username));
public void RaiseLoginSuccessEvent(int userId)
{
OnLoginSuccess(new IdentityAuditEventArgs(AuditEvent.LoginSucces, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId));
}
public void RaiseLoginRequiresVerificationEvent(IPrincipal currentUser, int userId) => OnLoginRequiresVerification(CreateArgs(AuditEvent.LoginRequiresVerification, currentUser, userId, string.Empty));
public void RaiseLogoutSuccessEvent(int userId)
{
OnLogoutSuccess(new IdentityAuditEventArgs(AuditEvent.LogoutSuccess, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId));
}
public void RaiseLoginSuccessEvent(IPrincipal currentUser, int userId) => OnLoginSuccess(CreateArgs(AuditEvent.LoginSucces, currentUser, userId, string.Empty));
public void RaisePasswordChangedEvent(int userId)
{
OnPasswordChanged(new IdentityAuditEventArgs(AuditEvent.PasswordChanged, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId));
}
public void RaiseLogoutSuccessEvent(IPrincipal currentUser, int userId) => OnLogoutSuccess(CreateArgs(AuditEvent.LogoutSuccess, currentUser, userId, string.Empty));
public void RaiseResetAccessFailedCountEvent(int userId)
{
OnResetAccessFailedCount(new IdentityAuditEventArgs(AuditEvent.ResetAccessFailedCount, IpResolver.GetCurrentRequestIpAddress(), affectedUser: userId));
}
public void RaisePasswordChangedEvent(IPrincipal currentUser, int userId) => OnPasswordChanged(CreateArgs(AuditEvent.LogoutSuccess, currentUser, userId, string.Empty));
public void RaiseResetAccessFailedCountEvent(IPrincipal currentUser, int userId) => OnResetAccessFailedCount(CreateArgs(AuditEvent.ResetAccessFailedCount, currentUser, userId, string.Empty));
public static event EventHandler AccountLocked;
public static event EventHandler AccountUnlocked;

2
src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs
View File

@@ -12,7 +12,7 @@ namespace Umbraco.Web.BackOffice.Controllers
//[ValidationFilter] // TODO: I don't actually think this is required with our custom Application Model conventions applied
[TypeFilter(typeof(AngularJsonOnlyConfigurationAttribute))] // TODO: This could be applied with our Application Model conventions
[IsBackOffice] // TODO: This could be applied with our Application Model conventions
public class AuthenticationController : UmbracoApiController
public class AuthenticationController : ControllerBase
{
private readonly IUmbracoContextAccessor _umbracoContextAccessor;
// TODO: We need to import the logic from Umbraco.Web.Editors.AuthenticationController and it should not be an auto-routed api controller

14
src/Umbraco.Web/Editors/AuthenticationController.cs
View File

@@ -256,7 +256,7 @@ namespace Umbraco.Web.Editors
{
// get the user
var user = Services.UserService.GetByUsername(loginModel.Username);
UserManager.RaiseLoginSuccessEvent(user.Id);
UserManager.RaiseLoginSuccessEvent(User, user.Id);
return SetPrincipalAndReturnUserDetail(user, owinContext.Request.User);
}
@@ -294,7 +294,7 @@ namespace Umbraco.Web.Editors
userId = attemptedUser.Id
});
UserManager.RaiseLoginRequiresVerificationEvent(attemptedUser.Id);
UserManager.RaiseLoginRequiresVerificationEvent(User, attemptedUser.Id);
return verifyResponse;
}
@@ -348,7 +348,7 @@ namespace Umbraco.Web.Editors
await _emailSender.SendAsync(mailMessage);
UserManager.RaiseForgotPasswordRequestedEvent(user.Id);
UserManager.RaiseForgotPasswordRequestedEvent(User, user.Id);
}
}
@@ -417,13 +417,13 @@ namespace Umbraco.Web.Editors
var user = Services.UserService.GetByUsername(userName);
if (result.Succeeded)
{
UserManager.RaiseLoginSuccessEvent(user.Id);
UserManager.RaiseLoginSuccessEvent(User, user.Id);
return SetPrincipalAndReturnUserDetail(user, owinContext.Request.User);
}
if (result.IsLockedOut)
{
UserManager.RaiseAccountLockedEvent(user.Id);
UserManager.RaiseAccountLockedEvent(User, user.Id);
return Request.CreateValidationErrorResponse("User is locked out");
}
@@ -487,7 +487,7 @@ namespace Umbraco.Web.Editors
}
}
UserManager.RaiseForgotPasswordChangedSuccessEvent(model.UserId);
UserManager.RaiseForgotPasswordChangedSuccessEvent(User, model.UserId);
return Request.CreateResponse(HttpStatusCode.OK);
}
return Request.CreateValidationErrorResponse(
@@ -514,7 +514,7 @@ namespace Umbraco.Web.Editors
if (UserManager != null)
{
int.TryParse(User.Identity.GetUserId(), out var userId);
UserManager.RaiseLogoutSuccessEvent(userId);
UserManager.RaiseLogoutSuccessEvent(User, userId);
}
return Request.CreateResponse(HttpStatusCode.OK);

4
src/Umbraco.Web/Security/BackOfficeSignInManager.cs
View File

@@ -159,7 +159,7 @@ namespace Umbraco.Web.Security
if (requestContext != null)
{
var backofficeUserManager = requestContext.GetBackOfficeUserManager();
if (backofficeUserManager != null) backofficeUserManager.RaiseAccountLockedEvent(user.Id);
if (backofficeUserManager != null) backofficeUserManager.RaiseAccountLockedEvent(_request.User, user.Id);
}
return SignInResult.LockedOut;
@@ -170,7 +170,7 @@ namespace Umbraco.Web.Security
{
var backofficeUserManager = requestContext.GetBackOfficeUserManager();
if (backofficeUserManager != null)
backofficeUserManager.RaiseInvalidLoginAttemptEvent(userName);
backofficeUserManager.RaiseInvalidLoginAttemptEvent(_request.User, userName);
}
return SignInResult.Failed;