AC-454. Update SCM URL validation and error messages, check for special github/bitbucket URLs.

awx/main/serializers.py

@@ -301,29 +301,63 @@ class ProjectSerializer(BaseSerializer):
     def validate_scm_url(self, attrs, source):
         if self.object:
             scm_type = attrs.get('scm_type', self.object.scm_type) or ''
-            scm_username = attrs.get('scm_username', self.object.scm_username) or ''
-            scm_password = attrs.get('scm_password', self.object.scm_password) or ''
         else:
             scm_type = attrs.get('scm_type', '') or ''
-            scm_username = attrs.get('scm_username', '') or ''
-            scm_password = attrs.get('scm_password', '') or ''
         scm_url = unicode(attrs.get(source, None) or '')
         if not scm_type:
             return attrs
         try:
-            if scm_username and scm_password:
+            scm_url = update_scm_url(scm_type, scm_url)
-                scm_url = update_scm_url(scm_type, scm_url, scm_username,
-                                         '********')
-            elif scm_username:
-                scm_url = update_scm_url(scm_type, scm_url, scm_username)
-            else:
-                scm_url = update_scm_url(scm_type, scm_url)
         except ValueError, e:
             raise serializers.ValidationError((e.args or ('Invalid SCM URL',))[0])
         scm_url_parts = urlparse.urlsplit(scm_url)
-        print scm_url_parts
+        #print scm_url_parts
         if scm_type and not any(scm_url_parts):
-            raise serializers.ValidationError('SCM URL must be provided')
+            raise serializers.ValidationError('SCM URL is required')
+        return attrs
+
+    def validate_scm_username(self, attrs, source):
+        if self.object:
+            scm_type = attrs.get('scm_type', self.object.scm_type) or ''
+            scm_url = unicode(attrs.get('scm_url', self.object.scm_url) or '')
+            scm_username = attrs.get('scm_username', self.object.scm_username) or ''
+        else:
+            scm_type = attrs.get('scm_type', '') or ''
+            scm_url = unicode(attrs.get('scm_url', '') or '')
+            scm_username = attrs.get('scm_username', '') or ''
+        if not scm_type:
+            return attrs
+        try:
+            if scm_url and scm_username:
+                update_scm_url(scm_type, scm_url, scm_username)
+        except ValueError, e:
+            raise serializers.ValidationError((e.args or ('Invalid SCM username',))[0])
+        return attrs
+
+    def validate_scm_password(self, attrs, source):
+        if self.object:
+            scm_type = attrs.get('scm_type', self.object.scm_type) or ''
+            scm_url = unicode(attrs.get('scm_url', self.object.scm_url) or '')
+            scm_username = attrs.get('scm_username', self.object.scm_username) or ''
+            scm_password = attrs.get('scm_password', self.object.scm_password) or ''
+        else:
+            scm_type = attrs.get('scm_type', '') or ''
+            scm_url = unicode(attrs.get('scm_url', '') or '')
+            scm_username = attrs.get('scm_username', '') or ''
+            scm_password = attrs.get('scm_password', '') or ''
+        if not scm_type:
+            return attrs
+        try:
+            try:
+                if scm_url and scm_username:
+                    update_scm_url(scm_type, scm_url, scm_username)
+            except ValueError:
+                pass
+            else:
+                if scm_url and scm_username and scm_password:
+                    update_scm_url(scm_type, scm_url, scm_username, '**')
+        except ValueError, e:
+            raise serializers.ValidationError((e.args or ('Invalid SCM password',))[0])
         return attrs
 
 class ProjectPlaybooksSerializer(ProjectSerializer):

awx/main/tests/projects.py

@@ -692,6 +692,17 @@ class ProjectUpdatesTest(BaseTransactionTest):
             # - file:///path/to/repo.git/
             ('git', 'file:///path/to/repo.git', ValueError, ValueError, ValueError),
             ('git', 'file://localhost/path/to/repo.git', ValueError, ValueError, ValueError),
+            # Invalid SSH URLs:
+            ('git', 'ssh:github.com:ansible/ansible-examples.git', ValueError, ValueError, ValueError),
+            ('git', 'ssh://github.com:ansible/ansible-examples.git', ValueError, ValueError, ValueError),
+            # Special case for github URLs:
+            ('git', 'git@github.com:ansible/ansible-examples.git', 'ssh://git@github.com/ansible/ansible-examples.git', ValueError, ValueError),
+            ('git', 'bob@github.com:ansible/ansible-examples.git', ValueError, ValueError, ValueError),
+            # Special case for bitbucket URLs:
+            ('git', 'ssh://git@bitbucket.org/foo/bar.git', None, ValueError, ValueError),
+            ('git', 'ssh://git@altssh.bitbucket.org:443/foo/bar.git', None, ValueError, ValueError),
+            ('git', 'ssh://hg@bitbucket.org/foo/bar.git', ValueError, ValueError, ValueError),
+            ('git', 'ssh://hg@altssh.bitbucket.org:443/foo/bar.git', ValueError, ValueError, ValueError),
 
             # hg: http://www.selenic.com/mercurial/hg.1.html#url-paths
             # - local/filesystem/path[#revision]
@@ -743,6 +754,11 @@ class ProjectUpdatesTest(BaseTransactionTest):
             ('hg', 'ssh://user@host.xz:1022/path/to/repo#rev', None, 'ssh://testuser@host.xz:1022/path/to/repo#rev', 'ssh://testuser:testpass@host.xz:1022/path/to/repo#rev'),
             ('hg', 'ssh://user:pass@host.xz/path/to/repo/#rev', None, 'ssh://testuser:pass@host.xz/path/to/repo/#rev', 'ssh://testuser:testpass@host.xz/path/to/repo/#rev'),
             ('hg', 'ssh://user:pass@host.xz:1022/path/to/repo#rev', None, 'ssh://testuser:pass@host.xz:1022/path/to/repo#rev', 'ssh://testuser:testpass@host.xz:1022/path/to/repo#rev'),
+            # Special case for bitbucket URLs:
+            ('hg', 'ssh://hg@bitbucket.org/foo/bar', None, ValueError, ValueError),
+            ('hg', 'ssh://hg@altssh.bitbucket.org:443/foo/bar', None, ValueError, ValueError),
+            ('hg', 'ssh://bob@bitbucket.org/foo/bar', ValueError, ValueError, ValueError),
+            ('hg', 'ssh://bob@altssh.bitbucket.org:443/foo/bar', ValueError, ValueError, ValueError),
 
             # svn: http://svnbook.red-bean.com/en/1.7/svn-book.html#svn.advanced.reposurls
             # - file:///    Direct repository access (on local disk)
@@ -780,11 +796,11 @@ class ProjectUpdatesTest(BaseTransactionTest):
             # FIXME: Add some invalid URLs.
         ]
         def is_exception(e):
-            return bool(isinstance(new_url, Exception) or
+            return bool(isinstance(e, Exception) or
-                        isinstance(new_url, type) and
+                        (isinstance(e, type) and issubclass(e, Exception)))
-                        issubclass(new_url, Exception))
         for url_opts in urls_to_test:
             scm_type, url, new_url, new_url_u, new_url_up = url_opts
+            #print url
             new_url = new_url or url
             new_url_u = new_url_u or url
             new_url_up = new_url_up or url

awx/main/utils.py

@@ -140,10 +140,14 @@ def update_scm_url(scm_type, url, username=True, password=True):
     # hg: http://www.selenic.com/mercurial/hg.1.html#url-paths
     # svn: http://svnbook.red-bean.com/en/1.7/svn-book.html#svn.advanced.reposurls
     if scm_type not in ('git', 'hg', 'svn'):
-        raise ValueError('unsupported SCM type "%s"' % str(scm_type))
+        raise ValueError('Unsupported SCM type "%s"' % str(scm_type))
     if not url.strip():
         return ''
     parts = urlparse.urlsplit(url)
+    try:
+        parts.port
+    except ValueError:
+        raise ValueError('Invalid %s URL' % scm_type)
     #print parts
     if '://' not in url:
         # Handle SCP-style URLs for git (e.g. [user@]host.xz:path/to/repo.git/).
@@ -153,6 +157,9 @@ def update_scm_url(scm_type, url, username=True, password=True):
             modified_url = '@'.join([userpass, hostpath])
             parts = urlparse.urlsplit('ssh://%s' % modified_url)
         elif scm_type == 'git' and ':' in url:
+            if url.count(':') > 1:
+                raise ValueError('Invalid %s URL' % scm_type)
+                
             modified_url = '/'.join(url.split(':', 1))
             parts = urlparse.urlsplit('ssh://%s' % modified_url)
         # Handle local paths specified without file scheme (e.g. /path/to/foo).
@@ -163,7 +170,7 @@ def update_scm_url(scm_type, url, username=True, password=True):
             else:
                 parts = urlparse.urlsplit('file://%s' % url)
         else:
-            raise ValueError('unsupported %s URL "%s"' % (scm_type, url))
+            raise ValueError('Invalid %s URL' % scm_type)
         #print parts
     # Validate that scheme is valid for given scm_type.
     scm_type_schemes = {
@@ -172,11 +179,11 @@ def update_scm_url(scm_type, url, username=True, password=True):
         'svn': ('http', 'https', 'svn', 'svn+ssh'),
     }
     if parts.scheme not in scm_type_schemes.get(scm_type, ()):
-        raise ValueError('unsupported %s scheme "%s"' % (scm_type, parts.scheme))
+        raise ValueError('Unsupported %s URL' % scm_type)
     if parts.scheme == 'file' and parts.netloc not in ('', 'localhost'):
-        raise ValueError('unsupported host "%s" for file:// URL' % (parts.netloc))
+        raise ValueError('Unsupported host "%s" for file:// URL' % (parts.netloc))
     elif parts.scheme != 'file' and not parts.netloc:
-        raise ValueError('host is required for %s URL' % parts.scheme)
+        raise ValueError('Host is required for %s URL' % parts.scheme)
     if username is True:
         netloc_username = parts.username or ''
     elif username:
@@ -189,6 +196,19 @@ def update_scm_url(scm_type, url, username=True, password=True):
         netloc_password = password
     else:
         netloc_password = ''
+
+    # Special handling for github/bitbucket SSH URLs.
+    special_git_hosts = ('github.com', 'bitbucket.org', 'altssh.bitbucket.org')
+    if scm_type == 'git' and parts.scheme == 'ssh' and parts.hostname in special_git_hosts and netloc_username != 'git':
+        raise ValueError('Username must be "git" for SSH access to %s.' % parts.hostname)
+    if scm_type == 'git' and parts.scheme == 'ssh' and parts.hostname in special_git_hosts and netloc_password:
+        raise ValueError('Password not allowed for SSH access to %s.' % parts.hostname)
+    special_hg_hosts = ('bitbucket.org', 'altssh.bitbucket.org')
+    if scm_type == 'hg' and parts.scheme == 'ssh' and parts.hostname in special_hg_hosts and netloc_username != 'hg':
+        raise ValueError('Username must be "hg" for SSH access to %s.' % parts.hostname)
+    if scm_type == 'hg' and parts.scheme == 'ssh' and netloc_password:
+        raise ValueError('Password not supported for SSH with Mercurial.')
+
     if netloc_username and parts.scheme != 'file':
         netloc = u':'.join(filter(None, [netloc_username, netloc_password]))
     else: