Skip to content

Commit b66223a

Browse files
authored
Auto authenticate users in web views (#16318)
2 parents 6a0f960 + 58a0ac8 commit b66223a

File tree

11 files changed

+120
-47
lines changed

11 files changed

+120
-47
lines changed

Modules/Sources/Networking/Model/Site.swift

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public struct Site: Decodable, Equatable, Hashable, GeneratedFakeable, Generated
117117
let siteID = try siteContainer.decode(Int64.self, forKey: .siteID)
118118
let name = try siteContainer.decode(String.self, forKey: .name)
119119
let description = try siteContainer.decode(String.self, forKey: .description)
120-
let url = try siteContainer.decode(String.self, forKey: .url)
120+
let url = Self.safeURL(try siteContainer.decode(String.self, forKey: .url))
121121
let capabilitiesContainer = try siteContainer.nestedContainer(keyedBy: CapabilitiesKeys.self, forKey: .capabilities)
122122
let isSiteOwner = try capabilitiesContainer.decode(Bool.self, forKey: .isSiteOwner)
123123
let isAdmin = try capabilitiesContainer.decode(Bool.self, forKey: .isAdmin)
@@ -130,8 +130,8 @@ public struct Site: Decodable, Equatable, Hashable, GeneratedFakeable, Generated
130130
let jetpackConnectionActivePlugins = try optionsContainer.decodeIfPresent([String].self, forKey: .jetpackConnectionActivePlugins) ?? []
131131
let timezone = try optionsContainer.decode(String.self, forKey: .timezone)
132132
let gmtOffset = try optionsContainer.decode(Double.self, forKey: .gmtOffset)
133-
let adminURL = try optionsContainer.decode(String.self, forKey: .adminURL)
134-
let loginURL = try optionsContainer.decode(String.self, forKey: .loginURL)
133+
let adminURL = Self.safeURL(try optionsContainer.decode(String.self, forKey: .adminURL))
134+
let loginURL = Self.safeURL(try optionsContainer.decode(String.self, forKey: .loginURL))
135135
let frameNonce = try optionsContainer.decode(String.self, forKey: .frameNonce)
136136
let canBlaze = optionsContainer.failsafeDecodeIfPresent(booleanForKey: .canBlaze) ?? false
137137
let visibility = optionsContainer.failsafeDecodeIfPresent(SiteVisibility.self, forKey: .visibility) ?? .privateSite
@@ -333,7 +333,8 @@ public enum SiteVisibility: Int, Codable, GeneratedFakeable {
333333
///
334334
public extension Site {
335335

336-
private var jetpackCanonicalURL: String {
336+
/// Force URL to use HTTPS if possible to avoid App Transport Security errors
337+
private static func safeURL(_ url: String) -> String {
337338
guard let originalURL = URL(string: url),
338339
originalURL.scheme?.lowercased() == "http"
339340
else {
@@ -360,7 +361,7 @@ public extension Site {
360361
}
361362

362363
func toJetpackSite() -> JetpackSite {
363-
JetpackSite(siteID: siteID, siteAddress: jetpackCanonicalURL, applicationPasswordAvailable: applicationPasswordAvailable)
364+
JetpackSite(siteID: siteID, siteAddress: url, applicationPasswordAvailable: applicationPasswordAvailable)
364365
}
365366
}
366367

Modules/Tests/NetworkingTests/Mapper/SiteListMapperTests.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,21 @@ final class SiteListMapperTests: XCTestCase {
3737
let second = try XCTUnwrap(sites[safe: 1])
3838
XCTAssertFalse(second.hasSSOEnabled)
3939
}
40+
41+
func test_http_urls_are_converted_to_https() throws {
42+
// Given
43+
let sites = mapLoadHTTPSiteListResponse()
44+
45+
// Then
46+
let site = try XCTUnwrap(sites.first)
47+
XCTAssertTrue(site.url.hasPrefix("https://"), "Site URL should be converted to HTTPS")
48+
XCTAssertTrue(site.adminURL.hasPrefix("https://"), "Admin URL should be converted to HTTPS")
49+
XCTAssertTrue(site.loginURL.hasPrefix("https://"), "Login URL should be converted to HTTPS")
50+
51+
XCTAssertEqual(site.url, "https://insecure-site.testing.blog")
52+
XCTAssertEqual(site.adminURL, "https://insecure-site.testing.blog/wp-admin/")
53+
XCTAssertEqual(site.loginURL, "https://insecure-site.testing.blog/wp-login.php")
54+
}
4055
}
4156

4257
private extension SiteListMapperTests {
@@ -55,4 +70,8 @@ private extension SiteListMapperTests {
5570
func mapLoadMalformedSiteListResponse() -> [Site] {
5671
return mapSiteListData(from: "sites-malformed")
5772
}
73+
74+
func mapLoadHTTPSiteListResponse() -> [Site] {
75+
return mapSiteListData(from: "sites-http")
76+
}
5877
}

Modules/Tests/NetworkingTests/Model/SiteTests.swift

Lines changed: 0 additions & 28 deletions
This file was deleted.
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
{
2+
"sites": [
3+
{
4+
"ID": 7777888899990000,
5+
"name": "HTTP Site",
6+
"description": "Testing HTTP to HTTPS conversion",
7+
"URL": "http://insecure-site.testing.blog",
8+
"capabilities": {
9+
"edit_pages": true,
10+
"edit_posts": true,
11+
"manage_options": true,
12+
"own_site": true
13+
},
14+
"jetpack": true,
15+
"jetpack_connection": true,
16+
"was_ecommerce_trial": false,
17+
"plan": {
18+
"product_id": 1008,
19+
"product_slug": "business-bundle"
20+
},
21+
"jetpack_modules": [],
22+
"options": {
23+
"timezone": "",
24+
"gmt_offset": 0,
25+
"blog_public": 1,
26+
"login_url": "http://insecure-site.testing.blog/wp-login.php",
27+
"admin_url": "http://insecure-site.testing.blog/wp-admin/",
28+
"is_wpcom_store": true,
29+
"woocommerce_is_active": true,
30+
"frame_nonce": "abc123def4"
31+
}
32+
}
33+
]
34+
}

RELEASE-NOTES.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
-----
66
- [**] 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]
77
- [*] Improve card payments onboarding error handling to show network errors correctly [https://github.com/woocommerce/woocommerce-ios/pull/16304]
8+
- [*] 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]
89

910
23.6
1011
-----

WooCommerce/Classes/ViewRelated/Custom Fields/CustomFieldsListView.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ struct CustomFieldsListView: View {
173173
}) {
174174
CustomFieldRow(title: customField.key,
175175
content: customField.value.removedHTMLTags,
176-
contentURL: nil)
176+
contentURL: customField.valueURL)
177177
}
178178
}
179179
.listStyle(.plain)
@@ -225,9 +225,11 @@ private struct CustomFieldRow: View {
225225
Text(content)
226226
.font(.footnote)
227227
.foregroundColor(Color(.textLink))
228-
.safariSheet(url: $displayedURL)
228+
.sheet(item: $displayedURL) { url in
229+
AuthenticatableWebView(url: url)
230+
}
229231
.onTapGesture {
230-
switch url.scheme {
232+
switch url.scheme?.lowercased() {
231233
case "http", "https":
232234
displayedURL = url // Open in `SafariSheet` in app
233235
default:

WooCommerce/Classes/ViewRelated/Dashboard/Settings/In-Person Payments/Onboarding Errors/InPersonPaymentsPluginNotSetup.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@ struct InPersonPaymentsPluginNotSetup: View {
5353
tappedAnalyticEvent: learnMoreAnalyticEvent))
5454
.padding(.vertical, 8)
5555
}
56-
.safariSheet(url: $presentedSetupURL, onDismiss: onRefresh)
56+
.sheet(item: $presentedSetupURL, onDismiss: onRefresh) { url in
57+
AuthenticatableWebView(url: url, title: Localization.primaryButton)
58+
}
5759
}
5860

5961
private var setupURL: URL? {

WooCommerce/Classes/ViewRelated/Orders/Order Creation/Taxes/NewTaxRateSelectorView.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,14 @@ struct NewTaxRateSelectorView: View {
113113
}
114114
}
115115
.wooNavigationBarStyle()
116-
.safariSheet(isPresented: $showingWPAdminWebView, url: viewModel.wpAdminTaxSettingsURL, onDismiss: {
116+
.sheet(isPresented: $showingWPAdminWebView, onDismiss: {
117117
viewModel.onRefreshAction()
118118
onDismissWpAdminWebView()
119119
showingWPAdminWebView = false
120+
}, content: {
121+
if let url = viewModel.wpAdminTaxSettingsURL {
122+
AuthenticatableWebView(url: url, title: Localization.editTaxRatesInWpAdminButtonTitle)
123+
}
120124
})
121125
}
122126
}

WooCommerce/Classes/ViewRelated/ReusableViews/SwiftUI Components/AuthenticatedWebView.swift

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import SwiftUI
22
import WebKit
3+
import WooFoundation
34

45
/// A default view model for authenticated web view
56
///
@@ -114,3 +115,40 @@ struct AuthenticatedWebView_Previews: PreviewProvider {
114115
}
115116
}
116117
#endif
118+
119+
/// A web view that can be authenticated automatically if possible ╮(─▽─)╭
120+
struct AuthenticatableWebView: View {
121+
let url: URL
122+
var title: String = ""
123+
124+
@Environment(\.dismiss) var dismiss
125+
126+
var body: some View {
127+
NavigationStack {
128+
let stores = ServiceLocator.stores
129+
let site = stores.sessionManager.defaultSite
130+
if let site, stores.shouldAuthenticateAdminPage(for: site) {
131+
AuthenticatedWebView(isPresented: .constant(true), url: url)
132+
.navigationTitle(title)
133+
.navigationBarTitleDisplayMode(.inline)
134+
.toolbar {
135+
ToolbarItem(placement: .confirmationAction) {
136+
Button(Localization.doneButton, action: { dismiss() })
137+
}
138+
}
139+
} else {
140+
SafariSheetView(url: url)
141+
}
142+
}
143+
}
144+
}
145+
146+
private extension AuthenticatableWebView {
147+
enum Localization {
148+
static let doneButton = NSLocalizedString(
149+
"authenticatableWebView.done",
150+
value: "Done",
151+
comment: "Button to dismiss a web view"
152+
)
153+
}
154+
}

WooCommerce/Classes/Yosemite/DefaultStoresManager.swift

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -378,10 +378,8 @@ class DefaultStoresManager: StoresManager {
378378
}
379379

380380
func shouldAuthenticateAdminPage(for site: Site) -> Bool {
381-
/// If the site is self-hosted and user is authenticated with WPCom,
382-
/// `AuthenticatedWebView` will attempt to authenticate and redirect to the admin page and fails.
383-
/// This should be prevented 💀⛔️
384-
guard site.isWordPressComStore || isAuthenticatedWithoutWPCom else {
381+
/// Auto-authentication for web view works if the site has SSO or if user is authenticated with site credentials
382+
guard site.hasSSOEnabled || isAuthenticatedWithoutWPCom else {
385383
return false
386384
}
387385
return true
@@ -786,6 +784,9 @@ private extension DefaultStoresManager {
786784
guard case .success(let site) = result else {
787785
return
788786
}
787+
sessionManager.defaultSite = site
788+
updateAndReloadWidgetInformation(with: siteID)
789+
789790
/// Triggers root endpoint to check if application password is available
790791
dispatch(SettingAction.retrieveSiteAPI(siteID: siteID) { [weak self] result in
791792
guard let self else { return }
@@ -794,9 +795,8 @@ private extension DefaultStoresManager {
794795
let updatedSite = site.copy(applicationPasswordAvailable: siteAPI.applicationPasswordAvailable)
795796
sessionManager.defaultSite = updatedSite
796797
updateAndReloadWidgetInformation(with: siteID)
797-
case .failure:
798-
sessionManager.defaultSite = site
799-
updateAndReloadWidgetInformation(with: siteID)
798+
case .failure(let error):
799+
DDLogError("⛔️ Cannot trigger root endpoint: \(error)")
800800
}
801801
})
802802
}

0 commit comments

Comments
 (0)