users: Replace disabled with readonly for admin group checkbox

- Added validation logic in the backend to compensate

Signed-off-by: Joseph Nuthalapati <njoseph@thoughtworks.com>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/forms.py

@@ -87,32 +87,34 @@ class LanguageSelectionForm(LanguageSelectionFormMixin, forms.Form):
     language = LanguageSelectionFormMixin.language
 
 
-class CheckboxSelectMultipleWithDisabled(forms.widgets.CheckboxSelectMultiple):
+class CheckboxSelectMultipleWithReadOnly(forms.widgets.CheckboxSelectMultiple):
     """
-    Subclass of Django's checkbox select multiple widget that allows disabling checkbox-options.
+    Subclass of Django's CheckboxSelectMultiple widget that allows setting
-    To disable an option, pass a dict instead of a string for its label,
+    individual fields as readonly
-    of the form: {'label': 'option label', 'disabled': True}
+    To mark a feature as readonly an option, pass a dict instead of a string
+    for its label, of the form: {'label': 'option label', 'disabled': True}
 
     Derived from https://djangosnippets.org/snippets/2786/
     """
 
     def render(self, name, value, attrs=None, choices=(), renderer=None):
-        if value is None: value = []
+        if value is None:
+            value = []
         final_attrs = self.build_attrs(attrs)
         output = [u'<ul>']
-        global_disabled = 'disabled' in final_attrs
+        global_readonly = 'readonly' in final_attrs
         str_values = set([v for v in value])
         for i, (option_value, option_label) in enumerate(
                 chain(self.choices, choices)):
-            if not global_disabled and 'disabled' in final_attrs:
+            if not global_readonly and 'readonly' in final_attrs:
-                # If the entire group is disabled keep all options disabled
+                # If the entire group is readonly keep all options readonly
-                del final_attrs['disabled']
+                del final_attrs['readonly']
             if isinstance(option_label, dict):
-                if dict.get(option_label, 'disabled'):
+                if dict.get(option_label, 'readonly'):
-                    final_attrs = dict(final_attrs, disabled='disabled')
+                    final_attrs = dict(final_attrs, readonly='readonly')
                 option_label = option_label['label']
-            final_attrs = dict(final_attrs, id='%s_%s' % (attrs['id'], i))
+            final_attrs = dict(final_attrs, id='{}_{}'.format(attrs['id'], i))
-            label_for = u' for="%s"' % final_attrs['id']
+            label_for = u' for="{}"'.format(final_attrs['id'])
             cb = CheckboxInput(final_attrs,
                                check_test=lambda value: value in str_values)
             rendered_cb = cb.render(name, option_value)

plinth/modules/users/forms.py

@@ -22,7 +22,6 @@ 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 _
 from django.utils.translation import ugettext_lazy
 
@@ -87,8 +86,8 @@ class CreateUserForm(ValidNewUsernameCheckMixin,
     """
     groups = forms.MultipleChoiceField(
         choices=get_group_choices(), label=ugettext_lazy('Permissions'),
-        required=False,
+        required=False, widget=forms.CheckboxSelectMultiple,
-        widget=forms.CheckboxSelectMultiple, help_text=ugettext_lazy(
+        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 '
@@ -147,8 +146,8 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
                      plinth.forms.LanguageSelectionFormMixin, forms.ModelForm):
     """When user info is changed, also updates LDAP user."""
     ssh_keys = forms.CharField(
-        label=ugettext_lazy('SSH Keys'),
+        label=ugettext_lazy('SSH Keys'), required=False, widget=forms.Textarea,
-        required=False, widget=forms.Textarea, help_text=ugettext_lazy(
+        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 '
@@ -162,7 +161,7 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
         fields = ('username', 'groups', 'ssh_keys', 'language', 'is_active')
         model = User
         widgets = {
-            'groups': plinth.forms.CheckboxSelectMultipleWithDisabled(),
+            'groups': plinth.forms.CheckboxSelectMultipleWithReadOnly(),
         }
 
     def __init__(self, request, username, *args, **kwargs):
@@ -174,7 +173,7 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
         self.request = request
         self.username = username
         super(UserUpdateForm, self).__init__(*args, **kwargs)
-        last_admin_user = get_last_admin_user()
+        self.is_last_admin_user = get_last_admin_user() == self.username
 
         choices = []
 
@@ -183,8 +182,11 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
             # applications not installed yet.
             if c[1] in group_choices:
                 # Replace group names with descriptions
-                if c[1] == 'admin' and last_admin_user is not None:
+                if c[1] == 'admin' and self.is_last_admin_user:
-                    choices.append((c[0], {'label': group_choices[c[1]], 'disabled': True}))
+                    choices.append((c[0], {
+                        'label': group_choices[c[1]],
+                        'readonly': True
+                    }))
                 else:
                     choices.append((c[0], group_choices[c[1]]))
 
@@ -195,7 +197,7 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
             self.fields['is_active'].widget = forms.HiddenInput()
             self.fields['groups'].disabled = True
 
-        if last_admin_user and last_admin_user == self.username:
+        if self.is_last_admin_user:
             self.fields['is_active'].disabled = True
 
     def save(self, commit=True):
@@ -261,6 +263,19 @@ class UserUpdateForm(ValidNewUsernameCheckMixin,
 
         return user
 
+    def validate_last_admin_user(self, groups):
+        group_names = [group.name for group in groups]
+        if 'admin' not in group_names:
+            raise ValidationError(
+                _('Cannot delete the only administrator in the system.'))
+
+    def clean(self):
+        """Override clean to add form validation logic."""
+        cleaned_data = super().clean()
+        if self.is_last_admin_user:
+            self.validate_last_admin_user(cleaned_data.get("groups"))
+        return cleaned_data
+
 
 class UserChangePasswordForm(SetPasswordForm):
     """Custom form that also updates password for LDAP users."""