diff --git a/Modules/Sources/Networking/Model/Site.swift b/Modules/Sources/Networking/Model/Site.swift index 3f84389292b..95896565bc8 100644 --- a/Modules/Sources/Networking/Model/Site.swift +++ b/Modules/Sources/Networking/Model/Site.swift @@ -117,7 +117,7 @@ public struct Site: Decodable, Equatable, Hashable, GeneratedFakeable, Generated let siteID = try siteContainer.decode(Int64.self, forKey: .siteID) let name = try siteContainer.decode(String.self, forKey: .name) let description = try siteContainer.decode(String.self, forKey: .description) - let url = try siteContainer.decode(String.self, forKey: .url) + let url = Self.safeURL(try siteContainer.decode(String.self, forKey: .url)) let capabilitiesContainer = try siteContainer.nestedContainer(keyedBy: CapabilitiesKeys.self, forKey: .capabilities) let isSiteOwner = try capabilitiesContainer.decode(Bool.self, forKey: .isSiteOwner) let isAdmin = try capabilitiesContainer.decode(Bool.self, forKey: .isAdmin) @@ -130,8 +130,8 @@ public struct Site: Decodable, Equatable, Hashable, GeneratedFakeable, Generated let jetpackConnectionActivePlugins = try optionsContainer.decodeIfPresent([String].self, forKey: .jetpackConnectionActivePlugins) ?? [] let timezone = try optionsContainer.decode(String.self, forKey: .timezone) let gmtOffset = try optionsContainer.decode(Double.self, forKey: .gmtOffset) - let adminURL = try optionsContainer.decode(String.self, forKey: .adminURL) - let loginURL = try optionsContainer.decode(String.self, forKey: .loginURL) + let adminURL = Self.safeURL(try optionsContainer.decode(String.self, forKey: .adminURL)) + let loginURL = Self.safeURL(try optionsContainer.decode(String.self, forKey: .loginURL)) let frameNonce = try optionsContainer.decode(String.self, forKey: .frameNonce) let canBlaze = optionsContainer.failsafeDecodeIfPresent(booleanForKey: .canBlaze) ?? false let visibility = optionsContainer.failsafeDecodeIfPresent(SiteVisibility.self, forKey: .visibility) ?? .privateSite @@ -333,7 +333,8 @@ public enum SiteVisibility: Int, Codable, GeneratedFakeable { /// public extension Site { - private var jetpackCanonicalURL: String { + /// Force URL to use HTTPS if possible to avoid App Transport Security errors + private static func safeURL(_ url: String) -> String { guard let originalURL = URL(string: url), originalURL.scheme?.lowercased() == "http" else { @@ -360,7 +361,7 @@ public extension Site { } func toJetpackSite() -> JetpackSite { - JetpackSite(siteID: siteID, siteAddress: jetpackCanonicalURL, applicationPasswordAvailable: applicationPasswordAvailable) + JetpackSite(siteID: siteID, siteAddress: url, applicationPasswordAvailable: applicationPasswordAvailable) } } diff --git a/Modules/Tests/NetworkingTests/Mapper/SiteListMapperTests.swift b/Modules/Tests/NetworkingTests/Mapper/SiteListMapperTests.swift index 38dbb33a420..f77de97bb2d 100644 --- a/Modules/Tests/NetworkingTests/Mapper/SiteListMapperTests.swift +++ b/Modules/Tests/NetworkingTests/Mapper/SiteListMapperTests.swift @@ -37,6 +37,21 @@ final class SiteListMapperTests: XCTestCase { let second = try XCTUnwrap(sites[safe: 1]) XCTAssertFalse(second.hasSSOEnabled) } + + func test_http_urls_are_converted_to_https() throws { + // Given + let sites = mapLoadHTTPSiteListResponse() + + // Then + let site = try XCTUnwrap(sites.first) + XCTAssertTrue(site.url.hasPrefix("https://"), "Site URL should be converted to HTTPS") + XCTAssertTrue(site.adminURL.hasPrefix("https://"), "Admin URL should be converted to HTTPS") + XCTAssertTrue(site.loginURL.hasPrefix("https://"), "Login URL should be converted to HTTPS") + + XCTAssertEqual(site.url, "https://insecure-site.testing.blog") + XCTAssertEqual(site.adminURL, "https://insecure-site.testing.blog/wp-admin/") + XCTAssertEqual(site.loginURL, "https://insecure-site.testing.blog/wp-login.php") + } } private extension SiteListMapperTests { @@ -55,4 +70,8 @@ private extension SiteListMapperTests { func mapLoadMalformedSiteListResponse() -> [Site] { return mapSiteListData(from: "sites-malformed") } + + func mapLoadHTTPSiteListResponse() -> [Site] { + return mapSiteListData(from: "sites-http") + } } diff --git a/Modules/Tests/NetworkingTests/Model/SiteTests.swift b/Modules/Tests/NetworkingTests/Model/SiteTests.swift deleted file mode 100644 index ac2783bfad9..00000000000 --- a/Modules/Tests/NetworkingTests/Model/SiteTests.swift +++ /dev/null @@ -1,28 +0,0 @@ -import Foundation -import Testing -@testable import Networking -@testable import NetworkingCore - -struct SiteTests { - - @Test(arguments: [ - "http://awesomesite.com", - "http://awesomesite.com/", - "http://awesomesite.whatever.com", - "HTTP://awesomesite.com", - "http://192.168.0.12", - "https://awesomesite.com" - ]) - private func forcingHttpsForJetpack(siteAddress: String) { - let site = Site.defaultMock().copy(url: siteAddress) - let jetpack = site.toJetpackSite() - - #expect(jetpack.siteAddress.isHttpsScheme) - } -} - -fileprivate extension String { - var isHttpsScheme: Bool { - return URL(string: self)?.scheme?.lowercased() == "https" - } -} diff --git a/Modules/Tests/NetworkingTests/Responses/sites-http.json b/Modules/Tests/NetworkingTests/Responses/sites-http.json new file mode 100644 index 00000000000..034bbbd810c --- /dev/null +++ b/Modules/Tests/NetworkingTests/Responses/sites-http.json @@ -0,0 +1,34 @@ +{ + "sites": [ + { + "ID": 7777888899990000, + "name": "HTTP Site", + "description": "Testing HTTP to HTTPS conversion", + "URL": "http://insecure-site.testing.blog", + "capabilities": { + "edit_pages": true, + "edit_posts": true, + "manage_options": true, + "own_site": true + }, + "jetpack": true, + "jetpack_connection": true, + "was_ecommerce_trial": false, + "plan": { + "product_id": 1008, + "product_slug": "business-bundle" + }, + "jetpack_modules": [], + "options": { + "timezone": "", + "gmt_offset": 0, + "blog_public": 1, + "login_url": "http://insecure-site.testing.blog/wp-login.php", + "admin_url": "http://insecure-site.testing.blog/wp-admin/", + "is_wpcom_store": true, + "woocommerce_is_active": true, + "frame_nonce": "abc123def4" + } + } + ] +} diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 6191fa4834a..608ae7dadc9 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -5,6 +5,7 @@ ----- - [**] We added support for collecting in-person payments (including Tap To Pay) using Stripe Payment Gateway extension in the UK. [https://github.com/woocommerce/woocommerce-ios/pull/16287] - [*] Improve card payments onboarding error handling to show network errors correctly [https://github.com/woocommerce/woocommerce-ios/pull/16304] +- [*] Authenticate the admin page automatically for sites with SSO enabled in custom fields, in-person payment setup, and editing tax rates flows. [https://github.com/woocommerce/woocommerce-ios/pull/16318] 23.6 ----- diff --git a/WooCommerce/Classes/ViewRelated/Custom Fields/CustomFieldsListView.swift b/WooCommerce/Classes/ViewRelated/Custom Fields/CustomFieldsListView.swift index 822d1b32f87..23ed0c8e27b 100644 --- a/WooCommerce/Classes/ViewRelated/Custom Fields/CustomFieldsListView.swift +++ b/WooCommerce/Classes/ViewRelated/Custom Fields/CustomFieldsListView.swift @@ -173,7 +173,7 @@ struct CustomFieldsListView: View { }) { CustomFieldRow(title: customField.key, content: customField.value.removedHTMLTags, - contentURL: nil) + contentURL: customField.valueURL) } } .listStyle(.plain) @@ -225,9 +225,11 @@ private struct CustomFieldRow: View { Text(content) .font(.footnote) .foregroundColor(Color(.textLink)) - .safariSheet(url: $displayedURL) + .sheet(item: $displayedURL) { url in + AuthenticatableWebView(url: url) + } .onTapGesture { - switch url.scheme { + switch url.scheme?.lowercased() { case "http", "https": displayedURL = url // Open in `SafariSheet` in app default: diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/Onboarding Errors/InPersonPaymentsPluginNotSetup.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/Onboarding Errors/InPersonPaymentsPluginNotSetup.swift index 169e7e62234..8e2cf5f2bfb 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/Onboarding Errors/InPersonPaymentsPluginNotSetup.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/Onboarding Errors/InPersonPaymentsPluginNotSetup.swift @@ -53,7 +53,9 @@ struct InPersonPaymentsPluginNotSetup: View { tappedAnalyticEvent: learnMoreAnalyticEvent)) .padding(.vertical, 8) } - .safariSheet(url: $presentedSetupURL, onDismiss: onRefresh) + .sheet(item: $presentedSetupURL, onDismiss: onRefresh) { url in + AuthenticatableWebView(url: url, title: Localization.primaryButton) + } } private var setupURL: URL? { diff --git a/WooCommerce/Classes/ViewRelated/Orders/Order Creation/Taxes/NewTaxRateSelectorView.swift b/WooCommerce/Classes/ViewRelated/Orders/Order Creation/Taxes/NewTaxRateSelectorView.swift index c10a3e4e869..e25c2d2fae0 100644 --- a/WooCommerce/Classes/ViewRelated/Orders/Order Creation/Taxes/NewTaxRateSelectorView.swift +++ b/WooCommerce/Classes/ViewRelated/Orders/Order Creation/Taxes/NewTaxRateSelectorView.swift @@ -113,10 +113,14 @@ struct NewTaxRateSelectorView: View { } } .wooNavigationBarStyle() - .safariSheet(isPresented: $showingWPAdminWebView, url: viewModel.wpAdminTaxSettingsURL, onDismiss: { + .sheet(isPresented: $showingWPAdminWebView, onDismiss: { viewModel.onRefreshAction() onDismissWpAdminWebView() showingWPAdminWebView = false + }, content: { + if let url = viewModel.wpAdminTaxSettingsURL { + AuthenticatableWebView(url: url, title: Localization.editTaxRatesInWpAdminButtonTitle) + } }) } } diff --git a/WooCommerce/Classes/ViewRelated/ReusableViews/SwiftUI Components/AuthenticatedWebView.swift b/WooCommerce/Classes/ViewRelated/ReusableViews/SwiftUI Components/AuthenticatedWebView.swift index 64d9bf10d9b..fb4b8f55ba9 100644 --- a/WooCommerce/Classes/ViewRelated/ReusableViews/SwiftUI Components/AuthenticatedWebView.swift +++ b/WooCommerce/Classes/ViewRelated/ReusableViews/SwiftUI Components/AuthenticatedWebView.swift @@ -1,5 +1,6 @@ import SwiftUI import WebKit +import WooFoundation /// A default view model for authenticated web view /// @@ -114,3 +115,40 @@ struct AuthenticatedWebView_Previews: PreviewProvider { } } #endif + +/// A web view that can be authenticated automatically if possible ╮(─▽─)╭ +struct AuthenticatableWebView: View { + let url: URL + var title: String = "" + + @Environment(\.dismiss) var dismiss + + var body: some View { + NavigationStack { + let stores = ServiceLocator.stores + let site = stores.sessionManager.defaultSite + if let site, stores.shouldAuthenticateAdminPage(for: site) { + AuthenticatedWebView(isPresented: .constant(true), url: url) + .navigationTitle(title) + .navigationBarTitleDisplayMode(.inline) + .toolbar { + ToolbarItem(placement: .confirmationAction) { + Button(Localization.doneButton, action: { dismiss() }) + } + } + } else { + SafariSheetView(url: url) + } + } + } +} + +private extension AuthenticatableWebView { + enum Localization { + static let doneButton = NSLocalizedString( + "authenticatableWebView.done", + value: "Done", + comment: "Button to dismiss a web view" + ) + } +} diff --git a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift index 138429ac5c0..cd55389f49b 100644 --- a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift +++ b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift @@ -378,10 +378,8 @@ class DefaultStoresManager: StoresManager { } func shouldAuthenticateAdminPage(for site: Site) -> Bool { - /// If the site is self-hosted and user is authenticated with WPCom, - /// `AuthenticatedWebView` will attempt to authenticate and redirect to the admin page and fails. - /// This should be prevented 💀⛔️ - guard site.isWordPressComStore || isAuthenticatedWithoutWPCom else { + /// Auto-authentication for web view works if the site has SSO or if user is authenticated with site credentials + guard site.hasSSOEnabled || isAuthenticatedWithoutWPCom else { return false } return true @@ -786,6 +784,9 @@ private extension DefaultStoresManager { guard case .success(let site) = result else { return } + sessionManager.defaultSite = site + updateAndReloadWidgetInformation(with: siteID) + /// Triggers root endpoint to check if application password is available dispatch(SettingAction.retrieveSiteAPI(siteID: siteID) { [weak self] result in guard let self else { return } @@ -794,9 +795,8 @@ private extension DefaultStoresManager { let updatedSite = site.copy(applicationPasswordAvailable: siteAPI.applicationPasswordAvailable) sessionManager.defaultSite = updatedSite updateAndReloadWidgetInformation(with: siteID) - case .failure: - sessionManager.defaultSite = site - updateAndReloadWidgetInformation(with: siteID) + case .failure(let error): + DDLogError("⛔️ Cannot trigger root endpoint: \(error)") } }) } diff --git a/WooCommerce/WooCommerceTests/ViewRelated/HubMenu/HubMenuViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/HubMenu/HubMenuViewModelTests.swift index 70a7ffb38ad..8043a4d5e36 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/HubMenu/HubMenuViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/HubMenu/HubMenuViewModelTests.swift @@ -479,12 +479,12 @@ final class HubMenuViewModelTests: XCTestCase { } @MainActor - func test_shouldAuthenticateAdminPage_returns_true_when_logged_in_with_wpcom_to_wpcom_site() { + func test_shouldAuthenticateAdminPage_returns_true_when_logged_in_with_wpcom_to_sso_site() { // Given let sampleStoreURL = "https://testshop.com" let sampleAdminURL = "" let sessionManager = SessionManager.makeForTesting(authenticated: true, isWPCom: true) - let site = Site.fake().copy(url: sampleStoreURL, adminURL: sampleAdminURL, isWordPressComStore: true) + let site = Site.fake().copy(url: sampleStoreURL, adminURL: sampleAdminURL, hasSSOEnabled: true) sessionManager.defaultSite = site let stores = MockStoresManager(sessionManager: sessionManager)