Neilj/fix 4229

changelog.d/4141.feature

@@ -0,0 +1 @@
+Special case a support user for use in verifying user behaviour of a given server, the support user does not appear in user directory or monthly active user counts.

changelog.d/4233.bugfix

@@ -0,0 +1 @@
+Fix synapse_admin_mau:current metric is not updating when config.mau_stats_only is True

synapse/api/auth.py

@@ -791,9 +791,10 @@ def check_auth_blocking(self, user_id=None, threepid=None):
             threepid should never be set at the same time.
         """
 
-        # Never fail an auth check for the server notices users
+        # Never fail an auth check for the server notices users or support user
         # This can be a problem where event creation is prohibited due to blocking
-        if user_id == self.hs.config.server_notices_mxid:
+        is_support = yield self.store.is_support_user(user_id)
+        if user_id == self.hs.config.server_notices_mxid or is_support:
             return
 
         if self.hs.config.hs_disabled:

synapse/api/constants.py

@@ -119,3 +119,9 @@ class RoomVersions(object):
 
 ServerNoticeMsgType = "m.server_notice"
 ServerNoticeLimitReached = "m.server_notice.usage_limit_reached"
+
+
+# Allows for user type specific behaviour, if we'd had a crystal ball would
+# probably have included admin and guest, normal users are type None
+class UserTypes(object):
+    SUPPORT = "support"

synapse/app/homeserver.py

@@ -549,7 +549,7 @@ def start_generate_monthly_active_users():
         )
 
     start_generate_monthly_active_users()
-    if hs.config.limit_usage_by_mau:
+    if hs.config.limit_usage_by_mau or hs.config.mau_stats_only:
         clock.looping_call(start_generate_monthly_active_users, 5 * 60 * 1000)
     # End of monthly active user settings
 

synapse/handlers/register.py

@@ -233,9 +233,16 @@ def _auto_join_rooms(self, user_id):
         # auto-join the user to any rooms we're supposed to dump them into
         fake_requester = create_requester(user_id)
 
-        # try to create the room if we're the first user on the server
+        # try to create the room if we're the first real user on the server. Note
+        # that an auto generated support user is not a real user and never be
+        # the user to create the room
         should_auto_create_rooms = False
-        if self.hs.config.autocreate_auto_join_rooms:
+        is_support = yield self.store.is_support_user(user_id)
+        # There is an edge case where the first user is the support user, then
+        # the room is never created, though this seems unlikely and
+        # recoverable from given the support user being involved in the first
+        # place.
+        if (self.hs.config.autocreate_auto_join_rooms and not is_support):
             count = yield self.store.count_all_users()
             should_auto_create_rooms = count == 1
         for r in self.hs.config.auto_join_rooms:

synapse/handlers/user_directory.py

@@ -125,18 +125,22 @@ def handle_local_profile_change(self, user_id, profile):
         """
         # FIXME(#3714): We should probably do this in the same worker as all
         # the other changes.
-        yield self.store.update_profile_in_user_dir(
-            user_id, profile.display_name, profile.avatar_url, None,
-        )
+        is_support = yield self.store.is_support_user(user_id)
+        if not is_support:
+            yield self.store.update_profile_in_user_dir(
+                user_id, profile.display_name, profile.avatar_url, None,
+            )
 
     @defer.inlineCallbacks
     def handle_user_deactivated(self, user_id):
         """Called when a user ID is deactivated
         """
         # FIXME(#3714): We should probably do this in the same worker as all
         # the other changes.
-        yield self.store.remove_from_user_dir(user_id)
-        yield self.store.remove_from_user_in_public_room(user_id)
+        is_support = yield self.store.is_support_user(user_id)
+        if not is_support:
+            yield self.store.remove_from_user_dir(user_id)
+            yield self.store.remove_from_user_in_public_room(user_id)
 
     @defer.inlineCallbacks
     def _unsafe_process(self):
@@ -329,13 +333,6 @@ def _handle_deltas(self, deltas):
                     public_value=Membership.JOIN,
                 )
 
-                if change is None:
-                    # Handle any profile changes
-                    yield self._handle_profile_change(
-                        state_key, room_id, prev_event_id, event_id,
-                    )
-                    continue
-
                 if not change:
                     # Need to check if the server left the room entirely, if so
                     # we might need to remove all the users in that room
@@ -349,21 +346,32 @@ def _handle_deltas(self, deltas):
                         # need to remove those users or not
                         user_ids = yield self.store.get_users_in_dir_due_to_room(room_id)
                         for user_id in user_ids:
-                            yield self._handle_remove_user(room_id, user_id)
+                            is_support = yield self.store.is_support_user(state_key)
+                            if not is_support:
+                                yield self._handle_remove_user(room_id, user_id)
                         return
                     else:
                         logger.debug("Server is still in room: %r", room_id)
 
-                if change:  # The user joined
-                    event = yield self.store.get_event(event_id, allow_none=True)
-                    profile = ProfileInfo(
-                        avatar_url=event.content.get("avatar_url"),
-                        display_name=event.content.get("displayname"),
-                    )
+                is_support = yield self.store.is_support_user(state_key)
+                if not is_support:
+                    if change is None:
+                        # Handle any profile changes
+                        yield self._handle_profile_change(
+                            state_key, room_id, prev_event_id, event_id,
+                        )
+                        continue
+
+                    if change:  # The user joined
+                        event = yield self.store.get_event(event_id, allow_none=True)
+                        profile = ProfileInfo(
+                            avatar_url=event.content.get("avatar_url"),
+                            display_name=event.content.get("displayname"),
+                        )
 
-                    yield self._handle_new_user(room_id, state_key, profile)
-                else:  # The user left
-                    yield self._handle_remove_user(room_id, state_key)
+                        yield self._handle_new_user(room_id, state_key, profile)
+                    else:  # The user left
+                        yield self._handle_remove_user(room_id, state_key)
             else:
                 logger.debug("Ignoring irrelevant type: %r", typ)
 

synapse/storage/monthly_active_users.py

@@ -182,6 +182,18 @@ def upsert_monthly_active_user(self, user_id):
         Args:
             user_id (str): user to add/update
         """
+        # Support user never to be included in MAU stats. Note I can't easily call this
+        # from upsert_monthly_active_user_txn because then I need a _txn form of
+        # is_support_user which is complicated because I want to cache the result.
+        # Therefore I call it here and ignore the case where
+        # upsert_monthly_active_user_txn is called directly from
+        # _initialise_reserved_users reasoning that it would be very strange to
+        #  include a support user in this context.
+
+        is_support = yield self.is_support_user(user_id)
+        if is_support:
+            return
+
         is_insert = yield self.runInteraction(
             "upsert_monthly_active_user", self.upsert_monthly_active_user_txn,
             user_id
@@ -208,6 +220,7 @@ def upsert_monthly_active_user_txn(self, txn, user_id):
             bool: True if a new entry was created, False if an
             existing one was updated.
         """
+
         # Am consciously deciding to lock the table on the basis that is ought
         # never be a big table and alternative approaches (batching multiple
         # upserts into a single txn) introduced a lot of extra complexity.

synapse/storage/prepare_database.py

@@ -25,7 +25,7 @@
 
 # Remember to update this number every time a change is made to database
 # schema files, so the users will be informed on server restarts.
-SCHEMA_VERSION = 52
+SCHEMA_VERSION = 53
 
 dir_path = os.path.abspath(os.path.dirname(__file__))
 

synapse/storage/registration.py

@@ -19,6 +19,7 @@
 
 from twisted.internet import defer
 
+from synapse.api.constants import UserTypes
 from synapse.api.errors import Codes, StoreError
 from synapse.storage import background_updates
 from synapse.storage._base import SQLBaseStore
@@ -167,7 +168,7 @@ def add_access_token_to_user(self, user_id, token, device_id=None):
 
     def register(self, user_id, token=None, password_hash=None,
                  was_guest=False, make_guest=False, appservice_id=None,
-                 create_profile_with_localpart=None, admin=False):
+                 create_profile_with_localpart=None, admin=False, user_type=None):
         """Attempts to register an account.
 
         Args:
@@ -183,6 +184,8 @@ def register(self, user_id, token=None, password_hash=None,
             appservice_id (str): The ID of the appservice registering the user.
             create_profile_with_localpart (str): Optionally create a profile for
                 the given localpart.
+            admin (boolean): is an admin user?
+            user_type (synapse.api.constants import UserTypes): type of user
         Raises:
             StoreError if the user_id could not be registered.
         """
@@ -196,7 +199,8 @@ def register(self, user_id, token=None, password_hash=None,
             make_guest,
             appservice_id,
             create_profile_with_localpart,
-            admin
+            admin,
+            user_type
         )
 
     def _register(
@@ -210,6 +214,7 @@ def _register(
         appservice_id,
         create_profile_with_localpart,
         admin,
+        user_type,
     ):
         now = int(self.clock.time())
 
@@ -244,6 +249,7 @@ def _register(
                         "is_guest": 1 if make_guest else 0,
                         "appservice_id": appservice_id,
                         "admin": 1 if admin else 0,
+                        "user_type": user_type
                     }
                 )
             else:
@@ -257,6 +263,7 @@ def _register(
                         "is_guest": 1 if make_guest else 0,
                         "appservice_id": appservice_id,
                         "admin": 1 if admin else 0,
+                        "user_type": user_type,
                     }
                 )
         except self.database_engine.module.IntegrityError:
@@ -450,6 +457,18 @@ def is_guest(self, user_id):
 
         defer.returnValue(res if res else False)
 
+    @cachedInlineCallbacks()
+    def is_support_user(self, user_id):
+        res = yield self._simple_select_one_onecol(
+            table="users",
+            keyvalues={"name": user_id},
+            retcol="user_type",
+            allow_none=True,
+            desc="is_support_user",
+        )
+
+        defer.returnValue(True if res == UserTypes.SUPPORT else False)
+
     @defer.inlineCallbacks
     def user_add_threepid(self, user_id, medium, address, validated_at, added_at):
         yield self._simple_upsert("user_threepids", {

synapse/storage/schema/delta/53/add_user_type_to_users.sql

@@ -0,0 +1,20 @@
+/* Copyright 2018 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* record whether we have sent a server notice about consenting to the
+ * privacy policy. Specifically records the version of the policy we sent
+ * a message about.
+ */
+ALTER TABLE users ADD COLUMN user_type TEXT DEFAULT NULL;

tests/api/test_auth.py

@@ -50,6 +50,8 @@ def setUp(self):
         # this is overridden for the appservice tests
         self.store.get_app_service_by_token = Mock(return_value=None)
 
+        self.store.is_support_user = Mock(return_value=defer.succeed(False))
+
     @defer.inlineCallbacks
     def test_get_user_by_req_user_valid_token(self):
         user_info = {"name": self.test_user, "token_id": "ditto", "device_id": "device"}

tests/handlers/test_register.py

@@ -17,7 +17,7 @@
 
 from twisted.internet import defer
 
-from synapse.api.errors import ResourceLimitError
+from synapse.api.errors import ResourceLimitError, SynapseError
 from synapse.handlers.register import RegistrationHandler
 from synapse.types import RoomAlias, UserID, create_requester
 
@@ -43,10 +43,6 @@ def setUp(self):
             self.addCleanup,
             expire_access_token=True,
         )
-        self.macaroon_generator = Mock(
-            generate_access_token=Mock(return_value='secret')
-        )
-        self.hs.get_macaroon_generator = Mock(return_value=self.macaroon_generator)
         self.handler = self.hs.get_handlers().registration_handler
         self.store = self.hs.get_datastore()
         self.hs.config.max_mau_value = 50
@@ -64,7 +60,7 @@ def test_user_is_created_and_logged_in_if_doesnt_exist(self):
             requester, frank.localpart, "Frankie"
         )
         self.assertEquals(result_user_id, user_id)
-        self.assertEquals(result_token, 'secret')
+        self.assertTrue(result_token is not None)
 
     @defer.inlineCallbacks
     def test_if_user_exists(self):
@@ -82,7 +78,7 @@ def test_if_user_exists(self):
             requester, local_part, None
         )
         self.assertEquals(result_user_id, user_id)
-        self.assertEquals(result_token, 'secret')
+        self.assertTrue(result_token is not None)
 
     @defer.inlineCallbacks
     def test_mau_limits_when_disabled(self):
@@ -184,6 +180,20 @@ def test_auto_create_auto_join_where_auto_create_is_false(self):
         rooms = yield self.store.get_rooms_for_user(res[0])
         self.assertEqual(len(rooms), 0)
 
+    @defer.inlineCallbacks
+    def test_auto_create_auto_join_rooms_when_support_user_exists(self):
+        room_alias_str = "#room:test"
+        self.hs.config.auto_join_rooms = [room_alias_str]
+
+        self.store.is_support_user = Mock(return_value=True)
+        res = yield self.handler.register(localpart='support')
+        rooms = yield self.store.get_rooms_for_user(res[0])
+        self.assertEqual(len(rooms), 0)
+        directory_handler = self.hs.get_handlers().directory_handler
+        room_alias = RoomAlias.from_string(room_alias_str)
+        with self.assertRaises(SynapseError):
+            yield directory_handler.get_association(room_alias)
+
     @defer.inlineCallbacks
     def test_auto_create_auto_join_where_no_consent(self):
         self.hs.config.user_consent_at_registration = True