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 <mzi...@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 <mzi...@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;
> +}
> +
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
