View Issue Details

IDProjectCategoryView StatusLast Update
0006785Talermerchant backendpublic2021-09-02 16:23
ReporterFlorian Dold Assigned ToChristian Grothoff Resolution deadline
ImplementerReviewer 
Release signed byDeployed by 
VerifierCommunicator 
Data protection authority notification requiredNo  
CSIRT notification requiredNo  
Estimated effort (hours)Actual effort (hours)0 
Fast-trackNo
Work packagenone
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.

Required notifications

Address or linkRemarkAdded byNotifiedActions

Activities

Christian Grothoff

2021-03-04 13:00

manager   ~0017599

Last edited: 2021-03-04 14: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 14: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 15:03

manager   ~0017601

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

Florian Dold

2021-03-04 15: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 11: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 10: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 21:54

manager   ~0017608

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

Florian Dold

2021-03-10 09: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 00: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 20:48

developer   ~0017767

looks good for me

Christian Grothoff

2021-09-02 16:23

manager   ~0018356

Fix committed to master branch.

Issue History

Date ModifiedUsernameFieldChange
2021-03-04 12:46Florian DoldNew Issue
2021-03-04 12:46Florian DoldStatusnew => assigned
2021-03-04 12:46Florian DoldAssigned To => Christian Grothoff
2021-03-04 13:00Christian GrothoffNote Added: 0017599
2021-03-04 13:00Christian GrothoffAssigned ToChristian Grothoff => Florian Dold
2021-03-04 13:00Christian GrothoffStatusassigned => feedback
2021-03-04 14:43sebasjmNote Added: 0017600
2021-03-04 14:43sebasjmFile Added: script-create.sh
2021-03-04 14:58Christian GrothoffAssigned ToFlorian Dold => Christian Grothoff
2021-03-04 14:58Christian GrothoffStatusfeedback => assigned
2021-03-04 14:58Christian GrothoffNote Edited: 0017599
2021-03-04 15:03Christian GrothoffNote Added: 0017601
2021-03-04 15:16Florian DoldNote Added: 0017602
2021-03-06 11:57Christian GrothoffNote Added: 0017604
2021-03-06 11:57Christian GrothoffAssigned ToChristian Grothoff => Florian Dold
2021-03-08 10:44Florian DoldNote Added: 0017607
2021-03-09 21:54Christian GrothoffNote Added: 0017608
2021-03-10 09:30Florian DoldNote Added: 0017609
2021-04-18 00:25Christian GrothoffNote Added: 0017760
2021-04-18 20:48sebasjmNote Added: 0017767
2021-04-18 22:04Christian GrothoffStatusassigned => informing
2021-04-18 22:04Christian GrothoffResolutionopen => fixed
2021-04-18 22:04Christian GrothoffFixed in Version => 0.9
2021-04-18 22:04Christian GrothoffProduct Version => git (master)
2021-04-18 22:04Christian GrothoffTarget Version => 0.9
2021-07-30 11:57Christian GrothoffFixed in Version0.9 => 0.8.1
2021-07-30 11:59Christian GrothoffTarget Version0.9 => 0.8.1
2021-07-30 12:01Christian GrothoffFixed in Version0.8.1 => 0.8
2021-07-30 12:02Christian GrothoffTarget Version0.8.1 => 0.8
2021-08-24 14:23Christian GrothoffStatusinforming => closed
2021-09-02 16:22Christian Grothoff
2021-09-02 16:22Christian Grothoff
2021-09-02 16:23Christian GrothoffNote Added: 0018356
2021-09-02 16:23Christian GrothoffAssigned ToFlorian Dold => Christian Grothoff
2025-03-22 13:09Christian GrothoffCategorymechant backend => merchant backend