View Issue Details

IDProjectCategoryView StatusLast Update
0006785Talermechant backendpublic2021-09-02 18:23
ReporterFlorian Dold Assigned ToChristian Grothoff  
PrioritynormalSeverityminorReproducibilityhave not tried
Status closedResolutionfixed 
Product Versiongit (master) 
Target Version0.8Fixed in Version0.8 
Summary0006785: instance management bugs (merchant-instances test)
DescriptionThere are two further bugs exposed by the merchant-instances test:

$ taler-wallet-cli testing run-integrationtests merchant-instances

First, http://localhost:8083/private/instances/default currently gives 404, when it should return the instance details of "default".

Second, DELETE http://localhost:8083/private/instances/myinst does *not* correctly check that the request is authenticated as the *default* user.
TagsNo tags attached.

Activities

Christian Grothoff

2021-03-04 14:00

manager   ~0017599

Last edited: 2021-03-04 15:58

First is wrong, you need to request: /instances/default/private

Second, DELETE http://localhost:8083/private/instances/myinst -- that should really already authenticate against the default instance. Could it be that you do have _any_ credentials on the default instance? Because if default instance access is open, it will go through regardless of you suppling a token or not.

=> Florian clarified to me what is wrong, and he is right. Fixing now.

sebasjm

2021-03-04 15:43

developer   ~0017600

I found this too and I think that it MAY be related with the creation of the instance.
In the script Im attaching, it send a request with auth method = token but the instance is created with auth method = external.
The setting /auth works ok, tho.
script-create.sh (1,081 bytes)   
#/bin/bash

ID=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 16 | head -n 1)

echo create instance with id $ID
#HOST=https://backend.demo.taler.net
#AUTH="ApiKey sandbox"
#CURRENCY=KUDOS
HOST=http://localhost:9966
AUTH="Bearer secret-token:super_secret"
CURRENCY=COL

set -x
curl -q $HOST/private/instances     -H "Authorization: $AUTH" --data-raw '{"default_wire_transfer_delay":{"d_ms":2000},"auth": { "method": "token", "token":"secret-token:asd" },"default_pay_delay":{"d_ms":1000},"jurisdiction":{},"address":{},"default_wire_fee_amortization":10,"default_max_wire_fee":"'$CURRENCY':2","default_max_deposit_fee":"'$CURRENCY':1","payto_uris":[],"name":"qwe","id":"'$ID'"}'

echo should be token but is external

curl -q $HOST/private/instances/$ID | jq '.auth'

echo setting into token

curl -q $HOST/private/instances/$ID/auth -d '{"method":"token","token":"secret-token:asd"}'

echo should fail

curl -q $HOST/private/instances/$ID | jq '.auth'

echo should get method token

curl -q $HOST/private/instances/$ID -H "Authorization: Bearer secret-token:asd" | jq '.auth'


script-create.sh (1,081 bytes)   

Christian Grothoff

2021-03-04 16:03

manager   ~0017601

1ca25d6..2cb5982 _MIGHT_ fix this, undertested.

Florian Dold

2021-03-04 16:16

manager   ~0017602

Your fix now triggers a *different* assertion, which previously passed:

After changing authentication from "external" to "token" on the default instance, I can still make a request to

  http://localhost:8083/private/instances

*without* providing any authorization header.

Christian Grothoff

2021-03-06 12:57

manager   ~0017604

I see this now (after 8e540b3..01a0946):

FATAL: test failed with exception AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:

401 !== 403

    at GlobalTestState.assertDeepEqual (/home/grothoff/lib/taler-wallet-cli/node_modules/taler-wallet-cli/dist/taler-wallet-cli.js:35137:16)
    at /home/grothoff/lib/taler-wallet-cli/node_modules/taler-wallet-cli/dist/taler-wallet-cli.js:42312:15
    at Generator.next (<anonymous>)
    at fulfilled (/home/grothoff/lib/taler-wallet-cli/node_modules/taler-wallet-cli/dist/taler-wallet-cli.js:128:58)
    at processTicksAndRejections (internal/process/task_queues.js:97:5) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 401,
  expected: 403,
  operator: 'deepStrictEqual'
}

I think you incorrectly expect a 403, the 401 seems right.

Florian Dold

2021-03-08 11:44

manager   ~0017607

Indeed both 401 and 403 would both work here (though 403 is technically more accurate if you are already authenticated, but the principal that you are authenticated as does not have the right permissions).

However, the integration test right now fails for me even before it gets to this part. Maybe something that happened in commit "01a094683c841" of the merchant?

After the test (successfully!) creates the default instance with "external" authentication, the

  GET http://localhost:8083/private/instances

request now fails with code 26, "secret-token:' prefix missing in 'Authorization' header".

Note that the test case specifically gives a bogus "Authorization: 'foo bar-baz'" header, which simulates a left-over authorization header from the external authentication mechanism.

We specifically discussed that this should be allowed!

(BTW: The way that the wallet test cases are currently reporting request errors is rather bad, but I'm working on that!)

Christian Grothoff

2021-03-09 22:54

manager   ~0017608

ef0ccea3ca567eb45b6c41359d5f86dc01ff7395 should fix the last point. Alas, the test still fails on some 'delete' operation now :-(.

Florian Dold

2021-03-10 10:30

manager   ~0017609

For me, something different happens:

* The authentication for the default instance gets changed from "token" to "external" (via POST /auth).
* The test case now expects the GET /private/instances with the old client (that uses external/no auth) to *fail*
* Instead, that request succeeds!

Christian Grothoff

2021-04-18 02:25

manager   ~0017760

I added some /auth C tests today, plus some fixes related to this. Could you please check if everything works now as you expect it to?

sebasjm

2021-04-18 22:48

developer   ~0017767

looks good for me

Christian Grothoff

2021-09-02 18:23

manager   ~0018356

Fix committed to master branch.

Related Changesets

merchant: master 2cb59820

2021-03-04 17:02

Christian Grothoff


Details Diff
fix 0006785 Affected Issues
0006785
mod - src/backend/taler-merchant-httpd.c Diff File
mod - src/backend/taler-merchant-httpd_private-delete-instances-ID.c Diff File
mod - src/backend/taler-merchant-httpd_private-delete-instances-ID.h Diff File
mod - src/backend/taler-merchant-httpd_private-get-instances-ID.c Diff File
mod - src/backend/taler-merchant-httpd_private-get-instances-ID.h Diff File
mod - src/backend/taler-merchant-httpd_private-patch-instances-ID.c Diff File
mod - src/backend/taler-merchant-httpd_private-patch-instances-ID.h Diff File
mod - src/backend/taler-merchant-httpd_private-post-instances-ID-auth.c Diff File
mod - src/backend/taler-merchant-httpd_private-post-instances-ID-auth.h Diff File

merchant: master ef0ccea3

2021-03-09 23:48

Christian Grothoff


Details Diff
Fix 0006785-0017607 Affected Issues
0006785
mod - src/backend/taler-merchant-httpd.c Diff File

Issue History

Date Modified Username Field Change
2021-03-04 13:46 Florian Dold New Issue
2021-03-04 13:46 Florian Dold Status new => assigned
2021-03-04 13:46 Florian Dold Assigned To => Christian Grothoff
2021-03-04 14:00 Christian Grothoff Note Added: 0017599
2021-03-04 14:00 Christian Grothoff Assigned To Christian Grothoff => Florian Dold
2021-03-04 14:00 Christian Grothoff Status assigned => feedback
2021-03-04 15:43 sebasjm Note Added: 0017600
2021-03-04 15:43 sebasjm File Added: script-create.sh
2021-03-04 15:58 Christian Grothoff Assigned To Florian Dold => Christian Grothoff
2021-03-04 15:58 Christian Grothoff Status feedback => assigned
2021-03-04 15:58 Christian Grothoff Note Edited: 0017599
2021-03-04 16:03 Christian Grothoff Note Added: 0017601
2021-03-04 16:16 Florian Dold Note Added: 0017602
2021-03-06 12:57 Christian Grothoff Note Added: 0017604
2021-03-06 12:57 Christian Grothoff Assigned To Christian Grothoff => Florian Dold
2021-03-08 11:44 Florian Dold Note Added: 0017607
2021-03-09 22:54 Christian Grothoff Note Added: 0017608
2021-03-10 10:30 Florian Dold Note Added: 0017609
2021-04-18 02:25 Christian Grothoff Note Added: 0017760
2021-04-18 22:48 sebasjm Note Added: 0017767
2021-04-19 00:04 Christian Grothoff Status assigned => resolved
2021-04-19 00:04 Christian Grothoff Resolution open => fixed
2021-04-19 00:04 Christian Grothoff Fixed in Version => 0.9
2021-04-19 00:04 Christian Grothoff Product Version => git (master)
2021-04-19 00:04 Christian Grothoff Target Version => 0.9
2021-07-30 13:57 Christian Grothoff Fixed in Version 0.9 => 0.8.1
2021-07-30 13:59 Christian Grothoff Target Version 0.9 => 0.8.1
2021-07-30 14:01 Christian Grothoff Fixed in Version 0.8.1 => 0.8
2021-07-30 14:02 Christian Grothoff Target Version 0.8.1 => 0.8
2021-08-24 16:23 Christian Grothoff Status resolved => closed
2021-09-02 18:22 Christian Grothoff Changeset attached => Taler-merchant master ef0ccea3
2021-09-02 18:22 Christian Grothoff Changeset attached => Taler-merchant master 2cb59820
2021-09-02 18:23 Christian Grothoff Note Added: 0018356
2021-09-02 18:23 Christian Grothoff Assigned To Florian Dold => Christian Grothoff