Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audit user changes #8137

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

josegar74
Copy link
Member

@josegar74 josegar74 commented Jun 3, 2024

This change request allows to audit the changes the users information, using Hibernate Envers.

The feature is disabled by default and can be enabled in the system settings:

setting

Once enabled, changes to users are audited and can be consulted by administrators in the users form or the users access report:

user-changes

The feature can be extended to support additional types of entities like Groups.


Initially the pull request used Javers, but due to an incompatible version of picocontainer, used also in Geotools, has been changed to Hibernate Envers.

Checklist

  • I have read the contribution guidelines
  • Pull request provided for main branch, backports managed with label
  • Good housekeeping of code, cleaning up comments, tests, and documentation
  • Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
  • Clean commit messages, longer verbose messages are encouraged
  • API Changes are identified in commit messages
  • Testing provided for features or enhancements using automatic tests
  • User documentation provided for new features or enhancements in manual
  • Build documentation provided for development instructions in README.md files
  • Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

@josegar74 josegar74 added this to the 4.4.6 milestone Jun 3, 2024
@josegar74 josegar74 force-pushed the 44-audit-user-changes branch 5 times, most recently from 9015aa9 to 5691db0 Compare June 27, 2024 08:00
@josegar74 josegar74 marked this pull request as ready for review June 27, 2024 09:14
Copy link
Member

@fxprunayre fxprunayre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine.

Would this be a better alternative for the metadata versionning (currently stored in metadatastatus)?

@fxprunayre fxprunayre modified the milestones: 4.4.6, 4.4.7 Oct 24, 2024
Comment on lines 240 to 243
List<String> groupsRegisteredUserList = new ArrayList<>();
List<String> groupsEditorList = new ArrayList<>();
List<String> groupsReviewerList = new ArrayList<>();
List<String> groupsUserAdminList = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm changing this to TreeSet to get this groups always in the same order.

Comment on lines 640 to 641
user = userRepository.save(user);
setUserGroups(user, groups);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed if in line 634 the user was already saved?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, removed in 64ff8f5

@@ -615,6 +637,16 @@ public ResponseEntity<String> updateUser(
setUserGroups(user, groups);
}

user = userRepository.save(user);
setUserGroups(user, groups);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In lines 636-638 the setUserGroups() is called conditionally. However, here is being called without any condition always saving the groups even when they shouldn't be saved.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, removed in 64ff8f5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

3 participants