-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
Clarify that the event field of the send_join is only required when performing a restricted join #1834
Clarify that the event field of the send_join is only required when performing a restricted join #1834
Conversation
…erforming a restricted join Signed-off-by: Matthias Ahouansou <matthias@ahouansou.cz>
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 looks correct to me given the discussion in #1708.
Thanks for bringing this one over the line!
I'm unsure it is. There's two definitions at play:
I think the intention was 1 to keep the API shape easier for implementations. |
What is the usecase of the former? The latter makes sense because you need the signature of the resident server, but if there is no signature, it's just useless because you already have the event you signed yourself.
|
As I mentioned above, to keep the API shape simpler -- the joining server doesn't need to worry about why it was allowed to join the room, it just grabs the event from the response and it will be the exact event sent into the room. This might be of dubious value, but I believe it was the original design. |
Required if the `content` of the event in the request contained the `join_authorised_via_users_server` | ||
field. The signed copy of the membership event sent to other servers by the resident server, |
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.
As I've just written over at #1708: this doesn't seem to match Synapse's behaviour. AFAICT, the presence of join_authorised_via_users_server
is not sufficient for Synapse to return a (signed) event here.
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.
Does Synapse return the event if join_authorised_via_users_server
is present and the room version supports restricted joins? Or what is it's behavior?
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 synapse code is at https://github.com/element-hq/synapse/blob/v1.108.0/synapse/federation/federation_server.py#L952-L956.
Currently, it returns the event for all requests to /send_join
. But it only signs the event if the user is actually performing a restricted join.
This PR is updated by #1840 |
Fixes #1708
Pull Request Checklist
Preview: https://pr1834--matrix-spec-previews.netlify.app