Some code cleanups and fixes

development.ini

@@ -53,6 +53,8 b' cut_off_limit = 256000'
 force_https = false
 commit_parse_limit = 25
 use_gravatar = true
+container_auth_enabled = false
+proxypass_auth_enabled = false
 
 ####################################
 ###        CELERY CONFIG        ####

production.ini

@@ -53,6 +53,8 b' cut_off_limit = 256000'
 force_https = false 
 commit_parse_limit = 50
 use_gravatar = true
+container_auth_enabled = false
+proxypass_auth_enabled = false
 
 ####################################
 ###        CELERY CONFIG        ####

rhodecode/controllers/login.py

@@ -49,7 +49,7 b' class LoginController(BaseController):'
         super(LoginController, self).__before__()
 
     def index(self):
-        #redirect if already logged in
+        # redirect if already logged in
         c.came_from = request.GET.get('came_from', None)
 
         if self.rhodecode_user.is_authenticated \
@@ -62,7 +62,7 b' class LoginController(BaseController):'
             login_form = LoginForm()
             try:
                 c.form_result = login_form.to_python(dict(request.POST))
-                #form checks for username/password, now we're authenticated
+                # form checks for username/password, now we're authenticated
                 username = c.form_result['username']
                 user = User.get_by_username(username, case_insensitive=True)
                 auth_user = AuthUser(user.user_id)

rhodecode/lib/auth.py

@@ -125,16 +125,23 b' def get_crypt_password(password):'
 def check_password(password, hashed):
     return RhodeCodeCrypto.hash_check(password, hashed)
 
-
-def generate_api_key(username, salt=None):
+def generate_api_key(str_, salt=None):
+    """
+    Generates API KEY from given string
+    
+    :param str_:
+    :param salt:
+    """
+    
     if salt is None:
         salt = _RandomNameSequence().next()
 
-    return hashlib.sha1(username + salt).hexdigest()
+    return hashlib.sha1(str_ + salt).hexdigest()
 
 
 def authfunc(environ, username, password):
-    """Dummy authentication function used in Mercurial/Git/ and access control,
+    """
+    Dummy authentication function used in Mercurial/Git/ and access control,
 
     :param environ: needed only for using in Basic auth
     """
@@ -142,7 +149,8 b' def authfunc(environ, username, password'
 
 
 def authenticate(username, password):
-    """Authentication function used for access control,
+    """
+    Authentication function used for access control,
     firstly checks for db authentication then if ldap is enabled for ldap
     authentication, also creates ldap user if not in database
 
@@ -228,33 +236,35 b' def login_container_auth(username):'
     if user is None:
         user_model = UserModel()
         user_attrs = {
-                 'name': username,
-                 'lastname': None,
-                 'email': None,
-                }
-        if not user_model.create_for_container_auth(username, user_attrs):
+            'name': username,
+            'lastname': None,
+            'email': None,
+        }
+        user = user_model.create_for_container_auth(username, user_attrs)
+        if not user:
             return None
-        user = User.get_by_username(username)
         log.info('User %s was created by container authentication', username)
 
     if not user.active:
         return None
 
     user.update_lastlogin()
-    log.debug('User %s is now logged in by container authentication', user.username)
+    log.debug('User %s is now logged in by container authentication', 
+              user.username)
     return user
 
-def get_container_username(environ, cfg=config):
+def get_container_username(environ, cfg):
     from paste.httpheaders import REMOTE_USER
     from paste.deploy.converters import asbool
 
+    proxy_pass_enabled = asbool(cfg.get('proxypass_auth_enabled', False))
     username = REMOTE_USER(environ)
-
-    if not username and asbool(cfg.get('proxypass_auth_enabled', False)):
+    
+    if not username and proxy_pass_enabled:
         username = environ.get('HTTP_X_FORWARDED_USER')
 
-    if username:
-        #Removing realm and domain from username
+    if username and proxy_pass_enabled:
+        # Removing realm and domain from username
         username = username.partition('@')[0]
         username = username.rpartition('\\')[2]
         log.debug('Received username %s from container', username)
@@ -276,7 +286,7 b' class AuthUser(object):'
         self.user_id = user_id
         self.api_key = None
         self.username = username
-        
+
         self.name = ''
         self.lastname = ''
         self.email = ''
@@ -290,14 +300,17 b' class AuthUser(object):'
         user_model = UserModel()
         self.anonymous_user = User.get_by_username('default')
         is_user_loaded = False
+        
+        # try go get user by api key
         if self._api_key and self._api_key != self.anonymous_user.api_key:
-            #try go get user by api key
             log.debug('Auth User lookup by API KEY %s', self._api_key)
             is_user_loaded = user_model.fill_data(self, api_key=self._api_key)
-        elif self.user_id is not None \
-            and self.user_id != self.anonymous_user.user_id:
+        # lookup by userid    
+        elif (self.user_id is not None and 
+              self.user_id != self.anonymous_user.user_id):
             log.debug('Auth User lookup by USER ID %s', self.user_id)
             is_user_loaded = user_model.fill_data(self, user_id=self.user_id)
+        # lookup by username    
         elif self.username:
             log.debug('Auth User lookup by USER NAME %s', self.username)
             dbuser = login_container_auth(self.username)
@@ -308,10 +321,10 b' class AuthUser(object):'
                 is_user_loaded = True
 
         if not is_user_loaded:
+            # if we cannot authenticate user try anonymous
             if self.anonymous_user.active is True:
-                user_model.fill_data(self,
-                                     user_id=self.anonymous_user.user_id)
-                #then we set this user is logged in
+                user_model.fill_data(self,user_id=self.anonymous_user.user_id)
+                # then we set this user is logged in
                 self.is_authenticated = True
             else:
                 self.user_id = None
@@ -337,13 +350,13 b' class AuthUser(object):'
                                               self.is_authenticated)
 
     def set_authenticated(self, authenticated=True):
-
         if self.user_id != self.anonymous_user.user_id:
             self.is_authenticated = authenticated
 
 
 def set_available_permissions(config):
-    """This function will propagate pylons globals with all available defined
+    """
+    This function will propagate pylons globals with all available defined
     permission given in db. We don't want to check each time from db for new
     permissions since adding a new permission also requires application restart
     ie. to decorate new views with the newly created permission
@@ -474,7 +487,7 b' class PermsDecorator(object):'
                 return redirect(url('login_home', came_from=p))
 
             else:
-                #redirect with forbidden ret code
+                # redirect with forbidden ret code
                 return abort(403)
 
     def check_permissions(self):
@@ -661,3 +674,4 b' class HasPermissionAnyMiddleware(object)'
             return True
         log.debug('permission denied')
         return False
+

rhodecode/lib/base.py

@@ -33,8 +33,6 b' class BaseController(WSGIController):'
         self.sa = meta.Session()
         self.scm_model = ScmModel(self.sa)
 
-        #c.unread_journal = scm_model.get_unread_journal()
-
     def __call__(self, environ, start_response):
         """Invoke the Controller"""
         # WSGIController.__call__ dispatches to the Controller method
@@ -42,15 +40,15 b' class BaseController(WSGIController):'
         # available in environ['pylons.routes_dict']
         start = time.time()
         try:
-            # putting this here makes sure that we update permissions each time
+            # make sure that we update permissions each time we call controller
             api_key = request.GET.get('api_key')
             user_id = getattr(session.get('rhodecode_user'), 'user_id', None)
             if asbool(config.get('container_auth_enabled', False)):
                 username = get_container_username(environ)
             else:
                 username = None
-
-            self.rhodecode_user = c.rhodecode_user = AuthUser(user_id, api_key, username)
+            auth_user = AuthUser(user_id, api_key, username)
+            self.rhodecode_user = c.rhodecode_user = auth_user
             if not self.rhodecode_user.is_authenticated and \
                        self.rhodecode_user.user_id is not None:
                 self.rhodecode_user.set_authenticated(
@@ -66,11 +64,13 b' class BaseController(WSGIController):'
 
 class BaseRepoController(BaseController):
     """
-    Base class for controllers responsible for loading all needed data
-    for those controllers, loaded items are
+    Base class for controllers responsible for loading all needed data for
+    repository loaded items are
 
-    c.rhodecode_repo: instance of scm repository (taken from cache)
-
+    c.rhodecode_repo: instance of scm repository
+    c.rhodecode_db_repo: instance of db
+    c.repository_followers: number of followers
+    c.repository_forks: number of forks
     """
 
     def __before__(self):
@@ -86,7 +86,6 b' class BaseRepoController(BaseController)'
 
                 redirect(url('home'))
 
-            c.repository_followers = \
-                self.scm_model.get_followers(c.repo_name)
+            c.repository_followers = self.scm_model.get_followers(c.repo_name)
             c.repository_forks = self.scm_model.get_forks(c.repo_name)
 

rhodecode/model/forms.py

@@ -208,7 +208,7 b' class ValidAuth(formencode.validators.Fa'
         password = value['password']
         username = value['username']
         user = User.get_by_username(username)
-
+        
         if authenticate(username, password):
             return value
         else:

rhodecode/model/user.py

@@ -106,20 +106,20 b' class UserModel(BaseModel):'
                 new_user.password = None
                 new_user.api_key = generate_api_key(username)
                 new_user.email = attrs['email']
-                new_user.active = True
+                new_user.active = attrs.get('active', True)
                 new_user.name = attrs['name']
                 new_user.lastname = attrs['lastname']
 
                 self.sa.add(new_user)
                 self.sa.commit()
-                return True
+                return new_user
             except (DatabaseError,):
                 log.error(traceback.format_exc())
                 self.sa.rollback()
                 raise
-        log.debug('User %s already exists. Skipping creation of account for container auth.',
-                  username)
-        return False
+        log.debug('User %s already exists. Skipping creation of account'
+                  ' for container auth.', username)
+        return None
 
     def create_ldap(self, username, password, user_dn, attrs):
         """
@@ -141,21 +141,21 b' class UserModel(BaseModel):'
                 new_user.password = get_crypt_password(password)
                 new_user.api_key = generate_api_key(username)
                 new_user.email = attrs['email']
-                new_user.active = attrs.get('active',True)
+                new_user.active = attrs.get('active', True)
                 new_user.ldap_dn = safe_unicode(user_dn)
                 new_user.name = attrs['name']
                 new_user.lastname = attrs['lastname']
 
                 self.sa.add(new_user)
                 self.sa.commit()
-                return True
+                return new_user
             except (DatabaseError,):
                 log.error(traceback.format_exc())
                 self.sa.rollback()
                 raise
         log.debug('this %s user exists skipping creation of ldap account',
                   username)
-        return False
+        return None
 
     def create_registration(self, form_data):
         from rhodecode.lib.celerylib import tasks, run_task

rhodecode/tests/__init__.py

@@ -21,7 +21,6 b' from webtest import TestApp'
 from rhodecode.model import meta
 import logging
 
-
 log = logging.getLogger(__name__)
 
 import pylons.test
@@ -31,7 +30,7 b' import pylons.test'
            'TEST_USER_ADMIN_LOGIN', 'TEST_USER_ADMIN_PASS' ]
 
 # Invoke websetup with the current config file
-#SetupCommand('setup-app').run([config_file])
+# SetupCommand('setup-app').run([config_file])
 
 ##RUNNING DESIRED TESTS
 # nosetests -x rhodecode.tests.functional.test_admin_settings:TestSettingsController.test_my_account
@@ -69,7 +68,7 b' class TestController(TestCase):'
         response = self.app.post(url(controller='login', action='index'),
                                  {'username':username,
                                   'password':password})
-
+        
         if 'invalid user name' in response.body:
             self.fail('could not login using %s %s' % (username, password))
 

test.ini

@@ -51,6 +51,8 b' cut_off_limit = 256000'
 force_https = false
 commit_parse_limit = 25
 use_gravatar = true
+container_auth_enabled = false
+proxypass_auth_enabled = false
 
 ####################################
 ###        CELERY CONFIG        ####