MM-68830: Preserve unknown permissions during migrations on downgrade (#36888)

* MM-68830: Preserve unknown permissions during migrations on downgrade

A server that was upgraded to a newer release (which introduced new
permissions and wrote them into roles) and then downgraded fails fatally
at startup: the permissions migration re-saves every role, and
Role.Save() rejects any permission the older binary does not recognize,
making the downgrade unrecoverable.

Add RoleStore.SavePreservingUnknownPermissions, used only by
doPermissionsMigration, which tolerates and preserves permissions this
build does not recognize (logging a warning) instead of rejecting the
role. The regular Save() — and therefore the role API path — stays
strict, so unknown permissions cannot be introduced through user input.

Unrecognized permissions are kept on disk so they are not lost on a
later re-upgrade.

* MM-68830: assert save forwarding in role cache tests

Address review feedback: assert the underlying store's Save and
SavePreservingUnknownPermissions are actually invoked (the cache
invalidation defer fires regardless of forwarding), and check the
returned errors.

* MM-68830: address review feedback

- Shorten log message in validateForSave
- Rename validationRole -> roleCopy for clarity
- Trim doc comments to describe behavior only
- List all unknown permissions in IsValidWithoutId error
- Assert specific error type in storetest

* MM-68830: add Role.Clone and use it in validateForSave

* MM-68830: add tests for Role.Clone

* MM-68830: fix scheme id deep copy assertion in Role.Clone test

server/channels/app/permissions_migrations.go

@@ -237,7 +237,10 @@ func (s *Server) doPermissionsMigration(key string, migrationMap permissionsMap,
 
 	for _, role := range roles {
 		role.Permissions = applyPermissionsMap(role, roleMap, migrationMap)
-		if _, err := s.Store().Role().Save(role); err != nil {
+		// Use SavePreservingUnknownPermissions so a server that was downgraded from a
+		// newer release (which wrote permissions this binary doesn't recognize) does
+		// not fail fatally here. Unknown permissions are logged and preserved (MM-68830).
+		if _, err := s.Store().Role().SavePreservingUnknownPermissions(role); err != nil {
 			var invErr *store.ErrInvalidInput
 			switch {
 			case errors.As(err, &invErr):

server/channels/app/permissions_migrations_test.go

@@ -4,6 +4,7 @@
 package app
 
 import (
+	"slices"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -48,7 +49,7 @@ func TestRestoreManageOAuthPermissionMigration(t *testing.T) {
 		return system.Name == model.MigrationKeyRestoreManageOAuthPermission && system.Value == "true"
 	})).Return(nil).Once()
 
-	roleStore.On("Save", mock.AnythingOfType("*model.Role")).
+	roleStore.On("SavePreservingUnknownPermissions", mock.AnythingOfType("*model.Role")).
 		Return(func(role *model.Role) *model.Role { return role }, nil).Twice()
 
 	appErr := th.App.Srv().doPermissionsMigration(model.MigrationKeyRestoreManageOAuthPermission, migrationMap, roles)
@@ -61,7 +62,7 @@ func TestRestoreManageOAuthPermissionMigration(t *testing.T) {
 	require.Nil(t, appErr)
 	assert.Len(t, systemAdminRole.Permissions, 2)
 
-	roleStore.AssertNumberOfCalls(t, "Save", 2)
+	roleStore.AssertNumberOfCalls(t, "SavePreservingUnknownPermissions", 2)
 	systemStore.AssertNumberOfCalls(t, "SaveOrUpdate", 1)
 }
 
@@ -98,7 +99,7 @@ func TestAddManageAgentPermissionsMigration(t *testing.T) {
 		return system.Name == model.MigrationKeyAddManageAgentPermissions && system.Value == "true"
 	})).Return(nil).Once()
 
-	roleStore.On("Save", mock.AnythingOfType("*model.Role")).
+	roleStore.On("SavePreservingUnknownPermissions", mock.AnythingOfType("*model.Role")).
 		Return(func(role *model.Role) *model.Role { return role }, nil).Twice()
 
 	appErr := th.App.Srv().doPermissionsMigration(model.MigrationKeyAddManageAgentPermissions, migrationMap, roles)
@@ -115,10 +116,52 @@ func TestAddManageAgentPermissionsMigration(t *testing.T) {
 	assert.Len(t, systemAdminRole.Permissions, 3, "system_admin should still have 3 permissions after idempotent run")
 	assert.Len(t, systemUserRole.Permissions, 2, "system_user should still have 2 permissions after idempotent run")
 
-	roleStore.AssertNumberOfCalls(t, "Save", 2)
+	roleStore.AssertNumberOfCalls(t, "SavePreservingUnknownPermissions", 2)
 	systemStore.AssertNumberOfCalls(t, "SaveOrUpdate", 1)
 }
 
+// TestPermissionsMigrationPreservesUnknownPermissions is the regression test for
+// MM-68830: a server downgraded from a newer release holds permissions the older
+// binary does not recognize. The permissions migration must not fail fatally, and
+// must preserve those unknown permissions rather than stripping them.
+func TestPermissionsMigrationPreservesUnknownPermissions(t *testing.T) {
+	mainHelper.Parallel(t)
+
+	th := SetupWithStoreMock(t)
+
+	migrationMap, err := th.App.getAddManageAgentPermissionsMigration()
+	require.NoError(t, err)
+
+	const unknownPermission = "manage_own_agent_from_the_future"
+	systemAdminRole := &model.Role{
+		Name:        model.SystemAdminRoleId,
+		Permissions: []string{model.PermissionManageSystem.Id, unknownPermission},
+	}
+	roles := []*model.Role{systemAdminRole}
+
+	mockStore := th.App.Srv().Store().(*mocks.Store)
+	roleStore := mocks.RoleStore{}
+	systemStore := mocks.SystemStore{}
+
+	mockStore.On("Role").Return(&roleStore)
+	mockStore.On("System").Return(&systemStore)
+
+	systemStore.On("GetByName", model.MigrationKeyAddManageAgentPermissions).
+		Return(nil, model.NewAppError("test", "missing", nil, "", 404)).Once()
+	systemStore.On("SaveOrUpdate", mock.AnythingOfType("*model.System")).Return(nil).Once()
+
+	// The migration must route through the tolerant save, and the unknown permission
+	// must still be present on the role handed to the store (not stripped).
+	roleStore.On("SavePreservingUnknownPermissions", mock.MatchedBy(func(role *model.Role) bool {
+		return role.Name == model.SystemAdminRoleId && slices.Contains(role.Permissions, unknownPermission)
+	})).Return(func(role *model.Role) *model.Role { return role }, nil).Once()
+
+	appErr := th.App.Srv().doPermissionsMigration(model.MigrationKeyAddManageAgentPermissions, migrationMap, roles)
+	require.Nil(t, appErr, "downgrade migration must not fail fatally on unknown permissions")
+	assert.Contains(t, systemAdminRole.Permissions, unknownPermission, "unknown permission must be preserved across the migration")
+	roleStore.AssertNumberOfCalls(t, "SavePreservingUnknownPermissions", 1)
+}
+
 func TestApplyPermissionsMap(t *testing.T) {
 	mainHelper.Parallel(t)
 	tt := []struct {

server/channels/store/localcachelayer/main_test.go

@@ -54,6 +54,7 @@ func getMockStore(t *testing.T) *mocks.Store {
 	fakeRole2 := model.Role{Id: "456", Name: "role-name2"}
 	mockRolesStore := mocks.RoleStore{}
 	mockRolesStore.On("Save", &fakeRole).Return(&model.Role{}, nil)
+	mockRolesStore.On("SavePreservingUnknownPermissions", &fakeRole).Return(&model.Role{}, nil)
 	mockRolesStore.On("Delete", "123").Return(&fakeRole, nil)
 	mockRolesStore.On("GetByName", context.Background(), "role-name").Return(&fakeRole, nil)
 	mockRolesStore.On("GetByNames", []string{"role-name"}).Return([]*model.Role{&fakeRole}, nil)

server/channels/store/localcachelayer/role_layer.go

@@ -44,6 +44,14 @@ func (s LocalCacheRoleStore) Save(role *model.Role) (*model.Role, error) {
 	return s.RoleStore.Save(role)
 }
 
+func (s LocalCacheRoleStore) SavePreservingUnknownPermissions(role *model.Role) (*model.Role, error) {
+	if role.Name != "" {
+		defer s.rootStore.doInvalidateCacheCluster(s.rootStore.roleCache, role.Name, nil)
+		defer s.rootStore.doClearCacheCluster(s.rootStore.rolePermissionsCache)
+	}
+	return s.RoleStore.SavePreservingUnknownPermissions(role)
+}
+
 func (s LocalCacheRoleStore) GetByName(ctx context.Context, name string) (*model.Role, error) {
 	var role *model.Role
 	if err := s.rootStore.doStandardReadCache(s.rootStore.roleCache, name, &role); err == nil {

server/channels/store/localcachelayer/role_layer_test.go

@@ -46,10 +46,31 @@ func TestRoleStoreCache(t *testing.T) {
 		cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider, logger)
 		require.NoError(t, err)
 
-		cachedStore.Role().GetByName(context.Background(), "role-name")
+		_, err = cachedStore.Role().GetByName(context.Background(), "role-name")
+		require.NoError(t, err)
 		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 1)
-		cachedStore.Role().Save(&fakeRole)
-		cachedStore.Role().GetByName(context.Background(), "role-name")
+		_, err = cachedStore.Role().Save(&fakeRole)
+		require.NoError(t, err)
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "Save", 1)
+		_, err = cachedStore.Role().GetByName(context.Background(), "role-name")
+		require.NoError(t, err)
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 2)
+	})
+
+	t.Run("first call not cached, save preserving unknown permissions, and then not cached again", func(t *testing.T) {
+		mockStore := getMockStore(t)
+		mockCacheProvider := getMockCacheProvider()
+		cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider, logger)
+		require.NoError(t, err)
+
+		_, err = cachedStore.Role().GetByName(context.Background(), "role-name")
+		require.NoError(t, err)
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 1)
+		_, err = cachedStore.Role().SavePreservingUnknownPermissions(&fakeRole)
+		require.NoError(t, err)
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "SavePreservingUnknownPermissions", 1)
+		_, err = cachedStore.Role().GetByName(context.Background(), "role-name")
+		require.NoError(t, err)
 		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 2)
 	})
 

server/channels/store/retrylayer/retrylayer.go

@@ -12384,6 +12384,27 @@ func (s *RetryLayerRoleStore) Save(role *model.Role) (*model.Role, error) {
 
 }
 
+func (s *RetryLayerRoleStore) SavePreservingUnknownPermissions(role *model.Role) (*model.Role, error) {
+
+	tries := 0
+	for {
+		result, err := s.RoleStore.SavePreservingUnknownPermissions(role)
+		if err == nil {
+			return result, nil
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+		timepkg.Sleep(100 * timepkg.Millisecond)
+	}
+
+}
+
 func (s *RetryLayerScheduledPostStore) CreateScheduledPost(scheduledPost *model.ScheduledPost) (*model.ScheduledPost, error) {
 
 	tries := 0

server/channels/store/sqlstore/role_store.go

@@ -13,6 +13,7 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/mattermost/mattermost/server/public/shared/mlog"
 	"github.com/mattermost/mattermost/server/v8/channels/store"
 )
 
@@ -99,10 +100,59 @@ func newSqlRoleStore(sqlStore *SqlStore) store.RoleStore {
 	return &s
 }
 
-func (s *SqlRoleStore) Save(role *model.Role) (_ *model.Role, err error) {
+func (s *SqlRoleStore) Save(role *model.Role) (*model.Role, error) {
+	return s.save(role, false)
+}
+
+// SavePreservingUnknownPermissions behaves like Save but tolerates and persists
+// permissions this server build does not recognize. See the RoleStore interface
+// and MM-68830 for the downgrade scenario this protects against.
+func (s *SqlRoleStore) SavePreservingUnknownPermissions(role *model.Role) (*model.Role, error) {
+	return s.save(role, true)
+}
+
+// validateForSave validates the role before it is persisted. When
+// preserveUnknownPermissions is true, permissions unknown to this server build are
+// logged and excluded from the validation (but left untouched on the role so they
+// are still persisted), rather than causing the save to fail.
+func (s *SqlRoleStore) validateForSave(role *model.Role, preserveUnknownPermissions bool) error {
+	roleToValidate := role
+
+	if preserveUnknownPermissions {
+		if unknown := role.UnknownPermissions(); len(unknown) > 0 {
+			s.Logger().Warn(
+				"Preserving role permissions not recognized by this server version (server likely downgraded from a newer release)",
+				mlog.String("role", role.Name),
+				mlog.Array("permissions", unknown),
+			)
+
+			unknownSet := make(map[string]bool, len(unknown))
+			for _, permission := range unknown {
+				unknownSet[permission] = true
+			}
+			known := make([]string, 0, len(role.Permissions))
+			for _, permission := range role.Permissions {
+				if !unknownSet[permission] {
+					known = append(known, permission)
+				}
+			}
+
+			roleCopy := role.Clone()
+			roleCopy.Permissions = known
+			roleToValidate = roleCopy
+		}
+	}
+
+	if err := roleToValidate.IsValidWithoutId(); err != nil {
+		return store.NewErrInvalidInput("Role", "<any>", err.Error())
+	}
+	return nil
+}
+
+func (s *SqlRoleStore) save(role *model.Role, preserveUnknownPermissions bool) (*model.Role, error) {
 	// Check the role is valid before proceeding.
-	if err = role.IsValidWithoutId(); err != nil {
-		return nil, store.NewErrInvalidInput("Role", "<any>", err.Error())
+	if err := s.validateForSave(role, preserveUnknownPermissions); err != nil {
+		return nil, err
 	}
 
 	if role.Id == "" {

server/channels/store/sqlstore/role_store_test.go

@@ -6,9 +6,43 @@ package sqlstore
 import (
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
+	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/mattermost/mattermost/server/public/shared/request"
+	"github.com/mattermost/mattermost/server/v8/channels/store"
 	"github.com/mattermost/mattermost/server/v8/channels/store/storetest"
 )
 
 func TestRoleStore(t *testing.T) {
 	StoreTestWithSqlStore(t, storetest.TestRoleStore)
 }
+
+// TestSqlRoleStoreCreateRoleValidates guards a regression: createRole must validate
+// the role itself. It is called directly by scheme_store (bypassing Save/save and
+// their validation), so removing its own validation would silently let invalid roles
+// through that path (see MM-68830 review).
+func TestSqlRoleStoreCreateRoleValidates(t *testing.T) {
+	StoreTestWithSqlStore(t, func(t *testing.T, rctx request.CTX, ss store.Store, s storetest.SqlStore) {
+		roleStore := ss.Role().(*SqlRoleStore)
+
+		transaction, err := roleStore.GetMaster().Begin()
+		require.NoError(t, err)
+		defer func() { _ = transaction.Rollback() }()
+
+		// A role carrying a permission this build does not recognize must be rejected
+		// by createRole, just as it is by Save. createRole does not tolerate unknown
+		// permissions; only the migration's SavePreservingUnknownPermissions path does.
+		invalid := &model.Role{
+			Name:        model.NewId(),
+			DisplayName: model.NewId(),
+			Description: model.NewId(),
+			Permissions: []string{"manage_own_agent_from_the_future"},
+		}
+
+		_, err = roleStore.createRole(invalid, transaction)
+		require.Error(t, err, "createRole must reject unknown permissions")
+		var invErr *store.ErrInvalidInput
+		require.ErrorAs(t, err, &invErr)
+	})
+}

server/channels/store/store.go

@@ -870,6 +870,10 @@ type PluginStore interface {
 
 type RoleStore interface {
 	Save(role *model.Role) (*model.Role, error)
+	// SavePreservingUnknownPermissions behaves like Save but tolerates and preserves
+	// permissions not recognized by this server build instead of rejecting the role.
+	// Unrecognized permissions are logged (see MM-68830).
+	SavePreservingUnknownPermissions(role *model.Role) (*model.Role, error)
 	Get(roleID string) (*model.Role, error)
 	GetAll() ([]*model.Role, error)
 	GetByName(ctx context.Context, name string) (*model.Role, error)

server/channels/store/storetest/mocks/RoleStore.go

@@ -304,6 +304,36 @@ func (_m *RoleStore) Save(role *model.Role) (*model.Role, error) {
 	return r0, r1
 }
 
+// SavePreservingUnknownPermissions provides a mock function with given fields: role
+func (_m *RoleStore) SavePreservingUnknownPermissions(role *model.Role) (*model.Role, error) {
+	ret := _m.Called(role)
+
+	if len(ret) == 0 {
+		panic("no return value specified for SavePreservingUnknownPermissions")
+	}
+
+	var r0 *model.Role
+	var r1 error
+	if rf, ok := ret.Get(0).(func(*model.Role) (*model.Role, error)); ok {
+		return rf(role)
+	}
+	if rf, ok := ret.Get(0).(func(*model.Role) *model.Role); ok {
+		r0 = rf(role)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*model.Role)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func(*model.Role) error); ok {
+		r1 = rf(role)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // NewRoleStore creates a new instance of RoleStore. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
 // The first argument is typically a *testing.T value.
 func NewRoleStore(t interface {

server/channels/store/storetest/role_store.go

@@ -18,6 +18,7 @@ import (
 
 func TestRoleStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) {
 	t.Run("Save", func(t *testing.T) { testRoleStoreSave(t, rctx, ss) })
+	t.Run("SavePreservingUnknownPermissions", func(t *testing.T) { testRoleStoreSavePreservingUnknownPermissions(t, rctx, ss) })
 	t.Run("Get", func(t *testing.T) { testRoleStoreGet(t, rctx, ss) })
 	t.Run("GetAll", func(t *testing.T) { testRoleStoreGetAll(t, rctx, ss) })
 	t.Run("GetByName", func(t *testing.T) { testRoleStoreGetByName(t, rctx, ss) })
@@ -103,6 +104,73 @@ func testRoleStoreSave(t *testing.T, rctx request.CTX, ss store.Store) {
 	assert.Error(t, err)
 }
 
+func testRoleStoreSavePreservingUnknownPermissions(t *testing.T, _ request.CTX, ss store.Store) {
+	// A role whose permissions are all valid for this build saves and round-trips
+	// unchanged, just like Save.
+	t.Run("preserves known permissions like Save", func(t *testing.T) {
+		r := &model.Role{
+			Name:          model.NewId(),
+			DisplayName:   model.NewId(),
+			Description:   model.NewId(),
+			Permissions:   []string{"invite_user", "add_user_to_team"},
+			SchemeManaged: false,
+		}
+
+		saved, err := ss.Role().SavePreservingUnknownPermissions(r)
+		require.NoError(t, err)
+		assert.Equal(t, r.Permissions, saved.Permissions)
+	})
+
+	// The downgrade scenario from MM-68830: an existing role gains a permission this
+	// build does not recognize (written by a newer release before the downgrade). The
+	// migration re-saves every role; Save would reject the unknown permission, but
+	// SavePreservingUnknownPermissions must keep it so it is not lost on a future upgrade.
+	t.Run("tolerates and persists unknown permissions on update", func(t *testing.T) {
+		unknown := "manage_own_agent_from_the_future"
+
+		// Create the role as a known-good role first (the pre-downgrade state).
+		existing, err := ss.Role().Save(&model.Role{
+			Name:        model.NewId(),
+			DisplayName: model.NewId(),
+			Description: model.NewId(),
+			Permissions: []string{"invite_user"},
+		})
+		require.NoError(t, err)
+
+		// Simulate the newer release having added an unknown permission to the role.
+		existing.Permissions = append(existing.Permissions, unknown)
+
+		// Sanity check: the regular Save rejects the unknown permission.
+		_, err = ss.Role().Save(existing)
+		require.Error(t, err)
+
+		saved, err := ss.Role().SavePreservingUnknownPermissions(existing)
+		require.NoError(t, err)
+		assert.Contains(t, saved.Permissions, unknown, "unknown permission should be preserved")
+
+		// It must actually be persisted, not just returned.
+		fetched, err := ss.Role().Get(saved.Id)
+		require.NoError(t, err)
+		assert.Contains(t, fetched.Permissions, unknown)
+	})
+
+	// Tolerating unknown permissions must not mask genuine structural problems.
+	t.Run("still rejects structurally invalid roles", func(t *testing.T) {
+		r := &model.Role{
+			Name:          "invalid-name",
+			DisplayName:   model.NewId(),
+			Description:   model.NewId(),
+			Permissions:   []string{"manage_own_agent_from_the_future"},
+			SchemeManaged: false,
+		}
+
+		_, err := ss.Role().SavePreservingUnknownPermissions(r)
+		require.Error(t, err)
+		var invErr *store.ErrInvalidInput
+		require.ErrorAs(t, err, &invErr)
+	})
+}
+
 func testRoleStoreGetAll(t *testing.T, rctx request.CTX, ss store.Store) {
 	prev, err := ss.Role().GetAll()
 	require.NoError(t, err)

server/channels/store/timerlayer/timerlayer.go

@@ -9817,6 +9817,22 @@ func (s *TimerLayerRoleStore) Save(role *model.Role) (*model.Role, error) {
 	return result, err
 }
 
+func (s *TimerLayerRoleStore) SavePreservingUnknownPermissions(role *model.Role) (*model.Role, error) {
+	start := time.Now()
+
+	result, err := s.RoleStore.SavePreservingUnknownPermissions(role)
+
+	elapsed := float64(time.Since(start)) / float64(time.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if err == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("RoleStore.SavePreservingUnknownPermissions", success, elapsed)
+	}
+	return result, err
+}
+
 func (s *TimerLayerScheduledPostStore) CreateScheduledPost(scheduledPost *model.ScheduledPost) (*model.ScheduledPost, error) {
 	start := time.Now()
 

server/public/model/role.go

@@ -428,6 +428,19 @@ type Role struct {
 	SchemeId      *string  `json:"scheme_id"`
 }
 
+func (r *Role) Clone() *Role {
+	rCopy := *r
+	if r.Permissions != nil {
+		rCopy.Permissions = make([]string, len(r.Permissions))
+		copy(rCopy.Permissions, r.Permissions)
+	}
+	if r.SchemeId != nil {
+		schemeId := *r.SchemeId
+		rCopy.SchemeId = &schemeId
+	}
+	return &rCopy
+}
+
 func (r *Role) Auditable() map[string]any {
 	return map[string]any{
 		"id":             r.Id,
@@ -802,6 +815,16 @@ func (r *Role) IsValidWithoutId() error {
 		return fmt.Errorf("role description exceeds maximum length of %d", RoleDescriptionMaxLength)
 	}
 
+	if unknown := r.UnknownPermissions(); len(unknown) > 0 {
+		return fmt.Errorf("unknown permissions: %s", strings.Join(unknown, ", "))
+	}
+
+	return nil
+}
+
+// UnknownPermissions returns the permissions on the role that are not present in
+// AllPermissions or DeprecatedPermissions (see MM-68830).
+func (r *Role) UnknownPermissions() []string {
 	check := func(perms []*Permission, permission string) bool {
 		for _, p := range perms {
 			if permission == p.Id {
@@ -810,13 +833,14 @@ func (r *Role) IsValidWithoutId() error {
 		}
 		return false
 	}
+
+	var unknown []string
 	for _, permission := range r.Permissions {
 		if !check(AllPermissions, permission) && !check(DeprecatedPermissions, permission) {
-			return fmt.Errorf("unknown permission %q", permission)
+			unknown = append(unknown, permission)
 		}
 	}
-
-	return nil
+	return unknown
 }
 
 func CleanRoleNames(roleNames []string) ([]string, bool) {

server/public/model/role_test.go

@@ -430,6 +430,79 @@ func TestRoleIsValidWithoutId(t *testing.T) {
 	})
 }
 
+func TestRoleUnknownPermissions(t *testing.T) {
+	t.Run("returns nil when all permissions are known", func(t *testing.T) {
+		r := &Role{
+			Permissions: []string{PermissionCreatePost.Id, PermissionCreateEmojis.Id},
+		}
+		assert.Empty(t, r.UnknownPermissions())
+	})
+
+	t.Run("tolerates deprecated permissions", func(t *testing.T) {
+		require.NotEmpty(t, DeprecatedPermissions)
+		r := &Role{
+			Permissions: []string{PermissionCreatePost.Id, DeprecatedPermissions[0].Id},
+		}
+		assert.Empty(t, r.UnknownPermissions())
+	})
+
+	t.Run("returns only the permissions this build does not recognize", func(t *testing.T) {
+		r := &Role{
+			Permissions: []string{PermissionCreatePost.Id, "manage_own_agent_from_the_future", "another_unknown"},
+		}
+		assert.ElementsMatch(t, []string{"manage_own_agent_from_the_future", "another_unknown"}, r.UnknownPermissions())
+	})
+
+	t.Run("empty permissions yields no unknowns", func(t *testing.T) {
+		assert.Empty(t, (&Role{}).UnknownPermissions())
+	})
+}
+
+func TestRoleClone(t *testing.T) {
+	schemeId := NewId()
+	original := &Role{
+		Id:            NewId(),
+		Name:          "test_role",
+		DisplayName:   "Test Role",
+		Description:   "desc",
+		CreateAt:      1000,
+		UpdateAt:      2000,
+		DeleteAt:      0,
+		Permissions:   []string{"invite_user", "add_user_to_team"},
+		SchemeManaged: true,
+		BuiltIn:       false,
+		SchemeId:      &schemeId,
+	}
+
+	t.Run("clone equals original", func(t *testing.T) {
+		cloned := original.Clone()
+		assert.Equal(t, original, cloned)
+	})
+
+	t.Run("permissions are deep copied", func(t *testing.T) {
+		cloned := original.Clone()
+		cloned.Permissions[0] = "mutated"
+		assert.Equal(t, "invite_user", original.Permissions[0])
+	})
+
+	t.Run("scheme id pointer is deep copied", func(t *testing.T) {
+		cloned := original.Clone()
+		require.NotSame(t, original.SchemeId, cloned.SchemeId)
+		*cloned.SchemeId = NewId()
+		assert.Equal(t, schemeId, *original.SchemeId)
+	})
+
+	t.Run("nil permissions stays nil", func(t *testing.T) {
+		r := &Role{}
+		assert.Nil(t, r.Clone().Permissions)
+	})
+
+	t.Run("nil scheme id stays nil", func(t *testing.T) {
+		r := &Role{}
+		assert.Nil(t, r.Clone().SchemeId)
+	})
+}
+
 func TestRoleIsValid(t *testing.T) {
 	validRole := func() *Role {
 		return &Role{