mirrors/mostlymatter
1
0
Форкнуть 0
Код Задачи Запросы на слияние Действия Пакеты Проекты Релизы Вики Активность

MM-66092 - enhance user permissions data structure validations (#34654) (#35006)

Просмотр исходного кода 
Automatic Merge
Этот коммит содержится в:
Pablo Vélez
2026-01-21 05:48:52 -05:00
коммит произвёл GitHub
родитель ebe1bd4e31
Коммит 43e797010b
6 изменённых файлов: 309 добавлений и 25 удалений
Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

16
server/channels/api4/group_test.go
Просмотреть файл

@@ -2071,16 +2071,8 @@ func TestGetGroups(t *testing.T) {
t.Run("not associated to channel", func(t *testing.T) {
opts := baseOpts
resp, err := th.SystemAdminClient.UpdateChannelRoles(context.Background(), th.BasicChannel.Id, th.BasicUser.Id, "")
require.NoError(t, err)
CheckOKStatus(t, resp)
opts.NotAssociatedToChannel = th.BasicChannel.Id
resp, err = th.SystemAdminClient.UpdateChannelRoles(context.Background(), th.BasicChannel.Id, th.BasicUser.Id, "channel_user channel_admin")
require.NoError(t, err)
CheckOKStatus(t, resp)
groups, resp, err := th.SystemAdminClient.GetGroups(context.Background(), opts)
require.NoError(t, err)
CheckOKStatus(t, resp)
@@ -2089,16 +2081,8 @@ func TestGetGroups(t *testing.T) {
t.Run("not associated to team", func(t *testing.T) {
opts := baseOpts
resp, err := th.SystemAdminClient.UpdateTeamMemberRoles(context.Background(), th.BasicTeam.Id, th.BasicUser.Id, "")
require.NoError(t, err)
CheckOKStatus(t, resp)
opts.NotAssociatedToTeam = th.BasicTeam.Id
resp, err = th.SystemAdminClient.UpdateTeamMemberRoles(context.Background(), th.BasicTeam.Id, th.BasicUser.Id, "team_user team_admin")
require.NoError(t, err)
CheckOKStatus(t, resp)
groups, resp, err := th.SystemAdminClient.GetGroups(context.Background(), opts)
require.NoError(t, err)
CheckOKStatus(t, resp)

30
server/channels/app/channel.go
Просмотреть файл

@@ -1251,14 +1251,29 @@ func buildChannelModerations(c request.CTX, channelType model.ChannelType, membe
return channelModerations
}
func (a *App) UpdateChannelMemberRoles(c request.CTX, channelID string, userID string, newRoles string) (*model.ChannelMember, *model.AppError) {
// UpdateChannelMemberRoles updates the roles for a channel member.
// This is the public API used by REST endpoints and plugins.
// It enforces strict validation requiring either SchemeUser or SchemeGuest to be true.
func (a *App) UpdateChannelMemberRoles(rctx request.CTX, channelID string, userID string, newRoles string) (*model.ChannelMember, *model.AppError) {
return a.updateChannelMemberRolesInternal(rctx, channelID, userID, newRoles, false)
}
// updateChannelMemberRolesInternal is the internal implementation of UpdateChannelMemberRoles.
// The allowSchemeUserUnset parameter controls whether to enforce the requirement that members
// must have either SchemeUser or SchemeGuest set to true.
//
// When allowSchemeUserUnset is false (default for API/plugin calls), the function enforces
// that members must have a base scheme role. When true (bulk import only), this validation
// is skipped to support the two-phase import pattern where explicit roles are set first,
// then scheme roles are set via UpdateChannelMemberSchemeRoles immediately after.
func (a *App) updateChannelMemberRolesInternal(rctx request.CTX, channelID string, userID string, newRoles string, allowSchemeUserUnset bool) (*model.ChannelMember, *model.AppError) {
var member *model.ChannelMember
var err *model.AppError
if member, err = a.GetChannelMember(c, channelID, userID); err != nil {
if member, err = a.GetChannelMember(rctx, channelID, userID); err != nil {
return nil, err
}
schemeGuestRole, schemeUserRole, schemeAdminRole, err := a.GetSchemeRolesForChannel(c, channelID)
schemeGuestRole, schemeUserRole, schemeAdminRole, err := a.GetSchemeRolesForChannel(rctx, channelID)
if err != nil {
return nil, err
}
@@ -1305,9 +1320,16 @@ func (a *App) UpdateChannelMemberRoles(c request.CTX, channelID string, userID s
return nil, model.NewAppError("UpdateChannelMemberRoles", "api.channel.update_channel_member_roles.changing_guest_role.app_error", nil, "", http.StatusBadRequest)
}
// Validate that the member has a base scheme role (SchemeUser or SchemeGuest).
// This ensures members always have the minimum required permissions.
// Bulk import operations may skip this validation temporarily.
if !allowSchemeUserUnset && !member.SchemeGuest && !member.SchemeUser {
return nil, model.NewAppError("UpdateChannelMemberRoles", "api.channel.update_channel_member_roles.unset_user_scheme.app_error", nil, "", http.StatusBadRequest)
}
member.ExplicitRoles = strings.Join(newExplicitRoles, " ")
return a.updateChannelMember(c, member)
return a.updateChannelMember(rctx, member)
}
func (a *App) UpdateChannelMemberSchemeRoles(c request.CTX, channelID string, userID string, isSchemeGuest bool, isSchemeUser bool, isSchemeAdmin bool) (*model.ChannelMember, *model.AppError) {

76
server/channels/app/channel_test.go
Просмотреть файл

@@ -1631,6 +1631,82 @@ func TestUpdateChannelMemberRolesChangingGuest(t *testing.T) {
})
}
func TestUpdateChannelMemberRolesRequireUser(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic()
t.Run("empty roles string requires user or guest scheme role", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, appErr)
_, appErr = th.App.AddUserToChannel(th.Context, ruser, th.BasicChannel, false)
require.Nil(t, appErr)
member, appErr := th.App.GetChannelMember(th.Context, th.BasicChannel.Id, ruser.Id)
require.Nil(t, appErr)
require.True(t, member.SchemeUser)
require.False(t, member.SchemeGuest)
_, appErr = th.App.UpdateChannelMemberRoles(th.Context, th.BasicChannel.Id, ruser.Id, "")
require.NotNil(t, appErr)
require.Equal(t, "api.channel.update_channel_member_roles.unset_user_scheme.app_error", appErr.Id)
})
t.Run("admin role requires user or guest scheme role", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, appErr)
_, appErr = th.App.AddUserToChannel(th.Context, ruser, th.BasicChannel, false)
require.Nil(t, appErr)
_, appErr = th.App.UpdateChannelMemberRoles(th.Context, th.BasicChannel.Id, ruser.Id, "channel_admin")
require.NotNil(t, appErr)
require.Equal(t, "api.channel.update_channel_member_roles.unset_user_scheme.app_error", appErr.Id)
})
t.Run("valid user and admin roles update succeeds", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, appErr)
_, appErr = th.App.AddUserToChannel(th.Context, ruser, th.BasicChannel, false)
require.Nil(t, appErr)
updatedMember, appErr := th.App.UpdateChannelMemberRoles(th.Context, th.BasicChannel.Id, ruser.Id, "channel_user channel_admin")
require.Nil(t, appErr)
require.True(t, updatedMember.SchemeUser)
require.True(t, updatedMember.SchemeAdmin)
require.False(t, updatedMember.SchemeGuest)
})
t.Run("removing admin role while keeping user role succeeds", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, appErr)
_, appErr = th.App.AddUserToChannel(th.Context, ruser, th.BasicChannel, false)
require.Nil(t, appErr)
_, appErr = th.App.UpdateChannelMemberRoles(th.Context, th.BasicChannel.Id, ruser.Id, "channel_user channel_admin")
require.Nil(t, appErr)
updatedMember, appErr := th.App.UpdateChannelMemberRoles(th.Context, th.BasicChannel.Id, ruser.Id, "channel_user")
require.Nil(t, appErr)
require.True(t, updatedMember.SchemeUser)
require.False(t, updatedMember.SchemeAdmin)
})
}
func TestDefaultChannelNames(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t)

6
server/channels/app/import_functions.go
Просмотреть файл

@@ -1117,7 +1117,8 @@ func (a *App) importUserTeams(rctx request.CTX, user *model.User, data *[]import
for _, member := range append(newMembers, oldMembers...) {
if member.ExplicitRoles != rolesByTeamID[member.TeamId] {
if _, appErr = a.UpdateTeamMemberRoles(rctx, member.TeamId, user.Id, rolesByTeamID[member.TeamId]); appErr != nil {
// Bulk import uses internal function to support two-phase role updates.
if _, appErr = a.updateTeamMemberRolesInternal(rctx, member.TeamId, user.Id, rolesByTeamID[member.TeamId], true); appErr != nil {
return appErr
}
}
@@ -1309,7 +1310,8 @@ func (a *App) importUserChannels(rctx request.CTX, user *model.User, team *model
for _, member := range append(newMembers, oldMembers...) {
if member.ExplicitRoles != rolesByChannelId[member.ChannelId] {
if _, err = a.UpdateChannelMemberRoles(rctx, member.ChannelId, user.Id, rolesByChannelId[member.ChannelId]); err != nil {
// Bulk import uses internal function to support two-phase role updates.
if _, err = a.updateChannelMemberRolesInternal(rctx, member.ChannelId, user.Id, rolesByChannelId[member.ChannelId], true); err != nil {
return err
}
}

28
server/channels/app/team.go
Просмотреть файл

@@ -392,8 +392,23 @@ func (a *App) GetSchemeRolesForTeam(teamID string) (string, string, string, *mod
return model.TeamGuestRoleId, model.TeamUserRoleId, model.TeamAdminRoleId, nil
}
func (a *App) UpdateTeamMemberRoles(c request.CTX, teamID string, userID string, newRoles string) (*model.TeamMember, *model.AppError) {
member, nErr := a.Srv().Store().Team().GetMember(c, teamID, userID)
// UpdateTeamMemberRoles updates the roles for a team member.
// This is the public API used by REST endpoints and plugins.
// It enforces strict validation requiring either SchemeUser or SchemeGuest to be true.
func (a *App) UpdateTeamMemberRoles(rctx request.CTX, teamID string, userID string, newRoles string) (*model.TeamMember, *model.AppError) {
return a.updateTeamMemberRolesInternal(rctx, teamID, userID, newRoles, false)
}
// updateTeamMemberRolesInternal is the internal implementation of UpdateTeamMemberRoles.
// The allowSchemeUserUnset parameter controls whether to enforce the requirement that members
// must have either SchemeUser or SchemeGuest set to true.
//
// When allowSchemeUserUnset is false (default for API/plugin calls), the function enforces
// that members must have a base scheme role. When true (bulk import only), this validation
// is skipped to support the two-phase import pattern where explicit roles are set first,
// then scheme roles are set via UpdateTeamMemberSchemeRoles immediately after.
func (a *App) updateTeamMemberRolesInternal(rctx request.CTX, teamID string, userID string, newRoles string, allowSchemeUserUnset bool) (*model.TeamMember, *model.AppError) {
member, nErr := a.Srv().Store().Team().GetMember(rctx, teamID, userID)
if nErr != nil {
var nfErr *store.ErrNotFound
switch {
@@ -454,9 +469,16 @@ func (a *App) UpdateTeamMemberRoles(c request.CTX, teamID string, userID string,
return nil, model.NewAppError("UpdateTeamMemberRoles", "api.channel.update_team_member_roles.changing_guest_role.app_error", nil, "", http.StatusBadRequest)
}
// Validate that the member has a base scheme role (SchemeUser or SchemeGuest).
// This ensures members always have the minimum required permissions.
// Bulk import operations may skip this validation temporarily.
if !allowSchemeUserUnset && !member.SchemeGuest && !member.SchemeUser {
return nil, model.NewAppError("UpdateTeamMemberRoles", "api.team.update_team_member_roles.unset_user_scheme.app_error", nil, "", http.StatusBadRequest)
}
member.ExplicitRoles = strings.Join(newExplicitRoles, " ")
member, nErr = a.Srv().Store().Team().UpdateMember(c, member)
member, nErr = a.Srv().Store().Team().UpdateMember(rctx, member)
if nErr != nil {
var appErr *model.AppError
switch {

178
server/channels/app/team_test.go
Просмотреть файл

@@ -1436,6 +1436,184 @@ func TestUpdateTeamMemberRolesChangingGuest(t *testing.T) {
})
}
func TestUpdateTeamMemberRolesRequireUser(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic()
t.Run("empty roles string requires user or guest scheme role", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
member, err := th.App.GetTeamMember(th.Context, th.BasicTeam.Id, ruser.Id)
require.Nil(t, err)
require.True(t, member.SchemeUser)
require.False(t, member.SchemeGuest)
_, err = th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.NotNil(t, err)
require.Equal(t, "api.team.update_team_member_roles.unset_user_scheme.app_error", err.Id)
})
t.Run("admin role requires user or guest scheme role", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
_, err = th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "team_admin")
require.NotNil(t, err)
require.Equal(t, "api.team.update_team_member_roles.unset_user_scheme.app_error", err.Id)
})
t.Run("valid user and admin roles update succeeds", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
updatedMember, err := th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "team_user team_admin")
require.Nil(t, err)
require.True(t, updatedMember.SchemeUser)
require.True(t, updatedMember.SchemeAdmin)
require.False(t, updatedMember.SchemeGuest)
})
t.Run("removing admin role while keeping user role succeeds", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
_, err = th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "team_user team_admin")
require.Nil(t, err)
updatedMember, err := th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "team_user")
require.Nil(t, err)
require.True(t, updatedMember.SchemeUser)
require.False(t, updatedMember.SchemeAdmin)
})
t.Run("team_post_all alone should fail", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
_, err = th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "team_post_all")
require.NotNil(t, err)
require.Equal(t, "api.team.update_team_member_roles.unset_user_scheme.app_error", err.Id)
})
t.Run("team_post_all_public alone should fail", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
_, err = th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "team_post_all_public")
require.NotNil(t, err)
require.Equal(t, "api.team.update_team_member_roles.unset_user_scheme.app_error", err.Id)
})
t.Run("system_post_all alone should fail", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
_, err = th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "system_post_all")
require.NotNil(t, err)
require.Equal(t, "api.team.update_team_member_roles.unset_user_scheme.app_error", err.Id)
})
t.Run("system_user_manager alone should fail", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
_, err = th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "system_user_manager")
require.NotNil(t, err)
require.Equal(t, "api.team.update_team_member_roles.unset_user_scheme.app_error", err.Id)
})
t.Run("multiple non-scheme-managed roles without user scheme should fail", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
_, err = th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "team_post_all team_post_all_public system_post_all")
require.NotNil(t, err)
require.Equal(t, "api.team.update_team_member_roles.unset_user_scheme.app_error", err.Id)
})
t.Run("team_post_all with team_user should succeed", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
updatedMember, err := th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "team_user team_post_all")
require.Nil(t, err)
require.True(t, updatedMember.SchemeUser)
require.Contains(t, updatedMember.ExplicitRoles, "team_post_all")
})
t.Run("team_post_all_public with team_user should succeed", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
updatedMember, err := th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "team_user team_post_all_public")
require.Nil(t, err)
require.True(t, updatedMember.SchemeUser)
require.Contains(t, updatedMember.ExplicitRoles, "team_post_all_public")
})
t.Run("multiple explicit roles with team_user should succeed", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
updatedMember, err := th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "team_user team_post_all team_post_all_public")
require.Nil(t, err)
require.True(t, updatedMember.SchemeUser)
require.Contains(t, updatedMember.ExplicitRoles, "team_post_all")
require.Contains(t, updatedMember.ExplicitRoles, "team_post_all_public")
})
t.Run("explicit role with admin should succeed", func(t *testing.T) {
user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Tester", Username: "tester" + model.NewId(), Password: "passwd1", AuthService: ""}
ruser, _ := th.App.CreateUser(th.Context, &user)
_, _, err := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, ruser.Id, "")
require.Nil(t, err)
updatedMember, err := th.App.UpdateTeamMemberRoles(th.Context, th.BasicTeam.Id, ruser.Id, "team_user team_admin team_post_all")
require.Nil(t, err)
require.True(t, updatedMember.SchemeUser)
require.True(t, updatedMember.SchemeAdmin)
require.Contains(t, updatedMember.ExplicitRoles, "team_post_all")
})
}
func TestInvalidateAllResendInviteEmailJobs(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t)