From 00e3787f5aad41b5d0ec9224fa5a44c3d8b9713e Mon Sep 17 00:00:00 2001 From: Chris Church Date: Thu, 12 Sep 2013 16:02:44 -0400 Subject: [PATCH] AC-454. Update SCM URL validation and error messages, check for special github/bitbucket URLs. --- awx/main/serializers.py | 60 +++++++++++++++++++++++++++++--------- awx/main/tests/projects.py | 22 ++++++++++++-- awx/main/utils.py | 30 +++++++++++++++---- 3 files changed, 91 insertions(+), 21 deletions(-) diff --git a/awx/main/serializers.py b/awx/main/serializers.py index cecf754c3d..e30748ca55 100644 --- a/awx/main/serializers.py +++ b/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_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) + 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): diff --git a/awx/main/tests/projects.py b/awx/main/tests/projects.py index 18bdc1b993..16baa7c655 100644 --- a/awx/main/tests/projects.py +++ b/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 - isinstance(new_url, type) and - issubclass(new_url, Exception)) + return bool(isinstance(e, Exception) or + (isinstance(e, type) and issubclass(e, 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 diff --git a/awx/main/utils.py b/awx/main/utils.py index 7231952bbb..562884a346 100644 --- a/awx/main/utils.py +++ b/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: