Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Bundle in some room state in invites. #275

Merged
merged 3 commits into from
Oct 2, 2015
Merged

Conversation

erikjohnston
Copy link
Member

Changes:

  • When an invite is sent to someone, bundle in the state of the room at the time under unsigned.invite_room_state. In v1 C-S API this is copied to a top level key of invite_room_state.
  • Include the invite event in initialSync.

The state events that are included are:

  • Room name
  • Room avatar
  • Canonical Alias
  • Join rules

The fields of the events that are included:

  • type
  • state_key
  • content

This is not a particularly nice way of getting this done, but to do it any nicer would probably involve changing the federation API in a non backwards compatible way.

Review on Reviewable

@ara4n
Copy link
Member

ara4n commented Sep 21, 2015

by putting this in unsigned does it make it vulnerable to spoofing and phishing?

@NegativeMjark
Copy link
Contributor

  • Does the unsigned key make it into the C-S APIs? In particular does it make it into initialSync?
  • Is this added to the specification?
  • Do we have tests in sytest for it?

@erikjohnston
Copy link
Member Author

by putting this in unsigned does it make it vulnerable to spoofing and phishing?

@ara4n: Nope. The event is sent directly to the server, and so is protected bySSL and requestsigning

  • Does the unsigned key make it into the C-S APIs? In particular does it make it into initialSync?

Yes. In particular: "In v1 C-S API this is copied to a top level key of invite_room_state." And the event has been added to initialSync.

  • Is this added to the specification?
  • Do we have tests in sytest for it?

No.

@ara4n
Copy link
Member

ara4n commented Sep 25, 2015

Is this added to the specification?
Do we have tests in sytest for it?
No.

Changes to serverside APIs need to be documented in the spec, even if they are a stopgap until a v2 API (and marked as such), as otherwise client developers (internal or external) have no hope of implementing them consistently. We should never expect client developers to read the synapse source code or comments on PRs to understand an API.

Less important, but we should certainly be adding tests whenever we add an API.

Please can this PR get finished, given it's been hanging around for over two weeks, so that client developers can get on and implement the functionality clientside? The point of these serverside quick fixes is to unblock clientside features; not to cut corners.

Please correct me if I'm reading this wrong.

@erikjohnston
Copy link
Member Author

So do you want me to spec this before landing the PR?

@ara4n
Copy link
Member

ara4n commented Sep 25, 2015 via email

@erikjohnston
Copy link
Member Author

Spec has landed, any objections to me landing this?

@ara4n
Copy link
Member

ara4n commented Oct 1, 2015

lgtm

@NegativeMjark
Copy link
Contributor

LGTM.

@erikjohnston
Copy link
Member Author

erikjohnston added a commit that referenced this pull request Oct 2, 2015
Bundle in some room state in invites.
@erikjohnston erikjohnston merged commit a086b7a into develop Oct 2, 2015
@erikjohnston erikjohnston deleted the erikj/invite_state branch November 19, 2015 16:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants