Re: [PATCH 5 of 5] auth: fix tests after changing API key handling
On 03/25/2015 12:01 PM, Thomas De Schampheleire wrote: # HG changeset patch # User Thomas De Schampheleire # Date 1427279629 -3600 # Wed Mar 25 11:33:49 2015 +0100 # Node ID eaeea9ea95b036e8d5eaac16aea1e6c8c62868c9 # Parent e1a755428e3abd3d011c7c033233272dadb34572 auth: fix tests after changing API key handling It seems like this changeset is fixing some failures that previous changes introduced? All tests should pass for all revisions. Please make the necessary test changes in the changesets where functionality is changed. Return codes when using API keys have changed, and so should the tests. Additionally, improve the auth logic to make a distinction between having no API key (and thus no checking of it, falling back to regular auth), and having a potentially empty one (401 if it is invalid). diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py --- a/kallithea/lib/auth.py +++ b/kallithea/lib/auth.py @@ -754,9 +754,9 @@ % (loc, user)) return redirect_to_login() -# check if we used an APIKEY and it's a valid one -_api_key = request.GET.get('api_key', '') -if _api_key: +# check if we used an API key and it's a valid one +_api_key = request.GET.get('api_key') +if _api_key is not None: oh - nice one ;-) It would be nice to have this chunk in one of the previos patches. Thanks for attacking this. It is essential functionality so it is nice to see it refactored to be more trust-worthy. /Mads ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 3 of 5] auth: return early in LoginRequired on API key validation
On 03/25/2015 12:01 PM, Thomas De Schampheleire wrote: # HG changeset patch # User Thomas De Schampheleire # Date 1427274589 -3600 # Wed Mar 25 10:09:49 2015 +0100 # Node ID 32ced9e32b506c7441339f67da3c4237de0e6fa3 # Parent 43b0bc9088d3eedacf7610ffbf2f4b429ae98c45 auth: return early in LoginRequired on API key validation Simplify the logic in the LoginRequired decorator when Kallithea is accessed using an API key. Either: - the key is valid and API access is allowed for the accessed method (continue), or - the key is invalid (401 Unauthorized), or - the accessed method does not allow API access (403 Forbidden) In none of these cases does it make sense to continue checking for user authentication, or redirecting to a login page, so return early. diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py --- a/kallithea/lib/auth.py +++ b/kallithea/lib/auth.py @@ -58,7 +58,7 @@ get_user_group_slug, conditional_cache from kallithea.lib.caching_query import FromCache -from webob.exc import HTTPUnauthorized +from webob.exc import HTTPUnauthorized, HTTPForbidden log = logging.getLogger(__name__) @@ -738,6 +738,7 @@ cls = fargs[0] user = cls.authuser loc = "%s:%s" % (cls.__class__.__name__, func.__name__) +log.debug('Checking access for user %s @ %s' % (user, loc)) def redirect_to_login(): p = url.current() @@ -754,37 +755,34 @@ return redirect_to_login() # check if we used an APIKEY and it's a valid one -# defined whitelist of controllers which API access will be enabled _api_key = request.GET.get('api_key', '') I think we should use request.GET.get('api_key') and thus get None if no key is provided (empty or not). (And let's get rid of the leading _ while touching it.) -api_access_valid = allowed_api_access(loc, api_key=_api_key) - -# explicit controller is enabled or API is in our whitelist -if self.api_access or api_access_valid: -log.debug('Checking API KEY access for %s' % cls) -if _api_key and _api_key in user.api_keys: -api_access_valid = True -log.debug('API KEY %s is VALID' % _api_key[-4:]) -else: -api_access_valid = False -if not _api_key: -log.debug("API KEY *NOT* present in request") +if _api_key: - and here we should use: if _api_key is not None That would make it more obvious that an empty key is an (invalid) key. /Mads +# explicit controller is enabled or API is in our whitelist +if self.api_access or allowed_api_access(loc, api_key=_api_key): +if _api_key in user.api_keys: +log.info('user %s authenticated with API key %s @ %s' + % (user, _api_key[-4:], loc)) +return func(*fargs, **fkwargs) else: -log.warning("API KEY %s *NOT* valid" % _api_key[-4:]) +log.warning('API key %s is NOT valid' % _api_key[-4:]) headers = [('WWW-Authenticate', 'APIKEY realm="Kallithea"')] raise HTTPUnauthorized(headers=headers) +else: +# controller does not allow API access +log.warning('API access to %s is not allowed' % loc) +raise HTTPForbidden() log.debug('Checking if %s is authenticated @ %s' % (user.username, loc)) reason = 'RegularAuth' if user.is_authenticated else 'APIAuth' -if user.is_authenticated or api_access_valid: +if user.is_authenticated: log.info('user %s authenticating with:%s IS authenticated on func %s ' % (user, reason, loc) ) return func(*fargs, **fkwargs) else: log.warning('user %s authenticating with:%s NOT authenticated on func: %s: ' - 'API_ACCESS:%s' - % (user, reason, loc, api_access_valid) + % (user, reason, loc) ) return redirect_to_login() ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 2 of 5] auth: return early in LoginRequired on invalid IP
On 03/25/2015 12:01 PM, Thomas De Schampheleire wrote: # HG changeset patch # User Thomas De Schampheleire # Date 1427271075 -3600 # Wed Mar 25 09:11:15 2015 +0100 # Node ID 43b0bc9088d3eedacf7610ffbf2f4b429ae98c45 # Parent c5828585502f1a061f162abe8cbd181c17039843 auth: return early in LoginRequired on invalid IP Simplify the code of the LoginRequired decorator by returning early when an unacceptable condition is met. diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py --- a/kallithea/lib/auth.py +++ b/kallithea/lib/auth.py @@ -739,13 +739,19 @@ user = cls.authuser loc = "%s:%s" % (cls.__class__.__name__, func.__name__) +def redirect_to_login(): +p = url.current() +log.debug('redirecting to login page with %s' % p) +return redirect(url('login_home', came_from=p)) Nested function definitions always makes me wonder which bindings the function has to the surrounding scope. Is it a pure function where everything is passed as parameters, or will it also use some "global" state? It seems like this is a general function that it could be nice to have in helpers.py or here in auth.py ... and use it everywhere instead of direct redirect to login_home. + # check if our IP is allowed -ip_access_valid = True if not user.ip_allowed: from kallithea.lib import helpers as h h.flash(h.literal(_('IP %s not allowed' % (user.ip_addr))), category='warning') -ip_access_valid = False +log.warning('Access to %s from unallowed IP address for user %s' + % (loc, user)) (It would be nice if h.flash automatically would log messages. (Messages might in some cases have different severity for the user and for the server admin - that could perhaps be controlled by an extra parameter to flash ...)) +return redirect_to_login() # check if we used an APIKEY and it's a valid one # defined whitelist of controllers which API access will be enabled @@ -770,21 +776,17 @@ log.debug('Checking if %s is authenticated @ %s' % (user.username, loc)) reason = 'RegularAuth' if user.is_authenticated else 'APIAuth' -if ip_access_valid and (user.is_authenticated or api_access_valid): +if user.is_authenticated or api_access_valid: log.info('user %s authenticating with:%s IS authenticated on func %s ' % (user, reason, loc) ) return func(*fargs, **fkwargs) else: log.warning('user %s authenticating with:%s NOT authenticated on func: %s: ' - 'IP_ACCESS:%s API_ACCESS:%s' - % (user, reason, loc, ip_access_valid, api_access_valid) + 'API_ACCESS:%s' + % (user, reason, loc, api_access_valid) ) -p = url.current() - -log.debug('redirecting to login page with %s' % p) -return redirect(url('login_home', came_from=p)) - +return redirect_to_login() /Mads ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 1 of 5] auth: do not redirect to login page on invalid API key
On 03/25/2015 12:01 PM, Thomas De Schampheleire wrote: # HG changeset patch # User Thomas De Schampheleire # Date 1427269791 -3600 # Wed Mar 25 08:49:51 2015 +0100 # Node ID c5828585502f1a061f162abe8cbd181c17039843 # Parent 6017996e4dcfda0f5623498a45c51bb184eb67bb auth: do not redirect to login page on invalid API key When accessing Kallithea through an API call, providing an API key, it doesn't make sense to redirect to a login page on failed authentication. Instead, raise a 401 Unauthorized exception. The WWW-authenticate header is a mandatory element for 401 Unauthorized, as specified by RFC 7235. The exact contents do not seem to be important, so define a custom auth scheme 'APIKEY' with a realm of 'Kallithea'. diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py --- a/kallithea/lib/auth.py +++ b/kallithea/lib/auth.py @@ -58,6 +58,7 @@ get_user_group_slug, conditional_cache from kallithea.lib.caching_query import FromCache +from webob.exc import HTTPUnauthorized log = logging.getLogger(__name__) @@ -763,6 +764,8 @@ log.debug("API KEY *NOT* present in request") else: log.warning("API KEY %s *NOT* valid" % _api_key[-4:]) +headers = [('WWW-Authenticate', 'APIKEY realm="Kallithea"')] The API does not use HTTP authentication. I thus think it is misleading to return 401 and WWW-Authenticate. I think it would be better to just fail with 400 Bad Request. When we don't add headers, we can just use "return abort(400)" as done elsewhere in the same file. /Mads +raise HTTPUnauthorized(headers=headers) log.debug('Checking if %s is authenticated @ %s' % (user.username, loc)) reason = 'RegularAuth' if user.is_authenticated else 'APIAuth' ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: 1st try - no repos found
On Wed, Mar 25, 2015 at 6:16 PM, Neal Becker wrote: > Thomas De Schampheleire wrote: > >> Hi Neal, >> >> On Wed, Mar 25, 2015 at 4:53 PM, Neal Becker >> wrote: >>> Thomas De Schampheleire wrote: >> [..] >>> >>> I'm running into a problem. I'm using hg-3.3.2, and I have a repo using >>> obsolete markers. When I start kallithea, go to admin page, it says hg >>> is >>> 3.1.2. I can't figure out where that comes from. I mv'd the hg in the >>> venv/bin, so kallithea would hopefully use my hg from /usr/bin, but that >>> didn't help. >>> >>> I get: >>> Abort: parsing obsolete marker: unknown version 1 >> >> During the 'python setup.py' step, mercurial is explicitly installed >> in the virtualenv. This will probably have selected 3.1.2. If you use >> 'pip install' to install a specific mercurial into the virtualenv, and >> restart kallithea, I guess things will be as you expect. >> >> Best regards, >> Thomas > > Unfortunately that didn't work either. After pip install mercurial: > > paster serve my.ini > Traceback (most recent call last): > File "/home/nbecker/kallithea/venv/bin/paster", line 9, in > load_entry_point('PasteScript==1.7.5', 'console_scripts', 'paster')() > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/script/command.py", line 104, in run > invoke(command, command_name, options, args[1:]) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/script/command.py", line 143, in invoke > exit_code = runner.run(args) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/script/command.py", line 238, in run > result = self.command() > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/script/serve.py", line 284, in command > relative_to=base, global_conf=vars) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/script/serve.py", line 321, in loadapp > **kw) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/deploy/loadwsgi.py", line 247, in loadapp > return loadobj(APP, uri, name=name, **kw) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/deploy/loadwsgi.py", line 271, in loadobj > global_conf=global_conf) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/deploy/loadwsgi.py", line 296, in loadcontext > global_conf=global_conf) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/deploy/loadwsgi.py", line 320, in _loadconfig > return loader.get_context(object_type, name, global_conf) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/deploy/loadwsgi.py", line 454, in get_context > section) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/deploy/loadwsgi.py", line 476, in _context_from_use > object_type, name=use, global_conf=global_conf) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/deploy/loadwsgi.py", line 406, in get_context > global_conf=global_conf) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/deploy/loadwsgi.py", line 296, in loadcontext > global_conf=global_conf) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/deploy/loadwsgi.py", line 328, in _loadegg > return loader.get_context(object_type, name, global_conf) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/deploy/loadwsgi.py", line 620, in get_context > object_type, name=name) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/paste/deploy/loadwsgi.py", line 640, in find_egg_entry_point > pkg_resources.require(self.spec) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/pkg_resources/__init__.py", line 918, in require > needed = self.resolve(parse_requirements(requirements)) > File "/home/nbecker/kallithea/venv/lib/python2.7/site- > packages/pkg_resources/__init__.py", line 810, in resolve > raise VersionConflict(dist, req).with_context(dependent_req) > pkg_resources.ContextualVersionConflict: (mercurial 3.3.2 > (/home/nbecker/kallithea/venv/lib/python2.7/site-packages), > Requirement.parse('mercurial<3.2,>=2.8.2'), set(['kallithea'])) Support for later Mercurial versions is already present in the development tree, not in release 0.1. I suggest to create an installation based on the latest sources, following the instructions at: http://kallithea.readthedocs.org/en/latest/installation.html Sorry I had missed that before. Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: 1st try - no repos found
Thomas De Schampheleire wrote: > Hi Neal, > > On Wed, Mar 25, 2015 at 4:53 PM, Neal Becker > wrote: >> Thomas De Schampheleire wrote: > [..] >> >> I'm running into a problem. I'm using hg-3.3.2, and I have a repo using >> obsolete markers. When I start kallithea, go to admin page, it says hg >> is >> 3.1.2. I can't figure out where that comes from. I mv'd the hg in the >> venv/bin, so kallithea would hopefully use my hg from /usr/bin, but that >> didn't help. >> >> I get: >> Abort: parsing obsolete marker: unknown version 1 > > During the 'python setup.py' step, mercurial is explicitly installed > in the virtualenv. This will probably have selected 3.1.2. If you use > 'pip install' to install a specific mercurial into the virtualenv, and > restart kallithea, I guess things will be as you expect. > > Best regards, > Thomas Unfortunately that didn't work either. After pip install mercurial: paster serve my.ini Traceback (most recent call last): File "/home/nbecker/kallithea/venv/bin/paster", line 9, in load_entry_point('PasteScript==1.7.5', 'console_scripts', 'paster')() File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/script/command.py", line 104, in run invoke(command, command_name, options, args[1:]) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/script/command.py", line 143, in invoke exit_code = runner.run(args) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/script/command.py", line 238, in run result = self.command() File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/script/serve.py", line 284, in command relative_to=base, global_conf=vars) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/script/serve.py", line 321, in loadapp **kw) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/deploy/loadwsgi.py", line 247, in loadapp return loadobj(APP, uri, name=name, **kw) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/deploy/loadwsgi.py", line 271, in loadobj global_conf=global_conf) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/deploy/loadwsgi.py", line 296, in loadcontext global_conf=global_conf) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/deploy/loadwsgi.py", line 320, in _loadconfig return loader.get_context(object_type, name, global_conf) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/deploy/loadwsgi.py", line 454, in get_context section) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/deploy/loadwsgi.py", line 476, in _context_from_use object_type, name=use, global_conf=global_conf) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/deploy/loadwsgi.py", line 406, in get_context global_conf=global_conf) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/deploy/loadwsgi.py", line 296, in loadcontext global_conf=global_conf) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/deploy/loadwsgi.py", line 328, in _loadegg return loader.get_context(object_type, name, global_conf) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/deploy/loadwsgi.py", line 620, in get_context object_type, name=name) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/paste/deploy/loadwsgi.py", line 640, in find_egg_entry_point pkg_resources.require(self.spec) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/pkg_resources/__init__.py", line 918, in require needed = self.resolve(parse_requirements(requirements)) File "/home/nbecker/kallithea/venv/lib/python2.7/site- packages/pkg_resources/__init__.py", line 810, in resolve raise VersionConflict(dist, req).with_context(dependent_req) pkg_resources.ContextualVersionConflict: (mercurial 3.3.2 (/home/nbecker/kallithea/venv/lib/python2.7/site-packages), Requirement.parse('mercurial<3.2,>=2.8.2'), set(['kallithea'])) -- Those who fail to understand recursion are doomed to repeat it ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: 1st try - no repos found
Hi Neal, On Wed, Mar 25, 2015 at 4:53 PM, Neal Becker wrote: > Thomas De Schampheleire wrote: [..] > > I'm running into a problem. I'm using hg-3.3.2, and I have a repo using > obsolete markers. When I start kallithea, go to admin page, it says hg is > 3.1.2. I can't figure out where that comes from. I mv'd the hg in the > venv/bin, so kallithea would hopefully use my hg from /usr/bin, but that > didn't help. > > I get: > Abort: parsing obsolete marker: unknown version 1 During the 'python setup.py' step, mercurial is explicitly installed in the virtualenv. This will probably have selected 3.1.2. If you use 'pip install' to install a specific mercurial into the virtualenv, and restart kallithea, I guess things will be as you expect. Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: 1st try - no repos found
Thomas De Schampheleire wrote: > On Wed, Mar 25, 2015 at 4:24 PM, Neal Becker > wrote: >> Thomas De Schampheleire wrote: >> >>> Hi Neal, >>> >>> On Wed, Mar 25, 2015 at 4:03 PM, Neal Becker >>> wrote: installed 0.1 on fedora f21 into venv using instructions for install and setup. I chose for root /home/nbecker/sigproc.export which is a clone of a working hg repo, under my home dir. >>> >>> Kallithea is about serving more than one repository. It is searching >>> inside sigproc.export for repositories, the root is not supposed to be >>> a repository itself. >>> >>> With the above example, the root could be /home/nbecker, but I would >>> recommend using a dedicated directory for kallithea repositories, like >>> /home/nbecker/kallithea-repos. >>> >>> Best regards, >>> Thomas >> >> Can I change the root on an already-setup kallithea, or should I start >> from scratch? > > You can change the root from the Admin->Settings menu, (first need to > unlock), then you should restart Kallithea, then from the > Admin->Settings->Remap and rescan page you should click the 'rescan' > button. > > Best regards, > Thomas Thanks! I'm running into a problem. I'm using hg-3.3.2, and I have a repo using obsolete markers. When I start kallithea, go to admin page, it says hg is 3.1.2. I can't figure out where that comes from. I mv'd the hg in the venv/bin, so kallithea would hopefully use my hg from /usr/bin, but that didn't help. I get: Abort: parsing obsolete marker: unknown version 1 -- Those who fail to understand recursion are doomed to repeat it ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: 1st try - no repos found
On Wed, Mar 25, 2015 at 4:24 PM, Neal Becker wrote: > Thomas De Schampheleire wrote: > >> Hi Neal, >> >> On Wed, Mar 25, 2015 at 4:03 PM, Neal Becker >> wrote: >>> installed 0.1 on fedora f21 into venv using instructions for install and >>> setup. >>> >>> I chose for root >>> >>> /home/nbecker/sigproc.export >>> >>> which is a clone of a working hg repo, under my home dir. >> >> Kallithea is about serving more than one repository. It is searching >> inside sigproc.export for repositories, the root is not supposed to be >> a repository itself. >> >> With the above example, the root could be /home/nbecker, but I would >> recommend using a dedicated directory for kallithea repositories, like >> /home/nbecker/kallithea-repos. >> >> Best regards, >> Thomas > > Can I change the root on an already-setup kallithea, or should I start from > scratch? You can change the root from the Admin->Settings menu, (first need to unlock), then you should restart Kallithea, then from the Admin->Settings->Remap and rescan page you should click the 'rescan' button. Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: 1st try - no repos found
Thomas De Schampheleire wrote: > Hi Neal, > > On Wed, Mar 25, 2015 at 4:03 PM, Neal Becker > wrote: >> installed 0.1 on fedora f21 into venv using instructions for install and >> setup. >> >> I chose for root >> >> /home/nbecker/sigproc.export >> >> which is a clone of a working hg repo, under my home dir. > > Kallithea is about serving more than one repository. It is searching > inside sigproc.export for repositories, the root is not supposed to be > a repository itself. > > With the above example, the root could be /home/nbecker, but I would > recommend using a dedicated directory for kallithea repositories, like > /home/nbecker/kallithea-repos. > > Best regards, > Thomas Can I change the root on an already-setup kallithea, or should I start from scratch? -- Those who fail to understand recursion are doomed to repeat it ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: 1st try - no repos found
Hi Neal, On Wed, Mar 25, 2015 at 4:03 PM, Neal Becker wrote: > installed 0.1 on fedora f21 into venv using instructions for install and > setup. > > I chose for root > > /home/nbecker/sigproc.export > > which is a clone of a working hg repo, under my home dir. Kallithea is about serving more than one repository. It is searching inside sigproc.export for repositories, the root is not supposed to be a repository itself. With the above example, the root could be /home/nbecker, but I would recommend using a dedicated directory for kallithea repositories, like /home/nbecker/kallithea-repos. Best regards, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
1st try - no repos found
installed 0.1 on fedora f21 into venv using instructions for install and setup. I chose for root /home/nbecker/sigproc.export which is a clone of a working hg repo, under my home dir. As me, I started paster server my.ini *** failed to import extension evolve from ~/mutable- history/hgext/evolve.py: 'module' object has no attribute 'formats' 2015-03-25 11:01:38.162 INFO [kallithea.model] initializing db for sqlite:home/nbecker/kallithea/kallithea.db?timeout=60 2015-03-25 11:01:38.162 INFO [kallithea.lib.auth] getting information about all available permissions Starting server in PID 9452. serving on http://127.0.0.1:5000 I'm assuming that message about mutable-history is harmless Visiting localhost:5000, I get no repositories found. -- Those who fail to understand recursion are doomed to repeat it ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: Invitation to "adopt pytest month"
On Wed, Mar 25, 2015 at 11:15 AM, Thomas De Schampheleire wrote: > Hi, > > On Mon, Mar 23, 2015 at 10:35 AM, Nick Coghlan wrote: >> >> On 20 Mar 2015 09:32, "Mads Kiilerich" wrote: >>> >>> On 03/19/2015 11:30 PM, Brianna Laugher wrote: Hello, I am a contributor to the testing library pytest, and in April we are holding "adopt pytest month". We are inviting open source projects who are interested in possibly taking up pytest, to be paired with experienced pytest users, who can help them convert and write tests, and show ways to make the most of pytest. More info here: https://pytest.org/latest/adopt.html Pytest tests typically require much less boilerplate and feel more "Pythonic" than unittest style tests, and there are also 100+ plugins available, as well as many API hooks for writing your own customisations. (Although, it can run unittest style tests by default, as well as nose and doctests.) So... we think it's pretty cool and have lots of volunteers eager to help spread the love. This is a bit of a cold call email, so please excuse me for butting in. Although I noticed testing is on your "road map", so... if there is someone who would be interested in working with a pytest volunteer on this...please let me know/sign up! :) >>> >>> >>> Thank you for the offer! We could certainly use that - there is a lot of >>> room for improvement here ;-) >>> >>> The big question is if someone "here" have time and timing to work on it >>> on our side and pair with you. That would be awesome. Anyone? >> >> I would be interested in working on it, but I'd be a Kallithea newbie as >> well when it came to actually submitting patches to the project. > > I would also like to express my support to this initiative. > My time is also severely limited, but I'm willing to help guide a > person from the pytest side via e-mail (in cooperation with others > like Mads and Nick, hopefully). > > What would the scope of this month be, besides switching the test runner? > > - One aspect I recently inquired about myself is related to fixtures > and the creation/modification of a repository for the execution of a > test. See > http://lists.sfconservancy.org/pipermail/kallithea-general/2015q1/000232.html > and Mads' reply: > http://lists.sfconservancy.org/pipermail/kallithea-general/2015q1/000272.html. > Fixing this would probably require both help from pytest as internal > knowledge of Kallithea. > > - Improving existing tests to write them in a more pytest way could be > interesting > > - Misc improvements > - do not dump all test files in /tmp but rather in a subdirectory > - check why parallel test execution seems to fail (at least on > nosetests it does) - investigate/profile why each test takes relatively long to complete, while really most tests are making a simple http request and check some output. ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 5 of 5] auth: fix tests after changing API key handling
# HG changeset patch # User Thomas De Schampheleire # Date 1427279629 -3600 # Wed Mar 25 11:33:49 2015 +0100 # Node ID eaeea9ea95b036e8d5eaac16aea1e6c8c62868c9 # Parent e1a755428e3abd3d011c7c033233272dadb34572 auth: fix tests after changing API key handling Return codes when using API keys have changed, and so should the tests. Additionally, improve the auth logic to make a distinction between having no API key (and thus no checking of it, falling back to regular auth), and having a potentially empty one (401 if it is invalid). diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py --- a/kallithea/lib/auth.py +++ b/kallithea/lib/auth.py @@ -754,9 +754,9 @@ % (loc, user)) return redirect_to_login() -# check if we used an APIKEY and it's a valid one -_api_key = request.GET.get('api_key', '') -if _api_key: +# check if we used an API key and it's a valid one +_api_key = request.GET.get('api_key') +if _api_key is not None: # explicit controller is enabled or API is in our whitelist if self.api_access or allowed_api_access(loc, api_key=_api_key): if _api_key in user.api_keys: diff --git a/kallithea/tests/functional/test_login.py b/kallithea/tests/functional/test_login.py --- a/kallithea/tests/functional/test_login.py +++ b/kallithea/tests/functional/test_login.py @@ -319,12 +319,12 @@ self.app.get(url(controller='changeset', action='changeset_raw', repo_name=HG_REPO, revision='tip', api_key=api_key), - status=302) + status=403) @parameterized.expand([ -('none', None, 302), -('empty_string', '', 302), -('fake_number', '123456', 302), +('none', None, 401), +('empty_string', '', 401), +('fake_number', '123456', 401), ('proper_api_key', None, 200) ]) def test_access_whitelisted_page_via_api_key(self, test_name, api_key, code): @@ -339,7 +339,7 @@ self.app.get(url(controller='changeset', action='changeset_raw', repo_name=HG_REPO, revision='tip', api_key=api_key), - status=code) + status=code) def test_access_page_via_extra_api_key(self): whitelist = self._get_api_whitelist(['ChangesetController:changeset_raw']) @@ -372,4 +372,4 @@ action='changeset_raw', repo_name=HG_REPO, revision='tip', api_key=new_api_key.api_key), - status=302) + status=401) ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 4 of 5] auth: simplify logging of regular authentication in LoginRequired
# HG changeset patch # User Thomas De Schampheleire # Date 1427274714 -3600 # Wed Mar 25 10:11:54 2015 +0100 # Node ID e1a755428e3abd3d011c7c033233272dadb34572 # Parent 32ced9e32b506c7441339f67da3c4237de0e6fa3 auth: simplify logging of regular authentication in LoginRequired diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py --- a/kallithea/lib/auth.py +++ b/kallithea/lib/auth.py @@ -772,18 +772,12 @@ log.warning('API access to %s is not allowed' % loc) raise HTTPForbidden() -log.debug('Checking if %s is authenticated @ %s' % (user.username, loc)) -reason = 'RegularAuth' if user.is_authenticated else 'APIAuth' - +# regular user authentication if user.is_authenticated: -log.info('user %s authenticating with:%s IS authenticated on func %s ' - % (user, reason, loc) -) +log.info('user %s authenticated with regular auth @ %s' % (user, loc)) return func(*fargs, **fkwargs) else: -log.warning('user %s authenticating with:%s NOT authenticated on func: %s: ' - % (user, reason, loc) -) +log.warning('user %s NOT authenticated with regular auth @ %s' % (user, loc)) return redirect_to_login() class NotAnonymous(object): ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 3 of 5] auth: return early in LoginRequired on API key validation
# HG changeset patch # User Thomas De Schampheleire # Date 1427274589 -3600 # Wed Mar 25 10:09:49 2015 +0100 # Node ID 32ced9e32b506c7441339f67da3c4237de0e6fa3 # Parent 43b0bc9088d3eedacf7610ffbf2f4b429ae98c45 auth: return early in LoginRequired on API key validation Simplify the logic in the LoginRequired decorator when Kallithea is accessed using an API key. Either: - the key is valid and API access is allowed for the accessed method (continue), or - the key is invalid (401 Unauthorized), or - the accessed method does not allow API access (403 Forbidden) In none of these cases does it make sense to continue checking for user authentication, or redirecting to a login page, so return early. diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py --- a/kallithea/lib/auth.py +++ b/kallithea/lib/auth.py @@ -58,7 +58,7 @@ get_user_group_slug, conditional_cache from kallithea.lib.caching_query import FromCache -from webob.exc import HTTPUnauthorized +from webob.exc import HTTPUnauthorized, HTTPForbidden log = logging.getLogger(__name__) @@ -738,6 +738,7 @@ cls = fargs[0] user = cls.authuser loc = "%s:%s" % (cls.__class__.__name__, func.__name__) +log.debug('Checking access for user %s @ %s' % (user, loc)) def redirect_to_login(): p = url.current() @@ -754,37 +755,34 @@ return redirect_to_login() # check if we used an APIKEY and it's a valid one -# defined whitelist of controllers which API access will be enabled _api_key = request.GET.get('api_key', '') -api_access_valid = allowed_api_access(loc, api_key=_api_key) - -# explicit controller is enabled or API is in our whitelist -if self.api_access or api_access_valid: -log.debug('Checking API KEY access for %s' % cls) -if _api_key and _api_key in user.api_keys: -api_access_valid = True -log.debug('API KEY %s is VALID' % _api_key[-4:]) -else: -api_access_valid = False -if not _api_key: -log.debug("API KEY *NOT* present in request") +if _api_key: +# explicit controller is enabled or API is in our whitelist +if self.api_access or allowed_api_access(loc, api_key=_api_key): +if _api_key in user.api_keys: +log.info('user %s authenticated with API key %s @ %s' + % (user, _api_key[-4:], loc)) +return func(*fargs, **fkwargs) else: -log.warning("API KEY %s *NOT* valid" % _api_key[-4:]) +log.warning('API key %s is NOT valid' % _api_key[-4:]) headers = [('WWW-Authenticate', 'APIKEY realm="Kallithea"')] raise HTTPUnauthorized(headers=headers) +else: +# controller does not allow API access +log.warning('API access to %s is not allowed' % loc) +raise HTTPForbidden() log.debug('Checking if %s is authenticated @ %s' % (user.username, loc)) reason = 'RegularAuth' if user.is_authenticated else 'APIAuth' -if user.is_authenticated or api_access_valid: +if user.is_authenticated: log.info('user %s authenticating with:%s IS authenticated on func %s ' % (user, reason, loc) ) return func(*fargs, **fkwargs) else: log.warning('user %s authenticating with:%s NOT authenticated on func: %s: ' - 'API_ACCESS:%s' - % (user, reason, loc, api_access_valid) + % (user, reason, loc) ) return redirect_to_login() ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 1 of 5] auth: do not redirect to login page on invalid API key
# HG changeset patch # User Thomas De Schampheleire # Date 1427269791 -3600 # Wed Mar 25 08:49:51 2015 +0100 # Node ID c5828585502f1a061f162abe8cbd181c17039843 # Parent 6017996e4dcfda0f5623498a45c51bb184eb67bb auth: do not redirect to login page on invalid API key When accessing Kallithea through an API call, providing an API key, it doesn't make sense to redirect to a login page on failed authentication. Instead, raise a 401 Unauthorized exception. The WWW-authenticate header is a mandatory element for 401 Unauthorized, as specified by RFC 7235. The exact contents do not seem to be important, so define a custom auth scheme 'APIKEY' with a realm of 'Kallithea'. diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py --- a/kallithea/lib/auth.py +++ b/kallithea/lib/auth.py @@ -58,6 +58,7 @@ get_user_group_slug, conditional_cache from kallithea.lib.caching_query import FromCache +from webob.exc import HTTPUnauthorized log = logging.getLogger(__name__) @@ -763,6 +764,8 @@ log.debug("API KEY *NOT* present in request") else: log.warning("API KEY %s *NOT* valid" % _api_key[-4:]) +headers = [('WWW-Authenticate', 'APIKEY realm="Kallithea"')] +raise HTTPUnauthorized(headers=headers) log.debug('Checking if %s is authenticated @ %s' % (user.username, loc)) reason = 'RegularAuth' if user.is_authenticated else 'APIAuth' ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
[PATCH 2 of 5] auth: return early in LoginRequired on invalid IP
# HG changeset patch # User Thomas De Schampheleire # Date 1427271075 -3600 # Wed Mar 25 09:11:15 2015 +0100 # Node ID 43b0bc9088d3eedacf7610ffbf2f4b429ae98c45 # Parent c5828585502f1a061f162abe8cbd181c17039843 auth: return early in LoginRequired on invalid IP Simplify the code of the LoginRequired decorator by returning early when an unacceptable condition is met. diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py --- a/kallithea/lib/auth.py +++ b/kallithea/lib/auth.py @@ -739,13 +739,19 @@ user = cls.authuser loc = "%s:%s" % (cls.__class__.__name__, func.__name__) +def redirect_to_login(): +p = url.current() +log.debug('redirecting to login page with %s' % p) +return redirect(url('login_home', came_from=p)) + # check if our IP is allowed -ip_access_valid = True if not user.ip_allowed: from kallithea.lib import helpers as h h.flash(h.literal(_('IP %s not allowed' % (user.ip_addr))), category='warning') -ip_access_valid = False +log.warning('Access to %s from unallowed IP address for user %s' + % (loc, user)) +return redirect_to_login() # check if we used an APIKEY and it's a valid one # defined whitelist of controllers which API access will be enabled @@ -770,21 +776,17 @@ log.debug('Checking if %s is authenticated @ %s' % (user.username, loc)) reason = 'RegularAuth' if user.is_authenticated else 'APIAuth' -if ip_access_valid and (user.is_authenticated or api_access_valid): +if user.is_authenticated or api_access_valid: log.info('user %s authenticating with:%s IS authenticated on func %s ' % (user, reason, loc) ) return func(*fargs, **fkwargs) else: log.warning('user %s authenticating with:%s NOT authenticated on func: %s: ' - 'IP_ACCESS:%s API_ACCESS:%s' - % (user, reason, loc, ip_access_valid, api_access_valid) + 'API_ACCESS:%s' + % (user, reason, loc, api_access_valid) ) -p = url.current() - -log.debug('redirecting to login page with %s' % p) -return redirect(url('login_home', came_from=p)) - +return redirect_to_login() class NotAnonymous(object): """ ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 0 of 1 RFC] login-required pragma
On Mon, Mar 23, 2015 at 8:36 PM, Mads Kiilerich wrote: > On 03/22/2015 02:34 PM, Thomas De Schampheleire wrote: >> >> Hi Mads, >> >> On Fri, Mar 20, 2015 at 1:09 AM, Mads Kiilerich >> wrote: >>> >>> On 03/19/2015 09:34 PM, Thomas De Schampheleire wrote: Hi, Issue I'm trying to solve is this: we're implementing a script to create a pull request directly from a repo, without using the web interface. This script uses an API key to authenticate. When authentication fails (invalid API key) you still get a valid redirection response, but to a login screen rather than to the pullrequest page. This makes it difficult for the script to differentiate a success from a failure. One would have to search the response body for a certain string, which is fragile. The RFC that I'm sending adds an HTTP pragma header 'login-required' to the response, which can be checked by the script in a reliable way. Let me know what you think of this, and whether you see alternative solutions. >>> >>> >>> So you are creating the web service / API for PR creation you mentioned? >> >> Yes indeed. However, it turned out to be much more easy: just adding >> the 'PullRequestController:create' method in the API whitelist is >> enough. I don't need any further adaptation, so I did not use a >> regular API. >> >> This login-required pragma could solve my problem, but meanwhile I >> think a better approach is to simply check the redirection URL: if it >> contains 'login' then authentication failed. This simpler solution >> does not require changes in Kallithea and is fine for me. So this >> patch can be rejected. > > > How about this: When accessing through API, don't redirect to login on > missing authentication; just fail. Sounds good. I implemented this, and cleaned up the LoginRequired logic. Will send patches in a few minutes... Thanks for the feedback, Thomas ___ kallithea-general mailing list kallithea-general@sfconservancy.org http://lists.sfconservancy.org/mailman/listinfo/kallithea-general