Synchronization of transports #7485
Conversation
link2xt
force-pushed
the
link2xt/transport-sync
branch
13 times, most recently
from
1e4a729 to
3b2f96a
Compare
link2xt
force-pushed
the
link2xt/transport-sync
branch
6 times, most recently
from
7baac2d to
7c28f2c
Compare
link2xt
changed the title
link2xt
force-pushed
the
link2xt/transport-sync
branch
from
7c28f2c to
fb9a033
Compare
src/sql/migrations.rs
Outdated
| disable_server_delete = true; | ||
|
|
||
| // Don't disable server delete if it was on by default (Nauta): | ||
| if let Some(provider) = context.get_configured_provider().await? |
There was a problem hiding this comment.
This old migration relying on get_configured_provider() is modified. This migration was introduced in 2021 and relies on high-level functions: ebccdbb
If user migrates from such old version which is very unlikely, then some setting will not be updated, not a big problem.
link2xt
force-pushed
the
link2xt/transport-sync-prepare
branch
from
759f4ba to
e6a2d94
Compare
link2xt
force-pushed
the
link2xt/transport-sync
branch
from
fb9a033 to
aa10a3d
Compare
link2xt
force-pushed
the
link2xt/transport-sync-prepare
branch
from
e6a2d94 to
4fa2153
Compare
link2xt
force-pushed
the
link2xt/transport-sync
branch
from
aa10a3d to
7ec3e47
Compare
|
|
||
| /// One or more transports has changed. | ||
| /// | ||
| /// Transport list should be refreshed. |
There was a problem hiding this comment.
should also document that it is only emitted on receiving sync messages, not when it changes on current device.
Though I don't know why you should make a difference between the source of the event, I would just emit it in all cases. Then UI just has to listen to one event to update the list and does not need extra code to update the list after it's own changes. Or am I missing something important here?
| // Receiving encrypted message from self updates primary transport. | ||
| context | ||
| .sql | ||
| .set_raw_config("configured_addr", Some(&mime_parser.from.addr)) | ||
| .await?; |
There was a problem hiding this comment.
do I understand it correctly that this handles sync of changing the primary transport? like only primary transport sends sync messages, so if we receive sync messages from a different address then we can assume that this address is the new primary transport?
There was a problem hiding this comment.
I think so. But shouldn't we additionally check here that from.addr is in the transports table? (If i got it right, checking that add_timestamp > remove_timestamp isn't necessary)
I don't remember where exactly, but it was discussed that for the UI code it's easier to do everything on events rather then to handle also UI-triggered config changes. But if this event is only for tests, that doesn't matter probably. |
| /// Removes the transport with the specified email address | ||
| /// (i.e. [EnteredLoginParam::addr]). | ||
| pub async fn delete_transport(&self, addr: &str) -> Result<()> { | ||
| let now = time(); |
There was a problem hiding this comment.
Shouldn't this be max(time(), add_timestamp) so that the deletion "wins" on all devices?
| // Receiving encrypted message from self updates primary transport. | ||
| context | ||
| .sql | ||
| .set_raw_config("configured_addr", Some(&mime_parser.from.addr)) | ||
| .await?; |
There was a problem hiding this comment.
I think so. But shouldn't we additionally check here that from.addr is in the transports table? (If i got it right, checking that add_timestamp > remove_timestamp isn't necessary)
| VALUES (?, ?, ?, ?) | ||
| ON CONFLICT (addr) | ||
| DO UPDATE SET entered_param=excluded.entered_param, | ||
| configured_param=excluded.configured_param", |
There was a problem hiding this comment.
Why not updating add_timestamp?
link2xt
force-pushed
the
link2xt/transport-sync
branch
from
7ec3e47 to
b5fea77
Compare
The last commits adds actual transport synchronization, commits before split into PR #7530 are refactorings and preparation.
New
TransportsModifiedevent is meant for tests, it is currently emitted when sync message is received and not when the user changes the transports manually as it is already known to the UI.Transport with ID 1 is never synced except for deletion to avoid ever sending the credentials of the first transport to other email servers. The only downside is that if the password is changed, the user will have to change it on all devices, but this avoids all the discussion of reducing security compared to the current state. The first transport with multi-transport is handled he same way as before without multi-transport.