From 8858774b354f7c8f9947c5f3fef30b01abd20264 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Fri, 2 Oct 2026 07:54:12 -0700 Subject: [PATCH] wireguard: Fix issues with QR codes and config download - Previous changes introduced a regression where QR codes and configuration download stopped working. This is because of removal of private key from session. Fix this. - Provide the QR code and configuration download inline so that private key does not have to stored in session. - Provide separate QR code and configuration for each of the available endpoints. - Move the manual parameters into a separate table for each of the endpoints. Make it look like this is only needed as the last option. - Immediately add the client when the page is presented instead of additional step (there is no use input needed). - Tone down the warnings into simple text. - Provide additional parameters in the manual configuration parameters tables to make it easy to configure (based on the parameters configured on the WireGuard app for Android). Use terminology that is used in app so that the fields can be mapped easily by the user. Tests: - Functional tests work more reliably now. - Clicking on the 'Add Client Automatically' creates the client successfully. A new message is shown with client public key. The description looks good. - Clicking on 'Download config file', 'Show QR code' and 'Show config parameters' works. Downloaded configuration file works. QR code works too and fills in expected parameters. Shown manual parameters are correct. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- freedombox/modules/wireguard/forms.py | 36 ++-- .../wireguard/templates/wireguard.html | 24 +-- .../templates/wireguard_auto_add_client.html | 135 ++++++++------- .../wireguard/tests/test_functional.py | 40 ++--- freedombox/modules/wireguard/urls.py | 10 +- freedombox/modules/wireguard/views.py | 157 ++++++------------ 6 files changed, 165 insertions(+), 237 deletions(-) diff --git a/freedombox/modules/wireguard/forms.py b/freedombox/modules/wireguard/forms.py index d0515bc15..fa480c2d6 100644 --- a/freedombox/modules/wireguard/forms.py +++ b/freedombox/modules/wireguard/forms.py @@ -72,11 +72,6 @@ class AddClientForm(forms.Form): validators=[validate_key]) -class AutoAddClientForm(forms.Form): - """Empty form for auto-client addition UX.""" - pass - - class AddServerForm(forms.Form): """Form to add server.""" peer_endpoint = forms.CharField( @@ -127,30 +122,29 @@ class AddServerForm(forms.Form): def _build_ipv4_settings(self, iface) -> dict: """Build IPv4 NM settings from interfaces.""" return { - 'method': 'manual', - 'address': str(iface.ip), - 'netmask': str(iface.netmask), - 'gateway': '', - 'dns': '', - 'second_dns': '', - } + 'method': 'manual', + 'address': str(iface.ip), + 'netmask': str(iface.netmask), + 'gateway': '', + 'dns': '', + 'second_dns': '', + } def _build_ipv6_settings(self, iface) -> dict: """Build IPv6 NM settings from interfaces.""" return { - 'method': 'manual', - 'address': str(iface.ip), - 'prefix': iface.network.prefixlen, - 'gateway': '', - 'dns': '', - 'second_dns': '', - } + 'method': 'manual', + 'address': str(iface.ip), + 'prefix': iface.network.prefixlen, + 'gateway': '', + 'dns': '', + 'second_dns': '', + } def get_settings(self) -> dict[str, dict]: """Return NM settings dict from cleaned data.""" ip_interface = ipaddress.ip_interface( - self.cleaned_data['ip_address_and_network'] - ) + self.cleaned_data['ip_address_and_network']) if self.cleaned_data['default_route']: allowed_ips = ['0.0.0.0/0', '::/0'] diff --git a/freedombox/modules/wireguard/templates/wireguard.html b/freedombox/modules/wireguard/templates/wireguard.html index 2b3f7b259..ac54d721a 100644 --- a/freedombox/modules/wireguard/templates/wireguard.html +++ b/freedombox/modules/wireguard/templates/wireguard.html @@ -85,18 +85,20 @@ {% else %}
diff --git a/freedombox/modules/wireguard/templates/wireguard_auto_add_client.html b/freedombox/modules/wireguard/templates/wireguard_auto_add_client.html index 7fb5ddad8..ffd36698c 100644 --- a/freedombox/modules/wireguard/templates/wireguard_auto_add_client.html +++ b/freedombox/modules/wireguard/templates/wireguard_auto_add_client.html @@ -11,78 +11,77 @@

{{ title }}

-
- {% csrf_token %} +
+ {% trans "New client added with public key:" %} {{ client_pubkey }} +
- {{ form|bootstrap }} - - {% if client_privkey %} -
- - - - - - - - - - - - - - - - - -
{% trans "IP Address" %}{{ next_ip }}
{% trans "Endpoint(s)" %}
{% for endpoint in endpoints %}{{ endpoint }}
-{% endfor %}
{% trans "Public Key" %}{{ client_pubkey }}
{% trans "Private Key" %} -
- {% trans "Click to reveal" %} - {{ client_privkey }} -
-
- -
- {% trans "Important:" %} - {% trans "Save the private key now. This page shows it only once!" %} -
- {% endif %} - -

{% trans "Client configuration file" %}

-

+

{% blocktrans trimmed %} - Download the configuration file (.conf) for manual import, - or use the QR code to import directly from your WireGuard mobile app. + To configure the client app, download the configuration file (.conf) for + import, scan the QR code, or use the parameters to configure from + scratch. This information can't be shown again. Be sure to get it before + navigating away from this page. Keep this information confidential. {% endblocktrans %} -

-

- {% trans "Warning:" %} - {% blocktrans trimmed %} - Treat this QR code like a password. - Anyone who scans it can gain VPN access if the connection is enabled. - {% endblocktrans %} -

-
- +

-
- - - {% trans "Cancel" %} + {% for domain, domain_info in domains.items %} +

{% trans "Using domain:" %} {{ domain }}

+
+ + {% icon 'download' %} + {% trans "Download config file" %} + +
- +
+ +
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
{% trans "Client Public Key" %}{{ client_pubkey }}
{% trans "Client Private Key" %}{{ client_privkey }}
{% trans "Client IP Address" %}{{ next_ip }}
{% trans "Client DNS Server" %}10.84.0.1
{% trans "Server/Peer Public Key" %}{{ server_pubkey }}
{% trans "Server/Peer Endpoint" %}{{ domain_info.endpoint }}
{% trans "Server/Peer Allowed IPs" %}0.0.0.0/0,::0/0
+
+ {% endfor %} + {% endblock %} diff --git a/freedombox/modules/wireguard/tests/test_functional.py b/freedombox/modules/wireguard/tests/test_functional.py index 5a58a7cc1..88b660862 100644 --- a/freedombox/modules/wireguard/tests/test_functional.py +++ b/freedombox/modules/wireguard/tests/test_functional.py @@ -139,35 +139,23 @@ class TestWireguardApp(functional.BaseAppTests): with functional.wait_for_page_update(session_browser): session_browser.find_by_css('.btn-auto-add-client').first.click() + # Verify that a new client was added + assert session_browser.find_by_css('.alert-client-added') + + # Read public key + session_browser.find_by_css( + '.btn-show-config-parameters').first.click() client_pubkey = session_browser.find_by_css( - '.pubkey-val').first.text.strip() + '.client-public-key').first.text.strip() + assert client_pubkey - # Verify private key reveal - privkey_reveal = session_browser.find_by_css('.privkey-val') - assert privkey_reveal, "Private key reveal should be present" - privkey_reveal.click() - client_privkey = session_browser.find_by_css( - '.privkey-val').text.splitlines()[1] - assert len(client_privkey) == 44, (("Private key should be base64 " - "(44 chars)")) + # Verify private key is shown + assert session_browser.find_by_css('.client-private-key').first.text - # Verify config download and QR links - download_link = session_browser.links.find_by_href( - '/freedombox/apps/wireguard/client/auto-add/action/download/') - qr_link = session_browser.links.find_by_href( - '/freedombox/apps/wireguard/client/auto-add/action/qr/') - assert download_link, "Download config link should exist" - assert qr_link, "QR code link should exist" - - # Submit to add the client - with functional.wait_for_page_update(session_browser): - session_browser.find_by_css( - '.btn-auto-add-connection').first.click() - - # Verify client was added successfully - assert self._client_exists( - session_browser, - client_pubkey), (("Auto-generated client should exist")) + # Assert that at least one of each buttons are present + assert session_browser.find_by_css('.btn-download-config-file') + assert session_browser.find_by_css('.btn-show-qr-code') + assert session_browser.find_by_css('.btn-show-config-parameters') # Clean up self._delete_client(session_browser, client_pubkey) diff --git a/freedombox/modules/wireguard/urls.py b/freedombox/modules/wireguard/urls.py index 07c287c1b..0ff51fefa 100644 --- a/freedombox/modules/wireguard/urls.py +++ b/freedombox/modules/wireguard/urls.py @@ -13,14 +13,8 @@ urlpatterns = [ views.EnableServerView.as_view(), name='enable-server'), re_path(r'^apps/wireguard/client/add/$', views.AddClientView.as_view(), name='add-client'), - re_path(r'^apps/wireguard/client/auto-add/$', - views.AutoAddClientView.as_view(), name='auto-add-client'), - re_path(r'^apps/wireguard/client/auto-add/action/download/$', - views.ClientActionsView.as_view(action='download'), - name='auto-add-client-download'), - re_path(r'^apps/wireguard/client/auto-add/action/qr/$', - views.ClientActionsView.as_view(action='qr'), - name='auto-add-client-qr'), + re_path(r'^apps/wireguard/client/auto-add/$', views.auto_add_client_view, + name='auto-add-client'), re_path(r'^apps/wireguard/client/(?P[^/]+)/show/$', views.ShowClientView.as_view(), name='show-client'), re_path(r'^apps/wireguard/client/(?P[^/]+)/edit/$', diff --git a/freedombox/modules/wireguard/views.py b/freedombox/modules/wireguard/views.py index 085d47ebd..17e929ced 100644 --- a/freedombox/modules/wireguard/views.py +++ b/freedombox/modules/wireguard/views.py @@ -3,15 +3,18 @@ Views for WireGuard application. """ +import base64 import urllib.parse from io import BytesIO from django.contrib import messages from django.contrib.messages.views import SuccessMessageMixin -from django.http import Http404, HttpResponse +from django.http import Http404 from django.shortcuts import redirect +from django.template.response import TemplateResponse from django.urls import reverse_lazy from django.utils.translation import gettext as _ +from django.views.decorators.http import require_POST from django.views.generic import FormView, TemplateView, View from freedombox.modules.names.components import DomainName @@ -82,114 +85,62 @@ class AddClientView(SuccessMessageMixin, FormView): return super().form_valid(form) -class SessionClientDataMixin: - """Shared session data loading for auto-client views.""" +def get_qr_code(config: str) -> bytes: + """Return a QR code in SVG format as a string.""" + import segno - def get_session_client_data(self, request): - """Extract client data from session.""" - next_ip = request.session.get('next_ip') - pubkey = request.session.get('client_pubkey') - privkey = request.session.get('client_privkey') - endpoint = request.session.get('endpoint') + qr_code = segno.make(config) + svg_buffer = BytesIO() + qr_code.save(svg_buffer, kind='svg', light='white', scale=5) - if not all([next_ip, privkey, pubkey, endpoint]): - raise Http404("Session expired") + return svg_buffer.getvalue() - return { - 'next_ip': next_ip, - 'privkey': privkey, - 'pubkey': pubkey, - 'endpoint': endpoint + +@require_POST +def auto_add_client_view(request): + """Add a new client automatically and show all the config information.""" + info = utils.get_info() + server_info = info['my_server'] + + if server_info: + domains = DomainName.list_names(filter_for_service='wireguard') + port = server_info.get('listen_port', 51820) + + # Generate key pair + client_privkey, client_pubkey = utils.generate_client_keypair() + + # Gather information + connection = utils._server_connection() + setting_name = utils.nm.SETTING_WIREGUARD_SETTING_NAME + settings = connection.get_setting_by_name(setting_name) + next_ip = utils._get_next_available_ip_address(settings) + domains_info = {} + + # Prepare config and QR codes based on endpoint. + for domain in domains: + endpoint = f'{domain}:{port}' + config = utils.build_client_config(next_ip, client_privkey, + client_pubkey, endpoint) + qr_code = get_qr_code(config) + + domains_info[domain] = { + 'endpoint': endpoint, + 'qr_code': base64.b64encode(qr_code).decode(), + 'config': base64.b64encode(config.encode()).decode(), } - def get_client_config(self, request): - """Rebuild client config from session.""" - data = self.get_session_client_data(request) + # Add a new client + utils.add_client(client_pubkey) - return utils.build_client_config(data['next_ip'], data['privkey'], - data['pubkey'], data['endpoint']) - - -class ClientActionsView(SessionClientDataMixin, View): - action = None - - def get(self, request): - import segno - - config = self.get_client_config(request) - if self.action == 'download': - response = HttpResponse(config, content_type='text/plain') - response['Content-Disposition'] = \ - 'attachment; filename="wg-client.conf"' - return response - elif self.action == 'qr': - qrcode = segno.make(config) - buffer = BytesIO() - qrcode.save(buffer, kind='svg', scale=5) - - return HttpResponse(buffer.getvalue(), - content_type='image/svg+xml') - - raise Http404("Invalid action") - - -class AutoAddClientView(SuccessMessageMixin, FormView): - """View to add a client with keypair generation.""" - form_class = forms.AutoAddClientForm - template_name = 'wireguard_auto_add_client.html' - success_url = reverse_lazy('wireguard:index') - success_message = _('Added new client.') - - def get_context_data(self, **kwargs): - """Return additional context for rendering the template.""" - context = super().get_context_data(**kwargs) - context['title'] = _('Add Allowed Client') - - context['domains'] = [] - info = utils.get_info() - server_info = info['my_server'] - - if server_info: - domains = DomainName.list_names(filter_for_service='wireguard') - port = server_info.get('listen_port', 51820) - endpoints = [f'{domain}:{port}' for domain in domains] - - try: - client_privkey, client_pubkey = utils.generate_client_keypair() - - # Get next IP - connection = utils._server_connection() - setting_name = utils.nm.SETTING_WIREGUARD_SETTING_NAME - settings = connection.get_setting_by_name(setting_name) - next_ip = utils._get_next_available_ip_address(settings) - - # Add properties to template context - context['client_pubkey'] = client_pubkey - context['client_privkey'] = client_privkey - context['next_ip'] = next_ip - context['endpoints'] = endpoints - - # Store info on instance for reuse - self.request.session['client_pubkey'] = client_pubkey - except Exception as exception: - messages.warning('Client key generation failed: %s', exception) - - return context - - def form_valid(self, form): - """Add client using generated public key.""" - try: - client_pubkey = self.request.session.pop('client_pubkey') - utils.add_client(client_pubkey) - except KeyError: - messages.warning(self.request, - _('Session expired. Please try again.')) - return redirect('wireguard:auto-add-client') - except ValueError: - messages.warning(self.request, _('Client already exists')) - return redirect('wireguard:index') - - return super().form_valid(form) + return TemplateResponse( + request, 'wireguard_auto_add_client.html', { + 'title': _('Client Added'), + 'server_pubkey': server_info['public_key'], + 'client_pubkey': client_pubkey, + 'client_privkey': client_privkey, + 'next_ip': next_ip, + 'domains': domains_info, + }) class ShowClientView(SuccessMessageMixin, TemplateView):