U4-10138 - Cannot upgrade to 7.7 due to user groups

Created by Stephan 11 Jul 2017, 06:59:23 Updated by Shannon Deminick 01 Aug 2017, 07:51:57

Relates to: U4-7907

Subtask of: U4-8632

The new user groups break 7.7 upgrade bc user cannot log into 7.7 bc the code expects tables that aren't there


Shannon Deminick 18 Jul 2017, 09:53:44

  • --Adds ability to query an IUser by username without security information (better for SQL performance, non-cached) and works for 7.6 upgrade queries--
  • Adds try/catch in the user service so that we can handle the 7.6 upgrade issue
  • Adds change tracking to BackOfficeIdentityUser so that properties that haven't changed are not overwritten, this is important with the 7.6 upgrade fix (IUser without security info) since we load in users without security information (based on a try/catch that you'll see) and without this on first login the user's groups would be overwritten with nothing **The change tracking also allows us to save queries so we don't need to re-query twice for validating the user if it's username or email hasn't changed
  • Implements http://issues.umbraco.org/issue/U4-7907
  • Saves a bunch of queries when logging in
  • Removes internal WebSecurity.GetBackOfficeUser - the original point of this was to lookup a user and if it didn't exist but there was a custom membership provider that did return a user than the login process could continue or the user would be auto-linked (created locally), however this never worked and the logic would never be hit because before that method was ever executed, the password checker would be executed which requires that a local user exists! if they didn't exist locally than this method would never be executed so it was basically useless
  • Fixes the Add/RemoveFromRoleAsync on the BackOfficeUserStore - this was checking against AllowedSections which was totally wrong
  • Fixes the implementation of the Roles for the BackOfficeUser
  • Obsoletes GetDefaultAllowedSections for autolink options since that is irrelevant now
  • Fixes some null ref checks

Shannon Deminick 19 Jul 2017, 04:28:00

PR here: https://github.com/umbraco/Umbraco-CMS/pull/2057

NOTE: This PR also contains code to complete http://issues.umbraco.org/issue/U4-7907

To Test:

  • Make sure you're logged out when working on this branch
  • Take a v7.6 (or below) installation, create some user types and users (ensure to set passwords), assign to various user types, adds various start nodes to the users, add various sections to users, add various permission structures to the users for the content nodes (maybe take a backup of this db in case you want to test again)
  • Point this branch at the new database, change the web.config to have the version you are upgrading from
  • Do the upgrade, this will make you login - this should work, the upgrade should succeeed
  • Verify that the upgrade worked: ** There should be user groups created for all user types ** There should be user groups created for all users except for the admin ** All groups created based on each user will have their associated allowed sections and the granular permissions assigned to content that you created (you'll see this on the user group created for the user)
  • You should be able to edit users and user groups
  • You should be able to log out

Shannon Deminick 19 Jul 2017, 08:27:25

Damnit, just noticed i have to change something with this, i'll fix

Shannon Deminick 19 Jul 2017, 08:45:09

Ok, will put this back in to review. Basically I need to remove this part https://github.com/umbraco/Umbraco-CMS/pull/2057/files#diff-5fc41f65b668eadac27a7d0fb2ef4be0R110 we cannot do this query without the security data because then the user's groups are not filled in which turn into the user's 'Roles' which go into the auth ticket that is created there. But... i'll fix that once this is reviewed and merged.

Shannon Deminick 20 Jul 2017, 12:15:19

@zpqrtbnk I've just pushed new changes that fix up some of the Roles stuff and the user lookup mentioned above. There's a lot in this branch and there might be stuff that is fixed in here that overlaps with what you might be working on so hopefully you don't end up debugging stuff that might already be fixed here.

Stephan 27 Jul 2017, 16:02:06

Test: took an existing 7.6 database and tried to upgrade to 7.7 with your PR + latest user-group-permissions. Still getting an error when trying to log in to authorize the upgrade:

"Message":"An error has occurred.",
"ExceptionMessage":"Invalid object name 'umbracoUser2UserGroup'.",
at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at System.Data.SqlClient.SqlDataReader.TryConsumeMetaData()
   at System.Data.SqlClient.SqlDataReader.get_MetaData()
   at System.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString, Boolean isInternal, Boolean forDescribeParameterEncryption)
   at System.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean async, Int32 timeout, Task& task, Boolean asyncWrite, Boolean inRetry, SqlDataReader ds, Boolean describeParameterEncryptionRequest)
   at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry)
   at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method)
   at System.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior, String method)
   at Umbraco.Core.Persistence.PetaPocoCommandExtensions.<>c__DisplayClass5_0.<ExecuteReaderWithRetry>b__0() in D:\\d\\Umbraco 7.7\\src\\Umbraco.Core\\Persistence\\PetaPocoCommandExtensions.cs:line 125
   at Umbraco.Core.Persistence.FaultHandling.RetryPolicy.ExecuteAction[TResult](Func`1 func) in D:\\d\\Umbraco 7.7\\src\\Umbraco.Core\\Persistence\\FaultHandling\\RetryPolicy.cs:line 174
   at Umbraco.Core.Persistence.PetaPocoCommandExtensions.ExecuteReaderWithRetry(IDbCommand command, RetryPolicy cmdRetryPolicy, RetryPolicy conRetryPolicy) in D:\\d\\Umbraco 7.7\\src\\Umbraco.Core\\Persistence\\PetaPocoCommandExtensions.cs:line 110
   at Umbraco.Core.Persistence.PetaPocoCommandExtensions.ExecuteReaderWithRetry(IDbCommand command, RetryPolicy retryPolicy) in D:\\d\\Umbraco 7.7\\src\\Umbraco.Core\\Persistence\\PetaPocoCommandExtensions.cs:line 93
   at Umbraco.Core.Persistence.PetaPocoCommandExtensions.ExecuteReaderWithRetry(IDbCommand command) in D:\\d\\Umbraco 7.7\\src\\Umbraco.Core\\Persistence\\PetaPocoCommandExtensions.cs:line 80
   at Umbraco.Core.Persistence.Database.<Query>d__115`1.MoveNext() in D:\\d\\Umbraco 7.7\\src\\Umbraco.Core\\Persistence\\PetaPoco.cs:line 1205
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Umbraco.Core.Persistence.Database.Fetch[T1,T2,T3,T4,TRet](Func`5 cb, Sql sql) in D:\\d\\Umbraco 7.7\\src\\Umbraco.Core\\Persistence\\PetaPoco.cs:line 929
   at Umbraco.Core.Persistence.Repositories.UserRepository.PerformGetByQuery(IQuery`1 query) in D:\\d\\Umbraco 7.7\\src\\Umbraco.Core\\Persistence\\Repositories\\UserRepository.cs:line 216
   at Umbraco.Core.Persistence.Repositories.RepositoryBase`2.GetByQuery(IQuery`1 query) in D:\\d\\Umbraco 7.7\\src\\Umbraco.Core\\Persistence\\Repositories\\RepositoryBase.cs:line 278
   at Umbraco.Core.Services.UserService.GetByEmail(String email) in D:\\d\\Umbraco 7.7\\src\\Umbraco.Core\\Services\\UserService.cs:line 189
   at Umbraco.Core.Security.BackOfficeUserStore.FindByEmailAsync(String email) in D:\\d\\Umbraco 7.7\\src\\Umbraco.Core\\Security\\BackOfficeUserStore.cs:line 303
   at Microsoft.AspNet.Identity.UserValidator`2.<ValidateEmailAsync>d__8.MoveNext()

which seems to come from BackOfficeUserValidator, and BackOfficeSignInManager, and ultimately AutenticationController.PostLogin.

Will try again - but if you can test again too?

Stephan 27 Jul 2017, 16:11:27

Would be in BackOfficeSignInManager.SignInAsync when doing UserManager.UpdateAsync(user) to track the last login date ... ends up validating the user ... and trying to find the user by email ... looking at the PR but getting slightly confused ... would love if you can test again?

Shannon Deminick 28 Jul 2017, 02:20:34

Have verified the problem there. The reason you encountered this problem was because you were entering the wrong password - but good catch! I've updated the PR so now it won't try to validate the user if it's just updating it's AccessFailedCount

Stephan 28 Jul 2017, 07:54:42

Re-tested OK + the avatar thing still works, merging.

Priority: Normal

Type: Task

State: Fixed


Difficulty: Normal


Backwards Compatible: True

Fix Submitted:

Affected versions:

Due in version: 7.7.0

Sprint: Sprint 64

Story Points: 3