Repository: typesafehub/conductr-cli

Issue:
bintray credential parsing
Apparently bintray credential file formats can screw up our parsing of them. Our parser will have to become more sophisticated. From @henrikengstrom:

> I haven’t changed the credentials file lately and it contained more than `user` and `password`, e.g. `realm` and `host`.


Discussion:
markusjura: The current code would fail if the `value` would contain a `=` sign, e.g.

```
secret = ah5k=sdh
```

We should split the line by key value only by the first `=` sign.
huntc: Good catch! I shall fix tomorrow!

Patch:
diff --git a/conductr_cli/resolvers/bintray_resolver.py b/conductr_cli/resolvers/bintray_resolver.py
index cfb0ce9..2186cb6 100644
--- a/conductr_cli/resolvers/bintray_resolver.py
+++ b/conductr_cli/resolvers/bintray_resolver.py
@@ -5,13 +5,14 @@ from requests.exceptions import HTTPError
 import json
 import logging
 import os
+import re
 import requests
 
 
 BINTRAY_API_BASE_URL = 'https://api.bintray.com'
 BINTRAY_DOWNLOAD_BASE_URL = 'https://dl.bintray.com'
-BINTRAY_DOWNLOAD_REALM = 'Bintray'
 BINTRAY_CREDENTIAL_FILE_PATH = '{}/.bintray/.credentials'.format(os.path.expanduser('~'))
+BINTRAY_PROPERTIES_RE = re.compile('^(\S+)\s*=\s*([\S]+)$')
 
 
 def resolve_bundle(cache_dir, uri):
@@ -23,8 +24,7 @@ def resolve_bundle(cache_dir, uri):
         bundle_download_url = bintray_download_url(bintray_username, bintray_password, org, repo, package_name,
                                                    compatibility_version, digest)
         if bundle_download_url:
-            auth = (BINTRAY_DOWNLOAD_REALM, bintray_username, bintray_password) if bintray_username else None
-            return uri_resolver.resolve_bundle(cache_dir, bundle_download_url, auth)
+            return uri_resolver.resolve_bundle(cache_dir, bundle_download_url)
         else:
             return False, None, None
     except MalformedBundleUriError:
@@ -61,8 +61,13 @@ def load_bintray_credentials():
             lines = [line.replace('\n', '') for line in cred_file.readlines()]
             data = dict()
             for line in lines:
-                key, value = line.replace(' = ', '=').split('=')
-                data[key] = value
+                match = BINTRAY_PROPERTIES_RE.match(line)
+                if match is not None:
+                    try:
+                        key, value = match.group(1, 2)
+                        data[key] = value
+                    except IndexError:
+                        pass
 
             username = None if 'user' not in data else data['user']
             password = None if 'password' not in data else data['password']
diff --git a/conductr_cli/resolvers/uri_resolver.py b/conductr_cli/resolvers/uri_resolver.py
index 8207d26..45a12df 100644
--- a/conductr_cli/resolvers/uri_resolver.py
+++ b/conductr_cli/resolvers/uri_resolver.py
@@ -6,10 +6,9 @@ from conductr_cli import screen_utils
 import os
 import logging
 import shutil
-import urllib
 
 
-def resolve_bundle(cache_dir, uri, auth=None):
+def resolve_bundle(cache_dir, uri):
     log = logging.getLogger(__name__)
 
     if not os.path.exists(cache_dir):
@@ -24,7 +23,7 @@ def resolve_bundle(cache_dir, uri, auth=None):
         if os.path.exists(tmp_download_path):
             os.remove(tmp_download_path)
 
-        download_bundle(log, bundle_url, tmp_download_path, auth)
+        download_bundle(log, bundle_url, tmp_download_path)
 
         shutil.move(tmp_download_path, cached_file)
         return True, bundle_name, cached_file
@@ -63,22 +62,12 @@ def cache_path(cache_dir, uri):
     return '{}/{}'.format(cache_dir, basename)
 
 
-def download_bundle(log, bundle_url, tmp_download_path, auth):
+def download_bundle(log, bundle_url, tmp_download_path):
     log.info('Retrieving {}'.format(bundle_url))
 
     parsed = urlparse(bundle_url, scheme='file')
     is_http_download = parsed.scheme == 'http' or parsed.scheme == 'https'
 
-    if is_http_download and auth:
-        realm, username, password = auth
-        authinfo = urllib.request.HTTPBasicAuthHandler()
-        authinfo.add_password(realm=realm,
-                              uri=bundle_url,
-                              user=username,
-                              passwd=password)
-        opener = urllib.request.build_opener(authinfo)
-        urllib.request.install_opener(opener)
-
     if log.is_progress_enabled() and is_http_download:
         urlretrieve(bundle_url, tmp_download_path, reporthook=show_progress(log))
     else:


Test patch:
diff --git a/conductr_cli/resolvers/test/test_bintray_resolver.py b/conductr_cli/resolvers/test/test_bintray_resolver.py
index 2c8a77b..1d7b723 100644
--- a/conductr_cli/resolvers/test/test_bintray_resolver.py
+++ b/conductr_cli/resolvers/test/test_bintray_resolver.py
@@ -32,8 +32,7 @@ class TestResolveBundle(TestCase):
         parse_mock.assert_called_with('bundle-name:v1')
         bintray_download_url_mock.assert_called_with('username', 'password', 'typesafe', 'bundle', 'bundle-name', 'v1',
                                                      'digest')
-        resolve_bundle_mock.assert_called_with('/cache-dir', 'https://dl.bintray.com/download.zip',
-                                               ('Bintray', 'username', 'password'))
+        resolve_bundle_mock.assert_called_with('/cache-dir', 'https://dl.bintray.com/download.zip')
 
     def test_bintray_version_not_found(self):
         load_bintray_credentials_mock = MagicMock(return_value=('username', 'password'))
@@ -390,7 +389,8 @@ class TestLoadBintrayCredentials(TestCase):
     def test_success(self):
         bintray_credential_file = strip_margin(
             """|user = user1
-               |password = secret
+               |password = sec=ret
+               |# Some comment
                |""")
 
         exists_mock = MagicMock(return_value=True)
@@ -400,7 +400,7 @@ class TestLoadBintrayCredentials(TestCase):
                 patch('builtins.open', open_mock):
             username, password = bintray_resolver.load_bintray_credentials()
             self.assertEqual('user1', username)
-            self.assertEqual('secret', password)
+            self.assertEqual('sec=ret', password)
 
         exists_mock.assert_called_with('{}/.bintray/.credentials'.format(os.path.expanduser('~')))
         open_mock.assert_called_with('{}/.bintray/.credentials'.format(os.path.expanduser('~')), 'r')