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

Improved delegation doc (adding .well-known info) #4781

Closed
wants to merge 1 commit into from

Conversation

vaab
Copy link

@vaab vaab commented Mar 2, 2019

Just a suggestion of improvement of the doc provided in the README... that's by no mean a perfect solution and am happy if it is not integrated for some reason, but at least if this kind of doc could have been there in the first place, I would definitively saved one days of battling my way through the numerous incomplete documentations and guide.

It this addition seems to help, I'll add the changelog file gladly for inclusion.

Pull Request Checklist

  • Pull request is based on the develop branch
  • [need request number] Pull request includes a changelog file
  • Pull request includes a sign off

@vaab vaab changed the title Improved delegation doc Improved delegation doc (adding .well-known info) Mar 2, 2019
@vaab vaab changed the base branch from master to develop March 2, 2019 15:09
Signed-off-by: Valentin Lab <valentin.lab@kalysto.org>
@vaab
Copy link
Author

vaab commented Mar 2, 2019

It definitively needs technical overview, as I'm very new to matrix and synapse, and basic wording as I'm not a native english speaker.

@neilisfragile
Copy link
Contributor

neilisfragile commented Mar 7, 2019

Hi @vaab this is great, thanks so much for putting the time in 👍

I'm actually trying to solve a similar problem problem at #4766 I'll either incorporate your work into #4766 (and ensure that you are credited) or make some changes on this PR before merging.

@neilisfragile
Copy link
Contributor

Tracking here #4832

@richvdh richvdh closed this Mar 8, 2019
README.rst Show resolved Hide resolved
neilisfragile added a commit that referenced this pull request Mar 12, 2019
Improved federation configuration docs.  Specifically detailing  .well-known and SRV based delegation methods. 

Inspiration Valentin Lab <valentin.lab@kalysto.org> for #4781
@neilisfragile
Copy link
Contributor

@vaab #4832 now merged - I had to rebase and squash merge, but have provided credit through the commit and changelog entry. Thanks for all your help - the docs are much better now.

anoadragon453 pushed a commit that referenced this pull request Mar 13, 2019
Improved federation configuration docs.  Specifically detailing  .well-known and SRV based delegation methods.

Inspiration Valentin Lab <valentin.lab@kalysto.org> for #4781
@vaab
Copy link
Author

vaab commented Mar 14, 2019

Thanks ! I noticed you nearly completely removed my (perhaps too heavy) warnings about DNS SRV method not actually delivering a complete delegation as .well-known method would do. This is a keypoint that is quite unexpected and completely invalidates DNS SRV as a working delegation solution in a common scenario : people using virtualhosts, and using both domain name (the delegatee and the delegated) in 2 different virtualhosts that have to answer 2 different SSL ids. In the pushed documentation, you still sell both delegation method as equivalent, which they are clearly not functionally speaking.

It appeared to me primordial to add this info in the introduction so as the people could safely choose the solution they wanted to use.

To be honest, the time I lost on that particular point was what pushed me trying to contribute here on a better doc, as I really felt misguided.

I'm just trying here to share what seemed important for me in my contribution, I'm very happy that the documentation moved forward anyway. I noticed your careful attention for reviewing my flawed contribution and make it better. And I wish I had more time to bring up this point at the right moment.

Feel free to ignore my point if you feel so.

@richvdh
Copy link
Member

richvdh commented Mar 15, 2019

@vaab: the point is that the two delegation methods offer different features; it's simply not true to say that .well-known is a "complete" delegation and that SRV is somehow incomplete. Arguably .well-known is incomplete because it doesn't offer round-robin and failover support.

Neil's rewrite fairly clearly spells out the requirements for the TLS certificate: here "This method requires the target server to provide a valid TLS certificate for the original server_name."

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