From 4e658be3ccf69d50366bd6f03a330176dd35adad Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 26 Sep 2023 15:41:31 +0200 Subject: [PATCH 1/4] Element call URL must have https scheme. --- features/call/src/main/AndroidManifest.xml | 1 - .../features/call/CallIntentDataParser.kt | 2 +- .../call/CallIntentDataParserTests.kt | 36 ++++++++++++++----- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/features/call/src/main/AndroidManifest.xml b/features/call/src/main/AndroidManifest.xml index d106d4e7b8..877b7fb0a8 100644 --- a/features/call/src/main/AndroidManifest.xml +++ b/features/call/src/main/AndroidManifest.xml @@ -39,7 +39,6 @@ - diff --git a/features/call/src/main/kotlin/io/element/android/features/call/CallIntentDataParser.kt b/features/call/src/main/kotlin/io/element/android/features/call/CallIntentDataParser.kt index 30e71d6201..f1c4291526 100644 --- a/features/call/src/main/kotlin/io/element/android/features/call/CallIntentDataParser.kt +++ b/features/call/src/main/kotlin/io/element/android/features/call/CallIntentDataParser.kt @@ -21,7 +21,7 @@ import javax.inject.Inject class CallIntentDataParser @Inject constructor() { - private val validHttpSchemes = sequenceOf("http", "https") + private val validHttpSchemes = sequenceOf("https") fun parse(data: String?): String? { val parsedUrl = data?.let { Uri.parse(data) } ?: return null diff --git a/features/call/src/test/kotlin/io/element/android/features/call/CallIntentDataParserTests.kt b/features/call/src/test/kotlin/io/element/android/features/call/CallIntentDataParserTests.kt index 71290f15b7..4d5beef7e9 100644 --- a/features/call/src/test/kotlin/io/element/android/features/call/CallIntentDataParserTests.kt +++ b/features/call/src/test/kotlin/io/element/android/features/call/CallIntentDataParserTests.kt @@ -52,13 +52,17 @@ class CallIntentDataParserTests { } @Test - fun `Element Call urls will be returned as is`() { + fun `Element Call http urls returns null`() { val httpBaseUrl = "http://call.element.io" val httpCallUrl = "http://call.element.io/some-actual-call?with=parameters" + assertThat(callIntentDataParser.parse(httpBaseUrl)).isNull() + assertThat(callIntentDataParser.parse(httpCallUrl)).isNull() + } + + @Test + fun `Element Call urls will be returned as is`() { val httpsBaseUrl = "https://call.element.io" val httpsCallUrl = "https://call.element.io/some-actual-call?with=parameters" - assertThat(callIntentDataParser.parse(httpBaseUrl)).isEqualTo(httpBaseUrl) - assertThat(callIntentDataParser.parse(httpCallUrl)).isEqualTo(httpCallUrl) assertThat(callIntentDataParser.parse(httpsBaseUrl)).isEqualTo(httpsBaseUrl) assertThat(callIntentDataParser.parse(httpsCallUrl)).isEqualTo(httpsCallUrl) } @@ -76,18 +80,34 @@ class CallIntentDataParserTests { } @Test - fun `element scheme with call host and url param gets url extracted`() { + fun `element scheme with call host and url with http will returns null`() { val embeddedUrl = "http://call.element.io/some-actual-call?with=parameters" val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") val url = "element://call?url=$encodedUrl" + assertThat(callIntentDataParser.parse(url)).isNull() + } + + @Test + fun `element scheme with call host and url param gets url extracted`() { + val embeddedUrl = "https://call.element.io/some-actual-call?with=parameters" + val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") + val url = "element://call?url=$encodedUrl" assertThat(callIntentDataParser.parse(url)).isEqualTo(embeddedUrl) } @Test - fun `element scheme 2 with url param gets url extracted`() { + fun `element scheme 2 with url param with http returns null`() { val embeddedUrl = "http://call.element.io/some-actual-call?with=parameters" val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") val url = "io.element.call:/?url=$encodedUrl" + assertThat(callIntentDataParser.parse(url)).isNull() + } + + @Test + fun `element scheme 2 with url param gets url extracted`() { + val embeddedUrl = "https://call.element.io/some-actual-call?with=parameters" + val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") + val url = "io.element.call:/?url=$encodedUrl" assertThat(callIntentDataParser.parse(url)).isEqualTo(embeddedUrl) } @@ -101,7 +121,7 @@ class CallIntentDataParserTests { @Test fun `element scheme 2 with no url returns null`() { - val embeddedUrl = "http://call.element.io/some-actual-call?with=parameters" + val embeddedUrl = "https://call.element.io/some-actual-call?with=parameters" val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") val url = "io.element.call:/?no_url=$encodedUrl" assertThat(callIntentDataParser.parse(url)).isNull() @@ -109,7 +129,7 @@ class CallIntentDataParserTests { @Test fun `element scheme with no call host returns null`() { - val embeddedUrl = "http://call.element.io/some-actual-call?with=parameters" + val embeddedUrl = "https://call.element.io/some-actual-call?with=parameters" val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") val url = "element://no-call?url=$encodedUrl" assertThat(callIntentDataParser.parse(url)).isNull() @@ -129,7 +149,7 @@ class CallIntentDataParserTests { @Test fun `element invalid scheme returns null`() { - val embeddedUrl = "http://call.element.io/some-actual-call?with=parameters" + val embeddedUrl = "https://call.element.io/some-actual-call?with=parameters" val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") val url = "bad.scheme:/?url=$encodedUrl" assertThat(callIntentDataParser.parse(url)).isNull() From 5ae265a94d93f7b061830c196c7e8cce0c75ac24 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 26 Sep 2023 16:19:38 +0200 Subject: [PATCH 2/4] Element call: Pass in custom call URL query parameters --- .../features/call/CallIntentDataParser.kt | 33 +++++++++--- .../call/CallIntentDataParserTests.kt | 50 +++++++++++++++---- 2 files changed, 67 insertions(+), 16 deletions(-) diff --git a/features/call/src/main/kotlin/io/element/android/features/call/CallIntentDataParser.kt b/features/call/src/main/kotlin/io/element/android/features/call/CallIntentDataParser.kt index f1c4291526..5992ccbc75 100644 --- a/features/call/src/main/kotlin/io/element/android/features/call/CallIntentDataParser.kt +++ b/features/call/src/main/kotlin/io/element/android/features/call/CallIntentDataParser.kt @@ -27,7 +27,7 @@ class CallIntentDataParser @Inject constructor() { val parsedUrl = data?.let { Uri.parse(data) } ?: return null val scheme = parsedUrl.scheme return when { - scheme in validHttpSchemes && parsedUrl.host == "call.element.io" -> data + scheme in validHttpSchemes && parsedUrl.host == "call.element.io" -> parsedUrl scheme == "element" && parsedUrl.host == "call" -> { // We use this custom scheme to load arbitrary URLs for other instances of Element Call, // so we can only verify it's an HTTP/HTTPs URL with a non-empty host @@ -40,14 +40,35 @@ class CallIntentDataParser @Inject constructor() { } // This should never be possible, but we still need to take into account the possibility else -> null - } + }?.withCustomParameters() } - private fun Uri.getUrlParameter(): String? { + private fun Uri.getUrlParameter(): Uri? { return getQueryParameter("url") - ?.takeIf { - val internalUri = Uri.parse(it) - internalUri.scheme in validHttpSchemes && !internalUri.host.isNullOrBlank() + ?.let { urlParameter -> + Uri.parse(urlParameter).takeIf { uri -> + uri.scheme in validHttpSchemes && !uri.host.isNullOrBlank() + } } } } + +/** + * Ensure the uri has the following parameters and value: + * - appPrompt=false + * - confineToRoom=true + */ +private fun Uri.withCustomParameters(): String { + val builder = buildUpon() + builder.clearQuery() + queryParameterNames.forEach { + if (it == APP_PROMPT_PARAMETER || it == CONFINE_TO_ROOM_PARAMETER) return@forEach + builder.appendQueryParameter(it, getQueryParameter(it)) + } + builder.appendQueryParameter(APP_PROMPT_PARAMETER, "false") + builder.appendQueryParameter(CONFINE_TO_ROOM_PARAMETER, "true") + return builder.build().toString() +} + +private const val APP_PROMPT_PARAMETER = "appPrompt" +private const val CONFINE_TO_ROOM_PARAMETER = "confineToRoom" diff --git a/features/call/src/test/kotlin/io/element/android/features/call/CallIntentDataParserTests.kt b/features/call/src/test/kotlin/io/element/android/features/call/CallIntentDataParserTests.kt index 4d5beef7e9..aee97ed982 100644 --- a/features/call/src/test/kotlin/io/element/android/features/call/CallIntentDataParserTests.kt +++ b/features/call/src/test/kotlin/io/element/android/features/call/CallIntentDataParserTests.kt @@ -62,9 +62,9 @@ class CallIntentDataParserTests { @Test fun `Element Call urls will be returned as is`() { val httpsBaseUrl = "https://call.element.io" - val httpsCallUrl = "https://call.element.io/some-actual-call?with=parameters" - assertThat(callIntentDataParser.parse(httpsBaseUrl)).isEqualTo(httpsBaseUrl) - assertThat(callIntentDataParser.parse(httpsCallUrl)).isEqualTo(httpsCallUrl) + val httpsCallUrl = VALID_CALL_URL_WITH_PARAM + assertThat(callIntentDataParser.parse(httpsBaseUrl)).isEqualTo("$httpsBaseUrl?$EXTRA_PARAMS") + assertThat(callIntentDataParser.parse(httpsCallUrl)).isEqualTo("$httpsCallUrl&$EXTRA_PARAMS") } @Test @@ -89,10 +89,10 @@ class CallIntentDataParserTests { @Test fun `element scheme with call host and url param gets url extracted`() { - val embeddedUrl = "https://call.element.io/some-actual-call?with=parameters" + val embeddedUrl = VALID_CALL_URL_WITH_PARAM val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") val url = "element://call?url=$encodedUrl" - assertThat(callIntentDataParser.parse(url)).isEqualTo(embeddedUrl) + assertThat(callIntentDataParser.parse(url)).isEqualTo("$VALID_CALL_URL_WITH_PARAM&$EXTRA_PARAMS") } @Test @@ -105,10 +105,10 @@ class CallIntentDataParserTests { @Test fun `element scheme 2 with url param gets url extracted`() { - val embeddedUrl = "https://call.element.io/some-actual-call?with=parameters" + val embeddedUrl = VALID_CALL_URL_WITH_PARAM val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") val url = "io.element.call:/?url=$encodedUrl" - assertThat(callIntentDataParser.parse(url)).isEqualTo(embeddedUrl) + assertThat(callIntentDataParser.parse(url)).isEqualTo("$VALID_CALL_URL_WITH_PARAM&$EXTRA_PARAMS") } @Test @@ -121,7 +121,7 @@ class CallIntentDataParserTests { @Test fun `element scheme 2 with no url returns null`() { - val embeddedUrl = "https://call.element.io/some-actual-call?with=parameters" + val embeddedUrl = VALID_CALL_URL_WITH_PARAM val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") val url = "io.element.call:/?no_url=$encodedUrl" assertThat(callIntentDataParser.parse(url)).isNull() @@ -129,7 +129,7 @@ class CallIntentDataParserTests { @Test fun `element scheme with no call host returns null`() { - val embeddedUrl = "https://call.element.io/some-actual-call?with=parameters" + val embeddedUrl = VALID_CALL_URL_WITH_PARAM val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") val url = "element://no-call?url=$encodedUrl" assertThat(callIntentDataParser.parse(url)).isNull() @@ -149,9 +149,39 @@ class CallIntentDataParserTests { @Test fun `element invalid scheme returns null`() { - val embeddedUrl = "https://call.element.io/some-actual-call?with=parameters" + val embeddedUrl = VALID_CALL_URL_WITH_PARAM val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") val url = "bad.scheme:/?url=$encodedUrl" assertThat(callIntentDataParser.parse(url)).isNull() } + + @Test + fun `element scheme 2 with url extra param appPrompt gets url extracted`() { + val embeddedUrl = "${VALID_CALL_URL_WITH_PARAM}&appPrompt=true" + val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") + val url = "io.element.call:/?url=$encodedUrl" + assertThat(callIntentDataParser.parse(url)).isEqualTo("$VALID_CALL_URL_WITH_PARAM&$EXTRA_PARAMS") + } + + @Test + fun `element scheme 2 with url extra param confineToRoom gets url extracted`() { + val embeddedUrl = "${VALID_CALL_URL_WITH_PARAM}&confineToRoom=false" + val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") + val url = "io.element.call:/?url=$encodedUrl" + assertThat(callIntentDataParser.parse(url)).isEqualTo("$VALID_CALL_URL_WITH_PARAM&$EXTRA_PARAMS") + } + + @Test + fun `element scheme 2 with url fragment gets url extracted`() { + val embeddedUrl = "${VALID_CALL_URL_WITH_PARAM}#fragment" + val encodedUrl = URLEncoder.encode(embeddedUrl, "utf-8") + val url = "io.element.call:/?url=$encodedUrl" + assertThat(callIntentDataParser.parse(url)).isEqualTo("$VALID_CALL_URL_WITH_PARAM&$EXTRA_PARAMS#fragment") + } + + + companion object { + const val VALID_CALL_URL_WITH_PARAM = "https://call.element.io/some-actual-call?with=parameters" + const val EXTRA_PARAMS = "appPrompt=false&confineToRoom=true" + } } From b6d94830b00a3bd84d29380617c51cf2b73aea44 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 26 Sep 2023 16:22:41 +0200 Subject: [PATCH 3/4] Changelog --- changelog.d/1434.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/1434.misc diff --git a/changelog.d/1434.misc b/changelog.d/1434.misc new file mode 100644 index 0000000000..8babe4771a --- /dev/null +++ b/changelog.d/1434.misc @@ -0,0 +1 @@ +Element call: add custom parameters to Element Call urls. From d4b83cfd454e0dd746f66865186c406bf98924f7 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 26 Sep 2023 16:48:13 +0200 Subject: [PATCH 4/4] Detekt: sentences must end with a period. --- .../io/element/android/features/call/CallIntentDataParser.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/features/call/src/main/kotlin/io/element/android/features/call/CallIntentDataParser.kt b/features/call/src/main/kotlin/io/element/android/features/call/CallIntentDataParser.kt index 5992ccbc75..b3e2f9227a 100644 --- a/features/call/src/main/kotlin/io/element/android/features/call/CallIntentDataParser.kt +++ b/features/call/src/main/kotlin/io/element/android/features/call/CallIntentDataParser.kt @@ -57,6 +57,7 @@ class CallIntentDataParser @Inject constructor() { * Ensure the uri has the following parameters and value: * - appPrompt=false * - confineToRoom=true + * to ensure that the rendering will bo correct on the embedded Webview. */ private fun Uri.withCustomParameters(): String { val builder = buildUpon()