From 1a592419912d91e2385f33458fb77feec9d2c8d6 Mon Sep 17 00:00:00 2001 From: Florian Dold Date: Mon, 16 Aug 2021 16:10:55 +0200 Subject: address CG's FIXME comments --- contrib/merchant-backoffice | 2 +- src/backend/taler-merchant-httpd_get-orders-ID.c | 40 +++++++++++++----------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/contrib/merchant-backoffice b/contrib/merchant-backoffice index 03c8c9b7..4320467d 160000 --- a/contrib/merchant-backoffice +++ b/contrib/merchant-backoffice @@ -1 +1 @@ -Subproject commit 03c8c9b794905878175d07366267bdc01c3795b9 +Subproject commit 4320467db1392e5f48a4acd079f7e2a253cf9984 diff --git a/src/backend/taler-merchant-httpd_get-orders-ID.c b/src/backend/taler-merchant-httpd_get-orders-ID.c index 29a906c6..07093a9f 100644 --- a/src/backend/taler-merchant-httpd_get-orders-ID.c +++ b/src/backend/taler-merchant-httpd_get-orders-ID.c @@ -914,7 +914,7 @@ TMH_get_orders_ID (const struct TMH_RequestHandler *rh, { god->claimed = true; } - else if (! token_match) // FIXME: This 'if' seems unnecessary, 'token_match' would seem always 'false' here. + else { struct TALER_ClaimTokenP db_claim_token; struct GNUNET_HashCode unused; @@ -939,9 +939,7 @@ TMH_get_orders_ID (const struct TMH_RequestHandler *rh, TALER_EC_GENERIC_DB_FETCH_FAILED, "lookup_order"); } - // FIXME: simplify: contract_available is always FALSE here! - if ( (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS == qs) && - (! contract_available) ) + if (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS == qs) { GNUNET_log (GNUNET_ERROR_TYPE_INFO, "Unknown order id given: `%s'\n", @@ -951,11 +949,6 @@ TMH_get_orders_ID (const struct TMH_RequestHandler *rh, TALER_EC_MERCHANT_GENERIC_ORDER_UNKNOWN, order_id); } - // FIXME: given that contract_available is always - // false and thus qs always non-zero, the RVAL simplifies - // to 'false', which should already be the LVAL. - god->claimed = ( (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS == qs) || - (contract_available) ); token_match = (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT == qs) && (0 == GNUNET_memcmp (&db_claim_token, &god->claim_token)); @@ -983,21 +976,28 @@ TMH_get_orders_ID (const struct TMH_RequestHandler *rh, contract_available, contract_match, god->claimed); - /* FIXME: this should be strengthend: two conditions - (claim_token_provided && ! token_match) || - (h_contract_provided && ! contract_match) - in separate branches with more specific error - replies (about token or contract) */ - if ( (claim_token_provided || h_contract_provided) && - (! (token_match || contract_match)) ) + + if (claim_token_provided && ! token_match) + { + /* Authentication provided but wrong. */ + GNUNET_break_op (0); + return TALER_MHD_reply_with_error (connection, + MHD_HTTP_FORBIDDEN, + TALER_EC_MERCHANT_GET_ORDERS_ID_INVALID_TOKEN, + "authentication with claim token provided but wrong"); + } + + if (h_contract_provided && ! contract_match) { /* Authentication provided but wrong. */ GNUNET_break_op (0); + /* FIXME: use better error code */ return TALER_MHD_reply_with_error (connection, MHD_HTTP_FORBIDDEN, TALER_EC_MERCHANT_GET_ORDERS_ID_INVALID_TOKEN, - "authentication with h_contract or token provided but wrong"); + "authentication with h_contract provided but wrong"); } + if (! (token_match || contract_match) ) { @@ -1006,7 +1006,11 @@ TMH_get_orders_ID (const struct TMH_RequestHandler *rh, /* FIXME: taking this branch seems wrong for unclaimed orders without claim token! Also seems to contradict the spec, as there 'authOk' is defined to include the - || "! ord.requireClaimToken" part! */ + || "! ord.requireClaimToken" part! + + FD: when "!ord.requireClaimToken" and the client doens't provide + a claim token (already checked!), then token_match==TRUE. + */ GNUNET_log (GNUNET_ERROR_TYPE_INFO, "Neither claim token nor contract matched\n"); public_reorder_url = json_string_value (json_object_get ( -- cgit v1.2.3