[Marijn0]

Fix passwords not being sent to the backend when changed without a keydown event.

This fixes an issue where passwords entered through a password manager, browser autofill, or right-click paste could appear to be accepted in the UI but were not actually saved after clicking Apply.

Those input methods can trigger a change event without triggering keydown. The frontend previously required both events before including password fields in the config payload, so the Apply request could be sent without the updated passwords.

Issue: #3345

[MichaIng]

I do not understand either why the keydown event was an additional condition.

Actually, if we pre-fill the fields with *****, we do not even need to track the "change" event, but take the value as only condition. The latter implies the first, hence the first is redundant, if I am not missing something.

[Marijn0]

Actually, if we pre-fill the fields with *****, we do not even need to track the "change" event, but take the value as only condition. The latter implies the first, hence the first is redundant, if I am not missing something.

Good point, so we can just use something like:

if ($('#adminPasswordEntry').val() !== '*****') {

which is much cleaner.

I am not sure why all those conditions were needed in the past. Maybe they were there to avoid submitting empty password fields. But now that the admin and surveillance user passwords are required, those extra conditions can be removed AFAICS.

[MichaIng]

Even a change + keydown event does not assure the field is not empty. I mean making it empty requires a change and a keydown (assuming no password manager can enter an empty password).

Maybe the conditions were incrementally appended ... yeah, see here: 818cd28

So the keydown condition was explicitly added to prevent saving autofilled password, not considering password managers, but browser autocompletion features which sometimes touch all input fields without actually changing the content.

The failsafe "addition" of checking ***** however made the other unreliable conditions obsolete.

This also explains why there was/is another ***** check in the backend. I guess this was added when it was not present in the frontend yet. Now that it is, the backend does not need to filter this anymore.

[Marijn0]

Good find!

This also explains why there was/is another ***** check in the backend. I guess this was added when it was not present in the frontend yet. Now that it is, the backend does not need to filter this anymore.

If there are still any such checks in the backend, could you please point me to them?

[MichaIng]

Great when a bug can be fixed by removing and simplifying code 👍🙂

[MichaIng]

If there are still any such checks in the backend, could you please point me to them?

It is not there anymore. I just remember this popped up in a PR, maybe got removed, or maybe someone wanted to add it in the backend. I just remember that I verified the frontend does the ***** filtering already, and that hence it wouldn't be needed in the backend.