borgbase / vorta

Desktop Backup Client for Borg Backup
https://vorta.borgbase.com
GNU General Public License v3.0
1.98k stars 130 forks source link

Expand WIFI check to all active networks. #1775

Open dietmarw opened 1 year ago

dietmarw commented 1 year ago

Description

I've set up a backup job that should only run if the laptop is on a certain network. Now it seems that when I tick the box of which network is allowed this setting is simply ignored and the backup runs anyway.

One thing which might be the reason is that the laptop is also connected to a wired network connection via docking station. So something with the check of the active network does not quite work as it should.

Reproduction

OS

Linux Ubuntu Mate 22.04

Version of Vorta

0.8.12

What did you install Vorta with?

Distribution package

Version of Borg

1.2.4

Logs

No response

real-yfprojects commented 1 year ago

Thank you for filing this bug report. Can you enclose the logs starting with the time of opening Vorta to reproducing this issue?

One thing which might be the reason is that the laptop is also connected to a wired network connection via docking station. So something with the check of the active network does not quite work as it should.

Vorta queries the PrimaryConnection property of NetworkManager. Do you know which connection is identified as the primary one on you machine? Also does the setting work when unplugging the wired network?

dietmarw commented 1 year ago

So here are three logs.

  1. startup: vorta_startup.log
  2. connected with docking station (via USB-c) which is connected with LAN but wifi is also activated (but connected with a different network than "eduroam"): vorta_lan.log
    • As you can see the backup gets started even though the condition (only on SSID "eduroam") is not met
  3. connected with docking station (via USB c-) but LAN cable removed: vorta_onWifi.log
    • This time vorta detects correctly that it is not on "eduroam" and cancels the run.

So it really looks like the whole wifi check is passed whenever one is connected to LAN. Which is unfortunate since this really is a nice feature to have.

dietmarw commented 1 year ago

One thing that came to my mind, would it not be better to check for the active connections rather than just the primary connection. So basically interpreting the ActiveConnections object rather than PrimaryConnection.

Would this be possible?

real-yfprojects commented 1 year ago

Quoting the source code:

# Only check the primary connection. VPN over WiFi will still show the WiFi as Primary Connection.
# We don't check all active connections, as NM won't disable WiFi when connecting a cable.

I suppose the person that wrote those lines thought of the following scenario. The user has a mobile hotspot they don't want to run backups over. So they disable the network inside Vorta. At home they plug the Computer into the router with a LAN-cable without disabling the mobile hotspot. The user now expects backups to be run over LAN instead of WIFI.

Thinking about that this reasoning doesn't make much sense though since Vorta doesn't know which connection the packages will be routed through. When running a backup in above scenario the packages might still be sent over the WIFI.

dietmarw commented 1 year ago

I would also argue that the presence of an active Wifi connection is more a confirmation of the location the user is currently in (like home, office, en route)* and hence allowing the backup to start based on that. If the operating system then decided to use the LAN-cable instead, this should not be a reason to not start the backup.

*) I'm well aware that this also has its flaws with for example the "eduroam" network.

dietmarw commented 1 year ago

Thinking more about, if mobile hotspots are an issue then those networks should be set to "metered" by the user which then could be queried by Vorta.

e.g.,

$ nmcli -t -f GENERAL.METERED dev show wlp0s20f3 
GENERAL.METERED:yes

EDIT: Which I just noticed is already an option and which Vorta would not do anyway unless one specifically allows it. So The check should really be changed to active and not just primary connections.