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

FilterEvaluation handler: bug/feature request #113

Closed
kalinchernev opened this issue May 3, 2023 · 2 comments
Closed

FilterEvaluation handler: bug/feature request #113

kalinchernev opened this issue May 3, 2023 · 2 comments

Comments

@kalinchernev
Copy link

  • I'm submitting a ...
    [x] bug report
    [x] feature request
    [ ] question about the decisions made in the repository
    [ ] question about how to use this project

  • Summary

Working with the latest version of the library I am not able to validate a presentation submission to comply with a presentation definition with multiple input descriptors.

inputDescriptorFilterEvaluationHandler.spec.zip
This is an updated test spec illustrating the issue. I provide it as a file here as I am not sure whether it's a desired feature of the library the current behavior or a bug worth opening a pull request for.

Debugging the corresponding handler I see 2 issues that prevent correct utilization of the library at the moment:

  1. No IDs are taken into account at the moment.

The descriptor_map object MUST include an id property. The value of this property MUST be a string that matches the id property of the Input Descriptor in the Presentation Definition that this Presentation Submission is related to.

Taken from https://identity.foundation/presentation-exchange/spec/v2.0.0/#presentation-submission

Holds true for both jp.nodes(pd, '$..fields[*]'); and wrappedVcs which hold no such context. Thus, it's impossible to extend current logic of the handler to map 1:1 PS to PD without broader surface change in the library. (I think)

  1. Each VC is expected to comply with all input descriptors of the PD

This is more problematic in my opinion. Is it wrong expectation from my side that

{
          id: 'same-device-in-time-credential',
          path: '$',
          format: 'jwt_vp',
          path_nested: {
            id: 'same-device-in-time-credential',
            path: '$.verifiableCredential[0]',
            format: 'jwt_vc',
         }
 }

should be compared only to

 {
          id: 'same-device-in-time-credential',
          constraints: {
            fields: [
              {
                path: ['$.type'],
                filter: {
                  type: 'array',
                  contains: { const: 'CTWalletSameInTime' },
                },
              },
            ],
          }
        }
  • Other information (e.g. detailed explanation, stack traces, related issues, suggestions how to fix, links for us to have context, eg. StackOverflow, personal fork, etc.)

I made an attempt of a workaround by assuming an index based comparison in hope to find a solution without a major refactoring, but this breaks a lot of existing tests

public handle(pd: IInternalPresentationDefinition, wrappedVcs: WrappedVerifiableCredential[]): void {
    const fields: { path: PathComponent[]; value: FieldV1 | FieldV2 }[] = jp.nodes(pd, '$..fields[*]');
    wrappedVcs.forEach((wvc: WrappedVerifiableCredential, vcIndex: number) => {
      this.createNoFieldResults(pd, vcIndex, wvc);
    });
    fields.forEach((field, inputDescriptorIndex) => {
      const wvc = wrappedVcs[inputDescriptorIndex];
      let inputField: { path: PathComponent[]; value: unknown }[] = [];
      if (field.value.path) {
        inputField = JsonPathUtils.extractInputField(wvc.credential, field.value.path);
      }
      let resultFound = false;
      for (const inputFieldKey of inputField) {
        if (this.evaluateFilter(inputFieldKey, field.value)) {
          resultFound = true;
          const payload = { result: { ...inputField[0] }, valid: true, format: wvc.format };
          this.getResults().push({
            ...this.createResultObject(jp.stringify(field.path.slice(0, 3)), inputDescriptorIndex, payload),
          });
        }
      }
      if (!resultFound) {
        if (!inputField.length) {
          const payload = { valid: false, format: wvc.format };
          this.createResponse(field, inputDescriptorIndex, payload, PexMessages.INPUT_CANDIDATE_DOESNT_CONTAIN_PROPERTY);
        } else {
          const payload = { result: { ...inputField[0] }, valid: false, format: wvc.format };
          this.createResponse(field, inputDescriptorIndex, payload, PexMessages.INPUT_CANDIDATE_FAILED_FILTER_EVALUATION);
        }
      }
    });
    this.updatePresentationSubmission(pd);
  }
@sksadjad
Copy link
Contributor

sksadjad commented May 4, 2023

Hey @kalinchernev thanks for opening this issue, this looks like a regression and we will look at it, and will have a fix soon.

@nklomp
Copy link
Contributor

nklomp commented Jul 10, 2023

Should be fixed in release 2.1.0

@nklomp nklomp closed this as completed Jul 10, 2023
@nklomp nklomp removed the in progress Work is being done label Jul 10, 2023
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

No branches or pull requests

3 participants