MM-58771 - Make manage_server permission, non updatable (mattermost#2…

…7481) * make manage_server, non updatable * remove blank line
devinbinnie · Jul 3, 2024 · 0dbef88 · 0dbef88
1 parent 35dda81
commit 0dbef88
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 10 deletions.
diff --git a/server/channels/api4/role.go b/server/channels/api4/role.go
@@ -18,6 +18,7 @@ var notAllowedPermissions = []string{
 	model.PermissionSysconsoleWriteUserManagementSystemRoles.Id,
 	model.PermissionSysconsoleReadUserManagementSystemRoles.Id,
 	model.PermissionManageRoles.Id,
+	model.PermissionManageSystem.Id,
 }
 
 func (api *API) InitRole() {

diff --git a/server/channels/api4/role_test.go b/server/channels/api4/role_test.go
@@ -46,7 +46,7 @@ func TestGetRole(t *testing.T) {
 		Name:          model.NewId(),
 		DisplayName:   model.NewId(),
 		Description:   model.NewId(),
-		Permissions:   []string{"manage_system", "create_public_channel"},
+		Permissions:   []string{"create_direct_channel", "create_public_channel"},
 		SchemeManaged: true,
 	}
 
@@ -85,7 +85,7 @@ func TestGetRoleByName(t *testing.T) {
 		Name:          model.NewId(),
 		DisplayName:   model.NewId(),
 		Description:   model.NewId(),
-		Permissions:   []string{"manage_system", "create_public_channel"},
+		Permissions:   []string{"create_direct_channel", "create_public_channel"},
 		SchemeManaged: true,
 	}
 
@@ -124,21 +124,21 @@ func TestGetRolesByNames(t *testing.T) {
 		Name:          model.NewId(),
 		DisplayName:   model.NewId(),
 		Description:   model.NewId(),
-		Permissions:   []string{"manage_system", "create_public_channel"},
+		Permissions:   []string{"create_direct_channel", "create_public_channel"},
 		SchemeManaged: true,
 	}
 	role2 := &model.Role{
 		Name:          model.NewId(),
 		DisplayName:   model.NewId(),
 		Description:   model.NewId(),
-		Permissions:   []string{"manage_system", "delete_private_channel"},
+		Permissions:   []string{"create_direct_channel", "delete_private_channel"},
 		SchemeManaged: true,
 	}
 	role3 := &model.Role{
 		Name:          model.NewId(),
 		DisplayName:   model.NewId(),
 		Description:   model.NewId(),
-		Permissions:   []string{"manage_system", "manage_public_channel_properties"},
+		Permissions:   []string{"create_direct_channel", "manage_public_channel_properties"},
 		SchemeManaged: true,
 	}
 
@@ -207,7 +207,7 @@ func TestPatchRole(t *testing.T) {
 		Name:          model.NewId(),
 		DisplayName:   model.NewId(),
 		Description:   model.NewId(),
-		Permissions:   []string{"manage_system", "create_public_channel", "manage_slash_commands"},
+		Permissions:   []string{"create_direct_channel", "create_public_channel", "manage_slash_commands"},
 		SchemeManaged: true,
 	}
 
@@ -216,7 +216,7 @@ func TestPatchRole(t *testing.T) {
 	defer th.App.Srv().Store().Job().Delete(role.Id)
 
 	patch := &model.RolePatch{
-		Permissions: &[]string{"manage_system", "create_public_channel", "manage_incoming_webhooks", "manage_outgoing_webhooks"},
+		Permissions: &[]string{"create_direct_channel", "create_public_channel", "manage_incoming_webhooks", "manage_outgoing_webhooks"},
 	}
 
 	th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
@@ -257,6 +257,14 @@ func TestPatchRole(t *testing.T) {
 		_, resp, err = client.PatchRole(context.Background(), systemManager.Id, patchManageRoles)
 		require.Error(t, err)
 		CheckNotImplementedStatus(t, resp)
+
+		patchManageSystem := &model.RolePatch{
+			Permissions: &[]string{model.PermissionManageSystem.Id},
+		}
+
+		_, resp, err = client.PatchRole(context.Background(), systemManager.Id, patchManageSystem)
+		require.Error(t, err)
+		CheckNotImplementedStatus(t, resp)
 	})
 
 	th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
@@ -267,7 +275,7 @@ func TestPatchRole(t *testing.T) {
 		assert.Equal(t, received.Name, role.Name)
 		assert.Equal(t, received.DisplayName, role.DisplayName)
 		assert.Equal(t, received.Description, role.Description)
-		perms := []string{"manage_system", "create_public_channel", "manage_incoming_webhooks", "manage_outgoing_webhooks"}
+		perms := []string{"create_direct_channel", "create_public_channel", "manage_incoming_webhooks", "manage_outgoing_webhooks"}
 		sort.Strings(perms)
 		assert.EqualValues(t, received.Permissions, perms)
 		assert.Equal(t, received.SchemeManaged, role.SchemeManaged)
@@ -290,7 +298,7 @@ func TestPatchRole(t *testing.T) {
 	CheckForbiddenStatus(t, resp)
 
 	patch = &model.RolePatch{
-		Permissions: &[]string{"manage_system", "manage_incoming_webhooks", "manage_outgoing_webhooks"},
+		Permissions: &[]string{"create_direct_channel", "manage_incoming_webhooks", "manage_outgoing_webhooks"},
 	}
 
 	th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
@@ -301,7 +309,7 @@ func TestPatchRole(t *testing.T) {
 		assert.Equal(t, received.Name, role.Name)
 		assert.Equal(t, received.DisplayName, role.DisplayName)
 		assert.Equal(t, received.Description, role.Description)
-		perms := []string{"manage_system", "manage_incoming_webhooks", "manage_outgoing_webhooks"}
+		perms := []string{"create_direct_channel", "manage_incoming_webhooks", "manage_outgoing_webhooks"}
 		sort.Strings(perms)
 		assert.EqualValues(t, received.Permissions, perms)
 		assert.Equal(t, received.SchemeManaged, role.SchemeManaged)