-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
An federation whitelist query endpoint extension #16848
Conversation
176a38e
to
298c530
Compare
298c530
to
7d37a8a
Compare
7d37a8a
to
d4c1270
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic looks correct! Just a few wording changes and a q about the config option placement.
I'll admit that introducing a new section for extensions still doesn't quite sit right with me.
synapse/config/extensions.py
Outdated
section = "extensions" | ||
|
||
def read_config(self, config: JsonDict, **kwargs: Any) -> None: | ||
self.federation_whitelist_endpoint: bool = config.get( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously @erikjohnston noted that the "extension" config section was created as this option didn't fit in anywhere else.
But would it not fit under "federation"?
synapse/synapse/config/federation.py
Line 28 in 23740ea
class FederationConfig(Config): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be beneficial to follow the approach mentioned here: #17147 (comment)
ie. to move this to an in-tree-module and have the config follow the normal module config rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The approach mentioned in that PR was deemed undesirable for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But would it not fit under "federation"?
On this, @erikjohnston wrote on Matrix:
Yeah, maybe. I suppose the federation section is relatively small. I'm just thinking of what this will look like if we have e.g. 10 optional features like this, and whether we'd want them grouped together in one place in the docs or not
and whether its clearer in the code or not
I suppose we can cross that bridge when we come to it
I don't see this as an optional feature, more a config option like any other. And they won't end up all under federation of course, but spread across the config. I don't think they'll build up in any noticeable way as a group.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I'll move it under the federation section.
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment on the previous review is still a concern of mine, but everything else lgtm.
If you're happy with everything the way it is now, I'll merge it in 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a couple references to extension features left which should now be removed. Otherwise this LGTM!
This commit fixes the request path in the `federation_whitelist_endpoint_enabled` config option. The path was changed towards the tail end of development in #16848.
No significant changes since 1.108.0rc1. - Add a feature that allows clients to query the configured federation whitelist. Disabled by default. ([\#16848](element-hq/synapse#16848), [\#17199](element-hq/synapse#17199)) - Add the ability to allow numeric user IDs with a specific prefix when in the CAS flow. Contributed by Aurélien Grimpard. ([\#17098](element-hq/synapse#17098)) - Fix bug where push rules would be empty in `/sync` for some accounts. Introduced in v1.93.0. ([\#17142](element-hq/synapse#17142)) - Add support for optional whitespace around the Federation API's `Authorization` header's parameter commas. ([\#17145](element-hq/synapse#17145)) - Fix bug where disabling room publication prevented public rooms being created on workers. ([\#17177](element-hq/synapse#17177), [\#17184](element-hq/synapse#17184)) - Document [`/v1/make_knock`](https://spec.matrix.org/v1.10/server-server-api/#get_matrixfederationv1make_knockroomiduserid) and [`/v1/send_knock/`](https://spec.matrix.org/v1.10/server-server-api/#put_matrixfederationv1send_knockroomideventid) federation endpoints as worker-compatible. ([\#17058](element-hq/synapse#17058)) - Update User Admin API with note about prefixing OIDC external_id providers. ([\#17139](element-hq/synapse#17139)) - Clarify the state of the created room when using the `autocreate_auto_join_room_preset` config option. ([\#17150](element-hq/synapse#17150)) - Update the Admin FAQ with the current libjemalloc version for latest Debian stable. Additionally update the name of the "push_rules" stream in the Workers documentation. ([\#17171](element-hq/synapse#17171)) - Add note to reflect that [MSC3886](matrix-org/matrix-spec-proposals#3886) is closed but will remain supported for some time. ([\#17151](element-hq/synapse#17151)) - Update dependency PyO3 to 0.21. ([\#17162](element-hq/synapse#17162)) - Fixes linter errors found in PR #17147. ([\#17166](element-hq/synapse#17166)) - Bump black from 24.2.0 to 24.4.2. ([\#17170](element-hq/synapse#17170)) - Cache literal sync filter validation for performance. ([\#17186](element-hq/synapse#17186)) - Improve performance by fixing a reactor pause. ([\#17192](element-hq/synapse#17192)) - Route `/make_knock` and `/send_knock` federation APIs to the federation reader worker in Complement test runs. ([\#17195](element-hq/synapse#17195)) - Prepare sync handler to be able to return different sync responses (`SyncVersion`). ([\#17200](element-hq/synapse#17200)) - Organize the sync cache key parameter outside of the sync config (separate concerns). ([\#17201](element-hq/synapse#17201)) - Refactor `SyncResultBuilder` assembly to its own function. ([\#17202](element-hq/synapse#17202)) - Rename to be obvious: `joined_rooms` -> `joined_room_ids`. ([\#17203](element-hq/synapse#17203), [\#17208](element-hq/synapse#17208)) - Add a short pause when rate-limiting a request. ([\#17210](element-hq/synapse#17210)) * Bump cryptography from 42.0.5 to 42.0.7. ([\#17180](element-hq/synapse#17180)) * Bump gitpython from 3.1.41 to 3.1.43. ([\#17181](element-hq/synapse#17181)) * Bump immutabledict from 4.1.0 to 4.2.0. ([\#17179](element-hq/synapse#17179)) * Bump sentry-sdk from 1.40.3 to 2.1.1. ([\#17178](element-hq/synapse#17178)) * Bump serde from 1.0.200 to 1.0.201. ([\#17183](element-hq/synapse#17183)) * Bump serde_json from 1.0.116 to 1.0.117. ([\#17182](element-hq/synapse#17182))
# Synapse 1.109.0 (2024-06-18) - Add the ability to auto-accept invites on the behalf of users. See the [`auto_accept_invites`](https://element-hq.github.io/synapse/latest/usage/configuration/config_documentation.html#auto-accept-invites) config option for details. ([\#17147](element-hq/synapse#17147)) - Add experimental [MSC3575](matrix-org/matrix-spec-proposals#3575) Sliding Sync `/sync/e2ee` endpoint for to-device messages and device encryption info. ([\#17167](element-hq/synapse#17167)) - Support [MSC3916](matrix-org/matrix-spec-proposals#3916) by adding unstable media endpoints to `/_matrix/client`. ([\#17213](element-hq/synapse#17213)) - Add logging to tasks managed by the task scheduler, showing CPU and database usage. ([\#17219](element-hq/synapse#17219)) # Synapse 1.108.0 (2024-05-28) - Add a feature that allows clients to query the configured federation whitelist. Disabled by default. ([\#16848](element-hq/synapse#16848), [\#17199](element-hq/synapse#17199)) - Add the ability to allow numeric user IDs with a specific prefix when in the CAS flow. Contributed by Aurélien Grimpard. ([\#17098](element-hq/synapse#17098)) Synapse 1.107.0 (2024-05-14) - Add preliminary support for [MSC3823: Account Suspension](matrix-org/matrix-spec-proposals#3823). ([\#17051](element-hq/synapse#17051)) - Declare support for [Matrix v1.10](https://matrix.org/blog/2024/03/22/matrix-v1.10-release/). Contributed by @clokep. ([\#17082](element-hq/synapse#17082)) - Add support for [MSC4115: membership metadata on events](matrix-org/matrix-spec-proposals#4115). ([\#17104](element-hq/synapse#17104), [\#17137](element-hq/synapse#17137)) # Synapse 1.106.0 (2024-04-30) - Send an email if the address is already bound to an user account. ([\#16819](element-hq/synapse#16819)) - Implement the rendezvous mechanism described by [MSC4108](matrix-org/matrix-spec-proposals#4108). ([\#17056](element-hq/synapse#17056)) - Support delegating the rendezvous mechanism described [MSC4108](matrix-org/matrix-spec-proposals#4108) to an external implementation. ([\#17086](element-hq/synapse#17086))
This is to allow clients to query the configured federation whitelist. Disabled by default.