samba: fixes and improvements

Signed-off-by: Veiko Aasa <veiko17@disroot.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

actions/samba

@@ -29,7 +29,7 @@ import subprocess
 
 import augeas
 from plinth import action_utils
-from plinth.modules.samba.manifest import SHARES_CONF_BACKUP_FILE, SHARES_PATH
+from plinth.modules.samba.manifest import SHARES_CONF_BACKUP_FILE
 
 DEFAULT_FILE = '/etc/default/samba'
 
@@ -106,15 +106,18 @@ def _conf_command(parameters, **kwargs):
 
 def _create_share(mount_point, windows_filesystem=False):
     """Create a samba share."""
-    open_share_path = os.path.join(mount_point, SHARES_PATH, 'open_share')
+    shares_path = _get_shares_path(mount_point)
+    open_share_path = os.path.join(mount_point, shares_path, 'open_share')
     os.makedirs(open_share_path, exist_ok=True)
 
     _make_mounts_readable_by_others(mount_point)
 
-    # FAT and NTFS partitions don't support chown and chmod
+    # FAT and NTFS partitions don't support setting permissions
     if not windows_filesystem:
-        shutil.chown(open_share_path, group='sambashare')
+        shutil.chown(open_share_path, group='freedombox-share')
         os.chmod(open_share_path, 0o2775)
+        subprocess.check_call(['setfacl', '-Rm', 'g::rwx', open_share_path])
+        subprocess.check_call(['setfacl', '-Rdm', 'g::rwx', open_share_path])
 
     share_name = _create_share_name(mount_point)
     _define_open_share(share_name, open_share_path, windows_filesystem)
@@ -137,10 +140,25 @@ def _define_open_share(name, path, windows_filesystem=False):
         pass
     _conf_command(['addshare', name, path, 'writeable=y', 'guest_ok=y'])
     if not windows_filesystem:
-        _conf_command(['setparm', name, 'force group', 'sambashare'])
+        _conf_command(['setparm', name, 'force group', 'freedombox-share'])
         _conf_command(['setparm', name, 'inherit permissions', 'yes'])
 
 
+def _get_mount_point(path):
+    """Get the mount point where the share is."""
+    subpath = 'FreedomBox/shares/'
+    if '/var/lib/freedombox/shares/' in path:
+        try:
+            # test whether var directory is a mount point
+            _validate_mount_point(path.split('lib/freedombox/shares/')[0])
+        except RuntimeError:
+            subpath = 'var/lib/freedombox/shares/'
+        else:
+            subpath = 'lib/freedombox/shares/'
+
+    return path.split(subpath)[0]
+
+
 def _get_shares():
     """Get shares."""
     shares = []
@@ -148,13 +166,26 @@ def _get_shares():
     config = configparser.ConfigParser()
     config.read_string(output.decode())
     for name in config.sections():
-        mount_point = config[name]['path'].split(SHARES_PATH)[0]
+        mount_point = _get_mount_point(config[name]['path'])
         mount_point = os.path.normpath(mount_point)
         shares.append(dict(name=name, mount_point=mount_point))
 
     return shares
 
 
+def _get_shares_path(mount_point):
+    """Return base path of the shared directories."""
+    if mount_point == '/var':
+        return 'lib/freedombox/shares/'
+    var_directory = os.path.join(mount_point, 'var')
+
+    if os.path.exists(var_directory) and os.stat(
+            mount_point).st_dev == os.stat(var_directory).st_dev:
+        return 'var/lib/freedombox/shares/'
+
+    return 'FreedomBox/shares/'
+
+
 def _make_mounts_readable_by_others(mount_point):
     """Make mounted devices readable/traversible by others."""
     dirname = os.path.dirname(mount_point)

plinth/modules/samba/__init__.py

@@ -28,6 +28,7 @@ from plinth import action_utils, actions
 from plinth import app as app_module
 from plinth import frontpage, menu
 from plinth.daemon import Daemon
+from plinth.modules.users import create_group
 from plinth.modules.firewall.components import Firewall
 from plinth.utils import format_lazy
 
@@ -50,9 +51,11 @@ description = [
         _('After installation, you can choose which disks to use for sharing. '
           'Enabled {hostname} shares are open to everyone in your local '
           'network and are accessible under Network section in the file '
-          'manager.'), hostname=socket.gethostname().upper())
+          'manager on your computer.'), hostname=socket.gethostname().upper())
 ]
 
+group = ('freedombox-share', _('Access shared folders from inside the server'))
+
 clients = clients
 
 app = None
@@ -100,6 +103,7 @@ def init():
 def setup(helper, old_version=None):
     """Install and configure the module."""
     helper.install(managed_packages)
+    create_group('freedombox-share')
     helper.call('post', actions.superuser_run, 'samba', ['setup'])
     helper.call('post', app.enable)
 

plinth/modules/samba/manifest.py

@@ -21,8 +21,6 @@ Application manifest for Samba.
 from plinth.clients import validate
 from plinth.modules.backups.api import validate as validate_backup
 
-# A directory where the 'open_share' subdirectory will be created
-SHARES_PATH = 'FreedomBox/shares/'
 SHARES_CONF_BACKUP_FILE = '/var/lib/plinth/backups-data/samba-shares-dump.conf'
 
 clients = validate([])

plinth/modules/samba/static/samba.js

@@ -1,3 +1,26 @@
+/**
+ * @licstart The following is the entire license notice for the JavaScript
+ * code in this page.
+ *
+ * This file is part of FreedomBox.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * @licend The above is the entire license notice for the JavaScript code
+ * in this page.
+ */
+
 const share_checkbox = $(".shareform > input[type='checkbox']");
 
 share_checkbox.change(function(event) {

plinth/modules/samba/templates/samba.html

@@ -38,8 +38,8 @@
     <h3>{% trans "Select disks for sharing" %}</h3>
     <p>
     {% blocktrans trimmed %}
-      NB! Only the directory FreedomBox/shares/open_share will be shared on
-      selected disks. The directory will be created if it doesn't exist.
+      Note: only specially created directory will be shared on selected disks,
+      not the whole disk.
     {% endblocktrans %}
     </p>
     <table class="table table-bordered table-condensed table-striped">