diff --git a/doc/topics/configuration.rst b/doc/topics/configuration.rst index 31777239..5f217b52 100644 --- a/doc/topics/configuration.rst +++ b/doc/topics/configuration.rst @@ -21,9 +21,6 @@ default. backend, and serve it. User must have ``cache_update`` permissions. If not, returns 404. -``cache redirect`` - If the user has ``cache_update`` permissions, download and -cache the package. If not, return a 302 to the ``fallback_url``. - ``none`` - Return a 404 ``pypi.fallback_url`` diff --git a/pypicloud/__init__.py b/pypicloud/__init__.py index 649fe246..0fe873f3 100644 --- a/pypicloud/__init__.py +++ b/pypicloud/__init__.py @@ -4,13 +4,14 @@ import logging from pyramid.config import Configurator from pyramid.renderers import JSON, render -from pyramid.settings import asbool, aslist +from pyramid.settings import asbool from pyramid_beaker import session_factory_from_settings from six.moves.urllib.parse import urlencode # pylint: disable=F0401,E0611 from .cache import get_cache_impl from .route import Root + __version__ = '0.2.1' LOG = logging.getLogger(__name__) @@ -64,18 +65,17 @@ def includeme(config): config.registry.fallback_url = settings.get('pypi.fallback_url', default_url) - fallback_modes = aslist(settings.get('pypi.fallback', ['redirect'])) + fallback_mode = settings.get('pypi.fallback', 'redirect') # Compatibility with the deprecated pypi.use_fallback option if 'pypi.fallback' not in settings and 'pypi.use_fallback' in settings: LOG.warn("Using deprecated option 'pypi.use_fallback'") use_fallback = asbool(settings['pypi.use_fallback']) - fallback_modes = ['redirect'] if use_fallback else [] + fallback_mode = 'redirect' if use_fallback else 'none' modes = ('redirect', 'cache', 'none') - for mode in fallback_modes: - if mode not in modes: - raise ValueError("Invalid value for 'pypi.fallback'. " - "Must be one of %s" % ', '.join(modes)) - config.registry.fallback = fallback_modes + if fallback_mode not in modes: + raise ValueError("Invalid value for 'pypi.fallback'. " + "Must be one of %s" % ', '.join(modes)) + config.registry.fallback = fallback_mode # CACHING DATABASE SETTINGS cache_impl = get_cache_impl(settings) diff --git a/pypicloud/auth.py b/pypicloud/auth.py index 3ae7cc9b..df467de2 100644 --- a/pypicloud/auth.py +++ b/pypicloud/auth.py @@ -7,7 +7,8 @@ from pyramid.security import Everyone, authenticated_userid -# Copied from http://docs.pylonsproject.org/projects/pyramid_cookbook/en/latest/auth/basic.html +# Copied from +# http://docs.pylonsproject.org/projects/pyramid_cookbook/en/latest/auth/basic.html def get_basicauth_credentials(request): """ Get the user/password from HTTP basic auth """ authorization = AUTHORIZATION(request.environ) @@ -148,6 +149,7 @@ def includeme(config): config.add_authentication_policy(BasicAuthenticationPolicy()) config.add_request_method(authenticated_userid, name='userid', reify=True) + config.add_request_method(_forbid, name='forbid') settings = config.get_settings() realm = settings.get('pypi.realm', 'pypi') diff --git a/pypicloud/views/api.py b/pypicloud/views/api.py index eca571fd..7a8e620e 100644 --- a/pypicloud/views/api.py +++ b/pypicloud/views/api.py @@ -63,10 +63,10 @@ def download_package(context, request): """ Download package, or redirect to the download link """ package = request.db.fetch(context.filename) if not package: - if 'cache' not in request.registry.fallback: + if request.registry.fallback != 'cache': return HTTPNotFound() if not request.access.can_update_cache(): - return HTTPForbidden() + return request.forbid() # If we are caching pypi, download the package from pypi and save it locator = FilenameScrapingLocator(request.registry.fallback_url) dists = locator.get_project(context.name) diff --git a/pypicloud/views/login.py b/pypicloud/views/login.py index b3140dea..9d3bea54 100644 --- a/pypicloud/views/login.py +++ b/pypicloud/views/login.py @@ -1,5 +1,4 @@ """ Render views for logging in and out of the web interface """ -from paste.httpheaders import WWW_AUTHENTICATE from pyramid.httpexceptions import HTTPForbidden, HTTPFound, HTTPBadRequest from pyramid.security import NO_PERMISSION_REQUIRED, remember, forget from pyramid.view import view_config @@ -11,36 +10,21 @@ @view_config(context=Root, name='login', request_method='GET', permission=NO_PERMISSION_REQUIRED, subpath=(), renderer='login.jinja2') -@view_config(context=HTTPForbidden, permission=NO_PERMISSION_REQUIRED, - renderer='login.jinja2') def get_login_page(request): """ Catch login and redirect to login wall """ - login_url = request.app_url('login') if request.userid is not None: # User is logged in and fetching /login, so redirect to / - if request.url == login_url: - return HTTPFound(location=request.app_url()) - else: - # If user is not authorized, hide the page - request.response.status_code = 404 - return request.response - if request.url != login_url: - # If pip requested a protected package and it's not authed, prompt for - # credentials - if (request.path.startswith('/simple') or - request.path.startswith('/pypi')): - request.response.status_code = 401 - realm = WWW_AUTHENTICATE.tuples('Basic realm="%s"' % - request.registry.realm) - request.response.headers.update(realm) - return request.response - else: - # If user is not logged in, hide the page - request.response.status_code = 404 - return request.response + return HTTPFound(location=request.app_url()) return {} +@view_config(context=HTTPForbidden, permission=NO_PERMISSION_REQUIRED, + renderer='login.jinja2') +def do_forbidden(request): + """ Intercept 403's and return 401's when necessary """ + return request.forbid() + + @view_config(context=Root, name='login', request_method='POST', subpath=(), renderer='json', permission=NO_PERMISSION_REQUIRED) @argify diff --git a/pypicloud/views/simple.py b/pypicloud/views/simple.py index c4d89b34..c306deb0 100644 --- a/pypicloud/views/simple.py +++ b/pypicloud/views/simple.py @@ -1,7 +1,6 @@ """ Views for simple pip interaction """ import six -from pyramid.httpexceptions import (HTTPBadRequest, HTTPFound, HTTPForbidden, - HTTPNotFound) +from pyramid.httpexceptions import HTTPBadRequest, HTTPFound, HTTPNotFound from pyramid.view import view_config import posixpath @@ -10,10 +9,9 @@ from pyramid_duh import argify, addslash -@view_config(context=Root, request_method='POST', subpath=(), - permission='login', renderer='json') +@view_config(context=Root, request_method='POST', subpath=(), renderer='json') @view_config(context=SimpleResource, request_method='POST', subpath=(), - permission='login', renderer='json') + renderer='json') @argify def upload(request, name, version, content): """ Handle update commands """ @@ -21,7 +19,7 @@ def upload(request, name, version, content): name = normalize_name(name) if action == 'file_upload': if not request.access.has_permission(name, 'write'): - return HTTPForbidden() + return request.forbid() try: return request.db.upload(content.filename, content.file, name=name, version=version) @@ -55,28 +53,30 @@ def package_versions(context, request): normalized_name = normalize_name(context.name) packages = request.db.all(normalized_name) - if not packages: - if ('cache' in request.registry.fallback and - request.access.can_update_cache()): - locator = FilenameScrapingLocator(request.registry.fallback_url) - dists = locator.get_project(context.name) - if not dists: - return HTTPNotFound() - pkgs = {} - for dist in six.itervalues(dists): - filename = posixpath.basename(dist.source_url) - url = request.app_url('api', 'package', dist.name, filename) - pkgs[filename] = url - return {'pkgs': pkgs} - elif 'redirect' in request.registry.fallback: - redirect_url = "%s/%s/" % ( - request.registry.fallback_url.rstrip('/'), context.name) - return HTTPFound(location=redirect_url) - else: + if packages: + if not request.access.has_permission(normalized_name, 'read'): + return request.forbid() + pkgs = {} + for package in packages: + pkgs[package.filename] = package.get_url(request) + return {'pkgs': pkgs} + + elif request.registry.fallback == 'cache': + if not request.access.can_update_cache(): + return request.forbid() + locator = FilenameScrapingLocator(request.registry.fallback_url) + dists = locator.get_project(context.name) + if not dists: return HTTPNotFound() - if not request.access.has_permission(normalized_name, 'read'): - raise HTTPForbidden() - pkgs = {} - for package in packages: - pkgs[package.filename] = package.get_url(request) - return {'pkgs': pkgs} + pkgs = {} + for dist in six.itervalues(dists): + filename = posixpath.basename(dist.source_url) + url = request.app_url('api', 'package', dist.name, filename) + pkgs[filename] = url + return {'pkgs': pkgs} + elif request.registry.fallback == 'redirect': + redirect_url = "%s/%s/" % ( + request.registry.fallback_url.rstrip('/'), context.name) + return HTTPFound(location=redirect_url) + else: + return HTTPNotFound() diff --git a/tests/__init__.py b/tests/__init__.py index 8c0a548b..6757ea9a 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -115,6 +115,7 @@ def setUp(self): self.request.userid = None self.db = self.request.db = DummyCache(self.request) self.request.path_url = '/path/' + self.request.forbid = MagicMock() self.params = {} self.request.param = lambda x: self.params[x] diff --git a/tests/test_api.py b/tests/test_api.py index e08c6325..d97c9fd8 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -114,7 +114,7 @@ def test_download_fallback_cache_no_perm(self): db.fetch.return_value = None context = MagicMock() ret = api.download_package(context, self.request) - self.assertEqual(ret.status_code, 403) + self.assertEqual(ret, self.request.forbid()) @patch('pypicloud.views.api.FilenameScrapingLocator') def test_download_fallback_cache_missing(self, locator): diff --git a/tests/test_auth.py b/tests/test_auth.py index 5f308973..5adfc047 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -55,6 +55,17 @@ def test_valid(self): creds = auth.get_basicauth_credentials(self.request) self.assertEqual(creds, {'login': username, 'password': password}) + def test_forbid(self): + """ When not logged in, forbid() returns 401 """ + ret = auth._forbid(self.request) + self.assertEqual(ret.status_code, 401) + + def test_forbid_logged_in(self): + """ When logged in, forbid() returns 403 """ + self.request.userid = 'abc' + ret = auth._forbid(self.request) + self.assertEqual(ret.status_code, 403) + class TestBasicAuthPolicy(MockServerTest): diff --git a/tests/test_login.py b/tests/test_login.py index 03ae03fb..2fa69256 100644 --- a/tests/test_login.py +++ b/tests/test_login.py @@ -21,32 +21,12 @@ def test_user_fetch_login(self): self.assertEqual(ret.status_code, 302) self.assertEqual(ret.location, '/') - def test_user_fetch_other(self): - """ If a logged-in user if fetching NOT /login, return 404 """ - self.request.userid = 'dsa' - self.request.url = '/abc' - ret = login.get_login_page(self.request) - self.assertEqual(ret.status_code, 404) - def test_anon_fetch_login(self): """ Anonymous user fetching /login renders login page """ self.request.url = '/login' ret = login.get_login_page(self.request) self.assertEqual(ret, {}) - def test_anon_fetch_other(self): - """ Anonymous user fetching NOT /login returns 404 """ - self.request.url = '/wibbles' - ret = login.get_login_page(self.request) - self.assertEqual(ret.status_code, 404) - - def test_anon_fetch_simple(self): - """ Anonymous user fetching /simple/* returns 401 """ - self.request.url = '/simple/requests' - self.request.path = '/simple/requests' - ret = login.get_login_page(self.request) - self.assertEqual(ret.status_code, 401) - class TestLogin(MockServerTest): diff --git a/tests/test_security.py b/tests/test_security.py index c7255871..495e5e1b 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -92,7 +92,7 @@ def test_api_pkg_unauthed(self): """ /api/package/ requires read perms """ response = self.app.get('/api/package/%s/' % self.package.name, expect_errors=True) - self.assertEqual(response.status_int, 404) + self.assertEqual(response.status_int, 401) def test_api_pkg_authed(self): """ /api/package/ requires read perms """ @@ -109,7 +109,7 @@ def test_api_pkg_versions_unauthed(self): self.package.filename) response = self.app.post(url, params, expect_errors=True, headers=_simple_auth('user', 'user')) - self.assertEqual(response.status_int, 404) + self.assertEqual(response.status_int, 403) def test_api_pkg_versions_authed(self): """ /api/package// requires write perms """ @@ -128,7 +128,7 @@ def test_api_delete_unauthed(self): self.package.filename) response = self.app.delete(url, expect_errors=True, headers=_simple_auth('user', 'user')) - self.assertEqual(response.status_int, 404) + self.assertEqual(response.status_int, 403) def test_api_delete_authed(self): """ delete /api/package// requires write perms """ diff --git a/tests/test_simple.py b/tests/test_simple.py index f93b6298..c935cae6 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -48,7 +48,7 @@ def test_upload_no_write_permission(self): content.filename = 'foo-1.2.tar.gz' self.request.access.has_permission.return_value = False response = upload(self.request, name, version, content) - self.assertEqual(response.status_code, 403) + self.assertEqual(response, self.request.forbid()) def test_upload_duplicate(self): """ Uploading a duplicate package returns 400 """ @@ -110,10 +110,10 @@ def setUp(self): self.context = SimplePackageResource(self.request, 'mypkg') def test_no_perms(self): - """ If not in the update_cache groups, return 404 """ + """ If not in the update_cache groups, return 403 """ self.request.access.can_update_cache.return_value = False result = package_versions(self.context, self.request) - self.assertEqual(result.status_code, 404) + self.assertEqual(result, self.request.forbid()) @patch('pypicloud.views.simple.FilenameScrapingLocator') def test_no_dists(self, locator):