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

change collection link to use the StatusList2021 collection #629

Conversation

PatStLouis
Copy link
Contributor

This PR is related to #482. The interoperability reporting GA currently tests the RevocationList2020, I would like to propose a change test revocation with StatusList2021 instead.

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

@OR13 OR13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its better to hold off on this, until we can support StatusList2021 and JWTs in these tests.

If you are willing to do that work, it would be greatly appreciated.

See related PR: #628

@nissimsan
Copy link
Collaborator

The update corresponds with the path to the path to the collection. This is an oversight from whenever we updated the collection. Even if outdated, this bug should still be fixed.

@OR13
Copy link
Collaborator

OR13 commented Mar 12, 2024

@nissimsan fixing a broken path, to tests that do not match current requirements, does not help demonstrate conformance.

My change request, stands, please fix all of https://github.com/w3c-ccg/traceability-interop/tree/main/docs/tutorials/credentials-status-update

Or don't register a test that is misleading.

@PatStLouis
Copy link
Contributor Author

There are conformance tests available for both RevocationList2020 and StatusList2021. There are also interoperability tests available for both RevocaitonList2020 and StatusList2021. However the interoperability reporting matrix uses the RevocationList2020 tests for revocation interop.

The current traceability interop spec states that:

Any system conforming with this specification for interoperability MUST support Status List 2021 to handle revocation and status tracking for Verifiable Credentials

This PR is just to change which of the available test collections will be used for revocation interoperability reporting without additional work/tests needed. I would like to drop support for RevocationList2020 in my implementation while still matching the interoperability requirements.

Regarding #628, there is mention of the data model 2.0, BitstringStatusList and JWT-VC. I would be happy to contribute to these test efforts in due time but is out of scope for my goal with this PR.

This being said, I'm also ok with leaving the tests as-is and keep support for RevocationList2020 if need be.

@OR13
Copy link
Collaborator

OR13 commented Mar 12, 2024

@PatStLouis Thanks for raising the PR.

As you can see from https://github.com/w3c-ccg/traceability-interop/tree/main/docs/tutorials/credentials-status-update

There is more work to do, in order to support interop tests on this than just registering the test suite.

If you prefer to tackle the requirements support in a separate PR, I would also be fine modifying this one to remove the test that does not match requirements.

I also question the general utility of testing interop for this endpoint, since its an internal implementation detail.

I think the best solution in the short term would be to delete the test and associated content entirely.

cc @mprorock @mkhraisha

@PatStLouis
Copy link
Contributor Author

I believe having a revocation interoperability test is important as this is the best way to ensure each participant has implemented the StatusList2021 properly (or whichever other status method is being implemented). If I'm only testing revocation against my own implementation, I might be able to verify the status of my own credentials without knowing I made a wrongful implementation, specially around the encoding/compression algorithm for the bitstring list. It's important to asses if implementationA can verify the status changes carried by implementationB, same way it's important to verify the proofs of issued VC across implementations.

A prime example of this is the vc-api issuer and verifier interop test suite reports. Even if each implementation passes the tests, yet when I try to verify a credential I issue across a range of the verifiers, half of them return a failed verification. The main goal of interoperability should be to test implementation to implementation interactions.

There might be another way to achieve this without testing this endpoint directly, but until such a method exist I would vote to keep this in as this is the most practical way for the time being.

I will have a look at these 2 test collections to see the required changes to make the latter suitable for interop testing and assess if I have the bandwidth.

Thank you for the comments/clarifications

@PatStLouis
Copy link
Contributor Author

@OR13 I had a comparison of both the revocation and status update collections and I can't see why the revocation collection is suitable for interop testing but the status update collection isn't. Could you provide more information as why you believe this to be the case?

@OR13
Copy link
Collaborator

OR13 commented Mar 22, 2024

I don't think we should host tests for either here. The reason is that these are internal APIs, so we are simply forcing vendors to align on APIs interfaces which they do not actually need to be interoperable.

The get status endpoint does not interoperability, since verifiers occasionally so make network requests cross vendor to verify things.

@OR13
Copy link
Collaborator

OR13 commented Mar 22, 2024

I approved, but I hope we can remove everything that is not essential before the next round of testing.

@PatStLouis
Copy link
Contributor Author

@OR13 my goal with this PR is to remove conformance/interop tests relating to RevocationList2020 so implementations aren't required to support it for testing. It's a step in that direction.

I'm interested in discussing how should testing be done moving forward. This deserves a separate issue to be addressed.

I agree with you that maybe we shouldn't host conformance tests for these endpoints, but I believe the interop tests are a must. There needs to be a way to test:

  • ImplementationA issues a VC
  • ImplementationB verifies the VC
  • ImplementationA updates the VC status
  • ImplementationB verifies the VC and confirms status update

All this needs to be orchestrated by a test-client simulating administrative operations (Postman). I agree some implementations might not have a POST /credentials/status endpoint in their production use cases for managing status, however they should provide such an endpoint for interoperability testing. The Aries agent test harness provides a similar api component referred to as a backchannel, which goal is to forward test payload to the component under test (in this case being the status management service). Since the vc-api has been chosen as the interface for test-suites to interact with, it should serve a similar purpose.

Maybe we can start by drafting a list of what are considered essential tests and do a cleanup exercise of the postman collection. I noticed a few dead links in some of the readme.

@OR13
Copy link
Collaborator

OR13 commented Mar 25, 2024

@PatStLouis I don't agree that we need to maintain tests for "internal endpoints"... because they are "internal".

I do agree that we need to test interoperability of credentials that have suspension and revocation abilities.

Vendor A presents suspended credential to Vendor B... what happens?
Vendor A presents revoked credential to Vendor B... what happens?

Vendor A might internally call "issue"
Vendor B might internally call "verify"

... but interop on these internal APIs is ... not relevant to the cross trust domain interop... and those internal APIs might be accomplished without ever implementing anything in HTTP... by testing internal APIs alongside external ones, we are muddying the waters on security, and creating more confusion.

Regardless, I approved this PR, because the place to fix is is a separate PR.

@TallTed
Copy link
Contributor

TallTed commented Mar 25, 2024

We have previously agreed that by design, there is no such thing as an "internal endpoint". Deployment might indeed put an endpoint inside a VPN or similar, such that authn and authz are unnecessary, but the design and spec have to include both authn and authz, such that the endpoint can be placed outside the VPN and supports both authn and authz even internally, where the deployer may indeed want to restrict use of that endpoint to specific users/clients which the deployer may define however they like.

@mkhraisha mkhraisha merged commit 20af507 into w3c-ccg:main Apr 2, 2024
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.

None yet

5 participants