From 077cf57abc88b832ff59633faa1ad04e82f6e12e Mon Sep 17 00:00:00 2001 From: Scott Brady Date: Thu, 30 Apr 2020 15:48:32 +0100 Subject: [PATCH] PR recommendations: added null checks where FindByX was added, using the same exception as ASP.NET Identity 2 --- .../Editors/AuthenticationController.cs | 1 + .../Editors/BackOfficeController.cs | 1 + .../Editors/CurrentUserController.cs | 2 ++ .../Security/BackOfficeUserManager.cs | 21 +++---------------- .../Security/IdentityFactoryMiddleware.cs | 17 +++------------ 5 files changed, 10 insertions(+), 32 deletions(-) diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index a0143548fc..c7b22fc02f 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -131,6 +131,7 @@ namespace Umbraco.Web.Editors public async Task PostUnLinkLogin(UnLinkLoginModel unlinkLoginModel) { var user = await UserManager.FindByIdAsync(User.Identity.GetUserId()); + if (user == null) throw new InvalidOperationException("Could not find user"); var result = await UserManager.RemoveLoginAsync( user, diff --git a/src/Umbraco.Web/Editors/BackOfficeController.cs b/src/Umbraco.Web/Editors/BackOfficeController.cs index 34bb2e6e03..307c826035 100644 --- a/src/Umbraco.Web/Editors/BackOfficeController.cs +++ b/src/Umbraco.Web/Editors/BackOfficeController.cs @@ -329,6 +329,7 @@ namespace Umbraco.Web.Editors } var user = await UserManager.FindByIdAsync(User.Identity.GetUserId()); + if (user == null) throw new InvalidOperationException("Could not find user"); var result = await UserManager.AddLoginAsync(user, new UserLoginInfo(loginInfo.Login.LoginProvider, loginInfo.Login.ProviderKey, loginInfo.Login.LoginProvider)); diff --git a/src/Umbraco.Web/Editors/CurrentUserController.cs b/src/Umbraco.Web/Editors/CurrentUserController.cs index 678b6e098d..ad07155bc0 100644 --- a/src/Umbraco.Web/Editors/CurrentUserController.cs +++ b/src/Umbraco.Web/Editors/CurrentUserController.cs @@ -159,6 +159,8 @@ namespace Umbraco.Web.Editors public async Task PostSetInvitedUserPassword([FromBody]string newPassword) { var user = await UserManager.FindByIdAsync(Security.GetUserId().ResultOr(0).ToString()); + if (user == null) throw new InvalidOperationException("Could not find user"); + var result = await UserManager.AddPasswordAsync(user, newPassword); if (result.Succeeded == false) diff --git a/src/Umbraco.Web/Security/BackOfficeUserManager.cs b/src/Umbraco.Web/Security/BackOfficeUserManager.cs index 443485529c..a143029327 100644 --- a/src/Umbraco.Web/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Web/Security/BackOfficeUserManager.cs @@ -306,6 +306,8 @@ namespace Umbraco.Web.Security public async Task ChangePasswordWithResetAsync(int userId, string token, string newPassword) { var user = await base.FindByIdAsync(userId.ToString()); + if (user == null) throw new InvalidOperationException("Could not find user"); + var result = await base.ResetPasswordAsync(user, token, newPassword); if (result.Succeeded) RaisePasswordChangedEvent(userId); return result; @@ -317,24 +319,7 @@ namespace Umbraco.Web.Security if (result.Succeeded) RaisePasswordChangedEvent(user.Id); return result; } - - /// - /// Override to determine how to hash the password - /// - /// - /// - /// - /// - protected override async Task VerifyPasswordAsync(IUserPasswordStore store, T user, string password) - { - var userAwarePasswordHasher = PasswordHasher; - if (userAwarePasswordHasher == null) - return await base.VerifyPasswordAsync(store, user, password); - - var hash = await store.GetPasswordHashAsync(user, CancellationToken.None); - return userAwarePasswordHasher.VerifyHashedPassword(user, hash, password); - } - + /// /// Override to determine how to hash the password /// diff --git a/src/Umbraco.Web/Security/IdentityFactoryMiddleware.cs b/src/Umbraco.Web/Security/IdentityFactoryMiddleware.cs index d091a51624..2975149107 100644 --- a/src/Umbraco.Web/Security/IdentityFactoryMiddleware.cs +++ b/src/Umbraco.Web/Security/IdentityFactoryMiddleware.cs @@ -12,22 +12,14 @@ namespace Umbraco.Web.Security where TResult : class, IDisposable where TOptions : IdentityFactoryOptions { - /// - /// Constructor - /// /// The next middleware in the OWIN pipeline to invoke /// Configuration options for the middleware public IdentityFactoryMiddleware(OwinMiddleware next, TOptions options) : base(next) { - if (options == null) - { - throw new ArgumentNullException(nameof(options)); - } - if (options.Provider == null) - { - throw new ArgumentNullException("options.Provider"); - } + if (options == null) throw new ArgumentNullException(nameof(options)); + if (options.Provider == null) throw new ArgumentException("options.Provider"); + Options = options; } @@ -74,9 +66,6 @@ namespace Umbraco.Web.Security public class IdentityFactoryProvider where T : class, IDisposable { - /// - /// Constructor - /// public IdentityFactoryProvider() { OnDispose = (options, instance) => { };