-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 12/03/2010 06:56 AM, Sumit Bose wrote:
On Thu, Dec 02, 2010 at 09:00:37AM -0500, Stephen Gallagher wrote:
On 11/25/2010 08:25 AM, Sumit Bose wrote:
>>> Hi,
>>>
>>> these two patches aim to fix trac ticket #672. While the first patch
>>> only makes a utility function public the second adds some new layers to
>>> the LDAP access provider. I've tried to make the changes in a way to
>>> make it easy to add new rules (#670) and new expire policies (#673,
>>> #674, #690).
>>>
>>> I would like to ask the reviewers to check if the new code is really as
>>> flexible as I think and if it enough to evaluate the shadow expire
>>> attribute here. TIA
>>>
>>> bye,
>>> Sumit
>>>
Patch 0001: Ack.
Patch 0002: Nack.
The manpage entry for ldap_account_expire_policy should list the valid
values for this option. The patch comment notes that this is only
"shadow" at present.
> done
sssm_ldap_access_init() should throw an error (or at least warn and
ignore it) if we get a duplicate access rule. e.g.:
ldap_access_order = filter, filter
> I prefer the error, because it might not be clear which order was
> intended.
Please rename sdap_access_decide_offline(), sdap_access_retry(),
sdap_access_connect_done() and sdap_access_get_access_done() to be clear
that they apply to the access_filter.
> done
I'm not sure we want to deny on a missing password expiration attribute.
Some users might be lacking this attribute with the intent that it means
"no expiration". Maybe we should add a strictness option here.
> I checked you pam_ldap handles this. A missing attribute or a value of 0
> means "no expiration".
next_access_rule() should probably return on PAM_ACCT_EXPIRED, not just
PAM_PERM_DENIED.
> done
> Thanks for the review, new version attached.
Nack.
Typo in sssd-ldap.5.xml (tat)
typo in ldap_init.c (dublicate)
check_list_for_duplicates() could be made more scalable by doing (in
pseudocode):
diff_string_lists(list, NULL, &nodupes, NULL, NULL); /* Pass in one
string, and get the list of entries that were only in that string */
if (array_len(nodupes) < array_len(list)) {
/* list had duplicates */
}
Internally, this just plays some tricks with a hash addition so that if
the same key is used, it will be dropped. So the resulting array
returned to *_list1_only will contain no duplicates. If the number of
entries in the _list1_only array is fewer than the number of entries in
the input list, duplicates were dropped.
- --
Stephen Gallagher
RHCE 804006346421761
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora -
http://enigmail.mozdev.org/
iEYEARECAAYFAkz47IgACgkQeiVVYja6o6PovwCfW8a0+X3KKPIJ1rRMm52yov0i
xzMAniR20aYjU5uJqHKlpF8AOe2Zx4eZ
=RXH6
-----END PGP SIGNATURE-----