Mercurial > kallithea
changeset 7696:077ba994ee03
auth: introduce AuthUser.make factory which can return None if user can't be authenticated
| author | Mads Kiilerich <mads@kiilerich.com> |
|---|---|
| date | Thu, 03 Jan 2019 01:22:45 +0100 |
| parents | 31aa5b6c107d |
| children | 226893a56a81 |
| files | kallithea/controllers/api/__init__.py kallithea/controllers/login.py kallithea/lib/auth.py kallithea/lib/auth_modules/__init__.py kallithea/lib/base.py |
| diffstat | 5 files changed, 62 insertions(+), 52 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/controllers/api/__init__.py Sun Apr 07 23:44:17 2019 +0200 +++ b/kallithea/controllers/api/__init__.py Thu Jan 03 01:22:45 2019 +0100 @@ -146,12 +146,11 @@ # check if we can find this session using api_key try: u = User.get_by_api_key(self._req_api_key) - if u is None: + auth_user = AuthUser.make(dbuser=u) + if auth_user is None: raise JSONRPCErrorResponse(retid=self._req_id, message='Invalid API key') - - auth_u = AuthUser(dbuser=u) - if not AuthUser.check_ip_allowed(auth_u, ip_addr): + 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: @@ -161,6 +160,8 @@ raise JSONRPCErrorResponse(retid=self._req_id, message='Invalid API key') + request.authuser = auth_user + self._error = None try: self._func = self._find_method() @@ -179,11 +180,6 @@ func_kwargs = dict(itertools.izip_longest(reversed(arglist), reversed(defaults), fillvalue=default_empty)) - # this is little trick to inject logged in user for - # perms decorators to work they expect the controller class to have - # authuser attribute set - request.authuser = auth_u - # This attribute will need to be first param of a method that uses # api_key, which is translated to instance of user at that name USER_SESSION_ATTR = 'apiuser'
--- a/kallithea/controllers/login.py Sun Apr 07 23:44:17 2019 +0200 +++ b/kallithea/controllers/login.py Thu Jan 03 01:22:45 2019 +0100 @@ -102,8 +102,8 @@ # Exception itself h.flash(e, 'error') else: - 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) + # TODO: handle auth_user is None as failed authentication? raise HTTPFound(location=c.came_from) else: # redirect if already logged in
--- a/kallithea/lib/auth.py Sun Apr 07 23:44:17 2019 +0200 +++ b/kallithea/lib/auth.py Thu Jan 03 01:22:45 2019 +0100 @@ -398,6 +398,20 @@ "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): + """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. + """ + 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 + 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 @@ -575,14 +589,12 @@ @staticmethod def from_cookie(cookie): """ - Deserializes an `AuthUser` from a cookie `dict`. + Deserializes an `AuthUser` from a cookie `dict` ... or return None. """ - - au = AuthUser( - user_id=cookie.get('user_id'), + return AuthUser.make( + dbuser=UserModel().get(cookie.get('user_id')), is_external_auth=cookie.get('is_external_auth', False), ) - return au @classmethod def get_allowed_ips(cls, user_id, cache=False): @@ -871,18 +883,17 @@ def __init__(self, *perms): self.required_perms = set(perms) - def __call__(self, user, repo_name, purpose=None): + def __call__(self, authuser, repo_name, purpose=None): # repo_name MUST be unicode, since we handle keys in ok # dict by unicode repo_name = safe_unicode(repo_name) - user = AuthUser(user.user_id) try: - ok = user.permissions['repositories'][repo_name] in self.required_perms + ok = authuser.permissions['repositories'][repo_name] in self.required_perms except KeyError: ok = False - log.debug('Middleware check %s for %s for repo %s (%s): %s', user.username, self.required_perms, repo_name, purpose, ok) + log.debug('Middleware check %s for %s for repo %s (%s): %s', authuser.username, self.required_perms, repo_name, purpose, ok) return ok
--- a/kallithea/lib/auth_modules/__init__.py Sun Apr 07 23:44:17 2019 +0200 +++ b/kallithea/lib/auth_modules/__init__.py Thu Jan 03 01:22:45 2019 +0100 @@ -362,7 +362,7 @@ settings=plugin_settings) log.debug('Plugin %s extracted user `%s`', module, user) - if user is not None and not user.active: + if user is not None and not user.active: # give up, way before creating AuthUser log.error("Rejecting authentication of in-active user %s", user) continue
--- a/kallithea/lib/base.py Sun Apr 07 23:44:17 2019 +0200 +++ b/kallithea/lib/base.py Thu Jan 03 01:22:45 2019 +0100 @@ -117,14 +117,16 @@ 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) + if auth_user is None: + return None + user.update_lastlogin() meta.Session().commit() - auth_user = AuthUser(dbuser=user, - is_external_auth=is_external_auth) - # It should not be possible to explicitly log in as the default user. - assert not auth_user.is_default_user - # Start new session to prevent session fixation attacks. session.invalidate() session['authuser'] = cookie = auth_user.to_cookie() @@ -210,18 +212,15 @@ Returns (user, None) on successful authentication and authorization. Returns (None, wsgi_app) to send the wsgi_app response to the client. """ - # Check if anonymous access is allowed. + # Use anonymous access if allowed for action on repo. default_user = User.get_default_user(cache=True) - is_default_user_allowed = (default_user.active and - self._check_permission(action, default_user, repo_name, ip_addr)) - if is_default_user_allowed: - return default_user, None - - if not default_user.active: - log.debug('Anonymous access is disabled') + default_authuser = AuthUser.make(dbuser=default_user) + if default_authuser is None: + log.debug('No anonymous access at all') # move on to proper user auth else: - log.debug('Not authorized to access this ' - 'repository as anonymous user') + if self._check_permission(action, default_authuser, repo_name, ip_addr): + return default_authuser, None + log.debug('Not authorized to access this repository as anonymous user') username = None #============================================================== @@ -252,15 +251,14 @@ #============================================================== try: user = User.get_by_username_or_email(username) - if user is None or not user.active: - return None, webob.exc.HTTPForbidden() except Exception: log.error(traceback.format_exc()) return None, webob.exc.HTTPInternalServerError() - # check permissions for this repository - perm = self._check_permission(action, user, repo_name, ip_addr) - if not perm: + authuser = AuthUser.make(dbuser=user) + if authuser is None: + return None, webob.exc.HTTPForbidden() + if not self._check_permission(action, authuser, repo_name, ip_addr): return None, webob.exc.HTTPForbidden() return user, None @@ -285,7 +283,7 @@ return '/'.join(data) - def _check_permission(self, action, user, repo_name, ip_addr=None): + def _check_permission(self, action, authuser, repo_name, ip_addr=None): """ Checks permissions using action (push/pull) user and repository name @@ -295,7 +293,7 @@ :param repo_name: repository name """ # check IP - ip_allowed = AuthUser.check_ip_allowed(user, ip_addr) + ip_allowed = AuthUser.check_ip_allowed(authuser, ip_addr) if ip_allowed: log.info('Access for IP:%s allowed', ip_addr) else: @@ -303,7 +301,7 @@ if action == 'push': if not HasPermissionAnyMiddleware('repository.write', - 'repository.admin')(user, + 'repository.admin')(authuser, repo_name): return False @@ -311,7 +309,7 @@ #any other action need at least read permission if not HasPermissionAnyMiddleware('repository.read', 'repository.write', - 'repository.admin')(user, + 'repository.admin')(authuser, repo_name): return False @@ -392,6 +390,7 @@ """ 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). """ # Authenticate by bearer token @@ -400,9 +399,9 @@ # Authenticate by API key if api_key is not None: - au = AuthUser(dbuser=User.get_by_api_key(api_key), - authenticating_api_key=api_key, is_external_auth=True) - if au.is_anonymous: + dbuser = User.get_by_api_key(api_key) + au = AuthUser.make(dbuser=dbuser, authenticating_api_key=api_key, is_external_auth=True) + 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 @@ -429,12 +428,14 @@ 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) # User is default user (if active) or anonymous default_user = User.get_default_user(cache=True) - return AuthUser(dbuser=default_user) + authuser = AuthUser.make(dbuser=default_user) + if authuser is None: # fall back to anonymous + authuser = AuthUser(dbuser=default_user) # TODO: somehow use .make? + return authuser @staticmethod def _basic_security_checks(): @@ -490,7 +491,9 @@ bearer_token, session.get('authuser'), ) - + 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()