Skip to content
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.

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

Fix background URLSession identifier collisions #351

Merged
merged 10 commits into from
Apr 18, 2024
7 changes: 3 additions & 4 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@ on:
jobs:
build:
name: Build and test
runs-on: macos-14
runs-on: [self-hosted, macOS, ARM64]
env:
DERIVED_DATA_PATH: 'DerivedData'
DEVICE: 'iPhone 15 Pro'
if: "!contains(github.event.head_commit.message, '[ci skip]') && !contains(github.event.head_commit.message, '[skip ci]')"
strategy:
matrix:
config: ['freemium', 'premium']
Expand All @@ -32,8 +31,8 @@ jobs:
run: |
cd fastlane
./scripts/create-cloud-access-secrets.sh
- name: Select Xcode 15.2
run: sudo xcode-select -s /Applications/Xcode_15.2.app
- name: Select Xcode 15.3
run: sudo xcode-select -s /Applications/Xcode_15.3.app
- name: Configuration for freemium
if: ${{ matrix.config == 'freemium' }}
run: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/cryptomator/cloud-access-swift.git",
"state" : {
"revision" : "bb9cc1c300be890f3a47efa0ac0808ee7c42146d",
"version" : "1.9.2"
"revision" : "cd7a18abcaf09349f066363c7524b738f4f4ad79",
"version" : "1.10.1"
}
},
{
Expand Down
9 changes: 5 additions & 4 deletions Cryptomator/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,17 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
cleanup()

// Set up cloud storage services
CloudProviderDBManager.shared.useBackgroundSession = false
DropboxSetup.constants = DropboxSetup(appKey: CloudAccessSecrets.dropboxAppKey, sharedContainerIdentifier: nil, keychainService: CryptomatorConstants.mainAppBundleId, forceForegroundSession: true)
GoogleDriveSetup.constants = GoogleDriveSetup(clientId: CloudAccessSecrets.googleDriveClientId, redirectURL: CloudAccessSecrets.googleDriveRedirectURL!, sharedContainerIdentifier: nil)
let oneDriveConfiguration = MSALPublicClientApplicationConfig(clientId: CloudAccessSecrets.oneDriveClientId, redirectUri: CloudAccessSecrets.oneDriveRedirectURI, authority: nil)
oneDriveConfiguration.cacheConfig.keychainSharingGroup = CryptomatorConstants.mainAppBundleId
do {
OneDriveSetup.clientApplication = try MSALPublicClientApplication(configuration: oneDriveConfiguration)
let oneDriveConfiguration = MSALPublicClientApplicationConfig(clientId: CloudAccessSecrets.oneDriveClientId, redirectUri: CloudAccessSecrets.oneDriveRedirectURI, authority: nil)
oneDriveConfiguration.cacheConfig.keychainSharingGroup = CryptomatorConstants.mainAppBundleId
let oneDriveClientApplication = try MSALPublicClientApplication(configuration: oneDriveConfiguration)
OneDriveSetup.constants = OneDriveSetup(clientApplication: oneDriveClientApplication, sharedContainerIdentifier: nil)
} catch {
DDLogError("Setting up OneDrive failed with error: \(error)")
}
PCloudSetup.constants = PCloudSetup(appKey: CloudAccessSecrets.pCloudAppKey, sharedContainerIdentifier: nil)

// Set up payment queue
SKPaymentQueue.default().add(StoreObserver.shared)
Expand Down
3 changes: 1 addition & 2 deletions Cryptomator/Common/CloudAuthenticator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ class CloudAuthenticator {
}

func authenticatePCloud(from viewController: UIViewController) -> Promise<CloudProviderAccount> {
let authenticator = PCloudAuthenticator(appKey: CloudAccessSecrets.pCloudAppKey)
return authenticator.authenticate(from: viewController).then { credential -> CloudProviderAccount in
return PCloudAuthenticator.authenticate(from: viewController).then { credential -> CloudProviderAccount in
try credential.saveToKeychain()
let account = CloudProviderAccount(accountUID: credential.userID, cloudProviderType: .pCloud)
try self.accountManager.saveNewAccount(account)
Expand Down
2 changes: 1 addition & 1 deletion CryptomatorCommon/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ let package = Package(
)
],
dependencies: [
.package(url: "https://github.com/cryptomator/cloud-access-swift.git", .upToNextMinor(from: "1.9.0")),
.package(url: "https://github.com/cryptomator/cloud-access-swift.git", .upToNextMinor(from: "1.10.0")),
.package(url: "https://github.com/CocoaLumberjack/CocoaLumberjack.git", .upToNextMinor(from: "3.8.0")),
.package(url: "https://github.com/PhilLibs/simple-swift-dependencies", .upToNextMajor(from: "0.1.0")),
.package(url: "https://github.com/siteline/SwiftUI-Introspect.git", .upToNextMajor(from: "0.3.0")),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,23 @@ import PCloudSDKSwift

public protocol CloudProviderManager {
func getProvider(with accountUID: String) throws -> CloudProvider
func getBackgroundSessionProvider(with accountUID: String, sessionIdentifier: String) throws -> CloudProvider
}

public protocol CloudProviderUpdating {
func providerShouldUpdate(with accountUID: String)
}

struct CachedProvider {
let accountUID: String
let provider: CloudProvider
let backgroundSessionIdentifier: String?
var isBackgroundSession: Bool { backgroundSessionIdentifier != nil }
}

public class CloudProviderDBManager: CloudProviderManager, CloudProviderUpdating {
static var cachedProvider = [String: CloudProvider]()
static var cachedProvider = [CachedProvider]()
public static let shared = CloudProviderDBManager(accountManager: CloudProviderAccountDBManager.shared)
public var useBackgroundSession = true
let accountManager: CloudProviderAccountDBManager

private let maxPageSizeForFileProvider = 500
Expand All @@ -31,84 +38,129 @@ public class CloudProviderDBManager: CloudProviderManager, CloudProviderUpdating
}

public func getProvider(with accountUID: String) throws -> CloudProvider {
if let provider = CloudProviderDBManager.cachedProvider[accountUID] {
return provider
if let entry = CloudProviderDBManager.cachedProvider.first(where: {
$0.accountUID == accountUID && !$0.isBackgroundSession
}) {
return entry.provider
}
return try createProvider(for: accountUID)
}

public func getBackgroundSessionProvider(with accountUID: String, sessionIdentifier: String) throws -> any CloudProvider {
if let entry = CloudProviderDBManager.cachedProvider.first(where: {
$0.accountUID == accountUID && $0.backgroundSessionIdentifier == sessionIdentifier
}) {
return entry.provider
}
return try createBackgroundSessionProvider(for: accountUID, sessionIdentifier: sessionIdentifier)
}

/**
Creates and returns a cloud provider for the given `accountUID`.

If `useBackgroundURLSession` is set to `true`, the number of returned items from a `fetchItemList(forFolderAt:pageToken:)` call is limited to 500.
This is necessary because otherwise memory limit problems can occur with folders with many items in the `FileProviderExtension` where a background `URLSession` is used.
*/
func createProvider(for accountUID: String) throws -> CloudProvider {
let cloudProviderType = try accountManager.getCloudProviderType(for: accountUID)
let provider: CloudProvider
switch cloudProviderType {
case .dropbox:
let credential = DropboxCredential(tokenUID: accountUID)
provider = DropboxCloudProvider(credential: credential, maxPageSize: useBackgroundSession ? maxPageSizeForFileProvider : .max)
provider = DropboxCloudProvider(credential: credential, maxPageSize: .max)
case .googleDrive:
let credential = GoogleDriveCredential(userID: accountUID)
provider = try GoogleDriveCloudProvider(credential: credential,
useBackgroundSession: useBackgroundSession,
maxPageSize: useBackgroundSession ? maxPageSizeForFileProvider : .max)
provider = try GoogleDriveCloudProvider(credential: credential, maxPageSize: .max)
case .oneDrive:
let credential = try OneDriveCredential(with: accountUID)
provider = try OneDriveCloudProvider(credential: credential,
useBackgroundSession: useBackgroundSession,
maxPageSize: useBackgroundSession ? maxPageSizeForFileProvider : .max)
provider = try OneDriveCloudProvider(credential: credential, maxPageSize: .max)
case .pCloud:
provider = try createPCloudProvider(for: accountUID)
let credential = try PCloudCredential(userID: accountUID)
let client = PCloud.createClient(with: credential.user)
provider = try PCloudCloudProvider(client: client)
case .webDAV:
guard let credential = WebDAVCredentialManager.shared.getCredentialFromKeychain(with: accountUID) else {
let credential = try getWebDAVCredential(for: accountUID)
let client = WebDAVClient(credential: credential)
provider = try WebDAVProvider(with: client, maxPageSize: .max)
case .localFileSystem:
guard let rootURL = try LocalFileSystemBookmarkManager.getBookmarkedRootURL(for: accountUID) else {
throw CloudProviderAccountError.accountNotFoundError
}
let client: WebDAVClient
if useBackgroundSession {
client = WebDAVClient.withBackgroundSession(credential: credential, sharedContainerIdentifier: CryptomatorConstants.appGroupName)
} else {
client = WebDAVClient(credential: credential)
}
provider = try WebDAVProvider(with: client, maxPageSize: useBackgroundSession ? maxPageSizeForFileProvider : .max)
provider = try LocalFileSystemProvider(rootURL: rootURL, maxPageSize: .max)
case .s3:
let credential = try getS3Credential(for: accountUID)
provider = try S3CloudProvider(credential: credential)
}
CloudProviderDBManager.cachedProvider.append(
.init(
accountUID: accountUID,
provider: provider,
backgroundSessionIdentifier: nil
)
)
return provider
}

/**
Creates and returns a cloud provider for the given `accountUID` using a background URLSession with the given `sessionIdentifier`.

The number of returned items from a `fetchItemList(forFolderAt:pageToken:)` call is limited to 500.
This is necessary because otherwise memory limit problems can occur with folders with many items in the `FileProviderExtension` where a background `URLSession` is used.
*/
func createBackgroundSessionProvider(for accountUID: String, sessionIdentifier: String) throws -> CloudProvider {
let cloudProviderType = try accountManager.getCloudProviderType(for: accountUID)
let provider: CloudProvider

switch cloudProviderType {
case .dropbox:
let credential = DropboxCredential(tokenUID: accountUID)
provider = DropboxCloudProvider(credential: credential, maxPageSize: maxPageSizeForFileProvider)
case .googleDrive:
let credential = GoogleDriveCredential(userID: accountUID)
provider = try GoogleDriveCloudProvider.withBackgroundSession(credential: credential, maxPageSize: maxPageSizeForFileProvider, sessionIdentifier: sessionIdentifier)
case .oneDrive:
let credential = try OneDriveCredential(with: accountUID)
provider = try OneDriveCloudProvider.withBackgroundSession(credential: credential, maxPageSize: maxPageSizeForFileProvider, sessionIdentifier: sessionIdentifier)
case .pCloud:
let credential = try PCloudCredential(userID: accountUID)
let client = PCloud.createBackgroundClient(with: credential.user, sessionIdentifier: sessionIdentifier)
provider = try PCloudCloudProvider(client: client)
case .webDAV:
let credential = try getWebDAVCredential(for: accountUID)
let client = WebDAVClient.withBackgroundSession(credential: credential, sessionIdentifier: sessionIdentifier, sharedContainerIdentifier: CryptomatorConstants.appGroupName)
provider = try WebDAVProvider(with: client, maxPageSize: maxPageSizeForFileProvider)
case .localFileSystem:
guard let rootURL = try LocalFileSystemBookmarkManager.getBookmarkedRootURL(for: accountUID) else {
throw CloudProviderAccountError.accountNotFoundError
}
provider = try LocalFileSystemProvider(rootURL: rootURL, maxPageSize: useBackgroundSession ? maxPageSizeForFileProvider : .max)
provider = try LocalFileSystemProvider(rootURL: rootURL, maxPageSize: maxPageSizeForFileProvider)
case .s3:
provider = try createS3Provider(for: accountUID)
let credential = try getS3Credential(for: accountUID)
provider = try S3CloudProvider.withBackgroundSession(credential: credential, sharedContainerIdentifier: CryptomatorConstants.appGroupName)
}
CloudProviderDBManager.cachedProvider[accountUID] = provider
CloudProviderDBManager.cachedProvider.append(
.init(
accountUID: accountUID,
provider: provider,
backgroundSessionIdentifier: sessionIdentifier
)
)
return provider
}

private func createS3Provider(for accountUID: String) throws -> CloudProvider {
private func getS3Credential(for accountUID: String) throws -> S3Credential {
guard let credential = S3CredentialManager.shared.getCredential(with: accountUID) else {
throw CloudProviderAccountError.accountNotFoundError
}
if useBackgroundSession {
return try S3CloudProvider.withBackgroundSession(credential: credential, sharedContainerIdentifier: CryptomatorConstants.appGroupName)
} else {
return try S3CloudProvider(credential: credential)
}
return credential
}

private func createPCloudProvider(for accountUID: String) throws -> CloudProvider {
let credential = try PCloudCredential(userID: accountUID)
let client: PCloudClient
if useBackgroundSession {
client = PCloud.createBackgroundClient(with: credential.user, sharedContainerIdentifier: CryptomatorConstants.appGroupName)
} else {
client = PCloud.createClient(with: credential.user)
private func getWebDAVCredential(for accountUID: String) throws -> WebDAVCredential {
guard let credential = WebDAVCredentialManager.shared.getCredentialFromKeychain(with: accountUID) else {
throw CloudProviderAccountError.accountNotFoundError
}
return try PCloudCloudProvider(client: client)
return credential
}

public func providerShouldUpdate(with accountUID: String) {
CloudProviderDBManager.cachedProvider[accountUID] = nil
CloudProviderDBManager.cachedProvider.removeAll(where: { $0.accountUID == accountUID })
// call XPCService for FileProvider
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,8 @@ public class VaultDBManager: VaultManager {
private func createVaultProvider(cachedVault: CachedVault, masterkey: Masterkey, masterkeyFile: MasterkeyFile) throws -> CloudProvider {
let vaultUID = cachedVault.vaultUID
let vaultAccount = try vaultAccountManager.getAccount(with: vaultUID)
let provider = try providerManager.getProvider(with: vaultAccount.delegateAccountUID)
// it's important to use the vaultUID as background URLSession identifier to avoid identifier collisions
let provider = try providerManager.getBackgroundSessionProvider(with: vaultAccount.delegateAccountUID, sessionIdentifier: vaultUID)
let decorator: CloudProvider
if let vaultConfigToken = cachedVault.vaultConfigToken {
let unverifiedVaultConfig = try UnverifiedVaultConfig(token: vaultConfigToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ class CloudProviderManagerTests: XCTestCase {
DropboxSetup.constants = DropboxSetup(appKey: "", sharedContainerIdentifier: nil, keychainService: nil, forceForegroundSession: false)
let account = CloudProviderAccount(accountUID: UUID().uuidString, cloudProviderType: .dropbox)
try accountManager.saveNewAccount(account)
XCTAssertNil(CloudProviderDBManager.cachedProvider[account.accountUID])
XCTAssert(CloudProviderDBManager.cachedProvider.isEmpty)
let provider = try manager.getProvider(with: account.accountUID)
guard provider is DropboxCloudProvider else {
XCTFail("Provider has wrong type")
return
}
XCTAssertNotNil(CloudProviderDBManager.cachedProvider[account.accountUID])
XCTAssertEqual(CloudProviderDBManager.cachedProvider.filter { $0.accountUID == account.accountUID }.count, 1)
}
}
5 changes: 3 additions & 2 deletions FileProviderExtension/FileProviderExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@ class FileProviderExtension: NSFileProviderExtension {
FileProviderExtension.sharedDatabaseInitialized = true
DropboxSetup.constants = DropboxSetup(appKey: CloudAccessSecrets.dropboxAppKey, sharedContainerIdentifier: CryptomatorConstants.appGroupName, keychainService: CryptomatorConstants.mainAppBundleId, forceForegroundSession: false)
GoogleDriveSetup.constants = GoogleDriveSetup(clientId: CloudAccessSecrets.googleDriveClientId, redirectURL: CloudAccessSecrets.googleDriveRedirectURL!, sharedContainerIdentifier: CryptomatorConstants.appGroupName)
OneDriveSetup.sharedContainerIdentifier = CryptomatorConstants.appGroupName
let oneDriveConfiguration = MSALPublicClientApplicationConfig(clientId: CloudAccessSecrets.oneDriveClientId, redirectUri: CloudAccessSecrets.oneDriveRedirectURI, authority: nil)
oneDriveConfiguration.cacheConfig.keychainSharingGroup = CryptomatorConstants.mainAppBundleId
OneDriveSetup.clientApplication = try MSALPublicClientApplication(configuration: oneDriveConfiguration)
let oneDriveClientApplication = try MSALPublicClientApplication(configuration: oneDriveConfiguration)
OneDriveSetup.constants = OneDriveSetup(clientApplication: oneDriveClientApplication, sharedContainerIdentifier: CryptomatorConstants.appGroupName)
PCloudSetup.constants = PCloudSetup(appKey: CloudAccessSecrets.pCloudAppKey, sharedContainerIdentifier: CryptomatorConstants.appGroupName)
} catch {
// MARK: Handle error

Expand Down
9 changes: 5 additions & 4 deletions FileProviderExtensionUI/RootViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,17 @@ class RootViewController: FPUIActionExtensionViewController {
LoggerSetup.oneTimeSetup()

// Set up cloud storage services
CloudProviderDBManager.shared.useBackgroundSession = false
DropboxSetup.constants = DropboxSetup(appKey: CloudAccessSecrets.dropboxAppKey, sharedContainerIdentifier: nil, keychainService: CryptomatorConstants.mainAppBundleId, forceForegroundSession: true)
GoogleDriveSetup.constants = GoogleDriveSetup(clientId: CloudAccessSecrets.googleDriveClientId, redirectURL: CloudAccessSecrets.googleDriveRedirectURL!, sharedContainerIdentifier: nil)
let oneDriveConfiguration = MSALPublicClientApplicationConfig(clientId: CloudAccessSecrets.oneDriveClientId, redirectUri: CloudAccessSecrets.oneDriveRedirectURI, authority: nil)
oneDriveConfiguration.cacheConfig.keychainSharingGroup = CryptomatorConstants.mainAppBundleId
do {
OneDriveSetup.clientApplication = try MSALPublicClientApplication(configuration: oneDriveConfiguration)
let oneDriveConfiguration = MSALPublicClientApplicationConfig(clientId: CloudAccessSecrets.oneDriveClientId, redirectUri: CloudAccessSecrets.oneDriveRedirectURI, authority: nil)
oneDriveConfiguration.cacheConfig.keychainSharingGroup = CryptomatorConstants.mainAppBundleId
let oneDriveClientApplication = try MSALPublicClientApplication(configuration: oneDriveConfiguration)
OneDriveSetup.constants = OneDriveSetup(clientApplication: oneDriveClientApplication, sharedContainerIdentifier: nil)
} catch {
DDLogError("Setting up OneDrive failed with error: \(error)")
}
PCloudSetup.constants = PCloudSetup(appKey: CloudAccessSecrets.pCloudAppKey, sharedContainerIdentifier: nil)
return {}
}()

Expand Down