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

Add PresentationExchange.selectCredentials #25

Merged
merged 15 commits into from
Mar 18, 2024
Merged

Add PresentationExchange.selectCredentials #25

merged 15 commits into from
Mar 18, 2024

Conversation

kirahsapong
Copy link
Collaborator

@kirahsapong kirahsapong commented Mar 8, 2024

Adds

  • PresentationExchange.selectCredentials to return an array of selectable vcJwts given a presentation definition
  • PresentationExchange.satisfiesPresentationDefinition that returns void and throws if a set of VCs fails to satisfy a given PD.

Inspired by our kt and js implementations ✨

Note: WIP but pls review early and lmk any questions. Still adding a few more test cases.

closes #3

@kirahsapong kirahsapong requested a review from amika-sq as a code owner March 8, 2024 20:31
@kirahsapong kirahsapong requested review from frankhinek, mistermoe, nitro-neal, jiyoonie9 and angiejones and removed request for amika-sq March 8, 2024 20:32
}
}

public static func parse(jwtString: String) throws -> ParsedJWT {
Copy link
Collaborator

@nitro-neal nitro-neal Mar 8, 2024

Choose a reason for hiding this comment

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

just fyi as parse is getting renamed in 'decode' I think (this is how it is in go):

lots of new decoding logic here for making sure jwt is valid vcjwt
decentralized-identity/web5-js#421

and when we get to the verify part of the vcJwt the plan is to split up verify into lots of parts with verify options - decentralized-identity/web5-js#425

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i will add in a follow up PR. tracked here: #28

thanks!

cc: @jiyoontbd

@nitro-neal
Copy link
Collaborator

Wow great job, looking good so far! 👏

Copy link

@jiyoonie9 jiyoonie9 left a comment

Choose a reason for hiding this comment

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

looks great! seems like there's a bit more work to be done wrt neal's comment about additional JWT decode checks in the works for web5-js also applying to this PR?

Sources/Web5/Credentials/PresentationExchange.swift Outdated Show resolved Hide resolved
Sources/Web5/Crypto/JOSE/JWT.swift Outdated Show resolved Hide resolved
Copy link
Collaborator

@nitro-neal nitro-neal left a comment

Choose a reason for hiding this comment

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

looks great,
As a quick follow up I would try to add the swift package to use the test vectors - https://github.com/TBD54566975/web5-spec/blob/main/test-vectors/presentation_exchange/select_credentials.json

( I can help with that )

and adding a lot more dynamic valid / invalid pd unit tests like we have here: https://github.com/TBD54566975/web5-kt/blob/main/credentials/src/test/resources/pd_filter_array.json

@kirahsapong
Copy link
Collaborator Author

nice, thanks @nitro-neal! yeah i wrote in the description for now

Note: WIP but pls review early and lmk any questions. Still adding a few more test cases.

appreciate the reviews so far everyone! will [at] everyone again when test cases are added lol

@kirahsapong kirahsapong merged commit 3bb1ddf into main Mar 18, 2024
4 checks passed
@kirahsapong kirahsapong deleted the credentials branch March 18, 2024 22:20
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.

Implement Presentation Exchange
5 participants