On 11/20/2012 02:09 AM, Simo Sorce wrote:
On Mon, 2012-11-19 at 23:31 +0100, Jakub Hrozek wrote:
On Mon, Nov 19, 2012 at 05:51:53PM +0100, Pavel Březina wrote:
On 11/18/2012 11:58 PM, Jakub Hrozek wrote:
[PATCH 1/2] SYSDB: Use the add_string convenience functions for managing
ghost user attribute
Using the convenience function instead of low-level ldb calls makes the
code more compact and more readable.
[PATCH 2/2] LDAP: Only convert direct parents' ghost attribute to member
https://fedorahosted.org/sssd/ticket/1612
I'm not too happy with the originalDN attribute exposed in the
sysdb_add_user/store_user functions but it seemed like a better way
than grabbing the attribute from the extended attributes in the "attrs"
array. At the same time, we need to adjust the attributes during all user
adds, not just during sdap_store_user from the LDAP provider. Ideas on
how to make the process cleaner are welcome.
This patch changes the handling of ghost attributes when saving the
actual user entry. Instead of always linking all groups that contained
the ghost attribute with the new user entry, the original member
attributes are now saved in the group object and the user entry is only
linked with its direct parents.
As the member attribute is compared against the originalDN of the user,
if either the originalDN or the originalMember attributes are missing,
the user object is linked with all the groups as a fallback.
The original member attributes are only saved if the LDAP schema
supports nesting.
@@ -235,6 +236,19 @@ sdap_process_ghost_members(struct sysdb_attrs *attrs,
return ret;
}
Hi,
just one nitpick.
+ if (store_original_member) {
+ DEBUG(SSSDBG_TRACE_FUNC, ("The group has %d members\n",
memberel->num_values));
+ for (i = 0; i < memberel->num_values; i++) {
+ ret = sysdb_attrs_add_string(sysdb_attrs, SYSDB_ORIG_MEMBER,
+ (const char *)
memberel->values[i].data);
+ if (ret) {
+ DEBUG(SSSDBG_OP_FAILURE, ("Could not add member [%s]\n",
+ (const char *) memberel->values[i].data));
wrong indentation^^
+ return ret;
+ }
+ }
+ }
+
Otherwise it looks good to me.
Thank you for the review. Simo requested some additional changes, in
particular:
* use "it" instead of "him" when talking about user
* don't misuse twopass, but use a separate variable
* fix code to adhere to 80-columns rule on several places
New patches are attached.
I haven't been able to test the patch yet, but at least looking at the
code I see no issues, so I would ack.
Simo.
I tried it and it seems to work fine.
Ack.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
