🎨 Refactor Multitenant Manager errors and exception handling (#3323)

* 🚚 move `MultitenantManagerError` definition to error module

Signed-off-by: ff137 <ff137@proton.me>

* ✨ implement `InvalidTokenError` and handle unauthorized attempts

Signed-off-by: ff137 <ff137@proton.me>

* ✨ implement `MissingProfileError`

Signed-off-by: ff137 <ff137@proton.me>

* ✨ implement `WalletAlreadyExistsError`

Signed-off-by: ff137 <ff137@proton.me>

* 🎨

Signed-off-by: ff137 <ff137@proton.me>

* ✨ collectively handle the remaining `MultitenantManagerError`s: `MissingProfileError`, `WalletAlreadyExistsError`, `WalletKeyMissingError`

Signed-off-by: ff137 <ff137@proton.me>

* 🎨 extend `MultitenantManagerError` in `WalletKeyMissingError` and set default message

Signed-off-by: ff137 <ff137@proton.me>

* ✨ group HTTPBadRequest in error handling with MultitenantManagerError

Signed-off-by: ff137 <ff137@proton.me>

* 🎨 re-add old todo from d90c141

Signed-off-by: ff137 <ff137@proton.me>

---------

Signed-off-by: ff137 <ff137@proton.me>

acapy_agent/admin/server.py

@@ -22,7 +22,8 @@
 from ..core.profile import Profile
 from ..ledger.error import LedgerConfigError, LedgerTransactionError
 from ..messaging.responder import BaseResponder
-from ..multitenant.base import BaseMultitenantManager, MultitenantManagerError
+from ..multitenant.base import BaseMultitenantManager
+from ..multitenant.error import InvalidTokenError, MultitenantManagerError
 from ..storage.base import BaseStorage
 from ..storage.error import StorageNotFoundError
 from ..storage.type import RECORD_TYPE_ACAPY_UPGRADING
@@ -149,6 +150,14 @@ async def ready_middleware(request: web.BaseRequest, handler: Coroutine):
             # redirect, typically / -> /api/doc
             LOGGER.info("Handler redirect to: %s", e.location)
             raise
+        except (web.HTTPUnauthorized, jwt.InvalidTokenError, InvalidTokenError) as e:
+            LOGGER.info(
+                "Unauthorized access during %s %s: %s", request.method, request.path, e
+            )
+            raise web.HTTPUnauthorized(reason=str(e)) from e
+        except (web.HTTPBadRequest, MultitenantManagerError) as e:
+            LOGGER.info("Bad request during %s %s: %s", request.method, request.path, e)
+            raise web.HTTPBadRequest(reason=str(e)) from e
         except asyncio.CancelledError:
             # redirection spawns new task and cancels old
             LOGGER.debug("Task cancelled")

acapy_agent/multitenant/admin/routes.py

@@ -221,6 +221,7 @@ class CreateWalletRequestSchema(OpenAPISchema):
 
     key_management_mode = fields.Str(
         dump_default=WalletRecord.MODE_MANAGED,
+        # MTODO: add unmanaged mode once implemented
         validate=validate.OneOf((WalletRecord.MODE_MANAGED,)),
         metadata={
             "description": "Key management method to use for this wallet.",

acapy_agent/multitenant/admin/tests/test_routes.py

@@ -11,7 +11,8 @@
 from ....messaging.models.base import BaseModelError
 from ....storage.error import StorageError, StorageNotFoundError
 from ....wallet.models.wallet_record import WalletRecord
-from ...base import BaseMultitenantManager, MultitenantManagerError
+from ...base import BaseMultitenantManager
+from ...error import MultitenantManagerError
 from .. import routes as test_module
 
 

acapy_agent/multitenant/base.py

@@ -8,7 +8,6 @@
 import jwt
 
 from ..config.injection_context import InjectionContext
-from ..core.error import BaseError
 from ..core.profile import Profile, ProfileSession
 from ..protocols.coordinate_mediation.v1_0.manager import (
     MediationManager,
@@ -21,27 +20,28 @@
 from ..transport.wire_format import BaseWireFormat
 from ..wallet.base import BaseWallet
 from ..wallet.models.wallet_record import WalletRecord
-from .error import WalletKeyMissingError
+from .error import (
+    InvalidTokenError,
+    MissingProfileError,
+    WalletAlreadyExistsError,
+    WalletKeyMissingError,
+)
 
 LOGGER = logging.getLogger(__name__)
 
 
-class MultitenantManagerError(BaseError):
-    """Generic multitenant error."""
-
-
 class BaseMultitenantManager(ABC):
     """Base class for handling multitenancy."""
 
-    def __init__(self, profile: Profile):
+    def __init__(self, profile: Optional[Profile]):
         """Initialize base multitenant Manager.
 
         Args:
             profile: The profile for this manager
         """
-        self._profile = profile
         if not profile:
-            raise MultitenantManagerError("Missing profile")
+            raise MissingProfileError()
+        self._profile = profile
 
     @property
     @abstractmethod
@@ -93,6 +93,7 @@ def get_webhook_urls(
         Args:
             base_context: Base context to get base_webhook_urls
             wallet_record: Wallet record to get dispatch_type and webhook_urls
+
         Returns:
             webhook urls according to dispatch_type
         """
@@ -156,7 +157,7 @@ async def create_wallet(
                 to not store the wallet key, or "managed" to store the wallet key
 
         Raises:
-            MultitenantManagerError: If the wallet name already exists
+            WalletAlreadyExistsError: If the wallet name already exists
 
         Returns:
             WalletRecord: The newly created wallet record
@@ -169,9 +170,7 @@ async def create_wallet(
         async with self._profile.session() as session:
             # Check if the wallet name already exists to avoid indy wallet errors
             if wallet_name and await self._wallet_name_exists(session, wallet_name):
-                raise MultitenantManagerError(
-                    f"Wallet with name {wallet_name} already exists"
-                )
+                raise WalletAlreadyExistsError(wallet_name)
 
             # In unmanaged mode we don't want to store the wallet key
             if key_management_mode == WalletRecord.MODE_UNMANAGED:
@@ -254,7 +253,7 @@ async def remove_wallet(self, wallet_id: str, wallet_key: Optional[str] = None):
 
         wallet_key = wallet_key or wallet.wallet_key
         if wallet.requires_external_key and not wallet_key:
-            raise WalletKeyMissingError("Missing key to open wallet")
+            raise WalletKeyMissingError()
 
         profile = await self.get_wallet_profile(
             self._profile.context,
@@ -353,7 +352,8 @@ async def get_profile_for_token(
 
         Raises:
             WalletKeyMissingError: If the wallet_key is missing for an unmanaged wallet
-            InvalidTokenError: If there is an exception while decoding the token
+            InvalidTokenError: If the decoded token is invalid
+            jwt.InvalidTokenError: If there is an exception while decoding the token
 
         Returns:
             Profile associated with the token
@@ -378,7 +378,7 @@ async def get_profile_for_token(
             extra_settings["wallet.key"] = wallet_key
 
         if wallet.jwt_iat and wallet.jwt_iat != iat:
-            raise MultitenantManagerError("Token not valid")
+            raise InvalidTokenError()
 
         profile = await self.get_wallet_profile(context, wallet, extra_settings)
 

acapy_agent/multitenant/error.py

@@ -3,5 +3,38 @@
 from ..core.error import BaseError
 
 
-class WalletKeyMissingError(BaseError):
+class MultitenantManagerError(BaseError):
+    """Generic multitenant error."""
+
+
+class InvalidTokenError(MultitenantManagerError):
+    """Exception raised for invalid tokens."""
+
+    def __init__(self, message: str = "Token not valid"):
+        """Initialize an instance of InvalidTokenError."""
+        super().__init__(message)
+
+
+class MissingProfileError(MultitenantManagerError):
+    """Exception raised when a profile is missing."""
+
+    def __init__(self, message: str = "Missing profile"):
+        """Initialize an instance of MissingProfileError."""
+        super().__init__(message)
+
+
+class WalletAlreadyExistsError(MultitenantManagerError):
+    """Exception raised when a wallet already exists."""
+
+    def __init__(self, wallet_name: str):
+        """Initialize an instance of WalletAlreadyExistsError."""
+        message = f"Wallet with name {wallet_name} already exists"
+        super().__init__(message)
+
+
+class WalletKeyMissingError(MultitenantManagerError):
     """Wallet key missing exception."""
+
+    def __init__(self, message: str = "Missing key to open wallet"):
+        """Initialize an instance of WalletKeyMissingError."""
+        super().__init__(message)

acapy_agent/multitenant/tests/test_base.py

@@ -22,8 +22,8 @@
 from ...wallet.key_type import ED25519
 from ...wallet.models.wallet_record import WalletRecord
 from .. import base as test_module
-from ..base import BaseMultitenantManager, MultitenantManagerError
-from ..error import WalletKeyMissingError
+from ..base import BaseMultitenantManager
+from ..error import MultitenantManagerError, WalletKeyMissingError
 
 
 class MockMultitenantManager(BaseMultitenantManager):