[gnome-system-tools] Fix interaction of password-less login with disabled accounts

[Milan Bouchet-Valat <milanbv src gnome org>]

commit 02c7200858d05d559f60e79cacc073a587871fa0
Author: Milan Bouchet-Valat <nalimilan club fr>
Date:   Mon Sep 13 11:55:33 2010 +0200

    Fix interaction of password-less login with disabled accounts
    
    A disabled account shouldn't be allowed to be in the password-less
    login group, since that creates unexpected effects of user being
    allowed to login via GDM, but not from the console.
    
    The most problematic case is when creating the user, keeping an
    empty password, and checking the password-less login checkbox:
    we report the account is no longer disabled, which is wrong, and
    create the weird situation described above.
    
    So when disabling accounts, remove them from the password-less
    login group. When changing password, don't unlock account if no
    new, non-empty password was provided, and refuse to enable password-
    less login. In the future, an error message or some GUI fix would be
    better.
    
    See https://bugs.launchpad.net/ubuntu/+source/gnome-system-tools/+bug/630430

 src/users/user-password.c |   81 +++++++++++++++++++++++++++-----------------
 src/users/user-settings.c |   14 +++++++-
 2 files changed, 63 insertions(+), 32 deletions(-)
---
diff --git a/src/users/user-password.c b/src/users/user-password.c
index 6840158..89d06d6 100644
--- a/src/users/user-password.c
+++ b/src/users/user-password.c
@@ -326,6 +326,7 @@ static void
 finish_password_change ()
 {
 	GtkWidget *user_passwd_dialog;
+	GtkWidget *passwd_entry;
 	GtkWidget *nocheck_toggle;
 	OobsUser  *user;
 	OobsGroup *no_passwd_login_group;
@@ -333,45 +334,64 @@ finish_password_change ()
 	gboolean   is_self;
 
 	user_passwd_dialog = gst_dialog_get_widget (tool->main_dialog, "user_passwd_dialog");
+	passwd_entry = gst_dialog_get_widget (tool->main_dialog, "user_settings_passwd1");
 	nocheck_toggle = gst_dialog_get_widget (tool->main_dialog, "user_passwd_no_check");
 	user = users_table_get_current ();
 	is_self = oobs_self_config_is_user_self (OOBS_SELF_CONFIG (GST_USERS_TOOL (tool)->self_config),
 	                                         user);
 
-	/* check whether user is allowed to login without password */
 	no_passwd_login_group =
 		oobs_groups_config_get_from_name (OOBS_GROUPS_CONFIG (GST_USERS_TOOL (tool)->groups_config),
 		                                  NO_PASSWD_LOGIN_GROUP);
-	no_passwd_login_changed =
-		gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (nocheck_toggle))
-		!= oobs_user_is_in_group (user, no_passwd_login_group);
-	if (no_passwd_login_changed) {
 
-		if (gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (nocheck_toggle)))
-			oobs_group_add_user (no_passwd_login_group, user);
-		else
-			oobs_group_remove_user (no_passwd_login_group, user);
-	}
-
-	/* Only commit if password-less login option has changed, or
-	 * if user is not self (changing password via the backends). */
-	if (no_passwd_login_changed || !is_self) {
-		/* commit both user and groups config
-		 * because of the no_passwd_login_group membership */
-		if (gst_tool_commit (tool, OOBS_OBJECT (user)) == OOBS_RESULT_OK) {
-			gst_tool_commit (tool, GST_USERS_TOOL (tool)->groups_config);
-
-			/* Update settings shown in the main dialog */
-			user_settings_show (user);
-		}
-	}
-
-	/* We know we've set a non-empty password */
-	oobs_user_set_password_empty (user, FALSE);
-
-	/* Unlock account, this may not be what is wanted, but that's the only solution
-	 * since 'passwd' doesn't differentiate accounts with no passwords yet from disabled ones */
-	oobs_user_set_password_disabled (user, FALSE);
+	/* FIXME: this is a hack; proper handling of this mess would involve
+	 * displaying an error message when people enable password-less login
+	 * without providing a non-empty password.
+	 *
+	 * Only unlock account if a new, non-empty password was provided.
+	 * Else, we would believe the account is enabled, while it's not,
+	 * and we would allow password-less login, which is unexpected. */
+	if (oobs_user_get_password_disabled (user)
+	    && strlen (gtk_entry_get_text (GTK_ENTRY (passwd_entry))) == 0)
+	  {
+		  /* Force removing user from this group, since results are unexpected */
+		  if (no_passwd_login_group)
+			  oobs_group_remove_user (no_passwd_login_group, user);
+	  }
+	else
+	  {
+		  /* check whether user is allowed to login without password */
+		  no_passwd_login_changed =
+			  gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (nocheck_toggle))
+			  != oobs_user_is_in_group (user, no_passwd_login_group);
+
+		  if (no_passwd_login_changed) {
+			  if (gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (nocheck_toggle)))
+				  oobs_group_add_user (no_passwd_login_group, user);
+			  else
+				  oobs_group_remove_user (no_passwd_login_group, user);
+		  }
+
+		  /* Unlock account, this may not be what is wanted, but that's the only solution
+		   * since 'passwd' doesn't differentiate accounts with no passwords yet from disabled ones */
+		  oobs_user_set_password_disabled (user, FALSE);
+
+		  /* Only commit if password-less login option has changed, or
+		   * if user is not self (changing password via the backends). */
+		  if (no_passwd_login_changed || !is_self) {
+			  /* commit both user and groups config
+			   * because of the no_passwd_login_group membership */
+			  if (gst_tool_commit (tool, OOBS_OBJECT (user)) == OOBS_RESULT_OK) {
+				  gst_tool_commit (tool, GST_USERS_TOOL (tool)->groups_config);
+
+				  /* Update settings shown in the main dialog */
+				  user_settings_show (user);
+			  }
+		  }
+
+		  /* We know we've set a non-empty password */
+		  oobs_user_set_password_empty (user, FALSE);
+	  }
 
 	user_settings_show (user);
 
@@ -379,7 +399,6 @@ finish_password_change ()
 	gtk_widget_hide (GTK_WIDGET (user_passwd_dialog));
 
 	g_object_unref (user);
-
 	if (no_passwd_login_group)
 		g_object_unref (no_passwd_login_group);
 }
diff --git a/src/users/user-settings.c b/src/users/user-settings.c
index fca6627..7c2e52a 100644
--- a/src/users/user-settings.c
+++ b/src/users/user-settings.c
@@ -1339,6 +1339,8 @@ on_edit_user_advanced (GtkButton *button, gpointer user_data)
 	GtkTreeIter iter;
 	OobsUser *user;
 	OobsGroup *main_group;
+	gboolean password_disabled;
+	OobsGroup *no_passwd_login_group;
 	int response;
 
 	TestBattery battery[] = {
@@ -1435,7 +1437,15 @@ on_edit_user_advanced (GtkButton *button, gpointer user_data)
 	oobs_user_set_uid (user, gtk_spin_button_get_value (GTK_SPIN_BUTTON (widget)));
 
 	widget = gst_dialog_get_widget (tool->main_dialog, "user_settings_locked_account");
-	oobs_user_set_password_disabled (user, gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (widget)));
+	password_disabled = gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (widget));
+	oobs_user_set_password_disabled (user, password_disabled);
+	/* Leaving user in the password-less login group would still allow him to login,
+	 * which defeats the purpose of disabling account */
+	no_passwd_login_group =
+		oobs_groups_config_get_from_name (GST_USERS_TOOL (tool)->groups_config,
+		                                  NO_PASSWD_LOGIN_GROUP);
+	if (password_disabled && no_passwd_login_group)
+		oobs_group_remove_user (no_passwd_login_group, user);
 
 	privileges_table_save (user);
 
@@ -1460,4 +1470,6 @@ on_edit_user_advanced (GtkButton *button, gpointer user_data)
 	                       on_commit_finish, user);
 
 	g_object_unref (user);
+	if (no_passwd_login_group)
+		g_object_unref (no_passwd_login_group);
 }