Skip to content

CMM-1949: Fix Application Password login when XML-RPC is unavailable#22782

Draft
adalpari wants to merge 12 commits intotrunkfrom
fix/CMM-1949-Application-Password-XML-RPC-bug
Draft

CMM-1949: Fix Application Password login when XML-RPC is unavailable#22782
adalpari wants to merge 12 commits intotrunkfrom
fix/CMM-1949-Application-Password-XML-RPC-bug

Conversation

@adalpari
Copy link
Copy Markdown
Contributor

@adalpari adalpari commented Apr 9, 2026

Description

Fixes Application Password login for self-hosted WordPress sites where XML-RPC is disabled or unavailable.

Changes:

  • REST fallback: When XML-RPC discovery fails (DiscoveryException), fall back to fetching the site via WP REST API instead of failing the login flow
  • Credential storage fix: SiteWPAPIRestClient.fetchWPAPISite() now stores credentials in the correct Application Password fields (apiRestUsernamePlain/apiRestPasswordPlain) instead of the XML-RPC username/password fields, which were being persisted to the database incorrectly on subsequent site refreshes
  • Crash prevention: Add WPAPI origin guards in SiteStore.fetchPostFormats(), SiteStore.fetchProfileXmlRpc(), and all 8 CommentsStore methods that route to XML-RPC clients, preventing NullPointerException on null xmlRpcUrl
  • XML-RPC Disabled UI: Show a warning card in MySite when credentials are valid but XML-RPC is unavailable, with a bottom sheet offering "Connect Jetpack" and "Learn more" options
  • Analytics: Add XMLRPC_DISABLED to JetpackConnectionSource for accurate tracking

Testing instructions

REST fallback login:

  1. Set up a self-hosted WordPress site with XML-RPC disabled: https://xmlrpc-disabled-by-plugin.wpmt.co/
  2. Log in using Application Password authentication
  • Verify login completes successfully

XML-RPC disabled warning:

  1. After logging in to a site with XML-RPC disabled, navigate to MySite
  • Verify a red warning card appears indicating XML-RPC is disabled
  1. Tap the warning card
  • Verify a bottom sheet appears with title, body text, a red notice icon, "Connect Jetpack" button, and "Learn more" link
  1. Tap "Connect Jetpack"
  • Verify the Jetpack connection flow starts and the bottom sheet dismisses
  1. Re-open the bottom sheet and tap "Learn more"
  • Verify it opens the support URL in the browser

  • Smoke test the app and notice how the user actions are limited BUT the app does NOT crash

Screenshot 2026-04-09 at 13 47 23 Screenshot 2026-04-09 at 13 48 27

Other regulars logins

Please DO thorough testing here:

  • Verify the app works with regular self-hosted site
  • Verify the app works with a WP account

adalpari and others added 7 commits April 8, 2026 18:59
…ailable

When a self-hosted site has XML-RPC disabled, blocked, or returning HTML,
the credential storage step fails even though authentication succeeded.
This adds a WPAPI fallback path: on DiscoveryException, dispatch a new
FETCH_SITE_WP_API_FROM_APPLICATION_PASSWORD action that fetches the site
via REST and stores credentials in the same format as the XML-RPC path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prevent NullPointerException in fetchPostFormats and fetchProfileXmlRpc
when a WPAPI-origin site has no XML-RPC URL. For WPAPI sites, skip
XML-RPC calls gracefully instead of crashing.

Add an "XML-RPC Disabled" warning card at the top of MySite when a
site has valid Application Password credentials but no XML-RPC endpoint.
Tapping the card opens a bottom sheet with options to connect Jetpack
or learn more.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SiteWPAPIRestClient.fetchWPAPISite() was storing AP credentials in the
XML-RPC username/password fields instead of apiRestUsernamePlain/
apiRestPasswordPlain. On subsequent FETCH_SITE refreshes, updateSite()
preserved existing encrypted AP creds from DB, causing
encryptAPIRestCredentials() to skip re-encryption while the wrong
username/password fields persisted to the database.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add WPAPI origin guards to all CommentsStore methods that route to
CommentsXMLRPCClient, preventing NullPointerException on null xmlRpcUrl.
WPAPI sites now receive an INVALID_INPUT error instead of crashing.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use red warning icon on MySite card for better visibility
- Center card icon vertically
- Use same red warning icon in bottom sheet at smaller size with
  additional vertical spacing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add XMLRPC_DISABLED JetpackConnectionSource for accurate analytics
- Revert shared card layout change; scope icon centering to XML-RPC
  card via centerImageVertically flag on SingleActionCard
- Add CommentsStore WPAPI guard tests (fetchComments,
  fetchCommentsPage, pushComment)
- Make fetchSiteWPAPIFromApplicationPassword internal
- Rename stale test name to reflect actual assertions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dangermattic
Copy link
Copy Markdown
Collaborator

dangermattic commented Apr 9, 2026

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Apr 9, 2026

App Icon📲 You can test the changes from this Pull Request in WordPress Android by scanning the QR code below to install the corresponding build.

App NameWordPress Android
Build TypeDebug
Versionpr22782-81d4f58
Build Number1488
Application IDorg.wordpress.android.prealpha
Commit81d4f58
Installation URL3gdkt8e0r7gf8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Apr 9, 2026

App Icon📲 You can test the changes from this Pull Request in Jetpack Android by scanning the QR code below to install the corresponding build.

App NameJetpack Android
Build TypeDebug
Versionpr22782-81d4f58
Build Number1488
Application IDcom.jetpack.android.prealpha
Commit81d4f58
Installation URL52dn2uhaoj938
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@adalpari
Copy link
Copy Markdown
Contributor Author

adalpari commented Apr 9, 2026

@claude

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 9, 2026

Claude finished @adalpari's task in 4m 28s —— View job


Code Review — CMM-1949: Fix Application Password login when XML-RPC is unavailable

  • Gather context and read changed files
  • Review authentication/login flow changes
  • Review credential storage fix
  • Review crash prevention guards
  • Review XML-RPC disabled UI
  • Review test coverage
  • Post final review

Overall this is a well-structured fix addressing a real crash path and improving the login fallback. A few issues are worth attention before merging.


Bugs / Medium Severity

1. fetchProfileXmlRpc silently swallows the action — hanging any observer

libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt — the new early-return guard:

private fun fetchProfileXmlRpc(site: SiteModel) {
    if (site.xmlRpcUrl.isNullOrEmpty()) {
        AppLog.w(T.API, "fetchProfileXmlRpc: skipping, xmlRpcUrl is null")
        return          // ← FETCHED_PROFILE_XML_RPC is never dispatched
    }
    siteXMLRPCClient.fetchProfile(site)
}

FETCH_PROFILE_XML_RPC is a fire-and-forget dispatch. The store's async machinery dispatches FETCHED_PROFILE_XML_RPC when siteXMLRPCClient.fetchProfile(site) finishes. When the method returns early, that event is never emitted. Any subscriber waiting for FETCHED_PROFILE_XML_RPC (or any UI that needs the profile loaded) will hang silently instead of receiving an error it can handle.

Compare with fetchPostFormats, which correctly returns an empty success payload rather than dropping the result:

site.origin == SiteModel.ORIGIN_WPAPI -> {
    AppLog.w(T.API, "fetchPostFormats: skipping for WPAPI site")
    FetchedPostFormatsPayload(site, emptyList())   // event IS emitted
}

The guard in fetchProfileXmlRpc should emit an error event (or at minimum a no-op success event) rather than returning void. Fix this →


2. waitingForFetchedSite is never reset — stale flag

WordPress/src/main/java/org/wordpress/android/ui/accounts/applicationpassword/ApplicationPasswordLoginViewModel.kt, line 99:

waitingForFetchedSite = true
fetchSites(...)   // on error this emits a NavigationActionData but leaves flag = true

After either the success path (handleSiteChangedSuccess) or the error path (logAndEmitSiteChangedError), the flag stays true. A subsequent onSiteChanged fired from any unrelated store action (e.g. an UPDATE_APPLICATION_PASSWORD dispatch from a background refresh) will be processed by handleSiteChangedSuccess / handleSiteChangedError as if it belongs to the login flow.

In practice the ViewModel is unregistered in onStop, so the window is narrow — but it's easy to close: set waitingForFetchedSite = false at the top of handleSiteChangedSuccess and handleSiteChangedError. Fix this →


Testing Gaps

3. Only 3 of 8 CommentsStore WPAPI guards have new tests

libs/fluxc/src/test/java/org/wordpress/android/fluxc/comments/CommentsStoreTest.kt adds tests for fetchComments, fetchCommentsPage, and pushComment. The WPAPI guard is also applied to:

  • fetchComment
  • createNewComment
  • createNewReply
  • updateEditComment
  • deleteComment

These five methods have no corresponding WPAPI-guard tests. If a regression accidentally removes or misorders those guards, no test will catch it. Each test is a 10-line copy-paste — worth adding for completeness. Fix this →


Minor Issues

4. fetchSiteWPAPIFromApplicationPassword is internal without @VisibleForTesting

libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt, line 1530:

@Suppress("TooGenericExceptionCaught")
internal suspend fun fetchSiteWPAPIFromApplicationPassword(

The method is internal purely to allow testing (vs the production access path through the dispatcher). Other similar methods in the file use @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) to communicate intent. Without the annotation a reader can't tell whether internal is intentional API surface or a test seam.


5. INVALID_INPUT error type is semantically misleading

libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/CommentsStore.kt, in every WPAPI guard:

return CommentsActionPayload(
    CommentError(INVALID_INPUT, XMLRPC_UNAVAILABLE_MSG)
)

INVALID_INPUT implies the caller passed bad arguments. The actual reason here is that the operation is unsupported for WPAPI sites. Consumers inspecting the error type to decide how to recover may incorrectly conclude they provided bad data. If a NOT_SUPPORTED or UNSUPPORTED_OPERATION error type doesn't exist, GENERIC_ERROR is at least semantically neutral. This is a low-severity UX/diagnostics issue, but worth a second look.


6. LEARN_MORE_URL in XmlRpcDisabledBottomSheetFragment is WordPress.com–specific

WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/xmlrpcdisabled/XmlRpcDisabledBottomSheetFragment.kt, line 69:

private const val LEARN_MORE_URL =
    "https://apps.wordpress.com/support/mobile/" +
        "login-signup/" +
        "inaccessible-xml-rpc-connection-error/"

This fragment is shown to Jetpack users too. Please confirm the linked support article is brand-agnostic (covers both apps) or gate the URL behind a flavor check, the same way other support links are handled in the codebase.


What's Well Done

  • The REST fallback approach is clean: catching only DiscoveryException keeps it targeted and lets other exceptions bubble up to the existing error handler.
  • Credential storage fix is correct — apiRestUsernamePlain/apiRestPasswordPlain are now used and a test directly verifies the fields are populated after fetch.
  • The when-based WPAPI guards in CommentsStore are readable and consistent.
  • The SingleActionCard.centerImageVertically addition is non-breaking with a safe default.
  • The onSiteChanged gate with waitingForFetchedSite = true correctly prevents the ViewModel from reacting to unrelated store events — the bug is only that it never resets, not the guard itself.

adalpari and others added 2 commits April 9, 2026 13:02
…ekt LargeClass

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Emit OnProfileFetched error event instead of silently returning in
  fetchProfileXmlRpc when xmlRpcUrl is null
- Reset waitingForFetchedSite flag in both success and error handlers
  to prevent stale flag processing unrelated events
- Add missing WPAPI guard tests for fetchComment, createNewComment,
  createNewReply, updateEditComment, and deleteComment
- Change WPAPI guard error type from INVALID_INPUT to GENERIC_ERROR
  since the issue is unsupported operation, not bad caller input

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 9, 2026

Codecov Report

❌ Patch coverage is 68.24324% with 47 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.44%. Comparing base (fbb252d) to head (81d4f58).
⚠️ Report is 11 commits behind head on trunk.

Files with missing lines Patch % Lines
...ava/org/wordpress/android/fluxc/store/SiteStore.kt 46.51% 22 Missing and 1 partial ⚠️
...ationpassword/ApplicationPasswordViewModelSlice.kt 0.00% 18 Missing ⚠️
...ationpassword/ApplicationPasswordLoginViewModel.kt 92.59% 0 Missing and 2 partials ⚠️
...ordpress/android/ui/mysite/SiteNavigationAction.kt 0.00% 2 Missing ⚠️
...uxc/network/rest/wpapi/site/SiteWPAPIRestClient.kt 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #22782      +/-   ##
==========================================
+ Coverage   37.41%   37.44%   +0.02%     
==========================================
  Files        2321     2321              
  Lines      123775   123878     +103     
  Branches    16804    16821      +17     
==========================================
+ Hits        46312    46381      +69     
- Misses      73747    73780      +33     
- Partials     3716     3717       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@adalpari adalpari marked this pull request as ready for review April 9, 2026 11:53
@adalpari adalpari requested a review from nbradbury April 9, 2026 11:53
@nbradbury
Copy link
Copy Markdown
Contributor

@adalpari Looks like there's a problem with the bottom sheet in landscape:

landscape.mp4

@nbradbury
Copy link
Copy Markdown
Contributor

nbradbury commented Apr 9, 2026

I think we can tighten up the wording in the bottom sheet. The title already says "XML-RPC Disabled" so we don't have to repeat it, and we can remove a few words.

So instead of this:

before

How about this:

after

Also, it feels off to have "Learn more" in both the card and the bottom sheet, but I can't think of an alternative.

@nbradbury
Copy link
Copy Markdown
Contributor

I noticed tapping the "Connect Jetpack" button on the bottom sheet uses the old web-based flow. Is it possible to use the new native one? The native flow is more reliable.

I asked Claude to summarize how to do this.

native-jetpack-connection-pr22782-2026-04-09.pdf

@nbradbury
Copy link
Copy Markdown
Contributor

7. Re-open the bottom sheet and tap "Learn more"

  • Verify it opens the support URL in the browser

Is it possible to use Chrome Custom Tabs to open the URL?

@nbradbury
Copy link
Copy Markdown
Contributor

  • Smoke test the app and notice how the user actions are limited BUT the app does NOT crash

What user actions are limited without XML-RPC?

@adalpari
Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @nbradbury . I'll work on it!
I've also noticed that there's a change that I'm not sure it could break other flow. I'll investigate it and will let you know when the PR is ready for another try.

@adalpari
Copy link
Copy Markdown
Contributor Author

  • Smoke test the app and notice how the user actions are limited BUT the app does NOT crash

What user actions are limited without XML-RPC?

Basically everything that has not been migrated to AP: posting, old post list?, comments, site settings... I'm not aware of the complete list. But the idea is to let the user know that some parts of the app won't work properly but the app will not crash.

@adalpari adalpari marked this pull request as draft April 10, 2026 09:11
adalpari and others added 3 commits April 10, 2026 12:26
…rom card

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… sheet

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add bottom margin to card text when Learn more is hidden
- Wrap bottom sheet content in ScrollView for landscape scrolling
- Expand bottom sheet to STATE_EXPANDED on show

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants