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

Prompt for ICE server fallback permission #3309

Merged
merged 6 commits into from
Aug 15, 2019
Merged

Conversation

jryans
Copy link
Collaborator

@jryans jryans commented Aug 13, 2019

When the homeserver does not have any ICE servers configured, this will cause a prompt to appear context of either placing a call or when a call fails. The user's choice is saved on the local device and can be changed in Settings. The fallback ICE server is only used if the user allows it. The dialog also recommends notifying the homeserver admin to rectify the issue.

2019-08-15 at 15 03

Fixes element-hq/element-web#10173
Depends on matrix-org/matrix-js-sdk#1015

@jryans
Copy link
Collaborator Author

jryans commented Aug 13, 2019

Added @nadonomy for design review, as I made up this text myself, so perhaps he has some better wording.

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

code lgtm. Do we need terms of service/privacy policy on the fallback TURN server?

@turt2live
Copy link
Member

what doesn't lgtm is that the tests are broken for Homeserver not configured to support calls showing up.

@jryans
Copy link
Collaborator Author

jryans commented Aug 13, 2019

what doesn't lgtm is that the tests are broken for Homeserver not configured to support calls showing up.

Ah yeah... 😓 I'll look around for the right place to make the tests work around this.

cancelButton: _t('Dismiss'),
onFinished: (confirmed) => {
if (confirmed) {
cli.setFallbackICEServerAllowed(true);
Copy link
Collaborator Author

@jryans jryans Aug 13, 2019

Choose a reason for hiding this comment

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

This is intended to also be saved as a device setting.

@jryans
Copy link
Collaborator Author

jryans commented Aug 13, 2019

Do we need terms of service/privacy policy on the fallback TURN server?

I wasn't sure either. After some review, since the only data exposed to the fallback STUN server, and that is called out explicitly in the permission dialog, it has been agreed that we don't need additional documents for this case.

jryans added a commit that referenced this pull request Aug 14, 2019
This stores the ICE server fallback permission in a device setting so it is
remembered across sessions.

Part of #3309
@jryans jryans requested a review from turt2live August 14, 2019 14:10
src/settings/Settings.js Outdated Show resolved Hide resolved
@turt2live
Copy link
Member

fwiw this could also be implemented as a dedicated SettingController to deal with the MatrixClient stuff (both reading and writing). It might not be worth the complexity though.

@jryans
Copy link
Collaborator Author

jryans commented Aug 15, 2019

After discussion with @nadonomy, we'll use the following dialog text for the prompt:

You cannot make calls

Your homeserver (test.convolv.es) isn’t configured to make voice or video calls. Ask your homeserver admin to set up a TURN server to enable them.

Alternatively, you can make voice & video calls using our public STUN server (hosted at turn.matrix.org), sharing your IP address with us. You can also manage this in Settings.

[Cancel] [Use Public Server]

@jryans
Copy link
Collaborator Author

jryans commented Aug 15, 2019

fwiw this could also be implemented as a dedicated SettingController to deal with the MatrixClient stuff (both reading and writing). It might not be worth the complexity though.

Ah yeah, good to know! It's probably okay for now as is I think, but if we were to sprinkle it around further, then I'd probably change to that.

@jryans jryans removed the request for review from nadonomy August 15, 2019 10:26
@jryans
Copy link
Collaborator Author

jryans commented Aug 15, 2019

A further revision from Matthew:

Call failed due to misconfigured server

Please ask the administrator of your homeserver (arasphere.net) to configure a TURN server in order for calls to work reliably.

Alternatively, you can try to use the public server at turn.matrix.org, but this will not be as reliable, and it will share your IP address with that server. You can also manage this in Settings.

[OK] [Try using turn.matrix.org]

This adds a prompt at the start of each session when the homeserver does not
have any ICE servers configured. The fallback ICE server is only used if the
user allows it. The dialog also recommends notifying the homeserver admin to
rectify the issue.

Fixes element-hq/element-web#10173
This stores the ICE server fallback permission in a device setting so it is
remembered across sessions.

Part of #3309
This moves the ICE fallback prompt out of session startup and instead it will
now appear contextually when your either place a call with no ICE server from
the homeserver or a call fails (in either direction).

Fixes element-hq/element-web#10546
@jryans jryans merged commit f3e1697 into develop Aug 15, 2019
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.

VoIP: Stop falling back to Google for STUN
2 participants