diff options
Diffstat (limited to 'synapse')
| -rw-r--r-- | synapse/api/auth/base.py | 48 | ||||
| -rw-r--r-- | synapse/api/auth/internal.py | 39 | ||||
| -rw-r--r-- | synapse/api/auth/msc3861_delegated.py | 4 | ||||
| -rw-r--r-- | synapse/storage/databases/main/client_ips.py | 21 |
4 files changed, 74 insertions, 38 deletions
diff --git a/synapse/api/auth/base.py b/synapse/api/auth/base.py index 9321d6f186..e2e3dc61b4 100644 --- a/synapse/api/auth/base.py +++ b/synapse/api/auth/base.py @@ -27,6 +27,8 @@ from synapse.api.errors import ( UnstableSpecAuthError, ) from synapse.appservice import ApplicationService +from synapse.http import get_request_user_agent +from synapse.http.site import SynapseRequest from synapse.logging.opentracing import trace from synapse.types import Requester, create_requester from synapse.util.cancellation import cancellable @@ -45,6 +47,9 @@ class BaseAuth: self.store = hs.get_datastores().main self._storage_controllers = hs.get_storage_controllers() + self._track_appservice_user_ips = hs.config.appservice.track_appservice_user_ips + self._track_puppeted_user_ips = hs.config.api.track_puppeted_user_ips + async def check_user_in_room( self, room_id: str, @@ -349,3 +354,46 @@ class BaseAuth: return create_requester( effective_user_id, app_service=app_service, device_id=effective_device_id ) + + async def _record_request( + self, request: SynapseRequest, requester: Requester + ) -> None: + """Record that this request was made. + + This updates the client_ips and monthly_active_user tables. + """ + ip_addr = request.get_client_ip_if_available() + + if ip_addr and (not requester.app_service or self._track_appservice_user_ips): + user_agent = get_request_user_agent(request) + access_token = self.get_access_token_from_request(request) + + # XXX(quenting): I'm 95% confident that we could skip setting the + # device_id to "dummy-device" for appservices, and that the only impact + # would be some rows which whould not deduplicate in the 'user_ips' + # table during the transition + recorded_device_id = ( + "dummy-device" + if requester.device_id is None and requester.app_service is not None + else requester.device_id + ) + await self.store.insert_client_ip( + user_id=requester.authenticated_entity, + access_token=access_token, + ip=ip_addr, + user_agent=user_agent, + device_id=recorded_device_id, + ) + + # Track also the puppeted user client IP if enabled and the user is puppeting + if ( + requester.user.to_string() != requester.authenticated_entity + and self._track_puppeted_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, + ) diff --git a/synapse/api/auth/internal.py b/synapse/api/auth/internal.py index 36ee9c8b8f..985cbb1278 100644 --- a/synapse/api/auth/internal.py +++ b/synapse/api/auth/internal.py @@ -22,7 +22,6 @@ from synapse.api.errors import ( InvalidClientTokenError, MissingClientTokenError, ) -from synapse.http import get_request_user_agent from synapse.http.site import SynapseRequest from synapse.logging.opentracing import active_span, force_tracing, start_active_span from synapse.types import Requester, create_requester @@ -48,8 +47,6 @@ class InternalAuth(BaseAuth): self._account_validity_handler = hs.get_account_validity_handler() self._macaroon_generator = hs.get_macaroon_generator() - 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._force_tracing_for_users = hs.config.tracing.force_tracing_for_users @cancellable @@ -115,9 +112,6 @@ class InternalAuth(BaseAuth): Once get_user_by_req has set up the opentracing span, this does the actual work. """ try: - ip_addr = request.get_client_ip_if_available() - user_agent = get_request_user_agent(request) - access_token = self.get_access_token_from_request(request) # First check if it could be a request from an appservice @@ -154,38 +148,7 @@ class InternalAuth(BaseAuth): errcode=Codes.EXPIRED_ACCOUNT, ) - if ip_addr and ( - not requester.app_service or self._track_appservice_user_ips - ): - # XXX(quenting): I'm 95% confident that we could skip setting the - # device_id to "dummy-device" for appservices, and that the only impact - # would be some rows which whould not deduplicate in the 'user_ips' - # table during the transition - recorded_device_id = ( - "dummy-device" - if requester.device_id is None and requester.app_service is not None - else requester.device_id - ) - await self.store.insert_client_ip( - user_id=requester.authenticated_entity, - access_token=access_token, - ip=ip_addr, - user_agent=user_agent, - device_id=recorded_device_id, - ) - - # Track also the puppeted user client IP if enabled and the user is puppeting - if ( - requester.user.to_string() != requester.authenticated_entity - and self._track_puppeted_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, - ) + await self._record_request(request, requester) if requester.is_guest and not allow_guest: raise AuthError( diff --git a/synapse/api/auth/msc3861_delegated.py b/synapse/api/auth/msc3861_delegated.py index 31bb035cc8..7373d81534 100644 --- a/synapse/api/auth/msc3861_delegated.py +++ b/synapse/api/auth/msc3861_delegated.py @@ -227,6 +227,10 @@ class MSC3861DelegatedAuth(BaseAuth): # so that we don't provision the user if they don't have enough permission: requester = await self.get_user_by_access_token(access_token, allow_expired) + # Do not record requests from MAS using the virtual `__oidc_admin` user. + if access_token != self._admin_token: + await self._record_request(request, requester) + if not allow_guest and requester.is_guest: raise OAuthInsufficientScopeError([SCOPE_MATRIX_API]) diff --git a/synapse/storage/databases/main/client_ips.py b/synapse/storage/databases/main/client_ips.py index c006129625..d4b14aaebe 100644 --- a/synapse/storage/databases/main/client_ips.py +++ b/synapse/storage/databases/main/client_ips.py @@ -589,6 +589,27 @@ class ClientIpWorkerStore(ClientIpBackgroundUpdateStore, MonthlyActiveUsersWorke device_id: Optional[str], now: Optional[int] = None, ) -> None: + """Record that `user_id` used `access_token` from this `ip` address. + + This method does two things. + + 1. It queues up a row to be upserted into the `client_ips` table. These happen + periodically; see _update_client_ips_batch. + 2. It immediately records this user as having taken action for the purposes of + MAU tracking. + + Any DB writes take place on the background tasks worker, falling back to the + main process. If we're not that worker, this method emits a replication payload + to run this logic on that worker. + + Two caveats to note: + + - We only take action once per LAST_SEEN_GRANULARITY, to avoid spamming the + DB with writes. + - Requests using the sliding-sync proxy's user agent are excluded, as its + requests are not directly driven by end-users. This is a hack and we're not + very proud of it. + """ # The sync proxy continuously triggers /sync even if the user is not # present so should be excluded from user_ips entries. if user_agent == "sync-v3-proxy-": |