Message Carbons: Siskin shows notifications for messages send from account in annother client (e.g. Monal) (#429)
Unknown opened 4 years ago

Describe the bug When I send a message from Monal, Siskin notifies me on my own sent messages.

To Reproduce Steps to reproduce the behavior:

  1. Have Siskin and another XMPP app like Monal installed.
  2. Have the same account active in both apps.
  3. Send a message from the non-Siskin app
  4. Get a notification on your own message from Siskin.

Expected behavior No notification (neither my nor others messages, if Siskin is in the background)

Smartphone

  • Device: iPhone 5s
  • OS: iOS 12..4.8

Additional context Monal Beta 4.7 (633)

Unknown commented 4 years ago

Which server do you use? Prosody? Ejabberd? Tigase?

Unknown commented 4 years ago

Server is running Prosody trunk nightly build 1309 (2020-06-30, df3ee12acd8c)

https://compliance.conversations.im/server/yax.im/

Ahh, it does not support MAM, right?

Unknown commented 4 years ago

I disabled the account... now it is still receiving messages and shows notifications^^

Unknown commented 4 years ago

While Siskin is in the background, after 30s it is disconnected (as that is the behaviour enforced by Apple - app cannot run in the background for a longer period of time).

To receive messages while in the background, Siskin depends on push notifications being sent by your server. If you are receiving "notifications" after 30s of Siskin being in the background, then most likely your server is sending those notifications as push notifications to the Siskin. That is a behaviour of that server and there is nothing we can do to fix that.

Unknown commented 4 years ago

ok thx. so push notification disabling does also not help?

Unknown commented 4 years ago

Disabling push notifications could help with that issue but then you will not receive any notifications about new messages while Siskin is in the background.

Unknown commented 4 years ago

Yes, I installed Siskin only for testing.

Hmm... now the Push Not. disable toggle does not work ??‍♂️??

Unknown commented 4 years ago

If there is an error and it was not possible to disable push notifications (ie. server returned an error) the switch will return to "enabled" position.

Unknown commented 4 years ago

Ok, maybe there should be a more explicit feedback to the user.

I will close this, thanks.

Unknown commented 4 years ago

I have the same issue. I installed Siskin for testing as I was told that it is better than Monal. So I run my iPhone SE (iOS 13.7) with Siskin and Monal.

When I send a message through another client (Gajim for Windows for example) I get a Siskin notification "new message" on the iPhone. Monal only tells me when the chat partner sent me a message. That's the way it should be.

I use this server and I have Push notifications enabled. Also MAM ist active on the server and also in Siskin.

The server I use is this one: https://compliance.conversations.im/server/jabber.hot-chilli.net/

Unknown commented 4 years ago

As it was already stated:

To receive messages while in the background, Siskin depends on push notifications being sent by your server. If you are receiving "notifications" after 30s of Siskin being in the background, then most likely your server is sending those notifications as push notifications to the Siskin. That is a behaviour of that server and there is nothing we can do to fix that.

You should report that to the person responsible for running the server which you are using or developers of that server so it could be fixed. Siskin just shows notification which are triggered by the server which you are using.

Unknown commented 4 years ago

As it was already stated:

To receive messages while in the background, Siskin depends on push notifications being sent by your server. If you are receiving "notifications" after 30s of Siskin being in the background, then most likely your server is sending those notifications as push notifications to the Siskin. That is a behaviour of that server and there is nothing we can do to fix that.

You should report that to the person responsible for running the server which you are using or developers of that server so it could be fixed. Siskin just shows notification which are triggered by the server which you are using.

Ok. I will.

But the question remains: Monal handles it correct. It shows notifications for the same account on same server, Monal installed on the same iOS device. But only messages from other people, not from myself. Like you would expect it.

Unknown commented 4 years ago

What Monal does, it reconnects on each received push notification to fetch new messages. So, if on each message sent from another device push is being sent, then each time Monal reconnects and fetches messages which in my opinion is just waisting energy from the battery. Servers should only send notification when there is actually something about which user (user not client) be aware of.

Unknown commented 4 years ago

@hantu85 unfortunately apple does not allow apps to keep persistent (idle) tcp connections open, which would consume virtually no energy, but only allows to start a new process on incoming push which is allowed to load the correct message from the server.

Yes, I would like apple to provide a better api, but it is as it is...

Unknown commented 4 years ago

@tmolitor-stud-tu while I agree on that, there are other ways to have a message on the client side, ie. send it encrypted in the push payload, but that requires additional support on the server-side and most of the servers do not support that. I'm not trying to say that Monal does something in a bad way, but in a different way and tried to explain why Siskin does it in a way it does.

Unknown commented 4 years ago

@hantu85 that is right, but even encrypting the stanza into the push will make it neccessary to implement a notification service extension that decrypts it and loads additional data like attached images, if necessary.

And the current push XEP does not contain any encrypted stuff

Unknown commented 4 years ago

From reading this issue: I suspect siskin is not handling the difference between a push providing a (dummy) last-message-body and a push without the last-message-body.

Pushes with last-message-body should be considered important and provide a user visible notification while those without last-message-body should only trigger silent pushes that wake up the app in background (if at all).

Unknown commented 4 years ago

@tmolitor-stud-tu we intend to promote this extension draft (https://xeps.tigase.net//docs/push-notifications/encrypt/) to full-xep.

Unknown commented 4 years ago

If you encrypt things: why not send the whole original stanza encrypted instead of this custom json payload? The json is perfect for your client but other clients maybe need other information not currently included in your xep. It may even be possible that new XEPs add some element to message stanzas that you need to process. I think it would not be feasible to update your push xep every time a new xep adds some field to a message stanza.

You should try to generalize your approach as much as possible.

Unknown commented 4 years ago

It is generalized and unfortunatelly size of push notifications is limited to 3-4KB (depending on used push notifications delivery platform). We suggest usage of encrypted notifications as even with unencrypted messages they normally are not forwarded to our push component or to APNS for delivery. To make it more private for people using unencrypted XMPP messages we decided to encrypt push notifications to make sure that push component (ie. our component) or any push notifications provider (APNS, FCM or anyone else) will not be able to look into content of this notifications.

Unknown commented 4 years ago

As for JSON, it could be replaced with XML and would also work. We started with JSON as it was simpler to use for us on the iOS.

Unknown commented 4 years ago

yes, encryption is fine, but it would be nice to do end-to-end encryption so that even your appserver is not capable to decrypt things, and use the notification service extension to decrypt the message and see what it contains and what notification to display to the user (if at all). The 3-4kb size limit should be no problem if you compress the stanza prior to encryption. you could even add stripping of known unneeded parts of the stanza to the xep, a blacklist approach rather than your current static whitelist approach which will need much more XEP updates in the future.

Unknown commented 4 years ago

This is initial document and mere proposal. We are open to improving the document and protocol :-)

Unknown commented 4 years ago

@tmolitor-stud-tu What do you mean by appserver? XMPP server? or the push component? if push component then it will get data already encrypted. if by XMPP server then I considered possibility of using this encrypted stanzas with OMEMO, see https://xeps.tigase.net//docs/push-notifications/encrypt-omemo/

Unknown commented 4 years ago

I mean the appserver as it was defined in XEP-0357. I guess this is what you mean by "push_component". This component should not have the key to decrypt the json/whatever payload but only proxy it through APNS to the iOS app where it will get decrypted by the Notification Service Extension.

Am Donnerstag, 17. September 2020, 19:14:41 CEST schrieb Andrzej Wójcik:

@tmolitor-stud-tu What do you mean by appserver? XMPP server? or the push component? if push component then it will get data already encrypted. if by XMPP server then I considered possibility of using this encrypted stanzas with OMEMO, see https://xeps.tigase.net//docs/push-notifications/encrypt-omemo/

Unknown commented 4 years ago

And in our approach push component does not have a decryption key as notification is generated by user's server and encrypted by it. And then decrypted on the user's device. So the encryption key is available only to the XMPP (for encrypting notification payload) and on the user's device for decryption.

Unknown commented 4 years ago

That sounds sensible :)

Am Donnerstag, 17. September 2020, 20:20:39 CEST schrieb Andrzej Wójcik:

And in our approach push component does not have a decryption key as notification is generated by user's server and encrypted by it. And then decrypted on the user's device. So the encryption key is available only to the XMPP (for encrypting notification payload) and on the user's device for decryption.

issue 1 of 1
Issue Votes (0)
Watchers (0)
Reference
tigase/_clients/siskin-im#429
Please wait...
Page is in error, reload to recover