reset advanced-config-settings on config-failures

[link2xt]

I have changed certificate checks configuration to Strict for my testrun account. After connection, logs show cert_strict for entered_account_settings but cert_automatic for used_account_settings afterwards.

Release 0.950.0

[r10s]

maybe this is a core issue, android does not touch the configured/used settings at all afaik.

does it work for desktop?

[link2xt]

It works with repl.

I did

> set addr ...
> set mail_pw ...
> configure

Configuration succeeds. At this point configured_{imap,smtp}_certificate_checks are 0.

Then I do

> set imap_certificate_checks 1
> set smtp_certificate_checks 1
> configure

Reconfiguration succeeds:

$ sqlite3 1.db 'select * from config where keyname like "%_checks"'
12|configured_imap_certificate_checks|1
17|configured_smtp_certificate_checks|1
23|imap_certificate_checks|1
24|smtp_certificate_checks|1

[r10s]

ahh, it's about changing the settings :) i will check this out!

[r10s]

hm, i just tried it out with a testrun account and the current rust/android master, and the entered_account_settings are set as expected (according to the log at settings/advanced/log)

what i did:

• create a new account, cert=automatic
• after login, go to settings/profile/password+account/advanced and set cert-checks to "strict"
• hit ok, configuration is re-done - and cert-checks are as expected.

for both, initial-login and later-reconfigure, the following code is run: https://github.com/deltachat/deltachat-android/blob/master/src/org/thoughtcrime/securesms/RegistrationActivity.java#L442 - no idea what can go wrong there.

[link2xt]

Did it again on Android, this time it worked.

Probably connection failed and configuration process didn't succeed. Yet settings show selected settings, not configured one by default, so configured settings were still set to "automatic" and entered settings set to strict, so dialog showed strict. This is misleading, user may think that strict settings were successfully configured.

I think the fields should be prefilled with configured_ values, not entered ones. This is what Desktop does.

[r10s]

Yet settings show selected settings, not configured one by default

this is by design.

the idea is that the "login parameters" are detected and stored and used internally. on re-configuring, the same parameters are used. this ensures that, if some server-settings do change, at least a re-login works for the user - otherwise the user has to change/delete parameters that were never entered by the user and where the user maybe just have no idea that they exist.

so, putting using the configured-parameters as the entered parameters have the potential to bring the whole settings to a state the user cannot handle anymore.

also, there is the idea that the core uses several configured parameters as useful - often autoconfig reports etc. starttls on port 123 and ssl on port 456 - some wlans may allow the one but not the other, so it would be nice if the core could switch as needed. also, this cannot be done when promoting configured paramters to the user.

if desktop does it differently, desktop is wrong here. in general, the configured_ parameters are no parameters that should be needed by the ui. in fact, for a long time, they were not even accessible to the ui.

This is misleading, user may think that strict settings were successfully configured.

i think the problem is that the "certificate checks" might be not treated as "login parameters" - and there is no need for the configured_-variant. if the core stops using that, things should be fine.

and if they are "login parameters" - before the "unstrict" setting is used, there should have been a warning that the actual configuration has failed. maybe we can be more clear that the previous "login parameters" are used in this case.

EDIT: clearify last points

[r10s]

Probably connection failed and configuration process didn't succeed.

so, there should be an error displayed then. however, of course, this is probably hard to say now :)

[link2xt]

Makes sense. If we change Automatic certificate checks behaviour to TOFU later, relogin should work if certificate becomes invalid.

But how do I see that strict certificate checks are used? Configured settings are non-strict and UI shows "strict". Maybe save the settings only if configuration succeeds? That would require another prefix or some other API modification.

[r10s]

But how do I see that strict certificate checks are used? Configured settings are non-strict and UI shows "strict". Maybe save the settings only if configuration succeeds?

saving only on success won't work as the core needs it.

but the ui can reset the options on reconfiguration on failure to the state before changes were done.

there is an older discussion about that at https://github.com/deltachat/deltachat-core/issues/246 - the outcome was to clearify dc_configure() - which, in fact, reads "UI-implementors should keep this in mind - eg. if the UI wants to prefill a configure-edit-dialog with these parameters, the UI should reset them if the user cancels the dialog after a configure-attempts has failed. Otherwise the parameters may not reflect the current configuation."

seems as if this has not yet been done on android.

[link2xt]

What if the client crashes during reconfiguration? Both "last successful entered" and "configured" settings need to be atomically updated at the same time. That is why I said

That would require another prefix or some other API modification.

above.

[link2xt]

@r10s Your solution will work most of the time, if it is not hard to implement maybe go for it now.

To make things more robust, we can implement the following API in the core:

1. Atomically remove all prefixed "new" settings (possibly remaining from previous crashes) and fill "new" settings in the database.
2. Client calls new API dc_atomic_configure()
3. It reads new_ configuration and tries to apply it. If it succeeds, it replaces unprefixed configuration with new_ and updates configured_ atomically.
4. If it fails, it removes new_ configuration.

[r10s]

just did a thing :) https://github.com/deltachat/deltachat-core-rust/pull/810