Skip to content

Commit 1274181

Browse files
adalpariclaude
adalpari
and
claude
authored
Fix application password site_not_found by adding fallback URL matching (#22724)
* Log available site URLs in application password site_not_found error Include the actual stored site URLs in the diagnostic log and Sentry report to help identify the URL mismatch causing login failures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add fallback URL matching for application password site lookup When the exact normalized URL match fails, retry by stripping the scheme (http/https) and www prefix. This handles cases where the stored site URL and the callback URL differ only in scheme or www. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Cache siteStore.sites to avoid redundant access in site lookup Read siteStore.sites once and pass the list to findSiteByUrl and logAndReportSiteNotFound, avoiding multiple accesses that may create new list instances each time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add tests for fallback URL matching in application password login Cover scheme mismatch (http vs https), www mismatch, combined scheme+www mismatch, and verify exact match takes priority over fallback. Also fix existing test assertion for sites access count. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Mask site URLs in Sentry reports for application password errors Replace the last 3 characters before the TLD with XXX in URLs sent to Sentry to avoid exposing full site URLs. AppLog retains the full URLs for local debugging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Harden maskUrl and findSiteByUrl in ApplicationPasswordLoginHelper - Extract host before masking in maskUrl to avoid incorrect masking when the URL contains dots in the path - Add null/empty guard to findSiteByUrl to prevent false matches when normalizedUrl is null - Add unit tests for maskUrl covering: no dot, standard domain, port, dot in path, and short host cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Refactor findSiteByUrl to fix detekt ReturnCount violation Use elvis operator chaining instead of early return to reduce return statements from 3 to 2. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review feedback for maskUrl and findSiteByUrl - Use replaceFirst instead of replace in maskUrl to avoid masking duplicate host occurrences in the URL path - Import java.net.URI instead of using fully-qualified reference - Add test for empty URL not matching site with empty URL Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Mask site URLs in analytics tracking to prevent PII leakage The trackStoringFailed and trackSuccessful methods were sending raw site URLs to the analytics service. Apply the same maskUrl treatment used for Sentry reports. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update maskUrl to show only first and last character of domain Replace the fixed-length masking with a scheme that preserves only the first and last characters of the domain name, masking everything in between with 'x' for stronger PII protection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Improve maskUrl to handle all domain lengths and clean up - Mask 1-2 char domains entirely with 'x' - Preserve first/last char for 3+ char domains - Fix import ordering for java.net.URI - Rename test to reflect current masking behavior - Add tests for 1-char, 2-char, and 3-char domains - Remove unused MIN_MASK_LENGTH constant Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add @Suppress for ReturnCount in maskUrl Multiple early returns improve readability for parse failure and edge case handling in the masking logic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5c15e26 commit 1274181

File tree

2 files changed

+197
-10
lines changed

2 files changed

+197
-10
lines changed

‎WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt‎

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.wordpress.android.ui.accounts.login
22

33
import android.content.Context
4+
import androidx.annotation.VisibleForTesting
45
import androidx.core.net.toUri
56
import com.automattic.android.tracks.crashlogging.CrashLogging
67
import org.wordpress.android.R
@@ -22,6 +23,7 @@ import org.wordpress.android.util.crashlogging.sendReportWithTag
2223
import rs.wordpress.api.kotlin.ApiDiscoveryResult
2324
import rs.wordpress.api.kotlin.WpLoginClient
2425
import uniffi.wp_api.applicationPasswordsUrl
26+
import java.net.URI
2527
import javax.inject.Inject
2628
import javax.inject.Named
2729

@@ -104,7 +106,8 @@ class ApplicationPasswordLoginHelper @Inject constructor(
104106

105107
return withContext(bgDispatcher) {
106108
val normalizedUrl = UrlUtils.normalizeUrl(urlLogin.siteUrl)
107-
val site = siteStore.sites.firstOrNull { UrlUtils.normalizeUrl(it.url) == normalizedUrl}
109+
val sites = siteStore.sites
110+
val site = findSiteByUrl(normalizedUrl, sites)
108111
if (site != null) {
109112
site.apply {
110113
apiRestUsernameEncrypted = ""
@@ -121,7 +124,7 @@ class ApplicationPasswordLoginHelper @Inject constructor(
121124
true
122125
} else {
123126
logAndReportSiteNotFound(
124-
urlLogin.siteUrl, normalizedUrl
127+
urlLogin.siteUrl, normalizedUrl, sites
125128
)
126129
false
127130
}
@@ -130,7 +133,7 @@ class ApplicationPasswordLoginHelper @Inject constructor(
130133

131134
fun trackStoringFailed(siteUrl: String?, reason: String) {
132135
val properties: MutableMap<String, String?> = HashMap()
133-
properties[URL_TAG] = siteUrl
136+
properties[URL_TAG] = maskUrl(siteUrl.orEmpty())
134137
properties[REASON_TAG] = reason
135138
AnalyticsTracker.track(
136139
Stat.APPLICATION_PASSWORD_STORING_FAILED,
@@ -168,22 +171,28 @@ class ApplicationPasswordLoginHelper @Inject constructor(
168171

169172
private fun logAndReportSiteNotFound(
170173
siteUrl: String?,
171-
normalizedUrl: String?
174+
normalizedUrl: String?,
175+
sites: List<SiteModel>
172176
) {
173-
val detail = "$siteUrl (normalized: $normalizedUrl)" +
174-
" — ${siteStore.sites.size} sites available"
177+
val availableSiteUrls = sites.joinToString { it.url }
178+
val logDetail = "$siteUrl (normalized: $normalizedUrl)" +
179+
" — ${sites.size} sites available: [$availableSiteUrls]"
175180
appLogWrapper.e(
176181
AppLog.T.DB,
177182
"A_P: Cannot save credentials" +
178-
" - site not found: $detail"
183+
" - site not found: $logDetail"
179184
)
180185
trackStoringFailed(siteUrl, "site_not_found")
181-
reportStoringFailedToSentry("site_not_found", detail)
186+
val maskedSiteUrls = sites.joinToString { maskUrl(it.url) }
187+
val sentryDetail =
188+
"${maskUrl(siteUrl.orEmpty())} (normalized: ${maskUrl(normalizedUrl.orEmpty())})" +
189+
" — ${sites.size} sites available: [$maskedSiteUrls]"
190+
reportStoringFailedToSentry("site_not_found", sentryDetail)
182191
}
183192

184193
private fun trackSuccessful(siteUrl: String) {
185194
val properties: MutableMap<String, String?> = HashMap()
186-
properties[URL_TAG] = siteUrl
195+
properties[URL_TAG] = maskUrl(siteUrl)
187196
properties[SUCCESS_TAG] = "true"
188197
AnalyticsTracker.track(
189198
if (buildConfigWrapper.isJetpackApp) {
@@ -232,6 +241,48 @@ class ApplicationPasswordLoginHelper @Inject constructor(
232241
}
233242
}
234243

244+
private fun findSiteByUrl(
245+
normalizedUrl: String?,
246+
sites: List<SiteModel>
247+
): SiteModel? {
248+
if (normalizedUrl.isNullOrEmpty()) return null
249+
250+
// Exact match first, fallback: compare ignoring scheme and www prefix
251+
val strippedUrl = normalizedUrl.stripSchemeAndWww()
252+
return sites.firstOrNull {
253+
UrlUtils.normalizeUrl(it.url) == normalizedUrl
254+
} ?: sites.firstOrNull {
255+
UrlUtils.normalizeUrl(it.url)?.stripSchemeAndWww() == strippedUrl
256+
}
257+
}
258+
259+
private fun String.stripSchemeAndWww(): String {
260+
return removePrefix("https://")
261+
.removePrefix("http://")
262+
.removePrefix("www.")
263+
}
264+
265+
@Suppress("ReturnCount")
266+
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
267+
internal fun maskUrl(url: String): String {
268+
val host = try {
269+
URI(url).host
270+
} catch (_: Exception) {
271+
null
272+
} ?: return url
273+
val dotIndex = host.lastIndexOf('.')
274+
if (dotIndex <= 0) return url
275+
val domain = host.substring(0, dotIndex)
276+
val tld = host.substring(dotIndex)
277+
val maskedDomain = when {
278+
domain.length <= 2 -> "x".repeat(domain.length)
279+
else -> domain.first() +
280+
"x".repeat(domain.length - 2) +
281+
domain.last()
282+
}
283+
return url.replaceFirst(host, maskedDomain + tld)
284+
}
285+
235286
private fun SiteModel.hasRegularCredentials(): Boolean {
236287
return !username.isNullOrEmpty() && !password.isNullOrEmpty()
237288
}

‎WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt‎

Lines changed: 137 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,11 +159,104 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() {
159159
val result = applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(testUriLogin)
160160

161161
assertFalse(result)
162-
verify(siteStore, times(2)).sites
162+
verify(siteStore).sites
163163
verify(dispatcherWrapper, times(0)).updateApplicationPassword(any())
164164
verify(dispatcherWrapper, times(0)).removeApplicationPassword(any())
165165
}
166166

167+
@Test
168+
fun `storeApplicationPasswordCredentialsFrom with empty url does not match site with empty url`() =
169+
runTest {
170+
val siteWithEmptyUrl = SiteModel().apply {
171+
url = ""
172+
}
173+
whenever(siteStore.sites).thenReturn(listOf(siteWithEmptyUrl))
174+
val emptyUrlLogin = UriLogin(
175+
"", TEST_USER, TEST_PASSWORD, TEST_API_ROOT_URL
176+
)
177+
178+
val result = applicationPasswordLoginHelper
179+
.storeApplicationPasswordCredentialsFrom(emptyUrlLogin)
180+
181+
assertFalse(result)
182+
verify(dispatcherWrapper, times(0)).updateApplicationPassword(any())
183+
}
184+
185+
@Test
186+
fun `storeApplicationPasswordCredentialsFrom matches site with different scheme`() =
187+
runTest {
188+
val siteModel = SiteModel().apply {
189+
url = "http://test.com"
190+
}
191+
whenever(siteStore.sites).thenReturn(listOf(siteModel))
192+
val httpsLogin = UriLogin(
193+
"https://test.com", TEST_USER, TEST_PASSWORD, TEST_API_ROOT_URL
194+
)
195+
196+
val result = applicationPasswordLoginHelper
197+
.storeApplicationPasswordCredentialsFrom(httpsLogin)
198+
199+
assertTrue(result)
200+
verify(dispatcherWrapper).updateApplicationPassword(eq(siteModel))
201+
}
202+
203+
@Test
204+
fun `storeApplicationPasswordCredentialsFrom matches site with www mismatch`() =
205+
runTest {
206+
val siteModel = SiteModel().apply {
207+
url = "https://www.test.com"
208+
}
209+
whenever(siteStore.sites).thenReturn(listOf(siteModel))
210+
val noWwwLogin = UriLogin(
211+
"https://test.com", TEST_USER, TEST_PASSWORD, TEST_API_ROOT_URL
212+
)
213+
214+
val result = applicationPasswordLoginHelper
215+
.storeApplicationPasswordCredentialsFrom(noWwwLogin)
216+
217+
assertTrue(result)
218+
verify(dispatcherWrapper).updateApplicationPassword(eq(siteModel))
219+
}
220+
221+
@Test
222+
fun `storeApplicationPasswordCredentialsFrom matches site with scheme and www mismatch`() =
223+
runTest {
224+
val siteModel = SiteModel().apply {
225+
url = "http://www.test.com"
226+
}
227+
whenever(siteStore.sites).thenReturn(listOf(siteModel))
228+
val httpsNoWwwLogin = UriLogin(
229+
"https://test.com", TEST_USER, TEST_PASSWORD, TEST_API_ROOT_URL
230+
)
231+
232+
val result = applicationPasswordLoginHelper
233+
.storeApplicationPasswordCredentialsFrom(httpsNoWwwLogin)
234+
235+
assertTrue(result)
236+
verify(dispatcherWrapper).updateApplicationPassword(eq(siteModel))
237+
}
238+
239+
@Test
240+
fun `storeApplicationPasswordCredentialsFrom prefers exact match over fallback`() =
241+
runTest {
242+
val exactSite = SiteModel().apply {
243+
url = TEST_URL
244+
id = 1
245+
}
246+
val fallbackSite = SiteModel().apply {
247+
url = "https://test.com"
248+
id = 2
249+
}
250+
whenever(siteStore.sites)
251+
.thenReturn(listOf(fallbackSite, exactSite))
252+
253+
val result = applicationPasswordLoginHelper
254+
.storeApplicationPasswordCredentialsFrom(testUriLogin)
255+
256+
assertTrue(result)
257+
verify(dispatcherWrapper).updateApplicationPassword(eq(exactSite))
258+
}
259+
167260
@Test
168261
fun `appendParamsToRestAuthorizationUrl with null authorizationUrl returns empty string`() {
169262
val result = ApplicationPasswordLoginHelper.UriLoginWrapper(context, apiRootUrlCache, buildConfigWrapper)
@@ -407,6 +500,49 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() {
407500
assertEquals("encrypted_user", site.apiRestUsernameEncrypted)
408501
}
409502

503+
@Test
504+
fun `maskUrl with no dot returns url unmasked`() {
505+
val result = applicationPasswordLoginHelper.maskUrl("https://localhost")
506+
assertEquals("https://localhost", result)
507+
}
508+
509+
@Test
510+
fun `maskUrl with standard domain masks middle characters`() {
511+
val result = applicationPasswordLoginHelper.maskUrl("https://example.com")
512+
assertEquals("https://exxxxxe.com", result)
513+
}
514+
515+
@Test
516+
fun `maskUrl with port preserves port`() {
517+
val result = applicationPasswordLoginHelper.maskUrl("https://test.com:8080")
518+
assertEquals("https://txxt.com:8080", result)
519+
}
520+
521+
@Test
522+
fun `maskUrl with dot in path masks only host`() {
523+
val result = applicationPasswordLoginHelper
524+
.maskUrl("https://example.com/wp-content/image.jpg")
525+
assertEquals("https://exxxxxe.com/wp-content/image.jpg", result)
526+
}
527+
528+
@Test
529+
fun `maskUrl with three char domain masks middle character`() {
530+
val result = applicationPasswordLoginHelper.maskUrl("https://abc.com")
531+
assertEquals("https://axc.com", result)
532+
}
533+
534+
@Test
535+
fun `maskUrl with single char domain replaces it with x`() {
536+
val result = applicationPasswordLoginHelper.maskUrl("https://a.com")
537+
assertEquals("https://x.com", result)
538+
}
539+
540+
@Test
541+
fun `maskUrl with two char domain replaces both with x`() {
542+
val result = applicationPasswordLoginHelper.maskUrl("https://ab.com")
543+
assertEquals("https://xx.com", result)
544+
}
545+
410546
@Test
411547
fun `getResettableApplicationPasswordSitesCount returns count of sites with regular credentials`() {
412548
val siteWithRegularCredentials = SiteModel().apply {

0 commit comments

Comments
 (0)
⚡