I believe that setting autologin and timedlogin users to a script is a feature that Red Hat adds to GDM, and not a part of the GDM code in CVS.  You may need to work with RedHat to get a resolution to your problem.

I think timed and automatic logins are a native feature in GDM for two reasons.  First, the features are documented on the GNOME GDM web site <http://www.gnome.org/projects/gdm/docs/2.14/configuration.html>.  Second, I don't see any related patches in the Fedora GDM source RPM <http://sunsite.mff.cuni.cz/pub/fedora/5/source/SRPMS/gdm-2.14.0-1.src.rpm>.

You are right, sorry.  This is in the gdm_parse_enriched_login function in daemon/slave.c.  Are you using gdmlogin or gdmgreeter as your login program.  If you are using a different theme than ships with GDM, could you attach the theme's xml file, so I can look at it?


Note that there was a change in GDM on 12/24/2005 to fix gdmlogin and gdmgreeter so they use the same character sequences.  Previously it used %d and %s differently.  Now it uses %u and %t.  It seems for some reason your code is using the old values.  But the the string is hardcoded into gui/gdmlogin.c and gui/greeter/greeter_parser.c (look for "User %u will login in %t"), so you shouldn't see this problem unless the theme file somehow is redefining timed-label to a different string. 

Ubuntu bug about that: https://launchpad.net/distros/ubuntu/+source/gdm/+bug/39869

"...
When you start the computer, you see the gdm log-in screen and you can read:

"User linux log-in after 5 seconds". (Or similar, I have a german Ubuntu).

But the right way is (and I think under Breezy it was) "User xyz log-in after 5 seconds".

There should be the right username xyz displayed in the gdm log-in screen instead of "linux"
..."

Yes, Sebastian.  Of course you are right, it needs to be fixed.  I'm not sure where the bug is coming from, though.  As I mention above, the string for timed login message did change the %d/%s to %u/%t, which would cause this problem.

However, normally these strings are hardcoded in gdmlogin.c and gui/greeter/greeter_parser.c and I would think you would only see this problem
if you were somehow using an old GUI program with a new daemon (which I don't think is allowed), or if the theme XML file uses its own label rather than the stock label, which would be weird.  

However, if logging in remotely via XDMCP, a mismatch of an old slave with a new daemon might be allowed and cause this problem.  Is this what is happening?

Brian

Created attachment 64818 [details]
my GDM theme--worked in GNOME 2.10 and Fedora Core 4

Running "ps -ef | grep gdm" shows gdmgreeter running (and not gdmlogin).

The problem is in your XML file.  Search for timed-label in there and notice that the <text> tags are redefining the label.  Change the "%d" and "%s" in those strings to "%u" and "%t", and the problem should go away.

This is a bug with the translations.  I think your distro isn't building this file properly.

Could that bug happen on upgrade maybe (running the previous gdm and using the new gdmflexiserver)? Anyway that seems to work fine for me too now. When I noticed it that was using gdmflexiserver and it's possible I didn't change the version installed without restarting gdm

>The problem is in your XML file.  Search for timed-label in there and notice
>that the <text> tags are redefining the label.  Change the "%d" and "%s" in
>those strings to "%u" and "%t", and the problem should go away.

The text is still not right.  With the setting of  <stock type="timed-label"/>, the display reads

    "User /usr/local/bin/autologin.sh| will login in 45 seconds."

But GDM knows better: when the countdown finishes, it asks me for the password for the user foouser123 (the correct username).

If I change the .xml so that the text is %u and %t, it's the same results as using a stock label.

I'm on GDM 2.14.8.  (The 2.14.9 seems to fix different problems, but I'll try 2.14.9 next week.)

>Could that bug happen on upgrade maybe (running the previous gdm and using the
>new gdmflexiserver)? 

This system automatically reboot following updates.

Could you attach the xml file you are using so I can verify it is correct?

If you remove all the tags with text like "<text xml:lang=..." does it 
still break?  I suspect that is where the problem lies.

Yes, and 2.14.9 should fix the problem with it asking for password when it
should not.

Brian

Created attachment 68036 [details]
gdm theme .xml

In this .xml I am trying with GDM 2.14.9, there is only a stock label and no hard-coded <text> for the timed login section.

One problem is that /usr/local/bin/autologin.sh| is always displayed for the username.  

The second problem is that if the script returns an empty string, the timed login behavior is still performed (inconsitent with GDM 2.12 actual behavior and with GDM 2.14 documentation).

The %u value should get replaced by whatever is stored in the GDM_KEY_TIMED_LOGIN
configuration parameter.

What does running "gdmflexiserver --command="GET_CONFIG daemon/TimedLogin"
return? 

I suspect it returns the same value.  Note that AutomaticLogin and TimeLogin value can be set to a script as described in the manual:

                Alternatively, the name may end with a vertical bar |, the
                pipe symbol.  The name is then used as a application to execute
                which returns the desired username on standard output. If an
                empty or otherwise invalid username is returned, automatic
                login is not performed. This feature is typically used when
                several remote displays are used as internet kiosks, with a
                specific user to automatically login for each display.

So the code in gdm_common_expand_text in gui/gdmcommon.c probably needs to
be enhanced to execute the script and display the value instead of just printing the name of the script used to get the username.

I'm guessing this is really the bug

Created attachment 68052 [details] [review]
patch to fix problem


Okay, looking into this problem a bit more, I think I understand the problem.  Does this patch fix the problem?

$ gdmflexiserver --command="GET_CONFIG daemon/TimedLogin"
OK /usr/local/bin/autologin.sh|

>Okay, looking into this problem a bit more, I think I understand the problem. 
>Does this patch fix the problem?

I want to test the patch, but it may not be very soon.

Using the patch (id=68052) on gdm 2.14.9 did not help much.  When the timed login script returns an empty string, almost everything is the same:

- GDM shows "User  will login in 45 seconds."  (Before, the script filename followed "User".)
- GDM counts down to 0.  Then, authentication fails.

I don't know GDM or its source very well, but the problem may be in the case statement GDM_STARTTIMER in gui/gdmlogin.c:1955.  As far as I can tell, the code always starts the timer for my configuration.  However, I think the condition needs to consider the *output of the timed login script* as is done in gdm_parse_enriched_login() in daemon/slave.c.

The login script should return a valid username.  If it is returning a blank string, then the behavior of it displaying "User  will login..." and failing
to authenticate would be expected.  Before it was printing out the value of the key, which would have been the scriopt filename.

Note that the patch is now displaying the value of the GDM_TIMED_LOGIN_OK environment variable.  Note in the function gdm_slave_greeter () it sets this to whatever username the daemon thinks is the timed login user.  So I would be surprised if it is set to a different value.

Note, I think that the patch is the correct fix.  So I have applied this to the source code.  But I'll wait for verification before I mark this bug as fixed.  Especially since you are suggesting you are still seeing the problem.

Make sure the script returns a valid username, and if it is - but things are still not working, then we'll have to look more into this.

>The login script should return a valid username. 

The GDM documentation states (quoted in comment #12) "If an empty or otherwise invalid username is returned, automatic login is not performed."  That exception is useful, for example, for our terminal server where the learning center terminals logon automatically (timed login) but staff terminals require manual logon.

By the way, an easy way to test the exception may be with:
    TimedLogin=/bin/true|

Since the problem is not just the message, but the whole timed login behavior should not happen in certain cases (according to document, quoted in last comment)...

Changing summary: 
"Timed Login message broken when daemon/TimedLogin is set to a script that returns the userid via the pipe syntax" --> 
"Timed Login behavior broken when daemon/TimedLogin is set to a script that returns the userid via the pipe syntax" --

Created attachment 71113 [details] [review]
patch that begins to fix problem (but doesn't compile)

This patch against 2.14.9 turns off the timed login behavior when the parsed username is blank.  However, the patch doesn't compile. I added a few "fixme" comments where I was not sure the best way to proceed.  Since you are much more familiar with GDM, perhaps you could finish it up?  

This patch obsoletes patch dated 2006-06-26.

Andrew, I'm not exactly sure I understand what problem you are trying to fix here. 
What specific problem are you seeing when the value returned from the script is
not valid?

The fix that I made should work in the situation where the login is valid.  The daemon sets the GDM_TIMED_LOGIN_OK environment variable, and the GUI program uses this value for display.  This should work.

If you are saying that there is a problem where if the value returned by the pipe is not a valid user, then perhaps the daemon code should be fixed to check the value returned and take appropriate action if the user is invalid.  For example, not set GDM_TIMED_LOGIN_OK and turn off the timed login configuration setting so the feature is turned off even though the config file says it should be on.

Looking at your patch, I don't think it is workable the way you are trying to fix the problem.  The GUI program does not have access to the GdmDisplay structure.  I suppose we could add some protocol to allow the daemon/slave to talk to each other to get/set needed information, but I don't think this is the right approach.

Note the gdm_is_user_valid () function in gui/gdmuser.c that shows how to
validate a user.  Probably can use similar getpwnam () calls to validate the
user.

>What specific problem are you seeing when the value returned from the script is
>not valid?

The timed login system is still on, but it should be off.  In other words, I see the timed login message (with the wrong username).  It counts down, and then it tries to log in.  Since the user name is not valid, the timed login fails.  Then, it repeats itself over and over again.

Some users may be confused because they expect their terminals not to have timed login behavior.

My /var/log/messages log file looks like this, and the messages continue every X seconds.
Sep 20 16:16:52 fedora5 gdm[2360]: Couldn't authenticate user
Sep 20 16:16:57 fedora5 gdm[15379]: Couldn't authenticate user
Sep 20 16:17:38 fedora5 gdm[2360]: Couldn't authenticate user
Sep 20 16:17:44 fedora5 gdm[15379]: Couldn't authenticate user

You should be able to easily reproduce this behavior by setting TimedLogin=/bin/true or creating a script which returns an invalid username.

>If you are saying that there is a problem where if the value returned by the
>pipe is not a valid user, then perhaps the daemon code should be fixed to check
>the value returned and take appropriate action if the user is invalid.  For
>example, not set GDM_TIMED_LOGIN_OK and turn off the timed login configuration
>setting so the feature is turned off even though the config file says it should
>be on.

I tried two different patches today, and neither worked.  Maybe you could more quickly fix the problem.

I'm confused.  Why do you say that "it is still on but should be off".
Could you run these commands:

gdmflexiserver --command="GET_CONFIG daemon/TimedLoginEnable"
gdmflexiserver --command="GET_CONFIG daemon/TimedLogin"

If the first responds true, and the second has a valid command, then GDM would think that you want this feature to be on.  Does GDM need to protect users from using a script that doesn't return valid usernames?

If you want to fix the code so that it doesn't turn on TimedLogin if the username isn't valid, then I'd copy the code from gui/gdmuser.c gdm_is_user_valid() function into daemon/slave.c and then in gdm_parse_enriched_login I would 
check if the user is valid.  If not, I would call gdm_set_value_bool (GDM_KEY_TIMED_LOGIN_ENABLE, FALSE) to tell the config system that this feature is turned off.

Created attachment 73154 [details] [review]
unsuccesful patch to turn off timed login when script returns invalid username

$ gdmflexiserver --command="GET_CONFIG daemon/TimedLoginEnable"
OK true
$ gdmflexiserver --command="GET_CONFIG daemon/TimedLogin"
OK /usr/local/bin/autologin.sh|

>Does GDM need to protect users from using a script that doesn't return valid 
>usernames?

It's not really "protection" but a useful way of managing timed login (which is currently documented and worked in GDM 2.12).  We have one terminal server with 20 terminals connected to it, and based on the host name, the script decides how to use timed login.  About half of those are in a "public" computer lab where we use timed login for guests.  The other half of terminals are for staff, so the script returns nothing to indicate those should not get timed login.  (The guest accounts have access to no special information, but the staff accounts must be secured with a password.)  

Or maybe more simply:
Hostname LAB001 gets username GUEST001
Hostname LAB002 gets username GUEST002
Hostname STAFF001 should have no timed login
Hostname STAFF002 should have no timed login

>If not, I would call gdm_set_value_bool (GDM_KEY_TIMED_LOGIN_ENABLE, FALSE) to 
>tell the config system that this feature is turned off.

Sorry, I am not having success, and I would appreciate help.  I added a gdm_debug() to confirm the code is being called, so my best guess now is that the GDM_KEY_TIMED_LOGIN_ENABLE is not communicated to the gui/gdmlogin.c where the timed login timer is started?

Also, this patch incorporates your patch to fix the displayed username, but the displayed username is still the script like "User /bin/true| will login in 30 seconds."

Regarding why you are still seeing the "/bin/true|" in the GUI program.  This is odd.  Can you verify that your code is setting GDM_TIMED_LOGIN_OK environment variable in the gdm_slave_greeter function (daemon/slave.c)?  Also verify that in gui/gdmcommon.c in the function gdm_common_expand_text that the code is using this environment variable to set the string in case 'u'?  If so, then I can't see how you would see this problem unless GDM_TIMED_LOGIN_OK is being filled in with the unexpanded text (which would be odd).  Can you look into this and see why this is failing?  You say "this patch incorporates your patch to fix the displayed username", but I don't see any GDM_TIMED_LOGIN_OK code in there. 

Regarding why our test in daemon/slave.c isn't actually working to turn off
TimedLogin.  I assume that even after you use your patch, you see the debug
message saying that the slave code is about to turn off the flag, but when
you run gdmflexiserver --command="GET_CONFIG daemon/TimedLoginEnable" it
still says "true". 

I suspect the problem is that GDM works like this.  The main GDM daemon forks off the slave daemon for the display.  THe code in daemon/slave.c corresponds to the forked slave daemon.  Then the forked slave daemon launches the GUI program (gdmgreeter/gdmlogin).  The GUI program then talks to the main daemon to get the config values using the gdmflexiserver -command argument.

So, we just might not be able to change this key in the slave daemon process.  This is unfortanate because we really need to call gdm_parse_enriched_login in the slave in order to generate a different user for each display.  If we read
this just once in the main daemon, it would be the same user for all displays.

So, thinking about this a bit more, I think the best way to make it work would
be to modify the slave to *not* set the GDM_TIMED_LOGIN_OK env-var when the username is invalid, and to modify gdmlogin/gdmgreeter to just assume that
TimedLogin is off if the environment variable is not set.  This would probably work better.

Created attachment 73444 [details] [review]
working patch

Please accept this patch which should solve the TimedLogin script problem in gdmgreeter and gdmlogin.  I tested various scenarios such as valid user name, invalid user name, blank script output, and root user.  I also tested TimedLogin without a script.

This patch does not address the analogous automatic login problem.

Also, this patch shows a way you could simplify gdm_is_user_valid().

Thanks, the patch is well done.  I applied to CVS head, and also fixed the gdm_is_user_valid in gui/gdmuser.c so it matches your implementation in daemon/slave.c.   Thanks.

Would you be willing to fix AutomaticLogin also?

Just backported patch to 2.16.  Didn't make it in time for 2.16.1, but will make 2.16.2.

>Would you be willing to fix AutomaticLogin also?

I am willing, but it may not be soon.  :)

Thanks.  I look forward to your help.

Two things.
1. It seem this patch was not applied to 2.14 branch.  Could you do it in case there may be a release forced by a security update?  GDM 2.14.11 came out in December, and the new Fedora package un-fixed the bug on my system.  :)
2. I don't expect to have time to fix AutomaticLogin in the foreseeable future.  Sorry.

I do not have any plans to do additional 2.14 releases unless there are security issues that force another release.

If you want to apply this patch to 2.14.11 and let me know if applies cleanly and works okay, then I will go ahead and commit it to the 2.14 branch so that it will be present in any future 2.14 releases.  If the patch needs any rework to apply to 2.14.11, then please update with a new attachment.

Nevermind about 2.14.  The patch does not cleanly apply to 2.14 anymore, but I use GDM 2.16 now.  :)

Closing, since the reporter no longer believes backporting to 2.14 is necessary.