Skip to content

Commit 6e32098

Browse files
authored
Improve authentication logic for authenticated web view (#15164)
2 parents ced5c64 + 75ec99a commit 6e32098

File tree

8 files changed

+433
-41
lines changed

8 files changed

+433
-41
lines changed

RELEASE-NOTES.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
- [*] Background image upload: Fix missing error notice in iPhones [https://github.com/woocommerce/woocommerce-ios/pull/15117]
1212
- [*] Background image upload: Show a notice when the user leaves product details while uploads are pending [https://github.com/woocommerce/woocommerce-ios/pull/15134]
1313
- [*] Filters applied in product selector no longer affect the main product list screen. [https://github.com/woocommerce/woocommerce-ios/pull/14764]
14+
- [Internal] Improve authentication logic for authenticated web view [https://github.com/woocommerce/woocommerce-ios/pull/15164]
1415
- [*] Better error messages if Application Password login is disabled on user's website. [https://github.com/woocommerce/woocommerce-ios/pull/15031]
1516
- [**] Product Images: Update error handling [https://github.com/woocommerce/woocommerce-ios/pull/15105]
1617
- [*] Merchants can mark and filter favorite products for quicker access. [https://github.com/woocommerce/woocommerce-ios/pull/14597]

WooCommerce/Classes/Authentication/AuthenticatedWebViewController.swift

Lines changed: 125 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
import Combine
22
import UIKit
33
import WebKit
4+
import Yosemite
45
import class Networking.UserAgent
56
import struct WordPressAuthenticator.WordPressOrgCredentials
6-
import enum Yosemite.Credentials
77

88
/// A web view which is authenticated for WordPress.com, when possible.
99
///
1010
final class AuthenticatedWebViewController: UIViewController {
1111

12+
private let currentSite: Site?
1213
private let viewModel: AuthenticatedWebViewModel
14+
private let authenticationFlow: WebViewAuthenticationFlow
1315

1416
private lazy var activityIndicator: UIActivityIndicatorView = {
1517
let activityIndicator = UIActivityIndicatorView(style: .medium)
@@ -45,12 +47,15 @@ final class AuthenticatedWebViewController: UIViewController {
4547

4648
private let wpcomCredentials: Credentials?
4749

50+
private var isFirstNavigation = true
4851

49-
init(viewModel: AuthenticatedWebViewModel, extraCredentials: Credentials? = nil) {
52+
init(stores: StoresManager = ServiceLocator.stores,
53+
viewModel: AuthenticatedWebViewModel,
54+
extraCredentials: Credentials? = nil) {
5055
self.viewModel = viewModel
51-
let currentCredentials = ServiceLocator.stores.sessionManager.defaultCredentials
56+
let currentCredentials = stores.sessionManager.defaultCredentials
5257

53-
self.siteCredentials = {
58+
let siteCredentials: WordPressOrgCredentials? = {
5459
if case let .wporg(username, password, siteAddress) = extraCredentials {
5560
return WordPressOrgCredentials(username: username,
5661
password: password,
@@ -65,14 +70,29 @@ final class AuthenticatedWebViewController: UIViewController {
6570
return nil
6671
}()
6772

68-
self.wpcomCredentials = {
73+
let wpcomCredentials: Credentials? = {
6974
if case .wpcom = extraCredentials {
7075
return extraCredentials
7176
} else if case .wpcom = currentCredentials {
7277
return currentCredentials
7378
}
7479
return nil
7580
}()
81+
82+
let currentSite = stores.sessionManager.defaultSite
83+
84+
self.authenticationFlow = {
85+
guard let currentSite else {
86+
return WebViewAuthenticationFlow.none
87+
}
88+
return viewModel.authenticationFlow(currentSite: currentSite,
89+
wpcomCredentialsAvailable: wpcomCredentials != nil,
90+
wporgCredentialsAvailable: siteCredentials != nil)
91+
}()
92+
self.currentSite = currentSite
93+
self.wpcomCredentials = wpcomCredentials
94+
self.siteCredentials = siteCredentials
95+
7696
super.init(nibName: nil, bundle: nil)
7797

7898
if let initialURL = viewModel.initialURL,
@@ -93,6 +113,7 @@ final class AuthenticatedWebViewController: UIViewController {
93113
configureWebView()
94114
configureActivityIndicator()
95115
configureProgressBar()
116+
observeWebView()
96117
startLoading()
97118
}
98119

@@ -144,7 +165,7 @@ private extension AuthenticatedWebViewController {
144165
])
145166
}
146167

147-
func startLoading() {
168+
func observeWebView() {
148169
webView.publisher(for: \.estimatedProgress)
149170
.sink { [weak self] progress in
150171
if progress == 1 {
@@ -157,42 +178,98 @@ private extension AuthenticatedWebViewController {
157178

158179
webView.publisher(for: \.url)
159180
.sink { [weak self] url in
160-
guard let self else { return }
161-
let initialURL = self.viewModel.initialURL
162-
// avoids infinite loop if the initial url happens to be the nonce retrieval path.
163-
if url?.absoluteString.contains(WKWebView.wporgNoncePath) == true,
164-
initialURL?.absoluteString.contains(WKWebView.wporgNoncePath) != true {
165-
self.loadContent()
166-
} else {
167-
self.viewModel.handleRedirect(for: url)
168-
}
181+
guard let url else { return }
182+
self?.handleRedirect(for: url)
169183
}
170184
.store(in: &subscriptions)
185+
}
171186

172-
if let siteCredentials, let request = try? webView.authenticateForWPOrg(with: siteCredentials) {
173-
webView.load(request)
174-
} else {
175-
loadContent()
187+
/// Authentication logic differs depending on the destination URL and the current site.
188+
/// More information: pe5sF9-3Si-p2
189+
///
190+
func startLoading() {
191+
guard let url = viewModel.initialURL else {
192+
return
193+
}
194+
195+
switch authenticationFlow {
196+
case .wpcom:
197+
authenticateWPComAndLoadContent(url: url)
198+
case .jetpackSSO:
199+
authenticateSSOAndLoadContent(url: url)
200+
case .siteCredentials:
201+
authenticateUsingSiteCredentialsAndLoadContent(url: url)
202+
case .none:
203+
loadContent(url: url)
176204
}
177205
}
178206
}
179207

180208
// MARK: - Helper methods
181209
private extension AuthenticatedWebViewController {
182-
func loadContent() {
183-
guard let url = viewModel.initialURL else {
210+
func authenticateWPComAndLoadContent(url: URL) {
211+
guard let wpcomCredentials, case .wpcom = wpcomCredentials else {
212+
return loadContent(url: url)
213+
}
214+
do {
215+
try webView.authenticateForWPComAndRedirect(to: url, credentials: wpcomCredentials)
216+
} catch {
217+
loadContent(url: url)
218+
}
219+
}
220+
221+
func authenticateSSOAndLoadContent(url: URL) {
222+
let tempURL = WooConstants.URLs.wpcomTempRedirectURL.asURL()
223+
authenticateWPComAndLoadContent(url: tempURL)
224+
}
225+
226+
func authenticateUsingSiteCredentialsAndLoadContent(url: URL) {
227+
guard let siteCredentials, let request = try? webView.authenticateForWPOrg(with: siteCredentials) else {
228+
return loadContent(url: url)
229+
}
230+
webView.load(request)
231+
}
232+
233+
func loadContent(url: URL) {
234+
let request = URLRequest(url: url)
235+
webView.load(request)
236+
}
237+
238+
func handleRedirect(for url: URL) {
239+
guard let initialURL = viewModel.initialURL else {
184240
return
185241
}
186242

187-
/// Authenticate for WP.com automatically if credentials are available.
188-
///
189-
if let wpcomCredentials, case .wpcom = wpcomCredentials {
190-
webView.authenticateForWPComAndRedirect(to: url, credentials: wpcomCredentials)
191-
} else {
192-
let request = URLRequest(url: url)
193-
webView.load(request)
243+
switch url.absoluteString {
244+
case WooConstants.URLs.wpcomTempRedirectURL.rawValue:
245+
guard let currentSite, let host = URL(string: currentSite.url)?.host else {
246+
return loadContent(url: initialURL)
247+
}
248+
let cookie = HTTPCookie(properties: [
249+
.domain: host,
250+
.path: "/",
251+
.name: Constants.ssoRedirectCookieName,
252+
.value: initialURL.absoluteString,
253+
])
254+
255+
let queryItem = URLQueryItem(name: Constants.actionParam, value: Constants.jetpackSSOAction)
256+
guard let cookie, let loginURL = URL(string: currentSite.loginURL)?.appending(queryItems: [queryItem]) else {
257+
return loadContent(url: initialURL)
258+
}
259+
webView.configuration.websiteDataStore.httpCookieStore.setCookie(cookie)
260+
loadContent(url: loginURL)
261+
262+
default:
263+
if url.absoluteString.contains(WKWebView.wporgNoncePath) == true,
264+
initialURL.absoluteString.contains(WKWebView.wporgNoncePath) != true {
265+
// Site credentials login completes, now proceed to load the initial URL.
266+
loadContent(url: initialURL)
267+
} else {
268+
viewModel.handleRedirect(for: url)
269+
}
194270
}
195271
}
272+
196273
}
197274

198275
extension AuthenticatedWebViewController: WKNavigationDelegate {
@@ -204,7 +281,19 @@ extension AuthenticatedWebViewController: WKNavigationDelegate {
204281
}
205282

206283
func webView(_ webView: WKWebView, decidePolicyFor navigationResponse: WKNavigationResponse) async -> WKNavigationResponsePolicy {
284+
defer {
285+
isFirstNavigation = false
286+
}
207287
let response = navigationResponse.response
288+
if let initialURL = viewModel.initialURL,
289+
viewModel.isAuthenticationFailure(response: response,
290+
currentSite: currentSite,
291+
authenticationFlow: authenticationFlow,
292+
isFirstNavigation: isFirstNavigation) {
293+
/// When automatic authentication fails, cancel the navigation and redirect to the original URL instead.
294+
loadContent(url: initialURL)
295+
return .cancel
296+
}
208297
return await viewModel.decidePolicy(for: response)
209298
}
210299

@@ -244,3 +333,11 @@ extension AuthenticatedWebViewController: WKUIDelegate {
244333
return nil
245334
}
246335
}
336+
337+
private extension AuthenticatedWebViewController {
338+
enum Constants {
339+
static let actionParam = "action"
340+
static let jetpackSSOAction = "jetpack-sso"
341+
static let ssoRedirectCookieName = "jetpack_sso_redirect_to"
342+
}
343+
}

WooCommerce/Classes/Authentication/AuthenticatedWebViewModel.swift

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import Foundation
22
import WebKit
3+
import struct Yosemite.Site
34

45
/// Optional conformance for a `AuthenticatedWebViewModel` implementation to reload a webview asynchronously.
56
protocol WebviewReloadable {
@@ -70,4 +71,78 @@ extension AuthenticatedWebViewModel {
7071
}
7172
return false
7273
}
74+
75+
/// Checks for the appropriate authentication flow based on the current site and the URL to be loaded.
76+
///
77+
func authenticationFlow(currentSite: Site,
78+
wpcomCredentialsAvailable: Bool,
79+
wporgCredentialsAvailable: Bool) -> WebViewAuthenticationFlow {
80+
guard let initialURL else {
81+
return .none
82+
}
83+
84+
if wpcomCredentialsAvailable {
85+
if let domain = initialURL.host, Constants.wpcomAcceptedDomains.contains(domain) {
86+
return .wpcom
87+
} else if currentSite.hasSSOEnabled,
88+
initialURL.absoluteString.hasPrefix(currentSite.url) {
89+
return .jetpackSSO
90+
}
91+
}
92+
93+
if wporgCredentialsAvailable {
94+
return .siteCredentials
95+
} else {
96+
return .none
97+
}
98+
}
99+
100+
/// Returns `true` if the response indicates an authentication failure and `false` otherwise.
101+
///
102+
func isAuthenticationFailure(response: URLResponse,
103+
currentSite: Site?,
104+
authenticationFlow: WebViewAuthenticationFlow,
105+
isFirstNavigation: Bool) -> Bool {
106+
guard authenticationFlow != .none,
107+
isFirstNavigation,
108+
let currentSite,
109+
let urlResponse = response as? HTTPURLResponse,
110+
let url = urlResponse.url else {
111+
return false
112+
}
113+
114+
// if the authentication request fails
115+
if Constants.errorResponseCodes ~= urlResponse.statusCode {
116+
return true
117+
}
118+
119+
let isAuthenticationFailure: Bool = {
120+
switch authenticationFlow {
121+
case .none:
122+
return false
123+
case .siteCredentials:
124+
return url.absoluteString == currentSite.loginURL
125+
case .jetpackSSO, .wpcom:
126+
return url.absoluteString == WooConstants.URLs.loginWPCom.rawValue
127+
}
128+
}()
129+
130+
guard isAuthenticationFailure else {
131+
return false
132+
}
133+
134+
return true
135+
}
136+
}
137+
138+
enum WebViewAuthenticationFlow {
139+
case wpcom
140+
case jetpackSSO
141+
case siteCredentials
142+
case none
143+
}
144+
145+
private enum Constants {
146+
static let errorResponseCodes = 400...599
147+
static let wpcomAcceptedDomains = ["wordpress.com", "wp.com", "jetpack.com", "woocommerce.com", "jetpack.wordpress.com"]
73148
}

WooCommerce/Classes/Extensions/WKWebView+Authenticated.swift

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,9 @@ extension WKWebView {
2727
return try URLEncoding.default.encode(request, with: parameters)
2828
}
2929

30-
func authenticateForWPComAndRedirect(to url: URL, credentials: Credentials?) {
30+
func authenticateForWPComAndRedirect(to url: URL, credentials: Credentials?) throws {
3131
customUserAgent = UserAgent.defaultUserAgent
32-
do {
33-
try load(authenticatedPostData(with: credentials, redirectTo: url))
34-
} catch {
35-
DDLogError("⛔️ Cannot load the authenticated web view on WPCom")
36-
}
32+
try load(authenticatedPostData(with: credentials, redirectTo: url))
3733
}
3834

3935
private func authenticatedPostData(with credentials: Credentials?, redirectTo url: URL) throws -> URLRequest {

WooCommerce/Classes/System/WooConstants.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,10 @@ extension WooConstants {
250250
///
251251
case pointOfSaleDocumentation = "https://woocommerce.com/document/woo-mobile-app-point-of-sale-mode/"
252252

253+
/// Temporary redirect URL for authenticated web view when authenticating WPCom automatically
254+
///
255+
case wpcomTempRedirectURL = "https://wordpress.com/mobile-redirect"
256+
253257
#if DEBUG
254258
case orderCreationFeedback = "https://automattic.survey.fm/woo-app-order-creation-testing"
255259
#else

WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,13 +1065,6 @@ private extension ProductFormViewController {
10651065
return
10661066
}
10671067

1068-
let stores = ServiceLocator.stores
1069-
guard let site = stores.sessionManager.defaultSite,
1070-
stores.shouldAuthenticateAdminPage(for: site) else {
1071-
WebviewHelper.launch(url.absoluteString, with: self)
1072-
return
1073-
}
1074-
10751068
let viewModel = DefaultAuthenticatedWebViewModel(title: product.name, initialURL: url)
10761069
let controller = AuthenticatedWebViewController(viewModel: viewModel)
10771070
let navigationController = UINavigationController(rootViewController: controller)

0 commit comments

Comments
 (0)