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

Should issuer be optional when requesting a credential be issued. #265

Closed
OR13 opened this issue Feb 22, 2022 · 14 comments
Closed

Should issuer be optional when requesting a credential be issued. #265

OR13 opened this issue Feb 22, 2022 · 14 comments
Assignees
Labels
ready for PR Issue ready to be resolved via a Pull Request

Comments

@OR13
Copy link
Contributor

OR13 commented Feb 22, 2022

Today, the issuer field is optional.

If it were forbidden, the server could dynamically assign an issuer, and the client would not be allowed to request a specific issuer which the server might not support.

this would also address the problems with the client thinking then can request a credential be issued with a specific key.

@peacekeeper
Copy link
Member

Today, the issuer field is required.

No it's not.

@OR13
Copy link
Contributor Author

OR13 commented Feb 22, 2022

hmm i guess you are right... https://github.com/w3c-ccg/vc-api/blob/main/components/Credential.yml#L33

So the current API is ambiguous with respect to solutions to this problem, since it would be legal to specify no issuer and no verificationMethod, or only an issuer and no verificationMethod or only a verificationMethod and no issuer.

I edited my comment to note that issuer is optional. the rest of the comments still stand I think.

@dlongley
Copy link
Contributor

I think the issuer should be fixed to the URL being used to issue. For the same reasons argued that we should remove optionality with verification method articulated here: #263.

In short, clients have no business configuring this information on the fly. They should be selecting from previously approved (and well considered) configurations that map to issuer URLs made available for use. Not only do clients have no business assembling bespoke configuration combinations, they may not be able to easily know or "guess" what they could acceptably change from the defaults via parameters.

The responsibilities with the current API are not cleanly separated. I feel like the current API has been optimized for a feature-based interop test suite implementation vs. real world use. Demonstration setups for interop testing may not best reflect real world use. It's understandable how we got here, but more implementation experience with real world use cases has led me to believe we need to make a few changes and better separate out roles. For example, the role that creates an acceptable issuer instance configuration is not the same role as a client application that invokes it.

@OR13
Copy link
Contributor Author

OR13 commented Mar 21, 2022

@dlongley I believe you have started PRs that take us down this path, would you like to comment on how POST /credentials/issue should handle issuer or issuer.id in the following cases:

  1. the value matches a "default issuer, previously configured"
  2. the value is absent making the payload not a valid credential or verifiable credential.
  3. the value is present, but does not match any supported issuer.

@dlongley
Copy link
Contributor

@OR13,

Well, answering these out of order... here's the the simplest thing, IMO, to start with:

  1. the value matches a "default issuer, previously configured"

The credential should be successfully issued (if there are no other unrelated problems) in this case.

  1. the value is present, but does not match any supported issuer.

The credential should be rejected with an error indicating that the issuer does not match. This results in a fail-closed scenario, which is preferable given the security context/purpose of VCs.

  1. the value is absent making the payload not a valid credential or verifiable credential.

It's not clear yet to me whether there are use cases that require success in this case. I would not be surprised if there are a number of use cases where a client should not need to know some values that will be populated by the issuing server -- it's just a question of whether or not this particular value is one that a client can acceptably not know. If so, we should accept this case and allow the issuer ID to be auto-populated (both in the case that issuer is absent or is an object without an id). If not, we should reject and require that the issuer ID be present so it can be checked against the expected value.

It may be harmful for us to bind ourselves so early to a rule that "the payload must be a valid credential or verifiable credential" -- as we may need to consider absent values. However, I think we should abide by this rule early and just know that we may need to loosen it if we find that it's unnecessarily restrictive. Others will have to weigh in with use cases here.

@OR13
Copy link
Contributor Author

OR13 commented Mar 21, 2022

@dlongley @msporny I can't make sense of your recommendation for the case:

  1. the value is absent making the payload not a valid credential or verifiable credential.

I would suggest we error with 400... the irony being that there is currently no way to know this value from the current APIs... if we agree to 400 for case 2, we will be blocked by no ability to "get issuers" for the "issuer apis".

@dlongley
Copy link
Contributor

dlongley commented Mar 21, 2022

@OR13,

I would suggest we error with 400... the irony being that there is currently no way to know this value from the current APIs... if we agree to 400 for case 2, we will be blocked by no ability to "get issuers" for the "issuer apis".

Hmm, this is starting to feel disconnected from real use cases. It's not like clients act in a vacuum where they will just call some random API to issue anything. This sounds like optimizing for a test harness again.

If clients somehow don't need to know via their own configuration what issuer ID to use -- then they shouldn't need to provide it to the API -- they should only need the issue URL. Otherwise, clients themselves should be internally configured with the issuer ID they expect to see (and that they will submit) when hitting the issuer endpoint. Both the issuer (and other information that the client is expecting to put in any VC) would be configured at the same time that the issuer endpoint is also configured.

@OR13
Copy link
Contributor Author

OR13 commented Mar 21, 2022

If clients somehow don't need to know via their own configuration what issuer ID to use -- then they shouldn't need to provide it to the API -- they should only need the issue URL.

How would they know this? There is no way to know it today.

There are no endpoints that tell you which DIDs an issuer supports.

And there are no endpoints to "configure an issuer".

@dlongley
Copy link
Contributor

@OR13,

How would they know this? There is no way to know it today.

Out of band. There would be no reason for a client to hit a issuer instance URL to get the issuer ID value and then insert that into their payload. Using a value in that way would indicate that it has no meaningful binding to anything from the client's perspective. If that was going to be the process, it would be better to just omit the value and let the server populate it; there's no need for the extra step.

Instead, deployments of software that include clients that use issuer APIs may very well be configured with the issuer ID that they expect to use and see in the VCs that they have issued via the issuer API. Some human is typically responsible for obtaining the issuer ID from a reputable source / via some other process and then putting it into the configuration prior to deployment. Real use cases do this today.

This latter approach ensures that if the issuer ID sent by the client does not match the one used by the issuing system at the issuer URL, an error will be produced. This kind of error would never occur if the client read, accepted, and used any issuer ID it got from some issuer GET endpoint -- except perhaps because of a race condition during an unusual state change. That alone, to me, is evidence that it would be an unnecessary step in such setups. Either the client has some issuer ID that they are expecting to be used that necessarily comes from some place other than the issuer system itself -- or they don't care and the server should just auto-populate.

There are no endpoints that tell you which DIDs an issuer supports.

And there are no endpoints to "configure an issuer".

Side note: In the proposal here, there's an endpoint to configure an issuer and get its existing configuration, including the configured assertion method key. However, it's true that we may want to have a separate endpoint that exposes client-only information from the config vs. the linked endpoint which is really for the party that does the configuring.

@OR13
Copy link
Contributor Author

OR13 commented Mar 22, 2022

@dlongley yes, there is a "proposal" to add support for "discovering supported issuers"... but no endpoint defined yet.

I suggest we just start throwing 400 when issuer is omitted, as you said, it can be communicated out of band, and we already just agreed that issuer when configured correctly should 200... which implies incorrect config (absent) should lead to 400.

this issue is about "making issuer optional"... I am suggesting that we NOT do that... we make issuer required and match the "default configuration".

@msporny
Copy link
Contributor

msporny commented Jun 14, 2022

The group discussed this on 2022-06-14, @dlongley summarized the discussion to the current point in time. At present, if you send an issuer to the endpoint, it must match the configuration w/ the issuer. @jandrieu noted that it seems like @OR13 is talking about the Issuer App vs. the Issuer Service. @dlongley is talking about the Issuer Service. The remaining question is whether or not the issuer field should be there. @David-Chadwick noted that the caller is already authenticated/authorized and therefore specifying the issuer is not necessary. There seemed to be general agreement that the issuer does not need to be specified either when calling the Issuer App or the Issuer Service. @dlongley noted that setting issuer when calling is untenable, but discovery to find different issuers is a valid use case and can be done to find the endpoint with the issuer you want.

The decision was to remove the ability to set the issuer parameter on the Issuer Service /issue endpoint since the issuer is assumed to be pre-configured for a specific issuer endpoint.

@msporny msporny added the ready for PR Issue ready to be resolved via a Pull Request label Jun 14, 2022
@msporny
Copy link
Contributor

msporny commented Jan 23, 2024

On 2024-01-23 the group discussed this item and suggested that the spec text should be changed to:

  • Clarify that it is expected that there will be a backing configuration for an issuance endpoint that specifies what the issuer value could/should be.
  • If there are multiple configurations for a particular endpoint, issuer could be used to select among more than one potential issuer at a specific endpoint.
  • If issuer is specified, it must be a value that is understood by the implementation and is expected at the endpoint based on its backing configuration.

@mburchil volunteered to take this one.

mburchil pushed a commit that referenced this issue Feb 5, 2024
… issued.

- Added a description defining the optional use of issuer object when issuing a verifiable credential.

Issue #275 - There should be a test that confirms a 4xx error when a credential issuance is requested with an unsupported issuer

- Updated description of 400 error response on /credentials/issue
msporny pushed a commit that referenced this issue Feb 27, 2024
… issued.

- Added a description defining the optional use of issuer object when issuing a verifiable credential.

Issue #275 - There should be a test that confirms a 4xx error when a credential issuance is requested with an unsupported issuer

- Updated description of 400 error response on /credentials/issue
@wes-smith
Copy link
Collaborator

Addressed by #365.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for PR Issue ready to be resolved via a Pull Request
Projects
None yet
Development

No branches or pull requests

5 participants