search

flathub / com.mojang.Minecraft

https://flathub.org/apps/details/com.mojang.Minecraft
32 stars 13 forks source link

Add org.freedesktop.secrets talk #132

Closed vulongm closed 1 year ago

vulongm commented 1 year ago

Resolves #116 on kwallet 5.97+.

edit: Do not merge this PR yet. See https://github.com/flathub/com.mojang.Minecraft/pull/132#issuecomment-1280736540 and following comments.

flathubbot commented 1 year ago

Started test build 112196

flathubbot commented 1 year ago

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

flatpak install --user https://dl.flathub.org/build-repo/109802/com.mojang.Minecraft.flatpakref
AsciiWolf commented 1 year ago

Hmm, I thought that this is handled by some part of xdg desktop portal (at least this is how it works on GNOME). Anyway, this would need testing before merge. We don't want to cause any regressions for current users.

AsciiWolf commented 1 year ago

Anyway, thanks for the PR!

MenacingPerson commented 1 year ago

This works perfectly fyi

subpop commented 1 year ago

This build doesn't appear to be available anymore. On Fedora 36, I tried replicating this change using overrides, but I couldn't get it to work.

$ flatpak --user override --show com.mojang.Minecraft 
[Context]
sockets=system-bus;session-bus

[System Bus Policy]
org.freedesktop.secrets=talk

[Session Bus Policy]
org.freedesktop.secrets=talk
$ rpm -q kf5-kwallet
kf5-kwallet-5.98.0-1.fc36.x86_64

I have a service exporting the org.freedesktop.secrets interface on the user bus:

$ busctl --user introspect org.freedesktop.secrets /org/freedesktop/secrets
NAME                                TYPE      SIGNATURE RESULT/VALUE                             FLAGS
org.freedesktop.DBus.Introspectable interface -         -                                        -
.Introspect                         method    -         s                                        -
org.freedesktop.DBus.Peer           interface -         -                                        -
.GetMachineId                       method    -         s                                        -
.Ping                               method    -         -                                        -
org.freedesktop.DBus.Properties     interface -         -                                        -
.Get                                method    ss        v                                        -
.GetAll                             method    s         a{sv}                                    -
.Set                                method    ssv       -                                        -
.PropertiesChanged                  signal    sa{sv}as  -                                        -
org.freedesktop.Secret.Service      interface -         -                                        -
.CreateCollection                   method    a{sv}s    oo                                       -
.GetSecrets                         method    aoo       a{o(oayays)}                             -
.Lock                               method    ao        aoo                                      -
.OpenSession                        method    sv        vo                                       -
.ReadAlias                          method    s         o                                        -
.SearchItems                        method    a{ss}     aoao                                     -
.SetAlias                           method    so        -                                        -
.Unlock                             method    ao        aoo                                      -
.Collections                        property  ao        1 "/org/freedesktop/secrets/collection/… emits-change
.CollectionChanged                  signal    o         -                                        -
.CollectionCreated                  signal    o         -                                        -
.CollectionDeleted                  signal    o         -                                        -

When I monitor org.freedesktop.secrets on the user bus and launch the flatpak, I don't see any method calls happening, and I don't have any entries in the "Secret Service" folder in my open kwallet. The only entry in that folder is "Firefox Encrypted Storage".

gnome-keyring-daemon is not running:

$ ps auxwww | grep gnome-keyring
link       10602  0.0  0.0 222172  2192 pts/3    S+   21:06   0:00 grep --color=auto gnome-keyring
MenacingPerson commented 1 year ago

Works for me using just:

$ flatpak --user override --show com.mojang.Minecraft
[Session Bus Policy]
org.freedesktop.secrets=talk
AsciiWolf commented 1 year ago

bot, build

flathubbot commented 1 year ago

Queued test build for com.mojang.Minecraft.

flathubbot commented 1 year ago

Started test build 1864

flathubbot commented 1 year ago

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

flatpak install --user https://dl.flathub.org/build-repo/114304/com.mojang.Minecraft.flatpakref
subpop commented 1 year ago

OK, so something else must be going on. Using build 1864, I'm still not seeing anything on org.freedesktop.secrets and the login still doesn't persist across launches.

Works for me using just:

$ flatpak --user override --show com.mojang.Minecraft
[Session Bus Policy]
org.freedesktop.secrets=talk

What version of kwalletd5 are you running?

MenacingPerson commented 1 year ago

5.99.0-1.1

subpop commented 1 year ago

5.99.0-1.1

Good, well, at least we found another difference between your system and mine. I'll test it again when 5.99 lands in Fedora.

AsciiWolf commented 1 year ago

Hmm, I thought that this is handled by some part of xdg desktop portal (at least this is how it works on GNOME). Anyway, this would need testing before merge. We don't want to cause any regressions for current users.

This needs to be resolved before I can merge the PR. Currently, there are two main problems:

  1. It is possible that adding the org.freedesktop.secrets talk name permission will broke (remove) the saved credentials for current users on GNOME. That would be a regression. This needs further testing.
  2. On GNOME, everything works fine with the current Flatpak although there is no org.freedesktop.secrets talk name permission. How? Is this somehow handled by the gtk/gnome xdg-desktop portal (but not by the kde one) or is something else going on?
subpop commented 1 year ago

There is a "Secret" interface in the xdg-desktop-portal D-Bus specification: https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.Secret. Perhaps you're on to something. Maybe xdg-desktop-portal-kde isn't implementing that interface.

AsciiWolf commented 1 year ago

Nice find! Any chance someone who has an account on the KDE bug tracker could file an RFE ticket for adding org.freedesktop.portal.Secret support to the xdg-desktop-portal-kde there?

AsciiWolf commented 1 year ago

@subpop Did you or anyone else filled the RFE ticket? Thanks!

flathubbot commented 1 year ago

Started test build 4278

flathubbot commented 1 year ago

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

flatpak install --user https://dl.flathub.org/build-repo/116718/com.mojang.Minecraft.flatpakref
Romz24 commented 1 year ago

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

flatpak install --user https://dl.flathub.org/build-repo/116718/com.mojang.Minecraft.flatpakref

link returns 404 Not Found error, tell me when you plan to merge this into master branch?

MenacingPerson commented 1 year ago

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

flatpak install --user https://dl.flathub.org/build-repo/116718/com.mojang.Minecraft.flatpakref

link returns 404 Not Found error, tell me when you plan to merge this into master branch?

$ flatpak override --user --talk-name=org.freedesktop.secrets com.mojang.Minecraft

Just do this in the meantime

AsciiWolf commented 1 year ago

link returns 404 Not Found error, tell me when you plan to merge this into master branch?

I am still waiting for someone to file the RFE ticket as mentioned here. Implementing it in xdg-desktop-portal-kde would be way better than merging this PR that can bring potential issues.

MenacingPerson commented 1 year ago

I am still waiting for someone to file the RFE ticket

Why not file it yourself?

Romz24 commented 1 year ago

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

flatpak install --user https://dl.flathub.org/build-repo/116718/com.mojang.Minecraft.flatpakref

link returns 404 Not Found error, tell me when you plan to merge this into master branch?

$ flatpak override --user --talk-name=org.freedesktop.secrets com.mojang.Minecraft

Just do this in the meantime

I ran this command but nothing changed.

AsciiWolf commented 1 year ago

Why not file it yourself?

Because I am too busy with too many other things? I am maintaining ~26 Flatpaks on Flathub, contributing to various Free Software projects and working on other (free and paid) things. I don't have enough time and energy to do everything myself, especially when the feature/bug is not much important to me and majority of other users. Also, I don't have a bugs.kde.org account. I will eventually report it myself if nobody else does it, but it will take some time.

Regarding this PR, it is unlikely that it will get merged. It is just a workaround (that can be problematic). The proper solution is to implement org.freedesktop.portal.Secret support in xdg-desktop-portal-kde just like it is in the GTK/GNOME portal.