diff --git a/src/Umbraco.Core/Models/Identity/BackOfficeIdentityUser.cs b/src/Umbraco.Core/Models/Identity/BackOfficeIdentityUser.cs index 8dc056d555..b7bde26fae 100644 --- a/src/Umbraco.Core/Models/Identity/BackOfficeIdentityUser.cs +++ b/src/Umbraco.Core/Models/Identity/BackOfficeIdentityUser.cs @@ -275,16 +275,23 @@ namespace Umbraco.Core.Models.Identity { get { - if (_getLogins != null && _getLogins.IsValueCreated == false) + // return if it exists + if (_logins != null) return _logins; + + _logins = new ObservableCollection(); + + // if the callback is there and hasn't been created yet then execute it and populate the logins + if (_getLogins != null && !_getLogins.IsValueCreated) { - _logins = new ObservableCollection(); foreach (var l in _getLogins.Value) { _logins.Add(l); } - //now assign events - _logins.CollectionChanged += Logins_CollectionChanged; } + + //now assign events + _logins.CollectionChanged += Logins_CollectionChanged; + return _logins; } } diff --git a/src/Umbraco.Core/Persistence/Dtos/ExternalLoginDto.cs b/src/Umbraco.Core/Persistence/Dtos/ExternalLoginDto.cs index d58d23a272..0a56552000 100644 --- a/src/Umbraco.Core/Persistence/Dtos/ExternalLoginDto.cs +++ b/src/Umbraco.Core/Persistence/Dtos/ExternalLoginDto.cs @@ -14,6 +14,8 @@ namespace Umbraco.Core.Persistence.Dtos [PrimaryKeyColumn(Name = "PK_umbracoExternalLogin")] public int Id { get; set; } + // TODO: This is completely missing a FK!!? + [Column("userId")] public int UserId { get; set; } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ExternalLoginRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ExternalLoginRepository.cs index 90a698f999..ad53a2d522 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ExternalLoginRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ExternalLoginRepository.cs @@ -22,7 +22,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement public void DeleteUserLogins(int memberId) { - Database.Execute("DELETE FROM ExternalLogins WHERE UserId=@userId", new { userId = memberId }); + Database.Delete("WHERE userId=@userId", new { userId = memberId }); } public void Save(int userId, IEnumerable logins) diff --git a/src/Umbraco.Core/Security/BackOfficeUserStore.cs b/src/Umbraco.Core/Security/BackOfficeUserStore.cs index 1ee5646ec5..7df328b5b7 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserStore.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserStore.cs @@ -96,9 +96,10 @@ namespace Umbraco.Core.Security IsLockedOut = user.IsLockedOut, }; - UpdateMemberProperties(userEntity, user); + // we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. + var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(BackOfficeIdentityUser.Logins)); - // TODO: We should deal with Roles --> User Groups here which we currently are not doing + UpdateMemberProperties(userEntity, user); _userService.Save(userEntity); @@ -107,6 +108,16 @@ namespace Umbraco.Core.Security //re-assign id user.Id = userEntity.Id; + if (isLoginsPropertyDirty) + { + _externalLoginService.Save( + user.Id, + user.Logins.Select(x => new ExternalLogin( + x.LoginProvider, + x.ProviderKey, + (x is IIdentityUserLoginExtended extended) ? extended.UserData : null))); + } + return Task.FromResult(0); } @@ -132,7 +143,7 @@ namespace Umbraco.Core.Security if (found != null) { // we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. - var isLoginsPropertyDirty = user.IsPropertyDirty("Logins"); + var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(BackOfficeIdentityUser.Logins)); if (UpdateMemberProperties(found, user)) { @@ -641,7 +652,7 @@ namespace Umbraco.Core.Security //don't assign anything if nothing has changed as this will trigger the track changes of the model - if (identityUser.IsPropertyDirty("LastLoginDateUtc") + if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.LastLoginDateUtc)) || (user.LastLoginDate != default(DateTime) && identityUser.LastLoginDateUtc.HasValue == false) || identityUser.LastLoginDateUtc.HasValue && user.LastLoginDate.ToUniversalTime() != identityUser.LastLoginDateUtc.Value) { @@ -650,33 +661,33 @@ namespace Umbraco.Core.Security var dt = identityUser.LastLoginDateUtc == DateTime.MinValue ? DateTime.MinValue : identityUser.LastLoginDateUtc.Value.ToLocalTime(); user.LastLoginDate = dt; } - if (identityUser.IsPropertyDirty("LastPasswordChangeDateUtc") + if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.LastPasswordChangeDateUtc)) || (user.LastPasswordChangeDate != default(DateTime) && identityUser.LastPasswordChangeDateUtc.HasValue == false) || identityUser.LastPasswordChangeDateUtc.HasValue && user.LastPasswordChangeDate.ToUniversalTime() != identityUser.LastPasswordChangeDateUtc.Value) { anythingChanged = true; user.LastPasswordChangeDate = identityUser.LastPasswordChangeDateUtc.Value.ToLocalTime(); } - if (identityUser.IsPropertyDirty("EmailConfirmed") + if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.EmailConfirmed)) || (user.EmailConfirmedDate.HasValue && user.EmailConfirmedDate.Value != default(DateTime) && identityUser.EmailConfirmed == false) || ((user.EmailConfirmedDate.HasValue == false || user.EmailConfirmedDate.Value == default(DateTime)) && identityUser.EmailConfirmed)) { anythingChanged = true; user.EmailConfirmedDate = identityUser.EmailConfirmed ? (DateTime?)DateTime.Now : null; } - if (identityUser.IsPropertyDirty("Name") + if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Name)) && user.Name != identityUser.Name && identityUser.Name.IsNullOrWhiteSpace() == false) { anythingChanged = true; user.Name = identityUser.Name; } - if (identityUser.IsPropertyDirty("Email") + if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Email)) && user.Email != identityUser.Email && identityUser.Email.IsNullOrWhiteSpace() == false) { anythingChanged = true; user.Email = identityUser.Email; } - if (identityUser.IsPropertyDirty("AccessFailedCount") + if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.AccessFailedCount)) && user.FailedPasswordAttempts != identityUser.AccessFailedCount) { anythingChanged = true; @@ -694,32 +705,32 @@ namespace Umbraco.Core.Security } } - if (identityUser.IsPropertyDirty("UserName") + if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.UserName)) && user.Username != identityUser.UserName && identityUser.UserName.IsNullOrWhiteSpace() == false) { anythingChanged = true; user.Username = identityUser.UserName; } - if (identityUser.IsPropertyDirty("PasswordHash") + if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.PasswordHash)) && user.RawPasswordValue != identityUser.PasswordHash && identityUser.PasswordHash.IsNullOrWhiteSpace() == false) { anythingChanged = true; user.RawPasswordValue = identityUser.PasswordHash; } - if (identityUser.IsPropertyDirty("Culture") + if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Culture)) && user.Language != identityUser.Culture && identityUser.Culture.IsNullOrWhiteSpace() == false) { anythingChanged = true; user.Language = identityUser.Culture; } - if (identityUser.IsPropertyDirty("StartMediaIds") + if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.StartMediaIds)) && user.StartMediaIds.UnsortedSequenceEqual(identityUser.StartMediaIds) == false) { anythingChanged = true; user.StartMediaIds = identityUser.StartMediaIds; } - if (identityUser.IsPropertyDirty("StartContentIds") + if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.StartContentIds)) && user.StartContentIds.UnsortedSequenceEqual(identityUser.StartContentIds) == false) { anythingChanged = true; @@ -732,7 +743,7 @@ namespace Umbraco.Core.Security } // TODO: Fix this for Groups too - if (identityUser.IsPropertyDirty("Roles") || identityUser.IsPropertyDirty("Groups")) + if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Roles)) || identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Groups))) { var userGroupAliases = user.Groups.Select(x => x.Alias).ToArray(); diff --git a/src/Umbraco.Web/Editors/BackOfficeController.cs b/src/Umbraco.Web/Editors/BackOfficeController.cs index 7d8d265440..aceacbf827 100644 --- a/src/Umbraco.Web/Editors/BackOfficeController.cs +++ b/src/Umbraco.Web/Editors/BackOfficeController.cs @@ -537,29 +537,39 @@ namespace Umbraco.Web.Editors private async Task LinkUser(BackOfficeIdentityUser autoLinkUser, ExternalLoginInfo loginInfo) { - var linkResult = await UserManager.AddLoginAsync(autoLinkUser.Id, loginInfo.Login); - if (linkResult.Succeeded == false) - { - ViewData.SetExternalSignInProviderErrors( - new BackOfficeExternalLoginProviderErrors( - loginInfo.Login.LoginProvider, - linkResult.Errors)); + var existingLogins = await UserManager.GetLoginsAsync(autoLinkUser.Id); + var exists = existingLogins.FirstOrDefault(x => x.LoginProvider == loginInfo.Login.LoginProvider && x.ProviderKey == loginInfo.Login.ProviderKey); - //If this fails, we should really delete the user since it will be in an inconsistent state! - var deleteResult = await UserManager.DeleteAsync(autoLinkUser); - if (deleteResult.Succeeded == false) - { - //DOH! ... this isn't good, combine all errors to be shown - ViewData.SetExternalSignInProviderErrors( - new BackOfficeExternalLoginProviderErrors( - loginInfo.Login.LoginProvider, - linkResult.Errors.Concat(deleteResult.Errors))); - } - } - else + // if it already exists (perhaps it was added in the AutoLink callbak) then we just continue + if (exists != null) { //sign in await SignInManager.SignInAsync(autoLinkUser, isPersistent: false, rememberBrowser: false); + return; + } + + var linkResult = await UserManager.AddLoginAsync(autoLinkUser.Id, loginInfo.Login); + if (linkResult.Succeeded) + { + //we're good! sign in + await SignInManager.SignInAsync(autoLinkUser, isPersistent: false, rememberBrowser: false); + return; + } + + ViewData.SetExternalSignInProviderErrors( + new BackOfficeExternalLoginProviderErrors( + loginInfo.Login.LoginProvider, + linkResult.Errors)); + + //If this fails, we should really delete the user since it will be in an inconsistent state! + var deleteResult = await UserManager.DeleteAsync(autoLinkUser); + if (!deleteResult.Succeeded) + { + //DOH! ... this isn't good, combine all errors to be shown + ViewData.SetExternalSignInProviderErrors( + new BackOfficeExternalLoginProviderErrors( + loginInfo.Login.LoginProvider, + linkResult.Errors.Concat(deleteResult.Errors))); } } diff --git a/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderErrorMiddlware.cs b/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderErrorMiddlware.cs index 7d3c48c078..6e6477443b 100644 --- a/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderErrorMiddlware.cs +++ b/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderErrorMiddlware.cs @@ -35,7 +35,7 @@ namespace Umbraco.Web.Security var serialized = Convert.ToBase64String(Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(errors))); - context.Response.Cookies.Append("ExternalSignInError", serialized, new CookieOptions + context.Response.Cookies.Append(ViewDataExtensions.TokenExternalSignInError, serialized, new CookieOptions { Expires = DateTime.Now.AddMinutes(5), HttpOnly = true,