Change return type of QrCode query to better indicate errors

back/boxtribute_server/authz.py

@@ -12,6 +12,7 @@
 from .errors import InsufficientPermission, UnauthorizedForBase
 from .exceptions import Forbidden
 from .models.definitions.base import Base
+from .models.definitions.organisation import Organisation
 from .models.definitions.shipment import Shipment
 from .models.definitions.shipment_detail import ShipmentDetail
 from .models.definitions.transfer_agreement import TransferAgreement
@@ -67,7 +68,13 @@ def inner(*args, **kwargs):
             if e.permission is not None:
                 return InsufficientPermission(name=e.permission)
             elif e.resource == "base":
-                return UnauthorizedForBase(id=e.value, name="")
+                base = (
+                    Base.select(Base.id, Base.name, Organisation.name)
+                    .join(Organisation)
+                    .where(Base.id == e.value)
+                    .get_or_none()
+                )
+                return UnauthorizedForBase(id=e.value, base=base)
             raise e
 
     return inner

back/boxtribute_server/business_logic/warehouse/box/mutations.py

@@ -31,7 +31,7 @@
 
 
 @dataclass(kw_only=True)
-class BoxResult:
+class BoxesResult:
     updated_boxes: list[Box]
     invalid_box_label_identifiers: list[str]
 
@@ -89,7 +89,7 @@ def resolve_update_box(*_, update_input):
 # - perform the requested action on all valid boxes
 # - create list of invalid boxes (difference of the set of input label identifiers and
 #   the set of valid label identifiers)
-# - return valid, updated boxes and invalid box label identifiers as BoxResult data
+# - return valid, updated boxes and invalid box label identifiers as BoxesResult data
 #   structure for GraphQL API
 @mutation.field("deleteBoxes")
 @handle_unauthorized
@@ -107,7 +107,7 @@ def resolve_delete_boxes(*_, label_identifiers):
     )
     valid_box_label_identifiers = {box.label_identifier for box in boxes}
 
-    return BoxResult(
+    return BoxesResult(
         updated_boxes=delete_boxes(user_id=g.user.id, boxes=boxes),
         invalid_box_label_identifiers=sorted(
             label_identifiers.difference(valid_box_label_identifiers)
@@ -142,7 +142,7 @@ def resolve_move_boxes_to_location(*_, update_input):
     )
     valid_box_label_identifiers = {box.label_identifier for box in boxes}
 
-    return BoxResult(
+    return BoxesResult(
         updated_boxes=move_boxes_to_location(
             user_id=g.user.id, boxes=boxes, location=location
         ),
@@ -190,7 +190,7 @@ def resolve_assign_tag_to_boxes(*_, update_input):
     )
     valid_box_label_identifiers = {box.label_identifier for box in boxes}
 
-    return BoxResult(
+    return BoxesResult(
         updated_boxes=assign_tag_to_boxes(user_id=g.user.id, boxes=boxes, tag=tag),
         invalid_box_label_identifiers=sorted(
             label_identifiers.difference(valid_box_label_identifiers)
@@ -249,7 +249,7 @@ def resolve_unassign_tag_from_boxes(*_, update_input):
     )
     valid_box_label_identifiers = {box.label_identifier for box in boxes}
 
-    return BoxResult(
+    return BoxesResult(
         updated_boxes=unassign_tag_from_boxes(user_id=g.user.id, boxes=boxes, tag=tag),
         invalid_box_label_identifiers=sorted(
             label_identifiers.difference(valid_box_label_identifiers)

back/boxtribute_server/business_logic/warehouse/qr_code/fields.py

@@ -1,13 +1,14 @@
 from ariadne import ObjectType
 
-from ....authz import authorize_for_reading_box
+from ....authz import authorize_for_reading_box, handle_unauthorized
 from ....models.definitions.box import Box
 from ....models.definitions.location import Location
 
 qr_code = ObjectType("QrCode")
 
 
 @qr_code.field("box")
+@handle_unauthorized
 def resolve_qr_code_box(qr_code_obj, _):
     try:
         box = (

back/boxtribute_server/business_logic/warehouse/qr_code/queries.py

@@ -1,22 +1,25 @@
 from ariadne import QueryType
 
-from ....authz import authorize
+from ....authz import authorize, handle_unauthorized
 from ....models.definitions.qr_code import QrCode
+from ....models.utils import handle_non_existing_resource
 
 query = QueryType()
 
 
 @query.field("qrExists")
-def resolve_qr_exists(*_, qr_code):
+def resolve_qr_exists(*_, code):
     authorize(permission="qr:read")
     try:
-        QrCode.get_id_from_code(qr_code)
+        QrCode.get_id_from_code(code)
     except QrCode.DoesNotExist:
         return False
     return True
 
 
 @query.field("qrCode")
-def resolve_qr_code(*_, qr_code):
+@handle_unauthorized
+@handle_non_existing_resource
+def resolve_qr_code(*_, code):
     authorize(permission="qr:read")
-    return QrCode.get(QrCode.code == qr_code)
+    return QrCode.get(QrCode.code == code)

back/boxtribute_server/errors.py

@@ -26,9 +26,13 @@ def __init__(self, *, name):
 
 
 class UnauthorizedForBase(UserError):
-    def __init__(self, *, id, name):
+    def __init__(self, *, id, base):
         self.id = id
-        self.name = name
+        self.name = ""
+        self.organisation_name = ""
+        if base is not None:
+            self.name = base.name
+            self.organisation_name = base.organisation.name
 
 
 class BoxesStillAssignedToProduct(UserError):

back/boxtribute_server/graph_ql/bindables.py

@@ -180,6 +180,8 @@ def resolve_location_type(obj, *_):
     UnionType("MoveBoxesResult", resolve_type_by_class_name),
     UnionType("AssignTagToBoxesResult", resolve_type_by_class_name),
     UnionType("UnassignTagFromBoxesResult", resolve_type_by_class_name),
+    UnionType("QrCodeResult", resolve_type_by_class_name),
+    UnionType("BoxResult", resolve_type_by_class_name),
 )
 interface_types = (
     InterfaceType("Location", resolve_location_type),

back/boxtribute_server/graph_ql/definitions/protected/mutations.graphql

@@ -7,13 +7,13 @@ type Mutation {
   createBox(creationInput: BoxCreationInput): Box
   " Update one or more properties of a box with specified label identifier. "
   updateBox(updateInput: BoxUpdateInput): Box
-  " Any boxes that are non-existing, already deleted, and/or in a base that the user must not access are returned in the `BoxResult.invalidBoxLabelIdentifiers` list. "
+  " Any boxes that are non-existing, already deleted, and/or in a base that the user must not access are returned in the `BoxesResult.invalidBoxLabelIdentifiers` list. "
   deleteBoxes(labelIdentifiers: [String!]!): DeleteBoxesResult
-  " Any boxes that are non-existing, already inside the requested location, inside a different base other than the one of the requested location, and/or in a base that the user must not access are returned in the `BoxResult.invalidBoxLabelIdentifiers` list. "
+  " Any boxes that are non-existing, already inside the requested location, inside a different base other than the one of the requested location, and/or in a base that the user must not access are returned in the `BoxesResult.invalidBoxLabelIdentifiers` list. "
   moveBoxesToLocation(updateInput: BoxMoveInput): MoveBoxesResult
-  " Any boxes that are non-existing, already assigned to the requested tag, and/or in a base that the user must not access are returned in the `BoxResult.invalidBoxLabelIdentifiers` list. "
+  " Any boxes that are non-existing, already assigned to the requested tag, and/or in a base that the user must not access are returned in the `BoxesResult.invalidBoxLabelIdentifiers` list. "
   assignTagToBoxes(updateInput: BoxAssignTagInput): AssignTagToBoxesResult
-  " Any boxes that are non-existing, don't have the requested tag assigned, and/or in a base that the user must not access are returned in the `BoxResult.invalidBoxLabelIdentifiers` list. "
+  " Any boxes that are non-existing, don't have the requested tag assigned, and/or in a base that the user must not access are returned in the `BoxesResult.invalidBoxLabelIdentifiers` list. "
   unassignTagFromBoxes(updateInput: BoxAssignTagInput): UnassignTagFromBoxesResult
 
   " Create a new beneficiary in a base, using first/last name, date of birth, and group identifier. Optionally pass tags to assign to the beneficiary. "

back/boxtribute_server/graph_ql/definitions/protected/queries.graphql

@@ -15,9 +15,9 @@ type Query {
   box(labelIdentifier: String!): Box
   " Return page of non-deleted [`Boxes`]({{Types.Box}}) in base with specified ID. Optionally pass filters "
   boxes(baseId: ID!, paginationInput: PaginationInput, filterInput: FilterBoxInput): BoxPage!
-  " Return [`QrCode`]({{Types.QrCode}}) with specified code (an MD5 hash in hex format of length 32) "
-  qrCode(qrCode: String!): QrCode
-  qrExists(qrCode: String): Boolean
+  " Return [`QrCode`]({{Types.QrCode}}) with specified code (an MD5 hash in hex format of length 32), or an error in case of insufficient permission or missing resource. "
+  qrCode(code: String!): QrCodeResult!
+  qrExists(code: String): Boolean
   " Return [`ClassicLocation`]({{Types.ClassicLocation}}) with specified ID. Accessible for clients who are members of the location's base "
   location(id: ID!): ClassicLocation
   " Return all [`ClassicLocations`]({{Types.ClassicLocation}}) that the client is authorized to view. "

back/boxtribute_server/graph_ql/definitions/protected/types.graphql

@@ -66,8 +66,8 @@ Representation of a QR code, possibly associated with a [`Box`]({{Types.Box}}).
 type QrCode {
   id: ID!
   code: String!
-  " [`Box`]({{Types.Box}}) associated with the QR code (`null` if none associated) "
-  box: Box
+  " [`Box`]({{Types.Box}}) associated with the QR code (`null` if none associated), or an error in case of insufficient permission or missing authorization for box's base "
+  box: BoxResult
   createdOn: Datetime
 }
 
@@ -213,7 +213,7 @@ type BoxPage {
 """
 Utility response type for box bulk-update mutations, containing both updated boxes and invalid boxes (ignored due to e.g. being deleted, in prohibited base, and/or non-existing).
 """
-type BoxResult {
+type BoxesResult {
   updatedBoxes: [Box!]!
   invalidBoxLabelIdentifiers: [String!]!
 }
@@ -580,7 +580,10 @@ type ResourceDoesNotExistError {
 type UnauthorizedForBaseError {
   " e.g. 'product:write' present but not for requested base "
   id: ID!
+  " Empty string if base does not exist "
   name: String!
+  " Empty string if base does not exist "
+  organisationName: String!
 }
 type InvalidPriceError {
   value: Int!
@@ -608,18 +611,21 @@ type DeletedTagError {
   name: String!
 }
 
-union MoveBoxesResult = BoxResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | DeletedLocationError
-union AssignTagToBoxesResult = BoxResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | TagTypeMismatchError | DeletedTagError
-union UnassignTagFromBoxesResult = BoxResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError
-union DeleteBoxesResult = BoxResult | InsufficientPermissionError
+union MoveBoxesResult = BoxesResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | DeletedLocationError
+union AssignTagToBoxesResult = BoxesResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | TagTypeMismatchError | DeletedTagError
+union UnassignTagFromBoxesResult = BoxesResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError
+union DeleteBoxesResult = BoxesResult | InsufficientPermissionError
 union CreateCustomProductResult = Product | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | InvalidPriceError | EmptyNameError
 union EditCustomProductResult = Product | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | InvalidPriceError | EmptyNameError | ProductTypeMismatchError
 union DeleteProductResult = Product | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | BoxesStillAssignedToProductError | ProductTypeMismatchError
 union EnableStandardProductResult = Product | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | InvalidPriceError | StandardProductAlreadyEnabledForBaseError
 union EditStandardProductInstantiationResult = Product | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | InvalidPriceError | ProductTypeMismatchError
 union DisableStandardProductResult = Product | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | BoxesStillAssignedToProductError | ProductTypeMismatchError
+
 union StandardProductResult = StandardProduct | InsufficientPermissionError | ResourceDoesNotExistError
 union StandardProductsResult = StandardProductPage | InsufficientPermissionError | UnauthorizedForBaseError
+union QrCodeResult = QrCode | InsufficientPermissionError | ResourceDoesNotExistError
+union BoxResult = Box | InsufficientPermissionError | UnauthorizedForBaseError
 
 type Metrics {
   """

back/test/auth0_integration_tests/test_operations.py

@@ -15,8 +15,8 @@ def _assert_successful_request(*args, **kwargs):
         return assert_successful_request(*args, **kwargs, endpoint=endpoint)
 
     query = """query BoxIdAndItems {
-                qrCode(qrCode: "093f65e080a295f8076b1c5722a46aa2") { box { id } }
-            }"""
+                qrCode(code: "093f65e080a295f8076b1c5722a46aa2") {
+                    ...on QrCode { box { ...on Box { id } } } } }"""
     queried_box = _assert_successful_request(auth0_client, query)["box"]
     assert queried_box == {"id": "100000000"}
 

back/test/endpoint_tests/test_app.py

@@ -113,13 +113,6 @@ def test_query_non_existent_box(read_only_client):
     assert "SQL" not in response.json["errors"][0]["message"]
 
 
-def test_query_non_existent_qr_code(read_only_client):
-    # Test case 8.1.31
-    query = """query { qrCode(qrCode: "-1") { id } }"""
-    response = assert_bad_user_input(read_only_client, query)
-    assert "SQL" not in response.json["errors"][0]["message"]
-
-
 @pytest.mark.parametrize("resource", ["base", "organisation", "user"])
 def test_query_non_existent_resource_for_god_user(read_only_client, mocker, resource):
     # Test case 99.1.3, 10.1.3
@@ -229,8 +222,8 @@ def test_update_non_existent_resource(
             "createCustomProduct",
             """creationInput:
             { baseId: 0, name: "a", categoryId: 1, sizeRangeId: 1, gender: none}""",
-            "...on UnauthorizedForBaseError { id }",
-            {"id": "0"},
+            "...on UnauthorizedForBaseError { id name organisationName }",
+            {"id": "0", "name": "", "organisationName": ""},
         ],
         # Test case 8.2.37
         [
@@ -280,8 +273,8 @@ def test_update_non_existent_resource(
         [
             "enableStandardProduct",
             """enableInput: { baseId: 0, standardProductId: 2 }""",
-            "...on UnauthorizedForBaseError { id }",
-            {"id": "0"},
+            "...on UnauthorizedForBaseError { id name organisationName }",
+            {"id": "0", "name": "", "organisationName": ""},
         ],
         # Test case 8.2.63
         [
@@ -352,6 +345,13 @@ def test_mutate_resource_does_not_exist(
 @pytest.mark.parametrize(
     "operation,query_input,field,response",
     [
+        # Test case 8.1.31
+        [
+            "qrCode",
+            'code: "0"',
+            "...on ResourceDoesNotExistError { id name }",
+            {"id": None, "name": "QrCode"},
+        ],
         # Test case 8.1.42
         [
             "standardProduct",