From 1d4fd271259967093870a22515409bf3622e98fd Mon Sep 17 00:00:00 2001 From: Jon Wayne Parrott Date: Tue, 24 Nov 2015 13:40:26 -0800 Subject: [PATCH] Adding dictionary storage for #319. * DictionaryStorage - implements an optionally-locked storage over a dictionary-like object. * Moved optional locking logic into `Storage`, previously this has been duplicated in subclasses. * Removed `flask_util.FlaskSessionStorage` and replaced it with `DictionaryStorage`. --- ...auth2client.contrib.dictionary_storage.rst | 7 ++ docs/source/oauth2client.contrib.rst | 1 + oauth2client/client.py | 16 ++++- oauth2client/contrib/appengine.py | 2 + oauth2client/contrib/dictionary_storage.py | 58 +++++++++++++++++ oauth2client/contrib/django_orm.py | 1 + oauth2client/contrib/django_util/storage.py | 1 + oauth2client/contrib/flask_util.py | 33 +--------- oauth2client/contrib/keyring_storage.py | 17 +---- oauth2client/file.py | 17 +---- tests/contrib/test_dictionary_storage.py | 65 +++++++++++++++++++ 11 files changed, 152 insertions(+), 66 deletions(-) create mode 100644 docs/source/oauth2client.contrib.dictionary_storage.rst create mode 100644 oauth2client/contrib/dictionary_storage.py create mode 100644 tests/contrib/test_dictionary_storage.py diff --git a/docs/source/oauth2client.contrib.dictionary_storage.rst b/docs/source/oauth2client.contrib.dictionary_storage.rst new file mode 100644 index 000000000..1b59a2c48 --- /dev/null +++ b/docs/source/oauth2client.contrib.dictionary_storage.rst @@ -0,0 +1,7 @@ +oauth2client.contrib.dictionary_storage module +============================================== + +.. automodule:: oauth2client.contrib.dictionary_storage + :members: + :undoc-members: + :show-inheritance: diff --git a/docs/source/oauth2client.contrib.rst b/docs/source/oauth2client.contrib.rst index f90c16f43..252afd9fe 100644 --- a/docs/source/oauth2client.contrib.rst +++ b/docs/source/oauth2client.contrib.rst @@ -14,6 +14,7 @@ Submodules .. toctree:: oauth2client.contrib.appengine + oauth2client.contrib.dictionary_storage oauth2client.contrib.django_orm oauth2client.contrib.flask_util oauth2client.contrib.keyring_storage diff --git a/oauth2client/client.py b/oauth2client/client.py index 20a59c482..d0b70d260 100644 --- a/oauth2client/client.py +++ b/oauth2client/client.py @@ -342,21 +342,31 @@ class Storage(object): such that multiple processes and threads can operate on a single store. """ + def __init__(self, lock=None): + """Create a Storage instance. + + Args: + lock: An optional threading.Lock-like object. Must implement at + least acquire() and release(). Does not need to be re-entrant. + """ + self._lock = lock def acquire_lock(self): """Acquires any lock necessary to access this Storage. This lock is not reentrant. """ - pass + if self._lock is not None: + self._lock.acquire() def release_lock(self): """Release the Storage lock. Trying to release a lock that isn't held will result in a - RuntimeError. + RuntimeError in the case of a threading.Lock or multiprocessing.Lock. """ - pass + if self._lock is not None: + self._lock.release() def locked_get(self): """Retrieve credential. diff --git a/oauth2client/contrib/appengine.py b/oauth2client/contrib/appengine.py index fcbef3e1b..a7e5dedc6 100644 --- a/oauth2client/contrib/appengine.py +++ b/oauth2client/contrib/appengine.py @@ -408,6 +408,8 @@ def __init__(self, model, key_name, property_name, cache=None, user=None): user: users.User object, optional. Can be used to grab user ID as a key_name if no key name is specified. """ + super(StorageByKeyName, self).__init__() + if key_name is None: if user is None: raise ValueError('StorageByKeyName called with no ' diff --git a/oauth2client/contrib/dictionary_storage.py b/oauth2client/contrib/dictionary_storage.py new file mode 100644 index 000000000..22c48eddf --- /dev/null +++ b/oauth2client/contrib/dictionary_storage.py @@ -0,0 +1,58 @@ +# Copyright 2014 Google Inc. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Dictionary storage for OAuth2 Credentials.""" + +from oauth2client.client import OAuth2Credentials +from oauth2client.client import Storage + + +class DictionaryStorage(Storage): + """Store and retrieve credentials to and from a dictionary-like object.""" + + def __init__(self, dictionary, key, lock=None): + """Construct a DictionaryStorage instance. + + Args: + dictionary: A dictionary or dictionary-like object. + key: A string or other hashable. The credentials will be stored in + ``dictionary[key]``. + lock: An optional threading.Lock-like object. The lock will be + acquired before anything is written or read from the + dictionary. + """ + super(DictionaryStorage, self).__init__(lock=lock) + self._dictionary = dictionary + self._key = key + + def locked_get(self): + """Retrieve the credentials from the dictionary, if they exist.""" + serialized = self._dictionary.get(self._key) + + if serialized is None: + return None + + credentials = OAuth2Credentials.from_json(serialized) + credentials.set_store(self) + + return credentials + + def locked_put(self, credentials): + """Save the credentials to the dictionary.""" + serialized = credentials.to_json() + self._dictionary[self._key] = serialized + + def locked_delete(self): + """Remove the credentials from the dictionary, if they exist.""" + self._dictionary.pop(self._key, None) diff --git a/oauth2client/contrib/django_orm.py b/oauth2client/contrib/django_orm.py index e6d31a602..cd22c1b71 100644 --- a/oauth2client/contrib/django_orm.py +++ b/oauth2client/contrib/django_orm.py @@ -124,6 +124,7 @@ def __init__(self, model_class, key_name, key_value, property_name): property_name: string, name of the property that is an CredentialsProperty """ + super(Storage, self).__init__() self.model_class = model_class self.key_name = key_name self.key_value = key_value diff --git a/oauth2client/contrib/django_util/storage.py b/oauth2client/contrib/django_util/storage.py index 72ec6cdcd..4f5637d32 100644 --- a/oauth2client/contrib/django_util/storage.py +++ b/oauth2client/contrib/django_util/storage.py @@ -31,6 +31,7 @@ class DjangoSessionStorage(client.Storage): """Storage implementation that uses Django sessions.""" def __init__(self, session): + super(DjangoSessionStorage, self).__init__() self.session = session def locked_get(self): diff --git a/oauth2client/contrib/flask_util.py b/oauth2client/contrib/flask_util.py index 8b03d872c..fd31a4f2f 100644 --- a/oauth2client/contrib/flask_util.py +++ b/oauth2client/contrib/flask_util.py @@ -183,9 +183,8 @@ def requires_calendar(): raise ImportError('The flask utilities require flask 0.9 or newer.') from oauth2client.client import FlowExchangeError -from oauth2client.client import OAuth2Credentials from oauth2client.client import OAuth2WebServerFlow -from oauth2client.client import Storage +from oauth2client.contrib.dictionary_storage import DictionaryStorage from oauth2client import clientsecrets @@ -264,7 +263,7 @@ def init_app(self, app, scopes=None, client_secrets_file=None, self.flow_kwargs = kwargs if storage is None: - storage = FlaskSessionStorage() + storage = DictionaryStorage(session, _CREDENTIALS_KEY) self.storage = storage if scopes is None: @@ -548,31 +547,3 @@ def http(self, *args, **kwargs): if not self.credentials: raise ValueError('No credentials available.') return self.credentials.authorize(httplib2.Http(*args, **kwargs)) - - -class FlaskSessionStorage(Storage): - """Storage implementation that uses Flask sessions. - - Note that flask's default sessions are signed but not encrypted. Users - can see their own credentials and non-https connections can intercept user - credentials. We strongly recommend using a server-side session - implementation. - """ - - def locked_get(self): - serialized = session.get(_CREDENTIALS_KEY) - - if serialized is None: - return None - - credentials = OAuth2Credentials.from_json(serialized) - credentials.set_store(self) - - return credentials - - def locked_put(self, credentials): - session[_CREDENTIALS_KEY] = credentials.to_json() - - def locked_delete(self): - if _CREDENTIALS_KEY in session: - del session[_CREDENTIALS_KEY] diff --git a/oauth2client/contrib/keyring_storage.py b/oauth2client/contrib/keyring_storage.py index 0a4c2857b..431b67b7a 100644 --- a/oauth2client/contrib/keyring_storage.py +++ b/oauth2client/contrib/keyring_storage.py @@ -59,24 +59,9 @@ def __init__(self, service_name, user_name): credentials are stored. user_name: string, The name of the user to store credentials for. """ + super(Storage, self).__init__(lock=threading.Lock()) self._service_name = service_name self._user_name = user_name - self._lock = threading.Lock() - - def acquire_lock(self): - """Acquires any lock necessary to access this Storage. - - This lock is not reentrant. - """ - self._lock.acquire() - - def release_lock(self): - """Release the Storage lock. - - Trying to release a lock that isn't held will result in a - RuntimeError. - """ - self._lock.release() def locked_get(self): """Retrieve Credential from file. diff --git a/oauth2client/file.py b/oauth2client/file.py index d0dd174f9..d48235919 100644 --- a/oauth2client/file.py +++ b/oauth2client/file.py @@ -36,29 +36,14 @@ class Storage(BaseStorage): """Store and retrieve a single credential to and from a file.""" def __init__(self, filename): + super(Storage, self).__init__(lock=threading.Lock()) self._filename = filename - self._lock = threading.Lock() def _validate_file(self): if os.path.islink(self._filename): raise CredentialsFileSymbolicLinkError( 'File: %s is a symbolic link.' % self._filename) - def acquire_lock(self): - """Acquires any lock necessary to access this Storage. - - This lock is not reentrant. - """ - self._lock.acquire() - - def release_lock(self): - """Release the Storage lock. - - Trying to release a lock that isn't held will result in a - RuntimeError. - """ - self._lock.release() - def locked_get(self): """Retrieve Credential from file. diff --git a/tests/contrib/test_dictionary_storage.py b/tests/contrib/test_dictionary_storage.py new file mode 100644 index 000000000..8774efd72 --- /dev/null +++ b/tests/contrib/test_dictionary_storage.py @@ -0,0 +1,65 @@ +# Copyright 2014 Google Inc. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Unit tests for oauth2client.dictionary_storage""" + +import unittest + +from oauth2client import GOOGLE_TOKEN_URI +from oauth2client.client import OAuth2Credentials +from oauth2client.contrib.dictionary_storage import DictionaryStorage + + +__author__ = 'jonwayne@google.com (Jon Wayne Parrott)' + + +class DictionaryStorageTests(unittest.TestCase): + + def _generate_credentials(self, scopes=None): + return OAuth2Credentials( + 'access_tokenz', + 'client_idz', + 'client_secretz', + 'refresh_tokenz', + '3600', + GOOGLE_TOKEN_URI, + 'Test', + id_token={ + 'sub': '123', + 'email': 'user@example.com' + }, + scopes=scopes) + + def test_string_key(self): + dictionary = {} + key = 'credentials' + credentials = self._generate_credentials() + storage = DictionaryStorage(dictionary, key) + + storage.put(credentials) + + self.assertTrue(key in dictionary) + + retrieved = storage.get() + + self.assertEqual(retrieved.access_token, credentials.access_token) + + storage.delete() + + self.assertTrue(key not in dictionary) + self.assertTrue(storage.get() is None) + + +if __name__ == '__main__': # pragma: NO COVER + unittest.main()