Bug 384051 - No reset of hcbi account pin possible
Summary: No reset of hcbi account pin possible
Status: RESOLVED UPSTREAM
Alias: None
Product: kmymoney
Classification: Applications
Component: onlinebanking (show other bugs)
Version: 4.8.0
Platform: Other Microsoft Windows
: NOR normal
Target Milestone: ---
Assignee: KMyMoney Devel Mailing List
URL:
Keywords:
Depends on:
Blocks: 426400
  Show dependency treegraph
 
Reported: 2017-08-26 12:42 UTC by Ralf Habacker
Modified: 2021-09-11 12:18 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ralf Habacker 2017-08-26 12:42:17 UTC
I tried to configure hbci for a newly created german volksbank account. 
After entering the account data kmymoney connected the hbci server and got the message that the pin needs to be changed before accessing this account. 
Then I openend a browser and changed the pin at the online banking web frontend. After returning to kmymoney I tried to finish the account registration but I did not found any button to reenter the changed pin. Instead it tries to continue with the old pin which results into a locked account. I need to cancel hbci account registration and shutdown and start kmmymoney again and have to add the hbci account again with the changed pin.

How to reproduce ?
See above with a newly created hbci account

What is expected ?
If there any problems with accessing the account on the account registration process it should be possible to save the recent setting and to finish the hbci account registration process later point there is no way to save the recent settings and to return back later. At this point it should not use the cached pin.
Comment 1 Stefan Vater 2019-10-02 14:36:12 UTC
Isn't this more a problem of aqbanking, than of kmymoney? I thought all the HBCI setup is done in the frontends of aqbanking...
Comment 2 AndreJ 2019-10-02 16:24:22 UTC
KMyMoney keeps the PIN in its own memory, and sends it by aqbanking modules to the bank. If the PIN was rejected, KMyMoney should destroy it, and ask the user again.

Or make a checkbox [x] keep PIN in memory in the PIN input box
That is the way some other banking software does it.
Comment 3 Thomas Baumgart 2019-10-03 07:06:41 UTC
Hm, that might be the impression to the user, KMyMoney just tells Gwenhywfar after a 60 second timeout to clear the cached password using

  GWEN_Gui_SetPasswordStatus(NULL, NULL, GWEN_Gui_PasswordStatus_Remove, 0);

The timer is started at the beginning and end of 

  int KBankingExt::executeQueue(AB_IMEXPORTER_CONTEXT *ctx)

but keeping the PIN is the sole activity of Gwenhywfar. Since the AqBanking configuration dialog is called without the usage of the job queue (it's a simple dialog) I am with Stefan and his comment #1.
Comment 4 Ralf Habacker 2019-10-06 09:54:13 UTC
(In reply to Thomas Baumgart from comment #3)
> Hm, that might be the impression to the user, KMyMoney just tells Gwenhywfar
> after a 60 second timeout to clear the cached password using
> 
>   GWEN_Gui_SetPasswordStatus(NULL, NULL, GWEN_Gui_PasswordStatus_Remove, 0);
For the record: this was added to kmymoney with version 4.6.

The remaining problem in kmymoney is that a user do not know that he needs to wait 60 seconds until he can retry. 

Would it be possible to detect the "incorrect password case" in kmymoney ?

If so, removing the password immediately from the password cache will be the solution, so that the user can enter a new one.

If not, I suggest to add a hint to the banking log that the user needs to wait 60 seconds before retrying.
Comment 5 Ralf Habacker 2019-10-06 13:34:54 UTC
(In reply to Ralf Habacker from comment #4)
> If not, I suggest to add a hint to the banking log that the user needs to wait 60 seconds before
> retrying.
Just saw that in the dialog where data is sent online to the tax office the app "ElsterFormular" shows a check box named "Save password until app is running", which looks like another alternative to deal with this issue. If the checkbox is unchecked, the password need to be entered each time.
Comment 6 Jack 2019-10-09 18:25:59 UTC
(In reply to Ralf Habacker from comment #5)
Not related to the bug itself, but that translation should be "Save password while app is running."  (You could also say "Save password until app stops running" but that is more awkward.)
Comment 7 Moritz Bunkus 2021-09-03 18:51:09 UTC
I just came over here to finally report this myself after being annoyed by it for many, many years. I'm using a rather long passphrase for aqbanking's certificate, and mistyping it is common occurrence. kmymoney not recognizing this and me having to restart kmymoney is incentivizing using shorter, easier to type and hence less secure passwords. Instead, a program dealing with such an important topic should make it easy to do the right (the secure) thing.

Ideally kmymoney should figure out that updating accounts through aqbanking failed due to a wrong password and un-cache the password.

I can imagine, though, that the API doesn't expose this kind of detail, only whether or not the update succeeded. If this is indeed the case, I'd very much appreciate if kmymoney always un-cached the passphrase when aqbanking fails to update. If that seems too hostile to the user[1], making this behavior optional via the settings would be totally fine.

[1] Of course aqbanking might fail for reasons other than wrong passphrases — network outages, DNS not working, the bank's servers not being reachable etc.
Comment 8 Thomas Baumgart 2021-09-05 13:18:29 UTC
As already pointed out in comment #3 the cache is way into the internals of AqBanking and Gwenhywfar and not directly (if at all) accessible to KMyMoney (other than clearing it). From what I know and remember, the return code of a job somehow does not identify a failing authentication. Therefore, it is kind of hard for KMyMoney to decide if the cache needs to be cleared right away or not. I am further investigating this by plodding through the AqBanking/Gwenhywfar code base and magics.
Comment 9 Moritz Bunkus 2021-09-05 13:50:31 UTC
Thanks for the insight. Like I said, I totally understand if this particular failure type isn't observable to kmymoney.

If I understand you & comment #3 correctly, kmymoney does have enough control to tell the rest of the stack to forget any cached passphrase. So it would be in kmymoney's power to give the user some control over this situation. I suggest to add one of the following:

1. An option in the settings to always clear cached passphrases if fetching the account failed, off by default.
2. A menu entry in the "Tools" menu called something like "Forget cached passphrases", which the user could use.
3. If updating an account fails, show a message box asking explaining the problem ("This might be due to a wrong passphrase or PIN.") & offering to clear the cached passphrase/PIN & try again; ideally with a checkbox to never show this type of question again.
4. Make passphrase/PIN caching completely optional via the settings.

I agree that this is fundamentally a thing inside aqbanking/its libraries, but from the PoV of a user, I'm not interacting with aqbanking at that moment. I'm using kmymoney. To the user it is totally unclear that

1. password caching is happening in the background,
2. restarting the application causes cached passwords to be forgotten,
3. waiting 60 seconds causes cached passwords to be forgotten (I found this out by reading this bug),
4. it isn't actually kmymoney that's caching the passphrase.

All of that (save for 1., I guess) will cause the user to spend a lot of time going through all options, all menus, dialogs etc., in order to find a way to recover. They'll most likely end up with the "restart application" way, 'cause "have you tried turning it off & on again" is so ingrained in general user troubleshooting. Doesn't make it good design, though.

Of course the ideal solution is to being able to auto-detect when clearing is necessary. My proposals above are only useful if that isn't possible due to API limitations.
Comment 10 Thomas Baumgart 2021-09-05 14:37:19 UTC
Full ACK! Although, I am currently still investigating/following the API route (the right thing to gain control over this).
Comment 11 Thomas Baumgart 2021-09-06 07:07:36 UTC
I discussed this problem with the author of AqBanking and he agreed to a change I proposed and already added it to the repo: https://www.aquamaniac.de/rdm/projects/gwenhywfar/repository/revisions/f0c8a65ada0b8387199b028a117df42a1f9ab468. Now we have to wait until this becomes available as part of a tar-ball, unless someone wants to try to build from source.
Comment 12 Ralf Habacker 2021-09-06 07:23:38 UTC
(In reply to Thomas Baumgart from comment #11)
> I discussed this problem with the author of AqBanking and he agreed to a
> change I proposed and already added it to the repo:
> https://www.aquamaniac.de/rdm/projects/gwenhywfar/repository/revisions/
> f0c8a65ada0b8387199b028a117df42a1f9ab468. Now we have to wait until this
> becomes available as part of a tar-ball, unless someone wants to try to
> build from source.
To have this in the next gwenhyfar version, the patch must also be applied to the `maint` git branch. Furthermore, the `maint` branch is also used to create the Windows snapshots for kmymoney on https://kmymoney.org/snapshots.php.
Comment 13 Thomas Baumgart 2021-09-10 18:43:11 UTC
@Ralf: AqBanking 6.3.1 and Gwenhywfar 5.7.1 are the new stable releases and maint branch points to them.
Comment 14 Moritz Bunkus 2021-09-11 12:18:46 UTC
Thanks again for your work & following up on this, Thomas. Much appreciated.