On (05/02/16 15:10), Jakub Hrozek wrote:
On Fri, Feb 05, 2016 at 02:54:53PM +0100, Lukas Slebodnik wrote:
> On (05/02/16 13:55), Pavel Reichl wrote:
> >
> >
> >On 02/05/2016 11:01 AM, Jakub Hrozek wrote:
> >>On Tue, Feb 02, 2016 at 08:48:43PM +0100, Pavel Reichl wrote:
> >...
> >>
> >>I would prefer to split this patch into two, one that patches the LDAP
> >>code to return ERR_ACCOUNT_LOCKED and one that passes on and displays
> >>the message.
> >
> >Done.
> >>
> >>> From 511ef599902827d76193a1e634ace193df15dead Mon Sep 17 00:00:00 2001
> >>>From: Pavel Reichl <preichl(a)redhat.com>
> >>>Date: Tue, 2 Feb 2016 14:35:15 -0500
> >>>Subject: [PATCH] PAM: Notify user of denial due to AD account lockout
> >>>
> >>>Resolves:
> >>>https://fedorahosted.org/sssd/ticket/2839
> >>>---
> >>>index
2d9b1184f5d30b9df7f1d3e4b980a7e0107c6830..763c5ed050bd482d334ad617349938dfc89f79da 100644
> >>>--- a/src/providers/ldap/sdap_async_connection.c
> >>>+++ b/src/providers/ldap/sdap_async_connection.c
> >>>@@ -754,6 +754,9 @@ static void simple_bind_done(struct sdap_op *op,
> >>>
> >>> if (result == LDAP_SUCCESS) {
> >>> ret = EOK;
> >>>+ } else if (result == LDAP_INVALID_CREDENTIALS
> >>>+ && strstr(errmsg, "data 775,") !=
NULL) {
> >> ~~~~~~~~~~~~~~
> >>I don't think this is safe, strstr() doesn't handle NULL input well.
> >>Please add a check for "&& errmgs != NULL" before calling
strstr.
> >Sure.
> >
> >>
> >>Otherwise the patch looks good, we just need to also ask some Native
> >>speaker for manpage comments..
> >
> >I pinged Dan about that.
> >
> >
> >Please see updated patch set, thanks!
>
> >From 4adb25b045be46dc0a178748e2b0aeb5ea09ed1d Mon Sep 17 00:00:00 2001
> >From: Pavel Reichl <preichl(a)redhat.com>
> >Date: Fri, 5 Feb 2016 07:27:38 -0500
> >Subject: [PATCH 1/2] SDAP: Add return code ERR_ACCOUNT_LOCKED
> >
> >Add code to distinquish state when account is locked in Active
> >Directory server.
> >
> >Resolves:
> >https://fedorahosted.org/sssd/ticket/2839
> >---
> > src/providers/data_provider.h | 2 ++
> > src/providers/ldap/ldap_auth.c | 4 ++++
> > src/providers/ldap/sdap_async_connection.c | 3 +++
> > 3 files changed, 9 insertions(+)
> >
> >diff --git a/src/providers/data_provider.h b/src/providers/data_provider.h
> >index
39051b90c3aad96f62dcbb86a20bcfd8c954879b..7332b677d19f70f4736e4d0b68d55cdd3c67a4af 100644
> >--- a/src/providers/data_provider.h
> >+++ b/src/providers/data_provider.h
> >@@ -182,6 +182,8 @@ struct pam_data {
> > bool offline_auth;
> > bool last_auth_saved;
> > int priv;
> >+ int account_locked;
> >+
> > #ifdef USE_KEYRING
> > key_serial_t key_serial;
> > #endif
> >diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c
> >index
c94ba15bb17aa1641eb36781cc59ce158d48ca66..8d6a37b2ceb3347cb8092858889d07e5615e5c77 100644
> >--- a/src/providers/ldap/ldap_auth.c
> >+++ b/src/providers/ldap/ldap_auth.c
> >@@ -1302,6 +1302,10 @@ static void sdap_pam_auth_done(struct tevent_req *req)
> > case ERR_PASSWORD_EXPIRED:
> > state->pd->pam_status = PAM_NEW_AUTHTOK_REQD;
> > break;
> >+ case ERR_ACCOUNT_LOCKED:
> >+ state->pd->account_locked = true;
> >+ state->pd->pam_status = PAM_PERM_DENIED;
> >+ break;
> > default:
> > state->pd->pam_status = PAM_SYSTEM_ERR;
> > dp_err = DP_ERR_FATAL;
> >diff --git a/src/providers/ldap/sdap_async_connection.c
b/src/providers/ldap/sdap_async_connection.c
> >index
2d9b1184f5d30b9df7f1d3e4b980a7e0107c6830..c1735513ff6dcc755daf06cb97da546eaded7eb9 100644
> >--- a/src/providers/ldap/sdap_async_connection.c
> >+++ b/src/providers/ldap/sdap_async_connection.c
> >@@ -754,6 +754,9 @@ static void simple_bind_done(struct sdap_op *op,
> >
> > if (result == LDAP_SUCCESS) {
> > ret = EOK;
> >+ } else if (result == LDAP_INVALID_CREDENTIALS
> >+ && errmsg != NULL && strstr(errmsg,
"data 775,") != NULL) {
> This searching of string seems to be vague to me.
> Will it work with any version of AD?
If not, we just don't display a message..I mean, this is already an
error path.
The ticket is about "SSSD should be about to display message to the user when
the account in Active Directory is 'locked out'"
If the string is not standardized among AD versions
than this ticket is NOT solved.
> Could you add link to msdn documentation where it is described
that this string
> is guaranted to be returned in such case?
It's not MSDN, but:
http://www-01.ibm.com/support/docview.wss?uid=swg21290631 I would prefer
official documantation (msdn) in code ar at least in
commit message.
LS