Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Allow tracking puppeted users for MAU #11561

Merged
merged 6 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/11561.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add `track_puppeted_user_ips` config flag to track puppeted user IP addresses. This also includes them in monthly active user counts.
5 changes: 5 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,11 @@ room_prejoin_state:
#additional_event_types:
# - org.example.custom.event.type

# If enabled, puppeted user IP's can also be tracked. By default when
# puppeting another user, the user who has created the access token
# for puppeting is tracked. If this is enabled, both requests are tracked.
#track_puppeted_user_ips: false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • this is missing a # line
  • I'm struggling to follow the description. What does the first sentence mean? What does "If this is enabled" mean (just uncommenting the example setting, ie setting track_puppeted_user_ips: false?)
  • convention is that the example should be the opposite to the default, which doesn't seem to be the case here.

More details at https://matrix-org.github.io/synapse/develop/code_style.html#configuration-file-format



# A list of application service config files to use
#
Expand Down
13 changes: 13 additions & 0 deletions synapse/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def __init__(self, hs: "HomeServer"):
self._auth_blocking = AuthBlocking(self.hs)

self._track_appservice_user_ips = hs.config.appservice.track_appservice_user_ips
self._track_puppeted_user_ips = hs.config.api.track_puppeted_user_ips
self._macaroon_secret_key = hs.config.key.macaroon_secret_key
self._force_tracing_for_users = hs.config.tracing.force_tracing_for_users

Expand Down Expand Up @@ -246,6 +247,18 @@ async def _wrapped_get_user_by_req(
user_agent=user_agent,
device_id=device_id,
)
# Track also the puppeted user client IP if enabled and the user is puppeting
if (
user_info.user_id != user_info.token_owner
and self._track_puppeted_user_ips
):
await self.store.insert_client_ip(
user_id=user_info.user_id,
access_token=access_token,
ip=ip_addr,
user_agent=user_agent,
device_id=device_id,
)
squahtx marked this conversation as resolved.
Show resolved Hide resolved

if is_guest and not allow_guest:
raise AuthError(
Expand Down
9 changes: 9 additions & 0 deletions synapse/config/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class ApiConfig(Config):
def read_config(self, config: JsonDict, **kwargs):
validate_config(_MAIN_SCHEMA, config, ())
self.room_prejoin_state = list(self._get_prejoin_state_types(config))
self.track_puppeted_user_ips = config.get("track_puppeted_user_ips", False)

def generate_config_section(cls, **kwargs) -> str:
formatted_default_state_types = "\n".join(
Expand Down Expand Up @@ -59,6 +60,11 @@ def generate_config_section(cls, **kwargs) -> str:
#
#additional_event_types:
# - org.example.custom.event.type

# If enabled, puppeted user IP's can also be tracked. By default when
# puppeting another user, the user who has created the access token
# for puppeting is tracked. If this is enabled, both requests are tracked.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

track_appservice_user_ips has a note about mau tracking ("Implicitly enables MAU tracking for application service users."). Shall we add a similar note here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I added that /o\ I was going to at least, I swear 😅

Added now!

#track_puppeted_user_ips: false
""" % {
"formatted_default_state_types": formatted_default_state_types
}
Expand Down Expand Up @@ -136,5 +142,8 @@ def _get_prejoin_state_types(self, config: JsonDict) -> Iterable[str]:
"properties": {
"room_prejoin_state": _ROOM_PREJOIN_STATE_CONFIG_SCHEMA,
"room_invite_state_types": _ROOM_INVITE_STATE_TYPES_SCHEMA,
"track_puppeted_user_ips": {
"type": "boolean",
},
},
}
33 changes: 33 additions & 0 deletions tests/api/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,39 @@ def test_get_user_by_req_appservice_valid_token_invalid_device_id(self):
self.assertEquals(failure.value.code, 400)
self.assertEquals(failure.value.errcode, Codes.EXCLUSIVE)

def test_get_user_by_req__puppeted_token__not_tracking_puppeted_mau(self):
self.store.get_user_by_access_token = simple_async_mock(
TokenLookupResult(
user_id="@baldrick:matrix.org",
device_id="device",
token_owner="@admin:matrix.org",
)
)
self.store.insert_client_ip = simple_async_mock(None)
request = Mock(args={})
request.getClientIP.return_value = "127.0.0.1"
request.args[b"access_token"] = [self.test_token]
request.requestHeaders.getRawHeaders = mock_getRawHeaders()
self.get_success(self.auth.get_user_by_req(request))
self.store.insert_client_ip.assert_called_once()

def test_get_user_by_req__puppeted_token__tracking_puppeted_mau(self):
self.auth._track_puppeted_user_ips = True
self.store.get_user_by_access_token = simple_async_mock(
TokenLookupResult(
user_id="@baldrick:matrix.org",
device_id="device",
token_owner="@admin:matrix.org",
)
)
self.store.insert_client_ip = simple_async_mock(None)
request = Mock(args={})
request.getClientIP.return_value = "127.0.0.1"
request.args[b"access_token"] = [self.test_token]
request.requestHeaders.getRawHeaders = mock_getRawHeaders()
self.get_success(self.auth.get_user_by_req(request))
self.assertEquals(self.store.insert_client_ip.call_count, 2)

def test_get_user_from_macaroon(self):
self.store.get_user_by_access_token = simple_async_mock(
TokenLookupResult(user_id="@baldrick:matrix.org", device_id="device")
Expand Down