[Koha-bugs] [Bug 24151] Add a pseudonymization process for patrons and transactions

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24151

--- Comment #93 from Jonathan Druart <jonathan.druart at bugs.koha-community.org> ---
(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".

-- 
You are receiving this mail because:
You are watching all bug changes.