-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
Use hub subscription header #333
Conversation
WalkthroughThe changes involve updating the authentication process in the CryptomatorHubAuthenticator. The success case of the Changes
Poem
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- CryptomatorCommon/Sources/CryptomatorCommonCore/Hub/CryptomatorHubAuthenticator.swift (4 hunks)
Additional comments: 3
CryptomatorCommon/Sources/CryptomatorCommonCore/Hub/CryptomatorHubAuthenticator.swift (3)
- 70-74: The changes made to the
switch
statement to capture the header in the.success
case are consistent with the PR objectives. The variablesencryptedVaultKey
andunlockHeader
are correctly assigned from the tuple returned by thegetVaultMasterKey
function.However, it's important to ensure that all other parts of the code that depend on the
HubAuthenticationFlowSuccess
structure and thegetVaultMasterKey
function are updated to handle the new tuple format.Verification successful
The verification process has revealed that the changes to the
getVaultMasterKey
function are localized to theCryptomatorHubAuthenticator.swift
file, and there is no evidence of theHubAuthenticationFlowSuccess
structure being used elsewhere in the codebase. This suggests that the changes made in the pull request are self-contained and consistent with the PR objectives, which state that the changes are confined to theCryptomatorHubAuthenticator.swift
file within theCryptomatorCommonCore
module.* 98-98: The return statement in line 98 correctly constructs a `HubAuthenticationFlowSuccess` object with the `encryptedUserKeyJWE`, `encryptedVaultKeyJWE`, and `unlockHeader`. This aligns with the changes made to the `HubAuthenticationFlowSuccess` struct to include the header. It's crucial to ensure that the consumers of this success case are expecting a tuple and handle the header appropriately.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new tuple format is correctly handled wherever `HubAuthenticationFlowSuccess` is used. ast-grep --lang swift --pattern $'HubAuthenticationFlowSuccess($_, $_, $_)' # Verify that the `getVaultMasterKey` function's new return type is correctly handled. ast-grep --lang swift --pattern $'getVaultMasterKey($_, $_, $_).success($_, $_)'Length of output: 158
Script:
#!/bin/bash # Search for the usage of the HubAuthenticationFlowSuccess structure with a more general pattern. ast-grep --lang swift --pattern $'HubAuthenticationFlowSuccess($$$)' # Search for the usage of the getVaultMasterKey function with a more general pattern. ast-grep --lang swift --pattern $'getVaultMasterKey($$$)'Length of output: 659
- 304-304: The update to the
RetrieveVaultMasterkeyEncryptedForUserResponse
enum to include the header in the.success
case is in line with the PR's objectives. This change is necessary to pass the header information along with the encrypted vault key. It's important to verify that all usages of this enum have been updated to handle the new case with the header.
During the implementation of #332 we somehow forgot to actually pass the header, which includes the Cryptomator Hub subscription state since Hub 1.3.1, to the
HubAuthenticationFlow
after a successful unlock of the Cryptomator Hub vault.This has now been fixed 🎉
Summary by CodeRabbit