View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0006785 | Taler | mechant backend | public | 2021-03-04 13:46 | 2021-09-02 18:23 |
Reporter | Florian Dold | Assigned To | Christian Grothoff | ||
Priority | normal | Severity | minor | Reproducibility | have not tried |
Status | closed | Resolution | fixed | ||
Product Version | git (master) | ||||
Target Version | 0.8 | Fixed in Version | 0.8 | ||
Summary | 0006785: instance management bugs (merchant-instances test) | ||||
Description | There 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. | ||||
Tags | No tags attached. | ||||
|
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. |
|
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' |
|
1ca25d6..2cb5982 _MIGHT_ fix this, undertested. |
|
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. |
|
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. |
|
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!) |
|
ef0ccea3ca567eb45b6c41359d5f86dc01ff7395 should fix the last point. Alas, the test still fails on some 'delete' operation now :-(. |
|
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! |
|
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? |
|
looks good for me |
|
Fix committed to master branch. |
merchant: master 2cb59820 2021-03-04 17:02 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 Details Diff |
Fix 0006785-0017607 |
Affected Issues 0006785 |
|
mod - src/backend/taler-merchant-httpd.c | Diff File |
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 |