From 7826560bdadd145b6ff07a21460dc05eba02cc4b Mon Sep 17 00:00:00 2001 From: Felix Schwarz Date: Tue, 12 Jan 2021 23:48:27 +0100 Subject: [PATCH 1/4] - SwiftLint: remove function_parameter_count check - Authentication: require username to stay the same when updating OAuth2 / OIDC bookmarks - SDK update to support this --- .swiftlint.yml | 1 + ios-sdk | 2 +- .../Bookmarks/BookmarkViewController.swift | 1 + .../Client/ClientAuthenticationUpdater.swift | 1 + .../Client/ClientRootViewController.swift | 95 +++++++++++-------- 5 files changed, 59 insertions(+), 41 deletions(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index 975ea6f57..c604212fb 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -34,6 +34,7 @@ disabled_rules: - implicit_getter - computed_accessors_order - unneeded_notification_center_removal + - function_parameter_count custom_rules: empty_line_after_guard_statement: included: ".*\\.swift" diff --git a/ios-sdk b/ios-sdk index 3c54a15c1..0cfc78c88 160000 --- a/ios-sdk +++ b/ios-sdk @@ -1 +1 @@ -Subproject commit 3c54a15c130c917650f40d2230b8858f94bc4c2e +Subproject commit 0cfc78c88b60c2b4877615928ccddf6b756c3608 diff --git a/ownCloud/Bookmarks/BookmarkViewController.swift b/ownCloud/Bookmarks/BookmarkViewController.swift index 43aefb1e3..81189cb33 100644 --- a/ownCloud/Bookmarks/BookmarkViewController.swift +++ b/ownCloud/Bookmarks/BookmarkViewController.swift @@ -472,6 +472,7 @@ class BookmarkViewController: StaticTableViewController { } options[.presentingViewControllerKey] = self + options[.requiredUsernameKey] = connectionBookmark.userName guard let bookmarkAuthenticationMethodIdentifier = bookmark?.authenticationMethodIdentifier else { return } diff --git a/ownCloud/Client/ClientAuthenticationUpdater.swift b/ownCloud/Client/ClientAuthenticationUpdater.swift index 9b4150e83..34478fbdd 100644 --- a/ownCloud/Client/ClientAuthenticationUpdater.swift +++ b/ownCloud/Client/ClientAuthenticationUpdater.swift @@ -67,6 +67,7 @@ class ClientAuthenticationUpdater: NSObject { } options[.presentingViewControllerKey] = viewController + options[.requiredUsernameKey] = bookmark.userName tempBookmark.authenticationMethodIdentifier = authenticationMethodID diff --git a/ownCloud/Client/ClientRootViewController.swift b/ownCloud/Client/ClientRootViewController.swift index c95f84a03..7c507c206 100644 --- a/ownCloud/Client/ClientRootViewController.swift +++ b/ownCloud/Client/ClientRootViewController.swift @@ -448,7 +448,11 @@ extension ClientRootViewController : Themeable { } extension ClientRootViewController : OCCoreDelegate { - func core(_ core: OCCore, handleError error: Error?, issue inIssue: OCIssue?) { + func core(_ core: OCCore, handleError error: Error?, issue: OCIssue?) { + self.handleIssue(for: core, error: error, issue: issue, allowSkipping: true) + } + + func handleIssue(for core: OCCore, error: Error?, issue inIssue: OCIssue?, allowSkipping: Bool) { var issue = inIssue var isAuthFailure : Bool = false var authFailureMessage : String? @@ -467,6 +471,7 @@ extension ClientRootViewController : OCCoreDelegate { issueNSError.isOCError(withCode: .authorizationMethodNotAllowed) || issueNSError.isOCError(withCode: .authorizationMethodUnknown) || issueNSError.isOCError(withCode: .authorizationNoMethodData) || + issueNSError.isOCError(withCode: .authorizationNotMatchingRequiredUserID) || issueNSError.isOCError(withCode: .authorizationMissingData) { nsError = issueNSError issue = nil @@ -536,46 +541,8 @@ extension ClientRootViewController : OCCoreDelegate { var queueCompletionHandlerScheduled : Bool = false if isAuthFailure { - let alertController = ThemedAlertController(title: authFailureTitle, - message: authFailureMessage, - preferredStyle: .alert) - - alertController.addAction(UIAlertAction(title: authFailureIgnoreLabel, style: authFailureIgnoreStyle, handler: { (_) in - queueCompletionHandler() - })) - - if authFailureHasEditOption { - alertController.addAction(UIAlertAction(title: "Sign in".localized, style: .default, handler: { (_) in - queueCompletionHandler() - - var notifyAuthDelegate = true - - if let bookmark = self?.bookmark { - let updater = ClientAuthenticationUpdater(with: bookmark, preferredAuthenticationMethods: preferredAuthenticationMethods) - - if updater.canUpdateInline, let self = self { - notifyAuthDelegate = false - - updater.updateAuthenticationData(on: self, completion: { (error) in - if error == nil { - OCSynchronized(self) { - self.skipAuthorizationFailure = false // Auth failure fixed -> allow new failures to prompt for sign in again - } - } - }) - } - } - - if notifyAuthDelegate { - if let authDelegate = self?.authDelegate, let self = self, let nsError = nsError { - authDelegate.handleAuthError(for: self, error: nsError, editBookmark: editBookmark, preferredAuthenticationMethods: preferredAuthenticationMethods) - } - } + self?.presentAuthAlert(for: editBookmark, error: nsError, title: authFailureTitle, message: authFailureMessage, ignoreLabel: authFailureIgnoreLabel, ignoreStyle: authFailureIgnoreStyle, hasEditOption: authFailureHasEditOption, preferredAuthenticationMethods: preferredAuthenticationMethods, completionHandler: queueCompletionHandler) - })) - } - - self?.present(alertController, animated: true, completion: nil) queueCompletionHandlerScheduled = true return @@ -666,6 +633,54 @@ extension ClientRootViewController : OCCoreDelegate { } } + func presentAuthAlert(for editBookmark: OCBookmark, error nsError: NSError?, title authFailureTitle: String, message authFailureMessage: String?, ignoreLabel authFailureIgnoreLabel: String, ignoreStyle authFailureIgnoreStyle: UIAlertAction.Style, hasEditOption authFailureHasEditOption: Bool, preferredAuthenticationMethods: [OCAuthenticationMethodIdentifier]?, completionHandler: @escaping () -> Void) { + let alertController = ThemedAlertController(title: authFailureTitle, + message: authFailureMessage, + preferredStyle: .alert) + + alertController.addAction(UIAlertAction(title: authFailureIgnoreLabel, style: authFailureIgnoreStyle, handler: { (_) in + completionHandler() + })) + + if authFailureHasEditOption { + alertController.addAction(UIAlertAction(title: "Sign in".localized, style: .default, handler: { [weak self] (_) in + completionHandler() + + var notifyAuthDelegate = true + + if let bookmark = self?.bookmark { + let updater = ClientAuthenticationUpdater(with: bookmark, preferredAuthenticationMethods: preferredAuthenticationMethods) + + if updater.canUpdateInline, let self = self { + notifyAuthDelegate = false + + updater.updateAuthenticationData(on: self, completion: { (error) in + if error == nil { + OCSynchronized(self) { + self.skipAuthorizationFailure = false // Auth failure fixed -> allow new failures to prompt for sign in again + } + } else { + // Error updating authentication -> inform the user and provide option to retry + self.alertQueue.async { [weak self] (queueCompletionHandler) in + self?.presentAuthAlert(for: editBookmark, error: error as NSError?, title: "Error".localized, message: error?.localizedDescription, ignoreLabel: authFailureIgnoreLabel, ignoreStyle: authFailureIgnoreStyle, hasEditOption: authFailureHasEditOption, preferredAuthenticationMethods: preferredAuthenticationMethods, completionHandler: queueCompletionHandler) + } + } + }) + } + } + + if notifyAuthDelegate { + if let authDelegate = self?.authDelegate, let self = self, let nsError = nsError { + authDelegate.handleAuthError(for: self, error: nsError, editBookmark: editBookmark, preferredAuthenticationMethods: preferredAuthenticationMethods) + } + } + + })) + } + + self.present(alertController, animated: true, completion: nil) + } + func presentAlertAsCard(viewController: UIViewController, withHandle: Bool = false, dismissable: Bool = true) { alertQueue.async { [weak self] (queueCompletionHandler) in if let startViewController = self { From f87679f9ff4acda7b873e1944382620741a4beb1 Mon Sep 17 00:00:00 2001 From: Felix Schwarz Date: Wed, 13 Jan 2021 00:07:50 +0100 Subject: [PATCH 2/4] - Cleanup --- ownCloud/Client/ClientRootViewController.swift | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ownCloud/Client/ClientRootViewController.swift b/ownCloud/Client/ClientRootViewController.swift index 7c507c206..7cc408080 100644 --- a/ownCloud/Client/ClientRootViewController.swift +++ b/ownCloud/Client/ClientRootViewController.swift @@ -448,11 +448,7 @@ extension ClientRootViewController : Themeable { } extension ClientRootViewController : OCCoreDelegate { - func core(_ core: OCCore, handleError error: Error?, issue: OCIssue?) { - self.handleIssue(for: core, error: error, issue: issue, allowSkipping: true) - } - - func handleIssue(for core: OCCore, error: Error?, issue inIssue: OCIssue?, allowSkipping: Bool) { + func core(_ core: OCCore, handleError error: Error?, issue inIssue: OCIssue?) { var issue = inIssue var isAuthFailure : Bool = false var authFailureMessage : String? From 5646c1598fb86907388e9dfec240a235e7283eca Mon Sep 17 00:00:00 2001 From: Felix Schwarz Date: Wed, 13 Jan 2021 14:53:24 +0100 Subject: [PATCH 3/4] - re-auth: do not show an error if the user has cancelled authentication --- ownCloud/Client/ClientRootViewController.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ownCloud/Client/ClientRootViewController.swift b/ownCloud/Client/ClientRootViewController.swift index 7cc408080..1afc70195 100644 --- a/ownCloud/Client/ClientRootViewController.swift +++ b/ownCloud/Client/ClientRootViewController.swift @@ -657,8 +657,10 @@ extension ClientRootViewController : OCCoreDelegate { } } else { // Error updating authentication -> inform the user and provide option to retry - self.alertQueue.async { [weak self] (queueCompletionHandler) in - self?.presentAuthAlert(for: editBookmark, error: error as NSError?, title: "Error".localized, message: error?.localizedDescription, ignoreLabel: authFailureIgnoreLabel, ignoreStyle: authFailureIgnoreStyle, hasEditOption: authFailureHasEditOption, preferredAuthenticationMethods: preferredAuthenticationMethods, completionHandler: queueCompletionHandler) + if let nsError = error as NSError?, !nsError.isOCError(withCode: .authorizationCancelled) { + self.alertQueue.async { [weak self] (queueCompletionHandler) in + self?.presentAuthAlert(for: editBookmark, error: error as NSError?, title: "Error".localized, message: error?.localizedDescription, ignoreLabel: authFailureIgnoreLabel, ignoreStyle: authFailureIgnoreStyle, hasEditOption: authFailureHasEditOption, preferredAuthenticationMethods: preferredAuthenticationMethods, completionHandler: queueCompletionHandler) + } } } }) From 1c3409fd7850b0a14b987a9fc0c9b3021abeba31 Mon Sep 17 00:00:00 2001 From: Felix Schwarz Date: Mon, 25 Jan 2021 13:45:05 +0100 Subject: [PATCH 4/4] ClientRootViewController: reduce nesting by unifying if statements --- ownCloud/Client/ClientRootViewController.swift | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/ownCloud/Client/ClientRootViewController.swift b/ownCloud/Client/ClientRootViewController.swift index 1afc70195..e49ed22a6 100644 --- a/ownCloud/Client/ClientRootViewController.swift +++ b/ownCloud/Client/ClientRootViewController.swift @@ -655,24 +655,19 @@ extension ClientRootViewController : OCCoreDelegate { OCSynchronized(self) { self.skipAuthorizationFailure = false // Auth failure fixed -> allow new failures to prompt for sign in again } - } else { + } else if let nsError = error as NSError?, !nsError.isOCError(withCode: .authorizationCancelled) { // Error updating authentication -> inform the user and provide option to retry - if let nsError = error as NSError?, !nsError.isOCError(withCode: .authorizationCancelled) { - self.alertQueue.async { [weak self] (queueCompletionHandler) in - self?.presentAuthAlert(for: editBookmark, error: error as NSError?, title: "Error".localized, message: error?.localizedDescription, ignoreLabel: authFailureIgnoreLabel, ignoreStyle: authFailureIgnoreStyle, hasEditOption: authFailureHasEditOption, preferredAuthenticationMethods: preferredAuthenticationMethods, completionHandler: queueCompletionHandler) - } + self.alertQueue.async { [weak self] (queueCompletionHandler) in + self?.presentAuthAlert(for: editBookmark, error: error as NSError?, title: "Error".localized, message: error?.localizedDescription, ignoreLabel: authFailureIgnoreLabel, ignoreStyle: authFailureIgnoreStyle, hasEditOption: authFailureHasEditOption, preferredAuthenticationMethods: preferredAuthenticationMethods, completionHandler: queueCompletionHandler) } } }) } } - if notifyAuthDelegate { - if let authDelegate = self?.authDelegate, let self = self, let nsError = nsError { - authDelegate.handleAuthError(for: self, error: nsError, editBookmark: editBookmark, preferredAuthenticationMethods: preferredAuthenticationMethods) - } + if notifyAuthDelegate, let authDelegate = self?.authDelegate, let self = self, let nsError = nsError { + authDelegate.handleAuthError(for: self, error: nsError, editBookmark: editBookmark, preferredAuthenticationMethods: preferredAuthenticationMethods) } - })) }