Review Request: [RFC] Import VPN connection

Discussion:

Review Request: [RFC] Import VPN connection

Rajeesh K Nambiar

2011-05-22 16:30:36 UTC

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

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 4c889c3
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/20110522/d02070cc/attachment.htm

Lamarque Vieira Souza

2011-05-22 23:21:08 UTC

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

libs/ui/vpnpreferences.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2901>

This should be "> 5" since the array below is indexed to the 6th element ([5] is the 6th element, not the fifth).

libs/ui/vpnpreferences.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2902>

same here.

vpnplugins/novellvpn/novellvpn.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2903>

Is this method supposed to do something? If so then add a TODO comment about what it is going to do once implemented.

vpnplugins/novellvpn/novellvpn.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2900>

Teh QVariantList is created cleared, this is redundant.

vpnplugins/openvpn/openvpn.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2904>

redundant.

vpnplugins/pptp/pptp.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2905>

redundant.

vpnplugins/strongswan/strongswan.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2906>

redundant.

vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2907>

Do not do this. If the readAll or the split calls return an unexpected result this will crash Plasma NM. Execute each call individually and test the result before using it.

vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2908>

This should be hardcoded unless you can garantee all Linux distributions, BSD*, Unix, etc store the exececutable in the same directory.

- Lamarque Vieira

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated May 22, 2011, 4:30 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 4c889c3
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/20110522/c8252afd/attachment-0001.htm

Lamarque Vieira Souza

2011-05-22 23:22:23 UTC

Post by Rajeesh K Nambiar

vpnplugins/vpnc/vpnc.cpp, line 104
<http://git.reviewboard.kde.org/r/101419/diff/1/?file=17707#file17707line104>
This should be hardcoded unless you can garantee all Linux distributions, BSD*, Unix, etc store the exececutable in the same directory.

I mean, this should not be hardcoded.

- Lamarque Vieira

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

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated May 22, 2011, 4:30 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 4c889c3
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/20110522/6583544b/attachment.htm

Rajeesh K Nambiar

2011-06-02 19:09:39 UTC

Post by Lamarque Vieira Souza

vpnplugins/vpnc/vpnc.cpp, line 104
<http://git.reviewboard.kde.org/r/101419/diff/1/?file=17707#file17707line104>
This should be hardcoded unless you can garantee all Linux distributions, BSD*, Unix, etc store the exececutable in the same directory.

I mean, this should not be hardcoded.

Updated patch (r2) is now looking at /usr/bin, /bin and /usr/local/bin/ (in that order) to find cisco-decrypt; which I guess would cover most normal cases.
OpenVpnSettingWidget::OpenVpnSettingWidget is doing something similar, I guess we would need to update that too. I'll provide a separate patch for that later.

- Rajeesh

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

Post by Lamarque Vieira Souza
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 2, 2011, 7: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 fdd350b
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/nm-vpnc-service.h e3f859a
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/20110602/f2f35138/attachment.htm

Lamarque Vieira Souza

2011-06-02 19:38:27 UTC

Post by Lamarque Vieira Souza

vpnplugins/vpnc/vpnc.cpp, line 104
<http://git.reviewboard.kde.org/r/101419/diff/1/?file=17707#file17707line104>
This should be hardcoded unless you can garantee all Linux distributions, BSD*, Unix, etc store the exececutable in the same directory.

I mean, this should not be hardcoded.
Updated patch (r2) is now looking at /usr/bin, /bin and /usr/local/bin/ (in that order) to find cisco-decrypt; which I guess would cover most normal cases.
OpenVpnSettingWidget::OpenVpnSettingWidget is doing something similar, I guess we would need to update that too. I'll provide a separate patch for that later.

The patch is good to go, just do the renames I mentioned. Please provides a version for nm09 branch, since nm09 branch does not use *Persistence classes anymore this patch is not going to apply cleanly.

- Lamarque Vieira

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

Post by Lamarque Vieira Souza
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 2, 2011, 7: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 fdd350b
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/nm-vpnc-service.h e3f859a
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/20110602/2139b6e6/attachment.htm

Rajeesh K Nambiar

2011-06-02 20:00:12 UTC

Post by Lamarque Vieira Souza

vpnplugins/vpnc/vpnc.cpp, line 104
<http://git.reviewboard.kde.org/r/101419/diff/1/?file=17707#file17707line104>
This should be hardcoded unless you can garantee all Linux distributions, BSD*, Unix, etc store the exececutable in the same directory.

I mean, this should not be hardcoded.
Updated patch (r2) is now looking at /usr/bin, /bin and /usr/local/bin/ (in that order) to find cisco-decrypt; which I guess would cover most normal cases.
OpenVpnSettingWidget::OpenVpnSettingWidget is doing something similar, I guess we would need to update that too. I'll provide a separate patch for that later.
The patch is good to go, just do the renames I mentioned. Please provides a version for nm09 branch, since nm09 branch does not use *Persistence classes anymore this patch is not going to apply cleanly.

I'll try to rebase the patch against nm09 and submit separately. What is the best way to do that? 'git-branch nm09' and apply the patch + modify?

- Rajeesh

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

Post by Lamarque Vieira Souza
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 2, 2011, 7:56 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 fdd350b
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/nm-vpnc-service.h e3f859a
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/20110602/ce058848/attachment-0001.htm

Lamarque Vieira Souza

2011-06-05 21:25:38 UTC

Post by Lamarque Vieira Souza

vpnplugins/vpnc/vpnc.cpp, line 104
<http://git.reviewboard.kde.org/r/101419/diff/1/?file=17707#file17707line104>
This should be hardcoded unless you can garantee all Linux distributions, BSD*, Unix, etc store the exececutable in the same directory.

I mean, this should not be hardcoded.
Updated patch (r2) is now looking at /usr/bin, /bin and /usr/local/bin/ (in that order) to find cisco-decrypt; which I guess would cover most normal cases.
OpenVpnSettingWidget::OpenVpnSettingWidget is doing something similar, I guess we would need to update that too. I'll provide a separate patch for that later.
The patch is good to go, just do the renames I mentioned. Please provides a version for nm09 branch, since nm09 branch does not use *Persistence classes anymore this patch is not going to apply cleanly.
I'll try to rebase the patch against nm09 and submit separately. What is the best way to do that? 'git-branch nm09' and apply the patch + modify?

Ship it, both NM-0.8 and NM-0.9 versions. Do you have write access to networkmanagement repository?

- Lamarque Vieira

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

Post by Lamarque Vieira Souza
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 5, 2011, 6:45 a.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/internals/settings/vpnsecrets.h 96803ec
libs/ui/vpnpreferences.cpp 843636c
libs/ui/vpnuiplugin.h 7a13027
settings/config/CMakeLists.txt 783d551
settings/config/addeditdeletebuttonset.h f7abef7
settings/config/addeditdeletebuttonset.cpp 4f3f97a
settings/config/manageconnectionwidget.h e6f1b93
settings/config/manageconnectionwidget.cpp dad1676
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/nm-vpnc-service.h e3f859a
vpnplugins/vpnc/vpnc.h aec2136
vpnplugins/vpnc/vpnc.cpp deb9108
vpnplugins/vpnc/vpncprop.ui 92093cf
vpnplugins/vpnc/vpncwidget.cpp 171d68b
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/20110605/c04e4462/attachment.htm

Rajeesh K Nambiar

2011-06-06 08:09:30 UTC

Post by Lamarque Vieira Souza

vpnplugins/vpnc/vpnc.cpp, line 104
<http://git.reviewboard.kde.org/r/101419/diff/1/?file=17707#file17707line104>
This should be hardcoded unless you can garantee all Linux distributions, BSD*, Unix, etc store the exececutable in the same directory.

I mean, this should not be hardcoded.
Updated patch (r2) is now looking at /usr/bin, /bin and /usr/local/bin/ (in that order) to find cisco-decrypt; which I guess would cover most normal cases.
OpenVpnSettingWidget::OpenVpnSettingWidget is doing something similar, I guess we would need to update that too. I'll provide a separate patch for that later.
The patch is good to go, just do the renames I mentioned. Please provides a version for nm09 branch, since nm09 branch does not use *Persistence classes anymore this patch is not going to apply cleanly.
I'll try to rebase the patch against nm09 and submit separately. What is the best way to do that? 'git-branch nm09' and apply the patch + modify?
Ship it, both NM-0.8 and NM-0.9 versions. Do you have write access to networkmanagement repository?

Thanks!
I don't have write access to networkmanagement yet.

- Rajeesh

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

Post by Lamarque Vieira Souza
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 5, 2011, 6:45 a.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/internals/settings/vpnsecrets.h 96803ec
libs/ui/vpnpreferences.cpp 843636c
libs/ui/vpnuiplugin.h 7a13027
settings/config/CMakeLists.txt 783d551
settings/config/addeditdeletebuttonset.h f7abef7
settings/config/addeditdeletebuttonset.cpp 4f3f97a
settings/config/manageconnectionwidget.h e6f1b93
settings/config/manageconnectionwidget.cpp dad1676
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/nm-vpnc-service.h e3f859a
vpnplugins/vpnc/vpnc.h aec2136
vpnplugins/vpnc/vpnc.cpp deb9108
vpnplugins/vpnc/vpncprop.ui 92093cf
vpnplugins/vpnc/vpncwidget.cpp 171d68b
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/20110606/4a2e9884/attachment-0001.htm

Lamarque Vieira Souza

2011-06-07 00:30:11 UTC

Post by Lamarque Vieira Souza

vpnplugins/vpnc/vpnc.cpp, line 104
<http://git.reviewboard.kde.org/r/101419/diff/1/?file=17707#file17707line104>
This should be hardcoded unless you can garantee all Linux distributions, BSD*, Unix, etc store the exececutable in the same directory.

I mean, this should not be hardcoded.
Updated patch (r2) is now looking at /usr/bin, /bin and /usr/local/bin/ (in that order) to find cisco-decrypt; which I guess would cover most normal cases.
OpenVpnSettingWidget::OpenVpnSettingWidget is doing something similar, I guess we would need to update that too. I'll provide a separate patch for that later.
The patch is good to go, just do the renames I mentioned. Please provides a version for nm09 branch, since nm09 branch does not use *Persistence classes anymore this patch is not going to apply cleanly.
I'll try to rebase the patch against nm09 and submit separately. What is the best way to do that? 'git-branch nm09' and apply the patch + modify?
Ship it, both NM-0.8 and NM-0.9 versions. Do you have write access to networkmanagement repository?
Thanks!
I don't have write access to networkmanagement yet.

Ok, then send me the NM-0.8 version by e-mail and I will apply this one to nm09 branch.

- Lamarque Vieira

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

Post by Lamarque Vieira Souza
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 5, 2011, 6:45 a.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/internals/settings/vpnsecrets.h 96803ec
libs/ui/vpnpreferences.cpp 843636c
libs/ui/vpnuiplugin.h 7a13027
settings/config/CMakeLists.txt 783d551
settings/config/addeditdeletebuttonset.h f7abef7
settings/config/addeditdeletebuttonset.cpp 4f3f97a
settings/config/manageconnectionwidget.h e6f1b93
settings/config/manageconnectionwidget.cpp dad1676
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/nm-vpnc-service.h e3f859a
vpnplugins/vpnc/vpnc.h aec2136
vpnplugins/vpnc/vpnc.cpp deb9108
vpnplugins/vpnc/vpncprop.ui 92093cf
vpnplugins/vpnc/vpncwidget.cpp 171d68b
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/20110607/6fbad579/attachment.htm

Rajeesh K Nambiar

2011-06-07 05:24:50 UTC

Post by Lamarque Vieira Souza

vpnplugins/vpnc/vpnc.cpp, line 104
<http://git.reviewboard.kde.org/r/101419/diff/1/?file=17707#file17707line104>
This should be hardcoded unless you can garantee all Linux distributions, BSD*, Unix, etc store the exececutable in the same directory.

I mean, this should not be hardcoded.
Updated patch (r2) is now looking at /usr/bin, /bin and /usr/local/bin/ (in that order) to find cisco-decrypt; which I guess would cover most normal cases.
OpenVpnSettingWidget::OpenVpnSettingWidget is doing something similar, I guess we would need to update that too. I'll provide a separate patch for that later.
The patch is good to go, just do the renames I mentioned. Please provides a version for nm09 branch, since nm09 branch does not use *Persistence classes anymore this patch is not going to apply cleanly.
I'll try to rebase the patch against nm09 and submit separately. What is the best way to do that? 'git-branch nm09' and apply the patch + modify?
Ship it, both NM-0.8 and NM-0.9 versions. Do you have write access to networkmanagement repository?
Thanks!
I don't have write access to networkmanagement yet.
Ok, then send me the NM-0.8 version by e-mail and I will apply this one to nm09 branch.

Final patch against NM-0.8 is revision 5 - https://git.reviewboard.kde.org/r/101419/diff/5/
revisions from 6 to 8 are against NM-0.9

- Rajeesh

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

Post by Lamarque Vieira Souza
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 5, 2011, 6:45 a.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/internals/settings/vpnsecrets.h 96803ec
libs/ui/vpnpreferences.cpp 843636c
libs/ui/vpnuiplugin.h 7a13027
settings/config/CMakeLists.txt 783d551
settings/config/addeditdeletebuttonset.h f7abef7
settings/config/addeditdeletebuttonset.cpp 4f3f97a
settings/config/manageconnectionwidget.h e6f1b93
settings/config/manageconnectionwidget.cpp dad1676
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/nm-vpnc-service.h e3f859a
vpnplugins/vpnc/vpnc.h aec2136
vpnplugins/vpnc/vpnc.cpp deb9108
vpnplugins/vpnc/vpncprop.ui 92093cf
vpnplugins/vpnc/vpncwidget.cpp 171d68b
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/20110607/437635e0/attachment.htm

Rajeesh K Nambiar

2011-05-23 19:27:04 UTC

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

(Updated May 23, 2011, 7:27 p.m.)

Review request for Network Management.

Changes
-------

Addressed the review comments with this patch.

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 (updated)
-----

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/20110523/a6647349/attachment.htm

Lamarque Vieira Souza

2011-05-23 20:01:07 UTC

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

settings/config/manageconnectionwidget.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2919>

0 instead of NULL.

vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2917>

Use 0 instead of NULL. NULL is used C, 0 is used in C++.

vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2918>

0 instead of NULL.

vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2920>

You should not proceed if the executable has not been found or there was any error running the process.

You can look at git://anongit.kde.org/kde-workspace/solid/wicd/wirednetworkinterface.cpp, WicdWiredNetworkInterface::hardwareAddress to see how I implemented the "ifconfig" call. The important parts are the line

qputenv("PATH", QString("/bin:/usr/bin:/sbin:/usr/sbin:" + env.value("PATH")).toAscii());

This way you can easily add new paths to find.

Use QProcess::waitForStarted() to check if the process was actually executed. If the executable was not found or the user does not has permission to run it waitForStarted will return error.

vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2921>

Check the returned value for possible errors and act accordingly.

vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2922>

same here.

- Lamarque Vieira

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated May 23, 2011, 7:27 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/20110523/ee309ae2/attachment-0001.htm

Lamarque Vieira Souza

2011-05-23 23:28:33 UTC

Post by Rajeesh K Nambiar

vpnplugins/vpnc/vpnc.cpp, line 117
<http://git.reviewboard.kde.org/r/101419/diff/2/?file=17809#file17809li
ne117>
You should not proceed if the executable has not been found or
there was any error running the process.
You can look at
git://anongit.kde.org/kde-workspace/solid/wicd/wirednetworkinterfa
ce.cpp, WicdWiredNetworkInterface::hardwareAddress to see how I
implemented the "ifconfig" call. The important parts are the line
qputenv("PATH", QString("/bin:/usr/bin:/sbin:/usr/sbin:" +
env.value("PATH")).toAscii());
This way you can easily add new paths to find.
Use QProcess::waitForStarted() to check if the process was actually
executed. If the executable was not found or the user does not has
permission to run it waitForStarted will return error.

Why not use KStandardDirs::findExe?

If it works like ' kde4-config --path exe --locate <file>' then it does
not search in /sbin and /usr/sbin.

- Kevin
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/#review3471
-----------------------------------------------------------

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated May 23, 2011, 7:27 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

--
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/
http://planetkde.org/pt-br
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110523/1c332035/attachment.html

Lamarque Vieira Souza

2011-05-23 23:46:57 UTC

Post by Lamarque Vieira Souza

Why not use KStandardDirs::findExe?

If it works like ' kde4-config --path exe --locate <file>' then it does
not search in /sbin and /usr/sbin.

findExe also searches in $PATH in addition to the above search path, so if
the distro puts /sbin and /usr/sbin into $PATH (at least current versions
of Fedora do that), it also finds executables there by default.
Plus, findExe can take an optional pathstr parameter which is searched
instead of $PATH, so you can pass a tweaked PATH to the function.
By default, findExe will also verify that the executable is actually
executable, not just existing. This too can be turned off, with an optional
third parameter, but I guess the check is useful anyway.
See the documentation.
Kevin Kofler

Yes, it works. Thanks for the tip.

--
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/
http://planetkde.org/pt-br
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110523/26101183/attachment.htm

Kevin Kofler

2011-05-23 23:36:38 UTC

Post by Lamarque Vieira Souza

Why not use KStandardDirs::findExe?

If it works like ' kde4-config --path exe --locate <file>' then it does
not search in /sbin and /usr/sbin.

findExe also searches in $PATH in addition to the above search path, so if the
distro puts /sbin and /usr/sbin into $PATH (at least current versions of
Fedora do that), it also finds executables there by default.

Plus, findExe can take an optional pathstr parameter which is searched instead
of $PATH, so you can pass a tweaked PATH to the function.

By default, findExe will also verify that the executable is actually
executable, not just existing. This too can be turned off, with an optional
third parameter, but I guess the check is useful anyway.

See the documentation.

Kevin Kofler

Kevin Kofler

2011-05-23 23:15:42 UTC

Post by Rajeesh K Nambiar

vpnplugins/vpnc/vpnc.cpp, line 117
<http://git.reviewboard.kde.org/r/101419/diff/2/?file=17809#file17809line117>
You should not proceed if the executable has not been found or there was any error running the process.
You can look at git://anongit.kde.org/kde-workspace/solid/wicd/wirednetworkinterface.cpp, WicdWiredNetworkInterface::hardwareAddress to see how I implemented the "ifconfig" call. The important parts are the line
qputenv("PATH", QString("/bin:/usr/bin:/sbin:/usr/sbin:" + env.value("PATH")).toAscii());
This way you can easily add new paths to find.
Use QProcess::waitForStarted() to check if the process was actually executed. If the executable was not found or the user does not has permission to run it waitForStarted will return error.

Why not use KStandardDirs::findExe?

- Kevin

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

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated May 23, 2011, 7:27 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/20110523/8ceefac3/attachment-0001.htm

Rajeesh K Nambiar

2011-05-24 14:09:54 UTC

-----------------------------------------------------------
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.

Changes
-------

Updated against comments on revision 2.

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 (updated)
-----

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/96da92b5/attachment.htm

Lamarque Vieira Souza

2011-05-24 17:44:31 UTC

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

settings/config/manageconnectionwidget.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2941>

Shouldn't you also emit a notification or a popup warning the user that the import failed?

settings/config/manageconnectionwidget.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2940>

Please, remove extra line.

settings/config/manageconnectionwidget.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2942>

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.

vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2944>

Notification or popup informing about failed execution.

vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2945>

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.

vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment2946>

Same here: remove extra space at line start and transation (or everything works or the entire import process is aborted).

We already have some sutle bugs in VPN code very difficult to debug/fix. If we can prevent some more to show up in new code the better.

- Lamarque Vieira

Post by Rajeesh K Nambiar
-----------------------------------------------------------
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/b6efe20a/attachment-0001.htm

Lamarque Vieira Souza

2011-05-24 19:03:47 UTC

settings/config/manageconnectionwidget.cpp, line 519
<http://git.reviewboard.kde.org/r/101419/diff/3/?file=18004#file18004li
ne519>
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?

No, notificationmanager.cpp is kind of passive, it always waits for
something to notify but only from the networkinterface. In your case you
should create the KNotification object and emit it yourself, something like:

#include <KNotification>
#include "../libs/service/events.h" // maybe you will need to change this path

K_GLOBAL_STATIC_WITH_ARGS(KComponentData, s_networkManagementComponentData,
("networkmanagement", "networkmanagement",
KComponentData::SkipMainComponentRegistration))

static const int m_iconSize = 48;
//end of global variables

KNotification::event(Event::VpnCertificateImportFailed, i18nc("@info:status
Notification when importing VPN certificate failed", "Certificate import
failed"), QPixmap(), 0, KNotification::CloseOnTimeout,
*s_networkManagementComponentDa
ta)->sendEvent();

Then in ../libs/service/events.h you declare
Event::VpnCertificateImportFailed and in ../libs/service/events.cpp define it:
const QString Event::VpnCertificateImportFailed =
QLatin1String("vpncertificateimportfailed");

In networkmanagement.notifyrc you add the info about the
vpncertificateimportfailed notification:

[Event/vpncertificateimportfailed]
Name=VPN certificate import failed
Comment=Importing of VPN certificate has failed

vpnplugins/vpnc/vpnc.cpp, line 139
<http://git.reviewboard.kde.org/r/101419/diff/3/?file=18014#file18014li
ne139>
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.

Well, NetworkManager-vpnc is not always installed. I for example did not
have it installed when I first tried to use VPN in my notebook. It took me a
while to figure out I should have it installed before I could use VPN. Warning
about missing dependencies is one thing we could improve in Plasma NM in the
future.

Ok then, no need to abort the import, just add a notification about it.

settings/config/manageconnectionwidget.cpp, line 528
<http://git.reviewboard.kde.org/r/101419/diff/3/?file=18004#file18004li
ne528>
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.

Ok.

--
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/
http://planetkde.org/pt-br
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110524/c1ff9ae3/attachment.htm

Rajeesh K Nambiar

2011-05-26 05:26:02 UTC

On Wed, May 25, 2011 at 12:33 AM, Lamarque Vieira Souza

Post by Lamarque Vieira Souza

settings/config/manageconnectionwidget.cpp, line 519
<http://git.reviewboard.kde.org/r/101419/diff/3/?file=18004#file18004li
ne519>
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?

No, notificationmanager.cpp is kind of passive, it always waits for
something to notify but only from the networkinterface. In your case you
#include <KNotification>
#include "../libs/service/events.h" // maybe you will need to change this path
K_GLOBAL_STATIC_WITH_ARGS(KComponentData, s_networkManagementComponentData,
("networkmanagement", "networkmanagement",
KComponentData::SkipMainComponentRegistration))
static const int m_iconSize = 48;
//end of global variables
Notification when importing VPN certificate failed", "Certificate import
failed"), QPixmap(), 0, KNotification::CloseOnTimeout,
*s_networkManagementComponentDa
ta)->sendEvent();
Then in ../libs/service/events.h you declare
const QString Event::VpnCertificateImportFailed =
QLatin1String("vpncertificateimportfailed");
In networkmanagement.notifyrc you add the info about the
[Event/vpncertificateimportfailed]
Name=VPN certificate import failed
Comment=Importing of VPN certificate has failed

Looking at manageconnectionwidget.cpp, deleteClicked(), I was
wondering - wouldn't it be simpler and in line with other places to
use KMessageBox about VPN connection import failure; also with
cisco-decrypt errors?

Post by Lamarque Vieira Souza

vpnplugins/vpnc/vpnc.cpp, line 139
<http://git.reviewboard.kde.org/r/101419/diff/3/?file=18014#file18014li
ne139>
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.

Well, NetworkManager-vpnc is not always installed. I for example did not
have it installed when I first tried to use VPN in my notebook. It took me a
while to figure out I should have it installed before I could use VPN.
Warning about missing dependencies is one thing we could improve in Plasma
NM in the future.
Ok then, no need to abort the import, just add a notification about it.

settings/config/manageconnectionwidget.cpp, line 528
<http://git.reviewboard.kde.org/r/101419/diff/3/?file=18004#file18004li
ne528>
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.

Ok.
--
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/
http://planetkde.org/pt-br

--
Cheers,
Rajeesh
http://rajeeshknambiar.wordpress.com

Lamarque Vieira Souza

2011-05-26 05:49:17 UTC

Post by Rajeesh K Nambiar
Looking at manageconnectionwidget.cpp, deleteClicked(), I was
wondering - wouldn't it be simpler and in line with other places to
use KMessageBox about VPN connection import failure; also with
cisco-decrypt errors?

Ok, in this case you can use popups since the user is going to wait for
the import status anyway. Users usually do not like popups, so try to minimize
the number of popups.

--
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/
http://planetkde.org/pt-br
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110526/637ec449/attachment.htm

Rajeesh K Nambiar

2011-05-24 18:45:02 UTC

Post by Rajeesh K Nambiar

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?

Post by Rajeesh K Nambiar

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.

Post by Rajeesh K Nambiar

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
-----------------------------------------------------------

Post by Rajeesh K Nambiar
-----------------------------------------------------------
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

Rajeesh K Nambiar

2011-06-02 19:09:21 UTC

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

(Updated June 2, 2011, 7:09 p.m.)

Review request for Network Management.

Changes
-------

Added VPN export functionality and addressed all the comments about r3.

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 (updated)
-----

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 fdd350b
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/nm-vpnc-service.h e3f859a
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/20110602/c76a8be7/attachment.htm

Lamarque Vieira Souza

2011-06-02 19:37:42 UTC

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

libs/ui/vpnuiplugin.h
<http://git.reviewboard.kde.org/r/101419/#comment3027>

I think importConnectionSettings sounds better than importConnectionDetails.

settings/config/manageconnectionwidget.cpp
<http://git.reviewboard.kde.org/r/101419/#comment3025>

"Import VPN connection settings" as last argument.

settings/config/manageconnectionwidget.cpp
<http://git.reviewboard.kde.org/r/101419/#comment3026>

"Could not import VPN connection settings"

settings/config/manageconnectionwidget.cpp
<http://git.reviewboard.kde.org/r/101419/#comment3024>

Change this to "VPN connection successfuly exported". Not all people will figure out at first glance that this imports/exports VPN connection settings.

- Lamarque Vieira

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 2, 2011, 7: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 fdd350b
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/nm-vpnc-service.h e3f859a
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/20110602/18f66fed/attachment-0001.htm

Rajeesh K Nambiar

2011-06-02 19:56:27 UTC

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

(Updated June 2, 2011, 7:56 p.m.)

Review request for Network Management.

Changes
-------

Updated diff renaming {im,ex}portConnectionDetails to {im,ex}portConnectionSettings.

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 (updated)
-----

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 fdd350b
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/nm-vpnc-service.h e3f859a
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/20110602/c807ccd9/attachment.htm

Rajeesh K Nambiar

2011-06-03 18:43:02 UTC

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

(Updated June 3, 2011, 6:43 p.m.)

Review request for Network Management.

Changes
-------

Patch against nm09 branch. Implemented VPNC import/export. Tested against NetworkManager-0.8.9997-1.git20110531.

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 (updated)
-----

libs/internals/settings/vpnsecrets.h 96803ec
libs/ui/vpnpreferences.cpp 843636c
libs/ui/vpnuiplugin.h 7a13027
settings/config/CMakeLists.txt 783d551
settings/config/addeditdeletebuttonset.h f7abef7
settings/config/addeditdeletebuttonset.cpp 4f3f97a
settings/config/manageconnectionwidget.h 41a449c
settings/config/manageconnectionwidget.cpp db69cb6
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/nm-vpnc-service.h e3f859a
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/20110603/b0b63dfc/attachment.htm

Lamarque Vieira Souza

2011-06-03 19:13:03 UTC

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

settings/config/manageconnectionwidget.cpp
<http://git.reviewboard.kde.org/r/101419/#comment3035>

At least for now I am assuming VPN connections are always user scope. User scope in NM-0.9 is different from NM-0.8: in NM-0.8 there is one DBus interface for each scope, so in Plasma NM-0.8 we have two objects, one for each interface. Furthermore the connection settings and secrets are stored or in system (/etc/NetworkManager/system-connections) or in user (~/.kde/share/apps/networkmanagement/connections) side.

In NM-0.9 there is only one DBus interface and only the secrets are stored in system or user side (AgentOwned in NM-0.9 terminology). The settings are always saved in system side (/etc/NetworkManager/system-connections).

That all means you do not need to check the scope here, just save all secrets as AgentOwned and that is it. When you do it please add a comment saying you are assuming the secrets are AgentOnwed. I still not figure out how to have system wide VPN connections, so maybe in the future I will have to change that and the comment will make life easier to find where to find in the source code.

vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment3036>

You also need to add entries like:

"ipsec-secret-type" -> "save"
"IPSec secret-flags" -> "0"

"xauth-password-type" -> "ask"
"Xauth password-flags" -> "2"

To indicate where the secrets are going to be saved. Look at how VpnSetting::data() in libs/internals/settings/vpn.h does it for example.

The integer values above (0, 2 in the examples) are of type secretsTypes and should alwas be "Or"ed with AgentOwned value (1), look at VpnSetting::storageTypeToSecretsType() does it.

That should be done for all secrets.

- Lamarque Vieira

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 3, 2011, 6:43 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/internals/settings/vpnsecrets.h 96803ec
libs/ui/vpnpreferences.cpp 843636c
libs/ui/vpnuiplugin.h 7a13027
settings/config/CMakeLists.txt 783d551
settings/config/addeditdeletebuttonset.h f7abef7
settings/config/addeditdeletebuttonset.cpp 4f3f97a
settings/config/manageconnectionwidget.h 41a449c
settings/config/manageconnectionwidget.cpp db69cb6
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/nm-vpnc-service.h e3f859a
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/20110603/29596190/attachment-0001.htm

Rajeesh K Nambiar

2011-06-04 09:35:50 UTC

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

(Updated June 4, 2011, 9:35 a.m.)

Review request for Network Management.

Changes
-------

Updated: replace scope checking with comment on AgentOwned.

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 (updated)
-----

libs/internals/settings/vpnsecrets.h 96803ec
libs/ui/vpnpreferences.cpp 843636c
libs/ui/vpnuiplugin.h 7a13027
settings/config/CMakeLists.txt 783d551
settings/config/addeditdeletebuttonset.h f7abef7
settings/config/addeditdeletebuttonset.cpp 4f3f97a
settings/config/manageconnectionwidget.h 41a449c
settings/config/manageconnectionwidget.cpp db69cb6
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/nm-vpnc-service.h e3f859a
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/20110604/9bf6c1c1/attachment.htm

Lamarque Vieira Souza

2011-06-04 18:19:01 UTC

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

vpnplugins/vpnc/vpnc.cpp
<http://git.reviewboard.kde.org/r/101419/#comment3038>

The "...-flags" entry is missing. The "...-type" is from NM-0.8 and probably is going to be phased out sometime in the future.

The "...-flags" also has higher priority in NM-0.9 and let us choose where the secrets are going to be saved using the AgentOwned flag. The way you did probably the secrets are being saved in a plain text file in /etc/NetworkManager/system-connections/. Plasma NM uses AgentOwned secrets by default exactly to prevent secrets being saved in plain text (well, at least when user has chosen kwallet as storage).

Please add the "...-flags" entries and check where the secrets are actually being saved (in /etc/... or in kwallet), that is important.

- Lamarque Vieira

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 4, 2011, 9:35 a.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/internals/settings/vpnsecrets.h 96803ec
libs/ui/vpnpreferences.cpp 843636c
libs/ui/vpnuiplugin.h 7a13027
settings/config/CMakeLists.txt 783d551
settings/config/addeditdeletebuttonset.h f7abef7
settings/config/addeditdeletebuttonset.cpp 4f3f97a
settings/config/manageconnectionwidget.h 41a449c
settings/config/manageconnectionwidget.cpp db69cb6
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/nm-vpnc-service.h e3f859a
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/20110604/0f2fa384/attachment.htm

Rajeesh K Nambiar

2011-06-04 18:36:50 UTC

Post by Rajeesh K Nambiar

vpnplugins/vpnc/vpnc.cpp, line 158
<http://git.reviewboard.kde.org/r/101419/diff/7/?file=20930#file20930line158>
The "...-flags" entry is missing. The "...-type" is from NM-0.8 and probably is going to be phased out sometime in the future.
The "...-flags" also has higher priority in NM-0.9 and let us choose where the secrets are going to be saved using the AgentOwned flag. The way you did probably the secrets are being saved in a plain text file in /etc/NetworkManager/system-connections/. Plasma NM uses AgentOwned secrets by default exactly to prevent secrets being saved in plain text (well, at least when user has chosen kwallet as storage).
Please add the "...-flags" entries and check where the secrets are actually being saved (in /etc/... or in kwallet), that is important.

Should this be taken care by mEditor->createConnection? I don't see "...-flags" added to the connection settings from anywhere else. Also, whether the secrets are stored in plain text or kwallet is to be handled elsewhere, no?
If not, what needs to be done by checking SecretStorageMode == PlainText or Secure?

- Rajeesh

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

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 4, 2011, 9:35 a.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/internals/settings/vpnsecrets.h 96803ec
libs/ui/vpnpreferences.cpp 843636c
libs/ui/vpnuiplugin.h 7a13027
settings/config/CMakeLists.txt 783d551
settings/config/addeditdeletebuttonset.h f7abef7
settings/config/addeditdeletebuttonset.cpp 4f3f97a
settings/config/manageconnectionwidget.h 41a449c
settings/config/manageconnectionwidget.cpp db69cb6
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/nm-vpnc-service.h e3f859a
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/20110604/58e8b8ba/attachment-0001.htm

Rajeesh K Nambiar

2011-06-05 06:45:09 UTC

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

(Updated June 5, 2011, 6:45 a.m.)

Review request for Network Management.

Changes
-------

Updated patch with '-type' and '-flags' settings. Also added the IKE DH Group for VPNC.

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 (updated)
-----

libs/internals/settings/vpnsecrets.h 96803ec
libs/ui/vpnpreferences.cpp 843636c
libs/ui/vpnuiplugin.h 7a13027
settings/config/CMakeLists.txt 783d551
settings/config/addeditdeletebuttonset.h f7abef7
settings/config/addeditdeletebuttonset.cpp 4f3f97a
settings/config/manageconnectionwidget.h e6f1b93
settings/config/manageconnectionwidget.cpp dad1676
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/nm-vpnc-service.h e3f859a
vpnplugins/vpnc/vpnc.h aec2136
vpnplugins/vpnc/vpnc.cpp deb9108
vpnplugins/vpnc/vpncprop.ui 92093cf
vpnplugins/vpnc/vpncwidget.cpp 171d68b

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/20110605/797f0504/attachment.htm

Commit Hook

2011-06-07 00:38:56 UTC

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

This review has been submitted with commit ec6e54763c009867444a6780322d75a289055642 by Lamarque V. Souza.

- Commit

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 5, 2011, 6:45 a.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/internals/settings/vpnsecrets.h 96803ec
libs/ui/vpnpreferences.cpp 843636c
libs/ui/vpnuiplugin.h 7a13027
settings/config/CMakeLists.txt 783d551
settings/config/addeditdeletebuttonset.h f7abef7
settings/config/addeditdeletebuttonset.cpp 4f3f97a
settings/config/manageconnectionwidget.h e6f1b93
settings/config/manageconnectionwidget.cpp dad1676
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/nm-vpnc-service.h e3f859a
vpnplugins/vpnc/vpnc.h aec2136
vpnplugins/vpnc/vpnc.cpp deb9108
vpnplugins/vpnc/vpncprop.ui 92093cf
vpnplugins/vpnc/vpncwidget.cpp 171d68b
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/20110607/ac2a2907/attachment.htm

Commit Hook

2011-06-07 00:40:48 UTC

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

This review has been submitted with commit 6ce880e55d0cfdb39a089d95aaef6b4831977999 by Lamarque V. Souza.

- Commit

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
(Updated June 5, 2011, 6:45 a.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/internals/settings/vpnsecrets.h 96803ec
libs/ui/vpnpreferences.cpp 843636c
libs/ui/vpnuiplugin.h 7a13027
settings/config/CMakeLists.txt 783d551
settings/config/addeditdeletebuttonset.h f7abef7
settings/config/addeditdeletebuttonset.cpp 4f3f97a
settings/config/manageconnectionwidget.h e6f1b93
settings/config/manageconnectionwidget.cpp dad1676
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/nm-vpnc-service.h e3f859a
vpnplugins/vpnc/vpnc.h aec2136
vpnplugins/vpnc/vpnc.cpp deb9108
vpnplugins/vpnc/vpncprop.ui 92093cf
vpnplugins/vpnc/vpncwidget.cpp 171d68b
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/20110607/e08521a7/attachment-0001.htm

Gökçen Eraslan

2011-08-04 09:58:56 UTC

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
Review request for Network Management.

In ManageConnectionWidget::tabChanged method, that patch adds those lines:

connect(mConnEditUi.buttonSetVpn->importButton(),SIGNAL(clicked()),SLOT(importClicked()));
connect(mConnEditUi.buttonSetVpn->exportButton(),SIGNAL(clicked()),SLOT(exportClicked()));

so every time we switch to VPN tab, another connection is
established for clicked() signal. As a result; after a
few tab changes, several Import dialogs appear one after another
when the Import button of VPN is clicked.

I'll commit attached patch, if everyone is OK.
--
G?k?en Eraslan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: connect-import-export-buttons-once.diff
Type: text/x-patch
Size: 1311 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110804/db38e9a2/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110804/db38e9a2/attachment.sig>

Lamarque Vieira Souza

2011-08-04 12:49:50 UTC

Post by GÃ¶kÃ§en Eraslan

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/101419/
-----------------------------------------------------------
Review request for Network Management.

connect(mConnEditUi.buttonSetVpn->importButton(),SIGNAL(clicked()),SLOT(imp
ortClicked()));
connect(mConnEditUi.buttonSetVpn->exportButton(),SIGNAL(clicked()),SLOT(ex
portClicked()));
so every time we switch to VPN tab, another connection is
established for clicked() signal. As a result; after a
few tab changes, several Import dialogs appear one after another
when the Import button of VPN is clicked.
I'll commit attached patch, if everyone is OK.

Yes, it is Ok. I already applied this patch to nm09 branch but forgot to
do it for master branch.

--
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/
http://planetkde.org/pt-br
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110804/869227f2/attachment.html>

34 Replies
1 View
Permalink to this page
Disable enhanced parsing

Thread Navigation

Rajeesh K Nambiar 2011-05-22 16:30:36 UTC

Lamarque Vieira Souza 2011-05-22 23:21:08 UTC

Lamarque Vieira Souza 2011-05-22 23:22:23 UTC

Rajeesh K Nambiar 2011-06-02 19:09:39 UTC

Lamarque Vieira Souza 2011-06-02 19:38:27 UTC

Rajeesh K Nambiar 2011-06-02 20:00:12 UTC

Lamarque Vieira Souza 2011-06-05 21:25:38 UTC

Rajeesh K Nambiar 2011-06-06 08:09:30 UTC

Lamarque Vieira Souza 2011-06-07 00:30:11 UTC

Rajeesh K Nambiar 2011-06-07 05:24:50 UTC

Rajeesh K Nambiar 2011-05-23 19:27:04 UTC

Lamarque Vieira Souza 2011-05-23 20:01:07 UTC

Lamarque Vieira Souza 2011-05-23 23:28:33 UTC

Lamarque Vieira Souza 2011-05-23 23:46:57 UTC

Kevin Kofler 2011-05-23 23:36:38 UTC

Kevin Kofler 2011-05-23 23:15:42 UTC

Rajeesh K Nambiar 2011-05-24 14:09:54 UTC

Lamarque Vieira Souza 2011-05-24 17:44:31 UTC

Lamarque Vieira Souza 2011-05-24 19:03:47 UTC

Rajeesh K Nambiar 2011-05-26 05:26:02 UTC

Lamarque Vieira Souza 2011-05-26 05:49:17 UTC

Rajeesh K Nambiar 2011-05-24 18:45:02 UTC

Rajeesh K Nambiar 2011-06-02 19:09:21 UTC

Lamarque Vieira Souza 2011-06-02 19:37:42 UTC

Rajeesh K Nambiar 2011-06-02 19:56:27 UTC

Rajeesh K Nambiar 2011-06-03 18:43:02 UTC

Lamarque Vieira Souza 2011-06-03 19:13:03 UTC

Rajeesh K Nambiar 2011-06-04 09:35:50 UTC

Lamarque Vieira Souza 2011-06-04 18:19:01 UTC

Rajeesh K Nambiar 2011-06-04 18:36:50 UTC

Rajeesh K Nambiar 2011-06-05 06:45:09 UTC

Commit Hook 2011-06-07 00:38:56 UTC

Commit Hook 2011-06-07 00:40:48 UTC

Gökçen Eraslan 2011-08-04 09:58:56 UTC

Lamarque Vieira Souza 2011-08-04 12:49:50 UTC

about - legalese

Loading...