Review Request: Secrets: fix missing secrets for VPN plugins without explicit storage type

Discussion:

Andrey Borzenkov

2011-03-14 18:07:03 UTC

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

Review request for Network Management.

Summary
-------

Patch series b1810..a1115e introduced the following regression:

Same function was misused for both "secrets were loaded from
stable storage" as well as "are there any volatile secrets".
Additionally, secretsAvailable() never really checked whether
secrets were loaded at all.

The patch reverts secretsAvailable() semantic to "secrets loaded
from stable storage" and adds additionally hasVolatileSecrets()
to check whether any secret is never stored and has to be always
asked from user.

Addtitionally doAskUser() never really reset error after user
supplied secrets so connection hung. Fixed as well.

Fixes https://bugs.kde.org/show_bug.cgi?id=262555

Special thanks to Xeno for providing exceptional good test case.

This addresses bug 262555.
http://bugs.kde.org/show_bug.cgi?id=262555

Diffs
-----

libs/internals/connection.h 752ae78
libs/internals/connection.cpp 2914b39
libs/internals/setting.h ec16bf1
libs/internals/settings/vpn.h d809c79
libs/internals/settings/vpn.cpp 4186655
libs/internals/settings/vpnpersistence.cpp cffbe53
libs/ui/connectionsecretsjob.cpp 5a90b99

Diff: http://git.reviewboard.kde.org/r/100856/diff

Testing
-------

Testing was done under current GIT using real OpenVPN server

Thanks,

Andrey

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110314/52ebbeb6/attachment.htm

Lamarque Vieira Souza

2011-03-14 18:54:22 UTC

Permalink

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

libs/internals/connection.h
<http://git.reviewboard.kde.org/r/100856/#comment1639>

I am not familiar with VPN code in networkmanagement. The patch seems ok, although have not tested it myself.

- Lamarque Vieira

Post by Andrey Borzenkov
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/100856/
-----------------------------------------------------------
(Updated March 14, 2011, 6:07 p.m.)
Review request for Network Management.
Summary
-------
Same function was misused for both "secrets were loaded from
stable storage" as well as "are there any volatile secrets".
Additionally, secretsAvailable() never really checked whether
secrets were loaded at all.
The patch reverts secretsAvailable() semantic to "secrets loaded
from stable storage" and adds additionally hasVolatileSecrets()
to check whether any secret is never stored and has to be always
asked from user.
Addtitionally doAskUser() never really reset error after user
supplied secrets so connection hung. Fixed as well.
Fixes https://bugs.kde.org/show_bug.cgi?id=262555
Special thanks to Xeno for providing exceptional good test case.
This addresses bug 262555.
http://bugs.kde.org/show_bug.cgi?id=262555
Diffs
-----
libs/internals/connection.h 752ae78
libs/internals/connection.cpp 2914b39
libs/internals/setting.h ec16bf1
libs/internals/settings/vpn.h d809c79
libs/internals/settings/vpn.cpp 4186655
libs/internals/settings/vpnpersistence.cpp cffbe53
libs/ui/connectionsecretsjob.cpp 5a90b99
Diff: http://git.reviewboard.kde.org/r/100856/diff
Testing
-------
Testing was done under current GIT using real OpenVPN server
Thanks,
Andrey

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

Sebastian Kügler

2011-03-15 10:32:51 UTC

Permalink

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

Ship it!

I also can't check (lack of VPN), but I trust Andrey with this one.

- Sebastian

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110315/869db47c/attachment.htm