Re: [Freeipa-devel] [PATCH 0116] Refactoring of service autobind

2014-09-26 Thread Martin Kosek

On 09/25/2014 03:06 PM, Martin Basti wrote:

On 25/09/14 14:47, Jan Cholasta wrote:

Dne 25.9.2014 v 10:51 Martin Basti napsal(a):

On 19/09/14 14:30, Jan Cholasta wrote:

Dne 19.9.2014 v 13:32 Martin Basti napsal(a):

On 01/09/14 16:26, Martin Basti wrote:

On 28/08/14 14:01, Jan Cholasta wrote:

Hi,

Dne 27.8.2014 v 15:22 Martin Basti napsal(a):

Patch attached.



1) Please rename object_exists to entry_exists.


2) Use empty attribute list in get_entry() in
object_exists/entry_exists.


3) Please update LDAPObject.get_dn_if_exists() to use
object_exists/entry_exists.


4) I'm not a fan of how do_bind() is laid out, IMHO something like
this would be better (untested):

+def do_bind(self, dm_password=None, autobind=AUTOBIND_AUTO,
timeout=DEFAULT_TIMEOUT):
+if dm_password:
+self.do_simple_bind(bindpw=dm_password, timeout=timeout)
+return
+
+if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and
self.ldapi:
+try:
+# autobind
+pw_name = pwd.getpwuid(os.geteuid()).pw_name
+self.do_external_bind(pw_name, timeout=timeout)
+return
+except errors.NotFound:
+if autobind == AUTOBIND_ENABLED:
+# autobind was required and failed, raise
+# exception that it failed
+raise
+
+# Fall back
+self.do_sasl_gssapi_bind(timeout=timeout)


Honza


3) skipped as we discuss on IRC

Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1


1) The patch need a rebase on top of current ipa-4-1.

I can apply it (Am I doing something wrong?)



2) You can remove import pwd from service.py, it is no longer used there.


3) Are named constants for the autobind argument the right thing to
do? It is a tri-state which can be expressed with None/True/False.
(I'm just asking, I don't have a strong opinion on this.)


As we discussed on IRC, using None/True/False, is not good approach.
Updated patch attached



ACK, but the patch still does not apply cleanly on ipa-4-1:

$ git apply freeipa-mbasti-0116.3-Refactoring-of-autobind-object_exists.patch
error: patch failed: ipaserver/install/service.py:20
error: ipaserver/install/service.py: patch does not apply


Rebased patches attached


Pushed to:
master: 29ba9d9d26b92498902d40d71adae193308b5c92
ipa-4-1: 8e0f8bc7ad8e91bcf9e30e3cc8159838977348e6

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0116] Refactoring of service autobind

2014-09-25 Thread Martin Basti

On 19/09/14 14:30, Jan Cholasta wrote:

Dne 19.9.2014 v 13:32 Martin Basti napsal(a):

On 01/09/14 16:26, Martin Basti wrote:

On 28/08/14 14:01, Jan Cholasta wrote:

Hi,

Dne 27.8.2014 v 15:22 Martin Basti napsal(a):

Patch attached.



1) Please rename object_exists to entry_exists.


2) Use empty attribute list in get_entry() in
object_exists/entry_exists.


3) Please update LDAPObject.get_dn_if_exists() to use
object_exists/entry_exists.


4) I'm not a fan of how do_bind() is laid out, IMHO something like
this would be better (untested):

+def do_bind(self, dm_password=None, autobind=AUTOBIND_AUTO,
timeout=DEFAULT_TIMEOUT):
+if dm_password:
+self.do_simple_bind(bindpw=dm_password, timeout=timeout)
+return
+
+if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and
self.ldapi:
+try:
+# autobind
+pw_name = pwd.getpwuid(os.geteuid()).pw_name
+self.do_external_bind(pw_name, timeout=timeout)
+return
+except errors.NotFound:
+if autobind == AUTOBIND_ENABLED:
+# autobind was required and failed, raise
+# exception that it failed
+raise
+
+# Fall back
+self.do_sasl_gssapi_bind(timeout=timeout)


Honza


3) skipped as we discuss on IRC

Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1


1) The patch need a rebase on top of current ipa-4-1.

I can apply it (Am I doing something wrong?)



2) You can remove import pwd from service.py, it is no longer used there.


3) Are named constants for the autobind argument the right thing to 
do? It is a tri-state which can be expressed with None/True/False. 
(I'm just asking, I don't have a strong opinion on this.)



As we discussed on IRC, using None/True/False, is not good approach.
Updated patch attached

--
Martin Basti

From 60a3e9972e6154ce4628a1d210941e7d621d0a1d Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Wed, 27 Aug 2014 15:06:42 +0200
Subject: [PATCH] Refactoring of autobind, object_exists

Required to prevent code duplications

ipaldap.IPAdmin now has method do_bind, which tries several bind methods
ipaldap.IPAClient now has method object_exists(dn)
---
 install/tools/ipa-adtrust-install |  4 ++--
 ipapython/ipaldap.py  | 37 +
 ipaserver/install/bindinstance.py | 25 +
 ipaserver/install/cainstance.py   |  2 +-
 ipaserver/install/dsinstance.py   |  2 +-
 ipaserver/install/service.py  | 30 --
 6 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 7b616c1b65c60945a2e5dc19c4afc39dad285978..6e55bbe3e57f1c609398dc571e90cb8677d91a33 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -25,7 +25,7 @@ from ipaserver.install import adtrustinstance
 from ipaserver.install.installutils import *
 from ipaserver.install import service
 from ipapython import version
-from ipapython import ipautil, sysrestore
+from ipapython import ipautil, sysrestore, ipaldap
 from ipalib import api, errors, util
 from ipapython.config import IPAOptionParser
 import krbV
@@ -405,7 +405,7 @@ def main():
 
 smb = adtrustinstance.ADTRUSTInstance(fstore)
 smb.realm = api.env.realm
-smb.autobind = service.ENABLED
+smb.autobind = ipaldap.AUTOBIND_ENABLED
 smb.setup(api.env.host, ip_address, api.env.realm, api.env.domain,
   netbios_name, reset_netbios_name,
   options.rid_base, options.secondary_rid_base,
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 2818f787bd8a78b358e27f88de6ccdb214011986..1702daa253d7eb568c27f66fda1810b4661656ad 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -27,6 +27,8 @@ from decimal import Decimal
 from copy import deepcopy
 import contextlib
 import collections
+import os
+import pwd
 
 import ldap
 import ldap.sasl
@@ -53,6 +55,10 @@ _debug_log_ldap = False
 
 _missing = object()
 
+# Autobind modes
+AUTOBIND_AUTO = 1
+AUTOBIND_ENABLED = 2
+AUTOBIND_DISABLED = 3
 
 def unicode_from_utf8(val):
 '''
@@ -1633,6 +1639,18 @@ class LDAPClient(object):
 with self.error_handler():
 self.conn.delete_s(dn)
 
+def entry_exists(self, dn):
+
+Test whether the given object exists in LDAP.
+
+assert isinstance(dn, DN)
+try:
+self.get_entry(dn, attrs_list=[])
+except errors.NotFound:
+return False
+else:
+return True
+
 
 class IPAdmin(LDAPClient):
 
@@ -1742,6 +1760,25 @@ class IPAdmin(LDAPClient):
 self.__bind_with_wait(
 

Re: [Freeipa-devel] [PATCH 0116] Refactoring of service autobind

2014-09-25 Thread Jan Cholasta

Dne 25.9.2014 v 10:51 Martin Basti napsal(a):

On 19/09/14 14:30, Jan Cholasta wrote:

Dne 19.9.2014 v 13:32 Martin Basti napsal(a):

On 01/09/14 16:26, Martin Basti wrote:

On 28/08/14 14:01, Jan Cholasta wrote:

Hi,

Dne 27.8.2014 v 15:22 Martin Basti napsal(a):

Patch attached.



1) Please rename object_exists to entry_exists.


2) Use empty attribute list in get_entry() in
object_exists/entry_exists.


3) Please update LDAPObject.get_dn_if_exists() to use
object_exists/entry_exists.


4) I'm not a fan of how do_bind() is laid out, IMHO something like
this would be better (untested):

+def do_bind(self, dm_password=None, autobind=AUTOBIND_AUTO,
timeout=DEFAULT_TIMEOUT):
+if dm_password:
+self.do_simple_bind(bindpw=dm_password, timeout=timeout)
+return
+
+if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and
self.ldapi:
+try:
+# autobind
+pw_name = pwd.getpwuid(os.geteuid()).pw_name
+self.do_external_bind(pw_name, timeout=timeout)
+return
+except errors.NotFound:
+if autobind == AUTOBIND_ENABLED:
+# autobind was required and failed, raise
+# exception that it failed
+raise
+
+# Fall back
+self.do_sasl_gssapi_bind(timeout=timeout)


Honza


3) skipped as we discuss on IRC

Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1


1) The patch need a rebase on top of current ipa-4-1.

I can apply it (Am I doing something wrong?)



2) You can remove import pwd from service.py, it is no longer used there.


3) Are named constants for the autobind argument the right thing to
do? It is a tri-state which can be expressed with None/True/False.
(I'm just asking, I don't have a strong opinion on this.)


As we discussed on IRC, using None/True/False, is not good approach.
Updated patch attached



ACK, but the patch still does not apply cleanly on ipa-4-1:

$ git apply 
freeipa-mbasti-0116.3-Refactoring-of-autobind-object_exists.patch

error: patch failed: ipaserver/install/service.py:20
error: ipaserver/install/service.py: patch does not apply

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0116] Refactoring of service autobind

2014-09-25 Thread Martin Basti

On 25/09/14 14:47, Jan Cholasta wrote:

Dne 25.9.2014 v 10:51 Martin Basti napsal(a):

On 19/09/14 14:30, Jan Cholasta wrote:

Dne 19.9.2014 v 13:32 Martin Basti napsal(a):

On 01/09/14 16:26, Martin Basti wrote:

On 28/08/14 14:01, Jan Cholasta wrote:

Hi,

Dne 27.8.2014 v 15:22 Martin Basti napsal(a):

Patch attached.



1) Please rename object_exists to entry_exists.


2) Use empty attribute list in get_entry() in
object_exists/entry_exists.


3) Please update LDAPObject.get_dn_if_exists() to use
object_exists/entry_exists.


4) I'm not a fan of how do_bind() is laid out, IMHO something like
this would be better (untested):

+def do_bind(self, dm_password=None, autobind=AUTOBIND_AUTO,
timeout=DEFAULT_TIMEOUT):
+if dm_password:
+self.do_simple_bind(bindpw=dm_password, 
timeout=timeout)

+return
+
+if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and
self.ldapi:
+try:
+# autobind
+pw_name = pwd.getpwuid(os.geteuid()).pw_name
+self.do_external_bind(pw_name, timeout=timeout)
+return
+except errors.NotFound:
+if autobind == AUTOBIND_ENABLED:
+# autobind was required and failed, raise
+# exception that it failed
+raise
+
+# Fall back
+self.do_sasl_gssapi_bind(timeout=timeout)


Honza


3) skipped as we discuss on IRC

Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1


1) The patch need a rebase on top of current ipa-4-1.

I can apply it (Am I doing something wrong?)



2) You can remove import pwd from service.py, it is no longer used 
there.



3) Are named constants for the autobind argument the right thing to
do? It is a tri-state which can be expressed with None/True/False.
(I'm just asking, I don't have a strong opinion on this.)


As we discussed on IRC, using None/True/False, is not good approach.
Updated patch attached



ACK, but the patch still does not apply cleanly on ipa-4-1:

$ git apply 
freeipa-mbasti-0116.3-Refactoring-of-autobind-object_exists.patch

error: patch failed: ipaserver/install/service.py:20
error: ipaserver/install/service.py: patch does not apply


Rebased patches attached

--
Martin Basti

From 851ab5beecfccfcc61323ef9127c61098c3d54be Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Wed, 27 Aug 2014 15:06:42 +0200
Subject: [PATCH] Refactoring of autobind, object_exists

Required to prevent code duplications

ipaldap.IPAdmin now has method do_bind, which tries several bind methods
ipaldap.IPAClient now has method object_exists(dn)
---
 install/tools/ipa-adtrust-install |  4 ++--
 ipapython/ipaldap.py  | 37 +
 ipaserver/install/bindinstance.py | 25 +
 ipaserver/install/cainstance.py   |  2 +-
 ipaserver/install/dsinstance.py   |  2 +-
 ipaserver/install/service.py  | 30 --
 6 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 7b616c1b65c60945a2e5dc19c4afc39dad285978..6e55bbe3e57f1c609398dc571e90cb8677d91a33 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -25,7 +25,7 @@ from ipaserver.install import adtrustinstance
 from ipaserver.install.installutils import *
 from ipaserver.install import service
 from ipapython import version
-from ipapython import ipautil, sysrestore
+from ipapython import ipautil, sysrestore, ipaldap
 from ipalib import api, errors, util
 from ipapython.config import IPAOptionParser
 import krbV
@@ -405,7 +405,7 @@ def main():
 
 smb = adtrustinstance.ADTRUSTInstance(fstore)
 smb.realm = api.env.realm
-smb.autobind = service.ENABLED
+smb.autobind = ipaldap.AUTOBIND_ENABLED
 smb.setup(api.env.host, ip_address, api.env.realm, api.env.domain,
   netbios_name, reset_netbios_name,
   options.rid_base, options.secondary_rid_base,
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 2818f787bd8a78b358e27f88de6ccdb214011986..1702daa253d7eb568c27f66fda1810b4661656ad 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -27,6 +27,8 @@ from decimal import Decimal
 from copy import deepcopy
 import contextlib
 import collections
+import os
+import pwd
 
 import ldap
 import ldap.sasl
@@ -53,6 +55,10 @@ _debug_log_ldap = False
 
 _missing = object()
 
+# Autobind modes
+AUTOBIND_AUTO = 1
+AUTOBIND_ENABLED = 2
+AUTOBIND_DISABLED = 3
 
 def unicode_from_utf8(val):
 '''
@@ -1633,6 +1639,18 @@ class LDAPClient(object):
 with self.error_handler():
 self.conn.delete_s(dn)
 
+def entry_exists(self, dn):
+
+Test whether the given 

Re: [Freeipa-devel] [PATCH 0116] Refactoring of service autobind

2014-09-19 Thread Martin Basti

On 01/09/14 16:26, Martin Basti wrote:

On 28/08/14 14:01, Jan Cholasta wrote:

Hi,

Dne 27.8.2014 v 15:22 Martin Basti napsal(a):

Patch attached.



1) Please rename object_exists to entry_exists.


2) Use empty attribute list in get_entry() in 
object_exists/entry_exists.



3) Please update LDAPObject.get_dn_if_exists() to use 
object_exists/entry_exists.



4) I'm not a fan of how do_bind() is laid out, IMHO something like 
this would be better (untested):


+def do_bind(self, dm_password=None, autobind=AUTOBIND_AUTO, 
timeout=DEFAULT_TIMEOUT):

+if dm_password:
+self.do_simple_bind(bindpw=dm_password, timeout=timeout)
+return
+
+if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and 
self.ldapi:

+try:
+# autobind
+pw_name = pwd.getpwuid(os.geteuid()).pw_name
+self.do_external_bind(pw_name, timeout=timeout)
+return
+except errors.NotFound:
+if autobind == AUTOBIND_ENABLED:
+# autobind was required and failed, raise
+# exception that it failed
+raise
+
+# Fall back
+self.do_sasl_gssapi_bind(timeout=timeout)


Honza


3) skipped as we discuss on IRC

Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1

--
Martin Basti

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0116] Refactoring of service autobind

2014-09-19 Thread Jan Cholasta

Dne 19.9.2014 v 13:32 Martin Basti napsal(a):

On 01/09/14 16:26, Martin Basti wrote:

On 28/08/14 14:01, Jan Cholasta wrote:

Hi,

Dne 27.8.2014 v 15:22 Martin Basti napsal(a):

Patch attached.



1) Please rename object_exists to entry_exists.


2) Use empty attribute list in get_entry() in
object_exists/entry_exists.


3) Please update LDAPObject.get_dn_if_exists() to use
object_exists/entry_exists.


4) I'm not a fan of how do_bind() is laid out, IMHO something like
this would be better (untested):

+def do_bind(self, dm_password=None, autobind=AUTOBIND_AUTO,
timeout=DEFAULT_TIMEOUT):
+if dm_password:
+self.do_simple_bind(bindpw=dm_password, timeout=timeout)
+return
+
+if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and
self.ldapi:
+try:
+# autobind
+pw_name = pwd.getpwuid(os.geteuid()).pw_name
+self.do_external_bind(pw_name, timeout=timeout)
+return
+except errors.NotFound:
+if autobind == AUTOBIND_ENABLED:
+# autobind was required and failed, raise
+# exception that it failed
+raise
+
+# Fall back
+self.do_sasl_gssapi_bind(timeout=timeout)


Honza


3) skipped as we discuss on IRC

Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1


1) The patch need a rebase on top of current ipa-4-1.


2) You can remove import pwd from service.py, it is no longer used there.


3) Are named constants for the autobind argument the right thing to do? 
It is a tri-state which can be expressed with None/True/False. (I'm just 
asking, I don't have a strong opinion on this.)


--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0116] Refactoring of service autobind

2014-09-19 Thread Martin Basti

On 19/09/14 14:30, Jan Cholasta wrote:

Dne 19.9.2014 v 13:32 Martin Basti napsal(a):

On 01/09/14 16:26, Martin Basti wrote:

On 28/08/14 14:01, Jan Cholasta wrote:

Hi,

Dne 27.8.2014 v 15:22 Martin Basti napsal(a):

Patch attached.



1) Please rename object_exists to entry_exists.


2) Use empty attribute list in get_entry() in
object_exists/entry_exists.


3) Please update LDAPObject.get_dn_if_exists() to use
object_exists/entry_exists.


4) I'm not a fan of how do_bind() is laid out, IMHO something like
this would be better (untested):

+def do_bind(self, dm_password=None, autobind=AUTOBIND_AUTO,
timeout=DEFAULT_TIMEOUT):
+if dm_password:
+self.do_simple_bind(bindpw=dm_password, timeout=timeout)
+return
+
+if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and
self.ldapi:
+try:
+# autobind
+pw_name = pwd.getpwuid(os.geteuid()).pw_name
+self.do_external_bind(pw_name, timeout=timeout)
+return
+except errors.NotFound:
+if autobind == AUTOBIND_ENABLED:
+# autobind was required and failed, raise
+# exception that it failed
+raise
+
+# Fall back
+self.do_sasl_gssapi_bind(timeout=timeout)


Honza


3) skipped as we discuss on IRC

Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1



Thank you for review


1) The patch need a rebase on top of current ipa-4-1.


2) You can remove import pwd from service.py, it is no longer used there.


3) Are named constants for the autobind argument the right thing to 
do? It is a tri-state which can be expressed with None/True/False. 
(I'm just asking, I don't have a strong opinion on this.)



Seems like good Idea to me,
is clear enough to have AUTO=None, DISABLED=False, ENABLED=True?

--
Martin Basti

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0116] Refactoring of service autobind

2014-09-19 Thread Jan Cholasta

Dne 19.9.2014 v 14:39 Martin Basti napsal(a):

On 19/09/14 14:30, Jan Cholasta wrote:

Dne 19.9.2014 v 13:32 Martin Basti napsal(a):

On 01/09/14 16:26, Martin Basti wrote:

On 28/08/14 14:01, Jan Cholasta wrote:

Hi,

Dne 27.8.2014 v 15:22 Martin Basti napsal(a):

Patch attached.



1) Please rename object_exists to entry_exists.


2) Use empty attribute list in get_entry() in
object_exists/entry_exists.


3) Please update LDAPObject.get_dn_if_exists() to use
object_exists/entry_exists.


4) I'm not a fan of how do_bind() is laid out, IMHO something like
this would be better (untested):

+def do_bind(self, dm_password=None, autobind=AUTOBIND_AUTO,
timeout=DEFAULT_TIMEOUT):
+if dm_password:
+self.do_simple_bind(bindpw=dm_password, timeout=timeout)
+return
+
+if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and
self.ldapi:
+try:
+# autobind
+pw_name = pwd.getpwuid(os.geteuid()).pw_name
+self.do_external_bind(pw_name, timeout=timeout)
+return
+except errors.NotFound:
+if autobind == AUTOBIND_ENABLED:
+# autobind was required and failed, raise
+# exception that it failed
+raise
+
+# Fall back
+self.do_sasl_gssapi_bind(timeout=timeout)


Honza


3) skipped as we discuss on IRC

Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1



Thank you for review


1) The patch need a rebase on top of current ipa-4-1.


2) You can remove import pwd from service.py, it is no longer used there.


3) Are named constants for the autobind argument the right thing to
do? It is a tri-state which can be expressed with None/True/False.
(I'm just asking, I don't have a strong opinion on this.)


Seems like good Idea to me,
is clear enough to have AUTO=None, DISABLED=False, ENABLED=True?



Well, I'm not sure, that's why I'm asking :)

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0116] Refactoring of service autobind

2014-09-01 Thread Martin Basti

On 28/08/14 14:01, Jan Cholasta wrote:

Hi,

Dne 27.8.2014 v 15:22 Martin Basti napsal(a):

Patch attached.



1) Please rename object_exists to entry_exists.


2) Use empty attribute list in get_entry() in object_exists/entry_exists.


3) Please update LDAPObject.get_dn_if_exists() to use 
object_exists/entry_exists.



4) I'm not a fan of how do_bind() is laid out, IMHO something like 
this would be better (untested):


+def do_bind(self, dm_password=None, autobind=AUTOBIND_AUTO, 
timeout=DEFAULT_TIMEOUT):

+if dm_password:
+self.do_simple_bind(bindpw=dm_password, timeout=timeout)
+return
+
+if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and 
self.ldapi:

+try:
+# autobind
+pw_name = pwd.getpwuid(os.geteuid()).pw_name
+self.do_external_bind(pw_name, timeout=timeout)
+return
+except errors.NotFound:
+if autobind == AUTOBIND_ENABLED:
+# autobind was required and failed, raise
+# exception that it failed
+raise
+
+# Fall back
+self.do_sasl_gssapi_bind(timeout=timeout)


Honza


3) skipped as we discuss on IRC

Updated patch attached

--
Martin Basti

From 6814a74a5fc588b145bf6767a6681d61927e93df Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Wed, 27 Aug 2014 15:06:42 +0200
Subject: [PATCH] Refactoring of autobind, object_exists

Required to prevent code duplications

ipaldap.IPAdmin now has method do_bind, which tries several bind methods
ipaldap.IPAClient now has method object_exists(dn)
---
 install/tools/ipa-adtrust-install |  4 ++--
 ipapython/ipaldap.py  | 37 +
 ipaserver/install/bindinstance.py | 25 +
 ipaserver/install/cainstance.py   |  2 +-
 ipaserver/install/dsinstance.py   |  2 +-
 ipaserver/install/service.py  | 29 -
 6 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 7b616c1b65c60945a2e5dc19c4afc39dad285978..6e55bbe3e57f1c609398dc571e90cb8677d91a33 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -25,7 +25,7 @@ from ipaserver.install import adtrustinstance
 from ipaserver.install.installutils import *
 from ipaserver.install import service
 from ipapython import version
-from ipapython import ipautil, sysrestore
+from ipapython import ipautil, sysrestore, ipaldap
 from ipalib import api, errors, util
 from ipapython.config import IPAOptionParser
 import krbV
@@ -405,7 +405,7 @@ def main():
 
 smb = adtrustinstance.ADTRUSTInstance(fstore)
 smb.realm = api.env.realm
-smb.autobind = service.ENABLED
+smb.autobind = ipaldap.AUTOBIND_ENABLED
 smb.setup(api.env.host, ip_address, api.env.realm, api.env.domain,
   netbios_name, reset_netbios_name,
   options.rid_base, options.secondary_rid_base,
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 2818f787bd8a78b358e27f88de6ccdb214011986..1702daa253d7eb568c27f66fda1810b4661656ad 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -27,6 +27,8 @@ from decimal import Decimal
 from copy import deepcopy
 import contextlib
 import collections
+import os
+import pwd
 
 import ldap
 import ldap.sasl
@@ -53,6 +55,10 @@ _debug_log_ldap = False
 
 _missing = object()
 
+# Autobind modes
+AUTOBIND_AUTO = 1
+AUTOBIND_ENABLED = 2
+AUTOBIND_DISABLED = 3
 
 def unicode_from_utf8(val):
 '''
@@ -1633,6 +1639,18 @@ class LDAPClient(object):
 with self.error_handler():
 self.conn.delete_s(dn)
 
+def entry_exists(self, dn):
+
+Test whether the given object exists in LDAP.
+
+assert isinstance(dn, DN)
+try:
+self.get_entry(dn, attrs_list=[])
+except errors.NotFound:
+return False
+else:
+return True
+
 
 class IPAdmin(LDAPClient):
 
@@ -1742,6 +1760,25 @@ class IPAdmin(LDAPClient):
 self.__bind_with_wait(
 self.conn.sasl_interactive_bind_s, timeout, None, auth_tokens)
 
+def do_bind(self, dm_password=, autobind=AUTOBIND_AUTO, timeout=DEFAULT_TIMEOUT):
+if dm_password:
+self.do_simple_bind(bindpw=dm_password, timeout=timeout)
+return
+if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and self.ldapi:
+try:
+# autobind
+pw_name = pwd.getpwuid(os.geteuid()).pw_name
+self.do_external_bind(pw_name, timeout=timeout)
+return
+except errors.NotFound, e:
+if autobind == AUTOBIND_ENABLED:
+# autobind was required and failed, raise
+# exception that it failed
+raise
+
+#fall back
+   

Re: [Freeipa-devel] [PATCH 0116] Refactoring of service autobind

2014-08-28 Thread Jan Cholasta

Hi,

Dne 27.8.2014 v 15:22 Martin Basti napsal(a):

Patch attached.



1) Please rename object_exists to entry_exists.


2) Use empty attribute list in get_entry() in object_exists/entry_exists.


3) Please update LDAPObject.get_dn_if_exists() to use 
object_exists/entry_exists.



4) I'm not a fan of how do_bind() is laid out, IMHO something like this 
would be better (untested):


+def do_bind(self, dm_password=None, autobind=AUTOBIND_AUTO, 
timeout=DEFAULT_TIMEOUT):

+if dm_password:
+self.do_simple_bind(bindpw=dm_password, timeout=timeout)
+return
+
+if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and 
self.ldapi:

+try:
+# autobind
+pw_name = pwd.getpwuid(os.geteuid()).pw_name
+self.do_external_bind(pw_name, timeout=timeout)
+return
+except errors.NotFound:
+if autobind == AUTOBIND_ENABLED:
+# autobind was required and failed, raise
+# exception that it failed
+raise
+
+# Fall back
+self.do_sasl_gssapi_bind(timeout=timeout)


Honza

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel