Skip to content

Commit 0f3fbf6

Browse files
jkmasseljkmassel
committed
Persist app-password credentials on XML-RPC login of an existing site
createOrUpdateSites (the XML-RPC app-password login store path) does a full-row write that excludes the credential columns, with no targeted writer to follow up — so re-logging into an already-stored self-hosted site silently dropped the credentials. Add SiteSqlUtils.insertOrUpdateSiteReturningId, which returns the local id of the row it wrote; insertOrUpdateSite becomes a thin rows-affected wrapper over it (behavior unchanged). createOrUpdateSites uses the returned id to persist credentials + wpApiRestUrl via the targeted writers on the exact row.
1 parent 66ada4c commit 0f3fbf6

4 files changed

Lines changed: 83 additions & 13 deletions

File tree

‎libs/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/SiteSqlUtils.kt‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,17 @@ class SiteSqlUtils
105105
.decryptAPIRestCredentials()
106106

107107
/**
108-
* Inserts the given SiteModel into the DB, or updates an existing entry where sites match.
108+
* Inserts or updates [site] and returns the number of rows affected (1 if a row was written, 0
109+
* otherwise). Use [insertOrUpdateSiteReturningId] when you need the local id of the written row.
110+
*/
111+
@Throws(DuplicateSiteException::class)
112+
fun insertOrUpdateSite(site: SiteModel?): Int =
113+
if (insertOrUpdateSiteReturningId(site) != 0) 1 else 0
114+
115+
/**
116+
* Inserts the given SiteModel into the DB, or updates an existing entry where sites match, returning the
117+
* local id of the written row (0 if nothing was written). Returning the id lets callers target that exact
118+
* row with the single-column writers without a second, fragile lookup.
109119
*
110120
* Possible cases:
111121
* 1. Exists in the DB already and matches by local id (simple update) -> UPDATE
@@ -117,7 +127,7 @@ class SiteSqlUtils
117127
*/
118128
@Suppress("LongMethod", "ReturnCount", "ComplexMethod")
119129
@Throws(DuplicateSiteException::class)
120-
fun insertOrUpdateSite(site: SiteModel?): Int {
130+
fun insertOrUpdateSiteReturningId(site: SiteModel?): Int {
121131
if (site == null) {
122132
return 0
123133
}
@@ -214,8 +224,9 @@ class SiteSqlUtils
214224
return if (siteResult.isEmpty()) {
215225
// No site with this local ID, REMOTE_ID + URL, or XMLRPC URL, then insert it
216226
AppLog.d(DB, "Inserting site: " + finalSiteModel.url)
227+
// WellSql back-fills the auto-assigned id onto the model on insert (Identifiable.setId).
217228
WellSql.insert(finalSiteModel).asSingleTransaction(true).execute()
218-
1
229+
finalSiteModel.id
219230
} else {
220231
// Update old site
221232
AppLog.d(DB, "Updating site: " + finalSiteModel.url)
@@ -245,6 +256,7 @@ class SiteSqlUtils
245256
SiteModelTable.API_REST_PASSWORD_IV
246257
)
247258
).execute()
259+
oldId
248260
} catch (e: SQLiteConstraintException) {
249261
AppLog.e(
250262
DB,

‎libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,11 +1836,21 @@ open class SiteStore @Inject constructor(
18361836
site.wpApiRestUrl = siteFromDB.wpApiRestUrl
18371837
}
18381838
}
1839-
val isUpdated = (siteSqlUtils.insertOrUpdateSite(site) == 1)
1840-
if (isUpdated) {
1839+
val localId = siteSqlUtils.insertOrUpdateSiteReturningId(site)
1840+
if (localId != 0) {
18411841
rowsAffected++
18421842
updatedSites.add(site)
18431843
}
1844+
// Credentials + wpApiRestUrl are excluded from the full-row write, so when this site carries
1845+
// app-password creds (XML-RPC app-password login), persist them on the row that was just
1846+
// written (localId) via the targeted writers.
1847+
val apUsername = site.apiRestUsernamePlain
1848+
val apPassword = site.apiRestPasswordPlain
1849+
if (localId != 0 && !apUsername.isNullOrEmpty() && !apPassword.isNullOrEmpty()) {
1850+
siteSqlUtils.updateApplicationPasswordCredentials(localId, apUsername, apPassword)
1851+
site.wpApiRestUrl?.takeIf { it.isNotEmpty() }
1852+
?.let { siteSqlUtils.updateWpApiRestUrl(localId, it) }
1853+
}
18441854
} catch (caughtException: DuplicateSiteException) {
18451855
duplicateSiteFound = true
18461856
}

‎libs/fluxc/src/test/java/org/wordpress/android/fluxc/persistence/SiteSqlUtilsTest.kt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,35 @@ class SiteSqlUtilsTest {
267267
.isEqualTo("https://migrated.test/xmlrpc.php")
268268
}
269269

270+
@Test
271+
fun `insertOrUpdateSiteReturningId returns the matched row id on update`() {
272+
WellSql.insert(SiteModel().apply {
273+
siteId = 42
274+
url = "https://example.test"
275+
}).execute()
276+
val existingId = siteSqlUtils.getSites().single().id
277+
278+
// A fresh inbound model (id = 0) that matches the existing row by SITE_ID + URL.
279+
val returnedId = siteSqlUtils.insertOrUpdateSiteReturningId(SiteModel().apply {
280+
siteId = 42
281+
url = "https://example.test"
282+
name = "Updated"
283+
})
284+
285+
assertThat(returnedId).isEqualTo(existingId)
286+
}
287+
288+
@Test
289+
fun `insertOrUpdateSiteReturningId returns the new row id on insert`() {
290+
val returnedId = siteSqlUtils.insertOrUpdateSiteReturningId(SiteModel().apply {
291+
siteId = 99
292+
url = "https://newsite.test"
293+
})
294+
295+
assertThat(returnedId).isNotEqualTo(0)
296+
assertThat(siteSqlUtils.getSites().single().id).isEqualTo(returnedId)
297+
}
298+
270299
// Raw read that bypasses SiteSqlUtils' decryptAPIRestCredentials, so tests can assert on the stored
271300
// ciphertext columns directly without invoking the AndroidKeyStore-backed EncryptionUtils.
272301
private fun storedSite(): SiteModel = WellSql.select(SiteModel::class.java).asModel.single()

‎libs/fluxc/src/test/java/org/wordpress/android/fluxc/store/SiteStoreTest.kt‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -180,16 +180,16 @@ class SiteStoreTest {
180180
val siteB = SiteModel()
181181
sitesModel.sites = listOf(siteA, siteB)
182182
whenever(siteRestClient.fetchSites(payload.filters, false)).thenReturn(sitesModel)
183-
whenever(siteSqlUtils.insertOrUpdateSite(siteA)).thenReturn(1)
184-
whenever(siteSqlUtils.insertOrUpdateSite(siteB)).thenReturn(1)
183+
whenever(siteSqlUtils.insertOrUpdateSiteReturningId(siteA)).thenReturn(1)
184+
whenever(siteSqlUtils.insertOrUpdateSiteReturningId(siteB)).thenReturn(1)
185185

186186
val onSiteChanged = siteStore.fetchSites(payload)
187187

188188
assertThat(onSiteChanged.rowsAffected).isEqualTo(2)
189189
assertThat(onSiteChanged.error).isNull()
190190
val inOrder = inOrder(siteSqlUtils)
191-
inOrder.verify(siteSqlUtils).insertOrUpdateSite(siteA)
192-
inOrder.verify(siteSqlUtils).insertOrUpdateSite(siteB)
191+
inOrder.verify(siteSqlUtils).insertOrUpdateSiteReturningId(siteA)
192+
inOrder.verify(siteSqlUtils).insertOrUpdateSiteReturningId(siteB)
193193
inOrder.verify(siteSqlUtils).removeWPComRestSitesAbsentFromList(postSqlUtils, sitesModel.sites)
194194
}
195195

@@ -222,8 +222,8 @@ class SiteStoreTest {
222222
sitesModel.sites = listOf(siteA, siteB)
223223
sitesModel.jetpackCPSites = listOf(siteC)
224224
whenever(siteRestClient.fetchSites(payload.filters, false)).thenReturn(sitesModel)
225-
whenever(siteSqlUtils.insertOrUpdateSite(siteA)).thenReturn(1)
226-
whenever(siteSqlUtils.insertOrUpdateSite(siteB)).thenReturn(1)
225+
whenever(siteSqlUtils.insertOrUpdateSiteReturningId(siteA)).thenReturn(1)
226+
whenever(siteSqlUtils.insertOrUpdateSiteReturningId(siteB)).thenReturn(1)
227227

228228
siteStore.fetchSites(payload)
229229

@@ -252,8 +252,8 @@ class SiteStoreTest {
252252
}
253253
sitesModel.sites = listOf(siteA, siteB)
254254
whenever(siteRestClient.fetchSites(payload.filters, false)).thenReturn(sitesModel)
255-
whenever(siteSqlUtils.insertOrUpdateSite(siteA)).thenReturn(1)
256-
whenever(siteSqlUtils.insertOrUpdateSite(siteB)).thenReturn(1)
255+
whenever(siteSqlUtils.insertOrUpdateSiteReturningId(siteA)).thenReturn(1)
256+
whenever(siteSqlUtils.insertOrUpdateSiteReturningId(siteB)).thenReturn(1)
257257

258258
siteStore.fetchSites(payload)
259259

@@ -654,6 +654,25 @@ class SiteStoreTest {
654654
verify(siteSqlUtils).updateXmlRpcUrl(9, "https://selfhosted.test/xmlrpc.php")
655655
}
656656

657+
@Test
658+
fun `createOrUpdateSites persists app-password credentials on the stamped row`() {
659+
val appPwSite = SiteModel().apply {
660+
selfHostedSiteId = 77L
661+
url = "https://selfhosted.test"
662+
xmlRpcUrl = "https://selfhosted.test/xmlrpc.php"
663+
origin = SiteModel.ORIGIN_XMLRPC
664+
apiRestUsernamePlain = "user"
665+
apiRestPasswordPlain = "pass"
666+
wpApiRestUrl = "https://selfhosted.test/wp-json/"
667+
}
668+
whenever(siteSqlUtils.insertOrUpdateSiteReturningId(any())).thenReturn(9)
669+
670+
siteStore.onAction(SiteActionBuilder.newUpdateSitesAction(SitesModel(listOf(appPwSite))))
671+
672+
verify(siteSqlUtils).updateApplicationPasswordCredentials(9, "user", "pass")
673+
verify(siteSqlUtils).updateWpApiRestUrl(9, "https://selfhosted.test/wp-json/")
674+
}
675+
657676
@Test
658677
fun `fetchPostFormats returns empty list for WPAPI site without crashing`() =
659678
test {

0 commit comments

Comments
 (0)