From 73b107ee2a8c9c81ddb9d620b246f5b3dd19f5b3 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 18 Jul 2017 19:53:34 +1000 Subject: [PATCH] Fixing U4-10138 Cannot upgrade to 7.7 due to user groups and U4-7907 With non OAuth external login providers we should have an 'auto-link' / 'auto-create' callback option --- .../Models/Identity/BackOfficeIdentityUser.cs | 93 ++++++++++++++++++- .../Interfaces/IUserRepository.cs | 29 +++++- .../Repositories/UserRepository.cs | 82 ++++++++++++++++ .../Security/BackOfficeSignInManager.cs | 34 +++++-- .../Security/BackOfficeUserManager.cs | 29 +++++- .../Security/BackOfficeUserStore.cs | 48 ++++++++-- .../Security/BackOfficeUserValidator.cs | 29 ++++++ .../IBackOfficeUserPasswordChecker.cs | 11 +++ src/Umbraco.Core/Services/IUserService.cs | 15 ++- src/Umbraco.Core/Services/UserService.cs | 64 ++++++++++++- src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../Editors/AuthenticationController.cs | 6 +- src/Umbraco.Web/Security/WebSecurity.cs | 56 ++++++----- 13 files changed, 448 insertions(+), 49 deletions(-) create mode 100644 src/Umbraco.Core/Security/BackOfficeUserValidator.cs diff --git a/src/Umbraco.Core/Models/Identity/BackOfficeIdentityUser.cs b/src/Umbraco.Core/Models/Identity/BackOfficeIdentityUser.cs index 8f396db956..a20da18222 100644 --- a/src/Umbraco.Core/Models/Identity/BackOfficeIdentityUser.cs +++ b/src/Umbraco.Core/Models/Identity/BackOfficeIdentityUser.cs @@ -3,15 +3,17 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Collections.Specialized; using System.Linq; +using System.Reflection; using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNet.Identity; +using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Models.Membership; using Umbraco.Core.Security; namespace Umbraco.Core.Models.Identity { - public class BackOfficeIdentityUser : IdentityUser, IdentityUserClaim> + public class BackOfficeIdentityUser : IdentityUser, IdentityUserClaim>, IRememberBeingDirty { public BackOfficeIdentityUser() @@ -33,6 +35,24 @@ namespace Umbraco.Core.Models.Identity var userIdentity = await manager.CreateIdentityAsync(this, Constants.Security.BackOfficeAuthenticationType); return userIdentity; } + + /// + /// Override Email so we can track changes to it + /// + public override string Email + { + get { return _email; } + set { _tracker.SetPropertyValueAndDetectChanges(value, ref _email, Ps.Value.EmailSelector); } + } + + /// + /// Override UserName so we can track changes to it + /// + public override string UserName + { + get { return _userName; } + set { _tracker.SetPropertyValueAndDetectChanges(value, ref _userName, Ps.Value.UsernameSelector); } + } /// /// Gets/sets the user's real name @@ -58,7 +78,7 @@ namespace Umbraco.Core.Models.Identity public override bool LockoutEnabled { get { return true; } - set + set { //do nothing } @@ -132,5 +152,74 @@ namespace Umbraco.Core.Models.Identity { get { return _allStartMediaIds ?? (_allStartMediaIds = StartMediaIds.Concat(Groups.Where(x => x.StartMediaId.HasValue).Select(x => x.StartMediaId.Value)).Distinct().ToArray()); } } + + #region Change tracking + /// + /// Since this class only has change tracking turned on for Email/Username this will return true if either of those have changed + /// + /// + bool ICanBeDirty.IsDirty() + { + return _tracker.IsDirty(); + } + + /// + /// Returns true if the specified property is dirty + /// + /// + /// + bool ICanBeDirty.IsPropertyDirty(string propName) + { + return _tracker.IsPropertyDirty(propName); + } + + /// + /// Resets dirty properties + /// + void ICanBeDirty.ResetDirtyProperties() + { + _tracker.ResetDirtyProperties(); + } + + bool IRememberBeingDirty.WasDirty() + { + return _tracker.WasDirty(); + } + + bool IRememberBeingDirty.WasPropertyDirty(string propertyName) + { + return _tracker.WasPropertyDirty(propertyName); + } + + void IRememberBeingDirty.ForgetPreviouslyDirtyProperties() + { + _tracker.ForgetPreviouslyDirtyProperties(); + } + + void IRememberBeingDirty.ResetDirtyProperties(bool rememberPreviouslyChangedProperties) + { + _tracker.ResetDirtyProperties(rememberPreviouslyChangedProperties); + } + + private static readonly Lazy Ps = new Lazy(); + private class PropertySelectors + { + public readonly PropertyInfo EmailSelector = ExpressionHelper.GetPropertyInfo(x => x.Email); + public readonly PropertyInfo UsernameSelector = ExpressionHelper.GetPropertyInfo(x => x.UserName); + } + + private readonly ChangeTracker _tracker = new ChangeTracker(); + private string _email; + private string _userName; + + /// + /// internal class used to track changes for properties that have it enabled + /// + private class ChangeTracker : TracksChangesEntityBase + { + } + #endregion + + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserRepository.cs index 010f158699..608d671a07 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserRepository.cs @@ -55,8 +55,35 @@ namespace Umbraco.Core.Persistence.Repositories /// IEnumerable GetPagedResultsByQuery(IQuery query, long pageIndex, int pageSize, out long totalRecords, Expression> orderBy, Direction orderDirection, string[] userGroups = null, UserState[] userState = null, IQuery filter = null); + /// + /// Returns a user by username + /// + /// + /// + /// This is really only used for a shim in order to upgrade to 7.6 but could be used + /// for slightly faster user lookups if the result doesn't require security data (i.e. groups, apps & start nodes) + /// + /// + /// A non cached instance + /// + IUser GetByUsername(string username, bool includeSecurityData); + + /// + /// Returns a user by id + /// + /// + /// + /// This is really only used for a shim in order to upgrade to 7.6 but could be used + /// for slightly faster user lookups if the result doesn't require security data (i.e. groups, apps & start nodes) + /// + /// + /// A non cached instance + /// + IUser Get(int id, bool includeSecurityData); + IProfile GetProfile(string username); IProfile GetProfile(int id); - IDictionary GetUserStates(); + IDictionary GetUserStates(); + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs index cb9c471103..eba6c535fd 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs @@ -52,6 +52,88 @@ namespace Umbraco.Core.Persistence.Repositories return user; } + /// + /// Returns a user by username + /// + /// + /// + /// Can be used for slightly faster user lookups if the result doesn't require security data (i.e. groups, apps & start nodes). + /// This is really only used for a shim in order to upgrade to 7.6. + /// + /// + /// A non cached instance + /// + public IUser GetByUsername(string username, bool includeSecurityData) + { + UserDto dto; + if (includeSecurityData) + { + var sql = GetQueryWithGroups(); + sql.Where(userDto => userDto.Login == username, SqlSyntax); + sql //must be included for relator to work + .OrderBy(d => d.Id, SqlSyntax) + .OrderBy(d => d.Id, SqlSyntax) + .OrderBy(d => d.Id, SqlSyntax); + dto = Database + .Fetch( + new UserGroupRelator().Map, sql) + .FirstOrDefault(); + } + else + { + var sql = GetBaseQuery("umbracoUser.*"); + sql.Where(userDto => userDto.Login == username, SqlSyntax); + dto = Database.FirstOrDefault(sql); + } + + if (dto == null) + return null; + + var user = UserFactory.BuildEntity(dto); + return user; + } + + /// + /// Returns a user by id + /// + /// + /// + /// This is really only used for a shim in order to upgrade to 7.6 but could be used + /// for slightly faster user lookups if the result doesn't require security data (i.e. groups, apps & start nodes) + /// + /// + /// A non cached instance + /// + public IUser Get(int id, bool includeSecurityData) + { + UserDto dto; + if (includeSecurityData) + { + var sql = GetQueryWithGroups(); + sql.Where(GetBaseWhereClause(), new { Id = id }); + sql //must be included for relator to work + .OrderBy(d => d.Id, SqlSyntax) + .OrderBy(d => d.Id, SqlSyntax) + .OrderBy(d => d.Id, SqlSyntax); + dto = Database + .Fetch( + new UserGroupRelator().Map, sql) + .FirstOrDefault(); + } + else + { + var sql = GetBaseQuery("umbracoUser.*"); + sql.Where(GetBaseWhereClause(), new { Id = id }); + dto = Database.FirstOrDefault(sql); + } + + if (dto == null) + return null; + + var user = UserFactory.BuildEntity(dto); + return user; + } + public IProfile GetProfile(string username) { var sql = GetBaseQuery(false).Where(userDto => userDto.UserName == username, SqlSyntax); diff --git a/src/Umbraco.Core/Security/BackOfficeSignInManager.cs b/src/Umbraco.Core/Security/BackOfficeSignInManager.cs index 9a6f9cceb3..1522d43dfb 100644 --- a/src/Umbraco.Core/Security/BackOfficeSignInManager.cs +++ b/src/Umbraco.Core/Security/BackOfficeSignInManager.cs @@ -101,20 +101,40 @@ namespace Umbraco.Core.Security { return SignInStatus.Failure; } - var user = await UserManager.FindByNameAsync(userName); + + var backOfficeUserMgr = UserManager as BackOfficeUserManager; + + var user = backOfficeUserMgr != null + //this will be a slightly faster lookup since we don't need the security data here (and works for upgrading to 7.6) + //it's worth mentioning here that getting a user by login name is never cached so we don't need to worry about that here + ? await backOfficeUserMgr.FindByNameAsync(userName, includeSecurityData:false) + //load normally - this would only be the case if someone has totally replaced the user manager + : await UserManager.FindByNameAsync(userName); + + //if the user is null, create an empty one which can be used for auto-linking if (user == null) { - return SignInStatus.Failure; - } - if (await UserManager.IsLockedOutAsync(user.Id)) - { - return SignInStatus.LockedOut; + user = new BackOfficeIdentityUser + { + UserName = userName, + Culture = GlobalSettings.DefaultUILanguage + }; } + + //check the password for the user, this will allow a developer to auto-link + //an account if they have specified an IBackOfficeUserPasswordChecker if (await UserManager.CheckPasswordAsync(user, password)) { + //the underlying call to this will query the user by Id which IS cached! + if (await UserManager.IsLockedOutAsync(user.Id)) + { + return SignInStatus.LockedOut; + } + await UserManager.ResetAccessFailedCountAsync(user.Id); return await SignInOrTwoFactor(user, isPersistent); } + if (shouldLockout) { // If lockout is requested, increment access failed count which might lock out the user @@ -125,7 +145,7 @@ namespace Umbraco.Core.Security } } return SignInStatus.Failure; - } + } /// /// Borrowed from Micorosoft's underlying sign in manager which is not flexible enough to tell it to use a different cookie type diff --git a/src/Umbraco.Core/Security/BackOfficeUserManager.cs b/src/Umbraco.Core/Security/BackOfficeUserManager.cs index ad22c1426e..e823a633c8 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserManager.cs @@ -146,7 +146,7 @@ namespace Umbraco.Core.Security IDataProtectionProvider dataProtectionProvider) { // Configure validation logic for usernames - manager.UserValidator = new UserValidator(manager) + manager.UserValidator = new BackOfficeUserValidator(manager) { AllowOnlyAlphanumericUserNames = false, RequireUniqueEmail = true @@ -192,6 +192,32 @@ namespace Umbraco.Core.Security //manager.SmsService = new SmsService(); } + + /// + /// Looks up a by username + /// + /// + /// + /// Can be used for slightly faster user lookups if the result doesn't require security data (i.e. groups, apps & start nodes). + /// This is really only used for a shim in order to upgrade to 7.6. + /// + /// + public async Task FindByNameAsync(string userName, bool includeSecurityData) + { + T result; + if (includeSecurityData) + { + result = await Store.FindByNameAsync(userName); + return result; + } + + var backOfficeUserStore = Store as BackOfficeUserStore; + if (backOfficeUserStore == null) + throw new InvalidOperationException("A custom IUserStore is in use which does not support querying users without security data"); + + result = (T)await backOfficeUserStore.FindByNameAsync(userName, false); + return result; + } /// /// Logic used to validate a username and password @@ -266,5 +292,6 @@ namespace Umbraco.Core.Security } throw new NotSupportedException("Cannot generate a password since the type of the password validator (" + PasswordValidator.GetType() + ") is not known"); } + } } diff --git a/src/Umbraco.Core/Security/BackOfficeUserStore.cs b/src/Umbraco.Core/Security/BackOfficeUserStore.cs index 0873f0a83c..6f036172f9 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserStore.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserStore.cs @@ -8,9 +8,11 @@ using System.Web.Security; using AutoMapper; using Microsoft.AspNet.Identity; using Microsoft.Owin; +using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Models.Identity; using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; +using IUser = Umbraco.Core.Models.Membership.IUser; namespace Umbraco.Core.Security { @@ -190,6 +192,29 @@ namespace Umbraco.Core.Security return await Task.FromResult(result); } + /// + /// Looks up a by username + /// + /// + /// + /// Can be used for slightly faster user lookups if the result doesn't require security data (i.e. groups, apps & start nodes). + /// This is really only used for a shim in order to upgrade to 7.6. + /// + /// + public virtual async Task FindByNameAsync(string userName, bool includeSecurityData) + { + ThrowIfDisposed(); + var user = _userService.GetByUsername(userName, includeSecurityData); + if (user == null) + { + return null; + } + + var result = AssignLoginsCallback(Mapper.Map(user)); + + return await Task.FromResult(result); + } + /// /// Set the user password hash /// @@ -367,12 +392,17 @@ namespace Umbraco.Core.Security var result = _externalLoginService.Find(login).ToArray(); if (result.Any()) { - //return the first member that matches the result - var output = (from l in result - select _userService.GetUserById(l.UserId) - into user - where user != null - select Mapper.Map(user)).FirstOrDefault(); + //return the first user that matches the result + BackOfficeIdentityUser output = null; + foreach (var l in result) + { + var user = _userService.GetUserById(l.UserId); + if (user != null) + { + output = Mapper.Map(user); + break; + } + } return Task.FromResult(AssignLoginsCallback(output)); } @@ -717,6 +747,9 @@ namespace Umbraco.Core.Security } } + //reset all changes + ((IRememberBeingDirty) identityUser).ResetDirtyProperties(false); + return anythingChanged; } @@ -726,7 +759,6 @@ namespace Umbraco.Core.Security if (_disposed) throw new ObjectDisposedException(GetType().Name); } - - + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Security/BackOfficeUserValidator.cs b/src/Umbraco.Core/Security/BackOfficeUserValidator.cs new file mode 100644 index 0000000000..b73a637463 --- /dev/null +++ b/src/Umbraco.Core/Security/BackOfficeUserValidator.cs @@ -0,0 +1,29 @@ +using System.Threading.Tasks; +using Microsoft.AspNet.Identity; +using Umbraco.Core.Models.EntityBase; +using Umbraco.Core.Models.Identity; + +namespace Umbraco.Core.Security +{ + /// + /// Custom validator to not validate a user's username or email if they haven't changed + /// + /// + internal class BackOfficeUserValidator : UserValidator + where T : BackOfficeIdentityUser + { + public BackOfficeUserValidator(UserManager manager) : base(manager) + { + } + + public override async Task ValidateAsync(T item) + { + //Don't validate if the user's email or username hasn't changed otherwise it's just wasting SQL queries. + if (((ICanBeDirty)item).IsDirty()) + { + return await base.ValidateAsync(item); + } + return IdentityResult.Success; + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Security/IBackOfficeUserPasswordChecker.cs b/src/Umbraco.Core/Security/IBackOfficeUserPasswordChecker.cs index 6ee65e0fef..1852be4c4c 100644 --- a/src/Umbraco.Core/Security/IBackOfficeUserPasswordChecker.cs +++ b/src/Umbraco.Core/Security/IBackOfficeUserPasswordChecker.cs @@ -9,6 +9,17 @@ namespace Umbraco.Core.Security /// public interface IBackOfficeUserPasswordChecker { + /// + /// Checks a password for a user + /// + /// + /// + /// + /// + /// This will allow a developer to auto-link a local account which is required if the user queried doesn't exist locally. + /// The user parameter will always contain the username, if the user doesn't exist locally, the other properties will not be filled in. + /// A developer can then create a local account by filling in the properties and using UserManager.CreateAsync + /// Task CheckPasswordAsync(BackOfficeIdentityUser user, string password); } } \ No newline at end of file diff --git a/src/Umbraco.Core/Services/IUserService.cs b/src/Umbraco.Core/Services/IUserService.cs index ea56264075..84c6590f23 100644 --- a/src/Umbraco.Core/Services/IUserService.cs +++ b/src/Umbraco.Core/Services/IUserService.cs @@ -69,7 +69,20 @@ namespace Umbraco.Core.Services /// /// Id of the user to retrieve /// - IUser GetUserById(int id); + IUser GetUserById(int id); + + /// + /// Get an by username + /// + /// Username to use for retrieval + /// + /// Can be used for slightly faster user lookups if the result doesn't require security data (i.e. groups, apps & start nodes). + /// This is really only used for a shim in order to upgrade to 7.6. + /// + /// + /// A non cached instance + /// + IUser GetByUsername(string username, bool includeSecurityData); /// /// Gets a users by Id diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index 2d5fff536c..53aaa197fe 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Data.Common; +using System.Data.SqlClient; using System.Globalization; using System.Linq; using System.Linq.Expressions; @@ -197,11 +198,49 @@ namespace Umbraco.Core.Services using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateUserRepository(uow); - var query = Query.Builder.Where(x => x.Username.Equals(username)); - return repository.GetByQuery(query).FirstOrDefault(); + + try + { + return repository.GetByUsername(username, includeSecurityData: true); + } + catch (SqlException ex) + { + //we need to handle this one specific case which is when we are upgrading to 7.6 since the user group + //tables don't exist yet. This is the 'easiest' way to deal with this without having to create special + //version checks in the BackOfficeSignInManager and calling into other special overloads that we'd need + //like "GetUserById(int id, bool includeSecurityData)" which may cause confusion because the result of + //that method would not be cached. + if (ApplicationContext.Current.IsUpgrading) + { + //NOTE: this will not be cached + return repository.GetByUsername(username, includeSecurityData: false); + } + throw; + } + } } + /// + /// Get an by username + /// + /// Username to use for retrieval + /// + /// Can be used for slightly faster user lookups if the result doesn't require security data (i.e. groups, apps & start nodes). + /// This is really only used for a shim in order to upgrade to 7.6. + /// + /// + /// A non cached instance + /// + public IUser GetByUsername(string username, bool includeSecurityData) + { + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) + { + var repository = RepositoryFactory.CreateUserRepository(uow); + return repository.GetByUsername(username, includeSecurityData); + } + } + /// /// Deletes an /// @@ -661,9 +700,26 @@ namespace Umbraco.Core.Services using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateUserRepository(uow); - return repository.Get(id); + try + { + return repository.Get(id); + } + catch (SqlException ex) + { + //we need to handle this one specific case which is when we are upgrading to 7.6 since the user group + //tables don't exist yet. This is the 'easiest' way to deal with this without having to create special + //version checks in the BackOfficeSignInManager and calling into other special overloads that we'd need + //like "GetUserById(int id, bool includeSecurityData)" which may cause confusion because the result of + //that method would not be cached. + if (ApplicationContext.Current.IsUpgrading) + { + //NOTE: this will not be cached + return repository.Get(id, includeSecurityData: false); + } + throw; + } } - } + } public IEnumerable GetUsersById(params int[] ids) { diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 0edbdc77cf..3ef0ea45d9 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -661,6 +661,7 @@ + diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index a9390ca5c7..978f8cbfd6 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -220,7 +220,7 @@ namespace Umbraco.Web.Editors case SignInStatus.Success: //get the user - var user = Security.GetBackOfficeUser(loginModel.Username); + var user = Security.GetOrCreateBackOfficeUser(loginModel.Username); return SetPrincipalAndReturnUserDetail(user); case SignInStatus.RequiresVerification: @@ -246,7 +246,7 @@ namespace Umbraco.Web.Editors typeof(IUmbracoBackOfficeTwoFactorOptions) + ".GetTwoFactorView returned an empty string")); } - var attemptedUser = Security.GetBackOfficeUser(loginModel.Username); + var attemptedUser = Security.GetOrCreateBackOfficeUser(loginModel.Username); //create a with information to display a custom two factor send code view var verifyResponse = Request.CreateResponse(HttpStatusCode.PaymentRequired, new @@ -365,7 +365,7 @@ namespace Umbraco.Web.Editors { case SignInStatus.Success: //get the user - var user = Security.GetBackOfficeUser(userName); + var user = Security.GetOrCreateBackOfficeUser(userName); return SetPrincipalAndReturnUserDetail(user); case SignInStatus.LockedOut: return Request.CreateValidationErrorResponse("User is locked out"); diff --git a/src/Umbraco.Web/Security/WebSecurity.cs b/src/Umbraco.Web/Security/WebSecurity.cs index e4a29bd6cc..b631b6dfe7 100644 --- a/src/Umbraco.Web/Security/WebSecurity.cs +++ b/src/Umbraco.Web/Security/WebSecurity.cs @@ -186,34 +186,31 @@ namespace Umbraco.Web.Security { var membershipProvider = Core.Security.MembershipProviderExtensions.GetUsersMembershipProvider(); return membershipProvider != null ? membershipProvider.GetUser(username, setOnline) : null; - } - + } + /// /// Gets (and creates if not found) the back office instance for the username specified + /// and updates the user's online flag /// /// /// /// + /// /// This will return an instance no matter what membership provider is installed for the back office, it will automatically - /// create any missing accounts if one is not found and a custom membership provider or is being used. + /// create any missing accounts if one is not found and a custom membership provider is being used. + /// + /// + /// This will try to deal with any custom membership provider that may exist, though this is not a recommended approach anymore people still + /// have these so we need to maintain compat. + /// + /// TODO: I don't think this method is needed so the todo statment below, pretty sure we can simplify this whole thing /// - internal IUser GetBackOfficeUser(string username) + internal IUser GetOrCreateBackOfficeUser(string username) { - //get the membership user (set user to be 'online' in the provider too) - var membershipUser = GetBackOfficeMembershipUser(username, true); - var provider = Core.Security.MembershipProviderExtensions.GetUsersMembershipProvider(); - - if (membershipUser == null) - { - throw new InvalidOperationException( - "The username & password validated but the membership provider '" + - provider.Name + - "' did not return a MembershipUser with the username supplied"); - } - - //regarldess of the membership provider used, see if this user object already exists in the umbraco data - var user = _applicationContext.Services.UserService.GetByUsername(membershipUser.UserName); + //See if this user object already exists in the umbraco data + var user = _applicationContext.Services.UserService.GetByUsername(username); + var provider = Core.Security.MembershipProviderExtensions.GetUsersMembershipProvider(); //we're using the built-in membership provider so the user will already be available if (provider.IsUmbracoUsersProvider()) { @@ -223,12 +220,27 @@ namespace Umbraco.Web.Security throw new InvalidOperationException("The user '" + username + "' could not be found in the Umbraco database"); } return user; - } - - //we are using a custom membership provider for the back office, in this case we need to create user accounts for the logged in member. - //if we already have a user object in Umbraco we don't need to do anything, otherwise we need to create a mapped Umbraco account. + } + + //TODO: I don't think this logic can ever get hit! This will only ever get called after the PasswordSignInAsync method will + // be executed and that requires that a user exists locally, it will not work if one doesn't! + // VERIFY THIS! + + //we are using a custom membership provider for the back office, in this case we need to create user accounts for the logged in member. + //if we already have a user object in Umbraco we don't need to do anything, otherwise we need to create a mapped Umbraco account. if (user != null) return user; + //get the membership user from the custom provider (set user to be 'online' in the provider too) + var membershipUser = GetBackOfficeMembershipUser(username, true); + + if (membershipUser == null) + { + throw new InvalidOperationException( + "The username & password validated but the membership provider '" + + provider.Name + + "' did not return a MembershipUser with the username supplied"); + } + var email = membershipUser.Email; if (email.IsNullOrWhiteSpace()) {