Merge lp:~nataliabidart/ubuntuone-client/update-sso into lp:ubuntuone-client

[Natalia Bidart]

- Removing the deprecated SSOCredentials interface (LP: #859635).

[dobey (dobey)]

Looks reasonable to me.

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~nataliabidart/ubuntuone-client/update-sso into lp:ubuntuone-client failed. Below is the output from the failed tests.

/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
  testing autoconf2.50... not found.
  testing autoconf... found 2.67
checking for automake >= 1.10...
  testing automake-1.11... found 1.11.1
checking for libtool >= 1.5...
  testing libtoolize... found 2.2.6b
checking for intltool >= 0.30...
  testing intltoolize... found 0.41.1
checking for pkg-config >= 0.14.0...
  testing pkg-config... found 0.25
checking for gtk-doc >= 1.0...
  testing gtkdocize... found 1.17
Checking for required M4 macros...
Checking for forbidden M4 macros...
Processing ./configure.ac
Running libtoolize...
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
Running intltoolize...
Running gtkdocize...
Running aclocal-1.11...
Running autoconf...
Running autoheader...
Running automake-1.11...
Running ./configure --enable-gtk-doc --enable-debug ...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for library containing strerror... none required
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking dependency style of gcc... (cached) gcc3
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking whether the shell understands some XSI constructs... yes
checking whether the shell understands "+="... yes
checking for /usr/bin/ld option to reload object files... -r
checking for objdump... objdump
checking how to...

[dobey (dobey)]

Just changing this to needs fixing, because I know you're doing more work on it, and I know I'll need to do a more involved review. :)

review: Needs Fixing

[Natalia Bidart (nataliabidart)]

Branch is now updated and ready for re-review. Depends on latest nightlies for ussoc and u1client.

[lp:~nataliabidart/ubuntuone-client/update-sso]

1148. By Natalia Bidart on 2011-10-05

Merged trunk in.

[dobey (dobey)]

+ g_cclosure_marshal_VOID__OBJECT,

This needs to be VOID__VOID for the CredentialsNotFound callback We think. It isn't passing any arguments to the callback.

Also, adding the credentials_not_found signal to the API will break the ABI and require anything that uses libsyncdaemon to be rebuilt, so We are a little hesitant to say it should be added here. Instead, we could use the error signal with "Credentials not found." as the error message, and not add the new signal, to preserve ABI compat.

review: Needs Fixing

[lp:~nataliabidart/ubuntuone-client/update-sso]

1149. By Natalia Bidart on 2011-10-06

Removing CredentialsNotFound signal as per review comment.

[dobey (dobey)]

review: Approve

=== modified file 'libsyncdaemon/syncdaemon-authentication.c'
--- libsyncdaemon/syncdaemon-authentication.c	2011-04-18 17:45:31 +0000
+++ libsyncdaemon/syncdaemon-authentication.c	2011-10-06 13:59:16 +0000
@@ -31,7 +31,6 @@
 struct _SyncdaemonAuthenticationPrivate {
 	DBusGConnection *bus;
 	DBusGProxy *proxy;
-	DBusGProxy *newer_proxy;
 	gboolean has_credentials;
 	SyncdaemonCredentials *credentials;
 };
@@ -56,8 +55,6 @@
 			g_object_unref (G_OBJECT (auth->priv->credentials));
 		if (auth->priv->proxy != NULL)
 			g_object_unref (G_OBJECT (auth->priv->proxy));
-		if (auth->priv->newer_proxy != NULL)
-			g_object_unref (G_OBJECT (auth->priv->newer_proxy));
 		if (auth->priv->bus != NULL)
 			dbus_g_connection_unref (auth->priv->bus);
 
@@ -101,14 +98,11 @@
 }
 
 static void
-credentials_found_cb (DBusGProxy *proxy, const gchar *app_name, GHashTable *result, gpointer user_data)
+credentials_found_cb (DBusGProxy *proxy, GHashTable *result, gpointer user_data)
 {
 	SyncdaemonAuthentication *auth = SYNCDAEMON_AUTHENTICATION (user_data);
 
-	if (g_strcmp0 (app_name, SSO_APP_NAME) != 0) {
-		g_debug ("Credentials are not for '" SSO_APP_NAME "', ignoring");
-		return;
-	}
+	g_debug ("Authorization completed!");
 
 	if (auth->priv->credentials != NULL)
 		g_object_unref (G_OBJECT (auth->priv->credentials));
@@ -120,31 +114,35 @@
 }
 
 static void
-authorization_denied_cb (DBusGProxy *proxy, const gchar *app_name, gpointer user_data)
+authorization_denied_cb (DBusGProxy *proxy, gpointer user_data)
 {
 	g_debug ("Authorization denied");
-	if (g_strcmp0 (app_name, SSO_APP_NAME) != 0) {
-		g_debug ("Credentials are not for '" SSO_APP_NAME "', ignoring");
-		return;
-	}
 
 	g_signal_emit (user_data, auth_signals[AUTHORIZATION_CANCELLED_SIGNAL], 0);
 }
 
 static void
-credentials_error_cb (DBusGProxy *proxy,
-		      const gchar *app_name,
-		      const gchar *error_message,
-		      const gchar *detailed_error,
-		      gpointer user_data)
+credentials_error_cb (DBusGProxy *proxy, GHashTable *error_dict, gpointer user_data)
 {
 	g_debug ("Authorization error");
-	if (g_strcmp0 (app_name, SSO_APP_NAME) != 0) {
-		g_debug ("Credentials are not for '" SSO_APP_NAME "', ignoring");
-		return;
-	}
-
-	g_signal_emit (user_data, auth_signals[ERROR_SIGNAL], 0, error_message);
+
+	g_signal_emit (user_data, auth_signals[ERROR_SIGNAL], 0,
+	               g_hash_table_lookup (error_dict, "message"));
+}
+
+static void
+credentials_not_found_cb (DBusGProxy *proxy, gpointer user_data)
+{
+	SyncdaemonAuthentication *auth = SYNCDAEMON_AUTHENTICATION (user_data);
+
+	g_debug ("Credentials for Ubuntu One not found!");
+
+	if (auth->priv->credentials != NULL)
+		g_object_unref (G_OBJECT (auth->priv->credentials));
+	auth->priv->has_credentials = FALSE;
+
+	g_signal_emit (auth, auth_signals[ERROR_SIGNAL], 0,
+	               "Credentials not found.");
 }
 
 static void
@@ -161,19 +159,17 @@
 		g_warning ("Couldn't get session bus: %s", error->message);
 		g_error_free (error);
 	} else {
-		auth->priv->proxy = dbus_g_proxy_new_for_name (auth->priv->bus, "com.ubuntu.sso",
-							       "/credentials", "com.ubuntu.sso.ApplicationCredentials");
+		auth->priv->proxy = dbus_g_proxy_new_for_name (auth->priv->bus, "com.ubuntuone.Credentials",
+		                                               "/credentials", "com.ubuntuone.CredentialsManagement");
 		if (auth->priv->proxy != NULL) {
-			dbus_g_object_register_marshaller (_syncdaemon_marshal_VOID__STRING_POINTER,
+			dbus_g_object_register_marshaller (_syncdaemon_marshal_VOID__POINTER,
 							   G_TYPE_NONE,
-							   G_TYPE_STRING,
 							   dbus_g_type_get_map ("GHashTable",
 										G_TYPE_STRING,
 										G_TYPE_STRING),
 							   G_TYPE_INVALID);
 
 			dbus_g_proxy_add_signal (auth->priv->proxy, "CredentialsFound",
-						 G_TYPE_STRING,
 						 dbus_g_type_get_map ("GHashTable",
 								      G_TYPE_STRING,
 								      G_TYPE_STRING),
@@ -181,43 +177,25 @@
 			dbus_g_proxy_connect_signal (auth->priv->proxy, "CredentialsFound",
 						     G_CALLBACK (credentials_found_cb), auth, NULL);
 
+			dbus_g_proxy_add_signal (auth->priv->proxy, "CredentialsNotFound",
+			                         G_TYPE_INVALID);
+			dbus_g_proxy_connect_signal (auth->priv->proxy, "CredentialsNotFound",
+			                             G_CALLBACK (credentials_not_found_cb), auth, NULL);
+
 			dbus_g_proxy_add_signal (auth->priv->proxy, "AuthorizationDenied",
-						 G_TYPE_STRING,
 						 G_TYPE_INVALID);
 			dbus_g_proxy_connect_signal (auth->priv->proxy, "AuthorizationDenied",
 						     G_CALLBACK (authorization_denied_cb), auth, NULL);
 
 			dbus_g_proxy_add_signal (auth->priv->proxy, "CredentialsError",
-						 G_TYPE_STRING,
-						 G_TYPE_STRING,
-						 G_TYPE_STRING,
+						 dbus_g_type_get_map ("GHashTable",
+								      G_TYPE_STRING,
+								      G_TYPE_STRING),
 						 G_TYPE_INVALID);
 			dbus_g_proxy_connect_signal (auth->priv->proxy, "CredentialsError",
 						     G_CALLBACK (credentials_error_cb), auth, NULL);
 		} else
-			g_warning ("Couldn't get proxy for com.ubuntu.sso.ApplicationCredentials");
-
-		auth->priv->newer_proxy = dbus_g_proxy_new_for_name (auth->priv->bus, "com.ubuntu.sso",
-							       "/com/ubuntu/sso/credentials", "com.ubuntu.sso.CredentialsManagement");
-		if (auth->priv->newer_proxy != NULL) {
-			dbus_g_object_register_marshaller (_syncdaemon_marshal_VOID__STRING_POINTER,
-							   G_TYPE_NONE,
-							   G_TYPE_STRING,
-							   dbus_g_type_get_map ("GHashTable",
-										G_TYPE_STRING,
-										G_TYPE_STRING),
-							   G_TYPE_INVALID);
-
-			dbus_g_proxy_add_signal (auth->priv->newer_proxy, "CredentialsFound",
-						 G_TYPE_STRING,
-						 dbus_g_type_get_map ("GHashTable",
-								      G_TYPE_STRING,
-								      G_TYPE_STRING),
-						 G_TYPE_INVALID);
-			dbus_g_proxy_connect_signal (auth->priv->newer_proxy, "CredentialsFound",
-						     G_CALLBACK (credentials_found_cb), auth, NULL);
-		} else
-			g_warning ("Couldn't get proxy for com.ubuntu.sso.CredentialsManagement");
+			g_warning ("Couldn't get proxy for com.ubuntuone.CredentialsManagement");
 
 	}
 }
@@ -228,35 +206,35 @@
 SyncdaemonCredentials *
 syncdaemon_authentication_find_credentials (SyncdaemonAuthentication *auth)
 {
+	SyncdaemonCredentials *result = NULL;
 	g_return_val_if_fail (SYNCDAEMON_IS_AUTHENTICATION (auth), NULL);
 
 	if (!auth->priv->has_credentials) {
 		GHashTable *credentials;
 		GError *error = NULL;
 
-		if (dbus_g_proxy_call (auth->priv->proxy, "find_credentials", &error,
-				       G_TYPE_STRING, SSO_APP_NAME,
+		if (dbus_g_proxy_call (auth->priv->proxy, "find_credentials_sync", &error,
 				       G_TYPE_INVALID,
 				       dbus_g_type_get_map ("GHashTable",
-                                                            G_TYPE_STRING,
-                                                            G_TYPE_STRING), &credentials,
+				                            G_TYPE_STRING,
+				                            G_TYPE_STRING), &credentials,
 				       G_TYPE_INVALID)) {
 			if (g_hash_table_size (credentials) >= 4) {
 				auth->priv->credentials = syncdaemon_credentials_new_from_hash_table (credentials);
 				auth->priv->has_credentials = TRUE;
-			} else
-				g_warning ("Got less number of items in credentials hash table than expected!");
-
+				result = auth->priv->credentials;
+			} else {
+				g_debug ("Got less number of items (%i) in credentials hash table than expected (4)!",
+				         g_hash_table_size (credentials));
+				}
 			g_hash_table_destroy (credentials);
 		} else {
-			g_warning ("Could not get credentials for '" SSO_APP_NAME "': %s", error->message);
+			g_warning ("Could not get credentials: %s", error->message);
 			g_error_free (error);
-
-			return NULL;
 		}
 	}
 
-	return auth->priv->credentials;
+	return result;
 }
 
 /**
@@ -279,15 +257,20 @@
 void
 syncdaemon_authentication_login_or_register (SyncdaemonAuthentication *auth)
 {
+	GHashTable *params = g_hash_table_new (g_str_hash, g_str_equal);
+
 	g_return_if_fail (SYNCDAEMON_IS_AUTHENTICATION (auth));
 
-	dbus_g_proxy_call_no_reply (auth->priv->proxy, "login_or_register_to_get_credentials",
-				    G_TYPE_STRING, SSO_APP_NAME,
-				    G_TYPE_STRING, SSO_TC_URL,
-				    G_TYPE_STRING, Q_("Ubuntu One requires an Ubuntu Single Sign On (SSO) account. This process will allow you to create a new account, if you do not yet have one."),
-				    G_TYPE_INT64, 0, /* FIXME */
+	g_hash_table_insert (params, "window_id", "0"); /* FIXME: get window ID */
+
+	dbus_g_proxy_call_no_reply (auth->priv->proxy, "register",
+				    dbus_g_type_get_map ("GHashTable",
+				                         G_TYPE_STRING,
+				                         G_TYPE_STRING), params,
 				    G_TYPE_INVALID,
 				    G_TYPE_INVALID);
+
+	g_hash_table_destroy (params);
 }
 
 /**
@@ -300,11 +283,10 @@
 
 	g_return_if_fail (SYNCDAEMON_IS_AUTHENTICATION (auth));
 
-	if (!dbus_g_proxy_call (auth->priv->proxy, "clear_token", &error,
-				G_TYPE_STRING, SSO_APP_NAME,
-				G_TYPE_INVALID,
-				G_TYPE_INVALID)) {
-		g_warning ("Error calling 'clear_token': %s", error->message);
+	if (!dbus_g_proxy_call (auth->priv->proxy, "clear_credentials", &error,
+		                    G_TYPE_INVALID,
+		                    G_TYPE_INVALID)) {
+		g_warning ("Error calling 'clear_credentials': %s", error->message);
 		g_error_free (error);
 	}
 }