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
[email protected]
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 <[email protected]>
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(
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
+ self.do_sasl_gssapi_bind(timeout=timeout)
+
def modify_s(self, *args, **kwargs):
# FIXME: for backwards compatibility only
return self.conn.modify_s(*args, **kwargs)
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 9a27c781764f3dc311d20cfcf9150fde31307b03..dfb086f109371ae97151622e2c44db27256896db 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -202,23 +202,11 @@ def named_conf_set_directive(name, value, section=NAMED_SECTION_IPA,
with open(NAMED_CONF, 'w') as f:
f.write("".join(new_lines))
-def dns_container_exists(fqdn, suffix, dm_password=None, ldapi=False, realm=None):
+def dns_container_exists(fqdn, suffix, dm_password=None, ldapi=False, realm=None,
+ autobind=ipaldap.AUTOBIND_DISABLED):
"""
Test whether the dns container exists.
"""
-
- def object_exists(dn): # FIXME, this should be a IPAdmin/ldap2 method so it can be shared
- """
- Test whether the given object exists in LDAP.
- """
- assert isinstance(dn, DN)
- try:
- conn.get_entry(dn)
- except errors.NotFound:
- return False
- else:
- return True
-
assert isinstance(suffix, DN)
try:
# At install time we may need to use LDAPI to avoid chicken/egg
@@ -228,14 +216,11 @@ def dns_container_exists(fqdn, suffix, dm_password=None, ldapi=False, realm=None
else:
conn = ipaldap.IPAdmin(host=fqdn, port=636, cacert=CACERT)
- if dm_password:
- conn.do_simple_bind(bindpw=dm_password)
- else:
- conn.do_sasl_gssapi_bind()
+ conn.do_bind(dm_password, autobind=autobind)
except ldap.SERVER_DOWN:
raise RuntimeError('LDAP server on %s is not responding. Is IPA installed?' % fqdn)
- ret = object_exists(DN(('cn', 'dns'), suffix))
+ ret = conn.entry_exists(DN(('cn', 'dns'), suffix))
conn.unbind()
return ret
@@ -461,7 +446,7 @@ class BindInstance(service.Service):
service_desc="DNS",
dm_password=dm_password,
ldapi=False,
- autobind=service.DISABLED
+ autobind=ipaldap.AUTOBIND_DISABLED
)
self.dns_backup = DnsBackup(self)
self.named_user = None
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 52e76b7dbf497090b7875b53b36ec5c7dc1627a7..04968d411fc5bc1073e86fab42743fc65f7b828a 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -272,7 +272,7 @@ class CADSInstance(service.Service):
service_desc="directory server for the CA",
dm_password=dm_password,
ldapi=False,
- autobind=service.DISABLED)
+ autobind=ipaldap.AUTOBIND_DISABLED)
self.serverid = "PKI-IPA"
self.realm = realm_name
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index ecef6b47ec61b0ef7b43649b17a6c633923c6a50..086d8d9e05228be537d296f88d051b787741b612 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -190,7 +190,7 @@ class DsInstance(service.Service):
service_desc="directory server",
dm_password=dm_password,
ldapi=False,
- autobind=service.DISABLED
+ autobind=ipaldap.AUTOBIND_DISABLED
)
self.nickname = 'Server-Cert'
self.dm_password = dm_password
diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 018c369ffb3c8f782b3d59b82e3bb71c898b9b9f..8fb802099d7e58a10fd8fb17ff5dc7511eb98c7f 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -20,7 +20,6 @@
import sys
import os, socket
import tempfile
-import pwd
import time
import datetime
import traceback
@@ -32,10 +31,6 @@ from ipalib import errors, certstore
from ipaplatform import services
from ipaplatform.paths import paths
-# Autobind modes
-AUTO = 1
-ENABLED = 2
-DISABLED = 3
# The service name as stored in cn=masters,cn=ipa,cn=etc. In the tuple
# the first value is the *nix service name, the second the start order.
@@ -74,7 +69,8 @@ def format_seconds(seconds):
class Service(object):
- def __init__(self, service_name, service_desc=None, sstore=None, dm_password=None, ldapi=True, autobind=AUTO):
+ def __init__(self, service_name, service_desc=None, sstore=None, dm_password=None, ldapi=True,
+ autobind=ipaldap.AUTOBIND_AUTO):
self.service_name = service_name
self.service_desc = service_desc
self.service = services.service(service_name)
@@ -110,26 +106,8 @@ class Service(object):
conn = ipaldap.IPAdmin(ldapi=self.ldapi, realm=self.realm)
else:
conn = ipaldap.IPAdmin(self.fqdn, port=389)
- if self.dm_password:
- conn.do_simple_bind(bindpw=self.dm_password)
- elif self.autobind in [AUTO, ENABLED]:
- if os.getegid() == 0 and self.ldapi:
- try:
- # autobind
- pw_name = pwd.getpwuid(os.geteuid()).pw_name
- conn.do_external_bind(pw_name)
- except errors.NotFound, e:
- if self.autobind == AUTO:
- # Fall back
- conn.do_sasl_gssapi_bind()
- else:
- # autobind was required and failed, raise
- # exception that it failed
- raise e
- else:
- conn.do_sasl_gssapi_bind()
- else:
- conn.do_sasl_gssapi_bind()
+
+ conn.do_bind(self.dm_password, autobind=self.autobind)
except Exception, e:
root_logger.debug("Could not connect to the Directory Server on %s: %s" % (self.fqdn, str(e)))
raise
--
1.8.3.1
From 60a3e9972e6154ce4628a1d210941e7d621d0a1d Mon Sep 17 00:00:00 2001
From: Martin Basti <[email protected]>
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(
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
+ self.do_sasl_gssapi_bind(timeout=timeout)
+
def modify_s(self, *args, **kwargs):
# FIXME: for backwards compatibility only
return self.conn.modify_s(*args, **kwargs)
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 9a27c781764f3dc311d20cfcf9150fde31307b03..dfb086f109371ae97151622e2c44db27256896db 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -202,23 +202,11 @@ def named_conf_set_directive(name, value, section=NAMED_SECTION_IPA,
with open(NAMED_CONF, 'w') as f:
f.write("".join(new_lines))
-def dns_container_exists(fqdn, suffix, dm_password=None, ldapi=False, realm=None):
+def dns_container_exists(fqdn, suffix, dm_password=None, ldapi=False, realm=None,
+ autobind=ipaldap.AUTOBIND_DISABLED):
"""
Test whether the dns container exists.
"""
-
- def object_exists(dn): # FIXME, this should be a IPAdmin/ldap2 method so it can be shared
- """
- Test whether the given object exists in LDAP.
- """
- assert isinstance(dn, DN)
- try:
- conn.get_entry(dn)
- except errors.NotFound:
- return False
- else:
- return True
-
assert isinstance(suffix, DN)
try:
# At install time we may need to use LDAPI to avoid chicken/egg
@@ -228,14 +216,11 @@ def dns_container_exists(fqdn, suffix, dm_password=None, ldapi=False, realm=None
else:
conn = ipaldap.IPAdmin(host=fqdn, port=636, cacert=CACERT)
- if dm_password:
- conn.do_simple_bind(bindpw=dm_password)
- else:
- conn.do_sasl_gssapi_bind()
+ conn.do_bind(dm_password, autobind=autobind)
except ldap.SERVER_DOWN:
raise RuntimeError('LDAP server on %s is not responding. Is IPA installed?' % fqdn)
- ret = object_exists(DN(('cn', 'dns'), suffix))
+ ret = conn.entry_exists(DN(('cn', 'dns'), suffix))
conn.unbind()
return ret
@@ -461,7 +446,7 @@ class BindInstance(service.Service):
service_desc="DNS",
dm_password=dm_password,
ldapi=False,
- autobind=service.DISABLED
+ autobind=ipaldap.AUTOBIND_DISABLED
)
self.dns_backup = DnsBackup(self)
self.named_user = None
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 45c72198df89cafd7de2775c1233f7cb4f0081fd..b0037dd568e2f4a115436a6cb621e10bcc02f4d9 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -271,7 +271,7 @@ class CADSInstance(service.Service):
service_desc="directory server for the CA",
dm_password=dm_password,
ldapi=False,
- autobind=service.DISABLED)
+ autobind=ipaldap.AUTOBIND_DISABLED)
self.serverid = "PKI-IPA"
self.realm = realm_name
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 0edd4ed635eff96e0d534848240dc30da2b4971e..6e79dc51fc2da03b4067f0f13dfd74ac201978f7 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -192,7 +192,7 @@ class DsInstance(service.Service):
service_desc="directory server",
dm_password=dm_password,
ldapi=False,
- autobind=service.DISABLED
+ autobind=ipaldap.AUTOBIND_DISABLED
)
self.nickname = 'Server-Cert'
self.dm_password = dm_password
diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 585c903bdd4dee66d93c426299d59de95d0ea625..7f812a99c557ae567cd1d8a0db30eba434c507f9 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -20,7 +20,6 @@
import sys
import os, socket
import tempfile
-import pwd
import time
import datetime
@@ -31,10 +30,6 @@ from ipalib import errors, certstore
from ipaplatform import services
from ipaplatform.paths import paths
-# Autobind modes
-AUTO = 1
-ENABLED = 2
-DISABLED = 3
# The service name as stored in cn=masters,cn=ipa,cn=etc. In the tuple
# the first value is the *nix service name, the second the start order.
@@ -73,7 +68,8 @@ def format_seconds(seconds):
class Service(object):
- def __init__(self, service_name, service_desc=None, sstore=None, dm_password=None, ldapi=True, autobind=AUTO):
+ def __init__(self, service_name, service_desc=None, sstore=None, dm_password=None, ldapi=True,
+ autobind=ipaldap.AUTOBIND_AUTO):
self.service_name = service_name
self.service_desc = service_desc
self.service = services.service(service_name)
@@ -109,26 +105,8 @@ class Service(object):
conn = ipaldap.IPAdmin(ldapi=self.ldapi, realm=self.realm)
else:
conn = ipaldap.IPAdmin(self.fqdn, port=389)
- if self.dm_password:
- conn.do_simple_bind(bindpw=self.dm_password)
- elif self.autobind in [AUTO, ENABLED]:
- if os.getegid() == 0 and self.ldapi:
- try:
- # autobind
- pw_name = pwd.getpwuid(os.geteuid()).pw_name
- conn.do_external_bind(pw_name)
- except errors.NotFound, e:
- if self.autobind == AUTO:
- # Fall back
- conn.do_sasl_gssapi_bind()
- else:
- # autobind was required and failed, raise
- # exception that it failed
- raise e
- else:
- conn.do_sasl_gssapi_bind()
- else:
- conn.do_sasl_gssapi_bind()
+
+ conn.do_bind(self.dm_password, autobind=self.autobind)
except Exception, e:
root_logger.debug("Could not connect to the Directory Server on %s: %s" % (self.fqdn, str(e)))
raise
--
1.8.3.1
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
