This value is not used for any logic
except the disappearing messages timer rollback prevention.
It is unlikely that non-chat clients
participate in chats with disappearing messages,
and the case was not tested anyway.
Adds following stock strings:
- You were removed by %1$s.
- You were added by %1$s.
and implicit equivalents:
- You were removed.
- You were added.
Closes: #8419
Signed-off-by: Jagoda Ślązak <jslazak@jslazak.com>
this PR adds the "message pinned state" to the "reaction state" that is
already sent together with resent channel messages since #8496.
this change is done in the first commit, and in fact only changes few
lines (one can consider to rename "broadcast_reactions.rs" and related
stuff to "broadcast_state.rs" in another PR: i did not to that in this
PR to keep diff and review simple).
moreover, in the second commit ,the "selection of messages to resent" is
changed to keep an additional focus on the pinned messages, so that the
most recent pinned messages are resent as well.
successor of #8496 and #8546
Adds `assert_warn`, `assert_error` and `assert_warns_or_errors`
methods to `TestContext`, that let us assert that
a certain warning or error is logged during the test.
Also asserts test logs should not contain any other
errors or warnings.
Adjusts tests accordingly.
Signed-off-by: Jagoda Ślązak <jslazak@jslazak.com>
this PR adds support for reactions in broadcast channels.
> the idea of broadcast reactions is that they are sent as usual from
subscribers to owner. after some time, the owner broadcasts them to all
subscribers, who only get to see reaction+count, not who-reacted-what
the PR is quite large, but a good share are tests and otherwise many
things are straight forward.
review should be done by-file, not by-commit. to make review easier,
here is a high-level overview:
first a change in the existing internal `Reactions` object was required.
before this PR, `Reactions` had a "contact to reaction map" only, and
the "frequencies map", that are actually mainly needed for UI, were
calculated as needed. with this PR, the "frequencies map" is the field
that always exist, the "contact map" is only available on top of that.
moreover, this PR shifts that part to the core, it was unfortunately in
the bindings before.
with that preparation things done as follows:
1. reactions from broadcast channel subscriber (`Chattype::InBroadcast`)
to broadcast channel owner (`Chattype::OutBroadcast`) are sent as usual,
only change for that step was to allow sending them at all
2. the owner receives reactions and saves them to the existing
`reactions` table as usual. additionally, the changed message is
remembered in `reactions_need_broadcast` table
3. in the IMAP loop, when ~10 minutes have passed, and
`reactions_need_broadcast` contains entries, a single, hidden message
with accumulated reactions is sent. this message may contain reactions
to different messages. for each message, all known reactions are sent as
reaction+count.
4. subscriber receive that message and save the accumulated reactions in
`reactions_broadcasted`
6. `get_message_reactions` is adapted so that `frequencies` are set
independently of who-reacted-what (the old and only field).
who-reacted-what is called `by_contact` now. it is always set for
compatibility reasons, however, it is not exhaustive for subscribers.
in general, UI should work with frequencies, the API itself, however,
has not changed.
other tweaks:
- outgoing channels are muted on creation, and UI shall allow to
unmute/mute them as all other chats. reason is that reactions are
notified, but in many cases not of large interest. this is also what
telegram is doing
- to have an intermediate feedback when reacting, the local state should
include ones own reaction, even if it is not yet broadcasted. for that,
we modify `reactions_broadcasted` using `modify_frequencies()` as needed
when sending an reaction. there are still some situations where the
update may not include ones own reaction, in this case it is added
lately by `refine_frequencies()`, so that `get_message_reactions()`
always contain SELF.
(in a first implementation, we always increased SELF reaction in
refine_frequencies(), however, that was worse and led to SELF counted
twice once the owner sent broadcast)
<details>
<summary>wire format</summary>
wire format is a JSON in the `Broadcast-Reactions:` header.
additionally, `Content-Disposition: reaction` is set to not show the
hidden message on existing devices.
using a header also allows us to broadcast reactions with resent channel
messages (on joining) later.
```
{
"messages": [
{
"id": "12345678",
"reactions": [
{ "emoji": "👍", "count": 4 },
{ "emoji": "🎉", "count": 2 }
]
},
{
"id": "23456789",
"reactions": []
}
]
}
```
for `id`, the wire format needs to use `rfc724_mid` as `msg_id` are
local only.
</details>
### known issues
- if the channel owner uses multiple devices, broadcasted reaction
updates are sent from each device. the updates are not that big, so that
is probably not a big deal. if it turns out that this is an issue, we
can think about fixes in another PR. might be done by restarting our
10-minute-wait once we see an update from another device
- we cannot set contact_id for DC_EVENT_REACTIONS_CHANGED - but i doubt
it was ever used
### for another pr
- ~~add `Broadcast-Reactions:` header also for resent channel messages,
so that new subscriber do not only get the latest messages, but also
their reactions. for that, the `Broadcast-Reactions:` header can go to
the corresponding message, no need to send extra messages. we would need
to change the sending part to send all reactions for a given message. on
receiving part, we need to make sure, `receive_broadcast_reactions()` is
called when the message actually exist.~~
EDIT: subsequent PR for resending broadcast reactions at
https://github.com/chatmail/core/pull/8496
- add api to allow only a subset of reactions, fiter incoming reactions
before broadcasting
### misc.
ui pr: https://github.com/deltachat/deltachat-ios/pull/3225 and
https://github.com/deltachat/deltachat-android/pull/4560 , which both
were tested successfully with this core PR already. desktop is meant to
be done once this is merged
---------
Co-authored-by: l <link2xt@testrun.org>
Unifies internal naming of single/direct/1:1/normal chats
to match the naming used in the API.
Closes: #8368
Signed-off-by: Jagoda Ślązak <jslazak@jslazak.com>
---------
Signed-off-by: Jagoda Ślązak <jslazak@jslazak.com>
Prevents showing "empty" chat on other devices,
when pinning a chat that did not complete securejoin yet.
Fixes: #7713
Signed-off-by: Jagoda Ślązak <jslazak@jslazak.com>
This way they also can be processed by `markseen_msgs()` resulting in self-MDNs which improves
multi-device synchronization. MDNs to contacts won't really be sent because silent group changes
messages are self-removal messages which don't request MDNs.
Otherwise it's not possible to see the full message text if it's too long in a received edit. On the
sender side let it still be text so that the message can be edited again.
Arbitrary timeouts often result in flaky tests,
especially if CI runners may be paused.
In this case however timeout was not used by any tests
and only slowed down the tests in cases where
no message is expected but non-zero timeout is set.
- Ignore "vc-request-with-auth" if the sender is blocked.
- Ignore SecureJoin messages on an observing device if the joiner is blocked. Even if it's a
"vc-contact-confirm" message, it might be issued by another device before the joiner was blocked
on the observing device. Still, to avoid membership inconsistency on devices, don't ignore
"vg-member-added".
- Ignore "vc-request-pubkey" if the sender is blocked by address. We don't know sender's key yet, so
we should handle the sender as an address contact at this point.
The sender can generate a new key and still join, but it's at least an extra work and also, in case
of a group, other members will see that there's a new unknown member and be more careful.
I.e. don't fail `remove_contact_from_chat()` for such a contact because there may be a race
condition with a remote removal of the contact done without trace, but don't send a removal message
and sync message in this case and don't emit a `ChatModified` event.
If a contact is already a past member, we still send a removal message to the chat, this is safe and
protects from lost removal messages, so there's no need to complicate the code in this case.
Fix https://github.com/chatmail/core/issues/8250. Not sure why anyone
would tamper with the link, but maybe the name gets ellipsized because
it is very long, or maybe we decide to ellipsize more aggressively in
the future.
The fix is easy enough, just set the GroupNameTimestamp when creating
the channel, so that `HeaderDef::ChatGroupNameTimestamp` gets set when
sending a message and the logic in
`apply_chat_name_avatar_and_description_changes()` recognizes the
incoming name's timestamp as newer than `chat_group_name_timestamp`
(which is 0 right after joining a chat).
The test is somewhat hacky and depends on the title being the last
parameter in the invite code URL, but this is fine; if we ever change
that, then the test will fail and we need to modify it.
This also removes some wrong doc comments.
Since groups are unpromoted until sending a first message, the same fix isn't needed for groups.
Follow-up to 970222f. We shouldn't update the message state to `OutDelivered` if message sending
failed before and the message was re-sent successfully only to a new broadcast member.
This change is a preparation for ignoring
unencrypted messages by default.
New test_utils::encrypt_raw_message and
test_utils::receive_encrypted_imf are
used to encrypt the messages before
"receiving" them with receive_imf.
There's no check for `from_id` so another group member can delete the
message, but this can only happen in case of message reordering and the
problem already exists for usual messages, so we may ignore it and
overall a group represents a scope of trust.
Co-authored-by: Hocuri <hocuri@gmx.de>
This causes problems for Delta Chat Desktop at
<https://github.com/deltachat/deltachat-desktop/pull/6278>
even though the logic was originally introduced for iOS.
If the problem remains on iOS,
heuristics can be added into iOS UI.
We shouldn't just revert ad7f873c68 because then we won't be able to
normally transfer a backup from a device having clock a bit in the future.
INDEXED BY msgs_index7 is to prevent SQLite from downgrading to using msgs_index2 which has less
ordering.
Last 10 messages in a broadcast channel are resent. They are sent and encrypted only to the new member, not to other subscribers.
Close#7678
Based on https://github.com/chatmail/core/pull/7854, with the following
changes:
- Refactor and simplify code, don't reuse the existing `get_chat_msgs()`
function
- Document that Param::Arg4 is also used for resent messages
cc818d9099
- It's unclear how exactly to resend webxdc status updates. After
discussing with @r10s, don't resend webxdc's at all for now.
38d57ebb30
- Don't set fake `msg_id` in resent messages
e7d0687d90
Setting the msg_id to `u32::MAX` is hacky, and may just as well break
things as it may fix things, because some code may use the msg.id to
load information from the database, like `get_iroh_topic_for_msg()`.
From reading the code, I couldn't find any problem with leaving the
correct `msg_id`, and if there is one, then we should add a function
parameter `is_resending` that is checked in the corresponding places.
Easiest to review file-by-file rather than individual commits, probably.
I'll squash-merge this.
---------
Co-authored-by: iequidoo <dgreshilov@gmail.com>
Broadcast subscribers can't change the chat name, so sending the "Chat-Group-Name{,-Timestamp}"
headers looks unnecessary. That could be useful for other subscriber's devices, but having only the
chat name isn't enough anyway, at least knowing the secret is necessary which is sent by the
broadcast owner.
Before, if the user fixed their clock incorrectly set to the future, they needed to delete
previously sent messages or wait until this future comes again so that new sent messages are added
to the bottom. Strictly speaking, the problem isn't fixable because we don't know if messages were
incorrectly timestamped into the future or they are timestamped correctly and the clock is now
incorrectly set to the past. Anyway, adding messages to the middle of the chat isn't a good way to
inform the user about the problem.
We already set sort_timestamp to 0 for "Messages are end-to-end encrypted."
since 8f1bf963b4.
Do this for "Others will only see this group after you sent a first message."
and "Messages in this chat use classic email and are not encrypted." as well
so no messages can be added on top.
This avoids creating 1:1 chat on a second device when joining a channel.
Now when joining a channel there may be no 1:1 chat with the inviter
when the channel is created. In this case we still create the channel
as unblocked even if 1:1 chat would be a contact request
because joining the channel is an explicit action
and it is not possible to add someone who did not scan a QR
to the channel manually.
The tests were originally generated with AI and then reworked.
Follow-up to https://github.com/chatmail/core/pull/7754 (c724e29)
This prevents the following attack:
/// Eve is subscribed to a channel and wants to know whether Alice is also subscribed to it.
/// To achieve this, Eve sends a message to Alice
/// encrypted with the symmetric secret of this broadcast channel.
///
/// If Alice sends an answer (or read receipt),
/// then Eve knows that Alice is in the broadcast channel.
///
/// A similar attack would be possible with auth tokens
/// that are also used to symmetrically encrypt messages.
///
/// To prevent this, a message that was unexpectedly
/// encrypted with a symmetric secret must be dropped.
If one of broadcast owner's devices didn't add a new subscriber for any reason, e.g. because of
missing SecureJoin messages, this device shall add "member added" messages when syncing the member
list from the `SetPgpContacts` message.
The test sometimes fails because of wrong message ordering for bob:
[...]
Waiting for the device of alice@example.org to reply… [NOTICED][INFO]
<Msg#2010🔒: (Contact#Contact#2001): hi [FRESH]
Msg#2008🔒: (Contact#Contact#2001): You joined the channel. [FRESH][INFO]
>Msg#2010🔒: (Contact#Contact#2001): hi [FRESH]
Msg#2011🔒: (Contact#Contact#2001): Member Me removed by alice@example.org. [FRESH][INFO]
This adds `SystemTime::shift(Duration::from_secs(1))` as a workaround.
Co-authored-by: Hocuri <hocuri@gmx.de>
Don't use first-person form in placeholder texts,
as these can be misleading when broadcasted to group.
Additionally ensures that broadcasted system messages
are not localized to not leak locally-set language
to the group chat.
Fixes#7930
Signed-off-by: Jagoda Ślązak <jslazak@jslazak.com>
Close https://github.com/chatmail/core/issues/7396. Before reviewing,
you should read the issue description of
https://github.com/chatmail/core/issues/7396.
I recommend to review with hidden whitespace changes.
TODO:
- [x] Implement the new protocol
- [x] Make Rust tests pass
- [x] Make Python tests pass
- [x] Test it manually on a phone
- [x] Print the sent messages, and check that they look how they should:
[test_secure_join_group_with_mime_printed.txt](https://github.com/user-attachments/files/24800556/test_secure_join_group.txt)
- [x] Fix bug: If Alice has a second device, then Bob's chat won't be
shown yet on that second device. Also, Bob's contact isn't shown in her
contact list. As soon as either party writes something into the chat,
the that shows up and everything is fine. All of this is still a way
better UX than in WhatsApp, where Bob always has to write first 😂
Still, I should fix that.
- This is actually caused by a larger bug: AUTH tokens aren't synced if
there is no corresponding INVITE token.
- Fixed by 6b658a0e0
- [x] Either make a new `auth_tokens` table with a proper UNIQUE bound,
or put a UNIQUE bound on the `tokens` table
- [x] Benchmarking
- [x] TODOs in the code, maybe change naming of the new functions
- [x] Write test for interop with older DC (esp. that the original
securejoin runs if you remove the &v=3 param)
- [x] From a cryptography perspective, is it fine that vc-request is
encrypted with AUTH, rather than a separate secret (like INVITE)?
- [x] Make sure that QR codes without INVITE work, so that we can remove
it eventually
- [x] Self-review, and comment on some of my code changes to explain
what they do
- [x] ~~Maybe use a new table rather than reusing AUTH token.~~ See
https://github.com/chatmail/core/pull/7754#discussion_r2728544725
- [ ] Update documentation; I'll do that in a separate PR. All necessary
information is in the https://github.com/chatmail/core/issues/7396 issue
description
- [ ] Update tests and other code to use the new names (e.g.
`request-pubkey` rather than `request` and `pubkey` rather than
`auth-required`); I'll do that in a follow-up PR
**Backwards compatibility:**
Everything works seamlessly in my tests. If both devices are updated,
then the new protocol is used; otherwise, the old protocol is used. If
there is a not-yet-updated second device, it will correctly observe the
protocol, and mark the chat partner as verified.
Note that I removed the `Auto-Submitted: auto-replied` header from
securejoin messages. We don't need it ourselves, it's a cleartext header
that leaks too much information, and I can't see any reason to have it.
---------
Co-authored-by: iequidoo <117991069+iequidoo@users.noreply.github.com>
