From 83567748f5f5c4eabc233680a553f3edd803a24d Mon Sep 17 00:00:00 2001

From: Ray Strode <rstrode@redhat.com>

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);