pyshop
pyshop copied to clipboard
mardiros
Fix upload error codes
In the pursuit of evaluating pyshop for use, I came across some of the same problems outlined in #38. Specifically, failing uploads which the distutils uploader report as HTTP 200 and the log file recording a raised HTTPForbidden exception.
As an any experience I could reasonably apply to Pyramid is work on a handful of TurboGears 1.x projects I thought it would be interesting to experiment with figuring out what might be wrong.
Where I started was just using curl to attempt to reproduce the condition. I was able to without much trouble. POSTing to /simple/ with a version that does not match the pyshop.upload.sanitize.regex does not return HTTP 403. Much to my surprise it actually returns an HTTP 302 redirect to the same URL:
$ curl -u admin:changeme -v -X POST --data "version=1.0dev" http://127.0.0.1:18000/simple/
* Hostname was NOT found in DNS cache
* Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 18000 (#0)
* Server auth using Basic with user 'admin'
> POST /simple/ HTTP/1.1
> Authorization: Basic YWRtaW46Y2hhbmdlbWU=
> User-Agent: curl/7.38.0
> Host: 127.0.0.1:18000
> Accept: */*
> Content-Length: 14
> Content-Type: application/x-www-form-urlencoded
>
* upload completely sent off: 14 out of 14 bytes
< HTTP/1.1 302 Found
< Content-Length: 198
< Content-Type: text/html; charset=UTF-8
< Date: Wed, 23 Sep 2015 15:53:34 GMT
< Location: http://127.0.0.1:18000/simple/
* Server waitress is not blacklisted
< Server: waitress
<
<html>
<head>
<title>302 Found</title>
</head>
<body>
<h1>302 Found</h1>
The resource was found at http://127.0.0.1:18000/simple/; you should be redirected automatically.
</body>
* Connection #0 to host 127.0.0.1 left intact
</html>
The log file does indeed record the exception:
2015-09-23 16:54:55,711 ERROR [pyshop.views.base][waitress] Error on view UploadReleaseFile
Traceback (most recent call last):
File "/Users/callan/v/pyshop/lib/python2.7/site-packages/pyshop/views/base.py", line 44, in __call__
response = self.render()
File "/Users/callan/v/pyshop/lib/python2.7/site-packages/pyshop/views/simple.py", line 82, in render
raise exc.HTTPForbidden()
HTTPForbidden: Access was denied to this resource.
Next, and as a start to resolving this issue I have added a couple tests, one which fails due to an HTTP 302 response rather than an HTTP 403 response. These are included on this PR. To my knowledge this new single test is the only test that is functionally testing the basic authentication stack.
I spent some time reading the code, looking at how the views are routed and exceptions are handled coming to the conclusion that the problem lies in pyshop.views.credentials.authbasic. Specifically the exception handlers that were added in what I can only assume was an attempt to support basic authentication. This is rather strange given that there is an authentication policy that also handles basic authentication support.
I have done some fiddling, removing references to pyshop.views.credentials.authbasic entirely and removing the associated exception handling views. I then see no effect in the passing number of tests but that could definitely just be an artifact. Again, as far as I'm aware there are no tests apart from the one I've added performing functional testing of the basic authentication support.
I'm pretty confident in saying that as things stand, none of the views associated with pyshop.views.credentials.authbasic can raise an HTTPForbidden exception and actually have a client receive an HTTP 403.
This leads to some fairly simple questions:
- Is
pyshop.views.credentials.authbasicrequired? An authentication policy exists that appears to work fine without it. - Are any of the exception handlers that globally react to
pyramid.exceptions.Forbiddena good idea? I expect that similar problems will apply to the one that currently displays the login page. There has to be a cleaner way to do this within Pyramid outside of resulting to a global exception handler forpyramid.exceptions.Forbidden, no?
/cc @powellchristoph
pip use basic authentication for priviate packages repositories, so it is used.
Thanks for you pull request, this is really a great job. I will look closely when I have the time
