Re: [gdm-list] [PATCH] some pam related improvements

[Ludwig Nussel <ludwig nussel suse de>]

Brian Cameron wrote:
> Ludwig:
> > ------------------------------------------------------------------------
> > 
> > - don't make the start again button insensitive as the prompt for
> >   the username is not necessarily the first.
> 
> Why change the calls to set_sensitive to always be TRUE, why not just
> rip out all calls to set the sensitivity for this button?

The buttons are still set to insensitive if there is nothing to do
for the GUI, e.g. when the backend sleeps due to a failed
authentication.

> Also, might there be a better solution.  We want the "Start Again"
> button to be active at all times except when the GUI is prompting for
> the first item.  Perhaps trying to set button sensitivity in the
> current GDM_PROMPT, GDM_NOECHO locations isn't the right place.  So I
> ask if the right fix is to rip out setting sensitivity or to move it so
> it is setting it more intelligently?

How do you know it's the first item? There can be multiple pam
conversations that belong to one authentication run. Anyways, with
multiple pam prompts the button is also useful on the first screen
to clear all entries.

> I think this change also needs to be made to gdmgreeter.

I haven't looked too closely into gdmgreeter yet.

> > - don't tell backend that a user was selected in the face browser if
> >   the backend itself caused that to avoid aborting an ongoing
> >   authentication.
> 
> I assume this is no longer needed because we are now managing this
> in verify-pam.c?  Is this correct?  But what about if GDM is using
> verify-crypt.c or verify-shadow.c, don't we also need some similar
> sort of hack to make this work the same for crypt/shadow?

I don't know, someone would need to test it. I wonder whether it
would make sense to emulate the PAM api for those two authentication
methods instead of duplicating lots of gdm logic.

cu
Ludwig

-- 
 (o_   Ludwig Nussel
 //\   SUSE LINUX Products GmbH, Development
 V_/_  http://www.suse.de/