From 8aefb4c9fa63e55664715e292d52e4584440e60e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 22 Jan 2025 11:29:25 +0100 Subject: [PATCH] Fix save and update user (#1019) (#1020) Co-authored-by: wlorenzetti (cherry picked from commit 97c79a2576056f64394b940e5d98e5b75bd891aa) Co-authored-by: Walter Lorenzetti --- g3w-admin/usersmanage/forms.py | 28 ++++++++++++++++++----- g3w-admin/usersmanage/tests/test_forms.py | 22 ++++++++++++++++++ 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/g3w-admin/usersmanage/forms.py b/g3w-admin/usersmanage/forms.py index a9a40ef67..79f6da72a 100644 --- a/g3w-admin/usersmanage/forms.py +++ b/g3w-admin/usersmanage/forms.py @@ -538,12 +538,32 @@ def filterFieldsByRoles(self, **kwargs): def save(self, commit=True): - user = super(UserCreationForm, self).save(commit=False) + + # Not inherit from ancestor for new Django 4.2 UserCreationForm + if self.errors: + raise ValueError( + "The %s could not be %s because the data didn't validate." + % ( + self.instance._meta.object_name, + "created" if self.instance._state.adding else "changed", + ) + ) + # If not committing, add a method to the form to allow deferred + # saving of m2m data. + self.save_m2m = self._save_m2m + + user = self.instance + # if editor maps groups user add viewer maps groups group to the user saved if commit: + + user.save() + if hasattr(self, "save_m2m"): + self.save_m2m() + if self.cleaned_data['password1']: user.set_password(self.cleaned_data['password1']) - user.save() + user.save() # for save groups if 'groups' not in self.cleaned_data: @@ -722,16 +742,12 @@ class G3WUserUpdateForm(G3WUserForm): widget=forms.PasswordInput, required=False, help_text=_("Enter the same password as above, for verification.")) - password = ReadOnlyPasswordHashField() - def clean_username(self): """Reject usernames that differ only in case.""" username = self.cleaned_data.get("username") return username - def clean_password(self): - return self.initial['password'] def clean_password2(self): password1 = self.cleaned_data.get("password1") diff --git a/g3w-admin/usersmanage/tests/test_forms.py b/g3w-admin/usersmanage/tests/test_forms.py index 1add11457..406fafb03 100644 --- a/g3w-admin/usersmanage/tests/test_forms.py +++ b/g3w-admin/usersmanage/tests/test_forms.py @@ -51,6 +51,7 @@ def test_user_form_crud(self): 'username': 'admin01_test', 'password1': self.users_password, 'password2': self.users_password, + 'is_active': True, 'is_superuser': True, 'is_staff': True, 'groups': [self.main_roles[G3W_EDITOR1].pk], # required also fro admin1 and admin2 @@ -65,6 +66,13 @@ def test_user_form_crud(self): u = User.objects.get(username='admin01_test') self.assertTrue(u.is_superuser) self.assertTrue(u.is_staff) + self.assertTrue(u.is_active) + + # Check password is not changed making a login + self.client.logout() + login = self.client.login(username='admin01_test', password=self.users_password) + self.assertTrue(login) + self.client.logout() u.delete() del(u) @@ -74,6 +82,7 @@ def test_user_form_crud(self): 'username': 'admin01_test', 'password1': self.users_password, 'password2': self.users_password, + 'is_active': True, 'is_superuser': True, 'is_staff': True, 'groups': [self.main_roles[G3W_EDITOR1].pk], # required also for admin1 and admin2 @@ -109,6 +118,11 @@ def test_user_form_crud(self): self.assertTrue(u.is_superuser) self.assertTrue(u.is_staff) + # Check password is not changed making a login + login = self.client.login(username='admin01_test_updated', password=self.users_password) + self.assertTrue(login) + self.client.logout() + u.delete() del(u) @@ -138,6 +152,7 @@ def test_user_form_crud(self): 'username': 'admin02_test', 'password1': self.users_password, 'password2': self.users_password, + 'is_active': True, 'is_superuser': True, 'is_staff': True, 'groups': [self.main_roles[G3W_EDITOR1].pk], @@ -171,6 +186,7 @@ def test_user_form_crud(self): 'username': 'editor1_test', 'password1': self.users_password, 'password2': self.users_password, + 'is_active': True, 'is_superuser': False, 'is_staff': True, 'groups': [self.main_roles[G3W_EDITOR1].pk, self.main_roles[G3W_VIEWER1].pk], # required also for admin1 and admin2 @@ -208,6 +224,7 @@ def test_user_form_crud(self): 'username': 'editor2_test', 'password1': self.users_password, 'password2': self.users_password, + 'is_active': True, 'is_superuser': True, 'is_staff': True, 'groups': [self.main_roles[G3W_EDITOR2].pk], @@ -253,6 +270,7 @@ def test_user_form_constraints(self): 'password1': self.users_password, 'password2': self.users_password, 'username': 'editor1_test_constraint', + 'is_active': True, 'is_superuser': False, 'is_staff': False, 'groups': [], @@ -272,6 +290,7 @@ def test_user_form_constraints(self): 'password1': self.users_password, 'password2': self.users_password, 'username': 'editor1_test_constraint', + 'is_active': 'on', 'is_superuser': 'on', 'is_staff': 'off', 'groups': [], @@ -286,6 +305,7 @@ def test_user_form_constraints(self): 'password1': self.users_password, 'password2': self.users_password, 'username': 'editor1_test_constraint', + 'is_active': 'on', 'is_superuser': 'off', 'is_staff': 'on', 'groups': [], @@ -300,6 +320,7 @@ def test_user_form_constraints(self): 'password1': self.users_password, 'password2': self.users_password, 'username': 'editor1_test_constraint', + 'is_active': 'on', 'is_superuser': 'on', 'is_staff': 'on', 'groups': [], @@ -319,6 +340,7 @@ def test_user_form_constraints(self): 'password1': self.users_password, 'password2': self.users_password, 'username': 'editor1_test_constraint', + 'is_active': True, 'is_superuser': False, 'is_staff': False, 'groups': [self.main_roles[G3W_EDITOR1].pk, self.main_roles[G3W_EDITOR2].pk],