search

elementary / switchboard-plug-network

Switchboard Network Plug
GNU General Public License v3.0
19 stars 24 forks source link

Allow setting a systemwide proxy #95

Closed davidmhewitt closed 4 years ago

davidmhewitt commented 6 years ago

Superseded by #255

Fixes #37

Uses the ubuntu-system-service DBus interface and a bit of Polkit to set the apt proxy and proxy environment variables for terminal use when configuring a proxy in the plug.

Since this depends on an ubuntu specific package, I've added a CMake option to enable the systemwide proxy setting functionality.

Compiling with cmake -DCMAKE_INSTALL_PREFIX=/usr -DUSE_UBUNTU_SERVICES=ON will enable the functionality.

Conan-Kudo commented 6 years ago

Why isn't this using PacRunner or the NetworkManager interfaces? Also, why is this necessary? NetworkManager already handles system proxy stuff to take care of that...

davidmhewitt commented 6 years ago

@Conan-Kudo This plug currently uses the GNOME dconf keys to set a proxy. This works quite well for desktop applications and browsers but does not take effect for commands like curl which use environment variables like http_proxy and also doesn't work with apt which expects proxy configuration to be in a config file. So you end up with proxy settings that work for some things but not others.

I may be missing something, but my understanding is that until every piece of software uses libproxy or PacRunner or something similar to resolve proxies, then this will continue to be a problem. I don't think NetworkManager can set environment variables for things like curl and I'm also struggling to find in their documentation how you set a manually configured proxy with it.

I believe this is why ubuntu-system-service continues to be used and shipped by Ubuntu to get around some of these minor proxy annoyances. However, if there's a missing link that would make this more distro independent, please point me in that direction and I'll rework this to use that.

Conan-Kudo commented 6 years ago

@davidmhewitt The correct thing to do would be to adjust ubuntu-system-service to fetch the configuration from NetworkManager if it exists and dynamically set it to the environment for all the things. Or make a separate specific service for this if you don't want to adjust ubuntu-system-service.

Per-connection proxy information is available through the connection settings enumeration.

davidmhewitt commented 6 years ago

@Conan-Kudo While I agree that would be better, I think that's a separate issue that's beyond the scope of this. Doing something like that would require reworking this switchboard plug to better configure per-connection proxies via NetworkManager. Plus the work involved in making a suitable system service to handle the environment variables etc...

The aim of this relatively low overhead PR is to bring our proxy settings back to feature parity with the GNOME control center settings page which elementary used to ship and then we lost the systemwide proxy functionality when writing this switchboard plug.

davidmhewitt commented 6 years ago

@donadigo @Conan-Kudo Thank you both for the feedback, I've made the changes as suggested.

davidmhewitt commented 6 years ago

@donadigo

  1. Thanks for spotting that, I found another one and fixed that too!
  2. The D-Bus server is supposed to create that file if it doesn't exist, however it looks like only the set method does and the clear method doesn't. So, if a clear happens before a set, we'd get that error. I think I'll write some code to not call the clear method if that file doesn't exist, since there won't be a proxy to clear in that case.
  3. Hopefully the above should stop any errors, the rest of the ubuntu-system-service code seems reasonably fail safe. So I think I'd like to avoid a dialog, but I'm open to @elementary/ux opinions.
  4. Yes, I agree with that, lets see what UX come back with.

Thanks again for the review.

davidmhewitt commented 4 years ago

Superseded by #255