Accept an optional web-client argument for the invite. #326

clokep · 2020-12-11T13:55:49Z

This allows a homeserver to ask for Sydent to direct the user to a custom web client, which is useful if using a branded Element Web not hosted at app.element.io.

Related to matrix-org/synapse#8863.

anoadragon453

Mostly lgtm, just some nitpicking and questions.

anoadragon453 · 2020-12-14T11:36:45Z

res/invite_template.eml

@@ -18,7 +18,7 @@ Matrix. To join the conversation, either pick a Matrix client from
 https://matrix.org/docs/projects/try-matrix-now.html or use the single-click
 link below to join via Element (requires Chrome, Firefox, Safari, iOS or Android)

-https://app.element.io/#/room/%(room_id_forurl)s?email=%(to_forurl)s&signurl=https%%3A%%2F%%2Fmatrix.org%%2F_matrix%%2Fidentity%%2Fapi%%2Fv1%%2Fsign-ed25519%%3Ftoken%%3D%(token)s%%26private_key%%3D%(ephemeral_private_key)s&room_name=%(room_name_forurl)s&room_avatar_url=%(room_avatar_url_forurl)s&inviter_name=%(sender_display_name_forurl)s&guest_access_token=%(guest_access_token_forurl)s&guest_user_id=%(guest_user_id_forurl)s
+%(web_client_location)/#/room/%(room_id_forurl)s?email=%(to_forurl)s&signurl=https%%3A%%2F%%2Fmatrix.org%%2F_matrix%%2Fidentity%%2Fapi%%2Fv1%%2Fsign-ed25519%%3Ftoken%%3D%(token)s%%26private_key%%3D%(ephemeral_private_key)s&room_name=%(room_name_forurl)s&room_avatar_url=%(room_avatar_url_forurl)s&inviter_name=%(sender_display_name_forurl)s&guest_access_token=%(guest_access_token_forurl)s&guest_user_id=%(guest_user_id_forurl)s


This is still quite Element-specific. Would this work with Hydrogen for instance? We could be fancy and allow the client to provide a URL filled with named placeholders.

Perhaps we don't care about this for right now though.

I'll see if it is reasonable for a homeserver to calculate the other items here. 👍

The way that sydent currently works I don't think this is easily doable since a unique key is included in this URL that sydent generates (ephemeral_private_key), see the code around:

sydent/sydent/http/servlets/store_invite_servlet.py

Lines 71 to 80 in c55007f

tokenStore = JoinTokenStore(self.sydent)

ephemeralPrivateKey = nacl.signing.SigningKey.generate()

ephemeralPublicKey = ephemeralPrivateKey.verify_key

ephemeralPrivateKeyBase64 = encode_base64(ephemeralPrivateKey.encode(), True)

ephemeralPublicKeyBase64 = encode_base64(ephemeralPublicKey.encode(), True)

tokenStore.storeEphemeralPublicKey(ephemeralPublicKeyBase64)

tokenStore.storeToken(medium, address, roomId, sender, token)

I see, thanks for digging. Note that I think we've talked about this key being either unhelpful or broken in Synapse's current implementation: matrix-org/synapse#6036

sydent/http/servlets/store_invite_servlet.py

anoadragon453 · 2020-12-14T11:54:47Z

sydent/http/servlets/store_invite_servlet.py

@@ -102,6 +105,9 @@ def render_POST(self, request):
        if substitutions["room_name"] != '':
            substitutions["bracketed_room_name"] = "(%s)" % substitutions["room_name"]

+        if 'web_client_location' not in substitutions:
+            substitutions["web_client_location"] = 'https://app.element.io'


It'd be nice to have this as a config option instead... but this could be easily done later on if needed.

I think this was done in bab8196.

clokep · 2020-12-14T13:33:12Z

sydent/http/servlets/store_invite_servlet.py

            'sender_display_name',
            'token',
            'room_name',
            'bracketed_room_name',
            'room_avatar_url',
-            'sender_display_name',
+            'sender_avatar_url',


Oh this is an unrelated fix, but send_display_name was here twice, I'm fairly certain this is supposed to be sender_avatar_url as that's a speced property.

clokep · 2020-12-17T14:52:54Z

@anoadragon453 Let me know what you think of these changes, although there's still some discussion going on with the Synapse PR.

anoadragon453

Current changes look good to me.

I was on the fence for a moment about still including app.element.io as the default config option. But seeing as it would be the easiest way for someone running Sydent to get started and serves as a working example in itself, I think it's a good thing.

clokep · 2020-12-18T12:33:30Z

I was on the fence for a moment about still including app.element.io as the default config option. But seeing as it would be the easiest way for someone running Sydent to get started and serves as a working example in itself, I think it's a good thing.

I think you pretty much have to include it for backwards compatibility. It would be unfortunate to require a change on upgrade.

anoadragon453

Seeing as the Synapse PR has been approved and no changes affecting the Sydent implementation have happened since then - tick from me too.

Sydent 2.3.0 (2021-04-15) ========================= **Note**: this will be the last release of Sydent to support Python 3.5 or earlier. Future releases will require at least Python 3.6. Security advisory ----------------- This release contains fixes to the following security issues: - Denial of service attack via disk space or memory exhaustion ([CVE-2021-29430](https://cve.mitre.org/cgi-bin/cvename.cgi?name=2021-29430)). - SSRF due to missing validation of hostnames ([CVE-2021-29431](https://cve.mitre.org/cgi-bin/cvename.cgi?name=2021-29431)). - Malicious users could control the content of invitation emails ([CVE-2021-29432](https://cve.mitre.org/cgi-bin/cvename.cgi?name=2021-29432)). - Denial of service (via resource exhaustion) due to improper input validation ([CVE-2021-29433](https://cve.mitre.org/cgi-bin/cvename.cgi?name=2021-29433)). Although we are not aware of these vulnerabilities being exploited in the wild, Sydent server administrators are advised to update as soon as possible. Note that as well as changes to the package, there are also changes to the default email templates. If any templates have been updated locally, they must also be updated in line with the changes to the defaults for full protection from CVE-2021-29432. Features -------- - Accept an optional `web_client_location` argument to the invite endpoint which allows customisation of the email template. ([\#326](#326)) - Move templates to a per-brand subdirectory of `/res`. Add `templates.path` and `brand.default` config options. ([\#328](#328)) Bugfixes -------- - Fix a regression in v2.2.0 where the wrong characters would be obfuscated in a 3pid invite. ([\#317](#317)) - Fix a long-standing bug where invalid JSON would be accepted over the HTTP interfaces. ([\#337](#337)) - During user registration on the identity server, validate that the MXID returned by the contacted homeserver is valid for that homeserver. ([cc97fff](cc97fff)) - Ensure that `/v2/` endpoints are correctly authenticated. ([ce04a68](ce04a68)) - Perform additional validation on the response received when requesting server signing keys. ([07e6da7](07e6da7)) - Validate the `matrix_server_name` parameter given during user registration. ([9e57334](9e57334), [8936925](8936925), [3d531ed](3d531ed), [0f00412](0f00412)) - Limit the size of requests received from HTTP clients. ([89071a1](89071a1), [0523511](0523511), [f56eee3](f56eee3)) - Limit the size of responses received from HTTP servers. ([89071a1](89071a1), [0523511](0523511), [f56eee3](f56eee3)) - In invite emails, randomise the multipart boundary, and include MXIDs where available. ([4469d1d](4469d1d), [6b405a8](6b405a8), [65a6e91](65a6e91)) - Perform additional validation on the `client_secret` and `email` parameters to various APIs. ([3175fd3](3175fd3)) Updates to the Docker image --------------------------- - Base docker image on Debian rather than Alpine Linux. ([\#335](#335)) Internal Changes ---------------- - Fix test logging to allow braces in log output. ([\#318](#318)) - Install prometheus_client in the Docker image. ([\#325](#325)) - Bump the version of signedjson to 1.1.1. ([\#334](#334))

Accept an optionalweb-client to redirect to on invite.

c55007f

clokep force-pushed the clokep/web-client-arg branch from 26100ee to c55007f Compare December 11, 2020 13:57

clokep mentioned this pull request Dec 11, 2020

Send the location of the web client to the IS when inviting via 3PIDs. matrix-org/synapse#8930

Merged

clokep requested a review from a team December 11, 2020 14:48

clokep self-assigned this Dec 11, 2020

anoadragon453 reviewed Dec 14, 2020

View reviewed changes

Clarify variable name.

0544875

clokep commented Dec 14, 2020

View reviewed changes

Allow specifying a default web client location in the config.

bab8196

clokep requested a review from anoadragon453 December 14, 2020 13:39

Add a namespace to argument.

9529821

clokep changed the title ~~Accept an optional web-client to redirect to on invite.~~ Accept an optional web-client argument for the invite. Dec 16, 2020

anoadragon453 reviewed Dec 18, 2020

View reviewed changes

anoadragon453 approved these changes Dec 18, 2020

View reviewed changes

clokep merged commit 4e0274d into master Dec 18, 2020

clokep deleted the clokep/web-client-arg branch December 18, 2020 16:02

clokep mentioned this pull request Sep 2, 2021

Use the provided web client in the HTML email. #380

Merged

turt2live mentioned this pull request Feb 2, 2022

Allow email invites to override/specify some custom options turt2live/matrix-bot-sdk#199

Open

richvdh mentioned this pull request Nov 25, 2022

Email invite flow uses unspecced org.matrix.web_client_location parameter matrix-org/matrix-spec#1360

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accept an optional web-client argument for the invite. #326

Accept an optional web-client argument for the invite. #326

clokep commented Dec 11, 2020 •

edited

Loading

anoadragon453 left a comment

anoadragon453 Dec 14, 2020

clokep Dec 14, 2020

clokep Dec 14, 2020

anoadragon453 Dec 18, 2020

anoadragon453 Dec 14, 2020

clokep Dec 14, 2020

clokep Dec 14, 2020

clokep commented Dec 17, 2020

anoadragon453 left a comment •

edited

Loading

clokep commented Dec 18, 2020

anoadragon453 left a comment

	tokenStore = JoinTokenStore(self.sydent)

	ephemeralPrivateKey = nacl.signing.SigningKey.generate()
	ephemeralPublicKey = ephemeralPrivateKey.verify_key

	ephemeralPrivateKeyBase64 = encode_base64(ephemeralPrivateKey.encode(), True)
	ephemeralPublicKeyBase64 = encode_base64(ephemeralPublicKey.encode(), True)

	tokenStore.storeEphemeralPublicKey(ephemeralPublicKeyBase64)
	tokenStore.storeToken(medium, address, roomId, sender, token)

Accept an optional web-client argument for the invite. #326

Accept an optional web-client argument for the invite. #326

Conversation

clokep commented Dec 11, 2020 • edited Loading

anoadragon453 left a comment

Choose a reason for hiding this comment

anoadragon453 Dec 14, 2020

Choose a reason for hiding this comment

clokep Dec 14, 2020

Choose a reason for hiding this comment

clokep Dec 14, 2020

Choose a reason for hiding this comment

anoadragon453 Dec 18, 2020

Choose a reason for hiding this comment

anoadragon453 Dec 14, 2020

Choose a reason for hiding this comment

clokep Dec 14, 2020

Choose a reason for hiding this comment

clokep Dec 14, 2020

Choose a reason for hiding this comment

clokep commented Dec 17, 2020

anoadragon453 left a comment • edited Loading

Choose a reason for hiding this comment

clokep commented Dec 18, 2020

anoadragon453 left a comment

Choose a reason for hiding this comment

clokep commented Dec 11, 2020 •

edited

Loading

anoadragon453 left a comment •

edited

Loading