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

Reduce file access, add StatusNotifierWatcher for KDE #20

Closed
wants to merge 3 commits into from

Conversation

fourstepper
Copy link

Closes #19 , improves security.

Possibility of closing #13, needs testing

Closes flathub#19 , improves security.

Possibility of closing flathub#13, needs testing
@flathubbot
Copy link
Contributor

Started test build 35878

@flathubbot
Copy link
Contributor

Build 35878 failed

@flathubbot
Copy link
Contributor

Started test build 35879

@flathubbot
Copy link
Contributor

Build 35879 failed

Copy link
Author

@fourstepper fourstepper left a comment

Choose a reason for hiding this comment

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

This should fix build.... or not

@flathubbot
Copy link
Contributor

Started test build 35880

@flathubbot
Copy link
Contributor

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

flatpak install --user https://dl.flathub.org/build-repo/34528/com.mattermost.Desktop.flatpakref

@SemaiCZE
Copy link
Collaborator

SemaiCZE commented Jan 4, 2021

Thank you. I used this version for a day and it seems sort of ok. But I don't like that whan I'm saving a file somewhere into my Documents, everything seems ok, but the file is not there. That is very confusing in my opinion. The only difference to saving into Downloads is that it shows Download completed notification.

For Slack it's different. The app always saves files into Downloads and won't let you choose the destination.

I don't want to make people angry and silently drop their files. So in my opinion we should wait until Mattermost Desktop notifies you when you haven't enough permissions to write the file into the selected destination or something like that. Does that make sense to you?

@fourstepper
Copy link
Author

Hi, thank you for your time to test and review this. This behaviour is unfortunate, but I would rather perhaps open up the :ro tags on those folders, if possible. I think this would still be quite a big upgrade from the current state of the app having access to all of $HOME while still supporting majority of the use cases.

What do you think?

@SemaiCZE
Copy link
Collaborator

SemaiCZE commented Jan 4, 2021

Can @TingPing, @nedrichards, @barthalion or some other flatpak guru help us to decide?

@apollo13
Copy link
Contributor

apollo13 commented Jan 4, 2021 via email

@barthalion
Copy link
Member

If it's broken in a not visible way, I don't think we want to go with this.

@SemaiCZE SemaiCZE marked this pull request as draft January 25, 2021 20:33
@vchernin
Copy link
Collaborator

vchernin commented Jun 28, 2021

Electron 14 will let apps use the xdg-desktop-portal file chooser which should eliminate the need to grant Mattermost any file system access. Users will be able to choose any file/folder securely through the use of the portal. Currently Mattermost is on Electron 12.

In the meantime, for those who want to grant Mattermost Flatpak less file access, I recomend either using the command line flatpak tools or using Flatseal.

"--talk-name=org.kde.StatusNotifierWatcher" appears to have been added elsewhere, so this branch probably isn't needed.

@fourstepper
Copy link
Author

I dont think this is needed anymore, closing

@TingPing
Copy link
Member

I'm confused by the closing.

Electron now added proper save dialog support, so surely now is the time to actually do this.

When Mattermost updates to Electron 14 of course

@SemaiCZE
Copy link
Collaborator

I've just tested this again with Mattermost 5.1.0-rc1 on Electron 18 and it's still bad. When you save a file into let's say your home folder, the file downloads and there is also notification that the download finished. So far so good. When you try to save another file into the same location, you'll see the previous file there, but when you open the same exact directory from the file manager on the computer, the file just isn't there. And if you try to download a file into Documents, it isn't event downloaded at all.

I don't know what I'm missing, but I think that we'll just keep write permissions for home filesystem.

@TingPing
Copy link
Member

It sounds like the portal isn't being used for some reason.

@fourstepper
Copy link
Author

Just letting you guys know that I don't use mattermost anywhere anymore and currently don't really have a linux system handy, so it's up to you :) I will watch the thread and be happy to edit the PR as needed, though :)

@fourstepper fourstepper reopened this Apr 19, 2022
@flathubbot
Copy link
Contributor

Started test build 87153

@flathubbot
Copy link
Contributor

Build 87153 failed

@SemaiCZE
Copy link
Collaborator

Hi @fourstepper, thanks. I'd close this PR since I don't want to merge it anytime soon and I'd keep track of home permissions in issue #19.

@fourstepper
Copy link
Author

@SemaiCZE ok, closing :)

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.

Open permissions to the user home directory
7 participants