search

ChatSecure / ChatSecure-iOS

ChatSecure is a free and open source encrypted chat client for iOS that supports OTR and OMEMO encryption over XMPP.
https://chatsecure.org
Other
3.13k stars 1.03k forks source link

Failing push notification #770

Closed ageru closed 6 years ago

ageru commented 7 years ago

Hi,

Sorry to create yet another bug about push...

Setup

Steps to reproduce

  1. Install Chatsecure, log to server
  2. Send a couple of messages to user on iPhone, they reach him, OMEMO works... Everything looks clear.
  3. Confirmed push is active in the app's interface
  4. Leave the app.

Actual result

Expected Result If iPhone is connected to the Internet, the iPhone user gets a notification straight away.

Bonus questions

Many thanks.

weiss commented 7 years ago

Or maybe you don't get it? I can't speak for @chrisballinger, but as far as I'm concerned, this sort of nagging is one of the most annoying things when working on open source projects.

Echolon commented 7 years ago

@weiss Yes - you are so I right - I now should definitely reclaim my 12$ donation! ;)

chrisballinger commented 7 years ago

I fixed the notification sound a few days ago but forgot which issue # was tracking it.

mimi89999 commented 7 years ago

@chrisballinger I have the notification sound now. Thanks a lot.

@Loader23 Are you still experiencing those issues? What ChatSecure and iOS version do you have? Does your server have the cloud_notify experimental module?

Loader23 commented 7 years ago

@chrisballinger No Problem, thanks alot for fixing it.

@mimi89999 I have the mod_cloud_notify.lua installed you posted above. iOS 10.3.3 with the last recent ChatSecure. Maybe its normal to not receive any Messages when the app is closed and the message is already received by Conversations?

thundergreen commented 7 years ago

Indeed @Loader23 as Chatsecure has no MAM

2017-09-11 8:39 GMT+02:00 Loader23 notifications@github.com:

@chrisballinger https://github.com/chrisballinger No Problem, thanks alot for fixing it.

@mimi89999 https://github.com/mimi89999 I have the mod_cloud_notify.lua installed you posted above. Maybe its normal to not receive any Messages when the app is closed and the message is already received by Conversations?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ChatSecure/ChatSecure-iOS/issues/770#issuecomment-328431189, or mute the thread https://github.com/notifications/unsubscribe-auth/AMtAf7Ba-Sc5WwaedLUfYAq3lkpyAxwnks5shNWzgaJpZM4NaW-w .

Loader23 commented 7 years ago

@thundergreen Yep, thats explains it. Thanks for info.

weiss commented 7 years ago

@danielreuther:

I'm currently assuming that [ejabberd's] mod_push would require similar modifications and I'm afraid those won't happen before the adjustments found their way into the spec.

I think we could cheat a bit and make it work by setting the existing last-message-body field to a string such as New Message for messages with a <body/> or <encrypted/> child. But yes, this stuff should really be standardized.

weiss commented 7 years ago

One thing I'm unsure about is whether this should also be done for notifications triggered from MAM messages. As per the business rules suggested by @iNPUTmice, I would do this, but as ChatSecure currently doesn't support MAM, this would lead to bogus "New Message" notifications. (You run into a similar problem with <encrypted/> messages that were only encrypted for other devices, but maybe we can ignore that case.)

Does the current version of the mod_cloud_notify patch generate high-priority notifications for MAM messages?

tmolitor-stud-tu commented 7 years ago

@weiss Yes it does obey those business rules completely. When there is no corresponding smacks session (either hibernated or alive) it does trigger push messages for MAM stanzas (and with Chris's patch MAM stanzas with body or encrypted elements are considered high-priority).

ErrorSource commented 7 years ago

Since maybe a week, I receive four "New Message!" notifications, each time I receive a message from a chat partner. Chat partner gets "New Message!" notification only once. I have installed latest mod_cloud_notify.lua (with provided patch to get notification even if App is not running in background). Has this something to do with multiple "results" from pubsub.chatsecure.org?

Sep 13 17:18:05 s2sin1c405e0 debug Received[s2sin]: <iq id='86a7161e5956bec8263f96be45ed4a31ee1fa35524154545e1c559f7a96aebc2' type='result' to='somedomain.com' from='pubsub.chatsecure.org'> Sep 13 17:18:05 stanzarouter debug Discarding iq from s2sin of type: result Sep 13 17:18:05 s2sin1c405e0 debug Received[s2sin]: <iq id='128dda856ce5b0e9181411f79cf846656dabae657f817d3c35a9f0e35ea1426e' type='result' to='somedomain.com' from='pubsub.chatsecure.org'> Sep 13 17:18:05 stanzarouter debug Discarding iq from s2sin of type: result Sep 13 17:18:05 s2sin1c405e0 debug Received[s2sin]: <iq id='906dfbc7c007acefa20583f83e5b4c0104c8ab8784df1e5567b9bbcc9bbd6207' type='result' to='somedomain.com' from='pubsub.chatsecure.org'> Sep 13 17:18:05 stanzarouter debug Discarding iq from s2sin of type: result Sep 13 17:18:05 s2sin1c405e0 debug Received[s2sin]: <iq id='0b5f8a3fdad18c0373463563c0cd4d95975d751e5c29245d287fc0d1087aa208' type='result' to='somedomain.com' from='pubsub.chatsecure.org'> Sep 13 17:18:05 stanzarouter debug Discarding iq from s2sin of type: result

chrisballinger commented 7 years ago

If you have duplicate push registrations, Go to Settings -> Account Details (i) button -> Server Features -> Push -> Reset

On Wed, Sep 13, 2017 at 8:40 AM, gipfeljagd notifications@github.com wrote:

Since maybe a week, I receive four "New Message!" notifications, each time I receive a message from a chat partner. Chat partner gets "New Message!" notification only once. I have installed latest mod_cloud_notify.lua (with provided patch to get notification even if App is not running in background). Has this something to do with multiple "results" from pubsub.chatsecure.org?

Sep 13 17:18:05 s2sin1c405e0 debug Received[s2sin]: <iq id=' 86a7161e5956bec8263f96be45ed4a31ee1fa35524154545e1c559f7a96aebc2' type='result' to='somedomain.com' from='pubsub.chatsecure.org'> Sep 13 17:18:05 stanzarouter debug Discarding iq from s2sin of type: result Sep 13 17:18:05 s2sin1c405e0 debug Received[s2sin]: <iq id=' 128dda856ce5b0e9181411f79cf846656dabae657f817d3c35a9f0e35ea1426e' type='result' to='somedomain.com' from='pubsub.chatsecure.org'> Sep 13 17:18:05 stanzarouter debug Discarding iq from s2sin of type: result Sep 13 17:18:05 s2sin1c405e0 debug Received[s2sin]: <iq id=' 906dfbc7c007acefa20583f83e5b4c0104c8ab8784df1e5567b9bbcc9bbd6207' type='result' to='somedomain.com' from='pubsub.chatsecure.org'> Sep 13 17:18:05 stanzarouter debug Discarding iq from s2sin of type: result Sep 13 17:18:05 s2sin1c405e0 debug Received[s2sin]: <iq id=' 0b5f8a3fdad18c0373463563c0cd4d95975d751e5c29245d287fc0d1087aa208' type='result' to='somedomain.com' from='pubsub.chatsecure.org'> Sep 13 17:18:05 stanzarouter debug Discarding iq from s2sin of type: result

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ChatSecure/ChatSecure-iOS/issues/770#issuecomment-329208541, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfqH2XKrzL3mm2wKhVDwkNcXM-xmXGfks5sh_dcgaJpZM4NaW-w .

ErrorSource commented 7 years ago

Yeah, that did the trick. Thanks for figuring this out. And sorry, if you mentioned that before somewhere and I didn't read it.

Rafaelrico77 commented 6 years ago

Hey, I have a question related to that topic. I use Prosody 0.9 with the default mod_cloud_notify downloaded with Mercury. But all of my users with Chatsecure (iOS) dont get the "New Message" Notification, when the App is closed. As I saw in the logs, the messages got forwarded correctly to the pupsub.chatsecure.com Server (no warnings or errors). Do I have to use the patched versions of the module, which are linked above or is the official one already patched, and there is maybe an other fault in my configuration or something ?

mimi89999 commented 6 years ago

You need to use the version I linked.

jsavage86 commented 6 years ago

@chrisballinger the patched mod_cloud_notify is working great; Thanks for your work!

@mimi89999 Thanks for the links.

Question: Does this patch require background activity to be enabled still?

Rafaelrico77 commented 6 years ago

@mimi89999 thank you for the links, the patched version works well for me too.

Another question: When I am right, the notifications work only in private chat. When I understand it right, there is no support for group chat notifications on client site at the moment. Will this be fixed in the next version (4.1.1) ? And when will this version arrive in the App Store ?

weiss commented 6 years ago

When I understand it right, there is no support for group chat notifications on client site at the moment.

The group chat procotol currently in use wasn't designed to support this kind of thing. But we can work around the issue by keeping the stream management session alive while the client is disconnected, as described in the business rules suggested by @iNPUTmice (and my followup message). This way, disconnected clients will remain joined to MUC rooms and receive push notifications for group chat messages. ejabberd's mod_push_keepalive implements those business rules, and I think the current version of Prosody's push module supports them as well.

tmolitor-stud-tu commented 6 years ago

@weiss yes, prosody's mod_cloud_notify does implement this business rules, too (I added support several month ago).

General problem regarding this smacks session workaround is:

Conclusion: Groupchats aren't very reliable. And to be honest it's not @chrisballinger's fault here. Apple is making it very hard for developers to deliver a good push experience without sending the messages through Apples's servers (which would be a privacy nightmare).

Sidenote: WhatsApp is allowed to deliver high priority silent pushes because it implements VoIP calling (the infamous and rarely working WhatsApp calling feature). If Chatsecure also supported such a feature, it would be allowed to use those high priority pushes as well, which is why I have this VoIP stuff for Chatsecure on my ToDo-list (but I don't have time to implement it :( ) This VoIP stuff would even start the app on incoming messages when it was swiped away (force closed).

weiss commented 6 years ago

groupchat messages only trigger silent pushes

You mean because the patched mod_cloud_notify doesn't mark groupchat messages as "high priority"? If so, shouldn't this be changed?

swiping away the app (maybe accidental) kills the smacks session and all pending groupchat messages are lost

The solution to this is implementing MUC MAM.

smacks sessions have a timeout (standard: 5 minutes) but can be higher. even then they usually arent' longer than a couple of hours.

I suggested to keep the session alive until at least one push notification was generated.

tmolitor-stud-tu commented 6 years ago

@weiss I didn't say these problems could not be somehow minimized (e.g. implement MAM), but currently these are the problems users will face and I just wanted to document them here :)

Regarding your suggestion to keep the session alive: I'll implement this in prosody. Does ejabberd do something similar?

weiss commented 6 years ago

I didn't say these problems could not be somehow minimized (e.g. implement MAM), but currently these are the problems users will face and I just wanted to document them here :)

Lack of MUC MAM support should be somewhat compensated by MUC services sending some backlog when you join. But yes, this may be incomplete (or nonexistent), of course. The other two issues you mentioned should be solvable (or are already solved) by trivial changes to the server-side.

Regarding your suggestion to keep the session alive: I'll implement this in prosody. Does ejabberd do something similar?

Yes, currently a separate module called mod_push_keepalive keeps the stream management session open for 24 hours (by default) if no push notification is generated during that time (the original timeout is restored on the first notification). Before the session times out, a (silent) push notification is generated.

jsavage86 commented 6 years ago

Not sure if this is the best place to do this but according to my testing, the patched mod_cloud_notify does not require background app refresh to be enabled. Hope this helps someone.

Also, @chrisballinger, because I have disabled background app refresh, it gives the red "X" under Server Info's Push Registration. If this patch becomes mainstream this part will need to be updated. Thanks for your work.

andreygursky commented 6 years ago

Has been https://modules.prosody.im/mod_cloud_notify.html already updated or where I can find a recent version dealing with this issue on iOS?

chrisballinger commented 6 years ago

@andreygursky No you will still need my patch. They don't want to merge it.

andreygursky commented 6 years ago

@chrisballinger, is the latest patched version available here: https://bitbucket.org/chrisballinger/prosody-modules/src/12b4387c0991eb24a15677b8bf59184ed880fa40/mod_cloud_notify/mod_cloud_notify.lua?at=default ?

I'm reading the success stories here and still can't believe that ChatSecure would become at last as usable as the proprietary software (since my communication partner blames me about no problems with whatsapp&Co and always working around free and open source on it's iOS). The only problem remaining to try it out is a server with the working mod_cloud_notify. That's why it would be great, if it goes upstream (it is already not so easy to convince a free server admin to pick up some module from hg tip, but applying a patch...). Were there any complains and modification requests or should one just ping upstream to take a look?

chrisballinger commented 6 years ago

@andreygursky No this is the latest: https://github.com/chrisballinger/prosody-modules/blob/xep_0357_priority/mod_cloud_notify/mod_cloud_notify.lua

The main thing is that this involves a change to the XMPP spec for XEP-0357, so beyond merging into Prosody it needs to get standardized for Ejabberd etc, and getting consensus on that is difficult.

ageru commented 6 years ago

Just had a brain fart: would the Prosody team maybe accept to include this altered version of mod_cloud_notify in their community repo as a module on its own (mod_cloud_notify_ios_hack or something along those lines) ? Since this alternate module might be needed for a while, it would at least make discoverability and maintenance less difficult for server administrators (although I acknowledge it might be a pain to maintain outside of github for you Chris)

Anyway, it's just a thought, I leave the smell test to you ;)

chrisballinger commented 6 years ago

It should probably just be merged into the regular mod_cloud_notify because they already have some references to my patch in there.

andreygursky commented 6 years ago

@chrisballinger, thanks for pointing to the repo! So these couples of lines should make a true wonder:

--- prosody-modules.hg/mod_cloud_notify/mod_cloud_notify.lua    2017-10-17 19:39:18.970486913 +0200
+++ prosody-modules.git/mod_cloud_notify/mod_cloud_notify.lua   2017-11-04 20:02:15.516762311 +0100
@@ -14,6 +14,7 @@ local hashes = require"util.hashes";
 local xmlns_push = "urn:xmpp:push:0";

 -- configuration
+local include_priority = module:get_option_boolean("push_notification_with_priority", true);
 local include_body = module:get_option_boolean("push_notification_with_body", false);
 local include_sender = module:get_option_boolean("push_notification_with_sender", false);
 local max_push_errors = module:get_option_number("push_max_errors", 50);
@@ -270,6 +271,13 @@ local function handle_notify_request(sta
                        if stanza and include_body then
                                form_data["last-message-body"] = stanza:get_child_text("body");
                        end
+                       if stanza and include_priority then
+                               if stanza:get_child("body") or stanza:get_child("encrypted", "eu.siacs.conversations.axolotl") then
+                                       form_data["last-message-priority"] = "high"
+                               else
+                                       form_data["last-message-priority"] = "low"
+                               end
+                       end
                        push_publish:add_child(push_form:form(form_data));
                        if stanza and push_info.include_payload == "stripped" then
                                push_publish:tag("payload", { type = "stripped" })
ThomasLeister commented 6 years ago

Me and many other XMPP server admins I know would very much appreciate if this patch was also merged onto the official mod_cloud_notify version on modules.prosody.im! If someone has skills and time to maintain this thing - please go for it! :-)

tmolitor-stud-tu commented 6 years ago

@ThomasLeister I won't merge the patch because the last-message-priority field isn't specified in XEP-0357.

But: if the last-message-body field is set, Rubdub (the appserver used by ChatSecure) interpretes the push as "high priority", too. I recently talked to @weiss (ejabberd developer) and we agreed to add a new config option to include a dummy message in last-message-body (something like "New message").

This way we won't break the standard, but still give you the functionality you need.

dholl commented 6 years ago

For anyone else still wrestling with push notifications even after implementing all the fine wisdom in this thread, also check for cipher suite compatibility issues noted in #822, such as ECDHE curves prime256v1 vs secp384r1...

ageru commented 6 years ago

Thank you for reporting this here @dholl . I still seem to be missing one piece of the puzzle, but whatever my problem is, now this curve/cipher issue is not part of it any more :)

tmolitor-stud-tu commented 6 years ago

FYI: I just pushed my proposed changes to modules.prosody.im over here: https://hg.prosody.im/prosody-modules/rev/df86ce6bb0b4

(the one I talked about over here: https://github.com/ChatSecure/ChatSecure-iOS/issues/770#issuecomment-368275301)

ageru commented 6 years ago

Hi @tmolitor-stud-tu and everyone,

So just to be sure I get everything right...

Currently, as a Prosody server admin, if I want my Chatsecure users to get notifications of new messages, I need to:

Thanks again for all your contributions on this issue.

tmolitor-stud-tu commented 6 years ago

@ageru Yes, push_notification_important_body has to be set to some text like "New Message". But to be honest it can be anything because ChatSecure's AppServer strips the text off when sending the notification via APNS. It's just needed for the AppServer to know the push is a high priority one.

chrisballinger commented 6 years ago

@ageru @tmolitor-stud-tu Please don't send us message content, make sure to replace it with something static.

ldvc commented 6 years ago

In https://github.com/ChatSecure/ChatSecure-iOS/issues/770#issuecomment-368275301:

I recently talked to @weiss (ejabberd developer) and we agreed to add a new config option to include a dummy message in last-message-body (something like "New message").

@weiss is this "dummy message" still planned for ejabberd?

weiss commented 6 years ago

@weiss is this "dummy message" still planned for ejabberd?

Yes, I'll add this soon™.

Lyokovic commented 6 years ago

Hi,

Just a comment to say that after updating my prosody modules and adding the push_notification_important_body option, push notifications now work for me.

Thanks for your work!

danielreuther commented 6 years ago

@weiss Sounds great. In case you have a development branch that contains it, I'd be happy to try it out.

ageru commented 6 years ago

I was despairing a bit after Lyokovic's message, since I was still getting the same "cancel" message as before.

So I tested a few things this evening, and it seems what did the trick for me was to activate S2S, which had been off until then. Does it means that S2S is required for the connection to pubsub.chatsecure.org to happen? Is that expected behaviour? This is a completely candid question, as I was expecting S2S to be for pure messaging only, not in this context.

Here is the config that was successful:

I'll keep testing in the next few day. Please do chip in with your insights and your own experience.

Fingers crossed I'm closing this one soon...

weiss commented 6 years ago

Does it means that S2S is required for the connection to pubsub.chatsecure.org to happen? Is that expected behaviour?

Yes.

weiss commented 6 years ago

@danielreuther

@weiss is this "dummy message" still planned for ejabberd?

Yes, I'll add this soon™.

Sounds great. In case you have a development branch that contains it, I'd be happy to try it out.

It's in the master branch, now. In order to enable it:

    modules:
      mod_push:
        include_body: "New message"
antonyablonsky commented 6 years ago

Hi everyone! Is there way to make s2s connection between ejabberd server and pubsub.chatsecure.org if server was not configured with DNS records and domain name like "example.com"? it just lives on my static ip and I connect straightforward, like user@ip_adress. Thanks

chrisballinger commented 6 years ago

Nope sorry. It requires a valid SSL certificate. You can get domains for pretty cheap these days like $2/year, and if you use Let's Encrypt you can get free SSL certificates.

On Sat, May 12, 2018 at 3:30 AM antonyablonsky notifications@github.com wrote:

Hi everyone! Is there way to make s2s connection between ejabberd server and pubsub.chatsecure.org if server was not configured with DNS records and domain name like "example.com"? it just lives on my static ip and I connect straightforward, like user@ip_adress. Thanks

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ChatSecure/ChatSecure-iOS/issues/770#issuecomment-388545777, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfqH549GlGfF6MjWHo3VtfDzhRh1zw9ks5txrmvgaJpZM4NaW-w .

Echolon commented 6 years ago

Hi,

just started an EtherCalc Pad to collect information about your and others experience about push. Feel free to add your data or additional columns (suggest only to add columns where the information can be given from a normal user)

https://ethercalc.org/29iyo1nunqdp

Cheers

ageru commented 6 years ago

For the past few months, I have been running Prosody 0.10.2, and my users have been running ChatSecure 4.3 and latest. Right now:

I says "mostly" because there are still 2 instances when the ChatSecure users don't get messages

But those are outside the scope of my initial issue, wich I consider solved, so I'm closing tis bug. Thanks everyone for participating!

Here is how my server is set up to allow for push, in case this is useful to anyone:

Echolon commented 6 years ago

Better leave this issue open