MM-42201: Enforce group is not the same as username. (#19660)

* MM-42201: Enforce group is not the same as username.

* MM-42201: Adjust test for change to translation id.

api4/group_test.go

@@ -119,6 +119,16 @@ func TestCreateGroup(t *testing.T) {
 	require.NoError(t, err)
 	CheckCreatedStatus(t, response)
 
+	usernameGroup := &model.Group{
+		DisplayName:    "dn_" + model.NewId(),
+		Name:           &th.BasicUser.Username,
+		Source:         model.GroupSourceCustom,
+		AllowReference: true,
+	}
+	_, response, err = th.SystemAdminClient.CreateGroup(usernameGroup)
+	require.Error(t, err)
+	CheckBadRequestStatus(t, response)
+
 	unReferenceableCustomGroup := &model.Group{
 		DisplayName:    "dn_" + model.NewId(),
 		Name:           model.NewString("name" + model.NewId()),

app/group.go

@@ -85,6 +85,11 @@ func (a *App) GetGroupsByUserId(userID string) ([]*model.Group, *model.AppError)
 }
 
 func (a *App) CreateGroup(group *model.Group) (*model.Group, *model.AppError) {
+	if err := a.isUniqueToUsernames(group.GetName()); err != nil {
+		err.Where = "CreateGroup"
+		return nil, err
+	}
+
 	group, err := a.Srv().Store.Group().Create(group)
 	if err != nil {
 		var invErr *store.ErrInvalidInput
@@ -102,7 +107,27 @@ func (a *App) CreateGroup(group *model.Group) (*model.Group, *model.AppError) {
 	return group, nil
 }
 
+func (a *App) isUniqueToUsernames(val string) *model.AppError {
+	if val == "" {
+		return nil
+	}
+	var notFoundErr *store.ErrNotFound
+	user, err := a.Srv().Store.User().GetByUsername(val)
+	if err != nil && !errors.As(err, &notFoundErr) {
+		return model.NewAppError("", "app.group.get_by_username_failure", nil, err.Error(), http.StatusInternalServerError)
+	}
+	if user != nil {
+		return model.NewAppError("", "app.group.username_conflict", nil, "", http.StatusBadRequest)
+	}
+	return nil
+}
+
 func (a *App) CreateGroupWithUserIds(group *model.GroupWithUserIds) (*model.Group, *model.AppError) {
+	if err := a.isUniqueToUsernames(group.GetName()); err != nil {
+		err.Where = "CreateGroupWithUserIds"
+		return nil, err
+	}
+
 	newGroup, err := a.Srv().Store.Group().CreateWithUserIds(group)
 	if err != nil {
 		var invErr *store.ErrInvalidInput
@@ -137,6 +162,11 @@ func (a *App) CreateGroupWithUserIds(group *model.GroupWithUserIds) (*model.Grou
 }
 
 func (a *App) UpdateGroup(group *model.Group) (*model.Group, *model.AppError) {
+	if err := a.isUniqueToUsernames(group.GetName()); err != nil {
+		err.Where = "UpdateGroup"
+		return nil, err
+	}
+
 	updatedGroup, err := a.Srv().Store.Group().Update(group)
 
 	if err == nil {

app/group_test.go

@@ -83,6 +83,18 @@ func TestCreateGroup(t *testing.T) {
 	g, err = th.App.CreateGroup(group)
 	require.NotNil(t, err)
 	require.Nil(t, g)
+
+	user := th.CreateUser()
+	usernameGroup := &model.Group{
+		DisplayName: "dn_" + model.NewId(),
+		Name:        &user.Username,
+		Source:      model.GroupSourceLdap,
+		RemoteId:    model.NewString(model.NewId()),
+	}
+	g, err = th.App.CreateGroup(usernameGroup)
+	require.NotNil(t, err)
+	require.Equal(t, "app.group.username_conflict", err.Id)
+	require.Nil(t, g)
 }
 
 func TestUpdateGroup(t *testing.T) {
@@ -94,6 +106,13 @@ func TestUpdateGroup(t *testing.T) {
 	g, err := th.App.UpdateGroup(group)
 	require.Nil(t, err)
 	require.NotNil(t, g)
+
+	user := th.CreateUser()
+	g.Name = &user.Username
+	g, err = th.App.UpdateGroup(g)
+	require.NotNil(t, err)
+	require.Equal(t, "app.group.username_conflict", err.Id)
+	require.Nil(t, g)
 }
 
 func TestDeleteGroup(t *testing.T) {

app/user.go

@@ -219,6 +219,11 @@ func (a *App) CreateGuest(c *request.Context, user *model.User) (*model.User, *m
 }
 
 func (a *App) createUserOrGuest(c *request.Context, user *model.User, guest bool) (*model.User, *model.AppError) {
+	if err := a.isUniqueToGroupNames(user.Username); err != nil {
+		err.Where = "createUserOrGuest"
+		return nil, err
+	}
+
 	ruser, nErr := a.ch.srv.userService.CreateUser(user, users.UserCreateOptions{Guest: guest})
 	if nErr != nil {
 		var appErr *model.AppError
@@ -1053,6 +1058,21 @@ func (a *App) sendUpdatedUserEvent(user model.User) {
 	a.Publish(sourceUserMessage)
 }
 
+func (a *App) isUniqueToGroupNames(val string) *model.AppError {
+	if val == "" {
+		return nil
+	}
+	var notFoundErr *store.ErrNotFound
+	group, err := a.Srv().Store.Group().GetByName(val, model.GroupSearchOpts{})
+	if err != nil && !errors.As(err, &notFoundErr) {
+		return model.NewAppError("", "app.user.get_by_name_failure", nil, err.Error(), http.StatusInternalServerError)
+	}
+	if group != nil {
+		return model.NewAppError("", "app.user.group_name_conflict", nil, "", http.StatusBadRequest)
+	}
+	return nil
+}
+
 func (a *App) UpdateUser(user *model.User, sendNotifications bool) (*model.User, *model.AppError) {
 	prev, err := a.ch.srv.userService.GetUser(user.Id)
 	if err != nil {
@@ -1065,6 +1085,13 @@ func (a *App) UpdateUser(user *model.User, sendNotifications bool) (*model.User,
 		}
 	}
 
+	if user.Username != prev.Username {
+		if err := a.isUniqueToGroupNames(user.Username); err != nil {
+			err.Where = "UpdateUser"
+			return nil, err
+		}
+	}
+
 	var newEmail string
 	if user.Email != prev.Email {
 		if !users.CheckUserDomain(user, *a.Config().TeamSettings.RestrictCreationToDomains) {

app/user_test.go

@@ -180,6 +180,46 @@ func TestUpdateUserToRestrictedDomain(t *testing.T) {
 	})
 }
 
+func TestUpdateUser(t *testing.T) {
+	th := Setup(t)
+	defer th.TearDown()
+
+	user := th.CreateUser()
+	group := th.CreateGroup()
+
+	t.Run("fails if the username matches a group name", func(t *testing.T) {
+		user.Username = *group.Name
+		u, err := th.App.UpdateUser(user, false)
+		require.NotNil(t, err)
+		require.Equal(t, "app.user.group_name_conflict", err.Id)
+		require.Nil(t, u)
+	})
+}
+
+func TestCreateUser(t *testing.T) {
+	th := Setup(t)
+	defer th.TearDown()
+
+	group := th.CreateGroup()
+
+	id := model.NewId()
+	user := &model.User{
+		Email:         "success+" + id + "@simulator.amazonses.com",
+		Username:      *group.Name,
+		Nickname:      "nn_" + id,
+		Password:      "Password1",
+		EmailVerified: true,
+	}
+
+	t.Run("fails if the username matches a group name", func(t *testing.T) {
+		user.Username = *group.Name
+		u, err := th.App.CreateUser(th.Context, user)
+		require.NotNil(t, err)
+		require.Equal(t, "app.user.group_name_conflict", err.Id)
+		require.Nil(t, u)
+	})
+}
+
 func TestUpdateUserActive(t *testing.T) {
 	th := Setup(t)
 	defer th.TearDown()

i18n/en.json

@@ -4931,6 +4931,10 @@
     "id": "app.group.crud_permission",
     "translation": "Unable to perform operation for that source type."
   },
+  {
+    "id": "app.group.get_by_username_failure",
+    "translation": " "
+  },
   {
     "id": "app.group.group_syncable_already_deleted",
     "translation": "group syncable was already deleted"
@@ -4951,6 +4955,10 @@
     "id": "app.group.uniqueness_error",
     "translation": "group member already exists"
   },
+  {
+    "id": "app.group.username_conflict",
+    "translation": " "
+  },
   {
     "id": "app.import.attachment.bad_file.error",
     "translation": "Error reading the file at: \"{{.FilePath}}\""
@@ -6467,6 +6475,10 @@
     "id": "app.user.get_by_auth.other.app_error",
     "translation": "We encountered an error trying to find the account by authentication type."
   },
+  {
+    "id": "app.user.get_by_name_failure",
+    "translation": " "
+  },
   {
     "id": "app.user.get_by_username.app_error",
     "translation": "Unable to find an existing account matching your username for this team. This team may require an invite from the team owner to join."
@@ -6523,6 +6535,10 @@
     "id": "app.user.get_users_batch_for_indexing.get_users.app_error",
     "translation": "Unable to get the users batch for indexing."
   },
+  {
+    "id": "app.user.group_name_conflict",
+    "translation": " "
+  },
   {
     "id": "app.user.missing_account.const",
     "translation": "Unable to find the user."

model/group.go

@@ -211,6 +211,13 @@ func (group *Group) IsValidName() *AppError {
 	return nil
 }
 
+func (group *Group) GetName() string {
+	if group.Name == nil {
+		return ""
+	}
+	return *group.Name
+}
+
 func (group *Group) GetRemoteId() string {
 	if group.RemoteId == nil {
 		return ""