Bug 36449 - Patron->store falls back to stored password when modifying patron
Summary: Patron->store falls back to stored password when modifying patron
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 21087
Blocks:
  Show dependency treegraph
 
Reported: 2024-03-28 07:28 UTC by Marcel de Rooy
Modified: 2024-03-28 09:27 UTC (History)
2 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 Marcel de Rooy 2024-03-28 07:28:01 UTC
At some point we added this in patron->store (ModMember-labeled part):

# Password must be updated using $self->set_password
                $self->password($self_from_storage->password);

I stumbled over that requirement in a custom script updating specific patron accounts.
But we do allow to set the password in the AddMember part. Which feels inconsistent at the least..

But why do we silently(!) ignore a new password here? A warn might have been handy too ;)
Couldnt we just add a call to set_password in store and perhaps add a param to not explicitly save it (when coming from store) or something like that?
Comment 1 Jonathan Druart 2024-03-28 09:19:56 UTC
Coming from bug 21087. I don't have time right now to deep dive into this but maybe the answer is there?
Comment 2 Marcel de Rooy 2024-03-28 09:27:00 UTC
(In reply to Jonathan Druart from comment #1)
> Coming from bug 21087. I don't have time right now to deep dive into this
> but maybe the answer is there?

Ah thanks. Yes thats the one:

From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Wed, 25 Jul 2018 10:38:48 -0300
Subject: [PATCH] Bug 21087: Do not store the password in ->store for existing
 patrons

When ->store is called on an existing patron we must not save the
password, but keep the one in DB instead.
There is a dedicated method (update_password) to call when the password
need to be updated

Signed-off-by: John Doe <you@example.com>


Why did John Doe sign that one off? LOL

It would be nice to know the reasons why we should not do it. We can always call the dedicated method from store (to perform needed checks etc)..
There was no discussion about this in that report btw.