[gnome-system-tools] Allow new users to explicitly choose an existing main group

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

commit 1831e9a89c716af44951b9b8c896844e7bae3e0c
Author: Milan Bouchet-Valat <nalimilan club fr>
Date:   Thu Aug 27 14:28:45 2009 +0200

    Allow new users to explicitly choose an existing main group
    
    The previous checks were so restrictive that you could not choose an existing group as main group for new users. Two issues have led to that: the old behavior was sheerly overwriting groups, raising to security problems; users were not added to their main group if it existed. Now, we consider that automatically adding an user to an existing group because it's named as its login is dangerous, so we avoid that. But we accept using any existing group if it has been manually chosen in the list. We also add users to their main group if needed. After the freeze, error messages should be improved and a dialog could ask before using system groups as main groups for a new user.
    
    Also add a new group_settings_get_group_from_name() function, which is almost identical to what group_settings_group_exists() did before, and use the former from the latter.

 src/users/group-settings.c |   29 +++++++++++++-----
 src/users/group-settings.h |    1 +
 src/users/user-settings.c  |   69 +++++++++++++++++++++++---------------------
 3 files changed, 58 insertions(+), 41 deletions(-)
---
diff --git a/src/users/group-settings.c b/src/users/group-settings.c
index b042a24..e3f3af4 100644
--- a/src/users/group-settings.c
+++ b/src/users/group-settings.c
@@ -212,13 +212,14 @@ is_group_root (OobsGroup *group)
 	return (strcmp (name, "root") == 0);
 }
 
-gboolean
-group_settings_group_exists (const gchar *name)
+/* Get the OobsGroup corresponding to a name, or NULL if it does not exist */
+OobsGroup *
+group_settings_get_group_from_name (const gchar *name)
 {
 	OobsGroupsConfig *config;
 	OobsList *groups_list;
 	OobsListIter iter;
-	GObject *group;
+	OobsGroup *group;
 	gboolean valid;
 	const gchar *group_name;
 
@@ -227,17 +228,29 @@ group_settings_group_exists (const gchar *name)
 	valid = oobs_list_get_iter_first (groups_list, &iter);
 
 	while (valid) {
-		group = oobs_list_get (groups_list, &iter);
-		group_name = oobs_group_get_name (OOBS_GROUP (group));
-		g_object_unref (group);
+		group = OOBS_GROUP (oobs_list_get (groups_list, &iter));
+		group_name = oobs_group_get_name (group);
 
 		if (group_name && strcmp (name, group_name) == 0)
-			return TRUE;
+			return group;
 
 		valid = oobs_list_iter_next (groups_list, &iter);
 	}
 
-	return FALSE;
+	return NULL;
+}
+
+gboolean
+group_settings_group_exists (const gchar *name)
+{
+	OobsGroup *group = group_settings_get_group_from_name (name);
+
+	if (group) {
+		g_object_unref (group);
+		return TRUE;
+	}
+	else
+		return FALSE;
 }
 
 /* FIXME: this function is duplicated in user-settings.c */
diff --git a/src/users/group-settings.h b/src/users/group-settings.h
index 6c7ad7b..77c8169 100644
--- a/src/users/group-settings.h
+++ b/src/users/group-settings.h
@@ -37,6 +37,7 @@ gint         group_settings_dialog_run          (GtkWidget    *dialog,
 
 gid_t        group_settings_find_new_gid        (void);
 gboolean     group_settings_group_exists	(const gchar *name);
+OobsGroup *  group_settings_get_group_from_name (const gchar *name);
 void         group_settings_dialog_get_data     (OobsGroup    *group);
 OobsGroup *  group_settings_dialog_get_group    (void);
 
diff --git a/src/users/user-settings.c b/src/users/user-settings.c
index eeaa9f6..d9d7f31 100644
--- a/src/users/user-settings.c
+++ b/src/users/user-settings.c
@@ -207,31 +207,38 @@ get_main_group (const gchar *name)
 	model = gtk_combo_box_get_model (GTK_COMBO_BOX (combo));
 
 	if (!gtk_combo_box_get_active_iter (GTK_COMBO_BOX (combo), &iter)) {
-		/* create new group for the user */
-		OobsGroupsConfig *config;
-		OobsList *groups_list;
-		OobsListIter list_iter;
+		group =	group_settings_get_group_from_name (name);
+		/* Most standard case when creating user:
+		 * no group using the login already exists, so create it */
+		if (group == NULL) {
+			OobsGroupsConfig *config;
+			OobsList *groups_list;
+			OobsListIter list_iter;
+
+			group = oobs_group_new (name);
+			oobs_group_set_gid (group, group_settings_find_new_gid ());
+
+			/* FIXME: this should be in a generic function */
+			config = OOBS_GROUPS_CONFIG (GST_USERS_TOOL (tool)->groups_config);
+			groups_list = oobs_groups_config_get_groups (config);
+			oobs_list_append (groups_list, &list_iter);
+			oobs_list_set (groups_list, &list_iter, group);
+
+			groups_table_add_group (group, &list_iter);
+			gst_tool_commit (tool, OOBS_OBJECT (config));
 
-		group = oobs_group_new (name);
-		oobs_group_set_gid (group, group_settings_find_new_gid ());
-
-		/* FIXME: this should be in a generic function */
-		config = OOBS_GROUPS_CONFIG (GST_USERS_TOOL (tool)->groups_config);
-		groups_list = oobs_groups_config_get_groups (config);
-		oobs_list_append (groups_list, &list_iter);
-		oobs_list_set (groups_list, &list_iter, group);
-
-		groups_table_add_group (group, &list_iter);
-		gst_tool_commit (tool, OOBS_OBJECT (config));
+			return group;
+		}
+		else /* Group exists, use it */
+			return group;
+	}
+	else { /* Group has been chosen in the list, use it */
+		gtk_tree_model_get (model, &iter,
+				    COL_GROUP_OBJECT, &group,
+				    -1);
 
 		return group;
 	}
-
-	gtk_tree_model_get (model, &iter,
-			    COL_GROUP_OBJECT, &group,
-			    -1);
-
-	return group;
 }
 
 /* Retrieve the NO_PASSWD_LOGIN_GROUP.
@@ -551,6 +558,7 @@ check_login (gchar **primary_text, gchar **secondary_text, gpointer data)
 {
 	OobsUser *user;
 	GtkWidget *widget;
+	GtkWidget *combo;
 	GtkTreeModel *model;
 	GtkTreeIter iter;
 	const gchar *login;
@@ -561,14 +569,7 @@ check_login (gchar **primary_text, gchar **secondary_text, gpointer data)
 	widget = gst_dialog_get_widget (tool->main_dialog, "user_settings_name");
 	login = gtk_entry_get_text (GTK_ENTRY (widget));
 
-	widget = gst_dialog_get_widget (tool->main_dialog, "user_settings_group");
-	if (gtk_combo_box_get_active_iter (GTK_COMBO_BOX (widget), &iter)) {
-		model = gtk_combo_box_get_model (GTK_COMBO_BOX (widget));
-		gtk_tree_model_get (model, &iter, COL_GROUP_NAME, &group, -1);
-		g_object_unref (model);
-	}
-	else /* If no main group is selected, login will be used to create it, so check that it's possible */
-		group = g_strdup (login);
+	combo = gst_dialog_get_widget (tool->main_dialog, "user_settings_group");
 
 	if (strlen (login) < 1) {
 		*primary_text = g_strdup (_("User name is empty"));
@@ -581,12 +582,13 @@ check_login (gchar **primary_text, gchar **secondary_text, gpointer data)
 	} else if (!user && login_exists (login)) {
 		*primary_text = g_strdup_printf (_("User name \"%s\" already exists"), login);
 		*secondary_text = g_strdup (_("Please choose a different user name."));
-	} else if (!user && group_settings_group_exists (group)) {
-		*primary_text = g_strdup_printf (_("Group \"%s\" already exists"), group);
+	/* Check that the new user won't be added to an already existing group
+	 * without that group being explicitly chosen in the list (temporary security check) */
+	} else if (!user && gtk_combo_box_get_active (GTK_COMBO_BOX (combo)) == -1
+	                 && group_settings_group_exists (login)) {
+		*primary_text = g_strdup_printf (_("Group \"%s\" already exists"), login);
 		*secondary_text = g_strdup (_("Please choose a different user name."));
 	}
-
-	g_free (group);
 }
 
 static void
@@ -812,6 +814,7 @@ user_settings_dialog_get_data (GtkWidget *dialog)
 
 	/* set main group */
 	group = get_main_group (oobs_user_get_login_name (user));
+	oobs_group_add_user (group, user);
 	oobs_user_set_main_group (user, group);
 	g_object_unref (group);