Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for PNI fields and fix fetching (encrypted groups) #224

Merged
merged 9 commits into from
Apr 27, 2023

Conversation

gferon
Copy link
Collaborator

@gferon gferon commented Apr 22, 2023

Tested in presage which should get us started with #206 and fix #223.

This requires to be able to provide two separate store implementations to the underlying libsignal. See https://github.com/signalapp/Signal-Android/blob/b4f6177e873cfd23cbce8fdbea8b573af8faf45d/app/src/main/java/org/thoughtcrime/securesms/crypto/storage/SignalServiceAccountDataStoreImpl.java#L26 for the requirements in Signal-Android.

@gferon gferon changed the title Add support for pni fields Add support for PNI fields and fix fetching (encrypted groups) Apr 22, 2023
@gferon
Copy link
Collaborator Author

gferon commented Apr 22, 2023

@rubdos did you investigate by any chance how one can "upgrade" an existing account and upload some PNI keypair? I don't care that much for linking, as one can simply ask to relink (without losing any data).

Copy link
Member

@rubdos rubdos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not looked into the migration logic here, no. I would rather actually implement the migration. It's not because relinking is supported in-protocol, that we can do that easily in Whisperfish already 🙈

libsignal-service/src/provisioning/manager.rs Show resolved Hide resolved
@gferon
Copy link
Collaborator Author

gferon commented Apr 22, 2023

I have not looked into the migration logic here, no. I would rather actually implement the migration. It's not because relinking is supported in-protocol, that we can do that easily in Whisperfish already 🙈

Yes this is what I figured. I can look into the migration, i suppose you can upload a PNI private and public key, and get some UUID back.

@gferon gferon requested a review from rubdos April 27, 2023 07:52
Copy link
Member

@rubdos rubdos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick on the docstrings for the ServiceIdType.

To what extend did you look at the migration, yet?

libsignal-service/src/push_service.rs Outdated Show resolved Hide resolved
@gferon
Copy link
Collaborator Author

gferon commented Apr 27, 2023

To what extend did you look at the migration, yet?

To repeat what was said on Matrix, it looks like the migration is as easy as registering pre-keys with the server for ServiceIdType::PhoneNumberIdentity. We should be able to merge this without changing existing behaviour, and it'll let me continue to experiment with presage.

@gferon gferon merged commit 1c6390b into whisperfish:main Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Groups v2 getAuthorizationForToday stopped working
2 participants