Bug 590062 - [RFE] Track last successful login
[RFE] Track last successful login
Status: CLOSED CURRENTRELEASE
Product: 389
Classification: Community
Component: Directory Server (Show other bugs)
1.2.7
All All
medium Severity medium
: ---
: ---
Assigned To: Rich Megginson
Viktor Ashirov
: FutureFeature, Triaged
: 580956 (view as bug list)
Depends On: 591313
Blocks: 525219 389_1.2.7
  Show dependency treegraph
 
Reported: 2010-05-07 11:52 EDT by Rich Megginson
Modified: 2015-12-07 12:13 EST (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of: 498097
Environment:
Last Closed: 2015-12-07 12:13:59 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
account policy plugin 1.0 (inactivity limits) (7.56 KB, application/postscript)
2010-05-11 16:47 EDT, Ulf Weltman
no flags Details
0001-add-the-account-policy-plugin-and-related-server-cod.patch (3.26 MB, patch)
2010-09-28 19:07 EDT, Rich Megginson
nhosoi: review+
Details | Diff
0002-fix-pblock-memory-leak.patch (2.70 KB, patch)
2010-09-29 10:47 EDT, Rich Megginson
nhosoi: review+
Details | Diff
0003-do-not-register-pre-post-op-plugins-if-disabled.patch (1.05 KB, patch)
2010-09-29 10:47 EDT, Rich Megginson
nhosoi: review+
Details | Diff
0001-add-support-for-global-inactivity-limit.patch (4.69 KB, patch)
2010-09-29 16:14 EDT, Rich Megginson
nhosoi: review+
Details | Diff
0001-fix-typos-in-Makefile.am-acctpolicy-schema.patch (2.61 KB, patch)
2010-09-29 16:18 EDT, Rich Megginson
nhosoi: review+
Details | Diff

  None (edit)
Comment 2 Ulf Weltman 2010-05-11 16:47:04 EDT
Created attachment 413256 [details]
account policy plugin 1.0 (inactivity limits)

The source in this tarball is for account policy plug-in 1.0, which currently only supports one policy: account inactivity limits.  It also records bind timestamp to support the account inactivity limit.  This is made available under GPLv2.
Comment 3 Rich Megginson 2010-08-04 17:33:33 EDT
*** Bug 580956 has been marked as a duplicate of this bug. ***
Comment 4 Rich Megginson 2010-09-16 11:23:49 EDT
Are there any test scripts for this plugin?
Comment 5 Ulf Weltman 2010-09-16 15:55:14 EDT
The tests I used were non-portable unit tests, not integrated to tetframework.  Also, getting approval for sharing them would meet with difficulty today.

I wrote this plug-in in 2006 in response to customer needs.  Since then there has been discussion of account inactivity on the ldapext mailing list, particularly around the middle of 2010.  The 10th iteration of Prasanta's password policy draft has password idling added:  http://tools.ietf.org/html/draft-behera-ldap-password-policy-10

Since my plug-in predates the draft there are some differences:
1) I define as account policy rather than password policy.  More account policies would be added to the plug-in, for example for account expiration.  If I had thought of it as a password policy I would have implemented in the frontend (libslapd) along with the current password policy (which ought to be moved out to a plug-in.  Also, updated for compatibility with the new PWP drafts - many things have changed, e.g. password expiration time is now computed at authentication relative to password change timestamp, rather than our absolute expiration timestamp).
2) I solved my provisioning problem where a new user doesn't have a last authentication time by reverting to createTimestamp when lastLoginTimestamp is missing.  I don't think this situation is addressed in the draft.
3) The schema differs, mine is defined in account policy terms under HP's OID, but otherwise I felt they're pretty similar.

HP's account inactivity policy schema:
( 2.16.840.1.113719.1.1.4.1.35 NAME 'lastLoginTime'
  DESC 'Last login time'
  SYNTAX 1.3.6.1.4.1.1466.115.121.1.24
  SINGLE-VALUE
  USAGE directoryOperation
  X-ORIGIN 'Account Policy Plugin' )

( 1.3.6.1.4.1.11.1.3.2.1.3 NAME 'accountInactivityLimit'
  DESC 'Account inactivity limit'
  SYNTAX 1.3.6.1.4.1.1466.115.121.1.15
  SINGLE-VALUE
  X-ORIGIN 'Account Policy Plugin' )

Password policy draft 10's password idling schema:
( 1.3.6.1.4.1.42.2.27.8.1.29 NAME 'pwdLastSuccess'
  DESC 'The timestamp of the last successful authentication'
  EQUALITY generalizedTimeMatch
  ORDERING generalizedTimeOrderingMatch
  SYNTAX 1.3.6.1.4.1.1466.115.121.1.24
  SINGLE-VALUE
  NO-USER-MODIFICATION
  USAGE directoryOperation )

( 1.3.6.1.4.1.42.2.27.8.1.26 NAME 'pwdMaxIdle'
  EQUALITY integerMatch
  SYNTAX 1.3.6.1.4.1.1466.115.121.1.27
  SINGLE-VALUE )
Comment 6 Rich Megginson 2010-09-16 16:38:24 EDT
Ok.  Which would you prefer?  Get the current account policy plugin into 389 now, or revise it to make it compatible with the current password policy I-D?
Comment 7 Ulf Weltman 2010-09-22 11:55:37 EDT
I would go with the current implementation for now.

There's no telling how long it will take for this part of the I-D to stabilize.  Reviewing an ldapext exchange from 8/10/2010 indicates more changes coming: the current I-D idle password policy might start disabling individual unused passwords while inactive account disabling is shoved off to a future account policy draft.  I take this to mean that my account policy implementation can co-exist with what's in the password policy I-D since.

By the way, I was already thinking that someone would have other ideas for where the login timestamp should be stored so in the plug-in's configuration entry one can specify what attribute name to use.
Comment 8 Rich Megginson 2010-09-28 19:07:33 EDT
Created attachment 450337 [details]
0001-add-the-account-policy-plugin-and-related-server-cod.patch

This differs from the previous patch in a couple of ways:
1) adds a configure switch --enable-acctpolicy (on by default) so that it is possible to disable building the plugin and schema
2) fixes some minor problems in the schema and config entries
3) fixes some minor memory leaks and compiler warnings
Comment 9 Rich Megginson 2010-09-29 10:47:02 EDT
Created attachment 450495 [details]
0002-fix-pblock-memory-leak.patch
Comment 10 Rich Megginson 2010-09-29 10:47:21 EDT
Created attachment 450496 [details]
0003-do-not-register-pre-post-op-plugins-if-disabled.patch
Comment 11 Noriko Hosoi 2010-09-29 13:16:11 EDT
I'm reviewing the patches.  Please help me understanding the spec...

1. Why bitwise is enabled together when acctpolicy is?  (The Account Policy Plugin it self does not depends on the bitwise plugin...)
+if enable_acctpolicy
+LIBACCTPOLICY_PLUGIN = libacctpolicy-plugin.la
+LIBACCTPOLICY_SCHEMA = $(srcdir)/ldap/schema/60acctpolicy.ldif
+enable_bitwise = 1 <=== ???
+endif

2. Can there be multiple policies in a backend?  If yes, what happens if they are nested and conflicted?
+dn: cn=AccountPolicy,dc=example,dc=com
+objectClass: top
+objectClass: ldapsubentry
+objectClass: extensibleObject
+objectClass: accountpolicy
+# 86400 seconds per day * 30 days = 2592000 seconds
+accountInactivityLimit: 2592000
+cn: AccountPolicy

+dn: cn=AccountPolicy,ou=people,dc=example,dc=com
+objectClass: top
+objectClass: ldapsubentry
+objectClass: extensibleObject
+objectClass: accountpolicy
+# 86400 seconds per day * 3 days = 259200 seconds
+accountInactivityLimit: 259200
+cn: AccountPolicy

3. Found one typo in a comment. ;)
+++ b/ldap/schema/60acctpolicy.ldif
+## acctPolicySubentry is an an account policy pointer (DN syntax)
                          ^^^^^

It'd be nice if we could have a design doc on fedora wiki...
http://directory.fedoraproject.org
Comment 12 Rich Megginson 2010-09-29 13:27:54 EDT
(In reply to comment #11)
> I'm reviewing the patches.  Please help me understanding the spec...
> 
> 1. Why bitwise is enabled together when acctpolicy is?  (The Account Policy
> Plugin it self does not depends on the bitwise plugin...)
> +if enable_acctpolicy
> +LIBACCTPOLICY_PLUGIN = libacctpolicy-plugin.la
> +LIBACCTPOLICY_SCHEMA = $(srcdir)/ldap/schema/60acctpolicy.ldif
> +enable_bitwise = 1 <=== ???
> +endif

Arg - copy/paste error - thanks

> 
> 2. Can there be multiple policies in a backend?  If yes, what happens if they
> are nested and conflicted?
> +dn: cn=AccountPolicy,dc=example,dc=com
> +objectClass: top
> +objectClass: ldapsubentry
> +objectClass: extensibleObject
> +objectClass: accountpolicy
> +# 86400 seconds per day * 30 days = 2592000 seconds
> +accountInactivityLimit: 2592000
> +cn: AccountPolicy
> 
> +dn: cn=AccountPolicy,ou=people,dc=example,dc=com
> +objectClass: top
> +objectClass: ldapsubentry
> +objectClass: extensibleObject
> +objectClass: accountpolicy
> +# 86400 seconds per day * 3 days = 259200 seconds
> +accountInactivityLimit: 259200
> +cn: AccountPolicy

It depends on how the operational attribute acctPolicySubentry is populated in the user's entry.  dn: uid=scarter can have
acctPolicySubentry: cn=AccountPolicy,ou=people,dc=example,dc=com
or
acctPolicySubentry: cn=AccountPolicy,dc=example,dc=com

If an admin sets it directly.  If acctPolicySubentry is provided by CoS, it is up to CoS locality or priority to determine which value is set.  If there really is more than one value, the acct policy code just picks the first one, which means it is non-deterministic, and the admin will have to enable plugin debugging in order to see which one it picks.

> 
> 3. Found one typo in a comment. ;)
> +++ b/ldap/schema/60acctpolicy.ldif
> +## acctPolicySubentry is an an account policy pointer (DN syntax)

Thanks.

>                           ^^^^^
> 
> It'd be nice if we could have a design doc on fedora wiki...
> http://directory.fedoraproject.org

Ulf did one a long time ago.
http://directory.fedoraproject.org/wiki/Account_Policy_Design
Comment 13 Noriko Hosoi 2010-09-29 13:53:24 EDT
(In reply to comment #12)
> It depends on how the operational attribute acctPolicySubentry is populated in
> the user's entry.  dn: uid=scarter can have
> acctPolicySubentry: cn=AccountPolicy,ou=people,dc=example,dc=com
> or
> acctPolicySubentry: cn=AccountPolicy,dc=example,dc=com
> 
> If an admin sets it directly.  If acctPolicySubentry is provided by CoS, it is
> up to CoS locality or priority to determine which value is set.  If there
> really is more than one value, the acct policy code just picks the first one,
> which means it is non-deterministic, and the admin will have to enable plugin
> debugging in order to see which one it picks.
> 
> Ulf did one a long time ago.
> http://directory.fedoraproject.org/wiki/Account_Policy_Design

Thank you, Rich and Ulf!

Other than a few items above, I don't see any defects.  Shall I mark "reviewed" on the 3 patches?  Or are you planning to attach a new one?
Comment 14 Ulf Weltman 2010-09-29 13:56:05 EDT
(In reply to comment #13)
> (In reply to comment #12)
> > It depends on how the operational attribute acctPolicySubentry is populated in
> > the user's entry.  dn: uid=scarter can have
> > acctPolicySubentry: cn=AccountPolicy,ou=people,dc=example,dc=com
> > or
> > acctPolicySubentry: cn=AccountPolicy,dc=example,dc=com
> > 
> > If an admin sets it directly.  If acctPolicySubentry is provided by CoS, it is
> > up to CoS locality or priority to determine which value is set.  If there
> > really is more than one value, the acct policy code just picks the first one,
> > which means it is non-deterministic, and the admin will have to enable plugin
> > debugging in order to see which one it picks.
Also, acctPolicySubentry is defined to be single-valued by schema.

> > 
> > Ulf did one a long time ago.
> > http://directory.fedoraproject.org/wiki/Account_Policy_Design
I just updated it with the new OIDs for my schema (HP's tree), removed some stale info on alternate implementation methods, and added a note to the outstanding issues regarding developments in the password policy drafts.

> 
> Thank you, Rich and Ulf!
> 
> Other than a few items above, I don't see any defects.  Shall I mark "reviewed"
> on the 3 patches?  Or are you planning to attach a new one?
Comment 15 Rich Megginson 2010-09-29 14:03:34 EDT
Thanks.  I have a couple more patches.
Comment 16 Rich Megginson 2010-09-29 16:14:17 EDT
Created attachment 450574 [details]
0001-add-support-for-global-inactivity-limit.patch
Comment 17 Rich Megginson 2010-09-29 16:18:28 EDT
Created attachment 450575 [details]
0001-fix-typos-in-Makefile.am-acctpolicy-schema.patch

This addresses Noriko's review comments
Comment 18 Rich Megginson 2010-10-01 19:09:11 EDT
These are all of the patches.  Do they look ok?  If so, I'll push them for 1.2.7
Comment 19 Rich Megginson 2010-10-04 11:32:58 EDT
commit d04ffbe70416df4b1b63e7d4e21dbcbb428afe49
Author: Rich Megginson <rmeggins@redhat.com>
Date:   Wed Sep 29 14:19:04 2010 -0600

    fix typos in Makefile.am, acctpolicy schema

commit 037623905acf1379c964821dbb00f82f2ef1ac95
Author: Rich Megginson <rmeggins@redhat.com>
Date:   Wed Sep 29 14:15:08 2010 -0600

    add support for global inactivity limit

commit 4ddeb0bd1e5b79360b1850ea454c08d9c2706ffa
Author: Rich Megginson <rmeggins@redhat.com>
Date:   Wed Sep 29 08:43:12 2010 -0600

    do not register pre/post op plugins if disabled

commit 5f01b7913cc0c19bf8757351506f73adb1c9e58b
Author: Rich Megginson <rmeggins@redhat.com>
Date:   Wed Sep 29 08:42:03 2010 -0600

    fix pblock memory leak

commit 32e2b04dd1d98d96d90fdfaa3841524b3003dcdb
Author: Rich Megginson <rmeggins@redhat.com>
Date:   Fri Sep 17 08:18:29 2010 -0600

    add the account policy plugin and related server code, schema, and config
Comment 20 Amita Sharma 2011-06-13 07:45:43 EDT
As per Rich's comments (https://bugzilla.redhat.com/show_bug.cgi?id=498097)
marking the bug as Sanity Only Verified.

Note You need to log in before you can comment on or make changes to this bug.