hoehermann / purple-signald

Pidgin libpurple bridge to signald.
GNU General Public License v3.0
153 stars 19 forks source link

Messages sent via other device show as to "unknown" #64

Closed dperelman closed 2 years ago

dperelman commented 2 years ago

Messages from other users show up fine, and sending messages to them via Pidgin also works and syncs to my phone (Android using the Signal app from the Google Play Store), but whenever I send a message from my phone, it shows up in a separate IM window to "unknown", which gets all of my messages to all contacts grouped together with no indication of which message was sent to which contact.

(This issue only applies to IMs: group chats work fine.)

Here's what shows up in the pidgin -d log when I send a message ("Test") to myself using my phone (phone number replaced with +15555551212 and GUIDs obfuscated to avoid identifying my Signal account):

(07:07:33) prpl-hehoe-signald: got newline delimited message: {"type":"IncomingMessage","version":"v1","data":{"account":"+15555551212","source":{"number":"+15555551212","uuid":"12345678-9abc-def0-1234-56789abcdef0"},"type":"CIPHERTEXT","timestamp":1648562851925,"source_device":1,"server_receiver_timestamp":1648562852941,"server_deliver_timestamp":1648562852942,"has_legacy_message":false,"has_content":true,"unidentified_sender":false,"sync_message":{"sent":{"destination":{"uuid":"12345678-9abc-def0-1234-56789abcdef0"},"timestamp":1648562851925,"expirationStartTimestamp":0,"message":{"timestamp":1648562851925,"body":"Test","endSession":false,"expiresInSeconds":0,"profileKeyUpdate":false,"viewOnce":false},"unidentifiedStatus":{"12345678-9abc-def0-1234-56789abcdef0":false},"isRecipientUpdate":false},"contactsComplete":false},"server_guid":"abcdef01-2345-6789-abcd-ef0123456789"},"account":"+15555551212"}

I'm using the latest HEAD of git:

commit f66768a5c372fc40769ad185b34c7ac76689b160 (HEAD -> master, origin/master, origin/HEAD)
Author: Avinash H. Duduskar <2946372+Strykar@users.noreply.github.com>
Date:   Wed Mar 2 19:19:11 2022 +0530

with what I believe is the latest signald binary:

Package: signald
Version: 0.17.0-18-1217a9f7

This issue did not happen before I updated from the previous versions I had installed in November 2021... but those had other issues which were fixed by upgraded.

demure commented 2 years ago

This started happening to me in the last few weeks, and I had not updated in a few months. The issue appears to have continued after I updated today.

I suspect that signal has changed something recently.

(I'm using bitlbee and getting an 'unknown' privmsg when I speak on my phone)

ttlmax commented 2 years ago

I can confirm this. The destination of a message does not contain the phone number of the desired destination contact anymore, only the uuid. The plugin uses the number to determine the recipient of the message, not the uuid. I will look for a way to use the uuid instead.

dperelman commented 2 years ago

Ah, I didn't notice that detail looking at the log of just the message to myself, but that makes sense. It's probably related to https://signal.org/blog/change-number/ and their long-standing plans to support not having a phone number at all in favor of some sort of username system, so they're getting away from using phone numbers as identifiers for accounts.

hoehermann commented 2 years ago

Maybe instead of Signal Numbers we can switch to Signal UUIDs as the Pidgin name (the primary identifier). This would break existing buddy lists once, but then would offer more consistency. Are we guaranteed to having a UUID for every contact (with the number being optional)? Does anyone know that?

@ttlmax I did some work on the train a few days ago. I want to push to a dev branch so we do not end up with merge conflicts. Please stand by.

Update: dev now exists.

thefinn93 commented 2 years ago

Are we guaranteed to having a UUID for every contact (with the number being optional)

yes, as far as I can tell

dperelman commented 2 years ago

It looks like UUIDs are indeed intended to be the account identifiers. I'm not finding any explicit protocol documentation, but looking at the Git history of the Signal project, I'm finding changes like https://github.com/signalapp/Signal-Server/commit/346c7cd74346b959ee218246da8cd653f046090d which includes the line

- // TODO Destination UUIDs will be present for all messages after 2021-12-16

and https://github.com/signalapp/Signal-Server/commit/c2ba8ab5622885f3b30a2d654d538d5457fcd2ee which changes a bunch of places from referencing accounts by phone number to referencing them by UUID instead.

ttlmax commented 2 years ago

@hoehermann The plugin stores the uuid for all buddies as protocol data, see commit 46c89bdac. This was added in order to get user names in group member lists. I am not working on a patch yet, thus there is no risk for merge conflicts yet.

hoehermann commented 2 years ago

In case you do work on a patch, please let me know here. I want to do the same. I might be able to spend some time on this issue this weekend.

ttlmax commented 2 years ago

I have pushed commit c7af0ee7e846079b92f1e3f45a7871a3627aedcc to the dev branch of my fork. This patch looks for the number related to the uuid given as destination of the message. This is only a fix for the issue reported here, not really a step towards replacing the number by the uuid as main user identification.

I had to change the Makefile in order be able to make the lib, see 6a300353c33ec1a602d0a6fe8f38191cf058d9b7

hoehermann commented 2 years ago

Thank you for the Makefile fix. I was struggling to get it right and just moved filenames around until it built, not really thinking about what I was doing.

In my dev branch, I added a commit which now uses Signal UUIDs for Pidgin buddy names. It is looking good in Pidgin, works for me in direct messages, but my test group seems to be borked. Feedback is appreciated. I want to implement some migration functions to be executed at login. They are absent for now (all contacts will be duplicated).

Also, I have no V1 group I can test with, so I removed all V1 group support. Are V1 groups still a thing out there?

Other than that, I noticed a lot of things I want to change in the plug-in's implementation, but I have no idea if/when I can work on them.

ttlmax commented 2 years ago

@hoehermann Great! For me, the members of a group were displayed by their UUID, instead of name or alias. The tiny commit 522e8225c428ed9acc258a486ceff219304e199b of the branch dev-upstream in my fork seems to fix this. I have opened a pull request.

hoehermann commented 2 years ago

As far as I can tell, thanks to the work done by @ttlmax, this problem is now solved.

In the mean-time, I added a basic migration function which renames contacts from the old style (identified by number) to the new style (identified by UUID). It works fine for me™. If users out there could back-up their buddy-lists and test against 89e64733, that would be of great help.

ttlmax commented 2 years ago

The migration was running smoothly for me. No duplicates left and local aliases preserved. Nice feature!

adatum commented 2 years ago

Thanks! v0.10.0 fixes the "unknown" issue, and migrates the identities from phone number to UUID without issues.

Would it be too much trouble to also include the phone numbers in pidgin's tool tips as hints for who the contact is? eg. some contacts leave a blank for their name and the phone number can help identify them and enter a local alias. If it's too much hassle, no worries, it is manageable.

hoehermann commented 2 years ago

Thank you for the messages. I close this issue as fix implemented, positive feedback.

Regarding the tooltip information: Yes, that is a sensible request. Reminds me of #28. However, now that my holidays are over, I have no idea when I can tend to this project.

ttlmax commented 2 years ago

@adatum Please give commit 4feb3d032ea46176fb0c5692c14912dca22744b9 in the dev branch of my fork a try for displaying the number in the buddy's tooltip. If it is working as expected, I'll open a merge request.

adatum commented 2 years ago

@ttlmax Excellent, commit 4feb3d0 is working for me. Tooltip shows the UUID, account, nickname, and number. No issues noticed within a few minutes of use.

hoehermann commented 2 years ago

Dang it, I started working on 0a89531 at 21:00 today and then did not keep an eye on github. Well, it should be a good orthogonal addition. :slightly_smiling_face:

ttlmax commented 2 years ago

@hoehermann Great! Exactly, both patches (info & tooltip) are not excluding each other. Should we also add "about" to the tooltip?

hoehermann commented 2 years ago

As you like. :) "about" is empty for all my contacts, but including it does not look like much work and some people might want it.