From 327f37bb62cbe9c058c93c97ce8e8cc07f86d600 Mon Sep 17 00:00:00 2001 From: Marcello Stanisci Date: Fri, 31 May 2019 15:00:51 +0200 Subject: 5715. Initial change in the structure. Just porting the /history validation to use the 'form' API for validating GET arguments. Subsequent changes (for GET requests) will follow the same pattern. --- talerbank/app/middleware.py | 9 ++++- talerbank/app/schemas.py | 94 +++++++++++++++++++++++++++------------------ talerbank/app/views.py | 89 +++++++++++++++++++----------------------- 3 files changed, 104 insertions(+), 88 deletions(-) diff --git a/talerbank/app/middleware.py b/talerbank/app/middleware.py index 220fa94..dd1b3ac 100644 --- a/talerbank/app/middleware.py +++ b/talerbank/app/middleware.py @@ -8,7 +8,7 @@ from .views import \ LoginFailed, RejectNoRightsException) from .schemas import \ (URLParameterMissing, URLParameterMalformed, - JSONFieldException) + JSONFieldException, URLParamValidationError) from .amount import \ (CurrencyMismatch, BadFormatAmount, NumberTooBig, NegativeNumber) @@ -72,6 +72,13 @@ class ExceptionMiddleware: DebitLimitException: 3, URLParameterMissing: 8, URLParameterMalformed: 9, + + ## + # This one unified class kills the distinction + # between a parameter being missing or malformed. + # It simplifies the error handling though, and + # MUST be reflected in taler_error_codes.h! + URLParamValidationError: 9, JSONFieldException: 6, CurrencyMismatch: 11, BadFormatAmount: 11, diff --git a/talerbank/app/schemas.py b/talerbank/app/schemas.py index cf11bdc..b75ff6a 100644 --- a/talerbank/app/schemas.py +++ b/talerbank/app/schemas.py @@ -24,10 +24,66 @@ from validictory import validate from validictory.validator import \ (RequiredFieldValidationError, FieldValidationError) - from django.conf import settings +from django.core.exceptions import ValidationError +from django import forms +from django.core.validators import RegexValidator +## +# Exception class to be raised when at least one expected URL +# parameter is either not found or malformed. +class URLParamValidationError(ValidationError): + + ## + # Init method. + # + # @param self the object itself. + # @param param the missing URL parameter name. + # @param http_status_code the HTTP response code to return + # to the caller (client). + def __init__(self, error, http_status_code): + self.hint = ["%s: %s, " % (k, error[k]) for k in error] + self.http_status_code = http_status_code + super().__init__() + + +## +# Form specification that validates GET parameters from a +# /history request. +class HistoryParams(forms.Form): + auth = forms.CharField( + validators=[RegexValidator( + "^basic$", + message="Only 'basic' is allowed")]) + + cancelled = forms.CharField( + required=False, + empty_value="show", + validators=[RegexValidator( + "^(omit|show)$", + message="Only 'omit' or 'show' are valid")]) + + # FIXME: adjust min/max values. + delta = forms.IntegerField() + # FIXME: adjust min/max values. + start = forms.IntegerField(required=False) + + ordering = forms.CharField( + required=False, + empty_value="descending", + validators=[RegexValidator( + "^(ascending|descending)$", + message="Only 'ascending' or 'descending' are valid")]) + + direction = forms.CharField( + validators=[RegexValidator( + "^(debit|credit|both|cancel\+|cancel-)$", + message="Only: debit/credit/both/cancel+/cancel-")]) + + # FIXME: adjust min/max values. + account_number = forms.IntegerField(required=False) + ## # Exception class to be raised when a expected URL parameter # is not found. @@ -86,7 +142,6 @@ AMOUNT_SCHEMA = { "type": "string", "pattern": "^[A-Za-z0-9_-]+:([0-9]+)\.?([0-9]+)?$"} - ## # Definition that withdraw request bodies have to match. WITHDRAW_SESSION_SCHEMA = { @@ -172,32 +227,6 @@ HISTORY_RANGE_REQUEST_SCHEMA = { } -## -# Definition for /history request URL parameters. -HISTORY_REQUEST_SCHEMA = { - "type": "object", - "properties": { - "auth": {"type": "string", "pattern": "^basic$"}, - "cancelled": {"type": "string", - "pattern": "^(omit|show)$", - "required": False}, - "delta": {"type": "string", - "pattern": r"^([\+-])?([0-9])+$"}, - "start": {"type": "string", - "pattern": "^([0-9]+)$", - "required": False}, - "ordering": {"type": "string", - "pattern": r"^(ascending|descending)$", - "required": False}, - "direction": {"type": "string", - "pattern": r"^(debit|credit|both|cancel\+|cancel-)$"}, - "account_number": {"type": "string", - "pattern": "^([0-9]+)$", - "required": False} - } -} - - ## # Definition for /add/incoming request bodies. INCOMING_REQUEST_SCHEMA = { @@ -210,7 +239,6 @@ INCOMING_REQUEST_SCHEMA = { } } - ## # Definition for PIN/TAN request URL parameters. PIN_TAN_ARGS = { @@ -267,13 +295,6 @@ def validate_pin_tan(data): def validate_reject(data): validate(data, REJECT_REQUEST_SCHEMA) -## -# Check /history input data. -# -# @param data dict representing /history's GET parameters. -def validate_history(data): - validate(data, HISTORY_REQUEST_SCHEMA) - ## # Check /history-range input data. # @@ -319,7 +340,6 @@ def check_withdraw_session(data): def validate_data(request, data): switch = { "/reject": validate_reject, - "/history": validate_history, "/history-range": validate_history_range, "/admin/add/incoming": validate_add_incoming, "/pin/verify": check_withdraw_session, diff --git a/talerbank/app/views.py b/talerbank/app/views.py index f4af47c..3d1f7bc 100644 --- a/talerbank/app/views.py +++ b/talerbank/app/views.py @@ -44,7 +44,7 @@ from django.shortcuts import render, redirect from datetime import datetime from .models import BankAccount, BankTransaction from .amount import Amount -from .schemas import validate_data +from .schemas import validate_data, HistoryParams, URLParamValidationError LOGGER = logging.getLogger(__name__) ## @@ -259,7 +259,7 @@ def profile_page(request): account_no=request.user.bankaccount.account_no, wt_form=wtf, history=extract_history(request.user.bankaccount, - True), + -1 * (UINT64_MAX / 2 / 2)) ) if settings.TALER_SUGGESTED_EXCHANGE: context["suggested_exchange"] = settings.TALER_SUGGESTED_EXCHANGE @@ -459,12 +459,14 @@ def logout_view(request): # entries will be younger / older than @a start. # @return the history array. def extract_history(account, - descending, - delta=None, - start=UINT64_MAX, - sign="-"): + delta, + start=UINT64_MAX): history = [] - qs = query_history(account, "both", delta, start, sign, True) + qs = query_history(account, + "both", + delta, + start, + "descending") for item in qs: if item.credit_account == account: counterpart = item.debit_account @@ -514,9 +516,9 @@ def serve_public_accounts(request, name=None, page=None): "both", # Note: the parameter below is used for slicing arrays # and django/python is not allowing slicing with big numbers. - (UINT64_MAX / 2 ) / 2, - start=0, - sign="+").count() + UINT64_MAX / 2 / 2, + 0, + "descending").count() DELTA = 30 # '//' operator is NO floating point. num_pages = max(num_records // DELTA, 1) @@ -525,10 +527,8 @@ def serve_public_accounts(request, name=None, page=None): # Retrieve DELTA records younger than 'start_row' (included). history = extract_history(user.bankaccount, - True, DELTA * page, - 0, - "+")[DELTA * (page - 1):(DELTA * page)] + 0)[DELTA * (page - 1):(DELTA * page)] pages = list(range(1, num_pages + 1)) @@ -636,34 +636,31 @@ def query_history_range(bank_account, # * cancel-: only entries where the querying user cancelled # the _paying_ of money will be returned. # @param delta how many history entries will be contained in the -# array (will be passed as-is to the internal routine -# @a query_history). +# array. # @param start any history will be searched starting from this # value (which is a row ID), and going to the past or to # the future (depending on the user choice). However, this # value itself will not be included in the history. # @param sign this value ("+"/"-") determines whether the history # entries will be younger / older than @a start. -# @param descending if True, then the results will have the -# youngest entry in the first position. +# @param ordering "descending" or anything else (for "ascending"). def query_history(bank_account, direction, delta, start, - sign, - descending=True): + ordering): - sign_filter = { - "+": Q(id__gt=start), - "-": Q(id__lt=start), - } + def sign_filter(delta): + if 0 > delta: + return Q(id__lt=start) + return Q(id__gt=start) qs = BankTransaction.objects.filter( direction_switch(bank_account, direction), - sign_filter.get(sign)) + sign_filter(delta)) - order = "-id" if descending else "id" - return qs.order_by(order)[:delta] + order = "-id" if "descending" == ordering else "id" + return qs.order_by(order)[:abs(delta)] ## @@ -733,8 +730,6 @@ def serve_history_range(request, user_account): return HttpResponse(status=204) return JsonResponse(dict(data=history), status=200) - - ## # Serve a request of /history. # @@ -744,33 +739,27 @@ def serve_history_range(request, user_account): @require_GET @login_via_headers def serve_history(request, user_account): - validate_data(request, request.GET.dict()) + get_params = HistoryParams(request.GET.dict()) + if not get_params.is_valid(): + raise URLParamValidationError(get_params.errors, 400) - # delta (it does exist: enforced by the check above.) - parsed_delta = re.search(r"([\+-])?([0-9]+)", - request.GET.get("delta")) - # normalize the sign. - sign = parsed_delta.group(1) - sign = sign if sign else "+" + delta = get_params.cleaned_data.get("delta") + start = get_params.cleaned_data.get("start") - # Ordering. - ordering = request.GET.get("ordering", "descending") - start = request.GET.get("start") - - if not start: - start = 0 if "+" == sign else UINT64_MAX + if None == start: + start = 0 if 0 <= delta else UINT64_MAX qs = query_history(user_account.bankaccount, - request.GET.get("direction"), - int(parsed_delta.group(2)), - int(start), - sign, - "descending" == ordering) + get_params.cleaned_data.get("direction"), + delta, + start, + get_params.cleaned_data.get("ordering")) + + history = build_history_response( + qs, + get_params.cleaned_data.get("cancelled"), + user_account) - history = build_history_response(qs, - request.GET.get("cancelled", - "show"), - user_account) if not history: return HttpResponse(status=204) return JsonResponse(dict(data=history), status=200) -- cgit v1.2.3