[Kimchi-devel] [PATCH] issue#474: Add check for ldap configuration

[Aline Manera]

On 12/09/2014 06:24 AM, lvroyce at linux.vnet.ibm.com wrote:
> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>
> Check ldap server and filter when start up.
> Ignore admin id if it does not specified in config.
>
> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
> ---
>   src/kimchi/auth.py | 14 +++++++++-----
>   src/kimchid.in     |  9 +++++++++
>   2 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py
> index 22c5c81..23bc5ec 100644
> --- a/src/kimchi/auth.py
> +++ b/src/kimchi/auth.py
> @@ -19,6 +19,7 @@
>
>   import base64
>   import cherrypy
> +from ConfigParser import NoOptionError
>   import fcntl
>   import ldap
>   import multiprocessing
> @@ -222,11 +223,14 @@ class LDAPUser(User):
>           return self.user[USER_GROUPS]

>       def get_roles(self):
> -        admin_ids = config.get(
> -            "authentication", "ldap_admin_id").strip('"').split(',')
> -        for admin_id in admin_ids:
> -            if self.user[USER_NAME] == admin_id.strip():
> -                self.user[USER_ROLES] = dict.fromkeys(tabs, 'admin')
> +        try:
> +            admin_ids = config.get(
> +                "authentication", "ldap_admin_id").strip('"').split(',')
> +            for admin_id in admin_ids:
> +                if self.user[USER_NAME] == admin_id.strip():
> +                    self.user[USER_ROLES] = dict.fromkeys(tabs, 'admin')
> +        except NoOptionError:
> +            pass

All those changes can be reduced by adding a default value to 
"ldap_admin_id" in src/kimchi/config.py.in
In the same way you did for the other LDAP options

     config.set("authentication", "method", "pam")
     config.set("authentication", "ldap_server", "")
     config.set("authentication", "ldap_search_base", "")
     config.set("authentication", "ldap_search_filter", "")

>           return self.user[USER_ROLES]
>
>       def get_user(self):
> diff --git a/src/kimchid.in b/src/kimchid.in
> index 0746ba6..0121f97 100644
> --- a/src/kimchid.in
> +++ b/src/kimchid.in
> @@ -49,6 +49,15 @@ def main(options):
>       federation = config.config.get("server", "federation")
>       logDir = config.config.get("logging", "log_dir")
>       logLevel = config.config.get("logging", "log_level")
> +    auth = config.config.get("authentication", "method")
> +    ldap_server = config.config.get("authentication", "ldap_server")
> +    search_base = config.config.get("authentication", "ldap_search_base")
> +    search_filter = config.config.get("authentication", "ldap_search_filter",
> +                                      vars={"username": "fake"})
> +
> +    if auth == "ldap" and \
> +            (not all([auth, ldap_server, search_base, search_filter])):
> +        sys.exit("LDAP server is not setup properly.Exiting ...")
>
>       parser = OptionParser()
>       parser.add_option('--host', type="string", default=host,