diff --git a/actions/change-user-password b/actions/change-user-password index 98509953f..a7c953207 100755 --- a/actions/change-user-password +++ b/actions/change-user-password @@ -30,5 +30,5 @@ fi echo "$username:$password" | chpasswd if [ $? -ne 0 ]; then echo "Failed: could not set user password" - exit + exit 2 fi diff --git a/actions/create-user b/actions/create-user index 748ba4025..b309c2c78 100755 --- a/actions/create-user +++ b/actions/create-user @@ -22,17 +22,17 @@ password="$2" adduser --disabled-password --gecos "" "$username" if [ $? -ne 0 ]; then echo "Failed to create user" - exit + exit 1 fi adduser "$username" sudo if [ $? -ne 0 ]; then echo "Failed to add user to sudo group" - exit + exit 2 fi echo "$username:$password" | chpasswd if [ $? -ne 0 ]; then echo "Failed to set user password" - exit + exit 3 fi diff --git a/actions/delete-user b/actions/delete-user index e5dd0ac4d..00c653191 100755 --- a/actions/delete-user +++ b/actions/delete-user @@ -31,4 +31,5 @@ if [ $? -eq 0 ]; then echo "Success: user deleted" else echo "Failed: userdel error" + exit 2 fi diff --git a/actions/disable-user b/actions/disable-user index 013ddbc39..a61bca536 100755 --- a/actions/disable-user +++ b/actions/disable-user @@ -20,7 +20,14 @@ username="$1" +getent passwd "$username" +if [ $? -ne 0 ]; then + echo "Failed: user not found" + exit +fi + usermod --expiredate 1 "$username" if [ $? -ne 0 ]; then echo "Failed" + exit 2 fi diff --git a/actions/enable-user b/actions/enable-user index 7cfe857a8..265c1f7c2 100755 --- a/actions/enable-user +++ b/actions/enable-user @@ -20,7 +20,14 @@ username="$1" +getent passwd "$username" +if [ $? -ne 0 ]; then + echo "Failed: user not found" + exit +fi + usermod --expiredate "" "$username" if [ $? -ne 0 ]; then echo "Failed" + exit 2 fi diff --git a/plinth/modules/first_boot/forms.py b/plinth/modules/first_boot/forms.py index bb26a401a..bbb22b563 100644 --- a/plinth/modules/first_boot/forms.py +++ b/plinth/modules/first_boot/forms.py @@ -21,6 +21,7 @@ from django.core import validators from gettext import gettext as _ from plinth import actions +from plinth.errors import ActionError from plinth.modules.config import config @@ -62,9 +63,13 @@ than 63 characters in length.'), user.set_password(self.cleaned_data['password']) if commit: user.save() - actions.superuser_run( - 'create-user', - [user.username, self.cleaned_data['password']]) + try: + actions.superuser_run( + 'create-user', + [user.get_username(), self.cleaned_data['password']]) + except ActionError: + messages.error(self.request, + _('Creating POSIX system user failed.')) self.login_user() return user diff --git a/plinth/modules/users/__init__.py b/plinth/modules/users/__init__.py index 65471cd80..6a68e6bac 100644 --- a/plinth/modules/users/__init__.py +++ b/plinth/modules/users/__init__.py @@ -19,12 +19,9 @@ Plinth module to manage users """ -from django.db.models.signals import post_delete -from django.contrib.auth.models import User from gettext import gettext as _ from plinth import cfg -from plinth import actions __all__ = ['init'] @@ -36,14 +33,3 @@ def init(): menu = cfg.main_menu.get('system:index') menu.add_urlname(_('Users and Groups'), 'glyphicon-user', 'users:index', 15) - - post_delete.connect(delete_posix_user) - - -def delete_posix_user(sender, instance=None, **kwargs): - try: - instance - except User.DoesNotExist: - pass - else: - actions.superuser_run('delete-user', [instance.username]) diff --git a/plinth/modules/users/forms.py b/plinth/modules/users/forms.py index aeaf54271..28454f060 100644 --- a/plinth/modules/users/forms.py +++ b/plinth/modules/users/forms.py @@ -16,29 +16,42 @@ # from django import forms +from django.contrib import messages from django.contrib.auth.models import User from django.contrib.auth.forms import UserCreationForm, SetPasswordForm from gettext import gettext as _ from plinth import actions +from plinth.errors import ActionError class CreateUserForm(UserCreationForm): """Custom user create form with option to also create POSIX user.""" add_posix_user = forms.BooleanField( - label=_('Also create POSIX user'), + label=_('Also create a POSIX system user'), required=False, help_text=_('This will allow the new user to log in to the system ' 'through SSH. The new user will also have administrative ' 'privileges (sudo).')) + def __init__(self, request, *args, **kwargs): + """Initialize the form with extra request argument.""" + self.request = request + super(CreateUserForm, self).__init__(*args, **kwargs) + def save(self, commit=True): + """Save the user model and create POSIX user if required.""" user = super(CreateUserForm, self).save(commit) if commit and self.cleaned_data['add_posix_user']: - actions.superuser_run( - 'create-user', - [user.username, self.cleaned_data['password1']]) + try: + actions.superuser_run( + 'create-user', + [user.get_username(), self.cleaned_data['password1']]) + except ActionError: + messages.error(self.request, + _('Creating POSIX system user failed.')) + return user @@ -46,29 +59,57 @@ class UserUpdateForm(forms.ModelForm): """When user is enabled/disabled, also enables/disables the POSIX user.""" class Meta: + """Metadata to control automatic form building.""" fields = ('username', 'groups', 'is_active') model = User widgets = { 'groups': forms.widgets.CheckboxSelectMultiple(), } + def __init__(self, request, username, *args, **kwargs): + """Initialize the form with extra request argument.""" + self.request = request + self.username = username + super(UserUpdateForm, self).__init__(*args, **kwargs) + def save(self, commit=True): + """Enable/disable POSIX user after saving user model.""" user = super(UserUpdateForm, self).save(commit) + if commit: - if user.is_active: - actions.superuser_run('enable-user', [user.username]) - else: - actions.superuser_run('disable-user', [user.username]) + try: + if user.is_active: + actions.superuser_run('enable-user', [user.get_username()]) + else: + actions.superuser_run('disable-user', + [user.get_username()]) + except ActionError: + messages.error( + self.request, + _('Setting active status for POSIX system user failed.')) + return user class UserChangePasswordForm(SetPasswordForm): """Custom form that also updates password for POSIX users.""" + def __init__(self, request, *args, **kwargs): + """Initialize the form with extra request argument.""" + self.request = request + super(UserChangePasswordForm, self).__init__(*args, **kwargs) + def save(self, commit=True): + """Save the user model and change POSIX password as well.""" user = super(UserChangePasswordForm, self).save(commit) if commit: - actions.superuser_run( - 'change-user-password', - [user.username, self.cleaned_data['new_password1']]) + try: + actions.superuser_run( + 'change-user-password', + [user.get_username(), self.cleaned_data['new_password1']]) + except ActionError: + messages.error( + self.request, + _('Changing POSIX system user password failed.')) + return user diff --git a/plinth/modules/users/views.py b/plinth/modules/users/views.py index 16f9be4a5..05d6a4f3d 100644 --- a/plinth/modules/users/views.py +++ b/plinth/modules/users/views.py @@ -25,8 +25,9 @@ from django.views.generic.edit import (CreateView, DeleteView, UpdateView, from django.views.generic import ListView from gettext import gettext as _ -from .forms import CreateUserForm, UserUpdateForm, UserChangePasswordForm +from .forms import CreateUserForm, UserChangePasswordForm, UserUpdateForm from plinth import actions +from plinth.errors import ActionError subsubmenu = [{'url': reverse_lazy('users:index'), @@ -54,6 +55,12 @@ class UserCreate(ContextMixin, SuccessMessageMixin, CreateView): success_url = reverse_lazy('users:create') title = _('Create User') + def get_form_kwargs(self): + """Make the request object available to the form.""" + kwargs = super(UserCreate, self).get_form_kwargs() + kwargs['request'] = self.request + return kwargs + class UserList(ContextMixin, ListView): """View to list users.""" @@ -71,7 +78,15 @@ class UserUpdate(ContextMixin, SuccessMessageMixin, UpdateView): success_message = _('User %(username)s updated.') title = _('Edit User') + def get_form_kwargs(self): + """Make the requst object available to the form.""" + kwargs = super(UserUpdate, self).get_form_kwargs() + kwargs['request'] = self.request + kwargs['username'] = self.object.username + return kwargs + def get_context_data(self, **kwargs): + """Return the data to be used for rendering templates.""" context = super(UserUpdate, self).get_context_data(**kwargs) output = actions.run('check-user-exists', [self.object.username]) context['is_posix_user'] = 'User exists' in output @@ -107,9 +122,17 @@ class UserDelete(ContextMixin, DeleteView): The SuccessMessageMixin doesn't work with the DeleteView on Django1.7, so set the success message manually here. """ - message = _('User %s deleted.') % self.kwargs['slug'] output = super(UserDelete, self).delete(*args, **kwargs) + + message = _('User %s deleted.') % self.kwargs['slug'] messages.success(self.request, message) + + try: + actions.superuser_run('delete-user', [self.kwargs['slug']]) + except ActionError: + messages.error(self.request, + _('Deleting POSIX system user failed.')) + return output @@ -121,8 +144,9 @@ class UserChangePassword(ContextMixin, SuccessMessageMixin, FormView): success_message = _('Password changed successfully.') def get_form_kwargs(self): - """Make the user object available to the form""" + """Make the user object available to the form.""" kwargs = super(UserChangePassword, self).get_form_kwargs() + kwargs['request'] = self.request kwargs['user'] = User.objects.get(username=self.kwargs['slug']) return kwargs @@ -134,9 +158,17 @@ class UserChangePassword(ContextMixin, SuccessMessageMixin, FormView): return context def get_success_url(self): + """Return the URL to go to on successful sumbission.""" return reverse('users:edit', kwargs={'slug': self.kwargs['slug']}) def form_valid(self, form): + """Save the form if the submission is valid. + + Django user session authentication hashes are based on password to have + the ability to logout all sessions on password change. Update the + session authentications to ensure that the current sessions is not + logged out. + """ form.save() update_session_auth_hash(self.request, form.user) return super(UserChangePassword, self).form_valid(form)