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

Feature/did document fields + add service entry for token endpoint #298

Conversation

andreibogus
Copy link
Contributor

This is implementation of enhancements: did document fields and "add service entry for token endpoint".
Closes: #292, #296

Pre-review checks

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

@andreibogus andreibogus changed the title Feature/did document fields Feature/did document fields + add service entry for token endpoint Apr 30, 2024
@@ -102,4 +102,7 @@ private StringPool() {

public static final String PRIVATE_KEY = "PRIVATE KEY";
public static final String PUBLIC_KEY = "PUBLIC KEY";
public static final String VERIFICATION_METHOD_TYPE = "JsonWebKey2020";
Copy link
Contributor

Choose a reason for hiding this comment

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

we already have a constant in SSI lib for JsonWebKey2020 : JWKVerificationMethod.DEFAULT_TYPE

public class MIWVerifiableCredentialType {

/** The constant MEMBERSHIP_CREDENTIAL. */
public static final String MEMBERSHIP_CREDENTIAL = "MembershipCredential";
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have constant in SSI lib VerifiableCredentialType.MEMBERSHIP_CREDENTIAL

*/
@SneakyThrows
public String getWalletKeyIdByWalletId(long walletId) {
return walletKeyRepository.getByWalletId(walletId).getKeyId();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we write a JPA query to fetch only keyid from the table instead of the whole object? ie. select keyId from WalletKey where walletId:=id

mutableContext.add(uri);
}
});
didDocument.put("@context", mutableContext);
Copy link
Contributor

Choose a reason for hiding this comment

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

use constant JsonLdObject.CONTEXT

didDocument = DidDocument.fromJson(didDocument.toJson());

didDocument.put(ASSERTION_METHOD, List.of(jwkVerificationMethod.getId()));
Map<String, Object> serviceData = Map.of("id", did.toUri(), "type", "CredentialService",
Copy link
Contributor

Choose a reason for hiding this comment

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

use constant from Verifiable for id and type


didDocument.put(ASSERTION_METHOD, List.of(jwkVerificationMethod.getId()));
Map<String, Object> serviceData = Map.of("id", did.toUri(), "type", "CredentialService",
"serviceEndpoint", HTTPS_SCHEME + miwSettings.host() + "/api/token");
Copy link
Contributor

Choose a reason for hiding this comment

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

use Service.SERVICE_ENDPOINT

@@ -175,7 +186,24 @@ void createWalletTest201() throws JsonProcessingException, JSONException {

Assertions.assertNotNull(response.getBody());
Assertions.assertNotNull(wallet.getDidDocument());
Assertions.assertEquals(2, wallet.getDidDocument().getVerificationMethods().size());
List<VerificationMethod> verificationMethods = wallet.getDidDocument().getVerificationMethods();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we write separate test cases to verify verification methods instead of modifying existing ones?
ie. Create a wallet with random BPN and then verify did document and verification method?

Copy link
Contributor

@nitin-vavdiya nitin-vavdiya left a comment

Choose a reason for hiding this comment

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

Need to update licence header in modified files

@nitin-vavdiya
Copy link
Contributor

Closing this PR as there has been no response from the author of this PR for a long time.

Changes are completed with: #325

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.

3 participants