From 56c436f589141401bbfcf8ead2e642eaefb1df38 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Fri, 20 Mar 2015 16:31:41 +0000 Subject: [PATCH 1/2] Pylint fixes. --- lib/galaxy/auth/providers/activedirectory.py | 26 ++++++++++---------- lib/galaxy/config.py | 6 ++--- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/galaxy/auth/providers/activedirectory.py b/lib/galaxy/auth/providers/activedirectory.py index 9365b923679..cec799e2e50 100644 --- a/lib/galaxy/auth/providers/activedirectory.py +++ b/lib/galaxy/auth/providers/activedirectory.py @@ -10,10 +10,10 @@ import logging log = logging.getLogger(__name__) -def _get_subs(d, k, vars, default=''): +def _get_subs(d, k, params, default=''): if k in d: - return str(d[k]).format(**vars) - return str(default).format(**vars) + return str(d[k]).format(**params) + return str(default).format(**params) class ActiveDirectory(AuthProvider): @@ -44,19 +44,19 @@ class ActiveDirectory(AuthProvider): return (failure_mode, '') # do AD search (if required) - vars = {'username': username, 'password': password} + params = {'username': username, 'password': password} if 'search-fields' in options: try: # setup connection ldap.set_option(ldap.OPT_REFERRALS, 0) - l = ldap.initialize(_get_subs(options, 'server', vars)) + l = ldap.initialize(_get_subs(options, 'server', params)) l.protocol_version = 3 - l.simple_bind_s(_get_subs(options, 'search-user', vars), _get_subs(options, 'search-password', vars)) + l.simple_bind_s(_get_subs(options, 'search-user', params), _get_subs(options, 'search-password', params)) scope = ldap.SCOPE_SUBTREE # setup search - attributes = map(lambda s: s.strip().format(**vars), options['search-fields'].split(',')) - result = l.search(_get_subs(options, 'search-base', vars), scope, _get_subs(options, 'search-filter', vars), attributes) + attributes = [_.strip().format(**params) for _ in options['search-fields'].split(',')] + result = l.search(_get_subs(options, 'search-base', params), scope, _get_subs(options, 'search-filter', params), attributes) # parse results _, suser = l.result(result, 60) @@ -65,9 +65,9 @@ class ActiveDirectory(AuthProvider): if hasattr(attrs, 'has_key'): for attr in attributes: if attr in attrs: - vars[attr] = str(attrs[attr][0]) + params[attr] = str(attrs[attr][0]) else: - vars[attr] = "" + params[attr] = "" except Exception: log.exception('ACTIVEDIRECTORY Search Exception for User: %s' % username) return (failure_mode, '') @@ -77,15 +77,15 @@ class ActiveDirectory(AuthProvider): try: # setup connection ldap.set_option(ldap.OPT_REFERRALS, 0) - l = ldap.initialize(_get_subs(options, 'server', vars)) + l = ldap.initialize(_get_subs(options, 'server', params)) l.protocol_version = 3 - l.simple_bind_s(_get_subs(options, 'bind-user', vars), _get_subs(options, 'bind-password', vars)) + l.simple_bind_s(_get_subs(options, 'bind-user', params), _get_subs(options, 'bind-password', params)) except Exception: log.exception('ACTIVEDIRECTORY Authenticate Exception for User %s' % username) return (failure_mode, '') log.debug("User: %s, ACTIVEDIRECTORY: True" % (username)) - return (True, _get_subs(options, 'auto-register-username', vars)) + return (True, _get_subs(options, 'auto-register-username', params)) def authenticate_user(self, user, password, options): """ diff --git a/lib/galaxy/config.py b/lib/galaxy/config.py index 3aa43bdc0b9..a6b7648ac26 100644 --- a/lib/galaxy/config.py +++ b/lib/galaxy/config.py @@ -26,7 +26,7 @@ log = logging.getLogger( __name__ ) def resolve_path( path, root ): """If 'path' is relative make absolute by prepending 'root'""" - if not( os.path.isabs( path ) ): + if not os.path.isabs( path ): path = os.path.join( root, path ) return path @@ -204,7 +204,7 @@ class Configuration( object ): with open( self.blacklist_file ) as blacklist: self.blacklist_content = [ line.rstrip() for line in blacklist.readlines() ] except IOError: - print ( "CONFIGURATION ERROR: Can't open supplied blacklist file from path: " + str( self.blacklist_file ) ) + print ( "CONFIGURATION ERROR: Can't open supplied blacklist file from path: " + str( self.blacklist_file ) ) self.smtp_server = kwargs.get( 'smtp_server', None ) self.smtp_username = kwargs.get( 'smtp_username', None ) self.smtp_password = kwargs.get( 'smtp_password', None ) @@ -636,7 +636,7 @@ class Configuration( object ): database in the future. """ admin_users = [ x.strip() for x in self.get( "admin_users", "" ).split( "," ) ] - return ( user is not None and user.email in admin_users ) + return user is not None and user.email in admin_users def resolve_path( self, path ): """ Resolve a path relative to Galaxy's root. From 9f2668ee86d70056836d6d355017c76cd89a97cb Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Fri, 20 Mar 2015 16:45:49 +0000 Subject: [PATCH 2/2] Raise exception if some Active Directory options are not defined. Generalize use of ConfigurationError exception. Bug fixed: if bind-user and bind-password are not configured in , password is not checked and the user is logged in. --- lib/galaxy/auth/providers/activedirectory.py | 9 +++++---- lib/galaxy/config.py | 7 ++----- lib/galaxy/exceptions/__init__.py | 4 ++++ lib/galaxy/exceptions/error_codes.json | 5 +++++ 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/auth/providers/activedirectory.py b/lib/galaxy/auth/providers/activedirectory.py index cec799e2e50..33c2e39aded 100644 --- a/lib/galaxy/auth/providers/activedirectory.py +++ b/lib/galaxy/auth/providers/activedirectory.py @@ -4,16 +4,17 @@ Created on 15/07/2014 @author: Andrew Robinson """ +from galaxy.exceptions import ConfigurationError from ..providers import AuthProvider import logging log = logging.getLogger(__name__) -def _get_subs(d, k, params, default=''): - if k in d: - return str(d[k]).format(**params) - return str(default).format(**params) +def _get_subs(d, k, params): + if k not in d: + raise ConfigurationError("Missing '%s' parameter in Active Directory options" % k) + return str(d[k]).format(**params) class ActiveDirectory(AuthProvider): diff --git a/lib/galaxy/config.py b/lib/galaxy/config.py index a6b7648ac26..e6f7c8556b7 100644 --- a/lib/galaxy/config.py +++ b/lib/galaxy/config.py @@ -15,6 +15,7 @@ import sys import tempfile from datetime import timedelta from galaxy import eggs +from galaxy.exceptions import ConfigurationError from galaxy.util import listify from galaxy.util import string_as_bool from galaxy.util.dbkeys import GenomeBuilds @@ -31,10 +32,6 @@ def resolve_path( path, root ): return path -class ConfigurationError( Exception ): - pass - - class Configuration( object ): deprecated_options = ( 'database_file', ) @@ -421,7 +418,7 @@ class Configuration( object ): self.pretty_datetime_format = expand_pretty_datetime_format( kwargs.get( 'pretty_datetime_format', '$locale (UTC)' ) ) self.master_api_key = kwargs.get( 'master_api_key', None ) if self.master_api_key == "changethis": # default in sample config file - raise Exception("Insecure configuration, please change master_api_key to something other than default (changethis)") + raise ConfigurationError("Insecure configuration, please change master_api_key to something other than default (changethis)") # Experimental: This will not be enabled by default and will hide # nonproduction code. diff --git a/lib/galaxy/exceptions/__init__.py b/lib/galaxy/exceptions/__init__.py index d881fce0a8f..65210229d55 100644 --- a/lib/galaxy/exceptions/__init__.py +++ b/lib/galaxy/exceptions/__init__.py @@ -146,6 +146,10 @@ class Conflict( MessageException ): err_code = error_codes.CONFLICT +class ConfigurationError( Exception ): + status_code = 500 + err_code = error_codes.CONFIG_ERROR + class InconsistentDatabase ( MessageException ): status_code = 500 err_code = error_codes.INCONSISTENT_DATABASE diff --git a/lib/galaxy/exceptions/error_codes.json b/lib/galaxy/exceptions/error_codes.json index 92c85e2f4d6..c681afdebb0 100644 --- a/lib/galaxy/exceptions/error_codes.json +++ b/lib/galaxy/exceptions/error_codes.json @@ -124,6 +124,11 @@ "code": 500002, "message": "Inconsistent database prevented fulfilling the request." }, + { + "name": "CONFIG_ERROR", + "code": 500003, + "message": "Error in a configuration file." + }, { "name": "NOT_IMPLEMENTED", "code": 501001,