Differential D15093 Diff 41874 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 <QLineEdit>
30​#include <QUrl>
31​#include <QPointer>
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 <QHostAddress>
jgrulichUnsubmitted
Done

Not needed.

jgrulich: Not needed.
33​
pinoUnsubmitted
Done

undeeded

pino: undeeded
34​#include <KProcess>
35​#include <KUrlRequester>
36​
37​#include "nm-wireguard-service.h"
38​
39​class WireGuardSettingWidget::Private
40​{
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​public:
42​ Ui_WireGuardProp ui;
43​ NetworkManager::VpnSetting::Ptr setting;
44​};
45​
46​
47​WireGuardSettingWidget::WireGuardSettingWidget(const NetworkManager::VpnSetting::Ptr &setting,
48​ QWidget *parent)
49​ : SettingWidget(setting, parent)
50​ , d(new Private)
51​{
52​ qDBusRegisterMetaType<NMStringMap>();
53​
jgrulichUnsubmitted
Not Done

Coding style, can be fixed afterwards.

jgrulich: Coding style, can be fixed afterwards.
54​ d->ui.setupUi(this);
55​ d->setting = setting;
56​
57​ connect(d->ui.addressIPv4LineEdit, &QLineEdit::textChanged, this, &WireGuardSettingWidget::slotWidgetChanged);
58​ connect(d->ui.addressIPv6LineEdit, &QLineEdit::textChanged, this, &WireGuardSettingWidget::slotWidgetChanged);
59​ connect(d->ui.privateKeyLineEdit, &PasswordField::textChanged, this, &WireGuardSettingWidget::slotWidgetChanged);
60​ connect(d->ui.publicKeyLineEdit, &QLineEdit::textChanged, this, &WireGuardSettingWidget::slotWidgetChanged);
61​ connect(d->ui.allowedIPsLineEdit, &QLineEdit::textChanged, this, &WireGuardSettingWidget::slotWidgetChanged);
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​
63​ d->ui.privateKeyLineEdit->setPasswordModeEnabled(true);
64​
65​ connect(d->ui.btnAdvanced, &QPushButton::clicked, this, &WireGuardSettingWidget::showAdvanced);
66​
67​ SimpleIpV4AddressValidator *ip4WithCidrValidator = new SimpleIpV4AddressValidator(this, SimpleIpV4AddressValidator::AddressStyle::WithCidr);
68​ d->ui.addressIPv4LineEdit->setValidator(ip4WithCidrValidator);
69​
70​ // Create a validator for the IPv6 address line edit
71​ // Address must be a valid IP address with a CIDR suffix
72​ SimpleIpV6AddressValidator *ip6WithCidrValidator = new SimpleIpV6AddressValidator(this, SimpleIpV6AddressValidator::AddressStyle::WithCidr);
73​
74​ d->ui.addressIPv6LineEdit->setValidator(ip6WithCidrValidator);
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​
76​ WireGuardKeyValidator *keyValidator = new WireGuardKeyValidator(this);
jgrulichUnsubmitted
Not Done

Coding style

jgrulich: Coding style
77​ d->ui.publicKeyLineEdit->setValidator(keyValidator);
78​
79​ // Create validator for DNS
80​ SimpleIpV4AddressValidator *dnsValidator = new SimpleIpV4AddressValidator(this);
81​ d->ui.dNSLineEdit->setValidator(dnsValidator);
82​
83​ // Create validator for Endpoint
84​ SimpleIpV4AddressValidator *endpointValidator = new SimpleIpV4AddressValidator(this, SimpleIpV4AddressValidator::AddressStyle::WithPort);
85​ d->ui.endpointLineEdit->setValidator(endpointValidator);
86​
87​ // Create validator for AllowedIPs
88​ SimpleIpListValidator *allowedIPsValidator = new SimpleIpListValidator(this,
89​ SimpleIpListValidator::WithCidr,
90​ SimpleIpListValidator::Both);
91​ d->ui.allowedIPsLineEdit->setValidator(allowedIPsValidator);
92​
93​ // Connect for setting check
94​ watchChangedSetting();
95​
96​ KAcceleratorManager::manage(this);
97​
98​ if (setting && !setting->isNull()) {
99​ loadConfig(d->setting);
100​ }
101​}
102​
103​WireGuardSettingWidget::~WireGuardSettingWidget()
104​{
105​ delete d;
106​}
107​
108​void WireGuardSettingWidget::loadConfig(const NetworkManager::Setting::Ptr &setting)
109​{
110​ Q_UNUSED(setting)
111​ // General settings
112​ const NMStringMap dataMap = d->setting->data();
113​
114​ d->ui.addressIPv4LineEdit->setText(dataMap[NM_WG_KEY_ADDR_IP4]);
115​ d->ui.addressIPv6LineEdit->setText(dataMap[NM_WG_KEY_ADDR_IP6]);
116​ d->ui.privateKeyLineEdit->setText(dataMap[NM_WG_KEY_PRIVATE_KEY]);
117​ d->ui.dNSLineEdit->setText(dataMap[NM_WG_KEY_DNS]);
118​ d->ui.publicKeyLineEdit->setText(dataMap[NM_WG_KEY_PUBLIC_KEY]);
119​ d->ui.allowedIPsLineEdit->setText(dataMap[NM_WG_KEY_ALLOWED_IPS]);
120​ d->ui.endpointLineEdit->setText(dataMap[NM_WG_KEY_ENDPOINT]);
121​}
122​
123​void WireGuardSettingWidget::loadSecrets(const NetworkManager::Setting::Ptr &setting)
124​{
125​ // Currently WireGuard does not have any secrets
126​ Q_UNUSED(setting)
127​}
128​
129​QVariantMap WireGuardSettingWidget::setting() const
130​{
jgrulichUnsubmitted
Done

Add Q_UNUSED(setting);

jgrulich: Add Q_UNUSED(setting);
131​ NMStringMap data = d->setting->data();
132​ NetworkManager::VpnSetting setting;
133​ setting.setServiceType(QLatin1String(NM_DBUS_SERVICE_WIREGUARD));
134​
135​ // required settings
136​
137​ setOrClear(data, QLatin1String(NM_WG_KEY_ADDR_IP4), d->ui.addressIPv4LineEdit->displayText());
138​ setOrClear(data, QLatin1String(NM_WG_KEY_ADDR_IP6), d->ui.addressIPv6LineEdit->displayText());
139​ setOrClear(data, QLatin1String(NM_WG_KEY_PRIVATE_KEY), d->ui.privateKeyLineEdit->text());
140​ setOrClear(data, QLatin1String(NM_WG_KEY_PUBLIC_KEY), d->ui.publicKeyLineEdit->displayText());
141​ setOrClear(data, QLatin1String(NM_WG_KEY_ALLOWED_IPS), d->ui.allowedIPsLineEdit->displayText());
142​ setOrClear(data, QLatin1String(NM_WG_KEY_DNS), d->ui.dNSLineEdit->displayText());
143​ setOrClear(data, QLatin1String(NM_WG_KEY_ENDPOINT), d->ui.endpointLineEdit->displayText());
144​
145​ setting.setData(data);
146​
147​ return setting.toMap();
148​}
149​
150​void WireGuardSettingWidget::setOrClear(NMStringMap &data, const QLatin1String &key, const QString &value) const
151​{
152​ if (!value.isEmpty())
153​ data.insert(key, value);
154​ else
155​ data.remove(key);
156​}
157​
158​void WireGuardSettingWidget::showAdvanced()
159​{
160​ QPointer<WireGuardAdvancedWidget> adv = new WireGuardAdvancedWidget(d->setting, this);
161​
162​ connect(adv.data(), &WireGuardAdvancedWidget::accepted,
163​ [adv, this] () {
164​ NetworkManager::VpnSetting::Ptr advData = adv->setting();
165​ if (!advData.isNull()) {
166​ d->setting->setData(advData->data());
167​ }
168​ });
169​ connect(adv.data(), &WireGuardAdvancedWidget::finished,
170​ [adv] () {
171​ if (adv) {
172​ adv->deleteLater();
173​ }
174​ });
175​ adv->setModal(true);
176​ adv->show();
177​}
178​
179​bool WireGuardSettingWidget::isValid() const
180​{
181​ return (!d->ui.addressIPv4LineEdit->displayText().isEmpty()
182​ || !d->ui.addressIPv6LineEdit->displayText().isEmpty())
183​ && (!d->ui.privateKeyLineEdit->text().isEmpty())
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​ && (!d->ui.publicKeyLineEdit->displayText().isEmpty())
185​ && (!d->ui.allowedIPsLineEdit->displayText().isEmpty());
186​}
187​
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…
jgrulichUnsubmitted
Done

Coding style

jgrulich: Coding style