Bug 20956 - BorrowersLog is not logging permission changes
Summary: BorrowersLog is not logging permission changes
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: master
Hardware: All All
: P2 major (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 20813
Blocks: 22794
  Show dependency treegraph
 
Reported: 2018-06-15 15:03 UTC by Christopher Brannon
Modified: 2024-01-30 16:56 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2018-06-15 15:03:42 UTC
I made changes to permissions, and I can see that it is not being logged.  This is a security issue and should be fixed immediately.  We cannot have people changing permissions undetected.
Comment 1 Jonathan Druart 2018-06-18 14:50:12 UTC
It sounds more like an enhancement request.
Comment 2 Katrin Fischer 2018-06-18 20:43:07 UTC
I agree with both Christopher and Jonathan. We never logged permissions and this has never come up before, so I would also classify this as an enhancement. But at the same time I think it would be a very good and valuable addition. Currently there is some work done on permissions, we might want to wait for bug 20813 before implementing this.
Comment 3 Christopher Brannon 2018-06-18 22:25:26 UTC
I'm fine with waiting.  Added dependency.
Comment 4 Katrin Fischer 2020-07-06 06:22:25 UTC
It looks like bug 20813 is not happening, we might want to come back to this.
Comment 5 Liz Rea 2021-11-19 20:36:26 UTC
We definitely want to come back to this, there is a whole slew of legislation out right now that is going to require certifications that changes such as this can be audited (at least in the US, and I presume elsewhere).
Comment 6 Liz Rea 2021-11-19 20:46:53 UTC
Also, due to the legalities, this is an actual missing feature that qualifies as a bug.
Comment 7 Tomás Cohen Arazi 2021-11-19 23:58:44 UTC
Should it be in the action_logs? Maybe another table?
Comment 8 Kyle M Hall 2021-12-02 18:03:09 UTC
It would be good to also log permission changes via Koha::Logger with a namespace so these changes can be logged to external facilities.
Comment 9 Katrin Fischer 2021-12-05 22:20:16 UTC
I think action_logs would be the logical place for it as it also has the other changes to the patron account for the librarians as it can be looked up easily then, but something extermal might be nice additionally.
Comment 10 David Cook 2022-11-02 01:34:26 UTC
I reckon we log it in action_logs, but just make it so that it can't be toggled off.
Comment 11 Katrin Fischer 2022-11-02 23:09:32 UTC
There are scripts that delete older entries from action_logs, something to think about if you don't want these to be deleted by accident.
Comment 12 David Cook 2022-11-02 23:33:13 UTC
(In reply to Katrin Fischer from comment #11)
> There are scripts that delete older entries from action_logs, something to
> think about if you don't want these to be deleted by accident.

I suppose that's a problem we'll face with any kind of logging potentially.

But that does explain Kyle's suggestion of logging it outside of Koha. Log rotation would be up to the sysadmins.
Comment 13 Katrin Fischer 2023-07-11 08:56:19 UTC
We just had a case where this kind of log would have been very useful. 

(In reply to Katrin Fischer from comment #11)
> There are scripts that delete older entries from action_logs, something to
> think about if you don't want these to be deleted by accident.

We can limit deletions now to certain modules and even actions I think. So that should no longer be a problem.

To be on the save side we could create a PermissionLog maybe? Keep it separate from other changes to the patron record and a separate module.
Comment 14 Magnus Enger 2024-01-29 14:13:28 UTC
How verbose should we do the logging? As far as I can tell, the information about permissions is stored in a combination of borrowers.flags and the user_permissions table. 

The information from the user_permissions table could probably be logged as is, and be pretty readable. 

But the flags is just a number, that needs to be interpreted in some way to make sense. Should we log just the number, or the interpretation?
Comment 15 Kyle M Hall 2024-01-29 14:16:44 UTC
(In reply to Magnus Enger from comment #14)
> How verbose should we do the logging? As far as I can tell, the information
> about permissions is stored in a combination of borrowers.flags and the
> user_permissions table. 
> 
> The information from the user_permissions table could probably be logged as
> is, and be pretty readable. 
> 
> But the flags is just a number, that needs to be interpreted in some way to
> make sense. Should we log just the number, or the interpretation?

My initial concern was performance, but considering user permissions changes are not a frequent activity I think either solution is reasonable. Either decode it and store it as a list of key/value pairs or just log the binary representation with a link to a wiki page where we can explain how to decode it.
Comment 16 Christopher Brannon 2024-01-30 15:53:21 UTC
(In reply to Magnus Enger from comment #14)
> How verbose should we do the logging? As far as I can tell, the information
> about permissions is stored in a combination of borrowers.flags and the
> user_permissions table. 
> 
> The information from the user_permissions table could probably be logged as
> is, and be pretty readable. 
> 
> But the flags is just a number, that needs to be interpreted in some way to
> make sense. Should we log just the number, or the interpretation?

If this is a log that a non-admin should be able to interpret, it should be the interpretation.  You could log both, especially for debugging, but the interpretation should definitely be there.  Why put only the number and make people jump through hoops to interpret if the system can do it without sacrificing performance?
Comment 17 Katrin Fischer 2024-01-30 16:56:18 UTC
I agree that we need a nice display in the log viewer at least, if we store it somewhat machine readable I would not mind that much as long as it display nicely in the GUI. Please also keep translatability in mind (also a pro for a template side solution).