Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add did web method type as a default option #2684

Merged

Conversation

PatStLouis
Copy link
Contributor

Addresses issue #2667.

Inspired by the did methods documentation.

Signed-off-by: pstlouis <patrick.st-louis@opsecid.ca>
@swcurran swcurran requested a review from dbluhm December 18, 2023 23:58
@swcurran
Copy link
Contributor

@PatStLouis — please check the Lint test failure.

@dbluhm — what do you think of this? OK as a start?

Signed-off-by: pstlouis <patrick.st-louis@opsecid.ca>
@PatStLouis
Copy link
Contributor Author

I tested issuance/verification in both single tenant/multitenant modes.

I had a strange error with verification at first but I'm unable to reproduce it....verification works now using a did:web issuer...I'll keep an eye out, here's the error the verification endpoint was returning
('Could not expand input before framing.',)\nType: jsonld.FrameError\nCause: ('Tried to nullify a context with protected terms outside of a term definition.',)\nType: jsonld.SyntaxError\nCode: invalid context nullification File \"/home/aries/.local/lib/python3.9/site-packages/pyld/jsonld.py\", line 1032, in frame\n expanded = self.expand(input_, options)\n File \"/home/aries/.local/lib/python3.9/site-packages/pyld/jsonld.py\", line 870, in expand\n expanded = self._expand(active_ctx, None, document, options,\n File \"/home/aries/.local/lib/python3.9/site-packages/pyld/jsonld.py\", line 2329, in _expand\n self._expand_object(\n File \"/home/aries/.local/lib/python3.9/site-packages/pyld/jsonld.py\", line 2728, in _expand_object\n expanded_value = self._expand(\n File \"/home/aries/.local/lib/python3.9/site-packages/pyld/jsonld.py\", line 2233, in _expand\n e = self._expand(\n File \"/home/aries/.local/lib/python3.9/site-packages/pyld/jsonld.py\", line 2324, in _expand\n active_ctx = self._process_context(\n File \"/home/aries/.local/lib/python3.9/site-packages/pyld/jsonld.py\", line 3076, in _process_context\n raise JsonLdError(\n"

@PatStLouis
Copy link
Contributor Author

I think I found the issue, it seems to be related with the traceability context url. Trying to verify a vc issued by a did:web issuer who has this context included in it's did document seems to raise this strange error. Here's a vc to test (can be verified with other verification services such as the univerifier). Verifying the same vc without the traceability context returns true in aca-py.

{
        "@context": [
            "https://www.w3.org/2018/credentials/v1"
        ],
        "id": "urn:uuid:31288134-1c6a-42a2-aa37-c5c38dea09c0",
        "type": [
            "VerifiableCredential"
        ],
        "issuer": "did:web:PatStLouis.github.io",
        "issuanceDate": "2023-12-19T01:13:13.401Z",
        "credentialSubject": {
            "id": "did:example:123"
        },
        "proof": {
            "type": "Ed25519Signature2018",
            "proofPurpose": "assertionMethod",
            "verificationMethod": "did:web:PatStLouis.github.io#verkey",
            "created": "2023-12-19T01:13:13.401Z",
            "jws": "eyJhbGciOiAiRWREU0EiLCAiYjY0IjogZmFsc2UsICJjcml0IjogWyJiNjQiXX0..ahIwxPaTgLmUS4aS9nFApVQmx-uklAAR0I-W_-IKzhvMQFmCl0g7etQw3DG8oKqVs-0hlewuCtUxKZGO1GyyBw"
        }
    }

@swcurran
Copy link
Contributor

That is quite an @context file!!

ACA-Py is now pre-loading contexts — I wonder if that is the issue. This was recently added in PR #2587 that is part of 0.11.0. It might be worth trying 0.10.5 to see if the same issue occurs. Given the pre-loading of context, I’m not sure how contexts that are not pre-loaded are handled. I assume they are loaded dynamically, but that is not considered “best practice” in JSON-LD land, so perhaps not. Which of course leads to the question, how does one add ones own contexts to an instance of ACA-Py?

@PatStLouis
Copy link
Contributor Author

I tried adding and caching the context in the document loader as well and get the same result. Also putting the agent in debug mode doesn't log any errors so this seem unhandled from the pyld package. I'm going to try and find out where in the code this verification fails. Now the did core spec recommends having a context file for your services, its not required so this won't prevent any test cases AFAIK. The issue raised about the service.type being an array does get tested. I'll have a look at the PyDID library like @dbluhm suggested to understand the impact.

// Service array is used to look up traceability API service endpoint
pm.test("Response must include 'service' array", function() {
    const { service } = pm.response.json().didDocument;
    pm.expect(service).to.be.an('array').that.is.not.empty;
});

@swcurran
Copy link
Contributor

swcurran commented Jan 3, 2024

@PatStLouis -- can you please check on the conflicts?

Signed-off-by: Patrick St-Louis <patrick.st-louis@idlab.org>
Copy link

sonarcloud bot commented Jan 4, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@PatStLouis
Copy link
Contributor Author

fixed the conflict between the did:peer and did:web wallet methods

@swcurran swcurran merged commit 7ea0f19 into openwallet-foundation:main Jan 4, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants