vc-api icon indicating copy to clipboard operation
vc-api copied to clipboard

credentialId in updateCredentialStatus may not fit all use cases

Open peacekeeper opened this issue 4 years ago • 1 comments

https://github.com/w3c-ccg/vc-http-api/pull/110 added the updateCredentialStatus operation which requires a caller to pass in a credentialId for updating a credential's status. This is a good start and I don't see an urgent problem with that, but wanted to point out that:

It's not quite clear if this field is meant to be the same as the id property of the VC, or if it can also be some internal reference that is not actually included in the VC itself.

  • If it is the former, then it's important to keep in mind that id in a VC is optional, and that the API would therefore not support VCs that have a credentialStatus property but not an id property.
  • If it is the latter, then there might be some edge cases where the entity that can change the status of a VC is not the same as the entity that originally issued it, and the internal reference needed for the API may not be available.

Also, perhaps in the future we will see more advanced credential status mechanisms (ZKP based?) where some piece of information other than a credential ID might be needed for changing the status.

Potential alternatives: 1. Pass the entire VC to the API for changing the status, 2. Pass the credentialStatus block of the VC to the API, 3. Pass some additional options to the API.

Like I said, not urgent, just wanted to capture these thoughts.

peacekeeper avatar Feb 23 '21 22:02 peacekeeper

We discussed this on the 2022-03-29 call. @mkhraisha summarized the item. @mprorock noted that now that we have GET/PUT, seems like we added those things so we have clean/normal restful way to deal with this issue. @dlongley noted that it would be good to have a use case where a credential doesn't have an id that needs to be updated in this way. @mkhraisha asked if there would be a way to get credentials with no id. @dlongley said you could issue them, but might not be able to get them by id. @mprorock mentioned that we added GET so that you can always access a credential even if it doesn't have an ID via the /credentials/{id} pattern.

The discussion continues...

msporny avatar Mar 29 '22 21:03 msporny

The group discussed this on 2022-11-01. @jandrieu asked which component this API call goes on, the group didn't know. @dlongley noted that it is associated with the Issuer Coordinator, because status is being updated by issuer (call from Issuer Coordinator to Issuer Service -- endpoint is on Issuer Service, called by Issuer Coordinator). He then wondered if you didn't have an ID, then you couldn't implement this endpoint. Maybe internally, there is another ID that could be stored/used in the issuing system. When you specify credentialID field, that could map into some internal space that the issuer has and can look up the credential. @dlongley didn't see how we could get rid of this field, but asked if the field has to necessarily map to credential.id in a credential. @jandrieu thought @dlongley was right, identifiers don't have to be the same identifier. @jandrieu noted that you could use a hash or some mechanism -- update by example, could do that to get around ID field. The id doesn't need to be ID in credential, has to be identifier in status architecture for that record. @dlongley noted that the Issuer Service needs to know how to map credentialId to a credential in the system. Issuers are responsible for making sure there are no conflicts by making sure the namespace is clear. Any credential.id MUST uniquely identify a credential from that issuer instance. The credential.id field is a URI that's supposed to be globally ambiguous, the credentialId field should be the same -- ones they use internally can't be in conflict with ones exposed through credential.id field. @msporny asked how credentialId is messaged back to caller -- if VC does not contain credential.id, then credentialId is specified in return value. gregb talked through the concerns, and identified that maybe we have a subfield that could be used for communication with the API. @jandrieu noted that if we wanted to revoke the credential, this is not an endpoint for anyone other than Issuer Coordinator to do anything. gregb noted that this is definitely not a holder talking to issuer. @jandrieu noted refresh endpoint, which might be closer to what gregb was mentioning, but that's a different function. @dlongley walked through how a system would use credential identifiier -- system has a set of users which it wants to issue credentials to, users can authenticate using desirable mechanism, choose to receive credential, then when that choice is made, credential identifier can be used/bound to internal record -- binding between local user account and credential that they're receiving. Then that credentialId could be used when credential is used... could either go into credential.id field, or could go into internal credentialId field. Instead of assuming issuing systems generate IDs, managing ID space, linking to local record, and ID goes into credential.id... but you can always refer to credentialId field, know user, ask system to get credentialID to make change to status. @dlongley noted that caller should say what ID is, and only if there is conflict, should there be an error generated, caller has complete control over what ID is used, if it appears in credential, gap we have is being able to specify credentialId property when you ask it to create a credential. We have multiple use cases 1) credentials that dont' have status and don't have credential.id, 2) credentials that have status and don't have crednetial.id. We need to enable contorl over identifiers in hands of parties using 3rd party software... only responsibility of issuer to know if they have a conflict. It was discussed that we could use a 201 response and Location header for auto-generated-by-the-issuer-service VCs, but it would shift the responsibility back onto the issuer services, which we didn't want to do at the time.

Need to raise a PR that specifies how credentialId is set when calling /credentials/issue ... That value is what is used when making a call to GET /credentials/{credentialId}. It is possible to NOT set this value. It is also possible that credential.id is set, and if that's set, then that value becomes credentialId. If the credential has credential.id that value is used for credentialId, if credential.id is not set then you may optionally specify credentialId in the API call. If you don't specify it, you can't change the status. If you do specify it, you can change the status on the credential using that field if the credential also has a status that can be changed.

msporny avatar Nov 01 '22 19:11 msporny

I would suggest giving this new value a more distinctive name such as credentialRef.

credentialId versus credential.id are just too close, bound to cause confusion.

(Irrespective of whether or not we end up going down the same path over on https://github.com/w3c-ccg/traceability-interop/issues/468).

nissimsan avatar Dec 01 '22 10:12 nissimsan

Can you clarify where the returned credentialId property would live in the response to a /credentials/issue request? Would it be a sibling to verifiableCredential?

brownoxford avatar Dec 01 '22 16:12 brownoxford

Can you clarify where the returned credentialId property would live in the response to a /credentials/issue request? Would it be a sibling to verifiableCredential?

At present, it was suggested that it would be a sibling, but that sounds like a bad idea, doesn't it? :)

@nissimsan's point also resonates with me.

We might want to put it in a container at the top level, much like the way DID Resolution returns its results:

https://w3c-ccg.github.io/did-resolution/#example-example-did-resolution-result

It uses the fields didResolutionMetadata and didDocumentMetadata ... so, perhaps we should have a field called issuanceMetadata that can then provide advisory information to the caller? So, the field would end up being issuanceMetadata.<SOME_REFERENCE_ID> ... thoughts?

msporny avatar Dec 03 '22 16:12 msporny

I made some comments over here where I noted potential problems with having the issuing service auto-generate identifiers for referencing issued credentials. I think the better model is for the client to create and manage such identifiers, otherwise there doesn't seem to be a sensible way to resolve error cases.

Copy and pasted here:

I think having the issuer service generate IDs is problematic for partitioned systems (which client + issuer service are an example). This is because clients cannot tell whether an error that occurs when communicating with the issuer service resulted in the issuance of a VC or not. If the client is always responsible for creating the ID, then when a network error occurs, they can retry the issuance call with the same ID. If the call fails with a conflict error then they know the VC was issued and the response was dropped after that. If the call succeeds, they know the error occurred before the VC was issued.

dlongley avatar Dec 03 '22 18:12 dlongley

@dlongley I am a fan of the idea that a "conflict" error can help identify whether or not VCs were issued in the case of a network (or similar) error. What are your thoughts on how to handle scenarios where issuers are not storing VCs (and therefore unable to identify conflicts).

brownoxford avatar Dec 12 '22 18:12 brownoxford

@brownoxford,

I am a fan of the idea that a "conflict" error can help identify whether or not VCs were issued in the case of a network (or similar) error. What are your thoughts on how to handle scenarios where issuers are not storing VCs (and therefore unable to identify conflicts).

In my view, there are two cases where issuers don't store full VCs:

  1. Issuers still store some meta data about the issued VCs, but the not the full VCs
  2. Issuers store nothing (no meta data and no VC)

In the first case, I'd expect the ID to be part of that meta data and the same "conflict" error can be used.

In the second case, I think you're dealing with bearer VCs where, if a call is made that fails -- no one ends up bearing the VC anyway -- there's nothing to be concerned about. The VC that was created goes into the ether ... and you can just generate it again with the same ID without any problems.

Perhaps a better way to put it is that the problems come from two or more systems tracking something different for the same ID. If you remove the issuer instance as one of those systems, that eliminates the problem. Of course, if the issuer system needs to track (for credential status, etc.), then you want the client to specify / manage IDs so conflicts can be resolved between the two systems.

So, generally, it seems to me that all cases are covered if the client specified / manages the identifier and relies on the issuer instance to track duplicates (or not -- because it's a bearer VC so it doesn't matter anyway and network failures don't create problems).

dlongley avatar Dec 12 '22 19:12 dlongley

The group discussed this on the 2024-01-16 call and determined that the PR that is raised should do the following things:

  • Allow a client to specify an id property on the VC.
  • Allow a client to NOT specify an id property on the VC, but provide an identifier that they can then use later to identify the issued VC specified via the credentialId option passed to the API.
  • Allow VCs to be issued that do not have the id property set on the VC and do not have the credentialId option specified (for bearer-style VCs w/ no status information). Warn that there is no way to deal with duplicate errors (and maybe that's ok in some systems).
  • Specify that the credentialId is not meant to be used as the id property on the VC.
  • credentialId is an opaque value.
  • Recommend against allow the server to auto-generate an id property on the VC (via a SHOULD NOT sentence) and warn that this can lead to unrecoverable partitioning errors (on a failed client call, you don't know what VC might have been created on the server). A client that's re-trying could create many VCs w/o knowing that they're creating many VCs due to a software error.

@wes-smith volunteered to attempt a PR.

msporny avatar Jan 16 '24 20:01 msporny