Multi-tenancy stale wallet clean up

aries_cloudagent/admin/server.py

@@ -7,6 +7,7 @@
 from typing import Callable, Coroutine
 import uuid
 import warnings
+import weakref
 
 from aiohttp import web
 from aiohttp_apispec import (
@@ -115,7 +116,11 @@ def __init__(
 
         """
         super().__init__(**kwargs)
-        self._profile = profile
+        # Weakly hold the profile so this reference doesn't prevent profiles
+        # from being cleaned up when appropriate.
+        # Binding this AdminResponder to the profile's context creates a circular
+        # reference.
+        self._profile = weakref.ref(profile)
         self._send = send
 
     async def send_outbound(self, message: OutboundMessage) -> OutboundSendStatus:
@@ -125,7 +130,10 @@ async def send_outbound(self, message: OutboundMessage) -> OutboundSendStatus:
         Args:
             message: The `OutboundMessage` to be sent
         """
-        return await self._send(self._profile, message)
+        profile = self._profile()
+        if not profile:
+            raise RuntimeError("weakref to profile has expired")
+        return await self._send(profile, message)
 
     async def send_webhook(self, topic: str, payload: dict):
         """
@@ -139,7 +147,10 @@ async def send_webhook(self, topic: str, payload: dict):
             "responder.send_webhook is deprecated; please use the event bus instead.",
             DeprecationWarning,
         )
-        await self._profile.notify("acapy::webhook::" + topic, payload)
+        profile = self._profile()
+        if not profile:
+            raise RuntimeError("weakref to profile has expired")
+        await profile.notify("acapy::webhook::" + topic, payload)
 
     @property
     def send_fn(self) -> Coroutine:

aries_cloudagent/admin/tests/test_admin_server.py

@@ -198,10 +198,9 @@ async def test_import_routes_multitenant_middleware(self):
         context = InjectionContext()
         context.injector.bind_instance(ProtocolRegistry, ProtocolRegistry())
         context.injector.bind_instance(GoalCodeRegistry, GoalCodeRegistry())
-        profile = InMemoryProfile.test_profile()
         context.injector.bind_instance(
             test_module.BaseMultitenantManager,
-            test_module.BaseMultitenantManager(profile),
+            async_mock.MagicMock(spec=test_module.BaseMultitenantManager),
         )
         await DefaultContextBuilder().load_plugins(context)
         server = self.get_admin_server(
@@ -486,3 +485,17 @@ async def test_on_record_event(server, event_topic, webhook_topic):
     ) as mock_send_webhook:
         await server._on_record_event(profile, Event(event_topic, None))
         mock_send_webhook.assert_called_once_with(profile, webhook_topic, None)
+
+
+@pytest.mark.asyncio
+async def test_admin_responder_profile_expired_x():
+    def _smaller_scope():
+        profile = InMemoryProfile.test_profile()
+        return test_module.AdminResponder(profile, None)
+
+    responder = _smaller_scope()
+    with pytest.raises(RuntimeError):
+        await responder.send_outbound(None)
+
+    with pytest.raises(RuntimeError):
+        await responder.send_webhook("test", {})

aries_cloudagent/askar/profile.py

@@ -36,11 +36,18 @@ class AskarProfile(Profile):
 
     BACKEND_NAME = "askar"
 
-    def __init__(self, opened: AskarOpenStore, context: InjectionContext = None):
+    def __init__(
+        self,
+        opened: AskarOpenStore,
+        context: InjectionContext = None,
+        *,
+        profile_id: str = None
+    ):
         """Create a new AskarProfile instance."""
         super().__init__(context=context, name=opened.name, created=opened.created)
         self.opened = opened
         self.ledger_pool: IndyVdrLedgerPool = None
+        self.profile_id = profile_id
         self.init_ledger_pool()
         self.bind_providers()
 
@@ -56,8 +63,8 @@ def store(self) -> Store:
 
     async def remove(self):
         """Remove the profile."""
-        if self.settings.get("multitenant.wallet_type") == "askar-profile":
-            await self.store.remove_profile(self.settings.get("wallet.askar_profile"))
+        if self.profile_id:
+            await self.store.remove_profile(self.profile_id)
 
     def init_ledger_pool(self):
         """Initialize the ledger pool."""
@@ -160,11 +167,10 @@ def __init__(
     ):
         """Create a new IndySdkProfileSession instance."""
         super().__init__(profile=profile, context=context, settings=settings)
-        profile_id = profile.context.settings.get("wallet.askar_profile")
         if is_txn:
-            self._opener = self.profile.store.transaction(profile_id)
+            self._opener = self.profile.store.transaction(profile.profile_id)
         else:
-            self._opener = self.profile.store.session(profile_id)
+            self._opener = self.profile.store.session(profile.profile_id)
         self._handle: Session = None
         self._acquire_start: float = None
         self._acquire_end: float = None

aries_cloudagent/askar/tests/test_profile.py

@@ -1,87 +1,88 @@
 import asyncio
+import logging
 import pytest
 
-from asynctest import TestCase as AsyncTestCase, mock
+from asynctest import mock
 
 from ...askar.profile import AskarProfile
 from ...config.injection_context import InjectionContext
 
 from .. import profile as test_module
 
 
-class TestProfile(AsyncTestCase):
-    @mock.patch("aries_cloudagent.askar.store.AskarOpenStore")
-    async def test_init_success(self, AskarOpenStore):
-        askar_profile = AskarProfile(
-            AskarOpenStore,
-        )
-
-        assert askar_profile.opened == AskarOpenStore
-
-    @mock.patch("aries_cloudagent.askar.store.AskarOpenStore")
-    async def test_remove_success(self, AskarOpenStore):
-        openStore = AskarOpenStore
-        context = InjectionContext()
-        profile_id = "profile_id"
-        context.settings = {
-            "multitenant.wallet_type": "askar-profile",
-            "wallet.askar_profile": profile_id,
-            "ledger.genesis_transactions": mock.MagicMock(),
-        }
-        askar_profile = AskarProfile(openStore, context)
-        remove_profile_stub = asyncio.Future()
-        remove_profile_stub.set_result(True)
-        openStore.store.remove_profile.return_value = remove_profile_stub
-
-        await askar_profile.remove()
-
-        openStore.store.remove_profile.assert_called_once_with(profile_id)
-
-    @mock.patch("aries_cloudagent.askar.store.AskarOpenStore")
-    async def test_remove_profile_not_removed_if_wallet_type_not_askar_profile(
-        self, AskarOpenStore
-    ):
-        openStore = AskarOpenStore
-        context = InjectionContext()
-        context.settings = {"multitenant.wallet_type": "basic"}
-        askar_profile = AskarProfile(openStore, context)
-
-        await askar_profile.remove()
-
-        openStore.store.remove_profile.assert_not_called()
-
-    @pytest.mark.asyncio
-    async def test_profile_manager_transaction(self):
-        profile = "profileId"
-
-        with mock.patch("aries_cloudagent.askar.profile.AskarProfile") as AskarProfile:
-            askar_profile = AskarProfile(None, True)
-            askar_profile_transaction = mock.MagicMock()
-            askar_profile.store.transaction.return_value = askar_profile_transaction
-            askar_profile.context.settings.get.return_value = profile
-
-            transactionProfile = test_module.AskarProfileSession(askar_profile, True)
-
-            assert transactionProfile._opener == askar_profile_transaction
-            askar_profile.context.settings.get.assert_called_once_with(
-                "wallet.askar_profile"
-            )
-            askar_profile.store.transaction.assert_called_once_with(profile)
-
-    @pytest.mark.asyncio
-    async def test_profile_manager_store(self):
-        profile = "profileId"
-
-        with mock.patch("aries_cloudagent.askar.profile.AskarProfile") as AskarProfile:
-            askar_profile = AskarProfile(None, False)
-            askar_profile_session = mock.MagicMock()
-            askar_profile.store.session.return_value = askar_profile_session
-            askar_profile.context.settings.get.return_value = profile
-
-            sessionProfile = test_module.AskarProfileSession(askar_profile, False)
-
-            assert sessionProfile._opener == askar_profile_session
-            askar_profile.context.settings.get.assert_called_once_with(
-                "wallet.askar_profile"
-            )
-            askar_profile.store.session.assert_called_once_with(profile)
+@pytest.fixture
+def open_store():
+    yield mock.MagicMock()
+
+
+@pytest.mark.asyncio
+async def test_init_success(open_store):
+    askar_profile = AskarProfile(
+        open_store,
+    )
+
+    assert askar_profile.opened == open_store
+
+
+@pytest.mark.asyncio
+async def test_remove_success(open_store):
+    openStore = open_store
+    context = InjectionContext()
+    profile_id = "profile_id"
+    context.settings = {
+        "multitenant.wallet_type": "askar-profile",
+        "wallet.askar_profile": profile_id,
+        "ledger.genesis_transactions": mock.MagicMock(),
+    }
+    askar_profile = AskarProfile(openStore, context, profile_id=profile_id)
+    remove_profile_stub = asyncio.Future()
+    remove_profile_stub.set_result(True)
+    openStore.store.remove_profile.return_value = remove_profile_stub
+
+    await askar_profile.remove()
+
+    openStore.store.remove_profile.assert_called_once_with(profile_id)
+
+
+@pytest.mark.asyncio
+async def test_remove_profile_not_removed_if_wallet_type_not_askar_profile(open_store):
+    openStore = open_store
+    context = InjectionContext()
+    context.settings = {"multitenant.wallet_type": "basic"}
+    askar_profile = AskarProfile(openStore, context)
+
+    await askar_profile.remove()
+
+    openStore.store.remove_profile.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_profile_manager_transaction():
+    profile = "profileId"
+
+    with mock.patch("aries_cloudagent.askar.profile.AskarProfile") as AskarProfile:
+        askar_profile = AskarProfile(None, True, profile_id=profile)
+        askar_profile.profile_id = profile
+        askar_profile_transaction = mock.MagicMock()
+        askar_profile.store.transaction.return_value = askar_profile_transaction
+
+        transactionProfile = test_module.AskarProfileSession(askar_profile, True)
+
+        assert transactionProfile._opener == askar_profile_transaction
+        askar_profile.store.transaction.assert_called_once_with(profile)
+
+
+@pytest.mark.asyncio
+async def test_profile_manager_store():
+    profile = "profileId"
+
+    with mock.patch("aries_cloudagent.askar.profile.AskarProfile") as AskarProfile:
+        askar_profile = AskarProfile(None, False, profile_id=profile)
+        askar_profile.profile_id = profile
+        askar_profile_session = mock.MagicMock()
+        askar_profile.store.session.return_value = askar_profile_session
+
+        sessionProfile = test_module.AskarProfileSession(askar_profile, False)
+
+        assert sessionProfile._opener == askar_profile_session
+        askar_profile.store.session.assert_called_once_with(profile)

aries_cloudagent/config/argparse.py

@@ -1620,13 +1620,15 @@ def add_arguments(self, parser: ArgumentParser):
         parser.add_argument(
             "--multitenancy-config",
             type=str,
-            metavar="<multitenancy-config>",
+            nargs="+",
+            metavar="key=value",
             env_var="ACAPY_MULTITENANCY_CONFIGURATION",
             help=(
-                'Specify multitenancy configuration ("wallet_type" and "wallet_name"). '
-                'For example: "{"wallet_type":"askar-profile","wallet_name":'
-                '"askar-profile-name", "key_derivation_method":"RAW"}"'
-                '"wallet_name" is only used when "wallet_type" is "askar-profile"'
+                "Specify multitenancy configuration in key=value pairs. "
+                'For example: "wallet_type=askar-profile wallet_name=askar-profile-name" '
+                "Possible values: wallet_name, wallet_key, cache_size, "
+                'key_derivation_method. "wallet_name" is only used when '
+                '"wallet_type" is "askar-profile"'
             ),
         )
 
@@ -1647,22 +1649,37 @@ def get_settings(self, args: Namespace):
                 settings["multitenant.admin_enabled"] = True
 
             if args.multitenancy_config:
-                multitenancyConfig = json.loads(args.multitenancy_config)
-
-                if multitenancyConfig.get("wallet_type"):
-                    settings["multitenant.wallet_type"] = multitenancyConfig.get(
-                        "wallet_type"
-                    )
-
-                if multitenancyConfig.get("wallet_name"):
-                    settings["multitenant.wallet_name"] = multitenancyConfig.get(
-                        "wallet_name"
-                    )
-
-                if multitenancyConfig.get("key_derivation_method"):
-                    settings[
-                        "multitenant.key_derivation_method"
-                    ] = multitenancyConfig.get("key_derivation_method")
+                # Legacy support
+                if (
+                    len(args.multitenancy_config) == 1
+                    and args.multitenancy_config[0][0] == "{"
+                ):
+                    multitenancy_config = json.loads(args.multitenancy_config[0])
+                    if multitenancy_config.get("wallet_type"):
+                        settings["multitenant.wallet_type"] = multitenancy_config.get(
+                            "wallet_type"
+                        )
+
+                    if multitenancy_config.get("wallet_name"):
+                        settings["multitenant.wallet_name"] = multitenancy_config.get(
+                            "wallet_name"
+                        )
+
+                    if multitenancy_config.get("cache_size"):
+                        settings["multitenant.cache_size"] = multitenancy_config.get(
+                            "cache_size"
+                        )
+
+                    if multitenancy_config.get("key_derivation_method"):
+                        settings[
+                            "multitenant.key_derivation_method"
+                        ] = multitenancy_config.get("key_derivation_method")
+
+                else:
+                    for value_str in args.multitenancy_config:
+                        key, value = value_str.split("=", maxsplit=1)
+                        value = yaml.safe_load(value)
+                        settings[f"multitenant.{key}"] = value
 
         return settings
 

aries_cloudagent/config/tests/test_argparse.py

@@ -231,7 +231,26 @@ async def test_multitenancy_settings(self):
                 "--jwt-secret",
                 "secret",
                 "--multitenancy-config",
-                '{"wallet_type":"askar","wallet_name":"test"}',
+                '{"wallet_type":"askar","wallet_name":"test", "cache_size": 10}',
+            ]
+        )
+
+        settings = group.get_settings(result)
+
+        assert settings.get("multitenant.enabled") == True
+        assert settings.get("multitenant.jwt_secret") == "secret"
+        assert settings.get("multitenant.wallet_type") == "askar"
+        assert settings.get("multitenant.wallet_name") == "test"
+
+        result = parser.parse_args(
+            [
+                "--multitenant",
+                "--jwt-secret",
+                "secret",
+                "--multitenancy-config",
+                "wallet_type=askar",
+                "wallet_name=test",
+                "cache_size=10",
             ]
         )
 

aries_cloudagent/core/conductor.py

@@ -488,7 +488,7 @@ async def stop(self, timeout=1.0):
         # close multitenant profiles
         multitenant_mgr = self.context.inject_or(BaseMultitenantManager)
         if multitenant_mgr:
-            for profile in multitenant_mgr._instances.values():
+            for profile in multitenant_mgr.open_profiles:
                 shutdown.run(profile.close())
 
         if self.root_profile: