Added Box support #352
Added Box support #352
Conversation
WalkthroughThe recent changes integrate Box cloud storage into the Cryptomator application. This includes adding references to the Box SDK, updating dependencies, and implementing authentication and token storage for Box. Additionally, modifications were made to support Box in the cloud provider management system and keychain token storage. Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant AppDelegate
participant CloudAuthenticator
participant BoxSdkGen
participant BoxTokenStore
User ->> AppDelegate: Launch App
AppDelegate ->> BoxSdkGen: Initialize with clientId and clientSecret
User ->> CloudAuthenticator: Authenticate Box
CloudAuthenticator ->> BoxSdkGen: Perform Box Authentication
BoxSdkGen ->> CloudAuthenticator: Return AccessToken
CloudAuthenticator ->> BoxTokenStore: Store AccessToken
BoxTokenStore ->> CloudAuthenticator: Token Stored
CloudAuthenticator ->> User: Authentication Success
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Actionable comments outside the diff hunks (1)
fastlane/scripts/create-cloud-access-secrets.sh (1)
Line range hint
5-5: Replace 'source' with '.' for better POSIX compliance.- source ./scripts/.cloud-access-secrets.sh "${1}" + . ./scripts/.cloud-access-secrets.sh "${1}"
| @@ -49,7 +49,7 @@ class SettingsCoordinator: Coordinator { | |||
| } | |||
|
|
|||
| func showCloudServices() { | |||
| let viewModel = ChooseCloudViewModel(clouds: [.dropbox, .googleDrive, .oneDrive, .pCloud, .webDAV(type: .custom), .s3(type: .custom)], headerTitle: "") | |||
| let viewModel = ChooseCloudViewModel(clouds: [.box, .dropbox, .googleDrive, .oneDrive, .pCloud, .webDAV(type: .custom), .s3(type: .custom)], headerTitle: "") | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The integration of the Box cloud service in the settings and vault creation parts of the Cryptomator app appears consistent. However, Box is not included in the cloud service list in a preview instance within ChooseCloudViewController.swift. This might be intentional for simplification in previews, but it could also be an oversight.
- File:
Cryptomator/AddVault/ChooseCloudViewController.swift- Context: Preview instance creation without Box in the cloud service list.
Analysis chain
Integration of Box cloud service in settings looks good.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other occurrences of cloud service lists to ensure Box is included consistently.
rg --type swift 'clouds: \['
Length of output: 1320
| @@ -37,6 +37,7 @@ class AccountListViewModel: AccountListViewModelProtocol { | |||
|
|
|||
| private(set) var accounts = [AccountCellContent]() | |||
| private(set) var accountInfos = [AccountInfo]() | |||
| private(set) var boxCredentials = [String: BoxCredential]() | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The review of the boxCredentials dictionary in AccountListViewModel.swift shows that the dictionary is declared with a private(set) access modifier, which restricts its modification to within the class itself, thereby controlling external access. Additionally, there is a comment indicating a workaround related to a lifecycle issue, which involves adding an entry to the dictionary. However, there is no explicit evidence of memory management practices such as clearing the dictionary or setting it to nil, which could potentially lead to memory retention issues if not handled elsewhere in the code.
Given the lack of direct evidence of memory leaks or unauthorized access, but also the absence of explicit memory management code, it is recommended to:
- Review the lifecycle management of
boxCredentialsto ensure that entries are appropriately managed and do not lead to memory retention. - Ensure that access controls like
private(set)are consistently used to safeguard against unauthorized modifications.
Analysis chain
Ensure that the boxCredentials dictionary is properly managed to avoid memory leaks or unauthorized access.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for memory management practices related to dictionaries in Swift.
rg --type swift 'boxCredentials' --files-with-matches | grep 'deinit'
Length of output: 67
Script:
#!/bin/bash
# Search for all occurrences of 'boxCredentials' to understand its usage and management in the codebase.
rg --type swift 'boxCredentials'
Length of output: 340
# Conflicts: # Cryptomator.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved # Cryptomator/AppDelegate.swift # CryptomatorCommon/Package.swift # CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/CloudProviderDBManager.swift # FileProviderExtension/FileProviderExtension.swift # FileProviderExtensionUI/RootViewController.swift
There was a problem hiding this comment.
Actionable comments posted: 3
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- Cryptomator.xcodeproj/project.pbxproj (2 hunks)
- Cryptomator.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (1 hunks)
- Cryptomator/AppDelegate.swift (2 hunks)
- Cryptomator/Common/CloudAuthenticator.swift (3 hunks)
- CryptomatorCommon/Sources/CryptomatorCommonCore/Box/BoxTokenStore.swift (1 hunks)
Files skipped from review due to trivial changes (1)
- Cryptomator.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Additional comments not posted (4)
CryptomatorCommon/Sources/CryptomatorCommonCore/Box/BoxTokenStore.swift (1)
16-40: The implementation ofBoxTokenStoreadheres to theTokenStorageprotocol and provides methods for storing, retrieving, and clearing tokens. The error handling is consistent, and custom errors are thrown for specific keychain failures.Cryptomator/AppDelegate.swift (1)
53-53: The setup for Box cloud service is correctly implemented using the provided client ID and secret. This is consistent with the setup patterns used for other cloud services in the app.Cryptomator/Common/CloudAuthenticator.swift (2)
63-74: TheauthenticateBoxmethod is well-integrated into the existing authentication framework. It uses aBoxTokenStorefor token management and correctly handles the authentication flow. The method is asynchronous and uses promises, which is consistent with other cloud provider authentication methods in this class.
94-95: The integration of Box into the genericauthenticateanddeauthenticatemethods is correctly implemented. This ensures that Box is treated similarly to other supported cloud providers, maintaining consistency in the application's cloud authentication strategy.Also applies to: 115-118
| @@ -1032,6 +1032,7 @@ | |||
| 74F5DC1B26DCD2FB00AFE989 /* StoreObserver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoreObserver.swift; sourceTree = "<group>"; }; | |||
| 74F5DC1E26DD036D00AFE989 /* StoreManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoreManager.swift; sourceTree = "<group>"; }; | |||
| 74FC576025ADED030003ED27 /* VaultCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VaultCell.swift; sourceTree = "<group>"; }; | |||
| B31428D72C04D02800BD61D5 /* cloud-access-swift */ = {isa = PBXFileReference; lastKnownFileType = wrapper; name = "cloud-access-swift"; path = "../cloud-access-swift"; sourceTree = "<group>"; }; | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The replacement of cloud-access-swift with box-swift-sdk-gen is incomplete. References to cloud-access-swift still exist in the project file.
Cryptomator.xcodeproj/project.pbxproj:- Line:
B31428D72C04D02800BD61D5 /* cloud-access-swift */ = {isa = PBXFileReference; lastKnownFileType = wrapper; name = "cloud-access-swift"; path = "../cloud-access-swift"; sourceTree = "<group>"; }; - Line:
B31428D72C04D02800BD61D5 /* cloud-access-swift */
- Line:
Analysis chain
Verify the replacement of cloud-access-swift with box-swift-sdk-gen.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the replacement of `cloud-access-swift` with `box-swift-sdk-gen` in the project file.
# Test: Search for both references in the project file. Expect: Only `box-swift-sdk-gen` should be present.
rg --type pbxproj $'cloud-access-swift|box-swift-sdk-gen' Cryptomator.xcodeproj/project.pbxproj
Length of output: 128
Script:
#!/bin/bash
# Description: Verify the replacement of `cloud-access-swift` with `box-swift-sdk-gen` in the project file.
# Test: Search for both references in the project file. Expect: Only `box-swift-sdk-gen` should be present.
rg 'cloud-access-swift|box-swift-sdk-gen' Cryptomator.xcodeproj/project.pbxproj
Length of output: 332
| @@ -1334,6 +1335,7 @@ | |||
| 4A5E5B202453119100BD6298 = { | |||
| isa = PBXGroup; | |||
| children = ( | |||
| B31428D72C04D02800BD61D5 /* cloud-access-swift */, | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The replacement of cloud-access-swift with box-swift-sdk-gen has not been completed in the project file.
cloud-access-swiftis still present in the project file.- There is no mention of
box-swift-sdk-gen.
Analysis chain
Verify the replacement of cloud-access-swift with box-swift-sdk-gen.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the replacement of `cloud-access-swift` with `box-swift-sdk-gen` in the project file.
# Test: Search for both references in the project file. Expect: Only `box-swift-sdk-gen` should be present.
rg --type pbxproj $'cloud-access-swift|box-swift-sdk-gen' Cryptomator.xcodeproj/project.pbxproj
Length of output: 128
Script:
#!/bin/bash
# Description: Verify the replacement of `cloud-access-swift` with `box-swift-sdk-gen` in the project file.
# Test: Search for both references in the project file. Expect: Only `box-swift-sdk-gen` should be present.
rg 'cloud-access-swift|box-swift-sdk-gen' Cryptomator.xcodeproj/project.pbxproj
Length of output: 332
| extension CryptomatorKeychain { | ||
| func getBoxTokenInfo() -> AccessToken? { | ||
| guard let data = getAsData("foo") else { | ||
| return nil | ||
| } | ||
| do { | ||
| let jsonDecoder = JSONDecoder() | ||
| return try jsonDecoder.decode(AccessToken.self, from: data) | ||
| } catch { | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| func saveBoxTokenInfo(_ tokenInfo: AccessToken) throws { | ||
| let jsonEncoder = JSONEncoder() | ||
| let encodedUser = try jsonEncoder.encode(tokenInfo) | ||
| try set("foo", value: encodedUser) | ||
| let encodedToken = try jsonEncoder.encode(tokenInfo) | ||
| try set("foo", value: encodedToken) | ||
| } | ||
|
|
||
| func deleteTokenInfo() throws { | ||
| try delete("foo") | ||
| } | ||
| } |
There was a problem hiding this comment.
The extension to CryptomatorKeychain introduces methods to handle Box token information. However, there's a potential issue with the hardcoded string "foo" used as a key. This could lead to conflicts or unintended behavior if other parts of the application use the same key for different purposes.
- func getBoxTokenInfo() -> AccessToken? {
- guard let data = getAsData("foo") else {
- return nil
- }
- ...
- try set("foo", value: encodedToken)
- }
+ func getBoxTokenInfo() -> AccessToken? {
+ guard let data = getAsData("boxToken") else {
+ return nil
+ }
+ ...
+ try set("boxToken", value: encodedToken)
+ }Committable suggestion was skipped due low confidence.
Adds support for Box as a cloud provider. (WIP)