On Fri, Mar 19, 2010 at 02:15:06PM +0100, Sumit Bose wrote:
On Fri, Mar 19, 2010 at 08:48:49AM -0400, Simo Sorce wrote:
On Fri, 19 Mar 2010 12:00:47 +0100 Sumit Bose sbose@redhat.com wrote:
On Thu, Mar 18, 2010 at 05:51:13PM -0400, Simo Sorce wrote:
Some time ago I added code to fetch the rootdse on connection, but didn't publicize it too much.
Attached find 2 patches.
- Rework the way we store data fetched from the rootdse so the it
is more useful and is actually attached to the ldap handle.
I'm in general fine with this one, although I haven't tested it throughly yet.
- Check controls are supported before using them.
I have two comments here:
- sdap_is_control_supported() should be integrated in sss_ldap_control_create() or a new call should be created which
calls sdap_is_control_supported() and then sss_ldap_control_create().
- the debug message 'Server does not support the Password Policy control' should have a much lower level than 7. Because if the
control is missing the user might not see the expected behaviour/performance.
Sumit, do you want to take over the second patch ? I don't have a way to thoroughly test it anyway and I am a bit short on time.
sure
Hi,
not even 4 months later here are the revised versions of these patches. I had it add two more changes to the first patch to make it build on master because of the change to sdap_cli_connect_recv().
In the second patch if have created a new call sdap_control_create() which checks if the control is supported with sdap_is_control_supported() and calls sss_ldap_control_create() on success. I think this approach is more clean, because it leaves sss_ldap_control_create() as a pure replacement function.
bye, Sumit