#1784 closed enhancement (fixed)
Add support for logging old values
Reported by: | Nicklas Nordborg | Owned by: | Nicklas Nordborg |
---|---|---|---|
Priority: | major | Milestone: | BASE 3.3 |
Component: | core | Version: | |
Keywords: | Cc: |
Description
The current logging implementation has support for detecting changes to individual properties, but it can only list which properties that was changed, not the store the old value.
If logging at this level should be enabled or not should be controlled by a flag in base.config. There is already a section with logging settings so this should not be a problem.
It should also be possible to display the old values in the gui.
To make things simple all values should be converted to strings before logging using their JRE .toString()
methods.
Change History (12)
comment:1 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 11 years ago
comment:3 by , 11 years ago
Here are a few ideas that can be implemented independently of each other:
- A new role-based permission that control access to the "old values log" in the same way that the current CHANGE_HISTORY permission control access to the existing change history. This should be fairly easy to implement but since it is still a global permission the administrator has to be careful when granting it if the log can contain sensitive data.
- A flag on
AnnotationType
that can disable the logging for annotations that contain sensitive date. It will still be visible that a change has been made but the actual values are not logged. This is a secure solution since the sensitive data is not logged in the first place, but not even administrators will know what the old data was.
2b. A variant is to add the flag to other item types as well to be able to control logging for each individual item. Instead of a simple on/off flag a numeric log-level can be used to control the amount of detail to log. However there is an issue with this that a user that want to "hide" changes may first change the log level to "off", make the change and then restore the log level. Even if there is a log entry telling that the log level has been modified it may not satisfy an administrator. If this variant is implemented it must probably be paired with a new access permission (eg. SET_LOG_LEVEL) for modifying the log level.
- Implement access control on for log entries that is compatible with the current access control for
Shareable
items. This requires syncing of permissions with a "sync parent" item so that the access permissions to the log entries is always the same as the access permissions to the "sync parent" item. The "sync parent" item may different from the item the log entry belongs to. For example, a log entry for a modified sample annotation has the annotation type as the "sync parent" item (not the sample).
The third option is the only option that doesn't impose any restrictions on the setup of logging and/or permissions for users. But this is also the most complicated to implement and performance considerations are required to make sure that the system is able to handle it.
Also note that the three options cover slightly different use cases and there is nothing that prevents all of them from being implemented for optimal(?) flexibility.
comment:4 by , 11 years ago
After a short meeting it was decided that 1 and 2 (not 2b) should be good enough and cover the use cases we need to cover at the moment.
comment:5 by , 11 years ago
(In [6351]) References #1784: Add support for logging old values
The core functionality has been implemented. Added two new columns in the ChangeHistorDetails
table (old_value and new_value). The DefaultEntityLogger
is used for testing and it works ok for some values (string and numbers), dates are ok but need better formatting than the default provided by Java. Need to think about how to format changes to entities that simply use the output from BasicData.toString()
(eg. old value=ItemSubtypeData[id=64]
) which is not very informative.
The update script will also modify existing log entries so that they as much as possible use the same format as the updated loggers. All 'Annotation updated: <name>' enties are replaced with 'Annotation[<name>]'. File changes get similar updates. Entries that list changes to multiple properties are cloned and modified to only display a single property each.
comment:6 by , 11 years ago
comment:7 by , 11 years ago
comment:8 by , 11 years ago
(In [6355]) References #1784: Add support for logging old values
Added permission check for reading old and new values from the change history. The permission is handled by the new Item.CHANGEHISTORY_VALUES
and can be assigned to roles. The permissions is only given to administrators by default.
Special care is needed when using the Metadata
class to load property values via reflection since that bypasses the regular permission checking. The Metadata
class is for example used by the table exporter plug-in and could potentially by exploited by a user to get access to the logged values without having the correct permission.
To solve this problem the PropertyPathProtected
annotation was created. If this annotation is present on a getter method, the property will not be loadable by the Metadata
class. The table exporter will have to use the specific ChangeHistoryDetailLoader
instead which uses the regular (permission-controlled) methods.
comment:9 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
There is at least one security-related issue that must be considered before implementing this. Access to annotations is controlled by the permissions on the annotation type. This means that a user that can see information about, for example, a sample may not have access to all annotations on that sample.
If an administrator changes the value of a restricted annotation, the old value is suddenly visible in the change history of that item. In a default BASE installation the change history is only visible for administrators so this may not actually be an issue. But it is easy to give other users access to the change log, and this also give those users access to (old) annotation values that they may not have access to otherwise.
So, we need to think about if the current permission control is enough (a global on/off depending on user role) or if we need more fine-grained access control to the change history log.