Differential D18394 Diff 50665 vpn/openconnect/openconnectauth.cpp

Changeset View

Standalone View

View Options

vpn/openconnect/openconnectauth.cpp

Context not available.
57#include <fcntl.h> 57​#include <fcntl.h>
58} 58​}
59 59​
60​#if !OPENCONNECT_CHECK_VER(2,1)
61​#define __openconnect_set_token_mode(...) -EOPNOTSUPP
62​#elif !OPENCONNECT_CHECK_VER(2,2)
63​#define __openconnect_set_token_mode(vpninfo, mode, secret) openconnect_set_stoken_mode(vpninfo, 1, secret)
64​#else
65​#define __openconnect_set_token_mode openconnect_set_token_mode
66​#endif
pinoUnsubmitted
Not Done

IMHO it is better to define the macros in the same way for all the cases, specifying their parameters

pino: IMHO it is better to define the macros in the same way for all the cases, specifying their…
67​
68​#if OPENCONNECT_CHECK_VER(3,4)
69​ static int updateToken(void*, const char*);
70​#endif
71​
60// name/address: IP/domain name of the host (OpenConnect accepts both, so no difference here) 72​// name/address: IP/domain name of the host (OpenConnect accepts both, so no difference here)
61// group: user group on the server 73​// group: user group on the server
62typedef struct { 74​typedef struct {
Context not available.
65 QString address; 77​ QString address;
66} VPNHost; 78​} VPNHost;
67 79​
80​typedef struct {
81​ oc_token_mode_t tokenMode;
82​ QByteArray tokenSecret;
83​} Token;
84​
68class OpenconnectAuthWidgetPrivate 85​class OpenconnectAuthWidgetPrivate
69{ 86​{
70public: 87​public:
Context not available.
82 int cancelPipes[2]; 99​ int cancelPipes[2];
83 QList<QPair<QString, int> > serverLog; 100​ QList<QPair<QString, int> > serverLog;
84 int passwordFormIndex; 101​ int passwordFormIndex;
102​ QByteArray tokenMode;
103​ Token token;
85 104​
86 enum LogLevels {Error = 0, Info, Debug, Trace}; 105​ enum LogLevels {Error = 0, Info, Debug, Trace};
87}; 106​};
Context not available.
122 connect(d->worker, &OpenconnectAuthWorkerThread::updateLog, this, &OpenconnectAuthWidget::updateLog); 141​ connect(d->worker, &OpenconnectAuthWorkerThread::updateLog, this, &OpenconnectAuthWidget::updateLog);
123 connect(d->worker, QOverload<const QString&>::of(&OpenconnectAuthWorkerThread::writeNewConfig), this, &OpenconnectAuthWidget::writeNewConfig); 142​ connect(d->worker, QOverload<const QString&>::of(&OpenconnectAuthWorkerThread::writeNewConfig), this, &OpenconnectAuthWidget::writeNewConfig);
124 connect(d->worker, &OpenconnectAuthWorkerThread::cookieObtained, this, &OpenconnectAuthWidget::workerFinished); 143​ connect(d->worker, &OpenconnectAuthWorkerThread::cookieObtained, this, &OpenconnectAuthWidget::workerFinished);
144​ connect(d->worker, &OpenconnectAuthWorkerThread::initTokens, this, &OpenconnectAuthWidget::initTokens);
125 145​
126 readConfig(); 146​ readConfig();
127 readSecrets(); 147​ readSecrets();
128 148​
149​#if OPENCONNECT_CHECK_VER(3,4)
150​ openconnect_set_token_callbacks(d->vpninfo, &d->secrets, NULL, &updateToken);
151​#endif
152​
129 // This might be set by readSecrets() so don't connect it until now 153​ // This might be set by readSecrets() so don't connect it until now
130 connect(d->ui.cmbHosts, QOverload<int>::of(&QComboBox::currentIndexChanged), this, &OpenconnectAuthWidget::connectHost); 154​ connect(d->ui.cmbHosts, QOverload<int>::of(&QComboBox::currentIndexChanged), this, &OpenconnectAuthWidget::connectHost);
131 155​
Context not available.
196 const QString protocol = dataMap[NM_OPENCONNECT_KEY_PROTOCOL]; 220​ const QString protocol = dataMap[NM_OPENCONNECT_KEY_PROTOCOL];
197 openconnect_set_protocol(d->vpninfo, OC3DUP(protocol == "juniper" ? "nc" : protocol.toUtf8().data())); 221​ openconnect_set_protocol(d->vpninfo, OC3DUP(protocol == "juniper" ? "nc" : protocol.toUtf8().data()));
198 } 222​ }
223​
224​ d->tokenMode = dataMap[NM_OPENCONNECT_KEY_TOKEN_MODE].toUtf8();
199} 225​}
pinoUnsubmitted
Not Done

extra empty line

pino: extra empty line
200 226​
201void OpenconnectAuthWidget::readSecrets() 227​void OpenconnectAuthWidget::readSecrets()
Context not available.
248 if (d->secrets["save_passwords"] == "yes") { 274​ if (d->secrets["save_passwords"] == "yes") {
249 d->ui.chkStorePasswords->setChecked(true); 275​ d->ui.chkStorePasswords->setChecked(true);
250 } 276​ }
277​
278​ d->token.tokenMode = OC_TOKEN_MODE_NONE;
279​ d->token.tokenSecret = nullptr;
pinoUnsubmitted
Not Done

this is needed only in the block within the if in the line below, so move it inside that block; also, make tokenSecret const

pino: this is needed only in the block within the if in the line below, so move it inside that block…
280​
281​ if (!d->tokenMode.isEmpty()) {
282​ int ret = 0;
283​ QByteArray tokenSecret = d->secrets[NM_OPENCONNECT_KEY_TOKEN_SECRET].toUtf8();
284​
285​ if (d->tokenMode == "manual" && !tokenSecret.isEmpty()) {
jgrulichUnsubmitted
Not Done

QStringLiteral("manual")

Same below.

jgrulich: QStringLiteral("manual") Same below.
286​ ret = __openconnect_set_token_mode(d->vpninfo, OC_TOKEN_MODE_STOKEN, tokenSecret);
287​ }
jgrulichUnsubmitted
Not Done

Missing space after "==".

jgrulich: Missing space after "==".
288​ else if (d->tokenMode =="stokenrc") {
jgrulichUnsubmitted
Not Done

Coding style. The "else if" should be on the same line as the "}" bracket. Same below.

jgrulich: Coding style. The "else if" should be on the same line as the "}" bracket. Same below.
289​ ret = __openconnect_set_token_mode(d->vpninfo, OC_TOKEN_MODE_STOKEN, NULL);
290​ }
291​ else if (d->tokenMode == "totp" && !tokenSecret.isEmpty()) {
292​ ret = __openconnect_set_token_mode(d->vpninfo, OC_TOKEN_MODE_TOTP, tokenSecret);
293​ }
jgrulichUnsubmitted
Not Done

And here is one additional space after "=="

jgrulich: And here is one additional space after "=="
294​#if OPENCONNECT_CHECK_VER(3,4)
295​ else if (d->tokenMode == "hotp" && !tokenSecret.isEmpty()) {
296​ ret = __openconnect_set_token_mode(d->vpninfo, OC_TOKEN_MODE_HOTP, tokenSecret);
297​ }
298​#endif
pinoUnsubmitted
Not Done

if tokenSecret is not empty, then its length will always be greater than 0, so the second condition is redundant

pino: if tokenSecret is not empty, then its length will always be greater than 0, so the second…
299​#if OPENCONNECT_CHECK_VER(5,0)
300​ else if (d->tokenMode == "yubioath") {
301​ /* This needs to be done from a thread because it can call back to
pinoUnsubmitted
Not Done

d->token.tokenSecret is a QByteArray, so if you want to clear it just call clear()

pino: d->token.tokenSecret is a QByteArray, so if you want to clear it just call clear()
302​ ask for the PIN */
303​ d->token.tokenMode = OC_TOKEN_MODE_YUBIOATH;
304​ if (!tokenSecret.isEmpty()) {
305​ d->token.tokenSecret = tokenSecret;
pinoUnsubmitted
Not Done

it looks like all the other messages for addFormInfo() do not have a trailing newline, so please remove it

pino: it looks like all the other messages for addFormInfo() do not have a trailing newline, so…
306​ }
307​ else {
308​ d->token.tokenSecret.clear();
309​ }
310​ }
311​#endif
312​ if (ret) {
313​ addFormInfo(QLatin1String("dialog-error"), i18n("Failed to initialize software token: %1", ret));
314​ }
315​ }
251} 316​}
252 317​
253void OpenconnectAuthWidget::acceptDialog() 318​void OpenconnectAuthWidget::acceptDialog()
Context not available.
269void OpenconnectAuthWidget::connectHost() 334​void OpenconnectAuthWidget::connectHost()
270{ 335​{
271 Q_D(OpenconnectAuthWidget); 336​ Q_D(OpenconnectAuthWidget);
337​
272 d->userQuit = true; 338​ d->userQuit = true;
273 if (write(d->cancelPipes[1], "x", 1)) { 339​ if (write(d->cancelPipes[1], "x", 1)) {
274 // not a lot we can do 340​ // not a lot we can do
pinoUnsubmitted
Not Done

extra empty line

pino: extra empty line
Context not available.
301 d->worker->start(); 367​ d->worker->start();
302} 368​}
303 369​
370​void OpenconnectAuthWidget::initTokens()
371​{
372​ Q_D(OpenconnectAuthWidget);
373​
374​ if (d->token.tokenMode != OC_TOKEN_MODE_NONE) {
pinoUnsubmitted
Not Done

extra empty line

pino: extra empty line
375​ __openconnect_set_token_mode(d->vpninfo, d->token.tokenMode, d->token.tokenSecret);
376​ }
377​}
378​
304QVariantMap OpenconnectAuthWidget::setting() const 379​QVariantMap OpenconnectAuthWidget::setting() const
305{ 380​{
pinoUnsubmitted
Not Done

extra empty line

pino: extra empty line
306 Q_D(const OpenconnectAuthWidget); 381​ Q_D(const OpenconnectAuthWidget);
Context not available.
346 return secretData; 421​ return secretData;
347} 422​}
348 423​
424​#if OPENCONNECT_CHECK_VER(3,4)
425​static int updateToken(void *cbdata, const char *tok)
426​{
427​ NMStringMap *secrets = static_cast<NMStringMap *>(cbdata);
428​ secrets->insert(QLatin1String(NM_OPENCONNECT_KEY_TOKEN_SECRET), QLatin1String(tok));
429​ return 0;
430​}
431​#endif
432​
349void OpenconnectAuthWidget::writeNewConfig(const QString & buf) 433​void OpenconnectAuthWidget::writeNewConfig(const QString & buf)
350{ 434​{
351 Q_D(OpenconnectAuthWidget); 435​ Q_D(OpenconnectAuthWidget);
Context not available.
355void OpenconnectAuthWidget::updateLog(const QString &message, const int &level) 439​void OpenconnectAuthWidget::updateLog(const QString &message, const int &level)
356{ 440​{
357 Q_D(OpenconnectAuthWidget); 441​ Q_D(OpenconnectAuthWidget);
442​
358 QPair<QString, int> pair; 443​ QPair<QString, int> pair;
359 pair.first = message; 444​ pair.first = message;
360 if (pair.first.endsWith(QLatin1String("\n"))) { 445​ if (pair.first.endsWith(QLatin1String("\n"))) {
Context not available.
401void OpenconnectAuthWidget::addFormInfo(const QString &iconName, const QString &message) 486​void OpenconnectAuthWidget::addFormInfo(const QString &iconName, const QString &message)
402{ 487​{
403 Q_D(OpenconnectAuthWidget); 488​ Q_D(OpenconnectAuthWidget);
489​
404 QHBoxLayout *layout = new QHBoxLayout(); 490​ QHBoxLayout *layout = new QHBoxLayout();
405 QLabel *icon = new QLabel(this); 491​ QLabel *icon = new QLabel(this);
406 QSizePolicy sizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed); 492​ QSizePolicy sizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed);
Context not available.
426void OpenconnectAuthWidget::processAuthForm(struct oc_auth_form *form) 512​void OpenconnectAuthWidget::processAuthForm(struct oc_auth_form *form)
427{ 513​{
428 Q_D(OpenconnectAuthWidget); 514​ Q_D(OpenconnectAuthWidget);
515​
429 deleteAllFromLayout(d->ui.loginBoxLayout); 516​ deleteAllFromLayout(d->ui.loginBoxLayout);
430 if (form->banner) { 517​ if (form->banner) {
431 addFormInfo(QLatin1String("dialog-information"), form->banner); 518​ addFormInfo(QLatin1String("dialog-information"), form->banner);
Context not available.
571 connect(buttons, &QDialogButtonBox::rejected, dialog.data(), &QDialog::reject); 658​ connect(buttons, &QDialogButtonBox::rejected, dialog.data(), &QDialog::reject);
572 dialog->layout()->addWidget(widget); 659​ dialog->layout()->addWidget(widget);
573 dialog->layout()->addWidget(buttons); 660​ dialog->layout()->addWidget(buttons);
661​
662​ const NMStringMap dataMap = d->setting->data();
jgrulichUnsubmitted
Not Done

Also leftover from the other review.

jgrulich: Also leftover from the other review.
663​ buttons->button(QDialogButtonBox::Ok)->setEnabled(dataMap[NM_OPENCONNECT_KEY_PREVENT_INVALID_CERT] != "yes");
664​
574 if(dialog.data()->exec() == QDialog::Accepted) { 665​ if(dialog.data()->exec() == QDialog::Accepted) {
575 *accepted = true; 666​ *accepted = true;
576 } else { 667​ } else {
Context not available.
641void OpenconnectAuthWidget::workerFinished(const int &ret) 732​void OpenconnectAuthWidget::workerFinished(const int &ret)
642{ 733​{
643 Q_D(OpenconnectAuthWidget); 734​ Q_D(OpenconnectAuthWidget);
735​
644 if (ret < 0) { 736​ if (ret < 0) {
645 QString message; 737​ QString message;
646 QList<QPair<QString, int> >::const_iterator i; 738​ QList<QPair<QString, int> >::const_iterator i;
Context not available.