Created attachment 246576 [details] [review]
login: Show a friendly error message

Review of attachment 246576 [details] [review]:

so this patch throws away information. That may be okay if all the information is gobbledygook that can be condensed down to 'didn't work', but we should look.
The possible pam messages are:

_("Success");
_("Critical error - immediate abort");
_("Failed to load module");
_("Symbol not found");
_("Error in service module");
_("System error");
_("Memory buffer error");
_("Permission denied");
_("Authentication failure");
_("Insufficient credentials to access authentication data");
_("Authentication service cannot retrieve authentication info");
_("User not known to the underlying authentication module");
_("Have exhausted maximum number of retries for service");
_("Authentication token is no longer valid; new one required");
_("User account has expired");
_("Cannot make/remove an entry for the specified session");
_("Authentication service cannot retrieve user credentials");
_("User credentials expired");
_("Failure setting user credentials");
_("No module specific data is present");
_("Bad item passed to pam_*_item()");
_("Conversation error");
_("Authentication token manipulation error");
_("Authentication information cannot be recovered");
_("Authentication token lock busy");
_("Authentication token aging disabled");
_("Failed preliminary check by password service");
_("The return value should be ignored by PAM dispatch");
_("Module is unknown");
_("Authentication token expired");
_("Conversation is waiting for event");
_("Application needs to call libpam again");
_("Unknown PAM error");

::: js/gdm/util.js
@@ +373,3 @@
         if (serviceName != PASSWORD_SERVICE_NAME)
             return;
+        logError (problem);

logError doesn't work this way. It takes an exception and a message

So all of those messages are garbage and I think you're right to disregard the text.

Some of the error messages have error codes associated with them that GDM should consider passing up so gnome-shell can do a better job than "that didn't work".
 
Things like account expired, token expired, server down, etc

but we should definitely ignore the messages themselves. Also, getting those corner cases right is less important, in my opinion, than getting the main case right, so we can do that later.

We should probably extend the GDM api to convey when those things happen semantically, and then the shell can pick a message it wants.

looking into this more, this is really a GDM problem.

We do this:

        /* blocking call, does the actual conversation */
        error_code = pam_authenticate (worker->priv->pam_handle, authentication_flags);

        if (error_code == PAM_AUTHINFO_UNAVAIL) {
...
        } else if (error_code != PAM_SUCCESS) {
                g_debug ("GdmSessionWorker: authentication returned %d: %s", error_code, pam_strerror (worker->priv->pam_handle, error_code));

                /*
                 * Do not display a different message for user unknown versus
                 * a failed password for a valid user.
                 */
                if (error_code == PAM_USER_UNKNOWN) {
                        error_code = PAM_AUTH_ERR;
                }

                g_set_error (error,
                             GDM_SESSION_WORKER_ERROR,
                             GDM_SESSION_WORKER_ERROR_AUTHENTICATING,
                             "%s", pam_strerror (worker->priv->pam_handle, error_code));
                goto out;
        }

Basically, we're using pam_strerror and shouldn't be since it's output isn't great. It's better to fix this in GDM than in gnome-shell, I think.

if we went with the hammer above we'd miss out on messages like:

You are required to change your password immediately (when admin creates a user's password mode to 'set at next login' in control-center)

and

Your account has expired; please contact your system administrator (when password expires)

Sorry, passwords do not match (when a user is typing a new password for the second time, and it doesn't match what they typed the first time)

and probably other things if sssd is enabled.  We should probably vet all these messages and get them changed if they need to be made better.

(In reply to comment #3)
> So all of those messages are garbage and I think you're right to disregard the
> text.
> 
> Some of the error messages have error codes associated with them that GDM
> should consider passing up so gnome-shell can do a better job than "that didn't
> work".
> 
> Things like account expired, token expired, server down, etc

If you can give me a list of the cases where we need to provide an alternative error message, I would be happy to convert them into strings for the ui.

Created attachment 250470 [details] [review]
worker: don't use pam_strerror in user visible messages

pam_strerror doesn't return user friendly error strings,
so displaying them in the UI is wrong.

This commit strips pam_strerror from the UI, using instead
more friendly messages.

Attachment 250470 [details] pushed as bab7015 - worker: don't use pam_strerror in user visible messages