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

Allow configuration of a default rendezvous server for use with Sign in with QR #11638

Closed
wants to merge 3 commits into from

Conversation

hughns
Copy link
Member

@hughns hughns commented Sep 20, 2023

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Here's what your changelog entry will look like:

✨ Features

  • Allow configuration of a default rendezvous server for use with Sign in with QR (#11638). Contributed by @hughns.

@hughns hughns added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Sep 20, 2023
@hughns hughns changed the title Allow configuration of a default rendezvous server Allow configuration of a default rendezvous server for use with Sign in with QR Sep 20, 2023
@hughns hughns marked this pull request as ready for review September 21, 2023 15:31
@hughns hughns requested a review from a team as a code owner September 21, 2023 15:31
@hughns hughns force-pushed the hughns/qr-signin-default-rz-server branch from 1595d36 to c01492c Compare September 21, 2023 15:31
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this have a user opt-in like the default STUN server does? What about privacy/terms acceptance for GDPR/etc

@hughns
Copy link
Member Author

hughns commented Sep 21, 2023

Shouldn't this have a user opt-in like the default STUN server does? What about privacy/terms acceptance for GDPR/etc

The assumed consent model is that whomever deploys and operates a build of Element Web that makes use of this config option would need to ensure that their terms of service cover such a use case.

The immediate use case is where a homeserver operator is also providing the Element Web installation. Therefore, the appropriate consent is handled the same as if the rendezvous server is built-in to the homeserver. This is because the default_rz_server would only be used in Element Web after the user has signed in to the homeserver (and therefore accepted any terms).

As such, I don't believe an opt-in mechanism is needed at this time.

@hughns hughns requested a review from t3chguy September 21, 2023 16:31
@t3chguy
Copy link
Member

t3chguy commented Sep 21, 2023

The assumed consent model is that whomever deploys and operates a build of Element Web that makes use of this config option would need to ensure that their terms of service cover such a use case.

But that isn't how other things work in the Element Web client. The person deploying Element Web chooses the HS and can even enforce it by use of https://github.com/vector-im/element-web/blob/develop/docs/config.md disable_custom_urls but the terms of the server will still be presented to the user.

Element Web running entirely on the user's device means it doesn't need its own privacy/terms, but as soon as you're reaching out to a server that changes.

This is because the default_rz_server would only be used in Element Web after the user has signed in to the homeserver (and therefore accepted any terms).

What protections are there to ensure the rz and hs are of the same operator? Given that HS can be changed (even if disable_custom_urls is specified) by use of a full MXID @bob:server_name on the login screen.

@t3chguy
Copy link
Member

t3chguy commented Sep 21, 2023

I think at the very least it'd need to be a map akin to enable_presence_by_hs_url given that an single RZ probably shouldn't be used for all HSes.

@hughns
Copy link
Member Author

hughns commented Sep 21, 2023

I think at the very least it'd need to be a map akin to enable_presence_by_hs_url given that an single RZ probably shouldn't be used for all HSes.

I will refactor to use this approach.

@t3chguy
Copy link
Member

t3chguy commented Sep 21, 2023

Such an approach is also considered tech debt and will likely need a chat with the WAT whether its appropriate for the record. Why isn't this done via a Synapse plugin or similar which listens to the RZ routes and advertises support via the versions/capabilities API?

@hughns hughns marked this pull request as draft September 21, 2023 16:50
@hughns
Copy link
Member Author

hughns commented Sep 21, 2023

Making draft as the feedback from the maintainer as that solution is unacceptable.

@germain-gg germain-gg removed their request for review September 22, 2023 08:55
@Johennes
Copy link
Contributor

Closing as we've aligned on pursuing #11655 instead.

@Johennes Johennes closed this Sep 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants