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

feat: [EUDIW-209] Add Remote presentation for Potential #174

Draft
wants to merge 32 commits into
base: eudiw-master
Choose a base branch
from

Conversation

manuraf
Copy link

@manuraf manuraf commented Jan 15, 2025

List of Changes

  • Move verifyTokenSignature to dedicated method which is executed after getting Request Object
  • Added the ability to fetch a presentation definition from multiple sources, including directly from the request object, from a URI in the Relying Party (RP) configuration, or based on the scope.
  • Extended evaluateInputDescriptionForSdJwt4VC() to enforce required/optional fields and validate JSON schema filters on disclosed claims.
  • Implemented limit_disclosure handling to restrict data revealed to only those fields explicitly required.
  • Replaced the old sendAuthorizationResponse flow with a new module that supports both direct post and encrypted direct post (JWT) response modes.
  • Updated documentation, code structure, and tests to reflect these new capabilities and ensure better maintainability.

Motivation and Context

These changes are specifically targeted for the Potential use case and some logic are been made in order to support also the Italian specification.

How Has This Been Tested?

Screenshots (if appropriate):

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@github-actions github-actions bot added the feat label Jan 15, 2025
@manuraf manuraf changed the title feat: [EUDIW-209] Add Remote presentation for Potential specs feat: [EUDIW-209] Add Remote presentation for Potential Jan 15, 2025
Comment on lines 16 to 29
let pubKey;

// verify token signature to ensure the request object is authentic
if (jwkKeys) {
pubKey = jwkKeys.find(
({ kid }) => kid === requestObjectJwt.protectedHeader.kid
);
}

if (!pubKey) {
throw new UnverifiedEntityError("Request Object signature verification!");
}
await verify(requestObjectEncodedJwt, pubKey);

Copy link
Contributor

Choose a reason for hiding this comment

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

Usually we avoid the usage of let as much as possibile to prefer an immutable data approach. This could be refactored as:

const pubKey = jwkKeys?.find(
  ({ kid }) => kid === requestObjectJwt.protectedHeader.kid
);

if (!pubKey) {
  throw new UnverifiedEntityError("Request Object signature verification!");
}

What do you think?

Comment on lines 157 to 158
let requiredClaimNames: string[] = [];
let optionalClaimNames: string[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Comment on lines +169 to +172
let [matchedPath, matchedValue] = findMatchedClaim(
field.path,
disclosuresAsPayload
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Author

Choose a reason for hiding this comment

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

here is more complicated using const because matchedPath really change...maybe enforce to use const could cause the code to become less readable

Comment on lines 225 to 239
let requestBody: string;
if (requestObject.response_mode === "direct_post.jwt") {
requestBody = await buildDirectPostJwtBody(
jwkKeys,
requestObject,
vp_token,
presentation_submission
);
} else {
requestBody = await buildDirectPostBody(
requestObject,
vp_token,
presentation_submission
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants