feat: IdentityHubCredentialsVerifier Fails if one VC signature verifi…

…cation fails (#24)

* IdentityHubCredentialsVerifier fails if credential is invalid (#49)

* IdentityHubCredentialsVerifier: Fails if one VC signature verification fails

IdentityHubCredentialsVerifier: Fails if one VC signature verification fails

* Applied comments. Added Override.

* Removed mergeFailureMessages method.

extensions/identity-hub-verifier/src/main/java/org/eclipse/dataspaceconnector/identityhub/verifier/AggregatedResult.java

@@ -0,0 +1,35 @@
+/*
+ *  Copyright (c) 2022 Microsoft Corporation
+ *
+ *  This program and the accompanying materials are made available under the
+ *  terms of the Apache License, Version 2.0 which is available at
+ *  https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  SPDX-License-Identifier: Apache-2.0
+ *
+ *  Contributors:
+ *       Microsoft Corporation - initial API and implementation
+ *
+ */
+
+package org.eclipse.dataspaceconnector.identityhub.verifier;
+
+import org.eclipse.dataspaceconnector.spi.result.AbstractResult;
+import org.eclipse.dataspaceconnector.spi.result.Failure;
+
+import java.util.List;
+
+/**
+ * A generic result type containing the result of processing plus the failures happening during the processing of the
+ * result.
+ * For example, when processing a List of objects, the processing can be successful for some objects, and unsuccessful
+ * for others.
+ * Using this class as a return type would let the client decide how to act about the failures.
+ *
+ * @param <T> Result type
+ */
+class AggregatedResult<T> extends AbstractResult<T, Failure> {
+    AggregatedResult(T successfulResult, List<String> failureMessage) {
+        super(successfulResult, failureMessage.isEmpty() ? null : new Failure(failureMessage));
+    }
+}

extensions/identity-hub-verifier/src/main/java/org/eclipse/dataspaceconnector/identityhub/verifier/DidJwtCredentialsVerifier.java

@@ -43,30 +43,34 @@ class DidJwtCredentialsVerifier implements JwtCredentialsVerifier {
         this.monitor = monitor;
     }
 
-    public boolean isSignedByIssuer(SignedJWT jwt) {
+    @Override
+    public Result<Void> isSignedByIssuer(SignedJWT jwt) {
         String issuer;
         try {
             issuer = jwt.getJWTClaimsSet().getIssuer();
         } catch (ParseException e) {
-            monitor.warning("Error parsing issuer from JWT", e);
-            return false;
+            var failureMessage = "Error parsing issuer from JWT";
+            monitor.warning(failureMessage, e);
+            return Result.failure(String.format("%s: %s", failureMessage, e.getMessage()));
         }
         var issuerPublicKey = didPublicKeyResolver.resolvePublicKey(issuer);
         if (issuerPublicKey.failed()) {
-            monitor.warning(String.format("Failed finding publicKey of issuer: %s", issuer));
-            return false;
+            var failureMessage = String.format("Failed finding publicKey of issuer: %s", issuer);
+            monitor.warning(failureMessage);
+            return Result.failure(failureMessage);
         }
-        Result<Void> signatureResult = verifySignature(jwt, issuerPublicKey.getContent());
-        return signatureResult.succeeded();
+        return verifySignature(jwt, issuerPublicKey.getContent());
     }
 
-    public boolean verifyClaims(SignedJWT jwt, String expectedSubject) {
+    @Override
+    public Result<Void> verifyClaims(SignedJWT jwt, String expectedSubject) {
         JWTClaimsSet jwtClaimsSet;
         try {
             jwtClaimsSet = jwt.getJWTClaimsSet();
         } catch (ParseException e) {
-            monitor.warning("Error parsing issuer from JWT", e);
-            return false;
+            var failureMessage = "Error parsing issuer from JWT";
+            monitor.warning(failureMessage, e);
+            return Result.failure(String.format("%s: %s", failureMessage, e.getMessage()));
         }
 
         // verify claims
@@ -80,12 +84,13 @@ public boolean verifyClaims(SignedJWT jwt, String expectedSubject) {
         try {
             claimsVerifier.verify(jwtClaimsSet);
         } catch (BadJWTException e) {
-            monitor.warning("Failure verifying JWT token", e);
-            return false;
+            var failureMessage = "Failure verifying JWT token";
+            monitor.warning(failureMessage, e);
+            return Result.failure(String.format("%s: %s", failureMessage, e.getMessage()));
         }
 
         monitor.debug(() -> "JWT claims verification successful");
-        return true;
+        return Result.success();
     }
 
     private Result<Void> verifySignature(SignedJWT jwt, PublicKeyWrapper issuerPublicKey) {
@@ -97,8 +102,9 @@ private Result<Void> verifySignature(SignedJWT jwt, PublicKeyWrapper issuerPubli
             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());
+            var failureMessage = "Unable to verify JWT token";
+            monitor.warning(failureMessage, e);
+            return Result.failure(String.format("%s: %s", failureMessage, e.getMessage()));
         }
     }
 }

extensions/identity-hub-verifier/src/main/java/org/eclipse/dataspaceconnector/identityhub/verifier/IdentityHubCredentialsVerifier.java

@@ -30,6 +30,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static java.util.stream.Collectors.partitioningBy;
 
@@ -103,45 +104,93 @@ public Result<Map<String, Object>> getVerifiedCredentials(DidDocument didDocumen
 
         var verifiedCredentials = verifyCredentials(verifiableCredentials, didDocument);
 
-        monitor.debug(() -> String.format("Verified %s credentials", verifiedCredentials.size()));
+        monitor.debug(() -> String.format("Verified %s credentials", verifiedCredentials.getContent().size()));
 
-        var claims = extractClaimsFromCredential(verifiedCredentials);
+        var claims = extractClaimsFromCredential(verifiedCredentials.getContent());
 
-        return Result.success(claims);
+        var failureMessages = Stream
+                .concat(verifiedCredentials.getFailureMessages().stream(), claims.getFailureMessages().stream())
+                .collect(Collectors.toList());
+
+        var result = new AggregatedResult<>(claims.getContent(), failureMessages);
+
+        // Fail if one verifiable credential is not valid. This is a temporary solution until the CredentialsVerifier
+        // contract is changed to support a result containing both successful results and failures.
+        if (result.failed()) {
+            monitor.severe(() -> String.format("Credentials verification failed: %s", claims.getFailureDetail()));
+            return Result.failure(result.getFailureDetail());
+        } else {
+            return Result.success(result.getContent());
+        }
     }
 
     @NotNull
-    private List<SignedJWT> verifyCredentials(StatusResult<Collection<SignedJWT>> jwts, DidDocument didDocument) {
-        var result = jwts.getContent()
+    private AggregatedResult<List<SignedJWT>> verifyCredentials(StatusResult<Collection<SignedJWT>> jwts, DidDocument didDocument) {
+        // Get valid credentials.
+        var verifiedJwts = jwts.getContent()
+                .stream()
+                .map(jwt -> verifyJwtClaims(jwt, didDocument))
+                .collect(partitioningBy(AbstractResult::succeeded));
+
+        var jwtsSignedByIssuer = verifiedJwts.get(true)
+                .stream()
+                .map(jwt -> verifySignature(jwt.getContent()))
+                .collect(partitioningBy(AbstractResult::succeeded));
+
+        var validCredentials = jwtsSignedByIssuer
+                .get(true)
+                .stream()
+                .map(AbstractResult::getContent)
+                .collect(Collectors.toList());
+
+        // Gather failure messages of invalid credentials.
+        var verificationFailures = verifiedJwts
+                .get(false)
+                .stream()
+                .map(AbstractResult::getFailureDetail);
+
+        var signatureVerificationFailures = jwtsSignedByIssuer
+                .get(false)
                 .stream()
-                .collect(partitioningBy((jwt) -> jwtCredentialsVerifier.verifyClaims(jwt, didDocument.getId()) && jwtCredentialsVerifier.isSignedByIssuer(jwt)));
+                .map(AbstractResult::getFailureDetail);
 
-        var successfulResults = result.get(true);
-        var failedResults = result.get(false);
+        var failedResults = Stream.concat(verificationFailures, signatureVerificationFailures)
+                .collect(Collectors.toList());
 
         if (!failedResults.isEmpty()) {
-            monitor.warning(String.format("Ignoring %s invalid verifiable credentials", failedResults.size()));
+            monitor.warning(String.format("Found %s invalid verifiable credentials", failedResults.size()));
         }
 
-        return successfulResults;
+        return new AggregatedResult<>(validCredentials, failedResults);
+    }
+
+    @NotNull
+    private Result<SignedJWT> verifyJwtClaims(SignedJWT jwt, DidDocument didDocument) {
+        var result = jwtCredentialsVerifier.verifyClaims(jwt, didDocument.getId());
+        return result.succeeded() ? Result.success(jwt) : Result.failure(result.getFailureMessages());
+    }
+
+    @NotNull
+    private Result<SignedJWT> verifySignature(SignedJWT jwt) {
+        var result = jwtCredentialsVerifier.isSignedByIssuer(jwt);
+        return result.succeeded() ? Result.success(jwt) : Result.failure(result.getFailureMessages());
     }
 
     @NotNull
-    private Map<String, Object> extractClaimsFromCredential(List<SignedJWT> verifiedCredentials) {
+    private AggregatedResult<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);
-
-        if (!failedResults.isEmpty()) {
-            failedResults.forEach(f -> monitor.warning("Invalid credentials: " + f.getFailureDetail()));
-        }
-
-        return successfulResults.stream()
+        var successfulResults = result.get(true).stream()
                 .map(AbstractResult::getContent)
                 .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
+        var failedResults = result.get(false).stream()
+                .map(AbstractResult::getFailureDetail)
+                .collect(Collectors.toList());
+
+        return new AggregatedResult<>(successfulResults, failedResults);
     }
 
     private String getIdentityHubBaseUrl(DidDocument didDocument) {

extensions/identity-hub-verifier/src/main/java/org/eclipse/dataspaceconnector/identityhub/verifier/JwtCredentialsVerifier.java

@@ -15,6 +15,7 @@
 package org.eclipse.dataspaceconnector.identityhub.verifier;
 
 import com.nimbusds.jwt.SignedJWT;
+import org.eclipse.dataspaceconnector.spi.result.Result;
 
 /**
  * Verifies verifiable credentials in JWT format.
@@ -27,7 +28,7 @@ public interface JwtCredentialsVerifier {
      * @param jwt to be verified.
      * @return if the JWT is signed by the claimed issuer.
      */
-    boolean isSignedByIssuer(SignedJWT jwt);
+    Result<Void> isSignedByIssuer(SignedJWT jwt);
 
     /**
      * Verifies if a JWT targets the given subject, and checks for the presence of the issuer ("iss") claim. The expiration ("exp") and not-before ("nbf") claims are verified if present as well.
@@ -36,5 +37,5 @@ public interface JwtCredentialsVerifier {
      * @param expectedSubject subject claim to verify.
      * @return if the JWT is valid and for the given subject
      */
-    boolean verifyClaims(SignedJWT jwt, String expectedSubject);
+    Result<Void> verifyClaims(SignedJWT jwt, String expectedSubject);
 }