Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide creds when saving and use netrc file #73

Merged
merged 5 commits into from
Apr 4, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
--------------------
Expand Down
2 changes: 1 addition & 1 deletion src/pytest_benchmark/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
5 changes: 5 additions & 0 deletions src/pytest_benchmark/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion src/pytest_benchmark/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
Expand Down
13 changes: 12 additions & 1 deletion src/pytest_benchmark/storage/elasticsearch.py
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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):
Expand Down Expand Up @@ -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 = {
Expand Down
36 changes: 32 additions & 4 deletions src/pytest_benchmark/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import argparse
import genericpath
import json
import netrc
import ntpath
import os
import platform
Expand Down Expand Up @@ -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 != "/":
Expand All @@ -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")
Expand Down
59 changes: 59 additions & 0 deletions tests/test_elasticsearch_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import json
import logging
import os
from io import BytesIO
from io import StringIO

Expand All @@ -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
Expand Down Expand Up @@ -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:[email protected],another.org'
hosts, _, _, _ = parse_elasticsearch_storage(string)
assert len(hosts) == 2
assert 'https://user:[email protected]' in hosts
assert 'https://another.org' in hosts


def test_parse_with_creds_in_second_host_of_url():
string = 'https://example.org,user:[email protected]'
hosts, _, _, _ = parse_elasticsearch_storage(string)
assert len(hosts) == 2
assert 'https://example.org' in hosts
assert 'https://user:[email protected]' 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:[email protected]' in hosts
assert 'https://user2:[email protected]' 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:[email protected],another.org'
hosts, _, _, _ = parse_elasticsearch_storage(string, netrc_file=netrc_file)
assert len(hosts) == 2
assert 'https://user3:[email protected]' in hosts # superseded by creds in url
assert 'https://user2:[email protected]' in hosts # got creds from netrc file


def test__mask_hosts():
hosts = ['https://user1:[email protected]', 'https://user2:[email protected]']
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