Added Box support #352
Added Box support #352
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe recent changes integrate Box cloud storage into the Cryptomator application, enhancing support for Box accounts through authentication, credential management, and various structural updates. The project has been organized to accommodate these features, including updates to dependencies and modifications to class methods related to Box functionalities. Changes
Sequence Diagram(s)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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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}"
# 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
CryptomatorCommon/Sources/CryptomatorCommonCore/Box/BoxTokenStore.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- Cryptomator.xcodeproj/project.pbxproj (2 hunks)
- Cryptomator/Common/CloudAccountList/AccountListViewModel.swift (5 hunks)
- Cryptomator/Common/CloudAuthenticator.swift (3 hunks)
- Cryptomator/Settings/SettingsCoordinator.swift (1 hunks)
- Cryptomator/VaultDetail/VaultDetailInfoFooterViewModel.swift (2 hunks)
- CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/CloudProviderDBManager.swift (2 hunks)
- CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/CloudProviderType.swift (1 hunks)
Additional comments not posted (14)
CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/CloudProviderType.swift (1)
17-17: Addition ofboxcase is appropriate.The addition of the
boxcase to theCloudProviderTypeenum is consistent with the PR's objective of adding Box support.Cryptomator/VaultDetail/VaultDetailInfoFooterViewModel.swift (2)
47-51: Integration of Box ingetUsernamemethod is consistent.The addition of the Box case in the
getUsernamemethod is consistent with the existing structure and supports the new cloud provider.
97-103: Addition ofgetUsername(for: BoxCredential)is well-implemented.The new function
getUsername(for: BoxCredential)is implemented in line with the existing pattern for other cloud providers.Cryptomator/Settings/SettingsCoordinator.swift (1)
52-52: Addition of Box to cloud services list is appropriate.The inclusion of Box in the
cloudsparameter ofChooseCloudViewModelaligns with the PR's objective and enhances the user experience by providing more cloud service options.Cryptomator/Common/CloudAuthenticator.swift (3)
63-74: LGTM!The
authenticateBoxfunction is implemented correctly and follows the existing pattern for other cloud providers.
94-95: LGTM!The addition of the
.boxcase in theauthenticatefunction is consistent with the existing structure for other cloud providers.
115-118: LGTM!The addition of the
.boxcase in thedeauthenticatefunction aligns with the existing logic for other cloud providers.CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/CloudProviderDBManager.swift (2)
78-81: LGTM!The addition of the
.boxcase in thecreateProviderfunction is consistent with the existing pattern for other cloud providers.
129-132: LGTM!The addition of the
.boxcase in thecreateBackgroundSessionProviderfunction aligns with the existing logic for other cloud providers.Cryptomator/Common/CloudAccountList/AccountListViewModel.swift (5)
70-82: LGTM!The
refreshBoxItemsfunction is implemented correctly and uses promises for asynchronous handling.
87-97: LGTM!The modification to return a placeholder for Box accounts in
createAccountCellContentis consistent with the handling of other cloud providers.
114-116: LGTM!The
createAccountCellContentPlaceholderfunction is straightforward and correctly implemented.
140-143: LGTM!The
createAccountCellContentfunction forBoxCredentialis implemented correctly and uses promises for asynchronous handling.
40-40: Verify memory management forboxCredentials.Ensure that the lifecycle of
boxCredentialsis properly managed to avoid potential memory retention issues.Run the following script to verify memory management practices:
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- Cryptomator.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (1 hunks)
- CryptomatorCommon/Package.swift (1 hunks)
Additional comments not posted (3)
CryptomatorCommon/Package.swift (1)
29-29: Consider the implications of using a branch-based dependency.Switching from a fixed version to a branch-based dependency (
develop) forcloud-access-swiftcan introduce instability if the branch contains untested or experimental code. Ensure that the branch is stable and regularly tested.To verify the stability of the
developbranch, you might want to check the branch's commit history and test coverage.Cryptomator.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (2)
57-65: Verify the compatibility of the newbox-swift-sdk-gendependency.Ensure that the new dependency
box-swift-sdk-genis compatible with the existing codebase and does not introduce conflicts with other dependencies.Consider running a dependency compatibility check or reviewing the documentation for potential issues.
71-72: Ensure consistency withPackage.swiftforcloud-access-swift.The
cloud-access-swiftdependency is now using a branch reference (develop). Ensure this is consistent with thePackage.swiftfile and verify that the branch is stable.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- Cryptomator.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (6 hunks)
- Cryptomator/AppDelegate.swift (1 hunks)
- Cryptomator/Common/CloudAccountList/AccountListViewController.swift (3 hunks)
- Cryptomator/Common/CloudAccountList/AccountListViewModel.swift (4 hunks)
- Cryptomator/Common/CloudAuthenticator.swift (2 hunks)
- Cryptomator/Common/CloudProviderType+Localization.swift (1 hunks)
- Cryptomator/Common/UIImage+CloudProviderType.swift (2 hunks)
- Cryptomator/Settings/SettingsCoordinator.swift (1 hunks)
- Cryptomator/VaultDetail/VaultDetailInfoFooterViewModel.swift (3 hunks)
- CryptomatorCommon/Sources/CryptomatorCommonCore/Box/BoxTokenStorage.swift (1 hunks)
- CryptomatorCommon/Sources/CryptomatorCommonCore/CryptomatorKeychain.swift (1 hunks)
- CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/CloudProviderDBManager.swift (2 hunks)
- CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/CloudProviderType.swift (1 hunks)
- CryptomatorIntents/SaveFileIntentHandler.swift (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- Cryptomator.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
- Cryptomator/Common/CloudAccountList/AccountListViewModel.swift
- Cryptomator/Common/CloudAuthenticator.swift
- Cryptomator/Settings/SettingsCoordinator.swift
- CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/CloudProviderType.swift
Additional context used
Learnings (2)
CryptomatorCommon/Sources/CryptomatorCommonCore/Box/BoxTokenStorage.swift (1)
Learnt from: iammajid PR: cryptomator/ios#352 File: CryptomatorCommon/Sources/CryptomatorCommonCore/Box/BoxTokenStore.swift:43-67 Timestamp: 2024-06-24T07:17:45.973Z Learning: The hardcoded key "foo" in `CryptomatorKeychain` methods for handling Box token information is acknowledged by the user and is kept unchanged for now.CryptomatorCommon/Sources/CryptomatorCommonCore/CryptomatorKeychain.swift (1)
Learnt from: iammajid PR: cryptomator/ios#352 File: CryptomatorCommon/Sources/CryptomatorCommonCore/Box/BoxTokenStore.swift:43-67 Timestamp: 2024-06-24T07:17:45.973Z Learning: The hardcoded key "foo" in `CryptomatorKeychain` methods for handling Box token information is acknowledged by the user and is kept unchanged for now.
Additional comments not posted (24)
Cryptomator/Common/CloudProviderType+Localization.swift (2)
15-16: Addition of Box case inlocalizedString().The addition of the
.boxcase is consistent with the existing pattern for other cloud providers. This change enhances the localization support for Box.
21-22: Reordering and handling of.localFileSystem.The reordering of the
.localFileSystemcase improves the organization of the code. The call tolocalizedString()onlocalFileSystemTypeis appropriate and maintains consistency.Cryptomator/Common/UIImage+CloudProviderType.swift (4)
21-22: Addition of Box case invaultIconFor.The addition of the
.boxcase with the asset name "box-vault" is consistent with the existing pattern for other cloud providers. This change enhances the icon support for Box.
27-28: Reordering and handling of.localFileSysteminvaultIconFor.The reordering of the
.localFileSystemcase and the use ofUIImage.getVaultIcon(for:)improves the organization and clarity of the code.
56-57: Addition of Box case instorageIconFor.The addition of the
.boxcase with the asset name "box" is consistent with the existing pattern for other cloud providers. This change enhances the icon support for Box.
62-63: Reordering and handling of.localFileSysteminstorageIconFor.The reordering of the
.localFileSystemcase and the use ofUIImage.getStorageIcon(for:)improves the organization and clarity of the code.CryptomatorCommon/Sources/CryptomatorCommonCore/Box/BoxTokenStorage.swift (5)
12-22: Introduction ofBoxTokenStorageclass.The
BoxTokenStorageclass is well-structured and implements theTokenStorageprotocol. The use ofdidSetforuserIDto handle pending tokens is a thoughtful design choice.Tools
SwiftLint
[Warning] 15-15: Prefer
!= niloverlet _ =(unused_optional_binding)
30-36: Implementation ofstore(token:).The
store(token:)method correctly handles the absence ofuserIDby storing the token as pending. The use ofCryptomatorKeychainfor saving tokens is appropriate.
38-46: Implementation ofget().The
get()method efficiently retrieves a pending token or fetches it from the keychain. The logic is clear and handles the absence ofuserIDgracefully.
48-53: Implementation ofclear().The
clear()method correctly deletes the token from the keychain ifuserIDis present. The logic is straightforward and effective.
56-77: Extension ofCryptomatorKeychainfor Box tokens.The extension methods for handling Box tokens in
CryptomatorKeychainare well-implemented. They provide clear and concise methods for token management.Cryptomator/VaultDetail/VaultDetailInfoFooterViewModel.swift (4)
47-51: Box integration ingetUsernamelooks good.The changes correctly integrate Box as a cloud provider.
97-103: Asynchronous username retrieval for Box is well-implemented.The function is consistent with similar functions for other providers.
Line range hint
83-87: No changes detected ingetUsername(for credential: DropboxCredential).The function is consistent with the new Box implementation.
Line range hint
91-95: No changes detected ingetUsername(for credential: PCloudCredential).The function is consistent with the new Box implementation.
CryptomatorCommon/Sources/CryptomatorCommonCore/CryptomatorKeychain.swift (1)
26-26: Addition ofboxproperty is consistent and enhances functionality.The new static property
boxis consistent with other cloud providers and enhances the functionality of theCryptomatorKeychainclass.Cryptomator/AppDelegate.swift (1)
52-52: Box setup inAppDelegateis consistent and enhances capability.The addition of Box setup is consistent with other cloud storage services and enhances the application's capability to support Box.
CryptomatorIntents/SaveFileIntentHandler.swift (1)
126-127: Addition of Box Asset NameThe
.boxcase has been correctly added to return "box-vault", aligning with the integration of Box as a cloud provider. Ensure that this asset name is used consistently across the application.Cryptomator/Common/CloudAccountList/AccountListViewController.swift (2)
16-16: Conformance to ASWebAuthenticationPresentationContextProvidingThe class now conforms to
ASWebAuthenticationPresentationContextProviding, which is necessary for handling web authentication sessions. This is a good addition for supporting Box authentication.
143-143: Addition of Box Case in supportsEditingThe
.boxcase has been added to thesupportsEditingfunction, which correctly reflects the editing support logic for Box accounts.CryptomatorCommon/Sources/CryptomatorCommonCore/Manager/CloudProviderDBManager.swift (4)
65-68: Addition of Box Provider CreationThe
.boxcase has been added to the provider creation method, correctly initializing theBoxCloudProviderwith the necessary credentials. This aligns with the Box integration.
87-89: Addition of S3 Provider CreationThe
.s3case has been added to the provider creation method, initializing theS3CloudProviderwith the appropriate credentials. This change supports the integration of S3.
116-119: Addition of Box Background Session ProviderThe
.boxcase has been added to the background session creation method, usingBoxCloudProvider.withBackgroundSession. This is a correct implementation for handling background sessions with Box.
138-140: Addition of S3 Background Session ProviderThe
.s3case has been added to the background session creation method, usingS3CloudProvider.withBackgroundSession. This addition is consistent with the S3 integration.
Cryptomator/Common/CloudAccountList/AccountListViewController.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- Cryptomator/Common/CloudAccountList/AccountListViewModel.swift (3 hunks)
- Cryptomator/Common/CloudAuthenticator.swift (2 hunks)
- CryptomatorCommon/Sources/CryptomatorCommonCore/Box/BoxTokenStorage.swift (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- Cryptomator/Common/CloudAccountList/AccountListViewModel.swift
- Cryptomator/Common/CloudAuthenticator.swift
- CryptomatorCommon/Sources/CryptomatorCommonCore/Box/BoxTokenStorage.swift
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- Cryptomator.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (6 hunks)
- Cryptomator/Common/CloudAccountList/AccountListViewController.swift (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- Cryptomator.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Additional comments not posted (2)
Cryptomator/Common/CloudAccountList/AccountListViewController.swift (2)
110-115: Improved error handling inpresentationAnchor.The method now logs a message instead of using
fatalError, which prevents the app from crashing if no window is found. This is a good practice for enhancing app stability.
145-145: Updated editing support for Box.The addition of
.boxto the list of cloud providers that do not support editing aligns with the new Box integration.
[ci skip]
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
Scripts/git-format-staged.py (1)
40-40: Theverboseparameter is not utilized in the current codebase.The
verboseparameter has been added to the function signature but is not being used in the function call withinScripts/git-format-staged.py. Consider reviewing its necessity or implementing its usage where appropriate.
- Location:
Scripts/git-format-staged.py
- Function definition includes
verboseparameter.- Function call does not utilize
verbose.Analysis chain
Addition of
verboseparameter is beneficial.The
verboseparameter enhances the script by allowing detailed logging, which is useful for debugging.Ensure that the
verboseparameter is correctly used throughout the codebase.Run the following script to verify the usage of the
verboseparameter:Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `format_staged_files` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type python -A 5 $'format_staged_files'Length of output: 74
Script:
#!/bin/bash # Description: Verify all function calls to `format_staged_files` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg -A 5 $'format_staged_files'Length of output: 944
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- Scripts/git-format-staged.py (11 hunks)
Additional context used
Ruff
Scripts/git-format-staged.py
24-24:
gettext.gettextimported but unusedRemove unused import:
gettext.gettext(F401)
Additional comments not posted (5)
Scripts/git-format-staged.py (5)
1-1: Shebang line update is appropriate.The change to explicitly require Python 3 is a good practice for ensuring compatibility with modern Python versions.
65-65: Addition ofverboseparameter is consistent.The
verboseparameter is consistent with the changes informat_staged_filesand allows for detailed logging.
94-101: Verbose logging enhances command execution transparency.The addition of a
verboseparameter and logic to print the command being executed improves the transparency and debuggability of the script.
90-90: Use of raw string notation for regex is appropriate.The change to use raw string notation for regular expressions improves readability and prevents potential issues with escape sequences.
Also applies to: 171-171, 189-189
261-265: Addition of--verboseflag is beneficial.The
--verboseflag allows users to enable detailed logging, enhancing the user experience by providing more context during execution.
| import os | ||
| import re | ||
| import subprocess | ||
| import sys | ||
| from fnmatch import fnmatch | ||
| from gettext import gettext as _ |
There was a problem hiding this comment.
Remove unused import.
The gettext.gettext import is not used in the script and should be removed to clean up the code.
Apply this diff to remove the unused import:
-from gettext import gettext as _Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from gettext import gettext as _ |
Tools
Ruff
24-24:
gettext.gettextimported but unusedRemove unused import:
gettext.gettext(F401)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- Cryptomator/Resources/about.html (1 hunks)
Files skipped from review due to trivial changes (1)
- Cryptomator/Resources/about.html
Adds support for Box as a cloud provider. (WIP)