secrets: simplify GetSecrets call flow

The Settings.Connection interface is now only provided by NetworkManager
itself since there is only one settings service.  NM can validate
requests for secrets internally and thus there's no need to lock down
GetSecrets using a separate D-Bus interface, since PolicyKit provides
that functionality on systems where this is desirable (ie multi-user).
Single-user systems that do not have PolicyKit will inherently trust
the user already, or if not D-Bus auth is flexible enough to lock
down the GetSecrets method individually even if it's not on a separate
D-Bus interface.

Second, since only clients like connection editors or applets will be
calling the GetSecrets method, there's no need for 'hints' or
'request_new' arguments here since this  method should never trigger
an interactive secrets request.  Only NM should send those requests
when it knows it needs to ask the user, either during connection or
after validating the incoming GetSecrets request.  A connection editor
type application should never be able to trigger the normal
"What's your passphrase" dialog thats provided by the secret agent
for that user's session.

6 files changed, 62 insertions, 104 deletions

diff --git a/introspection/nm-sysconfig-connection.xml b/introspection/nm-sysconfig-connection.xml
index da8c6a6b27..2d53d65ceb 100644
--- a/introspection/nm-sysconfig-connection.xml
+++ b/introspection/nm-sysconfig-connection.xml
@@ -9,13 +9,16 @@
 
         <method name="Update">
           <tp:docstring>
-            Update the connection with new settings and properties, replacing all previous settings and properties.
+            Update the connection with new settings and properties, replacing
+            all previous settings and properties.  Secrets may be part of the
+            update request, and will be either stored in persistent storage or
+            given to a Secret Agent for storage, depending on the request.
           </tp:docstring>
           <annotation name="org.freedesktop.DBus.GLib.CSymbol" value="impl_sysconfig_connection_update"/>
           <annotation name="org.freedesktop.DBus.GLib.Async" value=""/>
           <arg name="properties" type="a{sa{sv}}" direction="in">
             <tp:docstring>
-              New connection properties.
+              New connection settings, properties, and (optionally) secrets.
             </tp:docstring>
           </arg>
         </method>
@@ -30,7 +33,10 @@
 
         <method name="GetSettings">
             <tp:docstring>
-                Get the settings maps describing this object.
+                Get the settings maps describing this network configuration.
+                This will never include any secrets required for connection
+                to the network, as those are often protected.  Secrets must
+                be requested separately using the GetSecrets() call.
             </tp:docstring>
             <annotation name="org.freedesktop.DBus.GLib.CSymbol" value="impl_sysconfig_connection_get_settings"/>
           <annotation name="org.freedesktop.DBus.GLib.Async" value=""/>
@@ -41,58 +47,47 @@
             </arg>
         </method>
 
-        <signal name="Updated">
-            <tp:docstring>
-                Emitted when any settings or permissions change.  When handling
-                this signal, clients should re-read the connection using the
-                GetSettings method to get the changes and to ensure the client
-                still has permission to access the connection.
-            </tp:docstring>
-        </signal>
-
-        <signal name="Removed">
-            <tp:docstring>
-                Emitted when this connection is no longer available.  This happens when the connection is deleted or if it is no longer accessable by any of the system's logged in users.  After receipt of this signal, the object no longer exists.
-            </tp:docstring>
-        </signal>
-
-    </interface>
-
-    <interface name="org.freedesktop.NetworkManager.Settings.Connection.Secrets">
-        <tp:docstring>
-            Secrets have a separate interface so that they can be locked down.
-        </tp:docstring>
-
         <method name="GetSecrets">
             <tp:docstring>
-                Get the secrets encapsulated in this object.
+                Get the secrets belonging to this network configuration.  Only
+                secrets from persistent storage or a Secret Agent running in
+                the requestor's session will be returned.  The user will never
+                be prompted for secrets as a result of this request.
             </tp:docstring>
             <annotation name="org.freedesktop.DBus.GLib.CSymbol" value="impl_sysconfig_connection_get_secrets"/>
             <annotation name="org.freedesktop.DBus.GLib.Async" value=""/>
             <arg name="setting_name" type="s" direction="in">
                 <tp:docstring>
-                    Name of the setting to return.
-                </tp:docstring>
-            </arg>
-            <arg name="hints" type="as" direction="in">
-                <tp:docstring>
-                    Array of strings of key names in the Setting for which NM thinks
-                    a secrets may be required.
-                </tp:docstring>
-            </arg>
-            <arg name="request_new" type="b" direction="in">
-                <tp:docstring>
-                    Indicates whether new secrets should be requested or if the request can be fulfilled from storage.
+                    Name of the setting to return secrets for.  If empty, all
+                    all secrets will be returned.
                 </tp:docstring>
             </arg>
 
             <arg name="secrets" type="a{sa{sv}}" direction="out" tp:type="String_String_Variant_Map_Map">
                 <tp:docstring>
-                    Nested settings maps containing secrets.  Each setting MUST contain at least the 'name' field, containing the name of the setting, and one or more secrets.
+                    Nested settings maps containing secrets.
                 </tp:docstring>
             </arg>
         </method>
 
+        <signal name="Updated">
+            <tp:docstring>
+                Emitted when any settings or permissions change.  When handling
+                this signal, clients should re-read the connection using the
+                GetSettings method to get the changes and to ensure the client
+                still has permission to access the connection.
+            </tp:docstring>
+        </signal>
+
+        <signal name="Removed">
+            <tp:docstring>
+                Emitted when this connection is no longer available.  This
+                happens when the connection is deleted or if it is no longer
+                accessable by any of the system's logged-in users.  After
+                receipt of this signal, the object no longer exists.
+            </tp:docstring>
+        </signal>
+
     </interface>
 
 </node>
diff --git a/libnm-glib/nm-remote-connection.c b/libnm-glib/nm-remote-connection.c
index f11d10021e..8b7e575969 100644
--- a/libnm-glib/nm-remote-connection.c
+++ b/libnm-glib/nm-remote-connection.c
@@ -18,7 +18,7 @@
  * Boston, MA 02110-1301 USA.
  *
  * Copyright (C) 2007 - 2008 Novell, Inc.
- * Copyright (C) 2007 - 2010 Red Hat, Inc.
+ * Copyright (C) 2007 - 2011 Red Hat, Inc.
  */
 
 #include <string.h>
@@ -54,7 +54,6 @@ static guint signals[LAST_SIGNAL] = { 0 };
 
 typedef struct {
 	NMRemoteConnection *self;
-	DBusGProxy *proxy;
 	DBusGProxyCall *call;
 	GFunc callback;
 	gpointer user_data;
@@ -63,7 +62,6 @@ typedef struct {
 typedef struct {
 	DBusGConnection *bus;
 	DBusGProxy *proxy;
-	DBusGProxy *secrets_proxy;
 	GSList *calls;
 
 	NMRemoteConnectionInitResult init_result;
@@ -126,7 +124,6 @@ nm_remote_connection_commit_changes (NMRemoteConnection *self,
 	call->self = self;
 	call->callback = (GFunc) callback;
 	call->user_data = user_data;
-	call->proxy = priv->proxy;
 
 	settings = nm_connection_to_hash (NM_CONNECTION (self));
 
@@ -176,7 +173,6 @@ nm_remote_connection_delete (NMRemoteConnection *self,
 	call->self = self;
 	call->callback = (GFunc) callback;
 	call->user_data = user_data;
-	call->proxy = priv->proxy;
 
 	call->call = org_freedesktop_NetworkManager_Settings_Connection_delete_async (priv->proxy,
 	                                                                              delete_cb,
@@ -199,9 +195,6 @@ get_secrets_cb (DBusGProxy *proxy, GHashTable *secrets, GError *error, gpointer
  * nm_remote_connection_get_secrets:
  * @connection: the #NMRemoteConnection
  * @setting_name: the #NMSetting object name to get secrets for
- * @hints: #NMSetting key names to get secrets for (optional)
- * @request_new: hint that new secrets (instead of cached or stored secrets) 
- *  should be returned
  * @callback: (scope async): a function to be called when the update completes
  * @user_data: caller-specific data to be passed to @callback
  *
@@ -210,8 +203,6 @@ get_secrets_cb (DBusGProxy *proxy, GHashTable *secrets, GError *error, gpointer
 void
 nm_remote_connection_get_secrets (NMRemoteConnection *self,
                                   const char *setting_name,
-                                  const char **hints,
-                                  gboolean request_new,
                                   NMRemoteConnectionGetSecretsFunc callback,
                                   gpointer user_data)
 {
@@ -228,14 +219,11 @@ nm_remote_connection_get_secrets (NMRemoteConnection *self,
 	call->self = self;
 	call->callback = (GFunc) callback;
 	call->user_data = user_data;
-	call->proxy = priv->secrets_proxy;
-
-	call->call = org_freedesktop_NetworkManager_Settings_Connection_Secrets_get_secrets_async (priv->secrets_proxy,
-	                                                                                           setting_name,
-	                                                                                           hints,
-	                                                                                           request_new,
-	                                                                                           get_secrets_cb,
-	                                                                                           call);
+
+	call->call = org_freedesktop_NetworkManager_Settings_Connection_get_secrets_async (priv->proxy,
+	                                                                                   setting_name,
+	                                                                                   get_secrets_cb,
+	                                                                                   call);
 	g_assert (call->call);
 	priv->calls = g_slist_append (priv->calls, call);
 }
@@ -366,13 +354,6 @@ constructor (GType type,
 	g_assert (priv->proxy);
 	dbus_g_proxy_set_default_timeout (priv->proxy, G_MAXINT);
 
-	priv->secrets_proxy = dbus_g_proxy_new_for_name (priv->bus,
-	                                                 NM_DBUS_SERVICE,
-	                                                 nm_connection_get_path (NM_CONNECTION (object)),
-	                                                 NM_DBUS_IFACE_SETTINGS_CONNECTION_SECRETS);
-	g_assert (priv->secrets_proxy);
-	dbus_g_proxy_set_default_timeout (priv->secrets_proxy, G_MAXINT);
-
 	dbus_g_proxy_add_signal (priv->proxy, "Updated", G_TYPE_INVALID);
 	dbus_g_proxy_connect_signal (priv->proxy, "Updated", G_CALLBACK (updated_cb), object, NULL);
 
@@ -437,7 +418,6 @@ dispose (GObject *object)
 			remote_call_complete (self, priv->calls->data);
 
 		g_object_unref (priv->proxy);
-		g_object_unref (priv->secrets_proxy);
 		dbus_g_connection_unref (priv->bus);
 	}
 
diff --git a/libnm-glib/nm-remote-connection.h b/libnm-glib/nm-remote-connection.h
index 0c627a6044..809927ea4c 100644
--- a/libnm-glib/nm-remote-connection.h
+++ b/libnm-glib/nm-remote-connection.h
@@ -18,7 +18,7 @@
  * Boston, MA 02110-1301 USA.
  *
  * Copyright (C) 2007 - 2008 Novell, Inc.
- * Copyright (C) 2007 - 2009 Red Hat, Inc.
+ * Copyright (C) 2007 - 2011 Red Hat, Inc.
  */
 
 #ifndef __NM_REMOTE_CONNECTION_H__
@@ -38,8 +38,8 @@ G_BEGIN_DECLS
 #define NM_IS_REMOTE_CONNECTION_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((obj), NM_TYPE_REMOTE_CONNECTION))
 #define NM_REMOTE_CONNECTION_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_REMOTE_CONNECTION, NMRemoteConnectionClass))
 
-#define NM_REMOTE_CONNECTION_UPDATED               "updated"
-#define NM_REMOTE_CONNECTION_REMOVED               "removed"
+#define NM_REMOTE_CONNECTION_UPDATED         "updated"
+#define NM_REMOTE_CONNECTION_REMOVED         "removed"
 
 typedef struct {
 	NMConnection parent;
@@ -91,8 +91,6 @@ void nm_remote_connection_delete (NMRemoteConnection *connection,
 
 void nm_remote_connection_get_secrets (NMRemoteConnection *connection,
                                        const char *setting_name,
-                                       const char **hints,
-                                       gboolean request_new,
                                        NMRemoteConnectionGetSecretsFunc callback,
                                        gpointer user_data);
 G_END_DECLS
diff --git a/src/nm-agent-manager.c b/src/nm-agent-manager.c
index 2e88ee201c..46c6144399 100644
--- a/src/nm-agent-manager.c
+++ b/src/nm-agent-manager.c
@@ -570,10 +570,9 @@ request_start_secrets (gpointer user_data)
 	nm_log_dbg (LOGD_AGENTS, "(%p/%s) getting secrets from system settings",
 			    req, req->setting_name);
 
+	/* Grab any secrets from persistent storage */
 	secrets = nm_sysconfig_connection_get_secrets (NM_SYSCONFIG_CONNECTION (req->connection),
 	                                               req->setting_name,
-	                                               req->hint,
-	                                               req->flags ? TRUE : FALSE,
 	                                               &error);
 	if (secrets)
 		setting_secrets = g_hash_table_lookup (secrets, req->setting_name);
diff --git a/src/settings/nm-sysconfig-connection.c b/src/settings/nm-sysconfig-connection.c
index 3a280b9437..40823562a1 100644
--- a/src/settings/nm-sysconfig-connection.c
+++ b/src/settings/nm-sysconfig-connection.c
@@ -16,7 +16,7 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
  * (C) Copyright 2008 Novell, Inc.
- * (C) Copyright 2008 - 2010 Red Hat, Inc.
+ * (C) Copyright 2008 - 2011 Red Hat, Inc.
  */
 
 #include <string.h>
@@ -47,8 +47,6 @@ static void impl_sysconfig_connection_delete (NMSysconfigConnection *connection,
 
 static void impl_sysconfig_connection_get_secrets (NMSysconfigConnection *connection,
                                                    const gchar *setting_name,
-                                                   const gchar **hints,
-                                                   gboolean request_new,
                                                    DBusGMethodInvocation *context);
 
 #include "nm-sysconfig-connection-glue.h"
@@ -410,11 +408,18 @@ destroy_gvalue (gpointer data)
 	g_slice_free (GValue, value);
 }
 
+/**
+ * nm_sysconfig_connection_get_secrets:
+ * @connection: the #NMSysconfigConnection
+ * @setting_name: the setting to return secrets for
+ * @error: an error on return, if an error occured
+ *
+ * Return secrets in persistent storage, if any.  Does not query any Secret
+ * Agents for secrets.
+ **/
 GHashTable *
 nm_sysconfig_connection_get_secrets (NMSysconfigConnection *connection,
                                      const char *setting_name,
-                                     const char *hint,
-                                     gboolean request_new,
                                      GError **error)
 {
 	NMSysconfigConnectionPrivate *priv = NM_SYSCONFIG_CONNECTION_GET_PRIVATE (connection);
@@ -434,6 +439,8 @@ nm_sysconfig_connection_get_secrets (NMSysconfigConnection *connection,
 		return NULL;
 	}
 
+	/* FIXME: if setting_name is empty, return all secrets */
+
 	setting = nm_connection_get_setting_by_name (priv->secrets, setting_name);
 	if (!setting) {
 		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_SETTING,
@@ -739,19 +746,13 @@ impl_sysconfig_connection_delete (NMSysconfigConnection *self,
 	auth_start (self, context, TRUE, delete_auth_cb, NULL);
 }
 
-typedef struct {
-	char *setting_name;
-	char *hint;
-	gboolean request_new;
-} GetSecretsInfo;
-
 static void
 secrets_auth_cb (NMSysconfigConnection *self, 
 	             DBusGMethodInvocation *context,
 	             GError *error,
-	             gpointer data)
+	             gpointer user_data)
 {
-	GetSecretsInfo *info = data;
+	char *setting_name = user_data;
 	GHashTable *secrets;
 	GError *local = NULL;
 
@@ -760,11 +761,7 @@ secrets_auth_cb (NMSysconfigConnection *self,
 		goto out;
 	}
 
-	secrets = nm_sysconfig_connection_get_secrets (self,
-	                                               info->setting_name,
-	                                               info->hint,
-	                                               info->request_new,
-	                                               &error);
+	secrets = nm_sysconfig_connection_get_secrets (self, setting_name, &error);
 	if (secrets) {
 		dbus_g_method_return (context, secrets);
 		g_hash_table_destroy (secrets);
@@ -774,24 +771,15 @@ secrets_auth_cb (NMSysconfigConnection *self,
 	}
 
 out:
-	g_free (info->setting_name);
-	g_free (info->hint);
-	g_slice_free (GetSecretsInfo, info);
+	g_free (setting_name);
 }
 
 static void
 impl_sysconfig_connection_get_secrets (NMSysconfigConnection *self,
                                        const gchar *setting_name,
-                                       const gchar **hints,
-                                       gboolean request_new,
                                        DBusGMethodInvocation *context)
 {
-	GetSecretsInfo *info = g_slice_new (GetSecretsInfo);
-	info->setting_name = g_strdup (setting_name);
-	info->hint = (hints && hints[0]) ? g_strdup (hints[0]) : NULL;
-	info->request_new = request_new;
-
-	auth_start (self, context, TRUE, secrets_auth_cb, info);
+	auth_start (self, context, TRUE, secrets_auth_cb, g_strdup (setting_name));
 }
 
 /**************************************************************/
diff --git a/src/settings/nm-sysconfig-connection.h b/src/settings/nm-sysconfig-connection.h
index aaf926c1be..84196a366c 100644
--- a/src/settings/nm-sysconfig-connection.h
+++ b/src/settings/nm-sysconfig-connection.h
@@ -16,7 +16,7 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
  * (C) Copyright 2008 Novell, Inc.
- * (C) Copyright 2008 - 2010 Red Hat, Inc.
+ * (C) Copyright 2008 - 2011 Red Hat, Inc.
  */
 
 #ifndef NM_SYSCONFIG_CONNECTION_H
@@ -90,8 +90,6 @@ void nm_sysconfig_connection_delete (NMSysconfigConnection *connection,
 
 GHashTable *nm_sysconfig_connection_get_secrets (NMSysconfigConnection *connection,
                                                  const char *setting_name,
-                                                 const char *hint,
-                                                 gboolean request_new,
                                                  GError **error);
 
 gboolean nm_sysconfig_connection_is_visible (NMSysconfigConnection *self);