[MM-36472] Fix inconsistencies in Roles columns (#18390)

* Fix inconsistencies in Roles columns

* Add new migrations

model/channel_member.go

@@ -69,7 +69,6 @@ type ChannelMemberForExport struct {
 }
 
 func (o *ChannelMember) IsValid() *AppError {
-
 	if !IsValidId(o.ChannelId) {
 		return NewAppError("ChannelMember.IsValid", "model.channel_member.is_valid.channel_id.app_error", nil, "", http.StatusBadRequest)
 	}
@@ -106,6 +105,11 @@ func (o *ChannelMember) IsValid() *AppError {
 		}
 	}
 
+	if len(o.Roles) > UserRolesMaxLength {
+		return NewAppError("ChannelMember.IsValid", "model.channel_member.is_valid.roles_limit.app_error",
+			map[string]interface{}{"Limit": UserRolesMaxLength}, "", http.StatusBadRequest)
+	}
+
 	return nil
 }
 

model/session.go

@@ -4,6 +4,7 @@
 package model
 
 import (
+	"net/http"
 	"strconv"
 	"strings"
 
@@ -78,6 +79,27 @@ func (s *Session) DeepCopy() *Session {
 	return &copySession
 }
 
+func (s *Session) IsValid() *AppError {
+	if !IsValidId(s.Id) {
+		return NewAppError("Session.IsValid", "model.session.is_valid.id.app_error", nil, "", http.StatusBadRequest)
+	}
+
+	if !IsValidId(s.UserId) {
+		return NewAppError("Session.IsValid", "model.session.is_valid.user_id.app_error", nil, "", http.StatusBadRequest)
+	}
+
+	if s.CreateAt == 0 {
+		return NewAppError("Session.IsValid", "model.session.is_valid.create_at.app_error", nil, "", http.StatusBadRequest)
+	}
+
+	if len(s.Roles) > UserRolesMaxLength {
+		return NewAppError("Session.IsValid", "model.session.is_valid.roles_limit.app_error",
+			map[string]interface{}{"Limit": UserRolesMaxLength}, "session_id="+s.Id, http.StatusBadRequest)
+	}
+
+	return nil
+}
+
 func (s *Session) PreSave() {
 	if s.Id == "" {
 		s.Id = NewId()

model/session_test.go

@@ -5,12 +5,74 @@ package model
 
 import (
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+func TestSessionIsValid(t *testing.T) {
+	tcs := []struct {
+		name          string
+		input         Session
+		expectedError string
+	}{
+		{
+			"Invalid Id",
+			Session{},
+			"model.session.is_valid.id.app_error",
+		},
+		{
+			"Invalid UserId",
+			Session{
+				Id: NewId(),
+			},
+			"model.session.is_valid.user_id.app_error",
+		},
+		{
+			"Invalid CreateAt",
+			Session{
+				Id:     NewId(),
+				UserId: NewId(),
+			},
+			"model.session.is_valid.create_at.app_error",
+		},
+		{
+			"Invalid Roles",
+			Session{
+				Id:       NewId(),
+				UserId:   NewId(),
+				CreateAt: 1000,
+				Roles:    strings.Repeat("a", UserRolesMaxLength+1),
+			},
+			"model.session.is_valid.roles_limit.app_error",
+		},
+		{
+			"Valid",
+			Session{
+				Id:       NewId(),
+				UserId:   NewId(),
+				CreateAt: 1000,
+				Roles:    strings.Repeat("a", UserRolesMaxLength),
+			},
+			"",
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.input.IsValid()
+			if tc.expectedError != "" {
+				require.NotNil(t, err)
+				require.Equal(t, tc.expectedError, err.Id)
+			} else {
+				require.Nil(t, err)
+			}
+		})
+	}
+}
+
 func TestSessionDeepCopy(t *testing.T) {
 	sessionId := NewId()
 	userId := NewId()

model/team_member.go

@@ -98,7 +98,6 @@ func TeamMemberWithErrorToString(o *TeamMemberWithError) string {
 }
 
 func (o *TeamMember) IsValid() *AppError {
-
 	if !IsValidId(o.TeamId) {
 		return NewAppError("TeamMember.IsValid", "model.team_member.is_valid.team_id.app_error", nil, "", http.StatusBadRequest)
 	}
@@ -107,6 +106,11 @@ func (o *TeamMember) IsValid() *AppError {
 		return NewAppError("TeamMember.IsValid", "model.team_member.is_valid.user_id.app_error", nil, "", http.StatusBadRequest)
 	}
 
+	if len(o.Roles) > UserRolesMaxLength {
+		return NewAppError("TeamMember.IsValid", "model.team_member.is_valid.roles_limit.app_error",
+			map[string]interface{}{"Limit": UserRolesMaxLength}, "", http.StatusBadRequest)
+	}
+
 	return nil
 }
 

model/user.go

@@ -60,6 +60,7 @@ const (
 	UserPasswordMaxLength = 72
 	UserLocaleMaxLength   = 5
 	UserTimezoneMaxRunes  = 256
+	UserRolesMaxLength    = 256
 )
 
 //msgp:tuple User
@@ -261,7 +262,6 @@ func (u *User) DeepCopy() *User {
 // IsValid validates the user and returns an error if it isn't configured
 // correctly.
 func (u *User) IsValid() *AppError {
-
 	if !IsValidId(u.Id) {
 		return InvalidUserError("id", "")
 	}
@@ -332,6 +332,11 @@ func (u *User) IsValid() *AppError {
 		}
 	}
 
+	if len(u.Roles) > UserRolesMaxLength {
+		return NewAppError("User.IsValid", "model.user.is_valid.roles_limit.app_error",
+			map[string]interface{}{"Limit": UserRolesMaxLength}, "user_id="+u.Id, http.StatusBadRequest)
+	}
+
 	return nil
 }
 

model/user_test.go

@@ -132,6 +132,15 @@ func TestUserIsValid(t *testing.T) {
 	user.Position = strings.Repeat("a", 129)
 	err = user.IsValid()
 	require.True(t, HasExpectedUserIsValidError(err, "position", user.Id), "expected user is valid error: %s", err.Error())
+	user.Position = ""
+
+	user.Roles = strings.Repeat("a", UserRolesMaxLength)
+	err = user.IsValid()
+	require.Nil(t, err)
+
+	user.Roles = strings.Repeat("a", UserRolesMaxLength+1)
+	err = user.IsValid()
+	require.True(t, HasExpectedUserIsValidError(err, "roles_limit", user.Id), "expected user is valid error: %s", err.Error())
 }
 
 func HasExpectedUserIsValidError(err *AppError, fieldName string, userId string) bool {