From ee1a70dd8f254ee98b2b6794958967dc5554ff4b Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 5 Aug 2026 19:45:10 +1200 Subject: [PATCH 1/3] Add insecure connection predicate for self-hosted sign-in --- .../Login/URL+InsecureConnectionTests.swift | 41 +++++++++++++++++++ .../Login/URL+InsecureConnection.swift | 21 ++++++++++ 2 files changed, 62 insertions(+) create mode 100644 Tests/KeystoneTests/Tests/Login/URL+InsecureConnectionTests.swift create mode 100644 WordPress/Classes/Login/URL+InsecureConnection.swift diff --git a/Tests/KeystoneTests/Tests/Login/URL+InsecureConnectionTests.swift b/Tests/KeystoneTests/Tests/Login/URL+InsecureConnectionTests.swift new file mode 100644 index 000000000000..841eba2a3ecb --- /dev/null +++ b/Tests/KeystoneTests/Tests/Login/URL+InsecureConnectionTests.swift @@ -0,0 +1,41 @@ +import Foundation +import Testing + +@testable import WordPress + +struct URLInsecureConnectionTests { + @Test(arguments: [ + "https://example.com", + "https://example.com:8443/wp-json", + "HTTPS://EXAMPLE.COM" + ]) + func secureURLs(_ string: String) throws { + let url = try #require(URL(string: string)) + #expect(!url.isInsecureConnection) + } + + @Test(arguments: [ + "http://example.com", + "HTTP://EXAMPLE.COM", + "http://example.com:8080/wp-json", + "http://mymac.local", + "http://mysite.test", + "http://192.168.1.10:8881" + ]) + func insecureURLs(_ string: String) throws { + let url = try #require(URL(string: string)) + #expect(url.isInsecureConnection) + } + + @Test(arguments: [ + "http://localhost", + "http://localhost:8881/wp-admin", + "http://LOCALHOST:8881", + "http://127.0.0.1:8881", + "http://[::1]:8881" + ]) + func loopbackURLsAreExempt(_ string: String) throws { + let url = try #require(URL(string: string)) + #expect(!url.isInsecureConnection) + } +} diff --git a/WordPress/Classes/Login/URL+InsecureConnection.swift b/WordPress/Classes/Login/URL+InsecureConnection.swift new file mode 100644 index 000000000000..3879dc6e517b --- /dev/null +++ b/WordPress/Classes/Login/URL+InsecureConnection.swift @@ -0,0 +1,21 @@ +import Foundation + +extension URL { + private static let loopbackHosts: Set = ["localhost", "127.0.0.1", "::1"] + + /// Whether sending credentials to this URL would use an unencrypted connection to a remote host. + /// + /// Only loopback destinations are exempt. Names like `*.local` (resolved over the LAN via mDNS) + /// and `*.test` (resolved by whatever DNS the network provides) do not guarantee a local + /// connection, so they are treated the same as any other remote host. + var isInsecureConnection: Bool { + guard scheme?.lowercased() == "http" else { + return false + } + guard let host = host(percentEncoded: false)?.lowercased() else { + // A scheme of "http" with no parseable host cannot be proven local. Treat it as insecure. + return true + } + return !Self.loopbackHosts.contains(host) + } +} From 496f331b22fc66fc333b9f9d23df1271b367ffd9 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 5 Aug 2026 19:45:10 +1200 Subject: [PATCH 2/3] Warn before self-hosted sign-in over an insecure connection Present a confirmation alert at the top of the authenticate choke point when any pre-authorization credential destination (the site URL, REST API root, or application-password authorization URL) uses non-loopback http. Cancel reuses the existing SignInError.cancelled, and the debug launch-argument path never reaches this gate. The alert is presented from the topmost controller because the sign-in entry points already present the SwiftUI login flow. When the pre-authorization flow was fully secure, coerce an unexpectedly-http callback site URL to https, and skip the sign-in-time XML-RPC options fetch if discovery resolves an insecure endpoint, so a site that proved secure end-to-end never has its credentials sent over an unencrypted connection. --- .../Login/SelfHostedSiteAuthenticator.swift | 105 +++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/WordPress/Classes/Login/SelfHostedSiteAuthenticator.swift b/WordPress/Classes/Login/SelfHostedSiteAuthenticator.swift index 7e0029a0c859..355b9e2922d0 100644 --- a/WordPress/Classes/Login/SelfHostedSiteAuthenticator.swift +++ b/WordPress/Classes/Login/SelfHostedSiteAuthenticator.swift @@ -10,6 +10,7 @@ import WordPressShared import BuildSettingsKit import SVProgressHUD import WordPressSharedUI +import WordPressUI struct SelfHostedSiteAuthenticator { @@ -193,7 +194,12 @@ struct SelfHostedSiteAuthenticator { { credentials = parsed } else { - credentials = try await authenticate(details: details, from: viewController) + let authenticated = try await authenticate(details: details, from: viewController) + credentials = WpApiApplicationPasswordDetails( + siteUrl: details.sanitizedSiteUrl(authenticated.siteUrl), + userLogin: authenticated.userLogin, + password: authenticated.password + ) } let apiRootURL = details.apiRootUrl.asURL() @@ -211,6 +217,36 @@ struct SelfHostedSiteAuthenticator { } } + @MainActor + private func confirmInsecureConnection(host: String, from viewController: UIViewController) async -> Bool { + // The continuation is resumed only by the alert actions, matching the app's existing + // alert-to-async bridging. The alert is app-modal, so the only way it resolves without an + // action is the login flow being torn down while it is on screen. We accept that narrow edge + // case (a suspended continuation on an already-cancelled task) rather than add cancellation + // plumbing here. + await withCheckedContinuation { continuation in + let alert = UIAlertController( + title: Strings.insecureConnectionTitle, + message: Strings.insecureConnectionMessage(host: host), + preferredStyle: .alert + ) + alert.addAction( + UIAlertAction(title: SharedStrings.Button.cancel, style: .cancel) { _ in + continuation.resume(returning: false) + } + ) + alert.addAction( + UIAlertAction(title: Strings.insecureConnectionContinue, style: .destructive) { _ in + continuation.resume(returning: true) + } + ) + // The sign-in entry points hand us a controller that is already presenting the SwiftUI + // login flow, so present from the topmost controller to avoid a no-op present that would + // leave the continuation suspended forever. + viewController.topmostPresentedViewController.present(alert, animated: true) + } + } + @MainActor private func authenticate( details: AutoDiscoveryAttemptSuccess, @@ -228,6 +264,14 @@ struct SelfHostedSiteAuthenticator { throw .authentication(failure) } + if let insecureDestination = details.insecureURL { + let host = insecureDestination.host(percentEncoded: false) ?? details.parsedSiteUrl.url() + let proceed = await confirmInsecureConnection(host: host, from: viewController) + guard proceed else { + throw .cancelled + } + } + let appId = Self.wordPressAppId let appName = Self.wordPressAppName @@ -679,3 +723,62 @@ private final class EmptyAppNotifier: WpAppNotifier { // Do nothing. } } + +private extension AutoDiscoveryAttemptSuccess { + /// The first pre-authorization destination that would receive credentials over an unencrypted + /// connection, or nil when the whole flow is secure. + /// + /// The API root is included because discovery takes it verbatim from the site's Link header, + /// and a misconfigured https site can advertise an http API root that would receive the + /// application password. + var insecureURL: URL? { + var destinations = [parsedSiteUrl.asURL(), apiRootUrl.asURL()] + if case let .applicationPasswords(authUrl) = authentication { + destinations.append(authUrl.asURL()) + } + return destinations.first(where: \.isInsecureConnection) + } + + /// Sanitizes the site URL returned by the authorization callback before it is persisted as the + /// blog URL and used for XML-RPC discovery. + /// + /// When the pre-authorization flow was fully secure (the user was never warned), an http value + /// here is site misconfiguration and must not silently downgrade later traffic, so its scheme is + /// upgraded to https. When the user consented to an insecure flow, the value is left alone. + func sanitizedSiteUrl(_ callbackSiteUrl: String) -> String { + guard insecureURL == nil, + let url = URL(string: callbackSiteUrl), + url.isInsecureConnection, + var components = URLComponents(string: callbackSiteUrl) + else { + return callbackSiteUrl + } + components.scheme = "https" + return components.string ?? callbackSiteUrl + } +} + +private enum Strings { + static let insecureConnectionTitle = NSLocalizedString( + "addSite.selfHosted.insecureConnectionAlert.title", + value: "This site doesn't use a secure connection", + comment: "Title of an alert warning the user that the self-hosted site uses an unencrypted HTTP connection" + ) + + static func insecureConnectionMessage(host: String) -> String { + let format = NSLocalizedString( + "addSite.selfHosted.insecureConnectionAlert.message", + value: + "%@ uses HTTP, which is not encrypted. Your username, password, and site data could be seen by others on the network. Do you want to continue?", + comment: + "Message of an alert warning the user that the self-hosted site uses an unencrypted HTTP connection. The first argument is the site's host name." + ) + return String(format: format, host) + } + + static let insecureConnectionContinue = NSLocalizedString( + "addSite.selfHosted.insecureConnectionAlert.continue", + value: "Continue Anyway", + comment: "Button to proceed with signing in to a self-hosted site over an unencrypted HTTP connection" + ) +} From 6f65a56832b04f4d5854c662aecc2ad44f41770a Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 26 Aug 2026 18:44:44 +1200 Subject: [PATCH 3/3] Skip automatic application password creation for insecure sites ApplicationPasswordRepository must never transmit credentials to a non-loopback http destination on its own. Each path checks the destinations it contacts and throws insecureConnection for an insecure one: validation (the site URL and stored REST root) is checked in validatePasswords before any request, and self-hosted password creation (login_url, admin_url, and the wp-json base, all derived from xmlrpc and independently http-capable) is checked before creating. Jetpack sites create through the WordPress.com proxy over https and are not gated on the site's own scheme, so an https site with a valid token but a legacy http xmlrpc is not blocked. The REST API root resolved by discovery is validated before it is persisted to Blog.restApiRootURL, so an insecure value is never stored where other consumers could later send credentials to it. Getting an application password for an insecure site instead goes through the interactive sign-in flow, which shows the insecure-connection warning; existing repository callers already catch the error and degrade gracefully. --- .../ApplicationPasswordsRepositoryTests.swift | 210 ++++++++++++++++++ .../ApplicationPasswordRepository.swift | 68 ++++++ 2 files changed, 278 insertions(+) diff --git a/Tests/KeystoneTests/Tests/Utility/ApplicationPasswordsRepositoryTests.swift b/Tests/KeystoneTests/Tests/Utility/ApplicationPasswordsRepositoryTests.swift index b621bbc9ee1d..29c4158c8600 100644 --- a/Tests/KeystoneTests/Tests/Utility/ApplicationPasswordsRepositoryTests.swift +++ b/Tests/KeystoneTests/Tests/Utility/ApplicationPasswordsRepositoryTests.swift @@ -386,6 +386,183 @@ class ApplicationPasswordsRepositoryTests { let password = await password(of: blog) #expect(password == uuid) } + + @Test + func insecureSiteDoesNotTransmitCredentials() async throws { + defer { HTTPStubs.removeAllStubs() } + + let host = "insecure.example.com" + stub(condition: isHost(host)) { _ in + Issue.record("No request should be sent to an insecure site") + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + } + + let blog = try await coreDataStack.performAndSave { context in + let blog = Blog(context: context) + blog.url = "http://\(host)" + blog.xmlrpc = "http://\(host)/xmlrpc.php" + blog.username = "demo" + blog.password = "pass" + return TaggedManagedObjectID(blog) + } + + let repository = ApplicationPasswordRepository.forTesting(coreDataStack: coreDataStack, keychain: keychain) + await #expect(throws: ApplicationPasswordRepositoryError.insecureConnection) { + try await repository.createPasswordIfNeeded(for: blog) + } + } + + @Test + func insecureRestApiRootDoesNotTransmitCredentials() async throws { + defer { HTTPStubs.removeAllStubs() } + + let secureHost = "secure.example.com" + let insecureHost = "insecure-root.example.com" + stub(condition: isHost(secureHost) || isHost(insecureHost)) { _ in + Issue.record("No request should be sent when a credential destination is insecure") + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + } + + let blog = try await coreDataStack.performAndSave { context in + let blog = Blog(context: context) + blog.url = "https://\(secureHost)" + blog.xmlrpc = "https://\(secureHost)/xmlrpc.php" + blog.restApiRootURL = "http://\(insecureHost)/wp-json" + blog.username = "demo" + blog.password = "pass" + return TaggedManagedObjectID(blog) + } + + let repository = ApplicationPasswordRepository.forTesting(coreDataStack: coreDataStack, keychain: keychain) + await #expect(throws: ApplicationPasswordRepositoryError.insecureConnection) { + try await repository.createPasswordIfNeeded(for: blog) + } + } + + @Test + func insecureXMLRPCDerivedDestinationDoesNotTransmitCredentials() async throws { + defer { HTTPStubs.removeAllStubs() } + + // The site URL is https, so validation proceeds through an unauthenticated discovery over + // https. But login_url, admin_url, and the wp-json base are derived from the http xmlrpc + // endpoint, so self-hosted password creation is rejected before any credential-bearing + // request reaches the insecure host. + let secureHost = "secure-site.example.com" + let insecureHost = "insecure-xmlrpc.example.com" + stubApiDiscovery(siteHost: secureHost) + stub(condition: isHost(insecureHost)) { _ in + Issue.record("No request should be sent to an xmlrpc-derived insecure destination") + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + } + + let blog = try await coreDataStack.performAndSave { context in + let blog = Blog(context: context) + blog.url = "https://\(secureHost)" + blog.xmlrpc = "http://\(insecureHost)/xmlrpc.php" + blog.username = "demo" + blog.password = "pass" + return TaggedManagedObjectID(blog) + } + + let repository = ApplicationPasswordRepository.forTesting(coreDataStack: coreDataStack, keychain: keychain) + await #expect(throws: ApplicationPasswordRepositoryError.insecureConnection) { + try await repository.createPasswordIfNeeded(for: blog) + } + } + + @Test + func validTokenValidatesOverHttpsDespiteInsecureXMLRPC() async throws { + defer { HTTPStubs.removeAllStubs() } + + // A valid stored token validates over the https site URL and REST root. The http + // xmlrpc-derived login/admin/wp-json urls are only needed to create a password, so an + // existing token must not be blocked by them, and creation must not be attempted. + let secureHost = "valid-token.example.com" + let insecureHost = "insecure-xmlrpc.example.com" + stubCurrentApplicationPassword(host: secureHost) + stub(condition: isHost(insecureHost)) { _ in + Issue.record("No request should reach the insecure xmlrpc-derived host") + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + } + + let blog = try await coreDataStack.performAndSave { [keychain] context in + let blog = Blog(context: context) + blog.url = "https://\(secureHost)" + blog.xmlrpc = "http://\(insecureHost)/xmlrpc.php" + blog.restApiRootURL = "https://\(secureHost)/wp-json" + blog.username = "demo" + blog.password = "pass" + try blog.setApplicationToken("valid token", using: keychain) + return TaggedManagedObjectID(blog) + } + + let repository = ApplicationPasswordRepository.forTesting(coreDataStack: coreDataStack, keychain: keychain) + try await repository.createPasswordIfNeeded(for: blog) + + let password = await password(of: blog) + #expect(password == "valid token") + } + + @Test + func discoveredInsecureRestApiRootDoesNotTransmitCredentials() async throws { + defer { HTTPStubs.removeAllStubs() } + + let host = "discovered-insecure.example.com" + let blog = try await coreDataStack.performAndSave { context in + let blog = Blog(context: context) + blog.url = "https://\(host)" + blog.xmlrpc = "https://\(host)/xmlrpc.php" + blog.username = "demo" + blog.password = "pass" + return TaggedManagedObjectID(blog) + } + + // The blog has no stored REST root, so discovery resolves one. Discovery advertises an http + // root, and no credential-bearing request may follow. + stubApiDiscoveryWithInsecureRoot(siteHost: host) + stub( + condition: isPath("/wp-json/wp/v2/users/me") + || isPath("/wp-json/wp/v2/users/me/application-passwords") + ) { _ in + Issue.record("No credentials should be sent to a discovered insecure REST root") + return HTTPStubsResponse(error: URLError(.notConnectedToInternet)) + } + + let repository = ApplicationPasswordRepository.forTesting(coreDataStack: coreDataStack, keychain: keychain) + await #expect(throws: ApplicationPasswordRepositoryError.insecureConnection) { + try await repository.createPasswordIfNeeded(for: blog) + } + + // The rejected http root must not be persisted, or other consumers could later use it. + let storedRoot = await coreDataStack.performQuery { context in + (try? context.existingObject(with: blog))?.restApiRootURL + } + #expect(storedRoot == nil) + } + + @Test + func loopbackHttpSiteCreatesPassword() async throws { + defer { HTTPStubs.removeAllStubs() } + + let blog = try await coreDataStack.performAndSave { context in + let blog = Blog(context: context) + blog.url = "http://localhost:8881" + blog.xmlrpc = "http://localhost:8881/xmlrpc.php" + blog.username = "demo" + blog.password = "pass" + return TaggedManagedObjectID(blog) + } + + stubApiDiscovery(siteHost: "localhost") + stubSelfHostedSiteWpV2GetUser() + stubSelfHostedSiteCreateApplicationPassword(host: "localhost", password: "abcd efgh") + + let repository = ApplicationPasswordRepository.forTesting(coreDataStack: coreDataStack, keychain: keychain) + try await repository.createPasswordIfNeeded(for: blog) + + let password = await password(of: blog) + #expect(password == "abcd efgh") + } } // MARK: - Helpers @@ -758,6 +935,39 @@ private extension ApplicationPasswordsRepositoryTests { } } + func stubApiDiscoveryWithInsecureRoot(siteHost: String) { + stub(condition: isHost(siteHost) && isPath("/")) { _ in + HTTPStubsResponse( + data: "homepage".data(using: .utf8)!, + statusCode: 200, + headers: ["Link": "; rel=\"https://api.w.org/\""] + ) + } + stub(condition: isHost(siteHost) && isPath("/wp-json")) { _ in + let json = """ + { + "name": "Site", + "description": "", + "url": "http://\(siteHost)", + "home": "http://\(siteHost)", + "gmt_offset": "0", + "timezone_string": "", + "namespaces": ["wp/v2"], + "authentication": { + "application-passwords": { + "endpoints": { + "authorization": "http://\(siteHost)/wp-admin/authorize-application.php" + } + } + }, + "routes": {}, + "_links": {} + } + """ + return HTTPStubsResponse(data: json.data(using: .utf8)!, statusCode: 200, headers: nil) + } + } + func stubApiDiscoveryFailure(siteHost: String) { stub(condition: isHost(siteHost) && isPath("/")) { _ in HTTPStubsResponse(data: "homepage".data(using: .utf8)!, statusCode: 200, headers: nil) diff --git a/WordPress/Classes/Services/ApplicationPasswordRepository.swift b/WordPress/Classes/Services/ApplicationPasswordRepository.swift index 829fcefd0c72..93676210e7f2 100644 --- a/WordPress/Classes/Services/ApplicationPasswordRepository.swift +++ b/WordPress/Classes/Services/ApplicationPasswordRepository.swift @@ -89,6 +89,17 @@ actor ApplicationPasswordRepository { /// When returning true, a valid application password is guaranteed to be returned by the `Blog.getApplicationToken` function. /// + /// This function must never transmit credentials over an unencrypted connection on its own, so + /// each path checks the destinations it contacts and throws `insecureConnection` for a + /// non-loopback http one. Validation, which every path performs, contacts the site URL and REST + /// root, so those are checked up front. Self-hosted password creation additionally contacts the + /// cookie/nonce URLs (`login_url`, `admin_url`, wp-json, all derived from `xmlrpc` and + /// independently http-capable), checked before creating. A Jetpack-connected site creates its + /// password through the WordPress.com proxy over https, so it is not gated on the site's own + /// scheme. Getting an application password for an insecure site instead goes through the + /// interactive sign-in flow, which shows an insecure-connection warning. The REST root discovered + /// at runtime is validated in `updateRestAPIURLIfNeeded`. + /// /// This function is safe to call multiple times, but every call performs real work, including /// HTTP requests (password validation, and REST API root rediscovery when the stored root is /// stale). Limit calls to once per "site launch" (app launch, switching site, etc.) to avoid @@ -162,7 +173,27 @@ actor ApplicationPasswordRepository { } private extension ApplicationPasswordRepository { + /// Throws `insecureConnection` when any URL would send credentials over a non-loopback http + /// connection. Callers pass only the destinations the request they are about to make contacts. + static func checkInsecureURL(_ urls: [URL]) throws { + if urls.contains(where: \.isInsecureConnection) { + throw ApplicationPasswordRepositoryError.insecureConnection + } + } + func validatePasswords(in blogId: TaggedManagedObjectID) async throws -> ApplicationPassword? { + // Validation contacts the site URL and stored REST root, here and in the token copy below. + // Reject before any request when either is insecure, without contacting the site. + let validationDestinations = try await coreDataStack.performQuery { context in + let blog = try context.existingObject(with: blogId) + var destinations = [try blog.getUrl()] + if let restApiRootURL = blog.restApiRootURL, let parsed = URL(string: restApiRootURL) { + destinations.append(parsed) + } + return destinations + } + try Self.checkInsecureURL(validationDestinations) + try await saveApplicationPassword(of: blogId) let (owners, siteUrl) = try await coreDataStack.performQuery { context in @@ -258,6 +289,28 @@ private extension ApplicationPasswordRepository { parameters: parameters ) } else if let dotOrgApi { + // Creating a password here uses cookie and nonce authentication, contacting the site's + // login_url, admin_url, and wp-json base. Each derives from `xmlrpc` (or its own option), + // so each can use http independently. Gate on every credential destination before + // sending anything. + let destinations = try await coreDataStack.performQuery { context in + let blog = try context.existingObject(with: blogId) + var destinations: [URL] = [try blog.getUrl()] + if let restApiRootURL = blog.restApiRootURL, let parsed = URL(string: restApiRootURL) { + destinations.append(parsed) + } + if let restBase = blog.url(withPath: "wp-json/"), let parsed = URL(string: restBase) { + destinations.append(parsed) + } + if let loginURL = blog.loginURL { + destinations.append(loginURL) + } + if let adminURL = blog.makeAdminURL() { + destinations.append(adminURL) + } + return destinations + } + try Self.checkInsecureURL(destinations) password = try await createPasswordOnSelfHostedSites(api: dotOrgApi, parameters: parameters) } else { // This error should never happen since a blog is accessible via either dot-com or a dot-org API. @@ -385,6 +438,13 @@ private extension ApplicationPasswordRepository { throw error } + // Discovery can resolve an http REST root even for an https site (e.g. an advertised http + // API root). Reject it before persisting, so an insecure value is never stored where other + // consumers (WordPressSite, EditorConfiguration, ...) could later send credentials to it. + if let url = URL(string: apiRootURL.url()), url.isInsecureConnection { + throw ApplicationPasswordRepositoryError.insecureConnection + } + if apiRootURL.url() != restApiRootUrl { try await coreDataStack.performAndSave { context in let blog = try context.existingObject(with: blogId) @@ -506,6 +566,7 @@ extension ApplicationPasswordStorage { enum ApplicationPasswordRepositoryError: LocalizedError { case usernameNotFound case restApiInaccessible + case insecureConnection case unknown var errorDescription: String? { @@ -516,6 +577,13 @@ enum ApplicationPasswordRepositoryError: LocalizedError { value: "Unable to find username for the site", comment: "Error message when the username cannot be found for application password creation" ) + case .insecureConnection: + return NSLocalizedString( + "applicationPasswordRepository.error.insecureConnection", + value: "The site uses an unencrypted connection (HTTP).", + comment: + "Error message when application password creation is skipped because the site uses an insecure HTTP connection" + ) case .restApiInaccessible: return NSLocalizedString( "applicationPasswordRepository.error.restApiInaccessible",