diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py index 610abce858b..08f785757c8 100644 --- a/readthedocs/core/views/serve.py +++ b/readthedocs/core/views/serve.py @@ -201,14 +201,19 @@ def serve_docs( """Map existing proj, lang, version, filename views to the file format.""" if not version_slug: version_slug = project.get_default_version() + try: - version = project.versions.public(request.user).get(slug=version_slug) + version = project.versions.get(slug=version_slug) except Version.DoesNotExist: - # Properly raise a 404 if the version doesn't exist (or is inactive) and - # a 401 if it does - if project.versions.filter(slug=version_slug, active=True).exists(): - return _serve_401(request, project) raise Http404('Version does not exist.') + + # only project admins should see private versions + if not AdminPermission.is_member(user=request.user, obj=project): + if version.privacy_level == constants.PRIVATE: + return _serve_401(request, project) + if not version.active: + raise Http404('Version does not exist.') + filename = resolve_path( subproject or project, # Resolve the subproject if it exists version_slug=version_slug, @@ -216,9 +221,6 @@ def serve_docs( filename=filename, subdomain=True, # subdomain will make it a "full" path without a URL prefix ) - if (version.privacy_level == constants.PRIVATE and - not AdminPermission.is_member(user=request.user, obj=project)): - return _serve_401(request, project) return _serve_symlink_docs( request, filename=filename, diff --git a/readthedocs/rtd_tests/tests/test_redirects.py b/readthedocs/rtd_tests/tests/test_redirects.py index 9d0836a7516..eb99e9dfa51 100644 --- a/readthedocs/rtd_tests/tests/test_redirects.py +++ b/readthedocs/rtd_tests/tests/test_redirects.py @@ -1,6 +1,7 @@ import logging -from django.http import Http404 +from django.http import Http404, HttpResponse +from django.contrib.auth.models import User from django.test import TestCase from django.test.utils import override_settings from django_dynamic_fixture import fixture, get @@ -45,8 +46,25 @@ def test_proper_url(self): r = self.client.get('/docs/pip/') self.assertEqual(r.status_code, 302) self.assertEqual( - r['Location'], 'http://readthedocs.org/docs/pip/en/latest/', - ) + r['Location'], 'http://readthedocs.org/docs/pip/en/latest/') + + def test_owner_with_multiple_projects_can_serve_the_requested_version(self): + # GH #4350 + eric = User.objects.get(username='eric') + notpip = eric.projects.exclude(slug='pip').first() + notpip.versions.create_latest() + + self.client.login(username='eric', password='test') + with patch('readthedocs.core.views.serve._serve_symlink_docs') as _serve_docs: + _serve_docs.return_value = HttpResponse() + self.client.get('/docs/pip/en/latest/') + self.assertTrue(_serve_docs.called) + + def test_inactive_docs_should_return_404_if_the_user_is_not_the_project_admin(self): + pip = Project.objects.get(slug='pip') + pip.versions.filter(slug='latest').update(active=False) + response = self.client.get('/docs/pip/en/latest/') + self.assertEqual(response.status_code, 404) # Specific Page Redirects def test_proper_page_on_main_site(self):