From bbd96c47ed2d733fa33d990117682c790db3f024 Mon Sep 17 00:00:00 2001 From: Daniel Schick Date: Wed, 14 Jan 2026 14:39:56 +0100 Subject: [PATCH] Extra schemathesis validation. Still cases failing, but it is much better now --- misc/BreCalApi.yaml | 43 +++++++- src/server/BreCal/api/history.py | 9 +- src/server/BreCal/api/participant.py | 7 +- src/server/BreCal/api/shipcalls.py | 9 +- src/server/BreCal/api/ships.py | 18 +++ src/server/BreCal/api/times.py | 9 +- src/server/BreCal/impl/ships.py | 5 +- src/server/BreCal/impl/times.py | 7 +- src/server/BreCal/schemas/model.py | 103 +++++++++++------- .../validators/input_validation_times.py | 14 ++- 10 files changed, 168 insertions(+), 56 deletions(-) diff --git a/misc/BreCalApi.yaml b/misc/BreCalApi.yaml index 0339189..bb66bae 100644 --- a/misc/BreCalApi.yaml +++ b/misc/BreCalApi.yaml @@ -200,6 +200,8 @@ paths: $ref: '#/components/responses/400' '401': $ref: '#/components/responses/401' + '409': + $ref: '#/components/responses/409' '500': $ref: '#/components/responses/500' '503': @@ -256,6 +258,8 @@ paths: $ref: '#/components/responses/400' '401': $ref: '#/components/responses/401' + '409': + $ref: '#/components/responses/409' '500': $ref: '#/components/responses/500' '503': @@ -313,6 +317,8 @@ paths: $ref: '#/components/responses/401' '404': $ref: '#/components/responses/404' + '409': + $ref: '#/components/responses/409' '500': $ref: '#/components/responses/500' '503': @@ -418,6 +424,8 @@ paths: $ref: '#/components/responses/401' '404': $ref: '#/components/responses/404' + '409': + $ref: '#/components/responses/409' '500': $ref: '#/components/responses/500' '503': @@ -462,7 +470,10 @@ paths: required: false description: '**Id of user**. *Example: 2*. User id returned by login call.' schema: - type: integer + oneOf: + - type: integer + - type: string + pattern: '^[0-9]+$' example: 2 responses: '200': @@ -502,7 +513,10 @@ paths: in: query description: '**Id**. *Example: 42*. Id of referenced ship call.' schema: - type: integer + oneOf: + - type: integer + - type: string + pattern: '^[0-9]+$' example: 42 example: 42 responses: @@ -681,7 +695,10 @@ paths: required: false description: '**Id of participant**. *Example: 7*. Id of logged in participant.' schema: - type: integer + oneOf: + - type: integer + - type: string + pattern: '^[0-9]+$' example: 7 responses: '200': @@ -862,6 +879,7 @@ components: shipcall: type: object description: Ship call data + additionalProperties: false example: id: 6 ship_id: 8 @@ -981,6 +999,8 @@ components: nullable: true recommended_tugs: type: integer + minimum: 0 + maximum: 10 example: 2 nullable: true anchored: @@ -1650,14 +1670,17 @@ components: street: type: string maxLength: 128 + nullable: true example: Hermann-Hollerith-Str. 7 postal code: type: string maxLength: 5 + nullable: true example: '28359' city: type: string maxLength: 64 + nullable: true example: Bremen type: type: integer @@ -1816,13 +1839,14 @@ components: maxLength: 45 example: Doe user_phone: - maxLength: 128 + maxLength: 32 type: string nullable: true example: '1234567890' user_email: - maxLength: 128 + maxLength: 64 type: string + format: email nullable: true example: no@where.com notify_email: @@ -2049,6 +2073,15 @@ components: example: error_field: No such record error_description: The requested resource to update was not found + '409': + description: Conflict + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + example: + error_field: imo + error_description: Resource already exists '500': description: Unexpected error content: diff --git a/src/server/BreCal/api/history.py b/src/server/BreCal/api/history.py index 098423f..ddb8627 100644 --- a/src/server/BreCal/api/history.py +++ b/src/server/BreCal/api/history.py @@ -17,7 +17,14 @@ def GetHistory(): options = {} if not 'shipcall_id' in request.args: return create_dynamic_exception_response(ex=None, status_code=400, message="missing parameter: shipcall_id") - options["shipcall_id"] = request.args.get("shipcall_id") + shipcall_id_values = request.args.getlist("shipcall_id") + if len(shipcall_id_values) != 1: + return create_dynamic_exception_response(ex=None, status_code=400, message="invalid parameter: shipcall_id") + shipcall_id_raw = shipcall_id_values[0] + try: + options["shipcall_id"] = int(shipcall_id_raw) + except (TypeError, ValueError): + return create_dynamic_exception_response(ex=None, status_code=400, message="invalid parameter: shipcall_id") return impl.history.GetHistory(options) else: return create_dynamic_exception_response(ex=None, status_code=403, message="not authenticated") diff --git a/src/server/BreCal/api/participant.py b/src/server/BreCal/api/participant.py index 475d0f6..f010a1c 100644 --- a/src/server/BreCal/api/participant.py +++ b/src/server/BreCal/api/participant.py @@ -16,7 +16,12 @@ def GetParticipant(): if 'Authorization' in request.headers: payload = decode_jwt(request.headers.get("Authorization").split("Bearer ")[-1]) options = {} - options["user_id"] = request.args.get("user_id") + user_id_raw = request.args.get("user_id") + if user_id_raw is not None: + try: + options["user_id"] = int(user_id_raw) + except (TypeError, ValueError): + return create_dynamic_exception_response(ex=None, status_code=400, message="invalid parameter: user_id") if "participant_id" in payload: options["participant_id"] = payload["participant_id"] else: diff --git a/src/server/BreCal/api/shipcalls.py b/src/server/BreCal/api/shipcalls.py index a4e1045..2a54206 100644 --- a/src/server/BreCal/api/shipcalls.py +++ b/src/server/BreCal/api/shipcalls.py @@ -35,7 +35,14 @@ def GetShipcalls(): """ payload = decode_jwt(request.headers.get("Authorization").split("Bearer ")[-1]) options = {} - options["past_days"] = request.args.get("past_days", default=1, type=int) + past_days_raw = request.args.get("past_days", default=None) + if past_days_raw is None: + options["past_days"] = 1 + else: + try: + options["past_days"] = int(past_days_raw) + except (TypeError, ValueError): + return create_dynamic_exception_response(ex=None, status_code=400, message="invalid parameter: past_days") options["participant_id"] = payload["participant_id"] return impl.shipcalls.GetShipcalls(options) diff --git a/src/server/BreCal/api/ships.py b/src/server/BreCal/api/ships.py index e8e2626..45a3911 100644 --- a/src/server/BreCal/api/ships.py +++ b/src/server/BreCal/api/ships.py @@ -13,6 +13,18 @@ from BreCal.validators.input_validation_ship import InputValidationShip bp = Blueprint('ships', __name__) +def _message_contains(messages, needle: str) -> bool: + if not isinstance(messages, dict): + return False + for value in messages.values(): + if isinstance(value, list): + text = " ".join(map(str, value)) + else: + text = str(value) + if needle in text: + return True + return False + @bp.route('/ships', methods=['get']) @auth_guard() # no restriction by role def GetShips(): @@ -52,6 +64,8 @@ def PostShip(): return impl.ships.PostShip(loadedModel) except ValidationError as ex: + if _message_contains(ex.messages, "already exists"): + return create_validation_error_response(ex=ex, status_code=409) return create_validation_error_response(ex=ex, status_code=400) except Exception as ex: @@ -79,6 +93,8 @@ def PutShip(): return impl.ships.PutShip(loadedModel) except ValidationError as ex: + if _message_contains(ex.messages, "may not be changed"): + return create_validation_error_response(ex=ex, status_code=409) return create_validation_error_response(ex=ex, status_code=400) except Exception as ex: @@ -111,6 +127,8 @@ def DeleteShip(): return impl.ships.DeleteShip(options) except ValidationError as ex: + if _message_contains(ex.messages, "already deleted") or _message_contains(ex.messages, "Could not find a ship"): + return create_validation_error_response(ex=ex, status_code=404) return create_validation_error_response(ex=ex, status_code=400) except Exception as ex: diff --git a/src/server/BreCal/api/times.py b/src/server/BreCal/api/times.py index a22eabb..10e52b7 100644 --- a/src/server/BreCal/api/times.py +++ b/src/server/BreCal/api/times.py @@ -18,7 +18,14 @@ def GetTimes(): try: options = {} - options["shipcall_id"] = request.args.get("shipcall_id") + shipcall_id_raw = request.args.get("shipcall_id") + if shipcall_id_raw is not None: + try: + options["shipcall_id"] = int(shipcall_id_raw) + except (TypeError, ValueError): + return create_dynamic_exception_response(ex=None, status_code=400, message="invalid parameter: shipcall_id") + else: + options["shipcall_id"] = None return impl.times.GetTimes(options) except Exception as ex: diff --git a/src/server/BreCal/impl/ships.py b/src/server/BreCal/impl/ships.py index 8792716..1e4fb2d 100644 --- a/src/server/BreCal/impl/ships.py +++ b/src/server/BreCal/impl/ships.py @@ -152,10 +152,11 @@ def DeleteShip(options): commands = pydapper.using(pooledConnection) # query = SQLQuery.get_ship_delete_by_id() # affected_rows = commands.execute(query, param={"id" : options["id"]}) - affected_rows = commands.execute("UPDATE ship SET deleted = 1 WHERE id = ?id?", param={"id" : options["id"]}) + ship_id = int(options["id"]) if not isinstance(options["id"], int) else options["id"] + affected_rows = commands.execute("UPDATE ship SET deleted = 1 WHERE id = ?id?", param={"id" : ship_id}) if affected_rows == 1: - return json.dumps({"id" : options["id"]}), 200, {'Content-Type': 'application/json; charset=utf-8'} + return json.dumps({"id" : ship_id}), 200, {'Content-Type': 'application/json; charset=utf-8'} result = {} result["error_field"] = "no such record" diff --git a/src/server/BreCal/impl/times.py b/src/server/BreCal/impl/times.py index 531b3fd..a8fd118 100644 --- a/src/server/BreCal/impl/times.py +++ b/src/server/BreCal/impl/times.py @@ -194,8 +194,9 @@ def DeleteTimes(options): try: pooledConnection = local_db.getPoolConnection() commands = pydapper.using(pooledConnection) - shipcall_id = commands.execute_scalar("SELECT shipcall_id FROM times WHERE id = ?id?", param={"id" : options["id"]}) - affected_rows = commands.execute("DELETE FROM times WHERE id = ?id?", param={"id" : options["id"]}) + times_id = int(options["id"]) if not isinstance(options["id"], int) else options["id"] + shipcall_id = commands.execute_scalar("SELECT shipcall_id FROM times WHERE id = ?id?", param={"id" : times_id}) + affected_rows = commands.execute("DELETE FROM times WHERE id = ?id?", param={"id" : times_id}) # TODO: set ETA properly? @@ -205,7 +206,7 @@ def DeleteTimes(options): commands.execute(query, {"pid" : user_data["participant_id"], "shipcall_id" : shipcall_id, "uid" : user_data["id"]}) if affected_rows == 1: - return json.dumps({"id" : options["id"]}), 200, {'Content-Type': 'application/json; charset=utf-8'} + return json.dumps({"id" : times_id}), 200, {'Content-Type': 'application/json; charset=utf-8'} result = {} result["error_field"] = "no such record" diff --git a/src/server/BreCal/schemas/model.py b/src/server/BreCal/schemas/model.py index cab5523..7bc6b5e 100644 --- a/src/server/BreCal/schemas/model.py +++ b/src/server/BreCal/schemas/model.py @@ -1,5 +1,5 @@ from dataclasses import field, dataclass -from marshmallow import Schema, fields, INCLUDE, ValidationError, validate, validates, post_load +from marshmallow import Schema, fields, INCLUDE, ValidationError, validate, validates, post_load, pre_load, EXCLUDE from marshmallow.fields import Field from marshmallow_enum import EnumField from enum import IntEnum @@ -17,9 +17,18 @@ from BreCal.database.enums import ParticipantType, ParticipantFlag +def _format_datetime(value): + if value is None: + return None + if isinstance(value, datetime.datetime): + if value.tzinfo is None or value.tzinfo.utcoffset(value) is None: + return value.isoformat() + "Z" + return value.isoformat() + return value + def obj_dict(obj): if isinstance(obj, datetime.datetime): - return obj.isoformat() + return _format_datetime(obj) if hasattr(obj, 'to_json'): return obj.to_json() return obj.__dict__ @@ -53,8 +62,8 @@ class Berth(Schema): "owner_id": self.owner_id, "authority_id": self.authority_id, "port_id": self.port_id, - "created": self.created.isoformat() if self.created else "", - "modified": self.modified.isoformat() if self.modified else "", + "created": _format_datetime(self.created), + "modified": _format_datetime(self.modified), "deleted": _coerce_bool(self.deleted), } @@ -72,8 +81,8 @@ class Port(Schema): "id": self.id, "name": self.name, "locode": self.locode, - "created": self.created.isoformat() if self.created else "", - "modified": self.modified.isoformat() if self.modified else "", + "created": _format_datetime(self.created), + "modified": _format_datetime(self.modified), "deleted": _coerce_bool(self.deleted), } @@ -180,8 +189,8 @@ class History: "id": self.id, "participant_id": self.participant_id, "shipcall_id": self.shipcall_id, - "timestamp": self.timestamp.isoformat() if self.timestamp else "", - "eta": self.eta.isoformat() if self.eta else "", + "timestamp": _format_datetime(self.timestamp), + "eta": _format_datetime(self.eta), "type": self.type.name if isinstance(self.type, IntEnum) else ObjectType(self.type).name, "operation": self.operation.name if isinstance(self.operation, IntEnum) else OperationType(self.operation).name } @@ -222,8 +231,8 @@ class Notification: "level": self.level, "type": self.type.name if isinstance(self.type, IntEnum) else NotificationType(self.type).name, "message": self.message, - "created": self.created.isoformat() if self.created else "", - "modified": self.modified.isoformat() if self.modified else "" + "created": _format_datetime(self.created), + "modified": _format_datetime(self.modified) } @classmethod @@ -253,8 +262,8 @@ class Participant(Schema): "city": self.city, "type": self.type, "flags": self.flags, - "created": self.created.isoformat() if self.created else "", - "modified": self.modified.isoformat() if self.modified else "", + "created": _format_datetime(self.created), + "modified": _format_datetime(self.modified), "deleted": _coerce_bool(self.deleted), "ports": self.ports, } @@ -290,7 +299,7 @@ class ParticipantAssignmentSchema(Schema): class ShipcallSchema(Schema): def __init__(self): - super().__init__(unknown=None) + super().__init__(unknown=EXCLUDE) pass id = fields.Integer(required=True) @@ -412,9 +421,9 @@ class Shipcall: "id": self.id, "ship_id": self.ship_id, "type": self.type.name if isinstance(self.type, IntEnum) else ShipcallType(self.type).name, - "eta": self.eta.isoformat() if self.eta else "", + "eta": _format_datetime(self.eta), "voyage": self.voyage, - "etd": self.etd.isoformat() if self.etd else "", + "etd": _format_datetime(self.etd), "arrival_berth_id": self.arrival_berth_id, "departure_berth_id": self.departure_berth_id, "tug_required": _coerce_bool(self.tug_required), @@ -425,8 +434,8 @@ class Shipcall: "replenishing_terminal": _coerce_bool(self.replenishing_terminal), "replenishing_lock": _coerce_bool(self.replenishing_lock), "draft": self.draft, - "tidal_window_from": self.tidal_window_from.isoformat() if self.tidal_window_from else "", - "tidal_window_to": self.tidal_window_to.isoformat() if self.tidal_window_to else "", + "tidal_window_from": _format_datetime(self.tidal_window_from), + "tidal_window_to": _format_datetime(self.tidal_window_to), "rain_sensitive_cargo": _coerce_bool(self.rain_sensitive_cargo), "recommended_tugs": self.recommended_tugs, "anchored": _coerce_bool(self.anchored), @@ -434,12 +443,12 @@ class Shipcall: "canceled": _coerce_bool(self.canceled), "evaluation": self.evaluation.name if isinstance(self.evaluation, IntEnum) else EvaluationType(self.evaluation).name, "evaluation_message": self.evaluation_message, - "evaluation_time": self.evaluation_time.isoformat() if self.evaluation_time else "", + "evaluation_time": _format_datetime(self.evaluation_time), "evaluation_notifications_sent": _coerce_bool(self.evaluation_notifications_sent), "time_ref_point": self.time_ref_point, "port_id": self.port_id, - "created": self.created.isoformat() if self.created else "", - "modified": self.modified.isoformat() if self.modified else "", + "created": _format_datetime(self.created), + "modified": _format_datetime(self.modified), "participants": [participant.__dict__ for participant in self.participants] } @@ -596,8 +605,8 @@ class UserSchema(Schema): id = fields.Integer(required=True) first_name = fields.String(allow_none=True, required=False, validate=[validate.Length(max=45)]) last_name = fields.String(allow_none=True, required=False, validate=[validate.Length(max=45)]) - user_phone = fields.String(allow_none=True, required=False, validate=[validate.Length(max=128)]) - user_email = fields.String(allow_none=True, required=False, validate=[validate.Length(max=128)]) + user_phone = fields.String(allow_none=True, required=False, validate=[validate.Length(max=32)]) + user_email = fields.String(allow_none=True, required=False, validate=[validate.Length(max=64)]) old_password = fields.String(allow_none=True, required=False, validate=[validate.Length(max=128)]) new_password = fields.String(allow_none=True, required=False, validate=[validate.Length(min=6, max=128)]) notify_email = fields.Bool(allow_none=True, required=False) @@ -606,6 +615,14 @@ class UserSchema(Schema): notify_popup = fields.Bool(allow_none=True, required=False) notify_on = fields.List(fields.Enum(NotificationType), required=False, allow_none=True) + @pre_load + def validate_bool_types(self, data, **kwargs): + bool_fields = ["notify_email", "notify_whatsapp", "notify_signal", "notify_popup"] + for field_name in bool_fields: + if field_name in data and data[field_name] is not None and not isinstance(data[field_name], bool): + raise ValidationError({field_name: "must be a boolean"}) + return data + @validates("user_phone") def validate_user_phone(self, value, **kwargs): if value is not None: @@ -649,16 +666,16 @@ class Times: def to_json(self): return { "id": self.id, - "eta_berth": self.eta_berth.isoformat() if self.eta_berth else "", + "eta_berth": _format_datetime(self.eta_berth), "eta_berth_fixed": _coerce_bool(self.eta_berth_fixed), - "etd_berth": self.etd_berth.isoformat() if self.etd_berth else "", + "etd_berth": _format_datetime(self.etd_berth), "etd_berth_fixed": _coerce_bool(self.etd_berth_fixed), - "lock_time": self.lock_time.isoformat() if self.lock_time else "", + "lock_time": _format_datetime(self.lock_time), "lock_time_fixed": _coerce_bool(self.lock_time_fixed), - "zone_entry": self.zone_entry.isoformat() if self.zone_entry else "", + "zone_entry": _format_datetime(self.zone_entry), "zone_entry_fixed": _coerce_bool(self.zone_entry_fixed), - "operations_start": self.operations_start.isoformat() if self.operations_start else "", - "operations_end": self.operations_end.isoformat() if self.operations_end else "", + "operations_start": _format_datetime(self.operations_start), + "operations_end": _format_datetime(self.operations_end), "remarks": self.remarks, "participant_id": self.participant_id, "berth_id": self.berth_id, @@ -666,12 +683,12 @@ class Times: "pier_side": _coerce_bool(self.pier_side), "participant_type": self.participant_type, "shipcall_id": self.shipcall_id, - "ata": self.ata.isoformat() if self.ata else "", - "atd": self.atd.isoformat() if self.atd else "", - "eta_interval_end": self.eta_interval_end.isoformat() if self.eta_interval_end else "", - "etd_interval_end": self.etd_interval_end.isoformat() if self.etd_interval_end else "", - "created": self.created.isoformat() if self.created else "", - "modified": self.modified.isoformat() if self.modified else "", + "ata": _format_datetime(self.ata), + "atd": _format_datetime(self.atd), + "eta_interval_end": _format_datetime(self.eta_interval_end), + "etd_interval_end": _format_datetime(self.etd_interval_end), + "created": _format_datetime(self.created), + "modified": _format_datetime(self.modified), } @dataclass @@ -720,7 +737,7 @@ class Ship: def to_json(self): return { "id": self.id, - "name": self.name, + "name": self.name if self.name is not None else "", "imo": self.imo, "callsign": self.callsign, "participant_id": self.participant_id, @@ -729,8 +746,8 @@ class Ship: "is_tug": _coerce_bool(self.is_tug), "bollard_pull": self.bollard_pull, "eni": self.eni, - "created": self.created.isoformat() if self.created else "", - "modified": self.modified.isoformat() if self.modified else "", + "created": _format_datetime(self.created), + "modified": _format_datetime(self.modified), "deleted": _coerce_bool(self.deleted), } @@ -754,6 +771,14 @@ class ShipSchema(Schema): modified = fields.DateTime(allow_none=True, required=False) deleted = fields.Bool(allow_none=True, required=False, load_default=False, dump_default=False) + @pre_load + def validate_bool_types(self, data, **kwargs): + bool_fields = ["is_tug", "deleted"] + for field_name in bool_fields: + if field_name in data and data[field_name] is not None and not isinstance(data[field_name], bool): + raise ValidationError({field_name: "must be a boolean"}) + return data + @validates("name") def validate_name(self, value, **kwargs): character_length = len(str(value)) @@ -820,6 +845,6 @@ class ShipcallParticipantMap: "shipcall_id": self.shipcall_id, "participant_id": self.participant_id, "type": self.type.name if isinstance(self.type, IntEnum) else ShipcallType(self.type).name, - "created": self.created.isoformat() if self.created else "", - "modified": self.modified.isoformat() if self.modified else "", + "created": _format_datetime(self.created), + "modified": _format_datetime(self.modified), } diff --git a/src/server/BreCal/validators/input_validation_times.py b/src/server/BreCal/validators/input_validation_times.py index 6e3dd51..3cf4a44 100644 --- a/src/server/BreCal/validators/input_validation_times.py +++ b/src/server/BreCal/validators/input_validation_times.py @@ -177,6 +177,8 @@ class InputValidationTimes(InputValidationBase): # validates the times dataset. # ensure loadedModel["participant_type"] is of type ParticipantType + if loadedModel.get("participant_type") is None: + raise ValidationError({"participant_type": "participant_type is required"}) if not isinstance(loadedModel["participant_type"], ParticipantType): loadedModel["participant_type"] = ParticipantType(loadedModel["participant_type"]) @@ -229,8 +231,10 @@ class InputValidationTimes(InputValidationBase): The dependent and independent fields are validated by checking, whether the respective value in 'content' is undefined (returns None). When any of these fields is undefined, a ValidationError is raised. """ - participant_type = loadedModel["participant_type"] - shipcall_id = loadedModel["shipcall_id"] + participant_type = loadedModel.get("participant_type") + shipcall_id = loadedModel.get("shipcall_id") + if shipcall_id is None: + raise ValidationError({"shipcall_id": "shipcall_id is required"}) # build a dictionary of id:item pairs, so one can select the respective participant # must look-up the shipcall_type based on the shipcall_id @@ -319,7 +323,9 @@ class InputValidationTimes(InputValidationBase): """ ### TIMES DATASET (ShipcallParticipantMap) ### # identify shipcall_id - shipcall_id = loadedModel["shipcall_id"] + shipcall_id = loadedModel.get("shipcall_id") + if loadedModel.get("participant_type") is None: + raise ValidationError({"participant_type": "participant_type is required"}) DATASET_participant_type = ParticipantType(loadedModel["participant_type"]) if not isinstance(loadedModel["participant_type"],ParticipantType) else loadedModel["participant_type"] # get ShipcallParticipantMap for the shipcall_id @@ -434,6 +440,8 @@ class InputValidationTimes(InputValidationBase): # get the matching entry from the shipcall participant map, where the role matches. Raise an error, when there is no match. assigned_agency = get_assigned_participant_of_type(shipcall_id, participant_type=ParticipantType.AGENCY) + if assigned_agency is None: + raise ValidationError({"participant_type": "the assigned agency for this shipcall could not be resolved."}) # a) the user has the participant ID of the assigned entry for a given role user_is_assigned_role = user_participant_id == times_assigned_participant.id