[SSSD] [PATCH] Return all user members for RFC2307 groups (sssd-1.2)

Jakub Hrozek jhrozek at redhat.com
Fri Oct 8 09:09:30 UTC 2010


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/06/2010 01:18 PM, Jakub Hrozek wrote:
> On 10/05/2010 11:12 PM, Simo Sorce wrote:
>> On Tue, 05 Oct 2010 19:11:02 +0200
>> Jakub Hrozek <jhrozek at redhat.com> wrote:
> 
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>>
>>> [PATCH 1/3] sysdb interface for adding expired user entries
>>> These are added instead of looking up the members.
> 
>> I am not sure I like adding a uid and gid number.
>> I understand it makes it simpler as you can reuse the existing sysdb
>> interface but it makes me really nervous, I think a fake user shouldn't
>> have any attribute at all including gidnumber and uidnumber so that it
>> is clear it is a fake user.
> 
>> In any case, should we decide having uidnumber=0, gidnumber=0 is an
>> acceptable compromise, these shouldn't be exposed as arguments in the
>> sysdb interface. We should just use internal values store into a macro
>> like FAKE_UID_NUMBER and FAKE_GID_NUMBER.
> 
> 
> 
> As explained in the initgroups thread, it shouldn't matter as we always
> treat 0 as the "default" value. But I agree that a clear sysdb interface
> is very important, too, so I have changed the _send() call to only
> accept username and not save the UID, GID and additional attrs at all.
> 
>> Also if you always return ok in cleanup_users_logged_in() when uid = 0
>> then it means you will never clean up these entries. Am I reading it
>> right? If so that would be a problem. Fake users can never be logged in
>> so you should probably return ENOENT instead ...
> 
> Yes, in the version you reviewed they would never get cleaned. The
> reason was that the fake entries did not have any SYSDB_LAST_LOGIN
> attribute so the cleanup code always marks them as expired (the sysdb
> filter in the cleanup code is (SYSDB_CACHE_EXPIRE<time(NULL) && no
> SYSDB_LAST_LOGIN attribute). But on another thought, I don't think that
> adding SYSDB_LAST_LOGIN attribute should cause any harm on the fake
> entry so I added it, too.
> 
> So in the attached version, the cleanup_users_logged_in() report the
> user entries as they are never logged in (ret == EOK) and their cleanup
> is controlled by the account_cache_expiration parameter, same as regular
> user entries.
> 
> 
>>> [PATCH 2/3] Shortcut for save_group() to accept sysdb DNs as member
>>> attributes
>>>
>>> This patch is a backport of original written by Ralf Haferkamp.
> 
>> ack.
> 
>>> [PATCH 3/3] Add fake users during saving of RFC2307 group
>>> Instead of looking up the user entries, we just add dummy expired
>>> users that are refreshed when they are requested with
>>> getpwnam/getpwuid
> 
>> This looks reasonable, although a quite complex patch (due too async
>> sysdb unfortunately :-/).
> 
> 
> Unfortunately yes. I plan to give them another round of testing today to
> check for any issues but I wanted to get the patches on list to speed
> things up and allow Stephen to work on top of the patches.
> 
>> One thing I can't see though. At the end of
>> sdap_process_missing_member_2307_done() you save the missing group DN
>> in the state->sysdb_dns[] array.
>> But I don't see where you use these additional names you are saving
>> here... I may be missing some relationship, can you shed some light ?
> 
> 
> The sysdb_dns[] array points to the member attribute of the group. So
> when a fake user is added, its sysdb DN is added to the member attribute
> of the group at the same time. Using the second patch, the group is then
> saved into sysdb pointing at the cached users with no additional lookup.
> 
> This was actually introduced by Ralf in his RFC2307bis patch for master
> but I admit that it took me some time to understand this, too. Maybe
> this part of code requires a comment or renaming the sysdb_dns[] variable?
> 
>> Simo.
> 
> 
> Thank you for the review. New patches are attached.
> 
> 	Jakub

Another round of patches that apply cleanly on top of Stephen's
RFC2307bis nested group work in the thread named "[PATCHES] Handle
nested groups in RFC2307bis".
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkyu38oACgkQHsardTLnvCU+XgCfVkMK1RE7fbc5sNSYyIEY1ToX
JowAoNyILUzFaZVGdVjhVX0JE0HTh7a0
=e7mV
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-sysdb-interface-for-adding-expired-user-entries.patch
Type: text/x-patch
Size: 9632 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20101008/2c070e90/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Shortcut-for-save_group-to-accept-sysdb-DNs-as-membe.patch
Type: text/x-patch
Size: 5239 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20101008/2c070e90/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Add-fake-users-during-saving-of-RFC2307-group.patch
Type: text/x-patch
Size: 22345 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20101008/2c070e90/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-sysdb-interface-for-adding-expired-user-entries.patch.sig
Type: application/pgp-signature
Size: 72 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20101008/2c070e90/attachment.sig>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Shortcut-for-save_group-to-accept-sysdb-DNs-as-membe.patch.sig
Type: application/pgp-signature
Size: 72 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20101008/2c070e90/attachment-0001.sig>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Add-fake-users-during-saving-of-RFC2307-group.patch.sig
Type: application/pgp-signature
Size: 71 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20101008/2c070e90/attachment-0002.sig>


More information about the sssd-devel mailing list