[MM-51468] import: allow usernames to be uppercase (#29530)

server/channels/app/import_functions_test.go

@@ -748,29 +748,30 @@ func TestImportImportUser(t *testing.T) {
 	defer th.TearDown()
 
 	// Check how many users are in the database.
-	userCount, err := th.App.Srv().Store().User().Count(model.UserCountOptions{
+	userCount, cErr := th.App.Srv().Store().User().Count(model.UserCountOptions{
 		IncludeDeleted:     true,
 		IncludeBotAccounts: false,
 	})
-	require.NoError(t, err, "Failed to get user count.")
+	require.NoError(t, cErr, "Failed to get user count.")
 
-	// Do an invalid user in dry-run mode.
+	t.Run("import an invalid user in dry-run", func(t *testing.T) {
 		data := imports.UserImportData{
 			Username: model.NewPointer(model.NewUsername()),
 		}
-	err = th.App.importUser(th.Context, &data, true)
+		appErr := th.App.importUser(th.Context, &data, true)
-	require.Error(t, err, "Should have failed to import invalid user.")
+		require.NotNil(t, appErr, "Should have failed to import invalid user.")
 
 		// Check that no more users are in the DB.
-	userCount2, err := th.App.Srv().Store().User().Count(model.UserCountOptions{
+		userCountCurrent, err := th.App.Srv().Store().User().Count(model.UserCountOptions{
 			IncludeDeleted:     true,
 			IncludeBotAccounts: false,
 		})
 		require.NoError(t, err, "Failed to get user count.")
-	assert.Equal(t, userCount, userCount2, "Unexpected number of users")
+		assert.Equal(t, userCount, userCountCurrent, "Unexpected number of users")
+	})
 
-	// Do a valid user in dry-run mode.
+	t.Run("import a valid user in dry-run", func(t *testing.T) {
-	data = imports.UserImportData{
+		data := imports.UserImportData{
 			Username: model.NewPointer(model.NewUsername()),
 			Email:    model.NewPointer(model.NewId() + "@example.com"),
 		}
@@ -778,32 +779,34 @@ func TestImportImportUser(t *testing.T) {
 		require.Nil(t, appErr, "Should have succeeded to import valid user.")
 
 		// Check that no more users are in the DB.
-	userCount3, err := th.App.Srv().Store().User().Count(model.UserCountOptions{
+		userCountCurrent, err := th.App.Srv().Store().User().Count(model.UserCountOptions{
 			IncludeDeleted:     true,
 			IncludeBotAccounts: false,
 		})
 		require.NoError(t, err, "Failed to get user count.")
-	assert.Equal(t, userCount, userCount3, "Unexpected number of users")
+		assert.Equal(t, userCount, userCountCurrent, "Unexpected number of users")
+	})
 
-	// Do an invalid user in apply mode.
+	t.Run("import an invalid user in apply mode", func(t *testing.T) {
-	data = imports.UserImportData{
+		data := imports.UserImportData{
 			Username: model.NewPointer(model.NewUsername()),
 		}
-	err = th.App.importUser(th.Context, &data, false)
+		appErr := th.App.importUser(th.Context, &data, false)
-	require.Error(t, err, "Should have failed to import invalid user.")
+		require.NotNil(t, appErr, "Should have failed to import invalid user.")
 
 		// Check that no more users are in the DB.
-	userCount4, err := th.App.Srv().Store().User().Count(model.UserCountOptions{
+		userCountCurrent, err := th.App.Srv().Store().User().Count(model.UserCountOptions{
 			IncludeDeleted:     true,
 			IncludeBotAccounts: false,
 		})
 		require.NoError(t, err, "Failed to get user count.")
-	assert.Equal(t, userCount, userCount4, "Unexpected number of users")
+		assert.Equal(t, userCount, userCountCurrent, "Unexpected number of users")
+	})
 
-	// Do a valid user in apply mode.
+	t.Run("import a valid user in apply mode", func(t *testing.T) {
-	username := model.NewUsername()
+		username := "A" + model.NewUsername()[1:]
 		testsDir, _ := fileutils.FindDir("tests")
-	data = imports.UserImportData{
+		data := imports.UserImportData{
 			Avatar: imports.Avatar{
 				ProfileImage: model.NewPointer(filepath.Join(testsDir, "test.png")),
 			},
@@ -814,36 +817,63 @@ func TestImportImportUser(t *testing.T) {
 			LastName:  model.NewPointer(model.NewId()),
 			Position:  model.NewPointer(model.NewId()),
 		}
-	appErr = th.App.importUser(th.Context, &data, false)
+		appErr := th.App.importUser(th.Context, &data, false)
 		require.Nil(t, appErr, "Should have succeeded to import valid user.")
 
 		// Check that one more user is in the DB.
-	userCount5, err := th.App.Srv().Store().User().Count(model.UserCountOptions{
+		userCountCurrent, err := th.App.Srv().Store().User().Count(model.UserCountOptions{
 			IncludeDeleted:     true,
 			IncludeBotAccounts: false,
 		})
 		require.NoError(t, err, "Failed to get user count.")
-	assert.Equal(t, userCount+1, userCount5, "Unexpected number of users")
+		userCount++ // Increment the user count.
+		assert.Equal(t, userCount, userCountCurrent, "Unexpected number of users")
 
 		// Get the user and check all the fields are correct.
 		user, err2 := th.App.GetUserByUsername(username)
 		require.Nil(t, err2, "Failed to get user from database.")
 
-	userBool := user.Email != *data.Email || user.Nickname != *data.Nickname || user.FirstName != *data.FirstName || user.LastName != *data.LastName || user.Position != *data.Position
+		assert.Equal(t, *data.Email, user.Email)
-	require.False(t, userBool, "User properties do not match Import Data.")
+		assert.Equal(t, *data.Nickname, user.Nickname)
+		assert.Equal(t, *data.FirstName, user.FirstName)
+		assert.Equal(t, *data.LastName, user.LastName)
+		assert.Equal(t, *data.Position, user.Position)
 
 		// Check calculated properties.
+		require.Equal(t, strings.ToLower(username), user.Username, "Expected Username to be lower case.")
 		require.Empty(t, user.AuthService, "Expected Auth Service to be empty.")
-
 		require.Empty(t, user.AuthData, "Expected AuthData to be empty.")
-
 		require.NotEmpty(t, user.Password, "Expected password to be set.")
-
 		require.True(t, user.EmailVerified, "Expected EmailVerified to be true.")
-
 		require.Equal(t, user.Locale, *th.App.Config().LocalizationSettings.DefaultClientLocale, "Expected Locale to be the default.")
-
 		require.Equal(t, user.Roles, "system_user", "Expected roles to be system_user")
+	})
+
+	t.Run("import a valid user where there is an existing user", func(t *testing.T) {
+		username := model.NewUsername()
+		testsDir, _ := fileutils.FindDir("tests")
+		data := imports.UserImportData{
+			Avatar: imports.Avatar{
+				ProfileImage: model.NewPointer(filepath.Join(testsDir, "test.png")),
+			},
+			Username:  &username,
+			Email:     model.NewPointer(model.NewId() + "@example.com"),
+			Nickname:  model.NewPointer(model.NewId()),
+			FirstName: model.NewPointer(model.NewId()),
+			LastName:  model.NewPointer(model.NewId()),
+			Position:  model.NewPointer(model.NewId()),
+		}
+		appErr := th.App.importUser(th.Context, &data, false)
+		require.Nil(t, appErr, "Should have succeeded to import valid user.")
+
+		// Check that one more user is in the DB.
+		userCountCurrent, err := th.App.Srv().Store().User().Count(model.UserCountOptions{
+			IncludeDeleted:     true,
+			IncludeBotAccounts: false,
+		})
+		require.NoError(t, err, "Failed to get user count.")
+		userCount++ // Increment the user count.
+		assert.Equal(t, userCount, userCountCurrent, "Unexpected number of users")
 
 		// Alter all the fields of that user.
 		data.Email = model.NewPointer(model.NewId() + "@example.com")
@@ -861,31 +891,58 @@ func TestImportImportUser(t *testing.T) {
 		require.Nil(t, appErr, "Should have succeeded to update valid user %v", err)
 
 		// Check user count the same.
-	userCount6, err := th.App.Srv().Store().User().Count(model.UserCountOptions{
+		userCountCurrent, err = th.App.Srv().Store().User().Count(model.UserCountOptions{
 			IncludeDeleted:     true,
 			IncludeBotAccounts: false,
 		})
 		require.NoError(t, err, "Failed to get user count.")
-	assert.Equal(t, userCount+1, userCount6, "Unexpected number of users")
+		assert.Equal(t, userCount, userCountCurrent, "Unexpected number of users")
 
 		// Get the user and check all the fields are correct.
-	user, err2 = th.App.GetUserByUsername(username)
+		user, err2 := th.App.GetUserByUsername(username)
 		require.Nil(t, err2, "Failed to get user from database.")
 
-	userBool = user.Email != *data.Email || user.Nickname != *data.Nickname || user.FirstName != *data.FirstName || user.LastName != *data.LastName || user.Position != *data.Position
+		assert.Equal(t, *data.Email, user.Email)
-	require.False(t, userBool, "Updated User properties do not match Import Data.")
+		assert.Equal(t, *data.Nickname, user.Nickname)
+		assert.Equal(t, *data.FirstName, user.FirstName)
+		assert.Equal(t, *data.LastName, user.LastName)
+		assert.Equal(t, *data.Position, user.Position)
 
 		require.Equal(t, "ldap", user.AuthService, "Expected Auth Service to be ldap \"%v\"", user.AuthService)
-
 		require.Equal(t, user.AuthData, data.AuthData, "Expected AuthData to be set.")
-
 		require.Empty(t, user.Password, "Expected password to be empty.")
-
 		require.True(t, user.EmailVerified, "Expected EmailVerified to be true.")
-
 		require.Equal(t, *data.Locale, user.Locale, "Expected Locale to be the set.")
-
 		require.Equal(t, *data.Roles, user.Roles, "Expected roles to be set: %v", user.Roles)
+	})
+
+	t.Run("import invalid fields", func(t *testing.T) {
+		username := model.NewUsername()
+		testsDir, _ := fileutils.FindDir("tests")
+		data := imports.UserImportData{
+			Avatar: imports.Avatar{
+				ProfileImage: model.NewPointer(filepath.Join(testsDir, "test.png")),
+			},
+			Username:    &username,
+			Email:       model.NewPointer(model.NewId() + "@example.com"),
+			Nickname:    model.NewPointer(model.NewId()),
+			FirstName:   model.NewPointer(model.NewId()),
+			LastName:    model.NewPointer(model.NewId()),
+			Position:    model.NewPointer(model.NewId()),
+			AuthData:    &username,
+			AuthService: model.NewPointer("ldap"),
+		}
+		appErr := th.App.importUser(th.Context, &data, false)
+		require.Nil(t, appErr, "Should have succeeded to import valid user.")
+
+		// Check that one more user is in the DB.
+		userCountCurrent, err := th.App.Srv().Store().User().Count(model.UserCountOptions{
+			IncludeDeleted:     true,
+			IncludeBotAccounts: false,
+		})
+		require.NoError(t, err, "Failed to get user count.")
+		userCount++ // Increment the user count.
+		assert.Equal(t, userCount, userCountCurrent, "Unexpected number of users")
 
 		// Check Password and AuthData together.
 		data.Password = model.NewPointer("PasswordTest")
@@ -905,16 +962,23 @@ func TestImportImportUser(t *testing.T) {
 		appErr = th.App.importUser(th.Context, &data, false)
 		require.NotNil(t, appErr, "Should have failed to import invalid user.")
 
-	data.Password = model.NewPointer("TestPassword")
+		// Check that no more user is in the DB.
+		userCountCurrent, err = th.App.Srv().Store().User().Count(model.UserCountOptions{
+			IncludeDeleted:     true,
+			IncludeBotAccounts: false,
+		})
+		require.NoError(t, err, "Failed to get user count.")
+		assert.Equal(t, userCount, userCountCurrent, "Unexpected number of users")
+	})
 
-	// Test team and channel memberships
+	t.Run("import with team and channel memberships", func(t *testing.T) {
 		teamName := model.NewRandomTeamName()
-	appErr = th.App.importTeam(th.Context, &imports.TeamImportData{
+		tAppErr := th.App.importTeam(th.Context, &imports.TeamImportData{
 			Name:        &teamName,
 			DisplayName: model.NewPointer("Display Name"),
 			Type:        model.NewPointer("O"),
 		}, false)
-	require.Nil(t, appErr, "Failed to import team.")
+		require.Nil(t, tAppErr, "Failed to import team.")
 		team, appErr := th.App.GetTeamByName(teamName)
 		require.Nil(t, appErr, "Failed to get team from database.")
 
@@ -930,8 +994,8 @@ func TestImportImportUser(t *testing.T) {
 		channel, appErr := th.App.GetChannelByName(th.Context, channelName, team.Id, false)
 		require.Nil(t, appErr, "Failed to get channel from database.")
 
-	username = model.NewUsername()
+		username := model.NewUsername()
-	data = imports.UserImportData{
+		data := imports.UserImportData{
 			Username:  &username,
 			Email:     model.NewPointer(model.NewId() + "@example.com"),
 			Nickname:  model.NewPointer(model.NewId()),
@@ -947,7 +1011,7 @@ func TestImportImportUser(t *testing.T) {
 		channelMemberCount, appErr := th.App.GetChannelMemberCount(th.Context, channel.Id)
 		require.Nil(t, appErr, "Failed to get channel member count")
 
-	// Test with an invalid team & channel membership in dry-run mode.
+		t.Run("invalid team and channel memberships in dry-run mode", func(t *testing.T) {
 			data.Teams = &[]imports.UserTeamImportData{
 				{
 					Roles: model.NewPointer("invalid"),
@@ -960,8 +1024,9 @@ func TestImportImportUser(t *testing.T) {
 			}
 			appErr = th.App.importUser(th.Context, &data, true)
 			assert.NotNil(t, appErr)
+		})
 
-	// Test with an unknown team name & invalid channel membership in dry-run mode.
+		t.Run("unknown team name & invalid channel membership in dry-run mode", func(t *testing.T) {
 			data.Teams = &[]imports.UserTeamImportData{
 				{
 					Name: model.NewPointer(model.NewId()),
@@ -974,8 +1039,9 @@ func TestImportImportUser(t *testing.T) {
 			}
 			appErr = th.App.importUser(th.Context, &data, true)
 			assert.NotNil(t, appErr)
+		})
 
-	// Test with a valid team & invalid channel membership in dry-run mode.
+		t.Run("valid team & invalid channel membership in dry-run mode", func(t *testing.T) {
 			data.Teams = &[]imports.UserTeamImportData{
 				{
 					Name: &teamName,
@@ -988,8 +1054,9 @@ func TestImportImportUser(t *testing.T) {
 			}
 			appErr = th.App.importUser(th.Context, &data, true)
 			assert.NotNil(t, appErr)
+		})
 
-	// Test with a valid team & unknown channel name in dry-run mode.
+		t.Run("valid team & unknown channel name in dry-run mode", func(t *testing.T) {
 			data.Teams = &[]imports.UserTeamImportData{
 				{
 					Name: &teamName,
@@ -1002,8 +1069,9 @@ func TestImportImportUser(t *testing.T) {
 			}
 			appErr = th.App.importUser(th.Context, &data, true)
 			assert.Nil(t, appErr)
+		})
 
-	// Test with a valid team & valid channel name in dry-run mode.
+		t.Run("valid team & valid channel name in dry-run mode", func(t *testing.T) {
 			data.Teams = &[]imports.UserTeamImportData{
 				{
 					Name: &teamName,
@@ -1018,15 +1086,16 @@ func TestImportImportUser(t *testing.T) {
 			assert.Nil(t, appErr)
 
 			// Check no new member objects were created because dry run mode.
-	tmc, appErr := th.App.GetTeamMembers(team.Id, 0, 1000, nil)
+			tmc, appErr2 := th.App.GetTeamMembers(team.Id, 0, 1000, nil)
-	require.Nil(t, appErr, "Failed to get Team Member Count")
+			require.Nil(t, appErr2, "Failed to get Team Member Count")
 			require.Len(t, tmc, teamMemberCount, "Number of team members not as expected")
 
-	cmc, appErr := th.App.GetChannelMemberCount(th.Context, channel.Id)
+			cmc, appErr2 := th.App.GetChannelMemberCount(th.Context, channel.Id)
-	require.Nil(t, appErr, "Failed to get Channel Member Count")
+			require.Nil(t, appErr2, "Failed to get Channel Member Count")
 			require.Equal(t, channelMemberCount, cmc, "Number of channel members not as expected")
+		})
 
-	// Test with an invalid team & channel membership in apply mode.
+		t.Run("invalid team & channel membership in apply mode", func(t *testing.T) {
 			data.Teams = &[]imports.UserTeamImportData{
 				{
 					Roles: model.NewPointer("invalid"),
@@ -1039,8 +1108,9 @@ func TestImportImportUser(t *testing.T) {
 			}
 			appErr = th.App.importUser(th.Context, &data, false)
 			assert.NotNil(t, appErr)
+		})
 
-	// Test with an unknown team name & invalid channel membership in apply mode.
+		t.Run("unknown team name & invalid channel membership in apply mode", func(t *testing.T) {
 			data.Teams = &[]imports.UserTeamImportData{
 				{
 					Name: model.NewPointer(model.NewId()),
@@ -1053,8 +1123,9 @@ func TestImportImportUser(t *testing.T) {
 			}
 			appErr = th.App.importUser(th.Context, &data, false)
 			assert.NotNil(t, appErr)
+		})
 
-	// Test with a valid team & invalid channel membership in apply mode.
+		t.Run("import with valid team and invalid channel memberships in apply mode", func(t *testing.T) {
 			data.Teams = &[]imports.UserTeamImportData{
 				{
 					Name: &teamName,
@@ -1069,15 +1140,16 @@ func TestImportImportUser(t *testing.T) {
 			assert.NotNil(t, appErr)
 
 			// Check no new member objects were created because all tests should have failed so far.
-	tmc, appErr = th.App.GetTeamMembers(team.Id, 0, 1000, nil)
+			tmc, appErr2 := th.App.GetTeamMembers(team.Id, 0, 1000, nil)
-	require.Nil(t, appErr, "Failed to get Team Member Count")
+			require.Nil(t, appErr2, "Failed to get Team Member Count")
 			require.Len(t, tmc, teamMemberCount)
 
-	cmc, appErr = th.App.GetChannelMemberCount(th.Context, channel.Id)
+			cmc, appErr2 := th.App.GetChannelMemberCount(th.Context, channel.Id)
-	require.Nil(t, appErr, "Failed to get Channel Member Count")
+			require.Nil(t, appErr2, "Failed to get Channel Member Count")
 			require.Equal(t, channelMemberCount, cmc)
+		})
 
-	// Test with a valid team & unknown channel name in apply mode.
+		t.Run("valid team & unknown channel name in apply mode", func(t *testing.T) {
 			data.Teams = &[]imports.UserTeamImportData{
 				{
 					Name: &teamName,
@@ -1092,23 +1164,25 @@ func TestImportImportUser(t *testing.T) {
 			assert.NotNil(t, appErr)
 
 			// Check only new team member object created because dry run mode.
-	tmc, appErr = th.App.GetTeamMembers(team.Id, 0, 1000, nil)
+			tmc, appErr2 := th.App.GetTeamMembers(team.Id, 0, 1000, nil)
-	require.Nil(t, appErr, "Failed to get Team Member Count")
+			require.Nil(t, appErr2, "Failed to get Team Member Count")
-	require.Len(t, tmc, teamMemberCount+1)
+			teamMemberCount++
+			require.Len(t, tmc, teamMemberCount)
 
-	cmc, appErr = th.App.GetChannelMemberCount(th.Context, channel.Id)
+			cmc, appErr2 := th.App.GetChannelMemberCount(th.Context, channel.Id)
-	require.Nil(t, appErr, "Failed to get Channel Member Count")
+			require.Nil(t, appErr2, "Failed to get Channel Member Count")
 			require.Equal(t, channelMemberCount, cmc)
 
 			// Check team member properties.
-	user, appErr = th.App.GetUserByUsername(username)
+			user, appErr2 := th.App.GetUserByUsername(username)
-	require.Nil(t, appErr, "Failed to get user from database.")
+			require.Nil(t, appErr2, "Failed to get user from database.")
 
-	teamMember, appErr := th.App.GetTeamMember(th.Context, team.Id, user.Id)
+			teamMember, appErr2 := th.App.GetTeamMember(th.Context, team.Id, user.Id)
-	require.Nil(t, appErr, "Failed to get team member from database.")
+			require.Nil(t, appErr2, "Failed to get team member from database.")
 			require.Equal(t, "team_user", teamMember.Roles)
+		})
 
-	// Test with a valid team & valid channel name in apply mode.
+		t.Run("valid team & valid channel name in apply mode", func(t *testing.T) {
 			data.Teams = &[]imports.UserTeamImportData{
 				{
 					Name: &teamName,
@@ -1119,27 +1193,38 @@ func TestImportImportUser(t *testing.T) {
 					},
 				},
 			}
-	appErr = th.App.importUser(th.Context, &data, false)
+
-	assert.Nil(t, appErr)
+			// convert to a new user
+			username = model.NewUsername()
+			data.Username = &username
+			data.Email = model.NewPointer(model.NewId() + "@example.com")
+			appErr2 := th.App.importUser(th.Context, &data, false)
+			assert.Nil(t, appErr2)
 
 			// Check only new channel member object created because dry run mode.
-	tmc, appErr = th.App.GetTeamMembers(team.Id, 0, 1000, nil)
+			tmc, appErr2 := th.App.GetTeamMembers(team.Id, 0, 1000, nil)
-	require.Nil(t, appErr, "Failed to get Team Member Count")
+			require.Nil(t, appErr2, "Failed to get Team Member Count")
-	require.Len(t, tmc, teamMemberCount+1, "Number of team members not as expected")
+			teamMemberCount++
+			require.Len(t, tmc, teamMemberCount, "Number of team members not as expected")
 
-	cmc, appErr = th.App.GetChannelMemberCount(th.Context, channel.Id)
+			cmc, appErr2 := th.App.GetChannelMemberCount(th.Context, channel.Id)
-	require.Nil(t, appErr, "Failed to get Channel Member Count")
+			require.Nil(t, appErr2, "Failed to get Channel Member Count")
-	require.Equal(t, channelMemberCount+1, cmc, "Number of channel members not as expected")
+			channelMemberCount++
+			require.Equal(t, channelMemberCount, cmc, "Number of channel members not as expected")
+
+			user, err2 := th.App.GetUserByUsername(username)
+			require.Nil(t, err2, "Failed to get user from database.")
 
 			// Check channel member properties.
-	channelMember, appErr := th.App.GetChannelMember(th.Context, channel.Id, user.Id)
+			channelMember, appErr2 := th.App.GetChannelMember(th.Context, channel.Id, user.Id)
-	require.Nil(t, appErr, "Failed to get channel member from database.")
+			require.Nil(t, appErr2, "Failed to get channel member from database.")
 			assert.Equal(t, "channel_user", channelMember.Roles)
 			assert.Equal(t, "default", channelMember.NotifyProps[model.DesktopNotifyProp])
 			assert.Equal(t, "default", channelMember.NotifyProps[model.PushNotifyProp])
 			assert.Equal(t, "all", channelMember.NotifyProps[model.MarkUnreadNotifyProp])
+		})
 
-	// Test with the properties of the team and channel membership changed.
+		t.Run("test with the properties of the team and channel membership changed", func(t *testing.T) {
 			data.Teams = &[]imports.UserTeamImportData{
 				{
 					Name:  &teamName,
@@ -1159,16 +1244,25 @@ func TestImportImportUser(t *testing.T) {
 					},
 				},
 			}
-	appErr = th.App.importUser(th.Context, &data, false)
+
-	assert.Nil(t, appErr)
+			// convert to a new user
+			username = model.NewUsername()
+			data.Username = &username
+			data.Email = model.NewPointer(model.NewId() + "@example.com")
+
+			appErr2 := th.App.importUser(th.Context, &data, false)
+			assert.Nil(t, appErr2)
+
+			user, err2 := th.App.GetUserByUsername(username)
+			require.Nil(t, err2, "Failed to get user from database.")
 
 			// Check both member properties.
-	teamMember, appErr = th.App.GetTeamMember(th.Context, team.Id, user.Id)
+			teamMember, appErr2 := th.App.GetTeamMember(th.Context, team.Id, user.Id)
-	require.Nil(t, appErr, "Failed to get team member from database.")
+			require.Nil(t, appErr2, "Failed to get team member from database.")
 			require.Equal(t, "team_user team_admin", teamMember.Roles)
 
-	channelMember, appErr = th.App.GetChannelMember(th.Context, channel.Id, user.Id)
+			channelMember, appErr2 := th.App.GetChannelMember(th.Context, channel.Id, user.Id)
-	require.Nil(t, appErr, "Failed to get channel member Desktop from database.")
+			require.Nil(t, appErr2, "Failed to get channel member Desktop from database.")
 			assert.Equal(t, "channel_user channel_admin", channelMember.Roles)
 			assert.Equal(t, model.UserNotifyMention, channelMember.NotifyProps[model.DesktopNotifyProp])
 			assert.Equal(t, model.UserNotifyMention, channelMember.NotifyProps[model.PushNotifyProp])
@@ -1178,17 +1272,37 @@ func TestImportImportUser(t *testing.T) {
 			checkPreference(t, th.App, user.Id, model.PreferenceCategoryTheme, team.Id, *(*data.Teams)[0].Theme)
 
 			// No more new member objects.
-	tmc, appErr = th.App.GetTeamMembers(team.Id, 0, 1000, nil)
+			tmc, appErr2 := th.App.GetTeamMembers(team.Id, 0, 1000, nil)
-	require.Nil(t, appErr, "Failed to get Team Member Count")
+			require.Nil(t, appErr2, "Failed to get Team Member Count")
 			require.Len(t, tmc, teamMemberCount+1, "Number of team members not as expected")
 
-	cmc, appErr = th.App.GetChannelMemberCount(th.Context, channel.Id)
+			cmc, appErr2 := th.App.GetChannelMemberCount(th.Context, channel.Id)
-	require.Nil(t, appErr, "Failed to get Channel Member Count")
+			require.Nil(t, appErr2, "Failed to get Channel Member Count")
 			require.Equal(t, channelMemberCount+1, cmc, "Number of channel members not as expected")
+		})
+	})
 
-	// Add a user with some preferences.
+	t.Run("add a user with some preferences.", func(t *testing.T) {
-	username = model.NewUsername()
+		teamName := model.NewRandomTeamName()
-	data = imports.UserImportData{
+		appErr2 := th.App.importTeam(th.Context, &imports.TeamImportData{
+			Name:        &teamName,
+			DisplayName: model.NewPointer("Display Name"),
+			Type:        model.NewPointer("O"),
+		}, false)
+		require.Nil(t, appErr2, "Failed to import team.")
+
+		channelName := model.NewId()
+		chanTypeOpen := model.ChannelTypeOpen
+		appErr2 = th.App.importChannel(th.Context, &imports.ChannelImportData{
+			Team:        &teamName,
+			Name:        &channelName,
+			DisplayName: model.NewPointer("Display Name"),
+			Type:        &chanTypeOpen,
+		}, false)
+		require.Nil(t, appErr2, "Failed to import channel.")
+
+		username := model.NewUsername()
+		data := imports.UserImportData{
 			Username:                 &username,
 			Email:                    model.NewPointer(model.NewId() + "@example.com"),
 			Theme:                    model.NewPointer(`{"awayIndicator":"#DCBD4E","buttonBg":"#23A2FF","buttonColor":"#FFFFFF","centerChannelBg":"#ffffff","centerChannelColor":"#333333","codeTheme":"github","image":"/static/files/a4a388b38b32678e83823ef1b3e17766.png","linkColor":"#2389d7","mentionBg":"#2389d7","mentionColor":"#ffffff","mentionHighlightBg":"#fff2bb","mentionHighlightLink":"#2f81b7","newMessageSeparator":"#FF8800","onlineIndicator":"#7DBE00","sidebarBg":"#fafafa","sidebarHeaderBg":"#3481B9","sidebarHeaderTextColor":"#ffffff","sidebarText":"#333333","sidebarTextActiveBorder":"#378FD2","sidebarTextActiveColor":"#111111","sidebarTextHoverBg":"#e6f2fa","sidebarUnreadText":"#333333","type":"Mattermost"}`),
@@ -1210,12 +1324,12 @@ func TestImportImportUser(t *testing.T) {
 			ShowUnreadScrollPosition: model.NewPointer("start_from_newest"),
 			LimitVisibleDmsGms:       model.NewPointer("20"),
 		}
-	appErr = th.App.importUser(th.Context, &data, false)
+		appErr2 = th.App.importUser(th.Context, &data, false)
-	assert.Nil(t, appErr)
+		assert.Nil(t, appErr2)
 
 		// Check their values.
-	user, appErr = th.App.GetUserByUsername(username)
+		user, appErr2 := th.App.GetUserByUsername(username)
-	require.Nil(t, appErr, "Failed to get user from database.")
+		require.Nil(t, appErr2, "Failed to get user from database.")
 
 		checkPreference(t, th.App, user.Id, model.PreferenceCategoryTheme, "", *data.Theme)
 		checkPreference(t, th.App, user.Id, model.PreferenceCategoryDisplaySettings, model.PreferenceNameUseMilitaryTime, *data.UseMilitaryTime)
@@ -1248,8 +1362,8 @@ func TestImportImportUser(t *testing.T) {
 			TutorialStep:       model.NewPointer("2"),
 			EmailInterval:      model.NewPointer("hour"),
 		}
-	appErr = th.App.importUser(th.Context, &data, false)
+		appErr2 = th.App.importUser(th.Context, &data, false)
-	assert.Nil(t, appErr)
+		assert.Nil(t, appErr2)
 
 		// Check their values again.
 		checkPreference(t, th.App, user.Id, model.PreferenceCategoryTheme, "", *data.Theme)
@@ -1271,11 +1385,11 @@ func TestImportImportUser(t *testing.T) {
 			ChannelTrigger:   model.NewPointer("true"),
 			CommentsTrigger:  model.NewPointer(model.CommentsNotifyRoot),
 		}
-	appErr = th.App.importUser(th.Context, &data, false)
+		appErr2 = th.App.importUser(th.Context, &data, false)
-	assert.Nil(t, appErr)
+		assert.Nil(t, appErr2)
 
-	user, appErr = th.App.GetUserByUsername(username)
+		user, appErr2 = th.App.GetUserByUsername(username)
-	require.Nil(t, appErr, "Failed to get user from database.")
+		require.Nil(t, appErr2, "Failed to get user from database.")
 
 		checkNotifyProp(t, user, model.DesktopNotifyProp, model.UserNotifyAll)
 		checkNotifyProp(t, user, model.DesktopSoundNotifyProp, "true")
@@ -1297,11 +1411,11 @@ func TestImportImportUser(t *testing.T) {
 			CommentsTrigger:  model.NewPointer(model.CommentsNotifyRoot),
 			MentionKeys:      model.NewPointer("valid,misc"),
 		}
-	appErr = th.App.importUser(th.Context, &data, false)
+		appErr2 = th.App.importUser(th.Context, &data, false)
-	assert.Nil(t, appErr)
+		assert.Nil(t, appErr2)
 
-	user, appErr = th.App.GetUserByUsername(username)
+		user, appErr2 = th.App.GetUserByUsername(username)
-	require.Nil(t, appErr, "Failed to get user from database.")
+		require.Nil(t, appErr2, "Failed to get user from database.")
 
 		checkNotifyProp(t, user, model.DesktopNotifyProp, model.UserNotifyAll)
 		checkNotifyProp(t, user, model.DesktopSoundNotifyProp, "true")
@@ -1323,11 +1437,11 @@ func TestImportImportUser(t *testing.T) {
 			CommentsTrigger:  model.NewPointer(model.CommentsNotifyAny),
 			MentionKeys:      model.NewPointer("misc"),
 		}
-	appErr = th.App.importUser(th.Context, &data, false)
+		appErr2 = th.App.importUser(th.Context, &data, false)
-	assert.Nil(t, appErr)
+		assert.Nil(t, appErr2)
 
-	user, appErr = th.App.GetUserByUsername(username)
+		user, appErr2 = th.App.GetUserByUsername(username)
-	require.Nil(t, appErr, "Failed to get user from database.")
+		require.Nil(t, appErr2, "Failed to get user from database.")
 
 		checkNotifyProp(t, user, model.DesktopNotifyProp, model.UserNotifyMention)
 		checkNotifyProp(t, user, model.DesktopSoundNotifyProp, "false")
@@ -1348,11 +1462,11 @@ func TestImportImportUser(t *testing.T) {
 			ChannelTrigger:   model.NewPointer("false"),
 			CommentsTrigger:  model.NewPointer(model.CommentsNotifyAny),
 		}
-	appErr = th.App.importUser(th.Context, &data, false)
+		appErr2 = th.App.importUser(th.Context, &data, false)
-	assert.Nil(t, appErr)
+		assert.Nil(t, appErr2)
 
-	user, appErr = th.App.GetUserByUsername(username)
+		user, appErr2 = th.App.GetUserByUsername(username)
-	require.Nil(t, appErr, "Failed to get user from database.")
+		require.Nil(t, appErr2, "Failed to get user from database.")
 
 		checkNotifyProp(t, user, model.DesktopNotifyProp, model.UserNotifyMention)
 		checkNotifyProp(t, user, model.DesktopSoundNotifyProp, "false")
@@ -1380,11 +1494,11 @@ func TestImportImportUser(t *testing.T) {
 			MentionKeys:      model.NewPointer("misc"),
 		}
 
-	appErr = th.App.importUser(th.Context, &data, false)
+		appErr2 = th.App.importUser(th.Context, &data, false)
-	assert.Nil(t, appErr)
+		assert.Nil(t, appErr2)
 
-	user, appErr = th.App.GetUserByUsername(username)
+		user, appErr2 = th.App.GetUserByUsername(username)
-	require.Nil(t, appErr, "Failed to get user from database.")
+		require.Nil(t, appErr2, "Failed to get user from database.")
 
 		checkNotifyProp(t, user, model.DesktopNotifyProp, model.UserNotifyMention)
 		checkNotifyProp(t, user, model.DesktopSoundNotifyProp, "false")
@@ -1400,7 +1514,7 @@ func TestImportImportUser(t *testing.T) {
 		// to the appropriate scheme-managed-role booleans.
 
 		// Mark the phase 2 permissions migration as completed.
-	err = th.App.Srv().Store().System().Save(&model.System{Name: model.MigrationKeyAdvancedPermissionsPhase2, Value: "true"})
+		err := th.App.Srv().Store().System().Save(&model.System{Name: model.MigrationKeyAdvancedPermissionsPhase2, Value: "true"})
 		require.NoError(t, err)
 
 		defer func() {
@@ -1439,8 +1553,8 @@ func TestImportImportUser(t *testing.T) {
 			Description: model.NewPointer("description"),
 		}
 
-	appErr = th.App.importScheme(th.Context, teamSchemeData, false)
+		appErr2 = th.App.importScheme(th.Context, teamSchemeData, false)
-	assert.Nil(t, appErr)
+		assert.Nil(t, appErr2)
 
 		teamScheme, nErr := th.App.Srv().Store().Scheme().GetByName(*teamSchemeData.Name)
 		require.NoError(t, nErr, "Failed to import scheme")
@@ -1453,10 +1567,10 @@ func TestImportImportUser(t *testing.T) {
 			AllowOpenInvite: model.NewPointer(true),
 			Scheme:          &teamScheme.Name,
 		}
-	appErr = th.App.importTeam(th.Context, teamData, false)
+		appErr2 = th.App.importTeam(th.Context, teamData, false)
-	assert.Nil(t, appErr)
+		assert.Nil(t, appErr2)
-	team, appErr = th.App.GetTeamByName(teamName)
+		team, appErr2 := th.App.GetTeamByName(teamName)
-	require.Nil(t, appErr, "Failed to get team from database.")
+		require.Nil(t, appErr2, "Failed to get team from database.")
 
 		channelData := &imports.ChannelImportData{
 			Team:        &teamName,
@@ -1466,10 +1580,10 @@ func TestImportImportUser(t *testing.T) {
 			Header:      model.NewPointer("Channel Header"),
 			Purpose:     model.NewPointer("Channel Purpose"),
 		}
-	appErr = th.App.importChannel(th.Context, channelData, false)
+		appErr2 = th.App.importChannel(th.Context, channelData, false)
-	assert.Nil(t, appErr)
+		assert.Nil(t, appErr2)
-	channel, appErr = th.App.GetChannelByName(th.Context, *channelData.Name, team.Id, false)
+		channel, appErr2 := th.App.GetChannelByName(th.Context, *channelData.Name, team.Id, false)
-	require.Nil(t, appErr, "Failed to get channel from database")
+		require.Nil(t, appErr2, "Failed to get channel from database")
 
 		// Test with a valid team & valid channel name in apply mode.
 		userData := &imports.UserImportData{
@@ -1488,22 +1602,22 @@ func TestImportImportUser(t *testing.T) {
 				},
 			},
 		}
-	appErr = th.App.importUser(th.Context, userData, false)
+		appErr2 = th.App.importUser(th.Context, userData, false)
-	assert.Nil(t, appErr)
+		assert.Nil(t, appErr2)
 
-	user, appErr = th.App.GetUserByUsername(*userData.Username)
+		user, appErr2 = th.App.GetUserByUsername(*userData.Username)
-	require.Nil(t, appErr, "Failed to get user from database.")
+		require.Nil(t, appErr2, "Failed to get user from database.")
 
-	teamMember, appErr = th.App.GetTeamMember(th.Context, team.Id, user.Id)
+		teamMember, appErr2 := th.App.GetTeamMember(th.Context, team.Id, user.Id)
-	require.Nil(t, appErr, "Failed to get the team member")
+		require.Nil(t, appErr2, "Failed to get the team member")
 
 		assert.True(t, teamMember.SchemeAdmin)
 		assert.True(t, teamMember.SchemeUser)
 		assert.False(t, teamMember.SchemeGuest)
 		assert.Equal(t, "", teamMember.ExplicitRoles)
 
-	channelMember, appErr = th.App.GetChannelMember(th.Context, channel.Id, user.Id)
+		channelMember, appErr2 := th.App.GetChannelMember(th.Context, channel.Id, user.Id)
-	require.Nil(t, appErr, "Failed to get the channel member")
+		require.Nil(t, appErr2, "Failed to get the channel member")
 
 		assert.True(t, channelMember.SchemeAdmin)
 		assert.True(t, channelMember.SchemeUser)
@@ -1530,27 +1644,28 @@ func TestImportImportUser(t *testing.T) {
 				},
 			},
 		}
-	appErr = th.App.importUser(th.Context, deletedUserData, false)
+		appErr2 = th.App.importUser(th.Context, deletedUserData, false)
-	assert.Nil(t, appErr)
+		assert.Nil(t, appErr2)
 
-	user, appErr = th.App.GetUserByUsername(*deletedUserData.Username)
+		user, appErr2 = th.App.GetUserByUsername(*deletedUserData.Username)
-	require.Nil(t, appErr, "Failed to get user from database.")
+		require.Nil(t, appErr2, "Failed to get user from database.")
 
-	teamMember, appErr = th.App.GetTeamMember(th.Context, team.Id, user.Id)
+		teamMember, appErr2 = th.App.GetTeamMember(th.Context, team.Id, user.Id)
-	require.Nil(t, appErr, "Failed to get the team member")
+		require.Nil(t, appErr2, "Failed to get the team member")
 
 		assert.False(t, teamMember.SchemeAdmin)
 		assert.True(t, teamMember.SchemeUser)
 		assert.False(t, teamMember.SchemeGuest)
 		assert.Equal(t, "", teamMember.ExplicitRoles)
 
-	channelMember, appErr = th.App.GetChannelMember(th.Context, channel.Id, user.Id)
+		channelMember, appErr2 = th.App.GetChannelMember(th.Context, channel.Id, user.Id)
-	require.Nil(t, appErr, "Failed to get the channel member")
+		require.Nil(t, appErr2, "Failed to get the channel member")
 
 		assert.False(t, channelMember.SchemeAdmin)
 		assert.True(t, channelMember.SchemeUser)
 		assert.False(t, channelMember.SchemeGuest)
 		assert.Equal(t, "", channelMember.ExplicitRoles)
+	})
 
 	// Test importing deleted guest with a valid team & valid channel name in apply mode.
 	username = model.NewUsername()

server/channels/app/imports/import_validators.go

@@ -200,7 +200,7 @@ func ValidateUserImportData(data *UserImportData) *model.AppError {
 
 	if data.Username == nil {
 		return model.NewAppError("BulkImport", "app.import.validate_user_import_data.username_missing.error", nil, "", http.StatusBadRequest)
-	} else if !model.IsValidUsername(*data.Username) {
+	} else if !model.IsValidUsername(model.NormalizeUsername(*data.Username)) { // we already lowercase the username while saving and querying so we are more forgiving here
 		return model.NewAppError("BulkImport", "app.import.validate_user_import_data.username_invalid.error", nil, "", http.StatusBadRequest)
 	}
 
@@ -329,7 +329,7 @@ func ValidateBotImportData(data *BotImportData) *model.AppError {
 
 	if data.Username == nil {
 		return model.NewAppError("BulkImport", "app.import.validate_user_import_data.username_missing.error", nil, "", http.StatusBadRequest)
-	} else if !model.IsValidUsername(*data.Username) {
+	} else if !model.IsValidUsername(model.NormalizeUsername(*data.Username)) { // we already lowercase the username while saving and querying so we are more forgiving here
 		return model.NewAppError("BulkImport", "app.import.validate_user_import_data.username_invalid.error", nil, "", http.StatusBadRequest)
 	}
 

server/channels/app/imports/import_validators_test.go

@@ -549,6 +549,11 @@ func TestImportValidateUserImportData(t *testing.T) {
 	err = ValidateUserImportData(&data)
 	require.Nil(t, err, "Validation failed but should have been valid.")
 
+	// Test with not-all lowercase username
+	data.Username = model.NewPointer("Bob")
+	err = ValidateUserImportData(&data)
+	require.Nil(t, err, "Validation failed but should have been valid even the username has uppercase letters.")
+
 	// Test various invalid optional field values.
 	data.Nickname = model.NewPointer(strings.Repeat("abcdefghij", 7))
 	err = ValidateUserImportData(&data)
@@ -683,6 +688,11 @@ func TestImportValidateBotImportData(t *testing.T) {
 	err := ValidateBotImportData(&data)
 	require.Nil(t, err, "Validation failed but should have been valid.")
 
+	// Test with not-all lowercase username
+	data.Username = model.NewPointer("Bob")
+	err = ValidateBotImportData(&data)
+	require.Nil(t, err, "Validation failed but should have been valid even the username has uppercase letters.")
+
 	// Test with various invalid names.
 	data.Username = nil
 	err = ValidateBotImportData(&data)

server/public/model/bot.go

@@ -127,6 +127,7 @@ func (b *Bot) PreSave() {
 	b.CreateAt = GetMillis()
 	b.UpdateAt = b.CreateAt
 	b.DeleteAt = 0
+	b.Username = NormalizeUsername(b.Username)
 }
 
 // PreUpdate should be run before saving an updated bot to the database.