First draft of AS filters #1300
Conversation
| .. NOTE:: | ||
| Please note the filter options in the registration object above. Homeservers | ||
| should exclude traffic based on the filters. Filters are always off by default | ||
| so do not exclude traffic unless the option is explicitly set. |
There was a problem hiding this comment.
"Filters should be disabled by default." is good enough here I think.
| exclude_virtual_users: <bool> # This will exclude traffic from virtual users being | ||
| # recieved on the AS. | ||
| exclude_as_user: <bool> # This will exclude traffic from the AS user | ||
| # being recieved on the AS. |
| @@ -104,6 +104,11 @@ traffic to the AS is: | |||
| - ... | |||
| aliases: [] # Namespaces of room aliases which should be delegated to the AS | |||
| rooms: [] # Namespaces of room ids which should be delegated to the AS | |||
| filter: | |||
| exclude_virtual_users: <bool> # This will exclude traffic from virtual users being | |||
| # recieved on the AS. | |||
| @@ -143,6 +148,15 @@ be made without blocking other aspects of the homeserver. Homeservers MUST NOT | |||
| alter (e.g. add more) events they were going to send within that transaction ID | |||
| on retries, as the AS may have already processed the events. | |||
|
|
|||
| .. NOTE:: | |||
| Please note the filter options in the registration object above. Homeservers | |||
| should exclude traffic based on the filters. Filters are always off by default | |||
There was a problem hiding this comment.
"Homeservers should exclude traffic to each application service based on the filters they have set." to explicitly say it's on a per-appservice basis?
| should exclude traffic based on the filters. Filters are always off by default | ||
| so do not exclude traffic unless the option is explicitly set. | ||
|
|
||
| Additionally the filters are within the context of the transaction API, the |
There was a problem hiding this comment.
"the transaction API. The"
| so do not exclude traffic unless the option is explicitly set. | ||
|
|
||
| Additionally the filters are within the context of the transaction API, the | ||
| Client Server API must continue to show events that may be excluded by the |
There was a problem hiding this comment.
"Client Server API must continue to show events that would otherwise be excluded by the filter."
Also do we always do "MUST" instead of "must" or?
| @@ -104,6 +104,11 @@ traffic to the AS is: | |||
| - ... | |||
| aliases: [] # Namespaces of room aliases which should be delegated to the AS | |||
| rooms: [] # Namespaces of room ids which should be delegated to the AS | |||
| filter: | |||
| exclude_virtual_users: <bool> # This will exclude traffic from virtual users being | |||
There was a problem hiding this comment.
This is the first introduction (that I can see while skimming) for 'virtual'. As a suggestion, maybe have this be moved under the individual user namespaces as an echo flag or similar?
There was a problem hiding this comment.
I feel like namespaces was literally intended for namespacing regexes rather than filtering and would confuse things more.
There was a problem hiding this comment.
As far as naming goes, unsure :s
There was a problem hiding this comment.
use case for putting it under namespaces: disabling echo for one namespace, but not another. I'm not sure why you'd do this, but it feels wrong to exclude the option.
| filter: | ||
| exclude_virtual_users: <bool> # This will exclude traffic from virtual users being | ||
| # recieved on the AS. | ||
| exclude_as_user: <bool> # This will exclude traffic from the AS user |
There was a problem hiding this comment.
I'd recommend making this include_as_user given the default is false (and to avoid double negatives). Also, the comment should explain that this is the sender_localpart given this is the first introduction of an "AS user".
|
Because Github has hidden it, I'm still unsure what would make a good substitute for |
|
This is purely synapse's implementation stuff - why is it even in here? |
|
@maxidor While the registration file is currently an example only, I think there definitely is worth in speccing it as a proper thing,so that registration files can be used cross HS. A lot of ASs generate registration files for you, and it would be a shame if those generated files wouldn't work on MXHSD or Dendrite (or whatever new HS), right? |
@Half-Shot this option is my preference, although the existing spec uses the term "masqueraded user", so maybe continue using that somehow? |
|
FYI, #3806 includes the proposal. |
|
I'm going to close this while we're waiting for the MSC to go through. |
I consider this change small and niche enough that it probably doesn't warrant a gdoc, but happy to create if needed.
Fixes matrix-org/matrix-spec#301
-- Proposal created as #3806 --