From 83567748f5f5c4eabc233680a553f3edd803a24d Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Thu, 4 May 2017 12:04:05 -0400 Subject: [PATCH] daemon: don't treat explicitly requested users as "cached" The ListCachedUsers method currently returns users that have been explicitly requested by a client. It's weird that merely querying a user can make it show up in login screen user lists. Furthermore, UncacheUser is broken since commit 177509e9460b149ecbf85e75c930be2ea00b7d05 because the user has been explicitly requested in order to uncache it. So trying to uncache a user inadvertently caches the user. This commit fixes that. https://bugs.freedesktop.org/show_bug.cgi?id=101052 --- src/daemon.c | 71 +++++++++++++++++++++++++++++++++++++++--------------------- src/user.c | 17 +++++++++++++++ src/user.h | 3 +++ 3 files changed, 66 insertions(+), 25 deletions(-) diff --git a/src/daemon.c b/src/daemon.c index 4586eff..fce5a60 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -481,100 +481,108 @@ entry_generator_requested_users (Daemon *daemon, while (node != NULL) { const char *name; name = node->data; node = node->next; *state = node; if (!g_hash_table_lookup (users, name)) { pwent = getpwnam (name); if (pwent == NULL) { g_debug ("user '%s' requested previously but not present on system", name); } else { *shadow_entry = getspnam (pwent->pw_name); return pwent; } } } } /* Last iteration */ *state = NULL; return NULL; } static void load_entries (Daemon *daemon, GHashTable *users, - gboolean allow_system_users, + gboolean explicitly_requested, EntryGeneratorFunc entry_generator) { gpointer generator_state = NULL; struct passwd *pwent; struct spwd *spent = NULL; User *user = NULL; g_assert (entry_generator != NULL); for (;;) { spent = NULL; pwent = entry_generator (daemon, users, &generator_state, &spent); if (pwent == NULL) break; /* Skip system users... */ - if (!allow_system_users && !user_classify_is_human (pwent->pw_uid, pwent->pw_name, pwent->pw_shell, spent? spent->sp_pwdp : NULL)) { + if (!explicitly_requested && !user_classify_is_human (pwent->pw_uid, pwent->pw_name, pwent->pw_shell, spent? spent->sp_pwdp : NULL)) { g_debug ("skipping user: %s", pwent->pw_name); continue; } - /* ignore duplicate entries */ - if (g_hash_table_lookup (users, pwent->pw_name)) { - continue; - } + /* Only process users that haven't been processed yet. + * We do always make sure entries get promoted + * to "cached" status if they are supposed to be + */ + + user = g_hash_table_lookup (users, pwent->pw_name); - user = g_hash_table_lookup (daemon->priv->users, pwent->pw_name); if (user == NULL) { - user = user_new (daemon, pwent->pw_uid); - } else { - g_object_ref (user); - } + user = g_hash_table_lookup (daemon->priv->users, pwent->pw_name); + if (user == NULL) { + user = user_new (daemon, pwent->pw_uid); + } else { + g_object_ref (user); + } + + /* freeze & update users not already in the new list */ + g_object_freeze_notify (G_OBJECT (user)); + user_update_from_pwent (user, pwent, spent); - /* freeze & update users not already in the new list */ - g_object_freeze_notify (G_OBJECT (user)); - user_update_from_pwent (user, pwent, spent); + g_hash_table_insert (users, g_strdup (user_get_user_name (user)), user); + g_debug ("loaded user: %s", user_get_user_name (user)); + } - g_hash_table_insert (users, g_strdup (user_get_user_name (user)), user); - g_debug ("loaded user: %s", user_get_user_name (user)); + if (!explicitly_requested) { + user_set_cached (user, TRUE); + } } /* Generator should have cleaned up */ g_assert (generator_state == NULL); } static GHashTable * create_users_hash_table (void) { return g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_object_unref); } static void reload_users (Daemon *daemon) { GHashTable *users; GHashTable *old_users; GHashTable *local; GHashTableIter iter; gpointer name; User *user; /* Track the users that we saw during our (re)load */ users = create_users_hash_table (); /* * NOTE: As we load data from all the sources, notifies are @@ -586,71 +594,79 @@ reload_users (Daemon *daemon) load_entries (daemon, users, FALSE, entry_generator_fgetpwent); local = g_hash_table_new (g_str_hash, g_str_equal); g_hash_table_iter_init (&iter, users); while (g_hash_table_iter_next (&iter, &name, NULL)) g_hash_table_add (local, name); /* and add users to hash table that were explicitly requested */ load_entries (daemon, users, TRUE, entry_generator_requested_users); /* Now add/update users from other sources, possibly non-local */ load_entries (daemon, users, FALSE, entry_generator_cachedir); #ifdef HAVE_UTMPX_H wtmp_update_login_frequencies (users); #endif /* Mark which users are local, which are not */ g_hash_table_iter_init (&iter, users); while (g_hash_table_iter_next (&iter, &name, (gpointer *)&user)) user_update_local_account_property (user, g_hash_table_lookup (local, name) != NULL); g_hash_table_destroy (local); /* Swap out the users */ old_users = daemon->priv->users; daemon->priv->users = users; /* Remove all the old users */ g_hash_table_iter_init (&iter, old_users); while (g_hash_table_iter_next (&iter, &name, (gpointer *)&user)) { - if (!g_hash_table_lookup (users, name)) { + User *refreshed_user; + + refreshed_user = g_hash_table_lookup (users, name); + + if (!refreshed_user || !user_get_cached (refreshed_user)) { user_unregister (user); accounts_accounts_emit_user_deleted (ACCOUNTS_ACCOUNTS (daemon), user_get_object_path (user)); } } /* Register all the new users */ g_hash_table_iter_init (&iter, users); while (g_hash_table_iter_next (&iter, &name, (gpointer *)&user)) { - if (!g_hash_table_lookup (old_users, name)) { + User *stale_user; + + stale_user = g_hash_table_lookup (old_users, name); + + if (!stale_user || !user_get_cached (stale_user) && user_get_cached (user)) { user_register (user); accounts_accounts_emit_user_added (ACCOUNTS_ACCOUNTS (daemon), user_get_object_path (user)); } g_object_thaw_notify (G_OBJECT (user)); } g_hash_table_destroy (old_users); } static gboolean reload_users_timeout (Daemon *daemon) { reload_users (daemon); daemon->priv->reload_id = 0; return FALSE; } static gboolean load_autologin (Daemon *daemon, gchar **name, gboolean *enabled, GError **error); static gboolean reload_autologin_timeout (Daemon *daemon) { gboolean enabled; gchar *name = NULL; GError *error = NULL; @@ -1063,60 +1079,65 @@ list_user_data_new (Daemon *daemon, static void list_user_data_free (ListUserData *data) { g_object_unref (data->daemon); g_free (data); } static gboolean finish_list_cached_users (gpointer user_data) { ListUserData *data = user_data; GPtrArray *object_paths; GHashTableIter iter; const gchar *name; User *user; uid_t uid; const gchar *shell; object_paths = g_ptr_array_new (); g_hash_table_iter_init (&iter, data->daemon->priv->users); while (g_hash_table_iter_next (&iter, (gpointer *)&name, (gpointer *)&user)) { uid = user_get_uid (user); shell = user_get_shell (user); if (!user_classify_is_human (uid, name, shell, NULL)) { g_debug ("user %s %ld excluded", name, (long) uid); continue; } + if (!user_get_cached (user)) { + g_debug ("user %s %ld not cached", name, (long) uid); + continue; + } + g_debug ("user %s %ld not excluded", name, (long) uid); g_ptr_array_add (object_paths, (gpointer) user_get_object_path (user)); } g_ptr_array_add (object_paths, NULL); accounts_accounts_complete_list_cached_users (NULL, data->context, (const gchar * const *) object_paths->pdata); g_ptr_array_free (object_paths, TRUE); list_user_data_free (data); return FALSE; } static gboolean daemon_list_cached_users (AccountsAccounts *accounts, GDBusMethodInvocation *context) { Daemon *daemon = (Daemon*)accounts; ListUserData *data; data = list_user_data_new (daemon, context); if (daemon->priv->reload_id > 0) { /* reload in progress, wait a bit */ g_idle_add (finish_list_cached_users, data); } else { finish_list_cached_users (data); } @@ -1303,123 +1324,123 @@ daemon_cache_user (AccountsAccounts *accounts, static void daemon_uncache_user_authorized_cb (Daemon *daemon, User *dummy, GDBusMethodInvocation *context, gpointer data) { const gchar *user_name = data; gchar *filename; User *user; sys_log (context, "uncache user '%s'", user_name); user = daemon_local_find_user_by_name (daemon, user_name); if (user == NULL) { throw_error (context, ERROR_USER_DOES_NOT_EXIST, "No user with the name %s found", user_name); return; } /* Always use the canonical user name looked up */ user_name = user_get_user_name (user); filename = g_build_filename (USERDIR, user_name, NULL); g_remove (filename); g_free (filename); filename = g_build_filename (ICONDIR, user_name, NULL); g_remove (filename); g_free (filename); + user_set_cached (user, FALSE); + accounts_accounts_complete_uncache_user (NULL, context); queue_reload_users (daemon); } static gboolean daemon_uncache_user (AccountsAccounts *accounts, GDBusMethodInvocation *context, const gchar *user_name) { Daemon *daemon = (Daemon*)accounts; daemon_local_check_auth (daemon, NULL, "org.freedesktop.accounts.user-administration", TRUE, daemon_uncache_user_authorized_cb, context, g_strdup (user_name), g_free); return TRUE; } typedef struct { uid_t uid; gboolean remove_files; } DeleteUserData; static void daemon_delete_user_authorized_cb (Daemon *daemon, User *dummy, GDBusMethodInvocation *context, gpointer data) { DeleteUserData *ud = data; GError *error; gchar *filename; struct passwd *pwent; const gchar *argv[6]; + User *user; pwent = getpwuid (ud->uid); if (pwent == NULL) { throw_error (context, ERROR_USER_DOES_NOT_EXIST, "No user with uid %d found", ud->uid); return; } sys_log (context, "delete user '%s' (%d)", pwent->pw_name, ud->uid); - if (daemon->priv->autologin != NULL) { - User *user; + user = daemon_local_find_user_by_id (daemon, ud->uid); - user = daemon_local_find_user_by_id (daemon, ud->uid); + if (user != NULL) { + user_set_cached (user, FALSE); - g_assert (user != NULL); - - if (daemon->priv->autologin == user) { + if (daemon->priv->autologin == user) { daemon_local_set_automatic_login (daemon, user, FALSE, NULL); } - } filename = g_build_filename (USERDIR, pwent->pw_name, NULL); g_remove (filename); g_free (filename); filename = g_build_filename (ICONDIR, pwent->pw_name, NULL); g_remove (filename); g_free (filename); argv[0] = "/usr/sbin/userdel"; if (ud->remove_files) { argv[1] = "-f"; argv[2] = "-r"; argv[3] = "--"; argv[4] = pwent->pw_name; argv[5] = NULL; } else { argv[1] = "-f"; argv[2] = "--"; argv[3] = pwent->pw_name; argv[4] = NULL; } error = NULL; if (!spawn_with_login_uid (context, argv, &error)) { throw_error (context, ERROR_FAILED, "running '%s' failed: %s", argv[0], error->message); g_error_free (error); return; diff --git a/src/user.c b/src/user.c index 247ca2f..056be2f 100644 --- a/src/user.c +++ b/src/user.c @@ -77,60 +77,61 @@ struct User { GDBusConnection *system_bus_connection; gchar *object_path; Daemon *daemon; GKeyFile *keyfile; uid_t uid; gid_t gid; gchar *user_name; gchar *real_name; AccountType account_type; PasswordMode password_mode; gchar *password_hint; gchar *home_dir; gchar *shell; gchar *email; gchar *language; gchar *x_session; gchar *location; guint64 login_frequency; gint64 login_time; GVariant *login_history; gchar *icon_file; gchar *default_icon_file; gboolean locked; gboolean automatic_login; gboolean system_account; gboolean local_account; + gboolean cached; }; typedef struct UserClass { AccountsUserSkeletonClass parent_class; } UserClass; static void user_accounts_user_iface_init (AccountsUserIface *iface); G_DEFINE_TYPE_WITH_CODE (User, user, ACCOUNTS_TYPE_USER_SKELETON, G_IMPLEMENT_INTERFACE (ACCOUNTS_TYPE_USER, user_accounts_user_iface_init)); static gint account_type_from_pwent (struct passwd *pwent) { struct group *grp; gint i; if (pwent->pw_uid == 0) { g_debug ("user is root so account type is administrator"); return ACCOUNT_TYPE_ADMINISTRATOR; } grp = getgrnam (ADMIN_GROUP); if (grp == NULL) { g_debug (ADMIN_GROUP " group not found"); return ACCOUNT_TYPE_STANDARD; } for (i = 0; grp->gr_mem[i] != NULL; i++) { if (g_strcmp0 (grp->gr_mem[i], pwent->pw_name) == 0) { @@ -323,109 +324,112 @@ user_update_from_keyfile (User *user, user->location = s; g_object_notify (G_OBJECT (user), "location"); } s = g_key_file_get_string (keyfile, "User", "PasswordHint", NULL); if (s != NULL) { g_free (user->password_hint); user->password_hint = s; g_object_notify (G_OBJECT (user), "password-hint"); } s = g_key_file_get_string (keyfile, "User", "Icon", NULL); if (s != NULL) { g_free (user->icon_file); user->icon_file = s; g_object_notify (G_OBJECT (user), "icon-file"); } if (g_key_file_has_key (keyfile, "User", "SystemAccount", NULL)) { gboolean system_account; system_account = g_key_file_get_boolean (keyfile, "User", "SystemAccount", NULL); if (system_account != user->system_account) { user->system_account = system_account; g_object_notify (G_OBJECT (user), "system-account"); } } g_clear_pointer (&user->keyfile, g_key_file_unref); user->keyfile = g_key_file_ref (keyfile); + user_set_cached (user, TRUE); g_object_thaw_notify (G_OBJECT (user)); } void user_update_local_account_property (User *user, gboolean local) { if (local == user->local_account) return; user->local_account = local; g_object_notify (G_OBJECT (user), "local-account"); } void user_update_system_account_property (User *user, gboolean system) { if (system == user->system_account) return; user->system_account = system; g_object_notify (G_OBJECT (user), "system-account"); } static void user_save_to_keyfile (User *user, GKeyFile *keyfile) { g_key_file_remove_group (keyfile, "User", NULL); if (user->email) g_key_file_set_string (keyfile, "User", "Email", user->email); if (user->language) g_key_file_set_string (keyfile, "User", "Language", user->language); if (user->x_session) g_key_file_set_string (keyfile, "User", "XSession", user->x_session); if (user->location) g_key_file_set_string (keyfile, "User", "Location", user->location); if (user->password_hint) g_key_file_set_string (keyfile, "User", "PasswordHint", user->password_hint); if (user->icon_file) g_key_file_set_string (keyfile, "User", "Icon", user->icon_file); g_key_file_set_boolean (keyfile, "User", "SystemAccount", user->system_account); + + user_set_cached (user, TRUE); } static void save_extra_data (User *user) { gchar *filename; gchar *data; GError *error; user_save_to_keyfile (user, user->keyfile); error = NULL; data = g_key_file_to_data (user->keyfile, NULL, &error); if (error == NULL) { filename = g_build_filename (USERDIR, user->user_name, NULL); g_file_set_contents (filename, data, -1, &error); g_free (filename); g_free (data); } if (error) { g_warning ("Saving data for user %s failed: %s", user->user_name, error->message); g_error_free (error); } } static void move_extra_data (const gchar *old_name, @@ -524,60 +528,73 @@ user_get_user_name (User *user) gboolean user_get_system_account (User *user) { return user->system_account; } gboolean user_get_local_account (User *user) { return user->local_account; } const gchar * user_get_object_path (User *user) { return user->object_path; } uid_t user_get_uid (User *user) { return user->uid; } const gchar * user_get_shell(User *user) { return user->shell; } +gboolean +user_get_cached (User *user) +{ + return user->cached; +} + +void +user_set_cached (User *user, + gboolean cached) +{ + user->cached = cached; +} + static void throw_error (GDBusMethodInvocation *context, gint error_code, const gchar *format, ...) { va_list args; gchar *message; va_start (args, format); message = g_strdup_vprintf (format, args); va_end (args); g_dbus_method_invocation_return_error (context, ERROR, error_code, "%s", message); g_free (message); } static void user_change_real_name_authorized_cb (Daemon *daemon, User *user, GDBusMethodInvocation *context, gpointer data) { gchar *name = data; GError *error; const gchar *argv[6]; if (g_strcmp0 (user->real_name, name) != 0) { diff --git a/src/user.h b/src/user.h index 22548f9..39c6f13 100644 --- a/src/user.h +++ b/src/user.h @@ -36,47 +36,50 @@ G_BEGIN_DECLS #define IS_USER(object) (G_TYPE_CHECK_INSTANCE_TYPE ((object), TYPE_USER)) typedef enum { ACCOUNT_TYPE_STANDARD, ACCOUNT_TYPE_ADMINISTRATOR, #define ACCOUNT_TYPE_LAST ACCOUNT_TYPE_ADMINISTRATOR } AccountType; typedef enum { PASSWORD_MODE_REGULAR, PASSWORD_MODE_SET_AT_LOGIN, PASSWORD_MODE_NONE, #define PASSWORD_MODE_LAST PASSWORD_MODE_NONE } PasswordMode; /* local methods */ GType user_get_type (void) G_GNUC_CONST; User * user_new (Daemon *daemon, uid_t uid); void user_update_from_pwent (User *user, struct passwd *pwent, struct spwd *spent); void user_update_from_keyfile (User *user, GKeyFile *keyfile); void user_update_local_account_property (User *user, gboolean local); void user_update_system_account_property (User *user, gboolean system); +gboolean user_get_cached (User *user); +void user_set_cached (User *user, + gboolean cached); void user_register (User *user); void user_unregister (User *user); void user_changed (User *user); void user_save (User *user); const gchar * user_get_user_name (User *user); gboolean user_get_system_account (User *user); gboolean user_get_local_account (User *user); const gchar * user_get_object_path (User *user); uid_t user_get_uid (User *user); const gchar * user_get_shell (User *user); G_END_DECLS #endif -- 2.12.2