address CG's FIXME comments

2 files changed, 22 insertions, 18 deletions

diff --git a/contrib/merchant-backoffice b/contrib/merchant-backoffice
-Subproject 03c8c9b794905878175d07366267bdc01c3795b
+Subproject 4320467db1392e5f48a4acd079f7e2a253cf998
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 (