Skip to content

validate plotly_(api)_domain - HTTPS, not HTTP #997

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

Merged
merged 11 commits into from
May 4, 2018
2 changes: 1 addition & 1 deletion plotly/tests/test_core/test_stream/test_stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
tk = 'vaia8trjjb'
config = {'plotly_domain': 'https://plot.ly',
'plotly_streaming_domain': 'stream.plot.ly',
'plotly_api_domain': 'https://api.plot.ly',
'plotly_api_domain': 'https://api.plot.ly',
'plotly_ssl_verification': False}


Expand Down
27 changes: 26 additions & 1 deletion plotly/tests/test_core/test_tools/test_file_tools.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from plotly import tools, session
from plotly.tests.utils import PlotlyTestCase

import warnings


class FileToolsTest(PlotlyTestCase):

Expand Down Expand Up @@ -43,14 +45,37 @@ def test_set_config_file_two_entries(self):
self.assertEqual(config['plotly_streaming_domain'], streaming_domain)
tools.reset_config_file()


def test_set_config_file_world_readable(self):

# Return TypeError when world_readable type is not a bool

kwargs = {'world_readable': 'True'}
self.assertRaises(TypeError, tools.set_config_file, **kwargs)

def test_set_config_expected_warning_msg(self):

# Check that UserWarning is being called with http plotly_domain

with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
kwargs = {'plotly_domain': 'http://www.foo-bar.com'}
tools.set_config_file(**kwargs)
assert len(w) == 1
assert issubclass(w[-1].category, UserWarning)
assert "plotly_domain" in str(w[-1].message)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Could you also include a test to check that no warning is issued when https is used?


def test_set_config_no_warning_msg_if_plotly_domain_is_https(self):

# Check that no UserWarning is being called with https plotly_domain

with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
kwargs = {'plotly_domain': 'https://www.foo-bar.com'}
tools.set_config_file(**kwargs)
assert len(w) == 0


def test_reset_config_file(self):

# Check reset_config and get_config return the same values
Expand Down
6 changes: 6 additions & 0 deletions plotly/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ def set_config_file(plotly_domain=None,
ensure_local_plotly_files() # make sure what's there is OK
utils.validate_world_readable_and_sharing_settings({
'sharing': sharing, 'world_readable': world_readable})

settings = get_config_file()
if isinstance(plotly_domain, six.string_types):
settings['plotly_domain'] = plotly_domain
Expand All @@ -218,6 +219,11 @@ def set_config_file(plotly_domain=None,
elif auto_open is not None:
raise TypeError('auto_open should be a boolean')

# validate plotly_domain and plotly_api_domain
utils.validate_plotly_domains(
{'plotly_domain': plotly_domain, 'plotly_api_domain': plotly_api_domain}
)

if isinstance(world_readable, bool):
settings['world_readable'] = world_readable
settings.pop('sharing')
Expand Down
38 changes: 37 additions & 1 deletion plotly/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
"""
from __future__ import absolute_import

import decimal
import os.path
import re
import sys
import threading
import decimal
import warnings
from collections import deque

import pytz
Expand All @@ -32,6 +33,30 @@
lock = threading.Lock()


http_msg = (
"The plotly_domain and plotly_api_domain of your config file must start "
"with 'https', not 'http'. If you are not using On-Premise then run the "
"following code to ensure your plotly_domain and plotly_api_domain start "
"with 'https':\n\n\n"
"import plotly\n"
"plotly.tools.set_config_file(\n"
" plotly_domain='https://plot.ly',\n"
" plotly_api_domain='https://api.plot.ly'\n"
")\n\n\n"
"If you are using On-Premise then you will need to use your company's "
"domain and api_domain urls:\n\n\n"
"import plotly\n"
"plotly.tools.set_config_file(\n"
" plotly_domain='https://plotly.your-company.com',\n"
" plotly_api_domain='https://plotly.your-company.com'\n"
")\n\n\n"
"Make sure to replace `your-company.com` with the URL of your Plotly "
"On-Premise server.\nSee "
"https://plot.ly/python/getting-started/#special-instructions-for-plotly-onpremise-users "
"for more help with getting started with On-Premise."
)


### general file setup tools ###

def load_json_dict(filename, *args):
Expand Down Expand Up @@ -297,6 +322,7 @@ def encode_as_decimal(obj):
else:
raise NotEncodable


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it keeps the file consistent with 2 spaces between functions

### unicode stuff ###
def decode_unicode(coll):
if isinstance(coll, list):
Expand Down Expand Up @@ -436,6 +462,16 @@ def validate_world_readable_and_sharing_settings(option_set):
)


def validate_plotly_domains(option_set):
domains_not_none = []
for d in ['plotly_domain', 'plotly_api_domain']:
if d in option_set and option_set[d]:
domains_not_none.append(option_set[d])

if not all(d.lower().startswith('https') for d in domains_not_none):
warnings.warn(http_msg, category=UserWarning)


def set_sharing_and_world_readable(option_set):
if 'world_readable' in option_set and 'sharing' not in option_set:
option_set['sharing'] = (
Expand Down