From 5cf5d1244bb7f71ad955d96e44fb6018326843dd Mon Sep 17 00:00:00 2001 From: Joe Gregorio Date: Fri, 16 Nov 2012 16:36:12 -0500 Subject: [PATCH] Consolidate handling of scopes. Reviewed in https://codereview.appspot.com/6853060/. --- oauth2client/appengine.py | 15 +++++++-------- oauth2client/client.py | 18 +++++++----------- oauth2client/gce.py | 6 ++---- oauth2client/multistore_file.py | 5 ++--- oauth2client/util.py | 20 ++++++++++++++++++++ tests/test_oauth2client_appengine.py | 16 ++++++++++++++-- tests/test_oauth2client_util.py | 27 +++++++++++++++++++++++++++ 7 files changed, 79 insertions(+), 28 deletions(-) create mode 100644 tests/test_oauth2client_util.py diff --git a/oauth2client/appengine.py b/oauth2client/appengine.py index 5439a35..c747528 100644 --- a/oauth2client/appengine.py +++ b/oauth2client/appengine.py @@ -126,12 +126,10 @@ class AppAssertionCredentials(AssertionCredentials): """Constructor for AppAssertionCredentials Args: - scope: string or list of strings, scope(s) of the credentials being + scope: string or iterable of strings, scope(s) of the credentials being requested. """ - if type(scope) is list: - scope = ' '.join(scope) - self.scope = scope + self.scope = util.scopes_to_string(scope) super(AppAssertionCredentials, self).__init__( 'ignored' # assertion_type is ignore in this subclass. @@ -157,7 +155,8 @@ class AppAssertionCredentials(AssertionCredentials): AccessTokenRefreshError: When the refresh fails. """ try: - (token, _) = app_identity.get_access_token(self.scope) + scopes = self.scope.split() + (token, _) = app_identity.get_access_token(scopes) except app_identity.Error, e: raise AccessTokenRefreshError(str(e)) self.access_token = token @@ -399,7 +398,7 @@ class OAuth2Decorator(object): Args: client_id: string, client identifier. client_secret: string client secret. - scope: string or list of strings, scope(s) of the credentials being + scope: string or iterable of strings, scope(s) of the credentials being requested. auth_uri: string, URI for authorization endpoint. For convenience defaults to Google's endpoints but any OAuth 2.0 provider can be used. @@ -419,7 +418,7 @@ class OAuth2Decorator(object): self.credentials = None self._client_id = client_id self._client_secret = client_secret - self._scope = scope + self._scope = util.scopes_to_string(scope) self._auth_uri = auth_uri self._token_uri = token_uri self._user_agent = user_agent @@ -647,7 +646,7 @@ class OAuth2DecoratorFromClientSecrets(OAuth2Decorator): Args: filename: string, File name of client secrets. - scope: string or list of strings, scope(s) of the credentials being + scope: string or iterable of strings, scope(s) of the credentials being requested. message: string, A friendly string to display to the user if the clientsecrets file is missing or invalid. The message may contain HTML diff --git a/oauth2client/client.py b/oauth2client/client.py index dc9bc89..cce4ae6 100644 --- a/oauth2client/client.py +++ b/oauth2client/client.py @@ -775,7 +775,7 @@ if HAS_OPENSSL: Args: service_account_name: string, id for account, usually an email address. private_key: string, private key in P12 format. - scope: string or list of strings, scope(s) of the credentials being + scope: string or iterable of strings, scope(s) of the credentials being requested. private_key_password: string, password for private_key. user_agent: string, HTTP User-Agent to provide for this application. @@ -790,9 +790,7 @@ if HAS_OPENSSL: token_uri=token_uri, ) - if type(scope) is list: - scope = ' '.join(scope) - self.scope = scope + self.scope = util.scopes_to_string(scope) # Keep base64 encoded so it can be stored in JSON. self.private_key = base64.b64encode(private_key) @@ -936,7 +934,7 @@ def credentials_from_code(client_id, client_secret, scope, code, Args: client_id: string, client identifier. client_secret: string, client secret. - scope: string or list of strings, scope(s) to request. + scope: string or iterable of strings, scope(s) to request. code: string, An authroization code, most likely passed down from the client redirect_uri: string, this is generally set to 'postmessage' to match the @@ -973,7 +971,7 @@ def credentials_from_clientsecrets_and_code(filename, scope, code, Args: filename: string, File name of clientsecrets. - scope: string or list of strings, scope(s) to request. + scope: string or iterable of strings, scope(s) to request. code: string, An authorization code, most likely passed down from the client message: string, A friendly string to display to the user if the @@ -1024,7 +1022,7 @@ class OAuth2WebServerFlow(Flow): Args: client_id: string, client identifier. client_secret: string client secret. - scope: string or list of strings, scope(s) of the credentials being + scope: string or iterable of strings, scope(s) of the credentials being requested. redirect_uri: string, Either the string 'urn:ietf:wg:oauth:2.0:oob' for a non-web-based application, or a URI that handles the callback from @@ -1039,9 +1037,7 @@ class OAuth2WebServerFlow(Flow): """ self.client_id = client_id self.client_secret = client_secret - if type(scope) is list: - scope = ' '.join(scope) - self.scope = scope + self.scope = util.scopes_to_string(scope) self.redirect_uri = redirect_uri self.user_agent = user_agent self.auth_uri = auth_uri @@ -1169,7 +1165,7 @@ def flow_from_clientsecrets(filename, scope, redirect_uri=None, message=None, ca Args: filename: string, File name of client secrets. - scope: string or list of strings, scope(s) to request. + scope: string or iterable of strings, scope(s) to request. redirect_uri: string, Either the string 'urn:ietf:wg:oauth:2.0:oob' for a non-web-based application, or a URI that handles the callback from the authorization server. diff --git a/oauth2client/gce.py b/oauth2client/gce.py index d1b1234..ad27588 100644 --- a/oauth2client/gce.py +++ b/oauth2client/gce.py @@ -53,12 +53,10 @@ class AppAssertionCredentials(AssertionCredentials): """Constructor for AppAssertionCredentials Args: - scope: string or list of strings, scope(s) of the credentials being + scope: string or iterable of strings, scope(s) of the credentials being requested. """ - if type(scope) is list: - scope = ' '.join(scope) - self.scope = scope + self.scope = util.scopes_to_string(scope) super(AppAssertionCredentials, self).__init__( 'ignored' # assertion_type is ignore in this subclass. diff --git a/oauth2client/multistore_file.py b/oauth2client/multistore_file.py index c919573..b8e9ff0 100644 --- a/oauth2client/multistore_file.py +++ b/oauth2client/multistore_file.py @@ -69,7 +69,7 @@ def get_credential_storage(filename, client_id, user_agent, scope, filename: The JSON file storing a set of credentials client_id: The client_id for the credential user_agent: The user agent for the credential - scope: string or list of strings, Scope(s) being requested + scope: string or iterable of strings, Scope(s) being requested warn_on_readonly: if True, log a warning if the store is readonly Returns: @@ -83,8 +83,7 @@ def get_credential_storage(filename, client_id, user_agent, scope, filename, _MultiStore(filename, warn_on_readonly=warn_on_readonly)) finally: _multistores_lock.release() - if type(scope) is list: - scope = ' '.join(scope) + scope = util.scopes_to_string(scope) return multistore._get_storage(client_id, user_agent, scope) diff --git a/oauth2client/util.py b/oauth2client/util.py index bda14c6..8166d39 100644 --- a/oauth2client/util.py +++ b/oauth2client/util.py @@ -27,6 +27,7 @@ __all__ = [ import gflags import inspect import logging +import types logger = logging.getLogger(__name__) @@ -125,3 +126,22 @@ def positional(max_positional_args): else: args, _, _, defaults = inspect.getargspec(max_positional_args) return positional(len(args) - len(defaults))(max_positional_args) + + +def scopes_to_string(scopes): + """Converts scope value to a string. + + If scopes is a string then it is simply passed through. If scopes is an + iterable then a string is returned that is all the individual scopes + concatenated with spaces. + + Args: + scopes: string or iterable of strings, the scopes. + + Returns: + The scopes formatted as a single string. + """ + if isinstance(scopes, types.StringTypes): + return scopes + else: + return ' '.join(scopes) diff --git a/tests/test_oauth2client_appengine.py b/tests/test_oauth2client_appengine.py index 6039a0d..827a31f 100644 --- a/tests/test_oauth2client_appengine.py +++ b/tests/test_oauth2client_appengine.py @@ -175,7 +175,9 @@ class TestAppAssertionCredentials(unittest.TestCase): apiproxy_stub_map.apiproxy.RegisterStub( 'memcache', memcache_stub.MemcacheServiceStub()) - scope = ["http://www.googleapis.com/scope"] + scope = [ + "http://www.googleapis.com/scope", + "http://www.googleapis.com/scope2"] credentials = AppAssertionCredentials(scope) http = httplib2.Http() credentials.refresh(http) @@ -183,8 +185,18 @@ class TestAppAssertionCredentials(unittest.TestCase): json = credentials.to_json() credentials = Credentials.new_from_json(json) - self.assertEqual(scope[0], credentials.scope) + self.assertEqual( + 'http://www.googleapis.com/scope http://www.googleapis.com/scope2', + credentials.scope) + scope = "http://www.googleapis.com/scope http://www.googleapis.com/scope2" + credentials = AppAssertionCredentials(scope) + http = httplib2.Http() + credentials.refresh(http) + self.assertEqual('a_token_123', credentials.access_token) + self.assertEqual( + 'http://www.googleapis.com/scope http://www.googleapis.com/scope2', + credentials.scope) class TestFlowModel(db.Model): flow = FlowProperty() diff --git a/tests/test_oauth2client_util.py b/tests/test_oauth2client_util.py new file mode 100644 index 0000000..6c72521 --- /dev/null +++ b/tests/test_oauth2client_util.py @@ -0,0 +1,27 @@ +"""Unit tests for oauth2client.util.""" + +__author__ = 'jcgregorio@google.com (Joe Gregorio)' + +import unittest + +from oauth2client import util + + +class ScopeToStringTests(unittest.TestCase): + + def test_iterables(self): + cases = [ + ('', ''), + ('', ()), + ('', []), + ('', ('', )), + ('', ['', ]), + ('a', ('a', )), + ('b', ['b', ]), + ('a b', ['a', 'b']), + ('a b', ('a', 'b')), + ('a b', 'a b'), + ('a b', (s for s in ['a', 'b'])), + ] + for expected, case in cases: + self.assertEqual(expected, util.scopes_to_string(case))