Skip to content

Commit

Permalink
Add functionality for auto-revoking auth tokens (#224)
Browse files Browse the repository at this point in the history
Auth objects can now be closed, which prevents them from being
used to request new tokens. This will also revoke any existing
tokens. Also introduces a `closing()` context manager method,
which will auto-close the auth object on exit. Clients can use
this to make sure that their tokens don't live longer than they
need them for.
  • Loading branch information
jmoldow committed Aug 23, 2017
1 parent 23a3cd2 commit a764630
Show file tree
Hide file tree
Showing 6 changed files with 203 additions and 3 deletions.
5 changes: 5 additions & 0 deletions HISTORY.rst
Expand Up @@ -53,6 +53,11 @@ Release History
- When the ``auto_session_renewal`` is ``True`` when calling any of the request
methods on ``BoxSession``, if there is no access token, ``BoxSession`` will
renew the token _before_ making the request. This saves an API call.
- Auth objects can now be closed, which prevents them from being used to
request new tokens. This will also revoke any existing tokens (though that
feature can be disabled by passing ``revoke=False``). Also introduces a
``closing()`` context manager method, which will auto-close the auth object
on exit.
- Various enhancements to the ``JWTAuth`` baseclass:

- The ``authenticate_app_user()`` method is renamed to
Expand Down
91 changes: 91 additions & 0 deletions boxsdk/auth/oauth2.py
Expand Up @@ -2,10 +2,13 @@

from __future__ import unicode_literals

from contextlib import contextmanager
from threading import Lock
import random
import string # pylint:disable=deprecated-module
import sys

import six
from six.moves.urllib.parse import urlencode, urlunsplit # pylint:disable=import-error,no-name-in-module

from boxsdk.network.default_network import DefaultNetwork
Expand All @@ -16,6 +19,11 @@
class OAuth2(object):
"""
Responsible for handling OAuth2 for the Box API. Can authenticate and refresh tokens.
Can be used as a closeable resource, similar to a file. When `close()` is
called, the current tokens are revoked, and the object is put into a state
where it can no longer request new tokens. This action can also be managed
with the `closing()` context manager method.
"""

def __init__(
Expand Down Expand Up @@ -77,6 +85,7 @@ def __init__(
self._refresh_lock = refresh_lock or Lock()
self._box_device_id = box_device_id
self._box_device_name = box_device_name
self._closed = False

@property
def access_token(self):
Expand All @@ -90,6 +99,16 @@ def access_token(self):
"""
return self._access_token

@property
def closed(self):
"""True iff the auth object has been closed.
When in the closed state, it can no longer request new tokens.
:rtype: `bool`
"""
return self._closed

def get_authorization_url(self, redirect_url):
"""
Get the authorization url based on the client id and the redirect url passed in
Expand Down Expand Up @@ -198,7 +217,9 @@ def refresh(self, access_token_to_refresh):
:rtype:
`tuple` of (`unicode`, (`unicode` or `None`))
"""
self._check_closed()
with self._refresh_lock:
self._check_closed()
access_token, refresh_token = self._get_and_update_current_tokens()
# The lock here is for handling that case that multiple requests fail, due to access token expired, at the
# same time to avoid multiple session renewals.
Expand Down Expand Up @@ -275,6 +296,7 @@ def send_token_request(self, data, access_token, expect_refresh_token=True):
:rtype:
(`unicode`, `unicode`)
"""
self._check_closed()
url = '{base_auth_url}/token'.format(base_auth_url=API.OAUTH2_API_URL)
headers = {'content-type': 'application/x-www-form-urlencoded'}
network_response = self._network_layer.request(
Expand Down Expand Up @@ -320,3 +342,72 @@ def revoke(self):
if not network_response.ok:
raise BoxOAuthException(network_response.status_code, network_response.content, url, 'POST')
self._store_tokens(None, None)

def close(self, revoke=True):
"""Close the auth object.
After this action is performed, the auth object can no longer request
new tokens.
This method may be called even if the auth object is already closed.
:param revoke:
(optional) Whether the current tokens should be revoked, via `revoke()`.
Defaults to `True` as a security precaution, so that the tokens aren't usable
by any adversaries after you are done with them.
Note that the revoke isn't guaranteed to succeed (the network connection might
fail, or the API call might respond with a non-200 HTTP response), so this
isn't a fool-proof security mechanism.
If the revoke fails, an exception is raised.
The auth object is still considered to be closed, even if the revoke fails.
:type revoke: `bool`
"""
self._closed = True
if revoke:
self.revoke()

@contextmanager
def closing(self, **close_kwargs):
"""Context manager to close the auth object on exit.
The behavior is somewhat similar to `contextlib.closing(self)`, but has
some differences.
The context manager cannot be entered if the auth object is closed.
If a non-`Exception` (e.g. `KeyboardInterrupt`) is caught from the
block, this context manager prioritizes re-raising the exception as
fast as possible, without blocking. Thus, in this case, the tokens will
not be revoked, even if `revoke=True` was passed to this method.
If exceptions are raised both from the block and from `close()`, the
exception from the block will be reraised, and the exception from
`close()` will be swallowed. The assumption is that the exception from
the block is more relevant to the client, especially since the revoke
can fail if the network is unavailable.
:param **close_kwargs: Keyword arguments to pass to `close()`.
"""
self._check_closed()
exc_infos = []

# pylint:disable=broad-except
try:
yield self
except Exception:
exc_infos.append(sys.exc_info())
except BaseException:
exc_infos.append(sys.exc_info())
close_kwargs['revoke'] = False

try:
self.close(**close_kwargs)
except Exception:
exc_infos.append(sys.exc_info())

if exc_infos:
six.reraise(*exc_infos[0])

def _check_closed(self):
if self.closed:
raise ValueError("operation on a closed auth object")
2 changes: 1 addition & 1 deletion boxsdk/version.py
Expand Up @@ -3,4 +3,4 @@
from __future__ import unicode_literals, absolute_import


__version__ = '2.0.0a6'
__version__ = '2.0.0a7'
5 changes: 5 additions & 0 deletions pytest.ini
@@ -0,0 +1,5 @@
[pytest]
addopts = --strict --showlocals -r a --tb=long
xfail_strict = True
junit_suite_name = boxsdk
testpaths = test/
101 changes: 100 additions & 1 deletion test/unit/auth/test_oauth2.py
Expand Up @@ -7,7 +7,7 @@
from threading import Thread
import uuid

from mock import Mock
from mock import Mock, patch
import pytest
from six.moves import range # pylint:disable=redefined-builtin
from six.moves.urllib import parse as urlparse # pylint:disable=import-error,no-name-in-module,wrong-import-order
Expand All @@ -18,6 +18,14 @@
from boxsdk.config import API


class MyError(Exception):
pass


class MyBaseException(BaseException):
pass


@pytest.fixture(params=('https://url.com/foo?bar=baz', 'https://ȕŕľ.com/ƒőő?Ƅȁŕ=Ƅȁż', None))
def redirect_url(request):
"""A value for the `redirect_uri` query string parameter for OAuth2."""
Expand Down Expand Up @@ -354,3 +362,94 @@ def _get_tokens(self):

assert oauth.refresh(access_token) == new_tokens
assert oauth.access_token == new_access_token


def test_closed_is_false_after_init(client_id, client_secret, mock_network_layer):
auth = OAuth2(client_id=client_id, client_secret=client_secret, network_layer=mock_network_layer)
assert auth.closed is False


def test_closed_is_true_after_close(client_id, client_secret, mock_network_layer):
auth = OAuth2(client_id=client_id, client_secret=client_secret, network_layer=mock_network_layer)
auth.close()
assert auth.closed is True


def test_token_requests_fail_after_close(client_id, client_secret, mock_network_layer):
auth = OAuth2(client_id=client_id, client_secret=client_secret, network_layer=mock_network_layer)
auth.close()
with pytest.raises(ValueError):
auth.refresh(auth.access_token)


@pytest.mark.parametrize('raise_exception', [False, True])
def test_context_manager_closes_auth_object(client_id, client_secret, mock_network_layer, raise_exception):
auth = OAuth2(client_id=client_id, client_secret=client_secret, network_layer=mock_network_layer)
try:
with auth.closing():
if raise_exception:
raise MyError
except MyError:
pass
assert auth.closed is True


def test_context_manager_fails_after_close(client_id, client_secret, mock_network_layer):
auth = OAuth2(client_id=client_id, client_secret=client_secret, network_layer=mock_network_layer)
with auth.closing():
pass
with pytest.raises(ValueError):
with auth.closing():
assert False


@pytest.mark.parametrize(('close_args', 'close_kwargs'), [((), {}), ((True,), {}), ((), dict(revoke=True))])
def test_revoke_on_close(client_id, client_secret, access_token, mock_network_layer, close_args, close_kwargs):
auth = OAuth2(client_id=client_id, client_secret=client_secret, access_token=access_token, network_layer=mock_network_layer)
with patch.object(auth, 'revoke') as mock_revoke:
auth.close(*close_args, **close_kwargs)
mock_revoke.assert_called_once_with()


def test_auth_object_is_closed_even_if_revoke_fails(client_id, client_secret, access_token, mock_network_layer):
auth = OAuth2(client_id=client_id, client_secret=client_secret, access_token=access_token, network_layer=mock_network_layer)
with patch.object(auth, 'revoke', side_effect=BoxOAuthException(status=500)):
with pytest.raises(BoxOAuthException):
auth.close(revoke=True)
assert auth.closed is True


@pytest.mark.parametrize(('close_args', 'close_kwargs'), [((False,), {}), ((), dict(revoke=False))])
def test_revoke_on_close_can_be_skipped(client_id, client_secret, access_token, mock_network_layer, close_args, close_kwargs):
auth = OAuth2(client_id=client_id, client_secret=client_secret, access_token=access_token, network_layer=mock_network_layer)
with patch.object(auth, 'revoke') as mock_revoke:
auth.close(*close_args, **close_kwargs)
mock_revoke.assert_not_called()


@pytest.mark.parametrize(('raise_from_block', 'raise_from_close', 'expected_exception'), [
(MyError, None, MyError),
(None, BoxOAuthException(status=500), BoxOAuthException),
(MyError, BoxOAuthException(status=500), MyError),
])
@pytest.mark.parametrize('close_kwargs', [{}, dict(revoke=False), dict(revoke=True)])
def test_context_manager_reraises_first_exception_after_close(
client_id, client_secret, mock_network_layer, close_kwargs, raise_from_block, raise_from_close, expected_exception,
):
auth = OAuth2(client_id=client_id, client_secret=client_secret, network_layer=mock_network_layer)
with patch.object(auth, 'close', side_effect=raise_from_close) as mock_close:
with pytest.raises(expected_exception):
with auth.closing(**close_kwargs):
if raise_from_block:
raise raise_from_block
mock_close.assert_called_once_with(**close_kwargs)


@pytest.mark.parametrize('close_kwargs', [{}, dict(revoke=False), dict(revoke=True)])
def test_context_manager_skips_revoke_on_base_exception(client_id, client_secret, mock_network_layer, close_kwargs):
auth = OAuth2(client_id=client_id, client_secret=client_secret, network_layer=mock_network_layer)
with patch.object(auth, 'close') as mock_close:
with pytest.raises(MyBaseException):
with auth.closing(**close_kwargs):
raise MyBaseException
mock_close.assert_called_once_with(revoke=False)
2 changes: 1 addition & 1 deletion tox.ini
Expand Up @@ -20,7 +20,7 @@ envlist =

[testenv]
commands =
py.test test/ {posargs}
pytest {posargs}
deps = -rrequirements-dev.txt

[testenv:rst]
Expand Down

0 comments on commit a764630

Please sign in to comment.