Make restricted room join tests clearer #752
Conversation
…o strict ruma parsing requirements Signed-off-by: strawberry <[email protected]>
Signed-off-by: strawberry <[email protected]>
Signed-off-by: morguldir <[email protected]>
| // This should be ignored since this is a join -> join transition. | ||
| "join_authorised_via_users_server": "unused", | ||
| // This should be ignored by the server since this is a join -> join transition | ||
| "join_authorised_via_users_server": "@unused:unused.local", |
There was a problem hiding this comment.
What happens if a ruma-based server gets garbage here then? Does it still work?
There was a problem hiding this comment.
It can create the event then, but it won’t really work properly unless the server removes the field when handling the PUT like synapse does
So when verifying the signature of a federation event actually looking like that it won’t work still
This part i think is intended though, although seemingly synapse changed that behaviour at some point too 🧐
There was a problem hiding this comment.
I'm not really following your comment, sorry. Shouldn't the field be removed no matter what? So what isn't working for ruma?
There was a problem hiding this comment.
Sorry yeah it should, but if another server forgets to remove it, then ruma still tries to check the signature, but synapse seems to still allow the event
Firstly this PR makes the tests work on more implementations, for example ruma and GMS won't really understand
join_authorised_via_users_servernot looking like a user id, similar to what #224 tried to doMaking it a user id that isn't correct still makes sure that the server can handle clients updating the member event directly with /myroomnick or similar
Some errors were also silently being ignored, which made it harder to tell which part of the test was actually failing
Pull Request Checklist