Review Request: [RFC] Import VPN connection

Rajeesh K Nambiar rajeeshknambiar at gmail.com
Tue May 24 20:45:02 CEST 2011



> On May 24, 2011, 5:44 p.m., Lamarque Vieira Souza wrote:
> > settings/config/manageconnectionwidget.cpp, line 519
> > <http://git.reviewboard.kde.org/r/101419/diff/3/?file=18004#file18004line519>
> >
> >     Shouldn't you also emit a notification or a popup warning the user that the import failed?

Yes, certainly so. But I'm not sure how exactly to do it. Should I be modifying libs/service/notificationmanager.cpp? Could you suggest/point to how to implement this?


> On May 24, 2011, 5:44 p.m., Lamarque Vieira Souza wrote:
> > vpnplugins/vpnc/vpnc.cpp, line 139
> > <http://git.reviewboard.kde.org/r/101419/diff/3/?file=18014#file18014line139>
> >
> >     Please remove space at line start. Also, you do not insert the password into the connection map if the executable failed to run. I guess the VPN connection is not going to work in that case, right? If so I think you should abort the import process. Think of this function as a (SQL) transation: or everything works or the entire import process is aborted.

VPNC user password is almost uninteresting regarding the pcf file. Most of the time it will be empty and the user has to manually input. The important bit is Group Password/Encrypted Group Password. IMHO, it would be better not to abort the import if password decryption fails, but to inform the user; so that he can manually rectify it or contact the administrator.
Further, it is very rare for cisco-decrypt to fail, and guaranteed that it will be installed since vpnc package is required dependency for NetworkManager-vpnc which is required for kde-plasma-networkmanagement-vpnc package.


> On May 24, 2011, 5:44 p.m., Lamarque Vieira Souza wrote:
> > settings/config/manageconnectionwidget.cpp, line 528
> > <http://git.reviewboard.kde.org/r/101419/diff/3/?file=18004#file18004line528>
> >
> >     If this button is visible you should also emit a notification or a popup warning that this has not been implemented yet. Or just hide the export button.

I will try to implement the functionality in next iteration; and if it looks very difficult then we can simply hide it.


- Rajeesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101419/#review3484
-----------------------------------------------------------


On May 24, 2011, 2:09 p.m., Rajeesh K Nambiar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101419/
> -----------------------------------------------------------
> 
> (Updated May 24, 2011, 2:09 p.m.)
> 
> 
> Review request for Network Management.
> 
> 
> Summary
> -------
> 
> First stab at VPN connection import/export functionality. Currently implemented just VPNC Import support. Please review, especially the VpncUiPluginPrivate part which tries to abstract away cisco password decrypt function. If the general approach looks good, I'll proceed with this and try to extend for other VPN methods, as well as export function.
> 
> 
> This addresses bug 146159.
>     http://bugs.kde.org/show_bug.cgi?id=146159
> 
> 
> Diffs
> -----
> 
>   libs/ui/vpnpreferences.cpp 843636c 
>   libs/ui/vpnuiplugin.h 7a13027 
>   settings/config/CMakeLists.txt 268c23b 
>   settings/config/addeditdeletebuttonset.h f7abef7 
>   settings/config/addeditdeletebuttonset.cpp 4f3f97a 
>   settings/config/manageconnectionwidget.h 51f60a0 
>   settings/config/manageconnectionwidget.cpp 46910db 
>   vpnplugins/novellvpn/novellvpn.h 9e026e2 
>   vpnplugins/novellvpn/novellvpn.cpp 848b527 
>   vpnplugins/openvpn/openvpn.h a06b88e 
>   vpnplugins/openvpn/openvpn.cpp 60376ed 
>   vpnplugins/pptp/pptp.h 66ea79a 
>   vpnplugins/pptp/pptp.cpp c311f9f 
>   vpnplugins/strongswan/strongswan.h fcd5bde 
>   vpnplugins/strongswan/strongswan.cpp 5bffc2b 
>   vpnplugins/vpnc/vpnc.h aec2136 
>   vpnplugins/vpnc/vpnc.cpp deb9108 
> 
> Diff: http://git.reviewboard.kde.org/r/101419/diff
> 
> 
> Testing
> -------
> 
> Tested against latest git snapshot, with KDE SC 4.6.3
> 
> 
> Thanks,
> 
> Rajeesh
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110524/e86ea7bf/attachment-0001.htm 


More information about the kde-networkmanager mailing list