Skip to content

[feature/tokenauth-uidenforce] Enforce User ID when updating token-based bookmarks #869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hosy merged 5 commits into from
Jan 26, 2021
Merged

[feature/tokenauth-uidenforce] Enforce User ID when updating token-based bookmarks #869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7826560
- SwiftLint: remove function_parameter_count check
felix-schwarz Jan 12, 2021
f87679f
- Cleanup
felix-schwarz Jan 12, 2021
5646c15
- re-auth: do not show an error if the user has cancelled authentication
felix-schwarz Jan 13, 2021
1c3409f
ClientRootViewController: reduce nesting by unifying if statements
felix-schwarz Jan 25, 2021
2290b65
Merge branch 'milestone/11.5' into feature/tokenauth-uidenforce
hosy Jan 26, 2021
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
1 change: 1 addition & 0 deletions .swiftlint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions ownCloud/Bookmarks/BookmarkViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,7 @@ class BookmarkViewController: StaticTableViewController {
}

options[.presentingViewControllerKey] = self
options[.requiredUsernameKey] = connectionBookmark.userName

guard let bookmarkAuthenticationMethodIdentifier = bookmark?.authenticationMethodIdentifier else { return }

Expand Down
1 change: 1 addition & 0 deletions ownCloud/Client/ClientAuthenticationUpdater.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class ClientAuthenticationUpdater: NSObject {
}

options[.presentingViewControllerKey] = viewController
options[.requiredUsernameKey] = bookmark.userName

tempBookmark.authenticationMethodIdentifier = authenticationMethodID

Expand Down
86 changes: 47 additions & 39 deletions ownCloud/Client/ClientRootViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,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
Expand Down Expand Up @@ -536,46 +537,8 @@ extension ClientRootViewController : OCCoreDelegate {
var queueCompletionHandlerScheduled : Bool = false

if isAuthFailure {
let alertController = ThemedAlertController(title: authFailureTitle,
message: authFailureMessage,
preferredStyle: .alert)
self?.presentAuthAlert(for: editBookmark, error: nsError, title: authFailureTitle, message: authFailureMessage, ignoreLabel: authFailureIgnoreLabel, ignoreStyle: authFailureIgnoreStyle, hasEditOption: authFailureHasEditOption, preferredAuthenticationMethods: preferredAuthenticationMethods, completionHandler: queueCompletionHandler)

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?.present(alertController, animated: true, completion: nil)
queueCompletionHandlerScheduled = true

return
Expand Down Expand Up @@ -666,6 +629,51 @@ 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
Copy link
Contributor

@mneuwert mneuwert Jan 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felix-schwarz reduce nesting with guard let statements?

Copy link
Contributor Author

@felix-schwarz felix-schwarz Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but I didn't find any opportunity to simplify the logic there with guard let statements. Still could reduce nesting by unifying ifs that were nested unnecessarily.


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 if let nsError = error as NSError?, !nsError.isOCError(withCode: .authorizationCancelled) {
// 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, 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 {
Expand Down