From bd03969d95f1c5f5fe9843e76e9e0d6957c033be Mon Sep 17 00:00:00 2001
From: Veiko Aasa <veiko17@disroot.org>
Date: Sun, 15 Sep 2024 10:33:01 +0300
Subject: [PATCH] samba: Remove option to backup app

Reasons:
- Privileged action security: restoring Samba configuration from a backup file
could expose any folder in OS and allows to run any commmand as a root user.
- Samba backups aren't so useful as only app configuration is included.
Configured shares are trivial to enable without backups. Also, providing
backups could be misleading as stored user files aren't actually backupped.

Tests performed:
- All Samba functional tests pass.
- Restoring from an old backup that also includes Samba is not failing,
  restoring Samba is skipped.

Signed-off-by: Veiko Aasa <veiko17@disroot.org>
Reviewed-by: Sunil Mohan Adapa <sunil@medhas.org>
---
 plinth/modules/samba/__init__.py              | 19 ++----------------
 plinth/modules/samba/manifest.py              |  4 +---
 plinth/modules/samba/privileged.py            | 20 -------------------
 plinth/modules/samba/tests/test_functional.py | 10 ----------
 4 files changed, 3 insertions(+), 50 deletions(-)

diff --git a/plinth/modules/samba/__init__.py b/plinth/modules/samba/__init__.py
index 58c83640d..a433db765 100644
--- a/plinth/modules/samba/__init__.py
+++ b/plinth/modules/samba/__init__.py
@@ -86,8 +86,8 @@ class SambaApp(app_module.App):
                                           groups=groups)
         self.add(users_and_groups)
 
-        backup_restore = SambaBackupRestore('backup-restore-samba',
-                                            **manifest.backup)
+        backup_restore = BackupRestore('backup-restore-samba',
+                                       **manifest.backup)
         self.add(backup_restore)
 
     def setup(self, old_version):
@@ -103,21 +103,6 @@ class SambaApp(app_module.App):
         privileged.uninstall()
 
 
-class SambaBackupRestore(BackupRestore):
-    """Component to backup/restore Samba."""
-
-    def backup_pre(self, packet):
-        """Save registry share configuration."""
-        super().backup_pre(packet)
-        privileged.dump_shares()
-
-    def restore_post(self, packet):
-        """Restore configuration."""
-        super().restore_post(packet)
-        privileged.setup()
-        privileged.restore_shares()
-
-
 def add_share(mount_point, share_type, filesystem):
     """Add a share."""
     windows_filesystem = (filesystem in ['ntfs', 'vfat'])
diff --git a/plinth/modules/samba/manifest.py b/plinth/modules/samba/manifest.py
index 810fe9e11..821a48ece 100644
--- a/plinth/modules/samba/manifest.py
+++ b/plinth/modules/samba/manifest.py
@@ -7,8 +7,6 @@ from django.utils.translation import gettext_lazy as _
 
 from plinth.clients import store_url
 
-SHARES_CONF_BACKUP_FILE = '/var/lib/plinth/backups-data/samba-shares-dump.conf'
-
 clients = [{
     'name':
         _('Android Samba Client'),
@@ -85,4 +83,4 @@ clients = [{
     }]
 }]
 
-backup = {'data': {'files': [SHARES_CONF_BACKUP_FILE]}, 'services': ['smbd']}
+backup: dict = {}
diff --git a/plinth/modules/samba/privileged.py b/plinth/modules/samba/privileged.py
index 6c6e18786..c8f0078f1 100644
--- a/plinth/modules/samba/privileged.py
+++ b/plinth/modules/samba/privileged.py
@@ -9,7 +9,6 @@ import subprocess
 
 from plinth.actions import privileged
 
-SHARES_CONF_BACKUP_FILE = '/var/lib/plinth/backups-data/samba-shares-dump.conf'
 DEFAULT_FILE = '/etc/default/samba'
 
 CONF_PATH = '/etc/samba/smb-freedombox.conf'
@@ -298,25 +297,6 @@ def setup():
         action_utils.service_restart('smbd')
 
 
-@privileged
-def dump_shares():
-    """Dump registy share configuration."""
-    os.makedirs(os.path.dirname(SHARES_CONF_BACKUP_FILE), exist_ok=True)
-    with open(SHARES_CONF_BACKUP_FILE, 'w', encoding='utf-8') as backup_file:
-        command = ['net', 'conf', 'list']
-        subprocess.run(command, stdout=backup_file, check=True)
-
-
-@privileged
-def restore_shares():
-    """Restore registy share configuration."""
-    if not os.path.exists(SHARES_CONF_BACKUP_FILE):
-        raise RuntimeError(
-            'Backup file {0} does not exist.'.format(SHARES_CONF_BACKUP_FILE))
-    _conf_command(['drop'])
-    _conf_command(['import', SHARES_CONF_BACKUP_FILE])
-
-
 @privileged
 def uninstall():
     """Drop all Samba shares."""
diff --git a/plinth/modules/samba/tests/test_functional.py b/plinth/modules/samba/tests/test_functional.py
index c5c7ddc92..aac4d0629 100644
--- a/plinth/modules/samba/tests/test_functional.py
+++ b/plinth/modules/samba/tests/test_functional.py
@@ -26,16 +26,6 @@ class TestSambaApp(functional.BaseAppTests):
         functional.login(session_browser)
         functional.networks_set_firewall_zone(session_browser, 'internal')
 
-    @pytest.mark.backups
-    def test_backup_restore(self, session_browser):
-        """Test backing up and restoring."""
-        _set_share(session_browser, 'home', status='enabled')
-        functional.backup_create(session_browser, 'samba', 'test_samba')
-        _set_share(session_browser, 'home', status='disabled')
-        functional.backup_restore(session_browser, 'samba', 'test_samba')
-        assert functional.service_is_running(session_browser, 'samba')
-        _assert_share_is_writable('home')
-
     @pytest.mark.parametrize('share_type', ['open', 'group', 'home'])
     def test_enable_disable_samba_share(self, session_browser, share_type):
         """Test enabling and disabling Samba share."""