bug: The returned presentation id is not verifiable

verifiable-presentation / impl

An example of implementation of the Veriifiable Presentation Generation Service specification.

5 stars 1 forks source link

bug: The returned presentation id is not verifiable #5

Open AbhijeetFasate13 opened 2 years ago

AbhijeetFasate13 commented 2 years ago

Is there an existing issue for this?

[X] I have searched the existing issues

What is the bug related to?

Registry

Current Behavior/Issue

The presentation id returned is in the form did:web:localhost%3A9297:presentations:example-id which is a valid id , but returns the error A presentation with the specified ID does not exist.

Expected Behavior

The presentation must be returned as verified if valid id is entered.

Steps To Reproduce

No response

Environment

- OS: Fedora Linux 35
- Node:16.17.1
- Docker:Docker version 20.10.18, build b40c2f6

Anything else?

If we modify the localhost%3A9297 part as localhost%253A9297 in the id then the presentation is verified properly

gamemaker1 commented 2 years ago

This may be related to URL encoding of the %3A part in did:web:localhost%3A9297:presentations:example-id

Could you try replacing the %3A part with %253A?

gamemaker1 commented 2 years ago

If we modify the localhost%3A9297 part as localhost%253A9297 in the id then the presentation is verified properly

Yes, it's because while making the request httpie automatically percent-encodes the URL and converts %3A to a :. However, the ID needs to include %3A to be a valid DID - once you percent-encode the %3A part, it passes the ID to the server properly.

aabidk20 commented 2 years ago

once you percent-encode the %3A part, it passes the ID to the server properly

Is manual encoding required every time or it can be improved?

gamemaker1 commented 2 years ago

Is manual encoding required every time or it can be improved?

This issue can't be "fixed" on the server side, it's just that we are passing a valid percent-encoded symbol to the server, and fastify (the server framework) decodes it for us instead of keeping it as-is (which is intended and correct behaviour).

Also, putting in a manual check to change localhost:9297 to localhost%3A9297 in the code here:

https://github.com/verifiable-presentation/impl/blob/main/modules/registry/source/handlers/presentations.ts#L24

is not an acceptable solution as it changes the entity the DID refers to:

did:web:localhost:9297:presentations:123 refers to a DID document at http://localhost/9297/presentations/123/did.json
did:web:localhost%3A9297:presentations:123 refers to a DID document at http://localhost:9297/presentations/123/did.json.

aabidk20 commented 2 years ago

Thanks, that made it clear. We should keep this issue open for now.