D17834: Allow the use of One-Time Password

2019-01-07 Thread Jan Grulich
This revision was automatically updated to reflect the committed changes. Closed by commit R116:b8f580a749f8: Allow the use of One-Time Password (authored by enriquem, committed by jgrulich). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D17834?vs=48852&id=48856#toc REPOSITORY R116 Pl

D17834: Allow the use of One-Time Password

2019-01-07 Thread Enrique Melendez
enriquem updated this revision to Diff 48852. enriquem added a comment. Now it builds. I tested it to write and read from the config file, interchangeably with nm-connection-editor, REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.or

D17834: Allow the use of One-Time Password

2019-01-06 Thread Jan Grulich
jgrulich added a comment. It doesn't build now, can you please fix that? REPOSITORY R116 Plasma Network Management Applet REVISION DETAIL https://phabricator.kde.org/D17834 To: enriquem, jgrulich Cc: plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed

D17834: Allow the use of One-Time Password

2019-01-05 Thread Enrique Melendez
enriquem added inline comments. INLINE COMMENTS > jgrulich wrote in fortisslvpnwidget.cpp:201 > I don't understand why you hare check for prevData.value(), but later on you > get optFlag from data (not prevData). Still I don't think this is needed at > all. I would go back and use your previous

D17834: Allow the use of One-Time Password

2019-01-05 Thread Enrique Melendez
enriquem updated this revision to Diff 48760. enriquem added a comment. I implemented all of your comments, reverting to my previous approach REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17834?vs=48750&id=48760 REVISION DETAIL

D17834: Allow the use of One-Time Password

2019-01-05 Thread Jan Grulich
jgrulich added inline comments. INLINE COMMENTS > fortisslvpnauth.cpp:44 > +const NetworkManager::Setting::SecretFlags otpFlag = > static_cast(data.value(NM_FORTISSLVPN_KEY_OTP"-flags").toInt()); > +if (otpFlag == NetworkManager::Setting::NotSaved){ > +d->ui.otpFrame->setVisible(

D17834: Allow the use of One-Time Password

2019-01-05 Thread Enrique Melendez
enriquem updated this revision to Diff 48750. enriquem added a comment. Treat otp-flags the same way NetworkManager-fortisslvpn does REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17834?vs=48744&id=48750 REVISION DETAIL https://p

D17834: Allow the use of One-Time Password

2019-01-05 Thread Enrique Melendez
enriquem added a comment. >> I see. Maybe do not set it at all if one-time password is not used and let NM to set the default value? That way we won't be using a wrong value in case this changes in future. > > Fair enough. I will update the diff with this. However, judging from the curr

D17834: Allow the use of One-Time Password

2019-01-05 Thread Enrique Melendez
enriquem updated this revision to Diff 48744. enriquem added a comment. Removed setting the otp-flag in case it is not needed for the connection REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17834?vs=48701&id=48744 REVISION DETAIL

D17834: Allow the use of One-Time Password

2019-01-05 Thread Enrique Melendez
enriquem added a comment. > I see. Maybe do not set it at all if one-time password is not used and let NM to set the default value? That way we won't be using a wrong value in case this changes in future. Fair enough. I will update the diff with this. However, judging from the current b

D17834: Allow the use of One-Time Password

2019-01-05 Thread Jan Grulich
jgrulich added a comment. In D17834#386573 , @enriquem wrote: > I implemented all of your comments except one. I created a new VPN setting wit nm-connection-editor, and it sets it to 0, that is, to NetworkManager::Setting::None. Thus, I believe

D17834: Allow the use of One-Time Password

2019-01-04 Thread Enrique Melendez
enriquem updated this revision to Diff 48701. enriquem added a comment. I implemented all of your comments except one. I created a new VPN setting wit nm-connection-editor, and it sets it to 0, that is, to NetworkManager::Setting::None. Thus, I believe that part is correct. REPOSITORY R116

D17834: Allow the use of One-Time Password

2019-01-03 Thread Jan Grulich
jgrulich added inline comments. INLINE COMMENTS > fortisslvpnauth.cpp:44 > +const NetworkManager::Setting::SecretFlags otpFlag = > static_cast(data.value(NM_FORTISSLVPN_KEY_OTP"-flags").toInt()); > +if (otpFlag == NetworkManager::Setting::None){ > +d->ui.otpFrame->setVisible(fals

D17834: Allow the use of One-Time Password

2019-01-03 Thread Enrique Melendez
enriquem updated this revision to Diff 48614. enriquem added a comment. Treated otp-flags the same way as password-flags. But we have to be careful to consider the case where the option is not yet in the config file (as will certainly be the case the firs time it is used if there was a vpn al

D17834: Allow the use of One-Time Password

2019-01-03 Thread Jan Grulich
jgrulich added a comment. Can you please work with "otp-flags" same way we work with "password-flags"? You work with it as with string, it would be more readable if you use NetworkManager::Setting::SecretFlags. REPOSITORY R116 Plasma Network Management Applet REVISION DETAIL https://pha

D17834: Allow the use of One-Time Password

2018-12-30 Thread Enrique Melendez
enriquem updated this revision to Diff 48397. enriquem edited the summary of this revision. REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17834?vs=48328&id=48397 REVISION DETAIL https://phabricator.kde.org/D17834 AFFECTED FILES v

D17834: Allow the use of One-Time Password

2018-12-28 Thread Enrique Melendez
enriquem added a comment. You are, of course, correct. How can I have missed that? REVISION DETAIL https://phabricator.kde.org/D17834 To: enriquem, jgrulich Cc: plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17834: Allow the use of One-Time Password

2018-12-28 Thread Enrique Melendez
enriquem updated this revision to Diff 48328. enriquem edited the summary of this revision. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17834?vs=48289&id=48328 REVISION DETAIL https://phabricator.kde.org/D17834 AFFECTED FILES vpn/fortisslvpn/fortisslvpnadvanced.ui vpn/fortissl

D17834: Allow the use of One-Time Password

2018-12-28 Thread Jan Grulich
jgrulich added a comment. I don't think this is a complete support for this. You need to add support into the auth-dialog as well, without it the connection would expect an otp password, but there will be no way how to provide it. See https://github.com/GNOME/network-manager-fortisslvpn

D17834: Allow the use of One-Time Password

2018-12-28 Thread Enrique Melendez
enriquem created this revision. enriquem added a reviewer: jgrulich. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. enriquem requested review of this revision. REVISION SUMMARY NetworkManager-fortisslvpn allows the user to use a One-Time Password challenge, but this op