From 923692ec0f4ccc96e2a3cb3b29436d5ce9c1a722 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sondre=20Gr=C3=B8n=C3=A5s?= <44143748+sondregronas@users.noreply.github.com> Date: Wed, 9 Aug 2023 00:41:34 +0200 Subject: [PATCH] Validate forms on backend, TODO: unittests & cleanup A lot of extra, unecessary try & except blocks in inventory.py - and a lot of testcases to be made. --- BookingSystem/__init__.py | 1 + BookingSystem/api.py | 82 ++++++---- BookingSystem/app.py | 4 +- BookingSystem/inventory.py | 22 +-- BookingSystem/sanitizer.py | 167 +++++++++++++++++++++ BookingSystem/templates/inventar_add.html | 2 +- BookingSystem/templates/inventar_edit.html | 2 +- 7 files changed, 236 insertions(+), 44 deletions(-) create mode 100644 BookingSystem/sanitizer.py diff --git a/BookingSystem/__init__.py b/BookingSystem/__init__.py index 5917d9b..4632dea 100644 --- a/BookingSystem/__init__.py +++ b/BookingSystem/__init__.py @@ -16,6 +16,7 @@ LABEL_SERVER = os.getenv('LABEL_SERVER') KIOSK_FQDN = os.getenv('KIOSK_FQDN') API_TOKEN = os.getenv('API_TOKEN') +REGEX_ITEM = r'^(?:(?![\s])[ÆØÅæøåa-zA-Z0-9_\s\-]*[ÆØÅæøåa-zA-Z0-9_\-]+)$' # Logger setup logger = Logger(__name__) diff --git a/BookingSystem/api.py b/BookingSystem/api.py index 4041828..16ce6b4 100644 --- a/BookingSystem/api.py +++ b/BookingSystem/api.py @@ -16,6 +16,7 @@ import user from __init__ import DATABASE, LABEL_SERVER from inventory import Item +from sanitizer import VALIDATORS, MINMAX, sanitize, handle_api_exception from utils import login_required, next_july api = flask.Blueprint('api', __name__) @@ -62,46 +63,50 @@ def get_items_by_userid(userid: str) -> flask.Response: @api.route('/items', methods=['POST']) @login_required(admin_only=True) +@handle_api_exception def add_item() -> flask.Response: """Add an item to the database.""" - form = flask.request.form - item = {key: form.get(key) for key in form.keys() if key in Item.__annotations__} + # START: Validation + validation_map = { + 'id': VALIDATORS.UNIQUE_ID, + 'name': VALIDATORS.NAME, + 'category': VALIDATORS.CATEGORY, + } + item_dict = sanitize(validation_map, flask.request.form) + # END: Validation + + item = {key: val for key, val in item_dict.items()} item = Item(**item) - print_label_count = int(form.get('print_label_count')) - print_label_type = form.get('print_label_type') - try: - inventory.add(item) - except ValueError as e: - return flask.Response(str(e), status=400) - if print_label_count > 0: - url = f'{LABEL_SERVER}/print?count={print_label_count}&variant={print_label_type}&id={item.id}&name={item.name}&category={item.category}' - print(url) + inventory.add(item) return flask.Response(f'La til {item.id} i databasen.', status=201) @api.route('/items/', methods=['PUT']) @login_required(admin_only=True) +@handle_api_exception def edit_item(item_id: str) -> flask.Response: """Edit an item in the database.""" - form = flask.request.form - item = {key: form.get(key) for key in form.keys() if key in Item.__annotations__} - item = Item(**item) + # START: Validation + validation_map = { + 'id': VALIDATORS.UNIQUE_OR_SAME_ID, + 'name': VALIDATORS.NAME, + 'category': VALIDATORS.CATEGORY, + } + item_dict = sanitize(validation_map, flask.request.form, {'id': item_id}) + # END: Validation - try: - inventory.edit(item_id, item) - except ValueError as e: - return flask.Response(str(e), status=400) + item = {key: val for key, val in item_dict.items()} + item = Item(**item) + inventory.edit(item_id, item) return flask.Response(f'Redigerte {item_id} i databasen.', status=200) @api.route('/items/', methods=['DELETE']) @login_required(admin_only=True) +@handle_api_exception def delete_item(item_id: str) -> flask.Response: """Delete an item from the database.""" - try: - inventory.delete(item_id) - except ValueError as e: - return flask.Response(str(e), status=400) + inventory.delete(item_id) return flask.Response(f'Slettet {item_id} fra databasen.', status=200) @@ -116,41 +121,56 @@ def get_label_preview(item_id: str, variant: str = 'qr') -> flask.Response: @api.route('/items//label/print', methods=['POST']) @login_required(admin_only=True) +@handle_api_exception def print_label(item_id: str) -> flask.Response: - form = flask.request.form + # START: Validation + validation_map = { + 'print_label_count': VALIDATORS.INT, + 'print_label_count_minmax': MINMAX(0, 9), + 'print_label_type': VALIDATORS.LABEL_TYPE, + } + form = sanitize(validation_map, flask.request.form) + # END: Validation + variant = form.get('print_label_type', 'qr') count = int(form.get('print_label_count', '1')) item = inventory.get(item_id) url = f'{LABEL_SERVER}/print?id={item.id}&name={item.name}&variant={variant}&count={count}' try: response = requests.post(url) - except Exception as e: + except requests.exceptions.RequestException as e: return flask.Response(str(e), status=500) return flask.Response(response.text, status=response.status_code) @api.route('/book/out', methods=['POST']) @login_required(admin_only=True) +@handle_api_exception def book_equipment() -> flask.Response: """Book out equipment for a user.""" - form = flask.request.form + # START: Validation + validation_map = { + 'user': VALIDATORS.UNIQUE_ID, + 'days': VALIDATORS.INT, + 'days_minmax': MINMAX(1, 90), + 'equipment': VALIDATORS.ITEM_LIST_EXISTS, + } + form = sanitize(validation_map, flask.request.form) + # END: Validation userid = form.get('user') days = form.get('days') - item_ids = form.getlist('equipment') - for item in item_ids: + for item in form.get('equipment'): inventory.register_out(item_id=item, userid=userid, days=days) return flask.Response(f'Utstyret ble utlevert til {user.get(userid).get("name")}.', status=200) @api.route('/return/', methods=['POST']) @login_required(admin_only=True) +@handle_api_exception def return_equipment(item_id: str) -> flask.Response: """Return equipment from a user.""" - try: - inventory.register_in(item_id=item_id) - except ValueError as e: - return flask.Response(str(e), status=400) + inventory.register_in(item_id=item_id) return flask.Response('Utstyr ble innlevert.', status=200) diff --git a/BookingSystem/app.py b/BookingSystem/app.py index a9c47d1..6aa7425 100644 --- a/BookingSystem/app.py +++ b/BookingSystem/app.py @@ -13,7 +13,7 @@ import mail import routes import user -from __init__ import logger +from __init__ import logger, REGEX_ITEM from db import init_db from flask_session import Session @@ -51,7 +51,7 @@ def _jinja2_filter_split(string, split_char=',') -> list: @app.context_processor def context_processor() -> dict: - return dict(regex_item=r'^(?:(?![\s])[ÆØÅæøåa-zA-Z0-9_\s\-]*[ÆØÅæøåa-zA-Z0-9_\-]+)$', + return dict(regex_item=REGEX_ITEM, groups=groups.get_all(), categories=inventory.all_categories(), emails=mail.get_all_emails(), diff --git a/BookingSystem/inventory.py b/BookingSystem/inventory.py index 740c37c..550466b 100644 --- a/BookingSystem/inventory.py +++ b/BookingSystem/inventory.py @@ -8,6 +8,7 @@ import user from __init__ import logger, DATABASE from db import read_sql_query +from sanitizer import APIException """ Schema for items and functions to interact with the database. @@ -110,7 +111,7 @@ def add(item: Item) -> None: con = sqlite3.connect(DATABASE) if item.exists: logger.error(f'{item.id} er allerede i bruk.') - raise ValueError(f'{item.id} er allerede i bruk.') + raise APIException(f'{item.id} er allerede i bruk.') try: con.execute(read_sql_query('add_item.sql'), item.__dict__) con.commit() @@ -118,7 +119,7 @@ def add(item: Item) -> None: logger.debug(f'La til {item.id} med verdier: {item.__dict__}') except sqlite3.IntegrityError: logger.error(f'Ukjent feil ved innlegging av {item.id}.') - raise ValueError(f'Ukjent feil ved innlegging av {item.id}.') + raise APIException(f'Ukjent feil ved innlegging av {item.id}.') finally: con.close() _update_last_seen(item.id) @@ -131,7 +132,7 @@ def edit(old_item_id: str, new_item: Item) -> None: con = sqlite3.connect(DATABASE) if old_item_id.lower() != new_item.id.lower() and new_item.exists: logger.error(f'{new_item.id} er allerede i bruk.') - raise ValueError(f'{new_item.id} er allerede i bruk.') + raise APIException(f'{new_item.id} er allerede i bruk.') try: sql = 'UPDATE inventory SET id=:id, name=:name, category=:category, included_batteries=:included_batteries WHERE id=:old_item_id' con.execute(sql, {**new_item.__dict__, 'old_item_id': old_item_id}) @@ -140,7 +141,7 @@ def edit(old_item_id: str, new_item: Item) -> None: logger.debug(f'Redigerte utstyr {old_item_id}, differanse: {new_item.__dict__}') except sqlite3.IntegrityError: logger.error(f'Ukjent feil ved redigering av {old_item_id}.') - raise ValueError(f'Ukjent feil ved redigering av {old_item_id}.') + raise APIException(f'Ukjent feil ved redigering av {old_item_id}.') finally: con.close() _update_last_seen(new_item.id) @@ -160,7 +161,7 @@ def delete(item_id: str) -> None: logger.info(f'Slettet utstyr {item_id}.') except sqlite3.IntegrityError: logger.error(f'{item_id} eksisterer ikke.') - raise ValueError(f'{item_id} eksisterer ikke.') + raise APIException(f'{item_id} eksisterer ikke.') finally: con.close() audits.audit('ITEM_REM', f'{item_id} ble slettet.') @@ -171,6 +172,9 @@ def get(item_id: str) -> Item: con = sqlite3.connect(DATABASE) item = Item(*con.execute(read_sql_query('get_item.sql'), {'id': item_id}).fetchone()) con.close() + if not item: + logger.error(f'{item_id} eksisterer ikke.') + raise APIException(f'{item_id} eksisterer ikke.') return item @@ -210,7 +214,7 @@ def _update_last_seen(item_id: str) -> None: con.execute(sql, {'id': item_id}) con.commit() except sqlite3.IntegrityError: - print(f'{item_id} eksisterer ikke.') + logger.error(f'{item_id} eksisterer ikke.') finally: con.close() @@ -231,7 +235,7 @@ def register_out(item_id: str, userid: str, days: str = 1) -> None: logger.info(f'{item_id} er ikke lenger tilgjengelig.') except sqlite3.IntegrityError: logger.error(f'{item_id} eksisterer ikke.') - raise ValueError(f'{item_id} eksisterer ikke.') + raise APIException(f'{item_id} eksisterer ikke.') finally: con.close() _update_last_seen(item_id) @@ -245,7 +249,7 @@ def register_in(item_id: str) -> None: try: get(item_id) except TypeError: - raise ValueError(f'{item_id} eksisterer ikke i databasen.') + raise APIException(f'{item_id} eksisterer ikke.') con = sqlite3.connect(DATABASE) try: @@ -255,7 +259,7 @@ def register_in(item_id: str) -> None: logger.info(f'{item_id} er nå tilgjengelig.') except sqlite3.IntegrityError: logger.error(f'{item_id} eksisterer ikke.') - raise ValueError(f'{item_id} eksisterer ikke.') + raise APIException(f'{item_id} eksisterer ikke.') finally: con.close() _update_last_seen(item_id) diff --git a/BookingSystem/sanitizer.py b/BookingSystem/sanitizer.py new file mode 100644 index 0000000..53bfda7 --- /dev/null +++ b/BookingSystem/sanitizer.py @@ -0,0 +1,167 @@ +import re +from enum import Enum, auto +from functools import wraps + +import flask +from werkzeug.datastructures import ImmutableMultiDict + +import inventory +from __init__ import REGEX_ITEM, logger + + +class VALIDATORS(Enum): + ID = auto() + UNIQUE_ID = auto() + UNIQUE_OR_SAME_ID = auto() + NAME = auto() + CATEGORY = auto() + INT = auto() + LABEL_TYPE = auto() + ITEM_LIST_EXISTS = auto() + + +class APIException(Exception): + def __init__(self, message: str, status_code: int = 400) -> None: + super().__init__(message) + self.message = message + self.status_code = status_code + + +class MINMAX: + def __init__(self, mn: int, mx: int): + self.mn = mn + self.mx = mx + + def __iter__(self): + return iter((self.mn, self.mx)) + + def __str__(self): + return f'{self.mn}-{self.mx}' + + +def _sanitize_form(sanitization_map: dict[any: VALIDATORS | MINMAX], form, data: dict = dict) -> bool: + """Sanitize a form based on a sanitization map.""" + + def item_pattern(fkey: str) -> bool: + # Check if the ID/name is valid + r = re.compile(REGEX_ITEM) + return bool(r.match(form.get(fkey))) + + def unique(fkey: str) -> bool: + # Check if the ID is unique + l_val, l_ids = form.get(fkey).lower(), [i.lower() for i in inventory.get_all_ids()] + return l_val not in l_ids + + for key, sanitizer in sanitization_map.items(): + match sanitizer: + case VALIDATORS.ID | VALIDATORS.NAME: + # Check if the ID/name is valid + if not item_pattern(key): + logger.debug(f'Invalid item pattern for {key} ({form.get(key)})') + raise APIException(f'Ugyldig ID ({form.get(key)})') + + case VALIDATORS.UNIQUE_ID: + # Check if the ID is unique + if not unique(key): + logger.debug(f'Invalid unique id for {key} ({form.get(key)})') + raise APIException(f'{form.get(key)} er allerede i bruk.') + # Check if the ID is valid + if not item_pattern(key): + logger.debug(f'Invalid unique id for {key} ({form.get(key)})') + raise APIException(f'Ugyldig ID ({form.get(key)})') + + case VALIDATORS.UNIQUE_OR_SAME_ID: + # Check if the ID is unique or the same as the current ID + same_id = form.get(key).lower() == data.get(key).lower() + if not unique(key) and not same_id: + logger.debug(f'Invalid unique or same id for {key} ({form.get(key)})') + raise APIException(f'{form.get(key)} er allerede i bruk.') + # Check if the ID is valid + if not item_pattern(key): + logger.debug(f'Invalid unique or same id for {key} ({form.get(key)})') + raise APIException(f'Ugyldig ID ({form.get(key)})') + + case VALIDATORS.CATEGORY: + # Check if the category is valid + if form.get(key) not in inventory.all_categories(): + logger.debug(f'Invalid category for {key} ({form.get(key)})') + raise APIException(f'Ugyldig kategori ({form.get(key)})') + + case VALIDATORS.INT: + # Check if the value is an int + try: + int(form.get(key)) + except (ValueError, TypeError): + logger.debug(f'Invalid int for {key} ({form.get(key)})') + raise APIException(f'Ugyldig tallverdi ({form.get(key)})') + + case VALIDATORS.LABEL_TYPE: + # Check if the label type is valid + if form.get(key) not in ['barcode', 'qr']: + logger.debug(f'Invalid label type for {key} ({form.get(key)})') + raise APIException(f'Ugyldig etikett-type ({form.get(key)})') + + case VALIDATORS.ITEM_LIST_EXISTS: + # Check if the item list exists + ids = form.getlist(key) + all_ids = [i for i in inventory.get_all_ids()] + if not all(i in all_ids for i in ids): + logger.debug(f'Invalid item list for {key} ({form.getlist(key)})') + raise APIException(f'En eller flere gjenstander finnes ikke ({form.getlist(key)})') + + if key.endswith('_minmax'): + # Check if the value is between the min and max + mn, mx = sanitizer + if not mn <= int(form.get(key[:-7])) <= mx: + logger.debug(f'Invalid minmax for {key} ({form.get(key)})') + raise APIException(f'Tallverdien er ikke mellom {mn} og {mx} ({form.get(key)})') + + # Passed all checks! + logger.debug(f'Validated {key} ({sanitizer}, {form.get(key)})') + return True + + +def sanitize(validation_map: dict[any: VALIDATORS | MINMAX], + form: ImmutableMultiDict, + data: dict = dict) -> dict[str: any]: + """ + Validate a form based on a validation map, + return the same keys with form values if valid, else raise ValueError + + (Remove keys that end in _minmax from the returned dict) + """ + try: + _sanitize_form(validation_map, form, data) + sanitized = dict() + + for key, sanitizer in validation_map.items(): + # Remove keys that are not in the validation map + if key not in validation_map.keys(): + continue + # Remove keys that end in _minmax + if key.endswith('_minmax'): + continue + # Lists need to be handled as lists + if sanitizer == VALIDATORS.ITEM_LIST_EXISTS: + sanitized[key] = form.getlist(key) + continue + # Everything else can be handled as a single value + sanitized[key] = form.get(key) + + logger.debug(f'Sanitized, old form: {form}') + logger.debug(f'Sanitized, new form: {sanitized}') + return sanitized + except APIException as e: + logger.warning(f'Invalid form submitted: {form}') + raise APIException(e.message, e.status_code) + + +def handle_api_exception(f) -> callable: + @wraps(f) + def wrapper(*args, **kwargs): + try: + return f(*args, **kwargs) + except APIException as e: + return flask.Response(e.message, status=e.status_code) + + return wrapper diff --git a/BookingSystem/templates/inventar_add.html b/BookingSystem/templates/inventar_add.html index 2fb3a78..4389fa8 100644 --- a/BookingSystem/templates/inventar_add.html +++ b/BookingSystem/templates/inventar_add.html @@ -77,7 +77,7 @@

Her kan du legge til nytt inventar. Legg gjerne til flere, så forlater du b }, error: function (response) { iziToast.error({ - title: 'Ikke tilgjengelig', + title: 'Feil!', message: response.responseText, }) } diff --git a/BookingSystem/templates/inventar_edit.html b/BookingSystem/templates/inventar_edit.html index eda8485..8e135a1 100644 --- a/BookingSystem/templates/inventar_edit.html +++ b/BookingSystem/templates/inventar_edit.html @@ -54,7 +54,7 @@

Redigerer {{ item.id }} ({{ item.name }})

}, error: function (response) { iziToast.error({ - title: 'Ikke tilgjengelig', + title: 'Feil!', message: response.responseText, }); }