On Wed, Feb 11, 2015 at 04:56:48PM +0100, Pavel Reichl wrote: > Hello, > > please see attached patch. I'm not sure whether using pam_strerror() is the > right thing to do. It might be better to use our own string? > I'm also not sure about using _(STRING) macro on the output of > pam_strerror().
The _() macro will not work here. You can use it only to enclose literal strings. The strings will then be extracted into the *.pot file and translators can pick them for translation. In the pam_strerror() case libpam has to take care of the translations. > > I attached output of sequence of commands to show differences. > > 1) This is output without patch being applied. > $ su john > Password: > su: User account has expired > > ssh -l john `hostname` > Connection closed by 192.168.122.166 > > #not matching key > $ ssh -l john `hostname` -i /tmp/local > j...@dev.local.test's password: > Connection closed by 192.168.122.166 > > > 2) This is output when patch is applied. Please note the duplicity when > using su. The service name is available, so you can add this only if ssh is used. > $ su john > Password: > User account has expired > su: User account has expired > > $ ssh -l john `hostname` > User account has expired > Connection closed by 192.168.122.166 > > #not matching key > $ ssh -l john `hostname` -i /tmp/local > j...@dev.local.test's password: > User account has expired > Connection closed by 192.168.122.166 > > Thanks for comments. > From 953f1721996e6c2bf8ee53ea232de2240f168d94 Mon Sep 17 00:00:00 2001 > From: Pavel Reichl <prei...@redhat.com> > Date: Wed, 11 Feb 2015 19:38:16 -0500 > Subject: [PATCH] PAM: do not reject abruptly > > If account has expired use pam_conversation to pass message. > > Resolves: > https://fedorahosted.org/sssd/ticket/2050 > --- > src/sss_client/pam_sss.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c > index > fdf6c9e6da75c9f7eaa7c00d9a5792fbdd97eabc..767b2a839e9f001be52c5ff4c7651b0f06ba4221 > 100644 > --- a/src/sss_client/pam_sss.c > +++ b/src/sss_client/pam_sss.c > @@ -1585,6 +1585,13 @@ static int pam_sss(enum sss_cli_command task, > pam_handle_t *pamh, > D(("do_pam_conversation failed.")); > } > pam_status = PAM_NEW_AUTHTOK_REQD; > + } else if (pam_status == PAM_ACCT_EXPIRED) { > + ret = do_pam_conversation(pamh, PAM_TEXT_INFO, > + _(pam_strerror(pamh, pam_status)), > + NULL, NULL); > + if (ret != PAM_SUCCESS) { > + D(("do_pam_conversation failed.")); > + } I would recommend to not do this in pam_sss directly but send a SSS_PAM_USER_INFO response back to pam_sss. This response can e.g. be generated in the pam responder if pam_status == PAM_ACCT_EXPIRED and the service is sshd. Doing it in the pam responder has the advantage that you do not have to duplicate code in the backends. Additionally it is more easy configure the behavior. E.g. you can check pam_verbosity and only add this message is the level is 2 (1?) or higher. bye, Sumit > } > break; > case SSS_PAM_CHAUTHTOK: > -- > 2.1.0 > > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
