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.
It sounds more like an enhancement request.
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.
I'm fine with waiting. Added dependency.
It looks like bug 20813 is not happening, we might want to come back to this.
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).
Also, due to the legalities, this is an actual missing feature that qualifies as a bug.
Should it be in the action_logs? Maybe another table?
It would be good to also log permission changes via Koha::Logger with a namespace so these changes can be logged to external facilities.
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.
I reckon we log it in action_logs, but just make it so that it can't be toggled off.
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.
(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.
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.
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?
(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.
(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?
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).
Would like to resurrect this one please. Not storing changes to patron permissions is a security concern. We are also finding this requirement coming up in tenders.
Now that we store the diff in the json, maybe we can tie this into Bug 36698 to make a nice diff view of permission changes?
(In reply to Lisette Scheer from comment #19) > Now that we store the diff in the json, maybe we can tie this into Bug 36698 > to make a nice diff view of permission changes? Yeah, that could be nice. I think in the past I was concerned about using action_logs since they can be truncated periodically, but something would also be better than nothing, and this would be a nice way to do it.