Merge pull request #4160 from vitorpamplona/claude/walletconnect-uri-and-bunker-login

fix: wallet-connect deep links with one colon, and explain why bunker:// cannot sign in

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedOff/login/LoginViewModel.kt

@@ -128,6 +128,21 @@ class LoginViewModel : ViewModel() {
             return false
         }
 
+        // Caught here rather than left to the key parser. Neither of these is a key, so both used
+        // to fall through to the hex branch and come back as "Invalid key: ... Invalid hex
+        // bunker://...", which reads as "you mistyped it" for a string the user pasted correctly.
+        // Amethyst for Android is the bunker, never the bunker's client: the only remote signing it
+        // can persist is an external signer app (see AccountSettings.isWriteable).
+        val trimmedKey = key.text.trim()
+        if (trimmedKey.startsWith("bunker:", ignoreCase = true)) {
+            errorManager.error(R.string.login_bunker_not_supported)
+            return false
+        }
+        if (trimmedKey.startsWith("nostrconnect:", ignoreCase = true)) {
+            errorManager.error(R.string.login_nostrconnect_not_supported)
+            return false
+        }
+
         if (needsPassword && password.text.isBlank()) {
             errorManager.error(R.string.password_is_required)
             return false

amethyst/src/main/res/values/strings.xml

@@ -123,6 +123,8 @@
     <string name="follow_back">Follow back</string>
     <string name="hide_password">Hide Password</string>
     <string name="invalid_key">Invalid key</string>
+    <string name="login_bunker_not_supported">Amethyst for Android can\'t sign in with a bunker:// address. It hands these out so other apps can ask it to sign, but it cannot sign in with one. Use an external signer app, or Amethyst Desktop.</string>
+    <string name="login_nostrconnect_not_supported">That\'s an app asking to connect to your signer, not a way to sign in. Sign in first, then open it from the NIP-46 signer screen.</string>
     <string name="invalid_key_with_message">Invalid key: %1$s</string>
     <string name="password_is_required">Password is required</string>
     <string name="key_is_required">Key is required</string>

amethyst/src/test/java/com/vitorpamplona/amethyst/ui/UriToRouteTest.kt

@@ -93,6 +93,24 @@ class UriToRouteTest {
         )
     }
 
+    @Test
+    fun walletConnectDeepLinksRouteInEverySpellingIsWalletConnectRouteAccepts() {
+        // isWalletConnectRoute() accepts three spellings, so all three have to survive the parse
+        // behind it. They did not: `java.net.URI` calls a scheme followed by anything but `/` an
+        // *opaque* uri and reports it as having no query at all, so the one-colon form lost its
+        // `value=` and came back to the user as "that uri was invalid" -- while the `//` spelling
+        // of the very same link worked. Only the bare form was covered here before.
+        val encoded = URLEncoder.encode(NWC_URI, Charsets.UTF_8.name())
+
+        listOf(
+            "dlnwc?value=$encoded",
+            "amethyst+walletconnect:dlnwc?value=$encoded",
+            "amethyst+walletconnect://dlnwc?value=$encoded",
+        ).forEach { deepLink ->
+            assertEquals(deepLink, Route.WalletAddNwc(NWC_URI), uriToRoute(deepLink, account))
+        }
+    }
+
     @Test
     fun theLauncherShortcutOpensTheScannerDirectly() {
         // res/xml/shortcuts.xml fires `amethyst:scanqr`. If this stops resolving, long-pressing the

amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedOff/login/LoginViewModelKeyKindTest.kt

@@ -0,0 +1,81 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.amethyst.ui.screen.loggedOff.login
+
+import androidx.compose.ui.text.input.TextFieldValue
+import com.vitorpamplona.amethyst.R
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Test
+
+/**
+ * Amethyst for Android is the bunker, never the bunker's client.
+ *
+ * `AccountSettings.isWriteable()` is `privKey != null || externalSignerPackageName != null`: the
+ * only remote signing it can persist is an external signer app. A `bunker://` address pasted into
+ * the key field is therefore not a key it failed to read, it is a sign-in method it does not have
+ * — but it used to fall through to the hex branch and come back as
+ * "Invalid key: Could not sign in: Invalid hex bunker://…", which reads as "you mistyped it" for a
+ * string the user pasted correctly.
+ */
+class LoginViewModelKeyKindTest {
+    private fun viewModelWithKey(text: String) =
+        LoginViewModel().apply {
+            acceptedTerms = true
+            key = TextFieldValue(text)
+        }
+
+    private fun errorResIdOf(model: LoginViewModel) = (model.errorManager.error as? LoginErrorManager.SingleErrorMsg)?.errorResId
+
+    @Test
+    fun aBunkerAddressIsRefusedAsAnUnsupportedMethodNotAsABadKey() {
+        val model = viewModelWithKey("bunker://${"a".repeat(64)}?relay=wss%3A%2F%2Fr.example&secret=s1")
+
+        assertFalse(model.checkCanLogin())
+        assertEquals(R.string.login_bunker_not_supported, errorResIdOf(model))
+    }
+
+    /** Same defect, same field: a connect offer is not a sign-in method either. */
+    @Test
+    fun aNostrConnectOfferIsRefusedWithItsOwnExplanation() {
+        val model = viewModelWithKey("nostrconnect://${"b".repeat(64)}?relay=wss%3A%2F%2Fr.example&secret=s1")
+
+        assertFalse(model.checkCanLogin())
+        assertEquals(R.string.login_nostrconnect_not_supported, errorResIdOf(model))
+    }
+
+    /** Pasted from a chat app, which loves to add a trailing space. */
+    @Test
+    fun surroundingWhitespaceDoesNotHideTheExplanation() {
+        val model = viewModelWithKey("  bunker://${"a".repeat(64)}?secret=s1  ")
+
+        assertFalse(model.checkCanLogin())
+        assertEquals(R.string.login_bunker_not_supported, errorResIdOf(model))
+    }
+
+    /** An npub is still a key, so the guard must not swallow the ordinary path. */
+    @Test
+    fun anOrdinaryKeyStillPassesTheCheck() {
+        val model = viewModelWithKey("npub1${"q".repeat(58)}")
+
+        assertEquals(true, model.checkCanLogin())
+    }
+}

quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/utils/UriParserTest.kt

@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.quartz.utils
+
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNull
+
+/**
+ * The contract every actual of [UriParser] has to keep.
+ *
+ * The interesting case is the *opaque* URI -- a scheme followed by anything but `/`. The JVM and
+ * Android actual delegates to `java.net.URI`, which declares such a URI to have no query at all
+ * and folds `dlnwc?value=...` into one scheme-specific part, while the hand-rolled actuals look
+ * for `?` wherever it sits. That disagreement is what made a wallet-connect deep link report
+ * "that uri was invalid" on Android while the `//` spelling of the same link worked.
+ */
+class UriParserTest {
+    private val nwc = "nostr+walletconnect://abc?relay=wss://r.example&secret=s1"
+
+    @Test
+    fun readsTheQueryOfAnOpaqueUri() {
+        val parser = UriParser("amethyst+walletconnect:dlnwc?value=${encode(nwc)}")
+
+        assertEquals(nwc, parser.getQueryParameter("value")?.firstOrNull())
+    }
+
+    /** The spelling that already worked, so the rescue above cannot have broken it. */
+    @Test
+    fun readsTheQueryOfAHierarchicalUri() {
+        val parser = UriParser("amethyst+walletconnect://dlnwc?value=${encode(nwc)}")
+
+        assertEquals(nwc, parser.getQueryParameter("value")?.firstOrNull())
+    }
+
+    /** No scheme at all, which is how the callback arrives from some wallets. */
+    @Test
+    fun readsTheQueryOfASchemelessUri() {
+        val parser = UriParser("dlnwc?value=${encode(nwc)}")
+
+        assertEquals(nwc, parser.getQueryParameter("value")?.firstOrNull())
+    }
+
+    @Test
+    fun opaqueUriWithoutAQueryHasNoParameters() {
+        val parser = UriParser("cashu:sometoken")
+
+        assertNull(parser.getQueryParameter("value"))
+        assertEquals(emptySet(), parser.queryParameterNames())
+    }
+
+    @Test
+    fun namesEveryParameterOfAnOpaqueUri() {
+        val parser = UriParser("bunker:pubkeyhex?relay=wss%3A%2F%2Fr.example&secret=s1")
+
+        assertEquals(setOf("relay", "secret"), parser.queryParameterNames())
+        assertEquals("wss://r.example", parser.getQueryParameter("relay")?.firstOrNull())
+        assertEquals("s1", parser.getQueryParameter("secret")?.firstOrNull())
+    }
+
+    /** A fragment is parsed off an opaque URI by every actual already; keep it that way. */
+    @Test
+    fun keepsTheFragmentOfAnOpaqueUri() {
+        val parser = UriParser("scheme:thing?a=1#b=2")
+
+        assertEquals("1", parser.getQueryParameter("a")?.firstOrNull())
+        assertEquals(mapOf("b" to "2"), parser.fragments())
+    }
+
+    private fun encode(value: String) =
+        value
+            .replace("%", "%25")
+            .replace("+", "%2B")
+            .replace(":", "%3A")
+            .replace("/", "%2F")
+            .replace("?", "%3F")
+            .replace("&", "%26")
+            .replace("=", "%3D")
+}

quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/utils/UriParser.jvmAndroid.kt

@@ -28,8 +28,28 @@ actual class UriParser actual constructor(
 ) {
     private val myUri = URI.create(uri)
 
+    /**
+     * The query string, including the one [java.net.URI] refuses to find.
+     *
+     * A URI whose scheme is followed by anything other than `/` is *opaque* to `java.net.URI`:
+     * `amethyst+walletconnect:dlnwc?value=...` has no query at all as far as it is concerned, and
+     * the whole `dlnwc?value=...` is one scheme-specific part. Every other actual of this class
+     * looks for `?` wherever it sits, so on JVM and Android alone the parameters of an opaque URI
+     * silently vanished -- which is how a perfectly valid wallet-connect deep link came back as
+     * "that uri was invalid", while the `//` spelling of the very same link worked.
+     *
+     * The fragment needs no such rescue: `java.net.URI` does parse `#...` off an opaque URI.
+     */
+    private val rawQuery: String? =
+        myUri.rawQuery
+            ?: myUri
+                .takeIf { it.isOpaque }
+                ?.rawSchemeSpecificPart
+                ?.substringAfter('?', "")
+                ?.ifBlank { null }
+
     private val queryParameters: Map<String, List<String>> by lazy {
-        myUri.rawQuery?.ifBlank { null }?.let { query ->
+        rawQuery?.ifBlank { null }?.let { query ->
             val params = mutableMapOf<String, MutableList<String>>()
 
             query.split('&').forEach { paramValue ->