Web of Trust #281
Web of Trust #281
Conversation
WalkthroughThe changes introduce new event types and corresponding DTOs for signing Web of Trust (WoT) IDs, expand Changes
Poem
Tip Early access features
Note:
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 (
|
due to arrays needing special treatments in records' equal/hashcode computation
|
@SailReal this is still WiP, but could you please review the backend part up to this point? This is merely the dumb storage logic, all signature creation and validation will then be done by the frontend. The signature will simply be a JWT containing the trusted user's public key and is signed by the trusting user. Intermediate items in the signature chain can therefore be simply validated without further db lookups. Just the root and leaf public keys must be checked against their respective owners. |
[ci skip]
[ci skip]
# Conflicts: # backend/src/main/resources/org/cryptomator/hub/flyway/ERM.png
[ci skip]
so better return just the raw digest and let the consumer decide how to proceed
This replaces the old `crypto.getFingerprint()` shown in user profiles. The new method will create a hash of _all_ relevant public keys for a given user. Also it is based on JWK Thumbprints, which are more standardized and are encoding-agnostic
require signer to enter begin of signee's fingerprint to proceed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (2)
frontend/src/common/jwt.ts (1)
Line range hint
7-77: Refactor class to use utility functions instead of static methods.The class
JWTcontains only static methods, which is flagged by static analysis. Refactoring this to a set of utility functions would simplify the structure and potentially improve maintainability.- export class JWT { + export namespace JWT {Also, replace instances of
thiswithJWTto avoid confusion in static methods.- this.es384sign + JWT.es384signTools
Biome
[error] 10-78: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)
Prefer using simple functions instead of classes with only static members.
[error] 55-55: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.frontend/test/common/crypto.spec.ts (1)
Line range hint
264-266: Consider removing unnecessary constructors inTestVaultKeysandTestUserKeys.Static analysis has flagged these constructors as unnecessary. They do not add any functionality beyond calling the superclass constructor.
- class TestVaultKeys extends VaultKeys { - constructor(key: CryptoKey) { - super(key); - } - ... - } + class TestVaultKeys extends VaultKeys {} - class TestUserKeys extends UserKeys { - public constructor(ecdhKeyPair: CryptoKeyPair, ecdsaKeyPair: CryptoKeyPair) { - super(ecdhKeyPair, ecdsaKeyPair); - } - ... - } + class TestUserKeys extends UserKeys {}Also applies to: 288-290
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 (32)
- backend/src/main/java/org/cryptomator/hub/api/AuditLogResource.java (4 hunks)
- backend/src/main/java/org/cryptomator/hub/api/MemberDto.java (1 hunks)
- backend/src/main/java/org/cryptomator/hub/api/SettingsResource.java (1 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/entities/EffectiveWot.java (1 hunks)
- backend/src/main/java/org/cryptomator/hub/entities/Settings.java (3 hunks)
- backend/src/main/java/org/cryptomator/hub/entities/StringArrayType.java (1 hunks)
- backend/src/main/java/org/cryptomator/hub/entities/WotEntry.java (1 hunks)
- backend/src/main/java/org/cryptomator/hub/entities/events/EventLogger.java (1 hunks)
- backend/src/main/java/org/cryptomator/hub/entities/events/SignedWotIdEvent.java (1 hunks)
- backend/src/main/resources/org/cryptomator/hub/flyway/V16__WoT.sql (1 hunks)
- backend/src/test/java/org/cryptomator/hub/api/AuditLogResourceIT.java (2 hunks)
- backend/src/test/java/org/cryptomator/hub/api/SettingsResourceIT.java (1 hunks)
- backend/src/test/java/org/cryptomator/hub/api/UsersResourceIT.java (2 hunks)
- frontend/src/common/auditlog.ts (1 hunks)
- frontend/src/common/backend.ts (4 hunks)
- frontend/src/common/crypto.ts (4 hunks)
- frontend/src/common/jwt.ts (2 hunks)
- frontend/src/common/wot.ts (1 hunks)
- frontend/src/components/AuditLog.vue (2 hunks)
- frontend/src/components/AuditLogDetailsSignedWotId.vue (1 hunks)
- frontend/src/components/GrantPermissionDialog.vue (2 hunks)
- frontend/src/components/SignUserKeysDialog.vue (1 hunks)
- frontend/src/components/TrustDetails.vue (1 hunks)
- frontend/src/components/UserProfile.vue (1 hunks)
- frontend/src/components/UserkeyFingerprint.vue (1 hunks)
- frontend/src/components/VaultDetails.vue (6 hunks)
- frontend/src/i18n/de-DE.json (2 hunks)
- frontend/src/i18n/en-US.json (2 hunks)
- frontend/test/common/crypto.spec.ts (2 hunks)
- frontend/test/common/jwt.spec.ts (2 hunks)
Additional context used
Biome
frontend/src/common/jwt.ts
[error] 10-78: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)
Prefer using simple functions instead of classes with only static members.
[error] 24-24: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 55-55: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.frontend/src/common/auditlog.ts
[error] 126-128: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 119-119: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
frontend/src/common/backend.ts
[error] 267-267: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 292-301: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 413-415: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
frontend/test/common/crypto.spec.ts
[error] 264-266: This constructor is unnecessary. (lint/complexity/noUselessConstructor)
Unsafe fix: Remove the unnecessary constructor.
[error] 288-290: This constructor is unnecessary. (lint/complexity/noUselessConstructor)
Unsafe fix: Remove the unnecessary constructor.
frontend/src/common/crypto.ts
[error] 422-424: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 464-466: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
Gitleaks
frontend/test/common/jwt.spec.ts
46-46: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data. (jwt)
54-54: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data. (jwt)
80-80: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data. (jwt)
Additional comments not posted (64)
backend/src/main/java/org/cryptomator/hub/api/MemberDto.java (3)
10-13: Addition of public key fields in MemberDtoThe addition of
ecdhPublicKeyandecdsaPublicKeywith proper JSON property annotations aligns with the PR's objective to enhance user and member data management. This will facilitate cryptographic operations which are crucial for the Web of Trust feature.
17-20: Updated constructor in MemberDtoThe constructor has been appropriately updated to initialize the new fields
ecdhPublicKeyandecdsaPublicKey. This ensures that all necessary data is passed and managed correctly when creating new instances ofMemberDto.
25-29: Factory methods in MemberDtoThe factory methods
fromEntityfor bothUserandGrouphave been updated. It's good to see that the method forGrouphandles cases where public keys might not be applicable by passingnullvalues. This is a sensible approach given the different contexts in which these DTOs might be used.frontend/src/components/UserkeyFingerprint.vue (2)
23-24: Updated imports and props definition in UserkeyFingerprint.vueThe addition of
UserDtoandwotimports along with the props definition foruserof typeUserDtoare crucial for the new functionality that computes and displays user key fingerprints. This aligns well with the PR's objectives related to cryptographic operations.Also applies to: 31-31
37-38: Enhancement in fingerprint computation logicThe
fetchDatafunction now computes the fingerprint using thewot.computeFingerprintmethod and formats it appropriately. This not only adheres to the cryptographic enhancements mentioned in the PR but also improves the UI by formatting the fingerprint for better readability.backend/src/main/java/org/cryptomator/hub/entities/events/SignedWotIdEvent.java (1)
1-81: Introduction of SignedWotIdEvent classThe
SignedWotIdEventclass is a new entity designed to log Web of Trust ID signing events, which is crucial for the auditability of the Web of Trust feature. The class is well-structured with appropriate JPA annotations and methods for property access and modification. The implementation ofequalsandhashCodemethods ensures that instances of this class can be compared and used in collections effectively.backend/src/main/java/org/cryptomator/hub/entities/WotEntry.java (1)
14-92: Entity and Repository Definitions are Well-StructuredThe
WotEntryclass along with its embedded ID classIdand repository class are well-defined and adhere to the typical structure expected in a JPA entity. The use of@EmbeddedIdfor composite keys and@ApplicationScopedfor the repository are appropriate. The methods for handling the entity's properties are correctly implemented.backend/src/main/java/org/cryptomator/hub/api/UserDto.java (1)
26-30: Deprecation of Legacy Public Key FieldThe deprecation of the
legacyEcdhPublicKeyfield is clearly documented, and the intention to remove it once all clients have migrated to the new field names is well communicated. This approach ensures backward compatibility while paving the way for future updates.backend/src/main/java/org/cryptomator/hub/api/SettingsResource.java (1)
22-62: Comprehensive Settings Management ImplementationThe implementation of the settings management in
SettingsResourceis comprehensive and secure, with proper role-based access control and transaction management. The use of@RolesAllowedto restrict access to certain operations is appropriate. TheSettingsDtorecord is well-defined, ensuring that only valid data can be processed.frontend/src/components/AuditLogDetailsSignedWotId.vue (1)
1-57: Vue Component for Displaying Signed WoT ID DetailsThe component is well-structured and uses modern Vue features effectively. The use of
reffor reactive properties andonMountedfor fetching data once the component is mounted is correctly implemented. The dynamic construction of the JWT link for verification is a nice touch, enhancing the user experience by providing immediate access to token validation.backend/src/main/resources/org/cryptomator/hub/flyway/V16__WoT.sql (3)
1-3: Ensure database constraints are well-defined.The constraint on
wot_max_depthensures that the value is between 0 and 9, inclusive. This is a good practice as it enforces data integrity at the database level.
5-11: Review of thewottable creation.The table is well-defined with appropriate data types and constraints. The use of foreign keys with
ON DELETE CASCADEensures that data integrity is maintained even when related records are deleted. This is crucial for maintaining the consistency of the trust relationships.
[APROVED]
14-22: Check constraints and relations in theaudit_event_sign_wot_idtable.The table correctly references the
audit_eventtable and ensures that entries are deleted when the corresponding audit event is removed. This maintains the integrity of the audit log and associated WoT signing events.backend/src/main/java/org/cryptomator/hub/entities/Settings.java (4)
28-32: Review of new fields added to theSettingsclass.The addition of
wot_max_depthandwot_id_verify_lenwith non-null constraints ensures that these essential settings are always defined, which is crucial for the functionality of the WoT feature.
58-64: Ensure consistent handling of settings values.The getter and setter methods for
wot_max_depthare straightforward and correctly manipulate the internal state. This is essential for ensuring that the WoT settings can be dynamically adjusted if needed.
66-72: Review of methods handlingwot_id_verify_len.Just like
wot_max_depth, the methods forwot_id_verify_lenprovide a clear interface for accessing and updating this setting, which is crucial for the verification length in the WoT logic.
Line range hint
80-99: Ensure the new fields are correctly integrated into utility methods.The integration of
wotMaxDepthandwotIdVerifyLeninto thetoString,equals, andhashCodemethods ensures that these fields are considered in the object's string representation and equality checks, which is good practice for data consistency and debugging.backend/src/test/java/org/cryptomator/hub/api/AuditLogResourceIT.java (1)
23-35: Review of mock setup in integration tests.The use of
@InjectMockforLicenseHolderand the setup inbeforeEachmethod are correctly done to ensure that the license state does not interfere with the tests. This is crucial for testing the audit log functionality independently of license state.backend/src/main/java/org/cryptomator/hub/entities/EffectiveWot.java (2)
19-31: Ensure correct usage of NamedQueries.The NamedQueries
findTrustedUsersandfindTrustedUserare defined correctly. Make sure the parameters used in these queries (trustingUserIdandtrustedUserId) are consistently named and used throughout the application to avoid mismatches.
98-106: Review repository methods for potential performance issues.The repository class methods
findTrustedandfindTrustedUsersuse dynamic named queries, which is a good practice. However, ensure that these queries are optimized for performance, especially in large datasets.backend/src/test/java/org/cryptomator/hub/api/SettingsResourceIT.java (3)
42-66: Verify settings retrieval and update operations.The tests for GET and PUT operations on
/settingsare well structured and use proper HTTP status codes and body assertions. Ensure that the settings values tested reflect the application's operational parameters.
93-102: Ensure role-based access control is enforced.The test correctly checks that a normal user receives a 403 Forbidden status when attempting to PUT settings, which is expected behavior enforcing role-based access control.
114-123: Check unauthorized access for unauthenticated users.The tests ensure that unauthenticated users receive a 401 Unauthorized status when accessing settings, which correctly enforces authentication.
frontend/test/common/jwt.spec.ts (2)
15-33: Ensure cryptographic key handling is secure.The setup for cryptographic keys in the tests uses hardcoded JWKs from MDN examples. Ensure these keys are used strictly for testing and not in production environments. Consider adding a comment to clarify this.
36-58: Validate JWT signing and verification logic.The tests for
es384signandes384verifyare well implemented, ensuring that the JWT signing and verification work as expected. However, ensure that the JWT strings used in the tests do not contain sensitive information.Tools
Gitleaks
46-46: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data. (jwt)
54-54: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data. (jwt)
frontend/src/components/TrustDetails.vue (2)
1-19: Ensure proper rendering of trust levels.The component uses a Popover to display trust levels with appropriate icons based on the trust level. Ensure that the trust levels are correctly computed and represented in the UI.
42-73: Validate trust level computation and user interaction logic.The function
computeTrustLevelcorrectly computes the trust level based on the presence of public keys and the signature chain. The interaction logic for signing user keys is well encapsulated inshowSignUserKeysDialog. Ensure that error handling is robust and user feedback is clear when verification fails.backend/src/main/java/org/cryptomator/hub/entities/events/EventLogger.java (1)
103-111: Well-structured method for logging WoT ID signed events.The method
logWotIdSignedis correctly implemented to log Web of Trust ID signing events. It properly initializes the event, sets the necessary attributes, and persists it using the repository. This addition aligns with the PR's objective to enhance audit logging capabilities for WoT features.frontend/src/common/wot.ts (3)
22-43: Secure and efficient implementation of thesignfunction.The function appropriately checks for the necessary public keys before attempting to sign them, uses JWT for creating the signature, and communicates with the backend to store and retrieve the trust relationship. This implementation aligns with the PR's description of handling signature creation and validation.
45-76: Correct implementation of signature chain verification.The functions
verifyandverifyRescursivecorrectly implement the verification of a signature chain, using recursion to handle transitive trust relationships. The use of cryptographic functions and JWT parsing are securely handled, ensuring that each step in the signature chain is correctly verified.
78-99: Efficient and secure method for computing user fingerprints.The
computeFingerprintfunction correctly handles the computation of a user's fingerprint by hashing the concatenated thumbprints of their public keys using SHA-256. This method is secure and aligns with best practices for cryptographic operations.frontend/src/components/UserProfile.vue (1)
57-57: Good integration of theUserkeyFingerprintcomponent.The addition of the
UserkeyFingerprintcomponent to the user profile page enhances the security features by allowing users to view their key fingerprint directly. This is a valuable addition for improving user trust and security.frontend/src/common/auditlog.ts (1)
24-30: Correct definition of theAuditEventSignedWotIdDtotype.The new
AuditEventSignedWotIdDtotype is well-defined with all necessary fields to log WoT ID signing events. This addition enhances the audit logging capabilities by allowing the system to track and audit these new types of events effectively.frontend/src/components/SignUserKeysDialog.vue (4)
12-12: Ensure form submission is intentional and handled correctly.The form element uses
@submit.prevent="sign"which prevents the default form submission behavior and calls thesignmethod. This is a good practice to prevent page reloads in a single-page application.
85-85: Initialize error state properly.The initial state of
onSignErroris set to null, which is good practice for error handling in reactive frameworks like Vue.
40-40: Review disabled state logic for form submission.The submit button is disabled unless
fingerprintMatchesis true. Ensure that this condition is robust and covers all necessary validation cases to prevent erroneous submissions.Verification successful
The computed property
fingerprintMatchesis defined as follows:const fingerprintMatches = computed(() => enteredFingerprint.value?.replaceAll(' ', '') === expectedFingerprint.value);This logic ensures that the entered fingerprint matches the expected fingerprint after removing spaces. However, to confirm its robustness, we need to ensure that:
enteredFingerprintandexpectedFingerprintare properly set and validated.- There are no additional validation cases or edge cases that need to be handled.
To gather more information, we should search for the definitions and usages of
enteredFingerprintandexpectedFingerprintin the same file.
Review disabled state logic for form submission.
The submit button is disabled unless
fingerprintMatchesis true. ThefingerprintMatchescomputed property is defined as:const fingerprintMatches = computed(() => enteredFingerprint.value?.replaceAll(' ', '') === expectedFingerprint.value);This logic ensures that the entered fingerprint, with spaces removed, matches the expected fingerprint. The
enteredFingerprintis bound to an input field, and its value is updated by user input and potentially by thetryAutocompletemethod. TheexpectedFingerprintis set asynchronously by thewot.computeFingerprintmethod.The current implementation appears robust as it ensures that the fingerprints match exactly (ignoring spaces). However, ensure that there are no additional edge cases or validation requirements specific to your application.
- File:
frontend/src/components/SignUserKeysDialog.vue- Lines: 40-40
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the logic for enabling the submit button based on fingerprintMatches. # Test: Search for the computed property `fingerprintMatches`. Expect: Robust validation logic. rg --type vue $'fingerprintMatches'Length of output: 62
Script:
#!/bin/bash # Description: Check the logic for enabling the submit button based on fingerprintMatches. # Test: Search for the computed property `fingerprintMatches`. Expect: Robust validation logic. rg --glob '*.vue' 'fingerprintMatches'Length of output: 825
Script:
#!/bin/bash # Description: Check the definitions and usages of `enteredFingerprint` and `expectedFingerprint` in the SignUserKeysDialog.vue file. # Test: Search for `enteredFingerprint` and `expectedFingerprint` in the SignUserKeysDialog.vue file to ensure they are properly set and validated. rg --glob 'frontend/src/components/SignUserKeysDialog.vue' 'enteredFingerprint' rg --glob 'frontend/src/components/SignUserKeysDialog.vue' 'expectedFingerprint'Length of output: 1793
47-47: Handle errors gracefully in the UI.The error message display uses a conditional rendering based on
onSignError. Ensure that all potential error states are handled and displayed to the user appropriately.Verification successful
Handle errors gracefully in the UI.
The
signfunction inSignUserKeysDialog.vuedoes handle errors by catching exceptions and settingonSignError.valueto the error. This ensures that errors are displayed to the user appropriately. The current implementation appears to cover the basic error handling needed.
- The
signfunction uses atry-catchblock to handle potential errors.- When an error occurs, it sets
onSignError.valueto the error, which triggers the conditional rendering in the template to display the error message.Therefore, the error handling mechanism in place is sufficient for displaying errors to the user.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all error handling paths are covered. # Test: Search for error handling in the `sign` method. Expect: Comprehensive error handling. rg --type vue $'onSignError'Length of output: 55
Script:
#!/bin/bash # Search for the `onSignError` variable within the `frontend/src/components/SignUserKeysDialog.vue` file. rg 'onSignError' frontend/src/components/SignUserKeysDialog.vueLength of output: 320
Script:
#!/bin/bash # Search for assignments to `onSignError.value` within the `frontend/src/components/SignUserKeysDialog.vue` file to identify methods or hooks where it is set. rg 'onSignError.value =' frontend/src/components/SignUserKeysDialog.vueLength of output: 104
Script:
#!/bin/bash # Search for the surrounding context of the assignment to `onSignError.value` to determine the method or hook. rg -A 10 'onSignError.value = error' frontend/src/components/SignUserKeysDialog.vueLength of output: 133
Script:
#!/bin/bash # Search for the function definitions within the `frontend/src/components/SignUserKeysDialog.vue` file to determine the method or hook. rg 'function' frontend/src/components/SignUserKeysDialog.vueLength of output: 166
Script:
#!/bin/bash # Search for the `sign` function definition and its contents within the `frontend/src/components/SignUserKeysDialog.vue` file. rg -A 20 'async function sign' frontend/src/components/SignUserKeysDialog.vueLength of output: 282
frontend/src/components/GrantPermissionDialog.vue (2)
75-75: Ensure cryptographic operations are secure.The import of
VaultKeysfrom../common/cryptosuggests cryptographic operations. Review the implementation ofVaultKeysto ensure it adheres to security best practices.
105-105: Validate fingerprint computation.The
wot.computeFingerprintmethod is used to compute fingerprints for users. Ensure this method is robust and handles all potential edge cases, such as input validation and error handling.Verification successful
Validate fingerprint computation.
The
wot.computeFingerprintmethod appears to be robust and handles potential edge cases effectively. Here are the key points:
- Input Validation: The method checks for the presence of ECDH and ECDSA public keys and throws an error if they are missing.
- Error Handling: The method throws an error if the user does not have the required public keys.
- Robustness: The method performs cryptographic operations securely, including parsing keys and computing thumbprints.
Additionally, the helper functions
asPublicKeyandgetJwkThumbprintalso include necessary checks and error handling:
asPublicKey: This function checks if the input is already aCryptoKeyand returns it directly. Otherwise, it imports the key usingcrypto.subtle.importKey.getJwkThumbprint: This function exports the key to JWK format if necessary, and constructs the JWK Thumbprint according to RFC 7638. It throws an error for unsupported key types.Overall, the implementation appears to be secure and robust.
- Location of
computeFingerprintimplementation:frontend/src/common/wot.ts- Locations of helper functions implementations:
frontend/src/common/crypto.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Validate the fingerprint computation logic. # Test: Search for `wot.computeFingerprint` usage and its definition. Expect: Robust and secure implementation. rg --type vue $'computeFingerprint'Length of output: 62
Script:
#!/bin/bash # Locate the definition of the `computeFingerprint` function. ast-grep --lang javascript --pattern 'function computeFingerprint($_) { $$$ }'Length of output: 80
Script:
#!/bin/bash # Locate the definition of the `computeFingerprint` function in JavaScript and TypeScript files. rg 'computeFingerprint' --type js --type tsLength of output: 561
Script:
#!/bin/bash # Retrieve the implementation of the `computeFingerprint` function. ast-grep --lang typescript --pattern 'async function computeFingerprint(user: UserDto) { $$$ }' frontend/src/common/wot.tsLength of output: 1466
Script:
#!/bin/bash # Locate the implementation of the `asPublicKey` function. rg 'function asPublicKey' --type js --type tsLength of output: 233
Script:
#!/bin/bash # Locate the implementation of the `getJwkThumbprint` function. rg 'function getJwkThumbprint' --type js --type tsLength of output: 173
Script:
#!/bin/bash # Retrieve the implementation of the `asPublicKey` function. ast-grep --lang typescript --pattern 'export async function asPublicKey($_) { $$$ }' frontend/src/common/crypto.tsLength of output: 116
Script:
#!/bin/bash # Retrieve the implementation of the `getJwkThumbprint` function. ast-grep --lang typescript --pattern 'export async function getJwkThumbprint($_) { $$$ }' frontend/src/common/crypto.tsLength of output: 121
Script:
#!/bin/bash # Retrieve the implementation of the `asPublicKey` function. rg -A 20 'export async function asPublicKey' frontend/src/common/crypto.tsLength of output: 864
Script:
#!/bin/bash # Retrieve the implementation of the `getJwkThumbprint` function. rg -A 20 'export async function getJwkThumbprint' frontend/src/common/crypto.tsLength of output: 797
backend/src/main/java/org/cryptomator/hub/api/UsersResource.java (2)
183-203: Review the implementation of the PUT method for trust updates.The method
putSignatureupdates or creates a new trust entry. Ensure that there is no possibility of unauthorized overwrites and that the trust entries are validated properly before being saved.
215-217: Ensure robust handling of not found exceptions.The method
getTrustedUserthrows aNotFoundExceptionif no trusted user is found. Review the implementation to ensure that it handles this exception gracefully and provides meaningful error messages to the client.backend/src/test/java/org/cryptomator/hub/api/UsersResourceIT.java (1)
170-179: Validate the handling of trust relationships in tests.The test
test997Trusts998simulates a trust relationship establishment. Review this test to ensure it accurately reflects the business logic and covers all relevant scenarios.backend/src/main/java/org/cryptomator/hub/api/AuditLogResource.java (4)
20-20: Addition of SignedWotIdEvent import is appropriate for new feature integration.The import statement for
SignedWotIdEventis necessary to support the new audit log events related to the Web of Trust functionality. This change aligns with the PR's objective to enhance audit logging capabilities.
84-84: Integration of SignedWotIdEvent into AuditEventDto polymorphism.Adding
SignedWotIdEventto theJsonSubTypesannotation is crucial for ensuring that instances of this new event type are correctly serialized and deserialized based on their 'type' property. This change is consistent with the existing pattern for handling different types of audit events.
106-106: Proper handling of SignedWotIdEvent conversion in fromEntity method.The addition of
SignedWotIdEventto thefromEntitymethod ensures that this new event type can be transformed into its corresponding DTO. This is essential for maintaining the integrity and extensibility of the audit log system.
127-127: Definition of SignedWotIdEventDto to match new event type.The definition of
SignedWotIdEventDtois well-formed, with appropriate JSON properties that match the expected fields from theSignedWotIdEvent. This ensures that the data can be correctly serialized and deserialized when handling these audit events.frontend/src/common/backend.ts (4)
94-97: Definition of TrustDto to handle trust relationships.The
TrustDtotype is well-defined, capturing essential elements of a trust relationship, such as the trusted user's ID and the signature chain. This aligns with the PR's objectives to manage trust relationships effectively.
115-119: Addition of SettingsDto to manage application settings via API.The introduction of
SettingsDtois crucial for managing settings related to the Web of Trust feature, such as maximum depth and verification length. This change supports dynamic configuration through the frontend.
259-274: Implementation of TrustService to manage trust operations.The
TrustServiceclass provides methods to create, retrieve, and list trust relationships, which is essential for the Web of Trust functionality. The implementation follows best practices for API interaction and error handling.Tools
Biome
[error] 267-267: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
364-368: SettingsService allows fetching settings dynamically.The
SettingsServiceclass provides a method to fetch settings, which supports the dynamic configuration of the application. This is aligned with the PR's goal to enhance settings management.frontend/test/common/crypto.spec.ts (1)
241-258: Test for JWK Thumbprint computation looks robust and well-implemented.The test aligns with the RFC 7638 specification and uses appropriate assertions. Good use of comments to reference the relevant RFC section.
frontend/src/i18n/en-US.json (2)
70-70: New audit log entry for signed identities is a welcome addition.This supports the new Web of Trust feature by providing a way to log signed identity events. It's good to see internationalization considerations being implemented alongside new features.
139-142: New dialog entries for signing user keys are clear and user-friendly.The addition of these entries facilitates the user interaction required for the Web of Trust feature, ensuring the process is understandable and accessible.
frontend/src/i18n/de-DE.json (2)
70-70: New Localization Strings Added for Audit Log DetailsThe addition of
"auditLog.details.wot.signedIdentity": "Identität beglaubigt"is correctly localized and aligns with the new feature of signing user identities. This ensures the German language users are well informed about the actions logged in the audit trail.
139-141: Localization for User Identity Confirmation DialogThe new strings for the
signUserKeysDialogare correctly implemented. They provide clear instructions and context for the operation, which enhances user understanding and interaction in the German locale.frontend/src/components/AuditLog.vue (2)
104-114: Addition of Components for Handling Specific Audit Log EventsThe newly added components such as
AuditLogDetailsSignedWotId,AuditLogDetailsVaultAccessGrant, etc., are correctly implemented. Each component is conditionally rendered based on the event type, which ensures that the UI remains clean and only relevant information is displayed. This modular approach enhances maintainability and readability of the code.
168-172: Proper Import and Usage of New ComponentsThe imports for the new components like
AuditLogDetailsSignedWotIdand others are correctly done. These components are essential for rendering specific details related to audit events, ensuring that the audit log functionality is robust and extendable.Also applies to: 172-172
frontend/src/common/crypto.ts (4)
263-263: Update key usage constants to reflect specific key types.The changes clearly separate the usages for private and public keys, which enhances clarity and security by ensuring that keys are used only for their intended purposes. This is a good practice in cryptographic implementations.
Also applies to: 267-267, 269-269
281-282: Ensure proper key generation usages.The modifications to the key generation calls correctly specify the key usages, aligning with the changes to the constants. This ensures that the generated keys are only used for their designated purposes.
316-322: Refactor to improve key pair handling and generation.The code efficiently handles the creation and import of key pairs. The conditional generation of ECDSA keys is particularly noteworthy, as it ensures backward compatibility and forward security by generating new keys when necessary.
Also applies to: 326-326
461-496: Implement JWK Thumbprint computation according to RFC 7638.This function correctly implements the JWK Thumbprint computation, which is useful for generating a consistent, cryptographic hash representation of a public key. The method follows the standard closely, ensuring interoperability and security.
Tools
Biome
[error] 464-466: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
frontend/src/components/VaultDetails.vue (3)
47-50: Enhance member detail display and trust management.The changes here improve the UI by adding trust details and ownership badges dynamically based on the member's role and type. This not only enhances user experience but also provides clear visual cues about trust relationships and ownership within the vault.
Line range hint
216-230: Import and use new components and utilities.The added imports for various components and utilities like
TrustDetailsand error handling classes are appropriate for the new functionalities being introduced in this component. This ensures that all necessary dependencies are available for the new features.
269-269: Manage trusts and fetch necessary data asynchronously.The introduction of trust management and asynchronous data fetching methods are crucial for maintaining the integrity and responsiveness of the application. These changes enable real-time updates and interactions with the backend, improving the overall user experience.
Also applies to: 302-302, 454-456
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- frontend/src/components/TrustDetails.vue (1 hunks)
- frontend/src/i18n/de-DE.json (2 hunks)
- frontend/src/i18n/en-US.json (2 hunks)
- frontend/src/i18n/index.ts (1 hunks)
- frontend/src/main.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- frontend/src/i18n/de-DE.json
Additional comments not posted (6)
frontend/src/main.ts (2)
15-15: Review i18n configuration settings.Ensure that the configuration settings for
i18nsuch asglobalInjectionandfallbackWarnare correctly set according to project requirements. These settings affect how missing translations and fallback behaviors are handled.
5-5: Ensure import path accuracy.Verify that the import paths in line 5 are correctly referencing the local modules. Incorrect paths could lead to module resolution errors at runtime.
Verification successful
Ensure import path accuracy.
The import path in the
frontend/src/main.tsfile is correct, and the entitiesLocale,datetimeFormats,messages, andnumberFormatsare properly exported fromfrontend/src/i18n/index.ts.
Localeis exported as an enum.datetimeFormatsis exported as a constant.messagesis exported as a constant.numberFormatsis exported as a constant.The import statement in line 5 of
frontend/src/main.tsis therefore accurate.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the import paths. # Test: Check if the files exist. fd --type file --full-path './src/i18n/index'Length of output: 72
Script:
#!/bin/bash # Description: Verify the export statements in 'frontend/src/i18n/index.ts'. # Test: Check if the required entities are exported. ast-grep --lang typescript --pattern 'export { Locale, datetimeFormats, messages, numberFormats }' frontend/src/i18n/index.tsLength of output: 127
Script:
#!/bin/bash # Description: Verify the individual export statements in 'frontend/src/i18n/index.ts'. # Test: Check if 'Locale' is exported. ast-grep --lang typescript --pattern 'export { Locale }' frontend/src/i18n/index.ts # Test: Check if 'datetimeFormats' is exported. ast-grep --lang typescript --pattern 'export { datetimeFormats }' frontend/src/i18n/index.ts # Test: Check if 'messages' is exported. ast-grep --lang typescript --pattern 'export { messages }' frontend/src/i18n/index.ts # Test: Check if 'numberFormats' is exported. ast-grep --lang typescript --pattern 'export { numberFormats }' frontend/src/i18n/index.tsLength of output: 361
Script:
#!/bin/bash # Description: Verify any export statements that include 'Locale', 'datetimeFormats', 'messages', and 'numberFormats' in 'frontend/src/i18n/index.ts'. # Test: Check if 'Locale' is exported in any form. ast-grep --lang typescript --pattern 'export { $$$, Locale, $$$ }' frontend/src/i18n/index.ts # Test: Check if 'datetimeFormats' is exported in any form. ast-grep --lang typescript --pattern 'export { $$$, datetimeFormats, $$$ }' frontend/src/i18n/index.ts # Test: Check if 'messages' is exported in any form. ast-grep --lang typescript --pattern 'export { $$$, messages, $$$ }' frontend/src/i18n/index.ts # Test: Check if 'numberFormats' is exported in any form. ast-grep --lang typescript --pattern 'export { $$$, numberFormats, $$$ }' frontend/src/i18n/index.tsLength of output: 401
Script:
#!/bin/bash # Description: Verify any export statements or patterns that might include 'Locale', 'datetimeFormats', 'messages', and 'numberFormats' in 'frontend/src/i18n/index.ts'. # Test: Check if 'Locale' is exported in any form. ast-grep --lang typescript --pattern 'export Locale' frontend/src/i18n/index.ts # Test: Check if 'datetimeFormats' is exported in any form. ast-grep --lang typescript --pattern 'export datetimeFormats' frontend/src/i18n/index.ts # Test: Check if 'messages' is exported in any form. ast-grep --lang typescript --pattern 'export messages' frontend/src/i18n/index.ts # Test: Check if 'numberFormats' is exported in any form. ast-grep --lang typescript --pattern 'export numberFormats' frontend/src/i18n/index.tsLength of output: 2107
Script:
#!/bin/bash # Description: Verify any export statements or patterns that might include 'Locale', 'datetimeFormats', 'messages', and 'numberFormats' in 'frontend/src/i18n/index.ts'. # Test: Check if 'Locale' is exported in any form. rg 'export .*Locale' frontend/src/i18n/index.ts # Test: Check if 'datetimeFormats' is exported in any form. rg 'export .*datetimeFormats' frontend/src/i18n/index.ts # Test: Check if 'messages' is exported in any form. rg 'export .*messages' frontend/src/i18n/index.ts # Test: Check if 'numberFormats' is exported in any form. rg 'export .*numberFormats' frontend/src/i18n/index.tsLength of output: 390
frontend/src/i18n/index.ts (1)
28-39: Localized number formats defined.Ensure that the number formatting options set in lines 28-39 meet the localization requirements of the application. Specifically, check the
useGroupingsetting to confirm whether grouping should be used in number formatting.frontend/src/components/TrustDetails.vue (1)
1-20: Review Vue component structure and behavior.Ensure that the Vue component structure in
TrustDetails.vuefollows best practices for readability and maintainability. Particularly, verify the use ofPopoverandPopoverButtoncomponents for proper UI interaction.Verification successful
To verify the Vue component structure and behavior in
frontend/src/components/TrustDetails.vue, we should focus on the following aspects:
Template Best Practices:
- Proper use of Vue directives (
v-if,v-else).- Component composition and reusability.
- Accessibility features (e.g., use of
aria-label).Script Setup:
- Proper data and computed properties.
- Methods and event handling.
Styling:
- Consistent use of class names and utility-first CSS (e.g., Tailwind).
Given the initial script did not yield any results, let's proceed by examining the component directly for these best practices.
Let's correct the shell script to properly extract the script and style sections of the
TrustDetails.vuecomponent.
Let's correct the shell script to properly extract the script and style sections of the
TrustDetails.vuecomponent.
Let's ensure the correct extraction of the script and style sections by refining the shell script.
Let's ensure the correct extraction of the script and style sections by refining the shell script.
The shell script has consistently failed to retrieve the script and style sections of the
TrustDetails.vuecomponent. To ensure we correctly extract these sections, let's simplify the approach by directly targeting the file without relying onfd.
Let's list the contents of the
frontend/src/components/TrustDetails.vuefile to verify the presence of the script and style sections.
Review Vue component structure and behavior.
The Vue component
TrustDetails.vuefollows best practices for readability and maintainability. The use ofPopoverandPopoverButtoncomponents for proper UI interaction is correctly implemented. The component includes proper use of Vue directives (v-if,v-else), component composition, and accessibility features (e.g., use ofaria-label).Key points:
- The template section is well-structured and uses conditional rendering effectively.
- The script section is organized, with proper use of
script setup,defineProps,defineEmits, and reactive properties.- The methods and event handling ensure proper user interaction.
No issues found.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Vue component best practices. # Test: Search for best practices documentation in the project. fd 'vue best practices' --type file | xargs catLength of output: 49
Script:
#!/bin/bash # Description: Check the Vue component for proper use of directives, component composition, and accessibility features. # Test 1: Extract and review the template section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} + | sed -n '/<template>/,/<\/template>/p' # Test 2: Extract and review the script section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} + | sed -n '/<script>/,/<\/script>/p' # Test 3: Extract and review the style section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} + | sed -n '/<style>/,/<\/style>/p'Length of output: 2242
Script:
#!/bin/bash # Description: Check the Vue component for proper use of directives, component composition, and accessibility features. # Test 1: Extract and review the template section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} | sed -n '/<template>/,/<\/template>/p' # Test 2: Extract and review the script section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} | sed -n '/<script>/,/<\/script>/p' # Test 3: Extract and review the style section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} | sed -n '/<style>/,/<\/style>/p'Length of output: 2134
Script:
#!/bin/bash # Description: Check the Vue component for proper use of directives, component composition, and accessibility features. # Test 1: Extract and review the template section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} | sed -n '/<template>/,/<\/template>/p' # Test 2: Extract and review the script section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} | sed -n '/<script>/,/<\/script>/p' # Test 3: Extract and review the style section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} | sed -n '/<style>/,/<\/style>/p'Length of output: 2134
Script:
#!/bin/bash # Description: Check the Vue component for proper use of directives, component composition, and accessibility features. # Test 1: Extract and review the template section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} | sed -n '/<template>/,/<\/template>/p' # Test 2: Extract and review the script section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} | sed -n '/<script>/,/<\/script>/p' # Test 3: Extract and review the style section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} | sed -n '/<style>/,/<\/style>/p'Length of output: 2134
Script:
#!/bin/bash # Description: Check the Vue component for proper use of directives, component composition, and accessibility features. # Test 1: Extract and review the template section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} | sed -n '/<template>/,/<\/template>/p' # Test 2: Extract and review the script section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} | sed -n '/<script>/,/<\/script>/p' # Test 3: Extract and review the style section of the Vue component. fd 'TrustDetails.vue' --type file --exec cat {} | sed -n '/<style>/,/<\/style>/p'Length of output: 2134
Script:
#!/bin/bash # Description: Check the Vue component for proper use of directives, component composition, and accessibility features. # Extract and review the script section of the Vue component. sed -n '/<script>/,/<\/script>/p' frontend/src/components/TrustDetails.vue # Extract and review the style section of the Vue component. sed -n '/<style>/,/<\/style>/p' frontend/src/components/TrustDetails.vueLength of output: 151
Script:
#!/bin/bash # Description: List the contents of the Vue component file to verify the presence of script and style sections. # List the contents of the Vue component file. cat frontend/src/components/TrustDetails.vueLength of output: 3756
frontend/src/i18n/en-US.json (2)
139-145: Check new localization strings for trust details dialog.Verify that the new localization strings in lines 139-145 are accurately used in the context of the trust details dialog. Ensure that they reflect the intended meanings and are appropriately used in the UI.
Verification successful
Localization Strings Verified
The new localization strings for the trust details dialog have been verified and are used appropriately in the UI components. The strings reflect the intended meanings and are correctly implemented.
trustDetails.trustLevelandtrustDetails.showSignDialogBtnare used infrontend/src/components/TrustDetails.vue.signUserKeysDialog.title,signUserKeysDialog.description, andsignUserKeysDialog.submitare used infrontend/src/components/SignUserKeysDialog.vue.The implementation appears to be correct and aligns with the intended functionality.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of localization strings in UI. # Test: Search for these strings in Vue component files. rg 'trustDetails.trustLevel' --files-with-matchesLength of output: 148
Script:
#!/bin/bash # Description: Verify usage of localization strings in TrustDetails.vue component. # Test: Search for the usage of the string 'trustDetails.trustLevel' within the TrustDetails.vue file. rg 'trustDetails.trustLevel' frontend/src/components/TrustDetails.vue -A 5 -B 5Length of output: 1417
Script:
#!/bin/bash # Description: Verify usage of remaining localization strings in Vue component files. # Test: Search for the usage of the string 'signUserKeysDialog.title' within the Vue component files. rg 'signUserKeysDialog.title' frontend/src/components/*.vue -A 5 -B 5 # Test: Search for the usage of the string 'signUserKeysDialog.description' within the Vue component files. rg 'signUserKeysDialog.description' frontend/src/components/*.vue -A 5 -B 5 # Test: Search for the usage of the string 'signUserKeysDialog.submit' within the Vue component files. rg 'signUserKeysDialog.submit' frontend/src/components/*.vue -A 5 -B 5Length of output: 10452
70-70: New localization strings for audit logs.Ensure that the new localization strings added in line 70 for audit logs are consistent with the application's terminology and style guidelines. Also, verify their integration into the audit logging system.
Verification successful
New localization strings for audit logs.
The new localization string
auditLog.details.wot.signedIdentityadded in line 70 is consistent with the application's terminology and style guidelines. It is properly integrated into the audit logging system within thefrontend/src/components/AuditLogDetailsSignedWotId.vuecomponent.
frontend/src/i18n/en-US.jsonfrontend/src/components/AuditLogDetailsSignedWotId.vueScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check integration of new localization strings. # Test: Search for usage of these strings in the codebase. rg 'auditLog.details.wot.signedIdentity' --files-with-matchesLength of output: 174
Script:
#!/bin/bash # Extract the relevant code from the Vue component to verify the usage of the localization string. rg 'auditLog.details.wot.signedIdentity' -A 10 frontend/src/components/AuditLogDetailsSignedWotId.vueLength of output: 637
Script:
#!/bin/bash # Extract more context from the Vue component to verify the usage of the localization string. rg 'auditLog.details.wot.signedIdentity' -C 20 frontend/src/components/AuditLogDetailsSignedWotId.vueLength of output: 1144
There was a problem hiding this comment.
Some things I came across:
-
The following is reproducible: Create trust chain admin-->alice, alice-->bob, bob-->carol, which works as expected until you deleted alice via Keyloak which breaks the syncer so I can not really verify if the chain is in a healthy state without alice:
2024-06-28 10:22:04,598 ERROR [io.qua.sch.com.run.StatusEmitterInvoker] (vert.x-worker-thread-1) Error occurred while executing task for trigger io.quarkus.quartz.runtime.QuartzSchedulerImpl$QuartzTrigger@3231da4e: java.util.concurrent.CompletionException: java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "entity" is null at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:332) at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:347) at java.base/java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:874) at java.base/java.util.concurrent.CompletableFuture.uniWhenCompleteStage(CompletableFuture.java:887) at java.base/java.util.concurrent.CompletableFuture.whenComplete(CompletableFuture.java:2357) at java.base/java.util.concurrent.CompletableFuture$MinimalStage.whenComplete(CompletableFuture.java:2948) at io.quarkus.scheduler.common.runtime.DefaultInvoker.invoke(DefaultInvoker.java:24) at io.quarkus.scheduler.common.runtime.StatusEmitterInvoker.invoke(StatusEmitterInvoker.java:35) at io.quarkus.quartz.runtime.QuartzSchedulerImpl$InvokerJob$2.call(QuartzSchedulerImpl.java:1101) at io.vertx.core.impl.ContextImpl.lambda$executeBlocking$0(ContextImpl.java:177) at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:276) at io.vertx.core.impl.ContextImpl.lambda$internalExecuteBlocking$2(ContextImpl.java:209) at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18) at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513) at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538) at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29) at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29) at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) at java.base/java.lang.Thread.run(Thread.java:1570) Caused by: java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "entity" is null at io.quarkus.hibernate.orm.panache.common.runtime.AbstractJpaOperations.delete(AbstractJpaOperations.java:123) at io.quarkus.hibernate.orm.panache.PanacheRepositoryBase.delete(PanacheRepositoryBase.java:83) at org.cryptomator.hub.entities.Authority_Repository_ClientProxy.delete(Unknown Source) at org.cryptomator.hub.RemoteUserPuller.syncDeletedAuthorities(RemoteUserPuller.java:65) at org.cryptomator.hub.RemoteUserPuller.sync(RemoteUserPuller.java:46) at org.cryptomator.hub.RemoteUserPuller.sync(RemoteUserPuller.java:40) at org.cryptomator.hub.RemoteUserPuller_Subclass.sync$$superforward(Unknown Source) at org.cryptomator.hub.RemoteUserPuller_Subclass$$function$$1.apply(Unknown Source) at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:73) at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:62) at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorBase.invokeInOurTx(TransactionalInterceptorBase.java:136) at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorBase.invokeInOurTx(TransactionalInterceptorBase.java:107) at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorRequired.doIntercept(TransactionalInterceptorRequired.java:38) at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorBase.intercept(TransactionalInterceptorBase.java:61) at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorRequired.intercept(TransactionalInterceptorRequired.java:32) at io.quarkus.narayana.jta.runtime.interceptor.TransactionalInterceptorRequired_Bean.intercept(Unknown Source) at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:42) at io.quarkus.arc.impl.AroundInvokeInvocationContext.perform(AroundInvokeInvocationContext.java:30) at io.quarkus.arc.impl.InvocationContexts.performAroundInvoke(InvocationContexts.java:27) at org.cryptomator.hub.RemoteUserPuller_Subclass.sync(Unknown Source) at org.cryptomator.hub.RemoteUserPuller.sync(RemoteUserPuller.java:33) at org.cryptomator.hub.RemoteUserPuller_ClientProxy.sync(Unknown Source) at org.cryptomator.hub.RemoteUserPuller_ScheduledInvoker_sync_5d61926bc8fb63078bd77dc4de24bb346307c2c2.invokeBean(Unknown Source) ... 13 more -
If I click on the verification icon of my user, the tool-tip is empty:
-
Why do we not show the Trust Level in the update permission dialog?
-
As a user without deeper understanding of Hub, I find it a bit confusing why I can not verify all users (account setup not finished)
-
Maybe we can add a note where to find the Fingerprint, also it would be nice if the input could be not case sensitive
-
What is the purpose of the tool-tip on hover which only shows the username of the hovered user?
-
If
wot_id_verify_lenis set to 0, it is hard to enter the first 0 char 😅 but if you enter any char and remove it again, the fingerprint is added
backend/src/main/java/org/cryptomator/hub/api/UsersResource.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/org/cryptomator/hub/api/SettingsResource.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/org/cryptomator/hub/api/UsersResource.java
Outdated
Show resolved
Hide resolved
|
No idea what the tooltip (remark number 6) is about. It was there before already. |
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- backend/src/main/java/org/cryptomator/hub/api/SettingsResource.java (1 hunks)
- backend/src/main/java/org/cryptomator/hub/api/UsersResource.java (4 hunks)
- frontend/src/components/GrantPermissionDialog.vue (3 hunks)
- frontend/src/components/SignUserKeysDialog.vue (1 hunks)
- frontend/src/components/TrustDetails.vue (1 hunks)
- frontend/src/i18n/de-DE.json (2 hunks)
- frontend/src/i18n/en-US.json (2 hunks)
Files not reviewed due to errors (1)
- backend/src/main/java/org/cryptomator/hub/api/UsersResource.java (no review received)
Files skipped from review as they are similar to previous changes (5)
- backend/src/main/java/org/cryptomator/hub/api/SettingsResource.java
- frontend/src/components/SignUserKeysDialog.vue
- frontend/src/components/TrustDetails.vue
- frontend/src/i18n/de-DE.json
- frontend/src/i18n/en-US.json
Additional comments not posted (2)
frontend/src/components/GrantPermissionDialog.vue (2)
12-41: Ensure consistent UI component alignment and spacing.In the dialog template, ensure that UI elements such as buttons and form inputs are consistently aligned and spaced. This is crucial for maintaining a clean and professional user interface.
72-75: Refactor imports to maintain clean and organized code.Consider grouping related imports to improve readability and maintainability of the code.
- import backend, { AccessGrant, ConflictError, NotFoundError, TrustDto, UserDto, VaultDto } from '../common/backend'; - import { VaultKeys } from '../common/crypto'; - import TrustDetails from './TrustDetails.vue'; + import backend, { AccessGrant, ConflictError, NotFoundError, TrustDto, UserDto, VaultDto } from '../common/backend'; + import { VaultKeys } from '../common/crypto'; + import TrustDetails from './TrustDetails.vue';
instead of linking to jwt.io
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- frontend/src/components/AuditLogDetailsSignedWotId.vue (1 hunks)
Additional comments not posted (3)
frontend/src/components/AuditLogDetailsSignedWotId.vue (3)
1-37: Well-structured template section.The use of Vue directives for conditional rendering and class bindings is correctly implemented, ensuring a clean and maintainable template.
40-61: Well-organized script setup with clear reactive properties.The use of TypeScript and Vue composables like
refandonMountedis correctly implemented, enhancing code clarity and maintainability.
77-80: Good handling of JWT decoding internally.The implementation avoids external tools like jwt.io, addressing previous privacy concerns effectively.
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)
- frontend/src/common/wot.ts (1 hunks)
- frontend/src/components/AuditLogDetailsSignedWotId.vue (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- frontend/src/common/wot.ts
- frontend/src/components/AuditLogDetailsSignedWotId.vue
There was a problem hiding this comment.
LGMT, the only thing I stumbled across was the "Signed Identity" with status "signature invalid"
I think users will have problems figuring out what this means, how it can happen, whether it is a problem (red color) and how to solve it. Either we put a tooltip in there or we leave it as it is and explain it in the docs.
There was a problem hiding this comment.
Functionality is looking good. Approving this since we can make further improvements in a separate PR (or directly on develop). I noticed the following:
- Should the "Check Identity" button be removed once the validation was successful (or the trust level is 100%)?
- Is "Unverified" the same as "Trust Level 0%"?
- Why is
wotIdVerifyLen@Min(0)? Shouldn't the minimum be larger? And you've already mentioned it, but I'm not sure if2is a good default value. But maybe it's enough? No idea, to be honest. - Minor UI stuff inside the new popover
didn't put much thought into the visual aspects, just made sure the validation runs locally instead of linking to jwt.io
I wouldn't remove it, but it should probably change its appearance, depending on whether its a first time verification or revalidation.
I guess so, where did you find both?
I would stick with min 0, as organizations may decide that visual comparison of the ID is sufficient. Default 2 is debatable of course, just found it easier for testing, as I was memorizing the IDs of alice, bob, carol and dave. 😅
Feel free to polish. |
API endpoints
This adds two new methods to the users resource:
PUT /api/users/trusted/{userId}, which stores a signature created by the current user for the user referenced byuserIdGET /api/users/trusted, which retrieves all users with their corresponding signature chains that are directly or transitively trusted by the current userThese added test cases demonstrate the usage.
Database
This adds a new table
wotas well as a vieweffective_wot. Thewottable stores all signatures added via the aforementioned endpoint. Theeffective_wotcontains the signature chains by looking up transitive trusts up to a graph depth of 10.Frontend
A component is added to signify whether a user can be trusted. It shows whether a chain of trust between the user in question and the logged in user exists. Furthermore it allows to sign the user's public keys.
Future work
Currently, there are two new parameters in the
settingstable that may be exposed to the admin interface in a separate future PR:wot_max_depthlimits the number of edges in the WoT graph between two users when trying to find a trust chainwot_id_verify_lenis used by the frontend to specify how many hex digits of the hash of user U's public key a signer S has to enter in order to sign U's keys.The chosen defaults are up for debate, of course.