Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions Modules/Sources/WordPressKit/WordPressOrgXMLRPCApi.swift
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,27 @@ open class WordPressOrgXMLRPCApi: NSObject, WordPressOrgXMLRPCApiInterfacing {
let parameters: [AnyObject] = [0 as AnyObject, username as AnyObject, password as AnyObject]
callMethod("wp.getOptions", parameters: parameters, success: success, failure: failure)
}

public func isEnabled(username: String, password: String) async -> Bool {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) I'd suggest naming it isXMLRPCEnabled() for clarity.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I omitted the "XMLRPC" part because the type is WordPressOrgXMLRPCApi.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the logic behind omitting it, but IMHO it's a bit clearer if we re-state it. Alternatively, a good docblock would help here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should be able to emit an Error type that reproduces whatever error the server emits

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this is still needed after the changes in 1a7fde2?

let parameters: [AnyObject] = [0 as AnyObject, username as AnyObject, password as AnyObject]
let result = await call(method: "wp.getOptions", parameters: parameters)
guard case let .failure(error) = result else { return true }
// 405 is a proper fault code that indicates XML-RPC is disabled.
if case let .endpointError(fault) = error, fault.code == 405 {
return false
}
// Some plugins send HTTP 403 Forbidden response.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you an example? It's just 403 usually means "not authorized". What happens if it's a false positive?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "Disable XML-RPC-API" plugin sends 403.

By "false positive", do you mean like the site is temporarily down, or maybe user changes the password on website but not on the app?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, would a false positive also include "the username and password are incorrect"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the username and password are incorrect" will be thrown as an endpointError, with a strong-typed fault: WordPressOrgXMLRPCApiFault.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated this part in 1a7fde2. Let me know what you think.

if error.response?.statusCode == 403 {
return false
}
// Some plugins send HTTP 200 with html or no content at all.
if case let .unparsableResponse(response, _, _) = error,
response?.value(forHTTPHeaderField: "Content-Type")?.hasPrefix("text/xml") == false {
return false
}
return true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) This method seems to return true even if there is a network connection or other unrelated error. It seems to work fine based on how it's used in the UI, but it'd be clearer to make it throwing and ignore errors in the view layer if it needs to.

}

/**
Executes a XMLRPC call for the method specificied with the arguments provided.

Expand Down
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
26.7
-----
* [**] Stats: Add new "Adds" tab to show WordAdds earnings and stats [#25165]
* [*] For sites that are authenticated with application passwords, the app no longer requires xml-rpc to be enabled. But in that scenario, only limited features are available: XML-RPC needs to be enabled to use all features. [#25183]

26.6
-----
Expand Down
87 changes: 80 additions & 7 deletions WordPress/Classes/Login/SelfHostedSiteAuthenticator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,23 @@ struct SelfHostedSiteAuthenticator {
throw .mismatchedUser(expectedUsername: username)
}

let blog = try await fetchSiteDataUsingCoreRESTAPI(credentials: credentials, apiRootURL: apiRootURL, context: context)

switch context {
case .default:
NotificationCenter.default.post(name: Foundation.Notification.Name(rawValue: WordPressAuthenticator.WPSigninDidFinishNotification), object: nil)
case .reauthentication:
NotificationCenter.default.post(name: Self.applicationPasswordUpdated, object: nil)
}

return blog
}

private func fetchSiteDataUsingXMLRPC(
credentials: WpApiApplicationPasswordDetails,
apiRootURL: URL,
context: SignInContext
) async throws(SignInError) -> TaggedManagedObjectID<Blog> {
let xmlrpc: URL = try await discoverXMLRPCEndpoint(site: credentials.siteUrl)
let blogOptions: [AnyHashable: Any]
do {
Expand Down Expand Up @@ -235,13 +252,6 @@ struct SelfHostedSiteAuthenticator {
}
}

switch context {
case .default:
NotificationCenter.default.post(name: Foundation.Notification.Name(rawValue: WordPressAuthenticator.WPSigninDidFinishNotification), object: nil)
case .reauthentication:
NotificationCenter.default.post(name: Self.applicationPasswordUpdated, object: nil)
}

return blog
}

Expand Down Expand Up @@ -272,4 +282,67 @@ struct SelfHostedSiteAuthenticator {
}
}

// This is an alternative to `fetchSiteDataUsingXMLRPC`, without requiring the site's XML-RPC to be enabled.
private func fetchSiteDataUsingCoreRESTAPI(
credentials: WpApiApplicationPasswordDetails,
apiRootURL: URL,
context: SignInContext
) async throws(SignInError) -> TaggedManagedObjectID<Blog> {
let api = WordPressAPI(
urlSession: URLSession(configuration: .ephemeral),
apiRootUrl: try! ParsedUrl.parse(input: apiRootURL.absoluteString),
authentication: WpAuthentication(username: credentials.userLogin, password: credentials.password)
)

let siteTitle: String
let isAdmin: Bool
do {
async let settingsResponse = api.siteSettings.retrieveWithViewContext()
async let userResponse = api.users.retrieveMeWithEditContext()
let (settings, user) = try await (settingsResponse.data, userResponse.data)

isAdmin = user.roles.contains(.administrator)
siteTitle = settings.title
} catch {
throw .loadingSiteInfoFailure
}

// If the XMLRPC is disabled, we'll use the default endpoint.
let xmlrpc = (try? await discoverXMLRPCEndpoint(site: credentials.siteUrl))
?? URL(string: credentials.siteUrl)?.appending(component: "xmlrpc.php")
guard let xmlrpc else {
throw .loadingSiteInfoFailure
}

// FIXME: The XML-RPC version stores `wp.getOptions` result in `Blog.options`, which is used in a few places
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how accurate this is, but I did the CC check for this:

 Based on my search, here's a comprehensive analysis of what getOptionValue is used for and the impact of missing Blog.options:

  Current Usage of getOptionValue in the App

  The Blog.options dictionary (populated via XML-RPC's wp.getOptions) is used for 22 different settings across the codebase:

  Critical Features (High Impact)

  1. Jetpack Integration (BlogSyncFacade.m:69, Blog.m:797)
  - jetpack_user_login - Links Jetpack site to WordPress.com account
  - jetpack_active_modules - Determines which Jetpack features are available
  - jetpack_connection_active_plugins - Identifies active Jetpack plugins

  2. Site Type Detection (Blog.m:166-180)
  - is_wpcom_atomic - Atomic site detection
  - is_wpforteams_site - WP for Teams detection
  - is_automated_transfer - Automated Transfer detection
  - These affect feature availability and UI throughout the app

  3. Media Upload (Blog+Quota.swift:67)
  - max_upload_size - Maximum file upload size
  - Without this, uploads may fail or the app can't show size limits

  4. Image Resizing (Blog.m:400-405)
  - thumbnail_size_w/h, medium_size_w/h, large_size_w/h
  - Used to match site's configured image sizes
  - Fallbacks to defaults if missing

  Moderate Impact

  5. URL Construction (Blog.m:215, 244, 265, 290)
  - home_url - Site home URL (falls back to blog.url)
  - login_url - Login page URL (falls back to /wp-login.php)
  - admin_url - Admin dashboard URL (falls back to /wp-admin/)
  - unmapped_url - For custom domain mapping detection

  6. Post Previews (PreviewNonceHandler.swift:24, 39)
  - frame_nonce - Security nonce for previewing posts
  - unmapped_url - Needed for preview URL rewriting
  - May break post preview functionality

  7. Timezone (Blog.m:307-309)
  - timezone, gmt_offset, time_zone - Site timezone configuration
  - Used for displaying dates/times correctly

  8. Feature Capabilities (Blog.m:488, 493, 622)
  - can_blaze - Blaze promotional campaigns availability
  - post_thumbnail - Featured images support (has fallback)
  - publicize_disabled - Social sharing availability

  9. WordPress Version (Blog.m:429)
  - software_version - WordPress version number
  - May affect feature compatibility checks

  ---
  What's Available from REST API

  The WordPress REST API /wp/v2/settings endpoint provides only 20 fields:
  - ✅ title, description, url, email
  - ✅ timezone, dateFormat, timeFormat, startOfWeek
  - ✅ language, postsPerPage
  - ✅ defaultCategory, defaultPostFormat
  - ✅ useSmilies, showOnFront, pageOnFront, pageForPosts
  - ✅ defaultPingStatus, defaultCommentStatus
  - ✅ siteLogo, siteIcon

  Missing from REST API (18 fields)

  ❌ All Jetpack-related fields
  ❌ All site type detection flags (Atomic, WP for Teams, etc.)
  ❌ All image size settings
  ❌ max_upload_size
  ❌ frame_nonce (for previews)
  ❌ unmapped_url (for custom domains)
  ❌ can_blaze
  ❌ login_url, admin_url, home_url
  ❌ software_version

  ---
  Impact Analysis for This PR

  Features That Will Break

  1. Post Previews - Missing frame_nonce may cause preview failures
  2. Jetpack Features - Can't detect active modules or link to WP.com account
  3. Media Upload Limits - No way to know max upload size
  4. Custom Domain Sites - Missing unmapped_url for URL rewriting

  Features That Will Degrade

  1. Image Resizing - Falls back to hardcoded defaults
  2. Timezone Display - Partial support via REST API timezone field
  3. URL Construction - Falls back to default paths
  4. Blaze - Can't determine availability
  5. Site Type Detection - Can't identify Atomic/WP for Teams sites

  Features That Work

  1. Basic site info (title, description, URL)
  2. WordPress.com sites (don't use XML-RPC options)
  3. Most core functionality that doesn't depend on these settings

  ---
  Recommendations

  Option 1: Fetch Additional Data via REST API
  Some missing fields might be available through other REST API endpoints:
  - Check /wp/v2/media endpoint for upload size limits
  - Check /jetpack/v4/* endpoints for Jetpack data
  - Check site metadata endpoints for version info

  Option 2: Graceful Degradation
  Add fallbacks for critical features:
  - Show warning when previewing posts without nonce
  - Default to conservative upload size limits
  - Disable Jetpack-specific features

  Option 3: Optional XML-RPC
  Allow the app to make individual XML-RPC calls even when XML-RPC is "disabled":
  - Some security plugins block certain methods but not wp.getOptions
  - Try fetching options and fail gracefully if blocked

  Option 4: Feature Documentation
  Document exactly which features are unavailable when XML-RPC is disabled and show this to users in the alert/modal.

  ---
  The FIXME at SelfHostedSiteAuthenticator.swift:317-319 is well-placed - this is a significant limitation that affects multiple features, particularly
  Jetpack integration, media uploads, and post previews.

// in the app. We can't get the same "options" via REST API. We'll need to investigate the impact of missing
// "options".

let blog: TaggedManagedObjectID<Blog>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) should probably be named blogID

do {
blog = try await Blog.createRestApiBlog(
with: credentials,
restApiRootURL: apiRootURL,
xmlrpcEndpointURL: xmlrpc,
blogID: context.blogID,
in: ContextManager.shared
)

try await ContextManager.shared.performAndSave { context in
let blog = try context.existingObject(with: blog)

// Here we'll use the "application password" as the "account password".
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's stored in keychain under the xmlrpc name, which can be a bit confusing.

        [SFHFKeychainUtils storeUsername:self.username
                             andPassword:password
                          forServiceName:self.xmlrpc
                             accessGroup:nil
                          updateExisting:YES
                                   error:nil];

Is it possible to add a new field and store it separately to avoid any confusion in the future?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in fd789fc

blog.password = credentials.password
blog.isAdmin = isAdmin
blog.addSettingsIfNecessary()
blog.settings?.name = siteTitle
}

try await ApplicationPasswordRepository.shared.saveApplicationPassword(of: blog)
} catch {
throw .savingSiteFailure
}

return blog
}
}
1 change: 1 addition & 0 deletions WordPress/Classes/Services/Facades/BlogSyncFacade.m
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ - (void)syncBlogWithUsername:(NSString *)username
blog.url = url;
}
if (blogName) {
[blog addSettingsIfNecessary];
blog.settings.name = [blogName stringByDecodingXMLCharacters];
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public enum FeatureFlag: Int, CaseIterable {
case .allowApplicationPasswords:
return false
case .selfHostedSiteUserManagement:
return false
return true
case .readerGutenbergCommentComposer:
return false
case .pluginManagementOverhaul:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ private struct Section {
tableView.register(JetpackBrandingMenuCardCell.self, forCellReuseIdentifier: CellIdentifiers.jetpackBrandingCard)
tableView.register(JetpackRemoteInstallTableViewCell.self, forCellReuseIdentifier: CellIdentifiers.jetpackInstall)
tableView.register(ExtensiveLoggingCell.self, forCellReuseIdentifier: CellIdentifiers.extensiveLogging)
tableView.register(XMLRPCDisabledCell.self, forCellReuseIdentifier: CellIdentifiers.xmlrpcDisabled)

tableView.delegate = self
tableView.dataSource = self
Expand Down Expand Up @@ -106,6 +107,10 @@ private struct Section {
newSections.append(Section(rows: [], category: .extensiveLogging))
}

if viewController.showXMLRPCDisabled {
newSections.append(Section(rows: [], category: .xmlrpcDisabled))
}

if viewController.isDashboardEnabled() && isSplitViewDisplayed {
newSections.append(buildHomeSection())
}
Expand Down Expand Up @@ -243,7 +248,7 @@ extension BlogDetailsTableViewModel: UITableViewDataSource {
guard section < sections.count else { return 0 }

switch sections[section].category {
case .jetpackInstallCard, .migrationSuccess, .jetpackBrandingCard, .extensiveLogging:
case .jetpackInstallCard, .migrationSuccess, .jetpackBrandingCard, .extensiveLogging, .xmlrpcDisabled:
// The "card" sections do not set the `rows` property. It's hard-coded to show specific types of cards.
wpAssert(sections[section].rows.count == 0)
return 1
Expand All @@ -269,6 +274,8 @@ extension BlogDetailsTableViewModel: UITableViewDataSource {
cell = configureJetpackBrandingCell(tableView: tableView)
case .extensiveLogging:
cell = configureExtensiveLoggingCell(tableView: tableView)
case .xmlrpcDisabled:
cell = configureXMLRPCDisabledCell(tableView: tableView)
default:
if indexPath.row < section.rows.count {
let row = section.rows[indexPath.row]
Expand Down Expand Up @@ -496,6 +503,18 @@ private extension BlogDetailsTableViewModel {
cell.configure(with: viewController)
return cell
}

func configureXMLRPCDisabledCell(tableView: UITableView) -> UITableViewCell {
guard let cell = tableView.dequeueReusableCell(
withIdentifier: CellIdentifiers.xmlrpcDisabled
) as? XMLRPCDisabledCell,
let viewController else {
return UITableViewCell()
}

cell.configure(with: viewController)
return cell
}
}

private extension BlogDetailsTableViewModel {
Expand Down Expand Up @@ -827,6 +846,7 @@ private enum SectionCategory {
case reminders
case domainCredit
case extensiveLogging
case xmlrpcDisabled
case home
case general
case jetpack
Expand Down Expand Up @@ -1493,4 +1513,5 @@ private enum CellIdentifiers {
static let jetpackBrandingCard = "BlogDetailsJetpackBrandingCardCellIdentifier"
static let jetpackInstall = "BlogDetailsJetpackInstallCardCellIdentifier"
static let extensiveLogging = "BlogDetailsExtensiveLoggingCellIdentifier"
static let xmlrpcDisabled = "BlogDetailsXMLRPCDisabledCellIdentifier"
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import UIKit
import WordPressData
import WordPressKit
import WordPressShared
import WordPressUI
import Reachability
Expand All @@ -21,6 +22,7 @@ public class BlogDetailsViewController: UIViewController {

private lazy var blogService = BlogService(coreDataStack: ContextManager.shared)
private var hasLoggedDomainCreditPromptShownEvent = false
private(set) var showXMLRPCDisabled: Bool = false

init(blog: Blog) {
self.blog = blog
Expand Down Expand Up @@ -80,6 +82,7 @@ public class BlogDetailsViewController: UIViewController {
observeManagedObjectContextObjectsDidChangeNotification()
observeGravatarImageUpdate()
downloadGravatarImage()
checkXMLRPCStatus()

registerForTraitChanges([UITraitHorizontalSizeClass.self], action: #selector(handleTraitChanges))
}
Expand Down Expand Up @@ -192,6 +195,24 @@ public class BlogDetailsViewController: UIViewController {
blogService.refreshDomains(for: blog, success: nil, failure: nil)
}

private func checkXMLRPCStatus() {
guard let xmlrpcApi = blog.xmlrpcApi, let username = blog.username, let password = blog.password else {
showXMLRPCDisabled = false
return
}

Task { @MainActor in
let isEnabled = await xmlrpcApi.isEnabled(username: username, password: password)
let wasDisabled = self.showXMLRPCDisabled
self.showXMLRPCDisabled = !isEnabled

if wasDisabled != self.showXMLRPCDisabled {
self.configureTableViewData()
self.reloadTableViewPreservingSelection()
}
}
}

public func showRemoveSiteAlert() {
let model = UIDevice.current.localizedModel
let message = String(format: NSLocalizedString(
Expand Down Expand Up @@ -243,6 +264,7 @@ public class BlogDetailsViewController: UIViewController {
@objc private func handleWillEnterForeground(_ notification: NSNotification) {
configureTableViewData()
reloadTableViewPreservingSelection()
checkXMLRPCStatus()
}

private func observeManagedObjectContextObjectsDidChangeNotification() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import UIKit
import DesignSystem
import SwiftUI
import WordPressUI

class XMLRPCDisabledCell: UITableViewCell {
private weak var presenterViewController: UIViewController?

private lazy var cardView: UIView = {
let view = UIView()
view.translatesAutoresizingMaskIntoConstraints = false
view.backgroundColor = .secondarySystemGroupedBackground
view.layer.masksToBounds = true
view.layer.cornerRadius = DesignConstants.radius(.large)

let content = UIHostingView(view: CardContent())
content.translatesAutoresizingMaskIntoConstraints = false
view.addSubview(content)
view.pinSubviewToAllEdges(content)

view.addGestureRecognizer(UITapGestureRecognizer(target: self, action: #selector(showAlert)))
return view
}()

override init(style: UITableViewCell.CellStyle, reuseIdentifier: String?) {
super.init(style: style, reuseIdentifier: reuseIdentifier)
setupViews()
}

required init?(coder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}

private func setupViews() {
contentView.addSubview(cardView)
contentView.pinSubviewToAllEdges(cardView)
}

func configure(with viewController: BlogDetailsViewController) {
presenterViewController = viewController
}

@objc private func showAlert() {
// TODO: Change to a modal, with more information (i.e. show related plugins)
let alert = UIAlertController(
title: Strings.alertTitle,
message: Strings.alertMessage,
preferredStyle: .alert
)
alert.addAction(UIAlertAction(title: SharedStrings.Button.ok, style: .default))
presenterViewController?.present(alert, animated: true)
}
}

private struct CardContent: View {
var body: some View {
HStack(spacing: 8) {
Circle()
.fill(.orange)
.frame(width: 8, height: 8)

VStack(alignment: .leading) {
Text(Strings.cardTitle)
.font(.subheadline)
.fontWeight(.medium)
Text(Strings.cardSubtitle)
.font(.caption)
.foregroundStyle(.secondary)
}

Spacer()

Image(systemName: "info.circle.fill")
.foregroundStyle(.tertiary)
}
.padding(.horizontal, 12)
.padding(.vertical, 10)
}
}

private enum Strings {
static let cardTitle = NSLocalizedString(
"blogDetails.xmlrpcDisabled.card.title",
value: "XML-RPC Disabled",
comment: "Title for the XML-RPC disabled card on blog details"
)
static let cardSubtitle = NSLocalizedString(
"blogDetails.xmlrpcDisabled.card.subtitle",
value: "Some features may be limited",
comment: "Subtitle for the XML-RPC disabled card on blog details"
)

static let alertTitle = NSLocalizedString(
"blogDetails.xmlrpcDisabled.alert.title",
value: "XML-RPC Disabled",
comment: "Alert title for XML-RPC disabled"
)

static let alertMessage = NSLocalizedString(
"blogDetails.xmlrpcDisabled.alert.message",
value: "XML-RPC is currently unavailable on your site. The app is transitioning to WordPress REST API, but some features still require XML-RPC. You may experience limited functionality until this transition is complete.",
comment: "Alert message explaining that XML-RPC is disabled on the site and some features may be limited"
)
}
Loading