Feature: User Audit Log #310
Conversation
Co-authored-by: iammajid <[email protected]>
Co-authored-by: iammajid <[email protected]>
Co-authored-by: iammajid <[email protected]>
WalkthroughThis pull request introduces two new event types— Possibly related PRs
Suggested reviewers
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
frontend/src/components/AuditLogDetailsUserAccountSetupComplete.vue (1)
32-36: 🛠️ Refactor suggestionHandle potential errors in authority resolution.
Similar to AuditLogDetailsUserAccountReset.vue, the
onMountedhook should handle potential errors when fetching authority details.
🧹 Nitpick comments (6)
backend/src/main/java/org/cryptomator/hub/entities/events/UserAccountResetEvent.java (2)
15-18: Add field constraints and validation.Consider enhancing the
resetByfield with additional constraints:
- Add nullability constraint
- Add size limits
- Consider input validation for security
@Column(name = "reset_by") +@NotNull +@Size(min = 1, max = 255) +@Pattern(regexp = "^[a-zA-Z0-9@._-]+$") // adjust pattern based on your username requirements private String resetBy;
20-26: Consider adding validation in setter.The setter method could benefit from parameter validation to ensure data integrity.
public void setResetBy(String resetBy) { + Objects.requireNonNull(resetBy, "resetBy must not be null"); + if (resetBy.trim().isEmpty()) { + throw new IllegalArgumentException("resetBy must not be empty"); + } this.resetBy = resetBy; }backend/src/main/java/org/cryptomator/hub/entities/events/UserAccountSetupCompleteEvent.java (3)
15-15: Add Javadoc for the TYPE constant.Consider adding Javadoc to document the purpose and usage of this constant, especially since it's public and used in the @DiscriminatorValue annotation.
+ /** + * Discriminator value for user account setup completion events. + * Used to identify this event type in the audit log. + */ public static final String TYPE = "USER_ACCOUNT_SETUP_COMPLETE";
17-18: Add nullability constraint to the completedBy field.Consider adding a nullability constraint to explicitly define whether this field can be null. This helps maintain data integrity and makes the contract clearer.
- @Column(name = "completed_by") + @Column(name = "completed_by", nullable = false) private String completedBy;
29-34: Add self-comparison check to equals method.The equals method should first check if the object being compared is the same instance (self-comparison) for better performance.
@Override public boolean equals(Object o) { + if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; if (!super.equals(o)) return false; UserAccountSetupCompleteEvent that = (UserAccountSetupCompleteEvent) o; return Objects.equals(completedBy, that.completedBy); }frontend/src/components/AuditLogDetailsUserAccountReset.vue (1)
12-13: Consider handling loading and error states.The component should handle loading and error states when fetching authority details to improve user experience.
- <span v-if="resolvedResetBy != null">{{ resolvedResetBy.name }}</span> - <code class="text-xs" :class="{'text-gray-600': resolvedResetBy != null}">{{ event.resetBy }}</code> + <template v-if="loading"> + <span class="text-gray-500">Loading...</span> + </template> + <template v-else-if="error"> + <span class="text-red-500">Failed to load authority details</span> + <code class="text-xs">{{ event.resetBy }}</code> + </template> + <template v-else> + <span v-if="resolvedResetBy != null">{{ resolvedResetBy.name }}</span> + <code class="text-xs" :class="{'text-gray-600': resolvedResetBy != null}">{{ event.resetBy }}</code> + </template>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
backend/src/main/java/org/cryptomator/hub/api/AuditLogResource.java(4 hunks)backend/src/main/java/org/cryptomator/hub/api/UsersResource.java(2 hunks)backend/src/main/java/org/cryptomator/hub/entities/events/EventLogger.java(1 hunks)backend/src/main/java/org/cryptomator/hub/entities/events/UserAccountResetEvent.java(1 hunks)backend/src/main/java/org/cryptomator/hub/entities/events/UserAccountSetupCompleteEvent.java(1 hunks)backend/src/main/resources/org/cryptomator/hub/flyway/V18__Create_Audit_Event_User_Account_Setup_Or_Reset.sql(1 hunks)frontend/src/common/auditlog.ts(2 hunks)frontend/src/components/AuditLog.vue(2 hunks)frontend/src/components/AuditLogDetailsUserAccountReset.vue(1 hunks)frontend/src/components/AuditLogDetailsUserAccountSetupComplete.vue(1 hunks)frontend/src/i18n/de-DE.json(2 hunks)frontend/src/i18n/en-US.json(2 hunks)
🔇 Additional comments (19)
backend/src/main/java/org/cryptomator/hub/entities/events/UserAccountResetEvent.java (1)
1-13: LGTM! Well-structured entity class definition.The JPA annotations and inheritance setup are correctly implemented, following best practices for entity mapping and audit event hierarchy.
backend/src/main/java/org/cryptomator/hub/entities/events/UserAccountSetupCompleteEvent.java (1)
10-13: LGTM! Well-structured entity class.The class is properly annotated with JPA annotations and follows the established naming conventions for audit event entities.
frontend/src/components/AuditLogDetailsUserAccountSetupComplete.vue (1)
12-13: Consider handling loading and error states.Similar to AuditLogDetailsUserAccountReset.vue, this component should handle loading and error states when fetching authority details.
backend/src/main/java/org/cryptomator/hub/entities/events/EventLogger.java (1)
56-61: LGTM! The new event logging methods follow consistent patterns.The implementation maintains consistency with other event logging methods in the class and properly handles event persistence.
Also applies to: 63-68
frontend/src/common/auditlog.ts (2)
39-47: LGTM! The new DTOs follow consistent patterns.The new DTOs maintain consistency with existing DTOs and properly extend the base DTO type.
109-109: LGTM! Union type correctly updated.The AuditEventDto union type has been properly updated to include the new event types.
backend/src/main/java/org/cryptomator/hub/api/UsersResource.java (2)
86-86: LGTM! Well-placed audit logging for account setup.The logging statement is correctly placed after all user data is set but before persistence, ensuring accurate tracking of the setup completion event.
172-172: LGTM! Well-placed audit logging for account reset.The logging statement is correctly placed after user data is cleared but before device/token deletion, ensuring accurate tracking of the reset event.
backend/src/main/java/org/cryptomator/hub/api/AuditLogResource.java (4)
22-23: LGTM! Proper import organization.The new event class imports are correctly added and organized with related imports.
89-90: LGTM! Proper JSON type configuration.The new event types are correctly registered in the JsonSubTypes configuration, following the established pattern.
114-115: LGTM! Consistent event handling.The new cases in the switch statement follow the established pattern and correctly map to their respective DTOs.
144-148: LGTM! Well-structured DTOs.The new DTOs follow the established record pattern with proper JSON property annotations.
frontend/src/components/AuditLog.vue (2)
108-109: LGTM! Proper event type handling in template.The new event types are correctly handled with v-else-if conditions, following the established pattern.
177-178: LGTM! Well-organized component imports.The new component imports are correctly added and organized with related imports.
backend/src/main/resources/org/cryptomator/hub/flyway/V18__Create_Audit_Event_User_Account_Setup_Or_Reset.sql (2)
1-7: LGTM! Well-structured setup event table.The table definition follows best practices:
- Primary key constraint properly defined
- Foreign key with cascade delete ensures referential integrity
- Appropriate column types and NOT NULL constraints
9-15: LGTM! Well-structured reset event table.The table definition follows best practices and maintains consistency with the setup event table:
- Consistent naming conventions
- Similar structure and constraints
- Proper foreign key relationship
frontend/src/i18n/en-US.json (2)
84-85: New Audit Log Event Entries Added.
The entries"auditLog.details.user.account.reset": "Reset User Account"and"auditLog.details.user.account.setup.complete": "Complete Account Setup"have been added correctly to support the new user account events as described in the PR objectives. Please ensure that the backend and frontend components reference these keys consistently.
94-94: Repositioning of Existing Audit Detail.
The"auditLog.details.wot.signedIdentity": "Signed Identity"entry has been moved to a different location in the JSON file. Verify that any components using this key are updated if necessary and that the change in ordering does not cause lookup issues.frontend/src/i18n/de-DE.json (1)
84-85: New German Audit Log Event Entries Added.
The new entries"auditLog.details.user.account.reset": "Account zurückgesetzt"and"auditLog.details.user.account.setup.complete": "Account eingerichtet"are correctly added with appropriate German translations. Please double-check that these keys align with the corresponding backend event types.
backend/src/main/java/org/cryptomator/hub/entities/events/UserAccountResetEvent.java
Show resolved
Hide resolved
backend/src/main/java/org/cryptomator/hub/entities/events/UserAccountSetupCompleteEvent.java
Show resolved
Hide resolved
This pull request extends the audit log by adding entries for ‘Setup Complete’ and ‘Account Reset’.