diff --git a/back/boxtribute_server/authz.py b/back/boxtribute_server/authz.py index 1052eae1d..f4e71c286 100644 --- a/back/boxtribute_server/authz.py +++ b/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 diff --git a/back/boxtribute_server/business_logic/warehouse/box/mutations.py b/back/boxtribute_server/business_logic/warehouse/box/mutations.py index 67ee19691..38c9e96b6 100644 --- a/back/boxtribute_server/business_logic/warehouse/box/mutations.py +++ b/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) diff --git a/back/boxtribute_server/business_logic/warehouse/qr_code/fields.py b/back/boxtribute_server/business_logic/warehouse/qr_code/fields.py index a66cc0c43..b6a03eb73 100644 --- a/back/boxtribute_server/business_logic/warehouse/qr_code/fields.py +++ b/back/boxtribute_server/business_logic/warehouse/qr_code/fields.py @@ -1,6 +1,6 @@ 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 @@ -8,6 +8,7 @@ @qr_code.field("box") +@handle_unauthorized def resolve_qr_code_box(qr_code_obj, _): try: box = ( diff --git a/back/boxtribute_server/business_logic/warehouse/qr_code/queries.py b/back/boxtribute_server/business_logic/warehouse/qr_code/queries.py index fa808cc76..8d4909ca9 100644 --- a/back/boxtribute_server/business_logic/warehouse/qr_code/queries.py +++ b/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) diff --git a/back/boxtribute_server/errors.py b/back/boxtribute_server/errors.py index eb3ec3d2e..572d541fe 100644 --- a/back/boxtribute_server/errors.py +++ b/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): diff --git a/back/boxtribute_server/graph_ql/bindables.py b/back/boxtribute_server/graph_ql/bindables.py index da57ae017..3484f265f 100644 --- a/back/boxtribute_server/graph_ql/bindables.py +++ b/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), diff --git a/back/boxtribute_server/graph_ql/definitions/protected/mutations.graphql b/back/boxtribute_server/graph_ql/definitions/protected/mutations.graphql index 6196bbe87..7d7773466 100644 --- a/back/boxtribute_server/graph_ql/definitions/protected/mutations.graphql +++ b/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. " diff --git a/back/boxtribute_server/graph_ql/definitions/protected/queries.graphql b/back/boxtribute_server/graph_ql/definitions/protected/queries.graphql index 8dca5d779..9861065d8 100644 --- a/back/boxtribute_server/graph_ql/definitions/protected/queries.graphql +++ b/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. " diff --git a/back/boxtribute_server/graph_ql/definitions/protected/types.graphql b/back/boxtribute_server/graph_ql/definitions/protected/types.graphql index 1fc146371..1088097d8 100644 --- a/back/boxtribute_server/graph_ql/definitions/protected/types.graphql +++ b/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 { """ diff --git a/back/test/auth0_integration_tests/test_operations.py b/back/test/auth0_integration_tests/test_operations.py index 5da948e82..a008dc170 100644 --- a/back/test/auth0_integration_tests/test_operations.py +++ b/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"} diff --git a/back/test/endpoint_tests/test_app.py b/back/test/endpoint_tests/test_app.py index 298b1ac6c..ddd1d7621 100644 --- a/back/test/endpoint_tests/test_app.py +++ b/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", diff --git a/back/test/endpoint_tests/test_box.py b/back/test/endpoint_tests/test_box.py index a56cd4e5a..6c84f0be7 100644 --- a/back/test/endpoint_tests/test_box.py +++ b/back/test/endpoint_tests/test_box.py @@ -91,11 +91,8 @@ def test_box_query_by_label_identifier( def test_box_query_by_qr_code(read_only_client, default_box, default_qr_code): # Test case 8.1.5 query = f"""query {{ - qrCode(qrCode: "{default_qr_code['code']}") {{ - box {{ - labelIdentifier - }} - }} + qrCode(code: "{default_qr_code['code']}") {{ + ...on QrCode {{ box {{ ...on Box {{ labelIdentifier }} }} }} }} }}""" queried_box = assert_successful_request(read_only_client, query)["box"] assert queried_box["labelIdentifier"] == default_box["label_identifier"] @@ -315,7 +312,7 @@ def test_box_mutations( # Test case 8.2.22a, 8.2.22c mutation = f"""mutation {{ moveBoxesToLocation( updateInput: {{ labelIdentifiers: [{label_identifiers}], locationId: {location_id} }} ) {{ - ...on BoxResult {{ updatedBoxes {{ + ...on BoxesResult {{ updatedBoxes {{ location {{ id }} lastModifiedOn history {{ changes }} @@ -351,7 +348,7 @@ def test_box_mutations( # Test case 8.2.23a, 8.2.23c mutation = f"""mutation {{ assignTagToBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -363,7 +360,7 @@ def test_box_mutations( mutation = f"""mutation {{ assignTagToBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -376,7 +373,7 @@ def test_box_mutations( generic_tag_id = str(tags[2]["id"]) mutation = f"""mutation {{ assignTagToBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {generic_tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -392,7 +389,7 @@ def test_box_mutations( mutation = f"""mutation {{ assignTagToBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {another_generic_tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -415,7 +412,7 @@ def test_box_mutations( label_identifier = f'"{created_box["labelIdentifier"]}"' mutation = f"""mutation {{ unassignTagFromBoxes( updateInput: {{ labelIdentifiers: [{label_identifier}], tagId: {generic_tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -442,7 +439,7 @@ def test_box_mutations( # Test case 8.2.24c mutation = f"""mutation {{ unassignTagFromBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {generic_tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -463,7 +460,7 @@ def test_box_mutations( # Test case 8.2.24h mutation = f"""mutation {{ unassignTagFromBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {beneficiary_tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ id }} invalidBoxLabelIdentifiers }} }} }}""" @@ -485,7 +482,7 @@ def test_box_mutations( # Test case 8.2.24i mutation = f"""mutation {{ unassignTagFromBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {deleted_tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ id }} invalidBoxLabelIdentifiers }} }} }}""" @@ -497,7 +494,7 @@ def test_box_mutations( # Test case 8.2.25 mutation = f"""mutation {{ deleteBoxes( labelIdentifiers: [{label_identifiers}] ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ deletedOn history {{ changes }} @@ -521,7 +518,7 @@ def test_box_mutations( label_identifiers = ",".join(f'"{i}"' for i in raw_label_identifiers) mutation = f"""mutation {{ moveBoxesToLocation( updateInput: {{ labelIdentifiers: [{label_identifiers}], locationId: {location_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ id }} invalidBoxLabelIdentifiers }} }} }}""" @@ -534,7 +531,7 @@ def test_box_mutations( # Test case 8.2.23b, 8.2.23d, 8.2.23j mutation = f"""mutation {{ assignTagToBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -547,7 +544,7 @@ def test_box_mutations( # Test case 8.2.24b, 8.2.24d, 8.2.24j mutation = f"""mutation {{ unassignTagFromBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -559,7 +556,7 @@ def test_box_mutations( # Test cases 8.2.26, 8.2.27, 8.2.28 mutation = f"""mutation {{ deleteBoxes( labelIdentifiers: [{label_identifiers}] ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ id }} invalidBoxLabelIdentifiers }} }} }}""" @@ -575,7 +572,7 @@ def test_box_mutations( mutation = f"""mutation {{ moveBoxesToLocation( updateInput: {{ labelIdentifiers: [{label_identifiers}], locationId: {another_location_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ id location {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -1032,7 +1029,8 @@ def _create_query(label_identifier): return f"""query {{ box(labelIdentifier: "{label_identifier}") {{ id }} }}""" def _create_qr_query(qr_code): - return f"""query {{ qrCode(qrCode: "{qr_code['code']}") {{ box {{ id }} }} }}""" + return f"""query {{ qrCode(code: "{qr_code['code']}") {{ + ...on QrCode {{ box {{ ...on Box {{ id }} }} }} }} }}""" queries = { str(in_transit_box["id"]): _create_query(in_transit_box["label_identifier"]), diff --git a/back/test/endpoint_tests/test_cron.py b/back/test/endpoint_tests/test_cron.py index 4b3074d8f..afc056e5e 100644 --- a/back/test/endpoint_tests/test_cron.py +++ b/back/test/endpoint_tests/test_cron.py @@ -13,7 +13,7 @@ NR_OF_CREATED_TAGS_PER_BASE, NR_OF_DELETED_TAGS_PER_BASE, ) -from utils import assert_bad_user_input, assert_successful_request +from utils import assert_successful_request reseed_db_path = f"{CRON_PATH}/reseed-db" headers = [("X-AppEngine-Cron", "true")] @@ -45,14 +45,16 @@ def test_reseed_db(cron_client, monkeypatch, mocker): # Success; perform actual sourcing of seed (takes about 2s) # Create QR code and verify that it is removed after reseeding - mutation = "mutation { createQrCode { code } }" + mutation = "mutation { createQrCode { id code } }" response = assert_successful_request(cron_client, mutation) code = response["code"] response = cron_client.get(reseed_db_path, headers=headers) assert response.status_code == 200 assert response.json == {"message": "reseed-db job executed"} - query = f"""query {{ qrCode(qrCode: "{code}") {{ id }} }}""" - response = assert_bad_user_input(cron_client, query) + query = f"""query {{ qrCode(code: "{code}") {{ + ...on ResourceDoesNotExistError {{ id name }} }} }}""" + response = assert_successful_request(cron_client, query) + assert response == {"id": None, "name": "QrCode"} # Verify generation of fake data query = "query { tags { id } }" diff --git a/back/test/endpoint_tests/test_permissions.py b/back/test/endpoint_tests/test_permissions.py index 92a0baaf4..3c087cacf 100644 --- a/back/test/endpoint_tests/test_permissions.py +++ b/back/test/endpoint_tests/test_permissions.py @@ -58,10 +58,8 @@ def operation_name(operation): [ # Test case 8.1.3 """box( labelIdentifier: "12345678") { id }""", - # Test case 8.1.32 - """qrCode( qrCode: "1337beef" ) { id }""", # Test case 8.1.35 - """qrExists( qrCode: "1337beef" )""", + """qrExists( code: "1337beef" )""", ], ids=operation_name, ) @@ -300,14 +298,23 @@ def test_invalid_permission_for_location_boxes(read_only_client, mocker): def test_invalid_permission_for_qr_code_box( - read_only_client, mocker, default_qr_code, another_qr_code_with_box + read_only_client, + mocker, + default_qr_code, + another_qr_code_with_box, + another_base, + another_organisation, ): # Test case 8.1.10 # Verify missing stock:read permission mock_user_for_request(mocker, permissions=["qr:read"]) code = default_qr_code["code"] - query = f"""query {{ qrCode(qrCode: "{code}") {{ box {{ id }} }} }}""" - assert_forbidden_request(read_only_client, query, value={"box": None}) + query = f"""query {{ qrCode(code: "{code}") {{ + ...on QrCode {{ + box {{ ...on InsufficientPermissionError {{ name }} }} + }} }} }}""" + response = assert_successful_request(read_only_client, query) + assert response == {"box": {"name": "stock:read"}} # Test case 8.1.11 # Verify missing base-specific stock:read permission (the QR code belongs to a box @@ -325,9 +332,21 @@ def test_invalid_permission_for_qr_code_box( base_ids=[1], ) code = another_qr_code_with_box["code"] # the associated box is in base ID 3 - query = f"""query {{ qrCode(qrCode: "{code}") {{ - box {{ tags {{ taggedResources {{ ...on Beneficiary {{ id }} }} }} }} }} }}""" - assert_forbidden_request(read_only_client, query, value={"box": None}) + query = f"""query {{ qrCode(code: "{code}") {{ + ...on QrCode {{ + box {{ + ...on UnauthorizedForBaseError {{ id name organisationName }} + ...on Box {{ + tags {{ taggedResources {{ ...on Beneficiary {{ id }} }} }} + }} }} }} }} }}""" + response = assert_successful_request(read_only_client, query) + assert response == { + "box": { + "id": "3", + "name": another_base["name"], + "organisationName": another_organisation["name"], + } + } def test_invalid_permission_for_organisation_bases( @@ -648,6 +667,13 @@ def test_mutate_unauthorized_for_base( @pytest.mark.parametrize( "operation,query_input,field,response", [ + # Test case 8.1.32 + [ + "qrCode", + 'code: "1337beef"', + "...on InsufficientPermissionError { name }", + {"name": "qr:read"}, + ], # Test case 8.1.43 [ "standardProduct", diff --git a/back/test/endpoint_tests/test_qr.py b/back/test/endpoint_tests/test_qr.py index 1b87fe563..3890b8fc8 100644 --- a/back/test/endpoint_tests/test_qr.py +++ b/back/test/endpoint_tests/test_qr.py @@ -8,14 +8,14 @@ def test_qr_exists_query(read_only_client, default_qr_code): # Test case 8.1.33 code = default_qr_code["code"] query = f"""query CheckQrExistence {{ - qrExists(qrCode: "{code}") + qrExists(code: "{code}") }}""" qr_exists = assert_successful_request(read_only_client, query) assert qr_exists # Test case 8.1.34 query = """query CheckQrExistence { - qrExists(qrCode: "000") + qrExists(code: "000") }""" qr_exists = assert_successful_request(read_only_client, query) assert not qr_exists @@ -25,12 +25,12 @@ def test_qr_code_query(read_only_client, default_box, default_qr_code): # Test case 8.1.30 code = default_qr_code["code"] query = f"""query {{ - qrCode(qrCode: "{code}") {{ + qrCode(code: "{code}") {{ ...on QrCode {{ id code - box {{ id }} + box {{ ...on Box {{ id }} }} createdOn - }} + }} }} }}""" queried_code = assert_successful_request(read_only_client, query) assert queried_code == { @@ -44,7 +44,8 @@ def test_qr_code_query(read_only_client, default_box, default_qr_code): def test_code_not_associated_with_box(read_only_client, qr_code_without_box): # Test case 8.1.2a code = qr_code_without_box["code"] - query = f"""query {{ qrCode(qrCode: "{code}") {{ box {{ id }} }} }}""" + query = f"""query {{ qrCode(code: "{code}") {{ + ...on QrCode {{ box {{ ...on Box {{ id }} }} }} }} }}""" qr_code = assert_successful_request(read_only_client, query) assert qr_code == {"box": None} @@ -61,10 +62,10 @@ def test_qr_code_mutation(client, box_without_qr_code): createQrCode(boxLabelIdentifier: "{box_without_qr_code['label_identifier']}") {{ id - box {{ + box {{ ...on Box {{ id numberOfItems - }} + }} }} }} }}""" created_qr_code = assert_successful_request(client, mutation) diff --git a/front/src/components/QrReader/QrReaderContainer.tsx b/front/src/components/QrReader/QrReaderContainer.tsx index 1fa2bdeca..755cc7dd7 100644 --- a/front/src/components/QrReader/QrReaderContainer.tsx +++ b/front/src/components/QrReader/QrReaderContainer.tsx @@ -34,6 +34,7 @@ function QrReaderContainer({ onSuccess }: IQrReaderContainerProps) { const [isMultiBox, setIsMultiBox] = useState(!!qrReaderOverlayState.isMultiBox); const [isProcessingQrCode, setIsProcessingQrCode] = useState(false); const [isCameraNotPermited, setIsCameraNotPermited] = useState(false); + const [boxNotOnwned, setBoxNotOnwned] = useState(""); const setIsProcessingQrCodeDelayed = useCallback( (state: boolean) => { setTimeout(() => { @@ -69,6 +70,7 @@ function QrReaderContainer({ onSuccess }: IQrReaderContainerProps) { const onScan = async (qrReaderResultText: string, multiScan: boolean) => { if (!isProcessingQrCode) { setIsProcessingQrCode(true); + setBoxNotOnwned(""); const qrResolvedValue: IQrResolvedValue = await resolveQrCode( qrReaderResultText, multiScan ? "cache-first" : "network-only", @@ -76,16 +78,23 @@ function QrReaderContainer({ onSuccess }: IQrReaderContainerProps) { switch (qrResolvedValue.kind) { case IQrResolverResultKind.SUCCESS: { const boxLabelIdentifier = qrResolvedValue.box.labelIdentifier; - if (!multiScan) { - const boxBaseId = qrResolvedValue.box.location.base.id; + if (qrResolvedValue.box.__typename === "UnauthorizedForBaseError") { + setBoxNotOnwned( + `This box it at base ${qrResolvedValue.box.name}, which belongs to organization ${qrResolvedValue.box.organisationName}.`, + ); setIsProcessingQrCode(false); - onSuccess(); - navigate(`/bases/${boxBaseId}/boxes/${boxLabelIdentifier}`); } else { - // Only execute for Multi Box tab - // add box reference to query for list of all scanned boxes - await addBoxToScannedBoxes(qrResolvedValue.box); - setIsProcessingQrCode(false); + if (!multiScan) { + const boxBaseId = qrResolvedValue.box.location.base.id; + setIsProcessingQrCode(false); + onSuccess(); + navigate(`/bases/${boxBaseId}/boxes/${boxLabelIdentifier}`); + } else { + // Only execute for Multi Box tab + // add box reference to query for list of all scanned boxes + await addBoxToScannedBoxes(qrResolvedValue.box); + setIsProcessingQrCode(false); + } } break; } @@ -170,6 +179,12 @@ function QrReaderContainer({ onSuccess }: IQrReaderContainerProps) {
)} + {boxNotOnwned !== "" && ( + <> + +
+ + )} setIsMultiBox(index === 1)} diff --git a/front/src/hooks/useQrResolver.ts b/front/src/hooks/useQrResolver.ts index 9e17471c1..edf0228aa 100644 --- a/front/src/hooks/useQrResolver.ts +++ b/front/src/hooks/useQrResolver.ts @@ -52,7 +52,7 @@ export const useQrResolver = () => { }) .then(({ data, errors }) => { if ((errors?.length || 0) > 0) { - const errorCode = errors ? errors[0].extensions?.code : undefined; + const errorCode = errors ? errors[0]?.extensions?.code : undefined; if (errorCode === "FORBIDDEN") { triggerError({ message: "You don't have permission to access this box!", diff --git a/front/src/queries/queries.ts b/front/src/queries/queries.ts index 4c80f0427..21d9e8a2f 100644 --- a/front/src/queries/queries.ts +++ b/front/src/queries/queries.ts @@ -38,10 +38,29 @@ export const BOX_DETAILS_BY_LABEL_IDENTIFIER_QUERY = gql` export const GET_BOX_LABEL_IDENTIFIER_BY_QR_CODE = gql` ${BOX_BASIC_FIELDS_FRAGMENT} query GetBoxLabelIdentifierForQrCode($qrCode: String!) { - qrCode(qrCode: $qrCode) { - code - box { - ...BoxBasicFields + qrCode(code: $qrCode) { + __typename + ... on QrCode { + code + box { + __typename + ...on Box { + ...BoxBasicFields + } + ...on InsufficientPermissionError { + name + } + ...on UnauthorizedForBaseError { + name + organisationName + } + } + } + ... on InsufficientPermissionError { + name + } + ... on ResourceDoesNotExistError { + name } } } @@ -49,7 +68,7 @@ export const GET_BOX_LABEL_IDENTIFIER_BY_QR_CODE = gql` export const CHECK_IF_QR_EXISTS_IN_DB = gql` query CheckIfQrExistsInDb($qrCode: String!) { - qrExists(qrCode: $qrCode) + qrExists(code: $qrCode) } `; diff --git a/front/src/types/generated/graphql.ts b/front/src/types/generated/graphql.ts index 349848b0e..6e0eb1e40 100755 --- a/front/src/types/generated/graphql.ts +++ b/front/src/types/generated/graphql.ts @@ -14,7 +14,7 @@ export type Scalars = { Datetime: any; }; -export type AssignTagToBoxesResult = BoxResult | DeletedTagError | InsufficientPermissionError | ResourceDoesNotExistError | TagTypeMismatchError | UnauthorizedForBaseError; +export type AssignTagToBoxesResult = BoxesResult | DeletedTagError | InsufficientPermissionError | ResourceDoesNotExistError | TagTypeMismatchError | UnauthorizedForBaseError; /** * Representation of a base. @@ -247,12 +247,7 @@ export type BoxPage = { totalCount: Scalars['Int']; }; -/** 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). */ -export type BoxResult = { - __typename?: 'BoxResult'; - invalidBoxLabelIdentifiers: Array; - updatedBoxes: Array; -}; +export type BoxResult = Box | InsufficientPermissionError | UnauthorizedForBaseError; /** Classificators for [`Box`]({{Types.Box}}) state. */ export enum BoxState { @@ -280,6 +275,13 @@ export type BoxUpdateInput = { tagIdsToBeAdded?: InputMaybe>; }; +/** 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). */ +export type BoxesResult = { + __typename?: 'BoxesResult'; + invalidBoxLabelIdentifiers: Array; + updatedBoxes: Array; +}; + export type BoxesStillAssignedToProductError = { __typename?: 'BoxesStillAssignedToProductError'; labelIdentifiers: Array; @@ -371,7 +373,7 @@ export type DataCube = { facts?: Maybe>>; }; -export type DeleteBoxesResult = BoxResult | InsufficientPermissionError; +export type DeleteBoxesResult = BoxesResult | InsufficientPermissionError; export type DeleteProductResult = BoxesStillAssignedToProductError | InsufficientPermissionError | Product | ProductTypeMismatchError | ResourceDoesNotExistError | UnauthorizedForBaseError; @@ -652,7 +654,7 @@ export type MetricsNumberOfSalesArgs = { before?: InputMaybe; }; -export type MoveBoxesResult = BoxResult | DeletedLocationError | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError; +export type MoveBoxesResult = BoxesResult | DeletedLocationError | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError; export type MovedBoxDataDimensions = { __typename?: 'MovedBoxDataDimensions'; @@ -699,7 +701,7 @@ export type Mutation = { assignBoxToDistributionEvent?: Maybe; /** Assign a tag to a resource (box or beneficiary). If the resource already has this tag assigned, do nothing */ assignTag?: Maybe; - /** 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?: Maybe; /** Change state of specified shipment to `Canceled`. Only valid for shipments in `Preparing` state. Any boxes marked for shipment are moved back into stock. The client must be member of either source or target base of the shipment. */ cancelShipment?: Maybe; @@ -724,7 +726,7 @@ export type Mutation = { createTransferAgreement?: Maybe; /** Deactivate beneficiary with specified ID. */ deactivateBeneficiary?: Maybe; - /** 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?: Maybe; /** Soft-delete the custom product with specified ID. Return errors if the product is still assigned to any boxes. The client must be member of the base that the product is registered in. */ deleteProduct?: Maybe; @@ -740,7 +742,7 @@ export type Mutation = { enableStandardProduct?: Maybe; /** Change state of specified shipment to `Lost`, and state of all contained `InTransit` boxes to `NotDelivered`. Only valid for shipments in `Sent` state. The client must be member of either source or target base of the shipment. */ markShipmentAsLost?: Maybe; - /** 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?: Maybe; moveItemsFromBoxToDistributionEvent?: Maybe; moveItemsFromReturnTrackingGroupToBox?: Maybe; @@ -760,7 +762,7 @@ export type Mutation = { unassignBoxFromDistributionEvent?: Maybe; /** Remove a tag from a resource (box or beneficiary). If the resource does not have this tag assigned, do nothing */ unassignTag?: Maybe; - /** 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?: Maybe; /** Update one or more properties of a beneficiary with specified ID. */ updateBeneficiary?: Maybe; @@ -1197,13 +1199,15 @@ export type ProductTypeMismatchError = { /** Representation of a QR code, possibly associated with a [`Box`]({{Types.Box}}). */ export type QrCode = { __typename?: 'QrCode'; - /** [`Box`]({{Types.Box}}) associated with the QR code (`null` if none associated) */ - box?: Maybe; + /** [`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?: Maybe; code: Scalars['String']; createdOn?: Maybe; id: Scalars['ID']; }; +export type QrCodeResult = InsufficientPermissionError | QrCode | ResourceDoesNotExistError; + export type Query = { __typename?: 'Query'; /** Return [`Base`]({{Types.Base}}) with specified ID. Accessible for clients who are members of this base. */ @@ -1244,8 +1248,8 @@ export type Query = { productCategory?: Maybe; /** Return all [`Products`]({{Types.Product}}) (incl. deleted) that the client is authorized to view. */ products: ProductPage; - /** Return [`QrCode`]({{Types.QrCode}}) with specified code (an MD5 hash in hex format of length 32) */ - qrCode?: Maybe; + /** 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: QrCodeResult; qrExists?: Maybe; /** Return [`Shipment`]({{Types.Shipment}}) with specified ID. Clients are authorized to view a shipment if they're member of either the source or the target base */ shipment?: Maybe; @@ -1376,12 +1380,12 @@ export type QueryProductsArgs = { export type QueryQrCodeArgs = { - qrCode: Scalars['String']; + code: Scalars['String']; }; export type QueryQrExistsArgs = { - qrCode?: InputMaybe; + code?: InputMaybe; }; @@ -1830,13 +1834,16 @@ export enum TransferAgreementType { SendingTo = 'SendingTo' } -export type UnassignTagFromBoxesResult = BoxResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError; +export type UnassignTagFromBoxesResult = BoxesResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError; export type UnauthorizedForBaseError = { __typename?: 'UnauthorizedForBaseError'; /** e.g. 'product:write' present but not for requested base */ id: Scalars['ID']; + /** Empty string if base does not exist */ name: Scalars['String']; + /** Empty string if base does not exist */ + organisationName: Scalars['String']; }; export type UnboxedItemsCollection = ItemsCollection & { @@ -1948,7 +1955,7 @@ export type GetBoxLabelIdentifierForQrCodeQueryVariables = Exact<{ }>; -export type GetBoxLabelIdentifierForQrCodeQuery = { __typename?: 'Query', qrCode?: { __typename?: 'QrCode', code: string, box?: { __typename?: 'Box', labelIdentifier: string, state: BoxState, comment?: string | null, lastModifiedOn?: any | null, location?: { __typename?: 'ClassicLocation', id: string, base?: { __typename?: 'Base', id: string } | null } | { __typename?: 'DistributionSpot', id: string, base?: { __typename?: 'Base', id: string } | null } | null, shipmentDetail?: { __typename?: 'ShipmentDetail', id: string, shipment: { __typename?: 'Shipment', id: string } } | null } | null } | null }; +export type GetBoxLabelIdentifierForQrCodeQuery = { __typename?: 'Query', qrCode: { __typename: 'InsufficientPermissionError', name: string } | { __typename: 'QrCode', code: string, box?: { __typename: 'Box', labelIdentifier: string, state: BoxState, comment?: string | null, lastModifiedOn?: any | null, location?: { __typename?: 'ClassicLocation', id: string, base?: { __typename?: 'Base', id: string } | null } | { __typename?: 'DistributionSpot', id: string, base?: { __typename?: 'Base', id: string } | null } | null, shipmentDetail?: { __typename?: 'ShipmentDetail', id: string, shipment: { __typename?: 'Shipment', id: string } } | null } | { __typename: 'InsufficientPermissionError', name: string } | { __typename: 'UnauthorizedForBaseError', name: string, organisationName: string } | null } | { __typename: 'ResourceDoesNotExistError', name: string } }; export type CheckIfQrExistsInDbQueryVariables = Exact<{ qrCode: Scalars['String']; @@ -2033,7 +2040,7 @@ export type CreateBoxMutationVariables = Exact<{ }>; -export type CreateBoxMutation = { __typename?: 'Mutation', createBox?: { __typename?: 'Box', labelIdentifier: string, qrCode?: { __typename?: 'QrCode', code: string, box?: { __typename?: 'Box', labelIdentifier: string } | null } | null } | null }; +export type CreateBoxMutation = { __typename?: 'Mutation', createBox?: { __typename?: 'Box', labelIdentifier: string, qrCode?: { __typename?: 'QrCode', code: string, box?: { __typename?: 'Box', labelIdentifier: string } | { __typename?: 'InsufficientPermissionError' } | { __typename?: 'UnauthorizedForBaseError' } | null } | null } | null }; export type BoxByLabelIdentifierAndAllProductsWithBaseIdQueryVariables = Exact<{ baseId: Scalars['ID']; diff --git a/front/src/views/BoxCreate/BoxCreateView.tsx b/front/src/views/BoxCreate/BoxCreateView.tsx index 3e285873b..fd228143b 100644 --- a/front/src/views/BoxCreate/BoxCreateView.tsx +++ b/front/src/views/BoxCreate/BoxCreateView.tsx @@ -72,7 +72,9 @@ export const CREATE_BOX_MUTATION = gql` qrCode { code box { - labelIdentifier + ... on Box { + labelIdentifier + } } } } @@ -175,7 +177,7 @@ function BoxCreateView() { }) .then((mutationResult) => { if (mutationResult.errors) { - const errorCode = mutationResult.errors[0].extensions?.code; + const errorCode = mutationResult.errors[0]?.extensions?.code; if (errorCode === "BAD_USER_INPUT") { triggerError({ message: "The QR code is already used for another box.",