Merge pull request #73 from drebs/use-netrc-file

Hide creds when saving and use netrc file

AUTHORS.rst

@@ -11,3 +11,4 @@ Authors
 * Petr Šebek - https://github.com/Artimi
 * Swen Kooij - https://github.com/Photonios
 * Vara Canero - https://github.com/varac
+* Andre Bianchi - https://github.com/drebs

CHANGELOG.rst

@@ -7,6 +7,10 @@ Changelog
 
 * Added a ``time`` field in ``commit_info``. Contributed by Vara Canero in
   `#71 <https://github.com/ionelmc/pytest-benchmark/pull/71>`_.
+* Fixed the leaking of credentials by masking the URL printed when storing
+  data to elasticsearch.
+* Added a `--benchmark-netrc` option to use credentials from a netrc file when
+  storing data to elasticsearch.
 
 3.1.0a2 (2017-03-27)
 --------------------

src/pytest_benchmark/cli.py

@@ -147,7 +147,7 @@ def main():
     parser = make_parser()
     args = parser.parse_args()
     logger = Logger(args.verbose)
-    storage = load_storage(args.storage, logger=logger)
+    storage = load_storage(args.storage, logger=logger, netrc=args.netrc)
 
     if args.command == 'list':
         for file in storage.query():

src/pytest_benchmark/plugin.py

@@ -109,6 +109,11 @@ def add_global_options(addoption, prefix="benchmark-"):
              "(when --benchmark-save or --benchmark-autosave are used). For backwards compatibility unexpected values "
              "are converted to file://<value>. Default: %(default)r."
     )
+    addoption(
+        "--{0}netrc".format(prefix),
+        nargs="?", default='', const='~/.netrc',
+        help="Load elasticsearch credentials from a netrc file. Default: %(default)r.",
+    )
     addoption(
         "--{0}verbose".format(prefix), *[] if prefix else ['-v'],
         action="store_true", default=False,

src/pytest_benchmark/session.py

@@ -36,7 +36,8 @@ def __init__(self, config):
         self.storage = load_storage(
             config.getoption("benchmark_storage"),
             logger=self.logger,
-            default_machine_id=self.machine_id
+            default_machine_id=self.machine_id,
+            netrc=config.getoption("benchmark_netrc")
         )
         self.options = dict(
             min_time=SecondsDecimal(config.getoption("benchmark_min_time")),

src/pytest_benchmark/storage/elasticsearch.py

@@ -1,10 +1,12 @@
 from __future__ import absolute_import
 
+import re
 import sys
 import uuid
 from datetime import date
 from datetime import datetime
 from decimal import Decimal
+from functools import partial
 
 from ..compat import reraise
 
@@ -28,6 +30,13 @@ def default(self, data):
             return "UNSERIALIZABLE[%r]" % data
 
 
+def _mask_hosts(hosts):
+    m = re.compile('^([^:]+)://[^@]+@')
+    sub_fun = partial(m.sub, '\\1://***:***@')
+    masked_hosts = list(map(sub_fun, hosts))
+    return masked_hosts
+
+
 class ElasticsearchStorage(object):
     def __init__(self, hosts, index, doctype, project_name, logger,
                  default_machine_id=None):
@@ -162,8 +171,10 @@ def save(self, output_json, save):
                 body=bench,
                 id=doc_id,
             )
+        # hide user's credentials before logging
+        masked_hosts = _mask_hosts(self._es_hosts)
         self.logger.info("Saved benchmark data to %s to index %s as doctype %s" % (
-            self._es_hosts, self._es_index, self._es_doctype))
+            masked_hosts, self._es_index, self._es_doctype))
 
     def _create_index(self):
         mapping = {

src/pytest_benchmark/utils.py

@@ -4,6 +4,7 @@
 import argparse
 import genericpath
 import json
+import netrc
 import ntpath
 import os
 import platform
@@ -404,10 +405,34 @@ def parse_save(string):
     return string
 
 
-def parse_elasticsearch_storage(string, default_index="benchmark", default_doctype="benchmark"):
+def _parse_hosts(storage_url, netrc_file):
+
+    # load creds from netrc file
+    path = os.path.expanduser(netrc_file)
+    creds = None
+    if netrc_file and os.path.isfile(path):
+        creds = netrc.netrc(path)
+
+    # add creds to urls
+    urls = []
+    for netloc in storage_url.netloc.split(','):
+        auth = ""
+        if creds and '@' not in netloc:
+            host = netloc.split(':').pop(0)
+            res = creds.authenticators(host)
+            if res:
+                user, _, secret = res
+                auth = "{user}:{secret}@".format(user=user, secret=secret)
+        url = "{scheme}://{auth}{netloc}".format(scheme=storage_url.scheme,
+                                                 netloc=netloc, auth=auth)
+        urls.append(url)
+    return urls
+
+
+def parse_elasticsearch_storage(string, default_index="benchmark",
+                                default_doctype="benchmark", netrc_file=''):
     storage_url = urlparse(string)
-    hosts = ["{scheme}://{netloc}".format(scheme=storage_url.scheme, netloc=netloc) for netloc in
-             storage_url.netloc.split(',')]
+    hosts = _parse_hosts(storage_url, netrc_file)
     index = default_index
     doctype = default_doctype
     if storage_url.path and storage_url.path != "/":
@@ -426,13 +451,16 @@ def parse_elasticsearch_storage(string, default_index="benchmark", default_docty
 def load_storage(storage, **kwargs):
     if "://" not in storage:
         storage = "file://" + storage
+    netrc_file = kwargs.pop('netrc')  # only used by elasticsearch storage
     if storage.startswith("file://"):
         from .storage.file import FileStorage
         return FileStorage(storage[len("file://"):], **kwargs)
     elif storage.startswith("elasticsearch+"):
         from .storage.elasticsearch import ElasticsearchStorage
         # TODO update benchmark_autosave
-        return ElasticsearchStorage(*parse_elasticsearch_storage(storage[len("elasticsearch+"):]), **kwargs)
+        args = parse_elasticsearch_storage(storage[len("elasticsearch+"):],
+                                           netrc_file=netrc_file)
+        return ElasticsearchStorage(*args, **kwargs)
     else:
         raise argparse.ArgumentTypeError("Storage must be in form of file://path or "
                                          "elasticsearch+http[s]://host1,host2/index/doctype")

tests/test_elasticsearch_storage.py

@@ -2,6 +2,7 @@
 
 import json
 import logging
+import os
 from io import BytesIO
 from io import StringIO
 
@@ -16,6 +17,8 @@
 from pytest_benchmark.plugin import pytest_benchmark_generate_json
 from pytest_benchmark.plugin import pytest_benchmark_group_stats
 from pytest_benchmark.storage.elasticsearch import ElasticsearchStorage
+from pytest_benchmark.storage.elasticsearch import _mask_hosts
+from pytest_benchmark.utils import parse_elasticsearch_storage
 
 try:
     import unittest.mock as mock
@@ -173,3 +176,59 @@ def test_handle_saving(sess, logger_output, monkeypatch):
         body=ES_DATA,
         id='FoobarOS_commitId_tests/test_normal.py::test_xfast_parametrized[0]',
     )
+
+
+def test_parse_with_no_creds():
+    string = 'https://example.org,another.org'
+    hosts, _, _, _ = parse_elasticsearch_storage(string)
+    assert len(hosts) == 2
+    assert 'https://example.org' in hosts
+    assert 'https://another.org' in hosts
+
+
+def test_parse_with_creds_in_first_host_of_url():
+    string = 'https://user:pass@example.org,another.org'
+    hosts, _, _, _ = parse_elasticsearch_storage(string)
+    assert len(hosts) == 2
+    assert 'https://user:pass@example.org' in hosts
+    assert 'https://another.org' in hosts
+
+
+def test_parse_with_creds_in_second_host_of_url():
+    string = 'https://example.org,user:pass@another.org'
+    hosts, _, _, _ = parse_elasticsearch_storage(string)
+    assert len(hosts) == 2
+    assert 'https://example.org' in hosts
+    assert 'https://user:pass@another.org' in hosts
+
+
+def test_parse_with_creds_in_netrc(tmpdir):
+    netrc_file = os.path.join(tmpdir.strpath, 'netrc')
+    with open(netrc_file, 'w') as f:
+        f.write('machine example.org login user1 password pass1\n')
+        f.write('machine another.org login user2 password pass2\n')
+    string = 'https://example.org,another.org'
+    hosts, _, _, _ = parse_elasticsearch_storage(string, netrc_file=netrc_file)
+    assert len(hosts) == 2
+    assert 'https://user1:pass1@example.org' in hosts
+    assert 'https://user2:pass2@another.org' in hosts
+
+
+def test_parse_url_creds_supersedes_netrc_creds(tmpdir):
+    netrc_file = os.path.join(tmpdir.strpath, 'netrc')
+    with open(netrc_file, 'w') as f:
+        f.write('machine example.org login user1 password pass1\n')
+        f.write('machine another.org login user2 password pass2\n')
+    string = 'https://user3:pass3@example.org,another.org'
+    hosts, _, _, _ = parse_elasticsearch_storage(string, netrc_file=netrc_file)
+    assert len(hosts) == 2
+    assert 'https://user3:pass3@example.org' in hosts  # superseded by creds in url
+    assert 'https://user2:pass2@another.org' in hosts  # got creds from netrc file
+
+
+def test__mask_hosts():
+    hosts = ['https://user1:pass1@example.org', 'https://user2:pass2@another.org']
+    masked_hosts = _mask_hosts(hosts)
+    assert len(masked_hosts) == len(hosts)
+    assert 'https://***:***@example.org' in masked_hosts
+    assert 'https://***:***@another.org' in masked_hosts