Refactored User Keys #282
Refactored User Keys #282
Conversation
[ci skip]
both stored among devices and encrypted using the setup code
WalkthroughThe recent changes introduce a 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Outside diff range and nitpick comments (8)
frontend/src/router/index.ts (1)
Line range hint
152-152: Consider removing thedeleteoperator to improve performance.- delete to.query.sync_me; + Reflect.deleteProperty(to.query, 'sync_me');frontend/src/common/backend.ts (6)
Line range hint
164-164: Use template literals for better readability and consistency.- return axiosAuth.put(`/vaults/${vaultId}/users/${userId}` + (role ? `?role=${role}` : '')) + return axiosAuth.put(`/vaults/${vaultId}/users/${userId}${role ? `?role=${role}` : ''}`) - return axiosAuth.put(`/vaults/${vaultId}/groups/${groupId}` + (role ? `?role=${role}` : '')) + return axiosAuth.put(`/vaults/${vaultId}/groups/${groupId}${role ? `?role=${role}` : ''}`)Also applies to: 169-169
Line range hint
220-220: Specify a more precise type instead ofanyto enhance type safety.Please replace
anywith a more specific type where possible to ensure type safety and better code maintainability.Also applies to: 225-225, 374-374
Line range hint
235-235: Remove unnecessary type annotation.The type annotation on this variable is redundant and can be inferred from its initialization.
Line range hint
264-273: Simplify control flow by removing unnecessary else clauses.- if (condition) { - // code - } else { - return; - } + if (condition) { + // code + } + return;Also applies to: 377-379
Line range hint
1-1: Remove unused imports to clean up the code.Please remove the unused imports to keep the codebase clean and maintainable.
Also applies to: 1-2, 5-6
Line range hint
152-152: Avoid unnecessary variable reassignments.Consider initializing these variables directly with their intended values to avoid unnecessary reassignments.
Also applies to: 265-265
backend/src/main/java/org/cryptomator/hub/api/VaultResource.java (1)
Line range hint
339-339: Optimize the process of granting access.- const vaultKeyJwe = keys.encryptForUser(base64.parse(me.value.ecdhPublicKey)); - try { - await backend.vaults.grantAccess(props.vaultId, { userId: me.value.id, token: await vaultKeyJwe }); - } catch (error) { - if (error instanceof ConflictError) { - console.debug('User already member of this vault.'); - } else { - console.error('Failed to grant access to self.', error); - } - } + try { + const vaultKeyJwe = await keys.encryptForUserAndGrantAccess(me.value.ecdhPublicKey, props.vaultId, me.value.id); + } catch (error) { + console.error('Failed to grant access to self.', error); + }This refactor simplifies the process by moving the encryption and grant access logic into a single method, reducing complexity and potential for errors.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
backend/src/main/resources/org/cryptomator/hub/flyway/ERM.pngis excluded by!**/*.png
Files selected for processing (25)
- backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java (3 hunks)
- backend/src/main/java/org/cryptomator/hub/api/UserDto.java (1 hunks)
- backend/src/main/java/org/cryptomator/hub/api/UsersResource.java (4 hunks)
- backend/src/main/java/org/cryptomator/hub/api/VaultResource.java (1 hunks)
- backend/src/main/java/org/cryptomator/hub/entities/Device.java (4 hunks)
- backend/src/main/java/org/cryptomator/hub/entities/User.java (4 hunks)
- backend/src/main/resources/org/cryptomator/hub/flyway/V15__User_ECDSA.sql (1 hunks)
- backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java (5 hunks)
- backend/src/test/resources/org/cryptomator/hub/flyway/V9999__Test_Data.sql (2 hunks)
- frontend/src/common/backend.ts (1 hunks)
- frontend/src/common/crypto.ts (6 hunks)
- frontend/src/common/userdata.ts (1 hunks)
- frontend/src/components/AuthenticatedMain.vue (2 hunks)
- frontend/src/components/CreateVault.vue (2 hunks)
- frontend/src/components/DeviceList.vue (2 hunks)
- frontend/src/components/GrantPermissionDialog.vue (2 hunks)
- frontend/src/components/InitialSetup.vue (6 hunks)
- frontend/src/components/ManageSetupCode.vue (2 hunks)
- frontend/src/components/RecoverVaultDialog.vue (1 hunks)
- frontend/src/components/RegenerateSetupCodeDialog.vue (2 hunks)
- frontend/src/components/UnlockSuccess.vue (6 hunks)
- frontend/src/components/UserProfile.vue (3 hunks)
- frontend/src/components/VaultDetails.vue (4 hunks)
- frontend/src/router/index.ts (2 hunks)
- frontend/test/common/crypto.spec.ts (4 hunks)
Additional context used
Biome
frontend/src/common/userdata.ts
[error] 32-35: This else clause can be omitted because previous branches break early.
[error] 34-34: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 1-2: Some named imports are only used as types.
[error] 95-95: This let declares a variable that is only assigned once.
frontend/src/router/index.ts
[error] 136-136: The computed expression can be simplified without the use of a string literal.
[error] 152-152: Avoid the delete operator which can impact performance.
[error] 1-1: Some named imports are only used as types.
frontend/src/common/backend.ts
[error] 27-27: The computed expression can be simplified without the use of a string literal.
[error] 164-164: Template literals are preferred over string concatenation.
[error] 169-169: Template literals are preferred over string concatenation.
[error] 220-220: Unexpected any. Specify a different type.
[error] 225-225: Unexpected any. Specify a different type.
[error] 235-235: This type annotation is trivially inferred from its initialization.
[error] 264-273: This else clause can be omitted because previous branches break early.
[error] 374-374: Unexpected any. Specify a different type.
[error] 377-379: This else clause can be omitted because previous branches break early.
[error] 1-1: Some named imports are only used as types.
[error] 1-2: Some named imports are only used as types.
[error] 5-6: All these imports are only used as types.
[error] 152-152: This let declares a variable that is only assigned once.
[error] 265-265: This let declares a variable that is only assigned once.
frontend/test/common/crypto.spec.ts
[error] 34-34: Declare variables separately
[error] 74-74: The computed expression can be simplified without the use of a string literal.
[error] 147-147: The computed expression can be simplified without the use of a string literal.
[error] 244-246: This constructor is unnecessary.
[error] 268-270: This constructor is unnecessary.
[error] 43-43: This let declares a variable that is only assigned once.
[error] 44-44: This let declares a variable that is only assigned once.
[error] 64-64: This let declares a variable that is only assigned once.
frontend/src/common/crypto.ts
[error] 6-6: Unexpected any. Specify a different type.
[error] 8-8: Unexpected any. Specify a different type.
[error] 198-198: Template literals are preferred over string concatenation.
[error] 205-205: Template literals are preferred over string concatenation.
[error] 420-422: This else clause can be omitted because previous branches break early.
[error] 462-464: This else clause can be omitted because previous branches break early.
Additional comments not posted (63)
backend/src/main/resources/org/cryptomator/hub/flyway/V15__User_ECDSA.sql (4)
1-1: Renamingpublickeytoecdh_publickeyaligns with the new data model requirements.
2-2: Renamingprivatekeytoprivatekeyscorrectly reflects the storage of multiple keys.
3-3: Adding theecdsa_publickeycolumn is essential for supporting the new cryptographic functionality.
4-4: Renaminguser_privatekeytouser_privatekeysin thedevicetable ensures consistency across the database schema.frontend/src/components/AuthenticatedMain.vue (2)
24-25: Updated imports to use the centralizeduserdatamodule enhance maintainability and reduce redundancy.
39-39: Usinguserdata.mefor fetching user data centralizes and simplifies data management in the frontend.backend/src/main/java/org/cryptomator/hub/api/UserDto.java (3)
17-22: The addition ofecdhPublicKey,ecdsaPublicKey, and the renaming ofprivateKeytoprivateKeysare well-annotated and correctly reflect the new cryptographic architecture.
26-28: Maintaining a deprecatedpublicKeyfor backward compatibility while transitioning to new key structures is a prudent approach.
31-41: The constructor's update to support new cryptographic fields while maintaining backward compatibility is well-implemented.backend/src/main/java/org/cryptomator/hub/entities/Device.java (4)
59-60: Updating the field name touserPrivateKeysaligns with the new cryptographic key management approach.
105-110: The updated getter and setter methods foruserPrivateKeysensure consistency with the database schema changes.
129-129: IncludinguserPrivateKeysin thetoStringmethod ensures comprehensive logging and debugging capabilities.
144-144: Updating theequalsmethod to includeuserPrivateKeysensures accurate equality checks.frontend/src/components/ManageSetupCode.vue (2)
52-52: Using the centralizeduserdatamodule for managing setup codes enhances consistency and maintainability in the frontend architecture.
71-76: Centralizing the retrieval of setup codes throughuserdatasimplifies the management of sensitive data and enhances security.backend/src/main/java/org/cryptomator/hub/entities/User.java (4)
29-29: Ensure the query condition is correctly updated to useecdhPublicKey.
52-59: The addition ofecdhPublicKey,ecdsaPublicKey, andprivateKeyscolumns aligns with the PR's objective to handle multiple key types.
80-101: The getter and setter methods forecdhPublicKey,ecdsaPublicKey, andprivateKeysare correctly implemented.
142-150: TheequalsandhashCodemethods have been updated to include the new key fields, ensuring consistency in entity comparison and hash-based collections.frontend/src/components/UnlockSuccess.vue (4)
2-2: The conditional rendering based onaccountStateandhasBrowserKeysensures that the appropriate navigation bar is displayed.
41-41: The router link conditionally displayed based onhasBrowserKeysprovides a direct way for users to access their account key if available.
71-71: Importinguserdatafrom../common/userdatais essential for accessing user-specific data, aligning with the frontend changes described in the PR.
Line range hint
84-132: The computed properties and thefetchDatafunction are well-implemented to handle user setup states and fetch necessary data.frontend/src/components/UserProfile.vue (2)
57-57: TheUserkeyFingerprintcomponent is correctly receiving theecdhPublicKeyas a prop, reflecting the backend changes to user key handling.
71-71: The use ofuserdatafor fetching user details and the correct handling of potential errors infetchDataare in line with the PR's objectives to centralize user data management.Also applies to: 94-94
frontend/src/router/index.ts (2)
5-5: The import ofuserdatais crucial for the new user data management strategy described in the PR.
165-174: The new routing guards useuserdatato check for user setup and browser keys, ensuring that users are redirected to setup if necessary.backend/src/main/java/org/cryptomator/hub/api/UsersResource.java (3)
73-75: The update methods forecdhPublicKey,ecdsaPublicKey, andprivateKeysinputMeare correctly implemented to reflect the new key management strategy.
120-122: ThegetMemethod correctly maps device details and includes the new key fields in theUserDto.
134-135: TheresetMemethod correctly handles the nullification of keys, aligning with security best practices for account resets.frontend/src/components/RecoverVaultDialog.vue (1)
108-109: TherecoverVaultmethod correctly uses theecdhPublicKeyfor encryption, aligning with the updated key management system.frontend/src/components/DeviceList.vue (3)
88-88: Refactored to useuserdatafor fetching user data.This change centralizes the data fetching logic, which should help in maintaining and updating the code in the future.
105-106: Refactored to useuserdatafor fetching user and device data.This change simplifies the data fetching process by using the
userdatamodule, which is consistent with the PR's objective to centralize user data handling.
117-117: Added call touserdata.reload()after removing a device.This ensures that the UI is updated with the latest data after a device is removed, maintaining the integrity of the displayed information.
frontend/src/components/GrantPermissionDialog.vue (2)
104-104: Updated to fetch fingerprints using the newecdhPublicKey.This change aligns with the updated data model where
ecdhPublicKeyis used instead of the deprecatedpublicKey.
127-128: Updated encryption logic to useecdhPublicKey.This ensures that the encryption process uses the correct public key format as per the new data model, enhancing security and compatibility.
backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java (2)
104-104: Updated to storeuserPrivateKeysin the device.This change is necessary to accommodate the new key management strategy that involves handling multiple keys, aligning with the PR's objectives.
175-180: RefactoredDeviceDtoto use new key fields.This update ensures that the device data transfer objects are consistent with the new database schema and the updated key management system.
frontend/src/components/RegenerateSetupCodeDialog.vue (2)
106-106: Refactored to useuserdatafor user data operations.This change centralizes user data handling, making the code cleaner and more maintainable.
153-157: Updated to useuserdatafor key decryption and setup code regeneration.This ensures that all user key handling is centralized and uses the updated data model, which is crucial for maintaining security and data integrity.
backend/src/test/resources/org/cryptomator/hub/flyway/V9999__Test_Data.sql (2)
17-20: Updated test data to include new key fields (ecdh_publickey,ecdsa_publickey,privatekeys).This update is necessary to ensure that the test environment reflects the new data model, allowing for accurate testing of the system with the updated key management.
55-55: Updated device test data to includeuser_privatekeys.This change aligns the test data with the new database schema, ensuring that tests will accurately reflect the production environment.
frontend/src/common/backend.ts (1)
75-76: Update toUserDtoto include ECDSA keys aligns with backend changes.frontend/test/common/crypto.spec.ts (1)
44-44: Consider usingconstinstead ofletforecdhP384andecdsaP384since these variables are not reassigned.
[REFACTOR_SUGGESTion]- let ecdhP384: EcKeyImportParams = { name: 'ECDH', namedCurve: 'P-384' }; - let ecdsaP384: EcKeyImportParams = { name: 'ECDSA', namedCurve: 'P-384' }; + const ecdhP384: EcKeyImportParams = { name: 'ECDH', namedCurve: 'P-384' }; + const ecdsaP384: EcKeyImportParams = { name: 'ECDSA', namedCurve: 'P-384' };Also applies to: 43-43
frontend/src/components/InitialSetup.vue (5)
196-196: Refactor to useuserdatamodule for user data management.This change centralizes user data handling, improving maintainability and consistency across the application.
242-246: Ensure proper handling of user setup states.The logic correctly handles different states based on the user's setup status, which enhances the user experience by providing clear state management.
261-269: Update user key handling to use new ECDSA keys.This update aligns with the backend changes for ECDSA key management, ensuring that the frontend correctly handles the new key types.
285-288: Refactor to useuserdatafor key recovery.This change simplifies the key recovery process by utilizing the
userdatamodule, which centralizes key management logic.
310-310: Trigger a reload of user data after updating.Reloading the user data after updates ensures that the UI reflects the most current state, which is crucial for user experience.
frontend/src/common/crypto.ts (6)
36-43: IntroduceUserKeyPayloadinterface with deprecation notice.This interface update is necessary for transitioning to the new key structure while maintaining backward compatibility.
231-232: Refactor encryption method to support new key types.This method now supports encryption using either ECDH or ECDSA keys, aligning with the new cryptographic requirements.
263-325: Enhance key management with support for ECDSA.These changes introduce support for ECDSA keys, which are essential for the new security features in the application.
353-355: Improve encryption with setup code.This method provides enhanced security by allowing encryption of private keys using a setup code, which is crucial for secure key management.
370-381: Prepare keys for encryption with added security measures.This method prepares keys for encryption while ensuring that sensitive data is securely handled, which is critical for maintaining data integrity.
459-465: Refactor key import function for clarity and efficiency.This refactoring improves the clarity and efficiency of the key import process, making the code easier to maintain and understand.
backend/src/main/java/org/cryptomator/hub/api/VaultResource.java (1)
Line range hint
323-323: Validate user initialization before generating JWT.frontend/src/components/VaultDetails.vue (3)
219-219: Useuserdataconsistently for user data management.This line correctly uses the
userdatamodule for managing user data, aligning with the frontend changes described in the PR.
217-217: EnsureVaultKeysis used correctly throughout the component.Verification successful
The
VaultKeysimport appears to be used correctly and consistently throughout theVaultDetails.vuecomponent. Here are the relevant usages:
- Import statement:
import { VaultKeys } from '../common/crypto';- Declaration:
const vaultKeys = ref<VaultKeys>();- Assignment:
vaultKeys.value = await loadVaultKeys(vaultKeyJwe);- Function return type:
async function loadVaultKeys(vaultKeyJwe: string): Promise<VaultKeys> {- Method call:
return VaultKeys.decryptWithUserKey(vaultKeyJwe, userKeys.ecdhKeyPair.privateKey);- Function parameter:
async function provedOwnership(keys: VaultKeys, ownerKeyPair: CryptoKeyPair) {These usages indicate that
VaultKeysis being utilized as expected within the component.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `VaultKeys` is used correctly and consistently throughout the component. # Test: Search for `VaultKeys` usage. Expect: Consistent and correct usage patterns. rg --type vue $'VaultKeys' ./frontend/src/components/VaultDetails.vueLength of output: 96
Script:
#!/bin/bash # Description: Verify that `VaultKeys` is used correctly and consistently throughout the component. # Test: Search for `VaultKeys` usage. Expect: Consistent and correct usage patterns. rg 'VaultKeys' ./frontend/src/components/VaultDetails.vueLength of output: 431
323-323: Check user initialization before generating JWT.backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java (4)
449-449: Update SQL query to reflect new column names.The SQL update statement correctly reflects the new column names (
ecdh_publickey,ecdsa_publickey,privatekeys) as per the PR's objectives to handle cryptographic keys more securely.
562-562: Insert test data to simulate user and group membership.The SQL insert statement correctly adds test data for
user999with the necessary cryptographic details. This setup is essential for testing the new cryptographic key management features introduced in the PR.
503-503: Ensure proper cleanup of test modifications.This script checks if the cleanup SQL query effectively removes all test modifications related to
user2. It's crucial to ensure that test data does not persist in the database after tests are completed.Verification successful
The occurrences of 'user2' in the
V9999__Test_Data.sqlfile are part of the initial test data setup and do not indicate a failure in the cleanup process.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the cleanup SQL query correctly resets the changes made for testing. # Test: Search for any remaining test data that should have been cleaned up. Expect: No occurrences of test data. rg --type sql $'user2'Length of output: 2448
461-461: Ensure proper cleanup of test modifications.This script checks if the cleanup SQL query effectively removes all test modifications related to
user2. It's crucial to ensure that test data does not persist in the database after tests are completed.
There was a problem hiding this comment.
Actionable comments posted: 7
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- frontend/src/common/userdata.ts (1 hunks)
- frontend/test/common/crypto.spec.ts (4 hunks)
Files not reviewed due to errors (1)
- frontend/test/common/crypto.spec.ts (no review received)
Additional context used
Biome
frontend/src/common/userdata.ts
[error] 32-35: This else clause can be omitted because previous branches break early.
[error] 1-2: Some named imports are only used as types.
frontend/test/common/crypto.spec.ts
[error] 34-34: Declare variables separately
[error] 74-74: The computed expression can be simplified without the use of a string literal.
[error] 147-147: The computed expression can be simplified without the use of a string literal.
[error] 244-246: This constructor is unnecessary.
[error] 268-270: This constructor is unnecessary.
[error] 43-43: This let declares a variable that is only assigned once.
[error] 44-44: This let declares a variable that is only assigned once.
[error] 64-64: This let declares a variable that is only assigned once.
Additional comments not posted (3)
frontend/src/common/userdata.ts (3)
104-105: Ensure correct usage of default exports.Verify that the default export of the
UserDatainstance is correctly used throughout the project to maintain consistency and avoid potential issues with module resolution.
11-16: Ensure proper error handling inmemethod.Consider adding error handling for the backend call in the
memethod to manage scenarios where the user data cannot be fetched due to network issues or server errors.
52-62: Enhance security indecryptUserKeysWithSetupCodemethod.Ensure that the
decryptUserKeysWithSetupCodemethod securely handles the setup code and does not expose it in logs or error messages.Verification successful
The
decryptUserKeysWithSetupCodemethod does not appear to log or expose the setup code directly. However, to ensure comprehensive security practices, it is important to verify that the setup code is not logged or exposed elsewhere in the codebase.
The search results indicate that the
setupCodeis used in various parts of the codebase, but there is no evidence of it being logged or exposed in error messages. ThesetupCodeis primarily used for encryption and decryption purposes, which is appropriate.
frontend/src/common/userdata.ts: ThesetupCodeis used in thedecryptUserKeysWithSetupCodemethod and other related methods without being logged.frontend/src/common/crypto.ts: ThesetupCodeis used in encryption and decryption methods without being logged.Based on the gathered information, the
setupCodeis handled securely and is not exposed in logs or error messages.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify security handling in `decryptUserKeysWithSetupCode` method. # Test: Search for security practices in the `decryptUserKeysWithSetupCode` method. Expect: Security code. rg --type typescript $'decryptUserKeysWithSetupCode\\(\\) {'Length of output: 191
Script:
#!/bin/bash # Description: Verify security handling in `decryptUserKeysWithSetupCode` method. # Test: Search for security practices in the `decryptUserKeysWithSetupCode` method. Expect: Security code. rg --type typescript 'decryptUserKeysWithSetupCode\(\) {'Length of output: 191
Script:
#!/bin/bash # Description: Verify security handling in `decryptUserKeysWithSetupCode` method. # Test: Search for security practices in the `decryptUserKeysWithSetupCode` method. Expect: Security code. rg --type typescript 'decryptUserKeysWithSetupCode' --context 10Length of output: 99
Script:
#!/bin/bash # Description: Verify security handling in `decryptUserKeysWithSetupCode` method. # Test: Search for security practices in the `decryptUserKeysWithSetupCode` method. Expect: Security code. rg --type ts 'decryptUserKeysWithSetupCode' --context 10Length of output: 1532
Script:
#!/bin/bash # Description: Verify that the setup code is not logged or exposed elsewhere in the codebase. # Test: Search for any logging or error handling that might expose the setup code. rg 'setupCode' --type ts --context 5Length of output: 6614
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- frontend/src/common/userdata.ts (1 hunks)
- frontend/src/components/CreateVault.vue (2 hunks)
- frontend/src/components/RecoverVaultDialog.vue (3 hunks)
- frontend/src/components/VaultDetails.vue (4 hunks)
Files skipped from review as they are similar to previous changes (3)
- frontend/src/components/CreateVault.vue
- frontend/src/components/RecoverVaultDialog.vue
- frontend/src/components/VaultDetails.vue
Additional context used
Biome
frontend/src/common/userdata.ts
[error] 67-69: This else clause can be omitted because previous branches break early.
[error] 1-2: Some named imports are only used as types.
Additional comments not posted (6)
frontend/src/common/userdata.ts (6)
67-69: Simplify theecdsaPublicKeygetter by removing unnecessary else clause.
[REFACTOR_SUGGESTion]- } else { - return undefined; - }
14-18: The implementation of caching in themegetter method looks good.
24-29: The caching logic in thebrowserKeysgetter method is correctly implemented.
36-41: The implementation of thebrowsergetter method correctly handles asynchronous operations and potential undefined values.
76-79: Thereloadmethod correctly handles the invalidation and reloading of cached data. The concerns about synchronization were addressed in previous discussions.
86-92: ThecreateBrowserKeysmethod is implemented correctly, and the decision to let consumers handle errors is justified given the context.
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- backend/src/main/java/org/cryptomator/hub/api/UsersResource.java (4 hunks)
- frontend/src/common/userdata.ts (1 hunks)
Additional context used
Biome
frontend/src/common/userdata.ts
[error] 67-69: This else clause can be omitted because previous branches break early.
[error] 1-2: Some named imports are only used as types.
Additional comments not posted (1)
backend/src/main/java/org/cryptomator/hub/api/UsersResource.java (1)
73-75: Ensure that the new ECDSA key handling is secure and correctly implemented.Given the sensitivity of cryptographic key handling, it's crucial to ensure that the implementation adheres to best practices for security, such as avoiding logging sensitive data and ensuring keys are transmitted securely.
backend/src/main/java/org/cryptomator/hub/api/UsersResource.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- backend/src/main/java/org/cryptomator/hub/api/UsersResource.java (4 hunks)
- frontend/src/common/userdata.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- backend/src/main/java/org/cryptomator/hub/api/UsersResource.java
Additional context used
Biome
frontend/src/common/userdata.ts
[error] 1-2: Some named imports are only used as types.
Additional comments not posted (1)
frontend/src/common/userdata.ts (1)
1-2: Use explicit type imports for TypeScript.To clarify that certain imports are only used for type information, which can help during the TypeScript compilation process.
- import backend, { DeviceDto, UserDto } from './backend'; + import backend from './backend'; + import type { DeviceDto, UserDto } from './backend';Likely invalid or redundant comment.
This PR has changes in three areas:
user.privatekeysanddevice.user_privatekeys; just renamed those).userdata.ts, it adds a new facade object to the frontend. The purpose of this object is to channel all requests to/api/meas well as access to user keys and browser keys through a single place. This not only deduplicates some code and allows us to memoize seldom changing objects, it also makes sure we have a single entry point to inspect and interact with the user's key material.One of the sequence diagrams generated by coderabbit was actually quite good. I fetched it from its edit history:
sequenceDiagram participant User as User participant Frontend as Frontend participant UserData as UserData participant Backend as Backend User ->> Frontend: Create Vault Frontend ->> UserData: Get User Data UserData ->> Backend: Fetch User Information Backend -->> UserData: UserDto UserData -->> Frontend: UserDto Frontend ->> UserData: Get ECDH Public Key UserData -->> Frontend: ECDH Public Key Frontend ->> Backend: Create Vault with ECDH Public Key Backend -->> Frontend: Vault Created User ->> Frontend: Recover Vault Frontend ->> UserData: Get User Data UserData ->> Backend: Fetch User Information Backend -->> UserData: UserDto UserData -->> Frontend: UserDto Frontend ->> UserData: Decrypt User Keys with Browser UserData -->> Frontend: Decrypted User Keys Frontend ->> Backend: Recover Vault with User Keys Backend -->> Frontend: Vault Recovered