-
Notifications
You must be signed in to change notification settings - Fork 150
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
Add optional Credential_Manifest method to CAIP-169 #195
base: main
Are you sure you want to change the base?
Conversation
CAIPs/caip-169.md
Outdated
use-case, but in either case MUST be formated as a valid | ||
[credential_application object][] as specified in the [Credential Manifest][] | ||
specification. | ||
- `preferred_proofs` - OPTIONAL. An **ordered** array (from most to least |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Note: this is hard to review -- I can't figure out what's going on with the diffs. Visually it looks like there's no difference between store and verify, but they are getting pulled in. I tried messing with whitespace settings but no dice.]
I think there may be collision with this and credential application's format property:
The object MUST have a format property if the related [Credential Manifest](https://identity.foundation/credential-manifest/#term:credential-manifest) specifies a format property. Its value MUST be a subset of the format property in the [Credential Manifest](https://identity.foundation/credential-manifest/#term:credential-manifest) that this [Credential Submission](https://identity.foundation/credential-manifest/#term:credential-submission) is related to. This object informs the [Issuer](https://identity.foundation/credential-manifest/#term:issuer) which formats the [Holder](https://identity.foundation/credential-manifest/#term:holder) wants to receive the [Claims](https://identity.foundation/credential-manifest/#term:claims) in.
It looks like this spec is assuming the presence of a credential manifest, I think it shouldn't need preferred_proofs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed -- I think that was an OIDC interop thing, like the wallet metadata getter.
@awoie would it make sense to warn readers of the spec that preferred_proofs
is ONLY required for OIDC backwards compat and that the preferred way of communicating this is in the
formats
property of the credential_application
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(review most recent commit-- i may be confused about what the use-case for this redundant/fallback object is, let me know if i am!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you describe a typical flow how a developer could use that for issuance and presentation of credentials?
To allow more complex issuance.
Also rearranges methods to be more readable (and move all optional methods to the end)