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

cleanup issueCredentialOptions and VerifyOptions #375

Merged
merged 8 commits into from
Mar 26, 2024
Merged

Conversation

mkhraisha
Copy link
Collaborator

@mkhraisha mkhraisha commented Mar 5, 2024

Forked off #373
tackles #318

Merge conflicts will be cleaned up once once #373 is merged in.

Challenge changes are not intended to be reviewed in this PR, but are remnants from forking off #373.

  1. Updates IssueCredentialOptions to remove created, and updates the example
  2. Updates DeriveCredentialOptions to add selectivePointers
  3. Removes VerifyOptions this has now been split into:
  4. VerifyCredentialOptions: this currently only has returnCredential which is based on what the current spec shows.
  5. VerifyPresentationOptions: this currently has the properties domain and challenge. The issue mentions adding proofPurpose, but I wasn't sure, iirc the proofPurpose of a presentation is always authentication. if that is not true happy to update the PR. The Spec also mentions returnCredential as a possible option to this endpoint, I'm not sure if thats an error that should be returnPresentation or it should just be excluded entirely, I erred on the side of exclusion.
  6. CreatePresentationOptions: intended to provide options when creating a presentation, similar question on the proofPurpose field.
  7. Updates references that call VerifyOptions.

Preview | Diff
Don't remove this comment or modify anything below this line.
If you don't want a preview generated for this pull request,
just replace the whole of this comment's content by "no preview"
and remove what's below.
-->


Preview (#Don't…) (<a href="https://pr-preview.s3.amazonaws.com/w3c-ccg/vc-api/pull/375.html#just replace the whole of this comment's content by "no preview"
and remove what's below.

-->


<a href="https://pr-preview.s3.amazonaws.com/w3c-ccg/vc-api/pull/375.html" title="Last updated on Mar 5" title="#just replace the whole of this comment's content by "no preview"
and remove what's below.

-->


#just…) (#2024) (Preview | Preview | #9:44…) (#2024) (Diff" title="#9:44 PM UTC (ebd12e1)">Diff">#9:44…) | Diff

@mkhraisha mkhraisha changed the title 318/cleanup options cleanup issueCredentialOptions and VerifyOptions Mar 5, 2024
components/DeriveCredentialOptions.yml Outdated Show resolved Hide resolved
components/IssueCredentialOptions.yml Outdated Show resolved Hide resolved
@msporny msporny changed the base branch from main to 310-add-challenge-management-endpoint-to-verifying-apis March 12, 2024 19:10
@msporny
Copy link
Contributor

msporny commented Mar 12, 2024

@mkhraisha the group reviewed this today, looks like it's on a good path, we're getting ready to merge 373, please re-base once that happens and we'll merge this one as well. Thank you for the PR!

@wes-smith wes-smith force-pushed the 310-add-challenge-management-endpoint-to-verifying-apis branch from 3589fb3 to 0f47ef8 Compare March 19, 2024 18:40
@msporny
Copy link
Contributor

msporny commented Mar 26, 2024

Re-ping @mkhraisha, please re-base, we'd like to merge your PR.

@mkhraisha
Copy link
Collaborator Author

Rebased, hopefully this works now.

Base automatically changed from 310-add-challenge-management-endpoint-to-verifying-apis to main March 26, 2024 19:11
wes-smith and others added 5 commits March 26, 2024 15:12
- add challenge creation endpoint
- remove passing `options.challenge` and `options.domain` to incorrect endpoints
@msporny msporny merged commit ffe5571 into main Mar 26, 2024
1 check passed
@msporny msporny deleted the 318/cleanup_options branch March 26, 2024 19:49
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