fix: prevent VerifiableCredentialsJwtServiceImpl's "NotSuchMethodErro…

…r" (#43) (#20) * Fix VerifiableCredentialsJwtServiceImpl's "NotSuchMethodError" (#43) Fix NoSuchMethodError in VerifiableCredentialsJwtServiceImpl. Add more logging. * Do not use Optional in method signature
eclipse-edc · Aug 8, 2022 · 8602590 · 8602590
1 parent 17f8824
commit 8602590
Show file tree

Hide file tree

Showing 12 changed files with 99 additions and 38 deletions.
diff --git a/client-cli/src/main/java/org/eclipse/dataspaceconnector/identityhub/cli/IdentityHubCli.java b/client-cli/src/main/java/org/eclipse/dataspaceconnector/identityhub/cli/IdentityHubCli.java
@@ -58,6 +58,6 @@ private void init() {
         var objectMapper = new ObjectMapper();
         var monitor = new ConsoleMonitor();
         this.identityHubClient = new IdentityHubClientImpl(okHttpClient, objectMapper, monitor);
-        this.verifiableCredentialsJwtService = new VerifiableCredentialsJwtServiceImpl(objectMapper);
+        this.verifiableCredentialsJwtService = new VerifiableCredentialsJwtServiceImpl(objectMapper, monitor);
     }
 }
diff --git a/...java/org/eclipse/dataspaceconnector/identityhub/cli/ListVerifiableCredentialsCommand.java b/...java/org/eclipse/dataspaceconnector/identityhub/cli/ListVerifiableCredentialsCommand.java
@@ -44,6 +44,9 @@ class ListVerifiableCredentialsCommand implements Callable<Integer> {
     public Integer call() throws Exception {
         var out = spec.commandLine().getOut();
         var result = command.cli.identityHubClient.getVerifiableCredentials(command.cli.hubUrl);
+        if (result.failed()) {
+            throw new CliException("Failed to get verifiable credentials: " + result.getFailureDetail());
+        }
         var vcs = result.getContent().stream()
                 .map(this::getClaims)
                 .collect(toList());

diff --git a/client-cli/src/test/java/org/eclipse/dataspaceconnector/identityhub/cli/CliTestUtils.java b/client-cli/src/test/java/org/eclipse/dataspaceconnector/identityhub/cli/CliTestUtils.java
@@ -23,19 +23,21 @@
 import org.eclipse.dataspaceconnector.identityhub.credentials.VerifiableCredentialsJwtService;
 import org.eclipse.dataspaceconnector.identityhub.credentials.VerifiableCredentialsJwtServiceImpl;
 import org.eclipse.dataspaceconnector.identityhub.credentials.model.VerifiableCredential;
+import org.eclipse.dataspaceconnector.spi.monitor.Monitor;
 
 import java.util.Map;
 
 import static org.eclipse.dataspaceconnector.identityhub.credentials.CryptoUtils.readPrivateEcKey;
 import static org.eclipse.dataspaceconnector.identityhub.credentials.CryptoUtils.readPublicEcKey;
+import static org.mockito.Mockito.mock;
 
 class CliTestUtils {
     public static final String PUBLIC_KEY_PATH = "src/test/resources/test-public-key.pem";
     public static final String PRIVATE_KEY_PATH = "src/test/resources/test-private-key.pem";
     public static final PublicKeyWrapper PUBLIC_KEY;
     public static final PrivateKeyWrapper PRIVATE_KEY;
     private static final Faker FAKER = new Faker();
-    private static final VerifiableCredentialsJwtService VC_JWT_SERVICE = new VerifiableCredentialsJwtServiceImpl(new ObjectMapper());
+    private static final VerifiableCredentialsJwtService VC_JWT_SERVICE = new VerifiableCredentialsJwtServiceImpl(new ObjectMapper(), mock(Monitor.class));
 
     static {
         try {

diff --git a/...java/org/eclipse/dataspaceconnector/identityhub/cli/VerifiableCredentialsCommandTest.java b/...java/org/eclipse/dataspaceconnector/identityhub/cli/VerifiableCredentialsCommandTest.java
@@ -22,6 +22,7 @@
 import org.eclipse.dataspaceconnector.identityhub.client.IdentityHubClient;
 import org.eclipse.dataspaceconnector.identityhub.credentials.VerifiableCredentialsJwtServiceImpl;
 import org.eclipse.dataspaceconnector.identityhub.credentials.model.VerifiableCredential;
+import org.eclipse.dataspaceconnector.spi.monitor.Monitor;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
@@ -65,7 +66,7 @@ class VerifiableCredentialsCommandTest {
     @BeforeEach
     void setUp() {
         app.identityHubClient = mock(IdentityHubClient.class);
-        app.verifiableCredentialsJwtService = new VerifiableCredentialsJwtServiceImpl(new ObjectMapper());
+        app.verifiableCredentialsJwtService = new VerifiableCredentialsJwtServiceImpl(new ObjectMapper(), mock(Monitor.class));
         app.hubUrl = HUB_URL;
         cmd.setOut(new PrintWriter(out));
         cmd.setErr(new PrintWriter(err));

diff --git a/...ava/org/eclipse/dataspaceconnector/identityhub/verifier/CredentialsVerifierExtension.java b/...ava/org/eclipse/dataspaceconnector/identityhub/verifier/CredentialsVerifierExtension.java
@@ -59,13 +59,14 @@ public JwtCredentialsVerifier createJwtVerifier(ServiceExtensionContext context)
         if (didPublicKeyResolver == null) {
             didPublicKeyResolver = context.getService(DidPublicKeyResolver.class);
         }
+        Monitor monitor = context.getService(Monitor.class);
         return new DidJwtCredentialsVerifier(didPublicKeyResolver, monitor);
     }
 
     @Provider
     public CredentialsVerifier createCredentialsVerifier(ServiceExtensionContext context) {
         var client = new IdentityHubClientImpl(httpClient, typeManager.getMapper(), monitor);
-        var verifiableCredentialsJwtService = new VerifiableCredentialsJwtServiceImpl(typeManager.getMapper());
+        var verifiableCredentialsJwtService = new VerifiableCredentialsJwtServiceImpl(typeManager.getMapper(), monitor);
         return new IdentityHubCredentialsVerifier(client, monitor, jwtCredentialsVerifier, verifiableCredentialsJwtService);
     }
 }
diff --git a/...n/java/org/eclipse/dataspaceconnector/identityhub/verifier/DidJwtCredentialsVerifier.java b/...n/java/org/eclipse/dataspaceconnector/identityhub/verifier/DidJwtCredentialsVerifier.java
@@ -76,24 +76,28 @@ public boolean verifyClaims(SignedJWT jwt, String expectedSubject) {
         var requiredClaims = Set.of(ISSUER_CLAIM);
 
         var claimsVerifier = new DefaultJWTClaimsVerifier<>(exactMatchClaims, requiredClaims);
+
         try {
             claimsVerifier.verify(jwtClaimsSet);
         } catch (BadJWTException e) {
             monitor.warning("Failure verifying JWT token", e);
             return false;
         }
 
+        monitor.debug(() -> "JWT claims verification successful");
         return true;
     }
 
     private Result<Void> verifySignature(SignedJWT jwt, PublicKeyWrapper issuerPublicKey) {
         try {
             var verified = jwt.verify(issuerPublicKey.verifier());
             if (!verified) {
-                return Result.failure("Invalid signature");
+                return Result.failure("Invalid JWT signature");
             }
+            monitor.debug(() -> "JWT signature verification successful");
             return Result.success();
         } catch (JOSEException e) {
+            monitor.warning("Unable to verify JWT token", e);
             return Result.failure("Unable to verify JWT token. " + e.getMessage());
         }
     }

diff --git a/...a/org/eclipse/dataspaceconnector/identityhub/verifier/IdentityHubCredentialsVerifier.java b/...a/org/eclipse/dataspaceconnector/identityhub/verifier/IdentityHubCredentialsVerifier.java
@@ -14,14 +14,20 @@
 
 package org.eclipse.dataspaceconnector.identityhub.verifier;
 
+import com.nimbusds.jwt.SignedJWT;
 import org.eclipse.dataspaceconnector.iam.did.spi.credentials.CredentialsVerifier;
 import org.eclipse.dataspaceconnector.iam.did.spi.document.DidDocument;
+import org.eclipse.dataspaceconnector.iam.did.spi.document.Service;
 import org.eclipse.dataspaceconnector.identityhub.client.IdentityHubClient;
 import org.eclipse.dataspaceconnector.identityhub.credentials.VerifiableCredentialsJwtService;
 import org.eclipse.dataspaceconnector.spi.monitor.Monitor;
+import org.eclipse.dataspaceconnector.spi.response.StatusResult;
 import org.eclipse.dataspaceconnector.spi.result.AbstractResult;
 import org.eclipse.dataspaceconnector.spi.result.Result;
+import org.jetbrains.annotations.NotNull;
 
+import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 
@@ -61,41 +67,75 @@ public IdentityHubCredentialsVerifier(IdentityHubClient identityHubClient, Monit
      */
     @Override
     public Result<Map<String, Object>> getVerifiedCredentials(DidDocument didDocument) {
+        monitor.debug(() -> "Retrieving verified credentials for " + didDocument.getId());
+
         var hubBaseUrl = getIdentityHubBaseUrl(didDocument);
-        if (hubBaseUrl.failed()) {
-            return Result.failure(hubBaseUrl.getFailureMessages());
+        if (hubBaseUrl == null) {
+            var errorMessage = "Could not retrieve identity hub URL from DID document";
+            monitor.severe(errorMessage);
+            return Result.failure(errorMessage);
         }
 
-        var jwts = identityHubClient.getVerifiableCredentials(hubBaseUrl.getContent());
-        if (jwts.failed()) {
-            return Result.failure(jwts.getFailureMessages());
+        monitor.debug(() -> String.format("Using identity hub URL: %s", hubBaseUrl));
+
+        var verifiableCredentials = identityHubClient.getVerifiableCredentials(hubBaseUrl);
+        if (verifiableCredentials.failed()) {
+            monitor.severe("Could not retrieve verifiable credentials from identity hub");
+            return Result.failure(verifiableCredentials.getFailureMessages());
         }
-        var verifiedJwt = jwts.getContent()
+
+        monitor.debug(() -> String.format("Retrieved %s verifiable credentials from identity hub", verifiableCredentials.getContent().size()));
+
+        var verifiedCredentials = verifyCredentials(verifiableCredentials, didDocument);
+
+        monitor.debug(() -> String.format("Verified %s credentials", verifiedCredentials.size()));
+
+        var claims = extractClaimsFromCredential(verifiedCredentials);
+
+        return Result.success(claims);
+    }
+
+    @NotNull
+    private List<SignedJWT> verifyCredentials(StatusResult<Collection<SignedJWT>> jwts, DidDocument didDocument) {
+        var result = jwts.getContent()
                 .stream()
-                .filter(jwt -> jwtCredentialsVerifier.verifyClaims(jwt, didDocument.getId()))
-                .filter(jwtCredentialsVerifier::isSignedByIssuer);
+                .collect(partitioningBy((jwt) -> jwtCredentialsVerifier.verifyClaims(jwt, didDocument.getId()) && jwtCredentialsVerifier.isSignedByIssuer(jwt)));
 
-        var partitionedResult = verifiedJwt.map(verifiableCredentialsJwtService::extractCredential).collect(partitioningBy(AbstractResult::succeeded));
-        var successfulResults = partitionedResult.get(true);
-        var failedResults = partitionedResult.get(false);
+        var successfulResults = result.get(true);
+        var failedResults = result.get(false);
 
-        failedResults.forEach(result -> monitor.warning(String.join(",", result.getFailureMessages())));
+        if (!failedResults.isEmpty()) {
+            monitor.warning(String.format("Ignoring %s invalid verifiable credentials", failedResults.size()));
+        }
+
+        return successfulResults;
+    }
+
+    @NotNull
+    private Map<String, Object> extractClaimsFromCredential(List<SignedJWT> verifiedCredentials) {
+        var result = verifiedCredentials.stream()
+                .map(verifiableCredentialsJwtService::extractCredential)
+                .collect(partitioningBy(AbstractResult::succeeded));
+
+        var successfulResults = result.get(true);
+        var failedResults = result.get(false);
 
-        var claims = successfulResults.stream()
+        if (!failedResults.isEmpty()) {
+            failedResults.forEach(f -> monitor.warning("Invalid credentials: " + f.getFailureDetail()));
+        }
+
+        return successfulResults.stream()
                 .map(AbstractResult::getContent)
                 .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
-
-        return Result.success(claims);
     }
 
-    private Result<String> getIdentityHubBaseUrl(DidDocument didDocument) {
-        var hubBaseUrl = didDocument
+    private String getIdentityHubBaseUrl(DidDocument didDocument) {
+        return didDocument
                 .getService()
                 .stream()
                 .filter(s -> s.getType().equals(IDENTITY_HUB_SERVICE_TYPE))
-                .findFirst();
-
-        return hubBaseUrl.map(u -> Result.success(u.getServiceEndpoint()))
-                .orElse(Result.failure("Failed getting Identity Hub URL"));
+                .findFirst()
+                .map(Service::getServiceEndpoint)
+                .orElse(null);
     }
 }
diff --git a/...org/eclipse/dataspaceconnector/identityhub/verifier/CredentialsVerifierExtensionTest.java b/...org/eclipse/dataspaceconnector/identityhub/verifier/CredentialsVerifierExtensionTest.java
@@ -27,7 +27,6 @@
 import org.eclipse.dataspaceconnector.identityhub.client.IdentityHubClientImpl;
 import org.eclipse.dataspaceconnector.junit.extensions.EdcExtension;
 import org.eclipse.dataspaceconnector.junit.testfixtures.TestUtils;
-import org.eclipse.dataspaceconnector.spi.monitor.ConsoleMonitor;
 import org.eclipse.dataspaceconnector.spi.monitor.Monitor;
 import org.eclipse.dataspaceconnector.spi.result.Result;
 import org.junit.jupiter.api.BeforeEach;
@@ -52,13 +51,13 @@ public class CredentialsVerifierExtensionTest {
     private static final Faker FAKER = new Faker();
     private static final int PORT = getFreePort();
     private static final String API_URL = String.format("http://localhost:%d/api/identity-hub", PORT);
-    private static final String CREDENTIAL_ISSUER = "did:web:testdid";
-    private static final String SUBJECT = FAKER.internet().url();
+    private static final String CREDENTIAL_ISSUER = "did:web:" + FAKER.internet().domainName();
+    private static final String SUBJECT = "did:web:" + FAKER.internet().domainName();
     private IdentityHubClient identityHubClient;
 
     @BeforeEach
     void setUp(EdcExtension extension) {
-        identityHubClient = new IdentityHubClientImpl(TestUtils.testOkHttpClient(), new ObjectMapper(), new ConsoleMonitor());
+        identityHubClient = new IdentityHubClientImpl(TestUtils.testOkHttpClient(), new ObjectMapper(), mock(Monitor.class));
         extension.registerServiceMock(Monitor.class, mock(Monitor.class));
         extension.setConfiguration(Map.of("web.http.port", String.valueOf(PORT)));
     }

diff --git a/...g/eclipse/dataspaceconnector/identityhub/verifier/IdentityHubCredentialsVerifierTest.java b/...g/eclipse/dataspaceconnector/identityhub/verifier/IdentityHubCredentialsVerifierTest.java
@@ -58,7 +58,7 @@ public class IdentityHubCredentialsVerifierTest {
     private Monitor monitorMock = mock(Monitor.class);
     private IdentityHubClient identityHubClientMock = mock(IdentityHubClient.class);
     private JwtCredentialsVerifier jwtCredentialsVerifierMock = mock(JwtCredentialsVerifier.class);
-    private VerifiableCredentialsJwtServiceImpl verifiableCredentialsJwtService = new VerifiableCredentialsJwtServiceImpl(OBJECT_MAPPER);
+    private VerifiableCredentialsJwtServiceImpl verifiableCredentialsJwtService = new VerifiableCredentialsJwtServiceImpl(OBJECT_MAPPER, monitorMock);
     private CredentialsVerifier credentialsVerifier = new IdentityHubCredentialsVerifier(identityHubClientMock, monitorMock, jwtCredentialsVerifierMock, verifiableCredentialsJwtService);
 
     @Test
@@ -111,7 +111,7 @@ public void getVerifiedClaims_hubUrlNotResolved() {
 
         // Assert
         assertThat(credentials.failed()).isTrue();
-        assertThat(credentials.getFailureMessages()).containsExactly("Failed getting Identity Hub URL");
+        assertThat(credentials.getFailureMessages()).containsExactly("Could not retrieve identity hub URL from DID document");
     }
 
     @Test
@@ -165,4 +165,5 @@ public void getVerifiedClaims_verifiableCredentialsWithMissingId() {
         assertThat(credentials.getContent().isEmpty());
         verify(monitorMock, times(1)).warning(anyString());
     }
+
 }
diff --git a/spi/identity-hub-spi/build.gradle.kts b/spi/identity-hub-spi/build.gradle.kts
@@ -26,6 +26,7 @@ val edcGroup: String by project
 val edcVersion: String by project
 val jupiterVersion: String by project
 val assertj: String by project
+val mockitoVersion: String by project
 
 dependencies {
     api("org.jetbrains:annotations:${jetBrainsAnnotationsVersion}")
@@ -40,6 +41,7 @@ dependencies {
     testFixturesImplementation("${edcGroup}:identity-did-crypto:${edcVersion}")
     testImplementation("org.junit.jupiter:junit-jupiter-api:${jupiterVersion}")
     testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:${jupiterVersion}")
+    testImplementation("org.mockito:mockito-core:${mockitoVersion}")
     testImplementation("org.assertj:assertj-core:${assertj}")
     testImplementation("com.github.javafaker:javafaker:${faker}")
 }

diff --git a/...lipse/dataspaceconnector/identityhub/credentials/VerifiableCredentialsJwtServiceImpl.java b/...lipse/dataspaceconnector/identityhub/credentials/VerifiableCredentialsJwtServiceImpl.java
@@ -22,6 +22,7 @@
 import com.nimbusds.jwt.SignedJWT;
 import org.eclipse.dataspaceconnector.iam.did.spi.key.PrivateKeyWrapper;
 import org.eclipse.dataspaceconnector.identityhub.credentials.model.VerifiableCredential;
+import org.eclipse.dataspaceconnector.spi.monitor.Monitor;
 import org.eclipse.dataspaceconnector.spi.result.Result;
 
 import java.text.ParseException;
@@ -32,9 +33,11 @@
 
 public class VerifiableCredentialsJwtServiceImpl implements VerifiableCredentialsJwtService {
     private ObjectMapper objectMapper;
+    private Monitor monitor;
 
-    public VerifiableCredentialsJwtServiceImpl(ObjectMapper objectMapper) {
+    public VerifiableCredentialsJwtServiceImpl(ObjectMapper objectMapper, Monitor monitor) {
         this.objectMapper = objectMapper;
+        this.monitor = monitor;
     }
 
     @Override
@@ -57,16 +60,19 @@ public SignedJWT buildSignedJwt(VerifiableCredential credential, String issuer,
     @Override
     public Result<Map.Entry<String, Object>> extractCredential(SignedJWT jwt) {
         try {
-            var payload = jwt.getPayload().toJSONObject();
+            var payload = jwt.getJWTClaimsSet().getClaims();
             var vcObject = payload.get(VERIFIABLE_CREDENTIALS_KEY);
             if (vcObject == null) {
                 return Result.failure(String.format("No %s field found", VERIFIABLE_CREDENTIALS_KEY));
             }
             var verifiableCredential = objectMapper.convertValue(vcObject, VerifiableCredential.class);
 
+            monitor.debug(() -> "Extracted credentials from JWT");
+
             return Result.success(new AbstractMap.SimpleEntry<>(verifiableCredential.getId(), payload));
-        } catch (RuntimeException e) {
-            return Result.failure(Objects.requireNonNullElseGet(e.getMessage(), () -> e.toString()));
+        } catch (ParseException | RuntimeException e) {
+            monitor.severe("Failure extracting credentials from JWT", e);
+            return Result.failure(Objects.requireNonNullElseGet(e.getMessage(), e::toString));
         }
     }
 }
diff --git a/...lipse/dataspaceconnector/identityhub/credentials/VerifiableCredentialsJwtServiceTest.java b/...lipse/dataspaceconnector/identityhub/credentials/VerifiableCredentialsJwtServiceTest.java
@@ -23,6 +23,7 @@
 import org.eclipse.dataspaceconnector.iam.did.crypto.key.EcPrivateKeyWrapper;
 import org.eclipse.dataspaceconnector.iam.did.crypto.key.EcPublicKeyWrapper;
 import org.eclipse.dataspaceconnector.identityhub.credentials.model.VerifiableCredential;
+import org.eclipse.dataspaceconnector.spi.monitor.Monitor;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -34,6 +35,7 @@
 import static org.eclipse.dataspaceconnector.identityhub.credentials.VerifiableCredentialsJwtService.VERIFIABLE_CREDENTIALS_KEY;
 import static org.eclipse.dataspaceconnector.identityhub.junit.testfixtures.VerifiableCredentialTestUtil.generateEcKey;
 import static org.eclipse.dataspaceconnector.identityhub.junit.testfixtures.VerifiableCredentialTestUtil.generateVerifiableCredential;
+import static org.mockito.Mockito.mock;
 
 public class VerifiableCredentialsJwtServiceTest {
 
@@ -50,7 +52,7 @@ public void setUp() {
         var key = generateEcKey();
         privateKey = new EcPrivateKeyWrapper(key);
         publicKey = new EcPublicKeyWrapper(key);
-        service = new VerifiableCredentialsJwtServiceImpl(OBJECT_MAPPER);
+        service = new VerifiableCredentialsJwtServiceImpl(OBJECT_MAPPER, mock(Monitor.class));
     }
 
     @Test
@@ -67,7 +69,7 @@ public void buildSignedJwt_success() throws Exception {
         boolean result = signedJwt.verify(publicKey.verifier());
         assertThat(result).isTrue();
 
-        assertThat(signedJwt.getJWTClaimsSet().toJSONObject())
+        assertThat(signedJwt.getJWTClaimsSet().getClaims())
                 .containsEntry("iss", issuer)
                 .containsEntry("sub", subject)
                 .extractingByKey(VERIFIABLE_CREDENTIALS_KEY)