Fix invalid org name handling in configure and resolve (Azure#665)

* Fix invalid org name handling in configure and resolve

* Fix TFS renamed to Azure DevOps Server

* Style check fix

azure-devops/azext_devops/dev/common/services.py

@@ -30,7 +30,7 @@
 from ._credentials import get_credential
 from .git import get_remote_url
 from .vsts_git_url_info import VstsGitUrlInfo
-from .uri import uri_parse_instance_from_git_uri
+from .uri import uri_parse_instance_from_git_uri, is_valid_url
 from .uuid import is_uuid
 from .telemetry import vsts_tracking_data, init_telemetry
 
@@ -294,11 +294,11 @@ def get_base_url(organization):
 
 def _team_organization_arg_error():
     return CLIError('--organization must be specified. The value should be the URI of your Azure DevOps '
-                    'organization, for example: https://dev.azure.com/MyOrganization/ or your TFS organization. '
-                    'You can set a default value by running: az devops configure --defaults '
+                    'organization, for example: https://dev.azure.com/MyOrganization/ or your Azure DevOps Server '
+                    'organization. You can set a default value by running: az devops configure --defaults '
                     'organization=https://dev.azure.com/MyOrganization/. For auto detection to work '
                     '(--detect true), you must be in a local Git directory that has a "remote" referencing a '
-                    'Azure DevOps or TFS repository.')
+                    'Azure DevOps or Azure DevOps Server repository.')
 
 
 def _raise_team_project_arg_error():
@@ -344,6 +344,8 @@ def resolve_instance_project_and_repo(
         else:
             projectFromConfig = _resolve_project_from_config(project, False)
             vsts_tracking_data.properties[PROJECT_IGNORED_FROM_CONFIG] = projectFromConfig is not None
+    if not is_valid_url(organization):
+        raise _team_organization_arg_error()
     if project_required and project is None:
         _raise_team_project_arg_error()
     if repo_required and repo is None:

azure-devops/azext_devops/dev/common/uri.py

@@ -37,3 +37,10 @@ def uri_parse_instance_from_git_uri(uri):
             return parsed_uri.scheme + "://" + parsed_uri.hostname + "/" + org_name
 
     return uri
+
+
+def is_valid_url(url):
+    parsed_url = uri_parse(url)
+    if not parsed_url.scheme or not parsed_url.netloc:
+        return False
+    return True

azure-devops/azext_devops/dev/team/configure.py

@@ -15,6 +15,7 @@
 from azext_devops.dev.common.config import (set_global_config_value, AZ_DEVOPS_GLOBAL_CONFIG_PATH)
 from azext_devops.dev.common.const import (CLI_ENV_VARIABLE_PREFIX, DEFAULTS_SECTION, DEVOPS_ORGANIZATION_DEFAULT,
                                            DEVOPS_TEAM_PROJECT_DEFAULT)
+from azext_devops.dev.common.uri import is_valid_url
 
 logger = get_logger(__name__)
 
@@ -26,7 +27,7 @@ def configure(defaults=None, use_git_aliases=None, list_config=False):
     """Configure the Azure DevOps CLI or view your configuration.
     :param defaults: Space separated 'name=value' pairs for common arguments defaults,
         e.g. '--defaults project=my-project-name organization=https://dev.azure.com/organizationName
-        arg=value' Use '' to clear the defaults, e.g. --defaults project=''.
+        arg=value'. Use '' to clear the defaults, e.g. --defaults project=''.
     :type defaults: str
     :param use_git_aliases: Set to 'true' to configure Git aliases global git config file
         (to enable commands like "git pr list").
@@ -46,7 +47,8 @@ def configure(defaults=None, use_git_aliases=None, list_config=False):
             if parts[0] not in CONFIG_VALID_DEFAULT_KEYS_LIST:
                 raise CLIError('usage error: invalid default value setup. Supported values are {}.'
                                .format(CONFIG_VALID_DEFAULT_KEYS_LIST))
-            set_global_config_value(DEFAULTS_SECTION, parts[0], parts[1])
+            _validate_configuration(key=parts[0], value=parts[1])
+            set_global_config_value(section=DEFAULTS_SECTION, option=parts[0], value=parts[1])
     if use_git_aliases is not None:
         from azext_devops.dev.repos.git_alias import setup_git_aliases, clear_git_aliases
         if use_git_aliases:
@@ -78,6 +80,14 @@ def print_current_configuration(file_config=None):
         print('\n'.join(['{}'.format(ev) for ev in env_vars]))
 
 
+def _validate_configuration(key, value):
+    if key == DEVOPS_ORGANIZATION_DEFAULT:
+        # value can be '' or a valid url
+        if not value == '' and not is_valid_url(value):
+            raise CLIError('Organization should be a valid Azure DevOps or Azure DevOps Server repository url. '
+                           'See command help for details.')
+
+
 MSG_INTRO = '\nWelcome to the Azure DevOps CLI! This command will guide you through setting some default values.\n'
 MSG_CLOSING = '\nYou\'re all set! Here are some commands to try:\n' \
               ' $ az devops login\n' \

azure-devops/azext_devops/test/common/test_services.py

@@ -19,6 +19,7 @@
 
 from azext_devops.dev.common.services import (get_connection,
                                               clear_connection_cache,
+                                              resolve_instance,
                                               resolve_instance_project_and_repo)
 
 
@@ -50,7 +51,7 @@ def test_get_connection_cache_works_mulitple_organization(self):
     def test_resolve_instance_project_and_repo(self):
         try:
             resolve_instance_project_and_repo(detect='false',
-                                              organization='',
+                                              organization='https://dev.azure.com/someorg',
                                               project='',
                                               project_required=False,
                                               repo=None,
@@ -59,6 +60,17 @@ def test_resolve_instance_project_and_repo(self):
         except CLIError as ex:
             self.assertEqual(str(ex), '--repository must be specified')
 
+    def test_resolve_instance_should_fail_for_invalid_org_url(self):
+        with self.assertRaises(Exception) as exc:
+            resolve_instance(organization='myorg', detect=False)
+        self.assertEqual(str(exc.exception), self.ORG_ERROR_STRING)
+
+    ORG_ERROR_STRING = ('--organization must be specified. The value should be the URI of your Azure DevOps '
+                    'organization, for example: https://dev.azure.com/MyOrganization/ or your Azure DevOps Server organization. '
+                    'You can set a default value by running: az devops configure --defaults '
+                    'organization=https://dev.azure.com/MyOrganization/. For auto detection to work '
+                    '(--detect true), you must be in a local Git directory that has a "remote" referencing a '
+                    'Azure DevOps or Azure DevOps Server repository.')
 
 if __name__ == '__main__':
     unittest.main()

azure-devops/azext_devops/test/team/test_configure.py

@@ -4,15 +4,55 @@
 # --------------------------------------------------------------------------------------------
 
 import unittest
-from azext_devops.dev.team.configure import (print_current_configuration)
+try:
+    # Attempt to load mock (works on Python 3.3 and above)
+    from unittest.mock import patch
+except ImportError:
+    # Attempt to load mock (works on Python version below 3.3)
+    from mock import patch
+from azext_devops.dev.common.const import DEFAULTS_SECTION
+from azext_devops.dev.team.configure import print_current_configuration, configure
+from azext_devops.test.utils.authentication import AuthenticatedTests
+from azext_devops.dev.common.services import clear_connection_cache
 
+class TestConfigureMethods(AuthenticatedTests):
 
-class TestConfigureMethods(unittest.TestCase):
+    _TEST_DEVOPS_ORGANIZATION = 'https://someorganization.visualstudio.com'
+
+    def setUp(self):
+        self.authentication_setup()
+        self.set_config_patcher = patch('azext_devops.dev.team.configure.set_global_config_value')
+        self.mock_set_config = self.set_config_patcher.start()
+
+        #clear connection cache before running each test
+        clear_connection_cache()
+
+    def tearDown(self):
+        patch.stopall()
 
     def test_print_current_configuration(self):
         # simple validation that we don't get an exception
         print_current_configuration()
+
+    def test_setting_invalid_org_url_throws_error(self):
+        with self.assertRaises(Exception) as exc:
+            configure(defaults=['organization=abc'])
+        self.assertEqual(str(exc.exception),r'Organization should be a valid Azure DevOps or Azure DevOps Server repository url. See command help for details.')
+        self.mock_set_config.assert_not_called()
+
+    def test_setting_valid_org_url_should_work(self):
+        configure(defaults=['organization={}'.format(self._TEST_DEVOPS_ORGANIZATION)])
+        self.mock_set_config.assert_called_once_with(section=DEFAULTS_SECTION, option='organization', value=self._TEST_DEVOPS_ORGANIZATION)
+
+    def test_setting_org_to_blank_should_succeed(self):
+        configure(defaults=["organization="])
+        self.mock_set_config.assert_called_once_with(section=DEFAULTS_SECTION, option='organization', value='')
 
+    def test_setting_invalid_Default_key_should_fail(self):
+        with self.assertRaises(Exception) as exc:
+            configure(defaults=["abcd=abcd"])
+        self.assertEqual(str(exc.exception), r"usage error: invalid default value setup. Supported values are ['organization', 'project'].")
+        self.mock_set_config.assert_not_called()
 
 if __name__ == '__main__':
     unittest.main()