[liboobs] Don't remove unknown users from groups
- From: Milan Bouchet-Valat <milanbv src gnome org>
- To: svn-commits-list gnome org
- Cc:
- Subject: [liboobs] Don't remove unknown users from groups
- Date: Sat, 6 Feb 2010 11:47:46 +0000 (UTC)
commit b581b86d1cef4eb7ae9ffef9f94383677b231c34
Author: Milan Bouchet-Valat <nalimilan club fr>
Date: Thu Feb 4 11:51:47 2010 +0100
Don't remove unknown users from groups
Users not listed in /etc/passwd can still be valid ones (e.g. LDAP). The backends send us a raw list of usernames, we now keep it as a basis for commits, and only add/remove to it known users. OobsGroups now keep two users list: one with usernames, and one with references to OobsUsers, which is used for the public API. Unknown users are kept in the first list and get eventually committed without being exposed anywhere.
This requires us to remove manually users from all groups when removing them, else we would commit their names to /etc/group depite them not existing anymore. This is done in oobs_users_config_delete_user().
https://bugzilla.gnome.org/show_bug.cgi?id=608815
oobs/oobs-group-private.h | 1 -
oobs/oobs-group.c | 161 +++++++++++++++++++++++++++++----------------
oobs/oobs-groupsconfig.c | 85 +-----------------------
oobs/oobs-usersconfig.c | 20 ++++++
4 files changed, 124 insertions(+), 143 deletions(-)
---
diff --git a/oobs/oobs-group-private.h b/oobs/oobs-group-private.h
index 8e0a05f..2bb0cf3 100644
--- a/oobs/oobs-group-private.h
+++ b/oobs/oobs-group-private.h
@@ -30,7 +30,6 @@ G_BEGIN_DECLS
OobsGroup*
_oobs_group_create_from_dbus_reply (OobsObject *object,
- GList **users_ptr,
DBusMessage *reply,
DBusMessageIter struct_iter);
diff --git a/oobs/oobs-group.c b/oobs/oobs-group.c
index bb29b98..822c83a 100644
--- a/oobs/oobs-group.c
+++ b/oobs/oobs-group.c
@@ -53,12 +53,17 @@ struct _OobsGroupPrivate {
gchar *password;
gid_t gid;
+ /* List of names received from the backends, possibly with unknown users */
+ GList *usernames;
+ /* List of OobsUsers updated from the above, only containing known users,
+ * and working has a cache to access the above from public API. */
GList *users;
};
-static void oobs_group_class_init (OobsGroupClass *class);
-static void oobs_group_init (OobsGroup *group);
-static void oobs_group_finalize (GObject *object);
+static void oobs_group_class_init (OobsGroupClass *class);
+static void oobs_group_init (OobsGroup *group);
+static void oobs_group_constructed (GObject *object);
+static void oobs_group_finalize (GObject *object);
static void oobs_group_set_property (GObject *object,
guint prop_id,
@@ -73,8 +78,6 @@ static void oobs_group_commit (OobsObject *object);
static void oobs_group_update (OobsObject *object);
static void oobs_group_get_update_message (OobsObject *object);
-static GList* get_users_list (OobsGroup *group);
-
enum {
PROP_0,
PROP_GROUPNAME,
@@ -92,6 +95,7 @@ oobs_group_class_init (OobsGroupClass *class)
object_class->set_property = oobs_group_set_property;
object_class->get_property = oobs_group_get_property;
+ object_class->constructed = oobs_group_constructed;
object_class->finalize = oobs_group_finalize;
oobs_class->commit = oobs_group_commit;
@@ -137,11 +141,48 @@ oobs_group_init (OobsGroup *group)
priv->config = oobs_groups_config_get ();
priv->groupname = NULL;
priv->password = NULL;
+ priv->usernames = NULL;
priv->users = NULL;
group->_priv = priv;
}
+/*
+ * Clear OobsUsers list and fill it with updated references.
+ */
+static void
+oobs_group_users_updated (OobsGroup *group,
+ OobsUsersConfig *users_config)
+{
+ OobsGroupPrivate *priv;
+ OobsUser *user;
+ GList *l;
+
+ priv = OOBS_GROUP_GET_PRIVATE (group);
+
+ g_list_foreach (priv->users, (GFunc) g_object_unref, NULL);
+ g_list_free (priv->users);
+ priv->users = NULL;
+
+ for (l = priv->usernames; l; l = l->next)
+ {
+ user = oobs_users_config_get_from_login (users_config, l->data);
+
+ /* A reference has already been added by the above call,
+ * keep it until user is removed from list or the group is destroyed. */
+ if (user)
+ priv->users = g_list_prepend (priv->users, user);
+ }
+}
+
+static void
+oobs_group_constructed (GObject *object)
+{
+ /* stay tuned of changes in users config */
+ g_signal_connect_swapped (oobs_users_config_get (), "updated",
+ G_CALLBACK (oobs_group_users_updated), object);
+}
+
static void
oobs_group_set_property (GObject *object,
guint prop_id,
@@ -217,6 +258,9 @@ oobs_group_finalize (GObject *object)
g_free (priv->groupname);
g_free (priv->password);
+ g_list_foreach (priv->usernames, (GFunc) g_free, NULL);
+ g_list_free (priv->usernames);
+
g_list_foreach (priv->users, (GFunc) g_object_unref, NULL);
g_list_free (priv->users);
}
@@ -225,37 +269,17 @@ oobs_group_finalize (GObject *object)
(* G_OBJECT_CLASS (oobs_group_parent_class)->finalize) (object);
}
-static GList*
-get_users_list (OobsGroup *group)
-{
- GList *users, *elem, *usernames = NULL;
- OobsUser *user;
-
- users = elem = oobs_group_get_users (group);
-
- while (elem)
- {
- user = elem->data;
- usernames = g_list_prepend (usernames, (gpointer) oobs_user_get_login_name (user));
-
- elem = elem->next;
- }
-
- g_list_free (users);
- return usernames;
-}
-
OobsGroup*
_oobs_group_create_from_dbus_reply (OobsObject *object,
- GList **users_ptr,
DBusMessage *reply,
DBusMessageIter struct_iter)
{
DBusMessageIter iter;
guint32 gid;
const gchar *groupname, *passwd;
- GList *users;
OobsGroup *group;
+ OobsGroupPrivate *priv;
+ OobsObject *users_config;
dbus_message_iter_recurse (&struct_iter, &iter);
@@ -263,17 +287,26 @@ _oobs_group_create_from_dbus_reply (OobsObject *object,
passwd = utils_get_string (&iter);
gid = utils_get_uint (&iter);
- users = utils_get_string_list_from_dbus_reply (reply, &iter);
-
- if (users_ptr)
- *users_ptr = users;
-
group = oobs_group_new (groupname);
g_object_set (G_OBJECT (group),
"password", passwd,
"gid", gid,
NULL);
+ /* This list is kept in this form rather than as OobsUsers* because
+ * we don't want to remove unknown users from groups (users not in
+ * /etc/passwd such as that from LDAP). */
+ priv = OOBS_GROUP_GET_PRIVATE (group);
+ priv->usernames = utils_get_string_list_from_dbus_reply (reply, &iter);
+
+ /* just update users if the object was already
+ * updated, update will be forced later if required
+ */
+ users_config = oobs_users_config_get ();
+ if (oobs_object_has_updated (users_config))
+ oobs_group_users_updated (group,
+ OOBS_USERS_CONFIG (users_config));
+
return OOBS_GROUP (group);
}
@@ -282,10 +315,12 @@ _oobs_create_dbus_struct_from_group (OobsGroup *group,
DBusMessage *message,
DBusMessageIter *array_iter)
{
+ OobsGroupPrivate *priv;
DBusMessageIter struct_iter;
guint32 gid;
gchar *groupname, *passwd;
- GList *users;
+
+ priv = OOBS_GROUP_GET_PRIVATE (group);
g_object_get (group,
"name", &groupname,
@@ -293,19 +328,16 @@ _oobs_create_dbus_struct_from_group (OobsGroup *group,
"gid", &gid,
NULL);
- users = get_users_list (OOBS_GROUP (group));
-
dbus_message_iter_open_container (array_iter, DBUS_TYPE_STRUCT, NULL, &struct_iter);
utils_append_string (&struct_iter, groupname);
utils_append_string (&struct_iter, passwd);
utils_append_uint (&struct_iter, gid);
- utils_create_dbus_array_from_string_list (users, message, &struct_iter);
+ utils_create_dbus_array_from_string_list (priv->usernames, message, &struct_iter);
dbus_message_iter_close_container (array_iter, &struct_iter);
- g_list_free (users);
g_free (groupname);
g_free (passwd);
}
@@ -348,7 +380,7 @@ oobs_group_update (OobsObject *object)
reply = _oobs_object_get_dbus_message (object);
dbus_message_iter_init (reply, &iter);
- _oobs_group_create_from_dbus_reply (object, NULL, reply, iter);
+ _oobs_group_create_from_dbus_reply (object, reply, iter);
}
/**
@@ -463,22 +495,9 @@ oobs_group_get_users (OobsGroup *group)
g_return_val_if_fail (OOBS_IS_GROUP (group), NULL);
- priv = group->_priv;
- return g_list_copy (priv->users);
-}
-
-void
-oobs_group_clear_users (OobsGroup *group)
-{
- OobsGroupPrivate *priv;
-
- g_return_if_fail (OOBS_IS_GROUP (group));
-
- priv = group->_priv;
+ priv = OOBS_GROUP_GET_PRIVATE (group);
- g_list_foreach (priv->users, (GFunc) g_object_unref, NULL);
- g_list_free (priv->users);
- priv->users = NULL;
+ return g_list_copy (priv->users);
}
/**
@@ -494,13 +513,22 @@ oobs_group_add_user (OobsGroup *group,
OobsUser *user)
{
OobsGroupPrivate *priv;
+ const char *login;
g_return_if_fail (OOBS_IS_GROUP (group));
g_return_if_fail (OOBS_IS_USER (user));
- priv = group->_priv;
+ priv = OOBS_GROUP_GET_PRIVATE (group);
+
+ login = oobs_user_get_login_name (user);
+
+ /* Update usernames list and OobsUsers list. First is used to commit,
+ * second is used for public API. */
+
+ /* Try to avoid several occurrences */
+ if (!g_list_find_custom (priv->usernames, login, (GCompareFunc) strcmp))
+ priv->usernames = g_list_prepend (priv->usernames, g_strdup (login));
- /* try to avoid several instances */
if (!g_list_find (priv->users, user))
priv->users = g_list_prepend (priv->users, g_object_ref (user));
}
@@ -518,14 +546,31 @@ oobs_group_remove_user (OobsGroup *group,
OobsUser *user)
{
OobsGroupPrivate *priv;
+ GList *l;
+ const char *login;
g_return_if_fail (OOBS_IS_GROUP (group));
g_return_if_fail (OOBS_IS_USER (user));
- priv = group->_priv;
+ priv = OOBS_GROUP_GET_PRIVATE (group);
+
+ login = oobs_user_get_login_name (user);
- /* there might be several instances */
- priv->users = g_list_remove_all (priv->users, user);
+ /* Update usernames list and OobsUsers list. First is used to commit,
+ * second is used for public API. */
+
+ /* There might be several instances */
+ while ((l = g_list_find_custom (priv->usernames, login, (GCompareFunc) strcmp)))
+ {
+ g_free (l->data);
+ priv->usernames = g_list_delete_link (priv->usernames, l);
+ }
+
+ while ((l = g_list_find (priv->users, user)))
+ {
+ g_object_unref (user);
+ priv->users = g_list_delete_link (priv->users, l);
+ }
}
/**
diff --git a/oobs/oobs-groupsconfig.c b/oobs/oobs-groupsconfig.c
index f54a55c..9bf70df 100644
--- a/oobs/oobs-groupsconfig.c
+++ b/oobs/oobs-groupsconfig.c
@@ -50,8 +50,6 @@ struct _OobsGroupsConfigPrivate
{
OobsList *groups_list;
- GHashTable *users;
-
gid_t minimum_gid;
gid_t maximum_gid;
};
@@ -70,9 +68,6 @@ static void oobs_groups_config_get_property (GObject *object,
GValue *value,
GParamSpec *pspec);
-static void oobs_groups_config_users_updated (OobsGroupsConfig *groups,
- OobsUsersConfig *users);
-
static void oobs_groups_config_update (OobsObject *object);
static void oobs_groups_config_commit (OobsObject *object);
@@ -94,7 +89,6 @@ oobs_groups_config_class_init (OobsGroupsConfigClass *class)
object_class->set_property = oobs_groups_config_set_property;
object_class->get_property = oobs_groups_config_get_property;
- object_class->constructed = oobs_groups_config_constructed;
object_class->finalize = oobs_groups_config_finalize;
oobs_object_class->commit = oobs_groups_config_commit;
@@ -119,13 +113,6 @@ oobs_groups_config_class_init (OobsGroupsConfigClass *class)
}
static void
-free_users (GList *users)
-{
- g_list_foreach (users, (GFunc) g_free, NULL);
- g_list_free (users);
-}
-
-static void
oobs_groups_config_init (OobsGroupsConfig *config)
{
OobsGroupsConfigPrivate *priv;
@@ -136,18 +123,6 @@ oobs_groups_config_init (OobsGroupsConfig *config)
config->_priv = priv;
priv->groups_list = _oobs_list_new (OOBS_TYPE_GROUP);
-
- priv->users = g_hash_table_new_full (NULL, NULL,
- (GDestroyNotify) g_object_unref,
- (GDestroyNotify) free_users);
-}
-
-static void
-oobs_groups_config_constructed (GObject *object)
-{
- /* stay tuned of changes in users config */
- g_signal_connect_swapped (oobs_users_config_get (), "updated",
- G_CALLBACK (oobs_groups_config_users_updated), object);
}
static void
@@ -160,10 +135,7 @@ oobs_groups_config_finalize (GObject *object)
priv = OOBS_GROUPS_CONFIG (object)->_priv;
if (priv)
- {
g_object_unref (priv->groups_list);
- g_hash_table_unref (priv->users);
- }
if (G_OBJECT_CLASS (oobs_groups_config_parent_class)->finalize)
(* G_OBJECT_CLASS (oobs_groups_config_parent_class)->finalize) (object);
@@ -216,78 +188,32 @@ oobs_groups_config_get_property (GObject *object,
}
static void
-query_users_foreach (OobsGroup *group,
- GList *users,
- gpointer data)
-{
- OobsUsersConfig *users_config = OOBS_USERS_CONFIG (data);
- OobsUser *user;
-
- oobs_group_clear_users (group);
-
- while (users)
- {
- user = oobs_users_config_get_from_login (users_config, users->data);
-
- if (user)
- {
- oobs_group_add_user (group, user);
- g_object_unref (user);
- }
-
- users = users->next;
- }
-}
-
-static void
-oobs_groups_config_users_updated (OobsGroupsConfig *groups,
- OobsUsersConfig *users)
-{
- OobsGroupsConfigPrivate *priv;
-
- priv = groups->_priv;
- g_hash_table_foreach (priv->users, (GHFunc) query_users_foreach, users);
-}
-
-static void
oobs_groups_config_update (OobsObject *object)
{
OobsGroupsConfigPrivate *priv;
- OobsObject *users_config;
DBusMessage *reply;
DBusMessageIter iter, elem_iter;
OobsListIter list_iter;
GObject *group;
- GList *users;
priv = OOBS_GROUPS_CONFIG (object)->_priv;
reply = _oobs_object_get_dbus_message (object);
/* First of all, free the previous configuration */
oobs_list_clear (priv->groups_list);
- g_hash_table_remove_all (priv->users);
dbus_message_iter_init (reply, &iter);
dbus_message_iter_recurse (&iter, &elem_iter);
while (dbus_message_iter_get_arg_type (&elem_iter) == DBUS_TYPE_STRUCT)
{
- group = G_OBJECT (_oobs_group_create_from_dbus_reply (object, &users, reply, elem_iter));
+ group = G_OBJECT (_oobs_group_create_from_dbus_reply (object, reply, elem_iter));
oobs_list_append (priv->groups_list, &list_iter);
oobs_list_set (priv->groups_list, &list_iter, G_OBJECT (group));
- /* put the users list in the hashtable, will be used each time
- * the users config has changed, in order to get references to
- * the new user objects
- */
- g_hash_table_insert (priv->users,
- g_object_ref (group),
- users);
-
g_object_unref (group);
-
dbus_message_iter_next (&elem_iter);
}
@@ -295,15 +221,6 @@ oobs_groups_config_update (OobsObject *object)
priv->minimum_gid = utils_get_uint (&iter);
priv->maximum_gid = utils_get_uint (&iter);
-
- users_config = oobs_users_config_get ();
-
- /* just update users if the object was already
- * updated, update will be forced later if required
- */
- if (oobs_object_has_updated (users_config))
- oobs_groups_config_users_updated (OOBS_GROUPS_CONFIG (object),
- OOBS_USERS_CONFIG (users_config));
}
static void
diff --git a/oobs/oobs-usersconfig.c b/oobs/oobs-usersconfig.c
index 0790da6..da4c40a 100644
--- a/oobs/oobs-usersconfig.c
+++ b/oobs/oobs-usersconfig.c
@@ -505,19 +505,39 @@ oobs_users_config_delete_user (OobsUsersConfig *config, OobsUser *user)
OobsListIter list_iter;
gboolean valid;
OobsResult result;
+ OobsList *groups_list;
+ OobsGroup *group;
g_return_val_if_fail (config != NULL, OOBS_RESULT_MALFORMED_DATA);
g_return_val_if_fail (user != NULL, OOBS_RESULT_MALFORMED_DATA);
g_return_val_if_fail (OOBS_IS_USERS_CONFIG (config), OOBS_RESULT_MALFORMED_DATA);
g_return_val_if_fail (OOBS_IS_USER (user), OOBS_RESULT_MALFORMED_DATA);
+ /* Try to commit changes */
result = oobs_object_delete (OOBS_OBJECT (user));
if (result != OOBS_RESULT_OK)
return result;
+
priv = config->_priv;
+ /* Remove user from all groups, to avoid committing to /etc/group
+ * the name of a non-existent user */
+ groups_list = oobs_groups_config_get_groups (OOBS_GROUPS_CONFIG (oobs_groups_config_get ()));
+
+ valid = oobs_list_get_iter_first (groups_list, &list_iter);
+
+ while (valid) {
+ group = OOBS_GROUP (oobs_list_get (groups_list, &list_iter));
+
+ oobs_group_remove_user (group, user);
+
+ valid = oobs_list_iter_next (priv->users_list, &list_iter);
+ }
+
+
+ /* Then remove user from the list */
valid = oobs_list_get_iter_first (priv->users_list, &list_iter);
while (valid) {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
