flathub / org.signal.Signal

https://flathub.org/apps/details/org.signal.Signal
62 stars 38 forks source link

Show encryption notification only once #744

Closed bermeitinger-b closed 1 week ago

bermeitinger-b commented 1 week ago

Show the encryption warning only once.

flathubbot commented 1 week ago

Started test build 150002

flathubbot commented 1 week ago

Build 150002 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/133093/org.signal.Signal.flatpakref
bb010g commented 1 week ago

Script is still inconsistent between gnome_libsecret and gnome-libsecret.

flathubbot commented 1 week ago

Started test build 150012

bermeitinger-b commented 1 week ago

Always showing the warning when basic is selected seems out of scope for us. This should be an upstream discussion.

minosimo commented 1 week ago

It is not necessary to warn all users that we are using the unencrypted key store when the application has already been doing so for a long time.

The warning should appear:

flathubbot commented 1 week ago

Build 150012 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/133103/org.signal.Signal.flatpakref
bbhtt commented 1 week ago

It is not necessary to warn all users that we are using the unencrypted key store when the application has already been doing so for a long time.

The difference is previously the official package didn't support it and official builds used plaintext as well. Now the official ones don't do anymore

This makes Flatpak a worse option for some people. Users should be aware of that, new or old both and choose what's best for them.

bermeitinger-b commented 1 week ago

On an existing install, if the user switches from a system keystore to the basic keystore

This will never be intentional, right? Why would they change a running system and intentially lessen the security. It could happen, if they somehow reset the env var to default. But then, the warning is useless because Signal will complain anyway.

Is there a nice way to check if this is the first install? Then do this instead of the warning_shown file.

The difference is previously the native package didn't support it and official builds used plaintext as well. Now the official ones don't do anymore

They fallback to basic just as we do here. Apparently, the implementation is a bit broken, so it (Chromium, Electron, and/or Signal) can't find the backend correctly. This database error is not exclusive to flatpak.

minosimo commented 1 week ago

On an existing install, if the user switches from a system keystore to the basic keystore This will never be intentional, right? Why would they change a running system and intentially lessen the security. It could happen, if they somehow reset the env var to default. But then, the warning is useless because Signal will complain anyway.

Is there a nice way to check if this is the first install? Then do this instead of the warning_shown file.

The difference is previously the native package didn't support it and official builds used plaintext as well. Now the official ones don't do anymore They fallback to basic just as we do here. Apparently, the implementation is a bit broken, so it can't find the correct backend correctly. This database error is not exclusive to flatpak.

config.json, among others, are generated on first launch.

bbhtt commented 1 week ago

if it's broken on official ones as well you could use the first install idea, just check if the config folder or some file in it exists.

On first install it won't exist unless signal is executed which is at the very last line of the script.

flathubbot commented 1 week ago

Started test build 150015

bermeitinger-b commented 1 week ago

This will only warn upon the first run. (However, we differ here from upstream.)

Also, this is generally a workaround, The secrets portal should provide the correct backend name by its run.

flathubbot commented 1 week ago

Build 150015 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/133106/org.signal.Signal.flatpakref
flathubbot commented 1 week ago

Started test build 150017

bbhtt commented 1 week ago

You can drop the else condition that touch es the file in lines 26-28

flathubbot commented 1 week ago

Build 150017 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/133108/org.signal.Signal.flatpakref
flathubbot commented 1 week ago

Started test build 150018

flathubbot commented 1 week ago

Build 150018 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/133109/org.signal.Signal.flatpakref