From 32b2ef38c7103eb7b480f753fd262531cf23c42d Mon Sep 17 00:00:00 2001 From: Joseph Nuthalapati Date: Thu, 23 Nov 2017 12:59:57 +0530 Subject: [PATCH] Fixes for user groups - Edit user form fails because a 'wiki' group entry exists in the database though the ikiwiki app hasn't been installed yet. - Register group when a user group is created by an application, so that a plinth restart can be avoided. Signed-off-by: Joseph Nuthalapati Reviewed-by: James Valleroy --- actions/users | 104 +++++++++++-------------- plinth/modules/users/__init__.py | 35 ++++----- plinth/modules/users/forms.py | 127 +++++++++++++++---------------- 3 files changed, 121 insertions(+), 145 deletions(-) diff --git a/actions/users b/actions/users index 794aa87cd..a78d1af87 100755 --- a/actions/users +++ b/actions/users @@ -21,11 +21,11 @@ Configuration helper for the LDAP user directory """ import argparse -import augeas import re import subprocess import sys +import augeas from plinth import action_utils ACCESS_CONF = '/etc/security/access.conf' @@ -39,44 +39,41 @@ def parse_arguments(): subparsers.add_parser('setup', help='Setup LDAP') - subparser = subparsers.add_parser( - 'create-user', help='Create an LDAP user') + subparser = subparsers.add_parser('create-user', + help='Create an LDAP user') subparser.add_argument('username', help='Name of the LDAP user to create') - subparser = subparsers.add_parser( - 'remove-user', help='Delete an LDAP user') + subparser = subparsers.add_parser('remove-user', + help='Delete an LDAP user') subparser.add_argument('username', help='Name of the LDAP user to delete') - subparser = subparsers.add_parser( - 'rename-user', help='Rename an LDAP user') + subparser = subparsers.add_parser('rename-user', + help='Rename an LDAP user') subparser.add_argument('oldusername', help='Old name of the LDAP user') subparser.add_argument('newusername', help='New name of the LDAP user') - subparser = subparsers.add_parser( - 'set-user-password', help='Set the password of an LDAP user') + subparser = subparsers.add_parser('set-user-password', + help='Set the password of an LDAP user') subparser.add_argument( 'username', help='Name of the LDAP user to set the password for') - subparser = subparsers.add_parser( - 'create-group', help='Create an LDAP group') - subparser.add_argument( - 'groupname', help='Name of the LDAP group to create') + subparser = subparsers.add_parser('create-group', + help='Create an LDAP group') + subparser.add_argument('groupname', + help='Name of the LDAP group to create') - subparser = subparsers.add_parser( - 'remove-group', help='Delete an LDAP group') - subparser.add_argument( - 'groupname', help='Name of the LDAP group to delete') + subparser = subparsers.add_parser('remove-group', + help='Delete an LDAP group') + subparser.add_argument('groupname', + help='Name of the LDAP group to delete') subparser = subparsers.add_parser( 'get-user-groups', help='Get all the LDAP groups for an LDAP user') - subparser.add_argument( - 'username', help='LDAP user to retrieve the groups for') + subparser.add_argument('username', + help='LDAP user to retrieve the groups for') - subparser = subparsers.add_parser( - 'get-all-groups', help='Get a list of all the LDAP groups in the system') - - subparser = subparsers.add_parser( - 'add-user-to-group', help='Add an LDAP user to an LDAP group') + subparser = subparsers.add_parser('add-user-to-group', + help='Add an LDAP user to an LDAP group') subparser.add_argument('username', help='LDAP user to add to group') subparser.add_argument('groupname', help='LDAP group to add the user to') @@ -84,8 +81,8 @@ def parse_arguments(): 'remove-user-from-group', help='Remove an LDAP user from an LDAP group') subparser.add_argument('username', help='LDAP user to remove from group') - subparser.add_argument( - 'groupname', help='LDAP group to remove the user from') + subparser.add_argument('groupname', + help='LDAP group to remove the user from') subparsers.required = True return parser.parse_args() @@ -138,13 +135,10 @@ def create_organizational_unit(unit): """Create an organizational unit in LDAP.""" distinguished_name = 'ou={unit},dc=thisbox'.format(unit=unit) try: - subprocess.run( - [ - 'ldapsearch', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///', '-s', - 'base', '-b', distinguished_name, '(objectclass=*)' - ], - stdout=subprocess.DEVNULL, - check=True) + subprocess.run([ + 'ldapsearch', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///', '-s', + 'base', '-b', distinguished_name, '(objectclass=*)' + ], stdout=subprocess.DEVNULL, check=True) return # Already exists except subprocess.CalledProcessError: input = ''' @@ -152,23 +146,18 @@ dn: ou={unit},dc=thisbox objectClass: top objectClass: organizationalUnit ou: {unit}'''.format(unit=unit) - subprocess.run( - ['ldapadd', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'], - input=input.encode(), - stdout=subprocess.DEVNULL, - check=True) + subprocess.run(['ldapadd', '-Q', '-Y', 'EXTERNAL', '-H', + 'ldapi:///'], input=input.encode(), + stdout=subprocess.DEVNULL, check=True) def setup_admin(): """Remove LDAP admin password and Allow root to modify the users.""" - process = subprocess.run( - [ - 'ldapsearch', '-Q', '-L', '-L', '-L', '-Y', 'EXTERNAL', '-H', - 'ldapi:///', '-s', 'base', '-b', 'olcDatabase={1}mdb,cn=config', - '(objectclass=*)', 'olcRootDN', 'olcRootPW' - ], - check=True, - stdout=subprocess.PIPE) + process = subprocess.run([ + 'ldapsearch', '-Q', '-L', '-L', '-L', '-Y', 'EXTERNAL', '-H', + 'ldapi:///', '-s', 'base', '-b', 'olcDatabase={1}mdb,cn=config', + '(objectclass=*)', 'olcRootDN', 'olcRootPW' + ], check=True, stdout=subprocess.PIPE) ldap_object = {} for line in process.stdout.decode().splitlines(): if line: @@ -177,10 +166,8 @@ def setup_admin(): if 'olcRootPW' in ldap_object: subprocess.run( - ['ldapmodify', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'], - check=True, - stdout=subprocess.DEVNULL, - input=b''' + ['ldapmodify', '-Q', '-Y', 'EXTERNAL', '-H', + 'ldapi:///'], check=True, stdout=subprocess.DEVNULL, input=b''' dn: olcDatabase={1}mdb,cn=config changetype: modify delete: olcRootPW''') @@ -188,10 +175,8 @@ delete: olcRootPW''') root_dn = 'gidNumber=0+uidNumber=0,cn=peercred,cn=external,cn=auth' if ldap_object['olcRootDN'] != root_dn: subprocess.run( - ['ldapmodify', '-Q', '-Y', 'EXTERNAL', '-H', 'ldapi:///'], - check=True, - stdout=subprocess.DEVNULL, - input=b''' + ['ldapmodify', '-Q', '-Y', 'EXTERNAL', '-H', + 'ldapi:///'], check=True, stdout=subprocess.DEVNULL, input=b''' dn: olcDatabase={1}mdb,cn=config changetype: modify replace: olcRootDN @@ -261,8 +246,7 @@ def subcommand_rename_user(arguments): def set_user_password(username, password): """Set a user's password.""" - process = _run( - ['slappasswd', '-s', password], stdout=subprocess.PIPE) + process = _run(['slappasswd', '-s', password], stdout=subprocess.PIPE) password = process.stdout.decode().strip() _run(['ldapsetpasswd', username, password]) @@ -276,14 +260,14 @@ def get_user_groups(username): """Returns only the supplementary groups of the given user. Exclude the 'users' primary group from the returned list.""" - process = _run( - ['ldapid', username], stdout=subprocess.PIPE, check=False) + process = _run(['ldapid', username], stdout=subprocess.PIPE, check=False) output = process.stdout.decode().strip() if output: groups_part = output.split(' ')[2] groups = groups_part.split('=')[1] - group_names = [user.strip('()') - for user in re.findall('\(.*?\)', groups)] + group_names = [ + user.strip('()') for user in re.findall('\(.*?\)', groups) + ] group_names.remove('users') return group_names diff --git a/plinth/modules/users/__init__.py b/plinth/modules/users/__init__.py index 2dc6a9d5b..3db4d1210 100644 --- a/plinth/modules/users/__init__.py +++ b/plinth/modules/users/__init__.py @@ -14,26 +14,25 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . # - """ Plinth module to manage users """ -from django.utils.translation import ugettext_lazy as _ import subprocess -from plinth import action_utils -from plinth import actions -from plinth.errors import ActionError -from plinth.menu import main_menu +from django.utils.translation import ugettext_lazy as _ +from plinth import action_utils, actions +from plinth.menu import main_menu version = 1 is_essential = True -managed_packages = ['ldapscripts', 'ldap-utils', 'libnss-ldapd', - 'libpam-ldapd', 'nslcd', 'slapd'] +managed_packages = [ + 'ldapscripts', 'ldap-utils', 'libnss-ldapd', 'libpam-ldapd', 'nslcd', + 'slapd' +] first_boot_steps = [ { @@ -80,19 +79,22 @@ def _diagnose_ldap_entry(search_item): result = 'failed' try: - subprocess.check_output(['ldapsearch', '-x', '-b', 'dc=thisbox', - search_item]) + subprocess.check_output( + ['ldapsearch', '-x', '-b', 'dc=thisbox', search_item]) result = 'passed' except subprocess.CalledProcessError: pass - return [_('Check LDAP entry "{search_item}"') - .format(search_item=search_item), result] + return [ + _('Check LDAP entry "{search_item}"').format(search_item=search_item), + result + ] def create_group(group): """Add an LDAP group.""" actions.superuser_run('users', options=['create-group', group]) + register_group(group) def remove_group(group): @@ -100,14 +102,5 @@ def remove_group(group): actions.superuser_run('users', options=['remove-group', group]) -def get_all_groups(): - """Retrieve the set of all LDAP groups in the system""" - try: - groups = actions.superuser_run('users', options=['get-all-groups']) - return set(groups.strip().split()) - except ActionError: - return {} - - def register_group(group): groups.add(group) diff --git a/plinth/modules/users/forms.py b/plinth/modules/users/forms.py index 0925555b9..09ff81ed6 100644 --- a/plinth/modules/users/forms.py +++ b/plinth/modules/users/forms.py @@ -18,20 +18,18 @@ import subprocess from django import forms -from django.contrib import auth -from django.contrib import messages -from django.contrib.auth.models import User, Group -from django.contrib.auth.forms import UserCreationForm, SetPasswordForm +from django.contrib import auth, messages +from django.contrib.auth.forms import SetPasswordForm, UserCreationForm +from django.contrib.auth.models import Group, User from django.core.exceptions import ValidationError -from django.utils.translation import ugettext as _, ugettext_lazy +from django.utils.translation import ugettext as _ +from django.utils.translation import ugettext_lazy -from plinth import actions +from plinth import actions, module_loader from plinth.errors import ActionError -from plinth.modules import first_boot -from plinth.modules import users +from plinth.modules import first_boot, users from plinth.modules.security import set_restricted_access from plinth.utils import is_user_admin -from plinth import module_loader def get_group_choices(): @@ -50,8 +48,8 @@ class ValidNewUsernameCheckMixin(object): username = self.cleaned_data['username'] if self.instance.username != username and \ not self.is_valid_new_username(): - raise ValidationError(_('Username is taken or is reserved.'), - code='invalid') + raise ValidationError( + _('Username is taken or is reserved.'), code='invalid') return username @@ -80,11 +78,9 @@ class CreateUserForm(ValidNewUsernameCheckMixin, UserCreationForm): Include options to add user to groups. """ groups = forms.MultipleChoiceField( - choices=get_group_choices(), - label=ugettext_lazy('Permissions'), + choices=get_group_choices(), label=ugettext_lazy('Permissions'), required=False, - widget=forms.CheckboxSelectMultiple, - help_text=ugettext_lazy( + widget=forms.CheckboxSelectMultiple, help_text=ugettext_lazy( 'Select which services should be available to the new ' 'user. The user will be able to log in to services that ' 'support single sign-on through LDAP, if they are in the ' @@ -105,24 +101,23 @@ class CreateUserForm(ValidNewUsernameCheckMixin, UserCreationForm): if commit: try: - actions.superuser_run( - 'users', - ['create-user', user.get_username()], - input=self.cleaned_data['password1'].encode()) + actions.superuser_run('users', [ + 'create-user', user.get_username() + ], input=self.cleaned_data['password1'].encode()) except ActionError: - messages.error(self.request, - _('Creating LDAP user failed.')) + messages.error(self.request, _('Creating LDAP user failed.')) for group in self.cleaned_data['groups']: try: - actions.superuser_run( - 'users', - ['add-user-to-group', user.get_username(), group]) + actions.superuser_run('users', [ + 'add-user-to-group', + user.get_username(), group + ]) except ActionError: messages.error( self.request, - _('Failed to add new user to {group} group.') - .format(group=group)) + _('Failed to add new user to {group} group.').format( + group=group)) group_object, created = Group.objects.get_or_create(name=group) group_object.user_set.add(user) @@ -134,9 +129,7 @@ class UserUpdateForm(ValidNewUsernameCheckMixin, forms.ModelForm): """When user info is changed, also updates LDAP user.""" ssh_keys = forms.CharField( label=ugettext_lazy('SSH Keys'), - required=False, - widget=forms.Textarea, - help_text=ugettext_lazy( + required=False, widget=forms.Textarea, help_text=ugettext_lazy( 'Setting an SSH public key will allow this user to ' 'securely log in to the system without using a ' 'password. You may enter multiple keys, one on each ' @@ -161,9 +154,14 @@ class UserUpdateForm(ValidNewUsernameCheckMixin, forms.ModelForm): self.username = username super(UserUpdateForm, self).__init__(*args, **kwargs) - choices = [ # Replace group names with descriptions - (c[0], group_choices[c[1]]) - for c in sorted(self.fields['groups'].choices, key=lambda x: x[1])] + choices = [] + + for c in sorted(self.fields['groups'].choices, key=lambda x: x[1]): + # Handle case where groups exist in database for + # applications not installed yet. + if c[1] in group_choices: + # Replace group names with descriptions + choices.append((c[0], group_choices[c[1]])) self.fields['groups'].label = 'Permissions' self.fields['groups'].choices = choices @@ -177,16 +175,17 @@ class UserUpdateForm(ValidNewUsernameCheckMixin, forms.ModelForm): user = super(UserUpdateForm, self).save(commit) if commit: - output = actions.superuser_run( - 'users', ['get-user-groups', self.username]) + output = actions.superuser_run('users', + ['get-user-groups', self.username]) old_groups = output.strip().split('\n') old_groups = [group for group in old_groups if group] if self.username != user.get_username(): try: - actions.superuser_run( - 'users', - ['rename-user', self.username, user.get_username()]) + actions.superuser_run('users', [ + 'rename-user', self.username, + user.get_username() + ]) except ActionError: messages.error(self.request, _('Renaming LDAP user failed.')) @@ -195,10 +194,10 @@ class UserUpdateForm(ValidNewUsernameCheckMixin, forms.ModelForm): for old_group in old_groups: if old_group not in new_groups: try: - actions.superuser_run( - 'users', - ['remove-user-from-group', user.get_username(), - old_group]) + actions.superuser_run('users', [ + 'remove-user-from-group', + user.get_username(), old_group + ]) except ActionError: messages.error(self.request, _('Failed to remove user from group.')) @@ -206,18 +205,20 @@ class UserUpdateForm(ValidNewUsernameCheckMixin, forms.ModelForm): for new_group in new_groups: if new_group not in old_groups: try: - actions.superuser_run( - 'users', - ['add-user-to-group', user.get_username(), - new_group]) + actions.superuser_run('users', [ + 'add-user-to-group', + user.get_username(), new_group + ]) except ActionError: messages.error(self.request, _('Failed to add user to group.')) try: - actions.superuser_run( - 'ssh', ['set-keys', '--username', user.get_username(), - '--keys', self.cleaned_data['ssh_keys'].strip()]) + actions.superuser_run('ssh', [ + 'set-keys', '--username', + user.get_username(), '--keys', + self.cleaned_data['ssh_keys'].strip() + ]) except ActionError: messages.error(self.request, _('Unable to set SSH keys.')) @@ -237,14 +238,13 @@ class UserChangePasswordForm(SetPasswordForm): user = super(UserChangePasswordForm, self).save(commit) if commit: try: - actions.superuser_run( - 'users', - ['set-user-password', user.get_username()], - input=self.cleaned_data['new_password1'].encode()) + actions.superuser_run('users', [ + 'set-user-password', + user.get_username() + ], input=self.cleaned_data['new_password1'].encode()) except ActionError: - messages.error( - self.request, - _('Changing LDAP user password failed.')) + messages.error(self.request, + _('Changing LDAP user password failed.')) return user @@ -263,18 +263,17 @@ class FirstBootForm(ValidNewUsernameCheckMixin, auth.forms.UserCreationForm): first_boot.mark_step_done('users_firstboot') try: - actions.superuser_run( - 'users', - ['create-user', user.get_username()], - input=self.cleaned_data['password1'].encode()) + actions.superuser_run('users', [ + 'create-user', user.get_username() + ], input=self.cleaned_data['password1'].encode()) except ActionError: - messages.error(self.request, - _('Creating LDAP user failed.')) + messages.error(self.request, _('Creating LDAP user failed.')) try: - actions.superuser_run( - 'users', - ['add-user-to-group', user.get_username(), 'admin']) + actions.superuser_run('users', [ + 'add-user-to-group', + user.get_username(), 'admin' + ]) except ActionError: messages.error(self.request, _('Failed to add new user to admin group.'))