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

Revocable indicator #211

Closed
wants to merge 230 commits into from
Closed

Revocable indicator #211

wants to merge 230 commits into from

Conversation

nissimsan
Copy link
Contributor

Parameter to indicate credential revocability.

kimdhamilton and others added 30 commits February 13, 2018 08:56
* Created architecture document file.

* Added architecture image.

* Added vc-issuer-http-api.drawio

* Update vc-issuer-http-api.drawio

* Added vc-issuer-http-api.png

* Added component descriptions

* Finished up the first draft for the PR.

* Updated architecture.md based on PR feedback and open issues.
Known issues:
  no Issuer API -> DB interface defined.
  Holder -> Issuer API interface not defined.
  Missing logic around ZKP proof API requirements.

* updated PNG image from draw.io
* Define versioning scheme

* Fixed link to releases
* Define versioning scheme

* Fixed link to releases
* Add mandatory non-composition based API
* Make all other APIs optional

Co-authored-by: Manu Sporny <msporny@digitalbazaar.com>
Co-Authored-By: Markus Sabadello <markus@danubetech.com>
Co-Authored-By: Markus Sabadello <markus@danubetech.com>
Co-Authored-By: Markus Sabadello <markus@danubetech.com>
@@ -42,6 +42,10 @@ components:
type:
type: string
description: The type of credential status to issue the credential with
revocable:
type: string
enum: ["revocable", "non-revocable"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just a true/false value here?

Copy link
Contributor

@msporny msporny Jun 29, 2021

Choose a reason for hiding this comment

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

How is this intended to work with credentialStatus -- doesn't that indicate whether or not the VC is revocable? For example: https://github.com/w3c-ccg/vc-http-api/pull/211/files#L53 ... I think we already have this feature:

 "credentialStatus": { "type": "RevocationList2020Status" }

^^^ that indicates that the VC is revocable.

Copy link
Contributor

Choose a reason for hiding this comment

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

right now, you are required to submit a credential with a member of the following form:

    "credentialStatus": {
      "id": "https://issuer.oidp.uscis.gov/credentials/status/3#94567",
      "type": "RevocationList2020Status",
      "revocationListIndex": "0",
      "revocationListCredential": "https://w3c-ccg.github.io/vc-http-api/fixtures/revocationList.json"
    },

perhaps it would be better to define this shape, and note that when it is present the credential MUST have an id.

Using the API today, you can issue revocable credentials, you just need to know how to hand crafter perfect JSON-LD to do it...

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the question this raises, is... is the issuer really doing issuance, if they are not checking that the credentialStatus is wellformed? why are folks accepting / issuing vc's with credentialStatus that point to a CCG github repo?

I would expect an issuer to maintain their own list, or at the very least validate this entry from a schema perspective.

Copy link
Contributor

@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.

Prefer to see the data shape of a "revocable vc" defined in json schema and referenced, we don't appear too have consensus on how to "issue a revocable VC", but its possible to document how its done today as a first step... and to be clear, the only way to do this today, is to hand craft the VC payload and call the issue API.

Copy link
Contributor

@msporny msporny left a comment

Choose a reason for hiding this comment

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

This is related to #204 -- at the very least, we need to determine how the client tells the issuer that they 1) manage the revocation list, or 2) use an external revocation list.

@msporny
Copy link
Contributor

msporny commented Aug 1, 2021

Now that history has been filtered on main, this PR doesn't line up and shouldn't be merged as is. Do you want to raise a new PR, or rework this one, @nissimsan?

@OR13
Copy link
Contributor

OR13 commented Aug 2, 2021

I agree, suggest reimplementing.

@nissimsan
Copy link
Contributor Author

Hi guys - thanks!

I'll close this in any case to avoid github gymnastics.

I don't see much support for a less-techie-user-friendly approach which shields users from the intricacies of credentialStatus. It makes sense for the vc-http-api not to go into such default-behavior use case. So not planning on reopening the ticket. (Please let me know if I misinterpret your dialogue).

@nissimsan nissimsan closed this Aug 3, 2021
@OR13
Copy link
Contributor

OR13 commented Aug 3, 2021

@nissimsan my suggested changes was to define credentialStatus correctly here:

https://github.com/w3c-ccg/vc-http-api/blob/main/components/Credential.yml

as an optional field.

@nissimsan
Copy link
Contributor Author

@OR13, the credentialStatus field is already defined on IssueCredentialsOptions (not Credentials, as you suggest):
https://github.com/w3c-ccg/vc-http-api/blob/main/components/IssueCredentialOptions.yml#L38

I'm slightly hesitant - an "optional" field does seem to fit okay on the "options" object (passed right along the Credentials object at issuance). So, just confirming: is moving the field indeed what you had in mind?

@OR13
Copy link
Contributor

OR13 commented Aug 11, 2021

@nissimsan yes, I thought it would be best to focus on keeping the types as close to their output format as possible.

I suppose this could be improved by describing how revocable credentials are issued with these APIs directly, including how the options is used to mutate the request body before signing... generally mutating request bodies should be minimized.

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