From 64b30abbce32a1b883bf1b93ff585d3a9caf3739 Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Fri, 25 Oct 2024 11:44:13 +0200 Subject: [PATCH] [MM-51487] allow uppercase team/channel names for import (#28906) --- server/channels/app/import_functions.go | 22 +++++++----- server/channels/app/import_functions_test.go | 36 ++++++++++++------- .../channels/app/imports/import_validators.go | 4 +-- .../app/imports/import_validators_test.go | 2 +- 4 files changed, 40 insertions(+), 24 deletions(-) diff --git a/server/channels/app/import_functions.go b/server/channels/app/import_functions.go index bdee174bd2..dc9b333267 100644 --- a/server/channels/app/import_functions.go +++ b/server/channels/app/import_functions.go @@ -189,15 +189,17 @@ func (a *App) importTeam(rctx request.CTX, data *imports.TeamImportData, dryRun } rctx.Logger().Info("Importing team", fields...) + teamName := strings.ToLower(*data.Name) var team *model.Team - team, err := a.Srv().Store().Team().GetByName(*data.Name) + team, err := a.Srv().Store().Team().GetByName(teamName) if err != nil { - team = &model.Team{} + team = &model.Team{ + Name: teamName, + } } - team.Name = *data.Name team.DisplayName = *data.DisplayName team.Type = *data.Type @@ -264,22 +266,26 @@ func (a *App) importChannel(rctx request.CTX, data *imports.ChannelImportData, d return nil } + teamName := strings.ToLower(*data.Team) + channelName := strings.ToLower(*data.Name) + rctx.Logger().Info("Importing channel", fields...) - team, err := a.Srv().Store().Team().GetByName(*data.Team) + team, err := a.Srv().Store().Team().GetByName(teamName) if err != nil { - return model.NewAppError("BulkImport", "app.import.import_channel.team_not_found.error", map[string]any{"TeamName": *data.Team}, "", http.StatusBadRequest).Wrap(err) + return model.NewAppError("BulkImport", "app.import.import_channel.team_not_found.error", map[string]any{"TeamName": teamName}, "", http.StatusBadRequest).Wrap(err) } var channel *model.Channel - if result, gErr := a.Srv().Store().Channel().GetByNameIncludeDeleted(team.Id, *data.Name, true); gErr == nil { + if result, gErr := a.Srv().Store().Channel().GetByNameIncludeDeleted(team.Id, channelName, true); gErr == nil { channel = result } else { - channel = &model.Channel{} + channel = &model.Channel{ + Name: channelName, + } } channel.TeamId = team.Id - channel.Name = *data.Name channel.DisplayName = *data.DisplayName channel.Type = *data.Type diff --git a/server/channels/app/import_functions_test.go b/server/channels/app/import_functions_test.go index 62e4d7f005..41dc0159be 100644 --- a/server/channels/app/import_functions_test.go +++ b/server/channels/app/import_functions_test.go @@ -515,12 +515,16 @@ func TestImportImportTeam(t *testing.T) { teamsCount, err := th.App.Srv().Store().Team().AnalyticsTeamCount(nil) require.NoError(t, err, "Failed to get team count.") + // we also assert that the team name can be upper case + teamName := "A" + model.NewId() + sanitizedTeamName := strings.ToLower(teamName) + data := imports.TeamImportData{ - Name: ptrStr(model.NewId()), - DisplayName: ptrStr("Display Name"), - Type: ptrStr("XYZ"), - Description: ptrStr("The team description."), - AllowOpenInvite: ptrBool(true), + Name: model.NewPointer(teamName), + DisplayName: model.NewPointer("Display Name"), + Type: model.NewPointer("XYZ"), + Description: model.NewPointer("The team description."), + AllowOpenInvite: model.NewPointer(true), Scheme: &scheme1.Name, } @@ -553,7 +557,7 @@ func TestImportImportTeam(t *testing.T) { th.CheckTeamCount(t, teamsCount+1) // Get the team and check that all the fields are correct. - team, appErr := th.App.GetTeamByName(*data.Name) + team, appErr := th.App.GetTeamByName(sanitizedTeamName) require.Nil(t, appErr, "Failed to get team from database.") assert.Equal(t, *data.DisplayName, team.DisplayName) @@ -577,7 +581,7 @@ func TestImportImportTeam(t *testing.T) { th.CheckTeamCount(t, teamsCount+1) // Get the team and check that all fields are correct. - team, appErr = th.App.GetTeamByName(*data.Name) + team, appErr = th.App.GetTeamByName(sanitizedTeamName) require.Nil(t, appErr, "Failed to get team from database.") assert.Equal(t, *data.DisplayName, team.DisplayName) @@ -667,6 +671,12 @@ func TestImportImportChannel(t *testing.T) { // Do a valid channel in apply mode. data.Team = &teamName + + // we also assert that the channel name can be upper case + // for the import workflow + data.Name = model.NewPointer("channelName") + sanitizedChannelName := strings.ToLower(*data.Name) + err = th.App.importChannel(th.Context, &data, false) require.Nil(t, err, "Expected success in apply mode") @@ -674,10 +684,10 @@ func TestImportImportChannel(t *testing.T) { th.CheckChannelsCount(t, channelCount+1) // Get the Channel and check all the fields are correct. - channel, err := th.App.GetChannelByName(th.Context, *data.Name, team.Id, false) + channel, err := th.App.GetChannelByName(th.Context, sanitizedChannelName, team.Id, false) require.Nil(t, err, "Failed to get channel from database.") - assert.Equal(t, *data.Name, channel.Name) + assert.Equal(t, sanitizedChannelName, channel.Name) assert.Equal(t, *data.DisplayName, channel.DisplayName) assert.Equal(t, *data.Type, channel.Type) assert.Equal(t, *data.Header, channel.Header) @@ -698,10 +708,10 @@ func TestImportImportChannel(t *testing.T) { th.CheckChannelsCount(t, channelCount) // Get the Channel and check all the fields are correct. - channel, err = th.App.GetChannelByName(th.Context, *data.Name, team.Id, false) + channel, err = th.App.GetChannelByName(th.Context, sanitizedChannelName, team.Id, false) require.Nil(t, err, "Failed to get channel from database.") - assert.Equal(t, *data.Name, channel.Name) + assert.Equal(t, sanitizedChannelName, channel.Name) assert.Equal(t, *data.DisplayName, channel.DisplayName) assert.Equal(t, *data.Type, channel.Type) assert.Equal(t, *data.Header, channel.Header) @@ -719,9 +729,9 @@ func TestImportImportChannel(t *testing.T) { data.DeletedAt = &now err = th.App.importChannel(th.Context, &data, false) require.Nil(t, err, "Expected success in apply mode") - aChan, err := th.App.GetChannelByName(th.Context, *data.Name, team.Id, true) + aChan, err := th.App.GetChannelByName(th.Context, sanitizedChannelName, team.Id, true) require.Nil(t, err, "Failed to get channel from database.") - assert.Equal(t, *data.Name, aChan.Name) + assert.Equal(t, sanitizedChannelName, aChan.Name) } func TestImportImportUser(t *testing.T) { diff --git a/server/channels/app/imports/import_validators.go b/server/channels/app/imports/import_validators.go index 3993f84ca7..ee58f35d4e 100644 --- a/server/channels/app/imports/import_validators.go +++ b/server/channels/app/imports/import_validators.go @@ -122,7 +122,7 @@ func ValidateTeamImportData(data *TeamImportData) *model.AppError { return model.NewAppError("BulkImport", "app.import.validate_team_import_data.name_length.error", nil, "", http.StatusBadRequest) } else if model.IsReservedTeamName(*data.Name) { return model.NewAppError("BulkImport", "app.import.validate_team_import_data.name_reserved.error", nil, "", http.StatusBadRequest) - } else if !model.IsValidTeamName(*data.Name) { + } else if !model.IsValidTeamName(strings.ToLower(*data.Name)) { // uppercase letters are not allowed in team names, but for import path we are more forgiving return model.NewAppError("BulkImport", "app.import.validate_team_import_data.name_characters.error", nil, "", http.StatusBadRequest) } @@ -158,7 +158,7 @@ func ValidateChannelImportData(data *ChannelImportData) *model.AppError { return model.NewAppError("BulkImport", "app.import.validate_channel_import_data.name_missing.error", nil, "", http.StatusBadRequest) } else if len(*data.Name) > model.ChannelNameMaxLength { return model.NewAppError("BulkImport", "app.import.validate_channel_import_data.name_length.error", nil, "", http.StatusBadRequest) - } else if !model.IsValidChannelIdentifier(*data.Name) { + } else if !model.IsValidChannelIdentifier(strings.ToLower(*data.Name)) { // uppercase letters are not allowed in channel names, but for import path we are more forgiving return model.NewAppError("BulkImport", "app.import.validate_channel_import_data.name_characters.error", nil, "", http.StatusBadRequest) } diff --git a/server/channels/app/imports/import_validators_test.go b/server/channels/app/imports/import_validators_test.go index c3fa1dad7c..453261cf51 100644 --- a/server/channels/app/imports/import_validators_test.go +++ b/server/channels/app/imports/import_validators_test.go @@ -393,7 +393,7 @@ func TestImportValidateChannelImportData(t *testing.T) { data.Name = model.NewPointer("A") err = ValidateChannelImportData(&data) - require.NotNil(t, err, "Should have failed due to short name.") + require.Nil(t, err, "Should not have failed due to uppercased name.") // Test team various invalid display names. data = ChannelImportData{