Bug 24151

Summary: Add a pseudonymization process for patrons and transactions
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: PatronsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: new feature    
Priority: P5 - low CC: arthur.suzuki, aude.charillon, axelle.clarisse, black23, dcook, gmcharlt, janet.mcgowan, jonathan.druart, josef.moravec, julian.maurice, koha, kyle.m.hall, lucas, m.de.rooy, rbit
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Medium patch
Documentation contact: Aude Charillon Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/666
Text to go in the release notes:
This new feature adds a way to pseudonymize patron data, in a way that it will not be able to identify the person (https://en.wikipedia.org/wiki/Pseudonymization) There are different existing ways to anonymize patron information in Koha, but by removing information we loose the ability to make useful reports. This development introduces two new tables: * pseudonymized_transactions for transactions and patron data * pseudonymized_borrower_attributesfor patron attributes Entries to pseudonymized_transactions are added when a new transaction (checkout, checkin, renew, on-site checkout) is done. The table anonymized_borrower_attributes is populated if patron attributes are marked as "keep for pseudonymization". To make things configurable, three system preferences have been added: * Pseudonymization to turn on/off the whole feature * PseudonymizationPatronFields to list the information of the patrons to synchronize * PseudonymizationTransactionFields to list the information of the transactions to copy
Version(s) released in:
20.11.00
Bug Depends on: 20443, 24149, 24150    
Bug Blocks: 18081, 28911, 32438, 32440, 32441, 24152, 28912, 29341, 32439    
Attachments: Bug 24151: DB Changes
Bug 24151: Add sysprefs
Bug 24151: Copy info to the pseudonymized table when a patron is modified
Bug 24151: Add tests
Bug 24151: Pseudonymize transactions on the fly
Bug 24151: Add tests for transactions
Bug 24151: Move get_hash to Koha::Anonymized
Bug 24151: Add tests - if config does not exist
Bug 24151: Add warning to the about page if key is missing in the config
Bug 24151: Add key to the config
Bug 24151: Handle case where the setting is turned on later
Bug 24151: DB changes - anonymized_borrower_attributes
Bug 24151: Add new column borrower_attribute_types.keep_for_anonymized
Bug 24151: Adding keep_for_anonymized to the UI
Bug 24151: Add Koha::Anonymized::Patron::Attribute[s] classes
Bug 24151: Sync patron's attributes
Bug 24151: DBIC Changes
Bug 24151: Add boolean flag for has_cardnumber
Bug 24151: DB changes
Bug 24151: DBIC changes
Bug 24151: Copy info to the pseudonymized table when a transaction is done
Bug 24151: Add tests
Bug 24151: Add tests - if config does not exist
Bug 24151: Add warning to the about page if key is missing in the config
Bug 24151: Add key to the config
Bug 24151: DB changes - pseudonymized_borrower_attributes
Bug 24151: Add new column borrower_attribute_types.keep_for_pseudonymization
Bug 24151: Adding keep_for_anonymized to the UI
Bug 24151: Sync patron's attributes
Bug 24151: DBIC changes
Bug 24151: Add boolean flag for has_cardnumber
Bug 24151: DB changes
Bug 24151: DBIC changes
Bug 24151: Copy info to the pseudonymized table when a transaction is done
Bug 24151: Add tests
Bug 24151: Add tests - if config does not exist
Bug 24151: Add warning to the about page if key is missing in the config
Bug 24151: Add key to the config
Bug 24151: DB changes - pseudonymized_borrower_attributes
Bug 24151: Add new column borrower_attribute_types.keep_for_pseudonymization
Bug 24151: Adding keep_for_anonymized to the UI
Bug 24151: Sync patron's attributes
Bug 24151: DBIC changes
Bug 24151: Add boolean flag for has_cardnumber
Bug 24151: Fix location on return
Bug 24151: Add items.homebranch to the list
Bug 24151: DBIC changes
Bug 24151: DB changes
Bug 24151: DBIC changes
Bug 24151: Copy info to the pseudonymized table when a transaction is done
Bug 24151: Add tests
Bug 24151: Add tests - if config does not exist
Bug 24151: Add warning to the about page if key is missing in the config
Bug 24151: Add key to the config
Bug 24151: DB changes - pseudonymized_borrower_attributes
Bug 24151: Add new column borrower_attribute_types.keep_for_pseudonymization
Bug 24151: Adding keep_for_anonymized to the UI
Bug 24151: Sync patron's attributes
Bug 24151: DBIC changes
Bug 24151: Add boolean flag for has_cardnumber
Bug 24151: Fix location on return
Bug 24151: Add items.homebranch to the list
Bug 24151: DBIC changes
Bug 24151: DB changes
Bug 24151: DBIC changes
Bug 24151: Copy info to the pseudonymized table when a transaction is done
Bug 24151: Add tests
Bug 24151: Add tests - if config does not exist
Bug 24151: Add warning to the about page if key is missing in the config
Bug 24151: Add key to the config
Bug 24151: DB changes - pseudonymized_borrower_attributes
Bug 24151: Add new column borrower_attribute_types.keep_for_pseudonymization
Bug 24151: Adding keep_for_anonymized to the UI
Bug 24151: Sync patron's attributes
Bug 24151: DBIC changes
Bug 24151: Add boolean flag for has_cardnumber
Bug 24151: Fix location on return
Bug 24151: Add items.homebranch to the list
Bug 24151: DBIC changes
Bug 24151: (QA follow-up) Correct license in two modules
Bug 24151: (QA follow-up) Fix POD warnings from QA tools
Bug 24151: Add description for tests
Bug 24151: Rename the config key with 'bcrypt_settings'
Bug 24151: Use fully qualified names for UpdateStats
Bug 24151: (follow-up) Rename the config key with 'bcrypt_settings'

Description Jonathan Druart 2019-12-03 15:46:42 UTC

    
Comment 1 Jonathan Druart 2019-12-03 18:11:07 UTC
Created attachment 95959 [details] [review]
Bug 24151: DB Changes

Add tables anonymized_borrowers and anonymized_transactions

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 2 Jonathan Druart 2019-12-03 18:11:11 UTC
Created attachment 95960 [details] [review]
Bug 24151: Add sysprefs

3 new sysprefs:
 * Pseudonymization
 * PseudonymizationPatronFields
 * PseudonymizationTransactionFields

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 3 Jonathan Druart 2019-12-03 18:11:14 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2019-12-03 18:11:17 UTC
Created attachment 95962 [details] [review]
Bug 24151: Add tests

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 5 Jonathan Druart 2019-12-03 18:11:20 UTC
Created attachment 95963 [details] [review]
Bug 24151: Pseudonymize transactions on the fly

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 6 Jonathan Druart 2019-12-03 18:11:23 UTC
Created attachment 95964 [details] [review]
Bug 24151: Add tests for transactions

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 7 Jonathan Druart 2019-12-03 18:11:27 UTC
Created attachment 95965 [details] [review]
Bug 24151: Move get_hash to Koha::Anonymized

To avoid duplicated code.

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 8 Jonathan Druart 2019-12-03 18:11:30 UTC
Created attachment 95966 [details] [review]
Bug 24151: Add tests - if config does not exist

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 9 Jonathan Druart 2019-12-03 18:11:33 UTC
Created attachment 95967 [details] [review]
Bug 24151: Add warning to the about page if key is missing in the config

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 10 Jonathan Druart 2019-12-03 18:11:37 UTC
Created attachment 95968 [details] [review]
Bug 24151: Add key to the config

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 11 Jonathan Druart 2019-12-03 18:11:40 UTC
Created attachment 95969 [details] [review]
Bug 24151: Handle case where the setting is turned on later

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 12 Jonathan Druart 2019-12-03 18:11:44 UTC
Created attachment 95970 [details] [review]
Bug 24151: DB changes - anonymized_borrower_attributes

Add a new DB table anonymized_borrower_attributes

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 13 Jonathan Druart 2019-12-03 18:11:47 UTC
Created attachment 95971 [details] [review]
Bug 24151: Add new column borrower_attribute_types.keep_for_anonymized

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 14 Jonathan Druart 2019-12-03 18:11:51 UTC
Created attachment 95972 [details] [review]
Bug 24151: Adding keep_for_anonymized to the UI

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 15 Jonathan Druart 2019-12-03 18:11:54 UTC
Created attachment 95973 [details] [review]
Bug 24151: Add Koha::Anonymized::Patron::Attribute[s] classes

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 16 Jonathan Druart 2019-12-03 18:11:58 UTC
Created attachment 95974 [details] [review]
Bug 24151: Sync patron's attributes

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 17 Jonathan Druart 2019-12-03 18:12:01 UTC
Created attachment 95975 [details] [review]
Bug 24151: DBIC Changes
Comment 18 Jonathan Druart 2019-12-03 18:12:05 UTC
Created attachment 95976 [details] [review]
Bug 24151: Add boolean flag for has_cardnumber
Comment 19 Jonathan Druart 2019-12-03 18:15:59 UTC
The whole patch stack of this feature has been pushed to a remote branch (https://gitlab.com/joubu/Koha/commits/bug_24151)
It includes the tree dependency of bug 24153
Comment 20 Jonathan Druart 2020-01-11 18:12:45 UTC
Created attachment 97240 [details] [review]
Bug 24151: DB changes

1 new table:
 * pseudonymized_transactions

3 new sysprefs:
 * Pseudonymization
 * PseudonymizationPatronFields
 * PseudonymizationTransactionFields

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 21 Jonathan Druart 2020-01-11 18:12:49 UTC
Created attachment 97241 [details] [review]
Bug 24151: DBIC changes
Comment 22 Jonathan Druart 2020-01-11 18:12:55 UTC
Created attachment 97242 [details] [review]
Bug 24151: Copy info to the pseudonymized table when a transaction is done

This is the commit where you will find useful information about this development.

The goal of this new feature is to add a way to pseudonymize patron's
data, in a way they could not be personally identifiable.
https://en.wikipedia.org/wiki/Pseudonymization

There are different existing way to anonymize patron's information in
Koha, but we loose the ability to make useful report.
This development proposes to have 2 different tables:
  * 1 for transactions and patrons data (pseudonymized_transactions)
  * 1 for patrons' attributes (pseudonymized_borrower_attributes)
Entries to pseudonymized_transactions are added when a new transaction
(checkout, checkin, renew, on-site checkout) is done.
Also, anonymized_borrower_attributes is populated if patron's attributes are
marked as "keep for pseudonymization".

To make those informations not identifiable to a patron, we are having a
hashed_borrowernumber column in pseudonymized_transactions. This hash will be
generated (Blowfish-based crypt) using a key stored in the Koha
configuration.

To make things configurable, we are adding 3 sysprefs and 1 new DB
column:
  * syspref Pseudonymization to turn on/off the whole feature
  * syspref PseudonymizationPatronFields to list the informations of the
  patrons to sync
  * syspref PseudonymizationTransactionFields to list the informations
  of the transactions to copy
  * DB column borrower_attribute_types.keep_for_pseudonymization that is a
  boolean to enable/disable the copy of a given patron's attribute type.

Test plan:
1/ Turn on Pseudonymization
2/ Define in PseudonymizationPatronFields and
PseudonymizationTransactionFields the different fields you want to copy
3/ Go to the about page
=> You will see a warning about a missing config entry
4/ You need to generate a key and put it in the koha-conf.xml file. The
following command will generate one:
  % htpasswd -bnBC 10 "" password | tr -d ':\n' | sed 's/$2y/$2a/'
Then edit $KOHA_CONF and add it before of the end of the config section (</config)
  it should be something like:
    <key>$2a$10$PfdrEBdRcL2MZlEtKueyLegxI6zg735jD07GRnc1bt.N/ZYMvBAB2</key>
5/ Restart memcached then plack (alias restart_all)
=> Everything is setup!
6/ Create a new transaction (checkin for instance)
=> Confirm that a new entry has been added to pseudonymized_transaction with the data
you expect to be copied
7/ Edit some patron attribute types and tick "Keep for pseudonymization"
8/ Create a new transaction
=> Confirm that new entries have been added to pseudonymized_borrower_attributes
11/ Delete the patrons
=> Confirm that the entries still exist in the pseudonymized_* tables
12/ Purge the patrons (ie. use cleanup_database.pl to remove them from
the deleted_borrowers table)
=> Confirm that the entries still exist in the pseudonymized_* tables

See bug 24152 to remove data from the anonymized_* tables

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 23 Jonathan Druart 2020-01-11 18:12:58 UTC
Created attachment 97243 [details] [review]
Bug 24151: Add tests

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 24 Jonathan Druart 2020-01-11 18:13:02 UTC
Created attachment 97244 [details] [review]
Bug 24151: Add tests - if config does not exist

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 25 Jonathan Druart 2020-01-11 18:13:07 UTC
Created attachment 97245 [details] [review]
Bug 24151: Add warning to the about page if key is missing in the config

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 26 Jonathan Druart 2020-01-11 18:13:11 UTC
Created attachment 97246 [details] [review]
Bug 24151: Add key to the config

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 27 Jonathan Druart 2020-01-11 18:13:15 UTC
Created attachment 97247 [details] [review]
Bug 24151: DB changes - pseudonymized_borrower_attributes

Add a new DB table pseudonymized_borrower_attributes

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 28 Jonathan Druart 2020-01-11 18:13:19 UTC
Created attachment 97248 [details] [review]
Bug 24151: Add new column borrower_attribute_types.keep_for_pseudonymization

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 29 Jonathan Druart 2020-01-11 18:13:23 UTC
Created attachment 97249 [details] [review]
Bug 24151: Adding keep_for_anonymized to the UI

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 30 Jonathan Druart 2020-01-11 18:13:28 UTC
Created attachment 97250 [details] [review]
Bug 24151: Sync patron's attributes
Comment 31 Jonathan Druart 2020-01-11 18:13:32 UTC
Created attachment 97251 [details] [review]
Bug 24151: DBIC changes
Comment 32 Jonathan Druart 2020-01-11 18:13:36 UTC
Created attachment 97252 [details] [review]
Bug 24151: Add boolean flag for has_cardnumber
Comment 33 Michal Denar 2020-01-18 21:57:10 UTC
Nice work, Jonathan.
Comment 34 Jonathan Druart 2020-02-14 10:52:17 UTC
*** Bug 20604 has been marked as a duplicate of this bug. ***
Comment 35 Jonathan Druart 2020-03-10 10:25:51 UTC
Thanks Michal.

However I am switching it back to Needs Signoff as the patches needs a signoff from the sponsor.
Comment 36 Jonathan Druart 2020-03-10 10:28:17 UTC
Up-to-date patches are on the remote branch: https://gitlab.com/joubu/Koha/commits/bug_24151
Comment 37 Koha Team University Lyon 3 2020-03-19 16:02:36 UTC
I've tested for Association KohaLa.
I've followed the test plan and everything is Ok.
I've played to check / uncheck borrowers/transaction information in the PseudonymizationPatronFields and PseudonymizationTransactionFields and data saved in the pseudonymized_transactions are consistent.
I've also checked /unckecked "Keep for pseudonymization" for borrowers attributes and data in pseudonymized_borrower_attributes are also consistent.

The data are keeped after deleting borrowers and cleaning deletedborrowers.

Thank you for this job!
Comment 38 Jonathan Druart 2020-03-19 16:23:58 UTC
Created attachment 101034 [details] [review]
Bug 24151: DB changes

1 new table:
 * pseudonymized_transactions

3 new sysprefs:
 * Pseudonymization
 * PseudonymizationPatronFields
 * PseudonymizationTransactionFields

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 39 Jonathan Druart 2020-03-19 16:24:01 UTC
Created attachment 101035 [details] [review]
Bug 24151: DBIC changes

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 40 Jonathan Druart 2020-03-19 16:24:05 UTC
Created attachment 101036 [details] [review]
Bug 24151: Copy info to the pseudonymized table when a transaction is done

This is the commit where you will find useful information about this development.

The goal of this new feature is to add a way to pseudonymize patron's
data, in a way they could not be personally identifiable.
https://en.wikipedia.org/wiki/Pseudonymization

There are different existing way to anonymize patron's information in
Koha, but we loose the ability to make useful report.
This development proposes to have 2 different tables:
  * 1 for transactions and patrons data (pseudonymized_transactions)
  * 1 for patrons' attributes (pseudonymized_borrower_attributes)
Entries to pseudonymized_transactions are added when a new transaction
(checkout, checkin, renew, on-site checkout) is done.
Also, anonymized_borrower_attributes is populated if patron's attributes are
marked as "keep for pseudonymization".

To make those informations not identifiable to a patron, we are having a
hashed_borrowernumber column in pseudonymized_transactions. This hash will be
generated (Blowfish-based crypt) using a key stored in the Koha
configuration.

To make things configurable, we are adding 3 sysprefs and 1 new DB
column:
  * syspref Pseudonymization to turn on/off the whole feature
  * syspref PseudonymizationPatronFields to list the informations of the
  patrons to sync
  * syspref PseudonymizationTransactionFields to list the informations
  of the transactions to copy
  * DB column borrower_attribute_types.keep_for_pseudonymization that is a
  boolean to enable/disable the copy of a given patron's attribute type.

Test plan:
1/ Turn on Pseudonymization
2/ Define in PseudonymizationPatronFields and
PseudonymizationTransactionFields the different fields you want to copy
3/ Go to the about page
=> You will see a warning about a missing config entry
4/ You need to generate a key and put it in the koha-conf.xml file. The
following command will generate one:
  % htpasswd -bnBC 10 "" password | tr -d ':\n' | sed 's/$2y/$2a/'
Then edit $KOHA_CONF and add it before of the end of the config section (</config)
  it should be something like:
    <key>$2a$10$PfdrEBdRcL2MZlEtKueyLegxI6zg735jD07GRnc1bt.N/ZYMvBAB2</key>
5/ Restart memcached then plack (alias restart_all)
=> Everything is setup!
6/ Create a new transaction (checkin for instance)
=> Confirm that a new entry has been added to pseudonymized_transaction with the data
you expect to be copied
7/ Edit some patron attribute types and tick "Keep for pseudonymization"
8/ Create a new transaction
=> Confirm that new entries have been added to pseudonymized_borrower_attributes
11/ Delete the patrons
=> Confirm that the entries still exist in the pseudonymized_* tables
12/ Purge the patrons (ie. use cleanup_database.pl to remove them from
the deleted_borrowers table)
=> Confirm that the entries still exist in the pseudonymized_* tables

See bug 24152 to remove data from the anonymized_* tables

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 41 Jonathan Druart 2020-03-19 16:24:09 UTC
Created attachment 101037 [details] [review]
Bug 24151: Add tests

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 42 Jonathan Druart 2020-03-19 16:24:13 UTC
Created attachment 101038 [details] [review]
Bug 24151: Add tests - if config does not exist

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 43 Jonathan Druart 2020-03-19 16:24:17 UTC
Created attachment 101039 [details] [review]
Bug 24151: Add warning to the about page if key is missing in the config

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 44 Jonathan Druart 2020-03-19 16:24:21 UTC
Created attachment 101040 [details] [review]
Bug 24151: Add key to the config

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 45 Jonathan Druart 2020-03-19 16:24:25 UTC
Created attachment 101041 [details] [review]
Bug 24151: DB changes - pseudonymized_borrower_attributes

Add a new DB table pseudonymized_borrower_attributes

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 46 Jonathan Druart 2020-03-19 16:24:29 UTC
Created attachment 101042 [details] [review]
Bug 24151: Add new column borrower_attribute_types.keep_for_pseudonymization

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 47 Jonathan Druart 2020-03-19 16:24:33 UTC
Created attachment 101043 [details] [review]
Bug 24151: Adding keep_for_anonymized to the UI

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 48 Jonathan Druart 2020-03-19 16:24:37 UTC
Created attachment 101044 [details] [review]
Bug 24151: Sync patron's attributes

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 49 Jonathan Druart 2020-03-19 16:24:41 UTC
Created attachment 101045 [details] [review]
Bug 24151: DBIC changes

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 50 Jonathan Druart 2020-03-19 16:24:45 UTC
Created attachment 101046 [details] [review]
Bug 24151: Add boolean flag for has_cardnumber

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 51 Jonathan Druart 2020-03-19 16:24:49 UTC
Created attachment 101047 [details] [review]
Bug 24151: Fix location on return

The item's location where not passed to UpdateStat

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 52 Jonathan Druart 2020-03-19 16:24:53 UTC
Created attachment 101048 [details] [review]
Bug 24151: Add items.homebranch to the list

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 53 Jonathan Druart 2020-03-19 16:24:57 UTC
Created attachment 101049 [details] [review]
Bug 24151: DBIC changes

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 54 Julian Maurice 2020-06-12 13:44:00 UTC
Does not apply on master. Can you rebase please ?
Comment 55 Jonathan Druart 2020-06-12 14:04:55 UTC
Created attachment 105801 [details] [review]
Bug 24151: DB changes

1 new table:
 * pseudonymized_transactions

3 new sysprefs:
 * Pseudonymization
 * PseudonymizationPatronFields
 * PseudonymizationTransactionFields

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 56 Jonathan Druart 2020-06-12 14:05:01 UTC
Created attachment 105802 [details] [review]
Bug 24151: DBIC changes

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 57 Jonathan Druart 2020-06-12 14:05:05 UTC
Created attachment 105803 [details] [review]
Bug 24151: Copy info to the pseudonymized table when a transaction is done

This is the commit where you will find useful information about this development.

The goal of this new feature is to add a way to pseudonymize patron's
data, in a way they could not be personally identifiable.
https://en.wikipedia.org/wiki/Pseudonymization

There are different existing way to anonymize patron's information in
Koha, but we loose the ability to make useful report.
This development proposes to have 2 different tables:
  * 1 for transactions and patrons data (pseudonymized_transactions)
  * 1 for patrons' attributes (pseudonymized_borrower_attributes)
Entries to pseudonymized_transactions are added when a new transaction
(checkout, checkin, renew, on-site checkout) is done.
Also, anonymized_borrower_attributes is populated if patron's attributes are
marked as "keep for pseudonymization".

To make those informations not identifiable to a patron, we are having a
hashed_borrowernumber column in pseudonymized_transactions. This hash will be
generated (Blowfish-based crypt) using a key stored in the Koha
configuration.

To make things configurable, we are adding 3 sysprefs and 1 new DB
column:
  * syspref Pseudonymization to turn on/off the whole feature
  * syspref PseudonymizationPatronFields to list the informations of the
  patrons to sync
  * syspref PseudonymizationTransactionFields to list the informations
  of the transactions to copy
  * DB column borrower_attribute_types.keep_for_pseudonymization that is a
  boolean to enable/disable the copy of a given patron's attribute type.

Test plan:
1/ Turn on Pseudonymization
2/ Define in PseudonymizationPatronFields and
PseudonymizationTransactionFields the different fields you want to copy
3/ Go to the about page
=> You will see a warning about a missing config entry
4/ You need to generate a key and put it in the koha-conf.xml file. The
following command will generate one:
  % htpasswd -bnBC 10 "" password | tr -d ':\n' | sed 's/$2y/$2a/'
Then edit $KOHA_CONF and add it before of the end of the config section (</config)
  it should be something like:
    <key>$2a$10$PfdrEBdRcL2MZlEtKueyLegxI6zg735jD07GRnc1bt.N/ZYMvBAB2</key>
5/ Restart memcached then plack (alias restart_all)
=> Everything is setup!
6/ Create a new transaction (checkin for instance)
=> Confirm that a new entry has been added to pseudonymized_transaction with the data
you expect to be copied
7/ Edit some patron attribute types and tick "Keep for pseudonymization"
8/ Create a new transaction
=> Confirm that new entries have been added to pseudonymized_borrower_attributes
11/ Delete the patrons
=> Confirm that the entries still exist in the pseudonymized_* tables
12/ Purge the patrons (ie. use cleanup_database.pl to remove them from
the deleted_borrowers table)
=> Confirm that the entries still exist in the pseudonymized_* tables

See bug 24152 to remove data from the anonymized_* tables

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 58 Jonathan Druart 2020-06-12 14:05:10 UTC
Created attachment 105804 [details] [review]
Bug 24151: Add tests

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 59 Jonathan Druart 2020-06-12 14:05:14 UTC
Created attachment 105805 [details] [review]
Bug 24151: Add tests - if config does not exist

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 60 Jonathan Druart 2020-06-12 14:05:18 UTC
Created attachment 105806 [details] [review]
Bug 24151: Add warning to the about page if key is missing in the config

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 61 Jonathan Druart 2020-06-12 14:05:22 UTC
Created attachment 105807 [details] [review]
Bug 24151: Add key to the config

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 62 Jonathan Druart 2020-06-12 14:05:27 UTC
Created attachment 105808 [details] [review]
Bug 24151: DB changes - pseudonymized_borrower_attributes

Add a new DB table pseudonymized_borrower_attributes

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 63 Jonathan Druart 2020-06-12 14:05:31 UTC
Created attachment 105809 [details] [review]
Bug 24151: Add new column borrower_attribute_types.keep_for_pseudonymization

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 64 Jonathan Druart 2020-06-12 14:05:35 UTC
Created attachment 105810 [details] [review]
Bug 24151: Adding keep_for_anonymized to the UI

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 65 Jonathan Druart 2020-06-12 14:05:39 UTC
Created attachment 105811 [details] [review]
Bug 24151: Sync patron's attributes

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 66 Jonathan Druart 2020-06-12 14:05:43 UTC
Created attachment 105812 [details] [review]
Bug 24151: DBIC changes

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 67 Jonathan Druart 2020-06-12 14:05:48 UTC
Created attachment 105813 [details] [review]
Bug 24151: Add boolean flag for has_cardnumber

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 68 Jonathan Druart 2020-06-12 14:05:51 UTC
Created attachment 105814 [details] [review]
Bug 24151: Fix location on return

The item's location where not passed to UpdateStat

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 69 Jonathan Druart 2020-06-12 14:06:03 UTC
Created attachment 105815 [details] [review]
Bug 24151: Add items.homebranch to the list

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 70 Jonathan Druart 2020-06-12 14:06:09 UTC
Created attachment 105816 [details] [review]
Bug 24151: DBIC changes

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 71 Marcel de Rooy 2020-06-19 06:04:33 UTC
Looking here now
Comment 72 Marcel de Rooy 2020-06-19 07:29:46 UTC
Created attachment 106032 [details] [review]
Bug 24151: DB changes

1 new table:
 * pseudonymized_transactions

3 new sysprefs:
 * Pseudonymization
 * PseudonymizationPatronFields
 * PseudonymizationTransactionFields

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 73 Marcel de Rooy 2020-06-19 07:29:51 UTC
Created attachment 106033 [details] [review]
Bug 24151: DBIC changes

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 74 Marcel de Rooy 2020-06-19 07:29:56 UTC
Created attachment 106034 [details] [review]
Bug 24151: Copy info to the pseudonymized table when a transaction is done

This is the commit where you will find useful information about this development.

The goal of this new feature is to add a way to pseudonymize patron's
data, in a way they could not be personally identifiable.
https://en.wikipedia.org/wiki/Pseudonymization

There are different existing way to anonymize patron's information in
Koha, but we loose the ability to make useful report.
This development proposes to have 2 different tables:
  * 1 for transactions and patrons data (pseudonymized_transactions)
  * 1 for patrons' attributes (pseudonymized_borrower_attributes)
Entries to pseudonymized_transactions are added when a new transaction
(checkout, checkin, renew, on-site checkout) is done.
Also, anonymized_borrower_attributes is populated if patron's attributes are
marked as "keep for pseudonymization".

To make those informations not identifiable to a patron, we are having a
hashed_borrowernumber column in pseudonymized_transactions. This hash will be
generated (Blowfish-based crypt) using a key stored in the Koha
configuration.

To make things configurable, we are adding 3 sysprefs and 1 new DB
column:
  * syspref Pseudonymization to turn on/off the whole feature
  * syspref PseudonymizationPatronFields to list the informations of the
  patrons to sync
  * syspref PseudonymizationTransactionFields to list the informations
  of the transactions to copy
  * DB column borrower_attribute_types.keep_for_pseudonymization that is a
  boolean to enable/disable the copy of a given patron's attribute type.

Test plan:
1/ Turn on Pseudonymization
2/ Define in PseudonymizationPatronFields and
PseudonymizationTransactionFields the different fields you want to copy
3/ Go to the about page
=> You will see a warning about a missing config entry
4/ You need to generate a key and put it in the koha-conf.xml file. The
following command will generate one:
  % htpasswd -bnBC 10 "" password | tr -d ':\n' | sed 's/$2y/$2a/'
Then edit $KOHA_CONF and add it before of the end of the config section (</config)
  it should be something like:
    <key>$2a$10$PfdrEBdRcL2MZlEtKueyLegxI6zg735jD07GRnc1bt.N/ZYMvBAB2</key>
5/ Restart memcached then plack (alias restart_all)
=> Everything is setup!
6/ Create a new transaction (checkin for instance)
=> Confirm that a new entry has been added to pseudonymized_transaction with the data
you expect to be copied
7/ Edit some patron attribute types and tick "Keep for pseudonymization"
8/ Create a new transaction
=> Confirm that new entries have been added to pseudonymized_borrower_attributes
11/ Delete the patrons
=> Confirm that the entries still exist in the pseudonymized_* tables
12/ Purge the patrons (ie. use cleanup_database.pl to remove them from
the deleted_borrowers table)
=> Confirm that the entries still exist in the pseudonymized_* tables

See bug 24152 to remove data from the anonymized_* tables

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 75 Marcel de Rooy 2020-06-19 07:30:01 UTC
Created attachment 106035 [details] [review]
Bug 24151: Add tests

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 76 Marcel de Rooy 2020-06-19 07:30:06 UTC
Created attachment 106036 [details] [review]
Bug 24151: Add tests - if config does not exist

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 77 Marcel de Rooy 2020-06-19 07:30:11 UTC
Created attachment 106037 [details] [review]
Bug 24151: Add warning to the about page if key is missing in the config

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 78 Marcel de Rooy 2020-06-19 07:30:16 UTC
Created attachment 106038 [details] [review]
Bug 24151: Add key to the config

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 79 Marcel de Rooy 2020-06-19 07:30:23 UTC
Created attachment 106039 [details] [review]
Bug 24151: DB changes - pseudonymized_borrower_attributes

Add a new DB table pseudonymized_borrower_attributes

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 80 Marcel de Rooy 2020-06-19 07:30:31 UTC
Created attachment 106040 [details] [review]
Bug 24151: Add new column borrower_attribute_types.keep_for_pseudonymization

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 81 Marcel de Rooy 2020-06-19 07:30:37 UTC
Created attachment 106041 [details] [review]
Bug 24151: Adding keep_for_anonymized to the UI

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 82 Marcel de Rooy 2020-06-19 07:30:42 UTC
Created attachment 106042 [details] [review]
Bug 24151: Sync patron's attributes

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 83 Marcel de Rooy 2020-06-19 07:30:48 UTC
Created attachment 106043 [details] [review]
Bug 24151: DBIC changes

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 84 Marcel de Rooy 2020-06-19 07:30:53 UTC
Created attachment 106044 [details] [review]
Bug 24151: Add boolean flag for has_cardnumber

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 85 Marcel de Rooy 2020-06-19 07:30:58 UTC
Created attachment 106045 [details] [review]
Bug 24151: Fix location on return

The item's location where not passed to UpdateStat

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 86 Marcel de Rooy 2020-06-19 07:31:04 UTC
Created attachment 106046 [details] [review]
Bug 24151: Add items.homebranch to the list

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 87 Marcel de Rooy 2020-06-19 07:31:09 UTC
Created attachment 106047 [details] [review]
Bug 24151: DBIC changes

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 88 Marcel de Rooy 2020-06-19 07:31:14 UTC
Created attachment 106048 [details] [review]
Bug 24151: (QA follow-up) Correct license in two modules

Still listing the address..

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 89 Marcel de Rooy 2020-06-19 07:31:19 UTC
Created attachment 106049 [details] [review]
Bug 24151: (QA follow-up) Fix POD warnings from QA tools

POD is missing for get_hash
                POD is missing for new_from_statistic
                POD is missing for 'object_class'

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 90 Marcel de Rooy 2020-06-19 07:35:40 UTC
Looks good to me, some additional comments:

t/db_dependent/Koha/Pseudonymization.t
Please add test descriptions. This is a good habit. If we dont do it now, we will not.
# Subtest: Config does not exist
    ok 2
# Subtest: Koha::Anonymized::Transactions tests
    ok 3
    ok 4
    ok 5
    ok 6
    ok 7
    ok 8
    ok 9
    ok 10
    ok 11
    ok 12

$values->{branchcode} = $patron->branchcode; # FIXME Must be removed from the pref options, or FK removed (?)
Adding a FIXME !

my $attributes = Koha::Database->new->schema->resultset('PseudonymizedBorrowerAttribute')->search({transaction_id => $p->id });
So no Koha objects here but DBIx ?

$(htpasswd -bnBC 10 "" password | tr -d ':\n' | sed 's/$2y/$2a/');
Why do you change $2y to $2a ?
If I understand correctly, $2y signals use of a safer updated algorithm.

<key>$2a$10$PfdrEBdRcL2MZlEtKueyLegxI6zg735jD07GRnc1bt.N/ZYMvBAB2</key>
BAD choice for a new config variable! What is key? Where is it used for? Please add pseudonymize or something ?

In general: We are filling these tables with transactions?  Are we going to keep them indefinitely? What are the plans for cleanup etc.? This could be a large table in no time.
Comment 91 Marcel de Rooy 2020-06-19 07:36:52 UTC
(In reply to Marcel de Rooy from comment #90)
> In general: We are filling these tables with transactions?  Are we going to
> keep them indefinitely? What are the plans for cleanup etc.? This could be a
> large table in no time.

OK I see a follow-up for purging now !
Comment 92 Jonathan Druart 2020-06-23 09:49:33 UTC
Created attachment 106195 [details] [review]
Bug 24151: Add description for tests
Comment 93 Jonathan Druart 2020-06-23 09:50:56 UTC
(In reply to Marcel de Rooy from comment #90)
> Looks good to me, some additional comments:

Thanks Marcel.

> t/db_dependent/Koha/Pseudonymization.t
> Please add test descriptions. This is a good habit. If we dont do it now, we
> will not.
> # Subtest: Config does not exist
>     ok 2
> # Subtest: Koha::Anonymized::Transactions tests
>     ok 3
>     ok 4
>     ok 5
>     ok 6
>     ok 7
>     ok 8
>     ok 9
>     ok 10
>     ok 11
>     ok 12

Indeed, fixed.

> $values->{branchcode} = $patron->branchcode; # FIXME Must be removed from
> the pref options, or FK removed (?)
> Adding a FIXME !

I have to admit I cannot remember what is the meaning of this FIXME. It seems to make sense to record the patron's library (?)

> my $attributes =
> Koha::Database->new->schema->resultset('PseudonymizedBorrowerAttribute')-
> >search({transaction_id => $p->id });
> So no Koha objects here but DBIx ?

It's only used in test file. In pm file it's using create_related.
 97         $self->_result->create_related('pseudonymized_borrower_attributes', $attribute);

I don't think it is worth creating a new module only for that purpose.

> $(htpasswd -bnBC 10 "" password | tr -d ':\n' | sed 's/$2y/$2a/');
> Why do you change $2y to $2a ?
> If I understand correctly, $2y signals use of a safer updated algorithm.

Good question. I remember I first used some methods from Koha::AuthUtils, where we hardcode $2a. I guess it comes from there and so the replacement should be removed. I will investigate a bit further.

> <key>$2a$10$PfdrEBdRcL2MZlEtKueyLegxI6zg735jD07GRnc1bt.N/ZYMvBAB2</key>
> BAD choice for a new config variable! What is key? Where is it used for?
> Please add pseudonymize or something ?

We could, but the point of the generic wording was to make it use for something else, if needed. It could be better named "secret_key".
Comment 94 Marcel de Rooy 2020-06-24 12:44:10 UTC
(In reply to Jonathan Druart from comment #93)
> (In reply to Marcel de Rooy from comment #90)


> > my $attributes =
> > Koha::Database->new->schema->resultset('PseudonymizedBorrowerAttribute')-
> > >search({transaction_id => $p->id });
> > So no Koha objects here but DBIx ?
> 
> It's only used in test file. In pm file it's using create_related.
>  97        
> $self->_result->create_related('pseudonymized_borrower_attributes',
> $attribute);
> 
> I don't think it is worth creating a new module only for that purpose.

Agreed.

> > $(htpasswd -bnBC 10 "" password | tr -d ':\n' | sed 's/$2y/$2a/');
> > Why do you change $2y to $2a ?
> > If I understand correctly, $2y signals use of a safer updated algorithm.
> 
> Good question. I remember I first used some methods from Koha::AuthUtils,
> where we hardcode $2a. I guess it comes from there and so the replacement
> should be removed. I will investigate a bit further.

I would be inclined to remove the sed statement here.

> > <key>$2a$10$PfdrEBdRcL2MZlEtKueyLegxI6zg735jD07GRnc1bt.N/ZYMvBAB2</key>
> > BAD choice for a new config variable! What is key? Where is it used for?
> > Please add pseudonymize or something ?
> 
> We could, but the point of the generic wording was to make it use for
> something else, if needed. It could be better named "secret_key".

No blocker, but I prefer a more specific name. Using keys for several purposes may not always be so handy. 'key' is really too generic.
Comment 95 Jonathan Druart 2020-06-24 12:59:58 UTC
(In reply to Marcel de Rooy from comment #94)
> > > <key>$2a$10$PfdrEBdRcL2MZlEtKueyLegxI6zg735jD07GRnc1bt.N/ZYMvBAB2</key>
> > > BAD choice for a new config variable! What is key? Where is it used for?
> > > Please add pseudonymize or something ?
> > 
> > We could, but the point of the generic wording was to make it use for
> > something else, if needed. It could be better named "secret_key".
> 
> No blocker, but I prefer a more specific name. Using keys for several
> purposes may not always be so handy. 'key' is really too generic.

Tomas suggested me "encryption_key", would it work for you?
Comment 96 Julian Maurice 2020-06-24 13:32:50 UTC
(In reply to Jonathan Druart from comment #95)
> Tomas suggested me "encryption_key", would it work for you?

Another suggestion: since it seems to be a bcrypt settings string related to pseudonymization, why not "pseudonymization_bcrypt_settings" ? :)
Comment 97 Jonathan Druart 2020-06-24 13:43:22 UTC
(In reply to Julian Maurice from comment #96)
> (In reply to Jonathan Druart from comment #95)
> > Tomas suggested me "encryption_key", would it work for you?
> 
> Another suggestion: since it seems to be a bcrypt settings string related to
> pseudonymization, why not "pseudonymization_bcrypt_settings" ? :)

I think this setting could be reuse for other uses, and so the naming should not be too specific.
Comment 98 Julian Maurice 2020-06-24 13:47:06 UTC
(In reply to Jonathan Druart from comment #97)
> (In reply to Julian Maurice from comment #96)
> > (In reply to Jonathan Druart from comment #95)
> > > Tomas suggested me "encryption_key", would it work for you?
> > 
> > Another suggestion: since it seems to be a bcrypt settings string related to
> > pseudonymization, why not "pseudonymization_bcrypt_settings" ? :)
> 
> I think this setting could be reuse for other uses, and so the naming should
> not be too specific.

Can it be something else than a bcrypt settings string ? If not, I think we should have these words (bcrypt, settings) in the config variable name, it will probably easier to search "koha bcrypt settings" than "koha key"
Comment 99 Jonathan Druart 2020-06-24 14:45:28 UTC
Created attachment 106255 [details] [review]
Bug 24151: Rename the config key with 'bcrypt_settings'
Comment 100 Jonathan Druart 2020-06-24 14:47:08 UTC
I have renamed the config entry with "bcrypt_settings".

Marcel, the sed part needs to be there, otherwise I get "bad bcrypt settings".
POD says "It must begin with "$2", optional "a", "$", two digits, "$", and 22 base 64 digits", then "The function will die if SETTINGS does not have this format."
Comment 101 David Cook 2020-06-25 06:59:24 UTC
(In reply to Jonathan Druart from comment #100)
> I have renamed the config entry with "bcrypt_settings".
> 

Awesome. I was scratching my head wondering why it was called "key". I think "bcrypt_settings" is much better, although within the context of koha-conf.xml that is a bit too generic I think. It might be better to have it be something like "anonymizer_bcrypt_settings".

But that could just be me splitting hairs. My French teacher used to say something like "Ne coupons pas les cheveux en quatre, David."
Comment 102 David Cook 2020-06-25 07:00:08 UTC
Oh I just want to say thanks for this. Very useful functionality!
Comment 103 Jonathan Druart 2020-06-25 07:56:48 UTC
As I said previously it could be reused for other features. I don't think it should be tight to the "anonymization" feature.
Comment 104 David Cook 2020-06-26 02:40:22 UTC
(In reply to Jonathan Druart from comment #103)
> As I said previously it could be reused for other features. I don't think it
> should be tight to the "anonymization" feature.

I suppose. If other features wanted to deviate, then the onus could be on them to create a new setting. Yeah, ok.
Comment 105 Marcel de Rooy 2020-06-26 06:50:06 UTC
(In reply to Jonathan Druart from comment #100)
> Marcel, the sed part needs to be there, otherwise I get "bad bcrypt
> settings".
> POD says "It must begin with "$2", optional "a", "$", two digits, "$", and
> 22 base 64 digits", then "The function will die if SETTINGS does not have
> this format."

OK
Seems that for long nobody cared about this module. This is a ticket for supporting $2y from 2014:
https://rt.cpan.org/Public/Bug/Display.html?id=93688
Comment 106 Jonathan Druart 2020-07-20 15:48:16 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 107 Jonathan Druart 2020-07-21 08:03:55 UTC
Created attachment 107127 [details] [review]
Bug 24151: Use fully qualified names for UpdateStats

t/db_dependent/Koha/Account/Line.t .. 8/12     # Looks like you planned 16 tests but ran 3.
 #   Failed test 'void() tests'
 #   at t/db_dependent/Koha/Account/Line.t line 723.
Undefined subroutine &Koha::Account::UpdateStats called at /kohadevbox/koha/Koha/Account.pm line 289.

It's certainly caused by a circ dep from
  commit 5ce968e0e571f555261c02a9e1d27447611cdb89
  Bug 24151: Copy info to the pseudonymized table when a transaction is done

See also bug 17600 for more context.
Comment 108 Jonathan Druart 2020-07-21 10:24:11 UTC
Follow-up pushed to master for 20.11
Comment 109 Jonathan Druart 2020-07-23 10:15:13 UTC
Created attachment 107225 [details] [review]
Bug 24151: (follow-up) Rename the config key with 'bcrypt_settings'
Comment 110 Jonathan Druart 2020-07-23 10:16:33 UTC
(In reply to Jonathan Druart from comment #109)
> Created attachment 107225 [details] [review] [review]
> Bug 24151: (follow-up) Rename the config key with 'bcrypt_settings'

I missed half of the job in the first patch!

Pushed to master for 20.11.
Comment 111 Lucas Gass 2020-07-25 18:09:06 UTC
Not backporting enhancement to 20.05.x series.
Comment 112 Arthur Suzuki 2020-08-28 08:25:19 UTC
*** Bug 26315 has been marked as a duplicate of this bug. ***
Comment 113 Aude Charillon 2023-03-10 10:42:10 UTC
Re-opening just for documentation.
Comment 114 Aude Charillon 2023-04-25 16:20:15 UTC
New system preferences and information about Bcrypt settings added to the manual.