Skip to content

Commit

Permalink
[Login] Improve error messaging displaying a new error screen when ap…
Browse files Browse the repository at this point in the history
…plication password is disabled (#15031)
  • Loading branch information
pmusolino authored Feb 19, 2025
2 parents f5ccde0 + 5dd3e83 commit b71385b
Show file tree
Hide file tree
Showing 9 changed files with 172 additions and 59 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Foundation

enum RequestAuthenticatorError: Error {
public enum RequestAuthenticatorError: Error {
case applicationPasswordUseCaseNotAvailable
case applicationPasswordNotAvailable
}
Expand Down
5 changes: 4 additions & 1 deletion RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@
- [*] Background image upload: Fix missing error notice in iPhones [https://github.com/woocommerce/woocommerce-ios/pull/15117]
- [*] Background image upload: Show a notice when the user leaves product details while uploads are pending [https://github.com/woocommerce/woocommerce-ios/pull/15134]
- [*] Filters applied in product selector no longer affect the main product list screen. [https://github.com/woocommerce/woocommerce-ios/pull/14764]
- [*] Better error messages if Application Password login is disabled on user's website. [https://github.com/woocommerce/woocommerce-ios/pull/15031]
- [**] Product Images: Update error handling [https://github.com/woocommerce/woocommerce-ios/pull/15105]

21.7
-----
- [**] Fixed an issue with the WordPress Media Library not loading due to a decoding error in media details sizes. [https://github.com/woocommerce/woocommerce-ios/pull/15056]
- [*] Orders: Improved accessibility UI for Tax Rates related views [https://github.com/woocommerce/woocommerce-ios/pull/15043]
- [*] Now, usernames and emails in text fields across multiple login views are no longer capitalized. [https://github.com/woocommerce/woocommerce-ios/pull/15002]
- [*] Product Details: Display cover tag on the first product image [https://github.com/woocommerce/woocommerce-ios/pull/15041]
- [*] Payments: Update learn more links to open Stripe-specific docs when using that gateway [https://github.com/woocommerce/woocommerce-ios/pull/15035]
- [*] Orders: Improved accessibility UI for Tax Rates related views [https://github.com/woocommerce/woocommerce-ios/pull/15043]
- [*] Product Details: Display cover tag on the first product image [https://github.com/woocommerce/woocommerce-ios/pull/15041]
- [*] Now, usernames and emails in text fields across multiple login views are no longer capitalized. [https://github.com/woocommerce/woocommerce-ios/pull/15002]

21.6
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Yosemite
/// View model for `ApplicationPasswordAuthorizationWebViewController`.
///
final class ApplicationPasswordAuthorizationViewModel {
private let siteURL: String
let siteURL: String
private let stores: StoresManager

init(siteURL: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import UIKit
import WebKit
import protocol WooFoundation.Analytics
import struct Networking.ApplicationPassword
import enum Networking.RequestAuthenticatorError

/// View with embedded web view to authorize application password for a site.
///
Expand Down Expand Up @@ -34,6 +35,9 @@ final class ApplicationPasswordAuthorizationWebViewController: UIViewController
return indicator
}()

/// The view controller that was presenting the application password flow.
private var previousViewController: UIViewController?

/// WP Core requires that the UUID has lowercased letters.
private let appID = UUID().uuidString.lowercased()

Expand All @@ -50,9 +54,11 @@ final class ApplicationPasswordAuthorizationWebViewController: UIViewController
private var authorizationURL: String?

init(viewModel: ApplicationPasswordAuthorizationViewModel,
previousViewController: UIViewController?,
analytics: Analytics = ServiceLocator.analytics,
onSuccess: @escaping (ApplicationPassword, UINavigationController?) -> Void) {
self.viewModel = viewModel
self.previousViewController = previousViewController
self.onSuccess = onSuccess
self.analytics = analytics
super.init(nibName: nil, bundle: nil)
Expand Down Expand Up @@ -157,20 +163,46 @@ private extension ApplicationPasswordAuthorizationWebViewController {
guard let url = try await viewModel.fetchAuthURL() else {
DDLogError("⛔️ No authorization URL found for application passwords")
analytics.track(.applicationPasswordAuthorizationURLNotAvailable)
return showErrorAlert(message: Localization.applicationPasswordDisabled)
navigateToApplicationPasswordDisabledUI()
return
}
loadAuthorizationPage(url: url)
} catch {
DDLogError("⛔️ Error fetching authorization URL for application passwords \(error)")
analytics.track(.applicationPasswordAuthorizationURLFetchFailed, withError: error)
showErrorAlert(message: Localization.errorFetchingAuthURL, onRetry: { [weak self] in
self?.fetchAuthorizationURL()
})
if let authError = error as? Networking.RequestAuthenticatorError,
authError == .applicationPasswordNotAvailable {
navigateToApplicationPasswordDisabledUI()
} else {
showErrorAlert(message: Localization.errorFetchingAuthURL)
}
}
activityIndicator.stopAnimating()
}
}

/// Pops to the previous view controller (if provided) or pops one level otherwise.
@objc private func navigateToPreviousViewController() {
if let previousViewController, let navigationController {
navigationController.popToViewController(previousViewController, animated: true)
} else {
navigationController?.popViewController(animated: true)
}
}

private func navigateToApplicationPasswordDisabledUI() {
let errorUI = applicationPasswordDisabledUI(for: viewModel.siteURL)
// When the error view controller is popped, navigate to previous VC
errorUI.navigationItem.leftBarButtonItem = UIBarButtonItem(
image: UIImage(systemName: "chevron.backward"),
style: .plain,
target: self,
action: #selector(navigateToPreviousViewController)
)
// Push instead of present
navigationController?.pushViewController(errorUI, animated: true)
}

func loadAuthorizationPage(url: URL) {
let parameters: [URLQueryItem] = [
URLQueryItem(name: Constants.Query.appName, value: appName),
Expand Down Expand Up @@ -220,6 +252,14 @@ private extension ApplicationPasswordAuthorizationWebViewController {
}
present(alertController, animated: true)
}

/// The error screen to be displayed when the user tries to log in with site credentials
/// with application password disabled.
///
func applicationPasswordDisabledUI(for siteURL: String) -> UIViewController {
let viewModel = ApplicationPasswordDisabledViewModel(siteURL: siteURL, previousViewController: previousViewController)
return ULErrorViewController(viewModel: viewModel)
}
}

extension ApplicationPasswordAuthorizationWebViewController: WKNavigationDelegate {
Expand Down
13 changes: 8 additions & 5 deletions WooCommerce/Classes/Authentication/AuthenticationManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -685,9 +685,10 @@ private extension AuthenticationManager {

/// Web view to authorize application password for a given site.
///
func applicationPasswordWebView(for siteURL: String) -> UIViewController {
func applicationPasswordWebView(for siteURL: String, previousVC: UIViewController?) -> UIViewController {
let viewModel = ApplicationPasswordAuthorizationViewModel(siteURL: siteURL)
let controller = ApplicationPasswordAuthorizationWebViewController(viewModel: viewModel,
previousViewController: previousVC,
onSuccess: { [weak self] applicationPassword, navigationController in
guard let navigationController else {
DDLogInfo("⚠️ No navigation controller found")
Expand Down Expand Up @@ -754,13 +755,15 @@ private extension AuthenticationManager {
) else {
return assertionFailure("⛔️ Error creating application password use case")
}
checkSiteCredentialLogin(to: siteURL, with: useCase, in: navigationController)
checkSiteCredentialLogin(to: siteURL, with: useCase, in: navigationController, previousViewController: nil)
}

func checkSiteCredentialLogin(to siteURL: String,
with useCase: ApplicationPasswordUseCase,
in navigationController: UINavigationController) {
let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: useCase)
in navigationController: UINavigationController,
previousViewController: UIViewController? = nil) {
let checker = PostSiteCredentialLoginChecker(applicationPasswordUseCase: useCase,
previousViewController: previousViewController)
checker.checkEligibility(for: siteURL, from: navigationController) { [weak self] in
guard let self else { return }
// Tracking `signedIn` after the user logged in using site creds & application password is created
Expand Down Expand Up @@ -806,7 +809,7 @@ private extension AuthenticationManager {
/// Presents app password site login using a web view.
///
private func presentApplicationPasswordWebView(for siteURL: String, in viewController: UIViewController) {
let webViewController = applicationPasswordWebView(for: siteURL)
let webViewController = applicationPasswordWebView(for: siteURL, previousVC: viewController)
viewController.navigationController?.pushViewController(webViewController, animated: true)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,20 @@ import WordPressAuthenticator
/// modeling an error when application password is disabled.
///
struct ApplicationPasswordDisabledViewModel: ULErrorViewModel {
init(siteURL: String) {
init(siteURL: String,
previousViewController: UIViewController?,
authentication: Authentication = ServiceLocator.authenticationManager) {
self.siteURL = siteURL
self.previousViewController = previousViewController
self.authentication = authentication
}

let siteURL: String
let image: UIImage = .errorImage // TODO: update this if needed
let authentication: Authentication
let image: UIImage = .errorImage

// The VC that was showing before the application password flow. This is used to navigate back without guesswork.
let previousViewController: UIViewController?

var text: NSAttributedString {
let font: UIFont = .body
Expand All @@ -20,7 +28,7 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel {
attributes: [.font: boldFont])
let message = NSMutableAttributedString(string: Localization.errorMessage)

message.replaceFirstOccurrence(of: "%@", with: boldSiteAddress)
message.replaceFirstOccurrence(of: "%1$@", with: boldSiteAddress)

return message
}
Expand All @@ -34,14 +42,16 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel {
let secondaryButtonTitle = Localization.secondaryButtonTitle

func viewDidLoad(_ viewController: UIViewController?) {
// TODO: add tracks if necessary
}

// Pop to the previous VC
func didTapPrimaryButton(in viewController: UIViewController?) {
guard let viewController else {
return
guard let navigationController = viewController?.navigationController else { return }
if let previousViewController {
navigationController.popToViewController(previousViewController, animated: true)
} else {
navigationController.popViewController(animated: true)
}
WordPressAuthenticator.showLoginForJustWPCom(from: viewController)
}

func didTapSecondaryButton(in viewController: UIViewController?) {
Expand All @@ -55,28 +65,48 @@ struct ApplicationPasswordDisabledViewModel: ULErrorViewModel {
}
WebviewHelper.launch(Constants.applicationPasswordLink, with: viewController)
}

var rightBarButtonItemTitle: String? {
return Localization.helpButtonTitle
}

func didTapRightBarButtonItem(in viewController: UIViewController?) {
guard let viewController else {
return
}
authentication.presentSupport(from: viewController, screen: .noWooError)
}
}

private extension ApplicationPasswordDisabledViewModel {
enum Localization {
static let errorMessage = NSLocalizedString(
"It seems that your site %@ has Application Password disabled. Please enable it to use the WooCommerce app.",
"applicationPasswordDisabled.errorMessage",
value: "It seems that your site %1$@ has Application Password disabled. Please enable it to use the WooCommerce app.",
comment: "An error message displayed when the user tries to log in to the app with site credentials but has application password disabled. " +
"Reads like: It seems that your site google.com has Application Password disabled. " +
"Please enable it to use the WooCommerce app."
)
static let primaryButtonTitle = NSLocalizedString(
"applicationPasswordDisabled.retry.buttonTitle",
value: "Retry",
comment: "Button to retry fetching application password authorization if application password is disabled"
)
static let secondaryButtonTitle = NSLocalizedString(
"Log In With Another Account",
"applicationPasswordDisabled.secondaryButtonTitle",
value: "Log In With Another Account",
comment: "Action button that will restart the login flow."
+ "Presented when the user tries to log in to the app with site credentials but has application password disabled."
)
static let auxiliaryButtonTitle = NSLocalizedString(
"What is Application Password?",
"applicationPasswordDisabled.auxiliaryButtonTitle",
value: "What is Application Password?",
comment: "Button that will navigate to a web page explaining Application Password"
)
static let primaryButtonTitle = NSLocalizedString(
"Log in with WordPress.com",
comment: "Button that will navigate to the authentication flow with WP.com"
static let helpButtonTitle = NSLocalizedString(
"applicationPasswordDisabled.helpButtonTitle",
value: "Help",
comment: "Button that will navigate to the support area"
)
}
enum Constants {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,18 @@ final class PostSiteCredentialLoginChecker {
private let applicationPasswordUseCase: ApplicationPasswordUseCase
private let roleEligibilityUseCase: RoleEligibilityUseCaseProtocol
private let analytics: Analytics
private let previousViewController: UIViewController?

init(applicationPasswordUseCase: ApplicationPasswordUseCase,
roleEligibilityUseCase: RoleEligibilityUseCaseProtocol = RoleEligibilityUseCase(stores: ServiceLocator.stores),
stores: StoresManager = ServiceLocator.stores,
analytics: Analytics = ServiceLocator.analytics) {
analytics: Analytics = ServiceLocator.analytics,
previousViewController: UIViewController?) {
self.applicationPasswordUseCase = applicationPasswordUseCase
self.roleEligibilityUseCase = roleEligibilityUseCase
self.stores = stores
self.analytics = analytics
self.previousViewController = previousViewController
}

/// Checks whether the user is eligible to use the app.
Expand Down Expand Up @@ -56,8 +59,8 @@ private extension PostSiteCredentialLoginChecker {
analytics.track(event: .ApplicationPassword.applicationPasswordGenerationFailed(scenario: .generation, error: error))
switch error {
case ApplicationPasswordUseCaseError.applicationPasswordsDisabled:
// show application password disabled error
let errorUI = applicationPasswordDisabledUI(for: siteURL)
// show application password disabled error, and use the previous view controller.
let errorUI = applicationPasswordDisabledUI(for: siteURL, previousViewController: previousViewController)
navigationController.show(errorUI, sender: nil)
case ApplicationPasswordUseCaseError.unauthorizedRequest:
showAlert(message: Localization.unauthorizedForAppPassword, in: navigationController)
Expand Down Expand Up @@ -179,8 +182,8 @@ private extension PostSiteCredentialLoginChecker {
/// The error screen to be displayed when the user tries to log in with site credentials
/// with application password disabled.
///
func applicationPasswordDisabledUI(for siteURL: String) -> UIViewController {
let viewModel = ApplicationPasswordDisabledViewModel(siteURL: siteURL)
func applicationPasswordDisabledUI(for siteURL: String, previousViewController: UIViewController?) -> UIViewController {
let viewModel = ApplicationPasswordDisabledViewModel(siteURL: siteURL, previousViewController: previousViewController)
return ULErrorViewController(viewModel: viewModel)
}
}
Expand Down
Loading

0 comments on commit b71385b

Please sign in to comment.