From ad2a2c40d8b4ef4264f0e5f5d7f8662661361924 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 30 Sep 2024 13:46:47 +0800 Subject: [PATCH 1/7] Try to authenticate new account details before setting them Signed-off-by: Claudio Cambra --- ...ileProviderExtension+ClientInterface.swift | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift index 61f0a06052fb5..7d439245d66ce 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift @@ -119,11 +119,28 @@ extension FileProviderExtension: NSFileProviderServicing, ChangeNotificationInte ) ncKit.setup(delegate: changeObserver) - Logger.fileProviderExtension.info( - "Nextcloud account set up in File Provider extension for user: \(user, privacy: .public) at server: \(serverUrl, privacy: .public)" - ) - - signalEnumeratorAfterAccountSetup() + Task { + switch (await ncKit.tryAuthenticationAttempt()) { + case .authenticationError: + Logger.fileProviderExtension.info( + "\(user, privacy: .public) authentication failed due to bad creds, stopping" + ) + ncAccount = nil + ncKit.setup(user: "", userId: "", password: "", urlBase: "") // In case ongoing ops + case .connectionError: + Logger.fileProviderExtension.info( + "\(user, privacy: .public) authentication try failed due to internet connectivity issues." + ) + case .success: + Logger.fileProviderExtension.info( + """ + Authenticated! Nextcloud account set up in File Provider extension. + User: \(user, privacy: .public) at server: \(serverUrl, privacy: .public) + """ + ) + Task { @MainActor in signalEnumeratorAfterAccountSetup() } + } + } } @objc func removeAccountConfig() { From 9f9bcf00453375f7daa35625654a9ae8bb3f7e38 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 30 Sep 2024 13:52:18 +0800 Subject: [PATCH 2/7] Retry authentication if connection timed out Signed-off-by: Claudio Cambra --- .../FileProviderExtension+ClientInterface.swift | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift index 7d439245d66ce..e1bf70541dfc0 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift @@ -19,6 +19,10 @@ import NextcloudKit import NextcloudFileProviderKit import OSLog +let AuthenticationTimeouts: [UInt64] = [ // Have progressively longer timeouts to not hammer server + 3_000_000_000, 6_000_000_000, 30_000_000_000, 60_000_000_000, 120_000_000_000, 300_000_000_000 +] + extension FileProviderExtension: NSFileProviderServicing, ChangeNotificationInterface { /* This FileProviderExtension extension contains everything needed to communicate with the client. @@ -120,7 +124,17 @@ extension FileProviderExtension: NSFileProviderServicing, ChangeNotificationInte ncKit.setup(delegate: changeObserver) Task { - switch (await ncKit.tryAuthenticationAttempt()) { + var authAttemptState = AuthenticationAttemptResultState.connectionError // default + for authTimeout in AuthenticationTimeouts { // Retry if we have a connection issue + authAttemptState = await ncKit.tryAuthenticationAttempt() + guard authAttemptState == .connectionError else { break } + Logger.fileProviderExtension.info( + "\(user, privacy: .public) authentication try timed out. Trying again soon." + ) + try? await Task.sleep(nanoseconds: authTimeout) + } + + switch (authAttemptState) { case .authenticationError: Logger.fileProviderExtension.info( "\(user, privacy: .public) authentication failed due to bad creds, stopping" From 4d96dbbc7ae8037bc263878122912f7aafee0c1c Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 30 Sep 2024 13:53:55 +0800 Subject: [PATCH 3/7] Wait for task to complete synchronously after setting up domain account rather than wrapping everything in task Signed-off-by: Claudio Cambra --- ...ileProviderExtension+ClientInterface.swift | 75 +++++++++++-------- 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift index e1bf70541dfc0..ad9c24d0377d8 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift @@ -105,6 +105,47 @@ extension FileProviderExtension: NSFileProviderServicing, ChangeNotificationInte } @objc func setupDomainAccount(user: String, serverUrl: String, password: String) { + let semaphore = DispatchSemaphore(value: 0) + var authAttemptState = AuthenticationAttemptResultState.connectionError // default + Task { + let authTestNcKit = NextcloudKit() + authTestNcKit.setup(user: user, userId: user, password: password, urlBase: serverUrl) + + // Retry a few times if we have a connection issue + for authTimeout in AuthenticationTimeouts { + authAttemptState = await authTestNcKit.tryAuthenticationAttempt() + guard authAttemptState == .connectionError else { break } + + Logger.fileProviderExtension.info( + "\(user, privacy: .public) authentication try timed out. Trying again soon." + ) + try? await Task.sleep(nanoseconds: authTimeout) + } + semaphore.signal() + } + semaphore.wait() + + switch (authAttemptState) { + case .authenticationError: + Logger.fileProviderExtension.info( + "\(user, privacy: .public) authentication failed due to bad creds, stopping" + ) + return + case .connectionError: + // Despite multiple connection attempts we are still getting connection issues, so quit. + Logger.fileProviderExtension.info( + "\(user, privacy: .public) authentication try failed, no connection." + ) + return + case .success: + Logger.fileProviderExtension.info( + """ + Authenticated! Nextcloud account set up in File Provider extension. + User: \(user, privacy: .public) at server: \(serverUrl, privacy: .public) + """ + ) + } + let newNcAccount = Account(user: user, serverUrl: serverUrl, password: password) guard newNcAccount != ncAccount else { return } ncAccount = newNcAccount @@ -122,39 +163,7 @@ extension FileProviderExtension: NSFileProviderServicing, ChangeNotificationInte remoteInterface: ncKit, changeNotificationInterface: self, domain: domain ) ncKit.setup(delegate: changeObserver) - - Task { - var authAttemptState = AuthenticationAttemptResultState.connectionError // default - for authTimeout in AuthenticationTimeouts { // Retry if we have a connection issue - authAttemptState = await ncKit.tryAuthenticationAttempt() - guard authAttemptState == .connectionError else { break } - Logger.fileProviderExtension.info( - "\(user, privacy: .public) authentication try timed out. Trying again soon." - ) - try? await Task.sleep(nanoseconds: authTimeout) - } - - switch (authAttemptState) { - case .authenticationError: - Logger.fileProviderExtension.info( - "\(user, privacy: .public) authentication failed due to bad creds, stopping" - ) - ncAccount = nil - ncKit.setup(user: "", userId: "", password: "", urlBase: "") // In case ongoing ops - case .connectionError: - Logger.fileProviderExtension.info( - "\(user, privacy: .public) authentication try failed due to internet connectivity issues." - ) - case .success: - Logger.fileProviderExtension.info( - """ - Authenticated! Nextcloud account set up in File Provider extension. - User: \(user, privacy: .public) at server: \(serverUrl, privacy: .public) - """ - ) - Task { @MainActor in signalEnumeratorAfterAccountSetup() } - } - } + signalEnumeratorAfterAccountSetup() } @objc func removeAccountConfig() { From cbf4ea571ec545e59c361117efe37413144f25fe Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 30 Sep 2024 14:05:33 +0800 Subject: [PATCH 4/7] Fetch user profile (and user ID) when setting up domain account Signed-off-by: Claudio Cambra --- ...ileProviderExtension+ClientInterface.swift | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift index ad9c24d0377d8..8785699459192 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift @@ -163,6 +163,26 @@ extension FileProviderExtension: NSFileProviderServicing, ChangeNotificationInte remoteInterface: ncKit, changeNotificationInterface: self, domain: domain ) ncKit.setup(delegate: changeObserver) + + Task { + let (_, profile, _, error) = await ncKit.fetchUserProfile() + guard error == .success, let profile else { + // Note that since the authentication check checks for the user profile, this should + // not really occur + Logger.fileProviderExtension.error( + """ + Unable to get user profile for user \(user, privacy: .public) + at server \(serverUrl, privacy: .public) + error: \(error.errorDescription, privacy: .public) + """ + ) + return + } + ncKit.setup(user: user, userId: profile.userId, password: password, urlBase: serverUrl) + semaphore.signal() + } + semaphore.wait() + signalEnumeratorAfterAccountSetup() } From 6f1322fe474d24c58c43f3b196535c0277f108cd Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 30 Sep 2024 16:04:48 +0800 Subject: [PATCH 5/7] Properly provide both user-provided username AND backing user ID to File Provider Extension Signed-off-by: Claudio Cambra --- .../FileProviderExt/FileProviderSocketLineProcessor.swift | 7 ++++--- .../Services/ClientCommunicationProtocol.h | 1 + .../Services/ClientCommunicationService.swift | 4 +++- src/gui/macOS/fileprovidersocketcontroller.cpp | 8 +++++--- src/gui/macOS/fileproviderxpc_mac.mm | 2 ++ 5 files changed, 15 insertions(+), 7 deletions(-) diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderSocketLineProcessor.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderSocketLineProcessor.swift index 3a9f54a10abf4..ebc4bc4b999b0 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderSocketLineProcessor.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderSocketLineProcessor.swift @@ -46,11 +46,12 @@ class FileProviderSocketLineProcessor: NSObject, LineProcessor { delegate.removeAccountConfig() } else if command == "ACCOUNT_DETAILS" { guard let accountDetailsSubsequence = splitLine.last else { return } - let splitAccountDetails = accountDetailsSubsequence.split(separator: "~", maxSplits: 2) + let splitAccountDetails = accountDetailsSubsequence.split(separator: "~", maxSplits: 3) let user = String(splitAccountDetails[0]) - let serverUrl = String(splitAccountDetails[1]) - let password = String(splitAccountDetails[2]) + let userId = String(splitAccountDetails[1]) + let serverUrl = String(splitAccountDetails[2]) + let password = String(splitAccountDetails[3]) delegate.setupDomainAccount(user: user, serverUrl: serverUrl, password: password) } diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationProtocol.h b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationProtocol.h index 1ab5e8853d284..6f7f0e24da666 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationProtocol.h +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationProtocol.h @@ -21,6 +21,7 @@ - (void)getExtensionAccountIdWithCompletionHandler:(void(^)(NSString *extensionAccountId, NSError *error))completionHandler; - (void)configureAccountWithUser:(NSString *)user + userId:(NSString *)userId serverUrl:(NSString *)serverUrl password:(NSString *)password; - (void)removeAccountConfig; diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationService.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationService.swift index 76a0f00ce267a..41670fe557a8d 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationService.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationService.swift @@ -48,11 +48,13 @@ class ClientCommunicationService: NSObject, NSFileProviderServiceSource, NSXPCLi completionHandler(accountUserId, nil) } - func configureAccount(withUser user: String, + func configureAccount(withUser user: String, + userId: String, serverUrl: String, password: String) { Logger.desktopClientConnection.info("Received configure account information over client communication service") self.fpExtension.setupDomainAccount(user: user, + userId: userId, serverUrl: serverUrl, password: password) } diff --git a/src/gui/macOS/fileprovidersocketcontroller.cpp b/src/gui/macOS/fileprovidersocketcontroller.cpp index fcd5517b77de6..dad2032b5bd78 100644 --- a/src/gui/macOS/fileprovidersocketcontroller.cpp +++ b/src/gui/macOS/fileprovidersocketcontroller.cpp @@ -213,13 +213,15 @@ void FileProviderSocketController::sendAccountDetails() const const auto credentials = account->credentials(); Q_ASSERT(credentials); - const auto accountUser = account->davUser(); - const auto accountUrl = account->url().toString(); - const auto accountPassword = credentials->password(); + const auto accountUser = credentials->user(); // User-provided username/email + const auto accountUserId = account->davUser(); // Backing user id on server + const auto accountUrl = account->url().toString(); // Server base URL + const auto accountPassword = credentials->password(); // Account password // We cannot use colons as separators here due to "https://" in the url const auto message = QString(QStringLiteral("ACCOUNT_DETAILS:") + accountUser + "~" + + accountUserId + "~" + accountUrl + "~" + accountPassword); sendMessage(message); diff --git a/src/gui/macOS/fileproviderxpc_mac.mm b/src/gui/macOS/fileproviderxpc_mac.mm index e94dc2cbcca48..b7128cf6941c8 100644 --- a/src/gui/macOS/fileproviderxpc_mac.mm +++ b/src/gui/macOS/fileproviderxpc_mac.mm @@ -64,11 +64,13 @@ const auto account = accountState->account(); const auto credentials = account->credentials(); NSString *const user = credentials->user().toNSString(); + NSString *const userId = account->davUser().toNSString(); NSString *const serverUrl = account->url().toString().toNSString(); NSString *const password = credentials->password().toNSString(); const auto clientCommService = (NSObject *)_clientCommServices.value(extensionAccountId); [clientCommService configureAccountWithUser:user + userId:userId serverUrl:serverUrl password:password]; } From 4d7b3eeacb34f811280e57c5d71b5acd5c6a1b0b Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 30 Sep 2024 16:05:30 +0800 Subject: [PATCH 6/7] Handle userId correctly in File Provider Extension client interface Signed-off-by: Claudio Cambra --- .../FileProviderExtension+ClientInterface.swift | 10 ++++++---- .../FileProviderSocketLineProcessor.swift | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift index 8785699459192..99eaefb50886b 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift @@ -104,12 +104,14 @@ extension FileProviderExtension: NSFileProviderServicing, ChangeNotificationInte } } - @objc func setupDomainAccount(user: String, serverUrl: String, password: String) { + @objc func setupDomainAccount( + user: String, userId: String, serverUrl: String, password: String + ) { let semaphore = DispatchSemaphore(value: 0) var authAttemptState = AuthenticationAttemptResultState.connectionError // default Task { let authTestNcKit = NextcloudKit() - authTestNcKit.setup(user: user, userId: user, password: password, urlBase: serverUrl) + authTestNcKit.setup(user: user, userId: userId, password: password, urlBase: serverUrl) // Retry a few times if we have a connection issue for authTimeout in AuthenticationTimeouts { @@ -146,13 +148,13 @@ extension FileProviderExtension: NSFileProviderServicing, ChangeNotificationInte ) } - let newNcAccount = Account(user: user, serverUrl: serverUrl, password: password) + let newNcAccount = Account(user: user, id: userId, serverUrl: serverUrl, password: password) guard newNcAccount != ncAccount else { return } ncAccount = newNcAccount ncKit.setup( account: newNcAccount.ncKitAccount, user: newNcAccount.username, - userId: newNcAccount.username, + userId: newNcAccount.id, password: newNcAccount.password, urlBase: newNcAccount.serverUrl, userAgent: "Nextcloud-macOS/FileProviderExt", diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderSocketLineProcessor.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderSocketLineProcessor.swift index ebc4bc4b999b0..c9a49d4190f16 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderSocketLineProcessor.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderSocketLineProcessor.swift @@ -53,7 +53,7 @@ class FileProviderSocketLineProcessor: NSObject, LineProcessor { let serverUrl = String(splitAccountDetails[2]) let password = String(splitAccountDetails[3]) - delegate.setupDomainAccount(user: user, serverUrl: serverUrl, password: password) + delegate.setupDomainAccount(user: user, userId: userId, serverUrl: serverUrl, password: password) } } } From 690303f952cff11db608ee9157eeead043769d18 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Mon, 30 Sep 2024 16:05:50 +0800 Subject: [PATCH 7/7] Stop fetching user profile details now that is not necessary in file provider extension Signed-off-by: Claudio Cambra --- ...ileProviderExtension+ClientInterface.swift | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift index 99eaefb50886b..3cae7f945acb6 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension+ClientInterface.swift @@ -165,26 +165,6 @@ extension FileProviderExtension: NSFileProviderServicing, ChangeNotificationInte remoteInterface: ncKit, changeNotificationInterface: self, domain: domain ) ncKit.setup(delegate: changeObserver) - - Task { - let (_, profile, _, error) = await ncKit.fetchUserProfile() - guard error == .success, let profile else { - // Note that since the authentication check checks for the user profile, this should - // not really occur - Logger.fileProviderExtension.error( - """ - Unable to get user profile for user \(user, privacy: .public) - at server \(serverUrl, privacy: .public) - error: \(error.errorDescription, privacy: .public) - """ - ) - return - } - ncKit.setup(user: user, userId: profile.userId, password: password, urlBase: serverUrl) - semaphore.signal() - } - semaphore.wait() - signalEnumeratorAfterAccountSetup() }