Skip to content

Commit 63ecadd

Browse files
adalpariclaude
adalpari
and
claude
authored
Fix app-password login aborting when apiRootUrl was recovered (#23132)
* Fix app-password login aborting when apiRootUrl was recovered The recovered apiRootUrl was dropped between the helper and the ViewModel, causing valid logins to fail with empty_fetch_params and file it as a Sentry bug. Carry the effective login on SiteNotFound and stop reporting the user-recoverable empty_fetch_params condition to Sentry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Model reachable empty_fetch_params state in SiteNotFound test The empty_fetch_params branch is only reachable via an empty-string siteUrl: storeApplicationPasswordCredentialsFrom rejects an empty apiRootUrl as BadData before it can return SiteNotFound, but only guards siteUrl against null, so an empty string slips through to fetchSites. Update the test fixture accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent dfbc39f commit 63ecadd

4 files changed

Lines changed: 105 additions & 17 deletions

File tree

‎WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class ApplicationPasswordLoginViewModel @Inject constructor(
8989
val urlLogin = applicationPasswordLoginHelper.getSiteUrlLoginFromRawData(rawData)
9090
currentUrlLogin = urlLogin
9191
// Store credentials if the site already exists
92-
when (storeCredentials(urlLogin)) {
92+
when (val storeResult = storeCredentials(urlLogin)) {
9393
is StoreCredentialsResult.Success -> {
9494
_onFinishedEvent.emit(
9595
NavigationActionData(
@@ -102,11 +102,14 @@ class ApplicationPasswordLoginViewModel @Inject constructor(
102102
}
103103
is StoreCredentialsResult.SiteNotFound -> {
104104
waitingForFetchedSite = true
105+
// Use the effective login returned by the helper: it carries any apiRootUrl
106+
// recovered via fallback discovery, which the original urlLogin may still lack.
107+
val effectiveLogin = storeResult.urlLogin
105108
fetchSites(
106-
urlLogin.user.orEmpty(),
107-
urlLogin.password.orEmpty(),
108-
urlLogin.siteUrl.orEmpty(),
109-
urlLogin.apiRootUrl.orEmpty()
109+
effectiveLogin.user.orEmpty(),
110+
effectiveLogin.password.orEmpty(),
111+
effectiveLogin.siteUrl.orEmpty(),
112+
effectiveLogin.apiRootUrl.orEmpty()
110113
)
111114
}
112115
is StoreCredentialsResult.BadData -> {
@@ -186,9 +189,12 @@ class ApplicationPasswordLoginViewModel @Inject constructor(
186189
applicationPasswordLoginHelper.trackStoringFailed(
187190
siteUrl, "empty_fetch_params", creationSource
188191
)
192+
// User-recoverable data condition (missing callback params), not a bug — the
193+
// reason is preserved in analytics + AppLog above, so don't report it to Sentry.
189194
emitError(
190195
siteUrl = siteUrl,
191-
errorMessage = "empty_fetch_params"
196+
errorMessage = "empty_fetch_params",
197+
reportToSentry = false,
192198
)
193199
} else {
194200
discoverAndDispatchFetchSite(

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,9 @@ class ApplicationPasswordLoginHelper @Inject constructor(
102102

103103
sealed class StoreCredentialsResult {
104104
object Success : StoreCredentialsResult()
105-
object SiteNotFound : StoreCredentialsResult()
105+
// Carries the effective login (with any recovered apiRootUrl) so the caller can fetch
106+
// the site with valid params instead of the original, possibly-incomplete, urlLogin.
107+
data class SiteNotFound(val urlLogin: UriLogin) : StoreCredentialsResult()
106108
object BadData : StoreCredentialsResult()
107109
}
108110

@@ -156,7 +158,7 @@ class ApplicationPasswordLoginHelper @Inject constructor(
156158
StoreCredentialsResult.Success
157159
} else {
158160
logSiteNotFound(effectiveUrlLogin.siteUrl, normalizedUrl, sites)
159-
StoreCredentialsResult.SiteNotFound
161+
StoreCredentialsResult.SiteNotFound(effectiveUrlLogin)
160162
}
161163
}
162164
}

‎WordPress/src/test/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModelTest.kt‎

Lines changed: 70 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() {
201201
errorMessage = null
202202
)
203203
whenever(applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(eq(urlLogin), any()))
204-
.thenReturn(StoreCredentialsResult.SiteNotFound)
204+
.thenReturn(StoreCredentialsResult.SiteNotFound(urlLogin))
205205
whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(any())).thenThrow(RuntimeException())
206206

207207
// When
@@ -224,7 +224,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() {
224224
whenever(
225225
applicationPasswordLoginHelper
226226
.storeApplicationPasswordCredentialsFrom(eq(urlLogin), any())
227-
).thenReturn(StoreCredentialsResult.SiteNotFound)
227+
).thenReturn(StoreCredentialsResult.SiteNotFound(urlLogin))
228228
whenever(
229229
selfHostedEndpointFinder
230230
.verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!)
@@ -266,7 +266,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() {
266266
whenever(siteStore.hasSite()).thenReturn(true)
267267
whenever(siteStore.sites).thenReturn(listOf(testSite))
268268
whenever(applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(eq(urlLogin), any()))
269-
.thenReturn(StoreCredentialsResult.SiteNotFound)
269+
.thenReturn(StoreCredentialsResult.SiteNotFound(urlLogin))
270270
whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!))
271271
.thenReturn(xmlRpcEndpoint)
272272

@@ -304,7 +304,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() {
304304
whenever(siteStore.hasSite()).thenReturn(false)
305305
whenever(siteStore.sites).thenReturn(listOf(testSite))
306306
whenever(applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(eq(urlLogin), any()))
307-
.thenReturn(StoreCredentialsResult.SiteNotFound)
307+
.thenReturn(StoreCredentialsResult.SiteNotFound(urlLogin))
308308
whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!))
309309
.thenReturn(xmlRpcEndpoint)
310310

@@ -344,7 +344,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() {
344344
whenever(
345345
applicationPasswordLoginHelper
346346
.storeApplicationPasswordCredentialsFrom(eq(urlLogin), any())
347-
).thenReturn(StoreCredentialsResult.SiteNotFound)
347+
).thenReturn(StoreCredentialsResult.SiteNotFound(urlLogin))
348348
whenever(
349349
selfHostedEndpointFinder
350350
.verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!)
@@ -594,7 +594,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() {
594594
whenever(
595595
applicationPasswordLoginHelper
596596
.storeApplicationPasswordCredentialsFrom(eq(urlLogin), any())
597-
).thenReturn(StoreCredentialsResult.SiteNotFound)
597+
).thenReturn(StoreCredentialsResult.SiteNotFound(urlLogin))
598598
whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(any()))
599599
.thenThrow(RuntimeException("wrapped", IOException("offline")))
600600

@@ -626,7 +626,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() {
626626
whenever(
627627
applicationPasswordLoginHelper
628628
.storeApplicationPasswordCredentialsFrom(eq(urlLogin), any())
629-
).thenReturn(StoreCredentialsResult.SiteNotFound)
629+
).thenReturn(StoreCredentialsResult.SiteNotFound(urlLogin))
630630
whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(any()))
631631
.thenReturn("https://example.com/xmlrpc.php")
632632

@@ -656,7 +656,7 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() {
656656
.storeApplicationPasswordCredentialsFrom(
657657
eq(urlLogin), any()
658658
)
659-
).thenReturn(StoreCredentialsResult.SiteNotFound)
659+
).thenReturn(StoreCredentialsResult.SiteNotFound(urlLogin))
660660
whenever(
661661
selfHostedEndpointFinder
662662
.verifyOrDiscoverXMLRPCEndpoint(any())
@@ -679,12 +679,73 @@ class ApplicationPasswordLoginViewModelTest : BaseUnitTest() {
679679
}
680680
}
681681

682+
@Test
683+
fun `given SiteNotFound with recovered apiRootUrl, then fetchSites uses recovered login and dispatches`() =
684+
runTest {
685+
// Given — the parsed login is missing apiRootUrl (cache miss), but the helper
686+
// recovers it and returns it via SiteNotFound. The ViewModel must fetch with the
687+
// recovered login, not the original empty one, otherwise it aborts with empty_fetch_params.
688+
val parsedLogin = urlLogin.copy(apiRootUrl = "")
689+
val recoveredLogin = urlLogin.copy(apiRootUrl = "https://example.com/json")
690+
whenever(applicationPasswordLoginHelper.getSiteUrlLoginFromRawData(rawData))
691+
.thenReturn(parsedLogin)
692+
whenever(
693+
applicationPasswordLoginHelper
694+
.storeApplicationPasswordCredentialsFrom(eq(parsedLogin), any())
695+
).thenReturn(StoreCredentialsResult.SiteNotFound(recoveredLogin))
696+
whenever(selfHostedEndpointFinder.verifyOrDiscoverXMLRPCEndpoint(any()))
697+
.thenReturn("https://example.com/xmlrpc.php")
698+
699+
// When
700+
viewModel.onFinishedEvent.test {
701+
viewModel.setupSite(rawData)
702+
703+
// Then — no error emitted, fetch is dispatched with the recovered params
704+
expectNoEvents()
705+
verify(dispatcher, times(1)).dispatch(
706+
argThat {
707+
type == SiteAction.FETCH_SITES_XML_RPC_FROM_APPLICATION_PASSWORD
708+
}
709+
)
710+
verify(crashLogging, never()).sendReport(any(), any(), any())
711+
cancelAndIgnoreRemainingEvents()
712+
}
713+
}
714+
715+
@Test
716+
fun `given SiteNotFound with empty fetch params, then emit error without Sentry report`() =
717+
runTest {
718+
// Given — helper returns a login with an empty siteUrl, so fetchSites can't proceed.
719+
// (An empty apiRootUrl would be rejected as BadData before ever reaching SiteNotFound;
720+
// siteUrl is only guarded against null in the helper, so an empty string slips through.)
721+
val incompleteLogin = urlLogin.copy(siteUrl = "")
722+
whenever(applicationPasswordLoginHelper.getSiteUrlLoginFromRawData(rawData))
723+
.thenReturn(incompleteLogin)
724+
whenever(
725+
applicationPasswordLoginHelper
726+
.storeApplicationPasswordCredentialsFrom(eq(incompleteLogin), any())
727+
).thenReturn(StoreCredentialsResult.SiteNotFound(incompleteLogin))
728+
729+
// When
730+
viewModel.onFinishedEvent.test {
731+
viewModel.setupSite(rawData)
732+
733+
// Then — user-recoverable data condition surfaces in UI but is not noised into Sentry
734+
val result = awaitItem()
735+
assertTrue(result.isError)
736+
assertEquals("empty_fetch_params", result.errorMessage)
737+
verify(crashLogging, never()).sendReport(any(), any(), any())
738+
verify(selfHostedEndpointFinder, never()).verifyOrDiscoverXMLRPCEndpoint(any())
739+
cancelAndIgnoreRemainingEvents()
740+
}
741+
}
742+
682743
private suspend fun setupFetchSitesFlow() {
683744
val xmlRpcEndpoint = "https://example.com/xmlrpc.php"
684745
whenever(
685746
applicationPasswordLoginHelper
686747
.storeApplicationPasswordCredentialsFrom(eq(urlLogin), any())
687-
).thenReturn(StoreCredentialsResult.SiteNotFound)
748+
).thenReturn(StoreCredentialsResult.SiteNotFound(urlLogin))
688749
whenever(
689750
selfHostedEndpointFinder
690751
.verifyOrDiscoverXMLRPCEndpoint(urlLogin.siteUrl!!)

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,25 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() {
184184
verify(dispatcherWrapper).updateApplicationPassword(eq(siteModel))
185185
}
186186

187+
@Test
188+
fun `storeApplicationPasswordCredentialsFrom returns SiteNotFound carrying the recovered apiRootUrl`() = runTest {
189+
// Given — apiRootUrl is missing and recovered via discovery, but the site is not found
190+
// locally. The recovered value must be carried on SiteNotFound so the caller can fetch.
191+
val autoDiscoveryAttemptSuccess = AutoDiscoveryAttemptSuccess(
192+
mock(), mock(), mock(), DiscoveredAuthenticationMechanism.ApplicationPasswords(mock())
193+
)
194+
val apiDiscoveryResult = ApiDiscoveryResult.Success(autoDiscoveryAttemptSuccess)
195+
whenever(wpLoginClient.apiDiscovery(any())).thenReturn(apiDiscoveryResult)
196+
whenever(discoverSuccessWrapper.getApiRootUrl(eq(apiDiscoveryResult))).thenReturn(TEST_API_ROOT_URL)
197+
whenever(siteStore.sites).thenReturn(listOf())
198+
199+
val loginWithoutApiRoot = UriLogin(TEST_URL, TEST_USER, TEST_PASSWORD, null)
200+
val result = applicationPasswordLoginHelper.storeApplicationPasswordCredentialsFrom(loginWithoutApiRoot)
201+
202+
assertIs<StoreCredentialsResult.SiteNotFound>(result)
203+
assertEquals(TEST_API_ROOT_URL, result.urlLogin.apiRootUrl)
204+
}
205+
187206
@Test
188207
fun `storeApplicationPasswordCredentialsFrom does not run discovery when apiRootUrl is present`() = runTest {
189208
val siteModel = SiteModel().apply { url = TEST_URL }

0 commit comments

Comments
 (0)