From 5573167cc8dea784b2e8e7d5e9d1a7c2085dadc8 Mon Sep 17 00:00:00 2001 From: Sharmin Choksey Date: Mon, 13 Oct 2014 21:13:11 -0700 Subject: [PATCH 1/2] The fix is a clean up of the hybrid driver to simplify and consolidate the logic where if a user fails authentication in ldap, it will default the SQL driver for authentication. Specifically it addresses - bug in conf definition StrOpts v/s ListOpts for built-in users configuration paramter - Heat templates using cfn-signal where dynamic users are created by keystone - Consolidate the hybrid driver used between nimbus and telstra codebase Rally-bug: DE778 (Admin can create non-buildin user without checking LDAP via Keystone Hybrid Driver) Upstream: False --- keystone/identity/backends/hybrid.py | 146 ++++++++++----------------- 1 file changed, 53 insertions(+), 93 deletions(-) diff --git a/keystone/identity/backends/hybrid.py b/keystone/identity/backends/hybrid.py index cf58e8599..a601346c2 100644 --- a/keystone/identity/backends/hybrid.py +++ b/keystone/identity/backends/hybrid.py @@ -5,12 +5,17 @@ from keystone.identity.backends import sql from keystone.identity.backends import ldap from keystone.openstack.common import log as logging -from keystone import config +from oslo.config import cfg +from keystone import config as ks_cfg from keystone.common.ldap import core -import uuid import re -CONF = config.CONF +CONF = ks_cfg.CONF +# create and register custom opts +ks_cfg.CONF.register_opt(cfg.StrOpt('user_suffix'), group='ldap') +ks_cfg.CONF.register_opt(cfg.StrOpt('generic_tree_dn'), group='ldap') +ks_cfg.CONF.register_opt(cfg.StrOpt('builtin_users'), group='ldap') + DEFAULT_DOMAIN_ID = CONF.identity.default_domain_id LDAP_BIND_USER = CONF.ldap.user LDAP_BIND_PASSWORD = CONF.ldap.password @@ -24,120 +29,75 @@ LOG = logging.getLogger(__name__) - class Identity(sql.Identity): def __init__(self): super(Identity, self).__init__() - #TODO:: (schoksey) Inject as an additional driver from CONF file self.ldap_identity_api = ldap.Identity() def authenticate(self, user_id=None, password=None): """Authenticate based Cisco AD - The tenant will be defaulted to the value in keystone.conf - 1. if user is any of the built-in Openstack users, then do sql authC - 2. if user is non-built-in type, then perform AD authentication - 3. check if non-built-in user type is already in ks db, if not then add to default admin tenant - NOTE: Password is NEVER persisted in the keystone db + (schoksey): general flow + 0. Check if it is an openstack service user and default to SQL driver authentication + 1. If not, in built-in user list, lookup user in LDAP first to verify if user exists in LDAP + 2. If it does, then authenticate against ldap + 3. If it does not, then default to the keystone SQLDriver authentication + NOTE: Password is NEVER persisted in the keystone db for LDAP non-local users """ - LOG.debug("Authenticating against HYBRID Driver") - try: - user_ref = self.get_user(user_id) - except exception.UserNotFound: - pass - - if self._is_built_in_user(user_ref=user_ref, username=None): - return super(Identity, self).authenticate(user_ref.get('id'), password) - - try: - conn = self.ldap_identity_api.user.get_connection(self._resolve_cn_suffix(user_ref.get('name')), password) - if not conn: - raise AssertionError('Invalid user / password') - except Exception: - raise AssertionError('Invalid user / password') - - if not (user_ref.get('enabled')): - user = {} - user['enabled'] = True - self.update_user(user_id, user) + user_ref = self.get_user(user_id) + #try: + # user_ref = self.get_user(user_id) + #except exception.UserNotFound: + # pass - return (identity.filter_user(self.get_user(user_id))) + if self._is_built_in_user(user_ref.get('name')): + return super(Identity, self).authenticate(user_ref.get('id'), password) - def get_user_by_name(self, user_name, domain_id): - # do ldap look up on user validity - # if valid, then create a record in mysql with enabled status = False and return reference - # in subsequent calls to authentication, - # this user_id will be passed to do final authentication against ldap, if passed, update the enabled flag to True try: - return identity.filter_user(super(Identity, self).get_user_by_name(user_name, domain_id)) - except exception.UserNotFound: - if user_name != self._lookup_username_in_ad(user_name): - raise exception.UserNotFound(user_id=user_name) - - new_user_dict = self._build_new_user_dict(user_name, - self._resolve_cn_suffix(user_name), - enabled=False) - new_user_ref = self.create_user(new_user_dict['id'], new_user_dict) - - if 'tenantId' in new_user_dict and new_user_dict['tenantId'] is not None: - self.add_user_to_project(new_user_dict['tenantId'], new_user_dict['id']) - - return new_user_ref - - - def _build_new_user_dict(self, username, email, tenant=None, enabled=False, domain_id=DEFAULT_DOMAIN_ID): - new_user_ref = {} - if (tenant is not None): - new_user_ref['tenantId'] = (self.get_project_by_name(tenant, domain_id)).get('id') - new_user_ref['id'] = uuid.uuid4().hex - new_user_ref['name'] = username - new_user_ref['domain_id'] = domain_id - new_user_ref['email'] = email - new_user_ref['enabled'] = enabled - - LOG.debug("new_user_ref:****************** %s", new_user_ref) - - return new_user_ref - - - def _is_built_in_user(self, user_ref, username): - if user_ref is None and username is None: - return False - if username is None: - username = user_ref.get('name') - if username in LDAP_BUILTIN_USERS or re.search('-brokerWaitHandle-', username, re.IGNORECASE): - return True - else: - return False + ldap_user = self._lookup_username_in_ad(user_ref.get('name')) + + if ldap_user is not None \ + and user_ref.get('name') == self._lookup_username_in_ad(user_ref.get('name')): + conn = self.ldap_identity_api.user.get_connection( + self._resolve_cn_suffix(user_ref.get('name')), password) + if not conn: + raise AssertionError('Invalid user / password') + else: + if not (user_ref.get('enabled')): + user = {} + user['enabled'] = True + self.update_user(user_id, user) + return (identity.filter_user(self.get_user(user_id))) + else: + return super(Identity, self).authenticate(user_ref.get('id'), password) + except Exception as error: + LOG.error("EXCEPTION : %s" % error.message) + raise AssertionError(error.message) def _resolve_cn_suffix(self, user_id): - return ''.join([user_id,LDAP_CN_SUFFIX]) if LDAP_CN_SUFFIX else user_id + return ''.join([LDAP_USER_ID_ATTRIBUTE,'=',user_id,',',LDAP_CN_SUFFIX]) if LDAP_CN_SUFFIX else user_id def _lookup_username_in_ad(self, username): - conn = self.ldap_identity_api.user.get_connection(self._resolve_cn_suffix(LDAP_BIND_USER), LDAP_BIND_PASSWORD) + conn = self.ldap_identity_api.user.get_connection(LDAP_BIND_USER, LDAP_BIND_PASSWORD) baseDN = self._resolve_baseDN(username) - query = "(&({}={})(objectClass={}))".format(LDAP_USER_ID_ATTRIBUTE,username,LDAP_USER_OBJECT_CLASS) - attrlist = [LDAP_USER_ID_ATTRIBUTE] - o = conn.search_s(baseDN, core.LDAP_SCOPES.get('one'), query, attrlist) - return (o[0][1])[LDAP_USER_ID_ATTRIBUTE][0] + + query = "(&({}={})(objectClass={}))".format(LDAP_USER_ID_ATTRIBUTE,username,LDAP_USER_OBJECT_CLASS) + attr_list = [LDAP_USER_ID_ATTRIBUTE] + o = conn.search_s(baseDN, core.LDAP_SCOPES.get('one'), query, attr_list) + if o is not None and len(o) > 0: + return (o[0][1])[LDAP_USER_ID_ATTRIBUTE][0] def _resolve_baseDN(self, username): return LDAP_GENERIC_TREE_DN if re.search('\.gen$', username) else LDAP_USER_TREE_DN - - def create_user(self, user_id, user): - try: - if not self._is_built_in_user(user_ref=None, username=user['name']): - username = self._lookup_username_in_ad(user['name']) - if user['name'] != self._lookup_username_in_ad(username): - raise exception.UserNotFound(user_id=username) - except Exception: - raise AssertionError('Invalid user / password') - - return super(Identity, self).create_user(user_id, user) + def _is_built_in_user(self, username): + if username in LDAP_BUILTIN_USERS: + return True + else: + return False From 480e4aaaa5534652adc95fb0f11521e90edffa32 Mon Sep 17 00:00:00 2001 From: Sharmin Choksey Date: Mon, 13 Oct 2014 21:33:10 -0700 Subject: [PATCH 2/2] - convert from StrOpts to ListOpts for builtin_user param The patch cleans up config parameter proliferation for hybrid driver to enable isolation of the hybrid driver code. All parameters related to hybrid are not part of the driver code itself. Rally-bug: None (forward-looking enhancement for Icehouse / RHEL supportability Upstream: False --- keystone/common/config.py | 7 +------ keystone/identity/backends/hybrid.py | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/keystone/common/config.py b/keystone/common/config.py index 7f3807db8..dd14bb50e 100644 --- a/keystone/common/config.py +++ b/keystone/common/config.py @@ -238,12 +238,7 @@ cfg.StrOpt('tls_cacertfile', default=None), cfg.StrOpt('tls_cacertdir', default=None), cfg.BoolOpt('use_tls', default=False), - cfg.StrOpt('tls_req_cert', default='demand'), - - # (schoksey): start - New attributes added for Generic DN,user attribute suffix,built-in users list - cfg.StrOpt('user_suffix', default=None), - cfg.StrOpt('generic_tree_dn', default=None), - cfg.StrOpt('builtin_users', default=[])], + cfg.StrOpt('tls_req_cert', default='demand')], 'pam': [ cfg.StrOpt('userid', default=None), diff --git a/keystone/identity/backends/hybrid.py b/keystone/identity/backends/hybrid.py index a601346c2..c4402d61d 100644 --- a/keystone/identity/backends/hybrid.py +++ b/keystone/identity/backends/hybrid.py @@ -14,7 +14,7 @@ # create and register custom opts ks_cfg.CONF.register_opt(cfg.StrOpt('user_suffix'), group='ldap') ks_cfg.CONF.register_opt(cfg.StrOpt('generic_tree_dn'), group='ldap') -ks_cfg.CONF.register_opt(cfg.StrOpt('builtin_users'), group='ldap') +ks_cfg.CONF.register_opt(cfg.ListOpt('builtin_users'), group='ldap') DEFAULT_DOMAIN_ID = CONF.identity.default_domain_id LDAP_BIND_USER = CONF.ldap.user