Merge pull request #2683 from jamshale/feat/1487

Return 404 when schema not found

Author: swcurran

Diff for: aries_cloudagent/anoncreds/default/legacy_indy/registry.py

@@ -1,10 +1,12 @@
 """Legacy Indy Registry."""
-from asyncio import shield
 import json
 import logging
 import re
+from asyncio import shield
 from typing import List, Optional, Pattern, Sequence, Tuple
 
+from base58 import alphabet
+
 from ....cache.base import BaseCache
 from ....config.injection_context import InjectionContext
 from ....core.profile import Profile
@@ -40,14 +42,14 @@
     GetCredDefResult,
 )
 from ...models.anoncreds_revocation import (
-    GetRevRegDefResult,
     GetRevListResult,
-    RevRegDef,
-    RevRegDefResult,
-    RevRegDefState,
+    GetRevRegDefResult,
     RevList,
     RevListResult,
     RevListState,
+    RevRegDef,
+    RevRegDefResult,
+    RevRegDefState,
     RevRegDefValue,
 )
 from ...models.anoncreds_schema import (
@@ -56,7 +58,6 @@
     SchemaResult,
     SchemaState,
 )
-from base58 import alphabet
 
 LOGGER = logging.getLogger(__name__)
 
@@ -158,7 +159,7 @@ async def get_schema(self, profile: Profile, schema_id: str) -> GetSchemaResult:
                 schema = await ledger.get_schema(schema_id)
                 if schema is None:
                     raise AnonCredsObjectNotFound(
-                        f"Credential definition not found: {schema_id}",
+                        f"Schema not found: {schema_id}",
                         {"ledger_id": ledger_id},
                     )
 

Diff for: aries_cloudagent/anoncreds/routes.py

@@ -1,6 +1,6 @@
 """Anoncreds admin routes."""
-from asyncio import shield
 import logging
+from asyncio import shield
 
 from aiohttp import web
 from aiohttp_apispec import (
@@ -21,13 +21,13 @@
 from ..revocation_anoncreds.manager import RevocationManager, RevocationManagerError
 from ..revocation_anoncreds.routes import (
     PublishRevocationsSchema,
-    RevRegIdMatchInfoSchema,
     RevocationModuleResponseSchema,
     RevokeRequestSchema,
+    RevRegIdMatchInfoSchema,
     TxnOrPublishRevocationsResultSchema,
 )
 from ..storage.error import StorageError, StorageNotFoundError
-from .base import AnonCredsRegistrationError
+from .base import AnonCredsObjectNotFound, AnonCredsRegistrationError
 from .issuer import AnonCredsIssuer, AnonCredsIssuerError
 from .models.anoncreds_cred_def import CredDefResultSchema, GetCredDefResultSchema
 from .models.anoncreds_revocation import RevListResultSchema, RevRegDefResultSchema
@@ -186,9 +186,11 @@ async def schema_get(request: web.BaseRequest):
     context: AdminRequestContext = request["context"]
     anoncreds_registry = context.inject(AnonCredsRegistry)
     schema_id = request.match_info["schemaId"]
-    result = await anoncreds_registry.get_schema(context.profile, schema_id)
-
-    return web.json_response(result.serialize())
+    try:
+        schema = await anoncreds_registry.get_schema(context.profile, schema_id)
+        return web.json_response(schema.serialize())
+    except AnonCredsObjectNotFound:
+        raise web.HTTPNotFound(reason=f"Schema not found: {schema_id}")
 
 
 class SchemasQueryStringSchema(OpenAPISchema):

Diff for: aries_cloudagent/anoncreds/tests/test_routes.py

@@ -5,6 +5,7 @@
 from asynctest import TestCase as AsyncTestCase
 
 from aries_cloudagent.admin.request_context import AdminRequestContext
+from aries_cloudagent.anoncreds.base import AnonCredsObjectNotFound
 from aries_cloudagent.anoncreds.issuer import AnonCredsIssuer
 from aries_cloudagent.anoncreds.revocation import AnonCredsRevocation
 from aries_cloudagent.anoncreds.revocation_setup import DefaultRevocationSetup
@@ -95,16 +96,27 @@ async def test_get_schema(self):
         self.request.match_info = {"schemaId": "schema_id"}
         self.context.inject = mock.Mock(
             return_value=mock.MagicMock(
-                get_schema=mock.CoroutineMock(return_value=MockSchema("schemaId"))
+                get_schema=mock.CoroutineMock(
+                    side_effect=[
+                        MockSchema("schemaId"),
+                        AnonCredsObjectNotFound("test"),
+                    ]
+                )
             )
         )
         result = await test_module.schema_get(self.request)
         assert json.loads(result.body)["schema_id"] == "schemaId"
 
+        # missing schema_id
         self.request.match_info = {}
         with self.assertRaises(KeyError):
             await test_module.schema_get(self.request)
 
+        # schema not found
+        self.request.match_info = {"schemaId": "schema_id"}
+        with self.assertRaises(web.HTTPNotFound):
+            await test_module.schema_get(self.request)
+
     @mock.patch.object(
         AnonCredsIssuer,
         "get_created_schemas",

Diff for: aries_cloudagent/messaging/schemas/routes.py

@@ -12,7 +12,6 @@
     request_schema,
     response_schema,
 )
-
 from marshmallow import fields
 from marshmallow.validate import Regexp
 
@@ -397,6 +396,8 @@ async def schemas_get_schema(request: web.BaseRequest):
     async with ledger:
         try:
             schema = await ledger.get_schema(schema_id)
+            if not schema:
+                raise web.HTTPNotFound(reason=f"Schema not found: {schema_id}")
         except LedgerError as err:
             raise web.HTTPBadRequest(reason=err.roll_up) from err
 

Diff for: aries_cloudagent/messaging/schemas/tests/test_routes.py

@@ -1,7 +1,9 @@
 from unittest import IsolatedAsyncioTestCase
+
 from aries_cloudagent.tests import mock
 
 from ....admin.request_context import AdminRequestContext
+from ....connections.models.conn_record import ConnRecord
 from ....core.in_memory import InMemoryProfile
 from ....indy.issuer import IndyIssuer
 from ....ledger.base import BaseLedger
@@ -11,10 +13,7 @@
 from ....multitenant.base import BaseMultitenantManager
 from ....multitenant.manager import MultitenantManager
 from ....storage.base import BaseStorage
-
 from .. import routes as test_module
-from ....connections.models.conn_record import ConnRecord
-
 
 SCHEMA_ID = "WgWxqztrNooG92RXvxSTWv:2:schema_name:1.0"
 
@@ -304,6 +303,9 @@ async def test_created(self):
             mock_response.assert_called_once_with({"schema_ids": [SCHEMA_ID]})
 
     async def test_get_schema(self):
+        self.ledger.get_schema = mock.CoroutineMock(
+            side_effect=[{"schema": "def", "signed_txn": "..."}, None]
+        )
         self.profile_injector.bind_instance(
             IndyLedgerRequestsExecutor,
             mock.MagicMock(
@@ -323,6 +325,10 @@ async def test_get_schema(self):
                 }
             )
 
+        # test schema not found
+        with self.assertRaises(test_module.web.HTTPNotFound):
+            await test_module.schemas_get_schema(self.request)
+
     async def test_get_schema_multitenant(self):
         self.profile_injector.bind_instance(
             BaseMultitenantManager,