Skip to content

Validate signin #668

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 3 commits into from
Jan 18, 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ failures. Previously either a `PlotlyError`, `PlotlyRequestException`, or a
`requests.exceptions.ReqestException` could be raised. In particular, scripts
which depend on `try-except` blocks containing network requests should be
revisited.
- `plotly.py:sign_in` now validates to the plotly server specified in your
config. If it cannot make a successful request, it raises a `PlotlyError`.

### Deprecated
- `plotly.tools.FigureFactory`. Use `plotly.figure_factory.*`.
Expand Down
3 changes: 2 additions & 1 deletion plotly/api/v2/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from __future__ import absolute_import

from plotly.api.v2 import files, folders, grids, images, plot_schema, plots
from plotly.api.v2 import (files, folders, grids, images, plot_schema, plots,
users)
17 changes: 17 additions & 0 deletions plotly/api/v2/users.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
"""Interface to Plotly's /v2/files endpoints."""
from __future__ import absolute_import

from plotly.api.v2.utils import build_url, request

RESOURCE = 'users'


def current():
"""
Retrieve information on the logged-in user from Plotly.

:returns: (requests.Response) Returns response directly from requests.

"""
url = build_url(RESOURCE, route='current')
return request('get', url)
18 changes: 12 additions & 6 deletions plotly/plotly/plotly.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,9 @@
import six.moves
from requests.compat import json as _json

from plotly import exceptions, tools, utils, files
from plotly import exceptions, files, session, tools, utils
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just curious, why did you switch up the order like this? Is it because tools is moving into a different location for 2.0.0. and therefore its priority is being hierarchically diminished? (I don't know the proper jargon for what I'm describing...)

Copy link
Member

Choose a reason for hiding this comment

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

looks like it's just alphabetical 😎

from plotly.api import v1, v2
from plotly.plotly import chunked_requests
from plotly.session import (sign_in, update_session_plot_options,
get_session_plot_options)
from plotly.grid_objs import Grid, Column

# This is imported like this for backwards compat. Careful if changing.
Expand All @@ -50,8 +48,16 @@


# don't break backwards compatibility
sign_in = sign_in
update_plot_options = update_session_plot_options
def sign_in(username, api_key, **kwargs):
session.sign_in(username, api_key, **kwargs)
try:
# The only way this can succeed is if the user can be authenticated
# with the given, username, api_key, and plotly_api_domain.
v2.users.current()
except exceptions.PlotlyRequestError:
raise exceptions.PlotlyError('Sign in failed.')

update_plot_options = session.update_session_plot_options


def _plot_option_logic(plot_options_from_call_signature):
Expand All @@ -66,7 +72,7 @@ def _plot_option_logic(plot_options_from_call_signature):
"""
default_plot_options = copy.deepcopy(DEFAULT_PLOT_OPTIONS)
file_options = tools.get_config_file()
session_options = get_session_plot_options()
session_options = session.get_session_plot_options()
plot_options_from_call_signature = copy.deepcopy(plot_options_from_call_signature)

# Validate options and fill in defaults w world_readable and sharing
Expand Down
28 changes: 28 additions & 0 deletions plotly/tests/test_core/test_api/test_v2/test_users.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
from __future__ import absolute_import

from plotly.api.v2 import users
from plotly.tests.test_core.test_api import PlotlyApiTestCase


class UsersTest(PlotlyApiTestCase):

def setUp(self):
super(UsersTest, self).setUp()

# Mock the actual api call, we don't want to do network tests here.
self.request_mock = self.mock('plotly.api.v2.utils.requests.request')
self.request_mock.return_value = self.get_response()

# Mock the validation function since we can test that elsewhere.
self.mock('plotly.api.v2.utils.validate_response')

def test_current(self):
users.current()
self.request_mock.assert_called_once()
args, kwargs = self.request_mock.call_args
method, url = args
self.assertEqual(method, 'get')
self.assertEqual(
url, '{}/v2/users/current'.format(self.plotly_api_domain)
)
self.assertNotIn('params', kwargs)
61 changes: 36 additions & 25 deletions plotly/tests/test_core/test_plotly/test_credentials.py
Original file line number Diff line number Diff line change
@@ -1,39 +1,43 @@
from __future__ import absolute_import

from unittest import TestCase
from mock import patch

import plotly.plotly.plotly as py
import plotly.session as session
import plotly.tools as tls
from plotly import exceptions
from plotly.tests.utils import PlotlyTestCase


def test_get_credentials():
session_credentials = session.get_session_credentials()
if 'username' in session_credentials:
del session._session['credentials']['username']
if 'api_key' in session_credentials:
del session._session['credentials']['api_key']
creds = py.get_credentials()
file_creds = tls.get_credentials_file()
print(creds)
print(file_creds)
assert creds == file_creds
class TestSignIn(PlotlyTestCase):

def setUp(self):
super(TestSignIn, self).setUp()
patcher = patch('plotly.api.v2.users.current')
self.users_current_mock = patcher.start()
self.addCleanup(patcher.stop)

def test_sign_in():
un = 'anyone'
ak = 'something'
# TODO, add this!
# si = ['this', 'and-this']
py.sign_in(un, ak)
creds = py.get_credentials()
assert creds['username'] == un
assert creds['api_key'] == ak
# TODO, and check it!
# assert creds['stream_ids'] == si
def test_get_credentials(self):
session_credentials = session.get_session_credentials()
if 'username' in session_credentials:
del session._session['credentials']['username']
if 'api_key' in session_credentials:
del session._session['credentials']['api_key']
creds = py.get_credentials()
file_creds = tls.get_credentials_file()
self.assertEqual(creds, file_creds)


class TestSignIn(TestCase):
def test_sign_in(self):
un = 'anyone'
ak = 'something'
# TODO, add this!
# si = ['this', 'and-this']
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this TODO mean exactly? I'm guessing si means something here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's just old code. I just needed to add it to the TestCase class. I think it's for stream ids probably? I'm not going to futz with it in this PR though.

py.sign_in(un, ak)
creds = py.get_credentials()
self.assertEqual(creds['username'], un)
self.assertEqual(creds['api_key'], ak)
# TODO, and check it!
# assert creds['stream_ids'] == si

def test_get_config(self):
plotly_domain = 'test domain'
Expand Down Expand Up @@ -74,3 +78,10 @@ def test_sign_in_with_config(self):
self.assertEqual(
config['plotly_ssl_verification'], plotly_ssl_verification
)

def test_sign_in_cannot_validate(self):
self.users_current_mock.side_effect = exceptions.PlotlyRequestError(
'msg', 400, 'foobar'
)
with self.assertRaisesRegexp(exceptions.PlotlyError, 'Sign in failed'):
py.sign_in('foo', 'bar')
9 changes: 9 additions & 0 deletions plotly/tests/test_core/test_plotly/test_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from requests.compat import json as _json

from unittest import TestCase
from mock import patch
from nose.plugins.attrib import attr
from nose.tools import raises

Expand Down Expand Up @@ -223,6 +224,14 @@ class TestPlotOptionLogic(PlotlyTestCase):
{'world_readable': False, 'sharing': 'public'}
)

def setUp(self):
super(TestPlotOptionLogic, self).setUp()

# Make sure we don't hit sign-in validation failures.
patcher = patch('plotly.api.v2.users.current')
self.users_current_mock = patcher.start()
self.addCleanup(patcher.stop)

def test_default_options(self):
options = py._plot_option_logic({})
config_options = tls.get_config_file()
Expand Down