GNOME Bugzilla – Bug 737592
[PATCH] networkAgent: pass VPN hints to auth dialogs
Last modified: 2014-09-30 17:16:17 UTC
Created attachment 287356 [details] [review] 0001-networkAgent-pass-VPN-hints-to-auth-dialogs.patch Indicate to NetworkManager that the Shell's agent supports VPN hints, and pass those hints to VPN auth dialogs that also indicate that they support hints.
Review of attachment 287356 [details] [review]: First of all, why? Is this a new feature? What will these hints be? ::: js/ui/components/networkAgent.js @@ +606,3 @@ _init: function() { + this._native = new Shell.NetworkAgent({ identifier: 'org.gnome.Shell.NetworkAgent', + capabilities: NMClient.SecretAgentCapabilities.VPN_HINTS }); Bad indentation. Align with 'identifier', please. @@ +709,3 @@ try { externalUIMode = keyfile.get_boolean('GNOME', 'supports-external-ui-mode'); + hints = keyfile.get_boolean('GNOME', 'supports-hints'); This is wrong. If supports-external-ui-mode is missing, then we won't check for supports-hints. Or is that effectively true in reality? That there won't ever be a plugin that supports hints but doesn't support external UI mode? @@ +718,3 @@ + if (GLib.file_test(path, GLib.FileTest.IS_EXECUTABLE)) { + this._vpnBinaries[service] = { fileName: path, externalUIMode: externalUIMode, + supportsHints: hints }; Just keep it on the same line.
(In reply to comment #1) > Review of attachment 287356 [details] [review]: > > First of all, why? Is this a new feature? What will these hints be? It's a "new" feature from last year that never got GNOME Shell support for a few reasons, but those reasons have been resolved. I've been working on the Shell patch on and off for quite a while and have finally got all the testing done and other issues handled. This feature, paired with updated VPN plugins (which is one of the aforementioned issues) allows the VPN plugins to request that the UI provide new secrets. So, if you type the wrong password/token, instead of just killing the connection, the VPN will ask for a new password. If the authenticator needs the "next token code" then it can ask for that code. Hints are two things: 1) the specific secret that the VPN plugin needs, like if your user password with vpnc is wrong you only want to re-enter the user password, you don't need to re-enter the "group password". The VPN plugin itself tells NM that it wants that secret, and NM passes that back to the auth dialog via the hints. 2) a VPN specific message that will be displayed in the dialog when requesting new secrets, like "Enter next token code" or "Enter passphrase" or such. This usually comes directly from the VPN server itself (like for next token code) and has specific meaning. All these are VPN-specific and are opaque to NetworkManager and clients, becuase they are defined by the VPN itself and passed between the VPN and the auth dialogs. > ::: js/ui/components/networkAgent.js > @@ +606,3 @@ > _init: function() { > + this._native = new Shell.NetworkAgent({ identifier: > 'org.gnome.Shell.NetworkAgent', > + capabilities: NMClient.SecretAgentCapabilities.VPN_HINTS }); > > Bad indentation. Align with 'identifier', please. Fixed. > @@ +709,3 @@ > try { > externalUIMode = keyfile.get_boolean('GNOME', > 'supports-external-ui-mode'); > + hints = keyfile.get_boolean('GNOME', > 'supports-hints'); > > This is wrong. If supports-external-ui-mode is missing, then we won't check for > supports-hints. You're right, fixed. > Or is that effectively true in reality? That there won't ever be a plugin that > supports hints but doesn't support external UI mode? I'm not sure we can rely on that, unfortunately. I'd bet that openconnect would eventually support hints, but I'm not sure that it'll ever support external-ui-mode due to the amount of logic in their auth dialog. > @@ +718,3 @@ > + if (GLib.file_test(path, GLib.FileTest.IS_EXECUTABLE)) { > + this._vpnBinaries[service] = { fileName: path, > externalUIMode: externalUIMode, > + supportsHints: hints }; > > Just keep it on the same line. Done.
Created attachment 287454 [details] [review] 0001-networkAgent-pass-VPN-hints-to-auth-dialogs.patch
Review of attachment 287454 [details] [review]: *Excellent* explanation, and you already pre-empted me saying "now put it in the commit message". Thanks so much.
Created attachment 287469 [details] dialog.png Example of what an interactive secrets dialog looks like; the prompt comes directly from the VPN plugin/service itself.
Pushed to git master and gnome-3-12