Add concept of authenticated_entity vs target_user

pull/8616/head
Erik Johnston 2020-10-21 14:07:59 +01:00
parent bc422e1203
commit 17a3d942c3
5 changed files with 62 additions and 8 deletions

View File

@ -195,6 +195,7 @@ class Auth:
if user_id:
request.authenticated_entity = user_id
opentracing.set_tag("authenticated_entity", user_id)
opentracing.set_tag("target_user", user_id)
opentracing.set_tag("appservice_id", app_service.id)
if ip_addr and self._track_appservice_user_ips:
@ -218,8 +219,9 @@ class Auth:
# Deny the request if the user account has expired.
if self._account_validity.enabled and not allow_expired:
user_id = user.to_string()
if await self.store.is_account_expired(user_id, self.clock.time_msec()):
if await self.store.is_account_expired(
user_info.user_id, self.clock.time_msec()
):
raise AuthError(
403, "User account has expired", errcode=Codes.EXPIRED_ACCOUNT
)
@ -228,7 +230,7 @@ class Auth:
if access_token and ip_addr:
await self.store.insert_client_ip(
user_id=user.to_string(),
user_id=user_info.token_owner,
access_token=access_token,
ip=ip_addr,
user_agent=user_agent,
@ -242,8 +244,10 @@ class Auth:
errcode=Codes.GUEST_ACCESS_FORBIDDEN,
)
request.authenticated_entity = user.to_string()
opentracing.set_tag("authenticated_entity", user.to_string())
request.authenticated_entity = user_info.token_owner
request.target_user = user_info.user_id
opentracing.set_tag("authenticated_entity", user_info.token_owner)
opentracing.set_tag("target_user", user_info.user_id)
if device_id:
opentracing.set_tag("device_id", device_id)
@ -254,6 +258,7 @@ class Auth:
shadow_banned,
device_id,
app_service=app_service,
authenticated_entity=user_info.token_owner,
)
except KeyError:
raise MissingClientTokenError()

View File

@ -55,6 +55,7 @@ class SynapseRequest(Request):
self.site = channel.site
self._channel = channel # this is used by the tests
self.authenticated_entity = None
self.target_user = None
self.start_time = 0.0
# we can't yet create the logcontext, as we don't know the method.
@ -269,6 +270,11 @@ class SynapseRequest(Request):
if authenticated_entity is not None and isinstance(authenticated_entity, bytes):
authenticated_entity = authenticated_entity.decode("utf-8", "replace")
if self.target_user:
authenticated_entity = "{} as {}".format(
authenticated_entity, self.target_user,
)
# ...or could be raw utf-8 bytes in the User-Agent header.
# N.B. if you don't do this, the logger explodes cryptically
# with maximum recursion trying to log errors about

View File

@ -51,6 +51,8 @@ class TokenLookupResult:
token_id: The ID of the access token looked up
device_id: The device associated with the token, if any.
valid_until_ms: The timestamp the token expires, if any.
token_owner: The "owner" of the token. This is either the same as the
user, or a server admin who is logged in as the user.
"""
user_id = attr.ib(type=str)
@ -59,6 +61,12 @@ class TokenLookupResult:
token_id = attr.ib(type=Optional[int], default=None)
device_id = attr.ib(type=Optional[str], default=None)
valid_until_ms = attr.ib(type=Optional[int], default=None)
token_owner = attr.ib(type=str)
# Make the token owner default to the user ID, which is the common case.
@token_owner.default
def _default_token_owner(self):
return self.user_id
class RegistrationWorkerStore(CacheInvalidationWorkerStore):
@ -361,9 +369,10 @@ class RegistrationWorkerStore(CacheInvalidationWorkerStore):
users.shadow_banned,
access_tokens.id as token_id,
access_tokens.device_id,
access_tokens.valid_until_ms
access_tokens.valid_until_ms,
access_tokens.user_id as token_owner
FROM users
INNER JOIN access_tokens on users.name = access_tokens.user_id
INNER JOIN access_tokens on users.name = COALESCE(puppets_user_id, access_tokens.user_id)
WHERE token = ?
"""

View File

@ -0,0 +1,17 @@
/* Copyright 2020 The Matrix.org Foundation C.I.C
*
* 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.
*/
-- Whether the access token is an admin token for controlling another user.
ALTER TABLE access_tokens ADD COLUMN puppets_user_id TEXT;

View File

@ -74,6 +74,7 @@ class Requester(
"shadow_banned",
"device_id",
"app_service",
"authenticated_entity",
],
)
):
@ -104,6 +105,7 @@ class Requester(
"shadow_banned": self.shadow_banned,
"device_id": self.device_id,
"app_server_id": self.app_service.id if self.app_service else None,
"authenticated_entity": self.authenticated_entity,
}
@staticmethod
@ -129,6 +131,7 @@ class Requester(
shadow_banned=input["shadow_banned"],
device_id=input["device_id"],
app_service=appservice,
authenticated_entity=input["authenticated_entity"],
)
@ -139,6 +142,7 @@ def create_requester(
shadow_banned=False,
device_id=None,
app_service=None,
authenticated_entity=None,
):
"""
Create a new ``Requester`` object
@ -151,14 +155,27 @@ def create_requester(
shadow_banned (bool): True if the user making this request is shadow-banned.
device_id (str|None): device_id which was set at authentication time
app_service (ApplicationService|None): the AS requesting on behalf of the user
authenticated_entity: The entity that authenticatd when making the request,
this is different than the user_id when an admin user or the server is
"puppeting" the user.
Returns:
Requester
"""
if not isinstance(user_id, UserID):
user_id = UserID.from_string(user_id)
if authenticated_entity is None:
authenticated_entity = user_id.to_string()
return Requester(
user_id, access_token_id, is_guest, shadow_banned, device_id, app_service
user_id,
access_token_id,
is_guest,
shadow_banned,
device_id,
app_service,
authenticated_entity,
)