tor: Simplify upstream bridges action code

- There is no need for separate subcommand as arguments can be passed in
  even when they have special characters such as \n in them.

- Enable obfs3 and scramblesuit transports also.  Since it is easy to
  support them and bridges site still provides them.  scramblesuit is
  also a nice alternative to obfs4.

- Minor refactoring/fixes.

actions/tor

@@ -66,11 +66,8 @@ def parse_arguments():
     configure.add_argument('--use-upstream-bridges',
                            choices=['enable', 'disable'],
                            help='Configure use of upstream bridges')
-
-    upstream = subparsers.add_parser(
-        'set-upstream-bridges', help='Set list of upstream bridges')
-    upstream.add_argument('--bridges',
-                          help='List of upstream bridges to use')
+    configure.add_argument('--upstream-bridges',
+                           help='Set list of upstream bridges to use')
 
     subparsers.add_parser('restart', help='Restart Tor')
 
@@ -154,6 +151,10 @@ def subcommand_configure(arguments):
     if arguments.use_upstream_bridges == 'enable':
         arguments.relay = 'disable'
         arguments.bridge_relay = 'disable'
+
+    if arguments.upstream_bridges:
+        _set_upstream_bridges(arguments.upstream_bridges, aug=aug)
+
     _enable_relay(arguments.relay, arguments.bridge_relay, aug=aug)
 
     if arguments.hidden_service == 'enable':
@@ -170,23 +171,6 @@ def subcommand_configure(arguments):
         _disable_apt_transport_tor()
 
 
-def subcommand_set_upstream_bridges(arguments):
-    """Set list of upstream bridges."""
-    aug = augeas_load()
-
-    aug.remove(TOR_CONFIG + '/Bridge')
-    if arguments.bridges:
-        bridges = arguments.bridges.split('\n')
-        for bridge in bridges:
-            if bridge.strip():
-                aug.set(TOR_CONFIG + '/Bridge[last() + 1]', bridge.strip())
-
-    aug.set(TOR_CONFIG + '/ClientTransportPlugin',
-            'obfs4 exec /usr/bin/obfs4proxy')
-
-    aug.save()
-
-
 def subcommand_restart(_):
     """Restart Tor."""
     if is_enabled() and is_running():
@@ -341,6 +325,29 @@ def _use_upstream_bridges(use_upstream_bridges=None, aug=None):
     aug.save()
 
 
+def _set_upstream_bridges(upstream_bridges=None, aug=None):
+    """Set list of upstream bridges."""
+    if upstream_bridges is None:
+        return
+
+    if not aug:
+        aug = augeas_load()
+
+    aug.remove(TOR_CONFIG + '/Bridge')
+    if upstream_bridges:
+        bridges = [bridge.strip() for bridge in upstream_bridges.split('\n')]
+        bridges = [bridge for bridge in bridges if bridge]
+        for bridge in bridges:
+            parts = [part for part in bridge.split() if part]
+            bridge = ' '.join(parts)
+            aug.set(TOR_CONFIG + '/Bridge[last() + 1]', bridge.strip())
+
+    aug.set(TOR_CONFIG + '/ClientTransportPlugin',
+            'obfs3,scramblesuit,obfs4 exec /usr/bin/obfs4proxy')
+
+    aug.save()
+
+
 def _enable_relay(relay=None, bridge=None, aug=None):
     """Enable Tor bridge relay."""
     if relay is None and bridge is None:
@@ -349,7 +356,7 @@ def _enable_relay(relay=None, bridge=None, aug=None):
     if not aug:
         aug = augeas_load()
 
-    use_upstream_bridges = aug.get(TOR_CONFIG + '/UseBridges') == '1'
+    use_upstream_bridges = _are_upstream_bridges_enabled(aug)
 
     if relay == 'enable' and not use_upstream_bridges:
         aug.set(TOR_CONFIG + '/ORPort', 'auto')
@@ -477,7 +484,8 @@ def augeas_load():
     aug = augeas.Augeas(flags=augeas.Augeas.NO_LOAD +
                         augeas.Augeas.NO_MODL_AUTOLOAD)
     aug.set('/augeas/load/Tor/lens', 'Tor.lns')
-    aug.set('/augeas/load/Tor/incl[last() + 1]', '/etc/tor/instances/plinth/torrc')
+    aug.set('/augeas/load/Tor/incl[last() + 1]',
+            '/etc/tor/instances/plinth/torrc')
     aug.load()
     return aug
 

plinth/modules/tor/views.py

@@ -74,17 +74,7 @@ def __apply_changes(request, old_status, new_status):
         # Already running a configuration task
         return
 
-    setting_changed = False
     needs_restart = False
-
-    if old_status['upstream_bridges'] != new_status['upstream_bridges']:
-        if new_status['enabled'] and new_status['use_upstream_bridges']:
-            actions.superuser_run(
-                'tor', ['set-upstream-bridges', '--bridges',
-                        new_status['upstream_bridges']])
-            setting_changed = True
-            needs_restart = True
-
     arguments = []
 
     if old_status['relay_enabled'] != new_status['relay_enabled']:
@@ -120,6 +110,11 @@ def __apply_changes(request, old_status, new_status):
         arguments.extend(['--use-upstream-bridges', arg_value])
         needs_restart = True
 
+    if old_status['upstream_bridges'] != new_status['upstream_bridges']:
+        arguments.extend(['--upstream-bridges',
+                          new_status['upstream_bridges']])
+        needs_restart = True
+
     if old_status['enabled'] != new_status['enabled']:
         arg_value = 'enable' if new_status['enabled'] else 'disable'
         arguments.extend(['--service', arg_value])
@@ -129,14 +124,13 @@ def __apply_changes(request, old_status, new_status):
 
     if arguments:
         actions.superuser_run('tor', ['configure'] + arguments)
-        setting_changed = True
         if not needs_restart:
             messages.success(request, _('Configuration updated.'))
 
     if needs_restart and new_status['enabled']:
         config_process = actions.superuser_run('tor', ['restart'], async=True)
 
-    if not setting_changed:
+    if not arguments:
         messages.info(request, _('Setting unchanged'))