D20930: Update WireGuard to match NetworkManager 1.16 interface

This revision was automatically updated to reflect the committed changes. Closed by commit R116:751f4da01b42: Update WireGuard to match NetworkManager 1.16 interface (authored by jgrulich). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D20930?vs=57983=58052#toc REPOSITORY R116 Plasma

D20930: Update WireGuard to match NetworkManager 1.16 interface

jgrulich accepted this revision. jgrulich added a comment. This revision is now accepted and ready to land. Good work!! REPOSITORY R116 Plasma Network Management Applet BRANCH TwoTabs REVISION DETAIL https://phabricator.kde.org/D20930 To: andersonbruce, jgrulich, #vdg, ngraham Cc:

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce added a comment. The changes to the key passwordfield menus was made to remove the AlwaysAsk option. This is the last change I am aware of necessary to release this. REPOSITORY R116 Plasma Network Management Applet REVISION DETAIL https://phabricator.kde.org/D20930 To:

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce updated this revision to Diff 57983. andersonbruce added a comment. - Disable "AlwaysAsk" option in key password fields Caution: will not compile without changes to passwordfield.cpp and passwordfield.h made in the current master branch. REPOSITORY R116 Plasma

D20930: Update WireGuard to match NetworkManager 1.16 interface

jgrulich added a comment. I think it doesn't matter in this case, you can just change it in your code, you don't need to have that change in your local copy, if this is merged than it's applied on top of that so it will be ok. REPOSITORY R116 Plasma Network Management Applet REVISION

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce marked an inline comment as done. andersonbruce added a comment. In D20930#464412 , @jgrulich wrote: > In D20930#464403 , @andersonbruce wrote: > > > In D20930#464387

D20930: Update WireGuard to match NetworkManager 1.16 interface

jgrulich added a comment. In D20930#464403 , @andersonbruce wrote: > In D20930#464387 , @jgrulich wrote: > > > In D20930#464384 , @jgrulich wrote: > >

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce marked an inline comment as done. andersonbruce added a comment. In D20930#464387 , @jgrulich wrote: > In D20930#464384 , @jgrulich wrote: > > > In D20930#464377

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce updated this revision to Diff 57972. andersonbruce added a comment. - Return non-WireGuard logic to original state REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D20930?vs=57970=57972 BRANCH TwoTabs REVISION DETAIL

D20930: Update WireGuard to match NetworkManager 1.16 interface

jgrulich added a comment. In D20930#464384 , @jgrulich wrote: > In D20930#464377 , @andersonbruce wrote: > > > Updated SecretAgent class to always try to get the secrets from kwallet even if the

D20930: Update WireGuard to match NetworkManager 1.16 interface

jgrulich added inline comments. INLINE COMMENTS > secretagent.cpp:432 > +return true; > +} else if (requestNew || (allowInteraction && > !setting->needSecrets(requestNew).isEmpty()) || (allowInteraction && > userRequested) || ((isVpn || isWireGuard) && allowInteraction)) { > +

D20930: Update WireGuard to match NetworkManager 1.16 interface

jgrulich added a comment. In D20930#464377 , @andersonbruce wrote: > Updated SecretAgent class to always try to get the secrets from kwallet even if the 'allow-interaction' flag is set. The keys WireGuard uses are 43 random characters long and

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce added a comment. Updated SecretAgent class to always try to get the secrets from kwallet even if the 'allow-interaction' flag is set. The keys WireGuard uses are 43 random characters long and we don't expect the user to enter these manually when trying to make a connection. If

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce updated this revision to Diff 57970. andersonbruce added a comment. - Don't allow save of configuration with "AlwaysAsk" flag for keys REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D20930?vs=57969=57970 BRANCH

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce updated this revision to Diff 57969. andersonbruce marked an inline comment as done. andersonbruce added a comment. - Always get encrypted data from kwallet Don't ask user for key values. REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE

D20930: Update WireGuard to match NetworkManager 1.16 interface

jgrulich added inline comments. INLINE COMMENTS > andersonbruce wrote in connectionicon.cpp:403 > There is still a discrepancy between the enum defined by NetworkManager and > the one defined by NMQT. NetworkManager is returning a value of 29 but > NetworkManager::Device::WireGuard has a

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce marked 8 inline comments as done. andersonbruce added a comment. Update to fix some comments. INLINE COMMENTS > jgrulich wrote in connectionicon.cpp:403 > You can now use NetworkManager::Device::WireGuard. There is still a discrepancy between the enum defined by NetworkManager

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce updated this revision to Diff 57888. andersonbruce added a comment. - Correct review comments REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D20930?vs=57790=57888 BRANCH TwoTabs REVISION DETAIL

D20930: Update WireGuard to match NetworkManager 1.16 interface

jgrulich added inline comments. INLINE COMMENTS > kcm.cpp:527 > + > +if (connection.isEmpty()) { // the "positive" part will arrive > with connectionAdded > +// TODO display success if (!connection.isEmpty()) { return; } > connectionicon.cpp:403 >

D20930: Update WireGuard to match NetworkManager 1.16 interface

ngraham accepted this revision. ngraham added a comment. Thanks for the info. I appreciate that you've taken so much time to think this through, and thanks for helping me understand how this works. LGTM from a #VDG perspective! REPOSITORY R116 Plasma

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce updated this revision to Diff 57790. andersonbruce added a comment. - Update labels on Add and Remove peer buttons REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D20930?vs=57741=57790 BRANCH TwoTabs REVISION DETAIL

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce added a comment. In D20930#462775 , @ngraham wrote: > In D20930#462774 , @andersonbruce wrote: > > > Personally I like the Add and Remove buttons to be together where they were, and

D20930: Update WireGuard to match NetworkManager 1.16 interface

ngraham added a comment. In D20930#462774 , @andersonbruce wrote: > Personally I like the Add and Remove buttons to be together where they were, and to simply change the wording to "Add New Peer" and "Remove This Peer". I think this still

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce added a comment. In D20930#462547 , @ngraham wrote: > So much better! > > I have one more little suggestion for the buttons: move the "Remove Peer" button into the bottom of the tab for that peer to make it clear what it

D20930: Update WireGuard to match NetworkManager 1.16 interface

ngraham added a comment. So much better! I have one more little suggestion for the buttons: move the "Remove Peer" button into the bottom of the tab for that peer to make it clear what it applies to, and change the text to say, "Remove this Peer". The Remove button can stay where it

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce added a comment. In D20930#462365 , @ngraham wrote: > I'm afraid I don't have the ability to test this; could you post a new screenshot? Thanks! Here's a screenshot of the dialog with three peers:F6814829: Tabs.png

D20930: Update WireGuard to match NetworkManager 1.16 interface

ngraham added a comment. I'm afraid I don't have the ability to test this; could you post a new screenshot? Thanks! REPOSITORY R116 Plasma Network Management Applet REVISION DETAIL https://phabricator.kde.org/D20930 To: andersonbruce, jgrulich, #vdg Cc: ngraham, plasma-devel, jraleigh,

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce added a comment. A fairly large update was made to change the input of peer data to a use a QTabWidget interface to switch between peers rather than a spin box. REPOSITORY R116 Plasma Network Management Applet REVISION DETAIL https://phabricator.kde.org/D20930 To:

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce updated this revision to Diff 57741. andersonbruce added a comment. Change to tab bar interface for peers REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D20930?vs=57434=57741 BRANCH TwoTabs REVISION DETAIL

D20930: Update WireGuard to match NetworkManager 1.16 interface

ngraham added a comment. Thanks! Seems like a tab bar would be the ideal UI here, with each peer having its own tab. That communicates the idea that the settings within the tab are specific to that tab/peer. Then the add and remove button should go below the tab bar's frame and say

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce added a comment. In D20930#459505 , @ngraham wrote: > When #VDG assistance is requested, screenshots of the current/proposed UI are appreciated. :) F6804483: PeersDialog.png

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce marked 12 inline comments as done. andersonbruce added a comment. Requested changes are made. INLINE COMMENTS > jgrulich wrote in connectionicon.cpp:403 > Do you want me to add this to NMQT? Yes, please. This is part of the device type enum we discussed earlier where not all

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce updated this revision to Diff 57434. andersonbruce marked an inline comment as done. andersonbruce added a comment. - Correct review comment REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D20930?vs=57433=57434 BRANCH

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce updated this revision to Diff 57433. andersonbruce added a comment. - Correct CamelCase of Wireguard to WireGuard - Disable adding WireGuard for NetworkManager < 1.16 - Correct review comments REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE

D20930: Update WireGuard to match NetworkManager 1.16 interface

ngraham added a reviewer: VDG. ngraham added a comment. When #VDG assistance is requested, screenshots of the current/proposed UI are appreciated. :) REPOSITORY R116 Plasma Network Management Applet REVISION DETAIL

D20930: Update WireGuard to match NetworkManager 1.16 interface

jgrulich added a subscriber: ngraham. jgrulich added a comment. Can we maybe have some assistence from anyone from VDG? @ngraham maybe? INLINE COMMENTS > CMakeLists.txt:11 > > +set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -g") > + Please remove. > kcm.cpp:264 >

D20930: Update WireGuard to match NetworkManager 1.16 interface

andersonbruce created this revision. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. Herald added a reviewer: jgrulich. andersonbruce requested review of this revision. REVISION SUMMARY In NetworkManager 1.16 handling of WireGuard interfaces was changed from a VPN