Merge lp:~gagern/lightdm/preferred-users into lp:lightdm

[Martin von Gagern]

In the absence of AccountsService, this patch allows for a preferred-users setting in the users.conf file which will result in the named users getting sorted to the top of the list, in the order in which they have been specified. This is particularly important if the greeter presents the first user as the default selection, like the gtk greeter does. That way, the person owning the computer can log simply by typing his password, even if there are other accounts on the system.

[Robert Ancell (robert-ancell)]

I've been mulling this over and I need some convincing this feature is important.

You mention the case where the GTK+ greeter always selects the first user, not the most common user account. Isn't this just a bug in the GTK+ greeter? Would it selecting the last logged in user solve this?

For this to be in LightDM the feature should be useful for all greeters. Given the above statement, I'm not sure why other greeters might want this feature.

If this feature did exist, it should apply to the ordering of the user list even if it's from AccountsService otherwise the behaviour is inconsistent.

There are some bugs:
- You read the config from [UserList] but data/users.conf says it is in [UserAccounts]
- preferred_users should be a field in CommonUserListPrivate, not a static variable. At the moment you overwrite it each time load_passwd_file is called which happens each time /etc/passwd changes and also you may have more than one instance of CommonUserList.

review: Needs Information

[Martin von Gagern (gagern)]

Sorry that it took me so long to reply to this.

I patched my lightdm at the same time that I also disabled AccountsService support. It seemed to work at the time, but after your comment I found out that this was more of a coincidence.

It turns out that the GTK+ greeter will indeed store the last selected user, but it gives higher priority to lightdm_greeter_get_select_user_hint. It seems that at least on my system, AccountService sets these hints in a way that doesn't save the most recently used account name.
http://bazaar.launchpad.net/~lightdm-gtk-greeter-team/lightdm-gtk-greeter/trunk/view/285/src/lightdm-gtk-greeter.c#L1841

You are right that there is a discrepancy between [UserList] and [UserAccounts] as the title of the config file section. However, this is the result os some copy and paste, which means that existing code is affected by this discrepancy as well. So I guess you should fix this for the existing settings, although I'm not sure whether to better fix this by changing the program code or the example config.

I would very much welcome consistent behaviour for the case with and without AccountsService. But again this does affect existing settings as well. I see no reason against being able to manually hide users even if AccountsServices is in use.

As it stands, my patch is certainly not suited for merging. I'll try to address the issues you mentioned when I find the time. Before I'd do so, I'd welcome input whether you want the config section to be called [UserList] or [UserAccounts]. I'd also try to understand how these hints work, how AccountsService affects them, and whether it would make sense being able to override some of them.

[Robert Ancell (robert-ancell)]

You were right to use [UserList] - that is the correct name. It was pointed out in bug 1069218 that the example configuration had it wrong.

Unmerged revisions

1821. By Martin von Gagern on 2014-04-30

Document preferred-users setting in users.conf.

1820. By Martin von Gagern on 2014-04-30

Merge the rename from LightDMUser to CommonUser.

1819. By Martin von Gagern on 2014-04-30

Allow for a preferred-users setting to affect user list ordering.

=== modified file 'common/user-list.c'
--- common/user-list.c	2014-03-06 02:40:18 +0000
+++ common/user-list.c	2014-04-30 07:41:38 +0000
@@ -222,11 +222,31 @@
 
     return NULL;
 }
-  
+
+static gchar **preferred_users = NULL;
+
+static int
+prefer_user (CommonUser *user)
+{
+  int i;
+  const gchar *name;
+
+  if (preferred_users == NULL)
+    return 0;
+  name = common_user_get_name (user);
+  for (i = 0; preferred_users[i]; i++)
+    if (strcmp (name, preferred_users[i]) == 0)
+      return i;
+  return i;
+}
+
 static gint
 compare_user (gconstpointer a, gconstpointer b)
 {
     CommonUser *user_a = (CommonUser *) a, *user_b = (CommonUser *) b;
+    int prefer_a = prefer_user (user_a), prefer_b = prefer_user (user_b);
+    if (prefer_a != prefer_b)
+      return prefer_a - prefer_b;
     return g_strcmp0 (common_user_get_display_name (user_a), common_user_get_display_name (user_b));
 }
 
@@ -335,6 +355,12 @@
     hidden_shells = g_strsplit (value, " ", -1);
     g_free (value);
 
+    value = g_key_file_get_string (config, "UserList", "preferred-users", NULL);
+    if (!value)
+        value = g_strdup ("");
+    preferred_users = g_strsplit (value, " ", -1);
+    g_free (value);
+
     g_key_file_free (config);
 
     setpwent ();
@@ -392,6 +418,8 @@
     }
     g_strfreev (hidden_users);
     g_strfreev (hidden_shells);
+    g_strfreev (preferred_users);
+    preferred_users = NULL;
 
     if (errno != 0)
         g_warning ("Failed to read password database: %s", strerror (errno));

=== modified file 'data/users.conf'
--- data/users.conf	2011-10-18 02:41:06 +0000
+++ data/users.conf	2014-04-30 07:41:38 +0000
@@ -7,8 +7,10 @@
 # minimum-uid = Minimum UID required to be shown in greeter
 # hidden-users = Users that are not shown to the user
 # hidden-shells = Shells that indicate a user cannot login
+# preferred-users = Users to be sorted to the top of the list
 #
 [UserAccounts]
 minimum-uid=500
 hidden-users=nobody nobody4 noaccess
 hidden-shells=/bin/false /usr/sbin/nologin
+preferred-users=