Commit 1ba5e7a
authored
Make wpApiRestUrl, xmlRpcUrl, and app-password creds single-writer (#22947)
* Stop full-row site writes from clobbering wpApiRestUrl
On Atomic / Jetpack-WPCom-REST sites a recovered wpApiRestUrl was wiped
to NULL on every app foreground: any full-row insertOrUpdateSite UPDATE
built from a partial in-memory SiteModel (FETCH_SITE/FETCH_SITES, RN,
cookie-nonce) wrote every column, clobbering the healed value.
Exclude WP_API_REST_URL from the generic UpdateAllExceptId mapper so
updateWpApiRestUrl is the sole writer on an existing row, and route every
legitimate writer through the targeted helpers.
Fixes #22905.
* Stop full-row site writes from clobbering app-password credentials
The same full-row insertOrUpdateSite UPDATE that clobbered wpApiRestUrl
also zeroed the application-password credential columns: a credential-less
inbound SiteModel (e.g. a /me/sites sync) overwrote the encrypted
username/password and their IVs with empty values.
Exclude API_REST_USERNAME, API_REST_PASSWORD and their IV columns from the
UpdateAllExceptId mapper (as a set — the IVs are required to decrypt), and
route the credential writers through targeted single-column writers.
Extends the #22905 fix to the credential columns.
* Stop full-row site writes from clobbering xmlRpcUrl
xmlRpcUrl is set either out of band (XML-RPC rediscovery) or by the
WP.com REST sync (meta.links.xmlrpc). A full-row insertOrUpdateSite
UPDATE built from a partial SiteModel that doesn't carry it — e.g. the
WPAPI fetch, which builds a model with a null xmlRpcUrl — wrote that
null over a good value.
Preserve XMLRPC_URL on absence in the generic update path: keep the
stored value when the inbound model has none, persist it when the model
carries one (the WP.com sync, including a changed endpoint after a
domain migration). Add a targeted updateXmlRpcUrl writer for the
single-column rediscovery heal; the recovery flow that calls it lands
separately.
Extends the #22905 fix to XMLRPC_URL.
* Persist rediscovered xmlRpcUrl via a targeted write
attemptXmlRpcRediscovery dispatched newUpdateSiteAction to change one
field, which (with XMLRPC_URL excluded from the full-row mapper) dropped
the value and rewrote ~80 unchanged columns. Add SiteStore.persistXmlRpcUrl
and route rediscovery through it — one targeted write, mirroring the
wpApiRestUrl heal. Drops the now-unused dispatcher from the slice.
* 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.
* Remove dead site writes after the single-writer migration
- The cookie-nonce and React Native 404 handlers reset wpApiRestUrl in
memory to force rediscovery on retry, then persisted via
insertOrUpdateSite/persistSiteSafely — now a no-op for the excluded
column. Drop the dead DB write (keep the in-memory reset), which also
orphaned ReactNativeStore.persistSiteSafely and its injected persist
function; remove those.
- updateSite / createOrUpdateSites copied credentials + wpApiRestUrl from
the DB onto the inbound model before the write; the mapper exclusion
already preserves those columns, so drop the moot copy (editor-prefs
copy stays).
- Rework ReactNativeStoreWPAPITest to mock SiteSqlUtils and assert on
updateWpApiRestUrl (the discover path persists there now).
* Collapse application-password handlers to their targeted writers
After the single-writer migration the full-row write skips the credential and
WP_API_REST_URL columns, so the in-memory mutations and the self-insertOrUpdateSite
in updateApplicationPassword, removeApplicationPassword, and
clearApplicationPasswordColumns persisted nothing — the targeted writers do all the
work. Drop the dead code (keeping the new-site insert in updateApplicationPassword)
and remove the now-unused insertOrUpdateSite stubs from the tests.
* Skip credential decryption when the IV is missing
decryptAPIRestCredentials bailed only when the ciphertext columns were empty; a row
with ciphertext but a blank IV would reach decrypt(ciphertext, "") and throw on read.
Also short-circuit when either IV is empty, treating the malformed row as having no
credentials.
* Test application-password credential writers against a real DB
updateApplicationPasswordCredentials and its URL-keyed variant were
verified only against a mocked SiteSqlUtils, so their ContentValues
mapping never ran. A .first/.second swap, a wrong column, or a dropped
IV would ship uncaught — and the new blank-IV decrypt guard would mask
it as 'no credentials' rather than surfacing it.
EncryptionUtils can't run under Robolectric (AndroidKeyStore), so add a
stubbed-EncryptionUtils SiteSqlUtils and assert the column mapping via
the raw storedSite() read, plus a decrypt round-trip and the ORIGIN_WPAPI
URL-scoping (writes the matching row, leaves a same-url non-WPAPI row
untouched).
* Collapse the app-password persist paths behind one helper
UPDATE_APPLICATION_PASSWORD, the WPAPI app-password fetch, and
app-password XML-RPC login in createOrUpdateSites each duplicated the
read-plain / null-guard / write-credentials-then-wpApiRestUrl logic, with
subtle divergences (rowsAffected reassigned vs not; the URL write nested
under the credentials check in one path, independent in another).
Extract persistAppPasswordColumns(site, persistCredentials,
persistWpApiRestUrl); each caller passes its targeted writer pair (local
id or URL-keyed). wpApiRestUrl stays gated behind credentials on purpose:
a credential-less /me/sites model can be a WP.com simple site whose
getWpApiRestUrl() synthesizes a public-api proxy URL, and gating keeps
that synthetic value out of the DB.
removeApplicationPassword now sums both clear-writes instead of returning
only the wpApiRestUrl count.
* De-duplicate the credential-column writer; tighten a test
updateApplicationPasswordCredentials and clearApplicationPasswordCredentials
hand-wrote the same four-column ContentValues, so the column set lived in
two places — a future column change has to touch both or clear leaves a
stale column. Route both through one private writeApplicationPasswordCredentialColumns.
ApplicationPasswordViewModelSliceTest: the xmlRpc-rediscovery-success
assertion was pre-satisfied by @before seeding siteTest.xmlRpcUrl to the
same value; reset it to null first so the assertion proves rediscovery
assigned it.1 parent a88923a commit 1ba5e7a
10 files changed
Lines changed: 764 additions & 142 deletions
File tree
- WordPress/src
- main/java/org/wordpress/android/ui/mysite/cards/applicationpassword
- test/java/org/wordpress/android/ui/mysite/cards/applicationpassword
- libs/fluxc/src
- main/java/org/wordpress/android/fluxc
- network/rest/wpapi
- persistence
- store
- test/java/org/wordpress/android/fluxc
- persistence
- store
Lines changed: 4 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | | - | |
14 | 12 | | |
15 | 13 | | |
16 | 14 | | |
| |||
41 | 39 | | |
42 | 40 | | |
43 | 41 | | |
44 | | - | |
45 | 42 | | |
46 | 43 | | |
47 | 44 | | |
| |||
257 | 254 | | |
258 | 255 | | |
259 | 256 | | |
260 | | - | |
261 | | - | |
262 | | - | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
263 | 261 | | |
264 | 262 | | |
265 | 263 | | |
| |||
Lines changed: 9 additions & 11 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
22 | | - | |
23 | 22 | | |
24 | 23 | | |
25 | 24 | | |
| |||
71 | 70 | | |
72 | 71 | | |
73 | 72 | | |
74 | | - | |
75 | | - | |
76 | | - | |
77 | 73 | | |
78 | 74 | | |
79 | 75 | | |
| |||
96 | 92 | | |
97 | 93 | | |
98 | 94 | | |
99 | | - | |
100 | 95 | | |
101 | 96 | | |
102 | 97 | | |
| |||
380 | 375 | | |
381 | 376 | | |
382 | 377 | | |
383 | | - | |
| 378 | + | |
384 | 379 | | |
| 380 | + | |
| 381 | + | |
385 | 382 | | |
386 | 383 | | |
387 | 384 | | |
| |||
392 | 389 | | |
393 | 390 | | |
394 | 391 | | |
| 392 | + | |
395 | 393 | | |
396 | 394 | | |
397 | 395 | | |
398 | 396 | | |
399 | | - | |
| 397 | + | |
400 | 398 | | |
401 | 399 | | |
402 | 400 | | |
403 | 401 | | |
404 | | - | |
| 402 | + | |
405 | 403 | | |
406 | 404 | | |
407 | 405 | | |
| |||
421 | 419 | | |
422 | 420 | | |
423 | 421 | | |
424 | | - | |
| 422 | + | |
425 | 423 | | |
426 | 424 | | |
427 | 425 | | |
428 | 426 | | |
429 | | - | |
| 427 | + | |
430 | 428 | | |
431 | 429 | | |
432 | 430 | | |
| |||
441 | 439 | | |
442 | 440 | | |
443 | 441 | | |
444 | | - | |
| 442 | + | |
445 | 443 | | |
446 | 444 | | |
447 | 445 | | |
Lines changed: 7 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
59 | | - | |
60 | | - | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
61 | 64 | | |
62 | 65 | | |
63 | 66 | | |
| |||
71 | 74 | | |
72 | 75 | | |
73 | 76 | | |
74 | | - | |
| 77 | + | |
| 78 | + | |
75 | 79 | | |
76 | | - | |
77 | 80 | | |
78 | 81 | | |
79 | 82 | | |
| |||
Lines changed: 152 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
105 | 105 | | |
106 | 106 | | |
107 | 107 | | |
108 | | - | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
109 | 119 | | |
110 | 120 | | |
111 | 121 | | |
| |||
117 | 127 | | |
118 | 128 | | |
119 | 129 | | |
120 | | - | |
| 130 | + | |
121 | 131 | | |
122 | 132 | | |
123 | 133 | | |
| |||
214 | 224 | | |
215 | 225 | | |
216 | 226 | | |
| 227 | + | |
217 | 228 | | |
218 | | - | |
| 229 | + | |
219 | 230 | | |
220 | 231 | | |
221 | 232 | | |
222 | 233 | | |
223 | 234 | | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
224 | 247 | | |
225 | | - | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
226 | 260 | | |
227 | 261 | | |
228 | 262 | | |
| |||
260 | 294 | | |
261 | 295 | | |
262 | 296 | | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
263 | 302 | | |
264 | 303 | | |
265 | 304 | | |
| |||
270 | 309 | | |
271 | 310 | | |
272 | 311 | | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
273 | 415 | | |
274 | 416 | | |
275 | 417 | | |
| |||
573 | 715 | | |
574 | 716 | | |
575 | 717 | | |
576 | | - | |
| 718 | + | |
577 | 719 | | |
578 | 720 | | |
579 | 721 | | |
580 | 722 | | |
581 | 723 | | |
582 | | - | |
583 | | - | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
| 728 | + | |
584 | 729 | | |
585 | 730 | | |
586 | 731 | | |
| |||
Lines changed: 14 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
13 | 23 | | |
| 24 | + | |
14 | 25 | | |
15 | 26 | | |
16 | 27 | | |
17 | 28 | | |
18 | 29 | | |
19 | 30 | | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
20 | 34 | | |
21 | 35 | | |
22 | 36 | | |
0 commit comments