On Thursday 25 of July 2013 15:53:23 Jakub Hrozek wrote:
> On Thu, Jul 25, 2013 at 03:39:59PM +0200, Tomas Babej wrote:
> > On Thursday 25 of July 2013 09:30:22 Jan Cholasta wrote:
> > > On 25.7.2013 09:11, Petr Spacek wrote:
> > > > On 25.7.2013 09:03, Alexander Bokovoy wrote:
> > > >> On Thu, 25 Jul 2013, Petr Spacek wrote:
> > > >>> On 24.7.2013 22:18, Tomas Babej wrote:
> > > >>>> Hi,
> > > >>>>
> > > >>>> When converting the result obtained by python-ldap library,
> > > >>>> we need to skip unresolved referral entries, since they cannot
> > > >>>> be converted.
> > > >>>>
> > > >>>> https://fedorahosted.org/freeipa/ticket/3814
> > > >>>
> > > >>> I'm not sure if a simple 'skip it' approach is the right one.
> > > >>> Shouldn't it
> > > >>> print/log a warning at least? Do you know all implications? Are you 
> > > >>> sure
> > > >>> that this will not break something else silently?
> > > >>>
> > > >>> (BTW isn't the right approach to fix python-ldap? Or is it a quirk in
> > > >>> AD?)
> > > >> AD DC often answers with proper result and then several referrals to
> > > >> other internal resources to complement the search if you are asking for
> > > >> wide-open search (default). We are not interested in these referrals 
> > > >> for
> > > >> various reasons, including the fact that we are looking at the
> > > >> authoritative DC and it has all the needed info.
> > > >>
> > > >> At best, we could define an option that forces us doing referral 
> > > >> chasing
> > > >> to fetch remaining results but this is not something really needed 
> > > >> right
> > > >> now.
> > > >
> > > > I understand that we don't need referrals now, but the question is
> > > > 'Could it break something? Silently? In the future?'.
> > > >
> > > > E.g. the option 'follow referrals' (defaulting to False) is IMHO much
> > > > much better.
> > > >
> > > > The point is that we don't need to implement referral chasing right now,
> > > > just thrown an exception if somebody tries to switch 'follow referrals'
> > > > option to True. IMHO this will prevent surprises in the future, because
> > > > it is absolutely clear that referrals are not followed.
> > > >
> > > 
> > > IMO a comment is good enough. I don't think adding options that aren't 
> > > used anywhere is a good thing to do.
> > > 
> > > Honza
> > > 
> > > -- 
> > > Jan Cholasta
> > 
> > I considered adding an options for that, but decided against it in the end
> > since it would have to bubble down through many layers, while, as Honza 
> > says,
> > not being used anywhere.
> > 
> > To make sure that this change does not cause problems, I think we agree to
> > scream at DEBUG level to the log if the referral entry is ignored, and
> > at WARNING level if the referral resolution is turned on in underlying 
> > library
> > on the connection level.
> > 
> > Tomas
> 
> For what it's worth, the SSSD ignores referrals completely when talking
> to AD. So disabling or ignoring referrals is the right thing to do here.

After some investigation I decided the correct approach here is to
scream at the debug level only, when referral is being ignored.

We cannot guide ourselves by the ldap.OPT_REFFERALS option of the underlying
connection simply because even if referral chasing is turned on (and therefore
we should not get any referrals from python-ldap, since they should have been
resolved), queries for AD can return referrals (AD returns them often as a way 
to
provide additional information AFAIU). This can also happen if we are not able
to authenticate to the referred server, or resolve the LDAP uri.

In case ignoring referrals ever breaks something, we can find the information
in the log at the debug level. Doing otherwise would be unnecessarily spamming
the log now.

Updated patch attached.

Tomas
>From 39cc0a62cde8ff05e5913bf74ec8cee9ef637a12 Mon Sep 17 00:00:00 2001
From: Tomas Babej <[email protected]>
Date: Wed, 24 Jul 2013 21:59:49 +0200
Subject: [PATCH] Skip referrals when converting LDAP result to LDAPEntry

When converting the result obtained by python-ldap library,
we need to skip unresolved referral entries, since they cannot
be converted.

https://fedorahosted.org/freeipa/ticket/3814
---
 ipapython/ipaldap.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 6873511c44427edc4a7e573bb04da00732a63028..e8a6946590909d07c5b1fd311ec299eb5bb4be5e 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -425,6 +425,16 @@ class IPASimpleLDAPObject(object):
             original_dn = dn_tuple[0]
             original_attrs = dn_tuple[1]
 
+            # original_dn is None if referral instead of an entry was
+            # returned from the LDAP server, we need to skip this item
+            if original_dn is None:
+                log_msg = 'Referral entry {ref} ignored when '\
+                          'converting python-ldap result to LDAPEntry'
+                log_msg = log_msg.format(ref=str(dn_tuple))
+                self.log.debug(log_msg)
+
+                continue
+
             ipa_entry = LDAPEntry(self, DN(original_dn))
 
             for attr, original_values in original_attrs.items():
-- 
1.8.3.1

_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to