mirror of
https://github.com/element-hq/synapse
synced 2024-06-30 14:53:29 +00:00
Compare commits
6 commits
104a2db590
...
a5b7922de1
Author | SHA1 | Date | |
---|---|---|---|
|
a5b7922de1 | ||
|
3aae60f17b | ||
|
6156923114 | ||
|
0c4580d688 | ||
|
cdfed1c4fa | ||
|
944d7f6727 |
1
changelog.d/17149.misc
Normal file
1
changelog.d/17149.misc
Normal file
|
@ -0,0 +1 @@
|
||||||
|
Small performance improvement to limited incremental sync in large rooms.
|
1
changelog.d/17284.feature
Normal file
1
changelog.d/17284.feature
Normal file
|
@ -0,0 +1 @@
|
||||||
|
Do not require user-interactive authentication for uploading cross-signing keys for the first time, per MSC3967.
|
|
@ -393,9 +393,6 @@ class ExperimentalConfig(Config):
|
||||||
# MSC3391: Removing account data.
|
# MSC3391: Removing account data.
|
||||||
self.msc3391_enabled = experimental.get("msc3391_enabled", False)
|
self.msc3391_enabled = experimental.get("msc3391_enabled", False)
|
||||||
|
|
||||||
# MSC3967: Do not require UIA when first uploading cross signing keys
|
|
||||||
self.msc3967_enabled = experimental.get("msc3967_enabled", False)
|
|
||||||
|
|
||||||
# MSC3861: Matrix architecture change to delegate authentication via OIDC
|
# MSC3861: Matrix architecture change to delegate authentication via OIDC
|
||||||
try:
|
try:
|
||||||
self.msc3861 = MSC3861(**experimental.get("msc3861", {}))
|
self.msc3861 = MSC3861(**experimental.get("msc3861", {}))
|
||||||
|
|
|
@ -148,6 +148,12 @@ class TimelineBatch:
|
||||||
prev_batch: StreamToken
|
prev_batch: StreamToken
|
||||||
events: Sequence[EventBase]
|
events: Sequence[EventBase]
|
||||||
limited: bool
|
limited: bool
|
||||||
|
|
||||||
|
# All the events that were fetched from the DB while loading the room. This
|
||||||
|
# is a superset of `events`.
|
||||||
|
fetched_events: Sequence[EventBase]
|
||||||
|
fetched_limited: bool # Whether there is a gap between the previous timeline batch
|
||||||
|
|
||||||
# A mapping of event ID to the bundled aggregations for the above events.
|
# A mapping of event ID to the bundled aggregations for the above events.
|
||||||
# This is only calculated if limited is true.
|
# This is only calculated if limited is true.
|
||||||
bundled_aggregations: Optional[Dict[str, BundledAggregations]] = None
|
bundled_aggregations: Optional[Dict[str, BundledAggregations]] = None
|
||||||
|
@ -861,7 +867,11 @@ class SyncHandler:
|
||||||
)
|
)
|
||||||
|
|
||||||
return TimelineBatch(
|
return TimelineBatch(
|
||||||
events=recents, prev_batch=prev_batch_token, limited=False
|
events=recents,
|
||||||
|
prev_batch=prev_batch_token,
|
||||||
|
limited=False,
|
||||||
|
fetched_events=recents,
|
||||||
|
fetched_limited=False,
|
||||||
)
|
)
|
||||||
|
|
||||||
filtering_factor = 2
|
filtering_factor = 2
|
||||||
|
@ -878,6 +888,9 @@ class SyncHandler:
|
||||||
elif since_token and not newly_joined_room:
|
elif since_token and not newly_joined_room:
|
||||||
since_key = since_token.room_key
|
since_key = since_token.room_key
|
||||||
|
|
||||||
|
fetched_events: List[EventBase] = []
|
||||||
|
fetched_limited = True
|
||||||
|
|
||||||
while limited and len(recents) < timeline_limit and max_repeat:
|
while limited and len(recents) < timeline_limit and max_repeat:
|
||||||
# If we have a since_key then we are trying to get any events
|
# If we have a since_key then we are trying to get any events
|
||||||
# that have happened since `since_key` up to `end_key`, so we
|
# that have happened since `since_key` up to `end_key`, so we
|
||||||
|
@ -896,6 +909,10 @@ class SyncHandler:
|
||||||
room_id, limit=load_limit + 1, end_token=end_key
|
room_id, limit=load_limit + 1, end_token=end_key
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# We prepend as `fetched_events` is in ascending stream order,
|
||||||
|
# and `events` is from *before* the previously fetched events.
|
||||||
|
fetched_events = events + fetched_events
|
||||||
|
|
||||||
log_kv({"loaded_recents": len(events)})
|
log_kv({"loaded_recents": len(events)})
|
||||||
|
|
||||||
loaded_recents = (
|
loaded_recents = (
|
||||||
|
@ -947,6 +964,7 @@ class SyncHandler:
|
||||||
|
|
||||||
if len(events) <= load_limit:
|
if len(events) <= load_limit:
|
||||||
limited = False
|
limited = False
|
||||||
|
fetched_limited = False
|
||||||
break
|
break
|
||||||
max_repeat -= 1
|
max_repeat -= 1
|
||||||
|
|
||||||
|
@ -977,6 +995,8 @@ class SyncHandler:
|
||||||
# (to force client to paginate the gap).
|
# (to force client to paginate the gap).
|
||||||
limited=limited or newly_joined_room or gap_token is not None,
|
limited=limited or newly_joined_room or gap_token is not None,
|
||||||
bundled_aggregations=bundled_aggregations,
|
bundled_aggregations=bundled_aggregations,
|
||||||
|
fetched_events=fetched_events,
|
||||||
|
fetched_limited=fetched_limited,
|
||||||
)
|
)
|
||||||
|
|
||||||
async def get_state_after_event(
|
async def get_state_after_event(
|
||||||
|
@ -1514,8 +1534,12 @@ class SyncHandler:
|
||||||
#
|
#
|
||||||
# c.f. #16941 for an example of why we can't do this for all non-gappy
|
# c.f. #16941 for an example of why we can't do this for all non-gappy
|
||||||
# syncs.
|
# syncs.
|
||||||
|
#
|
||||||
|
# We can apply a similar optimization for gappy syncs if we know the room
|
||||||
|
# has been linear in the gap, so instead of just looking at the
|
||||||
|
# `timeline.batch` we can look at `timeline.fetched_events`.
|
||||||
is_linear_timeline = True
|
is_linear_timeline = True
|
||||||
if batch.events:
|
if batch.fetched_events:
|
||||||
# We need to make sure the first event in our batch points to the
|
# We need to make sure the first event in our batch points to the
|
||||||
# last event in the previous batch.
|
# last event in the previous batch.
|
||||||
last_event_id_prev_batch = (
|
last_event_id_prev_batch = (
|
||||||
|
@ -1532,8 +1556,19 @@ class SyncHandler:
|
||||||
break
|
break
|
||||||
prev_event_id = e.event_id
|
prev_event_id = e.event_id
|
||||||
|
|
||||||
if is_linear_timeline and not batch.limited:
|
if is_linear_timeline and not batch.fetched_limited:
|
||||||
state_ids: StateMap[str] = {}
|
batch_state_ids: MutableStateMap[str] = {}
|
||||||
|
|
||||||
|
# If the returned batch is actually limited, we need to add the
|
||||||
|
# state events that happened in the batch.
|
||||||
|
if batch.limited:
|
||||||
|
timeline_events = {e.event_id for e in batch.events}
|
||||||
|
batch_state_ids = {
|
||||||
|
(e.type, e.state_key): e.event_id
|
||||||
|
for e in batch.fetched_events
|
||||||
|
if e.is_state() and e.event_id not in timeline_events
|
||||||
|
}
|
||||||
|
|
||||||
if lazy_load_members:
|
if lazy_load_members:
|
||||||
if members_to_fetch and batch.events:
|
if members_to_fetch and batch.events:
|
||||||
# We're lazy-loading, so the client might need some more
|
# We're lazy-loading, so the client might need some more
|
||||||
|
@ -1542,7 +1577,7 @@ class SyncHandler:
|
||||||
# timeline here. The caller will then dedupe any redundant
|
# timeline here. The caller will then dedupe any redundant
|
||||||
# ones.
|
# ones.
|
||||||
|
|
||||||
state_ids = await self._state_storage_controller.get_state_ids_for_event(
|
ll_state_ids = await self._state_storage_controller.get_state_ids_for_event(
|
||||||
batch.events[0].event_id,
|
batch.events[0].event_id,
|
||||||
# we only want members!
|
# we only want members!
|
||||||
state_filter=StateFilter.from_types(
|
state_filter=StateFilter.from_types(
|
||||||
|
@ -1550,7 +1585,8 @@ class SyncHandler:
|
||||||
),
|
),
|
||||||
await_full_state=False,
|
await_full_state=False,
|
||||||
)
|
)
|
||||||
return state_ids
|
batch_state_ids.update(ll_state_ids)
|
||||||
|
return batch_state_ids
|
||||||
|
|
||||||
if batch:
|
if batch:
|
||||||
state_at_timeline_start = (
|
state_at_timeline_start = (
|
||||||
|
|
|
@ -41,7 +41,6 @@ class ExperimentalFeature(str, Enum):
|
||||||
|
|
||||||
MSC3026 = "msc3026"
|
MSC3026 = "msc3026"
|
||||||
MSC3881 = "msc3881"
|
MSC3881 = "msc3881"
|
||||||
MSC3967 = "msc3967"
|
|
||||||
|
|
||||||
|
|
||||||
class ExperimentalFeaturesRestServlet(RestServlet):
|
class ExperimentalFeaturesRestServlet(RestServlet):
|
||||||
|
|
|
@ -382,16 +382,21 @@ class SigningKeyUploadServlet(RestServlet):
|
||||||
master_key_updatable_without_uia,
|
master_key_updatable_without_uia,
|
||||||
) = await self.e2e_keys_handler.check_cross_signing_setup(user_id)
|
) = await self.e2e_keys_handler.check_cross_signing_setup(user_id)
|
||||||
|
|
||||||
# Before MSC3967 we required UIA both when setting up cross signing for the
|
# Resending exactly the same keys should just 200 OK without doing a UIA prompt.
|
||||||
# first time and when resetting the device signing key. With MSC3967 we only
|
keys_are_different = await self.e2e_keys_handler.has_different_keys(
|
||||||
# require UIA when resetting cross-signing, and not when setting up the first
|
user_id, body
|
||||||
# time. Because there is no UIA in MSC3861, for now we throw an error if the
|
)
|
||||||
# user tries to reset the device signing key when MSC3861 is enabled, but allow
|
if not keys_are_different:
|
||||||
# first-time setup.
|
return 200, {}
|
||||||
|
|
||||||
|
# The keys are different; is x-signing set up? If no, then this is first-time
|
||||||
|
# setup, and that is allowed without UIA, per MSC3967.
|
||||||
|
# If yes, then we need to authenticate the change.
|
||||||
|
if is_cross_signing_setup:
|
||||||
|
# With MSC3861, UIA is not possible. Instead, the auth service has to
|
||||||
|
# explicitly mark the master key as replaceable.
|
||||||
if self.hs.config.experimental.msc3861.enabled:
|
if self.hs.config.experimental.msc3861.enabled:
|
||||||
# The auth service has to explicitly mark the master key as replaceable
|
if not master_key_updatable_without_uia:
|
||||||
# without UIA to reset the device signing key with MSC3861.
|
|
||||||
if is_cross_signing_setup and not master_key_updatable_without_uia:
|
|
||||||
config = self.hs.config.experimental.msc3861
|
config = self.hs.config.experimental.msc3861
|
||||||
if config.account_management_url is not None:
|
if config.account_management_url is not None:
|
||||||
url = f"{config.account_management_url}?action=org.matrix.cross_signing_reset"
|
url = f"{config.account_management_url}?action=org.matrix.cross_signing_reset"
|
||||||
|
@ -404,22 +409,8 @@ class SigningKeyUploadServlet(RestServlet):
|
||||||
f"you first need to approve it at {url} and then try again.",
|
f"you first need to approve it at {url} and then try again.",
|
||||||
Codes.UNRECOGNIZED,
|
Codes.UNRECOGNIZED,
|
||||||
)
|
)
|
||||||
# But first-time setup is fine
|
else:
|
||||||
|
# Without MSC3861, we require UIA.
|
||||||
elif self.hs.config.experimental.msc3967_enabled:
|
|
||||||
# MSC3967 allows this endpoint to 200 OK for idempotency. Resending exactly the same
|
|
||||||
# keys should just 200 OK without doing a UIA prompt.
|
|
||||||
keys_are_different = await self.e2e_keys_handler.has_different_keys(
|
|
||||||
user_id, body
|
|
||||||
)
|
|
||||||
if not keys_are_different:
|
|
||||||
# FIXME: we do not fallthrough to upload_signing_keys_for_user because confusingly
|
|
||||||
# if we do, we 500 as it looks like it tries to INSERT the same key twice, causing a
|
|
||||||
# unique key constraint violation. This sounds like a bug?
|
|
||||||
return 200, {}
|
|
||||||
# the keys are different, is x-signing set up? If no, then the keys don't exist which is
|
|
||||||
# why they are different. If yes, then we need to UIA to change them.
|
|
||||||
if is_cross_signing_setup:
|
|
||||||
await self.auth_handler.validate_user_via_ui_auth(
|
await self.auth_handler.validate_user_via_ui_auth(
|
||||||
requester,
|
requester,
|
||||||
request,
|
request,
|
||||||
|
@ -428,18 +419,6 @@ class SigningKeyUploadServlet(RestServlet):
|
||||||
# Do not allow skipping of UIA auth.
|
# Do not allow skipping of UIA auth.
|
||||||
can_skip_ui_auth=False,
|
can_skip_ui_auth=False,
|
||||||
)
|
)
|
||||||
# Otherwise we don't require UIA since we are setting up cross signing for first time
|
|
||||||
else:
|
|
||||||
# Previous behaviour is to always require UIA but allow it to be skipped
|
|
||||||
await self.auth_handler.validate_user_via_ui_auth(
|
|
||||||
requester,
|
|
||||||
request,
|
|
||||||
body,
|
|
||||||
"add a device signing key to your account",
|
|
||||||
# Allow skipping of UI auth since this is frequently called directly
|
|
||||||
# after login and it is silly to ask users to re-auth immediately.
|
|
||||||
can_skip_ui_auth=True,
|
|
||||||
)
|
|
||||||
|
|
||||||
result = await self.e2e_keys_handler.upload_signing_keys_for_user(user_id, body)
|
result = await self.e2e_keys_handler.upload_signing_keys_for_user(user_id, body)
|
||||||
return 200, result
|
return 200, result
|
||||||
|
|
|
@ -541,6 +541,8 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
|
||||||
|
|
||||||
self.assertEqual(channel.code, 200, channel.json_body)
|
self.assertEqual(channel.code, 200, channel.json_body)
|
||||||
|
|
||||||
|
# Try uploading *different* keys; it should cause a 501 error.
|
||||||
|
keys_upload_body = self.make_device_keys(USER_ID, DEVICE)
|
||||||
channel = self.make_request(
|
channel = self.make_request(
|
||||||
"POST",
|
"POST",
|
||||||
"/_matrix/client/v3/keys/device_signing/upload",
|
"/_matrix/client/v3/keys/device_signing/upload",
|
||||||
|
|
|
@ -435,10 +435,6 @@ class ExperimentalFeaturesTestCase(unittest.HomeserverTestCase):
|
||||||
True,
|
True,
|
||||||
channel.json_body["features"]["msc3881"],
|
channel.json_body["features"]["msc3881"],
|
||||||
)
|
)
|
||||||
self.assertEqual(
|
|
||||||
False,
|
|
||||||
channel.json_body["features"]["msc3967"],
|
|
||||||
)
|
|
||||||
|
|
||||||
# test nothing blows up if you try to disable a feature that isn't already enabled
|
# test nothing blows up if you try to disable a feature that isn't already enabled
|
||||||
url = f"{self.url}/{self.other_user}"
|
url = f"{self.url}/{self.other_user}"
|
||||||
|
|
|
@ -155,71 +155,6 @@ class KeyQueryTestCase(unittest.HomeserverTestCase):
|
||||||
}
|
}
|
||||||
|
|
||||||
def test_device_signing_with_uia(self) -> None:
|
def test_device_signing_with_uia(self) -> None:
|
||||||
"""Device signing key upload requires UIA."""
|
|
||||||
password = "wonderland"
|
|
||||||
device_id = "ABCDEFGHI"
|
|
||||||
alice_id = self.register_user("alice", password)
|
|
||||||
alice_token = self.login("alice", password, device_id=device_id)
|
|
||||||
|
|
||||||
content = self.make_device_keys(alice_id, device_id)
|
|
||||||
|
|
||||||
channel = self.make_request(
|
|
||||||
"POST",
|
|
||||||
"/_matrix/client/v3/keys/device_signing/upload",
|
|
||||||
content,
|
|
||||||
alice_token,
|
|
||||||
)
|
|
||||||
|
|
||||||
self.assertEqual(channel.code, HTTPStatus.UNAUTHORIZED, channel.result)
|
|
||||||
# Grab the session
|
|
||||||
session = channel.json_body["session"]
|
|
||||||
# Ensure that flows are what is expected.
|
|
||||||
self.assertIn({"stages": ["m.login.password"]}, channel.json_body["flows"])
|
|
||||||
|
|
||||||
# add UI auth
|
|
||||||
content["auth"] = {
|
|
||||||
"type": "m.login.password",
|
|
||||||
"identifier": {"type": "m.id.user", "user": alice_id},
|
|
||||||
"password": password,
|
|
||||||
"session": session,
|
|
||||||
}
|
|
||||||
|
|
||||||
channel = self.make_request(
|
|
||||||
"POST",
|
|
||||||
"/_matrix/client/v3/keys/device_signing/upload",
|
|
||||||
content,
|
|
||||||
alice_token,
|
|
||||||
)
|
|
||||||
|
|
||||||
self.assertEqual(channel.code, HTTPStatus.OK, channel.result)
|
|
||||||
|
|
||||||
@override_config({"ui_auth": {"session_timeout": "15m"}})
|
|
||||||
def test_device_signing_with_uia_session_timeout(self) -> None:
|
|
||||||
"""Device signing key upload requires UIA buy passes with grace period."""
|
|
||||||
password = "wonderland"
|
|
||||||
device_id = "ABCDEFGHI"
|
|
||||||
alice_id = self.register_user("alice", password)
|
|
||||||
alice_token = self.login("alice", password, device_id=device_id)
|
|
||||||
|
|
||||||
content = self.make_device_keys(alice_id, device_id)
|
|
||||||
|
|
||||||
channel = self.make_request(
|
|
||||||
"POST",
|
|
||||||
"/_matrix/client/v3/keys/device_signing/upload",
|
|
||||||
content,
|
|
||||||
alice_token,
|
|
||||||
)
|
|
||||||
|
|
||||||
self.assertEqual(channel.code, HTTPStatus.OK, channel.result)
|
|
||||||
|
|
||||||
@override_config(
|
|
||||||
{
|
|
||||||
"experimental_features": {"msc3967_enabled": True},
|
|
||||||
"ui_auth": {"session_timeout": "15s"},
|
|
||||||
}
|
|
||||||
)
|
|
||||||
def test_device_signing_with_msc3967(self) -> None:
|
|
||||||
"""Device signing key follows MSC3967 behaviour when enabled."""
|
|
||||||
password = "wonderland"
|
password = "wonderland"
|
||||||
device_id = "ABCDEFGHI"
|
device_id = "ABCDEFGHI"
|
||||||
alice_id = self.register_user("alice", password)
|
alice_id = self.register_user("alice", password)
|
||||||
|
|
Loading…
Reference in a new issue