GNOME Bugzilla – Bug 557553
choose which users are visable
Last modified: 2010-07-02 19:12:44 UTC
Hi, I asked on gdm mailing list how to choose which users are visable and I got an answer that in versions after 2.20 there is no such option. So please make this option available again. Thank you.
just to add that gdm 2.20 and previous ones had option that could be set in /etc/gdm/custom.conf: IncludeAll=false Include=user1,user2,etc... not that functionality is not there any more.
So GDM gets its list of users dynamically based on two things: 1) users who log in frequently (see ConsoleKit's ck-history --frequent) 2) local users Would having some way to disable 2) be sufficient for what you want?
It should be possible to bring back to old config option for doing this, but can I ask first what your use case for this feature is ? Why isn't the current dynamic/heuristic approach good enough ?
I am not the reporter but, in my case, I use this because one of the systems I administrate has a lot of users (22) that don't login never in graphical mode, while only 7 users use gdm for loggining in
Pacho, what does ck-history --frequent say in your case?
It doesn't output anything, but. maybe, because I don't launch its service by default :-(
Hi everybody, I sent out an mail on the mailing list about this and was directed to this report, anyhow, this is my suggestion regarding this subject. 1. Users that logs in ONLY through gdm gets added to the "user list at gdm-login", NOT the users that logs in through ssh or something else. 2. Those users that are visible in the "user list at gdm-login" should be handled in that way a sysadmin easily could remove himself ( if he for some reason logs in through the gdm ), or other users that for different reasons have been logging in through gdm. Thanks for a great job guys ! Best regards, Patrik Martinsson, Sweden.
Hi, I reported this bug and the issue I reported is the same as in Comment #4. Cheers.
I have one request. I would like to have an option to display only one user on gdm screen, even if there are more users that login through gdm from time to time.
I am wondering if this discussion is still active. We have about 1000 student accounts on our machines but very few actually log in so I would be very interested in being able to disable the user list again. I am wondering if this request is still alive?
<metoo/> I've just updated to Fedora 10 which has the new gdm (from F8 which didn't). I have a small no. of not-quite system users (e.g., spam recipient) who never log in but who show up on gdm (not ck-history --frequent; in factm, this gives the limited list I'd like to see), and another batch who only log in remotely & don't need faces.
Is this on anybody's radar? I guess the best thing to do right now is not to use GDM and use KDM from KDE4 because it is in much better shape, has more features and has a graphical editor tool.
Note the userlist can get disabled via GConf, e.g., gconftool-2 --direct --config-source xml:readwrite:/etc/gconf/gconf.xml.defaults --type bool --set /apps/gdm/simple-greeter/disable_user_list true This isn't documented in the user manual but should be. I'll open a separate bug report for that.
see bug 588804 for the documentation issue mentioned in comment 13
Created attachment 142182 [details] [review] patch fixing issue This patch adds back the include, include_all, and exclude options to the new GDM. Can this go upstream?
Created attachment 142183 [details] [review] Docs for this change patch for the docs for this change.
seems okay.
well actually: + for (pwent = fgetpwent (fp); (pwent != NULL && manager->priv->include_all == TRUE); pwent = fgetpwent (fp)) { That's a little sloppy. Shouldn't check include_all every itereation of the loop.
Created attachment 142839 [details] [review] updated patch This patch moves the for-loop into an if-test so we only check it once. Can this go upstream now that this is fixed?
I committed the latest patch upstream to master. I didn't notice at first that you marked this accepted-commit-now, but I still fixed the minor issue that you raised.
I backed out my fix for this bug from master. It broke string freeze, and I think there needs to be more discussion about whether the fix in bug #593996 is a better approach.
Created attachment 146604 [details] [review] Updated patch This updated patch is similar to the last patch except that it makes the client programs (like the login GUI) use the custom.conf configuration to set the Include, Exclude, and IncludeAll options. This makes the patch backwards compatible with the old GDM, so it uses the old configuration options. I've tested and it works well. Can this version of the patch go upstream?
Please sent this patch upstream, there are lots of users waiting for this. Thank you Brian very, very much for writing this patch.
Review of attachment 146604 [details] [review]: Getting pretty close. ::: gdm-2.28.1/data/gdm.schemas.in.in-orig @@ +591,3 @@ + return g_strcmp0 ((char *) a, + (char *) b); +} why don't you jsut pass g_strcmp0 for the custom find func? Also, note the missing braces. @@ +594,3 @@ + +static gboolean +check_excludes (GdmUserManager *manager, const char *user) maybe this should be called user_in_exclude_list ? Also, parameters need to be on their own lines @@ +603,3 @@ + /* always exclude the "gdm" user. */ + if (user == NULL || (strcmp (user, GDM_USERNAME) == 0)) + return TRUE; missing braces @@ +610,3 @@ + match_name_cmpfunc); + if (found != NULL) + ret = TRUE; missing braces @@ +1386,3 @@ + return; + } +} is add_included_user missing some code? @@ +1422,3 @@ + if (manager->priv->include_all != TRUE) { + g_debug ("GdmUserManager: include_all is FALSE"); + } else { instead of moving all this code over an indention level, maybe just if (!manager->priv->include_all) { return ; } Or, just don't call reload_passwd if include_all is false. @@ +1681,3 @@ + + g_strfreev (temparr); +} Should the be moved to the gdm-settings api?
Created attachment 147326 [details] [review] updated patch Updated patch.
Here is an updated patch that resolves the issues raised. Note that Ray and I had a discussion about this patch on IRC, and I'll explain what was discussed. > ::: gdm-2.28.1/data/gdm.schemas.in.in-orig > why don't you jsut pass g_strcmp0 for the custom find func? Also, note the > missing braces. Note that the file you mean is gui/simple-greeter/gdm-user-manager.c in the function match_name_cmpfunc. Note there are other functions in this file (match_real_name_cmpfunc and match_real_name_hrfunc) that also use g_strcmp0, so I am just being consistent with the other functions here. Ray agreed it was okay to leave this. >+static gboolean >+check_excludes (GdmUserManager *manager, const char *user) > > maybe this should be called > user_in_exclude_list ? Changed. > Also, parameters need to be on their own lines Fixed. I also fixed the "missing braces" issues mentioned. > @@ +1386,3 @@ > + return; > + } > +} > > is add_included_user missing some code? No, note that the call to gdm_user_manager_get_user adds the user if it is valid and not already in the hash. I added a comment to make this more clear. > @@ +1422,3 @@ > + if (manager->priv->include_all != TRUE) { > + g_debug ("GdmUserManager: include_all is FALSE"); > + } else { > > instead of moving all this code over an indention level, maybe just if > (!manager->priv->include_all) { return ; } Or, just don't call reload_passwd > if include_all is false. No, note there is code at the end of the function that still needs to be called even if include_all is FALSE. After looking at this more closely, Ray agreed with the above, but he felt that looping over and adding the users specified by greeter/Include was a bit weird for the reload_passwd function since included users do not come from the passwd file. So, this logic was moved to a separate function "add_included_users" which is called after reload_passwd is called. > @@ +1681,3 @@ > + > + g_strfreev (temparr); > +} > > Should the be moved to the gdm-settings api? I did not feel it made sense to add the infrastructure to do this when this is only used by the Include/Exclude options in this single file. Looking at the old GDM code, these are the only options which use string lists (aside from daemon/ConsoleCannotHandle which is unlikely to be ported to the new GDM), so such infrastructure probably will not ever be needed. However, if more configuration options need to use string lists in the future, then it would make sense to add more infrastructure to support this more generally. Also, it isn't clear how to best add this infrastructure. Should it be managed at the GConf level, or at a higher level (as it is currently)? If more options start using this feature, then we can consider how to best add such infrastructure at that time. So, I added a comment to highlight this.
Review of attachment 147326 [details] [review]: Looking good. Feel free to commit after the remaining niggles are ironed out. ::: gdm-2.28.1/common/gdm-settings-keys.h-orig @@ +1443,3 @@ + } else { + g_debug ("GdmUserManager: include_all is TRUE"); + should be able to avoid the extra indentation now, right? @@ +1677,3 @@ +gdm_set_string_list (char *value, GSList **retval) +{ + char **temparr; I would probably call this array or temp_array. variable names aren't normally squeezed together like that. @@ +1689,3 @@ + + temparr = g_strsplit (value, ",", 0); + for (i=0 ; temparr != NULL && temparr[i] != NULL ; i++) { spacing's wrong here, there's extra check per iteration we don't need either, should be: for (i = 0; temparr[i] != NULL; i++) { @@ +1714,3 @@ + res = gdm_settings_client_get_string (GDM_KEY_INCLUDE, + &temp); + gdm_set_string_list (temp, &(manager->priv->include)); weird extra parentheses here @@ +1722,3 @@ + + res = gdm_settings_client_get_boolean (GDM_KEY_INCLUDE_ALL, + &(manager->priv->include_all)); and here.
Created attachment 147942 [details] [review] updated patch which resolves Ray's concerns Ray approved committing this version of the patch into master. This resolves the issues he raised.
*** Bug 602641 has been marked as a duplicate of this bug. ***
*** Bug 568326 has been marked as a duplicate of this bug. ***
Are these options available only for 2.30.x and newer gdm only? http://library.gnome.org/admin/gdm/2.30/configuration.html.en Will these patches be applied to 2.24 and 2.28?