Merge branch 'humitos/import/url-repo-validation'

readthedocs/core/validators.py

@@ -4,10 +4,12 @@
 from __future__ import absolute_import
 import re
 
+from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.utils.deconstruct import deconstructible
 from django.utils.translation import ugettext_lazy as _
 from django.core.validators import RegexValidator
+from future.backports.urllib.parse import urlparse
 
 
 domain_regex = (
@@ -45,3 +47,41 @@ class DomainNameValidator(RegexValidator):
             super(DomainNameValidator, self).__call__(idnavalue)
 
 validate_domain_name = DomainNameValidator()
+
+
+@deconstructible
+class RepositoryURLValidator(object):
+
+    def __call__(self, value):
+        allow_private_repos = getattr(settings, 'ALLOW_PRIVATE_REPOS', False)
+        public_schemes = ['https', 'http', 'git', 'ftps', 'ftp']
+        private_schemes = ['ssh', 'ssh+git']
+        valid_schemes = public_schemes
+        if allow_private_repos:
+            valid_schemes += private_schemes
+        url = urlparse(value)
+        if (
+                (
+                    url.scheme not in valid_schemes and \
+                    '@' not in value and \
+                    not value.startswith('lp:')
+                ) or \
+                (
+                    value.startswith('/') or \
+                    value.startswith('file://') or \
+                    value.startswith('.')
+                )
+        ):
+            # Avoid ``/path/to/local/file`` and ``file://`` scheme but allow
+            # ``git@github.com:user/project.git`` and ``lp:bazaar``
+            raise ValidationError(_('Invalid scheme for URL'))
+        elif '&&' in value or '|' in value:
+            raise ValidationError(_('Invalid character in the URL'))
+        elif (
+                ('@' in value or url.scheme in private_schemes) and
+                not allow_private_repos
+        ):
+            raise ValidationError('Clonning via SSH is not supported')
+        return value
+
+validate_repository_url = RepositoryURLValidator()

readthedocs/projects/forms.py

@@ -119,17 +119,6 @@ class ProjectBasicsForm(ProjectForm):
                     _('Invalid project name, a project already exists with that name'))  # yapf: disable # noqa
         return name
 
-    def clean_repo(self):
-        repo = self.cleaned_data.get('repo', '').strip()
-        pvt_repos = getattr(settings, 'ALLOW_PRIVATE_REPOS', False)
-        if '&&' in repo or '|' in repo:
-            raise forms.ValidationError(_('Invalid character in repo name'))
-        elif '@' in repo and not pvt_repos:
-            raise forms.ValidationError(
-                _('It looks like you entered a private repo - please use the '
-                  'public (http:// or git://) clone url'))  # yapf: disable
-        return repo
-
     def clean_remote_repository(self):
         remote_repo = self.cleaned_data.get('remote_repository', None)
         if not remote_repo:

readthedocs/projects/models.py

@@ -22,7 +22,7 @@ from taggit.managers import TaggableManager
 from readthedocs.builds.constants import LATEST, LATEST_VERBOSE_NAME, STABLE
 from readthedocs.core.resolver import resolve, resolve_domain
 from readthedocs.core.utils import broadcast, slugify
-from readthedocs.core.validators import validate_domain_name
+from readthedocs.core.validators import validate_domain_name, validate_repository_url
 from readthedocs.projects import constants
 from readthedocs.projects.exceptions import ProjectConfigurationError
 from readthedocs.projects.querysets import (
@@ -86,6 +86,7 @@ class Project(models.Model):
                                    help_text=_('The reStructuredText '
                                                'description of the project'))
     repo = models.CharField(_('Repository URL'), max_length=255,
+                            validators=[validate_repository_url],
                             help_text=_('Hosted documentation repository URL'))
     repo_type = models.CharField(_('Repository type'), max_length=10,
                                  choices=constants.REPO_CHOICES, default='git')

readthedocs/rtd_tests/tests/test_doc_building.py

@@ -896,7 +896,6 @@ class TestAutoWipeEnvironment(TestCase):
             exists.return_value = True
             self.assertTrue(python_env.is_obsolete)
 
-    @pytest.mark.xfail(reason='build.image is not being considered yet')
     def test_is_obsolete_with_json_different_build_image(self):
         config_data = {
             'build': {

readthedocs/rtd_tests/tests/test_project_forms.py

@@ -1,22 +1,25 @@
+# -*- coding: utf-8 -*-
+
 from __future__ import (
     absolute_import, division, print_function, unicode_literals)
 
 import mock
 from django.contrib.auth.models import User
 from django.test import TestCase
+from django.test.utils import override_settings
 from django_dynamic_fixture import get
 from textclassifier.validators import ClassifierValidator
 
 from readthedocs.projects.exceptions import ProjectSpamError
-from readthedocs.projects.forms import ProjectExtraForm, TranslationForm
+from readthedocs.projects.forms import (
+    ProjectBasicsForm, ProjectExtraForm, TranslationForm)
 from readthedocs.projects.models import Project
 
 
 class TestProjectForms(TestCase):
-
     @mock.patch.object(ClassifierValidator, '__call__')
     def test_form_spam(self, mocked_validator):
-        """Form description field fails spam validation"""
+        """Form description field fails spam validation."""
         mocked_validator.side_effect = ProjectSpamError
 
         data = {
@@ -28,6 +31,67 @@ class TestProjectForms(TestCase):
         with self.assertRaises(ProjectSpamError):
             form.is_valid()
 
+    def test_import_repo_url(self):
+        """Validate different type of repository URLs on importing a Project."""
+
+        common_urls = [
+            # Invalid
+            ('./path/to/relative/folder', False),
+            ('../../path/to/relative/folder', False),
+            ('../../path/to/@/folder', False),
+            ('/path/to/local/folder', False),
+            ('/path/to/@/folder', False),
+            ('file:///path/to/local/folder', False),
+            ('file:///path/to/@/folder', False),
+            ('github.com/humitos/foo', False),
+            ('https://github.com/|/foo', False),
+            ('git://github.com/&&/foo', False),
+            # Valid
+            ('git://github.com/humitos/foo', True),
+            ('http://github.com/humitos/foo', True),
+            ('https://github.com/humitos/foo', True),
+            ('http://gitlab.com/humitos/foo', True),
+            ('http://bitbucket.com/humitos/foo', True),
+            ('ftp://ftpserver.com/humitos/foo', True),
+            ('ftps://ftpserver.com/humitos/foo', True),
+            ('lp:zaraza', True),
+        ]
+
+        public_urls = [
+            ('git@github.com:humitos/foo', False),
+            ('ssh://git@github.com/humitos/foo', False),
+            ('ssh+git://github.com/humitos/foo', False),
+            ('strangeuser@bitbucket.org:strangeuser/readthedocs.git', False),
+            ('user@one-ssh.domain.com:22/_ssh/docs', False),
+        ] + common_urls
+
+        private_urls = [
+            ('git@github.com:humitos/foo', True),
+            ('ssh://git@github.com/humitos/foo', True),
+            ('ssh+git://github.com/humitos/foo', True),
+            ('strangeuser@bitbucket.org:strangeuser/readthedocs.git', True),
+            ('user@one-ssh.domain.com:22/_ssh/docs', True),
+         ] + common_urls
+
+        for url, valid in public_urls:
+            initial = {
+                'name': 'foo',
+                'repo_type': 'git',
+                'repo': url,
+            }
+            form = ProjectBasicsForm(initial)
+            self.assertEqual(form.is_valid(), valid, msg=url)
+
+        with override_settings(ALLOW_PRIVATE_REPOS=True):
+            for url, valid in private_urls:
+                initial = {
+                    'name': 'foo',
+                    'repo_type': 'git',
+                    'repo': url,
+                }
+                form = ProjectBasicsForm(initial)
+                self.assertEqual(form.is_valid(), valid, msg=url)
+
 
 class TestTranslationForm(TestCase):