mihaifm / HIBPOfflineCheck

Keepass plugin that performs offline and online checks against HaveIBeenPwned passwords
Other
317 stars 17 forks source link

Shows warning message during favicon download #45

Closed strarsis closed 1 year ago

strarsis commented 2 years ago

For automatically downloading and assigning favicons from the URL saved in the password entries, I use the KeePass Yet Another Favicon Downloader plugin. When that favicon plugin finished, I get the HIBPOfflineCheck warning messages, as I would if had saved the entries manually. I suspect a hook is used by HIBPOfflineCheck warnings for entries being modified. Can this be skipped when an automated tool like the favicon plugin modifies the entries?

mihaifm commented 2 years ago

I can look into it but in the mean time you can deactivate the warning from the plugin options. I suppose it’s a bit tedious since you’d have to toggle it every time you dowload favicons.

cristianst85 commented 1 year ago

There are two aspects to this.

1) One could argue that the Display warning message after editing insecure passwords does too much (i.e., it also shows the warning when you edit any other field beyond the password field, which is not what the description implies. Otherwise, it should have been Display warning message after editing entries with insecure passwords).

2) If we change the behavior only to show the warning when the password field is edited, it will also solve the issue described by @strarsis.

The tricky part is detecting whether the password field is changed when an entry is edited. I think this might be possible by looking at the entry's history and taking the previous version and then comparing the password field with the current one.

@mihaifm, any thoughts on this?

cristianst85 commented 1 year ago

The only limiting scenario will be when Limit number of history items per entry (File -> Database Settings...) is set to 0 (i.e., the entry will not have any history), but in that case, we could show the warning regardless.

mihaifm commented 1 year ago

I agree with this in principle, we should show the warning only after the password changes, not the entire entry. I'll take a look at this history approach, it looks promising.

mihaifm commented 1 year ago

This is fixed now, thanks @cristianst85 for the solution, worked perfectly.