Refactor get_user_by_req to keep the same logic for appservice user…

…s and regular ones

Signed-off-by: Quentin Gliech <quenting@element.io>

synapse/api/auth.py

@@ -175,45 +175,38 @@ async def _wrapped_get_user_by_req(
 
             # First check if it could be a request from an appservice
             requester = await self._get_appservice_user(request)
-            if requester:
-                if ip_addr and self._track_appservice_user_ips:
-                    await self.store.insert_client_ip(
-                        user_id=requester.user.to_string(),
-                        access_token=access_token,
-                        ip=ip_addr,
-                        user_agent=user_agent,
-                        device_id=requester.device_id,
-                    )
-
-                request.requester = requester
-                return requester
-
-            requester = await self.get_user_by_access_token(
-                access_token, allow_expired=allow_expired, mark_as_used=True
-            )
-
-            # Deny the request if the user account has expired.
-            if not allow_expired:
-                if await self._account_validity_handler.is_user_expired(
-                    requester.user.to_string()
-                ):
-                    # Raise the error if either an account validity module has determined
-                    # the account has expired, or the legacy account validity
-                    # implementation is enabled and determined the account has expired
-                    raise AuthError(
-                        403,
-                        "User account has expired",
-                        errcode=Codes.EXPIRED_ACCOUNT,
-                    )
+            if not requester:
+                # If not, it should be from a regular user
+                requester = await self.get_user_by_access_token(
+                    access_token, allow_expired=allow_expired, mark_as_used=True
+                )
 
-            if ip_addr:
+                # Deny the request if the user account has expired.
+                # This check is only done for regular users, not appservice ones.
+                if not allow_expired:
+                    if await self._account_validity_handler.is_user_expired(
+                        requester.user.to_string()
+                    ):
+                        # Raise the error if either an account validity module has determined
+                        # the account has expired, or the legacy account validity
+                        # implementation is enabled and determined the account has expired
+                        raise AuthError(
+                            403,
+                            "User account has expired",
+                            errcode=Codes.EXPIRED_ACCOUNT,
+                        )
+
+            if ip_addr and (
+                not requester.app_service or self._track_appservice_user_ips
+            ):
                 await self.store.insert_client_ip(
                     user_id=requester.authenticated_entity,
                     access_token=access_token,
                     ip=ip_addr,
                     user_agent=user_agent,
                     device_id=requester.device_id,
                 )
+
                 # Track also the puppeted user client IP if enabled and the user is puppeting
                 if (
                     requester.user.to_string() != requester.authenticated_entity
@@ -428,6 +421,7 @@ async def get_user_by_access_token(
                 is_guest=True,
                 # all guests get the same device id
                 device_id=GUEST_DEVICE_ID,
+                authenticated_entity=user_id,
             )
         except (
             pymacaroons.exceptions.MacaroonException,