models: Add is_realm_admin and is_guest setters.

Fixes #13452.

The migration from UserProfile.is_realm_admin/UserProfile.is_guest in
e10361a832 broke our LDAP-based support
for setting a user's role via LDAP properties, which relied on setting
those fields.  Because the django-auth-ldap feature powering that only
supports booleans (and in any case, we don't want to expose constants
like `ROLE_REALM_ADMINISTRATOR` to the LDAP configuration interface),
it makes sense to provide setters for these legacy fields for
backwards-compatibility.

We lint against using these setters directly in Zulip's codebase
directly.  The issue with using these is that when changing user's
.role we want to create appropriate RealmAuditLog entries and send
events. This isn't possible when using these setters - the log entries
and events should be created if the role change in the UserProfile is
actually save()-ed to the database - and on the level of the setter
function, it's not known whether the change will indeed be saved.

It would have to be somehow figured out on the level of post_save
signal handlers, but it doesn't seem like a good design to have such
complexity there, for the sake of setters that generally shouldn't be
used anyway - because we prefer the do_change_is_* functions.

The purpose of this change is narrowly to handle use cases like the
setattr on these boolean properties.

tools/linter_lib/custom_check.py

@@ -500,6 +500,20 @@ python_rules = RuleList(
          'include_only': set(["/management/commands/"]),
          'description': 'Raise CommandError to exit with failure in management commands',
          },
+        {'pattern': '.is_realm_admin =',
+         'description': 'Use do_change_is_admin function rather than setting UserProfile\'s is_realm_admin attribute directly.',
+         'exclude': set([
+             'zerver/migrations/0248_userprofile_role_start.py',
+             'zerver/tests/test_users.py',
+         ]),
+         },
+        {'pattern': '.is_guest =',
+         'description': 'Use do_change_is_guest function rather than setting UserProfile\'s is_guest attribute directly.',
+         'exclude': set([
+             'zerver/migrations/0248_userprofile_role_start.py',
+             'zerver/tests/test_users.py',
+         ]),
+         },
         *whitespace_rules,
         *comma_whitespace_rule,
     ],

zerver/models.py

@@ -1055,10 +1055,28 @@ class UserProfile(AbstractBaseUser, PermissionsMixin):
     def is_realm_admin(self) -> bool:
         return self.role == UserProfile.ROLE_REALM_ADMINISTRATOR
 
+    @is_realm_admin.setter
+    def is_realm_admin(self, value: bool) -> None:
+        if value:
+            self.role = UserProfile.ROLE_REALM_ADMINISTRATOR
+        elif self.role == UserProfile.ROLE_REALM_ADMINISTRATOR:
+            # We need to be careful to not accidentally change
+            # ROLE_GUEST to ROLE_MEMBER here.
+            self.role = UserProfile.ROLE_MEMBER
+
     @property
     def is_guest(self) -> bool:
         return self.role == UserProfile.ROLE_GUEST
 
+    @is_guest.setter
+    def is_guest(self, value: bool) -> None:
+        if value:
+            self.role = UserProfile.ROLE_GUEST
+        elif self.role == UserProfile.ROLE_GUEST:
+            # We need to be careful to not accidentally change
+            # ROLE_REALM_ADMINISTRATOR to ROLE_MEMBER here.
+            self.role = UserProfile.ROLE_MEMBER
+
     @property
     def is_incoming_webhook(self) -> bool:
         return self.bot_type == UserProfile.INCOMING_WEBHOOK_BOT

zerver/tests/test_users.py

@@ -75,6 +75,33 @@ class PermissionTest(ZulipTestCase):
         with self.assertRaises(AssertionError):
             do_change_is_admin(user_profile, True, permission='totally-not-valid-perm')
 
+    def test_role_setters(self) -> None:
+        user_profile = self.example_user('hamlet')
+
+        user_profile.is_realm_admin = True
+        self.assertEqual(user_profile.is_realm_admin, True)
+        self.assertEqual(user_profile.role, UserProfile.ROLE_REALM_ADMINISTRATOR)
+
+        user_profile.is_guest = False
+        self.assertEqual(user_profile.is_guest, False)
+        self.assertEqual(user_profile.role, UserProfile.ROLE_REALM_ADMINISTRATOR)
+
+        user_profile.is_realm_admin = False
+        self.assertEqual(user_profile.is_realm_admin, False)
+        self.assertEqual(user_profile.role, UserProfile.ROLE_MEMBER)
+
+        user_profile.is_guest = True
+        self.assertEqual(user_profile.is_guest, True)
+        self.assertEqual(user_profile.role, UserProfile.ROLE_GUEST)
+
+        user_profile.is_realm_admin = False
+        self.assertEqual(user_profile.is_guest, True)
+        self.assertEqual(user_profile.role, UserProfile.ROLE_GUEST)
+
+        user_profile.is_guest = False
+        self.assertEqual(user_profile.is_guest, False)
+        self.assertEqual(user_profile.role, UserProfile.ROLE_MEMBER)
+
     def test_get_admin_users(self) -> None:
         user_profile = self.example_user('hamlet')
         do_change_is_admin(user_profile, False)