kallithea Changeset - c64c076b96c3

Changeset - c64c076b96c3

Parent rev.

Child rev.

[Not reviewed]

default

0 1 0

Søren Løvborg - 10 years ago 2015-09-08 11:09:00
sorenl@unity3d.com

auth: avoid setting AuthUser.is_authenticated for unauthenticated users

AuthUser.is_authenticated could be True for three reasons: because the
user "was" the default user, because the user was authenticated by session
cookie, or because the user was just authenticated by an auth module
(including the internal auth module). In the last case, a session
cookie is emitted (even when using container auth), so the last two
cases are closely related.

This commit do that unauthenticated users (the first case) only get the
is_default_user attribute set, and that the is_authenticated attribute only is
set for authenticated users (for the second and third case).

This complicates some expressions, but allows others to be simplified.
More importantly, it makes the code more explicit, and makes the
"is_authenticated" name mean what it says.

(This will temporarily make the is_authenticated session value look even more
weird than before.)

1 file changed with 6 insertions and 11 deletions:

kallithea/lib/auth.py

0 comments (0 inline, 0 general)

kallithea/lib/auth.py

➞

Show inline comments

@@ @@ -456,26 +456,25 @@ class AuthUser(object): @@
     """
     Represents a Kallithea user, including various authentication and
     authorization information. Typically used to store the current user,
     but is also used as a generic user information data structure in
     parts of the code, e.g. user management.
     Constructed from a database `User` object, a user ID or cookie dict,
     it looks up the user (if needed) and copies all attributes to itself,
     adding various non-persistent data. If lookup fails but anonymous
     access to Kallithea is enabled, the default user is loaded instead.
     `AuthUser` does not by itself authenticate users and the constructor
     sets the `is_authenticated` field to False, except when falling back
     to the default anonymous user (if enabled). It's up to other parts
     sets the `is_authenticated` field to False. It's up to other parts
     of the code to check e.g. if a supplied password is correct, and if
     so, set `is_authenticated` to True.
     However, `AuthUser` does refuse to load a user that is not `active`.
     """
     def __init__(self, user_id=None, dbuser=None,
             is_external_auth=False):
         self.is_authenticated = False
         self.is_external_auth = is_external_auth
@@ @@ -499,28 +498,25 @@ class AuthUser(object): @@
             log.debug('Auth User lookup by USER ID %s', user_id)
             dbuser = user_model.get(user_id)
         else:
             # Note: dbuser is allowed to be None.
             log.debug('Auth User lookup by database user %s', dbuser)
         is_user_loaded = self._fill_data(dbuser)
         # If user cannot be found, try falling back to anonymous.
         if not is_user_loaded:
             is_user_loaded =  self._fill_data(self.anonymous_user)
         # The anonymous user is always "logged in".
         self.is_default_user = (self.user_id == self.anonymous_user.user_id)
         if self.is_default_user:
             self.is_authenticated = True
         if not self.username:
             self.username = 'None'
         log.debug('Auth User is now %s', self)
     def _fill_data(self, dbuser):
         """
         Copies database fields from a `db.User` to this `AuthUser`. Does
         not copy `api_keys` and `permissions` attributes.
         Checks that `dbuser` is `active` (and not None) before copying;
@@ @@ -615,48 +611,47 @@ class AuthUser(object): @@
         allowed_ips = AuthUser.get_allowed_ips(user.user_id, cache=True,
             inherit_from_default=user.inherit_default_permissions)
         if check_ip_access(source_ip=ip_addr, allowed_ips=allowed_ips):
             log.debug('IP:%s is in range of %s', ip_addr, allowed_ips)
             return True
         else:
             log.info('Access for IP:%s forbidden, '
                      'not in %s' % (ip_addr, allowed_ips))
             return False
     def __repr__(self):
         return "<AuthUser('id:%s[%s] auth:%s')>"\
             % (self.user_id, self.username, self.is_authenticated)
+            % (self.user_id, self.username, (self.is_authenticated or self.is_default_user))
     def to_cookie(self):
         """ Serializes this login session to a cookie `dict`. """
         return {
             'user_id': self.user_id,
             'is_authenticated': self.is_authenticated,
+            'is_authenticated': self.is_authenticated or self.is_default_user,
             'is_external_auth': self.is_external_auth,
+        }
     @staticmethod
     def from_cookie(cookie):
         """
         Deserializes an `AuthUser` from a cookie `dict`.
         """
         au = AuthUser(
             user_id=cookie.get('user_id'),
             is_external_auth=cookie.get('is_external_auth', False),
+        )
-        if not au.is_authenticated and au.user_id is not None:
+        if not au.is_default_user and au.user_id is not None:
             # user is not authenticated and not empty
             if not au.is_default_user:
                 au.is_authenticated = cookie.get('is_authenticated')
             au.is_authenticated = cookie.get('is_authenticated')
         return au
     @classmethod
     def get_allowed_ips(cls, user_id, cache=False, inherit_from_default=False):
         _set = set()
         if inherit_from_default:
             default_ips = UserIpMap.query().filter(UserIpMap.user ==
                                             User.get_default_user(cache=True))
             if cache:
                 default_ips = default_ips.options(FromCache("sql_cache_short",
                                                   "get_user_ips_default"))
@@ @@ -784,25 +779,25 @@ class LoginRequired(object): @@
             if not token or token != secure_form.authentication_token():
                 log.error('CSRF check failed')
                 raise HTTPForbidden()
         # WebOb already ignores request payload parameters for anything other
         # than POST/PUT, but double-check since other Kallithea code relies on
         # this assumption.
         if request.method not in ['POST', 'PUT'] and request.POST:
             log.error('%r request with payload parameters; WebOb should have stopped this', request.method)
             raise HTTPBadRequest()
         # regular user authentication
         if user.is_authenticated:
+        if user.is_authenticated or user.is_default_user:
             log.info('user %s authenticated with regular auth @ %s', user, loc)
             return func(*fargs, **fkwargs)
         else:
             log.warning('user %s NOT authenticated with regular auth @ %s', user, loc)
             return redirect_to_login()
 class NotAnonymous(object):
     """
     Must be logged in to execute this function else
     redirect to login page"""
     def __call__(self, func):

0 comments (0 inline, 0 general)