Differential D18394 Diff 57294 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 == QStringLiteral("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​ } else if (d->tokenMode ==QStringLiteral("stokenrc")) {
jgrulichUnsubmitted
Not Done

Missing space after "==".

jgrulich: Missing space after "==".
288​ ret = __openconnect_set_token_mode(d->vpninfo, OC_TOKEN_MODE_STOKEN, NULL);
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​ } else if (d->tokenMode == QStringLiteral("totp") && !tokenSecret.isEmpty()) {
290​ ret = __openconnect_set_token_mode(d->vpninfo, OC_TOKEN_MODE_TOTP, tokenSecret);
291​ }
292​#if OPENCONNECT_CHECK_VER(3,4)
293​ else if (d->tokenMode == QStringLiteral("hotp") && !tokenSecret.isEmpty()) {
jgrulichUnsubmitted
Not Done

And here is one additional space after "=="

jgrulich: And here is one additional space after "=="
294​ ret = __openconnect_set_token_mode(d->vpninfo, OC_TOKEN_MODE_HOTP, tokenSecret);
295​ }
296​#endif
297​#if OPENCONNECT_CHECK_VER(5,0)
298​ else if (d->tokenMode == "yubioath") {
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​ /* This needs to be done from a thread because it can call back to
300​ ask for the PIN */
301​ d->token.tokenMode = OC_TOKEN_MODE_YUBIOATH;
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​ if (!tokenSecret.isEmpty()) {
303​ d->token.tokenSecret = tokenSecret;
304​ }
305​ }
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​#endif
307​ if (ret) {
308​ addFormInfo(QLatin1String("dialog-error"), i18n("Failed to initialize software token: %1", ret));
309​ }
310​ }
251} 311​}
252 312​
253void OpenconnectAuthWidget::acceptDialog() 313​void OpenconnectAuthWidget::acceptDialog()
Context not available.
269void OpenconnectAuthWidget::connectHost() 329​void OpenconnectAuthWidget::connectHost()
270{ 330​{
271 Q_D(OpenconnectAuthWidget); 331​ Q_D(OpenconnectAuthWidget);
332​
272 d->userQuit = true; 333​ d->userQuit = true;
273 if (write(d->cancelPipes[1], "x", 1)) { 334​ if (write(d->cancelPipes[1], "x", 1)) {
274 // not a lot we can do 335​ // not a lot we can do
pinoUnsubmitted
Not Done

extra empty line

pino: extra empty line
Context not available.
301 d->worker->start(); 362​ d->worker->start();
302} 363​}
303 364​
365​void OpenconnectAuthWidget::initTokens()
366​{
367​ Q_D(OpenconnectAuthWidget);
368​
369​ if (d->token.tokenMode != OC_TOKEN_MODE_NONE) {
370​ __openconnect_set_token_mode(d->vpninfo, d->token.tokenMode, d->token.tokenSecret);
371​ }
372​}
373​
304QVariantMap OpenconnectAuthWidget::setting() const 374​QVariantMap OpenconnectAuthWidget::setting() const
pinoUnsubmitted
Not Done

extra empty line

pino: extra empty line
305{ 375​{
306 Q_D(const OpenconnectAuthWidget); 376​ Q_D(const OpenconnectAuthWidget);
pinoUnsubmitted
Not Done

extra empty line

pino: extra empty line
Context not available.
346 return secretData; 416​ return secretData;
347} 417​}
348 418​
419​#if OPENCONNECT_CHECK_VER(3,4)
420​static int updateToken(void *cbdata, const char *tok)
421​{
422​ NMStringMap *secrets = static_cast<NMStringMap *>(cbdata);
423​ secrets->insert(QLatin1String(NM_OPENCONNECT_KEY_TOKEN_SECRET), QLatin1String(tok));
424​ return 0;
425​}
426​#endif
427​
349void OpenconnectAuthWidget::writeNewConfig(const QString & buf) 428​void OpenconnectAuthWidget::writeNewConfig(const QString & buf)
350{ 429​{
351 Q_D(OpenconnectAuthWidget); 430​ Q_D(OpenconnectAuthWidget);
Context not available.
355void OpenconnectAuthWidget::updateLog(const QString &message, const int &level) 434​void OpenconnectAuthWidget::updateLog(const QString &message, const int &level)
356{ 435​{
357 Q_D(OpenconnectAuthWidget); 436​ Q_D(OpenconnectAuthWidget);
437​
358 QPair<QString, int> pair; 438​ QPair<QString, int> pair;
359 pair.first = message; 439​ pair.first = message;
360 if (pair.first.endsWith(QLatin1String("\n"))) { 440​ if (pair.first.endsWith(QLatin1String("\n"))) {
Context not available.
401void OpenconnectAuthWidget::addFormInfo(const QString &iconName, const QString &message) 481​void OpenconnectAuthWidget::addFormInfo(const QString &iconName, const QString &message)
402{ 482​{
403 Q_D(OpenconnectAuthWidget); 483​ Q_D(OpenconnectAuthWidget);
484​
404 QHBoxLayout *layout = new QHBoxLayout(); 485​ QHBoxLayout *layout = new QHBoxLayout();
405 QLabel *icon = new QLabel(this); 486​ QLabel *icon = new QLabel(this);
406 QSizePolicy sizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed); 487​ QSizePolicy sizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed);
Context not available.
426void OpenconnectAuthWidget::processAuthForm(struct oc_auth_form *form) 507​void OpenconnectAuthWidget::processAuthForm(struct oc_auth_form *form)
427{ 508​{
428 Q_D(OpenconnectAuthWidget); 509​ Q_D(OpenconnectAuthWidget);
510​
429 deleteAllFromLayout(d->ui.loginBoxLayout); 511​ deleteAllFromLayout(d->ui.loginBoxLayout);
430 if (form->banner) { 512​ if (form->banner) {
431 addFormInfo(QLatin1String("dialog-information"), form->banner); 513​ addFormInfo(QLatin1String("dialog-information"), form->banner);
Context not available.
571 connect(buttons, &QDialogButtonBox::rejected, dialog.data(), &QDialog::reject); 653​ connect(buttons, &QDialogButtonBox::rejected, dialog.data(), &QDialog::reject);
572 dialog->layout()->addWidget(widget); 654​ dialog->layout()->addWidget(widget);
573 dialog->layout()->addWidget(buttons); 655​ dialog->layout()->addWidget(buttons);
656​
657​ const NMStringMap dataMap = d->setting->data();
jgrulichUnsubmitted
Not Done

Also leftover from the other review.

jgrulich: Also leftover from the other review.
658​ buttons->button(QDialogButtonBox::Ok)->setEnabled(dataMap[NM_OPENCONNECT_KEY_PREVENT_INVALID_CERT] != "yes");
659​
574 if(dialog.data()->exec() == QDialog::Accepted) { 660​ if(dialog.data()->exec() == QDialog::Accepted) {
575 *accepted = true; 661​ *accepted = true;
576 } else { 662​ } else {
Context not available.
641void OpenconnectAuthWidget::workerFinished(const int &ret) 727​void OpenconnectAuthWidget::workerFinished(const int &ret)
642{ 728​{
643 Q_D(OpenconnectAuthWidget); 729​ Q_D(OpenconnectAuthWidget);
730​
644 if (ret < 0) { 731​ if (ret < 0) {
645 QString message; 732​ QString message;
646 QList<QPair<QString, int> >::const_iterator i; 733​ QList<QPair<QString, int> >::const_iterator i;
Context not available.