andersonbruce marked an inline comment as done.
andersonbruce added a comment.


  
  
  In D20930#464387 <https://phabricator.kde.org/D20930#464387>, @jgrulich wrote:
  
  > In D20930#464384 <https://phabricator.kde.org/D20930#464384>, @jgrulich 
wrote:
  >
  > > In D20930#464377 <https://phabricator.kde.org/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 we don't expect the user to enter these manually 
when trying to make a connection. If data is not available in kwallet then 
trying to make a connection will fail. Also updated the configuration screens 
to not allow a configuration with "AlwaysAsk" flags on either key.
  > >
  > >
  > > You can disable "AlwaysAsk" option with 
PasswordField::setPasswordNotRequiredEnabled(false).
  >
  >
  > Sorry, that's not it, I added a new option to do that, you can now use 
PasswordField::setPasswordNotSavedEnabled(false).
  
  
  Great!  Should I make the change in this review? If so, what is the correct 
procedure (with git, arc, ...) to merge that change into this review?

INLINE COMMENTS

> jgrulich wrote in secretagent.cpp:432
> Shouldn't it be
> 
>   (isVpn && !isWireguard) && allowInteraction
> 
> You don't want to display password dialog in case it's a WG connection.
> 
> Or maybe it should stay and instead we should return true and send error 
> (like we do when a dialog has an error) if it's a WG connection, to let 
> NetworkManager know we cannot handle that.

Actually what I did is move the WireGuard handling above this 'else if' so it 
shouldn't get to this point if it is WireGuard so I simply returned the logic 
to what it was originally.

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D20930

To: andersonbruce, jgrulich, #vdg, ngraham
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

Reply via email to