kallithea Changeset - 226893a56a81

Changeset - 226893a56a81

Parent rev.

Child rev.

[Not reviewed]

default

0 4 0

Mads Kiilerich - 7 years ago 2019-01-03 01:22:56
mads@kiilerich.com

auth: move IP check to AuthUser.make - it is more about accepting authentication than about permissions after authentication

4 files changed with 29 insertions and 50 deletions:

kallithea/controllers/api/__init__.py

kallithea/controllers/login.py

kallithea/lib/auth.py

kallithea/lib/base.py

0 comments (0 inline, 0 general)

kallithea/controllers/api/__init__.py

➞

Show inline comments

@@ @@ -100,13 +100,13 @@ class JSONRPCController(TGController): @@
         """
         # Since we are here we should respond as JSON
         response.content_type = 'application/json'
         environ = state.request.environ
         start = time.time()
-        ip_addr = request.ip_addr = self._get_ip_addr(environ)
         ip_addr = self._get_ip_addr(environ)
         self._req_id = None
         if 'CONTENT_LENGTH' not in environ:
             log.debug("No Content-Length")
             raise JSONRPCErrorResponse(retid=self._req_id,
                                        message="No Content-Length in request")
         else:
@@ @@ -143,27 +143,22 @@ class JSONRPCController(TGController): @@
             raise JSONRPCErrorResponse(retid=self._req_id,
                                        message='Incorrect JSON query missing %s' % e)
         # check if we can find this session using api_key
         try:
             u = User.get_by_api_key(self._req_api_key)
             auth_user = AuthUser.make(dbuser=u)
+            auth_user = AuthUser.make(dbuser=u, ip_addr=ip_addr)
             if auth_user is None:
                 raise JSONRPCErrorResponse(retid=self._req_id,
                                            message='Invalid API key')
             if not AuthUser.check_ip_allowed(auth_user, ip_addr):
                 raise JSONRPCErrorResponse(retid=self._req_id,
                                            message='request from IP:%s not allowed' % (ip_addr,))
             else:
                 log.info('Access for IP:%s allowed', ip_addr)
         except Exception as e:
             raise JSONRPCErrorResponse(retid=self._req_id,
                                        message='Invalid API key')
         request.authuser = auth_user
         request.ip_addr = ip_addr
         self._error = None
         try:
             self._func = self._find_method()
         except AttributeError as e:
             raise JSONRPCErrorResponse(retid=self._req_id,

kallithea/controllers/login.py

➞

Show inline comments

@@ @@ -99,13 +99,13 @@ class LoginController(BaseController): @@
                 # container auth or other auth functions that create users on
                 # the fly can throw this exception signaling that there's issue
                 # with user creation, explanation should be provided in
                 # Exception itself
                 h.flash(e, 'error')
             else:
                 auth_user = log_in_user(user, c.form_result['remember'], is_external_auth=False)
+                auth_user = log_in_user(user, c.form_result['remember'], is_external_auth=False, ip_addr=request.ip_addr)
                 # TODO: handle auth_user is None as failed authentication?
                 raise HTTPFound(location=c.came_from)
         else:
             # redirect if already logged in
             if not request.authuser.is_anonymous:
                 raise HTTPFound(location=c.came_from)

kallithea/lib/auth.py

➞

Show inline comments

@@ @@ -396,22 +396,27 @@ class AuthUser(object): @@
     `is_default_user` specifically checks if the AuthUser is the user named
     "default". Use `is_anonymous` to check for both "default" and "no user".
     """
     @classmethod
     def make(cls, dbuser=None, authenticating_api_key=None, is_external_auth=False):
+    def make(cls, dbuser=None, authenticating_api_key=None, is_external_auth=False, ip_addr=None):
         """Create an AuthUser to be authenticated ... or return None if user for some reason can't be authenticated.
-        Checks that a non-None dbuser is provided and is active.
+        Checks that a non-None dbuser is provided, is active, and that the IP address is ok.
         """
         assert ip_addr is not None
         if dbuser is None:
             log.info('No db user for authentication')
             return None
         if not dbuser.active:
             log.info('Db user %s not active', dbuser.username)
             return None
         allowed_ips = AuthUser.get_allowed_ips(dbuser.user_id, cache=True)
         if not check_ip_access(source_ip=ip_addr, allowed_ips=allowed_ips):
             log.info('Access for %s from %s forbidden - not in %s', dbuser.username, ip_addr, allowed_ips)
             return None
         return cls(dbuser=dbuser, authenticating_api_key=authenticating_api_key,
             is_external_auth=is_external_auth)
     def __init__(self, user_id=None, dbuser=None, authenticating_api_key=None,
             is_external_auth=False):
         self.is_external_auth = is_external_auth # container auth - don't show logout option
@@ @@ -558,45 +563,31 @@ class AuthUser(object): @@
         """
         Returns list of user groups you're an admin of
         """
         return [x[0] for x in self.permissions['user_groups'].iteritems()
                 if x[1] == 'usergroup.admin']
     @staticmethod
     def check_ip_allowed(user, ip_addr):
         """
         Check if the given IP address (a `str`) is allowed for the given
         user (an `AuthUser` or `db.User`).
         """
         allowed_ips = AuthUser.get_allowed_ips(user.user_id, cache=True)
         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]')>" % (self.user_id, self.username)
     def to_cookie(self):
         """ Serializes this login session to a cookie `dict`. """
         return {
             'user_id': self.user_id,
             'is_external_auth': self.is_external_auth,
+        }
     @staticmethod
     def from_cookie(cookie):
+    def from_cookie(cookie, ip_addr):
         """
         Deserializes an `AuthUser` from a cookie `dict` ... or return None.
         """
         return AuthUser.make(
             dbuser=UserModel().get(cookie.get('user_id')),
             is_external_auth=cookie.get('is_external_auth', False),
             ip_addr=ip_addr,
+        )
     @classmethod
     def get_allowed_ips(cls, user_id, cache=False):
         _set = set()

kallithea/lib/base.py

➞

Show inline comments

@@ @@ -106,24 +106,24 @@ def _get_access_path(environ): @@
     org_req = environ.get('tg.original_request')
     if org_req:
         path = org_req.environ.get('PATH_INFO')
     return path
 def log_in_user(user, remember, is_external_auth):
+def log_in_user(user, remember, is_external_auth, ip_addr):
     """
     Log a `User` in and update session and cookies. If `remember` is True,
     the session cookie is set to expire in a year; otherwise, it expires at
     the end of the browser session.
     Returns populated `AuthUser` object.
     """
     # It should not be possible to explicitly log in as the default user.
     assert not user.is_default_user, user
     auth_user = AuthUser.make(dbuser=user, is_external_auth=is_external_auth)
+    auth_user = AuthUser.make(dbuser=user, is_external_auth=is_external_auth, ip_addr=ip_addr)
     if auth_user is None:
         return None
     user.update_lastlogin()
     meta.Session().commit()
@@ @@ -211,17 +211,17 @@ class BaseVCSController(object): @@
         Returns (user, None) on successful authentication and authorization.
         Returns (None, wsgi_app) to send the wsgi_app response to the client.
         """
         # Use anonymous access if allowed for action on repo.
         default_user = User.get_default_user(cache=True)
         default_authuser = AuthUser.make(dbuser=default_user)
+        default_authuser = AuthUser.make(dbuser=default_user, ip_addr=ip_addr)
         if default_authuser is None:
             log.debug('No anonymous access at all') # move on to proper user auth
         else:
-            if self._check_permission(action, default_authuser, repo_name, ip_addr):
             if self._check_permission(action, default_authuser, repo_name):
                 return default_authuser, None
             log.debug('Not authorized to access this repository as anonymous user')
         username = None
         #==============================================================
         # DEFAULT PERM FAILED OR ANONYMOUS ACCESS IS DISABLED SO WE
@@ @@ -252,16 +252,16 @@ class BaseVCSController(object): @@
         try:
             user = User.get_by_username_or_email(username)
         except Exception:
             log.error(traceback.format_exc())
             return None, webob.exc.HTTPInternalServerError()
         authuser = AuthUser.make(dbuser=user)
+        authuser = AuthUser.make(dbuser=user, ip_addr=ip_addr)
         if authuser is None:
             return None, webob.exc.HTTPForbidden()
-        if not self._check_permission(action, authuser, repo_name, ip_addr):
         if not self._check_permission(action, authuser, repo_name):
             return None, webob.exc.HTTPForbidden()
         return user, None
     def _handle_request(self, environ, start_response):
         raise NotImplementedError()
@@ @@ -280,28 +280,21 @@ class BaseVCSController(object): @@
             by_id_match = get_repo_by_id(repo_name)
             if by_id_match:
                 data[1] = safe_str(by_id_match)
         return '/'.join(data)
-    def _check_permission(self, action, authuser, repo_name, ip_addr=None):
     def _check_permission(self, action, authuser, repo_name):
         """
         Checks permissions using action (push/pull) user and repository
         name
         :param action: 'push' or 'pull' action
         :param user: `User` instance
         :param repo_name: repository name
         """
         # check IP
         ip_allowed = AuthUser.check_ip_allowed(authuser, ip_addr)
         if ip_allowed:
             log.info('Access for IP:%s allowed', ip_addr)
         else:
             return False
         if action == 'push':
             if not HasPermissionAnyMiddleware('repository.write',
                                               'repository.admin')(authuser,
                                                                   repo_name):
                 return False
@@ @@ -383,39 +376,39 @@ class BaseController(TGController): @@
         c.my_pr_count = PullRequest.query(reviewer_id=request.authuser.user_id, include_closed=False).count()
         self.scm_model = ScmModel()
     @staticmethod
     def _determine_auth_user(api_key, bearer_token, session_authuser):
+    def _determine_auth_user(api_key, bearer_token, session_authuser, ip_addr):
         """
         Create an `AuthUser` object given the API key/bearer token
         (if any) and the value of the authuser session cookie.
         Returns None if no valid user is found (like not active).
+        Returns None if no valid user is found (like not active or no access for IP).
         """
         # Authenticate by bearer token
         if bearer_token is not None:
             api_key = bearer_token
         # Authenticate by API key
         if api_key is not None:
             dbuser = User.get_by_api_key(api_key)
             au = AuthUser.make(dbuser=dbuser, authenticating_api_key=api_key, is_external_auth=True)
+            au = AuthUser.make(dbuser=dbuser, authenticating_api_key=api_key, is_external_auth=True, ip_addr=ip_addr)
             if au is None or au.is_anonymous:
                 log.warning('API key ****%s is NOT valid', api_key[-4:])
                 raise webob.exc.HTTPForbidden(_('Invalid API key'))
             return au
         # Authenticate by session cookie
         # In ancient login sessions, 'authuser' may not be a dict.
         # In that case, the user will have to log in again.
         # v0.3 and earlier included an 'is_authenticated' key; if present,
         # this must be True.
         if isinstance(session_authuser, dict) and session_authuser.get('is_authenticated', True):
             return AuthUser.from_cookie(session_authuser)
+            return AuthUser.from_cookie(session_authuser, ip_addr=ip_addr)
         # Authenticate by auth_container plugin (if enabled)
         if any(
             plugin.is_container_auth
             for plugin in auth_modules.get_auth_plugins()
         ):
@@ @@ -425,17 +418,17 @@ class BaseController(TGController): @@
                 from kallithea.lib import helpers as h
                 h.flash(e, 'error', logf=log.error)
             else:
                 if user_info is not None:
                     username = user_info['username']
                     user = User.get_by_username(username, case_insensitive=True)
                     return log_in_user(user, remember=False, is_external_auth=True)
+                    return log_in_user(user, remember=False, is_external_auth=True, ip_addr=ip_addr)
         # User is default user (if active) or anonymous
         default_user = User.get_default_user(cache=True)
         authuser = AuthUser.make(dbuser=default_user)
+        authuser = AuthUser.make(dbuser=default_user, ip_addr=ip_addr)
         if authuser is None: # fall back to anonymous
             authuser = AuthUser(dbuser=default_user) # TODO: somehow use .make?
         return authuser
     @staticmethod
     def _basic_security_checks():
@@ @@ -466,13 +459,13 @@ class BaseController(TGController): @@
         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 webob.exc.HTTPBadRequest()
     def __call__(self, environ, context):
         try:
-            request.ip_addr = _get_ip_addr(environ)
             ip_addr = _get_ip_addr(environ)
             # make sure that we update permissions each time we call controller
             self._basic_security_checks()
             # set globals for auth user
@@ @@ -487,20 +480,20 @@ class BaseController(TGController): @@
                     bearer_token = params
             authuser = self._determine_auth_user(
                 request.GET.get('api_key'),
                 bearer_token,
                 session.get('authuser'),
                 ip_addr=ip_addr,
+            )
             if authuser is None:
                 log.info('No valid user found')
                 raise webob.exc.HTTPForbidden()
             if not AuthUser.check_ip_allowed(authuser, request.ip_addr):
                 raise webob.exc.HTTPForbidden()
             request.authuser = authuser
             request.ip_addr = ip_addr
             log.info('IP: %s User: %s accessed %s',
                 request.ip_addr, request.authuser,
                 safe_unicode(_get_access_path(environ)),
+            )
             return super(BaseController, self).__call__(environ, context)

0 comments (0 inline, 0 general)