diff --git a/CHANGELOG.md b/CHANGELOG.md index fb4f767b4..e485652f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,12 @@ _None._ _None._ +## 8.0.1 + +### Bug Fixes + +- Fix an issue where self-hosted sites are incorrectly flagged as non WordPress sites. [#796] + ## 8.0.0 ### Breaking Changes @@ -57,6 +63,7 @@ _None._ ### Internal Changes * Depend on WordPressKit 9.0.0 and make necessary code changes to adopt the new API. [808] +>>>>>>> trunk ## 7.3.1 diff --git a/WordPressAuthenticator.xcodeproj/project.pbxproj b/WordPressAuthenticator.xcodeproj/project.pbxproj index 9969171f0..d5ce397a7 100644 --- a/WordPressAuthenticator.xcodeproj/project.pbxproj +++ b/WordPressAuthenticator.xcodeproj/project.pbxproj @@ -7,6 +7,8 @@ objects = { /* Begin PBXBuildFile section */ + 01720C132AEB5013006713DF /* SiteAddressViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 01720C122AEB5013006713DF /* SiteAddressViewModelTests.swift */; }; + 01720C152AEB5101006713DF /* SiteAddressViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 01720C142AEB5101006713DF /* SiteAddressViewModel.swift */; }; 0193F7752A615521004D7C16 /* MemoryManagementTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0193F7742A615521004D7C16 /* MemoryManagementTests.swift */; }; 020BE74A23B0BD2E007FE54C /* WordPressAuthenticatorDisplayImages.swift in Sources */ = {isa = PBXBuildFile; fileRef = 020BE74923B0BD2E007FE54C /* WordPressAuthenticatorDisplayImages.swift */; }; 020DEF6428AA091100C85D51 /* MagicLinkRequester.swift in Sources */ = {isa = PBXBuildFile; fileRef = 020DEF6328AA091100C85D51 /* MagicLinkRequester.swift */; }; @@ -263,6 +265,8 @@ /* End PBXCopyFilesBuildPhase section */ /* Begin PBXFileReference section */ + 01720C122AEB5013006713DF /* SiteAddressViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SiteAddressViewModelTests.swift; sourceTree = ""; }; + 01720C142AEB5101006713DF /* SiteAddressViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SiteAddressViewModel.swift; sourceTree = ""; }; 0193F7742A615521004D7C16 /* MemoryManagementTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MemoryManagementTests.swift; sourceTree = ""; }; 020BE74923B0BD2E007FE54C /* WordPressAuthenticatorDisplayImages.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WordPressAuthenticatorDisplayImages.swift; sourceTree = ""; }; 020DEF6328AA091100C85D51 /* MagicLinkRequester.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MagicLinkRequester.swift; sourceTree = ""; }; @@ -565,6 +569,7 @@ 3F86A83F29D280DC005D20C0 /* SingIn */ = { isa = PBXGroup; children = ( + 01720C122AEB5013006713DF /* SiteAddressViewModelTests.swift */, 3F86A83D29D280D7005D20C0 /* AppleAuthenticatorTests.swift */, 3F86A84929D2A982005D20C0 /* LoginViewControllerTests.swift */, ); @@ -1079,6 +1084,7 @@ CEC77C6524854F2E00FB9050 /* SiteAddressViewController.swift */, CEC77C6724854F3E00FB9050 /* SiteAddress.storyboard */, CE73475524B77A3800A22660 /* SiteCredentialsViewController.swift */, + 01720C142AEB5101006713DF /* SiteAddressViewModel.swift */, ); path = "Site Address"; sourceTree = ""; @@ -1532,6 +1538,7 @@ 3F86A84229D28473005D20C0 /* SocialUserCreating.swift in Sources */, 3F9439BE27D6F9B60067183A /* LoginPrologueViewController.swift in Sources */, B560913B208A563800399AE4 /* LoginSelfHostedViewController.swift in Sources */, + 01720C152AEB5101006713DF /* SiteAddressViewModel.swift in Sources */, B5609136208A563800399AE4 /* Login2FAViewController.swift in Sources */, B56090E1208A4F9D00399AE4 /* WPWalkthroughTextField.m in Sources */, B56090EF208A527000399AE4 /* WPStyleGuide+Login.swift in Sources */, @@ -1560,6 +1567,7 @@ BA53D64824DFDF97001F1ABF /* WordPressSourceTagTests.swift in Sources */, 3F30A6BA299F12E30004452F /* Character+URLSafeTests.swift in Sources */, 4A1DEF4B29341B1F00322608 /* LoggingTests.swift in Sources */, + 01720C132AEB5013006713DF /* SiteAddressViewModelTests.swift in Sources */, 3F86A83E29D280D7005D20C0 /* AppleAuthenticatorTests.swift in Sources */, 3F3694022991E244006E923E /* JSONWebToken+Fixtures.swift in Sources */, 3F879FDF293A501D005C2B48 /* URLRequest+GoogleSignInTests.swift in Sources */, diff --git a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift index 1ae55f298..2515fee06 100644 --- a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift +++ b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift @@ -32,6 +32,15 @@ final class SiteAddressViewController: LoginViewController { /// private let isSiteDiscovery: Bool private let configuration = WordPressAuthenticator.shared.configuration + private lazy var viewModel: SiteAddressViewModel = { + return SiteAddressViewModel( + isSiteDiscovery: isSiteDiscovery, + xmlrpcFacade: WordPressXMLRPCAPIFacade(), + authenticationDelegate: authenticationDelegate, + blogService: WordPressComBlogService(), + loginFields: loginFields + ) + }() init?(isSiteDiscovery: Bool, coder: NSCoder) { self.isSiteDiscovery = isSiteDiscovery @@ -482,75 +491,29 @@ private extension SiteAddressViewController { } func guessXMLRPCURL(for siteAddress: String) { - let facade = WordPressXMLRPCAPIFacade() - facade.guessXMLRPCURL(forSite: siteAddress, success: { [weak self] (url) in - // Success! We now know that we have a valid XML-RPC endpoint. - // At this point, we do NOT know if this is a WP.com site or a self-hosted site. - if let url = url { - self?.loginFields.meta.xmlrpcURL = url as NSURL - } - // Let's try to grab site info in preparation for the next screen. - self?.fetchSiteInfo() - - }, failure: { [weak self] (error) in - guard let error = error, let self = self else { - return - } - // Intentionally log the attempted address on failures. - // It's not guaranteed to be included in the error object depending on the error. - WPAuthenticatorLogInfo("Error attempting to connect to site address: \(self.loginFields.siteAddress)") - WPAuthenticatorLogError(error.localizedDescription) - - self.tracker.track(failure: .loginFailedToGuessXMLRPC) - - self.configureViewLoading(false) - - guard self.isSiteDiscovery == false else { - WordPressAuthenticator.shared.delegate?.troubleshootSite(nil, in: self.navigationController) - return - } - - let err = self.originalErrorOrError(error: error as NSError) - - let errorMessage: String? = { - if let xmlrpcValidatorError = err as? WordPressOrgXMLRPCValidatorError { - return xmlrpcValidatorError.localizedDescription - } else if (err.domain == NSURLErrorDomain && err.code == NSURLErrorCannotFindHost) || - (err.domain == NSURLErrorDomain && err.code == NSURLErrorNetworkConnectionLost) { - // NSURLErrorNetworkConnectionLost can be returned when an invalid URL is entered. - let msg = NSLocalizedString( - "The site at this address is not a WordPress site. For us to connect to it, the site must use WordPress.", - comment: "Error message shown a URL does not point to an existing site.") - return msg - } else { - return nil - } - }() - - /// Check if the host app wants to provide custom UI to handle the error. - /// If it does, insert the custom UI provided by the host app and exit early - if self.authenticationDelegate.shouldHandleError(err) { - - // Send the error to the host app - self.authenticationDelegate.handleError(err) { customUI in - self.pushCustomUI(customUI) - } - - // Track error message as failure + viewModel.guessXMLRPCURL( + for: siteAddress, + loading: { [weak self] isLoading in + self?.configureViewLoading(isLoading) + }, + completion: { [weak self] result -> Void in + guard let self else { return } + switch result { + case .success: + // Let's try to grab site info in preparation for the next screen. + self.fetchSiteInfo() + case .error(let error, let errorMessage): if let message = errorMessage { - self.tracker.track(failure: message) + self.displayError(message: message, moveVoiceOverFocus: true) + } else { + self.displayError(error, sourceTag: self.sourceTag) } - - // Return as the error has been handled by the host app. - return - } - - if let message = errorMessage { - self.displayError(message: message, moveVoiceOverFocus: true) - } else { - self.displayError(error, sourceTag: self.sourceTag) + case .troubleshootSite: + WordPressAuthenticator.shared.delegate?.troubleshootSite(nil, in: self.navigationController) + case .customUI(let viewController): + self.pushCustomUI(viewController) } - }) + }) } func fetchSiteInfo() { diff --git a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewModel.swift b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewModel.swift new file mode 100644 index 000000000..59c2f46be --- /dev/null +++ b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewModel.swift @@ -0,0 +1,136 @@ +import Foundation +import WordPressKit + +struct SiteAddressViewModel { + private let isSiteDiscovery: Bool + private let xmlrpcFacade: WordPressXMLRPCAPIFacade + private unowned let authenticationDelegate: WordPressAuthenticatorDelegate + private let blogService: WordPressComBlogService + private var loginFields: LoginFields + + private let tracker = AuthenticatorAnalyticsTracker.shared + + init(isSiteDiscovery: Bool, + xmlrpcFacade: WordPressXMLRPCAPIFacade, + authenticationDelegate: WordPressAuthenticatorDelegate, + blogService: WordPressComBlogService, + loginFields: LoginFields + ) { + self.isSiteDiscovery = isSiteDiscovery + self.xmlrpcFacade = xmlrpcFacade + self.authenticationDelegate = authenticationDelegate + self.blogService = blogService + self.loginFields = loginFields + } + + enum GuessXMLRPCURLResult: Equatable { + case success + case error(NSError, String?) + case troubleshootSite + case customUI(UIViewController) + } + + func guessXMLRPCURL( + for siteAddress: String, + loading: @escaping ((Bool) -> ()), + completion: @escaping (GuessXMLRPCURLResult) -> () + ) { + xmlrpcFacade.guessXMLRPCURL(forSite: siteAddress, success: { url in + // Success! We now know that we have a valid XML-RPC endpoint. + // At this point, we do NOT know if this is a WP.com site or a self-hosted site. + if let url = url { + self.loginFields.meta.xmlrpcURL = url as NSURL + } + + completion(.success) + + }, failure: { error in + guard let error = error else { + return + } + // Intentionally log the attempted address on failures. + // It's not guaranteed to be included in the error object depending on the error. + WPAuthenticatorLogInfo("Error attempting to connect to site address: \(self.loginFields.siteAddress)") + WPAuthenticatorLogError(error.localizedDescription) + + self.tracker.track(failure: .loginFailedToGuessXMLRPC) + + loading(false) + + guard self.isSiteDiscovery == false else { + completion(.troubleshootSite) + return + } + + let err = self.originalErrorOrError(error: error as NSError) + self.handleGuessXMLRPCURLError(error: err, loading: loading, completion: completion) + }) + } + + private func handleGuessXMLRPCURLError( + error: NSError, + loading: @escaping ((Bool) -> ()), + completion: @escaping (GuessXMLRPCURLResult) -> () + ) { + let completion: (NSError, String?) -> Void = { error, errorMessage in + if self.authenticationDelegate.shouldHandleError(error) { + self.authenticationDelegate.handleError(error) { customUI in + completion(.customUI(customUI)) + } + if let message = errorMessage { + self.tracker.track(failure: message) + } + return + } + + completion(.error(error, errorMessage)) + } + + /// Confirm the site is not a WordPress site before describing it as an invalid WP site + if let xmlrpcValidatorError = error as? WordPressOrgXMLRPCValidatorError, xmlrpcValidatorError == .invalid { + loading(true) + isWPSite { isWP in + loading(false) + if isWP { + let error = WordPressOrgXMLRPCValidatorError.xmlrpc_missing + completion(error as NSError, error.localizedDescription) + } else { + completion(error, Strings.notWPSiteErrorMessage) + } + } + } else if (error.domain == NSURLErrorDomain && error.code == NSURLErrorCannotFindHost) || + (error.domain == NSURLErrorDomain && error.code == NSURLErrorNetworkConnectionLost) { + completion(error, Strings.notWPSiteErrorMessage) + } else { + completion(error, (error as? WordPressOrgXMLRPCValidatorError)?.localizedDescription) + } + } + + private func originalErrorOrError(error: NSError) -> NSError { + guard let err = error.userInfo[XMLRPCOriginalErrorKey] as? NSError else { + return error + } + + return err + } +} + +extension SiteAddressViewModel { + private func isWPSite(_ completion: @escaping (Bool) -> ()) { + let baseSiteUrl = WordPressAuthenticator.baseSiteURL(string: loginFields.siteAddress) + blogService.fetchUnauthenticatedSiteInfoForAddress( + for: baseSiteUrl, + success: { siteInfo in + completion(siteInfo.isWP) + }, + failure: { _ in + completion(false) + }) + } +} + +private extension SiteAddressViewModel { + struct Strings { + static let notWPSiteErrorMessage = NSLocalizedString("The site at this address is not a WordPress site. For us to connect to it, the site must use WordPress.", comment: "Error message shown when a URL does not point to an existing site.") + } +} diff --git a/WordPressAuthenticatorTests/Mocks/WordPressAuthenticatorDelegateSpy.swift b/WordPressAuthenticatorTests/Mocks/WordPressAuthenticatorDelegateSpy.swift index 6cce72e36..ff9ab9d51 100644 --- a/WordPressAuthenticatorTests/Mocks/WordPressAuthenticatorDelegateSpy.swift +++ b/WordPressAuthenticatorTests/Mocks/WordPressAuthenticatorDelegateSpy.swift @@ -8,6 +8,7 @@ class WordPressAuthenticatorDelegateSpy: WordPressAuthenticatorDelegate { var showSupportNotificationIndicator: Bool = true var supportEnabled: Bool = true var allowWPComLogin: Bool = true + var shouldHandleError: Bool = false private(set) var presentSignupEpilogueCalled = false private(set) var socialUser: SocialUser? @@ -50,11 +51,13 @@ class WordPressAuthenticatorDelegateSpy: WordPressAuthenticatorDelegate { } func shouldHandleError(_ error: Error) -> Bool { - true + shouldHandleError } func handleError(_ error: Error, onCompletion: @escaping (UIViewController) -> Void) { - // no-op + if shouldHandleError { + onCompletion(UIViewController()) + } } func shouldPresentSignupEpilogue() -> Bool { diff --git a/WordPressAuthenticatorTests/SingIn/SiteAddressViewModelTests.swift b/WordPressAuthenticatorTests/SingIn/SiteAddressViewModelTests.swift new file mode 100644 index 000000000..d74ba5017 --- /dev/null +++ b/WordPressAuthenticatorTests/SingIn/SiteAddressViewModelTests.swift @@ -0,0 +1,125 @@ +import XCTest +import WordPressKit +@testable import WordPressAuthenticator + +final class SiteAddressViewModelTests: XCTestCase { + private var isSiteDiscovery: Bool! + private var xmlrpcFacade: MockWordPressXMLRPCAPIFacade! + private var authenticationDelegateSpy: WordPressAuthenticatorDelegateSpy! + private var blogService: MockWordPressComBlogService! + private var loginFields: LoginFields! + private var viewModel: SiteAddressViewModel! + + override func setUp() { + super.setUp() + isSiteDiscovery = false + xmlrpcFacade = MockWordPressXMLRPCAPIFacade() + authenticationDelegateSpy = WordPressAuthenticatorDelegateSpy() + blogService = MockWordPressComBlogService() + loginFields = LoginFields() + + WordPressAuthenticator.initializeForTesting() + + viewModel = SiteAddressViewModel(isSiteDiscovery: isSiteDiscovery, xmlrpcFacade: xmlrpcFacade, authenticationDelegate: authenticationDelegateSpy, blogService: blogService, loginFields: loginFields) + } + + func testGuessXMLRPCURLSuccess() { + xmlrpcFacade.success = true + var result: SiteAddressViewModel.GuessXMLRPCURLResult? + viewModel.guessXMLRPCURL(for: "https://wordpress.com", loading: { _ in }) { res in + result = res + } + + XCTAssertEqual(result, .success) + } + + func testGuessXMLRPCURLError() { + xmlrpcFacade.error = NSError(domain: "SomeDomain", code: 1, userInfo: nil) + var result: SiteAddressViewModel.GuessXMLRPCURLResult? + viewModel.guessXMLRPCURL(for: "https://error.com", loading: { _ in }) { res in + result = res + } + if case .error(let error, _) = result { + XCTAssertEqual(error.code, 1) + } else { + XCTFail("Unexpected result: \(String(describing: result))") + } + } + + func testGuessXMLRPCURLErrorInvalidNotWP() { + xmlrpcFacade.error = WordPressOrgXMLRPCValidatorError.invalid as NSError + blogService.isWP = false + var result: SiteAddressViewModel.GuessXMLRPCURLResult? + viewModel.guessXMLRPCURL(for: "https://invalid.com", loading: { _ in }) { res in + result = res + } + + if case .error(let error, _) = result { + XCTAssertEqual(error.code, WordPressOrgXMLRPCValidatorError.invalid.rawValue) + } else { + XCTFail("Unexpected result: \(String(describing: result))") + } + } + + func testGuessXMLRPCURLErrorInvalidIsWP() { + xmlrpcFacade.error = WordPressOrgXMLRPCValidatorError.invalid as NSError + blogService.isWP = true + var result: SiteAddressViewModel.GuessXMLRPCURLResult? + viewModel.guessXMLRPCURL(for: "https://invalidwp.com", loading: { _ in }) { res in + result = res + } + if case .error(let error, _) = result { + XCTAssertEqual(error.code, WordPressOrgXMLRPCValidatorError.xmlrpc_missing.rawValue) + } else { + XCTFail("Unexpected result: \(String(describing: result))") + } + } + + func testGuessXMLRPCTroubleshootSite() { + viewModel = SiteAddressViewModel(isSiteDiscovery: true, xmlrpcFacade: xmlrpcFacade, authenticationDelegate: authenticationDelegateSpy, blogService: blogService, loginFields: loginFields) + xmlrpcFacade.error = NSError(domain: "SomeDomain", code: 1, userInfo: nil) + var result: SiteAddressViewModel.GuessXMLRPCURLResult? + viewModel.guessXMLRPCURL(for: "https://troubleshoot.com", loading: { _ in }) { res in + result = res + } + XCTAssertEqual(result, .troubleshootSite) + } + + func testGuessXMLRPCURLErrorHandledByDelegate() { + xmlrpcFacade.error = NSError(domain: "SomeDomain", code: 1, userInfo: nil) + authenticationDelegateSpy.shouldHandleError = true + + var result: SiteAddressViewModel.GuessXMLRPCURLResult? + viewModel.guessXMLRPCURL(for: "https://delegatehandles.com", loading: { _ in }) { res in + result = res + } + + if case .customUI = result { + XCTAssertTrue(true) + } else { + XCTFail("Unexpected result: \(String(describing: result))") + } + } +} + +private class MockWordPressXMLRPCAPIFacade: WordPressXMLRPCAPIFacade { + var success: Bool = false + var error: NSError? + + override func guessXMLRPCURL(forSite siteAddress: String, success: @escaping (URL?) -> (), failure: @escaping (Error?) -> ()) { + if self.success { + success(URL(string: "https://successful.site")) + } else { + failure(self.error) + } + } +} + +private class MockWordPressComBlogService: WordPressComBlogService { + var isWP = false + + override func fetchUnauthenticatedSiteInfoForAddress(for address: String, success: @escaping (WordPressComSiteInfo) -> Void, failure: @escaping (Error) -> Void) { + let siteInfo = WordPressComSiteInfo(remote: ["isWordPress": isWP]) + success(siteInfo) + } +}