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

make path_nested path relative in the presentation submission example #184

Merged

Conversation

nemqe
Copy link
Contributor

@nemqe nemqe commented May 29, 2024

resolves #57

@nemqe nemqe requested a review from Sakurann May 29, 2024 00:07
Copy link
Collaborator

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

I think the current examples are correct because the whole point is to say whether it is [0] or [1] in the VP Token..

@nemqe
Copy link
Contributor Author

nemqe commented Jun 3, 2024

I think that the path_nested needs to be scoped to the upper level of path_nested, i.e. if we have a presentation at index [0] of a vp_token, the credential we are pointing to needs to be contained in that presentation, and it cannot escape that 'sandbox'.

Maybe I am reading the spec wrong: https://identity.foundation/presentation-exchange/spec/v2.0.0/#presentation-submission, but I mostly opened this PR to get clarity on this.

@Sakurann
Copy link
Collaborator

@sudeshrshetty to review

@nemqe nemqe force-pushed the fix/paths_nested_abs_paths_pres_sub_example branch 2 times, most recently from 465d9d2 to 31ef9bd Compare June 21, 2024 14:48
@sudeshrshetty
Copy link

In OID4VP, presentation_submission and VerifiablePresentation(s) are part of different objects unlike those given in Presentation Exchange Examples.

In case of multiple presentations in vp_token, paths like [0], [1] are necessary.
But path_nested property always provides a relative path within a given nested value, so changes in this PR makes sense to me.

@jogu
Copy link
Collaborator

jogu commented Jul 15, 2024

This has unfortunately now conflicted with https://github.com/openid/OpenID4VP/pull/198/files - are you able to resolve the conflict please @nemqe ?

@nemqe nemqe force-pushed the fix/paths_nested_abs_paths_pres_sub_example branch from 31ef9bd to 921c699 Compare July 15, 2024 10:00
@nemqe
Copy link
Contributor Author

nemqe commented Jul 15, 2024

@jogu conflict resolved

@jogu
Copy link
Collaborator

jogu commented Jul 15, 2024

Thank you!

Is it definitely right how it is now:

{
  "id": "Presentation example 2",
  "definition_id": "Example with multiple VPs",
  "descriptor_map": [
    {
      "id": "ID Card with constraints",
      "format": "ldp_vp",
       "path": "$[0]",
       "path_nested": {
         "format": "ldp_vc",
         "path": "$.verifiableCredential[0]"
       }
     },
     {
      "id": "Example credential disclosing only address",
      "format": "vc+sd-jwt",
      "path": "$[1]"
    }
  ]
}

It's confusing me as I thought $ basically referred to the root of the response, can that be accessed as both an array ($[1]) and a object ($.verifiableCredential) ?

@nemqe
Copy link
Contributor Author

nemqe commented Jul 15, 2024

My understanding is that when path_nested you are 'sandboxed' inside of the object specified by path, and that your path is always relative to the 'nesting'.

In this particular example it would mean that path_nested.path=$ is not referring to the root, but to the path=$[0].

I think using $[0] would be wrong because the object pointed to by path=$[0] is not an array.

There is a section describing path_nested and an example in https://identity.foundation/presentation-exchange/spec/v2.0.0/#presentation-submission

But I am not expert on this subject.

@Sakurann Sakurann merged commit e040355 into openid:main Jul 15, 2024
2 checks passed
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.

PresentationSubmission example for multiple VPs seems to be wrong
4 participants