samba: private shares

- new share types - group and home shares
- users: when creating, deleting or changing user password, update also Samba
  tdbsam backend database
- users: new managed packages - samba-common-bin, tdb-tools
- module page: show current samba users who are in freedombox-share group
- module page: show users who should re-enter their password in the password change page
- fix: use os.path.ismount() from Python standard library to validate a mount point
- fix: samba share permissions, fixes #1729
- fix: delete a share - do not raise an exception if the share doesn't exist
- storage: show samba share type in the directory selection form

Closes #1727

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

actions/samba

@@ -51,11 +51,11 @@ CONF = r'''
    logging = file
    panic action = /usr/share/samba/panic-action %d
    server role = standalone server
-   obey pam restrictions = yes
+   #obey pam restrictions = yes
-   unix password sync = yes
+   #unix password sync = yes
-   passwd program = /usr/bin/passwd %u
+   #passwd program = /usr/bin/passwd %u
-   passwd chat = *Enter\snew\s*\spassword:* %n\n *Retype\snew\s*\spassword:* %n\n *password\supdated\ssuccessfully* .
+   #passwd chat = *Enter\snew\s*\spassword:* %n\n *Retype\snew\s*\spassword:* %n\n *password\supdated\ssuccessfully* .
-   pam password change = yes
+   #pam password change = yes
    map to guest = bad user
    # connection inactivity timeout in minutes
    deadtime = 5
@@ -73,9 +73,13 @@ def parse_arguments():
 
     subparsers.add_parser('get-shares', help='Get configured samba shares')
 
+    subparsers.add_parser('get-users', help='Get users from Samba database')
+
     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('--share-type', help='Type of the share',
+                           required=True, choices=['open', 'group', 'home'])
     subparser.add_argument('--windows-filesystem', required=False,
                            default=False, action='store_true',
                            help='Path is Windows filesystem')
@@ -84,6 +88,8 @@ def parse_arguments():
         'delete-share', help='Delete a samba share configuration')
     subparser.add_argument('--mount-point', help='Path of the mount point',
                            required=True)
+    subparser.add_argument('--share-type', help='Type of the share',
+                           required=True, choices=['open', 'group', 'home'])
 
     subparsers.add_parser('dump-shares',
                           help='Dump share configuration to file')
@@ -104,20 +110,38 @@ def _conf_command(parameters, **kwargs):
     subprocess.check_call(['net', 'conf'] + parameters, **kwargs)
 
 
-def _create_share(mount_point, windows_filesystem=False):
+def _create_share(mount_point, share_type, windows_filesystem=False):
-    """Create a samba share."""
+    """Create samba public, group and private shares."""
+    if share_type == 'open':
+        subdir = 'open_share'
+    elif share_type == 'group':
+        subdir = 'group_share'
+    elif share_type == 'home':
+        subdir = 'homes'
+
     shares_path = _get_shares_path(mount_point)
-    open_share_path = os.path.join(mount_point, shares_path, 'open_share')
+    share_path = os.path.join(mount_point, shares_path, subdir)
-    os.makedirs(open_share_path, exist_ok=True)
+    os.makedirs(share_path, exist_ok=True)
 
     _make_mounts_readable_by_others(mount_point)
 
     # FAT and NTFS partitions don't support setting permissions
     if not windows_filesystem:
-        _set_open_share_permissions(open_share_path)
+        if share_type in ['open', 'group']:
+            _set_share_permissions(share_path)
+        else:
+            shutil.chown(share_path, group='users')
+            os.chmod(share_path, 0o0775)
 
     share_name = _create_share_name(mount_point)
-    _define_open_share(share_name, open_share_path, windows_filesystem)
+
+    if share_type == 'open':
+        _define_open_share(share_name, share_path, windows_filesystem)
+    elif share_type == 'group':
+        _define_group_share(share_name + '_group', share_path,
+                            windows_filesystem)
+    elif share_type == 'home':
+        _define_homes_share(share_name + '_home', share_path)
 
 
 def _create_share_name(mount_point):
@@ -141,17 +165,40 @@ def _define_open_share(name, path, windows_filesystem=False):
         _conf_command(['setparm', name, 'inherit permissions', 'yes'])
 
 
+def _define_group_share(name, path, windows_filesystem=False):
+    """Define a group samba share."""
+    try:
+        _conf_command(['delshare', name], stderr=subprocess.DEVNULL)
+    except subprocess.CalledProcessError:
+        pass
+    _conf_command(['addshare', name, path, 'writeable=y', 'guest_ok=n'])
+    _conf_command(['setparm', name, 'valid users', '@freedombox-share @admin'])
+    if not windows_filesystem:
+        _conf_command(['setparm', name, 'force group', 'freedombox-share'])
+        _conf_command(['setparm', name, 'inherit permissions', 'yes'])
+
+
+def _define_homes_share(name, path):
+    """Define a samba share for private homes."""
+    try:
+        _conf_command(['delshare', name], stderr=subprocess.DEVNULL)
+    except subprocess.CalledProcessError:
+        pass
+    userpath = os.path.join(path, '%u')
+    _conf_command(['addshare', name, userpath, 'writeable=y', 'guest_ok=n'])
+    _conf_command(['setparm', name, 'valid users', '@freedombox-share @admin'])
+    _conf_command(
+        ['setparm', name, 'preexec', 'mkdir -p -m 755 {}'.format(userpath)])
+
+
 def _get_mount_point(path):
     """Get the mount point where the share is."""
     subpath = 'FreedomBox/shares/'
     if '/var/lib/freedombox/shares/' in path:
-        try:
+        if os.path.ismount(path.split('lib/freedombox/shares/')[0]):
-            # 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/'
+        else:
+            subpath = 'var/lib/freedombox/shares/'
 
     return path.split(subpath)[0]
 
@@ -160,13 +207,20 @@ def _get_shares():
     """Get shares."""
     shares = []
     output = subprocess.check_output(['net', 'conf', 'list'])
-    config = configparser.ConfigParser()
+    config = configparser.RawConfigParser()
     config.read_string(output.decode())
     for name in config.sections():
-        path = config[name]['path']
+        share_type = 'open'
-        mount_point = _get_mount_point(path)
+        if name.endswith('_group'):
+            share_type = 'group'
+        elif name.endswith('_home'):
+            share_type = 'home'
+        share_path = config[name]['path']
+        mount_point = _get_mount_point(share_path)
         mount_point = os.path.normpath(mount_point)
-        shares.append(dict(name=name, mount_point=mount_point, path=path))
+        shares.append(
+            dict(name=name, mount_point=mount_point, path=share_path,
+                 share_type=share_type))
 
     return shares
 
@@ -221,19 +275,31 @@ def _use_config_file(conf_file):
     aug.save()
 
 
-def _validate_mount_point(path):
+def _set_share_permissions(directory):
-    """Validate that given path string is a mount point."""
+    """Set file and directory permissions for a share."""
-    if path != '/':
+    shutil.chown(directory, group='freedombox-share')
-        parent_path = os.path.dirname(path)
+    os.chmod(directory, 0o2775)
-        if os.stat(path).st_dev == os.stat(parent_path).st_dev:
+    for root, dirs, files in os.walk(directory):
-            raise RuntimeError('Path "{0}" is not a mount point.'.format(path))
+        for subdir in dirs:
+            subdir_path = os.path.join(root, subdir)
+            shutil.chown(subdir_path, group='freedombox-share')
+            os.chmod(subdir_path, 0o2775)
+        for file in files:
+            file_path = os.path.join(root, file)
+            shutil.chown(file_path, group='freedombox-share')
+            os.chmod(file_path, 0o0664)
+    subprocess.check_call(['setfacl', '-Rm', 'g::rwX', directory])
+    subprocess.check_call(['setfacl', '-Rdm', 'g::rwX', directory])
 
 
 def subcommand_add_share(arguments):
     """Create a samba share."""
     mount_point = os.path.normpath(arguments.mount_point)
-    _validate_mount_point(mount_point)
+    if not os.path.ismount(mount_point):
-    _create_share(mount_point, arguments.windows_filesystem)
+        raise RuntimeError(
+            'Path "{0}" is not a mount point.'.format(mount_point))
+    _create_share(mount_point, arguments.share_type,
+                  arguments.windows_filesystem)
 
 
 def subcommand_delete_share(arguments):
@@ -241,13 +307,10 @@ def subcommand_delete_share(arguments):
     mount_point = os.path.normpath(arguments.mount_point)
     shares = _get_shares()
     for share in shares:
-        if share['mount_point'] == mount_point:
+        if share['mount_point'] == mount_point and share[
+                'share_type'] == arguments.share_type:
             _close_share(share['name'])
             _conf_command(['delshare', share['name']])
-            break
-    else:
-        raise RuntimeError(
-            'Mount point "{0}" is not shared.'.format(mount_point))
 
 
 def subcommand_get_shares(_):
@@ -255,11 +318,20 @@ def subcommand_get_shares(_):
     print(json.dumps(_get_shares()))
 
 
+def subcommand_get_users(_):
+    """Get users from Samba database."""
+    output = subprocess.check_output(['pdbedit', '-L']).decode()
+    samba_users = [line.split(':')[0] for line in output.split()]
+    print(json.dumps({'users': samba_users}))
+
+
 def subcommand_setup(_):
     """Configure samba, use custom samba config file."""
     with open(CONF_PATH, 'w') as file_handle:
         file_handle.write(CONF)
     _use_config_file(CONF_PATH)
+    os.makedirs('/var/lib/freedombox', exist_ok=True)
+    os.chmod('/var/lib/freedombox', 0o0755)
     if action_utils.service_is_running('smbd'):
         action_utils.service_restart('smbd')
 

actions/users

@@ -27,7 +27,6 @@ import subprocess
 import sys
 
 import augeas
-
 from plinth import action_utils
 
 ACCESS_CONF = '/etc/security/access.conf'
@@ -80,6 +79,12 @@ def parse_arguments():
     subparser.add_argument('username', help='LDAP user to add to group')
     subparser.add_argument('groupname', help='LDAP group to add the user to')
 
+    subparser = subparsers.add_parser('set-user-status',
+                                      help='Set user as active or inactive')
+    subparser.add_argument('username', help='User to change status')
+    subparser.add_argument('status', choices=['active', 'inactive'],
+                           help='New status of the user')
+
     subparser = subparsers.add_parser(
         'remove-user-from-group',
         help='Remove an LDAP user from an LDAP group')
@@ -227,6 +232,23 @@ def configure_ldapscripts():
     aug.save()
 
 
+def get_samba_users():
+    """Get users from the Samba user database."""
+    # 'pdbedit -L' is better for listing users but is installed only with samba
+    stdout = subprocess.check_output(
+        ['tdbdump', '/var/lib/samba/private/passdb.tdb']).decode()
+    return re.findall(r'USER_(.*)\\0', stdout)
+
+
+def disconnect_samba_user(username):
+    """Disconnect a Samba user."""
+    try:
+        subprocess.check_call(['pkill', '-U', username, 'smbd'])
+    except subprocess.CalledProcessError as error:
+        if error.returncode != 1:
+            raise
+
+
 def read_password():
     """Read the password from stdin."""
     return ''.join(sys.stdin)
@@ -235,8 +257,10 @@ def read_password():
 def subcommand_create_user(arguments):
     """Create an LDAP user, set password and flush cache."""
     _run(['ldapadduser', arguments.username, 'users'])
-    set_user_password(arguments.username, read_password())
+    password = read_password()
+    set_user_password(arguments.username, password)
     flush_cache()
+    set_samba_user(arguments.username, password)
 
 
 def subcommand_remove_user(arguments):
@@ -244,10 +268,15 @@ def subcommand_remove_user(arguments):
     username = arguments.username
     groups = get_user_groups(username)
 
+    if username in get_samba_users():
+        subprocess.check_call(['smbpasswd', '-x', username])
+        disconnect_samba_user(username)
+
     for group in groups:
         remove_user_from_group(username, group)
 
     _run(['ldapdeleteuser', username])
+
     flush_cache()
 
 
@@ -275,9 +304,25 @@ def set_user_password(username, password):
     _run(['ldapsetpasswd', username, password])
 
 
+def set_samba_user(username, password):
+    """Insert a user to the Samba database.
+
+    If a user already exists, update password.
+    """
+    proc = subprocess.Popen(['smbpasswd', '-a', '-s', username],
+                            stdin=subprocess.PIPE, stdout=subprocess.PIPE,
+                            stderr=subprocess.PIPE)
+    _, stderr = proc.communicate(input='{0}\n{0}\n'.format(password).encode(),
+                                 timeout=10)
+    if proc.returncode != 0:
+        raise RuntimeError('Unable to add Samba user: ', stderr)
+
+
 def subcommand_set_user_password(arguments):
     """Set a user's password."""
-    set_user_password(arguments.username, read_password())
+    password = read_password()
+    set_user_password(arguments.username, password)
+    set_samba_user(arguments.username, password)
 
 
 def get_user_groups(username):
@@ -352,6 +397,8 @@ def subcommand_remove_user_from_group(arguments):
     """Remove an LDAP user from an LDAP group."""
     remove_user_from_group(arguments.username, arguments.groupname)
     flush_cache()
+    if arguments.groupname == 'freedombox-share':
+        disconnect_samba_user(arguments.username)
 
 
 def subcommand_get_group_users(arguments):
@@ -369,6 +416,22 @@ def subcommand_get_group_users(arguments):
             print(user)
 
 
+def subcommand_set_user_status(arguments):
+    """Set the status of the user."""
+    username = arguments.username
+    status = arguments.status
+
+    if status == 'active':
+        flag = '-e'
+    else:
+        flag = '-d'
+
+    if username in get_samba_users():
+        subprocess.check_call(['smbpasswd', flag, username])
+        if status == 'inactive':
+            disconnect_samba_user(username)
+
+
 def flush_cache():
     """Flush nscd and apache2 cache."""
     _run(['nscd', '--invalidate=passwd'])

plinth/modules/samba/__init__.py

@@ -18,7 +18,10 @@
 FreedomBox app to configure samba.
 """
 
+import grp
 import json
+import os
+import pwd
 import socket
 
 from django.urls import reverse_lazy
@@ -34,7 +37,7 @@ from plinth.utils import format_lazy
 
 from .manifest import backup, clients  # noqa, pylint: disable=unused-import
 
-version = 1
+version = 2
 
 managed_services = ['smbd', 'nmbd']
 
@@ -51,12 +54,18 @@ description = [
       'other computers in your local network.'),
     format_lazy(
         _('After installation, you can choose which disks to use for sharing. '
-          'Enabled {hostname} shares are open to everyone in your local '
+          'Enabled shares are accessbile in the file manager on your computer '
-          'network and are accessible under Network section in the file '
+          'at location \\\\{hostname} (on Windows) or smb://{hostname}.local '
-          'manager on your computer.'), hostname=socket.gethostname().upper())
+          '(on Linux and Mac). There are three types of shares '
+          'you can choose from: '), hostname=socket.gethostname()),
+    _('Open share - accessible to everyone in your local network.'),
+    _('Group share - accessible only to FreedomBox users who are in the '
+      'freedombox-share group.'),
+    _('Home share - every user in the freedombox-share group can have their '
+      'own private space.'),
 ]
 
-group = ('freedombox-share', _('Access shared folders from inside the server'))
+group = ('freedombox-share', _('Access to the private shares'))
 
 clients = clients
 
@@ -124,18 +133,43 @@ def diagnose():
     return results
 
 
-def add_share(mount_point, filesystem):
+def add_share(mount_point, share_type, filesystem):
     """Add a share."""
-    command = ['add-share', '--mount-point', mount_point]
+    command = [
+        'add-share', '--mount-point', mount_point, '--share-type', share_type
+    ]
     if filesystem in ['ntfs', 'vfat']:
         command = command + ['--windows-filesystem']
     actions.superuser_run('samba', command)
 
 
-def delete_share(mount_point):
+def delete_share(mount_point, share_type):
     """Delete a share."""
-    command = ['delete-share', '--mount-point', mount_point]
+    actions.superuser_run('samba', [
-    actions.superuser_run('samba', command)
+        'delete-share', '--mount-point', mount_point, '--share-type',
+        share_type
+    ])
+
+
+def get_users():
+    """Get non-system users who are in the freedombox-share or admin group."""
+    output = actions.superuser_run('samba', ['get-users'])
+    samba_users = json.loads(output)['users']
+    group_users = grp.getgrnam('freedombox-share').gr_mem + grp.getgrnam(
+        'admin').gr_mem
+
+    allowed_users = []
+    for group_user in group_users:
+        uid = pwd.getpwnam(group_user).pw_uid
+        if uid > 1000:
+            allowed_users.append(group_user)
+
+    return {
+        'access_ok':
+            sorted(set(allowed_users) & set(samba_users)),
+        'password_re_enter_needed':
+            sorted(set(allowed_users) - set(samba_users))
+    }
 
 
 def get_shares():
@@ -145,6 +179,15 @@ def get_shares():
     return json.loads(output)
 
 
+def disk_name(mount_point):
+    """Get a disk name."""
+    share_name = os.path.basename(mount_point)
+    if not share_name:
+        share_name = 'disk'
+
+    return share_name
+
+
 def backup_pre(packet):
     """Save registry share configuration."""
     actions.superuser_run('samba', ['dump-shares'])

plinth/modules/samba/static/samba.js

@@ -1,33 +0,0 @@
-/**
- * @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.
- */
-
-
-$(document).on('turbolinks:load', function() {
-  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

@@ -20,6 +20,7 @@
 
 {% load bootstrap %}
 {% load i18n %}
+{% load plinth_extras %}
 {% load static %}
 
 {% block page_head %}
@@ -35,79 +36,91 @@
   {{ block.super }}
 
   {% if is_enabled %}
-    <h3>{% trans "Select disks for sharing" %}</h3>
+    <h3>{% trans "Shares" %}</h3>
     <p>
     {% blocktrans trimmed %}
-      Note: only specially created directory will be shared on selected disks,
+      Note: only specially created directories will be shared on selected disks,
       not the whole disk.
     {% endblocktrans %}
     </p>
-    <table class="table table-bordered table-condensed table-striped">
+      <table class="table table-bordered table-condensed table-striped">
       <thead>
         <tr>
-          <th></th>
+          <th>{% trans "Disk Name" %}</th>
-          <th>{% trans "Device" %}</th>
+          <th>{% trans "Shares" %}</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 %}
+        {% for disk in disks %}
         <tr>
+          <td>{{ disk.name|default_if_none:"" }}</td>
           <td>
-            {% if disk.mount_point in shared_mounts %}
+            <form class="form shareform" method="post"
-              <form class="form shareform" method="post"
+                  action="{% url 'samba:share' disk.mount_point|urlencode:'' %}">
-                    action="{% url 'samba:unshare' disk.mount_point|urlencode:'' %}">
+            {% csrf_token %}
-                {% csrf_token %}
+            <input type="hidden" name="filesystem_type" value="{{ disk.filesystem_type }}">
-                <input type="checkbox" value="{{ disk.mount_point }}"
+
-                       name="mount_point" autocomplete="off" checked/>
+            {% for share_type in share_types %}
-              </form>
+              <button type="submit"
-            {% else %}
+                {% if share_type.0 in shared_mounts|lookup:disk.mount_point %}
-              <form class="form shareform" method="post"
+                  class="btn btn-success" name="{{ share_type.0 }}_share" value="disable"
-                    action="{% url 'samba:share' disk.mount_point|urlencode:'' %}">
+                {% else %}
-                {% csrf_token %}
+                  class="btn btn-default" name="{{ share_type.0 }}_share" value="enable"
-                <input type="hidden" value="{{ disk.filesystem_type }}"
+                {% endif %}
-                       name="filesystem_type">
+                {% if disk.filesystem_type == 'vfat' %}
-                <input type="checkbox" value="{{ disk.mount_point }}"
+                  title='{% trans "vfat partitions are not supported" %}' disabled
-                       name="mount_point" autocomplete="off"
+                {% endif %}>{{ share_type.1 }}
-                       {% if disk.filesystem_type == 'vfat' %}
+              </button>
-                         title='{% trans "vfat partitions are not supported" %}' disabled
+            {% endfor %}
-                       {% endif %}/>
+            </form>
-              </form>
-            {% endif %}
           </td>
-          <td>{{ disk.device }}</td>
+          <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"
+                <div class="progress-bar progress-bar-striped progress-bar-warning"
               {% else %}
-                    <div class="progress-bar progress-bar-striped progress-bar-danger"
+                <div class="progress-bar progress-bar-striped progress-bar-danger"
               {% endif %}
-                         role="progressbar" aria-valuenow="disk.percent_used"
+                     role="progressbar" aria-valuenow="{{ disk.percent_used }}"
-                         aria-valuemin="0" aria-valuemax="100"
+                     aria-valuemin="0" aria-valuemax="100"
-                         style="width: {{ disk.percent_used }}%;">
+                     style="width: {{ disk.percent_used }}%;">
-                      {{ disk.percent_used }}%
+                  {{ disk.percent_used }}%
-                    </div>
+                </div>
-                  </div>
+            </div>
-                  <div>{{ disk.used_str }} / {{ disk.size_str }}</div>
+            <div>{{ disk.used_str }} / {{ disk.size_str }}</div>
           </td>
         </tr>
-      {% endfor %}
+        {% endfor %}
       </tbody>
-    </table>
+      </table>
+
+      <p>
+      {% url 'storage:index' as storage_url %}
+      {% url 'users:index' as users_url %}
+      {% blocktrans trimmed %}
+        You can find additional information about disks on the
+        <a href="{{ storage_url }}">storage</a> module page and configure
+        access to the shares on the <a href="{{ users_url }}">users</a> module page.
+      {% endblocktrans %}</p>
+
+      <p>{% trans "Users who can currently access group and home shares" %}:
+        {{ users.access_ok|join:", " }}</p>
+
+      {% if users.password_re_enter_needed %}
+        <p>{% trans "Users who need to re-enter their password on the password change page to access group and home shares" %}:
+          <strong>{{ users.password_re_enter_needed|join:", " }}</strong>.</p>
+      {% endif %}
 
     {% if unavailable_shares %}
-      <h3>{% trans "Shares configured but the disk is not available" %}</h3>
+      <h3>{% trans "Unavailable Shares" %}</h3>
       <p>
-      {% trans "If the disk is plugged back in, sharing will be automatically enabled." %}
+      {% blocktrans trimmed %}
+        Shares that are configured but the disk is not available. If the disk
+        is plugged back in, sharing will be automatically enabled.
+      {% endblocktrans %}
       </p>
       <table class="table table-bordered table-condensed table-striped">
         <thead>
@@ -122,9 +135,12 @@
             <td>{{ share.name }}</td>
             <td>
               <form class="form" method="post"
-                    action="{% url 'samba:unshare' share.mount_point|urlencode:'' %}">
+                    action="{% url 'samba:share' share.mount_point|urlencode:'' %}">
                 {% csrf_token %}
-                <input type="submit" class="btn btn-danger" value="Delete">
+                <button type="submit" class="btn btn-danger"
+                        name="{{ share.share_type }}_share"
+                        value="disable">{% trans "Delete" %}
+                </button>
               </form>
             </td>
           </tr>
@@ -135,6 +151,3 @@
   {% endif %}
 {% endblock %}
 
-{% block page_js %}
-  <script type="text/javascript" src="{% static 'samba/samba.js' %}"></script>
-{% endblock %}

plinth/modules/samba/urls.py

@@ -25,7 +25,5 @@ from . import views
 urlpatterns = [
     url(r'^apps/samba/$', views.SambaAppView.as_view(), name='index'),
     url(r'^apps/samba/share/(?P<mount_point>[A-Za-z0-9%_.\-~]+)/$',
-        views.share, name='share'),
+        views.share, name='share')
-    url(r'^apps/samba/unshare/(?P<mount_point>[A-Za-z0-9%_.\-~]+)/$',
-        views.unshare, name='unshare'),
 ]

plinth/modules/samba/views.py

@@ -20,13 +20,16 @@ Views for samba module.
 
 import logging
 import urllib.parse
+from collections import defaultdict
 
 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.errors import ActionError
 from plinth.modules import samba, storage
 
 logger = logging.getLogger(__name__)
@@ -45,9 +48,20 @@ class SambaAppView(views.AppView):
         """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]
+
+        for disk in disks:
+            disk['name'] = samba.disk_name(disk['mount_point'])
+        context['disks'] = disks
+
+        shared_mounts = defaultdict(list)
+        for share in shares:
+            shared_mounts[share['mount_point']].append(share['share_type'])
+        context['shared_mounts'] = shared_mounts
+
+        context['share_types'] = [('open', _('Open Share')),
+                                  ('group', _('Group Share')),
+                                  ('home', _('Home Share'))]
 
         unavailable_shares = []
         for share in shares:
@@ -58,6 +72,8 @@ class SambaAppView(views.AppView):
                 unavailable_shares.append(share)
         context['unavailable_shares'] = unavailable_shares
 
+        context['users'] = samba.get_users()
+
         return context
 
 
@@ -70,35 +86,30 @@ def share(request, mount_point):
     """
     mount_point = urllib.parse.unquote(mount_point)
     filesystem = request.POST.get('filesystem_type', '')
-    try:
+
-        samba.add_share(mount_point, filesystem)
+    share_types = ['open', 'group', 'home']
-        messages.success(request, _('Share enabled.'))
+
-    except Exception as exception:
+    for share_type in share_types:
-        logger.exception('Error enabling share')
+        action = request.POST.get(share_type + '_share', '')
-        messages.error(
+        if action == 'enable':
-            request,
+            try:
-            _('Error enabling share: {error_message}').format(
+                samba.add_share(mount_point, share_type, filesystem)
-                error_message=exception))
+                messages.success(request, _('Share enabled.'))
-
+            except ActionError as exception:
-    return redirect(reverse('samba:index'))
+                logger.exception('Error enabling share')
-
+                messages.error(
-
+                    request,
-@require_POST
+                    _('Error enabling share: {error_message}').format(
-def unshare(request, mount_point):
+                        error_message=exception))
-    """Disable sharing, given its name.
+        elif action == 'disable':
-
+            try:
-    mount_point is urlquoted.
+                samba.delete_share(mount_point, share_type)
-
+                messages.success(request, _('Share disabled.'))
-    """
+            except ActionError as exception:
-    mount_point = urllib.parse.unquote(mount_point)
+                logger.exception('Error disabling share')
-    try:
+                messages.error(
-        samba.delete_share(mount_point)
+                    request,
-        messages.success(request, _('Share disabled.'))
+                    _('Error disabling share: {error_message}').format(
-    except Exception as exception:
+                        error_message=exception))
-        logger.exception('Error disabling share')
-        messages.error(
-            request,
-            _('Error disabling share: {error_message}').format(
-                error_message=exception))
 
     return redirect(reverse('samba:index'))

plinth/modules/storage/forms.py

@@ -150,8 +150,15 @@ class DirectorySelectForm(AppForm):
                 _('Default'), self.default))]
         available_shares = get_available_samba_shares()
         for share in available_shares:
-            choices = choices + [(share['path'], '{0} ({1}): {2}'.format(
+            if share['share_type'] != 'home':
-                _('Samba share'), share['name'], share['path']))]
+                share_type = _('Share')
+                if share['share_type'] == 'group':
+                    share_type = _('Group Share')
+                elif share['share_type'] == 'open':
+                    share_type = _('Open Share')
+                selection_text = 'Samba {0} ({1}): {2}'.format(
+                    share_type, share['name'], share['path'])
+                choices = choices + [(share['path'], selection_text)]
         choices = choices + [('/', _('Other directory (specify below)'))]
 
         initial_value, subdir = self.get_initial(choices)

plinth/modules/users/__init__.py

@@ -34,7 +34,7 @@ is_essential = True
 
 managed_packages = [
     'ldapscripts', 'ldap-utils', 'libnss-ldapd', 'libpam-ldapd', 'nscd',
-    'nslcd', 'slapd'
+    'nslcd', 'samba-common-bin', 'slapd', 'tdb-tools'
 ]
 
 first_boot_steps = [

plinth/modules/users/forms.py

@@ -17,6 +17,7 @@
 
 import subprocess
 
+import plinth.forms
 from django import forms
 from django.contrib import auth, messages
 from django.contrib.auth.forms import SetPasswordForm, UserCreationForm
@@ -24,8 +25,6 @@ from django.contrib.auth.models import Group, User
 from django.core.exceptions import ValidationError
 from django.utils.translation import ugettext as _
 from django.utils.translation import ugettext_lazy
-
-import plinth.forms
 from plinth import actions, module_loader
 from plinth.errors import ActionError
 from plinth.modules import first_boot, users
@@ -262,6 +261,21 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
             except ActionError:
                 messages.error(self.request, _('Unable to set SSH keys.'))
 
+            is_active = self.cleaned_data['is_active']
+            if self.initial['is_active'] != is_active:
+                if is_active:
+                    status = 'active'
+                else:
+                    status = 'inactive'
+                try:
+                    actions.superuser_run(
+                        'users',
+                        ['set-user-status',
+                         user.get_username(), status])
+                except ActionError:
+                    messages.error(self.request,
+                                   _('Failed to change user status.'))
+
         return user
 
     def validate_last_admin_user(self, groups):