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

Proposed by Martin von Gagern on 2014-04-30
Status: Needs review
Proposed branch: lp:~gagern/lightdm/preferred-users
Merge into: lp:lightdm
Diff against target: 73 lines (+31/-1)
2 files modified
common/user-list.c (+29/-1)
data/users.conf (+2/-0)
To merge this branch: bzr merge lp:~gagern/lightdm/preferred-users
Reviewer Review Type Date Requested Status
Robert Ancell 2014-04-30 Needs Information on 2014-05-08
Review via email: mp+217720@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Robert Ancell (robert-ancell) wrote :

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) wrote :

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) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'common/user-list.c'
2--- common/user-list.c 2014-03-06 02:40:18 +0000
3+++ common/user-list.c 2014-04-30 07:41:38 +0000
4@@ -222,11 +222,31 @@
5
6 return NULL;
7 }
8-
9+
10+static gchar **preferred_users = NULL;
11+
12+static int
13+prefer_user (CommonUser *user)
14+{
15+ int i;
16+ const gchar *name;
17+
18+ if (preferred_users == NULL)
19+ return 0;
20+ name = common_user_get_name (user);
21+ for (i = 0; preferred_users[i]; i++)
22+ if (strcmp (name, preferred_users[i]) == 0)
23+ return i;
24+ return i;
25+}
26+
27 static gint
28 compare_user (gconstpointer a, gconstpointer b)
29 {
30 CommonUser *user_a = (CommonUser *) a, *user_b = (CommonUser *) b;
31+ int prefer_a = prefer_user (user_a), prefer_b = prefer_user (user_b);
32+ if (prefer_a != prefer_b)
33+ return prefer_a - prefer_b;
34 return g_strcmp0 (common_user_get_display_name (user_a), common_user_get_display_name (user_b));
35 }
36
37@@ -335,6 +355,12 @@
38 hidden_shells = g_strsplit (value, " ", -1);
39 g_free (value);
40
41+ value = g_key_file_get_string (config, "UserList", "preferred-users", NULL);
42+ if (!value)
43+ value = g_strdup ("");
44+ preferred_users = g_strsplit (value, " ", -1);
45+ g_free (value);
46+
47 g_key_file_free (config);
48
49 setpwent ();
50@@ -392,6 +418,8 @@
51 }
52 g_strfreev (hidden_users);
53 g_strfreev (hidden_shells);
54+ g_strfreev (preferred_users);
55+ preferred_users = NULL;
56
57 if (errno != 0)
58 g_warning ("Failed to read password database: %s", strerror (errno));
59
60=== modified file 'data/users.conf'
61--- data/users.conf 2011-10-18 02:41:06 +0000
62+++ data/users.conf 2014-04-30 07:41:38 +0000
63@@ -7,8 +7,10 @@
64 # minimum-uid = Minimum UID required to be shown in greeter
65 # hidden-users = Users that are not shown to the user
66 # hidden-shells = Shells that indicate a user cannot login
67+# preferred-users = Users to be sorted to the top of the list
68 #
69 [UserAccounts]
70 minimum-uid=500
71 hidden-users=nobody nobody4 noaccess
72 hidden-shells=/bin/false /usr/sbin/nologin
73+preferred-users=

Subscribers

People subscribed via source and target branches