Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
11 changes: 6 additions & 5 deletions Modules/Sources/Networking/Model/Site.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -360,7 +361,7 @@ public extension Site {
}

func toJetpackSite() -> JetpackSite {
JetpackSite(siteID: siteID, siteAddress: jetpackCanonicalURL, applicationPasswordAvailable: applicationPasswordAvailable)
JetpackSite(siteID: siteID, siteAddress: url, applicationPasswordAvailable: applicationPasswordAvailable)
}
}

Expand Down
19 changes: 19 additions & 0 deletions Modules/Tests/NetworkingTests/Mapper/SiteListMapperTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -55,4 +70,8 @@ private extension SiteListMapperTests {
func mapLoadMalformedSiteListResponse() -> [Site] {
return mapSiteListData(from: "sites-malformed")
}

func mapLoadHTTPSiteListResponse() -> [Site] {
return mapSiteListData(from: "sites-http")
}
}
28 changes: 0 additions & 28 deletions Modules/Tests/NetworkingTests/Model/SiteTests.swift

This file was deleted.

34 changes: 34 additions & 0 deletions Modules/Tests/NetworkingTests/Responses/sites-http.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
]
}
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ struct CustomFieldsListView: View {
}) {
CustomFieldRow(title: customField.key,
content: customField.value.removedHTMLTags,
contentURL: nil)
contentURL: customField.valueURL)
}
}
.listStyle(.plain)
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import SwiftUI
import WebKit
import WooFoundation

/// A default view model for authenticated web view
///
Expand Down Expand Up @@ -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"
)
}
}
14 changes: 7 additions & 7 deletions WooCommerce/Classes/Yosemite/DefaultStoresManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 }
Expand All @@ -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)")
}
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down