On Fri, Feb 27, 2015 at 04:28:39PM +0100, Sumit Bose wrote:
On Fri, Feb 27, 2015 at 03:17:41PM +0100, Pavel Reichl wrote:
>
> On 02/27/2015 12:01 PM, Sumit Bose wrote:
> >On Tue, Feb 24, 2015 at 04:44:28PM +0100, Pavel Reichl wrote:
> >>On 02/23/2015 11:47 AM, Sumit Bose wrote:
> >>>On Fri, Feb 20, 2015 at 02:44:45PM +0100, Pavel Reichl wrote:
> >>>>On 02/20/2015 02:33 PM, Lukas Slebodnik wrote:
> >>>>>On (20/02/15 14:23), Pavel Reichl wrote:
> >>>>>>On 02/19/2015 06:16 PM, Sumit Bose wrote:
> >>>>>>>On Tue, Feb 17, 2015 at 10:56:43PM +0100, Pavel Reichl
wrote:
> >>>>>>>>Hello,
> >>>>>>>>
> >>>>>>>>please see attached patches resolving
> >...
> >
> >>>>index
9f2e9ac34add13e40d316374094024afdcc4ae31..4e3f3510250b19b5f397125fa3e3a376e0d3701f 100644
> >>>>--- a/src/man/sssd-ldap.5.xml
> >>>>+++ b/src/man/sssd-ldap.5.xml
> >>>>@@ -1959,6 +1959,18 @@ ldap_access_filter = (employeeType=admin)
> >>>> ldap_account_expire_policy
> >>>> </para>
> >>>> <para>
> >>>>+
<emphasis>pwd_expire_policy</emphasis>:
> >>>>+ This option is useful if users are
interested in
> >>>>+ seeing password expiration warning when
authenticating
> >>>>+ using different method then passwords -
for example
> >>>>+ SSH keys.
> >>>It's not about seeing a warning but about denying access based on
an
> >>>expired password. I think you should be more clear here.
> >I'm sorry I wasn'T clear either. I meant to say 'it's not only
about a
> >warning'. So you should mention both, the user will see a warning if the
> >password is about to expire and will be rejected if the password is
> >expired as with password authentication
> >
> >>>>+ </para>
> >>>>+ <para>
> >>>>+ Please note that 'access_provider =
ldap' must
> >>>>+ be set for this feature to work. Also
'ldap_pwd_policy'
> >>>>+ must be set to appropriate password
policy.
> >>>>+ </para>
> >...
> >
> >>>>+ return ret;
> >>>I think you have to modify the return code here to match the access
> >>>control expectations. check_pwexpire_policy() will e.g. return
> >>>ERR_PASSWORD_EXPIRED but the access control code expects
> >>>ERR_ACCESS_DENIED. As a result I see the following in the logs:
> >>In attached patch I modified sdap_access_done() instead:
> >>
> >>static void sdap_access_done(struct tevent_req *req)
> >> case ERR_ACCOUNT_EXPIRED:
> >> pam_status = PAM_ACCT_EXPIRED;
> >> break;
> >>+ case ERR_PASSWORD_EXPIRED:
> >>+ pam_status = PAM_PERM_DENIED;
> >>+ break;
> >> default:
> >> DEBUG(SSSDBG_CRIT_FAILURE, "Error retrieving access check
> >>result.\n");
> >> pam_status = PAM_SYSTEM_ERR;
> >>
> >>If you are OK with this approach would setting pam_status to
> >>PAM_AUTHTOK_EXPIRED be more appropriate?
> >That's a good point and it made me re-read to original tickets again. I
> >was under the impression that they ask for rejecting access as well, but
> >it looks thy only ask for a warning. Nevertheless I think it is a good
> >idea to reject access and it relates a bit to #2534 as well.
> >Nevertheless even if we reject your comment above made me think who we
> >should react is the password is expired, just reject or ask for a new
> >password as it would have been with password authentication.
> >
> >As a result I wonder if it would make sense to add 3 options:
> >
> >- pwd_expire_policy_reject: will warn and reject,
> > pam_status = PAM_PERM_DENIED
> >- pwd_expire_policy_warn: will only warn,
> > pam_status = PAM_SUCCESS
> >- pwd_expire_policy_renew: will warn and ask for new password
> > pam_status = PAM_NEW_AUTHTOK_REQD
> >
> >What do you think?
> >
> >bye,
> >Sumit
> I think you are right, I just wonder if it would be better to create one
> option
>
> pwd_expire_policy_action which would have a value from {reject, warn, renew}
>
> Default value would be reject?
>
But this would mean to add a new option for a quite limited use-case.
Jakub, do you have a preference?
I fell into the same trap as Pavel :-) I didn't like adding a new
option, but new values for existing options sound good to me.