From 1ecf880318aad8d54f12e94de8fe69cee4a600c5 Mon Sep 17 00:00:00 2001 From: Francis Charette-Migneault Date: Thu, 5 May 2022 14:15:39 -0400 Subject: [PATCH 1/5] add request/response hooks to adapter utility --- CHANGES.rst | 5 +++++ requirements.txt | 2 ++ twitcher/adapter/base.py | 35 +++++++++++++++++++++++++++++++++-- twitcher/adapter/default.py | 6 ++++++ twitcher/models/service.py | 21 ++++++++++++++++++--- twitcher/owsproxy.py | 36 +++++++++++++++++++++++++----------- twitcher/typedefs.py | 6 ++++++ 7 files changed, 95 insertions(+), 16 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 5712b66b..a57d4a72 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,6 +4,11 @@ Changes Unreleased ========== +Changes: + +* Add request and response hooks operations to adapter allowing derived implementations to modify OWS proxied requests + and returned responses from the service. The default adapter applies no modifications to the original definitions. + 0.6.2 (2021-12-01) ================== diff --git a/requirements.txt b/requirements.txt index b2d084b0..bdb6ca5e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -32,3 +32,5 @@ pyramid_oauthlib>=0.4.1 oauthlib<3 requests_oauthlib<1.2.0 PyJWT>=2 +# typing extension required for TypedDict +typing_extensions; python_version < "3.8" diff --git a/twitcher/adapter/base.py b/twitcher/adapter/base.py index 1778b74f..f23c0f38 100644 --- a/twitcher/adapter/base.py +++ b/twitcher/adapter/base.py @@ -2,10 +2,13 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: - from twitcher.typedefs import AnySettingsContainer, JSON - from twitcher.interface import OWSSecurityInterface, OWSRegistryInterface from pyramid.config import Configurator from pyramid.request import Request + from pyramid.response import Response + + from twitcher.interface import OWSSecurityInterface, OWSRegistryInterface + from twitcher.models.service import ServiceConfig + from twitcher.typedefs import AnySettingsContainer, JSON class AdapterInterface(object): @@ -54,3 +57,31 @@ def owsproxy_config(self, container): Returns the 'owsproxy' implementation of the adapter. """ raise NotImplementedError + + def request_hook(self, request, service): + # type: (Request, ServiceConfig) -> Request + """ + Apply modifications onto the request before sending it. + + .. versionadded:: 0.7.0 + + Request members employed after this hook is called include: + - :meth:`Request.headers` + - :meth:`Request.method` + - :meth:`Request.body` + + This method can modified those members to adapt the request for specific service logic. + """ + raise NotImplementedError + + def response_hook(self, response, service): + # type: (Response, ServiceConfig) -> Response + """ + Apply modifications onto the response from sent request. + + .. versionadded:: 0.7.0 + + The received response from the proxied service is normally returned directly. + This method can modify the response to adapt it for specific service logic. + """ + raise NotImplementedError diff --git a/twitcher/adapter/default.py b/twitcher/adapter/default.py index 74a046de..55e11be2 100644 --- a/twitcher/adapter/default.py +++ b/twitcher/adapter/default.py @@ -37,3 +37,9 @@ def owsproxy_config(self, container): if not isinstance(container, Configurator): container = self.configurator_factory(container) owsproxy_defaultconfig(container) + + def pre_request_hook(self, request, service): + return request + + def post_request_hook(self, response, service): + return response diff --git a/twitcher/models/service.py b/twitcher/models/service.py index 2c26757d..61c112b5 100644 --- a/twitcher/models/service.py +++ b/twitcher/models/service.py @@ -4,10 +4,23 @@ String, ) from sqlalchemy.ext.hybrid import hybrid_property -from typing import Union +from typing import TYPE_CHECKING, Union from .meta import Base +if TYPE_CHECKING: + from twitcher.typedefs import TypedDict + + ServiceConfig = TypedDict("ServiceConfig", { + "url": str, + "name": str, + "type": str, + "purl": str, + "auth": str, + "public": bool, + "verify": bool + }, total=True) + class Service(Base): __tablename__ = 'services' @@ -21,20 +34,22 @@ class Service(Base): @hybrid_property def verify(self): + # type: () -> bool if self._verify == 1: return True return False @verify.setter - def verify(self, verify: Union[bool, int]): + def verify(self, verify: Union[bool, int]) -> None: self._verify = int(verify) @property - def public(self): + def public(self) -> bool: """Return true if public access.""" return self.auth not in ['token', 'cert'] def json(self): + # type: () -> ServiceConfig return { 'url': self.url, 'name': self.name, diff --git a/twitcher/owsproxy.py b/twitcher/owsproxy.py index 0534c176..35695185 100644 --- a/twitcher/owsproxy.py +++ b/twitcher/owsproxy.py @@ -12,19 +12,20 @@ from typing import TYPE_CHECKING from twitcher.owsexceptions import OWSAccessForbidden, OWSAccessFailed, OWSException, OWSNoApplicableCode -from twitcher.utils import ( - replace_caps_url, - get_settings, - get_twitcher_url, - is_valid_url) +from twitcher.utils import get_settings, get_twitcher_url, is_valid_url, replace_caps_url import logging LOGGER = logging.getLogger('TWITCHER') if TYPE_CHECKING: - from twitcher.typedefs import AnySettingsContainer # noqa: F401 - from pyramid.config import Configurator # noqa: F401 - from typing import AnyStr # noqa: F401 + from typing import Optional + + from pyramid.config import Configurator + from pyramid.request import Request + + from twitcher.adapter.base import AdapterInterface + from twitcher.models.service import ServiceConfig + from twitcher.typedefs import AnySettingsContainer allowed_content_types = ( @@ -64,6 +65,7 @@ def __iter__(self): def _send_request(request, service, extra_path=None, request_params=None): + # type: (Request, ServiceConfig, Optional[str], Optional[str]) -> Response # TODO: fix way to build url url = service['url'] @@ -141,19 +143,20 @@ def _send_request(request, service, extra_path=None, request_params=None): def owsproxy_base_path(container): - # type: (AnySettingsContainer) -> AnyStr + # type: (AnySettingsContainer) -> str settings = get_settings(container) return settings.get('twitcher.ows_proxy_protected_path', '/ows').rstrip('/').strip() def owsproxy_base_url(container): - # type: (AnySettingsContainer) -> AnyStr + # type: (AnySettingsContainer) -> str twitcher_url = get_twitcher_url(container) owsproxy_path = owsproxy_base_path(container) return twitcher_url + owsproxy_path def owsproxy_view(request): + # type: (Request) -> Response service_name = request.matchdict.get('service_name') try: extra_path = request.matchdict.get('extra_path') @@ -167,7 +170,10 @@ def owsproxy_view(request): try: if not request.is_verified: raise OWSAccessForbidden("Access to service is forbidden.") - return _send_request(request, service, extra_path, request_params=request.query_string) + request = request.adapter.request_hook(request, service) + response = _send_request(request, service, extra_path, request_params=request.query_string) + response = request.adapter.response_hook(response, service) + return response except OWSException as exc: LOGGER.warning("Security check failed but was not handled as expected by 'is_verified' method.", exc_info=exc) raise @@ -192,5 +198,13 @@ def owsproxy_defaultconfig(config): def includeme(config): + # type: (Configurator) -> None from twitcher.adapter import get_adapter_factory + + def get_adapter(request): + # type: (Request) -> AdapterInterface + adapter = get_adapter_factory(request) + return adapter + get_adapter_factory(config).owsproxy_config(config) + config.add_request_method(get_adapter, reify=False, property=True, name="adapter") diff --git a/twitcher/typedefs.py b/twitcher/typedefs.py index 037f9ce4..ef85ecfc 100644 --- a/twitcher/typedefs.py +++ b/twitcher/typedefs.py @@ -1,3 +1,4 @@ +import typing from typing import TYPE_CHECKING if TYPE_CHECKING: from typing import AnyStr, Dict, List, Tuple, Union @@ -10,6 +11,11 @@ from webob.headers import ResponseHeaders, EnvironHeaders from webtest.response import TestResponse + if hasattr(typing, "TypedDict"): + from typing import TypedDict # pylint: disable=E0611,no-name-in-module + else: + from typing_extensions import TypedDict # noqa + Number = Union[int, float] AnyValue = Union[AnyStr, Number, bool, None] AnyKey = Union[AnyStr, int] From 2372274da16e512a00729ed7cb886d9237fb07d6 Mon Sep 17 00:00:00 2001 From: Francis Charette-Migneault Date: Fri, 6 May 2022 12:45:55 -0400 Subject: [PATCH 2/5] add test that demonstrates the use of a custom adapter with request/response hooks --- requirements_dev.txt | 1 + tests/common.py | 17 ++--- tests/functional/base.py | 2 +- tests/functional/test_adapter.py | 91 +++++++++++++++++++++++++++ tests/functional/test_api.py | 2 +- tests/functional/test_oauth2_app.py | 2 +- tests/functional/test_owsproxy_app.py | 2 +- tests/test_adapter.py | 5 +- twitcher/adapter/default.py | 4 +- 9 files changed, 110 insertions(+), 16 deletions(-) create mode 100644 tests/functional/test_adapter.py diff --git a/requirements_dev.txt b/requirements_dev.txt index f63df39c..7b418354 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -6,3 +6,4 @@ sphinx nbsphinx bump2version twine +mock diff --git a/tests/common.py b/tests/common.py index 83490799..37c1cc0d 100644 --- a/tests/common.py +++ b/tests/common.py @@ -20,15 +20,16 @@ def dummy_request(dbsession): class BaseTest(unittest.TestCase): + settings = { + 'sqlalchemy.url': 'sqlite:///:memory:', + 'twitcher.username': 'testuser', + 'twitcher.password': 'testpassword', + 'twitcher.token.type': 'custom_token', + 'twitcher.token.secret': 'testsecret', + } + def setUp(self): - self.config = testing.setUp( - settings={ - 'sqlalchemy.url': 'sqlite:///:memory:', - 'twitcher.username': 'testuser', - 'twitcher.password': 'testpassword', - 'twitcher.token.type': 'custom_token', - 'twitcher.token.secret': 'testsecret', - }) + self.config = testing.setUp(settings=self.settings) self.config.include('twitcher.models') settings = self.config.get_settings() diff --git a/tests/functional/base.py b/tests/functional/base.py index 1b4bb6a1..d9e66566 100644 --- a/tests/functional/base.py +++ b/tests/functional/base.py @@ -6,7 +6,7 @@ class FunctionalTest(BaseTest): - def test_app(self): + def get_test_app(self): app = webtest.TestApp( self.config.make_wsgi_app(), extra_environ={'db.session': self.session, 'tm.active': True}) diff --git a/tests/functional/test_adapter.py b/tests/functional/test_adapter.py new file mode 100644 index 00000000..7463608c --- /dev/null +++ b/tests/functional/test_adapter.py @@ -0,0 +1,91 @@ +import json +import mock + +from twitcher.adapter.default import DefaultAdapter +from twitcher.owssecurity import OWSSecurityInterface +from twitcher.owsregistry import OWSRegistry +from twitcher.store import ServiceStore + +from ..common import dummy_request +from .base import FunctionalTest + + +class AdapterWithHooks(DefaultAdapter): + def owssecurity_factory(self): + class DummyOWSSecurity(OWSSecurityInterface): + def verify_request(self, request): return True # noqa: E704 + return DummyOWSSecurity() + + def request_hook(self, request, service): + request.headers["X-Hook-Test-Service"] = service["name"] + return request + + def response_hook(self, response, service): + # must edit body using text content, + # json property re-generates from it, cannot set value direct on dict returned by it + data = json.loads(response.text) + data["Hook-Test-Service"] = service["name"] + response.body = json.dumps(data).encode() + return response + + +class TestAdapterWithHooks(FunctionalTest): + @property + def settings(self): + adapter_name = '{}.{}'.format(AdapterWithHooks.__module__, AdapterWithHooks.__name__) + settings = super(TestAdapterWithHooks, self).settings.copy() + settings.update({ + 'twitcher.adapter': adapter_name + }) + return settings + + def setUp(self): + super(TestAdapterWithHooks, self).setUp() + self.init_database() + service_store = ServiceStore(dummy_request(dbsession=self.session)) + self.reg = OWSRegistry(servicestore=service_store) + + self.test_service_name = "test_adapter_svc" + self.test_service = { + 'url': 'http://localhost/wps', + 'name': self.test_service_name, + 'type': 'wps', + 'auth': 'token', + 'public': False, + 'verify': True, + 'purl': 'http://myservice/wps'} + resp = self.reg.register_service(**self.test_service) + assert resp == self.test_service + + self.config.include('twitcher.owsproxy') + self.app = self.get_test_app() + + def test_request_response_hooks(self): + test_request_handle = [] + + def mocked_request(method, url, data, headers, **_): + _req = dummy_request(self.session) + _req.method = method + _req.url = url + _req.headers = headers + _req.body = data + test_request_handle.append(_req) + _resp = _req.response + _resp.content_type = "application/json" + _resp.status_code = 200 + _resp.body = json.dumps({"response": "ok"}).encode() + _resp.content = _resp.body + _resp.ok = True + return _resp + + with mock.patch("requests.request", side_effect=mocked_request): + resp = self.app.get(f'/ows/proxy/{self.test_service_name}?service=wps&request=getcapabilities') + assert resp.status_code == 200 + assert resp.content_type == "application/json" + + # check added header by request hook + assert test_request_handle + assert test_request_handle[0].headers.get("X-Hook-Test-Service") == self.test_service_name + + # check added body content by response hook + assert resp.json == {"response": "ok", "Hook-Test-Service": self.test_service_name} diff --git a/tests/functional/test_api.py b/tests/functional/test_api.py index 6bc86132..3ae1b596 100644 --- a/tests/functional/test_api.py +++ b/tests/functional/test_api.py @@ -14,7 +14,7 @@ def setUp(self): self.init_store() self.config.include('twitcher.api') - self.app = self.test_app() + self.app = self.get_test_app() @pytest.mark.skip(reason="not working") def test_register_service_and_unregister_it(self): diff --git a/tests/functional/test_oauth2_app.py b/tests/functional/test_oauth2_app.py index d6b1f3f2..a8a5bdd9 100644 --- a/tests/functional/test_oauth2_app.py +++ b/tests/functional/test_oauth2_app.py @@ -18,7 +18,7 @@ def setUp(self): self.config.include('twitcher.oauth2') self.config.add_route('compute', '/api/compute') self.config.add_view(compute_view, route_name='compute', renderer='json') - self.app = self.test_app() + self.app = self.get_test_app() def test_compute_with_param(self): access_token = self.create_token() diff --git a/tests/functional/test_owsproxy_app.py b/tests/functional/test_owsproxy_app.py index 6916488a..474373e8 100644 --- a/tests/functional/test_owsproxy_app.py +++ b/tests/functional/test_owsproxy_app.py @@ -19,7 +19,7 @@ def setUp(self): self.init_store() self.config.include('twitcher.owsproxy') - self.app = self.test_app() + self.app = self.get_test_app() @pytest.mark.online def test_getcaps(self): diff --git a/tests/test_adapter.py b/tests/test_adapter.py index cae4c1d9..604b173c 100644 --- a/tests/test_adapter.py +++ b/tests/test_adapter.py @@ -2,6 +2,7 @@ from twitcher.adapter.base import AdapterInterface from twitcher.adapter.default import DefaultAdapter from twitcher.interface import OWSSecurityInterface + from pyramid.testing import DummyRequest from pathlib import Path import pytest @@ -30,7 +31,7 @@ def test_adapter_factory_none_specified(): # noinspection PyAbstractClass,PyMethodMayBeStatic class DummyAdapter(AdapterInterface): - def owssecurity_factory(self, request): + def owssecurity_factory(self): class DummyOWSSecurity(OWSSecurityInterface): def verify_request(self, request): return True # noqa: E704 return DummyOWSSecurity() @@ -101,6 +102,6 @@ def test_adapter_factory_TestAdapter_invalid_raised(): def test_adapter_factory_call_owssecurity_factory(): settings = {'twitcher.adapter': DummyAdapter({}).name} adapter = get_adapter_factory(settings) - security = adapter.owssecurity_factory(DummyRequest()) + security = adapter.owssecurity_factory() assert isinstance(security, OWSSecurityInterface) assert security.verify_request(DummyRequest()) is True, "Requested adapter should have been called." diff --git a/twitcher/adapter/default.py b/twitcher/adapter/default.py index 55e11be2..e66788d3 100644 --- a/twitcher/adapter/default.py +++ b/twitcher/adapter/default.py @@ -38,8 +38,8 @@ def owsproxy_config(self, container): container = self.configurator_factory(container) owsproxy_defaultconfig(container) - def pre_request_hook(self, request, service): + def request_hook(self, request, service): return request - def post_request_hook(self, response, service): + def response_hook(self, response, service): return response From e2eb7a0c5f7373cf55ba574936e7376c28122383 Mon Sep 17 00:00:00 2001 From: Francis Charette-Migneault Date: Fri, 6 May 2022 18:48:27 -0400 Subject: [PATCH 3/5] add utf-8 encode in samples --- tests/functional/test_adapter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/test_adapter.py b/tests/functional/test_adapter.py index 7463608c..c0e93048 100644 --- a/tests/functional/test_adapter.py +++ b/tests/functional/test_adapter.py @@ -25,7 +25,7 @@ def response_hook(self, response, service): # json property re-generates from it, cannot set value direct on dict returned by it data = json.loads(response.text) data["Hook-Test-Service"] = service["name"] - response.body = json.dumps(data).encode() + response.body = json.dumps(data).encode("UTF-8") return response @@ -73,7 +73,7 @@ def mocked_request(method, url, data, headers, **_): _resp = _req.response _resp.content_type = "application/json" _resp.status_code = 200 - _resp.body = json.dumps({"response": "ok"}).encode() + _resp.body = json.dumps({"response": "ok"}).encode("UTF-8") _resp.content = _resp.body _resp.ok = True return _resp From f66512dd7bb668f698eeac76f0247583b2bb5904 Mon Sep 17 00:00:00 2001 From: Francis Charette-Migneault Date: Mon, 9 May 2022 20:26:02 -0400 Subject: [PATCH 4/5] fix forwarding of request within response for use by hooks (apps usually also expect this reference) --- twitcher/owsproxy.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/twitcher/owsproxy.py b/twitcher/owsproxy.py index 35695185..bf16b15c 100644 --- a/twitcher/owsproxy.py +++ b/twitcher/owsproxy.py @@ -18,10 +18,11 @@ LOGGER = logging.getLogger('TWITCHER') if TYPE_CHECKING: - from typing import Optional + from typing import Iterable, Optional from pyramid.config import Configurator from pyramid.request import Request + from requests.models import Response as RequestsResponse from twitcher.adapter.base import AdapterInterface from twitcher.models.service import ServiceConfig @@ -58,9 +59,11 @@ # requests.models.Response defaults its chunk size to 128 bytes, which is very slow class BufferedResponse(object): def __init__(self, resp): + # type: (RequestsResponse) -> None self.resp = resp def __iter__(self): + # type: () -> Iterable[bytes] return self.resp.iter_content(64 * 1024) @@ -93,7 +96,7 @@ def _send_request(request, service, extra_path=None, request_params=None): hop_by_hop = ['connection', 'keep-alive', 'public', 'proxy-authenticate', 'transfer-encoding', 'upgrade'] return Response(app_iter=BufferedResponse(resp_iter), headers={k: v for k, v in list(resp_iter.headers.items()) if k.lower() not in hop_by_hop}, - status_code=resp_iter.status_code) + status_code=resp_iter.status_code, request=request) else: try: resp = requests.request(method=request.method.upper(), url=url, data=request.body, headers=h, @@ -139,7 +142,7 @@ def _send_request(request, service, extra_path=None, request_params=None): headers = {} if ct: headers["Content-Type"] = ct - return Response(content, status=resp.status_code, headers=headers) + return Response(content, status=resp.status_code, headers=headers, request=request) def owsproxy_base_path(container): @@ -170,9 +173,12 @@ def owsproxy_view(request): try: if not request.is_verified: raise OWSAccessForbidden("Access to service is forbidden.") - request = request.adapter.request_hook(request, service) + # since request can be modified by hooks, keep reference to original adapter + # in order to ensure both request/response operations are handled by the same logic + adapter = request.adapter + request = adapter.request_hook(request, service) response = _send_request(request, service, extra_path, request_params=request.query_string) - response = request.adapter.response_hook(response, service) + response = adapter.response_hook(response, service) return response except OWSException as exc: LOGGER.warning("Security check failed but was not handled as expected by 'is_verified' method.", exc_info=exc) From 7cfe3a1c386d83cb7809c54cf974e7ff3d533708 Mon Sep 17 00:00:00 2001 From: Francis Charette-Migneault Date: Mon, 9 May 2022 20:55:09 -0400 Subject: [PATCH 5/5] fix iter typing --- twitcher/owsproxy.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/twitcher/owsproxy.py b/twitcher/owsproxy.py index bf16b15c..811caf86 100644 --- a/twitcher/owsproxy.py +++ b/twitcher/owsproxy.py @@ -18,7 +18,7 @@ LOGGER = logging.getLogger('TWITCHER') if TYPE_CHECKING: - from typing import Iterable, Optional + from typing import Iterator, Optional from pyramid.config import Configurator from pyramid.request import Request @@ -63,7 +63,7 @@ def __init__(self, resp): self.resp = resp def __iter__(self): - # type: () -> Iterable[bytes] + # type: () -> Iterator[bytes] return self.resp.iter_content(64 * 1024)