Differential D15093 Diff 45010 vpn/wireguard/wireguardwidget.cpp

Changeset View

Standalone View

View Options

vpn/wireguard/wireguardwidget.cpp

  • This file was added.
1​/*
2​ Copyright 2018 Bruce Anderson <banderson19com@san.rr.com>
3​
4​ This program is free software; you can redistribute it and/or
5​ modify it under the terms of the GNU General Public License as
6​ published by the Free Software Foundation; either version 2 of
7​ the License or (at your option) version 3 or any later version
8​ accepted by the membership of KDE e.V. (or its successor approved
9​ by the membership of KDE e.V.), which shall act as a proxy
10​ defined in Section 14 of version 3 of the license.
11​
12​ This program is distributed in the hope that it will be useful,
13​ but WITHOUT ANY WARRANTY; without even the implied warranty of
14​ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15​ GNU General Public License for more details.
16​
17​ You should have received a copy of the GNU General Public License
18​ along with this program. If not, see <http://www.gnu.org/licenses/>.
19​*/
20​
21​#include "debug.h"
22​#include "wireguardwidget.h"
23​#include "wireguardadvancedwidget.h"
24​#include "simpleipv4addressvalidator.h"
25​#include "simpleiplistvalidator.h"
26​#include "wireguardkeyvalidator.h"
27​
28​#include <QDBusMetaType>
29​#include <QPointer>
30​#include <KColorScheme>
31​
jgrulichUnsubmitted
Done

KColorScheme has been removed/deprecated in KDE Frameworks and it's only in KDELibs4support which we don't want to use. You should use QPalette instead.

jgrulich: KColorScheme has been removed/deprecated in KDE Frameworks and it's only in KDELibs4support…
32​#include "nm-wireguard-service.h"
jgrulichUnsubmitted
Done

Not needed.

jgrulich: Not needed.
33​
pinoUnsubmitted
Done

undeeded

pino: undeeded
34​class WireGuardSettingWidget::Private
35​{
36​public:
37​ Private();
38​ Ui_WireGuardProp ui;
39​ NetworkManager::VpnSetting::Ptr setting;
40​ KSharedConfigPtr config;
jgrulichUnsubmitted
Not Done

You also will not need this, because QPalette already should have used colors thanks to our KDE platform theme.

jgrulich: You also will not need this, because QPalette already should have used colors thanks to our KDE…
41​ QPalette warningPalette;
42​ QPalette normalPalette;
43​ WireGuardKeyValidator *keyValidator;
44​ bool addressValid;
45​ bool privateKeyValid;
46​ bool publicKeyValid;
47​ bool dnsValid;
48​ bool allowedIpsValid;
49​ bool endpointValid;
50​};
51​
52​WireGuardSettingWidget::Private::Private(void) : addressValid(false),
53​ privateKeyValid(false),
jgrulichUnsubmitted
Not Done

Coding style, can be fixed afterwards.

jgrulich: Coding style, can be fixed afterwards.
54​ publicKeyValid(false),
55​ dnsValid(true), // optional so blank is valid
56​ allowedIpsValid(false),
57​ endpointValid(true) // optional so blank is valid
58​{
59​}
60​
61​WireGuardSettingWidget::WireGuardSettingWidget(const NetworkManager::VpnSetting::Ptr &setting,
jgrulichUnsubmitted
Not Done

Do not use KColorScheme. Use QPalette instead and you don't need to keeping it as class variable.

jgrulich: Do not use KColorScheme. Use QPalette instead and you don't need to keeping it as class…
andersonbruceAuthorUnsubmitted
Not Done

My apologies if I sound a little frustrated on this but I spent more than 4 hours trying to follow the very unclear documentation on handling colors before making this change. I thought the whole idea was that I was supposed to use the scheme the user picked to tell what color to make the background when it wasn't valid rather than assigning an arbitrary color. KColorScheme allows you to do this using the "NegativeBackground" role but there doesn't appear to be any corresponding concept using QPalette. What would you suggest that I use as the background for invalid entries? Or better yet. do you know of an example of something that uses QPalette in a similar context since I have been unable to find one?

As far as putting it in a class variable, I only did that so that I wouldn't be creating them each time I wanted to change a background on one of the widgets and to be able to pass them to the advanced widget and not have to create them there as well. If you think that palette creation is a relatively time efficient process, I'll just create them on the stack in the setBackground() function each time they are needed.

andersonbruce: My apologies if I sound a little frustrated on this but I spent more than 4 hours trying to…
jgrulichUnsubmitted
Not Done

I'm sorry, I was wrong and I thought that KColorPalette is deprecated, but it's actually not and it's part of KConfigWidgets. Still, given you just use those two colors, you don't need to save full palette, just construct a one to get the colors and save these as member variables. You can do basically same for the advanced widget, you don't need to pass them in constructor.

jgrulich: I'm sorry, I was wrong and I thought that KColorPalette is deprecated, but it's actually not…
62​ QWidget *parent)
63​ : SettingWidget(setting, parent),
64​ d(new Private)
65​{
66​ qDBusRegisterMetaType<NMStringMap>();
67​
68​ d->ui.setupUi(this);
69​ d->setting = setting;
70​
71​ d->config = KSharedConfig::openConfig();
72​ d->warningPalette = KColorScheme::createApplicationPalette(d->config);
73​ d->normalPalette = KColorScheme::createApplicationPalette(d->config);
74​ KColorScheme::adjustBackground(d->warningPalette,
jgrulichUnsubmitted
Done

Split isFooValid() functions, as mentioned in the advanced widget. Also slotWidgetChanged() will call isValid() I think, which means that each isFooValid() is called twice.

jgrulich: Split isFooValid() functions, as mentioned in the advanced widget. Also slotWidgetChanged()…
75​ KColorScheme::NegativeBackground,
76​ QPalette::Base,
jgrulichUnsubmitted
Not Done

Coding style

jgrulich: Coding style
77​ KColorScheme::ColorSet::View,
78​ d->config);
79​
80​ KColorScheme::adjustBackground(d->normalPalette,
81​ KColorScheme::NormalBackground,
82​ QPalette::Base,
83​ KColorScheme::ColorSet::View,
84​ d->config);
85​
86​
87​ connect(d->ui.addressIPv4LineEdit, &QLineEdit::textChanged, this,
88​ &WireGuardSettingWidget::checkAddressValid);
89​ connect(d->ui.addressIPv6LineEdit, &QLineEdit::textChanged, this,
90​ &WireGuardSettingWidget::checkAddressValid);
91​ connect(d->ui.privateKeyLineEdit, &PasswordField::textChanged, this,
92​ &WireGuardSettingWidget::checkPrivateKeyValid);
93​ connect(d->ui.publicKeyLineEdit, &QLineEdit::textChanged, this,
94​ &WireGuardSettingWidget::checkPublicKeyValid);
95​ connect(d->ui.allowedIPsLineEdit, &QLineEdit::textChanged, this,
96​ &WireGuardSettingWidget::checkAllowedIpsValid);
97​ connect(d->ui.endpointLineEdit, &QLineEdit::textChanged, this,
98​ &WireGuardSettingWidget::checkEndpointValid);
99​ connect(d->ui.dNSLineEdit, &QLineEdit::textChanged, this,
100​ &WireGuardSettingWidget::checkDnsValid);
101​
102​ d->ui.privateKeyLineEdit->setPasswordModeEnabled(true);
103​
104​ connect(d->ui.btnAdvanced, &QPushButton::clicked, this, &WireGuardSettingWidget::showAdvanced);
105​
106​ SimpleIpV4AddressValidator *ip4WithCidrValidator =
107​ new SimpleIpV4AddressValidator(this, SimpleIpV4AddressValidator::AddressStyle::WithCidr);
108​ d->ui.addressIPv4LineEdit->setValidator(ip4WithCidrValidator);
109​
110​ // Create a validator for the IPv6 address line edit
111​ // Address must be a valid IP address with a CIDR suffix
112​ SimpleIpV6AddressValidator *ip6WithCidrValidator =
113​ new SimpleIpV6AddressValidator(this, SimpleIpV6AddressValidator::AddressStyle::WithCidr);
114​
115​ d->ui.addressIPv6LineEdit->setValidator(ip6WithCidrValidator);
116​
117​ // This is done as a private variable rather than a local variable so it can be
118​ // used both here and to validate the private key later
119​ d->keyValidator = new WireGuardKeyValidator(this);
120​ d->ui.publicKeyLineEdit->setValidator(d->keyValidator);
121​
122​ // Create validator for DNS
123​ SimpleIpV4AddressValidator *dnsValidator = new SimpleIpV4AddressValidator(this);
124​ d->ui.dNSLineEdit->setValidator(dnsValidator);
125​
126​ // Create validator for Endpoint
127​ SimpleIpV4AddressValidator *endpointValidator =
128​ new SimpleIpV4AddressValidator(this, SimpleIpV4AddressValidator::AddressStyle::WithPort);
129​ d->ui.endpointLineEdit->setValidator(endpointValidator);
130​
jgrulichUnsubmitted
Done

Add Q_UNUSED(setting);

jgrulich: Add Q_UNUSED(setting);
131​ // Create validator for AllowedIPs
132​ SimpleIpListValidator *allowedIPsValidator = new SimpleIpListValidator(this,
133​ SimpleIpListValidator::WithCidr,
134​ SimpleIpListValidator::Both);
135​ d->ui.allowedIPsLineEdit->setValidator(allowedIPsValidator);
136​
137​ // Connect for setting check
138​ watchChangedSetting();
139​
140​ KAcceleratorManager::manage(this);
141​
142​ if (setting && !setting->isNull()) {
143​ loadConfig(d->setting);
144​ }
145​
146​ // Set the initial backgrounds on all the widgets
147​ checkAddressValid();
148​ checkPrivateKeyValid();
149​ checkPublicKeyValid();
150​ checkDnsValid();
151​ checkAllowedIpsValid();
152​ checkEndpointValid();
153​
154​}
155​
156​WireGuardSettingWidget::~WireGuardSettingWidget()
157​{
158​ delete d;
159​}
160​
161​void WireGuardSettingWidget::loadConfig(const NetworkManager::Setting::Ptr &setting)
162​{
163​ Q_UNUSED(setting)
164​ // General settings
165​ const NMStringMap dataMap = d->setting->data();
166​
167​ d->ui.addressIPv4LineEdit->setText(dataMap[NM_WG_KEY_ADDR_IP4]);
168​ d->ui.addressIPv6LineEdit->setText(dataMap[NM_WG_KEY_ADDR_IP6]);
169​ d->ui.privateKeyLineEdit->setText(dataMap[NM_WG_KEY_PRIVATE_KEY]);
170​ d->ui.dNSLineEdit->setText(dataMap[NM_WG_KEY_DNS]);
171​ d->ui.publicKeyLineEdit->setText(dataMap[NM_WG_KEY_PUBLIC_KEY]);
172​ d->ui.allowedIPsLineEdit->setText(dataMap[NM_WG_KEY_ALLOWED_IPS]);
173​ d->ui.endpointLineEdit->setText(dataMap[NM_WG_KEY_ENDPOINT]);
174​}
175​
176​void WireGuardSettingWidget::loadSecrets(const NetworkManager::Setting::Ptr &setting)
177​{
178​ // Currently WireGuard does not have any secrets
179​ Q_UNUSED(setting)
180​}
181​
182​QVariantMap WireGuardSettingWidget::setting() const
183​{
pinoUnsubmitted
Done

no need for the (...) for the whole condition:

return foo || bar;

is easier than

return (foo || bar);
pino: no need for the (...) for the whole condition: lang=c++ return foo || bar; is easier than…
pinoUnsubmitted
Done

this is not "done", btw

pino: this is not "done", btw
184​ NMStringMap data = d->setting->data();
185​ NetworkManager::VpnSetting setting;
186​ setting.setServiceType(QLatin1String(NM_DBUS_SERVICE_WIREGUARD));
187​
188​ // required settings
189​ setProperty(data, QLatin1String(NM_WG_KEY_ADDR_IP4), d->ui.addressIPv4LineEdit->displayText());
190​ setProperty(data, QLatin1String(NM_WG_KEY_ADDR_IP6), d->ui.addressIPv6LineEdit->displayText());
191​ setProperty(data, QLatin1String(NM_WG_KEY_PRIVATE_KEY), d->ui.privateKeyLineEdit->text());
192​ setProperty(data, QLatin1String(NM_WG_KEY_PUBLIC_KEY), d->ui.publicKeyLineEdit->displayText());
193​ setProperty(data, QLatin1String(NM_WG_KEY_ALLOWED_IPS), d->ui.allowedIPsLineEdit->displayText());
194​ setProperty(data, QLatin1String(NM_WG_KEY_DNS), d->ui.dNSLineEdit->displayText());
195​ setProperty(data, QLatin1String(NM_WG_KEY_ENDPOINT), d->ui.endpointLineEdit->displayText());
196​
197​ setting.setData(data);
jgrulichUnsubmitted
Done

Coding style

jgrulich: Coding style
198​
199​ return setting.toMap();
200​}
201​
202​void WireGuardSettingWidget::setProperty(NMStringMap &data,
203​ const QLatin1String &key,
204​ const QString &value) const
205​{
206​ if (!value.isEmpty())
207​ data.insert(key, value);
208​ else
209​ data.remove(key);
210​}
211​
212​void WireGuardSettingWidget::showAdvanced()
213​{
214​ QPointer<WireGuardAdvancedWidget> adv = new WireGuardAdvancedWidget(d->setting, this);
215​
216​ connect(adv.data(), &WireGuardAdvancedWidget::accepted,
217​ [adv, this] () {
218​ NetworkManager::VpnSetting::Ptr advData = adv->setting();
219​ if (!advData.isNull()) {
220​ d->setting->setData(advData->data());
221​ }
222​ });
223​ connect(adv.data(), &WireGuardAdvancedWidget::finished,
224​ [adv] () {
225​ if (adv) {
226​ adv->deleteLater();
227​ }
228​ });
229​ adv->setModal(true);
230​ adv->show();
231​}
232​
233​bool WireGuardSettingWidget::isValid() const
234​{
235​ return d->addressValid
236​ && d->privateKeyValid
237​ && d->publicKeyValid
238​ && d->dnsValid
239​ && d->allowedIpsValid
240​ && d->endpointValid;
241​}
242​
243​void WireGuardSettingWidget::checkAddressValid()
244​{
245​ int pos = 0;
246​ QLineEdit *widget = d->ui.addressIPv4LineEdit;
247​ QString value(widget->displayText());
248​ bool ip4valid = (widget->validator()->validate(value, pos) == QValidator::Acceptable);
249​ bool ip4present = !widget->displayText().isEmpty();
250​
251​ widget = d->ui.addressIPv6LineEdit;
252​ value = widget->displayText();
253​ bool ip6valid = QValidator::Acceptable == widget->validator()->validate(value, pos);
254​ bool ip6present = !widget->displayText().isEmpty();
255​
256​ d->addressValid = (ip4valid && ip6valid)
257​ || (ip4valid && !ip6present)
258​ || (!ip4present && ip6valid);
259​
260​ setBackground(d->ui.addressIPv4LineEdit, d->addressValid);
261​ setBackground(d->ui.addressIPv6LineEdit, d->addressValid);
262​
263​ slotWidgetChanged();
264​}
265​
266​void WireGuardSettingWidget::checkPrivateKeyValid()
267​{
268​ int pos = 0;
269​ PasswordField *widget = d->ui.privateKeyLineEdit;
270​ QString value = widget->text();
271​ d->privateKeyValid = QValidator::Acceptable == d->keyValidator->validate(value, pos);
272​ setBackground(widget, d->privateKeyValid);
273​ slotWidgetChanged();
274​}
275​
276​void WireGuardSettingWidget::checkPublicKeyValid()
277​{
278​ int pos = 0;
279​ QLineEdit *widget = d->ui.publicKeyLineEdit;
280​ QString value = widget->displayText();
281​ d->publicKeyValid = QValidator::Acceptable == widget->validator()->validate(value, pos);
282​ setBackground(widget, d->publicKeyValid);
283​ slotWidgetChanged();
284​}
285​
286​void WireGuardSettingWidget::checkDnsValid()
287​{
288​ int pos = 0;
289​ QLineEdit *widget = d->ui.dNSLineEdit;
290​ QString value = widget->displayText();
291​ d->dnsValid = QValidator::Acceptable == widget->validator()->validate(value, pos)
292​ || widget->displayText().isEmpty();
293​ setBackground(widget, d->dnsValid);
294​ slotWidgetChanged();
295​}
296​
297​void WireGuardSettingWidget::checkAllowedIpsValid()
lbeltrameUnsubmitted
Done

Why this? It will break proper coloring with people that use custom themes. IIRC you can have the widget handle it (but I can't check at the moment).

lbeltrame: Why this? It will break proper coloring with people that use custom themes. IIRC you can have…
298​{
299​ int pos = 0;
300​ QLineEdit *widget = d->ui.allowedIPsLineEdit;
301​ QString value = widget->displayText();
302​ d->allowedIpsValid = QValidator::Acceptable == widget->validator()->validate(value, pos);
303​ setBackground(widget, d->allowedIpsValid);
304​ slotWidgetChanged();
305​}
306​
307​void WireGuardSettingWidget::checkEndpointValid()
308​{
309​ int pos = 0;
310​ QLineEdit *widget = d->ui.endpointLineEdit;
311​ QString value = widget->displayText();
312​ d->endpointValid = QValidator::Acceptable == widget->validator()->validate(value, pos)
313​ || value.isEmpty();
314​ setBackground(widget, d->endpointValid);
315​ slotWidgetChanged();
316​}
317​
318​void WireGuardSettingWidget::setBackground(QWidget *w, bool result) const
319​{
320​ if (result)
321​ w->setPalette(d->normalPalette);
322​ else
323​ w->setPalette(d->warningPalette);
324​}