Skip to content

Commit 2945305

Browse files
adalpariclaude
andauthored
Fix reauth prompt persisting after successful re-authentication (#22747)
* Evict stale WpApiClient cache after re-authentication After re-authenticating application passwords, the cached WpApiClient in WpApiClientProvider still held old credentials, causing the reauth banner to persist until the app was killed and restarted. This adds a clearSelfHostedClient() method and calls it after credential storage so subsequent API calls use a fresh client with updated credentials. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove unused removeAllApplicationPasswordCredentials and related code removeAllApplicationPasswordCredentials and getResettableApplicationPasswordSitesCount had no callers in the codebase. Remove them along with hasRegularCredentials and their tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix checkstyle empty line before closing brace in test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Rename clearWpComClients to clearAllClients for accuracy The method clears both wpComClients and selfHostedClients maps but its name only mentioned WP.com. Rename to clearAllClients to reflect its actual behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Evict cached client before dispatching DB update Move clearSelfHostedClient() before updateApplicationPassword() so any concurrent reader gets a cache miss and uses the already-mutated in-memory SiteModel, closing a theoretical race window. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add negative-path verification for clearSelfHostedClient in tests Verify that clearSelfHostedClient is not called on failure paths (empty data, site not found) to make the contract explicit and guard against accidental regressions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove unused removeAllApplicationPasswordCredentials and related code DispatcherWrapper.removeApplicationPassword had no production callers and was only referenced in a negative-path test verification. Remove the dead code and its test assertion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2172865 commit 2945305

4 files changed

Lines changed: 25 additions & 284 deletions

File tree

WordPress/src/main/java/org/wordpress/android/AppInitializer.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,7 @@ class AppInitializer @Inject constructor(
699699

700700
// Clear cached wordpress-rs services and API clients
701701
wpServiceProvider.clearAll()
702-
wpApiClientProvider.clearWpComClients()
702+
wpApiClientProvider.clearAllClients()
703703
}
704704

705705
/*

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

Lines changed: 4 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import org.wordpress.android.analytics.AnalyticsTracker.Stat
1313
import org.wordpress.android.fluxc.Dispatcher
1414
import org.wordpress.android.fluxc.generated.SiteActionBuilder
1515
import org.wordpress.android.fluxc.model.SiteModel
16+
import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider
1617
import org.wordpress.android.fluxc.store.SiteStore
1718
import org.wordpress.android.fluxc.utils.AppLogWrapper
1819
import org.wordpress.android.modules.BG_THREAD
@@ -41,7 +42,8 @@ class ApplicationPasswordLoginHelper @Inject constructor(
4142
private val appLogWrapper: AppLogWrapper,
4243
private val apiRootUrlCache: ApiRootUrlCache,
4344
private val discoverSuccessWrapper: DiscoverSuccessWrapper,
44-
private val crashLogging: CrashLogging
45+
private val crashLogging: CrashLogging,
46+
private val wpApiClientProvider: WpApiClientProvider,
4547
) {
4648
private var processedAppPasswordData: String? = null
4749

@@ -118,6 +120,7 @@ class ApplicationPasswordLoginHelper @Inject constructor(
118120
apiRestPasswordPlain = urlLogin.password
119121
wpApiRestUrl = urlLogin.apiRootUrl
120122
}
123+
wpApiClientProvider.clearSelfHostedClient(site.id)
121124
dispatcherWrapper.updateApplicationPassword(site)
122125
trackSuccessful(urlLogin.siteUrl)
123126
processedAppPasswordData = urlLogin.siteUrl // Save locally to avoid duplicated calls
@@ -209,38 +212,6 @@ class ApplicationPasswordLoginHelper @Inject constructor(
209212
return uriLoginWrapper.parseUriLogin(url)
210213
}
211214

212-
/**
213-
* Removes Application Password credentials for sites that have regular credentials as fallback.
214-
* Sites without regular credentials (username/password) are excluded since they can only
215-
* authenticate using Application Password.
216-
* @return the number of sites that were affected
217-
*/
218-
suspend fun removeAllApplicationPasswordCredentials(): Int {
219-
return withContext(bgDispatcher) {
220-
val sites = siteStore.sites
221-
// Only reset sites that have regular credentials to fall back to
222-
val sitesToReset = sites.filter {
223-
!it.apiRestUsernameEncrypted.isNullOrEmpty() && it.hasRegularCredentials()
224-
}
225-
sitesToReset.forEach { site ->
226-
site.apply {
227-
apiRestUsernamePlain = ""
228-
apiRestPasswordPlain = ""
229-
apiRestUsernameEncrypted = ""
230-
apiRestPasswordEncrypted = ""
231-
apiRestUsernameIV = ""
232-
apiRestPasswordIV = ""
233-
}
234-
dispatcherWrapper.removeApplicationPassword(site)
235-
}
236-
appLogWrapper.d(
237-
AppLog.T.DB,
238-
"A_P: Removed application password credentials for: ${sitesToReset.size} sites"
239-
)
240-
sitesToReset.size
241-
}
242-
}
243-
244215
private fun findSiteByUrl(
245216
normalizedUrl: String?,
246217
sites: List<SiteModel>
@@ -283,20 +254,6 @@ class ApplicationPasswordLoginHelper @Inject constructor(
283254
return url.replaceFirst(host, maskedDomain + tld)
284255
}
285256

286-
private fun SiteModel.hasRegularCredentials(): Boolean {
287-
return !username.isNullOrEmpty() && !password.isNullOrEmpty()
288-
}
289-
290-
/**
291-
* Returns the count of sites with Application Password credentials that can be reset
292-
* because of having regular credentials
293-
*/
294-
fun getResettableApplicationPasswordSitesCount(): Int {
295-
return siteStore.sites.count {
296-
!it.apiRestUsernameEncrypted.isNullOrEmpty() && it.hasRegularCredentials()
297-
}
298-
}
299-
300257
fun siteHasBadCredentials(site: SiteModel) =
301258
site.apiRestUsernamePlain.isNullOrEmpty() || site.apiRestPasswordPlain.isNullOrEmpty()
302259

@@ -358,12 +315,6 @@ class ApplicationPasswordLoginHelper @Inject constructor(
358315
SiteActionBuilder.newUpdateApplicationPasswordAction(site)
359316
)
360317
}
361-
362-
fun removeApplicationPassword(site: SiteModel) {
363-
dispatcher.dispatch(
364-
SiteActionBuilder.newRemoveApplicationPasswordAction(site)
365-
)
366-
}
367318
}
368319

369320
class DiscoverSuccessWrapper @Inject constructor() {

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

Lines changed: 9 additions & 229 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import org.mockito.kotlin.verify
1717
import org.mockito.kotlin.whenever
1818
import org.wordpress.android.BaseUnitTest
1919
import org.wordpress.android.fluxc.model.SiteModel
20+
import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider
2021
import org.wordpress.android.fluxc.store.SiteStore
2122
import org.wordpress.android.fluxc.utils.AppLogWrapper
2223
import org.wordpress.android.ui.accounts.login.ApplicationPasswordLoginHelper.UriLogin
@@ -71,6 +72,9 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() {
7172
@Mock
7273
lateinit var crashLogging: CrashLogging
7374

75+
@Mock
76+
lateinit var wpApiClientProvider: WpApiClientProvider
77+
7478
private lateinit var applicationPasswordLoginHelper: ApplicationPasswordLoginHelper
7579

7680
@Before
@@ -86,7 +90,8 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() {
8690
appLogWrapper,
8791
apiRootUrlCache,
8892
discoverSuccessWrapper,
89-
crashLogging
93+
crashLogging,
94+
wpApiClientProvider
9095
)
9196
}
9297

@@ -96,6 +101,7 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() {
96101
UriLogin("", "", "", "")
97102
)
98103
assertFalse(result)
104+
verify(wpApiClientProvider, times(0)).clearSelfHostedClient(any())
99105
}
100106

101107
@Test
@@ -149,6 +155,7 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() {
149155
assertTrue(result)
150156
verify(siteStore).sites
151157
verify(dispatcherWrapper).updateApplicationPassword(eq(siteModel))
158+
verify(wpApiClientProvider).clearSelfHostedClient(eq(siteModel.id))
152159
}
153160

154161
@Test
@@ -161,7 +168,7 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() {
161168
assertFalse(result)
162169
verify(siteStore).sites
163170
verify(dispatcherWrapper, times(0)).updateApplicationPassword(any())
164-
verify(dispatcherWrapper, times(0)).removeApplicationPassword(any())
171+
verify(wpApiClientProvider, times(0)).clearSelfHostedClient(any())
165172
}
166173

167174
@Test
@@ -332,174 +339,6 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() {
332339
verify(wpLoginClient).apiDiscovery(eq(TEST_URL))
333340
}
334341

335-
@Test
336-
fun `removeAllApplicationPasswordCredentials with no sites completes without errors`() = runTest {
337-
whenever(siteStore.sites).thenReturn(emptyList())
338-
339-
applicationPasswordLoginHelper.removeAllApplicationPasswordCredentials()
340-
341-
verify(siteStore).sites
342-
verify(dispatcherWrapper, times(0)).updateApplicationPassword(any())
343-
verify(dispatcherWrapper, times(0)).removeApplicationPassword(any())
344-
}
345-
346-
@Test
347-
fun `removeAllApplicationPasswordCredentials clears all password fields for site with regular credentials`() =
348-
runTest {
349-
val site = SiteModel().apply {
350-
id = 1
351-
url = TEST_URL
352-
username = "regular_user"
353-
password = "regular_password"
354-
apiRestUsernamePlain = TEST_USER
355-
apiRestPasswordPlain = TEST_PASSWORD
356-
apiRestUsernameEncrypted = "encrypted_user"
357-
apiRestPasswordEncrypted = "encrypted_password"
358-
apiRestUsernameIV = "user_iv"
359-
apiRestPasswordIV = "password_iv"
360-
}
361-
whenever(siteStore.sites).thenReturn(listOf(site))
362-
363-
applicationPasswordLoginHelper.removeAllApplicationPasswordCredentials()
364-
365-
verify(siteStore).sites
366-
verify(dispatcherWrapper).removeApplicationPassword(eq(site))
367-
368-
// Verify all password fields are cleared
369-
assertEquals("", site.apiRestUsernamePlain)
370-
assertEquals("", site.apiRestPasswordPlain)
371-
assertEquals("", site.apiRestUsernameEncrypted)
372-
assertEquals("", site.apiRestPasswordEncrypted)
373-
assertEquals("", site.apiRestUsernameIV)
374-
assertEquals("", site.apiRestPasswordIV)
375-
}
376-
377-
@Test
378-
fun `removeAllApplicationPasswordCredentials only resets sites with regular credentials`() = runTest {
379-
val siteWithRegularCredentials = SiteModel().apply {
380-
id = 1
381-
url = "http://site1.com"
382-
username = "regular_user1"
383-
password = "regular_password1"
384-
apiRestUsernamePlain = "user1"
385-
apiRestPasswordPlain = "password1"
386-
apiRestUsernameEncrypted = "encrypted_user1"
387-
apiRestPasswordEncrypted = "encrypted_password1"
388-
apiRestUsernameIV = "user_iv1"
389-
apiRestPasswordIV = "password_iv1"
390-
}
391-
val siteWithoutRegularCredentials = SiteModel().apply {
392-
id = 2
393-
url = "http://site2.com"
394-
username = ""
395-
password = ""
396-
apiRestUsernamePlain = "user2"
397-
apiRestPasswordPlain = "password2"
398-
apiRestUsernameEncrypted = "encrypted_user2"
399-
apiRestPasswordEncrypted = "encrypted_password2"
400-
apiRestUsernameIV = "user_iv2"
401-
apiRestPasswordIV = "password_iv2"
402-
}
403-
val siteWithNoAppPassword = SiteModel().apply {
404-
id = 3
405-
url = "http://site3.com"
406-
username = "regular_user3"
407-
password = "regular_password3"
408-
// This site has no Application Password credentials set
409-
}
410-
whenever(siteStore.sites).thenReturn(
411-
listOf(siteWithRegularCredentials, siteWithoutRegularCredentials, siteWithNoAppPassword)
412-
)
413-
414-
applicationPasswordLoginHelper.removeAllApplicationPasswordCredentials()
415-
416-
verify(siteStore).sites
417-
// Only the site with regular credentials AND app password should be reset
418-
verify(dispatcherWrapper).removeApplicationPassword(eq(siteWithRegularCredentials))
419-
// Site without regular credentials should NOT be reset
420-
verify(dispatcherWrapper, times(0)).removeApplicationPassword(eq(siteWithoutRegularCredentials))
421-
// Site with no app password encrypted should NOT be reset
422-
verify(dispatcherWrapper, times(0)).removeApplicationPassword(eq(siteWithNoAppPassword))
423-
424-
// Verify password fields are cleared only for site with regular credentials
425-
assertEquals("", siteWithRegularCredentials.apiRestUsernamePlain)
426-
assertEquals("", siteWithRegularCredentials.apiRestUsernameEncrypted)
427-
428-
// Verify password fields are preserved for site without regular credentials
429-
assertEquals("user2", siteWithoutRegularCredentials.apiRestUsernamePlain)
430-
assertEquals("encrypted_user2", siteWithoutRegularCredentials.apiRestUsernameEncrypted)
431-
}
432-
433-
@Test
434-
fun `removeAllApplicationPasswordCredentials preserves other site fields`() = runTest {
435-
val site = SiteModel().apply {
436-
id = 1
437-
url = TEST_URL
438-
name = "Test Site"
439-
description = "Test Description"
440-
siteId = 12345L
441-
username = "regular_user"
442-
password = "regular_password"
443-
apiRestUsernamePlain = TEST_USER
444-
apiRestPasswordPlain = TEST_PASSWORD
445-
apiRestUsernameEncrypted = "encrypted_user"
446-
}
447-
whenever(siteStore.sites).thenReturn(listOf(site))
448-
449-
applicationPasswordLoginHelper.removeAllApplicationPasswordCredentials()
450-
451-
verify(dispatcherWrapper).removeApplicationPassword(eq(site))
452-
453-
// Verify non-password fields are preserved
454-
assertEquals(1, site.id)
455-
assertEquals(TEST_URL, site.url)
456-
assertEquals("Test Site", site.name)
457-
assertEquals("Test Description", site.description)
458-
assertEquals(12345L, site.siteId)
459-
460-
// Verify Application Password fields are cleared
461-
assertEquals("", site.apiRestUsernamePlain)
462-
assertEquals("", site.apiRestPasswordPlain)
463-
464-
// Verify regular credentials are preserved
465-
assertEquals("regular_user", site.username)
466-
assertEquals("regular_password", site.password)
467-
}
468-
469-
@Test
470-
fun `removeAllApplicationPasswordCredentials does not reset site with only username`() = runTest {
471-
val site = SiteModel().apply {
472-
id = 1
473-
url = TEST_URL
474-
username = "regular_user"
475-
password = "" // No password
476-
apiRestUsernameEncrypted = "encrypted_user"
477-
}
478-
whenever(siteStore.sites).thenReturn(listOf(site))
479-
480-
applicationPasswordLoginHelper.removeAllApplicationPasswordCredentials()
481-
482-
verify(dispatcherWrapper, times(0)).removeApplicationPassword(any())
483-
assertEquals("encrypted_user", site.apiRestUsernameEncrypted)
484-
}
485-
486-
@Test
487-
fun `removeAllApplicationPasswordCredentials does not reset site with only password`() = runTest {
488-
val site = SiteModel().apply {
489-
id = 1
490-
url = TEST_URL
491-
username = "" // No username
492-
password = "regular_password"
493-
apiRestUsernameEncrypted = "encrypted_user"
494-
}
495-
whenever(siteStore.sites).thenReturn(listOf(site))
496-
497-
applicationPasswordLoginHelper.removeAllApplicationPasswordCredentials()
498-
499-
verify(dispatcherWrapper, times(0)).removeApplicationPassword(any())
500-
assertEquals("encrypted_user", site.apiRestUsernameEncrypted)
501-
}
502-
503342
@Test
504343
fun `maskUrl with no dot returns url unmasked`() {
505344
val result = applicationPasswordLoginHelper.maskUrl("https://localhost")
@@ -542,63 +381,4 @@ class ApplicationPasswordLoginHelperTest : BaseUnitTest() {
542381
val result = applicationPasswordLoginHelper.maskUrl("https://ab.com")
543382
assertEquals("https://xx.com", result)
544383
}
545-
546-
@Test
547-
fun `getResettableApplicationPasswordSitesCount returns count of sites with regular credentials`() {
548-
val siteWithRegularCredentials = SiteModel().apply {
549-
id = 1
550-
username = "user"
551-
password = "password"
552-
apiRestUsernameEncrypted = "encrypted"
553-
}
554-
val siteWithoutRegularCredentials = SiteModel().apply {
555-
id = 2
556-
username = ""
557-
password = ""
558-
apiRestUsernameEncrypted = "encrypted"
559-
}
560-
val siteWithNoAppPassword = SiteModel().apply {
561-
id = 3
562-
username = "user"
563-
password = "password"
564-
apiRestUsernameEncrypted = ""
565-
}
566-
whenever(siteStore.sites).thenReturn(
567-
listOf(siteWithRegularCredentials, siteWithoutRegularCredentials, siteWithNoAppPassword)
568-
)
569-
570-
val count = applicationPasswordLoginHelper.getResettableApplicationPasswordSitesCount()
571-
572-
assertEquals(1, count)
573-
}
574-
575-
@Test
576-
fun `getResettableApplicationPasswordSitesCount returns zero when no sites have regular credentials`() {
577-
val site = SiteModel().apply {
578-
id = 1
579-
username = ""
580-
password = ""
581-
apiRestUsernameEncrypted = "encrypted"
582-
}
583-
whenever(siteStore.sites).thenReturn(listOf(site))
584-
585-
val count = applicationPasswordLoginHelper.getResettableApplicationPasswordSitesCount()
586-
587-
assertEquals(0, count)
588-
}
589-
590-
@Test
591-
fun `getResettableApplicationPasswordSitesCount returns zero when no sites have app password`() {
592-
val site = SiteModel().apply {
593-
id = 1
594-
username = "user"
595-
password = "password"
596-
apiRestUsernameEncrypted = ""
597-
}
598-
whenever(siteStore.sites).thenReturn(listOf(site))
599-
600-
val count = applicationPasswordLoginHelper.getResettableApplicationPasswordSitesCount()
601-
602-
assertEquals(0, count)
603-
}
604384
}

0 commit comments

Comments
 (0)