samba: user can select devices for sharing

- show share also if a device is not available
 - use folder Freedombox/shares/open_share for sharing on every disk
 - backup and restore share definitions
 - fix: do not hide status block
 - fix: add nmbd to the managed services

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

actions/samba

@@ -20,14 +20,16 @@ Configuration helper for samba.
 """
 
 import argparse
+import configparser
+import json
 import os
 import shutil
 import subprocess
 
 import augeas
 from plinth import action_utils
+from plinth.modules.samba.manifest import SHARES_CONF_BACKUP_FILE, SHARES_PATH
 
-SHARES_PATH = '/var/lib/samba/shares'
 DEFAULT_FILE = '/etc/default/samba'
 
 CONF_PATH = '/etc/samba/smb-freedombox.conf'
@@ -35,9 +37,10 @@ CONF = r'''
 #
 # This file is managed and overwritten by Plinth.  If you wish to manage
 # Samba yourself, disable Samba in Plinth, remove this file and remove
-# the --configfile parameter in /etc/default/samba
+# line with --configfile parameter in /etc/default/samba.
 #
-# To view configured samba shares use command `net conf list`
+# Configuration parameters which differ from Debian default configuration
+# are commented. To view configured samba shares use command `net conf list`.
 #
 
 [global]
@@ -53,6 +56,8 @@ CONF = r'''
    passwd chat = *Enter\snew\s*\spassword:* %n\n *Retype\snew\s*\spassword:* %n\n *password\supdated\ssuccessfully* .
    pam password change = yes
    map to guest = bad user
+   # connection inactivity timeout in minutes
+   deadtime = 5
    # enable registry based shares
    registry shares = yes
 '''  # noqa: E501
@@ -65,27 +70,85 @@ def parse_arguments():
 
     subparsers.add_parser('setup', help='Configure samba after install')
 
+    subparsers.add_parser('get-shares', help='Get configured samba shares')
+
+    subparser = subparsers.add_parser('add-share', help='Add new samba share')
+    subparser.add_argument('--mount-point', help='Path of the mount point',
+                           required=True)
+    subparser.add_argument('--windows-filesystem', required=False,
+                           default=False, action='store_true',
+                           help='Path is Windows filesystem')
+
+    subparser = subparsers.add_parser(
+        'delete-share', help='Delete a samba share configuration')
+    subparser.add_argument('--mount-point', help='Path of the mount point',
+                           required=True)
+
+    subparsers.add_parser('dump-shares',
+                          help='Dump share configuration to file')
+    subparsers.add_parser('restore-shares',
+                          help='Restore share configuration from file')
+
     subparsers.required = True
     return parser.parse_args()
 
 
-def _share_conf(parameters, **kwargs):
+def _conf_command(parameters, **kwargs):
-    """Run samba registry edit command."""
+    """Run samba configuration registry command."""
     subprocess.check_call(['net', 'conf'] + parameters, **kwargs)
 
 
-def _create_open_share(name, path):
+def _create_share(mount_point, windows_filesystem=False):
-    """Create an open samba share."""
+    """Create a samba share."""
+    open_share_path = os.path.join(mount_point, SHARES_PATH, 'open_share')
+    os.makedirs(open_share_path, exist_ok=True)
+
+    # FAT and NTFS partitions don't support chown and chmod
+    if not windows_filesystem:
+        shutil.chown(open_share_path, group='sambashare')
+        os.chmod(open_share_path, 0o2775)
+
+    share_name = _create_share_name(mount_point)
+    _define_open_share(share_name, open_share_path, windows_filesystem)
+
+
+def _define_open_share(name, path, windows_filesystem=False):
+    """Define an open samba share."""
     try:
-        _share_conf(['delshare', name], stderr=subprocess.DEVNULL)
+        _conf_command(['delshare', name], stderr=subprocess.DEVNULL)
     except subprocess.CalledProcessError:
         pass
-    _share_conf(['addshare', name, path, 'writeable=y', 'guest_ok=y'])
+    _conf_command(['addshare', name, path, 'writeable=y', 'guest_ok=y'])
-    _share_conf(['setparm', name, 'force group', 'sambashare'])
+    if not windows_filesystem:
-    _share_conf(['setparm', name, 'inherit permissions', 'yes'])
+        _conf_command(['setparm', name, 'force group', 'sambashare'])
+        _conf_command(['setparm', name, 'inherit permissions', 'yes'])
 
 
-def _use_config_file(conf):
+def _create_share_name(mount_point):
+    """Create a share name."""
+    share_name = os.path.basename(mount_point)
+    if not share_name:
+        share_name = "disk"
+
+    return share_name
+
+
+def _get_shares():
+    """Get shares"""
+    shares = []
+    command = ['net', 'conf', 'list']
+    output = subprocess.check_output(command)
+    config = configparser.ConfigParser()
+    config.read_string(output.decode())
+    for name in config.sections():
+        mount_point = config[name]['path'].split(SHARES_PATH)[0]
+        mount_point = os.path.normpath(mount_point)
+        shares.append(dict(name=name, mount_point=mount_point))
+
+    return shares
+
+
+def _use_config_file(conf_file):
     """Set samba configuration file location."""
     aug = augeas.Augeas(
         flags=augeas.Augeas.NO_LOAD + augeas.Augeas.NO_MODL_AUTOLOAD)
@@ -94,41 +157,74 @@ def _use_config_file(conf):
     aug.load()
 
     aug.set('/files' + DEFAULT_FILE + '/SMBDOPTIONS',
-            '--configfile={0}'.format(conf))
+            '--configfile={0}'.format(conf_file))
     aug.save()
 
 
+def _validate_mount_point(path):
+    """Validate that given path string is a mount point."""
+    if path != '/':
+        parent_path = os.path.dirname(path)
+        if os.stat(path).st_dev == os.stat(parent_path).st_dev:
+            raise RuntimeError('Path "{0}" is not a mount point.'.format(path))
+
+
+def subcommand_add_share(arguments):
+    """Create a samba share."""
+    mount_point = os.path.normpath(arguments.mount_point)
+    _validate_mount_point(mount_point)
+    _create_share(mount_point, arguments.windows_filesystem)
+
+
+def subcommand_delete_share(arguments):
+    """Delete a samba share configuration."""
+    mount_point = os.path.normpath(arguments.mount_point)
+    shares = _get_shares()
+    for share in shares:
+        if share['mount_point'] == mount_point:
+            _conf_command(['delshare', share['name']])
+            # restart samba to disconnect all users
+            action_utils.service_restart('smbd')
+            break
+    else:
+        raise RuntimeError(
+            'Mount path "{0}" is not shared.'.format(mount_point))
+
+
+def subcommand_get_shares(_):
+    """Get samba shares."""
+    print(json.dumps(_get_shares()))
+
+
 def subcommand_setup(_):
-    """Configure samba after install."""
+    """Configure samba. Use custom samba config file."""
-    try:
-        os.mkdir(SHARES_PATH)
-    except FileExistsError:
-        pass
-
-    open_share_path = os.path.join(SHARES_PATH, 'open_share')
-    try:
-        os.mkdir(open_share_path)
-    except FileExistsError:
-        pass
-    # set folder group writable, 2 turns on the setGID bit
-    #
-    # TODO: some filesystems doesn't support chown and chmod
-    # (and it is not needed if mounted with correct parameters)
-    shutil.chown(open_share_path, group='sambashare')
-    os.chmod(open_share_path, 0o2775)
-
-    # use custom samba config file
     with open(CONF_PATH, 'w') as file_handle:
         file_handle.write(CONF)
     _use_config_file(CONF_PATH)
-    _create_open_share('freedombox-open-share', open_share_path)
+    if action_utils.service_is_running('smbd'):
-    action_utils.service_restart('smbd')
+        action_utils.service_restart('smbd')
+
+
+def subcommand_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') as backup_file:
+        command = ['net', 'conf', 'list']
+        subprocess.run(command, stdout=backup_file, check=True)
+
+
+def subcommand_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])
 
 
 def main():
     """Parse arguments and perform all duties."""
     arguments = parse_arguments()
-
     subcommand = arguments.subcommand.replace('-', '_')
     subcommand_method = globals()['subcommand_' + subcommand]
     subcommand_method(arguments)

plinth/modules/samba/__init__.py

@@ -18,6 +18,7 @@
 FreedomBox app to configure samba.
 """
 
+import json
 import socket
 
 from django.urls import reverse_lazy
@@ -34,7 +35,7 @@ from .manifest import backup, clients  # noqa, pylint: disable=unused-import
 
 version = 1
 
-managed_services = ['smbd']
+managed_services = ['smbd', 'nmbd']
 
 managed_packages = ['samba']
 
@@ -43,12 +44,13 @@ name = _('Samba')
 short_description = _('Samba File Sharing')
 
 description = [
-    _('Samba file sharing allows to share files between computers in your '
+    _('Samba allows to share files and folders between computers in your '
-      'local network. '),
+      'local network.'),
     format_lazy(
-        _('If enabled, Samba share will be available at \\\\{hostname} on '
+        _('After installation, you can choose which disks to use for sharing. '
-          'Windows and smb://{hostname} on Linux and Mac'),
+          'Enabled {hostname} shares are open to everyone in your local '
-        hostname=socket.gethostname()),
+          'network and are accessible under Network section in the file '
+          'manager.'), hostname=socket.gethostname().upper())
 ]
 
 clients = clients
@@ -81,6 +83,9 @@ class SambaApp(app_module.App):
         daemon = Daemon('daemon-samba', managed_services[0])
         self.add(daemon)
 
+        daemon_nmbd = Daemon('daemon-samba-nmbd', managed_services[1])
+        self.add(daemon_nmbd)
+
 
 def init():
     """Initialize the module."""
@@ -111,3 +116,35 @@ def diagnose():
     results.append(action_utils.diagnose_port_listening(445, 'tcp6'))
 
     return results
+
+
+def add_share(mount_point, filesystem):
+    """Add a share."""
+    command = ['add-share', '--mount-point', mount_point]
+    if filesystem in ['ntfs', 'vfat']:
+        command = command + ['--windows-filesystem']
+    actions.superuser_run('samba', command)
+
+
+def delete_share(mount_point):
+    """Delete a share."""
+    command = ['delete-share', '--mount-point', mount_point]
+    actions.superuser_run('samba', command)
+
+
+def get_shares():
+    """Get defined shares."""
+    output = actions.superuser_run('samba', ['get-shares'])
+
+    return json.loads(output)
+
+
+def backup_pre(packet):
+    """Save registry share configuration."""
+    actions.superuser_run('samba', ['dump-shares'])
+
+
+def restore_post(packet):
+    """Restore configuration."""
+    actions.superuser_run('samba', ['setup'])
+    actions.superuser_run('samba', ['restore-config'])

plinth/modules/samba/manifest.py

@@ -14,10 +14,22 @@
 # 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/>.
 #
+"""
+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([])
 
-backup = validate_backup({})
+backup = validate_backup({
+    'data': {
+        'files': [SHARES_CONF_BACKUP_FILE]
+    },
+    'services': ['smbd', 'nmbd']
+})

plinth/modules/samba/static/samba.js

@@ -0,0 +1,7 @@
+const share_checkbox = $(".shareform > input[type='checkbox']");
+
+share_checkbox.change(function(event) {
+    this.disabled=true;
+    this.style.cursor='wait';
+    this.form.submit();
+});

plinth/modules/samba/templates/samba.html

@@ -0,0 +1,139 @@
+{% extends "app.html" %}
+{% comment %}
+#
+# 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/>.
+#
+{% endcomment %}
+
+{% load bootstrap %}
+{% load i18n %}
+{% load static %}
+
+{% block page_head %}
+  <style type="text/css">
+    .progress {
+        margin-bottom: 0;
+    }
+  </style>
+{% endblock %}
+
+
+{% block configuration %}
+  {{ block.super }}
+
+  {% if is_enabled %}
+    <h3>{% trans "Select devices for sharing:" %}</h3>
+    <p>
+    {% blocktrans trimmed %}
+      NB! Selecting device does not share the whole disk, only the folder
+      FreedomBox/shares/open_share will be shared on that disk.
+    {% endblocktrans %}
+    </p>
+    <table class="table table-bordered table-condensed table-striped">
+      <thead>
+        <tr>
+          <th></th>
+          <th>{% trans "Device" %}</th>
+          <th>{% trans "Label" %}</th>
+          <th>{% trans "Mount Point" %}</th>
+          <th>{% trans "Type" %}</th>
+          <th>{% trans "Used" %}</th>
+        </tr>
+      </thead>
+      <tbody>
+      {% for disk in disks %}
+        <tr>
+          <td>
+            {% if disk.mount_point in shared_mounts %}
+              <form class="form shareform" method="post"
+                    action="{% url 'samba:unshare' disk.mount_point|urlencode:'' %}">
+                {% csrf_token %}
+                <input type="checkbox" value="{{ disk.mount_point }}"
+                       name="mount_point" autocomplete="off" checked/>
+              </form>
+            {% else %}
+              <form class="form shareform" method="post"
+                    action="{% url 'samba:share' disk.mount_point|urlencode:'' %}">
+                {% csrf_token %}
+                <input type="hidden" value="{{ disk.filesystem_type }}"
+                       name="filesystem_type">
+                <input type="checkbox" value="{{ disk.mount_point }}"
+                       name="mount_point" autocomplete="off"/>
+              </form>
+            {% endif %}
+          </td>
+          <td>{{ disk.device }}</td>
+          <td>{{ disk.label|default_if_none:"" }}</td>
+          <td>{{ disk.mount_point }}</td>
+          <td>{{ disk.filesystem_type }}</td>
+          <td>
+            <div class="progress">
+              {% if disk.percent_used < 75 %}
+                <div class="progress-bar progress-bar-striped progress-bar-success"
+              {% elif disk.percent_used < 90 %}
+                  <div class="progress-bar progress-bar-striped progress-bar-warning"
+              {% else %}
+                    <div class="progress-bar progress-bar-striped progress-bar-danger"
+              {% endif %}
+                         role="progressbar" aria-valuenow="disk.percent_used"
+                         aria-valuemin="0" aria-valuemax="100"
+                         style="width: {{ disk.percent_used }}%;">
+                      {{ disk.percent_used }}%
+                    </div>
+                  </div>
+                  <div>{{ disk.used_str }} / {{ disk.size_str }}</div>
+          </td>
+        </tr>
+      {% endfor %}
+      </tbody>
+    </table>
+
+    {% if unavailable_shares %}
+      <h3>Shares configured but the disk is not available:</h3>
+      <p>
+      {% blocktrans trimmed %}
+        If the disk is plugged back in, sharing will be automatically enabled.
+      {% endblocktrans %}
+      </p>
+      <table class="table table-bordered table-condensed table-striped">
+        <thead>
+          <tr>
+            <th>{% trans "Share name" %}</th>
+            <th>{% trans "Action" %}</th>
+          </tr>
+        </thead>
+        <tbody>
+        {% for share in unavailable_shares %}
+          <tr>
+            <td>{{ share.name }}</td>
+            <td>
+              <form class="form" method="post"
+                    action="{% url 'samba:unshare' share.mount_point|urlencode:'' %}">
+                {% csrf_token %}
+                <input type="submit" class="btn btn-danger" value="Delete">
+              </form>
+            </td>
+          </tr>
+        {% endfor %}
+        </tbody>
+      </table>
+    {% endif %}
+  {% endif %}
+{% endblock %}
+
+{% block page_js %}
+  <script type="text/javascript" src="{% static 'samba/samba.js' %}"></script>
+{% endblock %}

plinth/modules/samba/urls.py

@@ -15,18 +15,17 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 """
-URLs for the Samba module.
+URLs for the samba module.
 """
 
 from django.conf.urls import url
-from plinth.modules import samba
+
-from plinth.views import AppView
+from . import views
 
 urlpatterns = [
-    url(
+    url(r'^samba/$', views.SambaAppView.as_view(), name='index'),
-        r'^apps/samba/$',
+    url(r'^samba/share/(?P<mount_point>[A-Za-z0-9%_.\-~]+)/$', views.share,
-        AppView.as_view(app_id='samba', name=samba.name,
+        name='share'),
-                        diagnostics_module_name='samba',
+    url(r'^samba/unshare/(?P<mount_point>[A-Za-z0-9%_.\-~]+)/$', views.unshare,
-                        description=samba.description,
+        name='unshare'),
-                        show_status_block=False), name='index')
 ]

plinth/modules/samba/views.py

@@ -0,0 +1,102 @@
+#
+# 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/>.
+#
+"""
+Views for samba module.
+"""
+
+import logging
+import urllib.parse
+
+from django.contrib import messages
+from django.shortcuts import redirect
+from django.urls import reverse
+from django.utils.translation import ugettext as _
+from django.views.decorators.http import require_POST
+from plinth import views
+from plinth.modules import samba, storage
+
+logger = logging.getLogger(__name__)
+
+
+class SambaAppView(views.AppView):
+    """Samba sharing basic configuration."""
+    name = samba.name
+    description = samba.description
+    app_id = 'samba'
+    template_name = 'samba.html'
+
+    def get_context_data(self, *args, **kwargs):
+        """Return template context data."""
+        context = super().get_context_data(*args, **kwargs)
+        disks = storage.get_disks()
+        context['disks'] = disks
+        shares = samba.get_shares()
+        context['shared_mounts'] = [share['mount_point'] for share in shares]
+
+        unavailable_shares = []
+        for share in shares:
+            for disk in disks:
+                if share['mount_point'] == disk['mount_point']:
+                    break
+            else:
+                unavailable_shares.append(share)
+        context['unavailable_shares'] = unavailable_shares
+
+        return context
+
+
+@require_POST
+def share(request, mount_point):
+    """Enable sharing, given its root path.
+
+    mount_point is urlquoted.
+
+    """
+    mount_point = urllib.parse.unquote(mount_point)
+    filesystem = request.POST.get('filesystem_type', '')
+    try:
+        samba.add_share(mount_point, filesystem)
+        messages.success(request, _('Share enabled.'))
+    except Exception as exception:
+        logger.exception('Error enabling share')
+        messages.error(
+            request,
+            _('Error enabling share: {error_message}').format(
+                error_message=exception))
+
+    return redirect(reverse('samba:index'))
+
+
+@require_POST
+def unshare(request, mount_point):
+    """Disable sharing, given its name.
+
+    mount_point is urlquoted.
+
+    """
+    mount_point = urllib.parse.unquote(mount_point)
+    try:
+        samba.delete_share(mount_point)
+        messages.success(request, _('Share disabled.'))
+    except Exception as exception:
+        logger.exception('Error disabling share')
+        messages.error(
+            request,
+            _('Error disabling share: {error_message}').format(
+                error_message=exception))
+
+    return redirect(reverse('samba:index'))