From 12ae3debf5485e257feaa3a33b1a9214ceddadcb Mon Sep 17 00:00:00 2001 From: jwijenbergh Date: Mon, 12 Aug 2024 11:52:30 +0200 Subject: [PATCH 1/6] NM: Calculate WG gateway using eduvpn-common --- eduvpn/app.py | 2 +- eduvpn/nm.py | 13 +++++++------ tests/test_nm.py | 8 ++++---- tests/test_stats.py | 2 +- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/eduvpn/app.py b/eduvpn/app.py index 0d5b722e..ac7a03c6 100644 --- a/eduvpn/app.py +++ b/eduvpn/app.py @@ -588,7 +588,7 @@ def search_custom(self, query: str) -> Iterator[Any]: class Application: def __init__(self, variant: ApplicationVariant, common: EduVPN) -> None: self.variant = variant - self.nm_manager = nm.NMManager(variant) + self.nm_manager = nm.NMManager(variant, common) self.common = common directory = variant.config_prefix self.config = Configuration.load(directory) diff --git a/eduvpn/nm.py b/eduvpn/nm.py index 4b8b81be..3c2d4116 100644 --- a/eduvpn/nm.py +++ b/eduvpn/nm.py @@ -1,5 +1,4 @@ import enum -import ipaddress import logging import os import time @@ -12,7 +11,7 @@ from tempfile import mkdtemp from typing import Any, Callable, Optional, TextIO, Tuple -from eduvpn_common.main import Jar +from eduvpn_common.main import EduVPN, Jar from gi.repository.Gio import Cancellable, Task # type: ignore from eduvpn.ovpn import Ovpn @@ -81,15 +80,16 @@ def from_active_state(cls, state: "NM.ActiveConnectionState") -> "ConnectionStat # A manager for a manager :-) class NMManager: - def __init__(self, variant: ApplicationVariant): + def __init__(self, variant: ApplicationVariant, common_lib: EduVPN): self.variant = variant self.proxy = None try: self._client = NM.Client.new(None) - self.wg_gateway_ip: Optional[ipaddress.IPv4Address] = None + self.wg_gateway_ip: Optional[str] = None except Exception: self._client = None self.cancel_jar = Jar(lambda x: x.cancel()) + self.common_lib = common_lib @property def client(self) -> "NM.Client": @@ -304,7 +304,7 @@ def failover_endpoint_ip(self) -> Optional[str]: if not self.wg_gateway_ip: _logger.debug("no wg gateway ip found in failover endpoint") return None - return str(self.wg_gateway_ip) + return self.wg_gateway_ip else: _logger.debug(f"Unknown protocol: {protocol}") return None @@ -452,7 +452,8 @@ def start_wireguard_connection( # noqa: C901 addr = ip_interface(ip.strip()) if addr.version == 4: if not self.wg_gateway_ip: - self.wg_gateway_ip = addr.network[1] + net_str = str(addr.network) + self.wg_gateway_ip = self.common_lib.calculate_gateway(net_str) ipv4s.append(NM.IPAddress(AF_INET, str(addr.ip), addr.network.prefixlen)) elif addr.version == 6: ipv6s.append(NM.IPAddress(AF_INET6, str(addr.ip), addr.network.prefixlen)) diff --git a/tests/test_nm.py b/tests/test_nm.py index 7bacd2e4..3adb4455 100644 --- a/tests/test_nm.py +++ b/tests/test_nm.py @@ -9,20 +9,20 @@ @skipIf(not NMManager(EDUVPN).available, "Network manager not available") class TestNm(TestCase): def test_nm_available(self): - nm_manager = NMManager(EDUVPN) + nm_manager = NMManager(EDUVPN, None) nm_manager.available def test_import_ovpn(self): - nm_manager = NMManager(EDUVPN) + nm_manager = NMManager(EDUVPN, None) ovpn = Ovpn.parse(mock_config) nm_manager.import_ovpn(ovpn) def test_get_add_connection(self): - nm_manager = NMManager(EDUVPN) + nm_manager = NMManager(EDUVPN, None) ovpn = Ovpn.parse(mock_config) simple_connection = nm_manager.import_ovpn(ovpn) nm_manager.add_connection(simple_connection) def test_get_uuid(self): - nm_manager = NMManager(EDUVPN) + nm_manager = NMManager(EDUVPN, None) nm_manager.uuid diff --git a/tests/test_stats.py b/tests/test_stats.py index 049a8343..eefe7146 100644 --- a/tests/test_stats.py +++ b/tests/test_stats.py @@ -27,7 +27,7 @@ def try_open(path: Path): @patch("eduvpn.nm.NMManager.iface", new_callable=PropertyMock, return_value=MOCK_IFACE) class TestStats(TestCase): def test_stat_bytes(self, _): - nm_manager = NMManager(EDUVPN) + nm_manager = NMManager(EDUVPN, None) with TemporaryDirectory() as tempdir: # Create test data in the wanted files # Use the tempdir so it is cleaned up later From dcfbd175cb2a226725a7afd7d35ace9f2b9560d6 Mon Sep 17 00:00:00 2001 From: jwijenbergh Date: Wed, 23 Oct 2024 11:24:51 +0200 Subject: [PATCH 2/6] ProxyGuard: Update to latest common integration --- eduvpn/app.py | 38 ++++++++++------------------- eduvpn/connection.py | 31 ++++++----------------- eduvpn/nm.py | 38 +++++++++++++++++++++++++---- eduvpn/proxy.py | 58 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 54 deletions(-) create mode 100644 eduvpn/proxy.py diff --git a/eduvpn/app.py b/eduvpn/app.py index ac7a03c6..f5085937 100644 --- a/eduvpn/app.py +++ b/eduvpn/app.py @@ -8,7 +8,7 @@ from eduvpn_common.main import EduVPN, ServerType, WrappedError from eduvpn_common.state import State, StateType -from eduvpn_common.types import ProxyReady, ProxySetup, ReadRxBytes, RefreshList # type: ignore[attr-defined] +from eduvpn_common.types import ProxySetup, ReadRxBytes, RefreshList # type: ignore[attr-defined] from eduvpn import nm from eduvpn.config import Configuration @@ -21,6 +21,7 @@ parse_tokens, ) from eduvpn.keyring import DBusKeyring, InsecureFileKeyring, TokenKeyring +from eduvpn.proxy import Proxy from eduvpn.server import ServerDatabase, parse_profiles, parse_required_transition from eduvpn.utils import ( handle_exception, @@ -144,8 +145,8 @@ def __init__( self.nm_manager = nm_manager self._was_tcp = False self._should_failover = False - self._peer_ips_proxy = None self._refresh_list_handler = RefreshList(self.refresh_list) + self._proxy_setup_handler = ProxySetup(self.on_proxy_setup) @property def keyring(self): @@ -327,22 +328,8 @@ def save_tokens(self, server_id: str, server_type: int, tokens: str): logger.error("Failed saving tokens with exception:") logger.error(e, exc_info=True) - def on_proxy_setup(self, fd, peer_ips): - logger.debug(f"got proxy fd: {fd}, peer_ips: {peer_ips}") - self._peer_ips_proxy = json.loads(peer_ips) - - @run_in_background_thread("start-proxy") - def start_proxy(self, proxy, callback): - try: - self.common.start_proxyguard( - proxy.listen, - proxy.source_port, - proxy.peer, - ProxySetup(self.on_proxy_setup), - ProxyReady(lambda: callback()), - ) - except Exception as e: - handle_exception(self.common, e) + def on_proxy_setup(self, fd): + logger.debug(f"got proxy fd: {fd}") def connect( self, @@ -424,21 +411,22 @@ def connect(config): if not self.common.in_state(State.CONNECTING): self.common.set_state(State.CONNECTING) connection = Connection.parse(config) + proxy = None + if config.proxy is not None: + wrapper_proxy = self.common.new_proxyguard( + config.proxy.listen_port, config.proxy.source_port, config.proxy.peer, self._proxy_setup_handler + ) + proxy = Proxy(self.common, config.proxy, wrapper_proxy) connection.connect( self.nm_manager, config.default_gateway, self.config.allow_wg_lan, config.dns_search_domains, - config.proxy, - self._peer_ips_proxy, + proxy, on_connect, ) - self._peer_ips_proxy = None - if config.proxy: - self.start_proxy(config.proxy, lambda: connect(config)) - else: - connect(config) + connect(config) def reconnect(self, callback: Optional[Callable] = None, prefer_tcp: bool = False): def on_disconnected(success: bool): diff --git a/eduvpn/connection.py b/eduvpn/connection.py index 56b63a0d..23f9caf7 100644 --- a/eduvpn/connection.py +++ b/eduvpn/connection.py @@ -3,7 +3,6 @@ from datetime import datetime, timedelta from enum import IntEnum from typing import Any, Dict, List, Optional -from urllib.parse import urlparse from eduvpn.ovpn import Ovpn @@ -38,35 +37,22 @@ class Protocol(IntEnum): WIREGUARDTCP = 3 -class Proxy: +class ProxyConfig: """The class that represents a proxyguard instance :param: peer: str: The remote peer string - :param: listen: str: The listen proxy string + :param source_port: int: The source port for the TCP connection + :param: listen_port: str: The listen port for the proxy """ def __init__( self, peer: str, source_port: int, - listen: str, + listen_port: int, ): self.peer = peer self.source_port = source_port - self.listen = listen - - @property - def peer_scheme(self) -> str: - try: - parsed = urlparse(self.peer) - return parsed.scheme - except Exception: - return "" - - @property - def peer_port(self): - if self.peer_scheme == "http": - return 80 - return 443 + self.listen_port = listen_port class Config: @@ -82,7 +68,7 @@ def __init__( protocol: Protocol, default_gateway: bool, dns_search_domains: List[str], - proxy: Proxy, + proxy: ProxyConfig, should_failover: bool, ): self.config = config @@ -105,7 +91,7 @@ def parse_config(config_json: str) -> Config: d = json.loads(config_json) proxy = d.get("proxy", None) if proxy: - proxy = Proxy(proxy["peer"], proxy["source_port"], proxy["listen"]) + proxy = ProxyConfig(proxy["peer"], proxy["source_port"], proxy["listen_port"]) cfg = Config( d["config"], Protocol(d["protocol"]), @@ -202,7 +188,6 @@ def connect( allow_lan, dns_search_domains, proxy, - proxy_peer_ips, callback, ): manager.start_openvpn_connection( @@ -231,7 +216,6 @@ def connect( allow_lan, dns_search_domains, proxy, - proxy_peer_ips, callback, ): manager.start_wireguard_connection( @@ -239,6 +223,5 @@ def connect( default_gateway, allow_wg_lan=allow_lan, callback=callback, - proxy_peer_ips=proxy_peer_ips, proxy=proxy, ) diff --git a/eduvpn/nm.py b/eduvpn/nm.py index 3c2d4116..9de6384d 100644 --- a/eduvpn/nm.py +++ b/eduvpn/nm.py @@ -4,10 +4,11 @@ import time import uuid from configparser import ConfigParser +from contextlib import closing from ipaddress import ip_address, ip_interface from pathlib import Path from shutil import rmtree -from socket import AF_INET, AF_INET6, IPPROTO_TCP +from socket import AF_INET, AF_INET6, IPPROTO_TCP, SOCK_DGRAM, socket from tempfile import mkdtemp from typing import Any, Callable, Optional, TextIO, Tuple @@ -16,7 +17,7 @@ from eduvpn.ovpn import Ovpn from eduvpn.storage import get_uuid, set_uuid, write_ovpn -from eduvpn.utils import run_in_glib_thread +from eduvpn.utils import run_in_background_thread, run_in_glib_thread from eduvpn.variants import ApplicationVariant _logger = logging.getLogger(__name__) @@ -78,6 +79,12 @@ def from_active_state(cls, state: "NM.ActiveConnectionState") -> "ConnectionStat raise ValueError(state) +def find_free_udp_port(): + with closing(socket(AF_INET, SOCK_DGRAM)) as s: + s.bind(("", 0)) + return s.getsockname()[1] + + # A manager for a manager :-) class NMManager: def __init__(self, variant: ApplicationVariant, common_lib: EduVPN): @@ -101,6 +108,15 @@ def client(self) -> "NM.Client": def available(self) -> bool: return self._client is not None + @run_in_background_thread("proxyguard-tunnel") + def proxy_tunnel(self, listen_port): + assert self.proxy is not None + _logger.debug(f"tunneling proxyguard with listen port: {listen_port}") + try: + self.proxy.tunnel(listen_port) + except Exception as e: + self.proxy.forward_exception(e) + # TODO: Move this somewhere else? def open_stats_file(self, filename: str) -> Optional[TextIO]: """ @@ -441,7 +457,6 @@ def start_wireguard_connection( # noqa: C901 *, allow_wg_lan=False, proxy=None, - proxy_peer_ips=None, callback=None, ) -> None: _logger.debug("writing wireguard configuration to Network Manager") @@ -547,6 +562,13 @@ def start_wireguard_connection( # noqa: C901 fwmark = int(os.environ.get("EDUVPN_WG_FWMARK", 51860)) listen_port = int(os.environ.get("EDUVPN_WG_LISTEN_PORT", 0)) + # if we are using proxyguard we need to know the port beforehand + # just get an available port + # Yes, this can race but there is not other option right now + # the NM API just gives port 0 from device.get_listen_port() + if proxy is not None and listen_port == 0: + listen_port = find_free_udp_port() + s_ip4.set_property(NM.SETTING_IP_CONFIG_ROUTE_TABLE, fwmark) s_ip6.set_property(NM.SETTING_IP_CONFIG_ROUTE_TABLE, fwmark) w_con.set_property(NM.DEVICE_WIREGUARD_FWMARK, fwmark) @@ -572,7 +594,7 @@ def start_wireguard_connection( # noqa: C901 if proxy: dport_proxy = proxy.peer_port - for proxy_peer_ip in proxy_peer_ips: + for proxy_peer_ip in proxy.peer_ips: address = ip_address(proxy_peer_ip) if address.version != ipver: continue @@ -613,7 +635,13 @@ def start_wireguard_connection( # noqa: C901 self.proxy = proxy - self.set_connection(profile, callback) # type: ignore + def set_callback(success: bool): + if success and proxy is not None: + self.proxy_tunnel(listen_port) + if callback is not None: + callback(success) + + self.set_connection(profile, set_callback) # type: ignore def new_cancellable(self): c = Cancellable.new() diff --git a/eduvpn/proxy.py b/eduvpn/proxy.py new file mode 100644 index 00000000..c6316e46 --- /dev/null +++ b/eduvpn/proxy.py @@ -0,0 +1,58 @@ +from typing import List +from urllib.parse import urlparse + +from eduvpn.utils import handle_exception + + +class Proxy: + """The class that represents a proxyguard instance + :param: common: The common library + :param: peer: str: The remote peer string + :param: listen: str: The listen proxy string + """ + + def __init__( + self, + common, + config, + wrapper, + ): + self.common = common + self.config = config + self.wrapper = wrapper + + def forward_exception(self, error): + handle_exception(self.common, error) + + def tunnel(self, wgport): + self.wrapper.tunnel(wgport) + + @property + def peer(self) -> str: + return self.config.peer + + @property + def source_port(self) -> int: + return self.config.source_port + + @property + def listen_port(self) -> int: + return self.config.listen_port + + @property + def peer_ips(self) -> List[str]: + return self.wrapper.peer_ips + + @property + def peer_scheme(self) -> str: + try: + parsed = urlparse(self.config.peer) + return parsed.scheme + except Exception: + return "" + + @property + def peer_port(self): + if self.peer_scheme == "http": + return 80 + return 443 From e2cc8baf8b4859827ecb9db1bceffa34886c5d11 Mon Sep 17 00:00:00 2001 From: jwijenbergh Date: Tue, 29 Oct 2024 14:17:07 +0100 Subject: [PATCH 3/6] All: Remove dead code --- eduvpn/config.py | 4 +-- eduvpn/nm.py | 23 +--------------- eduvpn/server.py | 25 ----------------- eduvpn/storage.py | 1 - eduvpn/ui/app.py | 2 +- eduvpn/ui/ui.py | 69 ++++++++++++++--------------------------------- 6 files changed, 24 insertions(+), 100 deletions(-) diff --git a/eduvpn/config.py b/eduvpn/config.py index 60532358..eb59bb5a 100644 --- a/eduvpn/config.py +++ b/eduvpn/config.py @@ -19,10 +19,10 @@ class SettingDescriptor(Generic[T]): - def __set_name__(self, owner: Type["Configuration"], name: str) -> None: + def __set_name__(self, _owner: Type["Configuration"], name: str) -> None: self.name = name - def __get__(self, instance: Type["Configuration"], owner: Type["Configuration"]) -> bool: + def __get__(self, instance: Type["Configuration"], _owner: Type["Configuration"]) -> bool: return instance.get_setting(self.name) # type: ignore def __set__(self, instance: Type["Configuration"], value: T) -> None: diff --git a/eduvpn/nm.py b/eduvpn/nm.py index 9de6384d..ac0a37d9 100644 --- a/eduvpn/nm.py +++ b/eduvpn/nm.py @@ -10,7 +10,7 @@ from shutil import rmtree from socket import AF_INET, AF_INET6, IPPROTO_TCP, SOCK_DGRAM, socket from tempfile import mkdtemp -from typing import Any, Callable, Optional, TextIO, Tuple +from typing import Any, Callable, Optional, TextIO from eduvpn_common.main import EduVPN, Jar from gi.repository.Gio import Cancellable, Task # type: ignore @@ -348,17 +348,6 @@ def ovpn_import(self, target: Path) -> Optional["NM.Connection"]: conn.normalize() return conn - def import_ovpn_with_certificate(self, ovpn: Ovpn, private_key: str, certificate: str) -> "NM.SimpleConnection": - """ - Import the OVPN string into Network Manager. - """ - target_parent = Path(mkdtemp()) - target = target_parent / f"{self.variant.name}.ovpn" - write_ovpn(ovpn, private_key, certificate, target) - connection = self.ovpn_import(target) - rmtree(target_parent) - return connection - def import_ovpn(self, ovpn: Ovpn) -> "NM.SimpleConnection": """ Import the OVPN string into Network Manager. @@ -911,16 +900,6 @@ def wrapped_connection_added(client: "NM.Client", active_con: "NM.ActiveConnecti self.client.connect("active-connection-added", wrapped_connection_added) return True - def connection_status( - self, - ) -> Tuple[Optional[str], Optional["NM.ActiveConnectionState"]]: - con = self.client.get_primary_connection() - if not isinstance(con, NM.VpnConnection): - return None, None - uuid = con.get_uuid() - status = con.get_state() - return uuid, status - def action_with_mainloop(action: Callable): _logger.debug("calling action with CLI mainloop") diff --git a/eduvpn/server.py b/eduvpn/server.py index d5299a6e..a546cd0b 100644 --- a/eduvpn/server.py +++ b/eduvpn/server.py @@ -15,7 +15,6 @@ from eduvpn.settings import IMAGE_PREFIX logger = logging.getLogger(__name__) -TranslatedStr = Union[str, Dict[str, str]] class Profile: @@ -85,14 +84,6 @@ def identifier(self) -> str: def category_id(self) -> ServerType: return ServerType.CUSTOM - @property - def category(self) -> str: - """Return the category of the server as a string - :return: The category string - :rtype: str - """ - return str(self.category_id) - class InstituteServer(Server): """The class that represents an Institute Access Server @@ -117,14 +108,6 @@ def __init__( def category_id(self) -> ServerType: return ServerType.INSTITUTE_ACCESS - @property - def category(self) -> str: - """Return the category of the server as a string - :return: The category string - :rtype: str - """ - return str(self.category_id) - class SecureInternetServer(Server): """The class that represents a Secure Internet Server @@ -157,14 +140,6 @@ def __init__( def category_id(self) -> ServerType: return ServerType.SECURE_INTERNET - @property - def category(self) -> str: - """Return the category of the server as a string - :return: The category string - :rtype: str - """ - return str(self.category_id) - def parse_secure_internet(si: dict) -> Optional[SecureInternetServer]: profiles = parse_profiles(si["profiles"]) diff --git a/eduvpn/storage.py b/eduvpn/storage.py index 2c933c74..5e1ba41c 100644 --- a/eduvpn/storage.py +++ b/eduvpn/storage.py @@ -2,7 +2,6 @@ This module contains code to maintain a simple metadata storage in ~/.config/eduvpn/ """ -from os import PathLike from typing import Optional from eduvpn.ovpn import Ovpn diff --git a/eduvpn/ui/app.py b/eduvpn/ui/app.py index 82ad2edd..6f160b7c 100644 --- a/eduvpn/ui/app.py +++ b/eduvpn/ui/app.py @@ -139,7 +139,7 @@ def expired_deactivate(): expired_deactivate() @ui_transition(State.GOT_CONFIG, StateType.ENTER) - def enter_NoActiveConnection(self, old_state, new_state): + def enter_NoActiveConnection(self, _old_state, _new_state): if not self.window.is_visible(): # Quit the app if no window is open when the connection is deactivated. logger.debug("connection deactivated while window closed") diff --git a/eduvpn/ui/ui.py b/eduvpn/ui/ui.py index 2103ed23..751b70c5 100644 --- a/eduvpn/ui/ui.py +++ b/eduvpn/ui/ui.py @@ -98,7 +98,6 @@ def is_dark(rgb): class ValidityTimers: def __init__(self): self.cancel_timers = [] - self.num = 0 def add_absolute(self, call: Callable, abstime: datetime): delta = abstime - datetime.now() @@ -182,7 +181,6 @@ def setup(self, builder: Builder, application: Type["EduVpnGtkApplication"]) -> # Whether or not the profile that is selected is the 'same' one as before # This is used so it doesn't fully trigger the callback self.set_same_profile = False - self.is_selected = False self.app_logo = builder.get_object("appLogo") self.app_logo_info = builder.get_object("appLogoInfo") @@ -681,29 +679,6 @@ def update_connection_validity(self, validity: Validity) -> None: self.eduvpn_app.enter_SessionExpiredState() - # Shows notifications according to https://docs.eduvpn.org/server/v3/client-implementation-notes.html#expiry - # The 0th case is handled with a separate notification inside of the expiry text handler - def ensure_expiry_notification_text(self, validity: Validity) -> None: - hours = [4, 2, 1] - for h in hours: - if h in self.shown_notification_times: - continue - delta = validity.remaining - timedelta(hours=h) - total_secs = delta.total_seconds() - if total_secs <= 0 and total_secs >= -120: - self.eduvpn_app.enter_SessionPendingExpiryState(h) - self.shown_notification_times.add(h) - break - - # Show renew button or not - def update_connection_renew(self, expire_time) -> None: - if self.app.model.should_renew_button(): - # Show renew button - self.renew_session_button.show() - - validity = self.app.model.get_expiry(expire_time) - self.ensure_expiry_notification_text(validity) - def update_connection_status(self, connected: bool) -> None: if connected: self.connection_status_label.set_text(_("Connected")) @@ -805,10 +780,6 @@ def exit_search(self): search.show_search_components(self, False) search.exit_server_search(self) - def exit_ConfigureCustomServer(self, old_state, new_state): - if not self.app.variant.use_predefined_servers: - self.add_custom_server_button_container.hide() - def fill_secure_location_combo(self, curr, locs): locs_store = Gtk.ListStore(GdkPixbuf.Pixbuf, GObject.TYPE_STRING, GObject.TYPE_STRING) active_loc = 0 @@ -869,46 +840,46 @@ def enter_MainState(self, old_state: str, servers): self.app.config.ignore_keyring_warning = self.keyring_do_not_show.get_active() @ui_transition(State.MAIN, StateType.LEAVE) - def exit_MainState(self, old_state, new_state): + def exit_MainState(self, _old_state, _data): search.show_result_components(self, False) self.add_other_server_button_container.hide() search.exit_server_search(self) self.change_location_combo.hide() @ui_transition(State.OAUTH_STARTED, StateType.ENTER) - def enter_oauth_setup(self, old_state, url): + def enter_oauth_setup(self, _old_state, data): self.show_page(self.oauth_page) self.oauth_cancel_button.show() @ui_transition(State.OAUTH_STARTED, StateType.LEAVE) - def exit_oauth_setup(self, old_state, data): + def exit_oauth_setup(self, _old_state, _data): self.hide_page(self.oauth_page) self.oauth_cancel_button.hide() @ui_transition(State.ADDING_SERVER, StateType.ENTER) - def enter_chosenServerInformation(self, new_state, data): + def enter_chosenServerInformation(self, _old_state, _data): self.show_loading_page( _("Adding server"), _("Loading server information..."), ) @ui_transition(State.ADDING_SERVER, StateType.LEAVE) - def exit_chosenServerInformation(self, old_state, data): + def exit_chosenServerInformation(self, _old_state, _data): self.hide_loading_page() @ui_transition(State.GETTING_CONFIG, StateType.ENTER) - def enter_GettingConfig(self, new_state, data): + def enter_GettingConfig(self, _old_state, _data): self.show_loading_page( _("Getting a VPN configuration"), _("Loading server information..."), ) @ui_transition(State.GETTING_CONFIG, StateType.LEAVE) - def exit_GettingConfig(self, old_state, data): + def exit_GettingConfig(self, _old_state, _data): self.hide_loading_page() @ui_transition(State.ASK_PROFILE, StateType.ENTER) - def enter_ChooseProfile(self, new_state, data): + def enter_ChooseProfile(self, _old_state, data): self.show_back_button(True) self.show_page(self.choose_profile_page) self.profile_list.show() @@ -935,13 +906,13 @@ def enter_ChooseProfile(self, new_state, data): profiles_list_model.append([str(profile), (setter, profile_id)]) @ui_transition(State.ASK_PROFILE, StateType.LEAVE) - def exit_ChooseProfile(self, old_state, data): + def exit_ChooseProfile(self, old_state, _data): self.show_back_button(False) self.hide_page(self.choose_profile_page) self.profile_list.hide() @ui_transition(State.ASK_LOCATION, StateType.ENTER) - def enter_ChooseSecureInternetLocation(self, old_state, data): + def enter_ChooseSecureInternetLocation(self, _old_state, data): self.show_back_button(True) self.show_page(self.choose_location_page) self.location_list.show() @@ -979,7 +950,7 @@ def enter_ChooseSecureInternetLocation(self, old_state, data): location_list_model.append([retrieve_country_name(location), flag, (setter, location)]) @ui_transition(State.ASK_LOCATION, StateType.LEAVE) - def exit_ChooseSecureInternetLocation(self, old_state, new_state): + def exit_ChooseSecureInternetLocation(self, _old_state, _data): self.show_back_button(False) self.hide_loading_page() self.hide_page(self.choose_location_page) @@ -990,7 +961,7 @@ def enter_GotConfig(self, old_state: str, server_info) -> None: self.enter_connecting(old_state, server_info) @ui_transition(State.DISCONNECTED, StateType.ENTER) - def enter_ConnectionStatus(self, old_state: str, server_info): + def enter_ConnectionStatus(self, _old_state: str, server_info): self.show_back_button(True) self.show_page(self.connection_page) self.update_connection_status(False) @@ -1005,12 +976,12 @@ def enter_ConnectionStatus(self, old_state: str, server_info): self.renew_session_button.hide() @ui_transition(State.DISCONNECTED, StateType.LEAVE) - def exit_ConnectionStatus(self, old_state, new_state): + def exit_ConnectionStatus(self, _old_state, _data): self.show_back_button(False) self.hide_page(self.connection_page) @ui_transition(State.CONNECTED, StateType.LEAVE) - def leave_ConnectedState(self, old_state, server_info): + def leave_ConnectedState(self, _old_state, _server_info): logger.debug("leave connected state") self.reconnect_tcp_button.hide() self.reconnect_tcp_text.hide() @@ -1025,7 +996,7 @@ def leave_ConnectedState(self, old_state, server_info): self.stop_connection_info() @ui_transition(State.CONNECTED, StateType.ENTER) - def enter_ConnectedState(self, old_state, server_data): + def enter_ConnectedState(self, _old_state, server_data): self.renew_session_button.hide() server_info, validity = server_data self.connection_info_expander.show() @@ -1081,11 +1052,11 @@ def stop_validity_threads(self) -> None: self.connection_validity_thread_cancel = None self.connection_validity_timers.clean() - def on_info_delete(self, widget, event): + def on_info_delete(self, widget, _): logger.debug("info dialog delete event") return widget.hide_on_delete() - def on_info_button(self, widget: EventBox, event: EventButton) -> None: + def on_info_button(self, _box: EventBox, _button: EventButton) -> None: logger.debug("clicked info button") self.info_dialog.set_title(f"{self.app.variant.name} - Info") self.info_dialog.show() @@ -1093,7 +1064,7 @@ def on_info_button(self, widget: EventBox, event: EventButton) -> None: self.info_dialog.hide() @ui_transition(SERVER_LIST_REFRESH_STATE, StateType.ENTER) # type: ignore - def enter_server_list_refresh(self, old_state, servers) -> None: + def enter_server_list_refresh(self, _old_state, servers) -> None: logger.debug("server list refresh") if self.is_searching_server: return @@ -1233,7 +1204,7 @@ def on_change_location(self, combo): # Set profile and connect self.call_model("change_secure_location", location) - def on_search_changed(self, _: Optional[SearchEntry] = None) -> None: + def on_search_changed(self, _searchentry: Optional[SearchEntry] = None) -> None: query = self.find_server_search_input.get_text() if self.app.variant.use_predefined_servers and query.count(".") < 2: results = self.app.model.search_predefined(query) @@ -1455,7 +1426,7 @@ def on_renew(success: bool): def on_reconnect_tcp_clicked(self, event): logger.debug("clicked on reconnect TCP") - def on_reconnected(_: bool): + def on_reconnected(_success: bool): logger.debug("done reconnecting with tcp") self.reconnect_tcp_button.hide() self.reconnect_tcp_text.hide() From 353c0099ee6903dd9f46a67c7d1f68b83f15de30 Mon Sep 17 00:00:00 2001 From: jwijenbergh Date: Tue, 29 Oct 2024 14:17:26 +0100 Subject: [PATCH 4/6] OpenVPN: Remove parser It was previously needed as we had to replace some stuff in the config IIRC. This is no longer needed, so instead of parsing and then recombining the config, let's pass it around as a string --- eduvpn/connection.py | 11 ++--- eduvpn/nm.py | 9 ++-- eduvpn/ovpn.py | 99 -------------------------------------------- eduvpn/storage.py | 12 ------ tests/test_nm.py | 7 +--- 5 files changed, 10 insertions(+), 128 deletions(-) delete mode 100644 eduvpn/ovpn.py diff --git a/eduvpn/connection.py b/eduvpn/connection.py index 23f9caf7..0b4f60c5 100644 --- a/eduvpn/connection.py +++ b/eduvpn/connection.py @@ -4,8 +4,6 @@ from enum import IntEnum from typing import Any, Dict, List, Optional -from eduvpn.ovpn import Ovpn - class Token: """The class that represents oauth Tokens @@ -172,14 +170,13 @@ def connect(self, callback): class OpenVPNConnection(Connection): - def __init__(self, ovpn: Ovpn): - self.ovpn = ovpn + def __init__(self, config_str): + self.config_str = config_str super().__init__() @classmethod def parse(cls, config_str: str) -> "OpenVPNConnection": # type: ignore - ovpn = Ovpn.parse(config_str) - return cls(ovpn=ovpn) + return cls(config_str=config_str) def connect( self, @@ -191,7 +188,7 @@ def connect( callback, ): manager.start_openvpn_connection( - self.ovpn, + self.config_str, default_gateway, dns_search_domains, callback=callback, diff --git a/eduvpn/nm.py b/eduvpn/nm.py index ac0a37d9..0d71f6e2 100644 --- a/eduvpn/nm.py +++ b/eduvpn/nm.py @@ -15,8 +15,7 @@ from eduvpn_common.main import EduVPN, Jar from gi.repository.Gio import Cancellable, Task # type: ignore -from eduvpn.ovpn import Ovpn -from eduvpn.storage import get_uuid, set_uuid, write_ovpn +from eduvpn.storage import get_uuid, set_uuid from eduvpn.utils import run_in_background_thread, run_in_glib_thread from eduvpn.variants import ApplicationVariant @@ -348,7 +347,7 @@ def ovpn_import(self, target: Path) -> Optional["NM.Connection"]: conn.normalize() return conn - def import_ovpn(self, ovpn: Ovpn) -> "NM.SimpleConnection": + def import_ovpn(self, ovpn: str) -> "NM.SimpleConnection": """ Import the OVPN string into Network Manager. """ @@ -356,7 +355,7 @@ def import_ovpn(self, ovpn: Ovpn) -> "NM.SimpleConnection": target = target_parent / f"{self.variant.name}.ovpn" _logger.debug(f"Writing configuration to {target}") with open(target, mode="w+t") as f: - ovpn.write(f) + f.write(ovpn) connection = self.ovpn_import(target) rmtree(target_parent) return connection @@ -401,7 +400,7 @@ def set_setting_ensure_permissions(self, con: "NM.SimpleConnection") -> "NM.Simp con.add_setting(s_con) return con - def start_openvpn_connection(self, ovpn: Ovpn, default_gateway, dns_search_domains, *, callback=None) -> None: + def start_openvpn_connection(self, ovpn: str, default_gateway, dns_search_domains, *, callback=None) -> None: _logger.debug("writing ovpn configuration to Network Manager") new_con = self.import_ovpn(ovpn) s_ip4 = new_con.get_setting_ip4_config() diff --git a/eduvpn/ovpn.py b/eduvpn/ovpn.py deleted file mode 100644 index f9cae47c..00000000 --- a/eduvpn/ovpn.py +++ /dev/null @@ -1,99 +0,0 @@ -from io import StringIO, TextIOWrapper -from typing import Iterable, List - - -class Item: - def to_string(self) -> str: - raise NotImplementedError - - def write(self, file: TextIOWrapper) -> None: - line = self.to_string() - file.write(f"{line}\n") - - def __eq__(self, other): - return self.__class__ is other.__class__ and self.__dict__ == other.__dict__ - - def __repr__(self): - fields = ",".join(f" {k}={v!r}" for k, v in self.__dict__.items()) - return f"<{self.__class__.__name__}{fields}>" - - -class Field(Item): - def __init__(self, name: str, arguments: List[str]) -> None: - self.name = name - self.arguments = arguments - - def to_string(self) -> str: - return f'{self.name} {" ".join(self.arguments)}' - - -class Section(Item): - def __init__(self, tag: str, content: List[str]) -> None: - self.tag = tag - self.content = content - - def to_string(self) -> str: - lines = [] - lines.append(f"<{self.tag}>") - lines.extend(self.content) - lines.append(f"") - return "\n".join(lines) - - -class Comment(Item): - def __init__(self, content: str) -> None: - self.content = content - - def to_string(self) -> str: - return f"#{self.content}" - - -class Empty(Item): - def to_string(self): - return "" - - -class InvalidOVPN(Exception): - pass - - -def parse_ovpn(lines: Iterable[str]) -> Iterable[Item]: - current_section = None - for _lineno, line in enumerate(lines): - if current_section is not None: - if line.strip() == f"": - yield current_section - current_section = None - else: - current_section.content.append(line) - elif line.startswith("#"): - yield Comment(line.rstrip()[1:]) - elif line.startswith("<"): - line = line.rstrip() - assert line.endswith(">") - section = Section(line[1:-1], []) - current_section = section - elif line.rstrip() == "": - yield Empty() - else: - field_name, *arguments = line.rstrip().split() - yield Field(field_name, arguments) - assert current_section is None - - -class Ovpn: - def __init__(self, content: List[Item]) -> None: - self.content = content - - @classmethod - def parse(cls, content: str) -> "Ovpn": - return cls(list(parse_ovpn(content.splitlines()))) - - def write(self, file: TextIOWrapper) -> None: - for item in self.content: - item.write(file) - - def to_string(self) -> str: - file = StringIO() - self.write(file) - return file.getvalue() diff --git a/eduvpn/storage.py b/eduvpn/storage.py index 5e1ba41c..8235b862 100644 --- a/eduvpn/storage.py +++ b/eduvpn/storage.py @@ -4,7 +4,6 @@ from typing import Optional -from eduvpn.ovpn import Ovpn from eduvpn.settings import CONFIG_DIR_MODE, CONFIG_PREFIX from eduvpn.utils import get_logger @@ -45,17 +44,6 @@ def set_setting(variant, what: str, value: str): f.write(value) -def write_ovpn(ovpn: Ovpn, private_key: str, certificate: str, target: PathLike): - """ - Write the OVPN configuration file to target. - """ - logger.info(f"Writing configuration to {target}") - with open(target, mode="w+t") as f: - ovpn.write(f) - f.writelines(f"\n\n{private_key}\n\n") - f.writelines(f"\n\n{certificate}\n\n") - - def get_uuid(variant) -> Optional[str]: """ Read the UUID of the last generated eduVPN Network Manager connection. diff --git a/tests/test_nm.py b/tests/test_nm.py index 3adb4455..a93251bb 100644 --- a/tests/test_nm.py +++ b/tests/test_nm.py @@ -1,7 +1,6 @@ from unittest import TestCase, skipIf from eduvpn.nm import NMManager -from eduvpn.ovpn import Ovpn from eduvpn.variants import EDUVPN from tests.mock_config import mock_config @@ -14,13 +13,11 @@ def test_nm_available(self): def test_import_ovpn(self): nm_manager = NMManager(EDUVPN, None) - ovpn = Ovpn.parse(mock_config) - nm_manager.import_ovpn(ovpn) + nm_manager.import_ovpn(mock_config) def test_get_add_connection(self): nm_manager = NMManager(EDUVPN, None) - ovpn = Ovpn.parse(mock_config) - simple_connection = nm_manager.import_ovpn(ovpn) + simple_connection = nm_manager.import_ovpn(mock_config) nm_manager.add_connection(simple_connection) def test_get_uuid(self): From ae63da468a54c8a8d049aab1feaf79fd78619b5d Mon Sep 17 00:00:00 2001 From: jwijenbergh Date: Mon, 9 Dec 2024 14:43:13 +0100 Subject: [PATCH 5/6] Profile: Implement priority sorting --- eduvpn/cli.py | 2 +- eduvpn/server.py | 11 +++++++++-- eduvpn/ui/ui.py | 19 +++++++++++++------ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/eduvpn/cli.py b/eduvpn/cli.py index 5899ded8..5ab243fe 100644 --- a/eduvpn/cli.py +++ b/eduvpn/cli.py @@ -67,7 +67,7 @@ def ask_profiles(setter, profiles, current: Optional[Profile] = None) -> bool: return False # Multiple profiles, print the index - sorted_profiles = sorted(profiles.profiles.items(), key=lambda pair: str(pair[1])) + sorted_profiles = sorted(profiles.profiles.items(), key=lambda pair: pair[1]) # Multiple profiles, print the index index = 0 choices = [] diff --git a/eduvpn/server.py b/eduvpn/server.py index a546cd0b..bf3aebdb 100644 --- a/eduvpn/server.py +++ b/eduvpn/server.py @@ -22,16 +22,23 @@ class Profile: :param: identifier: str: The identifier (id) of the profile :param: display_name: str: The display name of the profile :param: default_gateway: str: Whether or not this profile should have the default gateway set + :param: priority: int: The priority of the profile for sorting in the UI """ - def __init__(self, identifier: str, display_name: Dict[str, str], default_gateway: bool): + def __init__(self, identifier: str, display_name: Dict[str, str], default_gateway: bool, priority: int): self.identifier = identifier self.display_name = display_name self.default_gateway = default_gateway + self.priority = priority def __str__(self): return extract_translation(self.display_name) + def __lt__(self, o) -> bool: + if self.priority == o.priority: + return str(self) < str(o) + return self.priority > o.priority + class Profiles: """The class that represents a list of profiles @@ -178,7 +185,7 @@ def parse_profiles(profiles: dict) -> Profiles: profile_map = profiles.get("map", {}) for k, v in profile_map.items(): # TODO: Default gateway - returned[k] = Profile(k, v["display_name"], False) + returned[k] = Profile(k, v["display_name"], False, int(v.get("priority", 0))) return Profiles(returned, profiles["current"]) diff --git a/eduvpn/ui/ui.py b/eduvpn/ui/ui.py index 751b70c5..7d18ab26 100644 --- a/eduvpn/ui/ui.py +++ b/eduvpn/ui/ui.py @@ -589,7 +589,7 @@ def hide_page(self, page: Box) -> None: def get_profile_combo_sorted(self, server_info) -> Tuple[int, Gtk.ListStore]: profile_store = Gtk.ListStore(GObject.TYPE_STRING, GObject.TYPE_PYOBJECT) # type: ignore active_profile = 0 - sorted_profiles = sorted(server_info.profiles.profiles.items(), key=lambda v: str(v[1])) + sorted_profiles = sorted(server_info.profiles.profiles.items(), key=lambda v: v[1]) index = 0 for _id, profile in sorted_profiles: if _id == server_info.profiles.current_id: @@ -899,11 +899,18 @@ def enter_ChooseProfile(self, _old_state, data): profile_tree_view.append_column(column) sorted_model = Gtk.TreeModelSort(model=profiles_list_model) - sorted_model.set_sort_column_id(0, Gtk.SortType.ASCENDING) + + def custom_sort_func(model, iter1, iter2, user_data): + obj1 = model.get_value(iter1, 1)[2] + obj2 = model.get_value(iter2, 1)[2] + return -1 if obj1 < obj2 else (1 if obj2 < obj1 else 0) + + sorted_model.set_sort_func(1, custom_sort_func, None) + sorted_model.set_sort_column_id(1, Gtk.SortType.ASCENDING) profile_tree_view.set_model(sorted_model) profiles_list_model.clear() for profile_id, profile in profiles.profiles.items(): - profiles_list_model.append([str(profile), (setter, profile_id)]) + profiles_list_model.append([str(profile), (setter, profile_id, profile)]) @ui_transition(State.ASK_PROFILE, StateType.LEAVE) def exit_ChooseProfile(self, old_state, _data): @@ -1327,13 +1334,13 @@ def on_toggle_connection_info(self, _): def on_profile_row_activated(self, widget: TreeView, row: TreePath, _col: TreeViewColumn) -> None: model = widget.get_model() - setter, profile = model[row][1] - logger.debug(f"activated profile: {profile!r}") + setter, profile_id, _ = model[row][1] + logger.debug(f"activated profile: {profile_id!r}") @run_in_background_thread("set-profile") def set_profile(): try: - setter(profile) + setter(profile_id) except Exception as e: if should_show_error(e): self.show_error_revealer(str(e)) From 640559f7aa20c891375210acb2d38f00e787f2a4 Mon Sep 17 00:00:00 2001 From: jwijenbergh Date: Mon, 9 Dec 2024 15:59:02 +0100 Subject: [PATCH 6/6] Setup.cfg: Require common 2.99.0 --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 3031b8ef..49a2a9bb 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,7 +31,7 @@ packages = eduvpn.ui python_requires = >= 3.6 install_requires = - eduvpn_common >= 2.1.0,< 3.0.0 + eduvpn_common >= 2.99.0,< 3.0.0 pygobject [options.package_data]