Skip to content

Fix Electron Quirks #6

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ProjectSynchro
Copy link
Contributor

@ProjectSynchro ProjectSynchro commented Aug 29, 2024

Fixes #5

This allows access to the Gnome secrets, libsecret and kwallet sockets which should allow electron safeStorage to function correctly.

@flathubbot
Copy link
Contributor

Started test build 143310

@ProjectSynchro ProjectSynchro marked this pull request as draft August 29, 2024 23:50
@flathubbot
Copy link
Contributor

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

flatpak install --user https://dl.flathub.org/build-repo/126365/app.ytmdesktop.ytmdesktop.flatpakref

@flathubbot
Copy link
Contributor

Started test build 143319

@ProjectSynchro ProjectSynchro changed the title Allow access to GNOME secrets and KWallet Fix safeStorage support Aug 30, 2024
@flathubbot
Copy link
Contributor

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

flatpak install --user https://dl.flathub.org/build-repo/126375/app.ytmdesktop.ytmdesktop.flatpakref

…d to avoid bugs.

This unfortunately doesn't fix the issue seen in the lastfm integration.
@flathubbot
Copy link
Contributor

Started test build 143335

@flathubbot
Copy link
Contributor

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

flatpak install --user https://dl.flathub.org/build-repo/126391/app.ytmdesktop.ytmdesktop.flatpakref

@ProjectSynchro ProjectSynchro changed the title Fix safeStorage support Ensure we set the correct safeStorage backend Aug 30, 2024
@ProjectSynchro
Copy link
Contributor Author

It seems like there is a bug with how decryption of secrets works between Flatpak and Electron.

This essentially means that when using the Secrets portal, you can store a secret but not decrypt that secret without (potentially) some changes upstream as to how secrets are accessed.

See for context:
flathub/com.vscodium.codium#239
electron/electron#32598

The second issue (#5) is due to an issue with Electron again. (go figure)

Using the "basic" backend doesn't return isEncryptionAvailable as true for some reason..

So, this PR explicitly sets the password store to "basic" so the backend being used to store secrets (for decrypting login cookies and api keys) is consistent everywhere, a fix for the lastfm integration will have to come from an update upstream.

The XDG_CURRENT_DESKTOP environment variable is set to "unity" due to potential issues with.. you guessed it. Electron: electron/electron#39789. Setting this variable bypasses this issue.

This change doesn't cause any issues with the current Flatpak being updated, as we are already using the "basic" secrets backend due to process of elimination but setting it explicitly prevents issues in the future. It isn't as secure as using a proper secrets backend is, but it is the only backend that works.

@ProjectSynchro ProjectSynchro marked this pull request as ready for review August 30, 2024 02:47
@ProjectSynchro
Copy link
Contributor Author

Added a workaround for ANOTHER Electron bug: electron/electron#40461

This should solve crashes when the "Show notification on song change" option is enabled.

@flathubbot
Copy link
Contributor

Started test build 143352

@flathubbot
Copy link
Contributor

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

flatpak install --user https://dl.flathub.org/build-repo/126408/app.ytmdesktop.ytmdesktop.flatpakref

@ProjectSynchro ProjectSynchro changed the title Ensure we set the correct safeStorage backend Fix Electron Quirks Sep 1, 2024
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.

No option for Last.fm Login after v2 upgrade
2 participants