From a9d2daace2ee29caae7bd62abcd37779f317f0db Mon Sep 17 00:00:00 2001 From: Ronald De Rose Date: Mon, 16 May 2016 18:36:50 +0000 Subject: [PATCH] Move the oauth1 abstract base class out of core This patch moves the oauth1 abstract base class out of core and into backends/base.py This removes dependencies where backend code references code in the core. The reasoning being that the core should know about the backend interface, but the backends should not know anything about the core (separation of concerns). And part of the risk here is a potential for circular dependencies. Partial-Bug: #1563101 Change-Id: I730798c71a6e8be8f32edd5bddb9e86cabd6ff65 --- keystone/oauth1/backends/base.py | 220 ++++++++++++++++++++++++++ keystone/oauth1/backends/sql.py | 14 +- keystone/oauth1/core.py | 210 ++---------------------- keystone/oauth1/validator.py | 3 +- keystone/tests/unit/test_v3_oauth1.py | 6 +- 5 files changed, 242 insertions(+), 211 deletions(-) create mode 100644 keystone/oauth1/backends/base.py diff --git a/keystone/oauth1/backends/base.py b/keystone/oauth1/backends/base.py new file mode 100644 index 0000000000..fc41a61a2f --- /dev/null +++ b/keystone/oauth1/backends/base.py @@ -0,0 +1,220 @@ +# Copyright 2012 OpenStack Foundation +# +# 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. + +import abc +import string + +import six + +from keystone import exception + + +# The characters used to generate verifiers are limited to alphanumerical +# values for ease of manual entry. Commonly confused characters are omitted. +VERIFIER_CHARS = string.ascii_letters + string.digits +CONFUSED_CHARS = 'jiIl1oO0' +VERIFIER_CHARS = ''.join(c for c in VERIFIER_CHARS if c not in CONFUSED_CHARS) + + +def filter_token(access_token_ref): + """Filter out private items in an access token dict. + + 'access_secret' is never returned. + + :returns: access_token_ref + + """ + if access_token_ref: + access_token_ref = access_token_ref.copy() + access_token_ref.pop('access_secret', None) + return access_token_ref + + +def filter_consumer(consumer_ref): + """Filter out private items in a consumer dict. + + 'secret' is never returned. + + :returns: consumer_ref + + """ + if consumer_ref: + consumer_ref = consumer_ref.copy() + consumer_ref.pop('secret', None) + return consumer_ref + + +@six.add_metaclass(abc.ABCMeta) +class Oauth1DriverV8(object): + """Interface description for an OAuth1 driver.""" + + @abc.abstractmethod + def create_consumer(self, consumer_ref): + """Create consumer. + + :param consumer_ref: consumer ref with consumer name + :type consumer_ref: dict + :returns: consumer_ref + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def update_consumer(self, consumer_id, consumer_ref): + """Update consumer. + + :param consumer_id: id of consumer to update + :type consumer_id: string + :param consumer_ref: new consumer ref with consumer name + :type consumer_ref: dict + :returns: consumer_ref + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def list_consumers(self): + """List consumers. + + :returns: list of consumers + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def get_consumer(self, consumer_id): + """Get consumer, returns the consumer id (key) and description. + + :param consumer_id: id of consumer to get + :type consumer_id: string + :returns: consumer_ref + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def get_consumer_with_secret(self, consumer_id): + """Like get_consumer(), but also returns consumer secret. + + Returned dictionary consumer_ref includes consumer secret. + Secrets should only be shared upon consumer creation; the + consumer secret is required to verify incoming OAuth requests. + + :param consumer_id: id of consumer to get + :type consumer_id: string + :returns: consumer_ref containing consumer secret + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def delete_consumer(self, consumer_id): + """Delete consumer. + + :param consumer_id: id of consumer to get + :type consumer_id: string + :returns: None. + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def list_access_tokens(self, user_id): + """List access tokens. + + :param user_id: search for access tokens authorized by given user id + :type user_id: string + :returns: list of access tokens the user has authorized + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def delete_access_token(self, user_id, access_token_id): + """Delete access token. + + :param user_id: authorizing user id + :type user_id: string + :param access_token_id: access token to delete + :type access_token_id: string + :returns: None + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def create_request_token(self, consumer_id, requested_project, + request_token_duration): + """Create request token. + + :param consumer_id: the id of the consumer + :type consumer_id: string + :param requested_project_id: requested project id + :type requested_project_id: string + :param request_token_duration: duration of request token + :type request_token_duration: string + :returns: request_token_ref + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def get_request_token(self, request_token_id): + """Get request token. + + :param request_token_id: the id of the request token + :type request_token_id: string + :returns: request_token_ref + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def get_access_token(self, access_token_id): + """Get access token. + + :param access_token_id: the id of the access token + :type access_token_id: string + :returns: access_token_ref + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def authorize_request_token(self, request_token_id, user_id, role_ids): + """Authorize request token. + + :param request_token_id: the id of the request token, to be authorized + :type request_token_id: string + :param user_id: the id of the authorizing user + :type user_id: string + :param role_ids: list of role ids to authorize + :type role_ids: list + :returns: verifier + + """ + raise exception.NotImplemented() # pragma: no cover + + @abc.abstractmethod + def create_access_token(self, request_id, access_token_duration): + """Create access token. + + :param request_id: the id of the request token, to be deleted + :type request_id: string + :param access_token_duration: duration of an access token + :type access_token_duration: string + :returns: access_token_ref + + """ + raise exception.NotImplemented() # pragma: no cover diff --git a/keystone/oauth1/backends/sql.py b/keystone/oauth1/backends/sql.py index 166b69e10f..c20c803c7c 100644 --- a/keystone/oauth1/backends/sql.py +++ b/keystone/oauth1/backends/sql.py @@ -23,7 +23,7 @@ from keystone.common import sql from keystone.common import utils from keystone import exception from keystone.i18n import _ -from keystone.oauth1 import core +from keystone.oauth1.backends import base random = _random.SystemRandom() @@ -84,7 +84,7 @@ class AccessToken(sql.ModelBase, sql.DictBase): return dict(self.items()) -class OAuth1(core.Oauth1DriverV8): +class OAuth1(base.Oauth1DriverV8): def _get_consumer(self, session, consumer_id): consumer_ref = session.query(Consumer).get(consumer_id) if consumer_ref is None: @@ -97,7 +97,7 @@ class OAuth1(core.Oauth1DriverV8): return consumer_ref.to_dict() def get_consumer(self, consumer_id): - return core.filter_consumer( + return base.filter_consumer( self.get_consumer_with_secret(consumer_id)) def create_consumer(self, consumer_ref): @@ -135,7 +135,7 @@ class OAuth1(core.Oauth1DriverV8): def list_consumers(self): with sql.session_for_read() as session: cons = session.query(Consumer) - return [core.filter_consumer(x.to_dict()) for x in cons] + return [base.filter_consumer(x.to_dict()) for x in cons] def update_consumer(self, consumer_id, consumer_ref): with sql.session_for_write() as session: @@ -145,7 +145,7 @@ class OAuth1(core.Oauth1DriverV8): new_consumer = Consumer.from_dict(old_consumer_dict) consumer.description = new_consumer.description consumer.extra = new_consumer.extra - return core.filter_consumer(consumer.to_dict()) + return base.filter_consumer(consumer.to_dict()) def create_request_token(self, consumer_id, requested_project, request_token_duration): @@ -188,7 +188,7 @@ class OAuth1(core.Oauth1DriverV8): token_ref = self._get_request_token(session, request_token_id) token_dict = token_ref.to_dict() token_dict['authorizing_user_id'] = user_id - token_dict['verifier'] = ''.join(random.sample(core.VERIFIER_CHARS, + token_dict['verifier'] = ''.join(random.sample(base.VERIFIER_CHARS, 8)) token_dict['role_ids'] = jsonutils.dumps(role_ids) @@ -245,7 +245,7 @@ class OAuth1(core.Oauth1DriverV8): with sql.session_for_read() as session: q = session.query(AccessToken) user_auths = q.filter_by(authorizing_user_id=user_id) - return [core.filter_token(x.to_dict()) for x in user_auths] + return [base.filter_token(x.to_dict()) for x in user_auths] def delete_access_token(self, user_id, access_token_id): with sql.session_for_write() as session: diff --git a/keystone/oauth1/core.py b/keystone/oauth1/core.py index 2e52aefe35..53cc61ac62 100644 --- a/keystone/oauth1/core.py +++ b/keystone/oauth1/core.py @@ -16,15 +16,13 @@ from __future__ import absolute_import -import abc -import string import uuid import oauthlib.common from oauthlib import oauth1 from oslo_config import cfg from oslo_log import log -import six +from oslo_log import versionutils from keystone.common import dependency from keystone.common import extension @@ -32,6 +30,7 @@ from keystone.common import manager from keystone import exception from keystone.i18n import _LE from keystone import notifications +from keystone.oauth1.backends import base RequestValidator = oauth1.RequestValidator @@ -42,11 +41,6 @@ AuthorizationEndpoint = oauth1.AuthorizationEndpoint SIG_HMAC = oauth1.SIGNATURE_HMAC RequestTokenEndpoint = oauth1.RequestTokenEndpoint oRequest = oauthlib.common.Request -# The characters used to generate verifiers are limited to alphanumerical -# values for ease of manual entry. Commonly confused characters are omitted. -VERIFIER_CHARS = string.ascii_letters + string.digits -CONFUSED_CHARS = 'jiIl1oO0' -VERIFIER_CHARS = ''.join(c for c in VERIFIER_CHARS if c not in CONFUSED_CHARS) class Token(object): @@ -86,34 +80,6 @@ extension.register_admin_extension(EXTENSION_DATA['alias'], EXTENSION_DATA) extension.register_public_extension(EXTENSION_DATA['alias'], EXTENSION_DATA) -def filter_consumer(consumer_ref): - """Filter out private items in a consumer dict. - - 'secret' is never returned. - - :returns: consumer_ref - - """ - if consumer_ref: - consumer_ref = consumer_ref.copy() - consumer_ref.pop('secret', None) - return consumer_ref - - -def filter_token(access_token_ref): - """Filter out private items in an access token dict. - - 'access_secret' is never returned. - - :returns: access_token_ref - - """ - if access_token_ref: - access_token_ref = access_token_ref.copy() - access_token_ref.pop('access_secret', None) - return access_token_ref - - def get_oauth_headers(headers): parameters = {} @@ -199,169 +165,13 @@ class Manager(manager.Manager): return ret -@six.add_metaclass(abc.ABCMeta) -class Oauth1DriverV8(object): - """Interface description for an OAuth1 driver.""" - - @abc.abstractmethod - def create_consumer(self, consumer_ref): - """Create consumer. - - :param consumer_ref: consumer ref with consumer name - :type consumer_ref: dict - :returns: consumer_ref - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def update_consumer(self, consumer_id, consumer_ref): - """Update consumer. - - :param consumer_id: id of consumer to update - :type consumer_id: string - :param consumer_ref: new consumer ref with consumer name - :type consumer_ref: dict - :returns: consumer_ref - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def list_consumers(self): - """List consumers. - - :returns: list of consumers - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def get_consumer(self, consumer_id): - """Get consumer, returns the consumer id (key) and description. - - :param consumer_id: id of consumer to get - :type consumer_id: string - :returns: consumer_ref - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def get_consumer_with_secret(self, consumer_id): - """Like get_consumer(), but also returns consumer secret. - - Returned dictionary consumer_ref includes consumer secret. - Secrets should only be shared upon consumer creation; the - consumer secret is required to verify incoming OAuth requests. - - :param consumer_id: id of consumer to get - :type consumer_id: string - :returns: consumer_ref containing consumer secret - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def delete_consumer(self, consumer_id): - """Delete consumer. - - :param consumer_id: id of consumer to get - :type consumer_id: string - :returns: None. - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def list_access_tokens(self, user_id): - """List access tokens. - - :param user_id: search for access tokens authorized by given user id - :type user_id: string - :returns: list of access tokens the user has authorized - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def delete_access_token(self, user_id, access_token_id): - """Delete access token. - - :param user_id: authorizing user id - :type user_id: string - :param access_token_id: access token to delete - :type access_token_id: string - :returns: None - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def create_request_token(self, consumer_id, requested_project, - request_token_duration): - """Create request token. - - :param consumer_id: the id of the consumer - :type consumer_id: string - :param requested_project_id: requested project id - :type requested_project_id: string - :param request_token_duration: duration of request token - :type request_token_duration: string - :returns: request_token_ref - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def get_request_token(self, request_token_id): - """Get request token. - - :param request_token_id: the id of the request token - :type request_token_id: string - :returns: request_token_ref - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def get_access_token(self, access_token_id): - """Get access token. - - :param access_token_id: the id of the access token - :type access_token_id: string - :returns: access_token_ref - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def authorize_request_token(self, request_token_id, user_id, role_ids): - """Authorize request token. - - :param request_token_id: the id of the request token, to be authorized - :type request_token_id: string - :param user_id: the id of the authorizing user - :type user_id: string - :param role_ids: list of role ids to authorize - :type role_ids: list - :returns: verifier - - """ - raise exception.NotImplemented() # pragma: no cover - - @abc.abstractmethod - def create_access_token(self, request_id, access_token_duration): - """Create access token. - - :param request_id: the id of the request token, to be deleted - :type request_id: string - :param access_token_duration: duration of an access token - :type access_token_duration: string - :returns: access_token_ref - - """ - raise exception.NotImplemented() # pragma: no cover +@versionutils.deprecated( + versionutils.deprecated.NEWTON, + what='keystone.oauth1.Oauth1DriverV8', + in_favor_of='keystone.oauth1.backends.base.Oauth1DriverV8', + remove_in=+1) +class Oauth1DriverV8(base.Oauth1DriverV8): + pass -Driver = manager.create_legacy_driver(Oauth1DriverV8) +Driver = manager.create_legacy_driver(base.Oauth1DriverV8) diff --git a/keystone/oauth1/validator.py b/keystone/oauth1/validator.py index f21a02d7d0..0b7798e4ef 100644 --- a/keystone/oauth1/validator.py +++ b/keystone/oauth1/validator.py @@ -18,6 +18,7 @@ import six from keystone.common import dependency from keystone import exception +from keystone.oauth1.backends import base from keystone.oauth1 import core as oauth1 @@ -56,7 +57,7 @@ class OAuthValidator(oauth1.RequestValidator): return set(nonce) <= self.safe_characters def check_verifier(self, verifier): - return (all(i in oauth1.VERIFIER_CHARS for i in verifier) and + return (all(i in base.VERIFIER_CHARS for i in verifier) and len(verifier) == 8) def get_client_secret(self, client_key, request): diff --git a/keystone/tests/unit/test_v3_oauth1.py b/keystone/tests/unit/test_v3_oauth1.py index 88d04ecb74..666da6b957 100644 --- a/keystone/tests/unit/test_v3_oauth1.py +++ b/keystone/tests/unit/test_v3_oauth1.py @@ -25,8 +25,8 @@ from six.moves import urllib from keystone.contrib.oauth1 import routers from keystone import exception from keystone import oauth1 +from keystone.oauth1.backends import base from keystone.oauth1 import controllers -from keystone.oauth1 import core from keystone.tests import unit from keystone.tests.unit.common import test_notifications from keystone.tests.unit import ksfixtures @@ -272,7 +272,7 @@ class OAuthFlowTests(OAuth1Tests): body = {'roles': [{'id': self.role_id}]} resp = self.put(url, body=body, expected_status=http_client.OK) self.verifier = resp.result['token']['oauth_verifier'] - self.assertTrue(all(i in core.VERIFIER_CHARS for i in self.verifier)) + self.assertTrue(all(i in base.VERIFIER_CHARS for i in self.verifier)) self.assertEqual(8, len(self.verifier)) self.request_token.set_verifier(self.verifier) @@ -851,7 +851,7 @@ class OAuthNotificationTests(OAuth1Tests, body = {'roles': [{'id': self.role_id}]} resp = self.put(url, body=body, expected_status=http_client.OK) self.verifier = resp.result['token']['oauth_verifier'] - self.assertTrue(all(i in core.VERIFIER_CHARS for i in self.verifier)) + self.assertTrue(all(i in base.VERIFIER_CHARS for i in self.verifier)) self.assertEqual(8, len(self.verifier)) self.request_token.set_verifier(self.verifier)