diff --git a/src/_zkapauthorizer/_storage_client.py b/src/_zkapauthorizer/_storage_client.py index 182a739c3c171da1e6b45130ebca515d9eabc665..a51fbfd1402c74e0f95ebddbd325e99f7ea076f4 100644 --- a/src/_zkapauthorizer/_storage_client.py +++ b/src/_zkapauthorizer/_storage_client.py @@ -44,6 +44,26 @@ from .storage_common import ( get_required_new_passes_for_mutable_write, ) + +class IncorrectStorageServerReference(Exception): + """ + A Foolscap remote object which should reference a ZKAPAuthorizer storage + server instead references some other kind of object. This makes the + connection, and thus the configured storage server, unusable. + """ + def __init__(self, furl, actual_name, expected_name): + self.furl = furl + self.actual_name = actual_name + self.expected_name = expected_name + + def __str__(self): + return "RemoteReference via {} provides {} instead of {}".format( + self.furl, + self.actual_name, + self.expected_name, + ) + + @implementer(IStorageServer) @attr.s class ZKAPAuthorizerStorageClient(object): @@ -69,12 +89,32 @@ class ZKAPAuthorizerStorageClient(object): which they will be used. The second is an integer giving the number of passes to request. """ + _expected_remote_interface_name = ( + "RIPrivacyPassAuthorizedStorageServer.tahoe.privatestorage.io" + ) + _get_rref = attr.ib() _get_passes = attr.ib() @property def _rref(self): - return self._get_rref() + rref = self._get_rref() + # rref provides foolscap.ipb.IRemoteReference but in practice it is a + # foolscap.referenceable.RemoteReference instance. The interface + # doesn't give us enough functionality to verify that the reference is + # to the right sort of thing but the concrete type does. + # + # Foolscap development isn't exactly racing along and if we're lucky + # we'll switch to HTTP before too long anyway. + actual_name = rref.tracker.interfaceName + expected_name = self._expected_remote_interface_name + if actual_name != expected_name: + raise IncorrectStorageServerReference( + rref.tracker.getURL(), + actual_name, + expected_name, + ) + return rref def _get_encoded_passes(self, message, count): """ diff --git a/src/_zkapauthorizer/tests/_exception.py b/src/_zkapauthorizer/tests/_exception.py new file mode 100644 index 0000000000000000000000000000000000000000..504e383cb5a664cee0a0c11449721072e3bb80c4 --- /dev/null +++ b/src/_zkapauthorizer/tests/_exception.py @@ -0,0 +1,128 @@ +# Copyright (c) 2009-2012 testtools developers. +# +# Copyright 2019 PrivateStorage.io, LLC +# +# 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. + +__all__ = [ + 'MatchesExceptionType', + 'Raises', + 'raises', + ] + +import sys + +from testtools.matchers import ( + Matcher, + Mismatch, +) +from testtools.content import ( + TracebackContent, +) + + +def _is_exception(exc): + return isinstance(exc, BaseException) + + +def _is_user_exception(exc): + return isinstance(exc, Exception) + + +class MatchesExceptionType(Matcher): + """ + Match an exc_info tuple against an exception type. + """ + + def __init__(self, exception_type): + """ + Create a MatchesException that will match exc_info's for exception. + + :param exception: An exception type. + """ + Matcher.__init__(self) + self.expected = exception_type + + def match(self, other): + if type(other) != tuple: + return Mismatch('{!r} is not an exc_info tuple'.format(other)) + expected_class = self.expected + etype, evalue, etb = other + if not issubclass(etype, expected_class): + return Mismatch( + "{} is an instance of {}, expected an instance of {}.".format( + evalue, + etype, + expected_class, + ), + dict( + traceback=TracebackContent(other, None), + ), + ) + + def __str__(self): + return "MatchesExceptionType({!r})".format(self.expected) + + +class Raises(Matcher): + """Match if the matchee raises an exception when called. + + Exceptions which are not subclasses of Exception propogate out of the + Raises.match call unless they are explicitly matched. + """ + + def __init__(self, exception_matcher): + """ + Create a Raises matcher. + + :param exception_matcher: Validator for the exception raised by + matchee. The exc_info tuple for the exception raised is passed + into that matcher. + """ + self.exception_matcher = exception_matcher + + def match(self, matchee): + try: + result = matchee() + return Mismatch('%r returned %r' % (matchee, result)) + # Catch all exceptions: Raises() should be able to match a + # KeyboardInterrupt or SystemExit. + except: + exc_info = sys.exc_info() + mismatch = self.exception_matcher.match(exc_info) + exc_type = exc_info[1] + # It's safer not to keep the traceback around. + del exc_info + if mismatch: + # The exception did not match, or no explicit matching logic was + # performed. If the exception is a non-user exception then + # propagate it. + if _is_exception(exc_type) and not _is_user_exception(exc_type): + raise + return mismatch + return None + + def __str__(self): + return 'Raises()' + + +def raises(exception_type): + """Make a matcher that checks that a callable raises an exception. + + This is a convenience function, exactly equivalent to:: + + return Raises(MatchesExceptionType(exception_type)) + + See `Raises` and `MatchesExceptionType` for more information. + """ + return Raises(MatchesExceptionType(exception_type)) diff --git a/src/_zkapauthorizer/tests/foolscap.py b/src/_zkapauthorizer/tests/foolscap.py new file mode 100644 index 0000000000000000000000000000000000000000..c9bb485a927b8884b131c3b3e4f6c36fe264aadf --- /dev/null +++ b/src/_zkapauthorizer/tests/foolscap.py @@ -0,0 +1,120 @@ +# Copyright 2019 PrivateStorage.io, LLC +# +# 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. + +""" +Testing helpers related to Foolscap. +""" + +from __future__ import ( + absolute_import, +) + +from zope.interface import ( + implementer, +) + +import attr + +from twisted.internet.defer import ( + execute, +) + +from foolscap.api import ( + RemoteInterface, +) + +from allmydata.interfaces import ( + RIStorageServer, +) + +class RIStub(RemoteInterface): + pass + +@implementer(RIStorageServer) +class StubStorageServer(object): + pass + + +def get_anonymous_storage_server(): + return StubStorageServer() + + +@attr.s +class DummyReferenceable(object): + _interface = attr.ib() + + def getInterface(self): + return self._interface + + def doRemoteCall(self, *a, **kw): + return None + +@attr.s +class LocalTracker(object): + """ + Pretend to be a tracker for a ``LocalRemote``. + """ + interface = attr.ib() + interfaceName = attr.ib(default=None) + + def __attrs_post_init__(self): + self.interfaceName = self.interface.__remote_name__ + + def getURL(self): + return b"pb://abcd@127.0.0.1:12345/efgh" + + +@attr.s +class LocalRemote(object): + """ + Adapt a referenceable to behave as if it were a remote reference instead. + + This is only a partial implementation of ``IRemoteReference`` so it + doesn't declare the interface. + + ``foolscap.referenceable.LocalReferenceable`` is in many ways a better + adapter between these interfaces but it also uses ``eventually`` which + complicates matters immensely for testing. + + :ivar foolscap.ipb.IReferenceable _referenceable: The object to which this + provides a simulated remote interface. + """ + _referenceable = attr.ib() + check_args = attr.ib(default=True) + tracker = attr.ib(default=None) + + def __attrs_post_init__(self): + self.tracker = LocalTracker( + self._referenceable.getInterface(), + ) + + def callRemote(self, methname, *args, **kwargs): + """ + Call the given method on the wrapped object, passing the given arguments. + + Arguments are checked for conformance to the remote interface but the + return value is not (because I don't know how -exarkun). + + :return Deferred: The result of the call on the wrapped object. + """ + schema = self._referenceable.getInterface()[methname] + if self.check_args: + schema.checkAllArgs(args, kwargs, inbound=False) + # TODO: Figure out how to call checkResults on the result. + return execute( + self._referenceable.doRemoteCall, + methname, + args, + kwargs, + ) diff --git a/src/_zkapauthorizer/tests/matchers.py b/src/_zkapauthorizer/tests/matchers.py index b3730b75554bac25fa5432cb39f82a97da413cb9..09afa8dad01347c3ae08536525fcfca134790728 100644 --- a/src/_zkapauthorizer/tests/matchers.py +++ b/src/_zkapauthorizer/tests/matchers.py @@ -16,6 +16,15 @@ Testtools matchers useful for the test suite. """ +__all__ = [ + "Provides", + "raises", + "returns", + "matches_version_dictionary", + "between", + "leases_current", +] + from datetime import ( datetime, ) @@ -36,6 +45,10 @@ from testtools.matchers import ( AllMatch, ) +from ._exception import ( + raises, +) + @attr.s class Provides(object): """ diff --git a/src/_zkapauthorizer/tests/test_plugin.py b/src/_zkapauthorizer/tests/test_plugin.py index 55c30a5d2205b5427300359c427f33c95482dca1..e9c8eecc3ce3290ca8bafb2f286b600442f47c6a 100644 --- a/src/_zkapauthorizer/tests/test_plugin.py +++ b/src/_zkapauthorizer/tests/test_plugin.py @@ -27,8 +27,8 @@ from os import ( makedirs, ) import tempfile -from zope.interface import ( - implementer, +from functools import ( + partial, ) from fixtures import ( @@ -96,12 +96,19 @@ from twisted.plugins.zkapauthorizer import ( storage_server, ) +from ..foolscap import ( + RIPrivacyPassAuthorizedStorageServer, +) from ..model import ( VoucherStore, ) from ..controller import ( IssuerConfigurationMismatch, ) +from .._storage_client import ( + IncorrectStorageServerReference, +) + from ..lease_maintenance import ( SERVICE_NAME, ) @@ -123,23 +130,25 @@ from .strategies import ( ) from .matchers import ( Provides, + raises, ) - -SIGNING_KEY_PATH = FilePath(__file__).sibling(u"testing-signing.key") +from .foolscap import ( + LocalRemote, + get_anonymous_storage_server, + DummyReferenceable, +) -@implementer(RIStorageServer) -class StubStorageServer(object): - pass +SIGNING_KEY_PATH = FilePath(__file__).sibling(u"testing-signing.key") -def get_anonymous_storage_server(): - return StubStorageServer() +def get_rref(interface=None): + if interface is None: + interface = RIPrivacyPassAuthorizedStorageServer + return LocalRemote(DummyReferenceable(interface)) -def get_rref(): - return LocalReferenceable(None) class PluginTests(TestCase): @@ -319,6 +328,59 @@ class ClientPluginTests(TestCase): self.fail("get_storage_client didn't raise, returned: {}".format(result)) + @given( + tahoe_configs(), + announcements(), + storage_indexes(), + lease_renew_secrets(), + lease_cancel_secrets(), + sharenum_sets(), + sizes(), + ) + def test_mismatch_storage_server_furl( + self, + get_config, + announcement, + storage_index, + renew_secret, + cancel_secret, + sharenums, + size, + + ): + """ + If the ``get_rref`` passed to ``get_storage_client`` returns a reference + to something other than an ``RIPrivacyPassAuthorizedStorageServer`` + provider then the storage methods of the client raise exceptions that + clearly indicate this. + """ + tempdir = self.useFixture(TempDir()) + node_config = get_config( + tempdir.join(b"node"), + b"tub.port", + ) + + storage_client = storage_server.get_storage_client( + node_config, + announcement, + partial(get_rref, RIStorageServer), + ) + + def use_it(): + return storage_client.allocate_buckets( + storage_index, + renew_secret, + cancel_secret, + sharenums, + size, + LocalReferenceable(None), + ) + + self.assertThat( + use_it, + raises(IncorrectStorageServerReference), + ) + @given( tahoe_configs_with_dummy_redeemer, datetimes(), diff --git a/src/_zkapauthorizer/tests/test_storage_protocol.py b/src/_zkapauthorizer/tests/test_storage_protocol.py index faa4f8f76517062fbc1cea13279f830f73cd373c..7d9de02808dde0a547ded59ad4cd3592ff5fdcb1 100644 --- a/src/_zkapauthorizer/tests/test_storage_protocol.py +++ b/src/_zkapauthorizer/tests/test_storage_protocol.py @@ -20,8 +20,6 @@ from __future__ import ( absolute_import, ) -import attr - from fixtures import ( MonkeyPatch, ) @@ -59,9 +57,6 @@ from hypothesis.strategies import ( from twisted.python.filepath import ( FilePath, ) -from twisted.internet.defer import ( - execute, -) from foolscap.referenceable import ( LocalReferenceable, @@ -98,6 +93,9 @@ from .storage_common import ( cleanup_storage_server, write_toy_shares, ) +from .foolscap import ( + LocalRemote, +) from ..api import ( ZKAPAuthorizerStorageServer, ZKAPAuthorizerStorageClient, @@ -114,44 +112,6 @@ from ..foolscap import ( ShareStat, ) -@attr.s -class LocalRemote(object): - """ - Adapt a referenceable to behave as if it were a remote reference instead. - - This is only a partial implementation of ``IRemoteReference`` so it - doesn't declare the interface. - - ``foolscap.referenceable.LocalReferenceable`` is in many ways a better - adapter between these interfaces but it also uses ``eventually`` which - complicates matters immensely for testing. - - :ivar foolscap.ipb.IReferenceable _referenceable: The object to which this - provides a simulated remote interface. - """ - _referenceable = attr.ib() - check_args = attr.ib(default=True) - - def callRemote(self, methname, *args, **kwargs): - """ - Call the given method on the wrapped object, passing the given arguments. - - Arguments are checked for conformance to the remote interface but the - return value is not (because I don't know how -exarkun). - - :return Deferred: The result of the call on the wrapped object. - """ - schema = self._referenceable.getInterface()[methname] - if self.check_args: - schema.checkAllArgs(args, kwargs, inbound=False) - # TODO: Figure out how to call checkResults on the result. - return execute( - self._referenceable.doRemoteCall, - methname, - args, - kwargs, - ) - class RequiredPassesTests(TestCase): """