WIP: PGP contacts #6796
WIP: PGP contacts #6796
Conversation
link2xt
force-pushed
the
link2xt/pgp-contacts
branch
2 times, most recently
from
7acd920 to
280e5dd
Compare
link2xt
force-pushed
the
link2xt/pgp-contacts
branch
from
7e5dab4 to
479879a
Compare
| /// This first creates a contact, but does not import the key, | ||
| /// so may create a PGP-contact with a fingerprint | ||
| /// but without the key. | ||
| pub async fn get_pgp_chat(&self, other: &TestContext) -> Chat { |
There was a problem hiding this comment.
Since PGP chats are the more common type of chat now, I think it would make sense to call this function get_chat() and the other one get_email_chat() or get_unencrypted_chat() or similar. But it's not that important, e.g. if this would mean that a lot of test have to be changed.
link2xt
force-pushed
the
link2xt/pgp-contacts
branch
2 times, most recently
from
ebe2a09 to
cac8d20
Compare
|
a way to know if a chat is an encrypted chat is missing, currently there is only "isPgpContact" that can be used to mark contacts/1:1 chat but for unencrypted groups/threads of classic email non-pgp contacts there is no API to recognize such chats and also put the "classic email" marker as for classic email contacts |
link2xt
force-pushed
the
link2xt/pgp-contacts
branch
2 times, most recently
from
625f8f4 to
c87c241
Compare
| @@ -187,6 +191,7 @@ impl BasicChat { | |||
| id: chat_id, | |||
| name: chat.name.clone(), | |||
| is_protected: chat.is_protected(), | |||
| is_encrypted: chat.is_encrypted(context).await?, | |||
There was a problem hiding this comment.
This adds up to 2 additional database calls to loading BasicChat, though I can't think of a good way to avoid it, so maybe it's fine. Or maybe it should only be added to FullChat, idk.
| ) -> Result<(ContactId, Modifier)> { | ||
| Self::add_or_lookup_ex(context, name, addr, "", origin).await | ||
| } | ||
|
|
||
| /// Lookup a contact and create it if it does not exist yet. | ||
| /// The contact is identified by the email-address, a name and an "origin" can be given. |
There was a problem hiding this comment.
The comment here should explain how fingerprint & addr parameters relate; the way I understand the code:
/// If `fingerprint` is passed, a PGP-contact with this fingerprint added / looked up.
/// Otherwise, an email-contact with `addr` is added / looked up.
/// A name and an "origin" can be given.
| if !fingerprint.is_empty() { | ||
| let fingerprint_self = load_self_public_key(context).await?.dc_fingerprint().hex(); | ||
| if fingerprint == fingerprint_self { | ||
| return Ok((ContactId::SELF, sth_modified)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Loading the self fingerprint everytime a contact is looked up, just to check whether it's ContactId::SELF, seems unnecessarily slow to me.
I wonder if we can simplify & speed up things by not using the kepairs table anymore, and instead storing the private and public key in configs. This would mean that it's not possible to have multiple self keys anymore, but I'm not sure there is anyone who is actually making use of the fact that DC theoretically supports using multiple self keys. It would also mean that we have to store the key as base64, not sure about the consequences there. An alternative would be to store the self fingerprint in a OnceCell, since we don't allow changing the primary key anyway.
But we can open an issue for this, and do it in a follow-up PR.
| to_ids: &[ContactId], | ||
| past_ids: &[ContactId], | ||
| to_ids: &[Option<ContactId>], | ||
| past_ids: &[Option<ContactId>], |
There was a problem hiding this comment.
Would be good to put a comment what a None element means.
link2xt
force-pushed
the
link2xt/pgp-contacts
branch
from
fb034c4 to
ea48d78
Compare
link2xt
force-pushed
the
link2xt/pgp-contacts
branch
2 times, most recently
from
918277f to
9cf065a
Compare
link2xt
force-pushed
the
link2xt/pgp-contacts
branch
2 times, most recently
from
1cff5d4 to
907628e
Compare
link2xt
force-pushed
the
link2xt/pgp-contacts
branch
from
c156eca to
ec7c421
Compare
Still missing, not all of this should be fixed before merging but at least CI should pass:
Chat-Group-Member-Fprheader.Chat-Group-Member-Fprformat should be fixed before release.test_prefer_encrypt_mutual_if_encryptedhas TODOs for assigning to chat by issuer fingerprint. This should also work if the key is attached and no Autocrypt header is sent. Signed-only messages should probably go to PGP-chat without a padlock. Fixing this is out of scope of this PR. If Thunderbird is configured to send Autocrypt header, chatting should work, this is already tested.