Skip to content

Commit

Permalink
Merge pull request #351 from cryptomator/bugfix/bg-session-identifier…
Browse files Browse the repository at this point in the history
…-collision

Fix background URLSession identifier collisions
  • Loading branch information
tobihagemann authored Apr 18, 2024
2 parents f53e591 + 3910d16 commit 3f16570
Show file tree
Hide file tree
Showing 10 changed files with 117 additions and 63 deletions.
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

0 comments on commit 3f16570

Please sign in to comment.