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

414 problem details response #418

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

PatStLouis
Copy link
Collaborator

  • Created a new ProblemDetails object.
  • Modified the credentials/verify options and response

description: The verified credential
example: { "checks": ["proof"], "warnings": [], "errors": [] }
description: The verified credential used as the verification payload.
example: { "verified": True, "results": {"credentialSchema": [], "credentialStatus": [], "proof": []}, "warnings": [], "errors": [] }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
example: { "verified": True, "results": {"credentialSchema": [], "credentialStatus": [], "proof": []}, "warnings": [], "errors": [] }
example: { "verified": true, "results": {"credentialSchema": [], "credentialStatus": [], "proof": []}, "warnings": [], "errors": [] }

components/VerifyCredentialResult.yml Outdated Show resolved Hide resolved
components/VerifyCredentialResult.yml Outdated Show resolved Hide resolved
components/VerifyCredentialResult.yml Outdated Show resolved Hide resolved
components/VerifyCredentialResult.yml Outdated Show resolved Hide resolved
components/VerifyCredentialResult.yml Outdated Show resolved Hide resolved
components/VerifyCredentialResult.yml Outdated Show resolved Hide resolved
components/VerifyCredentialResult.yml Outdated Show resolved Hide resolved
type: string
description: Longer human-readable string for the problem.
example: {
"type": "https://www.w3.org/TR/vc-data-model#PARSING_ERROR",
Copy link
Contributor

Choose a reason for hiding this comment

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

I found it a bit confusing that this example was to a type that isn't actually defined in the VC data model as far as I can tell (a GET to https://www.w3.org/TR/vc-data-model#PARSING_ERROR just leads to the spec root for me). Are there actual error types defined for VC verification somewhere? If not, perhaps using example.com for now could be clearer that it is just an example.

Copy link
Contributor

@msporny msporny Sep 17, 2024

Choose a reason for hiding this comment

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

This URL works now, and the one in the spec will work after VCDM v2.0 becomes a global standard: https://www.w3.org/TR/vc-data-model-2.0/#PARSING_ERROR

Suggestion is to add a warning in the spec via class="issue" that warns about the URLs not working today, but will in the future, and how to hack at the URL to get it to work today.

An issue was also mentioned about the error type `https://www.w3.org/TR/vc-data-model#PARSING_ERROR` not resulting in any meaningful destination/description.

Co-authored-by: Ted Thibodeau Jr <[email protected]>
"credentialSchema": [],
"credentialStatus": [],
"proof": []
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation nit:

Suggested change
}
}

Comment on lines +48 to +50
details:
type: string
description: Optional information to provide in the response.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
details:
type: string
description: Optional information to provide in the response.
details:
type: string
description: Optional information to provide in the response.

Comment on lines +61 to +63
details:
type: string
description: Optional information to provide in the response.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
details:
type: string
description: Optional information to provide in the response.
details:
type: string
description: Optional information to provide in the response.

type: boolean
description: Results of validating the proof object.
details:
type: string
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be an object instead everywhere? Is this meant to be a description? If so, it should be "message" probably?

Comment on lines +121 to +122
"warnings": [],
"errors": [],
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"warnings": [],
"errors": [],
"problemDetails": [],

Comment on lines +74 to +79
valid:
type: boolean
description: Results of validating the credentialSchema object.
details:
type: string
description: Optional information to provide in the response.
Copy link
Contributor

@msporny msporny Sep 17, 2024

Choose a reason for hiding this comment

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

Suggested change
valid:
type: boolean
description: Results of validating the credentialSchema object.
details:
type: string
description: Optional information to provide in the response.
problemDetails:
type: [ProblemDetails]
description: Array of problem details objects

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.

5 participants