Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 10/28/2014 11:37 AM, lvroyce0210@gmail.com wrote:

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Add LDAP configuration to specify LDAP server, search base and filter for query user.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi.conf.in | 14 ++++++++++++++ src/kimchi/config.py.in | 5 +++++ 2 files changed, 19 insertions(+)

diff --git a/src/kimchi.conf.in b/src/kimchi.conf.in index ea39292..62eb40b 100644 --- a/src/kimchi.conf.in +++ b/src/kimchi.conf.in @@ -43,3 +43,17 @@ [display] # Port for websocket proxy to listen on #display_proxy_port = 64667 + +[authentication] +# Authentication method, available option: pam, ldap. +# method = pam + +# If specified method to ldap, following fields need to be specified. +# ldap server domain name used to authenticate. +# ldap_server = "localhost" + +# Search tree base in ldap +# ldap_search_base = "ou=People, dc=kimchi, dc=org" + +# User id filter +# ldap_search_filter = "uid=%(username)s" diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in index 097c017..887fe63 100644 --- a/src/kimchi/config.py.in +++ b/src/kimchi/config.py.in @@ -259,6 +259,11 @@ def _get_config(): config.set("server", "environment", "production") config.set("server", "federation", "off") config.set('server', 'max_body_size', '4*1024*1024') + config.add_section("authentication") + config.set("authentication", "method", "pam") + config.set("authentication", "ldap_server", "") + config.set("authentication", "ldap_search_base", "") + config.set("authentication", "ldap_search_filter", "") config.add_section("logging") config.set("logging", "log_dir", paths.log_dir) config.set("logging", "log_level", DEFAULT_LOG_LEVEL)

On 10/28/2014 11:37 AM, lvroyce0210@gmail.com wrote:

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Split PAM authentication implementation and abstract a common class.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/auth.py | 113 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 73 insertions(+), 40 deletions(-)

diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index c8801a5..ad3f3ac 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -31,6 +31,7 @@ import urllib2

from kimchi import template +from kimchi.config import config from kimchi.exception import InvalidOperation, OperationFailed from kimchi.utils import get_all_tabs, run_command

@@ -39,6 +40,7 @@ USER_NAME = 'username' USER_GROUPS = 'groups' USER_ROLES = 'roles' REFRESH = 'robot-refresh' +AUTH = 'auth_method'

tabs = get_all_tabs()

@@ -59,6 +61,22 @@ def debug(msg):

class User(object): + @classmethod + def create(cls, auth_args):

"create" seems we will create an user which it is not true. Change it to "get"

+ auth_type = auth_args.pop('auth_type') + for klass in cls.__subclasses__(): + if auth_type == klass.auth_type: + try: + if not klass.authenticate(**auth_args): + debug("cannot verify user with the given password") + return None + except OperationFailed: + raise + return klass(auth_args['username']) + + +class PAMUser(User): + auth_type = "pam"

def __init__(self, username): self.user = {} @@ -124,40 +142,54 @@ class User(object): def get_user(self): return self.user

+ @staticmethod + def authenticate(username, password, service="passwd"): + '''Returns True if authenticate is OK via PAM.''' + def _pam_conv(auth, query_list, userData=None): + resp = [] + for i in range(len(query_list)): + query, qtype = query_list[i] + if qtype == PAM.PAM_PROMPT_ECHO_ON: + resp.append((username, 0)) + elif qtype == PAM.PAM_PROMPT_ECHO_OFF: + resp.append((password, 0)) + elif qtype == PAM.PAM_PROMPT_ERROR_MSG: + cherrypy.log.error_log.error( + "PAM authenticate prompt error: %s" % query) + resp.append(('', 0)) + elif qtype == PAM.PAM_PROMPT_TEXT_INFO: + resp.append(('', 0)) + else: + return None + return resp + + auth = PAM.pam() + auth.start(service) + auth.set_item(PAM.PAM_USER, username) + auth.set_item(PAM.PAM_CONV, _pam_conv) + try: + auth.authenticate() + except PAM.error, (resp, code): + msg_args = {'username': username, 'code': code} + raise OperationFailed("KCHAUTH0001E", msg_args)

-def authenticate(username, password, service="passwd"): - '''Returns True if authenticate is OK via PAM.''' - def _pam_conv(auth, query_list, userData=None): - resp = [] - for i in range(len(query_list)): - query, qtype = query_list[i] - if qtype == PAM.PAM_PROMPT_ECHO_ON: - resp.append((username, 0)) - elif qtype == PAM.PAM_PROMPT_ECHO_OFF: - resp.append((password, 0)) - elif qtype == PAM.PAM_PROMPT_ERROR_MSG: - cherrypy.log.error_log.error("PAM authenticate prompt error " - "message: %s" % query) - resp.append(('', 0)) - elif qtype == PAM.PAM_PROMPT_TEXT_INFO: - resp.append(('', 0)) - else: - return None - return resp - - auth = PAM.pam() - auth.start(service) - auth.set_item(PAM.PAM_USER, username) - auth.set_item(PAM.PAM_CONV, _pam_conv) - - try: - auth.authenticate() - except PAM.error: - raise - - return True + return True

+class LDAPUser(User): + auth_type = "ldap" + def __init__(self, username): + self.user = {} + self.user[USER_NAME] = username + self.user[USER_GROUPS] = None + # FIXME: user roles will be changed according roles assignment after + # objstore is integrated + self.user[USER_ROLES] = dict.fromkeys(tabs, 'user') + + @staticmethod + def authenticate(username, password): + return False + def from_browser(): # Enable Basic Authentication for REST tools. # Ajax request sent from jQuery in browser will have "X-Requested-With" @@ -216,21 +248,22 @@ def check_auth_httpba():

def login(username, password, **kwargs): - try: - if not authenticate(username, password): - debug("User cannot be verified with the supplied password") - return None - except PAM.error, (resp, code): - msg_args = {'username': username, 'code': code} - raise OperationFailed("KCHAUTH0001E", msg_args) - - user = User(username) + auth_args = {'auth_type': config.get("authentication", "method"), + 'username': username, + 'password': password} + + user = User.create(auth_args) + if not user: + debug("User cannot be verified with the supplied password") + return None + debug("User verified, establishing session") cherrypy.session.acquire_lock() cherrypy.session.regenerate() cherrypy.session[USER_NAME] = username cherrypy.session[USER_GROUPS] = user.get_groups() cherrypy.session[USER_ROLES] = user.get_roles() + cherrypy.session[AUTH] = config.get("authentication", "method") cherrypy.session[REFRESH] = time.time() cherrypy.session.release_lock() return user.get_user()

On 10/28/2014 11:37 AM, lvroyce0210@gmail.com wrote:

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Split PAM authentication implementation and abstract a common class.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/auth.py | 113 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 73 insertions(+), 40 deletions(-)

diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index c8801a5..ad3f3ac 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -31,6 +31,7 @@ import urllib2

from kimchi import template +from kimchi.config import config from kimchi.exception import InvalidOperation, OperationFailed from kimchi.utils import get_all_tabs, run_command

@@ -39,6 +40,7 @@ USER_NAME = 'username' USER_GROUPS = 'groups' USER_ROLES = 'roles' REFRESH = 'robot-refresh'

+AUTH = 'auth_method'

tabs = get_all_tabs()

@@ -59,6 +61,22 @@ def debug(msg):

class User(object): + @classmethod + def create(cls, auth_args): + auth_type = auth_args.pop('auth_type') + for klass in cls.__subclasses__(): + if auth_type == klass.auth_type: + try: + if not klass.authenticate(**auth_args): + debug("cannot verify user with the given password") + return None + except OperationFailed: + raise + return klass(auth_args['username']) + + +class PAMUser(User): + auth_type = "pam"

def __init__(self, username): self.user = {} @@ -124,40 +142,54 @@ class User(object): def get_user(self): return self.user

+ @staticmethod + def authenticate(username, password, service="passwd"): + '''Returns True if authenticate is OK via PAM.''' + def _pam_conv(auth, query_list, userData=None): + resp = [] + for i in range(len(query_list)): + query, qtype = query_list[i] + if qtype == PAM.PAM_PROMPT_ECHO_ON: + resp.append((username, 0)) + elif qtype == PAM.PAM_PROMPT_ECHO_OFF: + resp.append((password, 0)) + elif qtype == PAM.PAM_PROMPT_ERROR_MSG: + cherrypy.log.error_log.error( + "PAM authenticate prompt error: %s" % query) + resp.append(('', 0)) + elif qtype == PAM.PAM_PROMPT_TEXT_INFO: + resp.append(('', 0)) + else: + return None + return resp + + auth = PAM.pam() + auth.start(service) + auth.set_item(PAM.PAM_USER, username) + auth.set_item(PAM.PAM_CONV, _pam_conv) + try: + auth.authenticate() + except PAM.error, (resp, code): + msg_args = {'username': username, 'code': code} + raise OperationFailed("KCHAUTH0001E", msg_args)

-def authenticate(username, password, service="passwd"): - '''Returns True if authenticate is OK via PAM.''' - def _pam_conv(auth, query_list, userData=None): - resp = [] - for i in range(len(query_list)): - query, qtype = query_list[i] - if qtype == PAM.PAM_PROMPT_ECHO_ON: - resp.append((username, 0)) - elif qtype == PAM.PAM_PROMPT_ECHO_OFF: - resp.append((password, 0)) - elif qtype == PAM.PAM_PROMPT_ERROR_MSG: - cherrypy.log.error_log.error("PAM authenticate prompt error " - "message: %s" % query) - resp.append(('', 0)) - elif qtype == PAM.PAM_PROMPT_TEXT_INFO: - resp.append(('', 0)) - else: - return None - return resp - - auth = PAM.pam() - auth.start(service) - auth.set_item(PAM.PAM_USER, username) - auth.set_item(PAM.PAM_CONV, _pam_conv) - - try: - auth.authenticate() - except PAM.error: - raise - - return True + return True

+class LDAPUser(User): + auth_type = "ldap" + def __init__(self, username): + self.user = {} + self.user[USER_NAME] = username + self.user[USER_GROUPS] = None + # FIXME: user roles will be changed according roles assignment after + # objstore is integrated + self.user[USER_ROLES] = dict.fromkeys(tabs, 'user') + + @staticmethod + def authenticate(username, password): + return False + def from_browser(): # Enable Basic Authentication for REST tools. # Ajax request sent from jQuery in browser will have "X-Requested-With" @@ -216,21 +248,22 @@ def check_auth_httpba():

def login(username, password, **kwargs): - try: - if not authenticate(username, password): - debug("User cannot be verified with the supplied password") - return None - except PAM.error, (resp, code): - msg_args = {'username': username, 'code': code} - raise OperationFailed("KCHAUTH0001E", msg_args) - - user = User(username) + auth_args = {'auth_type': config.get("authentication", "method"), + 'username': username, + 'password': password} + + user = User.create(auth_args) + if not user: + debug("User cannot be verified with the supplied password") + return None + debug("User verified, establishing session") cherrypy.session.acquire_lock() cherrypy.session.regenerate() cherrypy.session[USER_NAME] = username cherrypy.session[USER_GROUPS] = user.get_groups() cherrypy.session[USER_ROLES] = user.get_roles()

+ cherrypy.session[AUTH] = config.get("authentication", "method")

Do we really need to store it on the session? It was not used in the other patches.

cherrypy.session[REFRESH] = time.time() cherrypy.session.release_lock() return user.get_user()

On 11/06/2014 04:16 AM, Royce Lv wrote:

On 2014年10月31日 01:09, Aline Manera wrote:

...

On 10/28/2014 11:37 AM, lvroyce0210@gmail.com wrote:

...

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Split PAM authentication implementation and abstract a common class.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/auth.py | 113 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 73 insertions(+), 40 deletions(-)

diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index c8801a5..ad3f3ac 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -31,6 +31,7 @@ import urllib2

from kimchi import template +from kimchi.config import config from kimchi.exception import InvalidOperation, OperationFailed from kimchi.utils import get_all_tabs, run_command

@@ -39,6 +40,7 @@ USER_NAME = 'username' USER_GROUPS = 'groups' USER_ROLES = 'roles' REFRESH = 'robot-refresh'

...

+AUTH = 'auth_method'

...

tabs = get_all_tabs()

@@ -59,6 +61,22 @@ def debug(msg):

class User(object): + @classmethod + def create(cls, auth_args): + auth_type = auth_args.pop('auth_type') + for klass in cls.__subclasses__(): + if auth_type == klass.auth_type: + try: + if not klass.authenticate(**auth_args): + debug("cannot verify user with the given password") + return None + except OperationFailed: + raise + return klass(auth_args['username']) + + +class PAMUser(User): + auth_type = "pam"

def __init__(self, username): self.user = {} @@ -124,40 +142,54 @@ class User(object): def get_user(self): return self.user

+ @staticmethod + def authenticate(username, password, service="passwd"): + '''Returns True if authenticate is OK via PAM.''' + def _pam_conv(auth, query_list, userData=None): + resp = [] + for i in range(len(query_list)): + query, qtype = query_list[i] + if qtype == PAM.PAM_PROMPT_ECHO_ON: + resp.append((username, 0)) + elif qtype == PAM.PAM_PROMPT_ECHO_OFF: + resp.append((password, 0)) + elif qtype == PAM.PAM_PROMPT_ERROR_MSG: + cherrypy.log.error_log.error( + "PAM authenticate prompt error: %s" % query) + resp.append(('', 0)) + elif qtype == PAM.PAM_PROMPT_TEXT_INFO: + resp.append(('', 0)) + else: + return None + return resp + + auth = PAM.pam() + auth.start(service) + auth.set_item(PAM.PAM_USER, username) + auth.set_item(PAM.PAM_CONV, _pam_conv) + try: + auth.authenticate() + except PAM.error, (resp, code): + msg_args = {'username': username, 'code': code} + raise OperationFailed("KCHAUTH0001E", msg_args)

-def authenticate(username, password, service="passwd"): - '''Returns True if authenticate is OK via PAM.''' - def _pam_conv(auth, query_list, userData=None): - resp = [] - for i in range(len(query_list)): - query, qtype = query_list[i] - if qtype == PAM.PAM_PROMPT_ECHO_ON: - resp.append((username, 0)) - elif qtype == PAM.PAM_PROMPT_ECHO_OFF: - resp.append((password, 0)) - elif qtype == PAM.PAM_PROMPT_ERROR_MSG: - cherrypy.log.error_log.error("PAM authenticate prompt error " - "message: %s" % query) - resp.append(('', 0)) - elif qtype == PAM.PAM_PROMPT_TEXT_INFO: - resp.append(('', 0)) - else: - return None - return resp - - auth = PAM.pam() - auth.start(service) - auth.set_item(PAM.PAM_USER, username) - auth.set_item(PAM.PAM_CONV, _pam_conv) - - try: - auth.authenticate() - except PAM.error: - raise - - return True + return True

+class LDAPUser(User): + auth_type = "ldap" + def __init__(self, username): + self.user = {} + self.user[USER_NAME] = username + self.user[USER_GROUPS] = None + # FIXME: user roles will be changed according roles assignment after + # objstore is integrated + self.user[USER_ROLES] = dict.fromkeys(tabs, 'user') + + @staticmethod + def authenticate(username, password): + return False + def from_browser(): # Enable Basic Authentication for REST tools. # Ajax request sent from jQuery in browser will have "X-Requested-With" @@ -216,21 +248,22 @@ def check_auth_httpba():

def login(username, password, **kwargs): - try: - if not authenticate(username, password): - debug("User cannot be verified with the supplied password") - return None - except PAM.error, (resp, code): - msg_args = {'username': username, 'code': code} - raise OperationFailed("KCHAUTH0001E", msg_args) - - user = User(username) + auth_args = {'auth_type': config.get("authentication", "method"), + 'username': username, + 'password': password} + + user = User.create(auth_args) + if not user: + debug("User cannot be verified with the supplied password") + return None + debug("User verified, establishing session") cherrypy.session.acquire_lock() cherrypy.session.regenerate() cherrypy.session[USER_NAME] = username cherrypy.session[USER_GROUPS] = user.get_groups() cherrypy.session[USER_ROLES] = user.get_roles()

...

+ cherrypy.session[AUTH] = config.get("authentication", "method")

Do we really need to store it on the session? It was not used in the other patches.

As UI will be interested in what kind of authentication backend is using to choose display "pam" permission tagging or "ldap" permission tagging After talked with YuXin, he suggested to move it to /host/capabilities rather than put it in cookie, because not every part of UI interested in authentication method, do you agree?

Yeap. Sounds good for me.

...

...

cherrypy.session[REFRESH] = time.time() cherrypy.session.release_lock() return user.get_user()

_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

Just a minor comment below: On 10/28/2014 11:37 AM, lvroyce0210@gmail.com wrote:

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Add LDAP authentication, also deals with invalid user, LDAP search base configure error and other LDAP errors.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- contrib/DEBIAN/control.in | 1 + contrib/kimchi.spec.fedora.in | 1 + contrib/kimchi.spec.suse.in | 1 + docs/README.md | 12 ++++++----- src/kimchi.conf.in | 3 +++ src/kimchi/auth.py | 48 +++++++++++++++++++++++++++++++++++++++++-- 6 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/contrib/DEBIAN/control.in b/contrib/DEBIAN/control.in index 7372a58..0721960 100644 --- a/contrib/DEBIAN/control.in +++ b/contrib/DEBIAN/control.in @@ -27,6 +27,7 @@ Depends: python-cherrypy3 (>= 3.2.0), firewalld, nginx, python-guestfs, + python-ldap, libguestfs-tools Build-Depends: libxslt, python-libxml2, diff --git a/contrib/kimchi.spec.fedora.in b/contrib/kimchi.spec.fedora.in index 2ca3076..fcb8c11 100644 --- a/contrib/kimchi.spec.fedora.in +++ b/contrib/kimchi.spec.fedora.in @@ -29,6 +29,7 @@ Requires: nfs-utils Requires: nginx Requires: iscsi-initiator-utils Requires: policycoreutils-python +Requires: python-ldap Requires: python-libguestfs Requires: libguestfs-tools BuildRequires: libxslt diff --git a/contrib/kimchi.spec.suse.in b/contrib/kimchi.spec.suse.in index 9ea240c..b8f0531 100644 --- a/contrib/kimchi.spec.suse.in +++ b/contrib/kimchi.spec.suse.in @@ -23,6 +23,7 @@ Requires: python-psutil >= 0.6.0 Requires: python-jsonschema >= 1.3.0 Requires: python-ethtool Requires: python-ipaddr +Requires: python-ldap Requires: python-lxml Requires: python-xml Requires: nfs-client diff --git a/docs/README.md b/docs/README.md index 7703037..db219db 100644 --- a/docs/README.md +++ b/docs/README.md @@ -52,7 +52,7 @@ Install Dependencies libvirt libxml2-python python-imaging \ PyPAM m2crypto python-jsonschema rpm-build \ qemu-kvm python-psutil python-ethtool sos \ - python-ipaddr python-lxml nfs-utils \ + python-ipaddr python-ldap python-lxml nfs-utils \ iscsi-initiator-utils libxslt pyparted nginx \ policycoreutils-python python-libguestfs \ libguestfs-tools python-requests python-websockify \ @@ -77,8 +77,9 @@ for more information on how to configure your system to access this repository. libvirt-bin python-libxml2 python-imaging \ python-pam python-m2crypto python-jsonschema \ qemu-kvm libtool python-psutil python-ethtool \ - sosreport python-ipaddr python-lxml nfs-common \ - open-iscsi lvm2 xsltproc python-parted nginx \ + sosreport python-ipaddr python-ldap \ + python-lxml nfs-common open-iscsi lvm2 xsltproc \ + python-parted nginx \ firewalld python-guestfs libguestfs-tools \ python-requests websockify novnc

@@ -93,8 +94,9 @@ for more information on how to configure your system to access this repository. libvirt python-libxml2 python-imaging \ python-pam python-M2Crypto python-jsonschema \ rpm-build kvm python-psutil python-ethtool \ - python-ipaddr python-lxml nfs-client open-iscsi \ - libxslt-tools python-xml python-parted nginx \ + python-ipaddr python-ldap python-lxml nfs-client \ + open-iscsi libxslt-tools python-xml \ + python-parted nginx \ python-libguestfs guestfs-tools python-requests \ python-websockify novnc

diff --git a/src/kimchi.conf.in b/src/kimchi.conf.in index 62eb40b..c83ba09 100644 --- a/src/kimchi.conf.in +++ b/src/kimchi.conf.in @@ -57,3 +57,6 @@

# User id filter # ldap_search_filter = "uid=%(username)s" +

+# User IDs regarded as kimchi admin +# ldap_admin_id = "root, kimchi-admin"

Those example seems system users instead of LDAP. What about: # ldap_admin_id = "foo@foo.com, bar@bar.com"

diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index ad3f3ac..214c320 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -20,6 +20,7 @@ import base64 import cherrypy import fcntl +import ldap import multiprocessing import os import PAM @@ -178,17 +179,60 @@ class PAMUser(User):

class LDAPUser(User): auth_type = "ldap" + def __init__(self, username): self.user = {} self.user[USER_NAME] = username - self.user[USER_GROUPS] = None + self.user[USER_GROUPS] = list() # FIXME: user roles will be changed according roles assignment after # objstore is integrated self.user[USER_ROLES] = dict.fromkeys(tabs, 'user')

@staticmethod def authenticate(username, password): - return False + ldap_server = config.get("authentication", "ldap_server").strip('"') + ldap_search_base = config.get( + "authentication", "ldap_search_base").strip('"') + ldap_search_filter = config.get( + "authentication", "ldap_search_filter", + vars={"username": username.encode("utf-8")}).strip('"') + + connect = ldap.open(ldap_server) + try: + try: + result = connect.search_s( + ldap_search_base, ldap.SCOPE_SUBTREE, ldap_search_filter) + if len(result) == 0: + entity = ldap_search_filter % {'username': username} + raise ldap.LDAPError("Invalid ldap entity:%s" % entity) + except ldap.NO_SUCH_OBJECT: + # ldap search base specified wrongly. + raise ldap.LDAPError( + "invalid ldap search base %s" % ldap_search_base) + + try: + connect.bind_s(result[0][0], password) + except ldap.INVALID_CREDENTIALS: + # invalid user password + raise ldap.LDAPError("invalid user/passwd") + connect.unbind_s() + return True + except ldap.LDAPError, e: + arg = {"username": username, "code": e.message} + raise OperationFailed("KCHAUTH0001E", arg) + + def get_groups(self): + return self.user[USER_GROUPS] + + def get_roles(self): + admin_id = config.get("authentication", "ldap_admin_id").strip('"') + if self.user[USER_NAME] in admin_id.split(','): + self.user[USER_ROLES] = dict.fromkeys(tabs, 'admin') + return self.user[USER_ROLES] + + def get_user(self): + return self.user +

def from_browser(): # Enable Basic Authentication for REST tools.

On 2014年10月31日 01:02, Aline Manera wrote:

On 10/28/2014 11:37 AM, lvroyce0210@gmail.com wrote:

...

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Put ldap validation in a single function to resue in authorization.

...

Tested: 1. LDAP: GET /host/users?_user_id=a_valid_user_id GET /host/users?_user_id=invalid_user_id GET /host/groups 2. PAM: GET /host/users GET /host/groups

As we will use the same API for LDAP and PAM, I suggest change the API /host/users to /users and /host/groups to /groups. Originally, /host/users and /host.groups were created because they are related to the host system which is not the case for LDAP config.

ACK

...

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/control/host.py | 7 +++++ src/kimchi/i18n.py | 1 + src/kimchi/model/host.py | 67 ++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py index 7bcae72..20d4c2f 100644 --- a/src/kimchi/control/host.py +++ b/src/kimchi/control/host.py @@ -20,6 +20,7 @@ import cherrypy

from kimchi.control.base import Collection, Resource, SimpleCollection +from kimchi.control.utils import get_class_name, model_fn from kimchi.control.utils import UrlSubNode, validate_method from kimchi.exception import OperationFailed, NotFoundError from kimchi.template import render @@ -178,6 +179,12 @@ class Users(SimpleCollection): super(Users, self).__init__(model) self.role_key = 'guests'

+ def get(self, filter_params): + res_list = [] + get_list = getattr(self.model, model_fn(self, 'get_list')) + res_list = get_list(*self.model_args, **filter_params) + return render(get_class_name(self), res_list) +

class Groups(SimpleCollection): def __init__(self, model): diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 75fb076..27c0f44 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -39,6 +39,7 @@ messages = { "KCHAUTH0001E": _("Authentication failed for user '%(username)s'. [Error code: %(code)s]"), "KCHAUTH0002E": _("You are not authorized to access Kimchi"), "KCHAUTH0003E": _("Specify %(item)s to login into Kimchi"), + "KCHAUTH0004E": _("User %(user_id)s not found with given ldap settings."),

LDAP

...

"KCHDEVS0001E": _('Unknown "_cap" specified'), "KCHDEVS0002E": _('"_passthrough" should be "true" or "false"'), diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py index 5d31809..a2f0941 100644 --- a/src/kimchi/model/host.py +++ b/src/kimchi/model/host.py @@ -18,6 +18,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA

import grp +import ldap import libvirt import os import time @@ -32,6 +33,7 @@ from kimchi import disks from kimchi import netinfo from kimchi import xmlutils from kimchi.basemodel import Singleton +from kimchi.config import config from kimchi.model import hostdev from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed @@ -459,17 +461,78 @@ class RepositoryModel(object):

class UsersModel(object): + def __init__(self, **args): + auth_type = config.get("authentication", "method") + for klass in UsersModel.__subclasses__(): + if auth_type == klass.auth_type: + self.user = klass(**args) + + def get_list(self, **args): + return self.user._get_list(**args) + + +class PAMUsersModel(UsersModel): + auth_type = 'pam' def __init__(self, **kargs): pass

- def get_list(self): + def _get_list(self): return [user.pw_name for user in pwd.getpwall() if user.pw_shell.rsplit("/")[-1] not in ["nologin", "false"]]

+class LDAPUsersModel(UsersModel): + auth_type = 'ldap' + def __init__(self, **kargs): + pass + + def _get_list(self, _user_id=''): + return self._get_user(_user_id) + + def _get_user(self, _user_id): + ldap_server = config.get("authentication", "ldap_server").strip('"') + ldap_search_base = config.get( + "authentication", "ldap_search_base").strip('"') + ldap_search_filter = config.get( + "authentication", "ldap_search_filter", + vars={"username": _user_id.encode("utf-8")}).strip('"') + + connect = ldap.open(ldap_server) + try: + result = connect.search_s( + ldap_search_base, ldap.SCOPE_SUBTREE, ldap_search_filter) + if len(result) == 0: + raise NotFoundError("KCHAUTH0004E", {'user_id': _user_id}) + return result[0][1] + except ldap.NO_SUCH_OBJECT: + raise NotFoundError("KCHAUTH0004E", {'user_id': _user_id}) + + class GroupsModel(object): + def __init__(self, **args): + auth_type = config.get("authentication", "method") + for klass in GroupsModel.__subclasses__(): + if auth_type == klass.auth_type: + self.grp = klass(**args) + + + def get_list(self, **args): + if hasattr(self.grp, '_get_list'): + return self.grp._get_list(**args) + else: + return list() + + +class PAMGroupsModel(GroupsModel): + auth_type = 'pam' def __init__(self, **kargs): pass

- def get_list(self): + def _get_list(self): return [group.gr_name for group in grp.getgrall()] + + +class LDAPGroupsModel(GroupsModel): + auth_type = 'ldap' + def __init__(self, **kargs): + pass

_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

On 10/28/2014 11:37 AM, lvroyce0210@gmail.com wrote:

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Put validation in user and group class instead of validate in metadata update, so that different type of authorization can use their own authentication to validate input value.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/host.py | 30 ++++++++++++++++++++++++++++++ src/kimchi/model/vms.py | 16 ++++++++-------- 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py index a2f0941..cd47118 100644 --- a/src/kimchi/model/host.py +++ b/src/kimchi/model/host.py @@ -470,6 +470,9 @@ class UsersModel(object): def get_list(self, **args): return self.user._get_list(**args)

+ def validate(self, user): + return self.user.validate(user) +

class PAMUsersModel(UsersModel): auth_type = 'pam' @@ -480,6 +483,13 @@ class PAMUsersModel(UsersModel): return [user.pw_name for user in pwd.getpwall() if user.pw_shell.rsplit("/")[-1] not in ["nologin", "false"]]

+ def validate(self, user): + try: + user = pwd.getpwnam(user) + return user.pw_shell.rsplit("/")[-1] not in ["nologin", "false"] + except: + return False +

You can use _get_list() to do it: return user in self.get_list()

class LDAPUsersModel(UsersModel): auth_type = 'ldap' @@ -489,6 +499,13 @@ class LDAPUsersModel(UsersModel): def _get_list(self, _user_id=''): return self._get_user(_user_id)

+ def validate(self, user): + try: + self._get_user(user) + return True + except NotFoundError: + return False + def _get_user(self, _user_id): ldap_server = config.get("authentication", "ldap_server").strip('"') ldap_search_base = config.get( @@ -522,6 +539,9 @@ class GroupsModel(object): else: return list()

+ def validate(self, gid): + return self.grp.validate(gid) +

class PAMGroupsModel(GroupsModel): auth_type = 'pam' @@ -531,8 +551,18 @@ class PAMGroupsModel(GroupsModel): def _get_list(self): return [group.gr_name for group in grp.getgrall()]

+ def validate(self, gid): + try: + grp.getgrnam(gid) + except KeyError: + return False + return True +

class LDAPGroupsModel(GroupsModel): auth_type = 'ldap' def __init__(self, **kargs): pass + + def validate(self, gid): + return False diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 58686cd..777930d 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -266,16 +266,16 @@ class VMModel(object): users = groups = None if "users" in params: users = params["users"] - invalid_users = set(users) - set(self.users.get_list()) - if len(invalid_users) != 0: - raise InvalidParameter("KCHVM0027E", - {'users': ", ".join(invalid_users)}) + for user in users: + if not self.users.validate(user): + raise InvalidParameter("KCHVM0027E", + {'users': user}) if "groups" in params: groups = params["groups"] - invalid_groups = set(groups) - set(self.groups.get_list()) - if len(invalid_groups) != 0: - raise InvalidParameter("KCHVM0028E", - {'groups': ", ".join(invalid_groups)}) + for group in groups: + if not self.groups.validate(group): + raise InvalidParameter("KCHVM0028E", + {'groups': group})

if users is None and groups is None: return

On 11/06/2014 04:20 AM, Royce Lv wrote:

On 2014年10月31日 01:04, Aline Manera wrote:

...

On 10/28/2014 11:37 AM, lvroyce0210@gmail.com wrote:

...

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Put validation in user and group class instead of validate in metadata update, so that different type of authorization can use their own authentication to validate input value.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/host.py | 30 ++++++++++++++++++++++++++++++ src/kimchi/model/vms.py | 16 ++++++++-------- 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py index a2f0941..cd47118 100644 --- a/src/kimchi/model/host.py +++ b/src/kimchi/model/host.py @@ -470,6 +470,9 @@ class UsersModel(object): def get_list(self, **args): return self.user._get_list(**args)

+ def validate(self, user): + return self.user.validate(user) +

class PAMUsersModel(UsersModel): auth_type = 'pam' @@ -480,6 +483,13 @@ class PAMUsersModel(UsersModel): return [user.pw_name for user in pwd.getpwall() if user.pw_shell.rsplit("/")[-1] not in ["nologin", "false"]]

...

+ def validate(self, user): + try: + user = pwd.getpwnam(user) + return user.pw_shell.rsplit("/")[-1] not in ["nologin", "false"] + except: + return False +

You can use _get_list() to do it:

return user in self.get_list()

ACK, it changed from getpwall to getpwnam just for efficiency.

OK.

...

...

class LDAPUsersModel(UsersModel): auth_type = 'ldap' @@ -489,6 +499,13 @@ class LDAPUsersModel(UsersModel): def _get_list(self, _user_id=''): return self._get_user(_user_id)

+ def validate(self, user): + try: + self._get_user(user) + return True + except NotFoundError: + return False + def _get_user(self, _user_id): ldap_server = config.get("authentication", "ldap_server").strip('"') ldap_search_base = config.get( @@ -522,6 +539,9 @@ class GroupsModel(object): else: return list()

+ def validate(self, gid): + return self.grp.validate(gid) +

class PAMGroupsModel(GroupsModel): auth_type = 'pam' @@ -531,8 +551,18 @@ class PAMGroupsModel(GroupsModel): def _get_list(self): return [group.gr_name for group in grp.getgrall()]

+ def validate(self, gid): + try: + grp.getgrnam(gid) + except KeyError: + return False + return True +

class LDAPGroupsModel(GroupsModel): auth_type = 'ldap' def __init__(self, **kargs): pass + + def validate(self, gid): + return False diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 58686cd..777930d 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -266,16 +266,16 @@ class VMModel(object): users = groups = None if "users" in params: users = params["users"] - invalid_users = set(users) - set(self.users.get_list()) - if len(invalid_users) != 0: - raise InvalidParameter("KCHVM0027E", - {'users': ", ".join(invalid_users)}) + for user in users: + if not self.users.validate(user): + raise InvalidParameter("KCHVM0027E", + {'users': user}) if "groups" in params: groups = params["groups"] - invalid_groups = set(groups) - set(self.groups.get_list()) - if len(invalid_groups) != 0: - raise InvalidParameter("KCHVM0028E", - {'groups': ", ".join(invalid_groups)}) + for group in groups: + if not self.groups.validate(group): + raise InvalidParameter("KCHVM0028E", + {'groups': group})

if users is None and groups is None: return

_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 10/28/2014 11:37 AM, lvroyce0210@gmail.com wrote:

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Add authorization type to vm tag, and update set/retrieve access tag accordingly. So that we can switch between different types of authentication.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 777930d..728b2a7 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -32,7 +32,7 @@ from cherrypy.process.plugins import BackgroundTask

from kimchi import vnc from kimchi import xmlutils -from kimchi.config import READONLY_POOL_TYPE +from kimchi.config import READONLY_POOL_TYPE, config from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel @@ -253,13 +253,16 @@ class VMModel(object): return dom.name().decode('utf-8')

def _build_access_elem(self, users, groups): - access = E.access() + auth = config.get("authentication", "method") + auth_elem = E.auth(type=auth) for user in users: - access.append(E.user(user)) + auth_elem.append(E.user(user))

for group in groups: - access.append(E.group(group)) + auth_elem.append(E.group(group))

+ access = E.access() + access.append(auth_elem) return access

def _vm_update_access_metadata(self, dom, params): @@ -282,8 +285,9 @@ class VMModel(object):

access_xml = (get_metadata_node(dom, "access") or """<access></access>""") - old_users = xpath_get_text(access_xml, "/access/user") - old_groups = xpath_get_text(access_xml, "/access/group") + auth = config.get("authentication", "method") + old_users = xpath_get_text(access_xml, "/access/auth[@type='%s']/user" % auth) + old_groups = xpath_get_text(access_xml, "/access/auth[@type='%s']/group" % auth) users = old_users if users is None else users groups = old_groups if groups is None else groups

@@ -419,8 +423,10 @@ class VMModel(object):

access_xml = (get_metadata_node(dom, "access") or """<access></access>""") - users = xpath_get_text(access_xml, "/access/user") - groups = xpath_get_text(access_xml, "/access/group") + + auth = config.get("authentication", "method") + users = xpath_get_text(access_xml, "/access/auth[@type='%s']/user" % auth) + groups = xpath_get_text(access_xml, "/access/auth[@type='%s']/group" % auth)

return {'name': name, 'state': state,