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: Create VC as JWT #260

Merged

Conversation

mustafasalfiti
Copy link
Contributor

@mustafasalfiti mustafasalfiti commented Feb 27, 2024

Description

In the Controllers, an option has been added to generate a VC as a JWT (JSON Web Token). By default, this option is set to false. When set to true, the response will be a JWT containing the VC.

Additionally, certain exception handling has been revised and corrected due to the integration of the new SSI-lib

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

@mustafasalfiti
Copy link
Contributor Author

should be with the latest ssi agent lib current (main) in order for the tests to work @borisrizov-zf

@mustafasalfiti mustafasalfiti marked this pull request as ready for review April 19, 2024 21:36
@borisrizov-zf
Copy link
Contributor

@mustafasalfiti I'll ping you when the new release of the ssi-lib is out, so you can update.

Copy link
Contributor

@borisrizov-zf borisrizov-zf left a comment

Choose a reason for hiding this comment

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

Just two minor issues to look at.

[
{"type":"TraceabilityCredential"},
{"type":"SustainabilityCredential"},
{"type":"ResiliencyCredential"},
Copy link

Choose a reason for hiding this comment

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

Note, with the new release this type does not exist anymore. From my point of view it does not make sense to follow the old standard - we should rather use the new credentials as per 24.05. release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree on that but in this PR it has nothing to do with the types of the VC, we can create a different PR for it, to fix that in the source code. this test was already there with a JSON-LD reponses, was only recreated to make sure it gives the response as JWT independent of the VC type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, this PR is already large. Please create a new issue, link it here and we'll tackle that next.

On a side note: as soon as the IP check is done, I'll merge.

@borisrizov-zf
Copy link
Contributor

IP check link: https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/14592
IP check status: Approved

Ready to merge.

@borisrizov-zf
Copy link
Contributor

@mustafasalfiti please rebase to the latest develop version (there was a minor update merged a week ago).

@nitin-vavdiya
Copy link
Contributor

@borisrizov-zf Rebase is done

@borisrizov-zf borisrizov-zf mentioned this pull request May 23, 2024
3 tasks
Comment on lines 37 to 42
@Getter
@Setter
@NoArgsConstructor
@AllArgsConstructor
@Builder
public class IssueFrameworkCredentialRequest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting is off by one space. Please re-check this (and other files), rebase and ping me.

Copy link
Contributor

Choose a reason for hiding this comment

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

@borisrizov-zf formatting is done

I think a rebase is not needed as there has been no change in develop branch since the last rebase. Am I right?

Copy link

sonarcloud bot commented May 23, 2024

Quality Gate Passed Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.6% Duplication on New Code

See analysis details on SonarCloud

@borisrizov-zf
Copy link
Contributor

@nitin-vavdiya Hi, I'm doing manual tests. If all goes well, I'll merge and notify you.

@borisrizov-zf borisrizov-zf merged commit 1365b39 into eclipse-tractusx:develop May 28, 2024
8 checks passed
@borisrizov-zf
Copy link
Contributor

Copy link

🎉 This PR is included in version 0.5.0-develop.15 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Copy link

github-actions bot commented Jul 5, 2024

🎉 This issue has been resolved in version 0.5.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@thackerronak thackerronak deleted the feat/vc_asJwt branch September 26, 2024 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants