[SSSD] [PATCHES] Alternative objectClass for group maps

Jakub Hrozek jhrozek at redhat.com
Thu Sep 11 09:43:07 UTC 2014


On Wed, Sep 10, 2014 at 09:59:35PM +0200, Michal Židek wrote:
> On 09/10/2014 06:13 PM, Michal Židek wrote:
> >On 09/10/2014 06:00 PM, Michal Židek wrote:
> >>On 09/10/2014 02:49 PM, Jakub Hrozek wrote:
> >>>On Wed, Sep 10, 2014 at 01:23:15PM +0200, Michal Židek wrote:
> >>>>--- a/src/providers/ldap/sdap.c
> >>>>+++ b/src/providers/ldap/sdap.c
> >>>>@@ -348,9 +348,12 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
> >>>>          }
> >>>>
> >>>>          for (i = 0; vals[i]; i++) {
> >>>>-            /* the objectclass is always the first name in the map */
> >>>>-            if (strncasecmp(map[0].name,
> >>>>-                            vals[i]->bv_val, vals[i]->bv_len) == 0) {
> >>>>+            /* The objectclass is always the first name in the map.
> >>>>+             * Alternative objectclass for groups is second. */
> >>>>+            if (strncasecmp(map[0].name, vals[i]->bv_val,
> >>>>vals[i]->bv_len) == 0
> >>>>+                || (map[1].name != NULL
> >>>>+                    && strncasecmp(map[1].name,
> >>>>+                                   vals[i]->bv_val, vals[i]->bv_len)
> >>>>== 0)) {
> >>>>                  /* ok it's an entry of the right type */
> >>>>                  break;
> >>>>              }
> >>>>@@ -630,7 +633,9 @@ errno_t sdap_parse_deref(TALLOC_CTX *mem_ctx,
> >>>>
> >>>>          for (i=0; ocs[i]; i++) {
> >>>>              /* the objectclass is always the first name in the map */
> >>>>-            if (strcasecmp(minfo[mi].map[0].name, ocs[i]) == 0) {
> >>>>+            if (strcasecmp(minfo[mi].map[0].name, ocs[i]) == 0
> >>>>+                || (minfo[mi].map[1].name != NULL
> >>>>+                    && strcasecmp(minfo[mi].map[1].name, ocs[i]) ==
> >>>>0)) {
> >>>>                  DEBUG(SSSDBG_TRACE_ALL,
> >>>>                        "Found map for objectclass '%s'\n", ocs[i]);
> >>>>                  map = minfo[mi].map;
> >>>
> >>>It would also be nice to split these two places into a separate
> >>>function. That would make migrating to a list of objectclasses easier
> >>>later.
> >>
> >>Ok. I included these two changes:
> >>1. add function to generate ORed list of objectclasses, that can be
> >>used in filter.
> >>
> >>2. add small static function to match objectclass.
> >>
> >>I do not want to include more changes. It will be better to file
> >>tickets for the enhancements that will come out of the discussion
> >>on the list. The two added function probably be changed anyway.
> >>
> >>See the attached patches.
> >
> >Or better don't :) . I forgot to amend the first patch. Will send
> >new version soon.
> >
> 
> Sorry took a little longer. Here are the patches.
> 
> Michal
> 

> From 58b3750bbbcda0dafe0c73b8eae69264d1eb2c7e Mon Sep 17 00:00:00 2001
> From: Michal Zidek <mzidek at redhat.com>
> Date: Wed, 10 Sep 2014 12:41:16 +0200
> Subject: [PATCH 1/2] Add alternative objectClass to group attribute maps

ACK

> From 70c8c5ea07bceab536197c2d8424161988b156e0 Mon Sep 17 00:00:00 2001
> From: Michal Zidek <mzidek at redhat.com>
> Date: Wed, 10 Sep 2014 12:56:54 +0200
> Subject: [PATCH 2/2] Use the alternative objectclass in group maps.

Code looks mostly good to me. I haven't ran any tests yet, see one
question inline.

> 
> Use the alternative group objectclass in queries.
> 
> Fixes:
> https://fedorahosted.org/sssd/ticket/2436
> ---
>  src/providers/ldap/ldap_id.c                  | 18 ++++++----
>  src/providers/ldap/sdap.c                     | 41 +++++++++++++++++++---
>  src/providers/ldap/sdap.h                     |  2 ++
>  src/providers/ldap/sdap_async_enum.c          | 12 +++++--
>  src/providers/ldap/sdap_async_initgroups.c    | 50 +++++++++++++++++++++------
>  src/providers/ldap/sdap_async_initgroups_ad.c | 13 +++++--
>  src/providers/ldap/sdap_async_nested_groups.c | 11 ++++--
>  7 files changed, 117 insertions(+), 30 deletions(-)
> 

[...]

>  /* =Parse-msg============================================================= */
> -
> +static bool objectclass_matched(struct sdap_attr_map *map,
> +                                const char *objcl, int len);

Please put a newline here.

>  int sdap_parse_entry(TALLOC_CTX *memctx,
>                       struct sdap_handle *sh, struct sdap_msg *sm,
>                       struct sdap_attr_map *map, int attrs_num,
> @@ -348,9 +349,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
>          }
>  
>          for (i = 0; vals[i]; i++) {
> -            /* the objectclass is always the first name in the map */
> -            if (strncasecmp(map[0].name,
> -                            vals[i]->bv_val, vals[i]->bv_len) == 0) {
> +            if (objectclass_matched(map, vals[i]->bv_val, vals[i]->bv_len)) {
>                  /* ok it's an entry of the right type */
>                  break;
>              }
> @@ -530,6 +529,25 @@ done:
>      return ret;
>  }
>  
> +static bool objectclass_matched(struct sdap_attr_map *map,
> +                                const char *objcl, int len)
> +{
> +    if (len == 0) {
> +        len = strlen(objcl) + 1;
> +    }

When is len 0 ?

> +
> +    if (strncasecmp(map[SDAP_OC_GROUP].name, objcl, len) == 0) {
> +        return true;
> +    }
> +
> +    if (map[SDAP_OC_GROUP_ALT].name != NULL
> +        && strncasecmp(map[SDAP_OC_GROUP_ALT].name, objcl, len) == 0) {
> +        return true;
> +    }
> +
> +    return false;
> +}
> +




More information about the sssd-devel mailing list