search

dw-0 / kiauh

Klipper Installation And Update Helper
GNU General Public License v3.0
3.4k stars 492 forks source link

KlipperScreen permissions missing #212

Closed ghost closed 2 years ago

ghost commented 2 years ago

Linux Distribution

Mint 20.3

What happened

Unable to restart KlipperScreen from within KlipperScreen (ie when changing KS config)

What did you expect to happen

The restart should happen so that new settings can take effect.

How to reproduce

Install using KIAUH to linux without altered sudoers

Additional information

Chatted with KS dev to narrow this down. KS assumes installation on a Pi where the sudeoers conf has been altered to not request a password for the Pi user. When this is not the case, it cannot run systemctl with root perms and so cannot restart itself.

KS includes a script (which has a minor typo at present but works) to address this in a more secure fashion by means of creating a new group with only the required perms and assigning the user to that group.

Perhaps KIAUH could offer to run that script, or suggest it is run manually?

dw-0 commented 2 years ago

In the KS issue, you actually pointed out correctly that those new PolicyKit features of Moonraker are used by Mainsail. That's why restarting the service works. That is exactly one reason why that feature was added to Moonraker. See https://github.com/Arksine/moonraker/pull/346 for more info on that and the reasons why that feature was created.

I don't quite understand why KlipperScreen does not use the tools it is given by Moonraker to restart its own service (so by sending a websocket request) but in turn only uses them to restart all other services?

As of now, i think that this is something that should be adressed by the KlipperScreen developers. The answer you got here, that PolicyKit is relatively new, is not really an explanation why they don't make use of it and rather stick to using a python system command that requires elevated privileges.

Im hesitant to prompt a user to execute a sudo-fix script that technically isn't necessary in the first place anymore. And in that regard, im currently hesitant to try and patch an issue that also seems to shouldn't exist in the first place.

ghost commented 2 years ago

I understand, I agree with the hesitance toward changing superuser perms, I shared that same feeling about applying it to my PC.

I can see that it could be good for KS to be able to restart itself without the mainsail backend functioning, though, so perhaps that is why it's handled differently? But then it seems like KS could piggyback onto mainsail's existing polkit exclusions, or implement similar. Which I guess is a KS FR which will make all this go away.

Thanks for your advice